[PATCH 1/1] iproute2 bridge: bring to par with brctl show macs

[PATCH 1/1] iproute2 bridge: bring to par with brctl show macs

[Jamal Hadi Salim]

From: Jamal Hadi Salim <jhs@mojatatu.com>

Example usage with bridge sw1 with two entries (port sw1-p1)

root@jhs-1:iprtmay24# ./bridge/bridge fdb help
Usage: bridge fdb { add | append | del | replace } ADDR dev DEV
{self|master} [ temp ]
              [router] [ dst IPADDR] [ vlan VID ]
              [ port PORT] [ vni VNI ] [via DEV]
       bridge fdb {show} [ br BRDEV ] [ brport DEV ]
root@jhs-1:iprtmay24#
root@jhs-1:iprtmay24#
root@jhs-1:iprtmay24# bridge fdb show
33:33:00:00:00:01 dev bond0 self permanent
33:33:00:00:00:01 dev dummy0 self permanent
33:33:00:00:00:01 dev ifb0 self permanent
33:33:00:00:00:01 dev ifb1 self permanent
33:33:00:00:00:01 dev eth0 self permanent
01:00:5e:00:00:01 dev eth0 self permanent
33:33:ff:22:01:01 dev eth0 self permanent
33:33:00:00:00:01 dev eth1 self permanent
33:33:00:00:00:01 dev gretap0 self permanent
6e:9a:af:54:70:ff dev sw1-p1 vlan 0 permanent
00:17:42:8a:b4:05 dev sw1-p1 vlan 0 permanent
33:33:00:00:00:01 dev sw1-p1 self permanent
root@moja-1:iprtmay24#
root@moja-1:iprtmay24# bridge fdb show br sw1
6e:9a:af:54:70:ff dev sw1-p1 vlan 0 permanent
00:17:42:8a:b4:05 dev sw1-p1 vlan 0 permanent

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 bridge/fdb.c |   29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/bridge/fdb.c b/bridge/fdb.c
index 9b720e3..01a2d71 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -33,7 +33,7 @@ static void usage(void)
 	fprintf(stderr, "Usage: bridge fdb { add | append | del | replace } ADDR dev DEV {self|master} [ temp ]\n"
 		        "              [router] [ dst IPADDR] [ vlan VID ]\n"
 		        "              [ port PORT] [ vni VNI ] [via DEV]\n");
-	fprintf(stderr, "       bridge fdb {show} [ dev DEV ]\n");
+	fprintf(stderr, "       bridge fdb {show} [ br BRDEV ] [ brport DEV ]\n");
 	exit(-1);
 }
 
