+CC Ciara Power, Telemetry lib maintainer > From: Bruce Richardson [mailto:bruce.richard...@intel.com] > Sent: Wednesday, 15 June 2022 18.55 > > On Wed, Jun 15, 2022 at 03:54:57PM +0200, Morten Brørup wrote: > > > From: Chengwen Feng [mailto:fengcheng...@huawei.com] > > > Sent: Wednesday, 15 June 2022 09.39 > > > > > > Use 'strict=False' 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 <fengcheng...@huawei.com> > > > --- > > > 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=True): > > > """ Read data from socket and return it in JSON format """ > > > reply = sock.recv(buf_len).decode() > > > try: > > > - ret = json.loads(reply) > > > + ret = json.loads(reply, strict=False) > > > 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. > > > > I would tend to agree. > > 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.
