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

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

[Vlad Yasevich]

The series adds an ability to configure the bridge into a non-primiscuous
mode.   Instead, it provides the ability to identitfy some set of bridge
ports as uplinks and allows for MAC addresses to be programmed onto
those ports.  In case the port hardware does not support mac filter,
that port will be placed in promiscuous mode.

Default bridge operation continues to remain as "promiscuous".  The new
functionality has to be enabled via sysfs (similar to other bridge extensions).

The uplink mode is implemented as a flag on a bridge port.  The api to
change that flag follows the existing api to enable/disable other existing
flags.

All comments are welcome.

Changes since v1:
* Fixed submit log
* Simplifyied uplink logic.  Uplink is now a flag per port.  This removes the
  need for a separate list.
* Clean-up hw list once the port has been removed.

Vlad Yasevich (3):
  bridge: Add sysfs interface to control promisc mode
  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       |   52 +++++++++++++++++++++++++++++++++++++++++-
 net/bridge/br_fdb.c          |    6 +++++
 net/bridge/br_if.c           |   22 +++++++++++++----
 net/bridge/br_netlink.c      |    2 +
 net/bridge/br_private.h      |    3 ++
 net/bridge/br_sysfs_br.c     |   17 +++++++++++++
 net/bridge/br_sysfs_if.c     |    2 +
 8 files changed, 99 insertions(+), 6 deletions(-)

-- 
1.7.7.6

[RFC PATCH v2 net-next 1/3] bridge: Add sysfs interface to control promisc mode

[Vlad Yasevich]

Add an sysfs interface to turn promiscusous mode on and off on
the bridge.  By default all interfaces in the bridge
are in promisc mode.  When promisc mode is turned off, it is
turned off on all bridge ports and the bridge turns on IFF_ALLMULTI
to handle multicast traffic.

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..37f611f 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

