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