On 9/15/22 13:38, Tianyu Yuan wrote: > > On 9/15/22 19:28, Ilya Maximets wrote: >> On 9/14/22 16:19, Simon Horman wrote: >>> From: Tianyu Yuan <[email protected]> >>> >>> Since dd9881ed55e6 ("tc: Fix stats dump when using same meter table") >>> rule stats update will ignore meter police. Correspondingly, the >>> reference stats of the test should also be modified to ensure the test >>> could pass correctly. >>> >>> Fixes: dd9881ed55e6 ("tc: Fix stats dump when using same meter table") >>> Signed-off-by: Tianyu Yuan <[email protected]> >>> Signed-off-by: Simon Horman <[email protected]> >>> --- >>> tests/system-offloads-traffic.at | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/tests/system-offloads-traffic.at >>> b/tests/system-offloads-traffic.at >>> index d9b815a5ddf4..24e49d42f589 100644 >>> --- a/tests/system-offloads-traffic.at >>> +++ b/tests/system-offloads-traffic.at >>> @@ -264,7 +264,7 @@ NS_CHECK_EXEC([at_ns0], [echo "mark" | nc >>> $NC_EOF_OPT -u 10.1.1.2 5678 -p 6789]) done >>> >>> AT_CHECK([ovs-appctl dpctl/dump-flows | grep "meter" | >>> DUMP_CLEAN_SORTED], [0], [dnl >>> -in_port(2),eth(macs),eth_type(0x0800),ipv4(proto=17,frag=no), >>> packets:10, bytes:330, used:0.001s, actions:meter(0),3 >>> +in_port(2),eth(macs),eth_type(0x0800),ipv4(proto=17,frag=no), >>> +packets:1, bytes:33, used:0.001s, actions:meter(0),3 >> >> This looks very strange to me. The test does send 10 packets. >> Why the flow should report only one? >> > Thanks for your review Ilya. > The test does send 10 packets but 9 of them are dropped by > meter action. As we descript in commit (dd9881ed55e6), the > flow stats should not report the police stats. In this case, only > one packet passes the meter action and be used to update > flow stats.
The flow statistics counts how many packets hit the flow by the match criteria, not how many of them survived after the action execution. See the same test with offloads disabled (next to it in the file). It correctly counts all the 10 packets. If we will not count packets, OVS will eventually remove the flow from the datapath causing removal from TC and hardware and triggering upcalls on the next packet. And OpenFlow statistics will also be incorrect. Best regards, Ilya Maximets. >>> ]) >>> >>> AT_CHECK([ovs-ofctl -O OpenFlow13 meter-stats br0 | sed -e >>> 's/duration:[[0-9]].[[0-9]]*s/duration:0.001s/'], [0], [dnl >> >> These meter stats are correctly reporting 11 packets, so the datapath flow >> should report 10 (+1 on the upcall), AFAIU. >> >> Best regards, Ilya Maximets. > > Attach the tc filter information when running this test: > filter protocol ip pref 3 flower chain 0 > filter protocol ip pref 3 flower chain 0 handle 0x1 > dst_mac f0:00:00:01:01:02 > src_mac f0:00:00:01:01:01 > eth_type ipv4 > ip_proto udp > ip_flags nofrag > not_in_hw > action order 1: police 0x10000000 rate 0bit burst 0b mtu 64Kb > pkts_rate 1 pkts_burst 1 action drop/pipe overhead 0b linklayer unspec > ref 2 bind 1 installed 2 sec used 0 sec firstused 0 sec > Action statistics: > Sent 330 bytes 10 pkt (dropped 9, overlimits 9 requeues 0) > backlog 0b 0p requeues 0 > > action order 2: mirred (Egress Redirect to device ovs-p1) stolen > index 8 ref 1 bind 1 installed 1 sec used 0 sec firstused 0 sec > Action statistics: > Sent 33 bytes 1 pkt (dropped 0, overlimits 0 requeues 0) > backlog 0b 0p requeues 0 > cookie 8455fe4dcd4e3677d0ca43b42684d1a6 > no_percpu > > > Best regards, > Tianyu Yuan _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
