netdev - Re: [PATCH iproute2] ip link: initial support for bareudp devicesy

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: <20200703031056.GA3079@martin-VirtualBox>
Date:   Fri, 3 Jul 2020 08:40:56 +0530
From:   Martin Varghese <martinvarghesenokia@...il.com>
To:     Guillaume Nault <gnault@...hat.com>
Cc:     Stephen Hemminger <stephen@...workplumber.org>,
        netdev@...r.kernel.org
Subject: Re: [PATCH iproute2] ip link: initial support for bareudp devicesy

On Thu, Jul 02, 2020 at 11:57:46AM +0200, Guillaume Nault wrote:
> On Thu, Jul 02, 2020 at 02:45:39PM +0530, Martin Varghese wrote:
> > On Wed, Jul 01, 2020 at 09:45:04PM +0200, Guillaume Nault wrote:
> > > +		} else if (matches(*argv, "ethertype") == 0)  {
> > > +			NEXT_ARG();
> > > +			check_duparg(&attrs, IFLA_BAREUDP_ETHERTYPE,
> > > +				     "ethertype", *argv);
> > > +			if (ll_proto_a2n(&ethertype, *argv))
> > Does this function takes care of mpls proto names ?
> > 
> > The original idea of bareudp is to allow even reserved ethertypes.Hence i think we
> > must take ethertype in hex aswell
> 
> ll_proto_a2n() handles both symbolic and numeric ethertypes.
>
Yes it works. 
> > > +				invarg("ethertype", *argv);
> > > +		} else if (matches(*argv, "srcportmin") == 0) {
> > > +			NEXT_ARG();
> > > +			check_duparg(&attrs, IFLA_BAREUDP_SRCPORT_MIN,
> > > +				     "srcportmin", *argv);
> > > +			if (get_u16(&srcportmin, *argv, 0))
> > > +				invarg("srcportmin", *argv);
> > > +		} else if (matches(*argv, "multiproto") == 0) {
> > > +			check_duparg(&attrs, IFLA_BAREUDP_MULTIPROTO_MODE,
> > > +				     *argv, *argv);
> > > +			multiproto = true;
> > > +		} else if (matches(*argv, "nomultiproto") == 0) {
> > do we need nomultiproto flag. Is it redundant
> 
> It allows users to exlicitely disable multiproto without having to rely
> on default values. Also nomultiproto appears in the detailed output, so
> it should be usable as input.
> 
ok
> > > +	if (tb[IFLA_BAREUDP_MULTIPROTO_MODE])
> > > +		print_bool(PRINT_ANY, "multiproto", "multiproto ", true);
> > > +	else
> > > +		print_bool(PRINT_ANY, "multiproto", "nomultiproto ", false);
> > Comments from stephen@...workplumber.org on the first version patch is given below
> > 
> > One of the unwritten rules of ip commands is that the show format
> > should match the command line arguments.  In this case extmode is
> > really a presence flag not a boolean. best to print that with
> > json null command.
> 
> The detailed output prints either "multiproto" or "nomultiproto". Both
> keywords are accepted as configuration input. I can't see any deviation
> from the unwritten rule here.
> 
ok