[ovs-dev] [PATCH ovn v3 4/4] lflow: Consistent conjunction id generation.
hzhou at ovn.org
Fri Nov 19 20:02:01 UTC 2021
On Fri, Nov 19, 2021 at 11:13 AM Numan Siddique <numans at ovn.org> wrote:
> On Thu, Nov 11, 2021 at 5:39 PM Han Zhou <hzhou at ovn.org> 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 <hzhou at ovn.org>
> 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 -
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
> Also if required, we can add appctl command to dump the conj id
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.
More information about the dev