On 6/11/21 9:45 PM, Han Zhou wrote: > On Mon, Jun 7, 2021 at 12:16 AM Dumitru Ceara <[email protected]> wrote: >> >> On 6/7/21 7:33 AM, Han Zhou wrote: >>> On Fri, Jun 4, 2021 at 6:56 AM Dumitru Ceara <[email protected]> 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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
