| Message ID | 20210510215938.369355-1-hzhou@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v3] ovn-northd.c: Don't use DPG for MC_UNKNOWN related lflows. | expand |
Bleep bloop. Greetings Han Zhou, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: ERROR: Co-author Dumitru Ceara <dceara@redhat.com> needs to sign off. Lines checked: 87, Warnings: 0, Errors: 1 Please check this out. If you feel there has been an error, please email aconole@redhat.com Thanks, 0-day Robot
On 5/10/21 11:59 PM, Han Zhou wrote: > In commit b468c2c1 it avoided using DPG for multicast related lflows, but > commit dd94f126 update the MC_UNKNOWN related lflows with DPG used again > by mistake. 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 also fixes the ovn-northd-ddlog part, which missed using > UniqueFlow() for MC_UNKNOWN from the initial ddlog commit 0e77b3bcbf. > > Fixes: dd94f1266c ("northd: MAC learning: Add logical flows for fdb.") > Fixes: 0e77b3bcbf ("ovn-northd-ddlog: New implementation of ovn-northd based on ddlog.") > Cc: Numan Siddique <numans@ovn.org> > Cc: Leonid Ryzhyk <lryzhyk@vmware.com> > Co-authored-by: Dumitru Ceara <dceara@redhat.com> LGTM. For the ddlog part: Signed-off-by: Dumitru Ceara <dceara@redhat.com> > Signed-off-by: Han Zhou <hzhou@ovn.org> > --- Thanks! Dumitru
On Tue, May 11, 2021 at 4:41 AM Dumitru Ceara <dceara@redhat.com> wrote: > > On 5/10/21 11:59 PM, Han Zhou wrote: > > In commit b468c2c1 it avoided using DPG for multicast related lflows, but > > commit dd94f126 update the MC_UNKNOWN related lflows with DPG used again > > by mistake. 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 also fixes the ovn-northd-ddlog part, which missed using > > UniqueFlow() for MC_UNKNOWN from the initial ddlog commit 0e77b3bcbf. > > > > Fixes: dd94f1266c ("northd: MAC learning: Add logical flows for fdb.") > > Fixes: 0e77b3bcbf ("ovn-northd-ddlog: New implementation of ovn-northd based on ddlog.") > > Cc: Numan Siddique <numans@ovn.org> > > Cc: Leonid Ryzhyk <lryzhyk@vmware.com> > > Co-authored-by: Dumitru Ceara <dceara@redhat.com> > > LGTM. For the ddlog part: > > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > > > Signed-off-by: Han Zhou <hzhou@ovn.org> Thanks for fixing this. Acked-by: Numan Siddique <numans@ovn.org> Numan > > --- > > Thanks! > Dumitru > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
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 --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl index ffd09c35f..147b0fcb6 100644 --- a/northd/ovn_northd.dl +++ b/northd/ovn_northd.dl @@ -4218,17 +4218,22 @@ for (sw in &Switch(.ls = nb::Logical_Switch{._uuid = ls_uuid})) { .actions = "outport = get_fdb(eth.dst); next;", .external_ids = map_empty()); - Flow(.logical_datapath = ls_uuid, - .stage = s_SWITCH_IN_L2_UNKNOWN(), - .priority = 50, - .__match = "outport == \"none\"", - .actions = if (sw.has_unknown_ports) { - var mc_unknown = json_string_escape(mC_UNKNOWN().0); - "outport = ${mc_unknown}; output;" - } else { - "drop;" - }, - .external_ids = map_empty()); + if (sw.has_unknown_ports) { + var mc_unknown = json_string_escape(mC_UNKNOWN().0) in + UniqueFlow[Flow{.logical_datapath = ls_uuid, + .stage = s_SWITCH_IN_L2_UNKNOWN(), + .priority = 50, + .__match = "outport == \"none\"", + .actions = "outport = ${mc_unknown}; output;", + .external_ids = map_empty()}] + } else { + Flow(.logical_datapath = ls_uuid, + .stage = s_SWITCH_IN_L2_UNKNOWN(), + .priority = 50, + .__match = "outport == \"none\"", + .actions = "drop;", + .external_ids = map_empty()) + }; Flow(.logical_datapath = ls_uuid, .stage = s_SWITCH_IN_L2_UNKNOWN(),
In commit b468c2c1 it avoided using DPG for multicast related lflows, but commit dd94f126 update the MC_UNKNOWN related lflows with DPG used again by mistake. 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 also fixes the ovn-northd-ddlog part, which missed using UniqueFlow() for MC_UNKNOWN from the initial ddlog commit 0e77b3bcbf. Fixes: dd94f1266c ("northd: MAC learning: Add logical flows for fdb.") Fixes: 0e77b3bcbf ("ovn-northd-ddlog: New implementation of ovn-northd based on ddlog.") Cc: Numan Siddique <numans@ovn.org> Cc: Leonid Ryzhyk <lryzhyk@vmware.com> Co-authored-by: Dumitru Ceara <dceara@redhat.com> Signed-off-by: Han Zhou <hzhou@ovn.org> --- v1 -> v2: Addresses Dumitru's comment about commit msg with corrected "Fixes" tag and reworded the message. v2 -> v3: Added ddlog fix and added Dumitru as co-author. Updated the commit message. northd/ovn-northd.c | 6 +++--- northd/ovn_northd.dl | 27 ++++++++++++++++----------- 2 files changed, 19 insertions(+), 14 deletions(-)