[ovs-dev] netdev-tc-offloads: Fix probe tc block support

From: Raed Salem <raeds@mellanox.com>

Current implementation will try to create an qdisk of type ingress with
block id 1 to check for kernel ingress block support, this check is
insufficient as old kernels without ingress block support will
successfully create an ingress qdisc, ignoring the ingress block.

Fix by trying to add a test rule on the ingress block.

Fixes 093c9458fb02 ("tc: allow offloading of block ids")
Signed-off-by: Raed Salem <raeds@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
---
 lib/netdev-tc-offloads.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

On Mon, Apr 8, 2019 at 1:43 PM Roi Dayan <roid@mellanox.com> wrote:
>
> From: Raed Salem <raeds@mellanox.com>
>
> Current implementation will try to create an qdisk of type ingress with
> block id 1 to check for kernel ingress block support, this check is
> insufficient as old kernels without ingress block support will
> successfully create an ingress qdisc, ignoring the ingress block.
>
> Fix by trying to add a test rule on the ingress block.
>

Good catch.
Thanks for this.

Acked-by: John Hurley <john.hurley@netronome.com>

> Fixes 093c9458fb02 ("tc: allow offloading of block ids")
> Signed-off-by: Raed Salem <raeds@mellanox.com>
> Reviewed-by: Roi Dayan <roid@mellanox.com>
> ---
>  lib/netdev-tc-offloads.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
> index f5555e418e0d..e57de3b47f3d 100644
> --- a/lib/netdev-tc-offloads.c
> +++ b/lib/netdev-tc-offloads.c
> @@ -1497,6 +1497,7 @@ out:
>  static void
>  probe_tc_block_support(int ifindex)
>  {
> +    struct tc_flower flower;
>      uint32_t block_id = 1;
>      int error;
>
> @@ -1505,10 +1506,21 @@ probe_tc_block_support(int ifindex)
>          return;
>      }
>
> +    memset(&flower, 0, sizeof flower);
> +
> +    flower.key.eth_type = htons(ETH_P_IP);
> +    flower.mask.eth_type = OVS_BE16_MAX;
> +    memset(&flower.key.dst_mac, 0x11, sizeof flower.key.dst_mac);
> +    memset(&flower.mask.dst_mac, 0xff, sizeof flower.mask.dst_mac);
> +
> +    error = tc_replace_flower(ifindex, 1, 1, &flower, block_id);
> +
>      tc_add_del_ingress_qdisc(ifindex, false, block_id);
>
> -    block_support = true;
> -    VLOG_INFO("probe tc: block offload is supported.");
> +    if (!error) {
> +        block_support = true;
> +        VLOG_INFO("probe tc: block offload is supported.");
> +    }
>  }
>
>  int
> --
> 2.7.5
>

On Mon, Apr 08, 2019 at 02:02:53PM +0100, John Hurley wrote:
> On Mon, Apr 8, 2019 at 1:43 PM Roi Dayan <roid@mellanox.com> wrote:
> >
> > From: Raed Salem <raeds@mellanox.com>
> >
> > Current implementation will try to create an qdisk of type ingress with
> > block id 1 to check for kernel ingress block support, this check is
> > insufficient as old kernels without ingress block support will
> > successfully create an ingress qdisc, ignoring the ingress block.
> >
> > Fix by trying to add a test rule on the ingress block.
> >
> 
> Good catch.
> Thanks for this.
> 
> Acked-by: John Hurley <john.hurley@netronome.com>

Thank Roi,

applied to master, branch-2.11 and branch-2.10.