On Mon, May 10, 2021 at 12:42 AM Dumitru Ceara <[email protected]> wrote:
>
> On 5/8/21 5:08 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 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.
>
> At a second look, the ddlog implementation is mainly up to date wrt DPG
> and unique flows.
>
> I think we just miss the incremental below.
>
> Regards,
> Dumitru
>

Thanks Dumitru! Sorry that I only looked at commit b468c2c1 and didn't see
it update any *.dl files, but now I realized that it was before the whole
DDlog implemented was added. The DDlog implementation somehow missed using
UniqueFlow for the MC_UNKNOWN lflows. So I incorporated your change in v3
and updated the commit message accordingly, and I added you as co-author. I
need your sign-off, if you are ok with it:
https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/


> 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(),
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to