Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-upcall.c: Add counters for reasons of DP flow deletions.

2023-01-17 Thread Eelco Chaudron 


On 17 Jan 2023, at 4:11, Han Zhou wrote:

> On Mon, Jan 16, 2023 at 7:36 AM Eelco Chaudron  wrote:
>>
>>
>>
>> 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  > wrote:
>
>
>
> On 8/9/22 03:41, Han Zhou wrote:
>> Signed-off-by: Han Zhou 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/
>>
>>
> Thanks Eelco. To make it simpler, I'd wait for Kevin's patch to be merged
> and I can rebase this one on top of it. I have provide

Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-upcall.c: Add counters for reasons of DP flow deletions.

2023-01-16 Thread Han Zhou 
On Mon, Jan 16, 2023 at 7:36 AM Eelco Chaudron  wrote:
>
>
>
> 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 mailto:amore...@redhat.com>> wrote:
> >>>
> >>>
> >>>
> >>> On 8/9/22 03:41, Han Zhou wrote:
>  Signed-off-by: Han Zhou 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/
>
>
Thanks Eelco. To make it simpler, I'd wait for Kevin's patch to be merged
and I can rebase this one on top of it. I have provided some minor comments
on Kevin's patch, too. Please take a look.

Regards,
Han
_

Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-upcall.c: Add counters for reasons of DP flow deletions.

2023-01-16 Thread Eelco Chaudron 


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 > > wrote:
>>>
>>>
>>>
>>> On 8/9/22 03:41, Han Zhou wrote:
 Signed-off-by: Han Zhou 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/
>>>  
>>> 
>>>
>>> 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

Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-upcall.c: Add counters for reasons of DP flow deletions.

2022-10-18 Thread Adrian Moreno 



On 10/14/22 20:07, Han Zhou wrote:



On Thu, Oct 13, 2022 at 11:51 PM Adrian Moreno > wrote:

 >
 >
 >
 > On 8/9/22 03:41, Han Zhou wrote:
 > > Signed-off-by: Han Zhou 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/ 

 >
 > 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.



Thanks,
Han

 > --
 > Adrián Moreno
 >


--
Adrián Moreno

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-upcall.c: Add counters for reasons of DP flow deletions.

2022-10-14 Thread Han Zhou 
On Thu, Oct 13, 2022 at 11:51 PM Adrian Moreno  wrote:
>
>
>
> On 8/9/22 03:41, Han Zhou wrote:
> > Signed-off-by: Han Zhou 
> > ---
> >   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/
>
> 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?

Thanks,
Han

> --
> Adrián Moreno
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/2] ofproto-dpif-upcall.c: Add counters for reasons of DP flow deletions.

2022-10-13 Thread Adrian Moreno 



On 8/9/22 03:41, Han Zhou wrote:

Signed-off-by: Han Zhou 
---
  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/

Maybe this patch can be combined with Kevin's to provide unified visibility 
(coverage counters + USDT probes) to the revalidation results?


--
Adrián Moreno

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev