Message-ID: <20190205145033.5d90dc42@hermes.lan>
Date:   Tue, 5 Feb 2019 14:50:33 -0800
From:   Stephen Hemminger <stephen@...workplumber.org>
To:     Roopa Prabhu <roopa@...ulusnetworks.com>
Cc:     Stefano Brivio <sbrivio@...hat.com>,
        Nikolay Aleksandrov <nikolay@...ulusnetworks.com>,
        David Ahern <dsahern@...il.com>, Phil Sutter <phil@....cc>,
        Eric Garver <egarver@...hat.com>,
        Tomas Dolezal <todoleza@...hat.com>,
        Lennert Buytenhek <buytenh@....org>,
        netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH iproute2-next] Introduce ip-brctl shell script

On Thu, 31 Jan 2019 08:28:29 -0800
Roopa Prabhu <roopa@...ulusnetworks.com> wrote:

> On Thu, Jan 31, 2019 at 4:46 AM Stefano Brivio <sbrivio@...hat.com> wrote:
> >
> > On Wed, 30 Jan 2019 14:30:59 -0800
> > Roopa Prabhu <roopa@...ulusnetworks.com> wrote:
> >  
> > > On Sun, Jan 27, 2019 at 11:57 PM Stefano Brivio <sbrivio@...hat.com> wrote:  
> > > >
> > > > 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  
> >
> > It's not just ioctl vs. netlink: brctl operates on bridges, whereas
> > ip-link and 'bridge' operate on generic links. If you look at
> > cmd_showstp() and cmd_show() from my script (I wouldn't recommend that
> > right after breakfast ;)) that should be apparent.
> >
> > If you want to get basic information about a bridge, with ip-link or
> > 'bridge' you need multiple commands. This is very different from
> > ifconfig: there's no such ifconfig command that can't be implemented as
> > a single ip-link command.
> >  
> > > > > 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 ? :).  
> >
> > Oh, I see your point here. Well, it's not deprecating anything because
> > I'm using the netlink tool in the wrapper (which allows it to be
> > rather dumb and simple), but sure, I understand what you mean.
> >
> > Still, we could very easily get the wrapper to print equivalent ip-link
> > and bridge commands before executing them -- and this would actually
> > ease and encourage migration, compared to the current situation.
> >  
> > > We are in opposite camps, I strongly want people to move to bridge and
> > > ip link which has all the latest support.  
> >
> > I wouldn't say we are in opposite camps, I just think that whatever has
> > been done so far to get people to switch hasn't worked.  
> 
> agreed. I think not much has been done to make output prettier for
> humans. The detailed output especially is very tricky to read.
> I think we should fix that.
> 
> >  
> > > > > 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.  
> >
> > Let's take 'brctl showstp br0'. I wouldn't even know where to start
> > adding options to 'bridge' to have a similar functionality (any idea?),
> > and still, that brctl command is very convenient.  
> 
> 
> I don't see a reason not to have 'bridge stp show'  (similar to bridge
> vlan show).
> Maybe there are better ways to represent the same thing.
> 
> 
> >  
> > > 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  
> >
> > Not in my experience: the output of brctl showmacs and showstp commands
> > is human-readable, that's what a large category of users need. JSON is
> > well suited for other purposes.  
> 
> For showmacs i can tell you, we have been able to move every one to
> 'bridge fdb show' because beyond certain point brctl showmacs is not
> very useful. bridge fdb show on the other hand covers vxlan etc.
> 
> >  
> > > > - 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.  
> >
> > This is unrelated. I'm specifically referring to the fdb information,
> > which is similar to what ip neighbour displays, and which you can
> > indeed get with brctl, see cmd_showmacs() from my script.  
> 
> I understood what you are saying. But there is no point moving fdb to
> neigh now because there is so much around it 'bridge fdb show, 'bridge
> monitor fdb' etc)
> 
> 
> >  
> > > > - '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.  
> >
> > *This* is exactly what looks confusing to me.
> >  
> > > 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.  
> >
> > I'm not particularly interested in non-human users here, not in the
> > context of this discussion at least.  
> 
> sure, i am with u for better human readable output.
> 
> >  
> > > > > 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).  
> >
> > Again, it's not the same as ifconfig, for the reasons I mentioned
> > above. And by the way I think ifconfig doesn't even vaguely have the
> > same amount of users of brctl nowadays, and for a reason, but I would
> > only have anecdotal experience to support this.
> >
> > --
> > Stefano  

my $.02 mostly agrees with Roopa.
The code for bridge was written as part of the "lets make bridge code
use netlink" and tried as much as possible to look like other tools.

Providing brctl or ifconfig scripts is possible, but it really should
be put in a sample or demo directory and not installed by default.
It would just cause too much pain to distributions.

I love concise human readable output and hate long winded VMS style
commands.


Remember iproute has always had output formats that match input commands.
This was even required by some VPN tools in the past.

Powered by blists - more mailing lists