Re: [PATCH iproute2 1/2] tc: u32: add support for json output
From: Davide Caratti <hidden>
Date: 2021-09-07 15:46:23
On Mon, Sep 06, 2021 at 09:57:50PM -0400, Wen Liang wrote:
Currently u32 filter output does not support json. This commit uses proper json functions to add support for it. Signed-off-by: Wen Liang <redacted>
hello Wen, this series does not break TDC selftests for the u32 classifier (well, on net-next those tests are temporarily broken because it still misses [1]. However, the fix should propagate soon - and I verified that the tests keep passing after applying [1] and your series).
quoted hunk ↗ jump to hunk
--- tc/f_u32.c | 66 ++++++++++++++++++++++++++---------------------------- 1 file changed, 32 insertions(+), 34 deletions(-)diff --git a/tc/f_u32.c b/tc/f_u32.c index a5747f67..136fb740 100644 --- a/tc/f_u32.c +++ b/tc/f_u32.c@@ -1213,11 +1213,11 @@ static int u32_print_opt(struct filter_util *qu, FILE *f, struct rtattr *opt, if (handle) { SPRINT_BUF(b1); - fprintf(f, "fh %s ", sprint_u32_handle(handle, b1)); + print_string(PRINT_ANY, "fh", "fh %s ", sprint_u32_handle(handle, b1)); } if (TC_U32_NODE(handle)) - fprintf(f, "order %d ", TC_U32_NODE(handle)); + print_int(PRINT_ANY, "order", "order %d ", TC_U32_NODE(handle)); if (tb[TCA_U32_SEL]) { if (RTA_PAYLOAD(tb[TCA_U32_SEL]) < sizeof(*sel))@@ -1227,15 +1227,13 @@ static int u32_print_opt(struct filter_util *qu, FILE *f, struct rtattr *opt, } if (tb[TCA_U32_DIVISOR]) { - fprintf(f, "ht divisor %d ", - rta_getattr_u32(tb[TCA_U32_DIVISOR])); + print_int(PRINT_ANY, "ht divisor", "ht divisor %d ", rta_getattr_u32(tb[TCA_U32_DIVISOR]));
the JSON specification [2] does not forbid spaces in names (if they are enclosed in double quotes, like it happens in the the iproute2 case), however I think that "ht_divisor" should sound better than "ht divisor" in the JSON name. Look at this example (done on my fedora that uses fq_codel): $ tc -j qdisc show | jq '.[1].options.memory limit' jq: error: syntax error, unexpected IDENT, expecting $end (Unix shell quoting issues?) at <top-level>, line 1: .[1].options.memory limit jq: 1 compile error $ tc -j qdisc show | jq '.[1].options.memory_limit' 33554432 $ tc -j -j qdisc show | jq '.[1].options."memory limit"' $ Since it's a "memory_limit", and not a "memory limit", I can forget about quotes and my jq is happy either ways. Do you think it's worth respinning your patches with those names converted to avoid whitespaces in names? the same applies for other elements in your series. (Sorry if this comment might sound nit-picking, but the iproute2 output is known to be used thoroughly in scripts. So, maybe it's better to do a robust design) thanks! -- davide [1] https://lore.kernel.org/netdev/20210804091828.3783-1-phil@nwl.cc/ (local) [2] https://www.ecma-international.org/wp-content/uploads/ECMA-404_2nd_edition_december_2017.pdf