On 6/12/23 16:47, Simon Horman wrote: > On Mon, Jun 12, 2023 at 02:10:17AM +0000, Peng He wrote: >> OVS allows overlapping megaflows, as long as the actions of these >> megaflows are equal. However, the current implementation of action >> modification relies on flow_lookup instead of ufid, this could result >> in looking up a wrong megaflow and make the ukeys and megaflows inconsistent >> >> Just like the test case in the patch, at first we have a rule with the >> prefix: >> >> 10.1.2.0/24 >> >> and we will get a megaflow with prefixes 10.1.2.2/24 when a packet with IP >> 10.1.2.2 is received. >> >> Then suppose we change the rule into 10.1.0.0/16. OVS prefers to keep the >> 10.1.2.2/24 megaflow and just changes its action instead of extending >> the prefix into 10.1.2.2/16. >> >> then suppose we have a 10.1.0.2 packet, since it misses the megaflow, >> this time, we will have an overlapping megaflow with the right prefix: >> 10.1.0.2/16 >> >> now we have two megaflows: >> 10.1.2.2/24 >> 10.1.0.2/16 >> >> last, suppose we have changed the ruleset again. The revalidator this >> time still decides to change the actions of both megaflows instead of >> deleting them. >> >> The dpif_netdev_flow_put will search the megaflow to modify with unmasked >> keys, however it might lookup the wrong megaflow as the key 10.1.2.2 matches >> both 10.1.2.2/24 and 10.1.0.2/16! >> >> This patch changes the megaflow lookup code in modification path into >> relying the ufid to find the correct megaflow instead of key lookup. >> >> Signed-off-by: Peng He <hepeng.0...@bytedance.com> > > Hi Peng, > > I see one failure in CI. It is the " compacting online - cluster " > test for "liunx clang test asan". Is this something we should be concerned > about? > > Link: > https://github.com/ovsrobot/ovs/actions/runs/5238809864/jobs/9458063244#step:11:5573
This one actually succeeded on re-check, so it's not a real problem. The main issue is that the test introduced by the patch failed [1]: 1086: PMD - revalidator wrongly modify userspace megaflows FAILED (pmd.at:1342) --- - 2023-06-12 02:34:21.014979499 +0000 +++ /home/runner/work/ovs/ovs/openvswitch-3.1.90/_build/sub/tests/testsuite.dir/at-groups/1086/stdout 2023-06-12 02:34:21.008942243 +0000 @@ -1,4 +1,3 @@ recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(dst=10.1.0.2/255.255.0.0,frag=no), packets:0, bytes:0, used:0.0s, actions:2 -recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(dst=10.1.2.2/255.255.255.0,frag=no), packets:0, bytes:0, used:0.0s, actions:2 [1] https://github.com/ovsrobot/ovs/actions/runs/5238809864/jobs/9458063244#step:11:5569 So, either the test is unstable for some reason, or the patch doesn't really fix the issue it is trying to fix. Best regards, Ilya Maximets. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
