* [PATCH v2 net 0/3] bridge: Add vlan filtering support for default pvid
@ 2014-09-30 19:30 ` Vladislav Yasevich
0 siblings, 0 replies; 20+ messages in thread
From: Vladislav Yasevich @ 2014-09-30 19:30 UTC (permalink / raw
To: netdev; +Cc: stephen, bridge, Vladislav Yasevich
Version 2 of the series to introduce the default pvid support to
vlan filtering in the bridge. VLAN 1 (as recommended by 802.1q spec)
is used as default pvid on ports.
The the user can over-ride this configuration by configuring their
own vlan information.
The user can additionally change the default value throught the
sysfs interface (netlink comming shortly).
The user can turn off default pvid functionality by setting default
pvid to 0.
This series changes the default behavior of the bridge when
vlan filtering is turned on. Currently, ports without any vlan
filtering configured will not recevie any traffic at all. This patch
changes the behavior of the above ports to receive only untagged traffic.
Since v2:
- Add ability to turn off default_pvid settings.
- Drop the automiatic filtering support based on configured vlan devices (will
be its own series)
Thanks
-vlad
Vladislav Yasevich (3):
bridge: Add a default_pvid sysfs attribute
bridge: Simplify pvid checks.
bridge: Add filtering support for default_pvid
net/bridge/br_device.c | 8 ++-
net/bridge/br_if.c | 2 +
net/bridge/br_private.h | 17 +++++-
net/bridge/br_sysfs_br.c | 17 ++++++
net/bridge/br_vlan.c | 143 ++++++++++++++++++++++++++++++++++++++++++++++-
5 files changed, 179 insertions(+), 8 deletions(-)
--
1.9.3
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Bridge] [PATCH v2 net 0/3] bridge: Add vlan filtering support for default pvid
@ 2014-09-30 19:30 ` Vladislav Yasevich
0 siblings, 0 replies; 20+ messages in thread
From: Vladislav Yasevich @ 2014-09-30 19:30 UTC (permalink / raw
To: netdev; +Cc: stephen, bridge, Vladislav Yasevich
Version 2 of the series to introduce the default pvid support to
vlan filtering in the bridge. VLAN 1 (as recommended by 802.1q spec)
is used as default pvid on ports.
The the user can over-ride this configuration by configuring their
own vlan information.
The user can additionally change the default value throught the
sysfs interface (netlink comming shortly).
The user can turn off default pvid functionality by setting default
pvid to 0.
This series changes the default behavior of the bridge when
vlan filtering is turned on. Currently, ports without any vlan
filtering configured will not recevie any traffic at all. This patch
changes the behavior of the above ports to receive only untagged traffic.
Since v2:
- Add ability to turn off default_pvid settings.
- Drop the automiatic filtering support based on configured vlan devices (will
be its own series)
Thanks
-vlad
Vladislav Yasevich (3):
bridge: Add a default_pvid sysfs attribute
bridge: Simplify pvid checks.
bridge: Add filtering support for default_pvid
net/bridge/br_device.c | 8 ++-
net/bridge/br_if.c | 2 +
net/bridge/br_private.h | 17 +++++-
net/bridge/br_sysfs_br.c | 17 ++++++
net/bridge/br_vlan.c | 143 ++++++++++++++++++++++++++++++++++++++++++++++-
5 files changed, 179 insertions(+), 8 deletions(-)
--
1.9.3
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 net 1/3] bridge: Add a default_pvid sysfs attribute
2014-09-30 19:30 ` [Bridge] " Vladislav Yasevich
@ 2014-09-30 19:31 ` Vladislav Yasevich
-1 siblings, 0 replies; 20+ messages in thread
From: Vladislav Yasevich @ 2014-09-30 19:31 UTC (permalink / raw
To: netdev; +Cc: toshiaki.makita1, stephen, bridge, Vladislav Yasevich
This patch allows the user to set and retrieve default_pvid
value. A new value can only be stored when vlan filtering
is disabled.
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
net/bridge/br_private.h | 2 ++
net/bridge/br_sysfs_br.c | 17 +++++++++++++++++
net/bridge/br_vlan.c | 28 ++++++++++++++++++++++++++++
3 files changed, 47 insertions(+)
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index b6c04cb..6fe8680 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -299,6 +299,7 @@ struct net_bridge
#ifdef CONFIG_BRIDGE_VLAN_FILTERING
u8 vlan_enabled;
__be16 vlan_proto;
+ u16 default_pvid;
struct net_port_vlans __rcu *vlan_info;
#endif
};
@@ -605,6 +606,7 @@ void br_recalculate_fwd_mask(struct net_bridge *br);
int br_vlan_filter_toggle(struct net_bridge *br, unsigned long val);
int br_vlan_set_proto(struct net_bridge *br, unsigned long val);
void br_vlan_init(struct net_bridge *br);
+int br_vlan_set_default_pvid(struct net_bridge *br, unsigned long val);
int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags);
int nbp_vlan_delete(struct net_bridge_port *port, u16 vid);
void nbp_vlan_flush(struct net_bridge_port *port);
diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
index c9e2572..b969d9e 100644
--- a/net/bridge/br_sysfs_br.c
+++ b/net/bridge/br_sysfs_br.c
@@ -725,6 +725,22 @@ static ssize_t vlan_protocol_store(struct device *d,
return store_bridge_parm(d, buf, len, br_vlan_set_proto);
}
static DEVICE_ATTR_RW(vlan_protocol);
+
+static ssize_t default_pvid_show(struct device *d,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct net_bridge *br = to_bridge(d);
+ return sprintf(buf, "%d\n", br->default_pvid);
+}
+
+static ssize_t default_pvid_store(struct device *d,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ return store_bridge_parm(d, buf, len, br_vlan_set_default_pvid);
+}
+static DEVICE_ATTR_RW(default_pvid);
#endif
static struct attribute *bridge_attrs[] = {
@@ -771,6 +787,7 @@ static struct attribute *bridge_attrs[] = {
#ifdef CONFIG_BRIDGE_VLAN_FILTERING
&dev_attr_vlan_filtering.attr,
&dev_attr_vlan_protocol.attr,
+ &dev_attr_default_pvid.attr,
#endif
NULL
};
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 3ba57fc..295ad0d 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -499,9 +499,37 @@ err_filt:
goto unlock;
}
+int br_vlan_set_default_pvid(struct net_bridge *br, unsigned long val)
+{
+ u16 pvid = val;
+ int err = 0;
+
+ if (!pvid || pvid >= VLAN_VID_MASK)
+ return -EINVAL;
+
+ if (!rtnl_trylock())
+ return restart_syscall();
+
+ if (pvid == br->default_pvid)
+ goto unlock;
+
+ /* Only allow default pvid change when filtering is disabled */
+ if (br->vlan_enabled) {
+ err = -EPERM;
+ goto unlock;
+ }
+
+ br->default_pvid = vid;
+
+unlock:
+ rtnl_unlock();
+ return err;
+}
+
void br_vlan_init(struct net_bridge *br)
{
br->vlan_proto = htons(ETH_P_8021Q);
+ br->default_pvid = 1;
}
/* Must be protected by RTNL.
--
1.9.3
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Bridge] [PATCH v2 net 1/3] bridge: Add a default_pvid sysfs attribute
@ 2014-09-30 19:31 ` Vladislav Yasevich
0 siblings, 0 replies; 20+ messages in thread
From: Vladislav Yasevich @ 2014-09-30 19:31 UTC (permalink / raw
To: netdev; +Cc: stephen, bridge, Vladislav Yasevich
This patch allows the user to set and retrieve default_pvid
value. A new value can only be stored when vlan filtering
is disabled.
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
net/bridge/br_private.h | 2 ++
net/bridge/br_sysfs_br.c | 17 +++++++++++++++++
net/bridge/br_vlan.c | 28 ++++++++++++++++++++++++++++
3 files changed, 47 insertions(+)
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index b6c04cb..6fe8680 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -299,6 +299,7 @@ struct net_bridge
#ifdef CONFIG_BRIDGE_VLAN_FILTERING
u8 vlan_enabled;
__be16 vlan_proto;
+ u16 default_pvid;
struct net_port_vlans __rcu *vlan_info;
#endif
};
@@ -605,6 +606,7 @@ void br_recalculate_fwd_mask(struct net_bridge *br);
int br_vlan_filter_toggle(struct net_bridge *br, unsigned long val);
int br_vlan_set_proto(struct net_bridge *br, unsigned long val);
void br_vlan_init(struct net_bridge *br);
+int br_vlan_set_default_pvid(struct net_bridge *br, unsigned long val);
int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags);
int nbp_vlan_delete(struct net_bridge_port *port, u16 vid);
void nbp_vlan_flush(struct net_bridge_port *port);
diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
index c9e2572..b969d9e 100644
--- a/net/bridge/br_sysfs_br.c
+++ b/net/bridge/br_sysfs_br.c
@@ -725,6 +725,22 @@ static ssize_t vlan_protocol_store(struct device *d,
return store_bridge_parm(d, buf, len, br_vlan_set_proto);
}
static DEVICE_ATTR_RW(vlan_protocol);
+
+static ssize_t default_pvid_show(struct device *d,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct net_bridge *br = to_bridge(d);
+ return sprintf(buf, "%d\n", br->default_pvid);
+}
+
+static ssize_t default_pvid_store(struct device *d,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ return store_bridge_parm(d, buf, len, br_vlan_set_default_pvid);
+}
+static DEVICE_ATTR_RW(default_pvid);
#endif
static struct attribute *bridge_attrs[] = {
@@ -771,6 +787,7 @@ static struct attribute *bridge_attrs[] = {
#ifdef CONFIG_BRIDGE_VLAN_FILTERING
&dev_attr_vlan_filtering.attr,
&dev_attr_vlan_protocol.attr,
+ &dev_attr_default_pvid.attr,
#endif
NULL
};
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 3ba57fc..295ad0d 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -499,9 +499,37 @@ err_filt:
goto unlock;
}
+int br_vlan_set_default_pvid(struct net_bridge *br, unsigned long val)
+{
+ u16 pvid = val;
+ int err = 0;
+
+ if (!pvid || pvid >= VLAN_VID_MASK)
+ return -EINVAL;
+
+ if (!rtnl_trylock())
+ return restart_syscall();
+
+ if (pvid == br->default_pvid)
+ goto unlock;
+
+ /* Only allow default pvid change when filtering is disabled */
+ if (br->vlan_enabled) {
+ err = -EPERM;
+ goto unlock;
+ }
+
+ br->default_pvid = vid;
+
+unlock:
+ rtnl_unlock();
+ return err;
+}
+
void br_vlan_init(struct net_bridge *br)
{
br->vlan_proto = htons(ETH_P_8021Q);
+ br->default_pvid = 1;
}
/* Must be protected by RTNL.
--
1.9.3
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 net 2/3] bridge: Simplify pvid checks.
2014-09-30 19:30 ` [Bridge] " Vladislav Yasevich
@ 2014-09-30 19:31 ` Vladislav Yasevich
-1 siblings, 0 replies; 20+ messages in thread
From: Vladislav Yasevich @ 2014-09-30 19:31 UTC (permalink / raw
To: netdev; +Cc: toshiaki.makita1, stephen, bridge, Vladislav Yasevich
Currently, if the pvid is not set, we return an illegal vlan value
even though the pvid value is set to 0. Since pvid of 0 is currently
invalid, just return 0 instead. This makes the current and future
checks simpler.
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
net/bridge/br_private.h | 2 +-
net/bridge/br_vlan.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 6fe8680..873d9da 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -647,7 +647,7 @@ static inline u16 br_get_pvid(const struct net_port_vlans *v)
* vid wasn't set
*/
smp_rmb();
- return v->pvid ?: VLAN_N_VID;
+ return v->pvid;
}
static inline int br_vlan_enabled(struct net_bridge *br)
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 295ad0d..f5eee61 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -223,7 +223,7 @@ bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
* See if pvid is set on this port. That tells us which
* vlan untagged or priority-tagged traffic belongs to.
*/
- if (pvid == VLAN_N_VID)
+ if (!pvid)
goto drop;
/* PVID is set on this port. Any untagged or priority-tagged
@@ -292,7 +292,7 @@ bool br_should_learn(struct net_bridge_port *p, struct sk_buff *skb, u16 *vid)
if (!*vid) {
*vid = br_get_pvid(v);
- if (*vid == VLAN_N_VID)
+ if (!*vid)
return false;
return true;
--
1.9.3
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Bridge] [PATCH v2 net 2/3] bridge: Simplify pvid checks.
@ 2014-09-30 19:31 ` Vladislav Yasevich
0 siblings, 0 replies; 20+ messages in thread
From: Vladislav Yasevich @ 2014-09-30 19:31 UTC (permalink / raw
To: netdev; +Cc: stephen, bridge, Vladislav Yasevich
Currently, if the pvid is not set, we return an illegal vlan value
even though the pvid value is set to 0. Since pvid of 0 is currently
invalid, just return 0 instead. This makes the current and future
checks simpler.
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
net/bridge/br_private.h | 2 +-
net/bridge/br_vlan.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 6fe8680..873d9da 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -647,7 +647,7 @@ static inline u16 br_get_pvid(const struct net_port_vlans *v)
* vid wasn't set
*/
smp_rmb();
- return v->pvid ?: VLAN_N_VID;
+ return v->pvid;
}
static inline int br_vlan_enabled(struct net_bridge *br)
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 295ad0d..f5eee61 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -223,7 +223,7 @@ bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
* See if pvid is set on this port. That tells us which
* vlan untagged or priority-tagged traffic belongs to.
*/
- if (pvid == VLAN_N_VID)
+ if (!pvid)
goto drop;
/* PVID is set on this port. Any untagged or priority-tagged
@@ -292,7 +292,7 @@ bool br_should_learn(struct net_bridge_port *p, struct sk_buff *skb, u16 *vid)
if (!*vid) {
*vid = br_get_pvid(v);
- if (*vid == VLAN_N_VID)
+ if (!*vid)
return false;
return true;
--
1.9.3
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 net 3/3] bridge: Add filtering support for default_pvid
2014-09-30 19:30 ` [Bridge] " Vladislav Yasevich
@ 2014-09-30 19:31 ` Vladislav Yasevich
-1 siblings, 0 replies; 20+ messages in thread
From: Vladislav Yasevich @ 2014-09-30 19:31 UTC (permalink / raw
To: netdev; +Cc: toshiaki.makita1, stephen, bridge, Vladislav Yasevich
Currently when vlan filtering is turned on on the bridge, the bridge
will drop all traffic untill the user configures the filter. This
isn't very nice for ports that don't care about vlans and just
want untagged traffic.
A concept of a default_pvid was recently introduced. This patch
adds filtering support for default_pvid. Now, ports that don't
care about vlans and don't define there own filter will belong
to the VLAN of the default_pvid and continue to receive untagged
traffic.
This filtering can be disabled by setting default_pvid to 0.
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
net/bridge/br_device.c | 8 +++-
net/bridge/br_if.c | 2 +
net/bridge/br_private.h | 13 +++++-
net/bridge/br_vlan.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++--
4 files changed, 132 insertions(+), 7 deletions(-)
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 568cccd..af8f706 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -88,12 +88,17 @@ out:
static int br_dev_init(struct net_device *dev)
{
struct net_bridge *br = netdev_priv(dev);
+ int err;
br->stats = netdev_alloc_pcpu_stats(struct pcpu_sw_netstats);
if (!br->stats)
return -ENOMEM;
- return 0;
+ err = br_vlan_init(br);
+ if (err)
+ free_percpu(br->stats);
+
+ return err;
}
static int br_dev_open(struct net_device *dev)
@@ -389,5 +394,4 @@ void br_dev_setup(struct net_device *dev)
br_netfilter_rtable_init(br);
br_stp_timer_init(br);
br_multicast_init(br);
- br_vlan_init(br);
}
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 078d336..b7a00bc 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -500,6 +500,8 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
if (br_fdb_insert(br, p, dev->dev_addr, 0))
netdev_err(dev, "failed insert local address bridge forwarding table\n");
+ nbp_vlan_init(p);
+
spin_lock_bh(&br->lock);
changed_addr = br_stp_recalculate_bridge_id(br);
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 873d9da..49575b3 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -605,12 +605,13 @@ bool br_vlan_find(struct net_bridge *br, u16 vid);
void br_recalculate_fwd_mask(struct net_bridge *br);
int br_vlan_filter_toggle(struct net_bridge *br, unsigned long val);
int br_vlan_set_proto(struct net_bridge *br, unsigned long val);
-void br_vlan_init(struct net_bridge *br);
+int br_vlan_init(struct net_bridge *br);
int br_vlan_set_default_pvid(struct net_bridge *br, unsigned long val);
int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags);
int nbp_vlan_delete(struct net_bridge_port *port, u16 vid);
void nbp_vlan_flush(struct net_bridge_port *port);
bool nbp_vlan_find(struct net_bridge_port *port, u16 vid);
+int nbp_vlan_init(struct net_bridge_port *port);
static inline struct net_port_vlans *br_get_vlan_info(
const struct net_bridge *br)
@@ -643,6 +644,8 @@ static inline int br_vlan_get_tag(const struct sk_buff *skb, u16 *vid)
static inline u16 br_get_pvid(const struct net_port_vlans *v)
{
+ if (!v)
+ return 0;
/* Return just the VID if it is set, or VLAN_N_VID (invalid vid) if
* vid wasn't set
*/
@@ -706,8 +709,9 @@ static inline void br_recalculate_fwd_mask(struct net_bridge *br)
{
}
-static inline void br_vlan_init(struct net_bridge *br)
+static inline int br_vlan_init(struct net_bridge *br)
{
+ return 0;
}
static inline int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags)
@@ -740,6 +744,11 @@ static inline bool nbp_vlan_find(struct net_bridge_port *port, u16 vid)
return false;
}
+static inline int nbp_vlan_init(struct net_bridge_port *port)
+{
+ return 0;
+}
+
static inline u16 br_vlan_get_tag(const struct sk_buff *skb, u16 *tag)
{
return 0;
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index f5eee61..377cea6 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -499,12 +499,110 @@ err_filt:
goto unlock;
}
+static void br_vlan_disable_default_pvid(struct net_bridge *br)
+{
+ struct net_bridge_port *p;
+ unsigned short pvid = br->default_pvid;
+
+ /* This function runs under RTNL with vlan filter disabled.
+ * It is safe to directly access rcu pointers.
+ *
+ * Disable default_pvid on all ports where it is still
+ * configured.
+ */
+ if (pvid == br_get_pvid(br_get_vlan_info(br)) &&
+ test_bit(pvid, br->vlan_info->untagged_bitmap))
+ br_vlan_delete(br, pvid);
+
+ list_for_each_entry(p, &br->port_list, list) {
+ if (pvid == br_get_pvid(nbp_get_vlan_info(p)) &&
+ test_bit(pvid, p->vlan_info->untagged_bitmap))
+ nbp_vlan_delete(p, pvid);
+ }
+
+ br->default_pvid = 0;
+}
+
+static int __br_vlan_set_default_pvid(struct net_bridge *br, u16 pvid)
+{
+ struct net_bridge_port *p;
+ u16 old_pvid;
+ int err;
+ DECLARE_BITMAP(changed, BR_MAX_PORTS);
+
+ bitmap_zero(changed, BR_MAX_PORTS);
+
+ /* This function runs with filtering turned off so we can
+ * remove the old pvid configuration and add the new one after
+ * without impacting traffic.
+ */
+
+ old_pvid = br->default_pvid;
+
+ /* If the user has set a different PVID or if the new default pvid
+ * conflicts with user configuration, do not modify the configuration.
+ */
+ if (old_pvid != br_get_pvid(br_get_vlan_info(br)) ||
+ br_vlan_find(br, pvid))
+ goto do_ports;
+
+ set_bit(0, changed);
+ br_vlan_delete(br, old_pvid);
+ err = br_vlan_add(br, pvid,
+ BRIDGE_VLAN_INFO_PVID | BRIDGE_VLAN_INFO_UNTAGGED);
+ if (err)
+ goto err_br;
+
+do_ports:
+ list_for_each_entry(p, &br->port_list, list) {
+ /* If the user has set a different PVID or if the new default
+ * pvid conflicts with user configuration, do not modify the
+ * configuration.
+ */
+ if (old_pvid != br_get_pvid(nbp_get_vlan_info(p)) ||
+ nbp_vlan_find(p, pvid))
+ continue;
+
+ set_bit(p->port_no, changed);
+ err = nbp_vlan_add(p, pvid,
+ BRIDGE_VLAN_INFO_PVID |
+ BRIDGE_VLAN_INFO_UNTAGGED);
+ if (err)
+ goto err_port;
+ nbp_vlan_delete(p, old_pvid);
+ }
+
+ br->default_pvid = pvid;
+
+ return 0;
+
+err_port:
+ list_for_each_entry_continue_reverse(p, &br->port_list, list) {
+ if (!test_bit(p->port_no, changed))
+ continue;
+
+ nbp_vlan_delete(p, pvid);
+ nbp_vlan_add(p, old_pvid,
+ BRIDGE_VLAN_INFO_PVID |
+ BRIDGE_VLAN_INFO_UNTAGGED);
+ }
+
+err_br:
+ if (test_bit(0, changed)) {
+ br_vlan_delete(br, pvid);
+ br_vlan_add(br, old_pvid,
+ BRIDGE_VLAN_INFO_PVID |
+ BRIDGE_VLAN_INFO_UNTAGGED);
+ }
+ return err;
+}
+
int br_vlan_set_default_pvid(struct net_bridge *br, unsigned long val)
{
u16 pvid = val;
int err = 0;
- if (!pvid || pvid >= VLAN_VID_MASK)
+ if (pvid >= VLAN_VID_MASK)
return -EINVAL;
if (!rtnl_trylock())
@@ -519,17 +617,22 @@ int br_vlan_set_default_pvid(struct net_bridge *br, unsigned long val)
goto unlock;
}
- br->default_pvid = vid;
+ if (!pvid)
+ br_vlan_disable_default_pvid(br);
+ else
+ err = __br_vlan_set_default_pvid(br, pvid);
unlock:
rtnl_unlock();
return err;
}
-void br_vlan_init(struct net_bridge *br)
+int br_vlan_init(struct net_bridge *br)
{
br->vlan_proto = htons(ETH_P_8021Q);
br->default_pvid = 1;
+ return br_vlan_add(br, 1,
+ BRIDGE_VLAN_INFO_PVID | BRIDGE_VLAN_INFO_UNTAGGED);
}
/* Must be protected by RTNL.
@@ -621,3 +724,10 @@ out:
rcu_read_unlock();
return found;
}
+
+int nbp_vlan_init(struct net_bridge_port *p)
+{
+ return nbp_vlan_add(p, p->br->default_pvid,
+ BRIDGE_VLAN_INFO_PVID |
+ BRIDGE_VLAN_INFO_UNTAGGED);
+}
--
1.9.3
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Bridge] [PATCH v2 net 3/3] bridge: Add filtering support for default_pvid
@ 2014-09-30 19:31 ` Vladislav Yasevich
0 siblings, 0 replies; 20+ messages in thread
From: Vladislav Yasevich @ 2014-09-30 19:31 UTC (permalink / raw
To: netdev; +Cc: stephen, bridge, Vladislav Yasevich
Currently when vlan filtering is turned on on the bridge, the bridge
will drop all traffic untill the user configures the filter. This
isn't very nice for ports that don't care about vlans and just
want untagged traffic.
A concept of a default_pvid was recently introduced. This patch
adds filtering support for default_pvid. Now, ports that don't
care about vlans and don't define there own filter will belong
to the VLAN of the default_pvid and continue to receive untagged
traffic.
This filtering can be disabled by setting default_pvid to 0.
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
net/bridge/br_device.c | 8 +++-
net/bridge/br_if.c | 2 +
net/bridge/br_private.h | 13 +++++-
net/bridge/br_vlan.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++--
4 files changed, 132 insertions(+), 7 deletions(-)
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 568cccd..af8f706 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -88,12 +88,17 @@ out:
static int br_dev_init(struct net_device *dev)
{
struct net_bridge *br = netdev_priv(dev);
+ int err;
br->stats = netdev_alloc_pcpu_stats(struct pcpu_sw_netstats);
if (!br->stats)
return -ENOMEM;
- return 0;
+ err = br_vlan_init(br);
+ if (err)
+ free_percpu(br->stats);
+
+ return err;
}
static int br_dev_open(struct net_device *dev)
@@ -389,5 +394,4 @@ void br_dev_setup(struct net_device *dev)
br_netfilter_rtable_init(br);
br_stp_timer_init(br);
br_multicast_init(br);
- br_vlan_init(br);
}
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 078d336..b7a00bc 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -500,6 +500,8 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
if (br_fdb_insert(br, p, dev->dev_addr, 0))
netdev_err(dev, "failed insert local address bridge forwarding table\n");
+ nbp_vlan_init(p);
+
spin_lock_bh(&br->lock);
changed_addr = br_stp_recalculate_bridge_id(br);
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 873d9da..49575b3 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -605,12 +605,13 @@ bool br_vlan_find(struct net_bridge *br, u16 vid);
void br_recalculate_fwd_mask(struct net_bridge *br);
int br_vlan_filter_toggle(struct net_bridge *br, unsigned long val);
int br_vlan_set_proto(struct net_bridge *br, unsigned long val);
-void br_vlan_init(struct net_bridge *br);
+int br_vlan_init(struct net_bridge *br);
int br_vlan_set_default_pvid(struct net_bridge *br, unsigned long val);
int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags);
int nbp_vlan_delete(struct net_bridge_port *port, u16 vid);
void nbp_vlan_flush(struct net_bridge_port *port);
bool nbp_vlan_find(struct net_bridge_port *port, u16 vid);
+int nbp_vlan_init(struct net_bridge_port *port);
static inline struct net_port_vlans *br_get_vlan_info(
const struct net_bridge *br)
@@ -643,6 +644,8 @@ static inline int br_vlan_get_tag(const struct sk_buff *skb, u16 *vid)
static inline u16 br_get_pvid(const struct net_port_vlans *v)
{
+ if (!v)
+ return 0;
/* Return just the VID if it is set, or VLAN_N_VID (invalid vid) if
* vid wasn't set
*/
@@ -706,8 +709,9 @@ static inline void br_recalculate_fwd_mask(struct net_bridge *br)
{
}
-static inline void br_vlan_init(struct net_bridge *br)
+static inline int br_vlan_init(struct net_bridge *br)
{
+ return 0;
}
static inline int nbp_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags)
@@ -740,6 +744,11 @@ static inline bool nbp_vlan_find(struct net_bridge_port *port, u16 vid)
return false;
}
+static inline int nbp_vlan_init(struct net_bridge_port *port)
+{
+ return 0;
+}
+
static inline u16 br_vlan_get_tag(const struct sk_buff *skb, u16 *tag)
{
return 0;
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index f5eee61..377cea6 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -499,12 +499,110 @@ err_filt:
goto unlock;
}
+static void br_vlan_disable_default_pvid(struct net_bridge *br)
+{
+ struct net_bridge_port *p;
+ unsigned short pvid = br->default_pvid;
+
+ /* This function runs under RTNL with vlan filter disabled.
+ * It is safe to directly access rcu pointers.
+ *
+ * Disable default_pvid on all ports where it is still
+ * configured.
+ */
+ if (pvid == br_get_pvid(br_get_vlan_info(br)) &&
+ test_bit(pvid, br->vlan_info->untagged_bitmap))
+ br_vlan_delete(br, pvid);
+
+ list_for_each_entry(p, &br->port_list, list) {
+ if (pvid == br_get_pvid(nbp_get_vlan_info(p)) &&
+ test_bit(pvid, p->vlan_info->untagged_bitmap))
+ nbp_vlan_delete(p, pvid);
+ }
+
+ br->default_pvid = 0;
+}
+
+static int __br_vlan_set_default_pvid(struct net_bridge *br, u16 pvid)
+{
+ struct net_bridge_port *p;
+ u16 old_pvid;
+ int err;
+ DECLARE_BITMAP(changed, BR_MAX_PORTS);
+
+ bitmap_zero(changed, BR_MAX_PORTS);
+
+ /* This function runs with filtering turned off so we can
+ * remove the old pvid configuration and add the new one after
+ * without impacting traffic.
+ */
+
+ old_pvid = br->default_pvid;
+
+ /* If the user has set a different PVID or if the new default pvid
+ * conflicts with user configuration, do not modify the configuration.
+ */
+ if (old_pvid != br_get_pvid(br_get_vlan_info(br)) ||
+ br_vlan_find(br, pvid))
+ goto do_ports;
+
+ set_bit(0, changed);
+ br_vlan_delete(br, old_pvid);
+ err = br_vlan_add(br, pvid,
+ BRIDGE_VLAN_INFO_PVID | BRIDGE_VLAN_INFO_UNTAGGED);
+ if (err)
+ goto err_br;
+
+do_ports:
+ list_for_each_entry(p, &br->port_list, list) {
+ /* If the user has set a different PVID or if the new default
+ * pvid conflicts with user configuration, do not modify the
+ * configuration.
+ */
+ if (old_pvid != br_get_pvid(nbp_get_vlan_info(p)) ||
+ nbp_vlan_find(p, pvid))
+ continue;
+
+ set_bit(p->port_no, changed);
+ err = nbp_vlan_add(p, pvid,
+ BRIDGE_VLAN_INFO_PVID |
+ BRIDGE_VLAN_INFO_UNTAGGED);
+ if (err)
+ goto err_port;
+ nbp_vlan_delete(p, old_pvid);
+ }
+
+ br->default_pvid = pvid;
+
+ return 0;
+
+err_port:
+ list_for_each_entry_continue_reverse(p, &br->port_list, list) {
+ if (!test_bit(p->port_no, changed))
+ continue;
+
+ nbp_vlan_delete(p, pvid);
+ nbp_vlan_add(p, old_pvid,
+ BRIDGE_VLAN_INFO_PVID |
+ BRIDGE_VLAN_INFO_UNTAGGED);
+ }
+
+err_br:
+ if (test_bit(0, changed)) {
+ br_vlan_delete(br, pvid);
+ br_vlan_add(br, old_pvid,
+ BRIDGE_VLAN_INFO_PVID |
+ BRIDGE_VLAN_INFO_UNTAGGED);
+ }
+ return err;
+}
+
int br_vlan_set_default_pvid(struct net_bridge *br, unsigned long val)
{
u16 pvid = val;
int err = 0;
- if (!pvid || pvid >= VLAN_VID_MASK)
+ if (pvid >= VLAN_VID_MASK)
return -EINVAL;
if (!rtnl_trylock())
@@ -519,17 +617,22 @@ int br_vlan_set_default_pvid(struct net_bridge *br, unsigned long val)
goto unlock;
}
- br->default_pvid = vid;
+ if (!pvid)
+ br_vlan_disable_default_pvid(br);
+ else
+ err = __br_vlan_set_default_pvid(br, pvid);
unlock:
rtnl_unlock();
return err;
}
-void br_vlan_init(struct net_bridge *br)
+int br_vlan_init(struct net_bridge *br)
{
br->vlan_proto = htons(ETH_P_8021Q);
br->default_pvid = 1;
+ return br_vlan_add(br, 1,
+ BRIDGE_VLAN_INFO_PVID | BRIDGE_VLAN_INFO_UNTAGGED);
}
/* Must be protected by RTNL.
@@ -621,3 +724,10 @@ out:
rcu_read_unlock();
return found;
}
+
+int nbp_vlan_init(struct net_bridge_port *p)
+{
+ return nbp_vlan_add(p, p->br->default_pvid,
+ BRIDGE_VLAN_INFO_PVID |
+ BRIDGE_VLAN_INFO_UNTAGGED);
+}
--
1.9.3
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 net 0/3] bridge: Add vlan filtering support for default pvid
2014-09-30 19:30 ` [Bridge] " Vladislav Yasevich
@ 2014-09-30 21:07 ` David Miller
-1 siblings, 0 replies; 20+ messages in thread
From: David Miller @ 2014-09-30 21:07 UTC (permalink / raw
To: vyasevich; +Cc: netdev, toshiaki.makita1, stephen, bridge, vyasevic
From: Vladislav Yasevich <vyasevich@gmail.com>
Date: Tue, 30 Sep 2014 15:30:59 -0400
> Version 2 of the series to introduce the default pvid support to
> vlan filtering in the bridge. VLAN 1 (as recommended by 802.1q spec)
> is used as default pvid on ports.
> The the user can over-ride this configuration by configuring their
> own vlan information.
> The user can additionally change the default value throught the
> sysfs interface (netlink comming shortly).
> The user can turn off default pvid functionality by setting default
> pvid to 0.
> This series changes the default behavior of the bridge when
> vlan filtering is turned on. Currently, ports without any vlan
> filtering configured will not recevie any traffic at all. This patch
> changes the behavior of the above ports to receive only untagged traffic.
>
> Since v2:
> - Add ability to turn off default_pvid settings.
> - Drop the automiatic filtering support based on configured vlan devices (will
> be its own series)
Looks like a new feature to me, thus why is this being targetted at 'net'
instead of 'net-next'?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Bridge] [PATCH v2 net 0/3] bridge: Add vlan filtering support for default pvid
@ 2014-09-30 21:07 ` David Miller
0 siblings, 0 replies; 20+ messages in thread
From: David Miller @ 2014-09-30 21:07 UTC (permalink / raw
To: vyasevich; +Cc: stephen, netdev, vyasevic, bridge
From: Vladislav Yasevich <vyasevich@gmail.com>
Date: Tue, 30 Sep 2014 15:30:59 -0400
> Version 2 of the series to introduce the default pvid support to
> vlan filtering in the bridge. VLAN 1 (as recommended by 802.1q spec)
> is used as default pvid on ports.
> The the user can over-ride this configuration by configuring their
> own vlan information.
> The user can additionally change the default value throught the
> sysfs interface (netlink comming shortly).
> The user can turn off default pvid functionality by setting default
> pvid to 0.
> This series changes the default behavior of the bridge when
> vlan filtering is turned on. Currently, ports without any vlan
> filtering configured will not recevie any traffic at all. This patch
> changes the behavior of the above ports to receive only untagged traffic.
>
> Since v2:
> - Add ability to turn off default_pvid settings.
> - Drop the automiatic filtering support based on configured vlan devices (will
> be its own series)
Looks like a new feature to me, thus why is this being targetted at 'net'
instead of 'net-next'?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 net 0/3] bridge: Add vlan filtering support for default pvid
2014-09-30 21:07 ` [Bridge] " David Miller
@ 2014-09-30 23:16 ` Vlad Yasevich
-1 siblings, 0 replies; 20+ messages in thread
From: Vlad Yasevich @ 2014-09-30 23:16 UTC (permalink / raw
To: David Miller, vyasevich; +Cc: stephen, netdev, bridge
On 09/30/2014 05:07 PM, David Miller wrote:
> From: Vladislav Yasevich <vyasevich@gmail.com>
> Date: Tue, 30 Sep 2014 15:30:59 -0400
>
>> Version 2 of the series to introduce the default pvid support to
>> vlan filtering in the bridge. VLAN 1 (as recommended by 802.1q spec)
>> is used as default pvid on ports.
>> The the user can over-ride this configuration by configuring their
>> own vlan information.
>> The user can additionally change the default value throught the
>> sysfs interface (netlink comming shortly).
>> The user can turn off default pvid functionality by setting default
>> pvid to 0.
>> This series changes the default behavior of the bridge when
>> vlan filtering is turned on. Currently, ports without any vlan
>> filtering configured will not recevie any traffic at all. This patch
>> changes the behavior of the above ports to receive only untagged traffic.
>>
>> Since v2:
>> - Add ability to turn off default_pvid settings.
>> - Drop the automiatic filtering support based on configured vlan devices (will
>> be its own series)
>
> Looks like a new feature to me, thus why is this being targetted at 'net'
> instead of 'net-next'?
>
goof on my part. This should be net-next.
-vlad
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Bridge] [PATCH v2 net 0/3] bridge: Add vlan filtering support for default pvid
@ 2014-09-30 23:16 ` Vlad Yasevich
0 siblings, 0 replies; 20+ messages in thread
From: Vlad Yasevich @ 2014-09-30 23:16 UTC (permalink / raw
To: David Miller, vyasevich; +Cc: stephen, netdev, bridge
On 09/30/2014 05:07 PM, David Miller wrote:
> From: Vladislav Yasevich <vyasevich@gmail.com>
> Date: Tue, 30 Sep 2014 15:30:59 -0400
>
>> Version 2 of the series to introduce the default pvid support to
>> vlan filtering in the bridge. VLAN 1 (as recommended by 802.1q spec)
>> is used as default pvid on ports.
>> The the user can over-ride this configuration by configuring their
>> own vlan information.
>> The user can additionally change the default value throught the
>> sysfs interface (netlink comming shortly).
>> The user can turn off default pvid functionality by setting default
>> pvid to 0.
>> This series changes the default behavior of the bridge when
>> vlan filtering is turned on. Currently, ports without any vlan
>> filtering configured will not recevie any traffic at all. This patch
>> changes the behavior of the above ports to receive only untagged traffic.
>>
>> Since v2:
>> - Add ability to turn off default_pvid settings.
>> - Drop the automiatic filtering support based on configured vlan devices (will
>> be its own series)
>
> Looks like a new feature to me, thus why is this being targetted at 'net'
> instead of 'net-next'?
>
goof on my part. This should be net-next.
-vlad
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 net 1/3] bridge: Add a default_pvid sysfs attribute
2014-09-30 19:31 ` [Bridge] " Vladislav Yasevich
@ 2014-10-01 6:45 ` Toshiaki Makita
-1 siblings, 0 replies; 20+ messages in thread
From: Toshiaki Makita @ 2014-10-01 6:45 UTC (permalink / raw
To: Vladislav Yasevich, netdev; +Cc: stephen, bridge, Vladislav Yasevich
On 2014/10/01 4:31, Vladislav Yasevich wrote:
> This patch allows the user to set and retrieve default_pvid
> value. A new value can only be stored when vlan filtering
> is disabled.
>
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> ---
...
> +int br_vlan_set_default_pvid(struct net_bridge *br, unsigned long val)
> +{
> + u16 pvid = val;
> + int err = 0;
> +
> + if (!pvid || pvid >= VLAN_VID_MASK)
> + return -EINVAL;
This seems to accept a large value as a valid value.
For example, ((1 << 16) + 10) will be handled as 10.
How about using "val" for this check?
> +
> + if (!rtnl_trylock())
> + return restart_syscall();
> +
> + if (pvid == br->default_pvid)
> + goto unlock;
> +
> + /* Only allow default pvid change when filtering is disabled */
> + if (br->vlan_enabled) {
> + err = -EPERM;
Additional log message might help users know why it was rejected.
Thanks,
Toshiaki Makita
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Bridge] [PATCH v2 net 1/3] bridge: Add a default_pvid sysfs attribute
@ 2014-10-01 6:45 ` Toshiaki Makita
0 siblings, 0 replies; 20+ messages in thread
From: Toshiaki Makita @ 2014-10-01 6:45 UTC (permalink / raw
To: Vladislav Yasevich, netdev; +Cc: stephen, bridge, Vladislav Yasevich
On 2014/10/01 4:31, Vladislav Yasevich wrote:
> This patch allows the user to set and retrieve default_pvid
> value. A new value can only be stored when vlan filtering
> is disabled.
>
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> ---
...
> +int br_vlan_set_default_pvid(struct net_bridge *br, unsigned long val)
> +{
> + u16 pvid = val;
> + int err = 0;
> +
> + if (!pvid || pvid >= VLAN_VID_MASK)
> + return -EINVAL;
This seems to accept a large value as a valid value.
For example, ((1 << 16) + 10) will be handled as 10.
How about using "val" for this check?
> +
> + if (!rtnl_trylock())
> + return restart_syscall();
> +
> + if (pvid == br->default_pvid)
> + goto unlock;
> +
> + /* Only allow default pvid change when filtering is disabled */
> + if (br->vlan_enabled) {
> + err = -EPERM;
Additional log message might help users know why it was rejected.
Thanks,
Toshiaki Makita
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 net 2/3] bridge: Simplify pvid checks.
2014-09-30 19:31 ` [Bridge] " Vladislav Yasevich
@ 2014-10-01 6:50 ` Toshiaki Makita
-1 siblings, 0 replies; 20+ messages in thread
From: Toshiaki Makita @ 2014-10-01 6:50 UTC (permalink / raw
To: Vladislav Yasevich, netdev; +Cc: stephen, bridge, Vladislav Yasevich
On 2014/10/01 4:31, Vladislav Yasevich wrote:
> Currently, if the pvid is not set, we return an illegal vlan value
> even though the pvid value is set to 0. Since pvid of 0 is currently
> invalid, just return 0 instead. This makes the current and future
> checks simpler.
>
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> ---
> net/bridge/br_private.h | 2 +-
> net/bridge/br_vlan.c | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 6fe8680..873d9da 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -647,7 +647,7 @@ static inline u16 br_get_pvid(const struct net_port_vlans *v)
> * vid wasn't set
> */
The comment above should be changed as well.
> smp_rmb();
> - return v->pvid ?: VLAN_N_VID;
> + return v->pvid;
> }
Should fix br_get_pvid() when CONFIG_BRIDGE_VLAN_FILTERING=n?
It returns VLAN_N_VID, which might be confusing.
Thanks,
Toshiaki Makita
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Bridge] [PATCH v2 net 2/3] bridge: Simplify pvid checks.
@ 2014-10-01 6:50 ` Toshiaki Makita
0 siblings, 0 replies; 20+ messages in thread
From: Toshiaki Makita @ 2014-10-01 6:50 UTC (permalink / raw
To: Vladislav Yasevich, netdev; +Cc: stephen, bridge, Vladislav Yasevich
On 2014/10/01 4:31, Vladislav Yasevich wrote:
> Currently, if the pvid is not set, we return an illegal vlan value
> even though the pvid value is set to 0. Since pvid of 0 is currently
> invalid, just return 0 instead. This makes the current and future
> checks simpler.
>
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> ---
> net/bridge/br_private.h | 2 +-
> net/bridge/br_vlan.c | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 6fe8680..873d9da 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -647,7 +647,7 @@ static inline u16 br_get_pvid(const struct net_port_vlans *v)
> * vid wasn't set
> */
The comment above should be changed as well.
> smp_rmb();
> - return v->pvid ?: VLAN_N_VID;
> + return v->pvid;
> }
Should fix br_get_pvid() when CONFIG_BRIDGE_VLAN_FILTERING=n?
It returns VLAN_N_VID, which might be confusing.
Thanks,
Toshiaki Makita
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 net 3/3] bridge: Add filtering support for default_pvid
2014-09-30 19:31 ` [Bridge] " Vladislav Yasevich
@ 2014-10-01 6:54 ` Toshiaki Makita
-1 siblings, 0 replies; 20+ messages in thread
From: Toshiaki Makita @ 2014-10-01 6:54 UTC (permalink / raw
To: Vladislav Yasevich, netdev; +Cc: stephen, bridge, Vladislav Yasevich
On 2014/10/01 4:31, Vladislav Yasevich wrote:
> Currently when vlan filtering is turned on on the bridge, the bridge
> will drop all traffic untill the user configures the filter. This
> isn't very nice for ports that don't care about vlans and just
> want untagged traffic.
>
> A concept of a default_pvid was recently introduced. This patch
> adds filtering support for default_pvid. Now, ports that don't
> care about vlans and don't define there own filter will belong
> to the VLAN of the default_pvid and continue to receive untagged
> traffic.
>
> This filtering can be disabled by setting default_pvid to 0.
>
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> ---
...
> @@ -500,6 +500,8 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
> if (br_fdb_insert(br, p, dev->dev_addr, 0))
> netdev_err(dev, "failed insert local address bridge forwarding table\n");
>
> + nbp_vlan_init(p);
> +
The return value of nbp_vlan_init() is not handled.
How about logging in the same way as fdb-inserting?
> spin_lock_bh(&br->lock);
> changed_addr = br_stp_recalculate_bridge_id(br);
>
...
> +static void br_vlan_disable_default_pvid(struct net_bridge *br)
> +{
> + struct net_bridge_port *p;
> + unsigned short pvid = br->default_pvid;
u16 will be better than "unsigned short".
> +
> + /* This function runs under RTNL with vlan filter disabled.
> + * It is safe to directly access rcu pointers.
Is this OK with sparse?
(This isn't the same case as commit f9586f79bf61 "vlan: add
rtnl_dereference() annotations"?)
> + *
> + * Disable default_pvid on all ports where it is still
> + * configured.
> + */
> + if (pvid == br_get_pvid(br_get_vlan_info(br)) &&
> + test_bit(pvid, br->vlan_info->untagged_bitmap))
> + br_vlan_delete(br, pvid);
> +
> + list_for_each_entry(p, &br->port_list, list) {
> + if (pvid == br_get_pvid(nbp_get_vlan_info(p)) &&
> + test_bit(pvid, p->vlan_info->untagged_bitmap))
> + nbp_vlan_delete(p, pvid);
> + }
> +
> + br->default_pvid = 0;
> +}
> +
> +static int __br_vlan_set_default_pvid(struct net_bridge *br, u16 pvid)
> +{
> + struct net_bridge_port *p;
> + u16 old_pvid;
> + int err;
> + DECLARE_BITMAP(changed, BR_MAX_PORTS);
> +
> + bitmap_zero(changed, BR_MAX_PORTS);
> +
> + /* This function runs with filtering turned off so we can
> + * remove the old pvid configuration and add the new one after
> + * without impacting traffic.
> + */
> +
> + old_pvid = br->default_pvid;
> +
> + /* If the user has set a different PVID or if the new default pvid
> + * conflicts with user configuration, do not modify the configuration.
> + */
> + if (old_pvid != br_get_pvid(br_get_vlan_info(br)) ||
> + br_vlan_find(br, pvid))
> + goto do_ports;
Should check untagged_bitmap for old_pvid as well?
> +
> + set_bit(0, changed);
> + br_vlan_delete(br, old_pvid);
> + err = br_vlan_add(br, pvid,
> + BRIDGE_VLAN_INFO_PVID | BRIDGE_VLAN_INFO_UNTAGGED);
br_vlan_add() should be done before br_vlan_delete()?
br_vlan_delete() might free vlan_info which will be immediately
allocated by following br_vlan_add().
Thanks,
Toshiaki Makita
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Bridge] [PATCH v2 net 3/3] bridge: Add filtering support for default_pvid
@ 2014-10-01 6:54 ` Toshiaki Makita
0 siblings, 0 replies; 20+ messages in thread
From: Toshiaki Makita @ 2014-10-01 6:54 UTC (permalink / raw
To: Vladislav Yasevich, netdev; +Cc: stephen, bridge, Vladislav Yasevich
On 2014/10/01 4:31, Vladislav Yasevich wrote:
> Currently when vlan filtering is turned on on the bridge, the bridge
> will drop all traffic untill the user configures the filter. This
> isn't very nice for ports that don't care about vlans and just
> want untagged traffic.
>
> A concept of a default_pvid was recently introduced. This patch
> adds filtering support for default_pvid. Now, ports that don't
> care about vlans and don't define there own filter will belong
> to the VLAN of the default_pvid and continue to receive untagged
> traffic.
>
> This filtering can be disabled by setting default_pvid to 0.
>
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> ---
...
> @@ -500,6 +500,8 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
> if (br_fdb_insert(br, p, dev->dev_addr, 0))
> netdev_err(dev, "failed insert local address bridge forwarding table\n");
>
> + nbp_vlan_init(p);
> +
The return value of nbp_vlan_init() is not handled.
How about logging in the same way as fdb-inserting?
> spin_lock_bh(&br->lock);
> changed_addr = br_stp_recalculate_bridge_id(br);
>
...
> +static void br_vlan_disable_default_pvid(struct net_bridge *br)
> +{
> + struct net_bridge_port *p;
> + unsigned short pvid = br->default_pvid;
u16 will be better than "unsigned short".
> +
> + /* This function runs under RTNL with vlan filter disabled.
> + * It is safe to directly access rcu pointers.
Is this OK with sparse?
(This isn't the same case as commit f9586f79bf61 "vlan: add
rtnl_dereference() annotations"?)
> + *
> + * Disable default_pvid on all ports where it is still
> + * configured.
> + */
> + if (pvid == br_get_pvid(br_get_vlan_info(br)) &&
> + test_bit(pvid, br->vlan_info->untagged_bitmap))
> + br_vlan_delete(br, pvid);
> +
> + list_for_each_entry(p, &br->port_list, list) {
> + if (pvid == br_get_pvid(nbp_get_vlan_info(p)) &&
> + test_bit(pvid, p->vlan_info->untagged_bitmap))
> + nbp_vlan_delete(p, pvid);
> + }
> +
> + br->default_pvid = 0;
> +}
> +
> +static int __br_vlan_set_default_pvid(struct net_bridge *br, u16 pvid)
> +{
> + struct net_bridge_port *p;
> + u16 old_pvid;
> + int err;
> + DECLARE_BITMAP(changed, BR_MAX_PORTS);
> +
> + bitmap_zero(changed, BR_MAX_PORTS);
> +
> + /* This function runs with filtering turned off so we can
> + * remove the old pvid configuration and add the new one after
> + * without impacting traffic.
> + */
> +
> + old_pvid = br->default_pvid;
> +
> + /* If the user has set a different PVID or if the new default pvid
> + * conflicts with user configuration, do not modify the configuration.
> + */
> + if (old_pvid != br_get_pvid(br_get_vlan_info(br)) ||
> + br_vlan_find(br, pvid))
> + goto do_ports;
Should check untagged_bitmap for old_pvid as well?
> +
> + set_bit(0, changed);
> + br_vlan_delete(br, old_pvid);
> + err = br_vlan_add(br, pvid,
> + BRIDGE_VLAN_INFO_PVID | BRIDGE_VLAN_INFO_UNTAGGED);
br_vlan_add() should be done before br_vlan_delete()?
br_vlan_delete() might free vlan_info which will be immediately
allocated by following br_vlan_add().
Thanks,
Toshiaki Makita
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 net 0/3] bridge: Add vlan filtering support for default pvid
2014-09-30 19:30 ` [Bridge] " Vladislav Yasevich
@ 2014-10-02 1:53 ` David Miller
-1 siblings, 0 replies; 20+ messages in thread
From: David Miller @ 2014-10-02 1:53 UTC (permalink / raw
To: vyasevich; +Cc: stephen, netdev, vyasevic, bridge
From: Vladislav Yasevich <vyasevich@gmail.com>
Date: Tue, 30 Sep 2014 15:30:59 -0400
> Version 2 of the series to introduce the default pvid support to
> vlan filtering in the bridge. VLAN 1 (as recommended by 802.1q spec)
> is used as default pvid on ports.
> The the user can over-ride this configuration by configuring their
> own vlan information.
> The user can additionally change the default value throught the
> sysfs interface (netlink comming shortly).
> The user can turn off default pvid functionality by setting default
> pvid to 0.
> This series changes the default behavior of the bridge when
> vlan filtering is turned on. Currently, ports without any vlan
> filtering configured will not recevie any traffic at all. This patch
> changes the behavior of the above ports to receive only untagged traffic.
>
> Since v2:
> - Add ability to turn off default_pvid settings.
> - Drop the automiatic filtering support based on configured vlan devices (will
> be its own series)
Please address the given feedback, thanks Vladislav.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Bridge] [PATCH v2 net 0/3] bridge: Add vlan filtering support for default pvid
@ 2014-10-02 1:53 ` David Miller
0 siblings, 0 replies; 20+ messages in thread
From: David Miller @ 2014-10-02 1:53 UTC (permalink / raw
To: vyasevich; +Cc: stephen, netdev, vyasevic, bridge
From: Vladislav Yasevich <vyasevich@gmail.com>
Date: Tue, 30 Sep 2014 15:30:59 -0400
> Version 2 of the series to introduce the default pvid support to
> vlan filtering in the bridge. VLAN 1 (as recommended by 802.1q spec)
> is used as default pvid on ports.
> The the user can over-ride this configuration by configuring their
> own vlan information.
> The user can additionally change the default value throught the
> sysfs interface (netlink comming shortly).
> The user can turn off default pvid functionality by setting default
> pvid to 0.
> This series changes the default behavior of the bridge when
> vlan filtering is turned on. Currently, ports without any vlan
> filtering configured will not recevie any traffic at all. This patch
> changes the behavior of the above ports to receive only untagged traffic.
>
> Since v2:
> - Add ability to turn off default_pvid settings.
> - Drop the automiatic filtering support based on configured vlan devices (will
> be its own series)
Please address the given feedback, thanks Vladislav.
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2014-10-02 1:53 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-30 19:30 [PATCH v2 net 0/3] bridge: Add vlan filtering support for default pvid Vladislav Yasevich
2014-09-30 19:30 ` [Bridge] " Vladislav Yasevich
2014-09-30 19:31 ` [PATCH v2 net 1/3] bridge: Add a default_pvid sysfs attribute Vladislav Yasevich
2014-09-30 19:31 ` [Bridge] " Vladislav Yasevich
2014-10-01 6:45 ` Toshiaki Makita
2014-10-01 6:45 ` [Bridge] " Toshiaki Makita
2014-09-30 19:31 ` [PATCH v2 net 2/3] bridge: Simplify pvid checks Vladislav Yasevich
2014-09-30 19:31 ` [Bridge] " Vladislav Yasevich
2014-10-01 6:50 ` Toshiaki Makita
2014-10-01 6:50 ` [Bridge] " Toshiaki Makita
2014-09-30 19:31 ` [PATCH v2 net 3/3] bridge: Add filtering support for default_pvid Vladislav Yasevich
2014-09-30 19:31 ` [Bridge] " Vladislav Yasevich
2014-10-01 6:54 ` Toshiaki Makita
2014-10-01 6:54 ` [Bridge] " Toshiaki Makita
2014-09-30 21:07 ` [PATCH v2 net 0/3] bridge: Add vlan filtering support for default pvid David Miller
2014-09-30 21:07 ` [Bridge] " David Miller
2014-09-30 23:16 ` Vlad Yasevich
2014-09-30 23:16 ` [Bridge] " Vlad Yasevich
2014-10-02 1:53 ` David Miller
2014-10-02 1:53 ` [Bridge] " David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.