[RFC PATCH net-next 0/3] Allow bridge to function in non-promisc mode

[RFC PATCH net-next 0/3] Allow bridge to function in non-promisc mode

[Vlad Yasevich]

This is a first stab at the series that allows bridge to function
in non-promiscuouse mode.

The series adds a sysfs control to turn off promiscusouse mode all on ports of
the bridge.  To handle multicast, we turn on IFF_ALLMULTI for now.  Eventually
we would deal with multicast addresses.

The second patch adds an ability to designate an uplink port on the bridge.

Finally, we add an ability to add hw address to the bridge and sync those
addresses to the uplink ports.

In this way a bridge could be programmed with appropriate mac addresses and
could function in non-promiscuouse mode.

All comments are welcome.

Vlad Yasevich (3):
  bridge: Add mac_management sysfs interface
  bridge: Allow an ability to designate an uplink port
  bridge: Implement IFF_UNICAST_FLT

 include/uapi/linux/if_link.h |    1 +
 net/bridge/br_device.c       |   51 +++++++++++++++++++++++++++++++++++++++++-
 net/bridge/br_fdb.c          |    6 +++++
 net/bridge/br_if.c           |   38 +++++++++++++++++++++++++++----
 net/bridge/br_netlink.c      |    6 +++++
 net/bridge/br_private.h      |    6 +++++
 net/bridge/br_sysfs_br.c     |   17 ++++++++++++++
 net/bridge/br_sysfs_if.c     |   13 ++++++++++
 8 files changed, 132 insertions(+), 6 deletions(-)

-- 
1.7.7.6

[RFC PATCH net-next 1/3] bridge: Add mac_management sysfs interface

[Vlad Yasevich]

Add an sysfs interface to turn MAC address management on an off.
When MAC management is off (default), all interfaces in the bridge
are in promisc mode.  When MAC management is on, promisc mode is
turned off and the expectation is that user will manually program
the MAC filter.

Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
 net/bridge/br_device.c   |   41 +++++++++++++++++++++++++++++++++++++++++
 net/bridge/br_if.c       |   22 +++++++++++++++++-----
 net/bridge/br_private.h  |    2 ++
 net/bridge/br_sysfs_br.c |   17 +++++++++++++++++
 4 files changed, 77 insertions(+), 5 deletions(-)

diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index d5f1d3f..160bc74 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -185,6 +185,46 @@ static int br_set_mac_address(struct net_device *dev, void *p)
 	return 0;
 }
 