[RFC PATCH v2 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_netlink.c      |    2 ++
 net/bridge/br_private.h      |    1 +
 net/bridge/br_sysfs_if.c     |    2 ++
 4 files changed, 6 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_netlink.c b/net/bridge/br_netlink.c
index 27aa3ee..e08a50e 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 */
@@ -329,6 +330,7 @@ static int br_setport(struct net_bridge_port *p, struct nlattr *tb[])
 	br_set_port_flag(p, tb, IFLA_BRPORT_MODE, BR_HAIRPIN_MODE);
 	br_set_port_flag(p, tb, IFLA_BRPORT_GUARD, BR_BPDU_GUARD);
 	br_set_port_flag(p, tb, IFLA_BRPORT_FAST_LEAVE, BR_MULTICAST_FAST_LEAVE);
+	br_set_port_flag(p, tb, IFLA_BRPORT_UPLINK, BR_UPLINK);
 
 	if (tb[IFLA_BRPORT_COST]) {
 		err = br_stp_set_path_cost(p, nla_get_u32(tb[IFLA_BRPORT_COST]));
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 4a0fa29..44ae584 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -156,6 +156,7 @@ struct net_bridge_port
 #define BR_BPDU_GUARD           0x00000002
 #define BR_ROOT_BLOCK		0x00000004
 #define BR_MULTICAST_FAST_LEAVE	0x00000008
+#define BR_UPLINK		0x00000010
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 	u32				multicast_startup_queries_sent;
diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
index a1ef1b6..1f28cd4 100644
--- a/net/bridge/br_sysfs_if.c
+++ b/net/bridge/br_sysfs_if.c
@@ -158,6 +158,7 @@ static BRPORT_ATTR(flush, S_IWUSR, NULL, store_flush);
 BRPORT_ATTR_FLAG(hairpin_mode, BR_HAIRPIN_MODE);
 BRPORT_ATTR_FLAG(bpdu_guard, BR_BPDU_GUARD);
 BRPORT_ATTR_FLAG(root_block, BR_ROOT_BLOCK);
+BRPORT_ATTR_FLAG(uplink, BR_UPLINK);
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 static ssize_t show_multicast_router(struct net_bridge_port *p, char *buf)
@@ -195,6 +196,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 v2 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 |   11 ++++++++++-
 net/bridge/br_fdb.c    |    6 ++++++
 net/bridge/br_if.c     |    2 ++
 3 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 37f611f..91fff56 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -106,6 +106,15 @@ 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 *port;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(port, &br->port_list, list) {
+		if (port->flags & BR_UPLINK)
+			dev_uc_sync(uplink->dev, dev);
+	}
+	rcu_read_unlock();
 }
 
 static int br_dev_stop(struct net_device *dev)
@@ -384,7 +393,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");
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 02b4440..5196a79 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -145,6 +145,8 @@ static void del_nbp(struct net_bridge_port *p)
 
 	nbp_vlan_flush(p);
 	br_fdb_delete_by_port(br, p, 1);
+	if (p->flags & BR_UPLINK)
+		dev_uc_unsync(dev, br->dev);
 
 	list_del_rcu(&p->list);
 
-- 
1.7.7.6

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

[Stephen Hemminger]

On Thu,  7 Mar 2013 16:28:45 -0500
Vlad Yasevich <vyasevic@redhat.com> wrote:

> The series adds an ability to configure the bridge into a non-primiscuous
> mode.   Instead, it provides the ability to identitfy some set of bridge
> ports as uplinks and allows for MAC addresses to be programmed onto
> those ports.  In case the port hardware does not support mac filter,
> that port will be placed in promiscuous mode.
> 
> Default bridge operation continues to remain as "promiscuous".  The new
> functionality has to be enabled via sysfs (similar to other bridge extensions).
> 
> The uplink mode is implemented as a flag on a bridge port.  The api to
> change that flag follows the existing api to enable/disable other existing
> flags.
> 
> All comments are welcome.
> 

Can we make this a one step process and less visible to the user.
If user defines an uplink device, and the uplink device is capable of filtering
(and what ever other pre-conditions people can think of), then the bridge will
transparently switch to uplink/non-promisc mode.  This can also be used to trigger
edge only mode in RSTP in the future.

Less knobs.

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

[Vlad Yasevich]

On 03/08/2013 12:43 AM, Stephen Hemminger wrote:
> On Thu,  7 Mar 2013 16:28:45 -0500
> Vlad Yasevich <vyasevic@redhat.com> wrote:
>
>> The series adds an ability to configure the bridge into a non-primiscuous
>> mode.   Instead, it provides the ability to identitfy some set of bridge
>> ports as uplinks and allows for MAC addresses to be programmed onto
>> those ports.  In case the port hardware does not support mac filter,
>> that port will be placed in promiscuous mode.
>>
>> Default bridge operation continues to remain as "promiscuous".  The new
>> functionality has to be enabled via sysfs (similar to other bridge extensions).
>>
>> The uplink mode is implemented as a flag on a bridge port.  The api to
>> change that flag follows the existing api to enable/disable other existing
>> flags.
>>
>> All comments are welcome.
>>
>
> Can we make this a one step process and less visible to the user.
> If user defines an uplink device, and the uplink device is capable of filtering
> (and what ever other pre-conditions people can think of), then the bridge will
> transparently switch to uplink/non-promisc mode.  This can also be used to trigger
> edge only mode in RSTP in the future.
>
> Less knobs.
>

Ok.  Let me see what I can do.

Thanks
-vlad

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

[Vlad Yasevich]

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

On 03/08/2013 10:17 AM, Vlad Yasevich wrote:
> On 03/08/2013 12:43 AM, Stephen Hemminger wrote:
>> On Thu,  7 Mar 2013 16:28:45 -0500
>> Vlad Yasevich <vyasevic@redhat.com> wrote:
>>
>>> The series adds an ability to configure the bridge into a
>>> non-primiscuous
>>> mode.   Instead, it provides the ability to identitfy some set of bridge
>>> ports as uplinks and allows for MAC addresses to be programmed onto
>>> those ports.  In case the port hardware does not support mac filter,
>>> that port will be placed in promiscuous mode.
>>>
>>> Default bridge operation continues to remain as "promiscuous".  The new
>>> functionality has to be enabled via sysfs (similar to other bridge
>>> extensions).
>>>
>>> The uplink mode is implemented as a flag on a bridge port.  The api to
>>> change that flag follows the existing api to enable/disable other
>>> existing
>>> flags.
>>>
>>> All comments are welcome.
>>>
>>
>> Can we make this a one step process and less visible to the user.
>> If user defines an uplink device, and the uplink device is capable of
>> filtering
>> (and what ever other pre-conditions people can think of), then the
>> bridge will
>> transparently switch to uplink/non-promisc mode.  This can also be
>> used to trigger
>> edge only mode in RSTP in the future.
>>
>> Less knobs.
>>
>
> Ok.  Let me see what I can do.

So I started working through this and realized that this complicates the 
code significantly.

* I have to re-introduce the uplink-list since now I need to track
"filter capable" uplinks in addition to non-capable ones.
* The really nice and simple sysfs interface to set a flag turns into 
something that duplicates code.
* The bridge port removal can effect the promiscuity setting of the 
bridge if the last uplink is removed.
* We lose the ability to run a promisc edge bridge with uplinks.

I am really starting wonder if this is any better?  The changes
are much bigger and more complex while the functional flexibility is
reduced.  Is it really worth removing a configuration knob?

I've attached an in-progress patch to demonstrate the above.

-vlad

-vlad
>
> Thanks
> -vlad
>


[-- Attachment #2: test.patch --]
[-- Type: text/x-patch, Size: 8158 bytes --]

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 d5f1d3f..df0afb2 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -356,6 +356,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;
@@ -371,6 +372,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..8a3a085 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -60,6 +60,29 @@ static int port_cost(struct net_device *dev)
 	return 100;	/* assume old 10Mbps */
 }
 
+static void br_set_promisc(struct net_bridge *br, int val)
+{
+	struct net_bridge_port *p;
+
+	if (val) {
+		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 {
+		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);
+		}
+	}
+}
 
 /* Check for port carrier transistions. */
 void br_port_carrier_check(struct net_bridge_port *p)
