On Thu, Jan 11, 2024 at 7:32 AM <num...@ovn.org> wrote:
>
> From: Numan Siddique <num...@ovn.org>
>
> ovn_lflow_add() and other related functions/macros are now moved
> into a separate module - lflow-mgr.c. This module maintains a
> table 'struct lflow_table' for the logical flows. lflow table
> maintains a hmap to store the logical flows.
>
> It also maintains the logical switch and router dp groups.
>
> Previous commits which added lflow incremental processing for
> the VIF logical ports, stored the references to
> the logical ports' lflows using 'struct lflow_ref_list'. This
> struct is renamed to 'struct lflow_ref' and is part of lflow-mgr.c.
> It is modified a bit to store the resource to lflow references.
>
> Example usage of 'struct lflow_ref'.
>
> 'struct ovn_port' maintains 2 instances of lflow_ref. i,e
>
> struct ovn_port {
> ...
> ...
> struct lflow_ref *lflow_ref;
> struct lflow_ref *stateful_lflow_ref;
Hi Numan,
In addition to the lock discussion with you and Dumitru, I still want to
discuss another thing of this patch regarding the second lflow_ref:
stateful_lflow_ref.
I understand that you added this to achieve finer grained I-P especially
for router ports. I am wondering how much performance gain is from this.
For my understanding it shouldn't matter much since each ovn_port should be
associated with a very limited number of lflows. Could you provide more
insight/data on this? I think it would be better to keep things simple
(i.e. one object, one lflow_ref list) unless the benefit is obvious.
I am also trying to run another performance regression test for recompute,
since I am a little concerned about the DP refcnt hmap associated with each
lflow. I understand it's necessary to handle the duplicated lflow cases,
but it looks heavy and removes the opportunities for more efficient bitmap
operations. Let me know if you have already had evaluated its performance.
Thanks,
Han
> };
>
> All the logical flows generated by
> build_lswitch_and_lrouter_iterate_by_lsp() uses the ovn_port->lflow_ref.
>
> All the logical flows generated by build_lsp_lflows_for_lbnats()
> uses the ovn_port->stateful_lflow_ref.
>
> When handling the ovn_port changes incrementally, the lflows referenced
> in 'struct ovn_port' are cleared and regenerated and synced to the
> SB logical flows.
>
> eg.
>
> lflow_ref_clear_lflows(op->lflow_ref);
> build_lswitch_and_lrouter_iterate_by_lsp(op, ...);
> lflow_ref_sync_lflows_to_sb(op->lflow_ref, ...);
>
> This patch does few more changes:
> - Logical flows are now hashed without the logical
> datapaths. If a logical flow is referenced by just one
> datapath, we don't rehash it.
>
> - The synthetic 'hash' column of sbrec_logical_flow now
> doesn't use the logical datapath. This means that
> when ovn-northd is updated/upgraded and has this commit,
> all the logical flows with 'logical_datapath' column
> set will get deleted and re-added causing some disruptions.
>
> - With the commit [1] which added I-P support for logical
> port changes, multiple logical flows with same match 'M'
> and actions 'A' are generated and stored without the
> dp groups, which was not the case prior to
> that patch.
> One example to generate these lflows is:
> ovn-nbctl lsp-set-addresses sw0p1 "MAC1 IP1"
> ovn-nbctl lsp-set-addresses sw1p1 "MAC1 IP1"
> ovn-nbctl lsp-set-addresses sw2p1 "MAC1 IP1"
>
> Now with this patch we go back to the earlier way. i.e
> one logical flow with logical_dp_groups set.
>
> - With this patch any updates to a logical port which
> doesn't result in new logical flows will not result in
> deletion and addition of same logical flows.
> Eg.
> ovn-nbctl set logical_switch_port sw0p1 external_ids:foo=bar
> will be a no-op to the SB logical flow table.
>
> [1] - 8bbd678("northd: Incremental processing of VIF additions in 'lflow'
node.")
>
> Signed-off-by: Numan Siddique <num...@ovn.org>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
