netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: Ido Schimmel <idosch@idosch.org>
Cc: Vladimir Nikishkin <vladimir@nikishkin.pw>,
	dsahern@gmail.com, netdev@vger.kernel.org, davem@davemloft.net,
	edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
	eng.alaamohamedsoliman.am@gmail.com, gnault@redhat.com,
	razor@blackwall.org, idosch@nvidia.com, liuhangbin@gmail.com,
	eyal.birger@gmail.com, jtoppins@redhat.com
Subject: Re: [PATCH iproute2-next v5] ip-link: add support for nolocalbypass in vxlan
Date: Mon, 22 May 2023 08:32:16 -0700	[thread overview]
Message-ID: <20230522083216.09cc8fd7@hermes.local> (raw)
In-Reply-To: <ZGsIhkGT4RBUTS+F@shredder>

On Mon, 22 May 2023 09:15:34 +0300
Ido Schimmel <idosch@idosch.org> wrote:

> On Sun, May 21, 2023 at 12:47:41PM -0700, Stephen Hemminger wrote:
> > On Sun, 21 May 2023 22:23:25 +0300
> > Ido Schimmel <idosch@idosch.org> wrote:
> >   
> > > +       if (tb[IFLA_VXLAN_LOCALBYPASS])
> > > +               print_bool(PRINT_ANY, "localbypass", "localbypass ",
> > > +                          rta_getattr_u8(tb[IFLA_VXLAN_LOCALBYPASS]))  
> > 
> > That will not work for non json case.  It will print localbypass whether it is set or not.
> > The third argument is a format string used in the print routine.  
> 
> Yea, replied too late...
> 
> Anyway, my main problem is with the JSON output. Looking at other
> boolean VXLAN options, we have at least 3 different formats:
> 
> 1. Only print when "true" for both JSON and non-JSON output. Used for
> "external", "vnifilter", "proxy", "rsc", "l2miss", "l3miss",
> "remcsum_tx", "remcsum_rx".
> 
> 2. Print when both "true" and "false" for both JSON and non-JSON output.
> Used for "udp_csum", "udp_zero_csum6_tx", "udp_zero_csum6_rx".
> 
> 3. Print JSON when both "true" and "false" and non-JSON only when
> "false". Used for "learning".
> 
> I don't think we should be adding another format. We need to decide:
> 
> 1. What is the canonical format going forward?
> 
> 2. Do we change the format of existing options?
> 
> My preference is:
> 
> 1. Format 2. Can be implemented in a common helper used for all VXLAN
> options.
> 
> 2. Yes. It makes all the boolean options consistent and avoids future
> discussions such as this where a random option is used for a new option.

A fourth option is to us print_null(). The term null is confusing and people
seem to avoid it.  But it is often used by python programmers as way to represent
options. That would be my preferred option but others seem to disagree.

Option #2 is no good. Any printing of true/false in non-JSON output is a diveregence
from the most common practice across iproute2.

That leaves #3 as the correct and best output.

FYI - The iproute2 maintainers are David Ahern and me. The kernel bits have
other subsystem maintainers.

  reply	other threads:[~2023-05-22 15:32 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-21  5:49 [PATCH iproute2-next v5] ip-link: add support for nolocalbypass in vxlan Vladimir Nikishkin
2023-05-21 19:23 ` Ido Schimmel
2023-05-21 19:47   ` Stephen Hemminger
2023-05-22  6:15     ` Ido Schimmel
2023-05-22 15:32       ` Stephen Hemminger [this message]
2023-05-23  9:31         ` Petr Machata
2023-05-23  9:39         ` Andrea Claudi
2023-05-23  9:52           ` Vladimir Nikishkin
2023-05-23 10:32             ` Andrea Claudi
2023-05-22  6:03   ` Vladimir Nikishkin
2023-05-22  6:22     ` Ido Schimmel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230522083216.09cc8fd7@hermes.local \
    --to=stephen@networkplumber.org \
    --cc=davem@davemloft.net \
    --cc=dsahern@gmail.com \
    --cc=edumazet@google.com \
    --cc=eng.alaamohamedsoliman.am@gmail.com \
    --cc=eyal.birger@gmail.com \
    --cc=gnault@redhat.com \
    --cc=idosch@idosch.org \
    --cc=idosch@nvidia.com \
    --cc=jtoppins@redhat.com \
    --cc=kuba@kernel.org \
    --cc=liuhangbin@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=razor@blackwall.org \
    --cc=vladimir@nikishkin.pw \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).