On 22 Jun 2023, at 13:29, Ilya Maximets wrote:
> On 6/22/23 11:30, Eelco Chaudron wrote:
>>
>>
>> On 22 Jun 2023, at 11:15, Ilya Maximets wrote:
>>
>>> On 6/22/23 10:50, Eelco Chaudron wrote:
>>>>
>>>>
>>>> On 30 May 2023, at 9:32, Eelco Chaudron wrote:
>>>>
>>>>> On 26 May 2023, at 22:51, Ilya Maximets wrote:
>>>>>
>>>>>> On 5/26/23 15:09, Eelco Chaudron wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 26 May 2023, at 14:03, 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 issues 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. If an unexpected jump does happen, just log it as a
>>>>>>>> warning.
>>>>>>>>
>>>>>>>> Signed-off-by: Balazs Nemeth <[email protected]>
>>>>>>>
>>>>>>> Thanks for making the final change! Here is an example of the log
>>>>>>> message for others reviewing:
>>>>>>>
>>>>>>> 2023-05-26T12:55:07.590Z|00003|ofproto_dpif_upcall(revalidator14)|WARN|Unexpected
>>>>>>> jump in packet stats from 0 to 1 when handling ukey
>>>>>>> ufid:90c082b0-7aa6-442a-9b86-04d10fe9ede6
>>>>>>> recirc_id(0),dp_hash(0),skb_priority(0),in_port(2),skb_mark(0),ct_state(0),ct_zone(0)
>>>>>>> ,ct_mark(0),ct_label(0),eth(src=6e:48:c8:77:d3:8c,dst=33:33:00:00:00:16),eth_type(0x86dd),ipv6(src=::,dst=ff02::16,label=0,proto=58,tclass=0,hlimit=1,frag=no),key32(40
>>>>>>> 00),icmpv6(type=143,code=0), actions:1,3
>>>>>>>
>>>>>>> One nit on a missing “,” compared to the dp flow dump, but I think Ilya
>>>>>>> can add this on commit.
>>>>>>>
>>>>>>> Acked-by: Eelco Chaudron <[email protected]>
>>>>>>
>>>>>> Thanks! I added the comma and applied the patch.
>>>>>
>>>>> Are you planning on back porting this, as all other relevant patches
>>>>> where?
>>>>
>>>> Ilya did you maybe miss this email? Question still applies ;)
>>>
>>> Yeah, sorry, it fell through the cracks.
>>>
>>> I'm a bit worried about backporting this one, because the
>>> original reason for the stats jump is still not clear and
>>> so might still exist. I guess, we could backport to 3.1,
>>> but I'd like not to backport to LTS.
>>>
>>> What do you think?
>>
>> I’m happy with getting this in 3.1 ;)
>
> OK. Backported to 3.1 now.
>
> Best regards, Ilya Maximets.
Thanks!!
//Eelco
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
