[ovs-dev] [PATCH ovn 0/4] Fix ovn-controller I-P for multicast groups and lport changes

Han Zhou hzhou at ovn.org
Fri Jun 11 19:45:44 UTC 2021


On Mon, Jun 7, 2021 at 12:16 AM Dumitru Ceara <dceara at redhat.com> wrote:
>
> On 6/7/21 7:33 AM, Han Zhou wrote:
> > On Fri, Jun 4, 2021 at 6:56 AM Dumitru Ceara <dceara at redhat.com> wrote:
> >>
> >> On 5/28/21 9:23 PM, Han Zhou wrote:
> >>> The series fixes incremental processing for missing dependency
handling
> > for
> >>> multicast group and logical port binding changes when computing
logical
> > flows.
> >>> It also removes the workaround in northd that was required due to the
> > missing
> >>> dependency handling. In addition, the fix also allows us to monitor
all
> > DPGs as
> >>> an optimization, so it is also included in the series.
> >>>
> >>
> >> Hi Han,
> >>
> >> Thanks for working on fixing this!  I wonder however if there will be
> >> any performance impact due to the added resource references?  I didn't
> >> do any scale testing, did you have a chance to?
> >>
> > Hi Dumitru,
> >
>
> Hi Han,
>
> > Thanks for the review. There is no performance impact noticed because
the
> > port-binding reference is already tracked in the existing implementation
> > for lports used at outport/inport and is_chassis_resident. This fix
mainly
> > changes the way how it is tracked (by name instead of DP keys). I admit
> > that there are some extra references added compared with existing
> > implementation:
> > 1. References for the lports that are not found
> > 2. For lports that are used other than for
> > inport/outport/is_chassis_resident
>
> I'm not sure there is any other lport reference except the three above.
>
> > 3. For MC groups (found or not found)
> >
> > 1) is the very few cases that do not matter for performance but matter
for
> > correctness. 2) seems also rare, if exists at all. 3) shouldn't impact
> > performance either because the number of MC groups should be much
smaller
> > than that of lports.
> > So I think the performance impact is negligible and if there is any
cost it
> > is necessary for the correctness. I ran a test with 1k lswitches, 10k
> > lports and an ACL using half of the ports in a logical group (and
address
> > set) and triggers full recompute - there is no performance difference
> > noticed.
> >
>
> Great, thanks for the confirmation.
>
> >> As an alternative, maybe longer term though, would it make sense to use
> >> explicit references instead in the Southbound schema?  That would
> >> simplify the ovn-controller code and would rely on the IDL to propagate
> >> the change tracking to the flows that refer to multicast groups/port
> >> bindings/port groups.
> >
> > I wonder if this is possible because the end user specifies the "match"
> > directly in an ACL, which can use port names. We will have to change the
> > ACL syntax to support it.
>
> An alternative would be to perform some basic ACL match parsing in
> northd, parsing tokens like
> inport/outport/is_chassis_resident/$<address_set>/@<port_group>.
>
Parsing is expensive, so I think we'd better avoid parsing ACLs in northd
and then parsing logical flows again in ovn-controller. For maintaining the
references in the ovn-controller itself, I think it is not a big problem.
For the overall performance, it might be useful to define a more structured
ACL format that is sufficient for real world CMS use cases but with
predictable fields. I am not sure if this is necessary, and I think it may
depend on the outcome of the ovn-controller ddlog effort. Anyway, it is a
bigger discussion and maybe off topic :)

Regardless, I submitted v2 for the series which addresses your comments so
far. PTAL: https://patchwork.ozlabs.org/project/ovn/list/?series=248448

Thanks,
Han

> > On the other hand, if we can generate lflows using references to SB
> > port-bindings or MC groups, we would be able to put the DP key in the
lflow
> > directly, without the need for ovn-controller to parse the
lport/mc-group
> > at all, so there is no reference needed, right?
>
> Exactly, that was what I was thinking too.  I wonder how we'd maintain
> backwards compatibility though.  Would we need a logical_flow_v2 table
> in the Southbound?
>
> >
> > Thanks,
> > Han
> >
>
> Thanks,
> Dumitru
>


More information about the dev mailing list