[ovs-dev] [PATCH v2] ovn-controller: Fix duplicated flows in table 32.

[Darrell Ball]

From: Han Zhou <zhouhan at gmail.com>
Date: Wednesday, December 21, 2016 at 12:45 PM
To: Darrell Ball <dball at vmware.com>
Cc: "dev at openvswitch.org" <dev at openvswitch.org>
Subject: Re: [ovs-dev] [PATCH v2] ovn-controller: Fix duplicated flows in table 32.

Hi Darrell,
Sorry for late response. I posted v3 just now.

On Wed, Dec 14, 2016 at 12:19 PM, Darrell Ball <dball at vmware.com<mailto:dball at vmware.com>> wrote:
>
> ERROR log level is not appropriate since we always filter adding the extra flows anyways
>
> and it amount to extra processing and filtering.
>
> We anyways attempt to re-add every flow for every iteration of the ovn-controller main loop,
>
> not just physical flows; we filter these attempts with “installed_flows”, so we are doing lots
>
> of extra processing normally, at this time.
>

This is not true. There is a temporary flow_table constructed for desired flows, which is initiated and destructed in each iteration of main loop. The "duplicate flow" reported by the log is in fact for duplications in a single iteration. But I agree that ERROR log level is not necessary.
Which part was not true ? I don’t follow.
Here is the full context again in one paragraph.
Consider the case where there are no new flows added/removed from one iteration of the main loop
to another. For each iteration of the main loop, desired flows is initially empty, which means we add
each and every flow to desired_flows each iteration of the main loop.
The INFO log occurs because we try to add the same flow to desired_flows in one iteration
of the loop and we filter the attempt to add the new flow and log a message to that effect.
However, there is installed_flows as well, which keeps context across iterations.
When the desired_flows is checked against installed_flows which occurs every loop iteration,
we find out we already know about each and every flow in desired_flows (i.e. they are all
duplicates) so the attempted flow adds from desired_flows to installed_flows are all filtered.



>
>
> With the vtep test, the non-physical flows are not as many and diverse. So checking
>
> for attempting to add the same flow in the same iteration of ovn-controller main loop
>
> where the desired_flows log check is made (which is before the installed_flows check)
>
> should help to catch the physical flow duplicate attempts in a given iteration of the main loop.
>
>
>
> If we added the check there, we would be trying to catch some portion of the unnecessary extra
>
> processing and filtering which seems valid and avoid noisy logs, which could obscure other logs ?
>
>
I added the check for the 3 HVs test case, which I think is more appropriate since it is basic functionality.
Adding to any simple test is fine. I don’t care which one per se as long as you feel comfortable
with that test.
Thanks,
Han