@@ -157,18 +157,35 @@ int print_fdb(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 
 static int fdb_show(int argc, char **argv)
 {
+	struct ndmsg ndm = { };
 	char *filter_dev = NULL;
+	char *br = NULL;
+
+	ndm.ndm_family = PF_BRIDGE;
+	ndm.ndm_state = NUD_NOARP;
 
 	while (argc > 0) {
-		if (strcmp(*argv, "dev") == 0) {
+		if ((strcmp(*argv, "brport") == 0) || strcmp(*argv, "dev") == 0) {
 			NEXT_ARG();
-			if (filter_dev)
-				duparg("dev", *argv);
 			filter_dev = *argv;
+		} else if (strcmp(*argv, "br") == 0) {
+			NEXT_ARG();
+			br = *argv;
+		} else {
+			if (matches(*argv, "help") == 0)
+				usage();
 		}
 		argc--; argv++;
 	}
 
+	if (br) {
+		ndm.ndm_ifindex = ll_name_to_index(br);
+		if (ndm.ndm_ifindex == 0) {
+			fprintf(stderr, "Cannot find bridge device \"%s\"\n", br);
+			return -1;
+		}
+	}
+
 	if (filter_dev) {
 		filter_index = if_nametoindex(filter_dev);
 		if (filter_index == 0) {
@@ -176,13 +193,15 @@ static int fdb_show(int argc, char **argv)
 				filter_dev);
 			return -1;
 		}
+
 	}
 
-	if (rtnl_wilddump_request(&rth, PF_BRIDGE, RTM_GETNEIGH) < 0) {
+	if (rtnl_dump_request(&rth, RTM_GETNEIGH, &ndm, sizeof(struct ndmsg)) < 0) {
 		perror("Cannot send dump request");
 		exit(1);
 	}
 
+
 	if (rtnl_dump_filter(&rth, print_fdb, stdout) < 0) {
 		fprintf(stderr, "Dump terminated\n");
 		exit(1);
-- 
1.7.9.5

Re: [PATCH 1/1] iproute2 bridge: bring to par with brctl show macs

[Scott Feldman]

On May 25, 2014, at 6:22 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:

> From: Jamal Hadi Salim <jhs@mojatatu.com>
> 
> Example usage with bridge sw1 with two entries (port sw1-p1)
> 
> root@jhs-1:iprtmay24# ./bridge/bridge fdb help
> Usage: bridge fdb { add | append | del | replace } ADDR dev DEV
> {self|master} [ temp ]
>              [router] [ dst IPADDR] [ vlan VID ]
>              [ port PORT] [ vni VNI ] [via DEV]
>       bridge fdb {show} [ br BRDEV ] [ brport DEV ]
> root@jhs-1:iprtmay24#
> root@jhs-1:iprtmay24#
> root@jhs-1:iprtmay24# bridge fdb show
> 33:33:00:00:00:01 dev bond0 self permanent
> 33:33:00:00:00:01 dev dummy0 self permanent
> 33:33:00:00:00:01 dev ifb0 self permanent
> 33:33:00:00:00:01 dev ifb1 self permanent
> 33:33:00:00:00:01 dev eth0 self permanent
> 01:00:5e:00:00:01 dev eth0 self permanent
> 33:33:ff:22:01:01 dev eth0 self permanent
> 33:33:00:00:00:01 dev eth1 self permanent
> 33:33:00:00:00:01 dev gretap0 self permanent
> 6e:9a:af:54:70:ff dev sw1-p1 vlan 0 permanent
> 00:17:42:8a:b4:05 dev sw1-p1 vlan 0 permanent
> 33:33:00:00:00:01 dev sw1-p1 self permanent
> root@moja-1:iprtmay24#
> root@moja-1:iprtmay24# bridge fdb show br sw1
> 6e:9a:af:54:70:ff dev sw1-p1 vlan 0 permanent
> 00:17:42:8a:b4:05 dev sw1-p1 vlan 0 permanent
> 
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> ---
> bridge/fdb.c |   29 ++++++++++++++++++++++++-----
> 1 file changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/bridge/fdb.c b/bridge/fdb.c
> index 9b720e3..01a2d71 100644
> --- a/bridge/fdb.c
> +++ b/bridge/fdb.c
> @@ -33,7 +33,7 @@ static void usage(void)
> 	fprintf(stderr, "Usage: bridge fdb { add | append | del | replace } ADDR dev DEV {self|master} [ temp ]\n"
> 		        "              [router] [ dst IPADDR] [ vlan VID ]\n"
> 		        "              [ port PORT] [ vni VNI ] [via DEV]\n");
> -	fprintf(stderr, "       bridge fdb {show} [ dev DEV ]\n");
> +	fprintf(stderr, "       bridge fdb {show} [ br BRDEV ] [ brport DEV ]\n");
> 	exit(-1);
> }
> 
> @@ -157,18 +157,35 @@ int print_fdb(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
> 
> static int fdb_show(int argc, char **argv)
> {
> +	struct ndmsg ndm = { };
> 	char *filter_dev = NULL;
> +	char *br = NULL;
> +
> +	ndm.ndm_family = PF_BRIDGE;
> +	ndm.ndm_state = NUD_NOARP;
> 
> 	while (argc > 0) {
> -		if (strcmp(*argv, "dev") == 0) {
> +		if ((strcmp(*argv, "brport") == 0) || strcmp(*argv, "dev") == 0) {
> 			NEXT_ARG();
> -			if (filter_dev)
> -				duparg("dev", *argv);
> 			filter_dev = *argv;
> +		} else if (strcmp(*argv, "br") == 0) {
> +			NEXT_ARG();
> +			br = *argv;
> +		} else {
> +			if (matches(*argv, "help") == 0)
> +				usage();
> 		}
> 		argc--; argv++;
> 	}
> 
> +	if (br) {
> +		ndm.ndm_ifindex = ll_name_to_index(br);

Seems you could use your new kernel filter for brports also, when br is not specified.  If both br and brport is specified, then use filter in user-space, otherwise use filter in kernel on br or brport ifindex.

> +		if (ndm.ndm_ifindex == 0) {
> +			fprintf(stderr, "Cannot find bridge device \"%s\"\n", br);
> +			return -1;
> +		}
> +	}
> +
> 	if (filter_dev) {
> 		filter_index = if_nametoindex(filter_dev);
> 		if (filter_index == 0) {
> @@ -176,13 +193,15 @@ static int fdb_show(int argc, char **argv)
> 				filter_dev);
> 			return -1;
> 		}
> +
> 	}
> 
> -	if (rtnl_wilddump_request(&rth, PF_BRIDGE, RTM_GETNEIGH) < 0) {
> +	if (rtnl_dump_request(&rth, RTM_GETNEIGH, &ndm, sizeof(struct ndmsg)) < 0) {
> 		perror("Cannot send dump request");
> 		exit(1);
> 	}
> 
> +
> 	if (rtnl_dump_filter(&rth, print_fdb, stdout) < 0) {
> 		fprintf(stderr, "Dump terminated\n");
> 		exit(1);
> -- 
> 1.7.9.5
> 


-scott

Re: [PATCH 1/1] iproute2 bridge: bring to par with brctl show macs

[Jamal Hadi Salim]

On 05/27/14 11:59, Scott Feldman wrote:
>
>
> Seems you could use your new kernel filter for brports also, when br is not specified.
>If both br and brport is specified, then use filter in user-space, otherwise use filter
>in kernel on br or brport ifindex.
>

filtering by brport is very useful.
I wasnt as much concerned about that use case because i can already
filter in user space for brport when i dump (iproute2 does as well).
But i couldnt filter by bridge since that info is not even
present in the dumped data
I just saw the patch posted by Roopa - I think that now gives me the
ability to also filter by bridge in user space.

Note: All i am doing is stealing the ndm ifindex - which before was
a zero and ignore by the kernel as the bridge ifindex.
I think what you are suggesting is: if the user doesnt specify the
bridge i should use the ndm ifindex to send down the brport as
the filter. But that becomes an bridge "or" bridgeport filter
choice. A more useful scenario is to have bridge "and"
bridgeport filter. So almost seems like i need an attribute to send
down for the bridge port.
Sounds like a separate patch...

cheers,
jamal

Re: [PATCH 1/1] iproute2 bridge: bring to par with brctl show macs

[Jamal Hadi Salim]

On 05/27/14 17:41, Jamal Hadi Salim wrote:

> filtering by brport is very useful.
> I wasnt as much concerned about that use case because i can already
> filter in user space for brport when i dump (iproute2 does as well).
> But i couldnt filter by bridge since that info is not even
> present in the dumped data
> I just saw the patch posted by Roopa - I think that now gives me the
> ability to also filter by bridge in user space.
>
> Note: All i am doing is stealing the ndm ifindex - which before was
> a zero and ignore by the kernel as the bridge ifindex.
> I think what you are suggesting is: if the user doesnt specify the
> bridge i should use the ndm ifindex to send down the brport as
> the filter. But that becomes an bridge "or" bridgeport filter
> choice. A more useful scenario is to have bridge "and"
> bridgeport filter. So almost seems like i need an attribute to send
> down for the bridge port.
> Sounds like a separate patch...
>

Hrm. I wonder - I could use the NDA_MASTER attribute that Roopa
posted to define the bridge filter. I could then use the ndm ifindex to
define the bridgeport filter.

Thoughts?

cheers,
jamal

Re: [PATCH 1/1] iproute2 bridge: bring to par with brctl show macs

[Roopa Prabhu]

On 5/27/14, 2:43 PM, Jamal Hadi Salim wrote:
> On 05/27/14 17:41, Jamal Hadi Salim wrote:
>
>> filtering by brport is very useful.
>> I wasnt as much concerned about that use case because i can already
>> filter in user space for brport when i dump (iproute2 does as well).
>> But i couldnt filter by bridge since that info is not even
>> present in the dumped data
>> I just saw the patch posted by Roopa - I think that now gives me the
>> ability to also filter by bridge in user space.
>>
>> Note: All i am doing is stealing the ndm ifindex - which before was
>> a zero and ignore by the kernel as the bridge ifindex.
>> I think what you are suggesting is: if the user doesnt specify the
>> bridge i should use the ndm ifindex to send down the brport as
>> the filter. But that becomes an bridge "or" bridgeport filter
>> choice. A more useful scenario is to have bridge "and"
>> bridgeport filter. So almost seems like i need an attribute to send
>> down for the bridge port.
>> Sounds like a separate patch...
>>
>
> Hrm. I wonder - I could use the NDA_MASTER attribute that Roopa
> posted to define the bridge filter. I could then use the ndm ifindex to
> define the bridgeport filter.
>
> Thoughts?
>
I was just going to suggest that. ifindex is used for ports during 
notify as well. so, might makes sense to use ndm_ifindex for ports 
during dump.