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

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

[Vlad Yasevich]

The series adds an ability for the bridge to function in non-promiscuous mode.
We do it in 3 steps.
First we add an interface to palce the switch into non-promisc mode.  In
this mode, all port of the switch turn promisc off and turn on IFF_ALLMULTI
to continue handling multicast traffic.
Second we add an ability to designate a bridge port as uplink.
Third we add IFF_UNICAST_FLT support to the bridge and sync all unicast
HW addresses to the uplink ports.

Default bridge operation continues to remain "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.

Changes since rfc v2:
* Sync/unsync address on uplink upon the uplink flag change.  This allows
for uplink replacements without loss of addresses.

Changes since rfc 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 (4):
  bridge: Add sysfs interface to control promisc mode
  bridge: Allow an ability to designate an uplink port
  bridge: Implement IFF_UNICAST_FLT
  bridge: sync device list when a new uplink is designated

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

-- 
1.7.7.6

[PATCH net-next 1/4] 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

Re: [PATCH net-next 1/4] bridge: Add sysfs interface to control promisc mode

[Stephen Hemminger]

On Tue, 12 Mar 2013 21:45:23 -0400
Vlad Yasevich <vyasevic@redhat.com> wrote:

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

All attribute changes must also be available through netlink, and changes
to attributes should cause a notification to applications listening for
netlink events on that bridge.

Re: [PATCH net-next 1/4] bridge: Add sysfs interface to control promisc mode

[Vlad Yasevich]

On 03/13/2013 11:38 AM, Stephen Hemminger wrote:
> On Tue, 12 Mar 2013 21:45:23 -0400
> Vlad Yasevich <vyasevic@redhat.com> wrote:
>
>> 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(-)
>>
>
> All attribute changes must also be available through netlink, and changes
> to attributes should cause a notification to applications listening for
> netlink events on that bridge.
>

There is currently no attribute for promisc vs non-promisc in the 
bridge.  Are you suggesting that this patch should add one?

-vlad

[PATCH net-next 2/4] 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

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

[Michael S. Tsirkin]

On Tue, Mar 12, 2013 at 09:45:24PM -0400, Vlad Yasevich wrote:
> 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>

It looks like if you make two links uplink, bridging
between them won't work at all?

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

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

[Stephen Hemminger]

On Wed, 13 Mar 2013 22:33:33 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Tue, Mar 12, 2013 at 09:45:24PM -0400, Vlad Yasevich wrote:
> > 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>
> 
> It looks like if you make two links uplink, bridging
> between them won't work at all?

Maybe better to mark which ports are terminal ports rather than marking
uplink ports?

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

[Vlad Yasevich]

On 03/13/2013 04:33 PM, Michael S. Tsirkin wrote:
> On Tue, Mar 12, 2013 at 09:45:24PM -0400, Vlad Yasevich wrote:
>> 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>
>
> It looks like if you make two links uplink, bridging
> between them won't work at all?

only if they are non-promisc, but yes it does look like that...

hm... I need to think about this...

-vlad

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

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

[Dan Williams]

On Wed, 2013-03-13 at 22:33 +0200, Michael S. Tsirkin wrote:
> On Tue, Mar 12, 2013 at 09:45:24PM -0400, Vlad Yasevich wrote:
> > 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>
> 
> It looks like if you make two links uplink, bridging
> between them won't work at all?

Only caught the end of this thread...  is there any way to use the
UPLINK flag as part of the bridge master carrier calculation?  Obviously
that's technically possible, I'm asking if that's going to break all
sorts of userspace tools and such.

Traditionally for "wired-type" devices like bridge, bond, vlan, wired,
etc, the carrier/LOWER_UP is the way that higher levels know that IP
configuration is possible on the device.

In the case of a bridge, it's not worth trying DHCP or IPv6 RA on the
bridge master until we know that there's an uplink port, otherwise it's
just going to fail.  But right now, we have no way of knowing which port
is an uplink port, which this series would clearly solve.

However, bridge master devices calculate their carrier based on the
state of all their ports, but right now all ports are equal.  If we used
UPLINK as part of that calculation, then higher level tools would be
notified that the master is LOWER_UP only when it actually was.

