diff mbox series

[ovs-dev] ovn-northd.c: Don't use DPG for MC_UNKNOWN related lflows.

Message ID 20210508015150.3756956-1-hzhou@ovn.org
State Superseded
Headers show
Series [ovs-dev] ovn-northd.c: Don't use DPG for MC_UNKNOWN related lflows. | expand

Commit Message

Han Zhou May 8, 2021, 1:51 a.m. UTC
In commit b468c2c1 it avoided using DPG for multicast related lflows but
missed applying it for the MC_UNKNOWN related lflows. This patch fixes
it to avoid problems when a lflow using MC_UNKNOWN is monitored by a
ovn-controller before the related mc-group is monitored, which could
cause ovn-controller problem due to an incomplete dependency handling in
I-P.

This change can be removed (and applying DPG for all the other mc-group
related lflows) when the I-P dependency handling problem is fixed.

This change didn't address the ovn-northd-ddlog part because commit
b468c2c1 didn't update ovn-northd-ddlog either. To be consistent, the
ddlog change will be added together for all mc-group related flows.

Fixes: b468c2c1bd47 ("northd: Use _add_unique() functions for all multicast related flows.")
Cc: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Han Zhou <hzhou@ovn.org>
---
 northd/ovn-northd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Dumitru Ceara May 8, 2021, 7:51 a.m. UTC | #1
On 5/8/21 3:51 AM, Han Zhou wrote:
> In commit b468c2c1 it avoided using DPG for multicast related lflows but
> missed applying it for the MC_UNKNOWN related lflows. This patch fixes
> it to avoid problems when a lflow using MC_UNKNOWN is monitored by a
> ovn-controller before the related mc-group is monitored, which could
> cause ovn-controller problem due to an incomplete dependency handling in
> I-P.
> 
> This change can be removed (and applying DPG for all the other mc-group
> related lflows) when the I-P dependency handling problem is fixed.
> 
> This change didn't address the ovn-northd-ddlog part because commit
> b468c2c1 didn't update ovn-northd-ddlog either. To be consistent, the
> ddlog change will be added together for all mc-group related flows.
> 
> Fixes: b468c2c1bd47 ("northd: Use _add_unique() functions for all multicast related flows.")

Nit: this should actually be:

Fixes: dd94f1266ca4 ("northd: MAC learning: Add logical flows for fdb.")

And the commit log should, probably, be amended.

Otherwise:
Acked-by: Dumitru Ceara <dceara@redhat.com>

> Cc: Ilya Maximets <i.maximets@ovn.org>
> Signed-off-by: Han Zhou <hzhou@ovn.org>
> ---
>  northd/ovn-northd.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 1d4c5b67b..cccb9f3ed 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -6767,9 +6767,9 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *lflows)
>                        "outport = get_fdb(eth.dst); next;");
>  
>          if (od->has_unknown) {
> -            ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_UNKNOWN, 50,
> -                          "outport == \"none\"",
> -                          "outport = \""MC_UNKNOWN"\"; output;");
> +            ovn_lflow_add_unique(lflows, od, S_SWITCH_IN_L2_UNKNOWN, 50,
> +                                 "outport == \"none\"",
> +                                 "outport = \""MC_UNKNOWN "\"; output;");
>          } else {
>              ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_UNKNOWN, 50,
>                            "outport == \"none\"", "drop;");
>
Han Zhou May 8, 2021, 3:09 p.m. UTC | #2
On Sat, May 8, 2021 at 12:51 AM Dumitru Ceara <dceara@redhat.com> wrote:
>
> On 5/8/21 3:51 AM, Han Zhou wrote:
> > In commit b468c2c1 it avoided using DPG for multicast related lflows but
> > missed applying it for the MC_UNKNOWN related lflows. This patch fixes
> > it to avoid problems when a lflow using MC_UNKNOWN is monitored by a
> > ovn-controller before the related mc-group is monitored, which could
> > cause ovn-controller problem due to an incomplete dependency handling in
> > I-P.
> >
> > This change can be removed (and applying DPG for all the other mc-group
> > related lflows) when the I-P dependency handling problem is fixed.
> >
> > This change didn't address the ovn-northd-ddlog part because commit
> > b468c2c1 didn't update ovn-northd-ddlog either. To be consistent, the
> > ddlog change will be added together for all mc-group related flows.
> >
> > Fixes: b468c2c1bd47 ("northd: Use _add_unique() functions for all
multicast related flows.")
>
> Nit: this should actually be:
>
> Fixes: dd94f1266ca4 ("northd: MAC learning: Add logical flows for fdb.")
>
> And the commit log should, probably, be amended.
>
> Otherwise:
> Acked-by: Dumitru Ceara <dceara@redhat.com>

Thanks Dumitru, my mistake. I corrected the message in v2.

>
> > Cc: Ilya Maximets <i.maximets@ovn.org>
> > Signed-off-by: Han Zhou <hzhou@ovn.org>
> > ---
> >  northd/ovn-northd.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index 1d4c5b67b..cccb9f3ed 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -6767,9 +6767,9 @@ build_lswitch_flows(struct hmap *datapaths,
struct hmap *lflows)
> >                        "outport = get_fdb(eth.dst); next;");
> >
> >          if (od->has_unknown) {
> > -            ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_UNKNOWN, 50,
> > -                          "outport == \"none\"",
> > -                          "outport = \""MC_UNKNOWN"\"; output;");
> > +            ovn_lflow_add_unique(lflows, od, S_SWITCH_IN_L2_UNKNOWN,
50,
> > +                                 "outport == \"none\"",
> > +                                 "outport = \""MC_UNKNOWN "\";
output;");
> >          } else {
> >              ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_UNKNOWN, 50,
> >                            "outport == \"none\"", "drop;");
> >
>
diff mbox series

Patch

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 1d4c5b67b..cccb9f3ed 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -6767,9 +6767,9 @@  build_lswitch_flows(struct hmap *datapaths, struct hmap *lflows)
                       "outport = get_fdb(eth.dst); next;");
 
         if (od->has_unknown) {
-            ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_UNKNOWN, 50,
-                          "outport == \"none\"",
-                          "outport = \""MC_UNKNOWN"\"; output;");
+            ovn_lflow_add_unique(lflows, od, S_SWITCH_IN_L2_UNKNOWN, 50,
+                                 "outport == \"none\"",
+                                 "outport = \""MC_UNKNOWN "\"; output;");
         } else {
             ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_UNKNOWN, 50,
                           "outport == \"none\"", "drop;");