> -----Original Message-----
> From: Amber, Kumar <kumar.am...@intel.com>
> Sent: Thursday, July 1, 2021 10:50 AM
> To: Van Haaren, Harry <harry.van.haa...@intel.com>
> Cc: d...@openvswitch.org; i.maxim...@ovn.org; Flavio Leitner
> <f...@sysclose.org>
> Subject: RE: [ovs-dev] [PATCH] dpif/dpcls: limit count subtable search info
> logs
>
> Hi Harry,
>
> Any Insights on the following 😊
>
> > -----Original Message-----
> > From: Flavio Leitner <f...@sysclose.org>
> > Sent: Wednesday, June 30, 2021 12:28 AM
> > To: Amber, Kumar <kumar.am...@intel.com>
> > Cc: d...@openvswitch.org; i.maxim...@ovn.org
> > Subject: Re: [ovs-dev] [PATCH] dpif/dpcls: limit count subtable search info
> > logs
> >
> > On Tue, Jun 29, 2021 at 10:19:41PM +0530, Kumar Amber wrote:
> > > From: Harry van Haaren <harry.van.haa...@intel.com>
> > >
> > > This commit avoids many instances of "using subtable X for miniflow (x,y)"
> > > in the ovs-vswitchd log when using the DPCLS Autovalidator. This
> > > occurs when no specialized subtable is found, and the generic "_any"
> > > version of the avx512 subtable search implementation was used. This
> > > change logs the subtable usage once, avoiding duplicates.
> > >
> > > Signed-off-by: Harry van Haaren <harry.van.haa...@intel.com>
> > > ---
> > > lib/dpif-netdev-lookup-avx512-gather.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/lib/dpif-netdev-lookup-avx512-gather.c
> > > b/lib/dpif-netdev-lookup-avx512-gather.c
> > > index bc359dc4a..f1b44deb3 100644
> > > --- a/lib/dpif-netdev-lookup-avx512-gather.c
> > > +++ b/lib/dpif-netdev-lookup-avx512-gather.c
> > > @@ -411,7 +411,7 @@ dpcls_subtable_avx512_gather_probe(uint32_t
> > u0_bits, uint32_t u1_bits)
> > > */
> > > if (!f && (u0_bits + u1_bits) < (NUM_U64_IN_ZMM_REG * 2)) {
> > > f = dpcls_avx512_gather_mf_any;
> > > - VLOG_INFO("Using avx512_gather_mf_any for subtable (%d,%d)\n",
> > > + VLOG_INFO_ONCE("Using avx512_gather_mf_any for subtable
> > > + (%d,%d)\n",
> > > u0_bits, u1_bits);
> >
> > This will log only one time, but there are multiple subtables, so we won't
> > see
> > other subtable changing. If the subtable information is not relevant, then
> > it
> > shouldn't be in the msg.
Note that this only logs the subtables that are not specialized.
Basically, this if this log is seen in the wild, it tell us OVS community that
the
subtable fingerprint (u0_bits, u1_bits) combo should be specialized.
> > Also, the log only exists for *_mf_any, not for others specialized
> > functions.
Yes, intentionally. If the accelerated path is being chosen, why nag the user.
If we're missing a specialized subtable, it would be good for OVS community to
know,
so we log it, but only log it once. We could consider a rate-limited log
instead of
a "ONCE".
Note that DPCLS Autovalidator calls probe() for each subtable-lookup, as it
wants
to test all the different variants. ONCE avoids a lot of noise in this case,
but I'm OK
with a _RL version too.
> > Do we need that information in runtime? Unless I am missing other callers,
> > dpcls_subtable_get_best_impl() has a VLOG_DBG() logging all cases with the
> > same information.
Yes, for debugging it is useful to see for any subtable, which is being
selected.
The above prints are INFO level, so OVS community can see what subtables
could be specialized in future for better performance in those subtable lookups.
So the one open is to use VLOG with ONCE (as this patch does) or with a
RateLimit,
I'm OK with either approach.
Regards, -Harry
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