@@ -132,7 +155,14 @@ 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);
+
+	list_del_rcu(&p->uplink_list);
+	if (list_empty(&br->uplink_list))
+		br_set_promisc(br, 1);
 
 	spin_lock_bh(&br->lock);
 	br_stp_disable_port(p);
@@ -222,6 +252,7 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
 	p->priority = 0x8000 >> BR_PORT_BITS;
 	p->port_no = index;
 	p->flags = 0;
+	INIT_LIST_HEAD(&p->uplink_list);
 	br_init_port(p);
 	p->state = BR_STATE_DISABLED;
 	br_stp_port_timer_init(p);
@@ -350,9 +381,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 +451,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);
@@ -449,6 +489,30 @@ int br_del_if(struct net_bridge *br, struct net_device *dev)
 	return 0;
 }
 
+void br_set_uplink(struct net_bridge_port *p)
+{
+	if (p->flags & BR_UPLINK) {
+		if (p->dev->priv_flags & IFF_UNICAST_FLT) {
+			/* Turn off promisc mode if this is the first
+			 * filter capable uplink
+			 */
+			if (list_empty(&p->br->uplink_list))
+				br_set_promisc(p->br, 0);
+
+			list_add_rcu(&p->uplink_list, &p->br->uplink_list);
+		}
+	} else {
+		if (p->dev->priv_flags & IFF_UNICAST_FLT) {
+			list_del_rcu(&p->uplink_list);
+			/* Turn on promisc mode if this was the last
+			 * filter capable uplink
+			 */
+			if (list_empty(&p->br->uplink_list))
+				br_set_promisc(p->br, 1);
+		}
+	}
+}
+
 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..e53eb0b 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 */
