[patch net-next] iavf: use tc_cls_can_offload_basic() instead of chain check

[patch net-next] iavf: use tc_cls_can_offload_basic() instead of chain check

[Jiri Pirko]

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.
---
 drivers/net/ethernet/intel/iavf/iavf_main.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

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);
-- 
2.21.1

Re: [patch net-next] iavf: use tc_cls_can_offload_basic() instead of chain check

[David Miller]

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.

Re: [patch net-next] iavf: use tc_cls_can_offload_basic() instead of chain check

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 839 bytes --]

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(-)


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [patch net-next] iavf: use tc_cls_can_offload_basic() instead of chain check

[David Miller]

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?

Re: [patch net-next] iavf: use tc_cls_can_offload_basic() instead of chain check

[Jiri Pirko]

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?