| Message ID | ed6b04eab48a70d6416a6b021f04f9901f7e9f01.1547830302.git.sbrivio@redhat.com |
|---|---|
| State | Rejected |
| Delegated to: | David Ahern |
| Headers | show |
| Series | [iproute2-next] Introduce ip-brctl shell script | expand |
On 18/01/2019 19:00, Stefano Brivio wrote: > This script wraps 'ip' and 'bridge' tools to provide a drop-in replacement > of the standalone 'brctl' utility. > > It's bug-to-bug compatible with brctl as of bridge-utils version 1.6, > has no dependencies other than a POSIX shell, and it's less than half > the binary size of brctl on x86_64. > > As many users (including myself) seem to find brctl usage vastly more > intuitive than ip-link, possibly due to habit, this might be a lightweight > approach to provide brctl syntax without the need to maintain bridge-utils > any longer. > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com> > Acked-by: Phil Sutter <phil@nwl.cc> > --- > man/man8/Makefile | 5 +- > man/man8/ip-brctl.8 | 187 +++++++++++++++ > misc/Makefile | 9 +- > misc/ip-brctl.in | 572 ++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 770 insertions(+), 3 deletions(-) > create mode 100644 man/man8/ip-brctl.8 > create mode 100755 misc/ip-brctl.in 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. Thanks, Nik
On Wed, Jan 23, 2019 at 7:09 AM Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote: > > On 18/01/2019 19:00, Stefano Brivio wrote: > > This script wraps 'ip' and 'bridge' tools to provide a drop-in replacement > > of the standalone 'brctl' utility. > > > > It's bug-to-bug compatible with brctl as of bridge-utils version 1.6, > > has no dependencies other than a POSIX shell, and it's less than half > > the binary size of brctl on x86_64. > > > > As many users (including myself) seem to find brctl usage vastly more > > intuitive than ip-link, possibly due to habit, this might be a lightweight > > approach to provide brctl syntax without the need to maintain bridge-utils > > any longer. > > > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com> > > Acked-by: Phil Sutter <phil@nwl.cc> > > --- > > man/man8/Makefile | 5 +- > > man/man8/ip-brctl.8 | 187 +++++++++++++++ > > misc/Makefile | 9 +- > > misc/ip-brctl.in | 572 ++++++++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 770 insertions(+), 3 deletions(-) > > create mode 100644 man/man8/ip-brctl.8 > > create mode 100755 misc/ip-brctl.in > > 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 Adding a 4th one to the list is not going to ease the confusion. 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.
Hi Nik, On Wed, 23 Jan 2019 17:09:42 +0200 Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote: > 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. Indeed, it's not my intent here to push anybody to do anything. However, if you think there's some value in familiarising users with ip-link, we could, very easily with this script, print (perhaps on standard error?) the equivalent ip-link commands for any brctl command issued by the user. It's a couple of lines on top of this patch, because I'm already doing exactly that -- calculating equivalent ip-link commands. Something like: # brctl stp br0 on You might want to: "ip link set br0 type bridge stp_state 1" What do you think? > 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. My observation is that brctl is simply a different tool, not as generic as ip-link, and hence I find it acceptable and understandable that users (just as I do, I'll admit) feel more comfortable with it for some specific tasks. It's not a matter of syntax, ip-link is device-oriented and brctl is bridge-oriented. If you want to show a list of bridges and basic information about enslaved ports, 'brctl show' will do this for you. With ip-link, you'll need to iterate over devices, and list ports and information for each of them, while getting a significant amount of unwanted information in the process. However, getting ip-link to do something different would make it a different tool. > We will have to maintain this compatibility layer forever if > it gets accepted and we'll never get rid of brctl this way. I see this a bit differently: we're not getting rid of bridge-utils simply because it makes little sense to do so. It's been several years now that ip-link is able to access and set all the information and states brctl uses, but this didn't make brctl obsolete, in practice. However, getting rid of bridge-utils means bridge-utils doesn't need to be maintained, and I guess that's the reason you're advocating that. This is the very reason behind this script: it's smaller and simpler than bridge-utils, and I think we can reasonably assume it's going to need almost no maintenance, being a rather dumb implementation.
Hi Roopa, On Wed, 23 Jan 2019 08:33:27 -0800 Roopa Prabhu <roopa@cumulusnetworks.com> wrote: > On Wed, Jan 23, 2019 at 7:09 AM Nikolay Aleksandrov > <nikolay@cumulusnetworks.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. > 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.
On Fri, Jan 25, 2019 at 2:05 AM Stefano Brivio <sbrivio@redhat.com> wrote: > > Hi Roopa, > > On Wed, 23 Jan 2019 08:33:27 -0800 > Roopa Prabhu <roopa@cumulusnetworks.com> wrote: > > > On Wed, Jan 23, 2019 at 7:09 AM Nikolay Aleksandrov > > <nikolay@cumulusnetworks.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. So, I think its better to fix the first two instead of introducing another one. > > > 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 ? Its supposed to be the netlink based tool for all bridging and hence could be a good replacement for all brctl users.
On Sun, 27 Jan 2019 21:08:13 -0800 Roopa Prabhu <roopa@cumulusnetworks.com> wrote: > On Fri, Jan 25, 2019 at 2:05 AM Stefano Brivio <sbrivio@redhat.com> wrote: > > > > Hi Roopa, > > > > On Wed, 23 Jan 2019 08:33:27 -0800 > > Roopa Prabhu <roopa@cumulusnetworks.com> wrote: > > > > > On Wed, Jan 23, 2019 at 7:09 AM Nikolay Aleksandrov > > > <nikolay@cumulusnetworks.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). > 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. 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 - 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. - '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. > 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.
On 1/18/19 10:00 AM, Stefano Brivio wrote: > This script wraps 'ip' and 'bridge' tools to provide a drop-in replacement > of the standalone 'brctl' utility. > > It's bug-to-bug compatible with brctl as of bridge-utils version 1.6, > has no dependencies other than a POSIX shell, and it's less than half > the binary size of brctl on x86_64. > > As many users (including myself) seem to find brctl usage vastly more > intuitive than ip-link, possibly due to habit, this might be a lightweight > approach to provide brctl syntax without the need to maintain bridge-utils > any longer. > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com> > Acked-by: Phil Sutter <phil@nwl.cc> > --- > man/man8/Makefile | 5 +- > man/man8/ip-brctl.8 | 187 +++++++++++++++ > misc/Makefile | 9 +- > misc/ip-brctl.in | 572 ++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 770 insertions(+), 3 deletions(-) > create mode 100644 man/man8/ip-brctl.8 > create mode 100755 misc/ip-brctl.in I get your intent, but this seems more appropriate for you / Red Hat to carry than something we want to distribute as part of iproute2.
Hi David, On Tue, 29 Jan 2019 21:51:24 -0700 David Ahern <dsahern@gmail.com> wrote: > On 1/18/19 10:00 AM, Stefano Brivio wrote: > > This script wraps 'ip' and 'bridge' tools to provide a drop-in replacement > > of the standalone 'brctl' utility. > > > > It's bug-to-bug compatible with brctl as of bridge-utils version 1.6, > > has no dependencies other than a POSIX shell, and it's less than half > > the binary size of brctl on x86_64. > > > > As many users (including myself) seem to find brctl usage vastly more > > intuitive than ip-link, possibly due to habit, this might be a lightweight > > approach to provide brctl syntax without the need to maintain bridge-utils > > any longer. > > > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com> > > Acked-by: Phil Sutter <phil@nwl.cc> > > --- > > man/man8/Makefile | 5 +- > > man/man8/ip-brctl.8 | 187 +++++++++++++++ > > misc/Makefile | 9 +- > > misc/ip-brctl.in | 572 ++++++++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 770 insertions(+), 3 deletions(-) > > create mode 100644 man/man8/ip-brctl.8 > > create mode 100755 misc/ip-brctl.in > > I get your intent, but this seems more appropriate for you / Red Hat to > carry than something we want to distribute as part of iproute2. Sure, I could also do that, but: - me creating another project: similar maintenance burden for distribution maintainers as keeping bridge-utils around, for something that won't have any active development - carrying it in a single distribution downstream: I would have gone that way if I thought it wouldn't be useful for others. I myself use (also) distributions other than Fedora/RHEL and this would feel just... wrong Why do you think it's not appropriate to distribute this as part of iproute2? Too ugly? Bloated? Anything I can improve? I think it would be appropriate because it intimately depends on ip-link -- it's really nothing more than a helper for iproute2 tools.
On Sun, Jan 27, 2019 at 11:57 PM Stefano Brivio <sbrivio@redhat.com> wrote: > > On Sun, 27 Jan 2019 21:08:13 -0800 > Roopa Prabhu <roopa@cumulusnetworks.com> wrote: > > > On Fri, Jan 25, 2019 at 2:05 AM Stefano Brivio <sbrivio@redhat.com> wrote: > > > > > > Hi Roopa, > > > > > > On Wed, 23 Jan 2019 08:33:27 -0800 > > > Roopa Prabhu <roopa@cumulusnetworks.com> wrote: > > > > > > > On Wed, Jan 23, 2019 at 7:09 AM Nikolay Aleksandrov > > > > <nikolay@cumulusnetworks.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).
On 1/30/19 3:55 AM, Stefano Brivio wrote: >> I get your intent, but this seems more appropriate for you / Red Hat to >> carry than something we want to distribute as part of iproute2. > > Sure, I could also do that, but: > > - me creating another project: similar maintenance burden for > distribution maintainers as keeping bridge-utils around, > for something that won't have any active development > > - carrying it in a single distribution downstream: I would have gone > that way if I thought it wouldn't be useful for others. I myself use > (also) distributions other than Fedora/RHEL and this would feel > just... wrong > > Why do you think it's not appropriate to distribute this as part of > iproute2? Too ugly? Bloated? Anything I can improve? > > I think it would be appropriate because it intimately depends on > ip-link -- it's really nothing more than a helper for iproute2 tools. > Again, I understand your point ... I still, too often, type ifconfig from long in-grained muscle memory. This is a convenience wrapper around commands packaged in iproute2. If iproute2 adds this wrapper, it will have to carry it and maintain it forever. Distributions (Fedora, RHEL, Debian, etc) may see it differently and decide to add this patch onto iproute2 that they distribute as a means for dropping bridge-utils. That's a reasonable migration choice. It is just not something upstream iproute2 should carry.
On Wed, 30 Jan 2019 14:30:59 -0800 Roopa Prabhu <roopa@cumulusnetworks.com> wrote: > On Sun, Jan 27, 2019 at 11:57 PM Stefano Brivio <sbrivio@redhat.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. > > > 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 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. > > - 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. > > - '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. > > > 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.
On Wed, 30 Jan 2019 22:12:45 -0700 David Ahern <dsahern@gmail.com> wrote: > This is a convenience wrapper around commands packaged in iproute2. If > iproute2 adds this wrapper, it will have to carry it and maintain it > forever. Distributions (Fedora, RHEL, Debian, etc) may see it > differently and decide to add this patch onto iproute2 that they > distribute as a means for dropping bridge-utils. That's a reasonable > migration choice. It is just not something upstream iproute2 should carry. I see what you mean now, I didn't think of that. Sure, that also sounds reasonable. I still think this wrapper would require basically zero maintenance, and carrying it would outweigh the burden at large -- and we could also use it as a tool to get users familiar with ip-link and 'bridge' by printing the equivalent syntax before executing the commands. Anyway, I'll inform downstream maintainers about this option.
Hi, For your information: I recently submitted a patch that implements brctl as a wrapper using ip-link and 'bridge' from iproute2, that allows to drop bridge-utils while still providing brctl functionality: https://patchwork.ozlabs.org/patch/1027627/ This wasn't exactly met with enthusiasm upstream (full discussion at: https://marc.info/?l=linux-netdev&m=154783087917432), but feel free to carry it downstream if you're interested. Thanks,
On Thu, Jan 31, 2019 at 4:46 AM Stefano Brivio <sbrivio@redhat.com> wrote: > > On Wed, 30 Jan 2019 14:30:59 -0800 > Roopa Prabhu <roopa@cumulusnetworks.com> wrote: > > > On Sun, Jan 27, 2019 at 11:57 PM Stefano Brivio <sbrivio@redhat.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
On Thu, 31 Jan 2019 08:28:29 -0800 Roopa Prabhu <roopa@cumulusnetworks.com> wrote: > On Thu, Jan 31, 2019 at 4:46 AM Stefano Brivio <sbrivio@redhat.com> wrote: > > > > On Wed, 30 Jan 2019 14:30:59 -0800 > > Roopa Prabhu <roopa@cumulusnetworks.com> wrote: > > > > > On Sun, Jan 27, 2019 at 11:57 PM Stefano Brivio <sbrivio@redhat.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.
On Tue, 5 Feb 2019 14:50:33 -0800 Stephen Hemminger <stephen@networkplumber.org> wrote: > 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. On one hand, I did it this way exactly to make it easy for distributions (after all, I work for a distributor). The rationale is that it's easier to get rid of things from package scripts rather than add them. Also implementing a Debian package diversion looked more elegant this way. No idea about other ones though. On the other hand, I didn't even think of adding that to examples/. Maybe that would alleviate David's concern about having to maintain it forever (if it breaks temporarily, or if we need to remove it for some reason, it's not a drama). > I love concise human readable output and hate long winded VMS style > commands. That's exactly where I feel the current tools included in iproute2 fall rather short. Compare "brctl show" to the closest equivalent "IFS=' ' for b in $(ip -br link show type bridge); do ip -br link show type bridge_slave master ${b%% *}; done". Sure, as Roopa said, we could and should improve 'bridge', and by now I'm even almost convinced it's doable without breaking the existing syntax, but it's not something we're doing overnight.
diff --git a/man/man8/Makefile b/man/man8/Makefile index 932ba1f3c488..26d2370a9cbe 100644 --- a/man/man8/Makefile +++ b/man/man8/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 -TARGETS = ip-address.8 ip-link.8 ip-route.8 +TARGETS = ip-address.8 ip-link.8 ip-route.8 brctl.8 MAN8PAGES = $(TARGETS) $(filter-out $(TARGETS),$(wildcard *.8)) @@ -14,6 +14,9 @@ ip-link.8: ip-link.8.in ip-route.8: ip-route.8.in sed "s|@SYSCONFDIR@|$(CONFDIR)|g" $< > $@ +brctl.8: ip-brctl.8 + echo '.so man8/ip-brctl.8' >$@ + distclean: clean clean: diff --git a/man/man8/ip-brctl.8 b/man/man8/ip-brctl.8 new file mode 100644 index 000000000000..63c1bcdebe9f --- /dev/null +++ b/man/man8/ip-brctl.8 @@ -0,0 +1,187 @@ +.\" +.\" This program is free software; you can redistribute it and/or modify +.\" it under the terms of the GNU General Public License as published by +.\" the Free Software Foundation; either version 2 of the License, or +.\" (at your option) any later version. +.\" +.\" This program is distributed in the hope that it will be useful, +.\" but WITHOUT ANY WARRANTY; without even the implied warranty of +.\" MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +.\" GNU General Public License for more details. +.\" +.\" You should have received a copy of the GNU General Public License +.\" along with this program; if not, write to the Free Software +.\" Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. +.\" +.\" +.TH IP-BRCTL 8 "January 18, 2019" "" "" +.SH NAME +ip-brctl \- ethernet bridge administration +.SH SYNOPSIS +.BR "brctl [command]" +.SH DESCRIPTION +.B ip-brctl +is a reimplementation of the traditional +.B brctl +utility in shell making use of +.BR ip " and " bridge +tools internally. It is supposed to behave identically to +.BR brctl , +hence the remainder of this document will use that name instead of +.BR ip-brctl . + +.B brctl +is used to set up, maintain, and inspect the ethernet bridge +configuration in the Linux kernel. + +An ethernet bridge is a device commonly used to connect different +networks of ethernets together, so that these ethernets will appear as +one ethernet to the participants. + +Each of the ethernets being connected corresponds to one physical +interface in the bridge. These individual ethernets are bundled into +one bigger ('logical') ethernet, this bigger ethernet corresponds to +the bridge network interface. + + +.SH INSTANCES +The command +.B brctl addbr <name> +creates a new instance of the ethernet bridge. The network interface +corresponding to the bridge will be called <name>. + +The command +.B brctl delbr <name> +deletes the instance <name> of the ethernet bridge. The network +interface corresponding to the bridge must be down before it can be +deleted! + +The command +.B brctl show +shows all current instances of the ethernet bridge. + + +.SH PORTS +Each bridge has a number of ports attached to it. Network traffic +coming in on any of these ports will be forwarded to the other ports +transparently, so that the bridge is invisible to the rest of the +network (i.e. it will not show up in +.IR traceroute(8) +). + +The command +.B brctl addif <brname> <ifname> +will make the interface <ifname> a port of the bridge <brname>. This +means that all frames received on <ifname> will be processed as if +destined for the bridge. Also, when sending frames on <brname>, +<ifname> will be considered as a potential output interface. + +The command +.B brctl delif <brname> <ifname> +will detach the interface <ifname> from the bridge <brname>. + +The command +.B brctl show <brname> +will show some information on the bridge and its attached ports. + + +.SH AGEING +The bridge keeps track of ethernet addresses seen on each port. When +it needs to forward a frame, and it happens to know on which port the +destination ethernet address (specified in the frame) is located, it +can 'cheat' by forwarding the frame to that port only, thus saving a +lot of redundant copies and transmits. + +However, the ethernet address location data is not static +data. Machines can move to other ports, network cards can be replaced +(which changes the machine's ethernet address), etc. + +.B brctl showmacs <brname> +shows a list of learned MAC addresses for this bridge. + +.B brctl setageing <brname> <time> +sets the ethernet (MAC) address ageing time, in seconds. After <time> +seconds of not having seen a frame coming from a certain address, the +bridge will time out (delete) that address from the Forwarding +DataBase (fdb). + +.B brctl setgcint <brname> <time> +sets the garbage collection interval for the bridge <brname> to <time> +seconds. This means that the bridge will check the forwarding database +for timed out entries every <time> seconds. + + +.SH SPANNING TREE PROTOCOL +Multiple ethernet bridges can work together to create even larger +networks of ethernets using the IEEE 802.1d spanning tree +protocol. This protocol is used for finding the shortest path between +two ethernets, and for eliminating loops from the topology. As this +protocol is a standard, Linux bridges will interwork properly with +other third party bridge products. Bridges communicate with each other +by sending and receiving BPDUs (Bridge Protocol Data Units). These +BPDUs can be recognised by an ethernet destination address of +01:80:c2:00:00:00. + +The spanning tree protocol can also be turned off (for those +situations where it just doesn't make sense, for example when this +Linux box is the only bridge on the LAN, or when you know that there +are no loops in the topology.) + +.IR brctl(8) +can be used for configuring certain spanning tree protocol +parameters. For an explanation of these parameters, see the IEEE +802.1d specification (or send me an email). The default values should +be just fine. If you don't know what these parameters mean, you +probably won't feel the desire to tweak them. + +.B brctl stp <bridge> <state> +controls this bridge instance's participation in the spanning tree +protocol. If <state> is "on" or "yes" the STP will be turned on, +otherwise it will be turned off. When turned off, the bridge will not +send or receive BPDUs, and will thus not participate in the spanning +tree protocol. If your bridge isn't the only bridge on the LAN, or if +there are loops in the LAN's topology, DO NOT turn this option off. If +you turn this option off, please know what you are doing. + + +.B brctl setbridgeprio <bridge> <priority> +sets the bridge's priority to <priority>. The priority value is an +unsigned 16-bit quantity (a number between 0 and 65535), and has no +dimension. Lower priority values are 'better'. The bridge with the +lowest priority will be elected 'root bridge'. + +.B brctl setfd <bridge> <time> +sets the bridge's 'bridge forward delay' to <time> seconds. + +.B brctl sethello <bridge> <time> +sets the bridge's 'bridge hello time' to <time> seconds. + +.B brctl setmaxage <bridge> <time> +sets the bridge's 'maximum message age' to <time> seconds. + +.B brctl setpathcost <bridge> <port> <cost> +sets the port cost of the port <port> to <cost>. This is a +dimensionless metric. + +.B brctl setportprio <bridge> <port> <priority> +sets the port <port>'s priority to <priority>. The priority value is +an unsigned 8-bit quantity (a number between 0 and 255), and has no +dimension. This metric is used in the designated port and root port +selection algorithms. + +.SH NOTES +.BR brctl(8) +is obsolete. Some features such as STP guard, harpin mode, fastleave and root +block are intentionally not implemented in this command. +Instead use +.B bridge +command from +.B iproute2 +package for a more full set of features. + +.SH SEE ALSO +.BR iptables(8) + +.SH AUTHOR +Lennert Buytenhek <buytenh@gnu.org> +Stephen Hemminger <stephen@networkplumber.org> diff --git a/misc/Makefile b/misc/Makefile index 6a849af4be22..79c6be31b874 100644 --- a/misc/Makefile +++ b/misc/Makefile @@ -3,6 +3,7 @@ SSOBJ=ss.o ssfilter.o LNSTATOBJ=lnstat.o lnstat_util.o TARGETS=ss nstat ifstat rtacct lnstat +SCRIPTS=ip-brctl include ../config.mk @@ -10,7 +11,7 @@ ifeq ($(HAVE_BERKELEY_DB),y) TARGETS += arpd endif -all: $(TARGETS) +all: $(TARGETS) $(SCRIPTS) ss: $(SSOBJ) $(QUIET_LINK)$(CC) $^ $(LDFLAGS) $(LDLIBS) -o $@ @@ -33,10 +34,14 @@ ssfilter.c: ssfilter.y lnstat: $(LNSTATOBJ) $(QUIET_LINK)$(CC) $^ $(LDFLAGS) $(LDLIBS) -o $@ +ip-brctl: ip-brctl.in + @sed -e "s|^\(IP=\"\).*\"|\1$(DESTDIR)$(SBINDIR)\/ip\"|" -e "s|^\(BRIDGE=\"\).*\"|\1$(DESTDIR)$(SBINDIR)\/bridge\"|" $< > $@ + install: all - install -m 0755 $(TARGETS) $(DESTDIR)$(SBINDIR) + install -m 0755 $(TARGETS) $(SCRIPTS) $(DESTDIR)$(SBINDIR) ln -sf lnstat $(DESTDIR)$(SBINDIR)/rtstat ln -sf lnstat $(DESTDIR)$(SBINDIR)/ctstat + ln -sf ip-brctl $(DESTDIR)$(SBINDIR)/brctl clean: rm -f *.o $(TARGETS) ssfilter.c diff --git a/misc/ip-brctl.in b/misc/ip-brctl.in new file mode 100755 index 000000000000..7e7059aa2c06 --- /dev/null +++ b/misc/ip-brctl.in @@ -0,0 +1,572 @@ +#!/bin/sh +# +# ip-brctl: Implementation of brctl from bridge-utils as iproute2 wrapper +# +# Copyright (c) 2019 Red Hat, Inc. +# Author: Stefano Brivio <sbrivio@redhat.com> +# +# SPDX-License-Identifier: GPL-2.0 + +# Global variables ############################################################# + +# These will be replaced by Makefile on install with ip and bridge install paths +IP="../ip/ip" +BRIDGE="../bridge/bridge" + +usage_text="Usage: brctl [commands] +commands: + addbr <bridge> add bridge + delbr <bridge> delete bridge + addif <bridge> <device> add interface to bridge + delif <bridge> <device> delete interface from bridge + hairpin <bridge> <port> {on|off} turn hairpin on/off + setageing <bridge> <time> set ageing time + setbridgeprio <bridge> <prio> set bridge priority + setfd <bridge> <time> set bridge forward delay + sethello <bridge> <time> set hello time + setmaxage <bridge> <time> set max message age + setpathcost <bridge> <port> <cost> set path cost + setportprio <bridge> <port> <prio> set port priority + show [ <bridge> ] show a list of bridges + showmacs <bridge> show a list of mac addrs + showstp <bridge> show bridge stp info + stp <bridge> {on|off} turn stp on/off" + +# List of commands prefixed by minimum number of arguments +commands="1 addbr 1 delbr 2 addif 2 delif 3 hairpin 2 setageing 2 setbridgeprio + 2 setfd 2 sethello 2 setmaxage 3 setpathcost 3 setportprio 0 show + 1 showmacs 1 showstp 2 stp" + + +# Helper functions ############################################################# + +usage() { + echo "${usage_text}" + exit "${1:-1}" +} + +# Print a single line from usage for given command +usage_one() { + command="${1}" + + ifs="${IFS}" + IFS=' +' + for line in ${usage_text}; do + case ${line} in + " ${command}"*) + line="${line# ${command}*}" + while [ "${line}" != "${line#[[:blank:]]*}" ]; do + line="${line#[[:blank:]]*}" + done + echo "Usage: brctl ${command} ${line}" + break + ;; + esac + done + IFS="${ifs}" + + exit 1 +} + +# Print to standard error and exit +err() { + echo "${@}" > /dev/stderr + exit 1 +} + +# Output token following the given one, from a space-separated string +parse_next() { + needle=${1} + str=${2} + + next=0 + ifs="${IFS}" + IFS=' ' + for token in ${str}; do + [ ${next} -eq 1 ] && echo "${token}" && break + [ "${token}" = "${needle}" ] && next=1 + done + IFS="${ifs}" +} + +# Output value for given ip-link attribute, for the given device +parse_iplink() { + attr="${1}" + dev="${2}" + + out="$(${IP} -d link show dev "${dev}" 2>/dev/null)" + parse_next "${attr}" "${out}" +} + +# Once starting token is matched, for each token x with index n = 2 * k, assign +# value of token with index n + 1 to a variable named by the value of x, using +# state variables parse_assign_start and parse_assign_prev. Pass one token at +# a time. +parse_assign() { + token="${1}" + start_token="${2}" + + if [ "${token}" = "${start_token}" ]; then + parse_assign_start=1 + parse_assign_prev= + elif [ ${parse_assign_start} -eq 0 ]; then + : + elif [ -z "${parse_assign_prev}" ]; then + parse_assign_prev="${token}" + else + eval "${parse_assign_prev}"="${token}" + parse_assign_prev= + fi +} + +# Execute ip-link command with given arguments. On failure, print returned error +# message prefixed by given string and exit. +exec_iplink() { + args="${1}" + err_prefix="${2}" + + err_out="$(${IP} link "${args}" 2>&1)" || err "${err_prefix}: ${err_out#*:}" +} + +# Print passed error string and exit if the given device does or does not exist, +# depending on the condition given. Device type matching is optional. +err_dev_exists() { + cond="${1}" + dev="${2}" + err_string="${3}" + opt_type="${4}" + + [ -n "${opt_type}" ] && opt_type="type ${opt_type}" || opt_type= + + if [ -n "$(${IP} link show dev "${dev}" ${opt_type} 2>/dev/null)" ]; then + [ "${cond}" = "y" ] && err "${err_string}" + else + [ "${cond}" = "n" ] && err "${err_string}" + fi +} + + +# Type checks and conversions ################################################## + +# Don't attempt operations on values exceeding limits for a signed long. From +# POSIX.1-2017, XCU, par. 2.6.4 Arithmetic Expansion: +# +# Only signed long integer arithmetic is required. +# +# On failure, print returned error message prefixed by given string and exit. +check_long() { + in="${1}" + err_prefix="${2}" + + # Comparing x >= 2^31 may fail, allow up to one digit shorter than that + long_max=2147483647 + if [ ${#in} -ge ${#long_max} ]; then + err "${err_prefix}: Numerical result out of range" + fi +} + +# On failure, print passed error string and exit +check_float() { + in="${1}" + err_string="${2}" + + case ${in} in + [0-9.]*) ;; + *) err "${err_string}" ;; + esac +} + +# Convert values allowed as boolean to given strings for false and true values +make_bool() { + in="${1}" + str_false="${2}" + str_true="${2}" + + case ${in} in + off|no|0) echo "${str_false}" ;; + on|yes|1) echo "${str_true}" ;; + *) err "expect on/off for argument" ;; + esac +} + +# Divide by 100, output number with two fractional digits +make_float() { + in="${1}" + + int=$((in / 100)) + frac=$((in % 100)) + [ ${#frac} -eq 1 ] && frac="0${frac}" + echo "${int}.${frac}" +} + +# Multiply given float by 100. Multiple decimal separators are allowed, anything +# after the second one is discarded. +brctl_timeval() { + in="${1}" + + # Drop second decimal separator and following digits, if any + drop=${in#[0-9]*.[0-9]*.} + time=${in%%.${drop}} + + int=${time%.*} + + # Take up to two digits from fractional part + frac=${time#*.} + drop=${frac#[0-9][0-9]*} + frac=${frac%%${drop}} + frac=${frac:-0} + [ "${frac}" -lt 10 ] && frac=$((frac * 10)) + + echo "$((int * 100 + frac))" +} + +# Strip colons from MAC address in bridge identifiers, pad bytes with 0 +fixup_id() { + in="${1}" + + ifs="${IFS}" + IFS=':' + out= + for byte in ${in}; do + [ ${#byte} -eq 1 ] && byte="0${byte}" + out="${out}${byte}" + done + IFS="${ifs}" + echo "${out}" +} + + +# Display functions ############################################################ + +# Pad string to given width -- if not given, width is 7 if float, 4 otherwise +pad_width() { + str="${1}" + [ "${str}" != "${str%.*}" ] && width=${2:-7} || width=${2:-4} + + len=${#str} + while [ "${len}" -lt "${width}" ]; do + str=" ${str}" + len=$((len + 1)) + done + echo "${str}" +} + +# Dump bridge information from set variables in 'brctl showstp' format +# +# shellcheck disable=SC2154 # shellcheck won't see variables we set under eval +dump_bridge() { + name="${1}" + + for i in bridge_id designated_root; do + eval ${i}="\$(fixup_id \$${i})" + done + for i in max_age hello_time forward_delay ageing_time; do + eval ${i}="\$(make_float \$${i})" + done + for i in root_port root_path_cost max_age hello_time forward_delay \ + ageing_time hello_timer tcn_timer topology_change_timer \ + gc_timer; do + eval ${i}=\""\$(pad_width \$${i})"\" + done + flags= + [ "${topology_change}" != "0" ] && flags="TOPOLOGY_CHANGE " + [ "${topology_change_detected}" -ne "0" ] && flags="${flags}TOPOLOGY_CHANGE_DETECTED " + + echo "${name}" + echo " bridge id ${bridge_id}" + echo " designated root ${designated_root}" + echo " root port ${root_port} path cost ${root_path_cost}" + echo " max age ${max_age} bridge max age ${max_age}" + echo " hello time ${hello_time} bridge hello time ${hello_time}" + echo " forward delay ${forward_delay} bridge forward delay ${forward_delay}" + echo " ageing time ${ageing_time}" + echo " hello timer ${hello_timer} tcn timer ${tcn_timer}" + echo " topology change timer ${topology_change_timer} gc timer ${gc_timer}" + echo " flags ${flags}" + echo + echo +} + +# Dump port information from set variables in 'brctl showstp' format +# +# shellcheck disable=SC2154 # shellcheck won't see variables we set under eval +dump_port() { + for i in designated_root designated_bridge; do + eval ${i}="\$(fixup_id \$${i})" + done + + for i in message_age_timer cost message_age_timer forward_delay_timer \ + designated_cost hold_timer; do + eval ${i}=\""\$(pad_width \$${i})"\" + done + + flags= + [ "${config_pending}" != "0" ] && flags="CONFIG_PENDING " + [ "${topology_change_ack}" -ne "0" ] && flags="${flags}TOPOLOGY_CHANGE_ACK " + + echo "${port_name%*:} (${port_no#0x*})" + echo " port id ${port_id#0x*} state $(pad_width "${state}" 15)" + echo " designated root ${designated_root} path cost ${cost}" + echo " designated bridge ${designated_bridge} message age timer ${message_age_timer}" + echo " designated port $((8000 + designated_port % 32768)) forward delay timer ${forward_delay_timer}" + echo " designated cost ${designated_cost} hold timer ${hold_timer}" + echo " flags ${flags}" + [ "${hairpin}" != "off" ] && echo " hairpin mode $(pad_width 1)" + echo +} + + +# Commands ##################################################################### + +cmd_addbr() { + err_dev_exists y "${1}" "device ${1} already exists; can't create bridge with the same name" + + exec_iplink "add ${1} type bridge" "add bridge failed" +} + +cmd_delbr() { + err_dev_exists n "${1}" "bridge ${1} doesn't exist; can't delete it" + if [ "$(parse_iplink state "${1}")" != "DOWN" ]; then + err "bridge ${1} is still up; can't delete it" + fi + + exec_iplink "del ${1}" "can't delete bridge ${1}" +} + +cmd_addif() { + err_dev_exists n "${1}" "bridge ${1} does not exist!" + err_dev_exists n "${2}" "interface ${2} does not exist!" + if [ -n "$(parse_iplink master "${1}")" ]; then + err "device ${2} is already a member of a bridge; can't enslave it to bridge ${1}." + fi + err_dev_exists y "${1}" "device ${1} is a bridge device itself; can't enslave a bridge device to a bridge device." bridge + + exec_iplink "set ${2} master ${1}" "can't add ${2} to bridge ${1}" +} + +cmd_delif() { + err_dev_exists n "${1}" "bridge ${1} does not exist!" + err_dev_exists n "${2}" "interface ${2} does not exist!" + if [ "$(parse_iplink master "${2}")" != "${1}" ]; then + err "device ${1} is not a slave of ${2}" + fi + + exec_iplink "set ${2} nomaster" "can't delete ${2} from ${1}" +} + +cmd_hairpin() { + hairpin="$(make_bool "${3}" off on)" + [ -z "${hairpin}" ] && exit 1 + err_dev_exists n "${2}" "interface ${2} does not exist!" + err_dev_exists n "${1}" "bridge ${1} does not exist!" + + exec_iplink "set ${2} type bridge_slave ${hairpin}" "can't set ${2} to hairpin on bridge ${1}" +} + +cmd_setageing() { + check_long "${2}" "set ageing time failed" + check_float "${2}" "bad ageing time value" + err_dev_exists n "${1}" "set ageing time failed: No such device" + + exec_iplink "set ${1} type bridge ageing_time $(brctl_timeval "${2}")" "set ageing time failed" +} + +cmd_setbridgeprio() { + check_long "${2}" "set bridge priority failed" + check_float "${2}" "bad priority" + err_dev_exists n "${1}" "set bridge priority failed: No such device" + + prio=${2%%.*} + prio=$((prio % 65536)) + + exec_iplink "set ${1} type bridge priority ${prio}" "set bridge priority failed" +} + +cmd_setfd() { + check_long "${2}" "set forward delay failed" + check_float "${2}" "bad ageing time value" + err_dev_exists n "${1}" "set forward delay failed: No such device" + + exec_iplink "set ${1} type bridge forward_delay $(brctl_timeval "${2}")" "set forward delay failed" +} + +cmd_sethello() { + check_long "${2}" "set ageing time failed" + check_float "${2}" "bad hello timer value" + err_dev_exists n "${1}" "set hello timer failed: No such device" + + exec_iplink "set ${1} type bridge hello_time $(brctl_timeval "${2}")" "set hello timer failed" +} + +cmd_setmaxage() { + check_long "${2}" "set max age failed" + check_float "${2}" "bad max age value" + err_dev_exists n "${1}" "set max age failed: No such device" + + exec_iplink "set ${1} max_age $(brctl_timeval "${2}")" "set max age failed" +} + +cmd_setpathcost() { + check_long "${3}" "set path cost failed" + check_float "${3}" "bad path cost value" + err_dev_exists n "${2}" "set path cost failed: No such device" + + cost=${3%%.*} + + exec_iplink "set ${2} type bridge_slave cost ${cost}" "set path cost failed" +} + +cmd_setportprio() { + check_long "${3}" "set port priority failed" + check_float "${3}" "bad path priority value" + err_dev_exists n "${2}" "set port priority failed: No such device" + + prio=${3%%.*} + [ "${prio}" -ge 64 ] && err "set port priority failed: Numerical result out of range" + + exec_iplink "set ${2} type bridge_slave priority ${prio}" "set port priority failed" +} + +cmd_stp() { + stp="$(make_bool "${2}" 0 1)" + [ -z "${stp}" ] && exit 1 + err_dev_exists n "${1}" "set stp status failed: No such device" + + exec_iplink "set ${1} type bridge stp_state ${stp}" "set stp status failed" +} + +cmd_showstp() { + parse_assign_start=0 + for t in $(${IP} -d link show "${1}" 2>/dev/null); do + parse_assign "${t}" bridge + done + [ ${parse_assign_start} -eq 0 ] && err "${1}: can't get info No such device" + + dump_bridge "${1}" + + parse_assign_start=0 + for t in $(${IP} -d link show type bridge_slave master "${1}" 2>/dev/null); do + case ${t} in + [0-9]*:) + [ -n "${port_name}" ] && dump_port + port_name= + parse_assign_start=0 + continue + ;; + esac + [ -z "${port_name}" ] && port_name="${t}" + + parse_assign "${t}" bridge_slave + done + + [ -n "${port_name}" ] && dump_port +} + +cmd_show() { + dev="${1}" + + if [ -n "${dev}" ]; then + err_dev_exists n "${dev}" "bridge ${dev} does not exist!" + err_dev_exists n "${dev}" "device ${dev} is not a bridge!" bridge + fi + + echo "bridge name bridge id STP enabled interfaces" + ifs="${IFS}" + IFS=' +' + for line in $(${IP} -br link show type bridge "${dev}" 2>/dev/null); do + name="${line%% *}" + id="$(fixup_id "$(parse_iplink bridge_id "${name}")")" + [ "$(parse_iplink stp_state "${name}")" = "0" ] && stp="no" || stp="yes" + first=1 + for s in $(${IP} -br link show type bridge_slave master "${name}"); do + if [ ${first} -eq 1 ]; then + echo "${name} ${id} ${stp} ${s%% *}" + first=0 + else + echo " ${s%% *}" + fi + done + [ ${first} -eq 1 ] && echo "${name} ${id} ${stp}" + done + IFS="${ifs}" +} + +cmd_showmacs() { + err_dev_exists n "${1}" "read of forward table failed: No such device" + + echo "port no mac addr is local? ageing timer" + ifs="${IFS}" + IFS=' +' + for s in $(${IP} -br link show type bridge_slave master "${1}"); do + for fdb_entry in $(${BRIDGE} -s fdb show dev "${s%% *}"); do + case ${fdb_entry} in + *" ${1} "*) ;; + *) continue ;; + esac + + case ${fdb_entry} in + *" permanent"*) + is_local="yes" + timer="$(pad_width "0.00")" + ;; + *) + is_local="no" + timer="$(parse_next used "${fdb_entry}")" + timer="$(pad_width "${timer#*/}.00")" + ;; + esac + + port_no="$(parse_iplink port_no "${s%% *}")" + port_no="${port_no#0x*}" + + echo "$(pad_width "${port_no}" 3) ${fdb_entry%% *} ${is_local} ${timer}" + done + done + IFS="${ifs}" +} + + +# Start here ################################################################### + +cmdname="${1}" +[ -z "${cmdname}" ] && usage + +for arg do + case ${arg} in + "-V"|"--v"*) + echo "ip-link wrapper, compatible with bridge-utils, 1.6" + exit 0 + ;; + "-h"|"--h"*) + usage 0 + ;; + "--") + usage + ;; + "-") + break + ;; + "-"*) + echo "${0}: unrecognized option '${arg}'" >/dev/stderr + echo "Unknown option '?'" >/dev/stderr + usage + ;; + esac +done + +found=0 +min_arg= +for cmd in ${commands}; do + [ -n "${min_arg}" ] && [ "${cmd}" = "${cmdname}" ] && found=1 && break + min_arg="${cmd}" +done +[ ${found} -eq 0 ] && echo "never heard of command [${cmdname}]" && usage +[ ${#} -le "${min_arg}" ] && echo "Incorrect number of arguments for command" && usage_one "${cmd}" + +shift +eval "cmd_${cmdname}" "${@}" + +exit 0