[iproute2] bridge: link: allow filtering on bridge name

[iproute2] bridge: link: allow filtering on bridge name

[Nicolas Escande]

When using 'brige link show' we can either dump all links enslaved to any bridge
(called without arg ) or display a single link (called with dev arg).
However there is no way to dummp all links of a single bridge.

To do so, this adds new optional 'master XXX' arg to 'bridge link show' command.
usage: bridge link show master br0

Signed-off-by: Nicolas Escande <nico.escande@gmail.com>
---
 bridge/link.c | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/bridge/link.c b/bridge/link.c
index b3542986..3581c50c 100644
--- a/bridge/link.c
+++ b/bridge/link.c
@@ -17,7 +17,8 @@
 #include "utils.h"
 #include "br_common.h"
 
-static unsigned int filter_index;
+static unsigned int filter_dev_index;
+static unsigned int filter_master_index;
 
 static const char *stp_states[] = {
 	[BR_STATE_DISABLED] = "disabled",
@@ -244,11 +245,15 @@ int print_linkinfo(struct nlmsghdr *n, void *arg)
 	if (!(ifi->ifi_family == AF_BRIDGE || ifi->ifi_family == AF_UNSPEC))
 		return 0;
 
-	if (filter_index && filter_index != ifi->ifi_index)
+	if (filter_dev_index && filter_dev_index != ifi->ifi_index)
 		return 0;
 
 	parse_rtattr_flags(tb, IFLA_MAX, IFLA_RTA(ifi), len, NLA_F_NESTED);
 
+	if (filter_master_index && tb[IFLA_MASTER] &&
+		filter_master_index != rta_getattr_u32(tb[IFLA_MASTER]))
+		return 0;
+
 	name = get_ifname_rta(ifi->ifi_index, tb[IFLA_IFNAME]);
 	if (!name)
 		return -1;
@@ -312,7 +317,7 @@ static void usage(void)
 		"                               [ hwmode {vepa | veb} ]\n"
 		"                               [ backup_port DEVICE ] [ nobackup_port ]\n"
 		"                               [ self ] [ master ]\n"
-		"       bridge link show [dev DEV]\n");
+		"       bridge link show [dev DEV] [master DEVICE]\n");
 	exit(-1);
 }
 
@@ -607,6 +612,7 @@ static int brlink_modify(int argc, char **argv)
 static int brlink_show(int argc, char **argv)
 {
 	char *filter_dev = NULL;
+	char *filter_master = NULL;
 
 	while (argc > 0) {
 		if (strcmp(*argv, "dev") == 0) {
@@ -615,14 +621,25 @@ static int brlink_show(int argc, char **argv)
 				duparg("dev", *argv);
 			filter_dev = *argv;
 		}
+		if (strcmp(*argv, "master") == 0) {
+			NEXT_ARG();
+			if (filter_master)
+				duparg("master", *argv);
+			filter_master = *argv;
+		}
 		argc--; argv++;
 	}
 
 	if (filter_dev) {
-		filter_index = ll_name_to_index(filter_dev);
-		if (!filter_index)
+		filter_dev_index = ll_name_to_index(filter_dev);
+		if (!filter_dev_index)
 			return nodev(filter_dev);
 	}
+	if (filter_master) {
+		filter_master_index = ll_name_to_index(filter_master);
+		if (!filter_master_index)
+			return nodev(filter_master);
+	}
 
 	if (rtnl_linkdump_req(&rth, PF_BRIDGE) < 0) {
 		perror("Cannot send dump request");
-- 
2.41.0

Re: [iproute2] bridge: link: allow filtering on bridge name

[Stephen Hemminger]

On Tue, 25 Jul 2023 11:22:42 +0200
Nicolas Escande <nico.escande@gmail.com> wrote:

> When using 'brige link show' we can either dump all links enslaved to any bridge
> (called without arg ) or display a single link (called with dev arg).
> However there is no way to dummp all links of a single bridge.
> 
> To do so, this adds new optional 'master XXX' arg to 'bridge link show' command.
> usage: bridge link show master br0
> 
> Signed-off-by: Nicolas Escande <nico.escande@gmail.com>

Looks good to me, but we really need to address removing the term master
from bridge utility.

Re: [iproute2] bridge: link: allow filtering on bridge name

[Nicolas Escande]

On Tue Jul 25, 2023 at 6:37 PM CEST, Stephen Hemminger wrote:
> On Tue, 25 Jul 2023 11:22:42 +0200
> Nicolas Escande <nico.escande@gmail.com> wrote:
>
> > When using 'brige link show' we can either dump all links enslaved to any bridge
> > (called without arg ) or display a single link (called with dev arg).
> > However there is no way to dummp all links of a single bridge.
> > 
> > To do so, this adds new optional 'master XXX' arg to 'bridge link show' command.
> > usage: bridge link show master br0
> > 
> > Signed-off-by: Nicolas Escande <nico.escande@gmail.com>
>
> Looks good to me, but we really need to address removing the term master
> from bridge utility.

If you have a better term I can respin it.
But the thing is 'master' is still the most widely used and understood word when
it comes to bridge terminology. And as you said we have it in the output of many
'ip' and 'bridge' commands, that why IMHO it's the term we should use for now...

On another note, there is a slight indentation problem in the new if in
print_linkinfo(), if that can be corrected when the patch gets picked up that
would be perfect, otherwise I can send a v2.

Re: [iproute2] bridge: link: allow filtering on bridge name

[Stephen Hemminger]

On Tue, 25 Jul 2023 23:48:16 +0200
"Nicolas Escande" <nico.escande@gmail.com> wrote:

> On Tue Jul 25, 2023 at 6:37 PM CEST, Stephen Hemminger wrote:
> > On Tue, 25 Jul 2023 11:22:42 +0200
> > Nicolas Escande <nico.escande@gmail.com> wrote:
> >  
> > > When using 'brige link show' we can either dump all links enslaved to any bridge
> > > (called without arg ) or display a single link (called with dev arg).
> > > However there is no way to dummp all links of a single bridge.
> > > 
> > > To do so, this adds new optional 'master XXX' arg to 'bridge link show' command.
> > > usage: bridge link show master br0
> > > 
> > > Signed-off-by: Nicolas Escande <nico.escande@gmail.com>  
> >
> > Looks good to me, but we really need to address removing the term master
> > from bridge utility.  
> 
> If you have a better term I can respin it.
> But the thing is 'master' is still the most widely used and understood word when
> it comes to bridge terminology. And as you said we have it in the output of many
> 'ip' and 'bridge' commands, that why IMHO it's the term we should use for now...


Leave it there for now. Only Linux uses the term master.
FreeBSD and other use the term adding interface to bridge (addm)

IEEE uses the term relay mostly.

You won't find the terms master/slave in any current spec.

> On another note, there is a slight indentation problem in the new if in
> print_linkinfo(), if that can be corrected when the patch gets picked up that
> would be perfect, otherwise I can send a v2.

Send a v2, that would be easier

Re: [iproute2] bridge: link: allow filtering on bridge name

[Nicolas Escande]

On Wed Jul 26, 2023 at 4:16 AM CEST, Stephen Hemminger wrote:
>
> Leave it there for now. Only Linux uses the term master.
> FreeBSD and other use the term adding interface to bridge (addm)
>
> IEEE uses the term relay mostly.
>
> You won't find the terms master/slave in any current spec.
>
> > On another note, there is a slight indentation problem in the new if in
> > print_linkinfo(), if that can be corrected when the patch gets picked up that
> > would be perfect, otherwise I can send a v2.
>
> Send a v2, that would be easier

OK, I'll get right on it.