If we're worried about backwards compatibility, then perhaps the new
behavior could be enabled only if one or more ports was an UPLINK?  Then
userspace tools that cared would monitor bridge ports, and if they cared
about UPLINK could ignore bridge carrier until at least one UPLINK port
was added.

Dan

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

[PATCH net-next 3/4] 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..de190fa 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(port->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

[PATCH net-next 4/4] bridge: sync device list when a new uplink is designated

[Vlad Yasevich]

What a new device is designated as "uplink" sync any device
addresses to it.  This allows for dynamic replacement of
uplinks while maintaining the device list.

Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
 net/bridge/br_netlink.c  |   11 +++++++++++
 net/bridge/br_sysfs_if.c |   27 ++++++++++++++++++++++++++-
 2 files changed, 37 insertions(+), 1 deletions(-)

diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index e08a50e..02f3425 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -326,6 +326,7 @@ 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;
+	int old_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);
@@ -349,6 +350,16 @@ static int br_setport(struct net_bridge_port *p, struct nlattr *tb[])
 		if (err)
 			return err;
 	}
+
+	if ((old_flags ^ p->flags) & BR_UPLINK) {
+		/* UPLINK flag changed on the device.  Take care of device
+		 * addresses.
+		 */
+		if (p->flags & BR_UPLINK)
+			dev_uc_sync(p->dev, p->br->dev);
+		else
+			dev_uc_unsync(p->dev, p->br->dev);
+	}
 	return 0;
 }
 
diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
index 1f28cd4..d7a89c9 100644
--- a/net/bridge/br_sysfs_if.c
+++ b/net/bridge/br_sysfs_if.c
@@ -158,7 +158,32 @@ 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);
+
+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;
+		if (p->flags & BR_UPLINK)
+			dev_uc_sync(p->dev, p->br->dev);
+		else
+			dev_uc_unsync(p->dev, p->br->dev);
+		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)
-- 
1.7.7.6

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

["Oleg A. Arkhangelsky"]

13.03.2013, 05:45, "Vlad Yasevich" <vyasevic@redhat.com>:

> The series adds an ability for the bridge to function in non-promiscuous mode.

What is the practical applications for such setup? In other words,
in which cases I would want to put bridge into non-promiscuous
mode and specify some uplink ports?

-- 
wbr, Oleg.

"Anarchy is about taking complete responsibility for yourself."
 Alan Moore.

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

[Vlad Yasevich]

On 03/13/2013 02:22 AM, "Oleg A. Arkhangelsky" wrote:
>
>
> 13.03.2013, 05:45, "Vlad Yasevich" <vyasevic@redhat.com>:
>
>> The series adds an ability for the bridge to function in non-promiscuous mode.
>
> What is the practical applications for such setup? In other words,
> in which cases I would want to put bridge into non-promiscuous
> mode and specify some uplink ports?
>

On of the applications would be when bridge is an edge device servicing 
a VM deployment.  Each of the VMs knows the mac address that the guest
has and may program that mac onto the uplinks.

-vlad

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

[Stephen Hemminger]

On Wed, 13 Mar 2013 08:12:29 -0400
Vlad Yasevich <vyasevic@redhat.com> wrote:

> On 03/13/2013 02:22 AM, "Oleg A. Arkhangelsky" wrote:
> >
> >
> > 13.03.2013, 05:45, "Vlad Yasevich" <vyasevic@redhat.com>:
> >
> >> The series adds an ability for the bridge to function in non-promiscuous mode.
> >
> > What is the practical applications for such setup? In other words,
> > in which cases I would want to put bridge into non-promiscuous
> > mode and specify some uplink ports?
> >
> 
> On of the applications would be when bridge is an edge device servicing 
> a VM deployment.  Each of the VMs knows the mac address that the guest
> has and may program that mac onto the uplinks.

Why wouldn't that environment just use macvlan?
Is it because changing libvirt is harder than changing the kernel?

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

[Vlad Yasevich]

