Thu, Jan 25, 2018 at 01:17:46AM CET, jakub.kicin...@netronome.com wrote: >Very few (mlxsw) upstream drivers seem to allow offload of chains >other than 0. Save driver developers typing and add a helper for >checking both if ethtool's TC offload flag is on and if chain is 0. >This helper will set the extack appropriately in both error cases. > >Signed-off-by: Jakub Kicinski <jakub.kicin...@netronome.com> >Reviewed-by: Simon Horman <simon.hor...@netronome.com> >--- > drivers/net/ethernet/netronome/nfp/bpf/main.c | 4 +--- > drivers/net/netdevsim/bpf.c | 5 +---- > include/net/pkt_cls.h | 12 ++++++++++++ > 3 files changed, 14 insertions(+), 7 deletions(-) > >diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.c >b/drivers/net/ethernet/netronome/nfp/bpf/main.c >index b3206855535a..322027792fe8 100644 >--- a/drivers/net/ethernet/netronome/nfp/bpf/main.c >+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.c >@@ -130,7 +130,7 @@ static int nfp_bpf_setup_tc_block_cb(enum tc_setup_type >type, > "only offload of BPF classifiers supported"); > return -EOPNOTSUPP; > } >- if (!tc_can_offload_extack(nn->dp.netdev, cls_bpf->common.extack)) >+ if (!tc_cls_can_offload_and_chain0(nn->dp.netdev, &cls_bpf->common)) > return -EOPNOTSUPP; > if (!nfp_net_ebpf_capable(nn)) { > NL_SET_ERR_MSG_MOD(cls_bpf->common.extack, >@@ -142,8 +142,6 @@ static int nfp_bpf_setup_tc_block_cb(enum tc_setup_type >type, > "only ETH_P_ALL supported as filter > protocol"); > return -EOPNOTSUPP; > } >- if (cls_bpf->common.chain_index) >- return -EOPNOTSUPP; > > /* Only support TC direct action */ > if (!cls_bpf->exts_integrated || >diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c >index 8166f121bbcc..de73c1ff0939 100644 >--- a/drivers/net/netdevsim/bpf.c >+++ b/drivers/net/netdevsim/bpf.c >@@ -135,7 +135,7 @@ int nsim_bpf_setup_tc_block_cb(enum tc_setup_type type, > return -EOPNOTSUPP; > } > >- if (!tc_can_offload_extack(ns->netdev, cls_bpf->common.extack)) >+ if (!tc_cls_can_offload_and_chain0(ns->netdev, &cls_bpf->common)) > return -EOPNOTSUPP; > > if (cls_bpf->common.protocol != htons(ETH_P_ALL)) { >@@ -144,9 +144,6 @@ int nsim_bpf_setup_tc_block_cb(enum tc_setup_type type, > return -EOPNOTSUPP; > } > >- if (cls_bpf->common.chain_index) >- return -EOPNOTSUPP; >-
For nfp and netdevsim you do this in a patch that introduces the helper, for the rest you have a separate patch? Why this inconsistency? Again, from my perspective, this can be done in a single patch for all drivers. All the same hunks. > if (!ns->bpf_tc_accept) { > NSIM_EA(cls_bpf->common.extack, > "netdevsim configured to reject BPF TC offload"); >diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h >index 1a41513cec7f..4db08d7dd22c 100644 >--- a/include/net/pkt_cls.h >+++ b/include/net/pkt_cls.h >@@ -656,6 +656,18 @@ static inline bool tc_can_offload_extack(const struct >net_device *dev, > return can; > } > >+static inline bool >+tc_cls_can_offload_and_chain0(const struct net_device *dev, >+ struct tc_cls_common_offload *common) >+{ >+ if (common->chain_index) { >+ NL_SET_ERR_MSG(common->extack, >+ "Driver supports only offload of chain 0"); No need for a line-wrap. >+ return false; >+ } >+ return tc_can_offload_extack(dev, common->extack); >+} >+ > static inline bool tc_skip_hw(u32 flags) > { > return (flags & TCA_CLS_FLAGS_SKIP_HW) ? true : false; >-- >2.15.1 >