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

Dumitru Ceara dceara at redhat.com
Mon Jun 14 12:06:23 UTC 2021


On 6/11/21 9:45 PM, Han Zhou wrote:
> 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

Is there already a plan moving forward for ovn-controller ddlog?

> bigger discussion and maybe off topic :)

Agreed.

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

Sure, I'll have a look these days.

> 
> Thanks,
> Han
> 
Thanks,
Dumitru



More information about the dev mailing list