On 7/13/23 08:48, Priyankar Jain wrote: > Hi Dumitru, > > On 12/07/23 4:12 pm, Dumitru Ceara wrote: >> On 6/6/23 14:10, Xavier Simonart wrote: >>> Hi Priyankar, Mark >>> >>> Thanks for the patch. I agree with Mark - the description is really >>> great ! >>> Based on your description, I tried creating a unit-test reproducing the >>> issue, and checking that your patch fixes it. >>> I came up with [0]. It reproduces some issues (flows not deleted) on >>> origin/main, and the issues fixed using your patch. So, it looks good. >>> >>> However, if, in [0] I remove the "sleep 2" (see below), then it seems >>> that >>> there are still some issues. >>> It might not be exactly the same issue you saw, but is very similar - >>> the >>> same flow does not get properly deleted. >>> I think that the (new) issue is the following: >>> When a port is claimed by two different chassis (as part of the >>> migration), >>> ovn-controllers try to avoid a "war" between themselves, and postpone >>> port >>> claiming if the port got claimed very recently. >>> This works fine. But, if, while a port claim is postponed, the >>> interface is >>> deleted, it seems that some flows are not properly removed. >>> Checking that the port is postponed in is_binding_lport_this_chassis >>> might >>> be enough, but this requires additional check. >>> (if we want to add this unit test to the patch, then we probably need to >>> move some of the functions to ovn-macros to avoid duplication, as I >>> steal >>> them from the system tests) >> >> Hi Priyankar, >> >> Thanks for the fix! >> >> I guess we have two options: >> >> (1) go ahead with your patch but also include Xavier's test. >> (2) figure out a more generic solution which covers the second problem >> reported by Xavier too and respin this patch as a v2 that fixes all >> cases. >> >> I'd prefer we go for (2) mainly because we know there's a problem but >> also because the "sleep 2" in the test case makes me uneasy. >> >> Do you have some input on this? > > I concur. We should go ahead with (2). Lately I was not able to spend > time on the patch. I'll try to spend some cycles working on the generic > fix for the issues forementioned.
Thanks, I set the state of this patch to "changes requested" in patchwork and I'm looking forward to v2. Regards, Dumitru _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
