On 20 Feb 2022, at 14:07, Roi Dayan wrote:
> On 2022-02-20 1:47 PM, Roi Dayan wrote: <SNIP> >>> This is leaving the revalidators in some kind of a loop. Without the patch, >>> I do not see the problem. This is a log snippet: >>> >>> 2022-02-17T15:38:52.990Z|00050|tc|WARN|expected act csum with flags: 0x9 >>> 2022-02-17T15:38:52.990Z|00051|tc|WARN|Kernel flower acknowledgment does >>> not match request! Set dpif_netlink to dbg to see which rule caused this >>> error. >>> 2022-02-17T15:38:52.993Z|00052|tc|WARN|expected act csum with flags: 0x9 >>> 2022-02-17T15:38:53.052Z|00001|tc(revalidator18)|WARN|expected act csum >>> with flags: 0x9 >>> 2022-02-17T15:38:53.052Z|00002|netdev_offload_tc(revalidator18)|ERR|flow >>> get failed (dev ovs-system prio 3 handle 1): Invalid argument >>> 2022-02-17T15:38:53.052Z|00003|tc(revalidator18)|WARN|expected act csum >>> with flags: 0x9 >>> 2022-02-17T15:38:53.052Z|00004|netdev_offload_tc(revalidator18)|ERR|flow >>> get failed (dev enp4s0f1 prio 3 handle 1): Invalid argument >>> 2022-02-17T15:38:53.052Z|00005|netdev_offload_tc(revalidator18)|ERR|flow >>> get failed (dev enp4s0f0 prio 3 handle 1): Invalid argument >>> 2022-02-17T15:38:53.052Z|00006|netdev_offload_tc(revalidator18)|ERR|flow >>> get failed (dev ovs_pvp_br0 prio 3 handle 1): Invalid argument >>> 2022-02-17T15:38:53.052Z|00007|dpif(revalidator18)|WARN|system@ovs-system: >>> failed to flow_get (No such file or directory) >>> ufid:c5f9a0b1-3399-4436-b742-30825c64a1e5 <empty>, packets:0, bytes:0, >>> used:never >>> 2022-02-17T15:38:53.052Z|00008|ofproto_dpif_upcall(revalidator18)|WARN|Failed >>> to acquire udpif_key corresponding to unexpected flow (No such file or >>> directory): ufid:c5f9a0b1-3399-4436-b742-30825c64a1e5 >>> >>> So I guess this patch needs some fixing… I will do a popper code review for >>> v3, and maybe you can add those test cases :) >>> >>> //Eelco >>> >> >> hmm ok I see what's going on. >> >> before this commit all pedit actions merged into single one and then >> there was an additon of csum action after that single pedit. >> so csum always existed and the code zero flower->csum_update_flags. >> >> after this commit there are multiple pedits as the order is important >> and csum being calculated on the next non pedit action. >> since your last action in the example is pedit we now dont add the last >> csum action. >> >> when ovs dumps and parse the tc rules it hits this rule with last >> pedit action and expect a csum action after it but doesnt find it. >> >> so we basically missing calling nl_msg_put_csum_act() one more time >> after the loop of actions in nl_msg_put_flower_acts(). >> >> my test case of editing macs didn't update csum_update_flags which >> didn't trigger this error. your case changing ip header did. >> you case is enough and covers my case so no need for both. >> >> it's a small issue as real case wont have a rule ending with pedit >> and not a forward action like redirect, mirror, goto.. >> >> thanks for catching >> > > I have added the fix and tested manually and its ok but as for the test > in ovs testsuite i have a problem. > Adding tc rule with ovd-dpctl is deleted right away. manually it works > as you and I tested but I added in the testsuite and most of the times > its faster and ovs deletes the rule and the tc filter show doesn't see > the rule anymore and fails the test. I did not investigate, but I guess the revalidators are cleaning up these none OVS installed rules. I can only run the tests if I execute all on one shell line, if I do it manually it will fail, i.e.: ovs-appctl dpctl/add-flow "ufid:c5f9a0b1-3399-4436-b742-30825c64a1e5,recirc_id(0),in_port(2),eth_type(0x0800),eth(),ipv4(proto=6),tcp()" "set(ipv4(ttl=3)),3,set(ipv4(ttl=4)),2" && ovs-appctl dpctl/dump-flows type=tc && tc filter show dev enp4s0f1 ingress I guess for the real test cases you need to install OpenFlow rules, send a crafter packet, and see the results. //Eelco <SNIP> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