@@ -325,10 +326,12 @@ static void br_set_port_flag(struct net_bridge_port *p, struct nlattr *tb[],
 static int br_setport(struct net_bridge_port *p, struct nlattr *tb[])
 {
 	int err;
+	unsigned long flags = p->flags;
 
 	br_set_port_flag(p, tb, IFLA_BRPORT_MODE, BR_HAIRPIN_MODE);
 	br_set_port_flag(p, tb, IFLA_BRPORT_GUARD, BR_BPDU_GUARD);
 	br_set_port_flag(p, tb, IFLA_BRPORT_FAST_LEAVE, BR_MULTICAST_FAST_LEAVE);
+	br_set_port_flag(p, tb, IFLA_BRPORT_UPLINK, BR_UPLINK);
 
 	if (tb[IFLA_BRPORT_COST]) {
 		err = br_stp_set_path_cost(p, nla_get_u32(tb[IFLA_BRPORT_COST]));
@@ -347,6 +350,11 @@ static int br_setport(struct net_bridge_port *p, struct nlattr *tb[])
 		if (err)
 			return err;
 	}
+
+	/* If the uplink flag has changed do additional processing */
+	if ((flags ^ p->flags) & BR_UPLINK)
+		br_set_uplink(p);
+
 	return 0;
 }
 
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 6d314c4..ab8cb1a9 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;
@@ -156,6 +157,7 @@ struct net_bridge_port
 #define BR_BPDU_GUARD           0x00000002
 #define BR_ROOT_BLOCK		0x00000004
 #define BR_MULTICAST_FAST_LEAVE	0x00000008
+#define BR_UPLINK		0x00000010
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 	u32				multicast_startup_queries_sent;
@@ -277,6 +279,8 @@ 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;
 	struct net_port_vlans __rcu	*vlan_info;
@@ -426,6 +430,7 @@ 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);
 
 /* 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..454a0cf 100644
--- a/net/bridge/br_sysfs_if.c
+++ b/net/bridge/br_sysfs_if.c
@@ -159,6 +159,29 @@ 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", !!(p->flags & BR_UPLINK));
+}
+
+static int store_uplink(struct net_bridge_port *p, unsigned long v)
+{
+	unsigned long flags = p->flags;
+
+	if (v)
+		flags |= BR_UPLINK;
+	else
+		flags &= ~BR_UPLINK;
+
+	if (flags != p->flags) {
+		p->flags = flags;
+		br_set_uplink(p);
+		br_ifinfo_notify(RTM_NEWLINK, p);
+	}
+	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 +218,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,

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

[Stephen Hemminger]

On Fri, 08 Mar 2013 16:23:53 -0500
Vlad Yasevich <vyasevic@redhat.com> wrote:

> On 03/08/2013 10:17 AM, Vlad Yasevich wrote:
> > On 03/08/2013 12:43 AM, Stephen Hemminger wrote:
> >> On Thu,  7 Mar 2013 16:28:45 -0500
> >> Vlad Yasevich <vyasevic@redhat.com> wrote:
> >>
> >>> The series adds an ability to configure the bridge into a
> >>> non-primiscuous
> >>> mode.   Instead, it provides the ability to identitfy some set of bridge
> >>> ports as uplinks and allows for MAC addresses to be programmed onto
> >>> those ports.  In case the port hardware does not support mac filter,
> >>> that port will be placed in promiscuous mode.
> >>>
> >>> Default bridge operation continues to remain as "promiscuous".  The new
> >>> functionality has to be enabled via sysfs (similar to other bridge
> >>> extensions).
> >>>
> >>> The uplink mode is implemented as a flag on a bridge port.  The api to
> >>> change that flag follows the existing api to enable/disable other
> >>> existing
> >>> flags.
> >>>
> >>> All comments are welcome.
> >>>
> >>
> >> Can we make this a one step process and less visible to the user.
> >> If user defines an uplink device, and the uplink device is capable of
> >> filtering
> >> (and what ever other pre-conditions people can think of), then the
> >> bridge will
> >> transparently switch to uplink/non-promisc mode.  This can also be
> >> used to trigger
> >> edge only mode in RSTP in the future.
> >>
> >> Less knobs.
> >>
> >
> > Ok.  Let me see what I can do.
> 
> So I started working through this and realized that this complicates the 
> code significantly.
> 
> * I have to re-introduce the uplink-list since now I need to track
> "filter capable" uplinks in addition to non-capable ones.
> * The really nice and simple sysfs interface to set a flag turns into 
> something that duplicates code.
> * The bridge port removal can effect the promiscuity setting of the 
> bridge if the last uplink is removed.
> * We lose the ability to run a promisc edge bridge with uplinks.
> 
> I am really starting wonder if this is any better?  The changes
> are much bigger and more complex while the functional flexibility is
> reduced.  Is it really worth removing a configuration knob?
> 
> I've attached an in-progress patch to demonstrate the above.
> 
> -vlad
> 
> -vlad
> >
> > Thanks
> > -vlad
> >
> 

A two step process is fine (uplink and promisc as seperate step),
as long as invalid combinations are prevented. More options may lead to
more combinations that are invalid.

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

[Michael S. Tsirkin]

On Thu, Mar 07, 2013 at 09:43:04PM -0800, Stephen Hemminger wrote:
> On Thu,  7 Mar 2013 16:28:45 -0500
> Vlad Yasevich <vyasevic@redhat.com> wrote:
> 
> > The series adds an ability to configure the bridge into a non-primiscuous
> > mode.   Instead, it provides the ability to identitfy some set of bridge
> > ports as uplinks and allows for MAC addresses to be programmed onto
> > those ports.  In case the port hardware does not support mac filter,
> > that port will be placed in promiscuous mode.
> > 
> > Default bridge operation continues to remain as "promiscuous".  The new
> > functionality has to be enabled via sysfs (similar to other bridge extensions).
> > 
> > The uplink mode is implemented as a flag on a bridge port.  The api to
> > change that flag follows the existing api to enable/disable other existing
> > flags.
> > 
> > All comments are welcome.
> > 
> 
> Can we make this a one step process and less visible to the user.
> If user defines an uplink device, and the uplink device is capable of filtering
> (and what ever other pre-conditions people can think of), then the bridge will
> transparently switch to uplink/non-promisc mode.  This can also be used to trigger
> edge only mode in RSTP in the future.
> 
> Less knobs.
> 

Rephrasing what I suggested using 'uplink' term:
- add ability to specify wanted addresses on a port
- wheneever we don't specify addresses we call an uplink
  (either automatically so by default all
  ports are uplink, or add a separate flag - any preference?)
- if there are 0 uplinks, no ports are promisc
- if there is 1 uplink, it is not promisc but all others are promisc
- if there are >1 uplinks, everyone is promisc


> 
> --
> 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