From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id B504FA0032; Mon, 11 Jul 2022 13:40:52 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9226940684; Mon, 11 Jul 2022 13:40:52 +0200 (CEST) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id A640A40223 for ; Mon, 11 Jul 2022 13:40:51 +0200 (CEST) X-MimeOLE: Produced By Microsoft Exchange V6.5 Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: [PATCH] doc: add deprecation for restrictions in telemetry naming Date: Mon, 11 Jul 2022 13:40:48 +0200 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D871BF@smartserver.smartshare.dk> In-Reply-To: X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH] doc: add deprecation for restrictions in telemetry naming Thread-Index: AdiVFIk5xGDmLGydSVWHuhAa0XyLhAAADAgg References: <20220707133931.752248-1-bruce.richardson@intel.com> <98CBD80474FA8B44BF855DF32C47DC35D871B2@smartserver.smartshare.dk> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Bruce Richardson" Cc: , , X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > Sent: Monday, 11 July 2022 12.54 >=20 > On Fri, Jul 08, 2022 at 12:06:31AM +0200, Morten Br=F8rup wrote: > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > > > Sent: Thursday, 7 July 2022 15.40 > > > > > > Following discussion on-list [1], we will look to limited the > allowed > > > characters in names for items in telemetry. This will simplify the > > > escaping needed for json output, or any future output formats. The > > > lists > > > will initially be minimal, since expansion to allow more = characters > can > > > be done without affecting compatibility, while reducing the set > cannot. > > > > > > Cc: mb@smartsharesystems.com > > > Cc: stephen@networkplumber.org > > > Cc: ciara.power@intel.com > > > > > > Signed-off-by: Bruce Richardson > > > > > > [1] http://inbox.dpdk.org/dev/20220623164245.561371-1- > > > bruce.richardson@intel.com/#r > > > --- > > > doc/guides/rel_notes/deprecation.rst | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/doc/guides/rel_notes/deprecation.rst > > > b/doc/guides/rel_notes/deprecation.rst > > > index 4e5b23c53d..9366690ec5 100644 > > > --- a/doc/guides/rel_notes/deprecation.rst > > > +++ b/doc/guides/rel_notes/deprecation.rst > > > @@ -119,6 +119,12 @@ Deprecation Notices > > > * metrics: The function ``rte_metrics_init`` will have a non-void > > > return > > > in order to notify errors instead of calling ``rte_exit``. > > > > > > +* telemetry: The allowed characters in names for dictionary = values > > > will be limited to > > > + alphanumeric characters and a small subset of additional > printable > > > characters. > > > + This will ensure that all dictionary parameter names can be > output > > > without escaping > > > + in json - or in any future output format used. Names for the > > > > json -> JSON > > > Capital idea! (pun very much intended :-) ) >=20 > > > telemetry commands will > > > + be similarly limited. > > > > Perhaps also add a comment about parameters to telemetry commands, > for completeness. > > I intentionally phrased this comment vaguely, to see what you had been = thinking about this. And it certainly had the desired effect. :-) >=20 > I was not intending to impose restrictions on the parameters = themselves > since currently they are not output as part of any json. However, now > you > have got me thinking that perhaps we should look to scan parameters = for > invalid characters before we hand them over to the individual > functions. > That would allow the possibility of including parameters in any = replies > in > a future format. >=20 > Was this what you had in mind, or any other thoughts? Not exactly what I had in mind... Your patch adds that "Names for the telemetry commands will be similarly = limited.". This is input, not output. So you need to describe what = restrictions are imposed on input. The input commands and format don't follow any structured standard; = command names, hierarchy and parameter names are individually chosen by = each developer, and parameters are just a bunch of param=3Dvalue with no = types or limits to the values. Also, the input is not JSON formatted, but - without looking deeply into = the telemetry library - I suppose it might be URL encoded, where e.g. = space is encoded as "%20" and '&' is encoded as "%26". I think we should just leave the input without restrictions. Changing it = would require a major overhaul to provide any significant improvement, = e.g. attaching types to the parameters, so their values are not just = BLOBs. I don't strongly oppose to limiting the input command names; but we = shouldn't impose any limit on what follows the command. So I'm proposing = to explicitly mention that we don't impose any input limits beyond the = command names. Or we could provide input restrictions and parsing/formatting in a = separate patch set.