All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [net-next PATCH 1/2] bridge fdb dumping takes a filter device Dumping a bridge fdb dumps every fdb entry held. With this change we are going to filter on selected bridge port.
@ 2014-06-01 11:56 Jamal Hadi Salim
  2014-06-01 11:56 ` [net-next PATCH 2/2] bridge netlink dump interface at par with brctl Actually better than brctl showmacs because we can filter by bridge port in the kernel Jamal Hadi Salim
  2014-06-05  7:15 ` [net-next PATCH 1/2] bridge fdb dumping takes a filter device Dumping a bridge fdb dumps every fdb entry held. With this change we are going to filter on selected bridge port David Miller
  0 siblings, 2 replies; 8+ messages in thread
From: Jamal Hadi Salim @ 2014-06-01 11:56 UTC (permalink / raw
  To: davem, stephen
  Cc: netdev, vyasevic, sfeldma, john.r.fastabend, roopa,
	Jamal Hadi Salim

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

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c      |    3 ++-
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c |    5 +++--
 include/linux/netdevice.h                        |    1 +
 include/linux/rtnetlink.h                        |    1 +
 net/bridge/br_fdb.c                              |    5 +++++
 net/bridge/br_private.h                          |    2 +-
 net/core/rtnetlink.c                             |    7 ++++---
 7 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 8e15ced..6636e64 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6691,13 +6691,14 @@ static int i40e_ndo_fdb_del(struct ndmsg *ndm,
 static int i40e_ndo_fdb_dump(struct sk_buff *skb,
 			     struct netlink_callback *cb,
 			     struct net_device *dev,
+			     struct net_device *filter_dev,
 			     int idx)
 {
 	struct i40e_netdev_priv *np = netdev_priv(dev);
 	struct i40e_pf *pf = np->vsi->back;
 
 	if (pf->flags & I40E_FLAG_SRIOV_ENABLED)
-		idx = ndo_dflt_fdb_dump(skb, cb, dev, idx);
+		idx = ndo_dflt_fdb_dump(skb, cb, dev, filter_dev, idx);
 
 	return idx;
 }
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index f06ba90b..233f282 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -427,12 +427,13 @@ static int qlcnic_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
 }
 
 static int qlcnic_fdb_dump(struct sk_buff *skb, struct netlink_callback *ncb,
-			struct net_device *netdev, int idx)
+			struct net_device *netdev, struct net_device *filter_dev,
+			int idx)
 {
 	struct qlcnic_adapter *adapter = netdev_priv(netdev);
 
 	if (!adapter->fdb_mac_learn)
-		return ndo_dflt_fdb_dump(skb, ncb, netdev, idx);
+		return ndo_dflt_fdb_dump(skb, ncb, netdev, filter_dev, idx);
 
 	if ((adapter->flags & QLCNIC_ESWITCH_ENABLED) ||
 	    qlcnic_sriov_check(adapter))
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 2db1610..dd9a354 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1114,6 +1114,7 @@ struct net_device_ops {
 	int			(*ndo_fdb_dump)(struct sk_buff *skb,
 						struct netlink_callback *cb,
 						struct net_device *dev,
+						struct net_device *filter_dev,
 						int idx);
 
 	int			(*ndo_bridge_setlink)(struct net_device *dev,
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 953937e..167bae7 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -78,6 +78,7 @@ extern void __rtnl_unlock(void);
 extern int ndo_dflt_fdb_dump(struct sk_buff *skb,
 			     struct netlink_callback *cb,
 			     struct net_device *dev,
+			     struct net_device *filter_dev,
 			     int idx);
 extern int ndo_dflt_fdb_add(struct ndmsg *ndm,
 			    struct nlattr *tb[],
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 2c45c06..e803180 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -672,6 +672,7 @@ errout:
 int br_fdb_dump(struct sk_buff *skb,
 		struct netlink_callback *cb,
 		struct net_device *dev,
+		struct net_device *filter_dev,
 		int idx)
 {
 	struct net_bridge *br = netdev_priv(dev);
@@ -687,6 +688,10 @@ int br_fdb_dump(struct sk_buff *skb,
 			if (idx < cb->args[0])
 				goto skip;
 
+			if (filter_dev && (!f->dst || !f->dst->dev ||
+					   f->dst->dev != filter_dev))
+				goto skip;
+
 			if (fdb_fill_info(skb, br, f,
 					  NETLINK_CB(cb->skb).portid,
 					  cb->nlh->nlmsg_seq,
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 53d6e32..403e948 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -398,7 +398,7 @@ int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
 int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[], struct net_device *dev,
 	       const unsigned char *addr, u16 nlh_flags);
 int br_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb,
-		struct net_device *dev, int idx);
+		struct net_device *dev, struct net_device *fdev, int idx);
 int br_fdb_sync_static(struct net_bridge *br, struct net_bridge_port *p);
 void br_fdb_unsync_static(struct net_bridge *br, struct net_bridge_port *p);
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index f31268d..064418e 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2490,6 +2490,7 @@ skip:
 int ndo_dflt_fdb_dump(struct sk_buff *skb,
 		      struct netlink_callback *cb,
 		      struct net_device *dev,
+		      struct net_device *filter_dev,
 		      int idx)
 {
 	int err;
@@ -2520,13 +2521,13 @@ static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
 			br_dev = netdev_master_upper_dev_get(dev);
 			ops = br_dev->netdev_ops;
 			if (ops->ndo_fdb_dump)
-				idx = ops->ndo_fdb_dump(skb, cb, dev, idx);
+				idx = ops->ndo_fdb_dump(skb, cb, dev, NULL, idx);
 		}
 
 		if (dev->netdev_ops->ndo_fdb_dump)
-			idx = dev->netdev_ops->ndo_fdb_dump(skb, cb, dev, idx);
+			idx = dev->netdev_ops->ndo_fdb_dump(skb, cb, dev, NULL, idx);
 		else
-			idx = ndo_dflt_fdb_dump(skb, cb, dev, idx);
+			idx = ndo_dflt_fdb_dump(skb, cb, dev, NULL, idx);
 	}
 	rcu_read_unlock();
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [net-next PATCH 2/2] bridge netlink dump interface at par with brctl Actually better than brctl showmacs because we can filter by bridge port in the kernel
  2014-06-01 11:56 [net-next PATCH 1/2] bridge fdb dumping takes a filter device Dumping a bridge fdb dumps every fdb entry held. With this change we are going to filter on selected bridge port Jamal Hadi Salim
@ 2014-06-01 11:56 ` Jamal Hadi Salim
  2014-06-01 12:16   ` Jamal Hadi Salim
  2014-06-02 15:34   ` Vlad Yasevich
  2014-06-05  7:15 ` [net-next PATCH 1/2] bridge fdb dumping takes a filter device Dumping a bridge fdb dumps every fdb entry held. With this change we are going to filter on selected bridge port David Miller
  1 sibling, 2 replies; 8+ messages in thread
From: Jamal Hadi Salim @ 2014-06-01 11:56 UTC (permalink / raw
  To: davem, stephen
  Cc: netdev, vyasevic, sfeldma, john.r.fastabend, roopa,
	Jamal Hadi Salim

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

The current bridge netlink interface doesnt scale when you have many bridges each
with large fdbs or even bridges with many bridge ports

Example usage:

Lets start with two bridges each with a port...

root@moja-mojo:bridge# ./bridge link
8: eth1 state DOWN : <BROADCAST,MULTICAST> mtu 1500 master br0 state disabled priority 32 cost 19
17: sw1-p1 state DOWN : <BROADCAST,NOARP> mtu 1500 master sw1 state disabled priority 32 cost 100

show all...
root@moja-mojo:bridge# ./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
02:00:00:12:01:02 dev eth1 vlan 0 master br0 permanent
00:17:42:8a:b4:05 dev eth1 vlan 0 master br0 permanent
00:17:42:8a:b4:07 dev eth1 self permanent
33:33:00:00:00:01 dev eth1 self permanent
33:33:00:00:00:01 dev gretap0 self permanent
33:33:00:00:00:01 dev br0 self permanent
33:33:00:00:00:01 dev sw1 self permanent
a2:fb:21:4c:47:25 dev sw1-p1 vlan 0 master sw1 permanent
33:33:00:00:00:01 dev sw1-p1 self permanent

Lets see a port that is not attached to a bridge
root@moja-mojo:bridge# ./bridge fdb show brport eth0
33:33:00:00:00:01 self permanent
01:00:5e:00:00:01 self permanent
33:33:ff:22:01:01 self permanent

Lets see a port that is attached to a bridge
root@moja-mojo:bridge# ./bridge fdb show brport eth1
02:00:00:12:01:02 vlan 0 master br0 permanent
00:17:42:8a:b4:05 vlan 0 master br0 permanent
00:17:42:8a:b4:07 self permanent
33:33:00:00:00:01 self permanent

Specify the correct bridge and you get good stuff
root@moja-mojo:bridge# ./bridge fdb show brport eth1 br br0
02:00:00:12:01:02 vlan 0 master br0 permanent
00:17:42:8a:b4:05 vlan 0 master br0 permanent
00:17:42:8a:b4:07 self permanent
33:33:00:00:00:01 self permanent

Specify the wrong bridge and you get good nada
root@moja-mojo:bridge# ./bridge fdb show brport eth1 br sw1

dump only br0
root@moja-mojo:bridge# ./bridge fdb show br br0
02:00:00:12:01:02 dev eth1 vlan 0 master br0 permanent
00:17:42:8a:b4:05 dev eth1 vlan 0 master br0 permanent
00:17:42:8a:b4:07 dev eth1 self permanent
33:33:00:00:00:01 dev eth1 self permanent

Lets move a port from one bridge to another for shits-and-giggles
(as they say in New Brunswick)
root@moja-mojo:bridge# ip link set sw1-p1 master br0

Now dump again br0
root@moja-mojo:bridge# ./bridge fdb show br br0
02:00:00:12:01:02 dev eth1 vlan 0 master br0 permanent
00:17:42:8a:b4:05 dev eth1 vlan 0 master br0 permanent
00:17:42:8a:b4:07 dev eth1 self permanent
33:33:00:00:00:01 dev eth1 self permanent
a2:fb:21:4c:47:25 dev sw1-p1 vlan 0 master br0 permanent
33:33:00:00:00:01 dev sw1-p1 self permanent

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/core/rtnetlink.c |   68 +++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 56 insertions(+), 12 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 064418e..71e6bc8 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2508,26 +2508,70 @@ EXPORT_SYMBOL(ndo_dflt_fdb_dump);
 
 static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
 {
-	int idx = 0;
-	struct net *net = sock_net(skb->sk);
 	struct net_device *dev;
+	struct net_device *br_dev;
+	struct nlattr *tb[IFLA_MAX+1];
+	const struct net_device_ops *ops;
+	struct ifinfomsg *ifm = nlmsg_data(cb->nlh);
+	struct net *net = sock_net(skb->sk);
+	int brport_idx = 0;
+	int br_idx = 0;
+	int idx = 0;
+
+	if (nlmsg_parse(cb->nlh, sizeof(struct ifinfomsg), tb, IFLA_MAX,
+			ifla_policy) == 0) {
+		if (tb[IFLA_MASTER])
+			br_idx = nla_get_u32(tb[IFLA_MASTER]);
+	}
+	
+	brport_idx = ifm->ifi_index;
 
 	rcu_read_lock();
 	for_each_netdev_rcu(net, dev) {
-		if (dev->priv_flags & IFF_BRIDGE_PORT) {
-			struct net_device *br_dev;
-			const struct net_device_ops *ops;
 
-			br_dev = netdev_master_upper_dev_get(dev);
+		if (brport_idx && (dev->ifindex != brport_idx))
+			continue;
+
+		if (!br_idx) {
+			if (dev->priv_flags & IFF_BRIDGE_PORT) {
+				br_dev = netdev_master_upper_dev_get(dev);
+				ops = br_dev->netdev_ops;
+				if (ops->ndo_fdb_dump)
+					idx = ops->ndo_fdb_dump(skb, cb, br_dev,
+								dev, idx);
+			}
+
+			/* all of bridge fdb entries are dumped via brports fdb
+			 * therefore only allow for selfies for bridges
+			*/
+			if (!(dev->priv_flags & IFF_EBRIDGE) &&
+			      dev->netdev_ops->ndo_fdb_dump)
+				idx = dev->netdev_ops->ndo_fdb_dump(skb, cb, dev,
+								    NULL, idx);
+			else
+				idx = ndo_dflt_fdb_dump(skb, cb, dev, NULL, idx);
+
+		} else {
+			if (!(dev->priv_flags & IFF_BRIDGE_PORT))
+				continue;
+
+			br_dev = __dev_get_by_index(net, br_idx);
+			if (!br_dev)
+				return -ENODEV;
+
+			if (br_dev != netdev_master_upper_dev_get(dev))
+				continue;
+
 			ops = br_dev->netdev_ops;
 			if (ops->ndo_fdb_dump)
-				idx = ops->ndo_fdb_dump(skb, cb, dev, NULL, idx);
-		}
+				idx = ops->ndo_fdb_dump(skb, cb, br_dev, dev, idx);
 
-		if (dev->netdev_ops->ndo_fdb_dump)
-			idx = dev->netdev_ops->ndo_fdb_dump(skb, cb, dev, NULL, idx);
-		else
-			idx = ndo_dflt_fdb_dump(skb, cb, dev, NULL, idx);
+			if (dev->netdev_ops->ndo_fdb_dump)
+				idx = dev->netdev_ops->ndo_fdb_dump(skb, cb, dev,
+								    NULL, idx);
+			else
+				idx = ndo_dflt_fdb_dump(skb, cb, dev, NULL, idx);
+		}
 	}
 	rcu_read_unlock();
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [net-next PATCH 2/2] bridge netlink dump interface at par with brctl Actually better than brctl showmacs because we can filter by bridge port in the kernel
  2014-06-01 11:56 ` [net-next PATCH 2/2] bridge netlink dump interface at par with brctl Actually better than brctl showmacs because we can filter by bridge port in the kernel Jamal Hadi Salim
@ 2014-06-01 12:16   ` Jamal Hadi Salim
  2014-06-01 12:24     ` Jamal Hadi Salim
  2014-06-02 15:34   ` Vlad Yasevich
  1 sibling, 1 reply; 8+ messages in thread
From: Jamal Hadi Salim @ 2014-06-01 12:16 UTC (permalink / raw
  To: davem, stephen; +Cc: netdev, vyasevic, sfeldma, john.r.fastabend, roopa


This is mostly to you Vlad since you brought it up earlier.
I ended using ifm instead of ndm. Currently there is lack of
symettry - we send requests with ifm and get responses with
ndms. Unfortunately after spending 2-3 hours I came to the
conclusion i cant change it without breaking old iproute2s that
were expecting this behavior. What we have here is a magnitude
better filtering but we could have done slightly better if we
were able to use an ndm. A little acrobatics later on to filter
by vlans may work..

cheers,
jamal

On 06/01/14 07:56, Jamal Hadi Salim wrote:
> From: Jamal Hadi Salim <jhs@mojatatu.com>
>
> The current bridge netlink interface doesnt scale when you have many bridges each
> with large fdbs or even bridges with many bridge ports
>
> Example usage:
>
> Lets start with two bridges each with a port...
>
> root@moja-mojo:bridge# ./bridge link
> 8: eth1 state DOWN : <BROADCAST,MULTICAST> mtu 1500 master br0 state disabled priority 32 cost 19
> 17: sw1-p1 state DOWN : <BROADCAST,NOARP> mtu 1500 master sw1 state disabled priority 32 cost 100
>
> show all...
> root@moja-mojo:bridge# ./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
> 02:00:00:12:01:02 dev eth1 vlan 0 master br0 permanent
> 00:17:42:8a:b4:05 dev eth1 vlan 0 master br0 permanent
> 00:17:42:8a:b4:07 dev eth1 self permanent
> 33:33:00:00:00:01 dev eth1 self permanent
> 33:33:00:00:00:01 dev gretap0 self permanent
> 33:33:00:00:00:01 dev br0 self permanent
> 33:33:00:00:00:01 dev sw1 self permanent
> a2:fb:21:4c:47:25 dev sw1-p1 vlan 0 master sw1 permanent
> 33:33:00:00:00:01 dev sw1-p1 self permanent
>
> Lets see a port that is not attached to a bridge
> root@moja-mojo:bridge# ./bridge fdb show brport eth0
> 33:33:00:00:00:01 self permanent
> 01:00:5e:00:00:01 self permanent
> 33:33:ff:22:01:01 self permanent
>
> Lets see a port that is attached to a bridge
> root@moja-mojo:bridge# ./bridge fdb show brport eth1
> 02:00:00:12:01:02 vlan 0 master br0 permanent
> 00:17:42:8a:b4:05 vlan 0 master br0 permanent
> 00:17:42:8a:b4:07 self permanent
> 33:33:00:00:00:01 self permanent
>
> Specify the correct bridge and you get good stuff
> root@moja-mojo:bridge# ./bridge fdb show brport eth1 br br0
> 02:00:00:12:01:02 vlan 0 master br0 permanent
> 00:17:42:8a:b4:05 vlan 0 master br0 permanent
> 00:17:42:8a:b4:07 self permanent
> 33:33:00:00:00:01 self permanent
>
> Specify the wrong bridge and you get good nada
> root@moja-mojo:bridge# ./bridge fdb show brport eth1 br sw1
>
> dump only br0
> root@moja-mojo:bridge# ./bridge fdb show br br0
> 02:00:00:12:01:02 dev eth1 vlan 0 master br0 permanent
> 00:17:42:8a:b4:05 dev eth1 vlan 0 master br0 permanent
> 00:17:42:8a:b4:07 dev eth1 self permanent
> 33:33:00:00:00:01 dev eth1 self permanent
>
> Lets move a port from one bridge to another for shits-and-giggles
> (as they say in New Brunswick)
> root@moja-mojo:bridge# ip link set sw1-p1 master br0
>
> Now dump again br0
> root@moja-mojo:bridge# ./bridge fdb show br br0
> 02:00:00:12:01:02 dev eth1 vlan 0 master br0 permanent
> 00:17:42:8a:b4:05 dev eth1 vlan 0 master br0 permanent
> 00:17:42:8a:b4:07 dev eth1 self permanent
> 33:33:00:00:00:01 dev eth1 self permanent
> a2:fb:21:4c:47:25 dev sw1-p1 vlan 0 master br0 permanent
> 33:33:00:00:00:01 dev sw1-p1 self permanent
>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> ---
>   net/core/rtnetlink.c |   68 +++++++++++++++++++++++++++++++++++++++++---------
>   1 file changed, 56 insertions(+), 12 deletions(-)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 064418e..71e6bc8 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -2508,26 +2508,70 @@ EXPORT_SYMBOL(ndo_dflt_fdb_dump);
>
>   static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
>   {
> -	int idx = 0;
> -	struct net *net = sock_net(skb->sk);
>   	struct net_device *dev;
> +	struct net_device *br_dev;
> +	struct nlattr *tb[IFLA_MAX+1];
> +	const struct net_device_ops *ops;
> +	struct ifinfomsg *ifm = nlmsg_data(cb->nlh);
> +	struct net *net = sock_net(skb->sk);
> +	int brport_idx = 0;
> +	int br_idx = 0;
> +	int idx = 0;
> +
> +	if (nlmsg_parse(cb->nlh, sizeof(struct ifinfomsg), tb, IFLA_MAX,
> +			ifla_policy) == 0) {
> +		if (tb[IFLA_MASTER])
> +			br_idx = nla_get_u32(tb[IFLA_MASTER]);
> +	}
> +	
> +	brport_idx = ifm->ifi_index;
>
>   	rcu_read_lock();
>   	for_each_netdev_rcu(net, dev) {
> -		if (dev->priv_flags & IFF_BRIDGE_PORT) {
> -			struct net_device *br_dev;
> -			const struct net_device_ops *ops;
>
> -			br_dev = netdev_master_upper_dev_get(dev);
> +		if (brport_idx && (dev->ifindex != brport_idx))
> +			continue;
> +
> +		if (!br_idx) {
> +			if (dev->priv_flags & IFF_BRIDGE_PORT) {
> +				br_dev = netdev_master_upper_dev_get(dev);
> +				ops = br_dev->netdev_ops;
> +				if (ops->ndo_fdb_dump)
> +					idx = ops->ndo_fdb_dump(skb, cb, br_dev,
> +								dev, idx);
> +			}
> +
> +			/* all of bridge fdb entries are dumped via brports fdb
> +			 * therefore only allow for selfies for bridges
> +			*/
> +			if (!(dev->priv_flags & IFF_EBRIDGE) &&
> +			      dev->netdev_ops->ndo_fdb_dump)
> +				idx = dev->netdev_ops->ndo_fdb_dump(skb, cb, dev,
> +								    NULL, idx);
> +			else
> +				idx = ndo_dflt_fdb_dump(skb, cb, dev, NULL, idx);
> +
> +		} else {
> +			if (!(dev->priv_flags & IFF_BRIDGE_PORT))
> +				continue;
> +
> +			br_dev = __dev_get_by_index(net, br_idx);
> +			if (!br_dev)
> +				return -ENODEV;
> +
> +			if (br_dev != netdev_master_upper_dev_get(dev))
> +				continue;
> +
>   			ops = br_dev->netdev_ops;
>   			if (ops->ndo_fdb_dump)
> -				idx = ops->ndo_fdb_dump(skb, cb, dev, NULL, idx);
> -		}
> +				idx = ops->ndo_fdb_dump(skb, cb, br_dev, dev, idx);
>
> -		if (dev->netdev_ops->ndo_fdb_dump)
> -			idx = dev->netdev_ops->ndo_fdb_dump(skb, cb, dev, NULL, idx);
> -		else
> -			idx = ndo_dflt_fdb_dump(skb, cb, dev, NULL, idx);
> +			if (dev->netdev_ops->ndo_fdb_dump)
> +				idx = dev->netdev_ops->ndo_fdb_dump(skb, cb, dev,
> +								    NULL, idx);
> +			else
> +				idx = ndo_dflt_fdb_dump(skb, cb, dev, NULL, idx);
> +		}
>   	}
>   	rcu_read_unlock();
>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [net-next PATCH 2/2] bridge netlink dump interface at par with brctl Actually better than brctl showmacs because we can filter by bridge port in the kernel
  2014-06-01 12:16   ` Jamal Hadi Salim
@ 2014-06-01 12:24     ` Jamal Hadi Salim
  0 siblings, 0 replies; 8+ messages in thread
From: Jamal Hadi Salim @ 2014-06-01 12:24 UTC (permalink / raw
  To: davem, stephen; +Cc: netdev, vyasevic, sfeldma, john.r.fastabend, roopa

Additional note:
This is also on top of Roopa's patch.

cheers,
jamal

On 06/01/14 08:16, Jamal Hadi Salim wrote:
>
> This is mostly to you Vlad since you brought it up earlier.
> I ended using ifm instead of ndm. Currently there is lack of
> symettry - we send requests with ifm and get responses with
> ndms. Unfortunately after spending 2-3 hours I came to the
> conclusion i cant change it without breaking old iproute2s that
> were expecting this behavior. What we have here is a magnitude
> better filtering but we could have done slightly better if we
> were able to use an ndm. A little acrobatics later on to filter
> by vlans may work..
>
> cheers,
> jamal
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [net-next PATCH 2/2] bridge netlink dump interface at par with brctl Actually better than brctl showmacs because we can filter by bridge port in the kernel
  2014-06-01 11:56 ` [net-next PATCH 2/2] bridge netlink dump interface at par with brctl Actually better than brctl showmacs because we can filter by bridge port in the kernel Jamal Hadi Salim
  2014-06-01 12:16   ` Jamal Hadi Salim
@ 2014-06-02 15:34   ` Vlad Yasevich
  2014-06-02 22:17     ` Jamal Hadi Salim
  1 sibling, 1 reply; 8+ messages in thread
From: Vlad Yasevich @ 2014-06-02 15:34 UTC (permalink / raw
  To: Jamal Hadi Salim, davem, stephen; +Cc: netdev, sfeldma, john.r.fastabend, roopa

On 06/01/2014 07:56 AM, Jamal Hadi Salim wrote:
> From: Jamal Hadi Salim <jhs@mojatatu.com>
> 
> The current bridge netlink interface doesnt scale when you have many bridges each
> with large fdbs or even bridges with many bridge ports
> 
> Example usage:
> 
> Lets start with two bridges each with a port...
> 
> root@moja-mojo:bridge# ./bridge link
> 8: eth1 state DOWN : <BROADCAST,MULTICAST> mtu 1500 master br0 state disabled priority 32 cost 19
> 17: sw1-p1 state DOWN : <BROADCAST,NOARP> mtu 1500 master sw1 state disabled priority 32 cost 100
> 
> show all...
> root@moja-mojo:bridge# ./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
> 02:00:00:12:01:02 dev eth1 vlan 0 master br0 permanent
> 00:17:42:8a:b4:05 dev eth1 vlan 0 master br0 permanent
> 00:17:42:8a:b4:07 dev eth1 self permanent
> 33:33:00:00:00:01 dev eth1 self permanent
> 33:33:00:00:00:01 dev gretap0 self permanent
> 33:33:00:00:00:01 dev br0 self permanent
> 33:33:00:00:00:01 dev sw1 self permanent
> a2:fb:21:4c:47:25 dev sw1-p1 vlan 0 master sw1 permanent
> 33:33:00:00:00:01 dev sw1-p1 self permanent
> 
> Lets see a port that is not attached to a bridge
> root@moja-mojo:bridge# ./bridge fdb show brport eth0
> 33:33:00:00:00:01 self permanent
> 01:00:5e:00:00:01 self permanent
> 33:33:ff:22:01:01 self permanent
> 
> Lets see a port that is attached to a bridge
> root@moja-mojo:bridge# ./bridge fdb show brport eth1
> 02:00:00:12:01:02 vlan 0 master br0 permanent
> 00:17:42:8a:b4:05 vlan 0 master br0 permanent
> 00:17:42:8a:b4:07 self permanent
> 33:33:00:00:00:01 self permanent
> 
> Specify the correct bridge and you get good stuff
> root@moja-mojo:bridge# ./bridge fdb show brport eth1 br br0
> 02:00:00:12:01:02 vlan 0 master br0 permanent
> 00:17:42:8a:b4:05 vlan 0 master br0 permanent
> 00:17:42:8a:b4:07 self permanent
> 33:33:00:00:00:01 self permanent
> 
> Specify the wrong bridge and you get good nada
> root@moja-mojo:bridge# ./bridge fdb show brport eth1 br sw1
> 
> dump only br0
> root@moja-mojo:bridge# ./bridge fdb show br br0
> 02:00:00:12:01:02 dev eth1 vlan 0 master br0 permanent
> 00:17:42:8a:b4:05 dev eth1 vlan 0 master br0 permanent
> 00:17:42:8a:b4:07 dev eth1 self permanent
> 33:33:00:00:00:01 dev eth1 self permanent
> 
> Lets move a port from one bridge to another for shits-and-giggles
> (as they say in New Brunswick)
> root@moja-mojo:bridge# ip link set sw1-p1 master br0
> 
> Now dump again br0
> root@moja-mojo:bridge# ./bridge fdb show br br0
> 02:00:00:12:01:02 dev eth1 vlan 0 master br0 permanent
> 00:17:42:8a:b4:05 dev eth1 vlan 0 master br0 permanent
> 00:17:42:8a:b4:07 dev eth1 self permanent
> 33:33:00:00:00:01 dev eth1 self permanent
> a2:fb:21:4c:47:25 dev sw1-p1 vlan 0 master br0 permanent
> 33:33:00:00:00:01 dev sw1-p1 self permanent
> 
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> ---
>  net/core/rtnetlink.c |   68 +++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 56 insertions(+), 12 deletions(-)
> 
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 064418e..71e6bc8 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -2508,26 +2508,70 @@ EXPORT_SYMBOL(ndo_dflt_fdb_dump);
>  
>  static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
>  {
> -	int idx = 0;
> -	struct net *net = sock_net(skb->sk);
>  	struct net_device *dev;
> +	struct net_device *br_dev;
> +	struct nlattr *tb[IFLA_MAX+1];
> +	const struct net_device_ops *ops;
> +	struct ifinfomsg *ifm = nlmsg_data(cb->nlh);
> +	struct net *net = sock_net(skb->sk);
> +	int brport_idx = 0;
> +	int br_idx = 0;
> +	int idx = 0;
> +
> +	if (nlmsg_parse(cb->nlh, sizeof(struct ifinfomsg), tb, IFLA_MAX,
> +			ifla_policy) == 0) {
> +		if (tb[IFLA_MASTER])
> +			br_idx = nla_get_u32(tb[IFLA_MASTER]);
> +	}
> +	
> +	brport_idx = ifm->ifi_index;
>  
>  	rcu_read_lock();
>  	for_each_netdev_rcu(net, dev) {
> -		if (dev->priv_flags & IFF_BRIDGE_PORT) {
> -			struct net_device *br_dev;
> -			const struct net_device_ops *ops;
>  
> -			br_dev = netdev_master_upper_dev_get(dev);
> +		if (brport_idx && (dev->ifindex != brport_idx))
> +			continue;
> +
> +		if (!br_idx) {
> +			if (dev->priv_flags & IFF_BRIDGE_PORT) {
> +				br_dev = netdev_master_upper_dev_get(dev);
> +				ops = br_dev->netdev_ops;
> +				if (ops->ndo_fdb_dump)
> +					idx = ops->ndo_fdb_dump(skb, cb, br_dev,
> +								dev, idx);
> +			}
> +
> +			/* all of bridge fdb entries are dumped via brports fdb
> +			 * therefore only allow for selfies for bridges
> +			*/
> +			if (!(dev->priv_flags & IFF_EBRIDGE) &&
> +			      dev->netdev_ops->ndo_fdb_dump)
> +				idx = dev->netdev_ops->ndo_fdb_dump(skb, cb, dev,
> +								    NULL, idx);
> +			else
> +				idx = ndo_dflt_fdb_dump(skb, cb, dev, NULL, idx);
> +
> +		} else {
> +			if (!(dev->priv_flags & IFF_BRIDGE_PORT))
> +				continue;
> +
> +			br_dev = __dev_get_by_index(net, br_idx);
> +			if (!br_dev)
> +				return -ENODEV;
> +
> +			if (br_dev != netdev_master_upper_dev_get(dev))
> +				continue;
> +

I think that after this code, if you set a bridge mac address thus
causing an fdb like:
  <mac> dev br0 vlan 0 master permanent  (old notation)

you will not show it if you set the br_idx with
  # bridge fdb show br br0


I looks like the only way to show such fdb is not set any filters at all
since if you set a port filter, you will not see it either as it will be
filtered out in bridge code.

-vlad

>  			ops = br_dev->netdev_ops;
>  			if (ops->ndo_fdb_dump)
> -				idx = ops->ndo_fdb_dump(skb, cb, dev, NULL, idx);
> -		}
> +				idx = ops->ndo_fdb_dump(skb, cb, br_dev, dev, idx);
>  
> -		if (dev->netdev_ops->ndo_fdb_dump)
> -			idx = dev->netdev_ops->ndo_fdb_dump(skb, cb, dev, NULL, idx);
> -		else
> -			idx = ndo_dflt_fdb_dump(skb, cb, dev, NULL, idx);
> +			if (dev->netdev_ops->ndo_fdb_dump)
> +				idx = dev->netdev_ops->ndo_fdb_dump(skb, cb, dev,
> +								    NULL, idx);
> +			else
> +				idx = ndo_dflt_fdb_dump(skb, cb, dev, NULL, idx);
> +		}
>  	}
>  	rcu_read_unlock();
>  
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [net-next PATCH 2/2] bridge netlink dump interface at par with brctl Actually better than brctl showmacs because we can filter by bridge port in the kernel
  2014-06-02 15:34   ` Vlad Yasevich
@ 2014-06-02 22:17     ` Jamal Hadi Salim
  0 siblings, 0 replies; 8+ messages in thread
From: Jamal Hadi Salim @ 2014-06-02 22:17 UTC (permalink / raw
  To: vyasevic, davem, stephen; +Cc: netdev, sfeldma, john.r.fastabend, roopa

On 06/02/14 11:34, Vlad Yasevich wrote:

>
> I think that after this code, if you set a bridge mac address thus
> causing an fdb like:
>    <mac> dev br0 vlan 0 master permanent  (old notation)
>
> you will not show it if you set the br_idx with
>    # bridge fdb show br br0
>
>
> I looks like the only way to show such fdb is not set any filters at all
> since if you set a port filter, you will not see it either as it will be
> filtered out in bridge code.
>

I thought the comment which says "selfie" would take care of that; i.e
the default dump would do it.
If you give me an example of setting such an entry I will try it out
and see if it works.

cheers,
jamal

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [net-next PATCH 1/2] bridge fdb dumping takes a filter device Dumping a bridge fdb dumps every fdb entry held. With this change we are going to filter on selected bridge port.
  2014-06-01 11:56 [net-next PATCH 1/2] bridge fdb dumping takes a filter device Dumping a bridge fdb dumps every fdb entry held. With this change we are going to filter on selected bridge port Jamal Hadi Salim
  2014-06-01 11:56 ` [net-next PATCH 2/2] bridge netlink dump interface at par with brctl Actually better than brctl showmacs because we can filter by bridge port in the kernel Jamal Hadi Salim
@ 2014-06-05  7:15 ` David Miller
  2014-06-07 12:41   ` Jamal Hadi Salim
  1 sibling, 1 reply; 8+ messages in thread
From: David Miller @ 2014-06-05  7:15 UTC (permalink / raw
  To: jhs; +Cc: stephen, netdev, vyasevic, sfeldma, john.r.fastabend, roopa

From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: Sun,  1 Jun 2014 07:56:19 -0400

> From: Jamal Hadi Salim <jhs@mojatatu.com>
> 
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

Jamal take it easy with the subject lines :-)

The subject line should be as concise, and be formatted as:

[net-next PATCH N/M] ${SUBSYSTEM}: Description.

And "Description." needs to be shorter and more reasonable that what
you supplied here.  Much of that text belongs in the commit message
body instead.

Same goes for patch #2 of this series.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [net-next PATCH 1/2] bridge fdb dumping takes a filter device Dumping a bridge fdb dumps every fdb entry held. With this change we are going to filter on selected bridge port.
  2014-06-05  7:15 ` [net-next PATCH 1/2] bridge fdb dumping takes a filter device Dumping a bridge fdb dumps every fdb entry held. With this change we are going to filter on selected bridge port David Miller
@ 2014-06-07 12:41   ` Jamal Hadi Salim
  0 siblings, 0 replies; 8+ messages in thread
From: Jamal Hadi Salim @ 2014-06-07 12:41 UTC (permalink / raw
  To: David Miller; +Cc: stephen, netdev, vyasevic, sfeldma, john.r.fastabend, roopa

On 06/05/14 03:15, David Miller wrote:

>
> Jamal take it easy with the subject lines :-)
>

LOL.Cant think of anything clever to say to that.
Maybe you should just accept my attachments ;->

Anyways, I have to resend the patches to consider
the comments from Vlad.

cheers,
jamal

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-06-07 12:41 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-01 11:56 [net-next PATCH 1/2] bridge fdb dumping takes a filter device Dumping a bridge fdb dumps every fdb entry held. With this change we are going to filter on selected bridge port Jamal Hadi Salim
2014-06-01 11:56 ` [net-next PATCH 2/2] bridge netlink dump interface at par with brctl Actually better than brctl showmacs because we can filter by bridge port in the kernel Jamal Hadi Salim
2014-06-01 12:16   ` Jamal Hadi Salim
2014-06-01 12:24     ` Jamal Hadi Salim
2014-06-02 15:34   ` Vlad Yasevich
2014-06-02 22:17     ` Jamal Hadi Salim
2014-06-05  7:15 ` [net-next PATCH 1/2] bridge fdb dumping takes a filter device Dumping a bridge fdb dumps every fdb entry held. With this change we are going to filter on selected bridge port David Miller
2014-06-07 12:41   ` Jamal Hadi Salim

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.