* [RFC PATCH net-next 1/3] bridge: Add mac_management sysfs interface
2013-03-07 2:31 [RFC PATCH net-next 0/3] Allow bridge to function in non-promisc mode Vlad Yasevich
@ 2013-03-07 2:31 ` Vlad Yasevich
2013-03-07 2:35 ` Vlad Yasevich
2013-03-07 2:31 ` [RFC PATCH net-next 2/3] bridge: Allow an ability to designate an uplink port Vlad Yasevich
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Vlad Yasevich @ 2013-03-07 2:31 UTC (permalink / raw)
To: netdev; +Cc: bridge, 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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RFC PATCH net-next 1/3] bridge: Add mac_management sysfs interface
2013-03-07 2:31 ` [RFC PATCH net-next 1/3] bridge: Add mac_management sysfs interface Vlad Yasevich
@ 2013-03-07 2:35 ` Vlad Yasevich
0 siblings, 0 replies; 12+ messages in thread
From: Vlad Yasevich @ 2013-03-07 2:35 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev, bridge
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
> };
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC PATCH net-next 2/3] bridge: Allow an ability to designate an uplink port
2013-03-07 2:31 [RFC PATCH net-next 0/3] Allow bridge to function in non-promisc mode Vlad Yasevich
2013-03-07 2:31 ` [RFC PATCH net-next 1/3] bridge: Add mac_management sysfs interface Vlad Yasevich
@ 2013-03-07 2:31 ` Vlad Yasevich
2013-03-07 2:31 ` [RFC PATCH net-next 3/3] bridge: Implement IFF_UNICAST_FLT Vlad Yasevich
2013-03-07 7:19 ` [RFC PATCH net-next 0/3] Allow bridge to function in non-promisc mode Stephen Hemminger
3 siblings, 0 replies; 12+ messages in thread
From: Vlad Yasevich @ 2013-03-07 2:31 UTC (permalink / raw)
To: netdev; +Cc: bridge, 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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [RFC PATCH net-next 3/3] bridge: Implement IFF_UNICAST_FLT
2013-03-07 2:31 [RFC PATCH net-next 0/3] Allow bridge to function in non-promisc mode Vlad Yasevich
2013-03-07 2:31 ` [RFC PATCH net-next 1/3] bridge: Add mac_management sysfs interface Vlad Yasevich
2013-03-07 2:31 ` [RFC PATCH net-next 2/3] bridge: Allow an ability to designate an uplink port Vlad Yasevich
@ 2013-03-07 2:31 ` Vlad Yasevich
2013-03-07 3:10 ` John Fastabend
2013-03-07 7:19 ` [RFC PATCH net-next 0/3] Allow bridge to function in non-promisc mode Stephen Hemminger
3 siblings, 1 reply; 12+ messages in thread
From: Vlad Yasevich @ 2013-03-07 2:31 UTC (permalink / raw)
To: netdev; +Cc: bridge, 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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RFC PATCH net-next 3/3] bridge: Implement IFF_UNICAST_FLT
2013-03-07 2:31 ` [RFC PATCH net-next 3/3] bridge: Implement IFF_UNICAST_FLT Vlad Yasevich
@ 2013-03-07 3:10 ` John Fastabend
2013-03-07 15:08 ` Vlad Yasevich
0 siblings, 1 reply; 12+ messages in thread
From: John Fastabend @ 2013-03-07 3:10 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev, bridge
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH net-next 3/3] bridge: Implement IFF_UNICAST_FLT
2013-03-07 3:10 ` John Fastabend
@ 2013-03-07 15:08 ` Vlad Yasevich
0 siblings, 0 replies; 12+ messages in thread
From: Vlad Yasevich @ 2013-03-07 15:08 UTC (permalink / raw)
To: John Fastabend; +Cc: netdev, bridge
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
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH net-next 0/3] Allow bridge to function in non-promisc mode
2013-03-07 2:31 [RFC PATCH net-next 0/3] Allow bridge to function in non-promisc mode Vlad Yasevich
` (2 preceding siblings ...)
2013-03-07 2:31 ` [RFC PATCH net-next 3/3] bridge: Implement IFF_UNICAST_FLT Vlad Yasevich
@ 2013-03-07 7:19 ` Stephen Hemminger
2013-03-07 15:35 ` Vlad Yasevich
3 siblings, 1 reply; 12+ messages in thread
From: Stephen Hemminger @ 2013-03-07 7:19 UTC (permalink / raw)
To: vyasevic; +Cc: netdev, bridge
[-- 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 --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH net-next 0/3] Allow bridge to function in non-promisc mode
2013-03-07 7:19 ` [RFC PATCH net-next 0/3] Allow bridge to function in non-promisc mode Stephen Hemminger
@ 2013-03-07 15:35 ` Vlad Yasevich
2013-03-07 17:13 ` Stephen Hemminger
0 siblings, 1 reply; 12+ messages in thread
From: Vlad Yasevich @ 2013-03-07 15:35 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, bridge
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH net-next 0/3] Allow bridge to function in non-promisc mode
2013-03-07 15:35 ` Vlad Yasevich
@ 2013-03-07 17:13 ` Stephen Hemminger
2013-03-07 17:21 ` Vlad Yasevich
2013-03-07 17:38 ` Vlad Yasevich
0 siblings, 2 replies; 12+ messages in thread
From: Stephen Hemminger @ 2013-03-07 17:13 UTC (permalink / raw)
To: vyasevic; +Cc: netdev, bridge
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH net-next 0/3] Allow bridge to function in non-promisc mode
2013-03-07 17:13 ` Stephen Hemminger
@ 2013-03-07 17:21 ` Vlad Yasevich
2013-03-07 17:38 ` Vlad Yasevich
1 sibling, 0 replies; 12+ messages in thread
From: Vlad Yasevich @ 2013-03-07 17:21 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, bridge
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH net-next 0/3] Allow bridge to function in non-promisc mode
2013-03-07 17:13 ` Stephen Hemminger
2013-03-07 17:21 ` Vlad Yasevich
@ 2013-03-07 17:38 ` Vlad Yasevich
1 sibling, 0 replies; 12+ messages in thread
From: Vlad Yasevich @ 2013-03-07 17:38 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, bridge
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.
>
^ permalink raw reply [flat|nested] 12+ messages in thread