On Fri, Nov 19, 2021 at 11:13 AM Numan Siddique <[email protected]> wrote: > > On Thu, Nov 11, 2021 at 5:39 PM Han Zhou <[email protected]> wrote: > > > > When a logical flow translation uses conjunction ids, it is better to > > keep the conjuntion ids consistent between iterations, so that the > > generated OVS flows don't change unnecessarily and avoid reinstalling > > unchanged flows to OVS. The problem was partially solved when > > lflow-cache was enabled but there was a bug for a corner case so the > > lflow-cache based approach was reverted in a previous commit. > > > > This patch takes an approach that generates conjunction ids based on the > > logical flow's uuid, which ensures the same ids being used when > > processing the same lflow in most cases. There is a very little chance > > that different logial flows happen to have the same 32-bit prefix in > > their uuids, but the probability is really low. For the very unlikely > > situation, the conflicts are handled properly and correctness is > > ensured. > > > > A new module lflow-conj-ids is created to manage the id allocations and > > maintaining the usage of each ids and their owner lflows. > > > > Unlike the previously lflow-cache based approach, this approach works > > equally well when lflow-cache is not in use. > > > > Signed-off-by: Han Zhou <[email protected]> > > Hi Han, > > Few minor comments. > > 1. I think it would be nice to reorganize the newly added file - > controller/lflow-conj-ids.c to > be organized with public functions first and then static functions as > per the coding > guidelines - https://github.com/ovn-org/ovn/blob/main/Documentation/internals/contributing/coding-style.rst > Ack. Thanks for reminding.
> 2. I'd suggest to add a debug function in > controller/lflow-conj-ids.c which would dump > the allocated conj ids and the lflow conj_id mapping. And the > test file can verify the dump is as expected. > What do you think ? I'm fine if you think this can be a follow up patch. > Also if required, we can add appctl command to dump the conj id > allocations. > Sounds good. And yes I think it can be a follow-up patch. I will merge it for now and try to add the suggested debug command before the 21.12 release. Thanks, Han _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
