On 16 Sep 2022, at 15:11, Ilya Maximets wrote:
> On 9/16/22 11:25, Eelco Chaudron wrote:
>>
>>
>> On 16 Sep 2022, at 4:25, Tianyu Yuan wrote:
>>
>>> On 9/16/22 10:14, 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 <tianyu.y...@corigine.com>
>>>>>>>
>>>>>>> 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 <tianyu.y...@corigine.com>
>>>>>>> Signed-off-by: Simon Horman <simon.hor...@corigine.com>
>>>>>>> ---
>>>>>>> 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.
>>>>
>>>
>>> Thanks Ilya, I previously had some misunderstanding of flow stats.
>>>
>>> Now the problem is, once police action(meter) is the first action in a
>>> filter,
>>> flow stats will ignore the police action and only be updated using the
>>> survived
>>> packets, which means the flow stats either before or after the commit
>>> dd9881ed55e6
>>> is incorrect.
>>>
>>> We will keep looking into this issue.
>>
>>
>> I while back before doing chk_pkt_len I suggested a patch, but some told me
>> not all actions are implemented, it might be worth looking into this again
>> if needed:
>>
>> https://www.mail-archive.com/ovs-dev@openvswitch.org/msg62960.html
>
> For the rte_flow case, the similar issue is solved by always adding
> an RTE_FLOW_ACTION_TYPE_COUNT action at the beginning of the action
> list. Is there some similar action in TC?
TC does not have a specific count action. It’s part of each action, however
some action type always return 0 (I was told).
>>
>> I can’t remember how I fixed this for chk_pkt_len, as it does work there, as
>> there are some specific test cases for it :)
>>
>>
>> /ovs-master/tests/system-offloads-traffic.at
>>
>> # This test verifies the total packet counters work when individual branches
>> # are taken.
>>
>>
>> //Eelco
>>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