On 03/13/2013 11:39 AM, Stephen Hemminger wrote:
> On Wed, 13 Mar 2013 08:12:29 -0400
> Vlad Yasevich <vyasevic@redhat.com> wrote:
>
>> On 03/13/2013 02:22 AM, "Oleg A. Arkhangelsky" wrote:
>>>
>>>
>>> 13.03.2013, 05:45, "Vlad Yasevich" <vyasevic@redhat.com>:
>>>
>>>> The series adds an ability for the bridge to function in non-promiscuous mode.
>>>
>>> What is the practical applications for such setup? In other words,
>>> in which cases I would want to put bridge into non-promiscuous
>>> mode and specify some uplink ports?
>>>
>>
>> On of the applications would be when bridge is an edge device servicing
>> a VM deployment.  Each of the VMs knows the mac address that the guest
>> has and may program that mac onto the uplinks.
>
> Why wouldn't that environment just use macvlan?
> Is it because changing libvirt is harder than changing the kernel?
>

No, because macvlan has a drawback that it doesn't easily let guests 
talk to the host.  Bridge is still most commonly used for just that reason.

-vlad

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

[Stephen Hemminger]

On Wed, 13 Mar 2013 11:45:40 -0400
Vlad Yasevich <vyasevic@redhat.com> wrote:

> On 03/13/2013 11:39 AM, Stephen Hemminger wrote:
> > On Wed, 13 Mar 2013 08:12:29 -0400
> > Vlad Yasevich <vyasevic@redhat.com> wrote:
> >
> >> On 03/13/2013 02:22 AM, "Oleg A. Arkhangelsky" wrote:
> >>>
> >>>
> >>> 13.03.2013, 05:45, "Vlad Yasevich" <vyasevic@redhat.com>:
> >>>
> >>>> The series adds an ability for the bridge to function in non-promiscuous mode.
> >>>
> >>> What is the practical applications for such setup? In other words,
> >>> in which cases I would want to put bridge into non-promiscuous
> >>> mode and specify some uplink ports?
> >>>
> >>
> >> On of the applications would be when bridge is an edge device servicing
> >> a VM deployment.  Each of the VMs knows the mac address that the guest
> >> has and may program that mac onto the uplinks.
> >
> > Why wouldn't that environment just use macvlan?
> > Is it because changing libvirt is harder than changing the kernel?
> >
> 
> No, because macvlan has a drawback that it doesn't easily let guests 
> talk to the host.  Bridge is still most commonly used for just that reason.
> 
> -vlad

Maybe fixing that with a flag to macvlan would be easier?

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

[Vlad Yasevich]

On 03/13/2013 12:09 PM, Stephen Hemminger wrote:
> On Wed, 13 Mar 2013 11:45:40 -0400
> Vlad Yasevich <vyasevic@redhat.com> wrote:
>
>> On 03/13/2013 11:39 AM, Stephen Hemminger wrote:
>>> On Wed, 13 Mar 2013 08:12:29 -0400
>>> Vlad Yasevich <vyasevic@redhat.com> wrote:
>>>
>>>> On 03/13/2013 02:22 AM, "Oleg A. Arkhangelsky" wrote:
>>>>>
>>>>>
>>>>> 13.03.2013, 05:45, "Vlad Yasevich" <vyasevic@redhat.com>:
>>>>>
>>>>>> The series adds an ability for the bridge to function in non-promiscuous mode.
>>>>>
>>>>> What is the practical applications for such setup? In other words,
>>>>> in which cases I would want to put bridge into non-promiscuous
>>>>> mode and specify some uplink ports?
>>>>>
>>>>
>>>> On of the applications would be when bridge is an edge device servicing
>>>> a VM deployment.  Each of the VMs knows the mac address that the guest
>>>> has and may program that mac onto the uplinks.
>>>
>>> Why wouldn't that environment just use macvlan?
>>> Is it because changing libvirt is harder than changing the kernel?
>>>
>>
>> No, because macvlan has a drawback that it doesn't easily let guests
>> talk to the host.  Bridge is still most commonly used for just that reason.
>>
>> -vlad
>
> Maybe fixing that with a flag to macvlan would be easier?
>

