From: Han Zhou <[email protected]> Date: Wednesday, December 21, 2016 at 12:45 PM To: Darrell Ball <[email protected]> Cc: "[email protected]" <[email protected]> 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 <[email protected]<mailto:[email protected]>> 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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
