From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jakub Kicinski Subject: Re: [PATCH iproute2-next] iplink: add support for reporting multiple XDP programs Date: Fri, 13 Jul 2018 15:59:59 -0700 Message-ID: <20180713155959.70361b7e@cakuba.lan> References: <20180713204359.1161-1-jakub.kicinski@netronome.com> <20180713135941.3791c3ff@xeon-e3> <20180713142037.503b7b01@cakuba.lan> <20180713152330.7a2ec1dc@xeon-e3> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: alexei.starovoitov@gmail.com, daniel@iogearbox.net, dsahern@gmail.com, netdev@vger.kernel.org, oss-drivers@netronome.com To: Stephen Hemminger Return-path: Received: from mail-qt0-f196.google.com ([209.85.216.196]:39966 "EHLO mail-qt0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727241AbeGMXQo (ORCPT ); Fri, 13 Jul 2018 19:16:44 -0400 Received: by mail-qt0-f196.google.com with SMTP id h4-v6so28515899qtj.7 for ; Fri, 13 Jul 2018 16:00:03 -0700 (PDT) In-Reply-To: <20180713152330.7a2ec1dc@xeon-e3> Sender: netdev-owner@vger.kernel.org List-ID: On Fri, 13 Jul 2018 15:23:30 -0700, Stephen Hemminger wrote: > > > I prefer to not use "printf(fp," and use print_string(PRINT_FP, NULL, "%s", ...) > > > because otherwise you end up mixing strings and json format output in the > > > same result. > > > > > > You should be able to do > > > tc -j ... > > > and always get valid JSON output. > > > > > > One quick way to test json validation is to pipe it into python: > > > tc -j ... | python -mjson.tool > > > > Note that XDP has separate print functions for plain text and JSON, and > > the flow gets separated early on: > > > > mode = rta_getattr_u8(tb[IFLA_XDP_ATTACHED]); > > if (mode == XDP_ATTACHED_NONE) > > return; > > else if (is_json_context()) > > return details ? (void)0 : xdp_dump_json(tb); > > > > ... non-JSON handling follows... > > > > The use of fprintfs is therefore okay. Do you have a preference for > > using the wrapper, even if fprintf is safe? It's brevity vs > > consistency, I guess. We'd need a separate patch for that, 'cause I'm > > not touching all the fprintfs in the file, anyway. > > The only preference for the wrapper is that it is easy way to make > sure all code is JSON aware. Yes... > Since fp is always stdout in current code, maybe just convert to printf. ...or maybe we could consider adding a wrapper for printf that wouldn't take all the unnecessary parameters print_string() takes, yet clearly indicate autor knows about JSON output concerns?