From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0975424E96 for ; Fri, 26 May 2023 19:42:28 +0000 (UTC) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CF307BD for ; Fri, 26 May 2023 12:42:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1685130145; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=in+eQSmTGhBovw9OL0eJDGkd0WCUkhom4FQ91Qm5HHs=; b=TchnAG/kDgebJ8XUXp98ni6jqhk8Y6WMjHIP4CsJa2Ghkf1UQu6nOqr8ZDFx6++UYpvMTF nWRMNQKEfmp8vhDTLDvroQ8/UJwkNr96mCXyBdY/C70EMI6OXyLM5H3JhJuNMQ6FJ3/+QB Vfi9W2iV9uyNSFjodI4Woc+p9xj9w+U= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-627-k6NReiDDOwivHog2Bqy0ZA-1; Fri, 26 May 2023 15:42:23 -0400 X-MC-Unique: k6NReiDDOwivHog2Bqy0ZA-1 Received: by mail-wr1-f69.google.com with SMTP id ffacd0b85a97d-3078b9943d6so452822f8f.1 for ; Fri, 26 May 2023 12:42:23 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1685130142; x=1687722142; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=in+eQSmTGhBovw9OL0eJDGkd0WCUkhom4FQ91Qm5HHs=; b=jpX/rC6VmC2E7oYK1UzplYn1GCApNKpyCG9eylMTxA8x60dU9+9JBZ6QCfuI/BruYd c2CwG7c4r01JSQ37GADZWEbo2Vvc8f5+L6dfD92tO5E58Fk1PDpsycAXhqtmYXVAeFX4 f0peeQ/QSpot/UDrYoFulob39fOS78GpG65Ti4lRKhRJoxQ4+kN7RnFfgiy8fjX54tF8 ZpN4BQ5bghuQfFY74EOxRCSbhCmRkv4/uZ7vybbzLACpSmpJKkGs5kaHyWoZ6yzY0dvx mzu719fNnW4tAIluatMMSWaPAVXCvFj521Xha/mgq1JQ++6H8E9X+5aW4ZDJ33JMpYsI I1sg== X-Gm-Message-State: AC+VfDxfAqBHmLNNlztJuXYf8hgqjo2gb8oLR+9y0N1rYiKntaYtk+1B O3/xyeclmQiRqasmY89QX0zGXv2DC4fzeqa2IxICMYEzY9+zhZaS/ZUvz2zi6W2jhSsb7sDCojv P5yxfQU50R+86QZhBkAgE+nLz X-Received: by 2002:adf:f702:0:b0:2ee:f77f:3d02 with SMTP id r2-20020adff702000000b002eef77f3d02mr2057035wrp.0.1685130142128; Fri, 26 May 2023 12:42:22 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ7TcJRcTqct9LMQgKq9jB8sgTpAUT0XkNEqX34l42h/p5UzE0Lw1rotysQIv2dW5oZoDo796A== X-Received: by 2002:adf:f702:0:b0:2ee:f77f:3d02 with SMTP id r2-20020adff702000000b002eef77f3d02mr2057026wrp.0.1685130141766; Fri, 26 May 2023 12:42:21 -0700 (PDT) Received: from localhost ([37.162.134.31]) by smtp.gmail.com with ESMTPSA id x4-20020a5d54c4000000b002c70ce264bfsm5972926wrv.76.2023.05.26.12.42.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 26 May 2023 12:42:21 -0700 (PDT) Date: Fri, 26 May 2023 21:42:17 +0200 From: Andrea Claudi To: Stephen Hemminger Cc: netdev@vger.kernel.org Subject: Re: [PATCH iproute2 v4 2/2] vxlan: make option printing more consistent Message-ID: References: <20230526174141.5972-1-stephen@networkplumber.org> <20230526174141.5972-3-stephen@networkplumber.org> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230526174141.5972-3-stephen@networkplumber.org> X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net On Fri, May 26, 2023 at 10:41:41AM -0700, Stephen Hemminger wrote: > Add new helper function print_bool_opt() which prints > with no prefix and use it for vxlan options. > > If the option matches the expected default value, > it is not printed if in non JSON mode unless the details > setting is repeated. > > Use a table for the vxlan options. This will change > the order of the printing of options. > > Signed-off-by: Stephen Hemminger > --- > include/json_print.h | 9 ++++ > ip/iplink_vxlan.c | 110 +++++++++++++------------------------------ > lib/json_print.c | 19 ++++++++ > 3 files changed, 60 insertions(+), 78 deletions(-) > > diff --git a/include/json_print.h b/include/json_print.h > index 91b34571ceb0..49d3cc14789c 100644 > --- a/include/json_print.h > +++ b/include/json_print.h > @@ -101,6 +101,15 @@ static inline int print_rate(bool use_iec, enum output_type t, > return print_color_rate(use_iec, t, COLOR_NONE, key, fmt, rate); > } > > +int print_color_bool_opt(enum output_type type, enum color_attr color, > + const char *key, bool value, bool show); > + > +static inline int print_bool_opt(enum output_type type, > + const char *key, bool value, bool show) > +{ > + return print_color_bool_opt(type, COLOR_NONE, key, value, show); > +} > + > /* A backdoor to the size formatter. Please use print_size() instead. */ > char *sprint_size(__u32 sz, char *buf); > > diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c > index cb6745c74507..e77c3aa2e3a5 100644 > --- a/ip/iplink_vxlan.c > +++ b/ip/iplink_vxlan.c > @@ -19,6 +19,25 @@ > > #define VXLAN_ATTRSET(attrs, type) (((attrs) & (1L << (type))) != 0) > > +static const struct vxlan_bool_opt { > + const char *key; > + int type; > + bool default_value; > +} vxlan_opts[] = { > + { "metadata", IFLA_VXLAN_COLLECT_METADATA, false }, Here you are changing the output from "external" to "metadata", while continuing to use "external" to toggle the option. This may surprise a user which checks the output for this option after enabling it. Moreover, looking at the man page for ip link, this option is used to specify "whether an external control plane (e.g. ip route encap) or the internal FDB should be used", so maybe "external" is more close to the true meaning of this option. I would suggest either to continue using "external" or to switch to "metadata" also for option toggling (and update the manpage, too). All the rest looks good to me. > + { "vnifilter", IFLA_VXLAN_VNIFILTER, false }, > + { "learning", IFLA_VXLAN_LEARNING, true }, > + { "proxy", IFLA_VXLAN_PROXY, false }, > + { "rsc", IFLA_VXLAN_RSC, false }, > + { "l2miss", IFLA_VXLAN_L2MISS, false }, > + { "l3miss", IFLA_VXLAN_L3MISS, false }, > + { "udp_csum", IFLA_VXLAN_UDP_CSUM, true }, > + { "udp_zero_csum6_tx", IFLA_VXLAN_UDP_ZERO_CSUM6_TX, false }, > + { "udp_zero_csum6_rx", IFLA_VXLAN_UDP_ZERO_CSUM6_RX, false }, > + { "remcsum_tx", IFLA_VXLAN_REMCSUM_TX, false }, > + { "remcsum_rx", IFLA_VXLAN_REMCSUM_RX, false }, > +}; > + > static void print_explain(FILE *f) > { > fprintf(f, > @@ -420,6 +439,7 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv, > > static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[]) > { > + unsigned int i; > __u8 ttl = 0; > __u8 tos = 0; > __u32 maxaddr; > @@ -427,16 +447,6 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[]) > if (!tb) > return; > > - if (tb[IFLA_VXLAN_COLLECT_METADATA] && > - rta_getattr_u8(tb[IFLA_VXLAN_COLLECT_METADATA])) { > - print_bool(PRINT_ANY, "external", "external ", true); > - } > - > - if (tb[IFLA_VXLAN_VNIFILTER] && > - rta_getattr_u8(tb[IFLA_VXLAN_VNIFILTER])) { > - print_bool(PRINT_ANY, "vnifilter", "vnifilter", true); > - } > - > if (tb[IFLA_VXLAN_ID] && > RTA_PAYLOAD(tb[IFLA_VXLAN_ID]) >= sizeof(__u32)) { > print_uint(PRINT_ANY, "id", "id %u ", rta_getattr_u32(tb[IFLA_VXLAN_ID])); > @@ -529,26 +539,6 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[]) > "dstport %u ", > rta_getattr_be16(tb[IFLA_VXLAN_PORT])); > > - if (tb[IFLA_VXLAN_LEARNING]) { > - __u8 learning = rta_getattr_u8(tb[IFLA_VXLAN_LEARNING]); > - > - print_bool(PRINT_JSON, "learning", NULL, learning); > - if (!learning) > - print_bool(PRINT_FP, NULL, "nolearning ", true); > - } > - > - if (tb[IFLA_VXLAN_PROXY] && rta_getattr_u8(tb[IFLA_VXLAN_PROXY])) > - print_bool(PRINT_ANY, "proxy", "proxy ", true); > - > - if (tb[IFLA_VXLAN_RSC] && rta_getattr_u8(tb[IFLA_VXLAN_RSC])) > - print_bool(PRINT_ANY, "rsc", "rsc ", true); > - > - if (tb[IFLA_VXLAN_L2MISS] && rta_getattr_u8(tb[IFLA_VXLAN_L2MISS])) > - print_bool(PRINT_ANY, "l2miss", "l2miss ", true); > - > - if (tb[IFLA_VXLAN_L3MISS] && rta_getattr_u8(tb[IFLA_VXLAN_L3MISS])) > - print_bool(PRINT_ANY, "l3miss", "l3miss ", true); > - > if (tb[IFLA_VXLAN_TOS]) > tos = rta_getattr_u8(tb[IFLA_VXLAN_TOS]); > if (tos) { > @@ -601,58 +591,22 @@ static void vxlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[]) > ((maxaddr = rta_getattr_u32(tb[IFLA_VXLAN_LIMIT])) != 0)) > print_uint(PRINT_ANY, "limit", "maxaddr %u ", maxaddr); > > - if (tb[IFLA_VXLAN_UDP_CSUM]) { > - __u8 udp_csum = rta_getattr_u8(tb[IFLA_VXLAN_UDP_CSUM]); > - > - if (is_json_context()) { > - print_bool(PRINT_ANY, "udp_csum", NULL, udp_csum); > - } else { > - if (!udp_csum) > - fputs("no", f); > - fputs("udpcsum ", f); > - } > - } > - > - if (tb[IFLA_VXLAN_UDP_ZERO_CSUM6_TX]) { > - __u8 csum6 = rta_getattr_u8(tb[IFLA_VXLAN_UDP_ZERO_CSUM6_TX]); > - > - if (is_json_context()) { > - print_bool(PRINT_ANY, > - "udp_zero_csum6_tx", NULL, csum6); > - } else { > - if (!csum6) > - fputs("no", f); > - fputs("udp6zerocsumtx ", f); > - } > - } > - > - if (tb[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]) { > - __u8 csum6 = rta_getattr_u8(tb[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]); > - > - if (is_json_context()) { > - print_bool(PRINT_ANY, > - "udp_zero_csum6_rx", > - NULL, > - csum6); > - } else { > - if (!csum6) > - fputs("no", f); > - fputs("udp6zerocsumrx ", f); > - } > - } > - > - if (tb[IFLA_VXLAN_REMCSUM_TX] && > - rta_getattr_u8(tb[IFLA_VXLAN_REMCSUM_TX])) > - print_bool(PRINT_ANY, "remcsum_tx", "remcsumtx ", true); > - > - if (tb[IFLA_VXLAN_REMCSUM_RX] && > - rta_getattr_u8(tb[IFLA_VXLAN_REMCSUM_RX])) > - print_bool(PRINT_ANY, "remcsum_rx", "remcsumrx ", true); > - > if (tb[IFLA_VXLAN_GBP]) > print_null(PRINT_ANY, "gbp", "gbp ", NULL); > if (tb[IFLA_VXLAN_GPE]) > print_null(PRINT_ANY, "gpe", "gpe ", NULL); > + > + for (i = 0; i < ARRAY_SIZE(vxlan_opts); i++) { > + const struct vxlan_bool_opt *opt = &vxlan_opts[i]; > + __u8 val; > + > + if (!tb[opt->type]) > + continue; > + val = rta_getattr_u8(tb[opt->type]); > + > + print_bool_opt(PRINT_ANY, opt->key, val, > + val != opt->default_value || show_details > 1); > + } > } > > static void vxlan_print_help(struct link_util *lu, int argc, char **argv, > diff --git a/lib/json_print.c b/lib/json_print.c > index d7ee76b10de8..602de027ca27 100644 > --- a/lib/json_print.c > +++ b/lib/json_print.c > @@ -215,6 +215,25 @@ int print_color_bool(enum output_type type, > value ? "true" : "false"); > } > > +/* In JSON mode, acts like print_color_bool. > + * Otherwise, will print key with prefix of "no" if false. > + * The show flag is used to suppres printing in non-JSON mode > + */ > +int print_color_bool_opt(enum output_type type, > + enum color_attr color, > + const char *key, > + bool value, bool show) > +{ > + int ret = 0; > + > + if (_IS_JSON_CONTEXT(type)) > + jsonw_bool_field(_jw, key, value); > + else if (_IS_FP_CONTEXT(type) && show) > + ret = color_fprintf(stdout, color, "%s%s ", > + value ? "" : "no", key); > + return ret; > +} > + > int print_color_on_off(enum output_type type, > enum color_attr color, > const char *key, > -- > 2.39.2 > >