| Message ID | 1552314845-23758-1-git-send-email-roid@mellanox.com |
|---|---|
| State | Accepted |
| Delegated to: | Simon Horman |
| Headers | show |
| Series | [ovs-dev] netdev-tc-offloads: Remove ingress qdisc on tc init flow api | expand |
On Mon, Mar 11, 2019 at 2:34 PM Roi Dayan <roid@mellanox.com> wrote: > > It could be a port added to ovs bridge already has ingress qdisc > which will make the block probe fail. > The probes should start clean and ingress is being added later > so just remove ingress in case it exists. > > Signed-off-by: Roi Dayan <roid@mellanox.com> Acked-by: John Hurley <john.hurley@netronome.com> > --- > lib/netdev-tc-offloads.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c > index da43549f2f22..b33a79bb14a2 100644 > --- a/lib/netdev-tc-offloads.c > +++ b/lib/netdev-tc-offloads.c > @@ -1527,6 +1527,9 @@ netdev_tc_init_flow_api(struct netdev *netdev) > return -ifindex; > } > > + /* make sure there is no ingress qdisc */ > + tc_add_del_ingress_qdisc(ifindex, false, 0); > + > if (ovsthread_once_start(&block_once)) { > probe_tc_block_support(ifindex); > ovsthread_once_done(&block_once); > -- > 2.7.0 >
On Mon, Mar 11, 2019 at 03:25:09PM +0000, John Hurley wrote: > On Mon, Mar 11, 2019 at 2:34 PM Roi Dayan <roid@mellanox.com> wrote: > > > > It could be a port added to ovs bridge already has ingress qdisc > > which will make the block probe fail. > > The probes should start clean and ingress is being added later > > so just remove ingress in case it exists. > > > > Signed-off-by: Roi Dayan <roid@mellanox.com> > > Acked-by: John Hurley <john.hurley@netronome.com> Thanks, I have applied this to master and branch-2.11. It also appears to apply cleanly to branch-2.10, but currently travis-ci fails on that branch so I'm seeing if I can get to the bottom of that. If the patch is also appropriate for older branches please consider supplying a backport. > > > --- > > lib/netdev-tc-offloads.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c > > index da43549f2f22..b33a79bb14a2 100644 > > --- a/lib/netdev-tc-offloads.c > > +++ b/lib/netdev-tc-offloads.c > > @@ -1527,6 +1527,9 @@ netdev_tc_init_flow_api(struct netdev *netdev) > > return -ifindex; > > } > > > > + /* make sure there is no ingress qdisc */ > > + tc_add_del_ingress_qdisc(ifindex, false, 0); > > + > > if (ovsthread_once_start(&block_once)) { > > probe_tc_block_support(ifindex); > > ovsthread_once_done(&block_once); > > -- > > 2.7.0 > >
On 12/03/2019 17:05, Simon Horman wrote: > On Mon, Mar 11, 2019 at 03:25:09PM +0000, John Hurley wrote: >> On Mon, Mar 11, 2019 at 2:34 PM Roi Dayan <roid@mellanox.com> wrote: >>> >>> It could be a port added to ovs bridge already has ingress qdisc >>> which will make the block probe fail. >>> The probes should start clean and ingress is being added later >>> so just remove ingress in case it exists. >>> >>> Signed-off-by: Roi Dayan <roid@mellanox.com> >> >> Acked-by: John Hurley <john.hurley@netronome.com> > > Thanks, > > I have applied this to master and branch-2.11. > > It also appears to apply cleanly to branch-2.10, but currently travis-ci > fails on that branch so I'm seeing if I can get to the bottom of that. > > If the patch is also appropriate for older branches please consider > supplying a backport. > The patches only needed because we might need ingress or shared block. so we must first clean ingress in case it's available. The shared block patches are merged in v2.10 but not v2.9. In v2.9 we don't care if ingress already added to a port or not. so should be good there. Now thinking it, the issue might repeat itself if a port added to a bridge might had shared block (any number). Maybe we should clean also clean any ingress_block that might be on that port. or is it too much? >> >>> --- >>> lib/netdev-tc-offloads.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c >>> index da43549f2f22..b33a79bb14a2 100644 >>> --- a/lib/netdev-tc-offloads.c >>> +++ b/lib/netdev-tc-offloads.c >>> @@ -1527,6 +1527,9 @@ netdev_tc_init_flow_api(struct netdev *netdev) >>> return -ifindex; >>> } >>> >>> + /* make sure there is no ingress qdisc */ >>> + tc_add_del_ingress_qdisc(ifindex, false, 0); >>> + >>> if (ovsthread_once_start(&block_once)) { >>> probe_tc_block_support(ifindex); >>> ovsthread_once_done(&block_once); >>> -- >>> 2.7.0 >>>
On Tue, Mar 12, 2019 at 04:05:16PM +0100, Simon Horman wrote: > On Mon, Mar 11, 2019 at 03:25:09PM +0000, John Hurley wrote: > > On Mon, Mar 11, 2019 at 2:34 PM Roi Dayan <roid@mellanox.com> wrote: > > > > > > It could be a port added to ovs bridge already has ingress qdisc > > > which will make the block probe fail. > > > The probes should start clean and ingress is being added later > > > so just remove ingress in case it exists. > > > > > > Signed-off-by: Roi Dayan <roid@mellanox.com> > > > > Acked-by: John Hurley <john.hurley@netronome.com> > > Thanks, > > I have applied this to master and branch-2.11. > > It also appears to apply cleanly to branch-2.10, but currently travis-ci > fails on that branch so I'm seeing if I can get to the bottom of that. branch-2.10 passes travis-ci testing once again and I have gone ahead and applied this patch to that branch. > If the patch is also appropriate for older branches please consider > supplying a backport. Any comment on this? > > > > > > --- > > > lib/netdev-tc-offloads.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c > > > index da43549f2f22..b33a79bb14a2 100644 > > > --- a/lib/netdev-tc-offloads.c > > > +++ b/lib/netdev-tc-offloads.c > > > @@ -1527,6 +1527,9 @@ netdev_tc_init_flow_api(struct netdev *netdev) > > > return -ifindex; > > > } > > > > > > + /* make sure there is no ingress qdisc */ > > > + tc_add_del_ingress_qdisc(ifindex, false, 0); > > > + > > > if (ovsthread_once_start(&block_once)) { > > > probe_tc_block_support(ifindex); > > > ovsthread_once_done(&block_once); > > > -- > > > 2.7.0 > > >
On 14/03/2019 10:50, Simon Horman wrote: > On Tue, Mar 12, 2019 at 04:05:16PM +0100, Simon Horman wrote: >> On Mon, Mar 11, 2019 at 03:25:09PM +0000, John Hurley wrote: >>> On Mon, Mar 11, 2019 at 2:34 PM Roi Dayan <roid@mellanox.com> wrote: >>>> >>>> It could be a port added to ovs bridge already has ingress qdisc >>>> which will make the block probe fail. >>>> The probes should start clean and ingress is being added later >>>> so just remove ingress in case it exists. >>>> >>>> Signed-off-by: Roi Dayan <roid@mellanox.com> >>> >>> Acked-by: John Hurley <john.hurley@netronome.com> >> >> Thanks, >> >> I have applied this to master and branch-2.11. >> >> It also appears to apply cleanly to branch-2.10, but currently travis-ci >> fails on that branch so I'm seeing if I can get to the bottom of that. > > branch-2.10 passes travis-ci testing once again and > I have gone ahead and applied this patch to that branch. > >> If the patch is also appropriate for older branches please consider >> supplying a backport. > > Any comment on this? I replied on this. did u get my reply? I don't think older branches need this. > >> >>> >>>> --- >>>> lib/netdev-tc-offloads.c | 3 +++ >>>> 1 file changed, 3 insertions(+) >>>> >>>> diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c >>>> index da43549f2f22..b33a79bb14a2 100644 >>>> --- a/lib/netdev-tc-offloads.c >>>> +++ b/lib/netdev-tc-offloads.c >>>> @@ -1527,6 +1527,9 @@ netdev_tc_init_flow_api(struct netdev *netdev) >>>> return -ifindex; >>>> } >>>> >>>> + /* make sure there is no ingress qdisc */ >>>> + tc_add_del_ingress_qdisc(ifindex, false, 0); >>>> + >>>> if (ovsthread_once_start(&block_once)) { >>>> probe_tc_block_support(ifindex); >>>> ovsthread_once_done(&block_once); >>>> -- >>>> 2.7.0 >>>>
On Sun, Mar 17, 2019 at 05:58:47AM +0000, Roi Dayan wrote: > > > On 14/03/2019 10:50, Simon Horman wrote: > > On Tue, Mar 12, 2019 at 04:05:16PM +0100, Simon Horman wrote: > >> On Mon, Mar 11, 2019 at 03:25:09PM +0000, John Hurley wrote: > >>> On Mon, Mar 11, 2019 at 2:34 PM Roi Dayan <roid@mellanox.com> wrote: > >>>> > >>>> It could be a port added to ovs bridge already has ingress qdisc > >>>> which will make the block probe fail. > >>>> The probes should start clean and ingress is being added later > >>>> so just remove ingress in case it exists. > >>>> > >>>> Signed-off-by: Roi Dayan <roid@mellanox.com> > >>> > >>> Acked-by: John Hurley <john.hurley@netronome.com> > >> > >> Thanks, > >> > >> I have applied this to master and branch-2.11. > >> > >> It also appears to apply cleanly to branch-2.10, but currently travis-ci > >> fails on that branch so I'm seeing if I can get to the bottom of that. > > > > branch-2.10 passes travis-ci testing once again and > > I have gone ahead and applied this patch to that branch. > > > >> If the patch is also appropriate for older branches please consider > >> supplying a backport. > > > > Any comment on this? > > I replied on this. did u get my reply? > I don't think older branches need this. Thanks and sorry for not responding to your earlier reply. I don't think any further action is required on this patch. > >>>> --- > >>>> lib/netdev-tc-offloads.c | 3 +++ > >>>> 1 file changed, 3 insertions(+) > >>>> > >>>> diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c > >>>> index da43549f2f22..b33a79bb14a2 100644 > >>>> --- a/lib/netdev-tc-offloads.c > >>>> +++ b/lib/netdev-tc-offloads.c > >>>> @@ -1527,6 +1527,9 @@ netdev_tc_init_flow_api(struct netdev *netdev) > >>>> return -ifindex; > >>>> } > >>>> > >>>> + /* make sure there is no ingress qdisc */ > >>>> + tc_add_del_ingress_qdisc(ifindex, false, 0); > >>>> + > >>>> if (ovsthread_once_start(&block_once)) { > >>>> probe_tc_block_support(ifindex); > >>>> ovsthread_once_done(&block_once); > >>>> -- > >>>> 2.7.0 > >>>>
diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c index da43549f2f22..b33a79bb14a2 100644 --- a/lib/netdev-tc-offloads.c +++ b/lib/netdev-tc-offloads.c @@ -1527,6 +1527,9 @@ netdev_tc_init_flow_api(struct netdev *netdev) return -ifindex; } + /* make sure there is no ingress qdisc */ + tc_add_del_ingress_qdisc(ifindex, false, 0); + if (ovsthread_once_start(&block_once)) { probe_tc_block_support(ifindex); ovsthread_once_done(&block_once);
It could be a port added to ovs bridge already has ingress qdisc which will make the block probe fail. The probes should start clean and ingress is being added later so just remove ingress in case it exists. Signed-off-by: Roi Dayan <roid@mellanox.com> --- lib/netdev-tc-offloads.c | 3 +++ 1 file changed, 3 insertions(+)