On 12/12/2022 10:06, Roi Dayan wrote:
>
>
> On 23/11/2022 13:19, Eelco Chaudron wrote:
>> If a tc flow was installed but has not yet been used, report it as such.
>>
>> In addition, add a delay to the "IGMP - flood under normal action" test
>> case to make it work with many repetitions. This delay is also present
>> in other ICMP/IGMP tests.
>>
>> Signed-off-by: Eelco Chaudron <[email protected]>
>> Acked-by: Roi Dayan <[email protected]>
>> ---
>> lib/tc.c | 8 +++++++-
>> tests/system-offloads.at | 3 +--
>> tests/system-traffic.at | 2 ++
>> 3 files changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/tc.c b/lib/tc.c
>> index a66dc432f..934df2e5e 100644
>> --- a/lib/tc.c
>> +++ b/lib/tc.c
>> @@ -1361,7 +1361,13 @@ get_user_hz(void)
>> static void
>> nl_parse_tcf(const struct tcf_t *tm, struct tc_flower *flower)
>> {
>> - uint64_t lastused = time_msec() - (tm->lastuse * 1000 / get_user_hz());
>> + uint64_t lastused;
>> +
>> + if (tm->firstuse == 0) {
>> + lastused = 0;
>> + } else {
>> + lastused = time_msec() - (tm->lastuse * 1000 / get_user_hz());
>> + }
>>
>> if (flower->lastused < lastused) {
>> flower->lastused = lastused;
>> diff --git a/tests/system-offloads.at b/tests/system-offloads.at
>> index 102e89a1f..9db68b2a0 100644
>> --- a/tests/system-offloads.at
>> +++ b/tests/system-offloads.at
>> @@ -76,8 +76,7 @@ conntrack - multiple zones, local
>> conntrack - multi-stage pipeline, local
>> conntrack - ICMP related with NAT
>> conntrack - DNAT load balancing
>> -conntrack - DNAT load balancing with NC
>> -IGMP - flood under normal action"
>> +conntrack - DNAT load balancing with NC"
>> echo "$ovs_test_skip_list" | sed "s/<SPC>/ /g"])
>>
>> m4_include([tests/system-traffic.at])
>> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
>> index 07913a192..57ff83b51 100644
>> --- a/tests/system-traffic.at
>> +++ b/tests/system-traffic.at
>> @@ -7002,6 +7002,8 @@ f0 00 00 01 01 01 08 00 46 c0 00 28 00 00 40 00 01 02
>> d3 49 45 65 eb 4a e0 dnl
>> 00 00 16 94 04 00 00 22 00 f9 02 00 00 00 01 04 00 00 00 e0 00 00 fb 00 00
>> dnl
>> 00 00 00 00 > /dev/null])
>>
>> +sleep 1
>> +
>> AT_CHECK([ovs-appctl dpctl/dump-flows --names | grep -e .*ipv4 | sort | dnl
>> strip_stats | strip_used | strip_recirc | dnl
>> sed 's/,packet_type(ns=[[0-9]]*,id=[[0-9]]*),/,/'],
>>
>
>
> Hi Eelco,
>
> I missed but we found an issue with this commit.
> hw offloaded rules dont get their firtuse attr being updated.
> This cause tc hw offloaded rules to always report as used:never.
> We added a specific check for this to catch this.
>
> This looks like a kernel bug that .stats_update op in tc actions
> should have a wrapper like tcf_lastuse_update() in .act op to
> update firstuse if 0.
or maybe to update the call to the offload driver to do it as the sw
can't tell and doing it in .stats_update won't be real. it will be
the first time dump stats was done.
> Will you send a fix for the kernel?
>
> Since we don't want to break working with current kernels I think
> we should avoid this patch for now and/or add a check to do this
> with newer kernels only.
>
> Thanks,
> Roi
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
