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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87ttw35qe5.fsf@nvidia.com>
Date: Tue, 23 May 2023 11:31:45 +0200
From: Petr Machata <petrm@...dia.com>
To: Stephen Hemminger <stephen@...workplumber.org>
CC: Ido Schimmel <idosch@...sch.org>, Vladimir Nikishkin
	<vladimir@...ishkin.pw>, <dsahern@...il.com>, <netdev@...r.kernel.org>,
	<davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
	<pabeni@...hat.com>, <eng.alaamohamedsoliman.am@...il.com>,
	<gnault@...hat.com>, <razor@...ckwall.org>, <idosch@...dia.com>,
	<liuhangbin@...il.com>, <eyal.birger@...il.com>, <jtoppins@...hat.com>
Subject: Re: [PATCH iproute2-next v5] ip-link: add support for nolocalbypass
 in vxlan


Stephen Hemminger <stephen@...workplumber.org> writes:

> On Mon, 22 May 2023 09:15:34 +0300
> Ido Schimmel <idosch@...sch.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@...sch.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.

I think Ido means printing something in both true and false cases, not
using literally the words true and false. That would be a divergence,
yes.

> That leaves #3 as the correct and best output.

The attribute should IMHO be present in JSON output always, even when
the value is false. JSON is for programmatic consumption, and always
having the value available makes the code less error prone. You can hard
expect the value to be there, and just test what it is, instead of
assuming that absence means false, and possibly silently consuming the
wrong object, or interpreting based on an old version of iproute2.