| Message ID | 20200225121023.6011-1-jiri@resnulli.us |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next] iavf: use tc_cls_can_offload_basic() instead of chain check | expand |
From: Jiri Pirko <jiri@resnulli.us> Date: Tue, 25 Feb 2020 13:10:23 +0100 > From: Jiri Pirko <jiri@mellanox.com> > > Looks like the iavf code actually experienced a race condition, when a > developer took code before the check for chain 0 was put to helper. > So use tc_cls_can_offload_basic() helper instead of direct check and > move the check to _cb() so this is similar to i40e code. > > Signed-off-by: Jiri Pirko <jiri@mellanox.com> > --- > This was originally part of "net: allow user specify TC filter HW stats type" > patchset, but it is no longer related after the requested changes. > Sending separatelly. Jeff, do you want me to apply this directly? If so, please give your ack. Thanks.
On Tue, 2020-02-25 at 13:10 +0100, Jiri Pirko wrote: > From: Jiri Pirko <jiri@mellanox.com> > > Looks like the iavf code actually experienced a race condition, when > a > developer took code before the check for chain 0 was put to helper. > So use tc_cls_can_offload_basic() helper instead of direct check and > move the check to _cb() so this is similar to i40e code. > > Signed-off-by: Jiri Pirko <jiri@mellanox.com> Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com> Go ahead and pick this up Dave, thanks! > --- > This was originally part of "net: allow user specify TC filter HW > stats type" > patchset, but it is no longer related after the requested changes. > Sending separatelly. > --- > drivers/net/ethernet/intel/iavf/iavf_main.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-)
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com> Date: Tue, 25 Feb 2020 13:15:46 -0800 > On Tue, 2020-02-25 at 13:10 +0100, Jiri Pirko wrote: >> From: Jiri Pirko <jiri@mellanox.com> >> >> Looks like the iavf code actually experienced a race condition, when >> a >> developer took code before the check for chain 0 was put to helper. >> So use tc_cls_can_offload_basic() helper instead of direct check and >> move the check to _cb() so this is similar to i40e code. >> >> Signed-off-by: Jiri Pirko <jiri@mellanox.com> > > Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com> > > Go ahead and pick this up Dave, thanks! Hmmm, Jiri this doesn't compile? CC [M] drivers/net/ethernet/intel/iavf/iavf_main.o drivers/net/ethernet/intel/iavf/iavf_main.c: In function ‘iavf_setup_tc_block_cb’: drivers/net/ethernet/intel/iavf/iavf_main.c:3089:7: error: implicit declaration of function ‘tc_cls_can_offload_basic’; did you mean ‘tc_cls_common_offload_init’? [-Werror=implicit-function-declaration] if (!tc_cls_can_offload_basic(adapter->netdev, type_data)) ^~~~~~~~~~~~~~~~~~~~~~~~ tc_cls_common_offload_init Maybe it does depend upon something in the tc filter patch series?
Tue, Feb 25, 2020 at 10:20:41PM CET, davem@davemloft.net wrote: >From: Jeff Kirsher <jeffrey.t.kirsher@intel.com> >Date: Tue, 25 Feb 2020 13:15:46 -0800 > >> On Tue, 2020-02-25 at 13:10 +0100, Jiri Pirko wrote: >>> From: Jiri Pirko <jiri@mellanox.com> >>> >>> Looks like the iavf code actually experienced a race condition, when >>> a >>> developer took code before the check for chain 0 was put to helper. >>> So use tc_cls_can_offload_basic() helper instead of direct check and >>> move the check to _cb() so this is similar to i40e code. >>> >>> Signed-off-by: Jiri Pirko <jiri@mellanox.com> >> >> Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com> >> >> Go ahead and pick this up Dave, thanks! > >Hmmm, Jiri this doesn't compile? Crap :/ I shuffled with the patches too much. Will send v2. Sorry. > > CC [M] drivers/net/ethernet/intel/iavf/iavf_main.o >drivers/net/ethernet/intel/iavf/iavf_main.c: In function ‘iavf_setup_tc_block_cb’: >drivers/net/ethernet/intel/iavf/iavf_main.c:3089:7: error: implicit declaration of function ‘tc_cls_can_offload_basic’; did you mean ‘tc_cls_common_offload_init’? [-Werror=implicit-function-declaration] > if (!tc_cls_can_offload_basic(adapter->netdev, type_data)) > ^~~~~~~~~~~~~~~~~~~~~~~~ > tc_cls_common_offload_init > >Maybe it does depend upon something in the tc filter patch series?
diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c index 62fe56ddcb6e..8bc0d287d025 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_main.c +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c @@ -3061,9 +3061,6 @@ static int iavf_delete_clsflower(struct iavf_adapter *adapter, static int iavf_setup_tc_cls_flower(struct iavf_adapter *adapter, struct flow_cls_offload *cls_flower) { - if (cls_flower->common.chain_index) - return -EOPNOTSUPP; - switch (cls_flower->command) { case FLOW_CLS_REPLACE: return iavf_configure_clsflower(adapter, cls_flower); @@ -3087,6 +3084,11 @@ static int iavf_setup_tc_cls_flower(struct iavf_adapter *adapter, static int iavf_setup_tc_block_cb(enum tc_setup_type type, void *type_data, void *cb_priv) { + struct iavf_adapter *adapter = cb_priv; + + if (!tc_cls_can_offload_basic(adapter->netdev, type_data)) + return -EOPNOTSUPP; + switch (type) { case TC_SETUP_CLSFLOWER: return iavf_setup_tc_cls_flower(cb_priv, type_data);