On 15 Sep 2022, at 14:03, Ilya Maximets wrote:
> On 9/15/22 13:56, Ilya Maximets wrote: >> 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 previous patch, after parsing police action, the flower stats will > be updated by dumped meter table stats" > > I suppose, that is the root cause. We should not mix these two > completely different types of statistics. I didn't look at the > code though to say why this was implemented in a way it is or > how to fix that. Tianyu I might have missed this, but is there a follow on fix for this? Asking as the test in the current master is broken. Ilya/Simon should we undo “dd9881ed5 tc: Fix stats dump when using same meter table” and get a proper fix, fixing these counter issues? >>> 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
