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.
Thanks,
Han
> --
> Adrián Moreno
>
--
Adrián Moreno
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
