[ovs-dev] netdev-tc-offloads: Use correct hook qdisc at init tc flow
diff mbox series

Message ID 1560167920-15493-1-git-send-email-roid@mellanox.com
State New
Headers show
Series
  • [ovs-dev] netdev-tc-offloads: Use correct hook qdisc at init tc flow
Related show

Commit Message

Roi Dayan June 10, 2019, 11:58 a.m. UTC
From: Raed Salem <raeds@mellanox.com>

A preliminary netdev qdisc cleanup is done during init tc flow.
The cited commit allows for creating of egress hook qdiscs on internal
ports. This breaks the netdev qdisc cleanup as currently only ingress
hook qdiscs type is deleted. As a consequence the check for tc ingress
shared block support fails when the check is done on internal port.

Issue can be reproduced by the following steps:
- start openvswitch service
- create ovs bridge
- restart openvswitch service

Fix by using the correct hook qdisc type at netdev hook qdisc cleanup.

Fixes 608ff46aaf0d ("ovs-tc: offload datapath rules matching on internal ports")
Signed-off-by: Raed Salem <raeds@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
---
 lib/netdev-tc-offloads.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

John Hurley June 10, 2019, 3:03 p.m. UTC | #1
On Mon, Jun 10, 2019 at 12:58 PM Roi Dayan <roid@mellanox.com> wrote:
>
> From: Raed Salem <raeds@mellanox.com>
>
> A preliminary netdev qdisc cleanup is done during init tc flow.
> The cited commit allows for creating of egress hook qdiscs on internal
> ports. This breaks the netdev qdisc cleanup as currently only ingress
> hook qdiscs type is deleted. As a consequence the check for tc ingress
> shared block support fails when the check is done on internal port.
>
> Issue can be reproduced by the following steps:
> - start openvswitch service
> - create ovs bridge
> - restart openvswitch service
>
> Fix by using the correct hook qdisc type at netdev hook qdisc cleanup.
>

Nice catch here. Thanks.

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


> Fixes 608ff46aaf0d ("ovs-tc: offload datapath rules matching on internal ports")
> Signed-off-by: Raed Salem <raeds@mellanox.com>
> Reviewed-by: Roi Dayan <roid@mellanox.com>
> ---
>  lib/netdev-tc-offloads.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
> index ef9ee0786215..97fbe03ed11d 100644
> --- a/lib/netdev-tc-offloads.c
> +++ b/lib/netdev-tc-offloads.c
> @@ -1571,8 +1571,8 @@ netdev_tc_init_flow_api(struct netdev *netdev)
>          return -ifindex;
>      }
>
> -    /* make sure there is no ingress qdisc */
> -    tc_add_del_qdisc(ifindex, false, 0, TC_INGRESS);
> +    /* make sure there is no ingress/egress qdisc */
> +    tc_add_del_qdisc(ifindex, false, 0, hook);
>
>      if (ovsthread_once_start(&block_once)) {
>          probe_tc_block_support(ifindex);
> --
> 2.7.5
>
Simon Horman June 12, 2019, 3:07 p.m. UTC | #2
On Mon, Jun 10, 2019 at 04:03:59PM +0100, John Hurley wrote:
> On Mon, Jun 10, 2019 at 12:58 PM Roi Dayan <roid@mellanox.com> wrote:
> >
> > From: Raed Salem <raeds@mellanox.com>
> >
> > A preliminary netdev qdisc cleanup is done during init tc flow.
> > The cited commit allows for creating of egress hook qdiscs on internal
> > ports. This breaks the netdev qdisc cleanup as currently only ingress
> > hook qdiscs type is deleted. As a consequence the check for tc ingress
> > shared block support fails when the check is done on internal port.
> >
> > Issue can be reproduced by the following steps:
> > - start openvswitch service
> > - create ovs bridge
> > - restart openvswitch service
> >
> > Fix by using the correct hook qdisc type at netdev hook qdisc cleanup.
> >
> 
> Nice catch here. Thanks.
> 
> Acked-by: John Hurley <john.hurley@netronome.com>

Thanks, applied to master branch.

Patch
diff mbox series

diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
index ef9ee0786215..97fbe03ed11d 100644
--- a/lib/netdev-tc-offloads.c
+++ b/lib/netdev-tc-offloads.c
@@ -1571,8 +1571,8 @@  netdev_tc_init_flow_api(struct netdev *netdev)
         return -ifindex;
     }
 
-    /* make sure there is no ingress qdisc */
-    tc_add_del_qdisc(ifindex, false, 0, TC_INGRESS);
+    /* make sure there is no ingress/egress qdisc */
+    tc_add_del_qdisc(ifindex, false, 0, hook);
 
     if (ovsthread_once_start(&block_once)) {
         probe_tc_block_support(ifindex);