On 18 Oct 2022, at 18:11, Adrian Moreno wrote:
> On 10/14/22 20:07, Han Zhou wrote: >> >> >> On Thu, Oct 13, 2022 at 11:51 PM Adrian Moreno <amore...@redhat.com >> <mailto:amore...@redhat.com>> wrote: >>> >>> >>> >>> On 8/9/22 03:41, Han Zhou wrote: >>>> Signed-off-by: Han Zhou <hz...@ovn.org <mailto:hz...@ovn.org>> >>>> --- >>>> ofproto/ofproto-dpif-upcall.c | 13 ++++++++++++- >>>> 1 file changed, 12 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c >>>> index 23b52ef40..f26a035f4 100644 >>>> --- a/ofproto/ofproto-dpif-upcall.c >>>> +++ b/ofproto/ofproto-dpif-upcall.c >>>> @@ -58,6 +58,10 @@ COVERAGE_DEFINE(upcall_ukey_replace); >>>> COVERAGE_DEFINE(revalidate_missed_dp_flow); >>>> COVERAGE_DEFINE(upcall_flow_limit_hit); >>>> COVERAGE_DEFINE(upcall_flow_limit_kill); >>>> +COVERAGE_DEFINE(upcall_flow_del_rev); >>>> +COVERAGE_DEFINE(upcall_flow_del_no_rev); >>>> +COVERAGE_DEFINE(upcall_flow_del_idle_or_limit); >>>> +COVERAGE_DEFINE(upcall_flow_del_purge); >>>> >>>> /* A thread that reads upcalls from dpif, forwards each upcall's packet, >>>> * and possibly sets up a kernel flow as a cache. */ >>>> @@ -2334,7 +2338,12 @@ revalidate_ukey(struct udpif *udpif, struct >>>> udpif_key *ukey, >>>> } >>>> result = revalidate_ukey__(udpif, ukey, push.tcp_flags, >>>> odp_actions, recircs, >>>> ukey->xcache); >>>> - } /* else delete; too expensive to revalidate */ >>>> + if (result == UKEY_DELETE) { >>>> + COVERAGE_INC(upcall_flow_del_rev); >>>> + } >>>> + } else { /* else delete; too expensive to revalidate */ >>>> + COVERAGE_INC(upcall_flow_del_no_rev); >>>> + } >>>> } else if (!push.n_packets || ukey->xcache >>>> || !populate_xcache(udpif, ukey, push.tcp_flags)) { >>>> result = UKEY_KEEP; >>>> @@ -2771,6 +2780,7 @@ revalidate(struct revalidator *revalidator) >>>> } >>>> if (kill_them_all || (used && used < now - max_idle)) { >>>> result = UKEY_DELETE; >>>> + COVERAGE_INC(upcall_flow_del_idle_or_limit); >>>> } else { >>>> result = revalidate_ukey(udpif, ukey, &stats, >>>> &odp_actions, >>>> reval_seq, &recircs, >>>> @@ -2852,6 +2862,7 @@ revalidator_sweep__(struct revalidator *revalidator, >>>> bool purge) >>>> >>>> if (purge) { >>>> result = UKEY_DELETE; >>>> + COVERAGE_INC(upcall_flow_del_purge); >>>> } else if (!seq_mismatch) { >>>> result = UKEY_KEEP; >>>> } else { >>> >>> >>> This has some overlap with the work Kevin has been doing [1]. >>> >>> https://patchwork.ozlabs.org/project/openvswitch/patch/20221003200742.78047-1-ksprague0...@gmail.com/ >>> >>> <https://patchwork.ozlabs.org/project/openvswitch/patch/20221003200742.78047-1-ksprague0...@gmail.com/> >>> >>> Maybe this patch can be combined with Kevin's to provide unified visibility >>> (coverage counters + USDT probes) to the revalidation results? >>> >> Thanks Adrian for reviewing! This has been there for 2 months and I was >> about to send a reminder. >> And thanks for the pointer to the related patch. These two patches are for >> similar purposes, while they provide different ways to access the >> information, and I think both are useful. The coverage counters added here >> try to provide stats of basic DP flow deletion categories, while USDT probes >> may provide more details, so I am not sure whether these two need to match >> to each other exactly. But I agree that at least for the same del reasons >> these two patches should use the same names to avoid confusion. >> Do you have any detailed comments regarding the counters in this patch? >> > > I think the counters added by this patch are useful. I just think (as you > mention above) that the reason names should be unified with Kevin's work so > we have both USDT and counters reporting revalidator decisions. I was adding a lot of changes above on naming, but I think it would be better to maybe merge his patch with yours?! However, as his patch is waiting for a v7. Maybe you can take his code changes without the script, and add some counters @ the probe location. As I’m fine with his patch, except for the python script. Latest Kevin patch, https://patchwork.ozlabs.org/project/openvswitch/patch/20221021163543.508906-1-ksprague0...@gmail.com/ _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev