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

[Martin Varghese]

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 step...@networkplumber.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