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

[Dumitru Ceara]

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>.

> 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