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 10D1EA00BE; Wed, 15 Jun 2022 20:01:21 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 01DDB427EB; Wed, 15 Jun 2022 20:01:21 +0200 (CEST) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id AADFB410D0 for ; Wed, 15 Jun 2022 20:01:18 +0200 (CEST) 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 1/5] usertools: use non-strict when json-loads in telemetry X-MimeOLE: Produced By Microsoft Exchange V6.5 Date: Wed, 15 Jun 2022 20:01:15 +0200 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D87128@smartserver.smartshare.dk> In-Reply-To: X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH 1/5] usertools: use non-strict when json-loads in telemetry Thread-Index: AdiA2MNooZl1WFEUSuWo0JroMdNuiwABj+Gw References: <20220615073915.14041-1-fengchengwen@huawei.com> <20220615073915.14041-2-fengchengwen@huawei.com> <98CBD80474FA8B44BF855DF32C47DC35D87126@smartserver.smartshare.dk> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Bruce Richardson" , "Ciara Power" , "Chengwen Feng" 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 +CC Ciara Power, Telemetry lib maintainer > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > Sent: Wednesday, 15 June 2022 18.55 >=20 > On Wed, Jun 15, 2022 at 03:54:57PM +0200, Morten Br=F8rup wrote: > > > From: Chengwen Feng [mailto:fengchengwen@huawei.com] > > > Sent: Wednesday, 15 June 2022 09.39 > > > > > > Use 'strict=3DFalse' in json-loads, it will ignore control = characters > > > (e.g. '\n\t'), this patch is prepared for the support of telemetry > dump > > > in the future. > > > > > > Signed-off-by: Chengwen Feng > > > --- > > > usertools/dpdk-telemetry.py | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/usertools/dpdk-telemetry.py b/usertools/dpdk- > telemetry.py > > > index a81868a547..63f8004566 100755 > > > --- a/usertools/dpdk-telemetry.py > > > +++ b/usertools/dpdk-telemetry.py > > > @@ -27,7 +27,7 @@ def read_socket(sock, buf_len, echo=3DTrue): > > > """ Read data from socket and return it in JSON format """ > > > reply =3D sock.recv(buf_len).decode() > > > try: > > > - ret =3D json.loads(reply) > > > + ret =3D json.loads(reply, strict=3DFalse) > > > except json.JSONDecodeError: > > > print("Error in reply: ", reply) > > > sock.close() > > > -- > > > 2.33.0 > > > > > > > As I understand this patch, it accepts non-JSON data from the > telemetry socket. > > > > Isn't it is a major protocol violation if the telemetry socket > produces output requiring this modification? Doing that would break > other applications expecting strictly JSON formatted output from the > telemetry socket. > > >=20 > I would tend to agree. >=20 > As an alternative, I think you should add to the telemetry library an > "escape string" function which can then be used by the telemetry > functions > to properly json encode the strings back from the dump functions = before > sending them out. Instead of adding a separate JSON encode function, the = rte_tel_data_string() and rte_tel_data_add_array_string() functions = should simply JSON encode the provided strings if required. Their = descriptions do not mention any requirements to the strings provided, = and being control plane functions, I would certainly expect them to JSON = encode. Warning: Although I consider such a change a bugfix, others might = consider it an ABI breakage. :-( @Ciara, what is your opinion about my suggestion here? For minimal changes, RTE_TEL_MAX_STRING_LEN and = RTE_TEL_MAX_SINGLE_STRING_LEN should keep their meaning, i.e. define the = maximum length of the string after any JSON encoding. And optionally, new rte_tel_data_[array_]string_raw() performance = optimized functions could be provided for strings known not to require = any encoding.