On Wed, Oct 19, 2022 at 12:53 PM Bruce Richardson
<bruce.richard...@intel.com> wrote:
>
> On Tue, Oct 18, 2022 at 02:14:37PM +0100, Bruce Richardson wrote:
> > On Thu, Oct 13, 2022 at 09:49:28AM +0200, David Marchand wrote:
> > > Register telemetry commands to list and configure trace points and later
> > > save traces for a running DPDK application.
> > >
> > > Note: trace point names contain a '.', so the list of valid characters
> > > used in telemetry commands and dictionary keys has been extended.
> > >
> <snip>
>
> > > diff --git a/lib/telemetry/telemetry_data.c
> > > b/lib/telemetry/telemetry_data.c
> > > index 34366ecee3..5b319c18fb 100644
> > > --- a/lib/telemetry/telemetry_data.c
> > > +++ b/lib/telemetry/telemetry_data.c
> > > @@ -106,6 +106,7 @@ valid_name(const char *name)
> > > ['a' ... 'z'] = 1,
> > > ['_'] = 1,
> > > ['/'] = 1,
> > > + ['.'] = 1,
> > > };
> > > while (*name != '\0') {
> > > if ((size_t)*name >= RTE_DIM(allowed) || allowed[(int)*name]
> > > == 0)
> >
> > I don't see an issue with allowing "." characters in dictionary names, so
> > for this part:
> >
> > Acked-by: Bruce Richardson <bruce.richard...@intel.com>
>
> By way of additional minor follow-up:
> * I think the addition of "." to the list of allowed characters should be a
> separate patch, rather than just added as part of this larger patch. If
> doing a separate commit to add boolean type, this could be a change in that
> set.
> * There are a couple of API doxygen comments for the dictionary functions,
> rte_tel_add_dict_*, where the allowed characters in the name are called
> out. You probably should add "." to those comments too.
Oh indeed, so ok let's go with a separate patch.
Previously, I thought the telemetry additions for traces were fine to
go with the fixes series.
But seeing how it evolved, I'll merge the traces fixes (who got acked)
now and respin a separate series for the rest.
--
David Marchand
