Message-ID: <CAJieiUir5WVaQmWF3d46mo1J4+C_RdVmD=iAButhuEFsd7001A@mail.gmail.com>
Date:   Wed, 30 Jan 2019 14:30:59 -0800
From:   Roopa Prabhu <roopa@...ulusnetworks.com>
To:     Stefano Brivio <sbrivio@...hat.com>
Cc:     Nikolay Aleksandrov <nikolay@...ulusnetworks.com>,
        David Ahern <dsahern@...il.com>, Phil Sutter <phil@....cc>,
        Eric Garver <egarver@...hat.com>,
        Tomas Dolezal <todoleza@...hat.com>,
        Stephen Hemminger <stephen@...workplumber.org>,
        Lennert Buytenhek <buytenh@....org>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH iproute2-next] Introduce ip-brctl shell script

On Sun, Jan 27, 2019 at 11:57 PM Stefano Brivio <sbrivio@...hat.com> wrote:
>
> On Sun, 27 Jan 2019 21:08:13 -0800
> Roopa Prabhu <roopa@...ulusnetworks.com> wrote:
>
> > On Fri, Jan 25, 2019 at 2:05 AM Stefano Brivio <sbrivio@...hat.com> wrote:
> > >
> > > Hi Roopa,
> > >
> > > On Wed, 23 Jan 2019 08:33:27 -0800
> > > Roopa Prabhu <roopa@...ulusnetworks.com> wrote:
> > >
> > > > On Wed, Jan 23, 2019 at 7:09 AM Nikolay Aleksandrov
> > > > <nikolay@...ulusnetworks.com> wrote:
> > > >
> > > > > Hi,
> > > > > IMO the effort should be towards improving iproute2 to be
> > > > > easier to use and more intuitive. We should be pushing people to
> > > > > use the new tools instead of trying to find workarounds to keep the
> > > > > old tools alive. I do like to idea of deprecating bridge-utils, but
> > > > > I think it should be done via improving ip/bridge enough to be
> > > > > pleasant to use. We will have to maintain this compatibility layer
> > > > > forever if it gets accepted and we'll never get rid of brctl this
> > > > > way.
> > > >
> > > > +1, we should move people away from brtcl. there is enough confusion
> > > > among users looking at bridge attributes.,
> > > >
> > > > ip -d link show
> > > > bridge -d link show
> > > > brctl
> > >
> > > Why is this confusing? One can simply pick the most appropriate tool.
> > >
> > > > Adding a 4th one  to the list is not going to ease the confusion.
> > >
> > > Why do you say I'm adding a fourth (I guess) tool? I'm replacing the
> > > third one.
> >
> > I know. But the first two commands were supposed to replace the third
> > one already.
> > and they should be.
>
> They can't replace brctl not because they are badly designed or
> unusable, but simply because they are different tools with different
> purposes (see also my comments to Nikolay).

I don't think i understand that they are different tools. The new netlink tools
are supposed to deprecate the old tools that use ioctls. this is the same reason
we don't have a ip-ifconfig today


>
> > So, I think its better to fix the first two instead of introducing
> > another one.
>
> This is really not the same thing: I'm not introducing a new tool, I'm
> effectively replacing a 1794-LoC, non-trivial, ioctl-based
> implementation with a trivial, 572-lines shell script, with half
> the binary size.

you are replacing a ioctl-based tool from another package into
iproute2..and maybe there-by deprecating the netlink based tool ? :).
We are in opposite camps, I strongly want people to move to bridge and
ip link which has all the latest support.

>
> I'm not doing this on bridge-utils directly because that would imply
> the need to still maintain a different tool. For all practical
> maintenance purposes, I'm actually getting rid of a separate tool,
> which is my only goal here.
>
> I'd rather say we go from 3 tools to slightly more than 2.
>
> > > > We should try to make the 'ip -d link show and bridge -d link show'
> > > > outputs better. Any suggestions there from people will be useful.
> > >
> > > To be honest, I don't see any problem with them -- they just do
> > > different things.
> >
> > Can we extend 'bridge' tool with extra options to provide a summary
> > view of all bridges like brctl ?
>
> We could, and I initially thought of that approach instead, but that
> has a number of fundamental downsides:
>
> - we can't provide a brctl-compatible syntax, unless we want to
>   substantially rewrite the 'bridge' interface, and I think it's a
>   bad idea to break 'bridge' syntax for users, while we won't be able to
>   replace brctl if we don't provide a similar syntax, history showed

I am certainly not suggesting we break existing bridge users. I am
talking about new options.

I understand some people are finding it hard to move away from brctl
output, but in my experience,
these are also the people who want new things in brctl like json
output etc. which is already available in the bridge command



>
> - the fdb implementation has a long-dated comment by Stephen in its
>   header,
>         * TODO: merge/replace this with ip neighbour
>   and this is actually the only part of 'bridge' I'm using in ip-brctl.
>   Code is conceptually duplicated there, and I think we should actually
>   get rid of that -- but then 'bridge' wouldn't even give information
>   about the FDB, one would need to use ip neighbour instead.

This could be comment from initial days. Today bridge has support for
fdb, vlans and vlan tunnels which you
cannot get from brctl and any brctl compat tool.


>
> - 'bridge' doesn't implement settings for basic bridge features (say,
>   STP), which are convenient for users, especially if they are used to
>   brctl. To get that, even at an interface/syntax level, we would need
>   to duplicate some parts of ip-link, which looks like a bad idea per
>   se.

thats fine IMO. Today ip link set extended bridge attribute support is
only for convenience.
You can set most attributes both from ip link set and bridge link
command. We can see if they can share code.

You can set a vlan on a bridge today via the bridge command. I dont
see why we should hesitate about STP here.
And you will get the json output for free.


>
> > Its supposed to be the netlink based tool for all bridging and hence
> > could be a good replacement for all brctl users.
>
> I still think the best replacement for users is the one that changes
> absolutely nothing, and if that's easily achievable, I'd rather go for
> it.

That would also mean we add ip-ifconfig and ip-ethtool (if we
deprecate ethtool tomorrow. i am not saying its going away....,
but just giving you an example of ioctl to netlink based tools).

Powered by blists - more mailing lists