+int br_set_promisc(struct net_bridge* br, unsigned long val)
+{
+	struct net_bridge_port *p;
+
+	if (!rtnl_trylock())
+		return restart_syscall();
+
+	spin_lock_bh(&br->lock);
+
+	if (val) {
+		if (br->promisc_enabled)
+			goto unlock;
+
+		br->promisc_enabled = 1;
+		/* For each port on the bridge, turn on promisc mode and
+		 * turn off ALLMULTI to handle multicast traffic.
+		 */
+		list_for_each_entry(p, &br->port_list, list) {
+			dev_set_promiscuity(p->dev, 1);
+			dev_set_allmulti(p->dev, -1);
+		}
+
+	} else {
+		if (!br->promisc_enabled)
+			goto unlock;
+
+		br->promisc_enabled = 0;
+		/* For each port on the bridge, turn off promisc mode */
+		list_for_each_entry(p, &br->port_list, list) {
+			dev_set_allmulti(p->dev, 1);
+			dev_set_promiscuity(p->dev, -1);
+		}
+	}
+unlock:
+	spin_unlock_bh(&br->lock);
+	rtnl_unlock();
+	return 0;
+}
+
+
 static void br_getinfo(struct net_device *dev, struct ethtool_drvinfo *info)
 {
 	strlcpy(info->driver, "bridge", sizeof(info->driver));
@@ -371,6 +411,7 @@ void br_dev_setup(struct net_device *dev)
 	br->bridge_hello_time = br->hello_time = 2 * HZ;
 	br->bridge_forward_delay = br->forward_delay = 15 * HZ;
 	br->ageing_time = 300 * HZ;
+	br->promisc_enabled = 1;
 
 	br_netfilter_rtable_init(br);
 	br_stp_timer_init(br);
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index ef1b914..02b4440 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -132,7 +132,10 @@ static void del_nbp(struct net_bridge_port *p)
 
 	sysfs_remove_link(br->ifobj, p->dev->name);
 
-	dev_set_promiscuity(dev, -1);
+	if (br->promisc_enabled)
+		dev_set_promiscuity(dev, -1);
+	else
+		dev_set_allmulti(dev, -1);
 
 	spin_lock_bh(&br->lock);
 	br_stp_disable_port(p);
@@ -350,9 +353,15 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
 
 	call_netdevice_notifiers(NETDEV_JOIN, dev);
 
-	err = dev_set_promiscuity(dev, 1);
-	if (err)
-		goto put_back;
+	if (br->promisc_enabled) {
+		err = dev_set_promiscuity(dev, 1);
+		if (err)
+			goto put_back;
+	} else {
+		err = dev_set_allmulti(dev, 1);
+		if (err)
+			goto put_back;
+	}
 
 	err = kobject_init_and_add(&p->kobj, &brport_ktype, &(dev->dev.kobj),
 				   SYSFS_BRIDGE_PORT_ATTR);
@@ -414,7 +423,10 @@ err2:
 	kobject_put(&p->kobj);
 	p = NULL; /* kobject_put frees */
 err1:
-	dev_set_promiscuity(dev, -1);
+	if (br->promisc_enabled)
+		dev_set_promiscuity(dev, -1);
+	else
+		dev_set_allmulti(dev, -1);
 put_back:
 	dev_put(dev);
 	kfree(p);
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 6d314c4..4a0fa29 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -277,6 +277,7 @@ struct net_bridge
 	struct timer_list		topology_change_timer;
 	struct timer_list		gc_timer;
 	struct kobject			*ifobj;
+	u8				promisc_enabled;
 #ifdef CONFIG_BRIDGE_VLAN_FILTERING
 	u8				vlan_enabled;
 	struct net_port_vlans __rcu	*vlan_info;
@@ -327,6 +328,7 @@ extern void br_dev_setup(struct net_device *dev);
 extern void br_dev_delete(struct net_device *dev, struct list_head *list);
 extern netdev_tx_t br_dev_xmit(struct sk_buff *skb,
 			       struct net_device *dev);
+extern int br_set_promisc(struct net_bridge *br, unsigned long val);
 #ifdef CONFIG_NET_POLL_CONTROLLER
 static inline struct netpoll_info *br_netpoll_info(struct net_bridge *br)
 {
diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
index 8baa9c0..5489219 100644
--- a/net/bridge/br_sysfs_br.c
+++ b/net/bridge/br_sysfs_br.c
@@ -710,6 +710,22 @@ static DEVICE_ATTR(vlan_filtering, S_IRUGO | S_IWUSR,
 		   show_vlan_filtering, store_vlan_filtering);
 #endif
 
+static ssize_t show_promisc(struct device *d,
+			    struct device_attribute *attr, char *buf)
+{
+	struct net_bridge *br = to_bridge(d);
+	return sprintf(buf, "%d\n", br->promisc_enabled);
+}
+
+static ssize_t store_promisc(struct device *d,
+			     struct device_attribute *attr,
+			     const char *buf, size_t len)
+{
+	return store_bridge_parm(d, buf, len, br_set_promisc);
+}
+static DEVICE_ATTR(promisc, S_IRUGO | S_IWUSR, show_promisc,
+		   store_promisc);
+
 static struct attribute *bridge_attrs[] = {
 	&dev_attr_forward_delay.attr,
 	&dev_attr_hello_time.attr,
@@ -753,6 +769,7 @@ static struct attribute *bridge_attrs[] = {
 #ifdef CONFIG_BRIDGE_VLAN_FILTERING
 	&dev_attr_vlan_filtering.attr,
 #endif
+	&dev_attr_promisc.attr,
 	NULL
 };
 
-- 
1.7.7.6

Re: [RFC PATCH net-next 1/3] bridge: Add mac_management sysfs interface

[Vlad Yasevich]

On 03/06/2013 09:31 PM, Vlad Yasevich wrote:
> Add an sysfs interface to turn MAC address management on an off.
> When MAC management is off (default), all interfaces in the bridge
> are in promisc mode.  When MAC management is on, promisc mode is
> turned off and the expectation is that user will manually program
> the MAC filter.

Sorry, the description is stale,  I forgot to update it after fixing up
the patch.  The change is is actually to add sysfs to control promisc
mode.  By default, it's on.  It can be turned off if user wants to.

-vlad

>
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> ---
>   net/bridge/br_device.c   |   41 +++++++++++++++++++++++++++++++++++++++++
>   net/bridge/br_if.c       |   22 +++++++++++++++++-----
>   net/bridge/br_private.h  |    2 ++
>   net/bridge/br_sysfs_br.c |   17 +++++++++++++++++
>   4 files changed, 77 insertions(+), 5 deletions(-)
>
> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
> index d5f1d3f..160bc74 100644
> --- a/net/bridge/br_device.c
> +++ b/net/bridge/br_device.c
> @@ -185,6 +185,46 @@ static int br_set_mac_address(struct net_device *dev, void *p)
>   	return 0;
>   }
>
> +int br_set_promisc(struct net_bridge* br, unsigned long val)
> +{
> +	struct net_bridge_port *p;
> +
> +	if (!rtnl_trylock())
> +		return restart_syscall();
> +
> +	spin_lock_bh(&br->lock);
> +
> +	if (val) {
> +		if (br->promisc_enabled)
> +			goto unlock;
> +
> +		br->promisc_enabled = 1;
> +		/* For each port on the bridge, turn on promisc mode and
> +		 * turn off ALLMULTI to handle multicast traffic.
> +		 */
> +		list_for_each_entry(p, &br->port_list, list) {
> +			dev_set_promiscuity(p->dev, 1);
> +			dev_set_allmulti(p->dev, -1);
> +		}
> +
> +	} else {
> +		if (!br->promisc_enabled)
> +			goto unlock;
> +
> +		br->promisc_enabled = 0;
> +		/* For each port on the bridge, turn off promisc mode */
> +		list_for_each_entry(p, &br->port_list, list) {
> +			dev_set_allmulti(p->dev, 1);
> +			dev_set_promiscuity(p->dev, -1);
> +		}
> +	}
> +unlock:
> +	spin_unlock_bh(&br->lock);
> +	rtnl_unlock();
> +	return 0;
> +}
> +
> +
>   static void br_getinfo(struct net_device *dev, struct ethtool_drvinfo *info)
>   {
>   	strlcpy(info->driver, "bridge", sizeof(info->driver));
> @@ -371,6 +411,7 @@ void br_dev_setup(struct net_device *dev)
>   	br->bridge_hello_time = br->hello_time = 2 * HZ;
>   	br->bridge_forward_delay = br->forward_delay = 15 * HZ;
>   	br->ageing_time = 300 * HZ;
> +	br->promisc_enabled = 1;
>
>   	br_netfilter_rtable_init(br);
>   	br_stp_timer_init(br);
> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> index ef1b914..02b4440 100644
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -132,7 +132,10 @@ static void del_nbp(struct net_bridge_port *p)
>
>   	sysfs_remove_link(br->ifobj, p->dev->name);
>
> -	dev_set_promiscuity(dev, -1);
> +	if (br->promisc_enabled)
> +		dev_set_promiscuity(dev, -1);
> +	else
> +		dev_set_allmulti(dev, -1);
>
>   	spin_lock_bh(&br->lock);
>   	br_stp_disable_port(p);
> @@ -350,9 +353,15 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
>
>   	call_netdevice_notifiers(NETDEV_JOIN, dev);
>
> -	err = dev_set_promiscuity(dev, 1);
> -	if (err)
> -		goto put_back;
> +	if (br->promisc_enabled) {
> +		err = dev_set_promiscuity(dev, 1);
> +		if (err)
> +			goto put_back;
> +	} else {
> +		err = dev_set_allmulti(dev, 1);
> +		if (err)
> +			goto put_back;
> +	}
>
>   	err = kobject_init_and_add(&p->kobj, &brport_ktype, &(dev->dev.kobj),
>   				   SYSFS_BRIDGE_PORT_ATTR);
> @@ -414,7 +423,10 @@ err2:
>   	kobject_put(&p->kobj);
>   	p = NULL; /* kobject_put frees */
>   err1:
> -	dev_set_promiscuity(dev, -1);
> +	if (br->promisc_enabled)
> +		dev_set_promiscuity(dev, -1);
> +	else
> +		dev_set_allmulti(dev, -1);
>   put_back:
>   	dev_put(dev);
>   	kfree(p);
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 6d314c4..4a0fa29 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -277,6 +277,7 @@ struct net_bridge
>   	struct timer_list		topology_change_timer;
>   	struct timer_list		gc_timer;
>   	struct kobject			*ifobj;
> +	u8				promisc_enabled;
>   #ifdef CONFIG_BRIDGE_VLAN_FILTERING
>   	u8				vlan_enabled;
>   	struct net_port_vlans __rcu	*vlan_info;
> @@ -327,6 +328,7 @@ extern void br_dev_setup(struct net_device *dev);
>   extern void br_dev_delete(struct net_device *dev, struct list_head *list);
>   extern netdev_tx_t br_dev_xmit(struct sk_buff *skb,
>   			       struct net_device *dev);
> +extern int br_set_promisc(struct net_bridge *br, unsigned long val);
>   #ifdef CONFIG_NET_POLL_CONTROLLER
>   static inline struct netpoll_info *br_netpoll_info(struct net_bridge *br)
>   {
> diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
> index 8baa9c0..5489219 100644
> --- a/net/bridge/br_sysfs_br.c
> +++ b/net/bridge/br_sysfs_br.c
> @@ -710,6 +710,22 @@ static DEVICE_ATTR(vlan_filtering, S_IRUGO | S_IWUSR,
>   		   show_vlan_filtering, store_vlan_filtering);
>   #endif
>
> +static ssize_t show_promisc(struct device *d,
> +			    struct device_attribute *attr, char *buf)
> +{
> +	struct net_bridge *br = to_bridge(d);
> +	return sprintf(buf, "%d\n", br->promisc_enabled);
> +}
> +
> +static ssize_t store_promisc(struct device *d,
> +			     struct device_attribute *attr,
> +			     const char *buf, size_t len)
> +{
> +	return store_bridge_parm(d, buf, len, br_set_promisc);
> +}
> +static DEVICE_ATTR(promisc, S_IRUGO | S_IWUSR, show_promisc,
> +		   store_promisc);
> +
>   static struct attribute *bridge_attrs[] = {
>   	&dev_attr_forward_delay.attr,
>   	&dev_attr_hello_time.attr,
> @@ -753,6 +769,7 @@ static struct attribute *bridge_attrs[] = {
>   #ifdef CONFIG_BRIDGE_VLAN_FILTERING
>   	&dev_attr_vlan_filtering.attr,
>   #endif
> +	&dev_attr_promisc.attr,
>   	NULL
>   };
>
>

[RFC PATCH net-next 2/3] bridge: Allow an ability to designate an uplink port

[Vlad Yasevich]

Allow a ports to be designated as uplink.  Multiple ports
may be designated as uplinks and they will be kept in a
list.

Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
 include/uapi/linux/if_link.h |    1 +
 net/bridge/br_device.c       |    1 +
 net/bridge/br_if.c           |   16 ++++++++++++++++
 net/bridge/br_netlink.c      |    6 ++++++
 net/bridge/br_private.h      |    4 ++++
 net/bridge/br_sysfs_if.c     |   13 +++++++++++++
 6 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index c4edfe1..b9444e9 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -220,6 +220,7 @@ enum {
 	IFLA_BRPORT_GUARD,	/* bpdu guard              */
 	IFLA_BRPORT_PROTECT,	/* root port protection    */
 	IFLA_BRPORT_FAST_LEAVE,	/* multicast fast leave    */
+	IFLA_BRPORT_UPLINK,	/* uplink port */
 	__IFLA_BRPORT_MAX
 };
 #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 160bc74..c525e36 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -396,6 +396,7 @@ void br_dev_setup(struct net_device *dev)
 	br->dev = dev;
 	spin_lock_init(&br->lock);
 	INIT_LIST_HEAD(&br->port_list);
+	INIT_LIST_HEAD(&br->uplink_list);
 	spin_lock_init(&br->hash_lock);
 
 	br->bridge_id.prio[0] = 0x80;
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 02b4440..4b0636b 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -131,6 +131,7 @@ static void del_nbp(struct net_bridge_port *p)
 	struct net_device *dev = p->dev;
 
 	sysfs_remove_link(br->ifobj, p->dev->name);
+	list_del_rcu(&p->uplink_list);
 
 	if (br->promisc_enabled)
 		dev_set_promiscuity(dev, -1);
@@ -461,6 +462,21 @@ int br_del_if(struct net_bridge *br, struct net_device *dev)
 	return 0;
 }
 
+void br_set_uplink(struct net_bridge_port *p, unsigned long v)
+{
+	if (v) {
+		if (!list_empty(&p->uplink_list))
+			return;
+
+		list_add_rcu(&p->uplink_list, &p->br->uplink_list);
+	} else {
+		if (list_empty(&p->uplink_list))
+			return;
+
+		list_del_rcu(&p->uplink_list);
+	}
+}
+
 void __net_exit br_net_exit(struct net *net)
 {
 	struct net_device *dev;
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 27aa3ee..8965b51 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -283,6 +283,7 @@ static const struct nla_policy ifla_brport_policy[IFLA_BRPORT_MAX + 1] = {
 	[IFLA_BRPORT_MODE]	= { .type = NLA_U8 },
 	[IFLA_BRPORT_GUARD]	= { .type = NLA_U8 },
 	[IFLA_BRPORT_PROTECT]	= { .type = NLA_U8 },
+	[IFLA_BRPORT_UPLINK]	= { .type = NLA_U8 },
 };
 
 /* Change the state of the port and notify spanning tree */
@@ -347,6 +348,11 @@ static int br_setport(struct net_bridge_port *p, struct nlattr *tb[])
 		if (err)
 			return err;
 	}
+
+	if (tb[IFLA_BRPORT_UPLINK]) {
+		br_set_uplink(p, nla_get_u8(tb[IFLA_BRPORT_UPLINK]));
+	}
+
 	return 0;
 }
 
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 4a0fa29..9502b1e 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -130,6 +130,7 @@ struct net_bridge_port
 	struct net_bridge		*br;
 	struct net_device		*dev;
 	struct list_head		list;
+	struct list_head		uplink_list;
 
 	/* STP */
 	u8				priority;
@@ -277,6 +278,7 @@ struct net_bridge
 	struct timer_list		topology_change_timer;
 	struct timer_list		gc_timer;
 	struct kobject			*ifobj;
+	struct list_head		uplink_list;
 	u8				promisc_enabled;
 #ifdef CONFIG_BRIDGE_VLAN_FILTERING
 	u8				vlan_enabled;
@@ -428,6 +430,8 @@ extern int br_del_if(struct net_bridge *br,
 extern int br_min_mtu(const struct net_bridge *br);
 extern netdev_features_t br_features_recompute(struct net_bridge *br,
 	netdev_features_t features);
+extern void br_set_uplink(struct net_bridge_port *port, unsigned long v);
+
 
 /* br_input.c */
 extern int br_handle_frame_finish(struct sk_buff *skb);
diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
index a1ef1b6..5e1d861 100644
--- a/net/bridge/br_sysfs_if.c
+++ b/net/bridge/br_sysfs_if.c
@@ -159,6 +159,18 @@ BRPORT_ATTR_FLAG(hairpin_mode, BR_HAIRPIN_MODE);
 BRPORT_ATTR_FLAG(bpdu_guard, BR_BPDU_GUARD);
 BRPORT_ATTR_FLAG(root_block, BR_ROOT_BLOCK);
 
+static ssize_t show_uplink(struct net_bridge_port *p, char *buf)
+{
+	return sprintf(buf, "%d\n", list_empty(&p->uplink_list) ? 0 : 1);
+}
+
+static int store_uplink(struct net_bridge_port *p, unsigned long v)
+{
+	br_set_uplink(p, v);
+	return 0;
+}
+static BRPORT_ATTR(uplink, S_IRUGO | S_IWUSR, show_uplink, store_uplink);
+
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 static ssize_t show_multicast_router(struct net_bridge_port *p, char *buf)
 {
@@ -195,6 +207,7 @@ static const struct brport_attribute *brport_attrs[] = {
 	&brport_attr_hairpin_mode,
 	&brport_attr_bpdu_guard,
 	&brport_attr_root_block,
+	&brport_attr_uplink,
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 	&brport_attr_multicast_router,
 	&brport_attr_multicast_fast_leave,
-- 
1.7.7.6

[RFC PATCH net-next 3/3] bridge: Implement IFF_UNICAST_FLT

[Vlad Yasevich]

Implement IFF_UNICAST_FLT on the bridge.  Unicast addresses added
to the bridge device are synched to the uplink devices.  This
allows for uplink devices to change while preserving mac assignment.

Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
 net/bridge/br_device.c |    9 ++++++++-
 net/bridge/br_fdb.c    |    6 ++++++
 2 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index c525e36..61514c1 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -106,6 +106,13 @@ static int br_dev_open(struct net_device *dev)
 
 static void br_dev_set_multicast_list(struct net_device *dev)
 {
+	struct net_bridge *br = netdev_priv(dev);
+	struct net_bridge_port *uplink;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(uplink, &br->uplink_list, uplink_list)
+		dev_uc_sync(uplink->dev, dev);
+	rcu_read_unlock();
 }
 
 static int br_dev_stop(struct net_device *dev)
@@ -384,7 +391,7 @@ void br_dev_setup(struct net_device *dev)
 	SET_ETHTOOL_OPS(dev, &br_ethtool_ops);
 	SET_NETDEV_DEVTYPE(dev, &br_type);
 	dev->tx_queue_len = 0;
-	dev->priv_flags = IFF_EBRIDGE;
+	dev->priv_flags = IFF_EBRIDGE | IFF_UNICAST_FLT;
 
 	dev->features = NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_HIGHDMA |
 			NETIF_F_GSO_MASK | NETIF_F_HW_CSUM | NETIF_F_LLTX |
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index b0812c9..ef7b51e 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -677,6 +677,9 @@ int br_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
 	struct net_port_vlans *pv;
 	unsigned short vid = VLAN_N_VID;
 
+	if ((ndm->ndm_flags & NTF_SELF) && (dev->priv_flags & IFF_EBRIDGE))
+		return ndo_dflt_fdb_add(ndm, tb, dev, addr, nlh_flags);
+
 	if (!(ndm->ndm_state & (NUD_PERMANENT|NUD_NOARP|NUD_REACHABLE))) {
 		pr_info("bridge: RTM_NEWNEIGH with invalid state %#x\n", ndm->ndm_state);
 		return -EINVAL;
@@ -774,6 +777,9 @@ int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
 	struct net_port_vlans *pv;
 	unsigned short vid = VLAN_N_VID;
 
+	if ((ndm->ndm_flags & NTF_SELF) && (dev->priv_flags & IFF_EBRIDGE))
+		return ndo_dflt_fdb_del(ndm, tb, dev, addr);
+
 	if (tb[NDA_VLAN]) {
 		if (nla_len(tb[NDA_VLAN]) != sizeof(unsigned short)) {
 			pr_info("bridge: RTM_NEWNEIGH with invalid vlan\n");
-- 
1.7.7.6

Re: [RFC PATCH net-next 3/3] bridge: Implement IFF_UNICAST_FLT

[John Fastabend]

On 03/06/2013 06:31 PM, Vlad Yasevich wrote:
> Implement IFF_UNICAST_FLT on the bridge.  Unicast addresses added
> to the bridge device are synched to the uplink devices.  This
> allows for uplink devices to change while preserving mac assignment.
>
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> ---

[...]

> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index b0812c9..ef7b51e 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -677,6 +677,9 @@ int br_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
>   	struct net_port_vlans *pv;
>   	unsigned short vid = VLAN_N_VID;
>
> +	if ((ndm->ndm_flags & NTF_SELF) && (dev->priv_flags & IFF_EBRIDGE))
> +		return ndo_dflt_fdb_add(ndm, tb, dev, addr, nlh_flags);
> +
>   	if (!(ndm->ndm_state & (NUD_PERMANENT|NUD_NOARP|NUD_REACHABLE))) {
>   		pr_info("bridge: RTM_NEWNEIGH with invalid state %#x\n", ndm->ndm_state);
>   		return -EINVAL;
> @@ -774,6 +777,9 @@ int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>   	struct net_port_vlans *pv;
>   	unsigned short vid = VLAN_N_VID;
>
> +	if ((ndm->ndm_flags & NTF_SELF) && (dev->priv_flags & IFF_EBRIDGE))
> +		return ndo_dflt_fdb_del(ndm, tb, dev, addr);
> +
>   	if (tb[NDA_VLAN]) {
>   		if (nla_len(tb[NDA_VLAN]) != sizeof(unsigned short)) {
>   			pr_info("bridge: RTM_NEWNEIGH with invalid vlan\n");
>

How is this different then calling the fdb op from rtnetlink.c when the
NTF_SELF bit is set after your previous patch

	net: generic fdb support for drivers without ndo_fdb_<op>

the generic routine gets called if a specific op is not supplied via
ndo ops anyways right?

Also I suspect if the driver supplies a specific ndo_fdb_<op> we should
use it over the generic one.

What am I missing?

Thanks,
John

-- 
John Fastabend         Intel Corporation

Re: [RFC PATCH net-next 3/3] bridge: Implement IFF_UNICAST_FLT

[Vlad Yasevich]

On 03/06/2013 10:10 PM, John Fastabend wrote:
> On 03/06/2013 06:31 PM, Vlad Yasevich wrote:
>> Implement IFF_UNICAST_FLT on the bridge.  Unicast addresses added
>> to the bridge device are synched to the uplink devices.  This
>> allows for uplink devices to change while preserving mac assignment.
>>
>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>> ---
>
> [...]
>
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index b0812c9..ef7b51e 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -677,6 +677,9 @@ int br_fdb_add(struct ndmsg *ndm, struct nlattr
>> *tb[],
>>       struct net_port_vlans *pv;
>>       unsigned short vid = VLAN_N_VID;
>>
>> +    if ((ndm->ndm_flags & NTF_SELF) && (dev->priv_flags & IFF_EBRIDGE))
>> +        return ndo_dflt_fdb_add(ndm, tb, dev, addr, nlh_flags);
>> +
>>       if (!(ndm->ndm_state & (NUD_PERMANENT|NUD_NOARP|NUD_REACHABLE))) {
>>           pr_info("bridge: RTM_NEWNEIGH with invalid state %#x\n",
>> ndm->ndm_state);
>>           return -EINVAL;
>> @@ -774,6 +777,9 @@ int br_fdb_delete(struct ndmsg *ndm, struct nlattr
>> *tb[],
>>       struct net_port_vlans *pv;
>>       unsigned short vid = VLAN_N_VID;
>>
>> +    if ((ndm->ndm_flags & NTF_SELF) && (dev->priv_flags & IFF_EBRIDGE))
>> +        return ndo_dflt_fdb_del(ndm, tb, dev, addr);
>> +
>>       if (tb[NDA_VLAN]) {
>>           if (nla_len(tb[NDA_VLAN]) != sizeof(unsigned short)) {
>>               pr_info("bridge: RTM_NEWNEIGH with invalid vlan\n");
>>
>
> How is this different then calling the fdb op from rtnetlink.c when the
> NTF_SELF bit is set after your previous patch
>
>      net: generic fdb support for drivers without ndo_fdb_<op>
>
> the generic routine gets called if a specific op is not supplied via
> ndo ops anyways right?
>
> Also I suspect if the driver supplies a specific ndo_fdb_<op> we should
> use it over the generic one.
>
> What am I missing?

The bridge provides the ndo_fdb_<add|del>, so it will be used.  The 
bridge op assumes that the dev passed to it is a port.  This code adds
support for when dev is the bridge.

In fact this patch is counting that rtnetlink will call into the bridge
and lets bridge do the work.

-vlad
>
> Thanks,
> John
>

Re: [RFC PATCH net-next 0/3] Allow bridge to function in non-promisc mode

[Stephen Hemminger]

[-- Attachment #1: Type: text/plain, Size: 415 bytes --]

I understand the desire to add more functionality, but in this case it
would introduce lots more problems. STP would break and it doesn't seem to
gain anything that can't be done by other means.

Turning bridge into macvlan seems unnecessary. Combining apples and bananas
doesn't always make a tasty smoothy, sometimes it is just a mess.

Maybe adding a little more to macvlan to do what you want would be simpler.

[-- Attachment #2: Type: text/html, Size: 476 bytes --]

Re: [RFC PATCH net-next 0/3] Allow bridge to function in non-promisc mode

[Vlad Yasevich]

On 03/07/2013 02:19 AM, Stephen Hemminger wrote:
> I understand the desire to add more functionality, but in this case it
> would introduce lots more problems. STP would break and it doesn't seem to
> gain anything that can't be done by other means.
>
> Turning bridge into macvlan seems unnecessary. Combining apples and bananas
> doesn't always make a tasty smoothy, sometimes it is just a mess.
>
> Maybe adding a little more to macvlan to do what you want would be simpler.
>


It's not really a macvlan over the bridge.  I would agree that 
particular setup would be a bit odd.  This work enables VMs to manage
their mac addresses and to reduce the load on the host by keeping the
bridge in promisc mode.

Sadly, most kvm network configs still use bridging and have not 
transitioned to OVS.  macvlan has some limitations as well and I working
to address those, but there is a desire for non-promisc bridge.  In
this case VMs can manage their mac addresses and can write that data to
the bridge.

STP is not broken as STP uses multicast mac and we set IFF_ALLMULTI thus
continuing to receive and process STP BPDUs.

The one thing that would appear to suffer from this is VLAN reception, 
but the bridge does allow vlan config now and that would have to be 
configured if VMs wish to use vlans.

I am not changing default operation of the bridge.  Default is still 
promisc.  In fact, one can switch back and forth without any network 
outages.  This simply adds another mode the the bridge operation.

-vlad

Re: [RFC PATCH net-next 0/3] Allow bridge to function in non-promisc mode

[Stephen Hemminger]

On Thu, 07 Mar 2013 10:35:37 -0500
Vlad Yasevich <vyasevic@redhat.com> wrote:

> On 03/07/2013 02:19 AM, Stephen Hemminger wrote:
> > I understand the desire to add more functionality, but in this case it
> > would introduce lots more problems. STP would break and it doesn't seem to
> > gain anything that can't be done by other means.
> >
> > Turning bridge into macvlan seems unnecessary. Combining apples and bananas
> > doesn't always make a tasty smoothy, sometimes it is just a mess.
> >
> > Maybe adding a little more to macvlan to do what you want would be simpler.
> >
> 
> 
> It's not really a macvlan over the bridge.  I would agree that 
> particular setup would be a bit odd.  This work enables VMs to manage
> their mac addresses and to reduce the load on the host by keeping the
> bridge in promisc mode.
> 
> Sadly, most kvm network configs still use bridging and have not 
> transitioned to OVS.  macvlan has some limitations as well and I working
> to address those, but there is a desire for non-promisc bridge.  In
> this case VMs can manage their mac addresses and can write that data to
> the bridge.
> 
> STP is not broken as STP uses multicast mac and we set IFF_ALLMULTI thus
> continuing to receive and process STP BPDUs.
> 
> The one thing that would appear to suffer from this is VLAN reception, 
> but the bridge does allow vlan config now and that would have to be 
> configured if VMs wish to use vlans.
> 
> I am not changing default operation of the bridge.  Default is still 
> promisc.  In fact, one can switch back and forth without any network 
> outages.  This simply adds another mode the the bridge operation.
> 

1. I am not a fan of the added complexity. 
2, Don't use sysfs for new API's use netlink instead.
3. It depends on the uplink port providing UNICAST filtering which some
   physical devices don't do.

Re: [RFC PATCH net-next 0/3] Allow bridge to function in non-promisc mode

[Vlad Yasevich]

On 03/07/2013 12:13 PM, Stephen Hemminger wrote:
> On Thu, 07 Mar 2013 10:35:37 -0500
> Vlad Yasevich <vyasevic@redhat.com> wrote:
>
>> On 03/07/2013 02:19 AM, Stephen Hemminger wrote:
>>> I understand the desire to add more functionality, but in this case it
>>> would introduce lots more problems. STP would break and it doesn't seem to
>>> gain anything that can't be done by other means.
>>>
>>> Turning bridge into macvlan seems unnecessary. Combining apples and bananas
>>> doesn't always make a tasty smoothy, sometimes it is just a mess.
>>>
>>> Maybe adding a little more to macvlan to do what you want would be simpler.
>>>
>>
>>
>> It's not really a macvlan over the bridge.  I would agree that
>> particular setup would be a bit odd.  This work enables VMs to manage
>> their mac addresses and to reduce the load on the host by keeping the
>> bridge in promisc mode.
>>
>> Sadly, most kvm network configs still use bridging and have not
>> transitioned to OVS.  macvlan has some limitations as well and I working
>> to address those, but there is a desire for non-promisc bridge.  In
>> this case VMs can manage their mac addresses and can write that data to
>> the bridge.
>>
>> STP is not broken as STP uses multicast mac and we set IFF_ALLMULTI thus
>> continuing to receive and process STP BPDUs.
>>
>> The one thing that would appear to suffer from this is VLAN reception,
>> but the bridge does allow vlan config now and that would have to be
>> configured if VMs wish to use vlans.
>>
>> I am not changing default operation of the bridge.  Default is still
>> promisc.  In fact, one can switch back and forth without any network
>> outages.  This simply adds another mode the the bridge operation.
>>
>
> 1. I am not a fan of the added complexity.

These patches are very simple and don't impact bridge operations.

> 2, Don't use sysfs for new API's use netlink instead.

Ok, I can remove the sysfs uplink interface.

> 3. It depends on the uplink port providing UNICAST filtering which some
>     physical devices don't do.
>

If the physical device doesn't provide filtering, it will be placed in 
promiscuous mode.  So, it essentially reverts back to the old behavior.
However, on devices that do support unicast filtering, there is a win.

-vlad

Re: [RFC PATCH net-next 0/3] Allow bridge to function in non-promisc mode

[Vlad Yasevich]

On 03/07/2013 12:13 PM, Stephen Hemminger wrote:
> On Thu, 07 Mar 2013 10:35:37 -0500
> Vlad Yasevich <vyasevic@redhat.com> wrote:
>
>> On 03/07/2013 02:19 AM, Stephen Hemminger wrote:
>>> I understand the desire to add more functionality, but in this case it
>>> would introduce lots more problems. STP would break and it doesn't seem to
>>> gain anything that can't be done by other means.
>>>
>>> Turning bridge into macvlan seems unnecessary. Combining apples and bananas
>>> doesn't always make a tasty smoothy, sometimes it is just a mess.
>>>
>>> Maybe adding a little more to macvlan to do what you want would be simpler.
>>>
>>
>>
>> It's not really a macvlan over the bridge.  I would agree that
>> particular setup would be a bit odd.  This work enables VMs to manage
>> their mac addresses and to reduce the load on the host by keeping the
>> bridge in promisc mode.
>>
>> Sadly, most kvm network configs still use bridging and have not
>> transitioned to OVS.  macvlan has some limitations as well and I working
>> to address those, but there is a desire for non-promisc bridge.  In
>> this case VMs can manage their mac addresses and can write that data to
>> the bridge.
>>
>> STP is not broken as STP uses multicast mac and we set IFF_ALLMULTI thus
>> continuing to receive and process STP BPDUs.
>>
>> The one thing that would appear to suffer from this is VLAN reception,
>> but the bridge does allow vlan config now and that would have to be
>> configured if VMs wish to use vlans.
>>
>> I am not changing default operation of the bridge.  Default is still
>> promisc.  In fact, one can switch back and forth without any network
>> outages.  This simply adds another mode the the bridge operation.
>>
>
> 1. I am not a fan of the added complexity.
> 2, Don't use sysfs for new API's use netlink instead.

Oh, I had a question for you about this.  I am changing the uplink code 
slightly to pattern more after some of the security features you added 
(like root_block and bpdu_guard).  I makes things simpler.  I would 
really like to provide the sysfs interface, because I checked iproute 
code and I don't see any netlink implementation of those things.

Would that be more agreeable to you?

Thanks
-vlad

I
> 3. It depends on the uplink port providing UNICAST filtering which some
>     physical devices don't do.
>