On Thu, Oct 13, 2022 at 11:51 PM Adrian Moreno <amore...@redhat.com> wrote:
>
>
>
> On 8/9/22 03:41, Han Zhou wrote:
> > Signed-off-by: Han Zhou <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?
Thanks,
Han
> --
> Adrián Moreno
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