Not really.  macvlan to physical device could be made simple enough.
However, physical to macvlan is non-trivial at all.

We get around this right now by crating a macvlan on the host and
have macvlan to macvlan communication essentially turning it into
bridge.  But that doesn't work in all scenarios either.

-vlad

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

[Joel Wirāmu Pauling]

I agree that this is a useful behavior, often we will use a bridge in
the same scenario (multiple virtio tap adapters belonging to VM's)
another scenario is when creating a bridge for chaining multiple VPN's
together.

-Joel

On 14 March 2013 06:04, Vlad Yasevich <vyasevic@redhat.com> wrote:
> On 03/13/2013 12:09 PM, Stephen Hemminger wrote:
>>
>> On Wed, 13 Mar 2013 11:45:40 -0400
>> Vlad Yasevich <vyasevic@redhat.com> wrote:
>>
>>> On 03/13/2013 11:39 AM, Stephen Hemminger wrote:
>>>>
>>>> On Wed, 13 Mar 2013 08:12:29 -0400
>>>> Vlad Yasevich <vyasevic@redhat.com> wrote:
>>>>
>>>>> On 03/13/2013 02:22 AM, "Oleg A. Arkhangelsky" wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> 13.03.2013, 05:45, "Vlad Yasevich" <vyasevic@redhat.com>:
>>>>>>
>>>>>>> The series adds an ability for the bridge to function in
>>>>>>> non-promiscuous mode.
>>>>>>
>>>>>>
>>>>>> What is the practical applications for such setup? In other words,
>>>>>> in which cases I would want to put bridge into non-promiscuous
>>>>>> mode and specify some uplink ports?
>>>>>>
>>>>>
>>>>> On of the applications would be when bridge is an edge device servicing
>>>>> a VM deployment.  Each of the VMs knows the mac address that the guest
>>>>> has and may program that mac onto the uplinks.
>>>>
>>>>
>>>> Why wouldn't that environment just use macvlan?
>>>> Is it because changing libvirt is harder than changing the kernel?
>>>>
>>>
>>> No, because macvlan has a drawback that it doesn't easily let guests
>>> talk to the host.  Bridge is still most commonly used for just that
>>> reason.
>>>
>>> -vlad
>>
>>
>> Maybe fixing that with a flag to macvlan would be easier?
>>
>
> Not really.  macvlan to physical device could be made simple enough.
> However, physical to macvlan is non-trivial at all.
>
> We get around this right now by crating a macvlan on the host and
> have macvlan to macvlan communication essentially turning it into
> bridge.  But that doesn't work in all scenarios either.
>
> -vlad

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

[Michael S. Tsirkin]

On Wed, Mar 13, 2013 at 01:04:51PM -0400, Vlad Yasevich wrote:
> On 03/13/2013 12:09 PM, Stephen Hemminger wrote:
> >On Wed, 13 Mar 2013 11:45:40 -0400
> >Vlad Yasevich <vyasevic@redhat.com> wrote:
> >
> >>On 03/13/2013 11:39 AM, Stephen Hemminger wrote:
> >>>On Wed, 13 Mar 2013 08:12:29 -0400
> >>>Vlad Yasevich <vyasevic@redhat.com> wrote:
> >>>
> >>>>On 03/13/2013 02:22 AM, "Oleg A. Arkhangelsky" wrote:
> >>>>>
> >>>>>
> >>>>>13.03.2013, 05:45, "Vlad Yasevich" <vyasevic@redhat.com>:
> >>>>>
> >>>>>>The series adds an ability for the bridge to function in non-promiscuous mode.
> >>>>>
> >>>>>What is the practical applications for such setup? In other words,
> >>>>>in which cases I would want to put bridge into non-promiscuous
> >>>>>mode and specify some uplink ports?
> >>>>>
> >>>>
> >>>>On of the applications would be when bridge is an edge device servicing
> >>>>a VM deployment.  Each of the VMs knows the mac address that the guest
> >>>>has and may program that mac onto the uplinks.
> >>>
> >>>Why wouldn't that environment just use macvlan?
> >>>Is it because changing libvirt is harder than changing the kernel?
> >>>
> >>
> >>No, because macvlan has a drawback that it doesn't easily let guests
> >>talk to the host.  Bridge is still most commonly used for just that reason.
> >>
> >>-vlad
> >
> >Maybe fixing that with a flag to macvlan would be easier?
> >
> 
> Not really.  macvlan to physical device could be made simple enough.
> However, physical to macvlan is non-trivial at all.
> 
> We get around this right now by crating a macvlan on the host and
> have macvlan to macvlan communication essentially turning it into
> bridge.  But that doesn't work in all scenarios either.
> 
> -vlad

Yea macvlan bridged mode is a strange beast.  It almost wants to be a
bridge, but does not dare, for example it punts on all the issues we
have fixed in the bridge like multicast snooping, and just floods
packets.

Another issue is that different VMs have different needs. You might want
to run VMs that simply use a single MAC, so no need for promisc mode,
then want to start another VM that does bridging so we need to enable
promisc mode dynamically. I guess we could switch from macvtap to
tap and back, but it's pretty nasty.

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

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

[Michael S. Tsirkin]

On Tue, Mar 12, 2013 at 09:45:22PM -0400, Vlad Yasevich wrote:
> The series adds an ability for the bridge to function in non-promiscuous mode.
> We do it in 3 steps.
> First we add an interface to palce the switch into non-promisc mode.  In
> this mode, all port of the switch turn promisc off and turn on IFF_ALLMULTI
> to continue handling multicast traffic.

Hmm why not go the extra mile and do same thing for multicast?

> Second we add an ability to designate a bridge port as uplink.
> Third we add IFF_UNICAST_FLT support to the bridge and sync all unicast
> HW addresses to the uplink ports.
> Default bridge operation continues to remain "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.

Actually, it seems a bit tricky to use correctly.  Specifying MACs is
straight-forward enough, but as you add and remove interfaces, you would
need to play with uplink and promisc bits.  I also think that for setups
you describe, where we really want to forward traffic to VMs and that's
all, promisc mode is an implementation detail. If true it's a mistake to
expose it in an interface.  Here's an idea how we could make it work
automatically:

- Like your patch does, allow two kinds of ports: port that only wants
  specific addresses and "wildcard" port that can get all addresses
  All ports default to wildcard as a compatible way
- For port A, if there is some wildcard port besides A, A must be
  promiscous since maybe it needs to get packets that will be later
  forwardd to A
- For port A, if there are no wildcard ports, or if A is the only
  wildcard, A does not need to be promiscous.
  Instead we can program it with addresses of all other ports.

Exactly same logic applies, separately, for unicast, where promisc
means all uni, and multicast, where promisc means all multi.

Now there's no special unplink port, no need to enable/disable special
mode, nothing: just program addresses for ports and go.


> Changes since rfc v2:
> * Sync/unsync address on uplink upon the uplink flag change.  This allows
> for uplink replacements without loss of addresses.
> 
> Changes since rfc 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 (4):
>   bridge: Add sysfs interface to control promisc mode
>   bridge: Allow an ability to designate an uplink port
>   bridge: Implement IFF_UNICAST_FLT
>   bridge: sync device list when a new uplink is designated
> 
>  include/uapi/linux/if_link.h |    1 +
>  net/bridge/br_device.c       |   52 +++++++++++++++++++++++++++++++++++++++++-
>  net/bridge/br_fdb.c          |    6 +++++
>  net/bridge/br_if.c           |   24 +++++++++++++++----
>  net/bridge/br_netlink.c      |   13 ++++++++++
>  net/bridge/br_private.h      |    3 ++
>  net/bridge/br_sysfs_br.c     |   17 +++++++++++++
>  net/bridge/br_sysfs_if.c     |   27 +++++++++++++++++++++
>  8 files changed, 137 insertions(+), 6 deletions(-)
> 
> -- 
> 1.7.7.6
> 
> --
> 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

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

[Vlad Yasevich]

On 03/13/2013 04:31 PM, Michael S. Tsirkin wrote:
> On Tue, Mar 12, 2013 at 09:45:22PM -0400, Vlad Yasevich wrote:
>> The series adds an ability for the bridge to function in non-promiscuous mode.
>> We do it in 3 steps.
>> First we add an interface to palce the switch into non-promisc mode.  In
>> this mode, all port of the switch turn promisc off and turn on IFF_ALLMULTI
>> to continue handling multicast traffic.
>
> Hmm why not go the extra mile and do same thing for multicast?

That would be simple enough to add, as I think all this would require is 
an additional dev_mc_sync() call.

I wanted to get basic functionality and unicast working.

>
>> Second we add an ability to designate a bridge port as uplink.
>> Third we add IFF_UNICAST_FLT support to the bridge and sync all unicast
>> HW addresses to the uplink ports.
>> Default bridge operation continues to remain "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.
>
> Actually, it seems a bit tricky to use correctly.  Specifying MACs is
> straight-forward enough, but as you add and remove interfaces, you would
> need to play with uplink and promisc bits.

Not the promisc bits since this patch series proposes a bridge wide 
setting.  Yes, uplink bits would need to be played with though.  I'd 
really love to specify those bits when the link is added, but the 
netlink operation for bridge is a bit odd.

May be I should re-fix that.

>  I also think that for setups
> you describe, where we really want to forward traffic to VMs and that's
> all, promisc mode is an implementation detail. If true it's a mistake to
> expose it in an interface.  Here's an idea how we could make it work
> automatically:

This is something Stephen suggested earlier and controlling promisc mode
based on whether uplinks are set or not.

>
> - Like your patch does, allow two kinds of ports: port that only wants
>    specific addresses and "wildcard" port that can get all addresses
>    All ports default to wildcard as a compatible way
> - For port A, if there is some wildcard port besides A, A must be
>    promiscous since maybe it needs to get packets that will be later
>    forwardd to A
> - For port A, if there are no wildcard ports, or if A is the only
>    wildcard, A does not need to be promiscous.
>    Instead we can program it with addresses of all other ports.

So, according to the above, you can only ever have 1 port that is 
'address specific'.  If you have more then one, they have to be 
promiscuous to allow bridging between them.

The alternative might be to manipulate mac filter based on the
addresses learned.  This will most likely rather quickly put the port in 
promiscuous mode anyway if the event that a bridge is not really an edge 
device.

-vlad
>
> Exactly same logic applies, separately, for unicast, where promisc
> means all uni, and multicast, where promisc means all multi.
>
> Now there's no special unplink port, no need to enable/disable special
> mode, nothing: just program addresses for ports and go.
>
>
>> Changes since rfc v2:
>> * Sync/unsync address on uplink upon the uplink flag change.  This allows
>> for uplink replacements without loss of addresses.
>>
>> Changes since rfc 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 (4):
>>    bridge: Add sysfs interface to control promisc mode
>>    bridge: Allow an ability to designate an uplink port
>>    bridge: Implement IFF_UNICAST_FLT
>>    bridge: sync device list when a new uplink is designated
>>
>>   include/uapi/linux/if_link.h |    1 +
>>   net/bridge/br_device.c       |   52 +++++++++++++++++++++++++++++++++++++++++-
>>   net/bridge/br_fdb.c          |    6 +++++
>>   net/bridge/br_if.c           |   24 +++++++++++++++----
>>   net/bridge/br_netlink.c      |   13 ++++++++++
>>   net/bridge/br_private.h      |    3 ++
>>   net/bridge/br_sysfs_br.c     |   17 +++++++++++++
>>   net/bridge/br_sysfs_if.c     |   27 +++++++++++++++++++++
>>   8 files changed, 137 insertions(+), 6 deletions(-)
>>
>> --
>> 1.7.7.6
>>
>> --
>> 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

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

[Michael S. Tsirkin]

On Thu, Mar 14, 2013 at 11:10:34AM -0400, Vlad Yasevich wrote:
> On 03/13/2013 04:31 PM, Michael S. Tsirkin wrote:
> >On Tue, Mar 12, 2013 at 09:45:22PM -0400, Vlad Yasevich wrote:
> >>The series adds an ability for the bridge to function in non-promiscuous mode.
> >>We do it in 3 steps.
> >>First we add an interface to palce the switch into non-promisc mode.  In
> >>this mode, all port of the switch turn promisc off and turn on IFF_ALLMULTI
> >>to continue handling multicast traffic.
> >
> >Hmm why not go the extra mile and do same thing for multicast?
> 
> That would be simple enough to add, as I think all this would
> require is an additional dev_mc_sync() call.
> 
> I wanted to get basic functionality and unicast working.
> 
> >
> >>Second we add an ability to designate a bridge port as uplink.
> >>Third we add IFF_UNICAST_FLT support to the bridge and sync all unicast
> >>HW addresses to the uplink ports.
> >>Default bridge operation continues to remain "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.
> >
> >Actually, it seems a bit tricky to use correctly.  Specifying MACs is
> >straight-forward enough, but as you add and remove interfaces, you would
> >need to play with uplink and promisc bits.
> 
> Not the promisc bits since this patch series proposes a bridge wide
> setting.  Yes, uplink bits would need to be played with though.  I'd
> really love to specify those bits when the link is added, but the
> netlink operation for bridge is a bit odd.
> 
> May be I should re-fix that.
> 
> > I also think that for setups
> >you describe, where we really want to forward traffic to VMs and that's
> >all, promisc mode is an implementation detail. If true it's a mistake to
> >expose it in an interface.  Here's an idea how we could make it work
> >automatically:
> 
> This is something Stephen suggested earlier and controlling promisc mode
> based on whether uplinks are set or not.
> 
> >
> >- Like your patch does, allow two kinds of ports: port that only wants
> >   specific addresses and "wildcard" port that can get all addresses
> >   All ports default to wildcard as a compatible way
> >- For port A, if there is some wildcard port besides A, A must be
> >   promiscous since maybe it needs to get packets that will be later
> >   forwardd to A
> >- For port A, if there are no wildcard ports, or if A is the only
> >   wildcard, A does not need to be promiscous.
> >   Instead we can program it with addresses of all other ports.
> 
> So, according to the above, you can only ever have 1 port that is
> 'address specific'.  If you have more then one, they have to be
> promiscuous to allow bridging between them.

No, the reverse. Only 1 wildcard port without promisc.

> The alternative might be to manipulate mac filter based on the
> addresses learned.  This will most likely rather quickly put the
> port in promiscuous mode anyway if the event that a bridge is not
> really an edge device.
> 
> -vlad

It won't work, learning requires that you flood
addresses that are not learned yet. If you disable promisc
hardware filters them.

> >
> >Exactly same logic applies, separately, for unicast, where promisc
> >means all uni, and multicast, where promisc means all multi.
> >
> >Now there's no special unplink port, no need to enable/disable special
> >mode, nothing: just program addresses for ports and go.
> >
> >
> >>Changes since rfc v2:
> >>* Sync/unsync address on uplink upon the uplink flag change.  This allows
> >>for uplink replacements without loss of addresses.
> >>
> >>Changes since rfc 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 (4):
> >>   bridge: Add sysfs interface to control promisc mode
> >>   bridge: Allow an ability to designate an uplink port
> >>   bridge: Implement IFF_UNICAST_FLT
> >>   bridge: sync device list when a new uplink is designated
> >>
> >>  include/uapi/linux/if_link.h |    1 +
> >>  net/bridge/br_device.c       |   52 +++++++++++++++++++++++++++++++++++++++++-
> >>  net/bridge/br_fdb.c          |    6 +++++
> >>  net/bridge/br_if.c           |   24 +++++++++++++++----
> >>  net/bridge/br_netlink.c      |   13 ++++++++++
> >>  net/bridge/br_private.h      |    3 ++
> >>  net/bridge/br_sysfs_br.c     |   17 +++++++++++++
> >>  net/bridge/br_sysfs_if.c     |   27 +++++++++++++++++++++
> >>  8 files changed, 137 insertions(+), 6 deletions(-)
> >>
> >>--
> >>1.7.7.6
> >>
> >>--
> >>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