* [PATCH iproute2 v3] bridge: Add master device name to bridge fdb show
@ 2014-06-08 5:23 roopa
2014-06-08 11:24 ` Jamal Hadi Salim
0 siblings, 1 reply; 7+ messages in thread
From: roopa @ 2014-06-08 5:23 UTC (permalink / raw)
To: davem, stephen, netdev; +Cc: jhs, wkok, sfeldma, shm
From: Roopa Prabhu <roopa@cumulusnetworks.com>
This patch adds master dev name from NDA_MASTER netlink attribute
to bridge fdb show output
current iproute2 tries to print 'master' in the output if NTF_MASTER
is present. But, kernel today does not set NTF_MASTER during dump
requests. Which means I have not seen iproute2 bridge cmd print 'master' atall.
This patch overrides the NTF_MASTER flag if NDA_MASTER attribute is present.
Example output:
before this patch:
# bridge fdb show
44:38:39:00:27:ba dev bond2.2003 permanent
44:38:39:00:27:bb dev bond4.2003 permanent
44:38:39:00:27:bc dev bond2.2004 permanent
After this patch:
# bridge fdb show
44:38:39:00:27:ba dev bond2.2003 master br-2003 permanent
44:38:39:00:27:bb dev bond4.2003 master br-2003 permanent
44:38:39:00:27:bc dev bond2.2004 master br-2004 permanent
For comparision with the above, below is the output for NTF_SELF today,
# bridge fdb show
33:33:00:00:00:01 dev eth0 self permanent
01:00:5e:00:00:01 dev eth0 self permanent
33:33:ff:00:01:cc dev eth0 self permanent
If change in output is a concern, 'master' can be put at the end of the fdb
output line or made optional with -d[etails] option.
change from v1 to v2:
use 'bridge' instead of 'master' in fdb show output
change from v2 to v3:
use 'master' instead of 'bridge' in fdb show output
(master could also be a vxlan device)
Signed-off-by: Wilson Kok <wkok@cumulusnetworks.com>
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
bridge/fdb.c | 5 ++++-
include/linux/neighbour.h | 1 +
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/bridge/fdb.c b/bridge/fdb.c
index 9b720e3..d1c3da6 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -146,7 +146,10 @@ int print_fdb(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
}
if (r->ndm_flags & NTF_SELF)
fprintf(fp, "self ");
- if (r->ndm_flags & NTF_MASTER)
+ if (tb[NDA_MASTER])
+ fprintf(fp, "master %s ",
+ ll_index_to_name(rta_getattr_u32(tb[NDA_MASTER])));
+ else if (r->ndm_flags & NTF_MASTER)
fprintf(fp, "master ");
if (r->ndm_flags & NTF_ROUTER)
fprintf(fp, "router ");
diff --git a/include/linux/neighbour.h b/include/linux/neighbour.h
index d3ef583..4a1d7e9 100644
--- a/include/linux/neighbour.h
+++ b/include/linux/neighbour.h
@@ -24,6 +24,7 @@ enum {
NDA_PORT,
NDA_VNI,
NDA_IFINDEX,
+ NDA_MASTER,
__NDA_MAX
};
--
1.7.10.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH iproute2 v3] bridge: Add master device name to bridge fdb show
2014-06-08 5:23 [PATCH iproute2 v3] bridge: Add master device name to bridge fdb show roopa
@ 2014-06-08 11:24 ` Jamal Hadi Salim
2014-06-08 14:44 ` Roopa Prabhu
0 siblings, 1 reply; 7+ messages in thread
From: Jamal Hadi Salim @ 2014-06-08 11:24 UTC (permalink / raw)
To: roopa, davem, stephen, netdev; +Cc: wkok, sfeldma, shm, Vlad Yasevich
note: The "Self" addresses are derived from uni/multicast addresses of
the port i.e they are _not_ part of the fdb.
IMO, it is unfortunate that the fdb command is being used for
dealing with device specific multicast/unicast address management
as is "ip link" - but the API is out there already.
Therefore they dont really belong to the fdb table and displaying
the master as the bridge maybe misleading.
For that reason, I like your earlier patch.
Vlad?
cheers,
jamal
On 06/08/14 01:23, roopa@cumulusnetworks.com wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> This patch adds master dev name from NDA_MASTER netlink attribute
> to bridge fdb show output
>
> current iproute2 tries to print 'master' in the output if NTF_MASTER
> is present. But, kernel today does not set NTF_MASTER during dump
> requests. Which means I have not seen iproute2 bridge cmd print 'master' atall.
> This patch overrides the NTF_MASTER flag if NDA_MASTER attribute is present.
>
> Example output:
>
> before this patch:
> # bridge fdb show
> 44:38:39:00:27:ba dev bond2.2003 permanent
> 44:38:39:00:27:bb dev bond4.2003 permanent
> 44:38:39:00:27:bc dev bond2.2004 permanent
>
> After this patch:
> # bridge fdb show
> 44:38:39:00:27:ba dev bond2.2003 master br-2003 permanent
> 44:38:39:00:27:bb dev bond4.2003 master br-2003 permanent
> 44:38:39:00:27:bc dev bond2.2004 master br-2004 permanent
>
> For comparision with the above, below is the output for NTF_SELF today,
> # bridge fdb show
> 33:33:00:00:00:01 dev eth0 self permanent
> 01:00:5e:00:00:01 dev eth0 self permanent
> 33:33:ff:00:01:cc dev eth0 self permanent
>
> If change in output is a concern, 'master' can be put at the end of the fdb
> output line or made optional with -d[etails] option.
>
> change from v1 to v2:
> use 'bridge' instead of 'master' in fdb show output
>
> change from v2 to v3:
> use 'master' instead of 'bridge' in fdb show output
> (master could also be a vxlan device)
>
> Signed-off-by: Wilson Kok <wkok@cumulusnetworks.com>
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> ---
> bridge/fdb.c | 5 ++++-
> include/linux/neighbour.h | 1 +
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/bridge/fdb.c b/bridge/fdb.c
> index 9b720e3..d1c3da6 100644
> --- a/bridge/fdb.c
> +++ b/bridge/fdb.c
> @@ -146,7 +146,10 @@ int print_fdb(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
> }
> if (r->ndm_flags & NTF_SELF)
> fprintf(fp, "self ");
> - if (r->ndm_flags & NTF_MASTER)
> + if (tb[NDA_MASTER])
> + fprintf(fp, "master %s ",
> + ll_index_to_name(rta_getattr_u32(tb[NDA_MASTER])));
> + else if (r->ndm_flags & NTF_MASTER)
> fprintf(fp, "master ");
> if (r->ndm_flags & NTF_ROUTER)
> fprintf(fp, "router ");
> diff --git a/include/linux/neighbour.h b/include/linux/neighbour.h
> index d3ef583..4a1d7e9 100644
> --- a/include/linux/neighbour.h
> +++ b/include/linux/neighbour.h
> @@ -24,6 +24,7 @@ enum {
> NDA_PORT,
> NDA_VNI,
> NDA_IFINDEX,
> + NDA_MASTER,
> __NDA_MAX
> };
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH iproute2 v3] bridge: Add master device name to bridge fdb show
2014-06-08 11:24 ` Jamal Hadi Salim
@ 2014-06-08 14:44 ` Roopa Prabhu
2014-06-08 15:24 ` Jamal Hadi Salim
0 siblings, 1 reply; 7+ messages in thread
From: Roopa Prabhu @ 2014-06-08 14:44 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: davem, stephen, netdev, wkok, sfeldma, shm, Vlad Yasevich
On 6/8/14, 4:24 AM, Jamal Hadi Salim wrote:
>
> note: The "Self" addresses are derived from uni/multicast addresses of
> the port i.e they are _not_ part of the fdb.
> IMO, it is unfortunate that the fdb command is being used for
> dealing with device specific multicast/unicast address management
> as is "ip link" - but the API is out there already.
> Therefore they dont really belong to the fdb table and displaying
> the master as the bridge maybe misleading.
sure, you seemed to be using my below patch in your examples for fdb
filtering.
so thought i should resubmit. I am now ok with either.
Thanks,
Roopa
>
> On 06/08/14 01:23, roopa@cumulusnetworks.com wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> This patch adds master dev name from NDA_MASTER netlink attribute
>> to bridge fdb show output
>>
>> current iproute2 tries to print 'master' in the output if NTF_MASTER
>> is present. But, kernel today does not set NTF_MASTER during dump
>> requests. Which means I have not seen iproute2 bridge cmd print
>> 'master' atall.
>> This patch overrides the NTF_MASTER flag if NDA_MASTER attribute is
>> present.
>>
>> Example output:
>>
>> before this patch:
>> # bridge fdb show
>> 44:38:39:00:27:ba dev bond2.2003 permanent
>> 44:38:39:00:27:bb dev bond4.2003 permanent
>> 44:38:39:00:27:bc dev bond2.2004 permanent
>>
>> After this patch:
>> # bridge fdb show
>> 44:38:39:00:27:ba dev bond2.2003 master br-2003 permanent
>> 44:38:39:00:27:bb dev bond4.2003 master br-2003 permanent
>> 44:38:39:00:27:bc dev bond2.2004 master br-2004 permanent
>>
>> For comparision with the above, below is the output for NTF_SELF today,
>> # bridge fdb show
>> 33:33:00:00:00:01 dev eth0 self permanent
>> 01:00:5e:00:00:01 dev eth0 self permanent
>> 33:33:ff:00:01:cc dev eth0 self permanent
>>
>> If change in output is a concern, 'master' can be put at the end of
>> the fdb
>> output line or made optional with -d[etails] option.
>>
>> change from v1 to v2:
>> use 'bridge' instead of 'master' in fdb show output
>>
>> change from v2 to v3:
>> use 'master' instead of 'bridge' in fdb show output
>> (master could also be a vxlan device)
>>
>> Signed-off-by: Wilson Kok <wkok@cumulusnetworks.com>
>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>> ---
>> bridge/fdb.c | 5 ++++-
>> include/linux/neighbour.h | 1 +
>> 2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/bridge/fdb.c b/bridge/fdb.c
>> index 9b720e3..d1c3da6 100644
>> --- a/bridge/fdb.c
>> +++ b/bridge/fdb.c
>> @@ -146,7 +146,10 @@ int print_fdb(const struct sockaddr_nl *who,
>> struct nlmsghdr *n, void *arg)
>> }
>> if (r->ndm_flags & NTF_SELF)
>> fprintf(fp, "self ");
>> - if (r->ndm_flags & NTF_MASTER)
>> + if (tb[NDA_MASTER])
>> + fprintf(fp, "master %s ",
>> + ll_index_to_name(rta_getattr_u32(tb[NDA_MASTER])));
>> + else if (r->ndm_flags & NTF_MASTER)
>> fprintf(fp, "master ");
>> if (r->ndm_flags & NTF_ROUTER)
>> fprintf(fp, "router ");
>> diff --git a/include/linux/neighbour.h b/include/linux/neighbour.h
>> index d3ef583..4a1d7e9 100644
>> --- a/include/linux/neighbour.h
>> +++ b/include/linux/neighbour.h
>> @@ -24,6 +24,7 @@ enum {
>> NDA_PORT,
>> NDA_VNI,
>> NDA_IFINDEX,
>> + NDA_MASTER,
>> __NDA_MAX
>> };
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH iproute2 v3] bridge: Add master device name to bridge fdb show
2014-06-08 14:44 ` Roopa Prabhu
@ 2014-06-08 15:24 ` Jamal Hadi Salim
2014-06-08 22:25 ` Roopa Prabhu
0 siblings, 1 reply; 7+ messages in thread
From: Jamal Hadi Salim @ 2014-06-08 15:24 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: davem, stephen, netdev, wkok, sfeldma, shm, Vlad Yasevich
On 06/08/14 10:44, Roopa Prabhu wrote:
> sure, you seemed to be using my below patch in your examples for fdb
> filtering.
> so thought i should resubmit. I am now ok with either.
Yes, my patch is on top of your earlier (version 1) patch. I preferred
that version.
I looked at the code a little more after sending that email.
There is some value in displaying a bridge port's unicast addresses
and even setting them; but only in the case that the bridge port is
itself an owner of an fdb. As an example of such a case when you have
another bridge attached to a bridge as a port; or when you have
something like a vxlan as a bridge port etc.
However, current code does not discriminate. I cant make sense of
why this would be intentional but possibly i am missing some use cases.
cheers,
jamal
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH iproute2 v3] bridge: Add master device name to bridge fdb show
2014-06-08 15:24 ` Jamal Hadi Salim
@ 2014-06-08 22:25 ` Roopa Prabhu
2014-06-09 10:21 ` Jamal Hadi Salim
0 siblings, 1 reply; 7+ messages in thread
From: Roopa Prabhu @ 2014-06-08 22:25 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: davem, stephen, netdev, wkok, sfeldma, shm, Vlad Yasevich
On 6/8/14, 8:24 AM, Jamal Hadi Salim wrote:
> On 06/08/14 10:44, Roopa Prabhu wrote:
>
>> sure, you seemed to be using my below patch in your examples for fdb
>> filtering.
>> so thought i should resubmit. I am now ok with either.
>
> Yes, my patch is on top of your earlier (version 1) patch. I preferred
> that version.
ok, yes, i thought so. And this version (v3) is same as v1
>
> I looked at the code a little more after sending that email.
> There is some value in displaying a bridge port's unicast addresses
> and even setting them; but only in the case that the bridge port is
> itself an owner of an fdb. As an example of such a case when you have
> another bridge attached to a bridge as a port; or when you have
> something like a vxlan as a bridge port etc.
> However, current code does not discriminate. I cant make sense of
> why this would be intentional but possibly i am missing some use cases.
>
> cheers,
> jamal
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH iproute2 v3] bridge: Add master device name to bridge fdb show
2014-06-08 22:25 ` Roopa Prabhu
@ 2014-06-09 10:21 ` Jamal Hadi Salim
2014-06-09 19:56 ` Stephen Hemminger
0 siblings, 1 reply; 7+ messages in thread
From: Jamal Hadi Salim @ 2014-06-09 10:21 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: davem, stephen, netdev, wkok, sfeldma, shm, Vlad Yasevich
On 06/08/14 18:25, Roopa Prabhu wrote:
> ok, yes, i thought so. And this version (v3) is same as v1
Ignore my babbling then ;->
I think the new paragraph in the commit message and the one line
movement confused me.
cheers,
jamal
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH iproute2 v3] bridge: Add master device name to bridge fdb show
2014-06-09 10:21 ` Jamal Hadi Salim
@ 2014-06-09 19:56 ` Stephen Hemminger
0 siblings, 0 replies; 7+ messages in thread
From: Stephen Hemminger @ 2014-06-09 19:56 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Roopa Prabhu, davem, netdev, wkok, sfeldma, shm, Vlad Yasevich
On Mon, 09 Jun 2014 06:21:06 -0400
Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> On 06/08/14 18:25, Roopa Prabhu wrote:
>
> > ok, yes, i thought so. And this version (v3) is same as v1
>
> Ignore my babbling then ;->
> I think the new paragraph in the commit message and the one line
> movement confused me.
>
> cheers,
> jamal
Applied to net-next branch
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-06-09 19:57 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-08 5:23 [PATCH iproute2 v3] bridge: Add master device name to bridge fdb show roopa
2014-06-08 11:24 ` Jamal Hadi Salim
2014-06-08 14:44 ` Roopa Prabhu
2014-06-08 15:24 ` Jamal Hadi Salim
2014-06-08 22:25 ` Roopa Prabhu
2014-06-09 10:21 ` Jamal Hadi Salim
2014-06-09 19:56 ` Stephen Hemminger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.