Send from my phone
> Op 17 mrt. 2023 om 21:11 heeft Marcelo Ricardo Leitner <mleit...@redhat.com>
> het volgende geschreven:
>
> On Thu, Mar 16, 2023 at 09:51:34AM +0100, Eelco Chaudron wrote:
>>
>>
>>> On 22 Dec 2022, at 13:32, Eelco Chaudron wrote:
>>>
>>>> On 22 Dec 2022, at 10:26, Balazs Nemeth wrote:
>>>
>>>> The only way that stats->{n_packets,n_bytes} would decrease is due to an
>>>> overflow, or if there are bugs in how statistics are handled. In the
>>>> past, there were multiple bugs that caused a jump backward. A
>>>> workaround was in place to set the statistics to 0 in that case. When
>>>> this happened while the revalidator was under heavy load, the workaround
>>>> had an unintended side effect where should_revalidate returned false
>>>> causing the flow to be removed because the metric it calculated was
>>>> based on a bogus value. Since many of those bugs have now been
>>>> identified and resolved, there is no need to set the statistics to 0. In
>>>> addition, the (unlikely) overflow still needs to be handled
>>>> appropriately.
>>>>
>>>> Signed-off-by: Balazs Nemeth <bnem...@redhat.com>
>>>> ---
>>>>
>>>> Depends on:
>>>> - netdev-offload-tc: Preserve tc statistics when flow gets modified.
>>>> - tc: Add support for TCA_STATS_PKT64
>>>> - ofproto-dpif-upcall: Wait for valid hw flow stats before applying
>>>> min-revalidate-pps
>>>> - ofproto-dpif-upcall: Use last known stats ukey stats on revalidate
>>>> missed dp flows.
>>>
>>> Did a lot of tests with this patch applied, but as mentioned it does need
>>> all four patches mentioned above applied first.
>>>
>>> Acked-by: Eelco Chaudron <echau...@redhat.com>
>>
>> Now that all the dependent patches have been applied and backported to 2.17,
>> can we have another look at this patch?
>
> Not sure what you mean here, Eelco. You mean, just a patch review or
> you mean something else, like evaluating if the patch is still needed
> at all?
Guess I was not clear enough :) what I meant was for the maintainers to have
another look at this patch and if no further objections apply it.
//Eelco
> Thanks,
> Marcelo
>
>>
>> Thanks,
>>
>> Eelco
>>
>>>> ofproto/ofproto-dpif-upcall.c | 8 ++------
>>>> 1 file changed, 2 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
>>>> index ad9635496..9117a1aef 100644
>>>> --- a/ofproto/ofproto-dpif-upcall.c
>>>> +++ b/ofproto/ofproto-dpif-upcall.c
>>>> @@ -2334,12 +2334,8 @@ revalidate_ukey(struct udpif *udpif, struct
>>>> udpif_key *ukey,
>>>>
>>>> push.used = stats->used;
>>>> push.tcp_flags = stats->tcp_flags;
>>>> - push.n_packets = (stats->n_packets > ukey->stats.n_packets
>>>> - ? stats->n_packets - ukey->stats.n_packets
>>>> - : 0);
>>>> - push.n_bytes = (stats->n_bytes > ukey->stats.n_bytes
>>>> - ? stats->n_bytes - ukey->stats.n_bytes
>>>> - : 0);
>>>> + push.n_packets = stats->n_packets - ukey->stats.n_packets;
>>>> + push.n_bytes = stats->n_bytes - ukey->stats.n_bytes;
>>>>
>>>> if (need_revalidate) {
>>>> if (should_revalidate(udpif, push.n_packets, ukey->stats.used)) {
>>>> --
>>>> 2.38.1
>>
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
