Re: [PATCH iproute2-next v6] ip-link: add support for nolocalbypass in vxlan

[Andrea Claudi <aclaudi@redhat.com>]

On Tue, May 23, 2023 at 09:04:41AM -0700, Stephen Hemminger wrote:
> On Tue, 23 May 2023 12:48:05 +0800
> Vladimir Nikishkin <vladimir@nikishkin.pw> wrote:
> 
> > +	if (tb[IFLA_VXLAN_LOCALBYPASS]) {
> > +		__u8 localbypass = rta_getattr_u8(tb[IFLA_VXLAN_LOCALBYPASS]);
> > +
> > +		print_bool(PRINT_JSON, "localbypass", NULL, localbypass);
> > +		if (!localbypass)
> > +			print_bool(PRINT_FP, NULL, "nolocalbypass ", true);
> > +	}
> 
> This is backwards since nolocalbypass is the default.
>

Stephen, I'll try to summarize the discussion we had in v5 here.

- We agree that it's a good idea to have JSON attributes printed both
  when 'true' and 'false'. As Petr said, this makes the code less error
  prone and makes it clear attribute is supported.
- I have some concerns about printing options only when non-default
  values are set. Non-JSON output is mostly consumed by humans, that
  usually expects something to be visible if present/true/enabled. I
  know I'm advocating for a change in the iproute output here, and we
  usually don't do that, but I argue there's value in having a less
  cluttered and confusing output.

  For example, let's take what you see with a default vxlan:
  $ ip link add type vxlan id 12
  $ ip -j link show vxlan0
  [...] udpcsum noudp6zerocsumtx noudp6zerocsumrx [...]

  IMHO printing only "udpcsum" is enough to make the user aware that
  the "udpcsum" feature is enabled and the rest is off.

I'm not against Vladimir's change, of course. But I would be very happy
if we can agree on a direction for the output from now on, and try to
enforce it, maybe deprecating the "old way" to print out stuff step by
step, if we find it useful.

What do you think?
Andrea