* [PATCH V2 00/12] Add basic VLAN support to bridges
@ 2012-12-18 19:00 Vlad Yasevich
2012-12-18 19:00 ` [PATCH V2 01/12] bridge: Add vlan filtering infrastructure Vlad Yasevich
` (13 more replies)
0 siblings, 14 replies; 35+ messages in thread
From: Vlad Yasevich @ 2012-12-18 19:00 UTC (permalink / raw)
To: netdev; +Cc: shemminger, davem, or.gerlitz, jhs, mst
This series of patches provides an ability to add VLANs to the bridge
ports. This is similar to what can be found in most switches. The bridge
port may have any number of VLANs added to it including vlan 0 priority tagged
traffic. When vlans are added to the port, only traffic tagged with particular
vlan will forwarded over this port. Additionally, vlan ids are added to FDB
entries and become part of the lookup. This way we correctly identify the FDB
entry.
A single vlan may also be designated as untagged. Any untagged traffic
recieved by the port will be assigned to this vlan. Any traffic exiting
the port with a VID matching the untagged vlan will exit untagged (the
bridge will strip the vlan header). This is similar to "Native Vlan" support
available in most switches.
The default behavior ofthe bridge is unchanged if no vlans have been
configured.
Changes since v1:
- Fixed some forwarding bugs.
- Add vlan to local fdb entries. New local entries are created per vlan
to facilite correct forwarding to bridge interface.
- Allow configuration of vlans directly on the bridge master device
in addition to ports.
Changes since rfc v2:
- Per-port vlan bitmap is gone and is replaced with a vlan list.
- Added bridge vlan list, which is referenced by each port. Entries in
the birdge vlan list have port bitmap that shows which port are parts
of which vlan.
- Netlink API changes.
- Dropped sysfs support for now. If people think this is really usefull,
can add it back.
- Support for native/untagged vlans.
Changes since rfc v1:
- Comments addressed regarding formatting and RCU usage
- iocts have been removed and changed over the netlink interface.
- Added support of user added ndb entries.
- changed sysfs interface to export a bitmap. Also added a write interface.
I am not sure how much I like it, but it made my testing easier/faster. I
might change the write interface to take text instead of binary.
Vlad Yasevich (12):
bridge: Add vlan filtering infrastructure
bridge: Validate that vlan is permitted on ingress
bridge: Verify that a vlan is allowed to egress on give port
bridge: Cache vlan in the cb for faster egress lookup.
bridge: Add vlan to unicast fdb entries
bridge: Add vlan id to multicast groups
bridge: Add netlink interface to configure vlans on bridge ports
bridge: Add vlan support to static neighbors
bridge: Add the ability to configure untagged vlans
bridge: Implement untagged vlan handling
bridge: Dump vlan information from a bridge port
bridge: Add vlan support for local fdb entries
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 +-
drivers/net/macvlan.c | 2 +-
drivers/net/vxlan.c | 3 +-
include/linux/netdevice.h | 4 +-
include/uapi/linux/if_bridge.h | 23 ++-
include/uapi/linux/neighbour.h | 1 +
include/uapi/linux/rtnetlink.h | 1 +
net/bridge/br_device.c | 34 ++-
net/bridge/br_fdb.c | 253 ++++++++++++---
net/bridge/br_forward.c | 160 ++++++++++
net/bridge/br_if.c | 404 ++++++++++++++++++++++++-
net/bridge/br_input.c | 65 ++++-
net/bridge/br_multicast.c | 71 +++--
net/bridge/br_netlink.c | 178 ++++++++++--
net/bridge/br_private.h | 71 ++++-
net/core/rtnetlink.c | 40 ++-
16 files changed, 1190 insertions(+), 125 deletions(-)
--
1.7.7.6
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH V2 01/12] bridge: Add vlan filtering infrastructure
2012-12-18 19:00 [PATCH V2 00/12] Add basic VLAN support to bridges Vlad Yasevich
@ 2012-12-18 19:00 ` Vlad Yasevich
2012-12-18 21:13 ` Eric Dumazet
2012-12-18 19:00 ` [PATCH V2 02/12] bridge: Validate that vlan is permitted on ingress Vlad Yasevich
` (12 subsequent siblings)
13 siblings, 1 reply; 35+ messages in thread
From: Vlad Yasevich @ 2012-12-18 19:00 UTC (permalink / raw)
To: netdev; +Cc: shemminger, davem, or.gerlitz, jhs, mst
This is an infrastructure patch. It adds 2 structures types:
net_bridge_vlan - list element of all vlans that have been configured
on the bridge.
net_port_vlan - list element of all vlans configured on a specific port.
references net_bridge_vlan.
In this implementation, bridge has a hash list of all vlans that have
been added to the bridge. Each vlan element holds a vid and port_bitmap
where each port sets its bit if a given vlan is added to the port.
Each port has its own list of vlans. Each element here refrences a vlan
from the bridge list.
Write access to both lists is protected by RTNL, and read access is
protected by RCU.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
net/bridge/br_device.c | 3 +
net/bridge/br_if.c | 251 +++++++++++++++++++++++++++++++++++++++++++++++
net/bridge/br_private.h | 33 ++++++
3 files changed, 287 insertions(+), 0 deletions(-)
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 7c78e26..9546742 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -332,6 +332,7 @@ static struct device_type br_type = {
void br_dev_setup(struct net_device *dev)
{
struct net_bridge *br = netdev_priv(dev);
+ int i;
eth_hw_addr_random(dev);
ether_setup(dev);
@@ -354,6 +355,8 @@ void br_dev_setup(struct net_device *dev)
spin_lock_init(&br->lock);
INIT_LIST_HEAD(&br->port_list);
spin_lock_init(&br->hash_lock);
+ for (i = 0; i < BR_VID_HASH_SIZE; i++)
+ INIT_HLIST_HEAD(&br->vlan_hlist[i]);
br->bridge_id.prio[0] = 0x80;
br->bridge_id.prio[1] = 0x00;
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 1c8fdc3..14c7c6a 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -83,6 +83,254 @@ void br_port_carrier_check(struct net_bridge_port *p)
spin_unlock_bh(&br->lock);
}
+static void br_vlan_destroy(struct net_bridge_vlan *vlan)
+{
+ if (!bitmap_empty(vlan->port_bitmap, PORT_BITMAP_LEN)) {
+ pr_err("Attempt to delete a VLAN %d from the bridge with "
+ "non-empty port bitmap (%p)\n", vlan->vid, vlan);
+ BUG();
+ }
+
+ hlist_del_rcu(&vlan->hlist);
+ synchronize_net();
+ kfree_rcu(vlan, rcu);
+}
+
+static void br_vlan_hold(struct net_bridge_vlan *vlan)
+{
+ atomic_inc(&vlan->refcnt);
+}
+
+static void br_vlan_put(struct net_bridge_vlan *vlan)
+{
+ if (atomic_dec_and_test(&vlan->refcnt))
+ br_vlan_destroy(vlan);
+}
+
+struct net_bridge_vlan *br_vlan_find(struct net_bridge *br, u16 vid)
+{
+ struct net_bridge_vlan *vlan;
+ struct hlist_node *node;
+
+ hlist_for_each_entry_rcu(vlan, node,
+ &br->vlan_hlist[br_vlan_hash(vid)], hlist) {
+ if (vlan->vid == vid)
+ return vlan;
+ }
+
+ return NULL;
+}
+
+/* Must be protected by RTNL */
+struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
+ u16 flags)
+{
+ struct net_bridge_vlan *vlan;
+
+ ASSERT_RTNL();
+
+ vlan = br_vlan_find(br, vid);
+ if (vlan)
+ return vlan;
+
+ vlan = kzalloc(sizeof(struct net_bridge_vlan), GFP_KERNEL);
+ if (!vlan)
+ return NULL;
+
+ vlan->vid = vid;
+ atomic_set(&vlan->refcnt, 1);
+
+ if (flags & BRIDGE_FLAGS_SELF) {
+ /* Set bit 0 that is associated with the bridge master
+ * device. Port numbers start with 1.
+ */
+ set_bit(0, vlan->port_bitmap);
+ }
+
+ hlist_add_head_rcu(&vlan->hlist, &br->vlan_hlist[br_vlan_hash(vid)]);
+ return vlan;
+}
+
+/* Must be protected by RTNL */
+static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
+{
+ ASSERT_RTNL();
+
+ if (flags & BRIDGE_FLAGS_SELF) {
+ /* Clear bit 0 that is associated with the bridge master
+ * device.
+ */
+ clear_bit(0, vlan->port_bitmap);
+ }
+
+ /* Try to remove the vlan, but only once all the ports have
+ * been removed from the port bitmap
+ */
+ if (!bitmap_empty(vlan->port_bitmap, PORT_BITMAP_LEN))
+ return;
+
+ vlan->vid = BR_INVALID_VID;
+
+ /* Drop the self-ref to trigger descrution. */
+ br_vlan_put(vlan);
+}
+
+/* Must be protected by RTNL */
+int br_vlan_delete(struct net_bridge *br, u16 vid, u16 flags)
+{
+ struct net_bridge_vlan *vlan;
+
+ ASSERT_RTNL();
+
+ vlan = br_vlan_find(br, vid);
+ if (!vlan)
+ return -ENOENT;
+
+ br_vlan_del(vlan, flags);
+ return 0;
+}
+
+static void br_vlan_flush(struct net_bridge *br)
+{
+ struct net_bridge_vlan *vlan;
+ struct hlist_node *node;
+ struct hlist_node *tmp;
+ int i;
+
+ /* Make sure that there are no vlans left in the bridge after
+ * all the ports have been removed.
+ */
+ for (i = 0; i < BR_VID_HASH_SIZE; i++) {
+ hlist_for_each_entry_safe(vlan, node, tmp,
+ &br->vlan_hlist[i], hlist) {
+ br_vlan_del(vlan, BRIDGE_FLAGS_SELF);
+ }
+ }
+}
+
+struct net_port_vlan *nbp_vlan_find(const struct net_bridge_port *p, u16 vid)
+{
+ struct net_port_vlan *pve;
+
+ /* Must be done either in rcu critical section or with RTNL held */
+ WARN_ON_ONCE(!rcu_read_lock_held() && !rtnl_is_locked());
+
+ list_for_each_entry_rcu(pve, &p->vlan_list, list) {
+ if (pve->vid == vid)
+ return pve;
+ }
+
+ return NULL;
+}
+
+/* Must be protected by RTNL */
+int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
+{
+ struct net_port_vlan *pve;
+ struct net_bridge_vlan *vlan;
+ struct net_device *dev = p->dev;
+ int err;
+
+ ASSERT_RTNL();
+
+ /* Find a vlan in the bridge vlan list. If it isn't there,
+ * create it
+ */
+ vlan = br_vlan_add(p->br, vid, flags);
+ if (!vlan)
+ return -ENOMEM;
+
+ /* Check to see if this port is already part of the vlan. If
+ * it is, there is nothing more to do.
+ */
+ if (test_bit(p->port_no, vlan->port_bitmap))
+ return -EEXIST;
+
+ /* Create port vlan, link it to bridge vlan list, and add port the
+ * portgroup.
+ */
+ pve = kmalloc(sizeof(*pve), GFP_KERNEL);
+ if (!pve) {
+ err = -ENOMEM;
+ goto clean_up;
+ }
+
+ /* Add VLAN to the device filter if it is supported.
+ * Stricly speaking, this is not necessary now, since devices
+ * are made promiscuous by the bridge, but if that ever changes
+ * this code will allow tagged traffic to enter the bridge.
+ */
+ if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
+ dev->netdev_ops->ndo_vlan_rx_add_vid &&
+ dev->netdev_ops->ndo_vlan_rx_kill_vid) {
+ err = dev->netdev_ops->ndo_vlan_rx_add_vid(dev, vid);
+ if (err)
+ goto clean_up;
+ }
+
+ pve->vid = vid;
+ pve->vlan = vlan;
+ br_vlan_hold(vlan);
+ set_bit(p->port_no, vlan->port_bitmap);
+
+ list_add_tail_rcu(&pve->list, &p->vlan_list);
+ return 0;
+
+clean_up:
+ kfree(pve);
+ br_vlan_del(vlan, flags);
+ return err;
+}
+
+/* Must be protected by RTNL */
+int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
+{
+ struct net_device *dev = p->dev;
+ struct net_port_vlan *pve;
+ struct net_bridge_vlan *vlan;
+
+ ASSERT_RTNL();
+
+ pve = nbp_vlan_find(p, vid);
+ if (!pve)
+ return -ENOENT;
+
+ /* Remove VLAN from the device filter if it is supported. */
+ if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
+ dev->netdev_ops->ndo_vlan_rx_kill_vid) {
+ int err;
+
+ err = dev->netdev_ops->ndo_vlan_rx_kill_vid(dev, vid);
+ if (err)
+ pr_warn("failed to kill vid %d for device %s\n",
+ vid, dev->name);
+ }
+ pve->vid = BR_INVALID_VID;
+
+ vlan = pve->vlan;
+ pve->vlan = NULL;
+ clear_bit(p->port_no, vlan->port_bitmap);
+ br_vlan_put(vlan);
+
+ list_del_rcu(&pve->list);
+ kfree_rcu(pve, rcu);
+
+ br_vlan_del(vlan, flags);
+
+ return 0;
+}
+
+static void nbp_vlan_flush(struct net_bridge_port *p)
+{
+ struct net_port_vlan *pve;
+ struct net_port_vlan *tmp;
+
+ ASSERT_RTNL();
+
+ list_for_each_entry_safe(pve, tmp, &p->vlan_list, list)
+ nbp_vlan_delete(p, pve->vid, BRIDGE_FLAGS_SELF);
+}
+
static void release_nbp(struct kobject *kobj)
{
struct net_bridge_port *p
@@ -139,6 +387,7 @@ static void del_nbp(struct net_bridge_port *p)
br_ifinfo_notify(RTM_DELLINK, p);
+ nbp_vlan_flush(p);
br_fdb_delete_by_port(br, p, 1);
list_del_rcu(&p->list);
@@ -170,6 +419,7 @@ void br_dev_delete(struct net_device *dev, struct list_head *head)
del_nbp(p);
}
+ br_vlan_flush(br);
del_timer_sync(&br->gc_timer);
br_sysfs_delbr(br->dev);
@@ -222,6 +472,7 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
p->flags = 0;
br_init_port(p);
p->state = BR_STATE_DISABLED;
+ INIT_LIST_HEAD(&p->vlan_list);
br_stp_port_timer_init(p);
br_multicast_add_port(p);
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index ae0a6ec..76d9fbc 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -18,6 +18,7 @@
#include <linux/netpoll.h>
#include <linux/u64_stats_sync.h>
#include <net/route.h>
+#include <linux/if_vlan.h>
#define BR_HASH_BITS 8
#define BR_HASH_SIZE (1 << BR_HASH_BITS)
@@ -26,6 +27,7 @@
#define BR_PORT_BITS 10
#define BR_MAX_PORTS (1<<BR_PORT_BITS)
+#define PORT_BITMAP_LEN BITS_TO_LONGS(BR_MAX_PORTS)
#define BR_VERSION "2.3"
@@ -63,6 +65,27 @@ struct br_ip
__be16 proto;
};
+#define BR_INVALID_VID (1<<15)
+#define BR_UNTAGGED_VID (1<<14)
+
+#define BR_VID_HASH_SIZE (1<<6)
+#define br_vlan_hash(vid) ((vid) % (BR_VID_HASH_SIZE - 1))
+
+struct net_bridge_vlan {
+ struct hlist_node hlist;
+ atomic_t refcnt;
+ struct rcu_head rcu;
+ u16 vid;
+ unsigned long port_bitmap[PORT_BITMAP_LEN];
+};
+
+struct net_port_vlan {
+ struct list_head list;
+ struct net_bridge_vlan *vlan;
+ struct rcu_head rcu;
+ u16 vid;
+};
+
struct net_bridge_fdb_entry
{
struct hlist_node hlist;
@@ -155,6 +178,7 @@ struct net_bridge_port
#ifdef CONFIG_NET_POLL_CONTROLLER
struct netpoll *np;
#endif
+ struct list_head vlan_list;
};
#define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT)
@@ -259,6 +283,7 @@ struct net_bridge
struct timer_list topology_change_timer;
struct timer_list gc_timer;
struct kobject *ifobj;
+ struct hlist_head vlan_hlist[BR_VID_HASH_SIZE];
};
struct br_input_skb_cb {
@@ -400,6 +425,14 @@ 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 struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
+ u16 flags);
+extern int br_vlan_delete(struct net_bridge *br, u16 vid, u16 flags);
+extern struct net_bridge_vlan *br_vlan_find(struct net_bridge *br, u16 vid);
+extern int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags);
+extern int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags);
+extern struct net_port_vlan *nbp_vlan_find(const struct net_bridge_port *p,
+ u16 vid);
/* br_input.c */
extern int br_handle_frame_finish(struct sk_buff *skb);
--
1.7.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH V2 02/12] bridge: Validate that vlan is permitted on ingress
2012-12-18 19:00 [PATCH V2 00/12] Add basic VLAN support to bridges Vlad Yasevich
2012-12-18 19:00 ` [PATCH V2 01/12] bridge: Add vlan filtering infrastructure Vlad Yasevich
@ 2012-12-18 19:00 ` Vlad Yasevich
2012-12-18 19:00 ` [PATCH V2 03/12] bridge: Verify that a vlan is allowed to egress on give port Vlad Yasevich
` (11 subsequent siblings)
13 siblings, 0 replies; 35+ messages in thread
From: Vlad Yasevich @ 2012-12-18 19:00 UTC (permalink / raw)
To: netdev; +Cc: shemminger, davem, or.gerlitz, jhs, mst
When a frame arrives on a port, if we have VLANs configured,
validate that a given VLAN is allowed to ingress on a given
port.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
net/bridge/br_input.c | 23 +++++++++++++++++++++++
net/bridge/br_private.h | 15 +++++++++++++--
2 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 4b34207..54c0894 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -17,6 +17,7 @@
#include <linux/etherdevice.h>
#include <linux/netfilter_bridge.h>
#include <linux/export.h>
+#include <linux/rculist.h>
#include "br_private.h"
/* Hook for brouter */
@@ -41,6 +42,25 @@ static int br_pass_frame_up(struct sk_buff *skb)
netif_receive_skb);
}
+static bool br_allowed_ingress(struct net_bridge_port *p, struct sk_buff *skb)
+{
+ struct net_port_vlan *pve;
+ u16 vid;
+
+ /* If there are no vlan in the permitted list, all packets are
+ * permitted.
+ */
+ if (list_empty(&p->vlan_list))
+ return true;
+
+ vid = br_get_vlan(skb);
+ pve = nbp_vlan_find(p, vid);
+ if (pve)
+ return true;
+
+ return false;
+}
+
/* note: already called with rcu_read_lock */
int br_handle_frame_finish(struct sk_buff *skb)
{
@@ -54,6 +74,9 @@ int br_handle_frame_finish(struct sk_buff *skb)
if (!p || p->state == BR_STATE_DISABLED)
goto drop;
+ if (!br_allowed_ingress(p, skb))
+ goto drop;
+
/* insert into forwarding database after filtering to avoid spoofing */
br = p->br;
br_fdb_update(br, p, eth_hdr(skb)->h_source);
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 76d9fbc..1ba76b4 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -66,8 +66,6 @@ struct br_ip
};
#define BR_INVALID_VID (1<<15)
-#define BR_UNTAGGED_VID (1<<14)
-
#define BR_VID_HASH_SIZE (1<<6)
#define br_vlan_hash(vid) ((vid) % (BR_VID_HASH_SIZE - 1))
@@ -197,6 +195,19 @@ static inline struct net_bridge_port *br_port_get_rtnl(struct net_device *dev)
rtnl_dereference(dev->rx_handler_data) : NULL;
}
+static inline u16 br_get_vlan(const struct sk_buff *skb)
+{
+ u16 tag;
+
+ if (vlan_tx_tag_present(skb))
+ return vlan_tx_tag_get(skb) & VLAN_VID_MASK;
+
+ if (vlan_get_tag(skb, &tag))
+ return 0;
+
+ return tag & VLAN_VID_MASK;
+}
+
struct br_cpu_netstats {
u64 rx_packets;
u64 rx_bytes;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH V2 03/12] bridge: Verify that a vlan is allowed to egress on give port
2012-12-18 19:00 [PATCH V2 00/12] Add basic VLAN support to bridges Vlad Yasevich
2012-12-18 19:00 ` [PATCH V2 01/12] bridge: Add vlan filtering infrastructure Vlad Yasevich
2012-12-18 19:00 ` [PATCH V2 02/12] bridge: Validate that vlan is permitted on ingress Vlad Yasevich
@ 2012-12-18 19:00 ` Vlad Yasevich
2012-12-18 19:00 ` [PATCH V2 04/12] bridge: Cache vlan in the cb for faster egress lookup Vlad Yasevich
` (10 subsequent siblings)
13 siblings, 0 replies; 35+ messages in thread
From: Vlad Yasevich @ 2012-12-18 19:00 UTC (permalink / raw)
To: netdev; +Cc: shemminger, davem, or.gerlitz, jhs, mst
When bridge forwards a frame, make sure that a frame is allowed
to egress on that port.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
net/bridge/br_forward.c | 18 ++++++++++++++++++
net/bridge/br_private.h | 1 +
2 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 02015a5..0c7ffc2 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -26,11 +26,29 @@ static int deliver_clone(const struct net_bridge_port *prev,
void (*__packet_hook)(const struct net_bridge_port *p,
struct sk_buff *skb));
+static inline bool br_allowed_egress(const struct net_bridge_port *p,
+ const struct sk_buff *skb)
+{
+ struct net_port_vlan *pve;
+ u16 vid;
+
+ if (list_empty(&p->vlan_list))
+ return true;
+
+ vid = br_get_vlan(skb);
+ pve = nbp_vlan_find(p, vid);
+ if (pve)
+ return true;
+
+ return false;
+}
+
/* Don't forward packets to originating port or forwarding diasabled */
static inline int should_deliver(const struct net_bridge_port *p,
const struct sk_buff *skb)
{
return (((p->flags & BR_HAIRPIN_MODE) || skb->dev != p->dev) &&
+ br_allowed_egress(p, skb) &&
p->state == BR_STATE_FORWARDING);
}
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 1ba76b4..5090134 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -202,6 +202,7 @@ static inline u16 br_get_vlan(const struct sk_buff *skb)
if (vlan_tx_tag_present(skb))
return vlan_tx_tag_get(skb) & VLAN_VID_MASK;
+ /* Untagged and VLAN 0 traffic is handled the same way */
if (vlan_get_tag(skb, &tag))
return 0;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH V2 04/12] bridge: Cache vlan in the cb for faster egress lookup.
2012-12-18 19:00 [PATCH V2 00/12] Add basic VLAN support to bridges Vlad Yasevich
` (2 preceding siblings ...)
2012-12-18 19:00 ` [PATCH V2 03/12] bridge: Verify that a vlan is allowed to egress on give port Vlad Yasevich
@ 2012-12-18 19:00 ` Vlad Yasevich
2012-12-18 19:00 ` [PATCH V2 05/12] bridge: Add vlan to unicast fdb entries Vlad Yasevich
` (9 subsequent siblings)
13 siblings, 0 replies; 35+ messages in thread
From: Vlad Yasevich @ 2012-12-18 19:00 UTC (permalink / raw)
To: netdev; +Cc: shemminger, davem, or.gerlitz, jhs, mst
On input, cache the pointer to the bridge vlan info, so that
on egress, we have can simply look at the port bitmap instead
of traversing a vlan list.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
net/bridge/br_device.c | 8 ++++++++
net/bridge/br_forward.c | 14 ++++++++++++++
net/bridge/br_input.c | 6 +++++-
net/bridge/br_private.h | 1 +
4 files changed, 28 insertions(+), 1 deletions(-)
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 9546742..57c5bac 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -30,6 +30,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
struct net_bridge_fdb_entry *dst;
struct net_bridge_mdb_entry *mdst;
struct br_cpu_netstats *brstats = this_cpu_ptr(br->stats);
+ struct net_bridge_vlan *vlan;
rcu_read_lock();
#ifdef CONFIG_BRIDGE_NETFILTER
@@ -47,6 +48,13 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
BR_INPUT_SKB_CB(skb)->brdev = dev;
+ /* Any vlan transmitted by the bridge itself is permitted.
+ * Try to cache the vlan in the CB to speed up forwarding.
+ */
+ vlan = br_vlan_find(br, br_get_vlan(skb));
+ if (vlan)
+ BR_INPUT_SKB_CB(skb)->vlan = vlan;
+
skb_reset_mac_header(skb);
skb_pull(skb, ETH_HLEN);
diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 0c7ffc2..4ae5f55 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -30,11 +30,25 @@ static inline bool br_allowed_egress(const struct net_bridge_port *p,
const struct sk_buff *skb)
{
struct net_port_vlan *pve;
+ struct net_bridge_vlan *vlan = NULL;
u16 vid;
if (list_empty(&p->vlan_list))
return true;
+ vlan = BR_INPUT_SKB_CB(skb)->vlan;
+ if (vlan) {
+ /* If we have cached VLAN information, use port_bitmap
+ * of the vlan to make the decision
+ */
+ if (test_bit(p->port_no, vlan->port_bitmap))
+ return true;
+ return false;
+ }
+
+ /* We don't have cached vlan information, so we need to do
+ * it the hard way.
+ */
vid = br_get_vlan(skb);
pve = nbp_vlan_find(p, vid);
if (pve)
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 54c0894..e475f49 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -47,6 +47,8 @@ static bool br_allowed_ingress(struct net_bridge_port *p, struct sk_buff *skb)
struct net_port_vlan *pve;
u16 vid;
+ BR_INPUT_SKB_CB(skb)->vlan = NULL;
+
/* If there are no vlan in the permitted list, all packets are
* permitted.
*/
@@ -55,8 +57,10 @@ static bool br_allowed_ingress(struct net_bridge_port *p, struct sk_buff *skb)
vid = br_get_vlan(skb);
pve = nbp_vlan_find(p, vid);
- if (pve)
+ if (pve) {
+ BR_INPUT_SKB_CB(skb)->vlan = pve->vlan;
return true;
+ }
return false;
}
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 5090134..6793088 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -300,6 +300,7 @@ struct net_bridge
struct br_input_skb_cb {
struct net_device *brdev;
+ struct net_bridge_vlan *vlan;
#ifdef CONFIG_BRIDGE_IGMP_SNOOPING
int igmp;
int mrouters_only;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH V2 05/12] bridge: Add vlan to unicast fdb entries
2012-12-18 19:00 [PATCH V2 00/12] Add basic VLAN support to bridges Vlad Yasevich
` (3 preceding siblings ...)
2012-12-18 19:00 ` [PATCH V2 04/12] bridge: Cache vlan in the cb for faster egress lookup Vlad Yasevich
@ 2012-12-18 19:00 ` Vlad Yasevich
2012-12-18 19:00 ` [PATCH V2 06/12] bridge: Add vlan id to multicast groups Vlad Yasevich
` (8 subsequent siblings)
13 siblings, 0 replies; 35+ messages in thread
From: Vlad Yasevich @ 2012-12-18 19:00 UTC (permalink / raw)
To: netdev; +Cc: shemminger, davem, or.gerlitz, jhs, mst
This patch adds vlan to unicast fdb entries that are created for
learned addresses (not the manually configured ones). It adds
vlan id into the hash mix and uses vlan as an addditional parameter
for an entry match.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
include/uapi/linux/if_bridge.h | 2 +-
net/bridge/br_device.c | 6 ++-
net/bridge/br_fdb.c | 70 +++++++++++++++++++++++----------------
net/bridge/br_input.c | 16 +++++----
net/bridge/br_private.h | 7 +++-
5 files changed, 60 insertions(+), 41 deletions(-)
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 9a0f6ff..52aa738 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -94,7 +94,7 @@ struct __fdb_entry {
__u32 ageing_timer_value;
__u8 port_hi;
__u8 pad0;
- __u16 unused;
+ __u16 fdb_vid;
};
/* Bridge Flags */
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 57c5bac..1f9d0f9 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -31,6 +31,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
struct net_bridge_mdb_entry *mdst;
struct br_cpu_netstats *brstats = this_cpu_ptr(br->stats);
struct net_bridge_vlan *vlan;
+ u16 vid;
rcu_read_lock();
#ifdef CONFIG_BRIDGE_NETFILTER
@@ -51,7 +52,8 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
/* Any vlan transmitted by the bridge itself is permitted.
* Try to cache the vlan in the CB to speed up forwarding.
*/
- vlan = br_vlan_find(br, br_get_vlan(skb));
+ vid = br_get_vlan(skb);
+ vlan = br_vlan_find(br, vid);
if (vlan)
BR_INPUT_SKB_CB(skb)->vlan = vlan;
@@ -75,7 +77,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
br_multicast_deliver(mdst, skb);
else
br_flood_deliver(br, skb);
- } else if ((dst = __br_fdb_get(br, dest)) != NULL)
+ } else if ((dst = __br_fdb_get(br, dest, vid)) != NULL)
br_deliver(dst->dst, skb);
else
br_flood_deliver(br, skb);
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index d9576e6..a244efc 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -23,6 +23,7 @@
#include <linux/slab.h>
#include <linux/atomic.h>
#include <asm/unaligned.h>
+#include <linux/if_vlan.h>
#include "br_private.h"
static struct kmem_cache *br_fdb_cache __read_mostly;
@@ -67,11 +68,11 @@ static inline int has_expired(const struct net_bridge *br,
time_before_eq(fdb->updated + hold_time(br), jiffies);
}
-static inline int br_mac_hash(const unsigned char *mac)
+static inline int br_mac_hash(const unsigned char *mac, __u16 vid)
{
- /* use 1 byte of OUI cnd 3 bytes of NIC */
+ /* use 1 byte of OUI and 3 bytes of NIC */
u32 key = get_unaligned((u32 *)(mac + 2));
- return jhash_1word(key, fdb_salt) & (BR_HASH_SIZE - 1);
+ return jhash_2words(key, vid, fdb_salt) & (BR_HASH_SIZE - 1);
}
static void fdb_rcu_free(struct rcu_head *head)
@@ -132,7 +133,7 @@ void br_fdb_change_mac_address(struct net_bridge *br, const u8 *newaddr)
struct net_bridge_fdb_entry *f;
/* If old entry was unassociated with any port, then delete it. */
- f = __br_fdb_get(br, br->dev->dev_addr);
+ f = __br_fdb_get(br, br->dev->dev_addr, 0);
if (f && f->is_local && !f->dst)
fdb_delete(br, f);
@@ -231,13 +232,16 @@ void br_fdb_delete_by_port(struct net_bridge *br,
/* No locking or refcounting, assumes caller has rcu_read_lock */
struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
- const unsigned char *addr)
+ const unsigned char *addr,
+ __u16 vid)
{
struct hlist_node *h;
struct net_bridge_fdb_entry *fdb;
- hlist_for_each_entry_rcu(fdb, h, &br->hash[br_mac_hash(addr)], hlist) {
- if (ether_addr_equal(fdb->addr.addr, addr)) {
+ hlist_for_each_entry_rcu(fdb, h,
+ &br->hash[br_mac_hash(addr, vid)], hlist) {
+ if (ether_addr_equal(fdb->addr.addr, addr) &&
+ fdb->vlan_id == vid) {
if (unlikely(has_expired(br, fdb)))
break;
return fdb;
@@ -261,7 +265,7 @@ int br_fdb_test_addr(struct net_device *dev, unsigned char *addr)
if (!port)
ret = 0;
else {
- fdb = __br_fdb_get(port->br, addr);
+ fdb = __br_fdb_get(port->br, addr, 0);
ret = fdb && fdb->dst && fdb->dst->dev != dev &&
fdb->dst->state == BR_STATE_FORWARDING;
}
@@ -313,6 +317,7 @@ int br_fdb_fillbuf(struct net_bridge *br, void *buf,
fe->is_local = f->is_local;
if (!f->is_static)
fe->ageing_timer_value = jiffies_delta_to_clock_t(jiffies - f->updated);
+ fe->fdb_vid = f->vlan_id;
++fe;
++num;
}
@@ -325,26 +330,30 @@ int br_fdb_fillbuf(struct net_bridge *br, void *buf,
}
static struct net_bridge_fdb_entry *fdb_find(struct hlist_head *head,
- const unsigned char *addr)
+ const unsigned char *addr,
+ __u16 vid)
{
struct hlist_node *h;
struct net_bridge_fdb_entry *fdb;
hlist_for_each_entry(fdb, h, head, hlist) {
- if (ether_addr_equal(fdb->addr.addr, addr))
+ if (ether_addr_equal(fdb->addr.addr, addr) &&
+ fdb->vlan_id == vid)
return fdb;
}
return NULL;
}
static struct net_bridge_fdb_entry *fdb_find_rcu(struct hlist_head *head,
- const unsigned char *addr)
+ const unsigned char *addr,
+ __u16 vid)
{
struct hlist_node *h;
struct net_bridge_fdb_entry *fdb;
hlist_for_each_entry_rcu(fdb, h, head, hlist) {
- if (ether_addr_equal(fdb->addr.addr, addr))
+ if (ether_addr_equal(fdb->addr.addr, addr) &&
+ fdb->vlan_id == vid)
return fdb;
}
return NULL;
@@ -352,7 +361,8 @@ static struct net_bridge_fdb_entry *fdb_find_rcu(struct hlist_head *head,
static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
struct net_bridge_port *source,
- const unsigned char *addr)
+ const unsigned char *addr,
+ __u16 vid)
{
struct net_bridge_fdb_entry *fdb;
@@ -360,6 +370,7 @@ static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
if (fdb) {
memcpy(fdb->addr.addr, addr, ETH_ALEN);
fdb->dst = source;
+ fdb->vlan_id = vid;
fdb->is_local = 0;
fdb->is_static = 0;
fdb->updated = fdb->used = jiffies;
@@ -371,13 +382,13 @@ static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
const unsigned char *addr)
{
- struct hlist_head *head = &br->hash[br_mac_hash(addr)];
+ struct hlist_head *head = &br->hash[br_mac_hash(addr, 0)];
struct net_bridge_fdb_entry *fdb;
if (!is_valid_ether_addr(addr))
return -EINVAL;
- fdb = fdb_find(head, addr);
+ fdb = fdb_find(head, addr, 0);
if (fdb) {
/* it is okay to have multiple ports with same
* address, just use the first one.
@@ -390,7 +401,7 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
fdb_delete(br, fdb);
}
- fdb = fdb_create(head, source, addr);
+ fdb = fdb_create(head, source, addr, 0);
if (!fdb)
return -ENOMEM;
@@ -412,9 +423,9 @@ int br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
}
void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
- const unsigned char *addr)
+ const unsigned char *addr, u16 vid)
{
- struct hlist_head *head = &br->hash[br_mac_hash(addr)];
+ struct hlist_head *head = &br->hash[br_mac_hash(addr, vid)];
struct net_bridge_fdb_entry *fdb;
/* some users want to always flood. */
@@ -426,7 +437,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
source->state == BR_STATE_FORWARDING))
return;
- fdb = fdb_find_rcu(head, addr);
+ fdb = fdb_find_rcu(head, addr, vid);
if (likely(fdb)) {
/* attempt to update an entry for a local interface */
if (unlikely(fdb->is_local)) {
@@ -441,8 +452,8 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
}
} else {
spin_lock(&br->hash_lock);
- if (likely(!fdb_find(head, addr))) {
- fdb = fdb_create(head, source, addr);
+ if (likely(!fdb_find(head, addr, vid))) {
+ fdb = fdb_create(head, source, addr, vid);
if (fdb)
fdb_notify(br, fdb, RTM_NEWNEIGH);
}
@@ -571,18 +582,18 @@ out:
/* Update (create or replace) forwarding database entry */
static int fdb_add_entry(struct net_bridge_port *source, const __u8 *addr,
- __u16 state, __u16 flags)
+ __u16 state, __u16 flags, __u16 vid)
{
struct net_bridge *br = source->br;
- struct hlist_head *head = &br->hash[br_mac_hash(addr)];
+ struct hlist_head *head = &br->hash[br_mac_hash(addr, vid)];
struct net_bridge_fdb_entry *fdb;
- fdb = fdb_find(head, addr);
+ fdb = fdb_find(head, addr, vid);
if (fdb == NULL) {
if (!(flags & NLM_F_CREATE))
return -ENOENT;
- fdb = fdb_create(head, source, addr);
+ fdb = fdb_create(head, source, addr, vid);
if (!fdb)
return -ENOMEM;
fdb_notify(br, fdb, RTM_NEWNEIGH);
@@ -629,11 +640,12 @@ int br_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
if (ndm->ndm_flags & NTF_USE) {
rcu_read_lock();
- br_fdb_update(p->br, p, addr);
+ br_fdb_update(p->br, p, addr, 0);
rcu_read_unlock();
} else {
spin_lock_bh(&p->br->hash_lock);
- err = fdb_add_entry(p, addr, ndm->ndm_state, nlh_flags);
+ err = fdb_add_entry(p, addr, ndm->ndm_state, nlh_flags,
+ 0);
spin_unlock_bh(&p->br->hash_lock);
}
@@ -643,10 +655,10 @@ int br_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
static int fdb_delete_by_addr(struct net_bridge_port *p, const u8 *addr)
{
struct net_bridge *br = p->br;
- struct hlist_head *head = &br->hash[br_mac_hash(addr)];
+ struct hlist_head *head = &br->hash[br_mac_hash(addr, 0)];
struct net_bridge_fdb_entry *fdb;
- fdb = fdb_find(head, addr);
+ fdb = fdb_find(head, addr, 0);
if (!fdb)
return -ENOENT;
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index e475f49..e51eb24 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -42,10 +42,10 @@ static int br_pass_frame_up(struct sk_buff *skb)
netif_receive_skb);
}
-static bool br_allowed_ingress(struct net_bridge_port *p, struct sk_buff *skb)
+static bool br_allowed_ingress(struct net_bridge_port *p, struct sk_buff *skb,
+ u16 vid)
{
struct net_port_vlan *pve;
- u16 vid;
BR_INPUT_SKB_CB(skb)->vlan = NULL;
@@ -55,7 +55,6 @@ static bool br_allowed_ingress(struct net_bridge_port *p, struct sk_buff *skb)
if (list_empty(&p->vlan_list))
return true;
- vid = br_get_vlan(skb);
pve = nbp_vlan_find(p, vid);
if (pve) {
BR_INPUT_SKB_CB(skb)->vlan = pve->vlan;
@@ -74,16 +73,18 @@ int br_handle_frame_finish(struct sk_buff *skb)
struct net_bridge_fdb_entry *dst;
struct net_bridge_mdb_entry *mdst;
struct sk_buff *skb2;
+ u16 vid;
if (!p || p->state == BR_STATE_DISABLED)
goto drop;
- if (!br_allowed_ingress(p, skb))
+ vid = br_get_vlan(skb);
+ if (!br_allowed_ingress(p, skb, vid))
goto drop;
/* insert into forwarding database after filtering to avoid spoofing */
br = p->br;
- br_fdb_update(br, p, eth_hdr(skb)->h_source);
+ br_fdb_update(br, p, eth_hdr(skb)->h_source, vid);
if (!is_broadcast_ether_addr(dest) && is_multicast_ether_addr(dest) &&
br_multicast_rcv(br, p, skb))
@@ -118,7 +119,8 @@ int br_handle_frame_finish(struct sk_buff *skb)
skb2 = skb;
br->dev->stats.multicast++;
- } else if ((dst = __br_fdb_get(br, dest)) && dst->is_local) {
+ } else if ((dst = __br_fdb_get(br, dest, vid)) &&
+ dst->is_local) {
skb2 = skb;
/* Do not forward the packet since it's local. */
skb = NULL;
@@ -147,7 +149,7 @@ static int br_handle_local_finish(struct sk_buff *skb)
{
struct net_bridge_port *p = br_port_get_rcu(skb->dev);
- br_fdb_update(p->br, p, eth_hdr(skb)->h_source);
+ br_fdb_update(p->br, p, eth_hdr(skb)->h_source, br_get_vlan(skb));
return 0; /* process further */
}
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 6793088..7a28900 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -95,6 +95,7 @@ struct net_bridge_fdb_entry
mac_addr addr;
unsigned char is_local;
unsigned char is_static;
+ __u16 vlan_id;
};
struct net_bridge_port_group {
@@ -392,7 +393,8 @@ extern void br_fdb_cleanup(unsigned long arg);
extern void br_fdb_delete_by_port(struct net_bridge *br,
const struct net_bridge_port *p, int do_all);
extern struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
- const unsigned char *addr);
+ const unsigned char *addr,
+ __u16 vid);
extern int br_fdb_test_addr(struct net_device *dev, unsigned char *addr);
extern int br_fdb_fillbuf(struct net_bridge *br, void *buf,
unsigned long count, unsigned long off);
@@ -401,7 +403,8 @@ extern int br_fdb_insert(struct net_bridge *br,
const unsigned char *addr);
extern void br_fdb_update(struct net_bridge *br,
struct net_bridge_port *source,
- const unsigned char *addr);
+ const unsigned char *addr,
+ u16 vid);
extern int br_fdb_delete(struct ndmsg *ndm,
struct net_device *dev,
--
1.7.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH V2 06/12] bridge: Add vlan id to multicast groups
2012-12-18 19:00 [PATCH V2 00/12] Add basic VLAN support to bridges Vlad Yasevich
` (4 preceding siblings ...)
2012-12-18 19:00 ` [PATCH V2 05/12] bridge: Add vlan to unicast fdb entries Vlad Yasevich
@ 2012-12-18 19:00 ` Vlad Yasevich
2012-12-18 19:00 ` [PATCH V2 07/12] bridge: Add netlink interface to configure vlans on bridge ports Vlad Yasevich
` (7 subsequent siblings)
13 siblings, 0 replies; 35+ messages in thread
From: Vlad Yasevich @ 2012-12-18 19:00 UTC (permalink / raw)
To: netdev; +Cc: shemminger, davem, or.gerlitz, jhs, mst
Add vlan_id to multicasts groups so that we know which vlan
each group belongs to and can correctly forward to appropriate vlan.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
net/bridge/br_multicast.c | 64 +++++++++++++++++++++++++++++++--------------
net/bridge/br_private.h | 1 +
2 files changed, 45 insertions(+), 20 deletions(-)
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 68e375a..072aa2d 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -51,6 +51,8 @@ static inline int br_ip_equal(const struct br_ip *a, const struct br_ip *b)
{
if (a->proto != b->proto)
return 0;
+ if (a->vid != b->vid)
+ return 0;
switch (a->proto) {
case htons(ETH_P_IP):
return a->u.ip4 == b->u.ip4;
@@ -62,16 +64,19 @@ static inline int br_ip_equal(const struct br_ip *a, const struct br_ip *b)
return 0;
}
-static inline int __br_ip4_hash(struct net_bridge_mdb_htable *mdb, __be32 ip)
+static inline int __br_ip4_hash(struct net_bridge_mdb_htable *mdb, __be32 ip,
+ __u16 vid)
{
- return jhash_1word(mdb->secret, (__force u32)ip) & (mdb->max - 1);
+ return jhash_2words((__force u32)ip, vid, mdb->secret) & (mdb->max - 1);
}
#if IS_ENABLED(CONFIG_IPV6)
static inline int __br_ip6_hash(struct net_bridge_mdb_htable *mdb,
- const struct in6_addr *ip)
+ const struct in6_addr *ip,
+ __u16 vid)
{
- return jhash2((__force u32 *)ip->s6_addr32, 4, mdb->secret) & (mdb->max - 1);
+ return jhash_2words(ipv6_addr_hash(ip), vid,
+ mdb->secret) & (mdb->max - 1);
}
#endif
@@ -80,10 +85,10 @@ static inline int br_ip_hash(struct net_bridge_mdb_htable *mdb,
{
switch (ip->proto) {
case htons(ETH_P_IP):
- return __br_ip4_hash(mdb, ip->u.ip4);
+ return __br_ip4_hash(mdb, ip->u.ip4, ip->vid);
#if IS_ENABLED(CONFIG_IPV6)
case htons(ETH_P_IPV6):
- return __br_ip6_hash(mdb, &ip->u.ip6);
+ return __br_ip6_hash(mdb, &ip->u.ip6, ip->vid);
#endif
}
return 0;
@@ -113,24 +118,27 @@ static struct net_bridge_mdb_entry *br_mdb_ip_get(
}
static struct net_bridge_mdb_entry *br_mdb_ip4_get(
- struct net_bridge_mdb_htable *mdb, __be32 dst)
+ struct net_bridge_mdb_htable *mdb, __be32 dst, __u16 vid)
{
struct br_ip br_dst;
br_dst.u.ip4 = dst;
br_dst.proto = htons(ETH_P_IP);
+ br_dst.vid = vid;
return br_mdb_ip_get(mdb, &br_dst);
}
#if IS_ENABLED(CONFIG_IPV6)
static struct net_bridge_mdb_entry *br_mdb_ip6_get(
- struct net_bridge_mdb_htable *mdb, const struct in6_addr *dst)
+ struct net_bridge_mdb_htable *mdb, const struct in6_addr *dst,
+ __u16 vid)
{
struct br_ip br_dst;
br_dst.u.ip6 = *dst;
br_dst.proto = htons(ETH_P_IPV6);
+ br_dst.vid = vid;
return br_mdb_ip_get(mdb, &br_dst);
}
@@ -692,7 +700,8 @@ err:
static int br_ip4_multicast_add_group(struct net_bridge *br,
struct net_bridge_port *port,
- __be32 group)
+ __be32 group,
+ __u16 vid)
{
struct br_ip br_group;
@@ -701,6 +710,7 @@ static int br_ip4_multicast_add_group(struct net_bridge *br,
br_group.u.ip4 = group;
br_group.proto = htons(ETH_P_IP);
+ br_group.vid = vid;
return br_multicast_add_group(br, port, &br_group);
}
@@ -708,7 +718,8 @@ static int br_ip4_multicast_add_group(struct net_bridge *br,
#if IS_ENABLED(CONFIG_IPV6)
static int br_ip6_multicast_add_group(struct net_bridge *br,
struct net_bridge_port *port,
- const struct in6_addr *group)
+ const struct in6_addr *group,
+ __u16 vid)
{
struct br_ip br_group;
@@ -717,6 +728,7 @@ static int br_ip6_multicast_add_group(struct net_bridge *br,
br_group.u.ip6 = *group;
br_group.proto = htons(ETH_P_IPV6);
+ br_group.vid = vid;
return br_multicast_add_group(br, port, &br_group);
}
@@ -928,7 +940,8 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br,
continue;
}
- err = br_ip4_multicast_add_group(br, port, group);
+ err = br_ip4_multicast_add_group(br, port, group,
+ br_get_vlan(skb));
if (err)
break;
}
@@ -988,7 +1001,8 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
continue;
}
- err = br_ip6_multicast_add_group(br, port, &grec->grec_mca);
+ err = br_ip6_multicast_add_group(br, port, &grec->grec_mca,
+ br_get_vlan(skb));
if (!err)
break;
}
@@ -1106,7 +1120,8 @@ static int br_ip4_multicast_query(struct net_bridge *br,
if (!group)
goto out;
- mp = br_mdb_ip4_get(mlock_dereference(br->mdb, br), group);
+ mp = br_mdb_ip4_get(mlock_dereference(br->mdb, br), group,
+ br_get_vlan(skb));
if (!mp)
goto out;
@@ -1178,7 +1193,8 @@ static int br_ip6_multicast_query(struct net_bridge *br,
if (!group)
goto out;
- mp = br_mdb_ip6_get(mlock_dereference(br->mdb, br), group);
+ mp = br_mdb_ip6_get(mlock_dereference(br->mdb, br), group,
+ br_get_vlan(skb));
if (!mp)
goto out;
@@ -1283,7 +1299,8 @@ out:
static void br_ip4_multicast_leave_group(struct net_bridge *br,
struct net_bridge_port *port,
- __be32 group)
+ __be32 group,
+ __u16 vid)
{
struct br_ip br_group;
@@ -1292,6 +1309,7 @@ static void br_ip4_multicast_leave_group(struct net_bridge *br,
br_group.u.ip4 = group;
br_group.proto = htons(ETH_P_IP);
+ br_group.vid = vid;
br_multicast_leave_group(br, port, &br_group);
}
@@ -1299,7 +1317,8 @@ static void br_ip4_multicast_leave_group(struct net_bridge *br,
#if IS_ENABLED(CONFIG_IPV6)
static void br_ip6_multicast_leave_group(struct net_bridge *br,
struct net_bridge_port *port,
- const struct in6_addr *group)
+ const struct in6_addr *group,
+ __u16 vid)
{
struct br_ip br_group;
@@ -1308,6 +1327,7 @@ static void br_ip6_multicast_leave_group(struct net_bridge *br,
br_group.u.ip6 = *group;
br_group.proto = htons(ETH_P_IPV6);
+ br_group.vid = vid;
br_multicast_leave_group(br, port, &br_group);
}
@@ -1390,7 +1410,8 @@ static int br_multicast_ipv4_rcv(struct net_bridge *br,
case IGMP_HOST_MEMBERSHIP_REPORT:
case IGMPV2_HOST_MEMBERSHIP_REPORT:
BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
- err = br_ip4_multicast_add_group(br, port, ih->group);
+ err = br_ip4_multicast_add_group(br, port, ih->group,
+ br_get_vlan(skb2));
break;
case IGMPV3_HOST_MEMBERSHIP_REPORT:
err = br_ip4_multicast_igmp3_report(br, port, skb2);
@@ -1399,7 +1420,8 @@ static int br_multicast_ipv4_rcv(struct net_bridge *br,
err = br_ip4_multicast_query(br, port, skb2);
break;
case IGMP_HOST_LEAVE_MESSAGE:
- br_ip4_multicast_leave_group(br, port, ih->group);
+ br_ip4_multicast_leave_group(br, port, ih->group,
+ br_get_vlan(skb2));
break;
}
@@ -1519,7 +1541,8 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
}
mld = (struct mld_msg *)skb_transport_header(skb2);
BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
- err = br_ip6_multicast_add_group(br, port, &mld->mld_mca);
+ err = br_ip6_multicast_add_group(br, port, &mld->mld_mca,
+ br_get_vlan(skb2));
break;
}
case ICMPV6_MLD2_REPORT:
@@ -1536,7 +1559,8 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
goto out;
}
mld = (struct mld_msg *)skb_transport_header(skb2);
- br_ip6_multicast_leave_group(br, port, &mld->mld_mca);
+ br_ip6_multicast_leave_group(br, port, &mld->mld_mca,
+ br_get_vlan(skb2));
}
}
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 7a28900..2569afb 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -63,6 +63,7 @@ struct br_ip
#endif
} u;
__be16 proto;
+ __u16 vid;
};
#define BR_INVALID_VID (1<<15)
--
1.7.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH V2 07/12] bridge: Add netlink interface to configure vlans on bridge ports
2012-12-18 19:00 [PATCH V2 00/12] Add basic VLAN support to bridges Vlad Yasevich
` (5 preceding siblings ...)
2012-12-18 19:00 ` [PATCH V2 06/12] bridge: Add vlan id to multicast groups Vlad Yasevich
@ 2012-12-18 19:00 ` Vlad Yasevich
2012-12-18 19:00 ` [PATCH V2 08/12] bridge: Add vlan support to static neighbors Vlad Yasevich
` (6 subsequent siblings)
13 siblings, 0 replies; 35+ messages in thread
From: Vlad Yasevich @ 2012-12-18 19:00 UTC (permalink / raw)
To: netdev; +Cc: shemminger, davem, or.gerlitz, jhs, mst
Add a netlink interface to add and remove vlan configuration on bridge port.
The interface uses the RTM_SETLINK message and encodes the vlan
configuration inside the IFLA_AF_SPEC. It is possble to include multiple
vlans to either add or remove in a single message.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
include/uapi/linux/if_bridge.h | 17 ++++++
net/bridge/br_if.c | 1 +
net/bridge/br_netlink.c | 107 +++++++++++++++++++++++++++++++++------
3 files changed, 108 insertions(+), 17 deletions(-)
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 52aa738..d0b4f5c 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -108,15 +108,32 @@ struct __fdb_entry {
* [IFLA_AF_SPEC] = {
* [IFLA_BRIDGE_FLAGS]
* [IFLA_BRIDGE_MODE]
+ * [IFLA_BRIDGE_VLAN_INFO]
* }
*/
enum {
IFLA_BRIDGE_FLAGS,
IFLA_BRIDGE_MODE,
+ IFLA_BRIDGE_VLAN_INFO,
__IFLA_BRIDGE_MAX,
};
#define IFLA_BRIDGE_MAX (__IFLA_BRIDGE_MAX - 1)
+/* Bridge VLAN info
+ * [IFLA_BRIDGE_VLAN_INFO]
+ */
+enum {
+ BR_VLAN_ADD,
+ BR_VLAN_DEL,
+};
+
+struct bridge_vlan_info {
+ u16 op_code;
+ u16 flags;
+ u16 vid;
+ u16 unused;
+};
+
/* Bridge multicast database attributes
* [MDBA_MDB] = {
* [MDBA_MDB_ENTRY] = {
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 14c7c6a..57bbb35 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -23,6 +23,7 @@
#include <linux/if_ether.h>
#include <linux/slab.h>
#include <net/sock.h>
+#include <linux/if_vlan.h>
#include "br_private.h"
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index dead9df..9cf2879 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -16,6 +16,7 @@
#include <net/rtnetlink.h>
#include <net/net_namespace.h>
#include <net/sock.h>
+#include <uapi/linux/if_bridge.h>
#include "br_private.h"
#include "br_private_stp.h"
@@ -123,6 +124,9 @@ void br_ifinfo_notify(int event, struct net_bridge_port *port)
struct sk_buff *skb;
int err = -ENOBUFS;
+ if (!port)
+ return;
+
br_debug(port->br, "port %u(%s) event %d\n",
(unsigned int)port->port_no, port->dev->name, event);
@@ -162,6 +166,60 @@ out:
return err;
}
+const struct nla_policy ifla_br_policy[IFLA_MAX+1] = {
+ [IFLA_BRIDGE_FLAGS] = { .type = NLA_U16 },
+ [IFLA_BRIDGE_MODE] = { .type = NLA_U16 },
+ [IFLA_BRIDGE_VLAN_INFO] = { .type = NLA_BINARY,
+ .len = sizeof(struct bridge_vlan_info), },
+};
+
+static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
+ struct nlattr *af_spec)
+{
+ struct nlattr *tb[IFLA_BRIDGE_MAX+1];
+ int err = 0;
+
+ if (nla_type(af_spec) != AF_BRIDGE)
+ return -EINVAL;
+
+ err = nla_parse_nested(tb, IFLA_BRIDGE_MAX, af_spec, ifla_br_policy);
+ if (err)
+ return err;
+
+ if (tb[IFLA_BRIDGE_VLAN_INFO]) {
+ struct bridge_vlan_info *vinfo;
+
+ vinfo = nla_data(tb[IFLA_BRIDGE_VLAN_INFO]);
+
+ if (vinfo->vid > VLAN_N_VID)
+ return -EINVAL;
+
+ switch (vinfo->op_code) {
+ case BR_VLAN_ADD:
+ if (p)
+ err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
+ else {
+ u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
+ if (!br_vlan_add(br, vinfo->vid, flags))
+ err = -ENOMEM;
+ }
+ break;
+
+ case BR_VLAN_DEL:
+ if (p)
+ err = nbp_vlan_delete(p, vinfo->vid,
+ vinfo->flags);
+ else {
+ u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
+ err = br_vlan_delete(br, vinfo->vid, flags);
+ }
+ break;
+ }
+ }
+
+ return err;
+}
+
static const struct nla_policy ifla_brport_policy[IFLA_BRPORT_MAX + 1] = {
[IFLA_BRPORT_STATE] = { .type = NLA_U8 },
[IFLA_BRPORT_COST] = { .type = NLA_U32 },
@@ -238,6 +296,7 @@ int br_setlink(struct net_device *dev, struct nlmsghdr *nlh)
{
struct ifinfomsg *ifm;
struct nlattr *protinfo;
+ struct nlattr *afspec;
struct net_bridge_port *p;
struct nlattr *tb[IFLA_BRPORT_MAX + 1];
int err;
@@ -245,35 +304,49 @@ int br_setlink(struct net_device *dev, struct nlmsghdr *nlh)
ifm = nlmsg_data(nlh);
protinfo = nlmsg_find_attr(nlh, sizeof(*ifm), IFLA_PROTINFO);
- if (!protinfo)
+ afspec = nlmsg_find_attr(nlh, sizeof(*ifm), IFLA_AF_SPEC);
+ if (!protinfo && !afspec)
return 0;
p = br_port_get_rtnl(dev);
- if (!p)
+ /* We want to accept dev as bridge itself if the AF_SPEC
+ * is set to see if someone is setting vlan info on the brigde.
+ */
+ if (!p && ((dev->priv_flags & IFF_EBRIDGE) && !afspec))
return -EINVAL;
- if (protinfo->nla_type & NLA_F_NESTED) {
- err = nla_parse_nested(tb, IFLA_BRPORT_MAX,
- protinfo, ifla_brport_policy);
+ if (p && protinfo) {
+ if (protinfo->nla_type & NLA_F_NESTED) {
+ err = nla_parse_nested(tb, IFLA_BRPORT_MAX,
+ protinfo, ifla_brport_policy);
+ if (err)
+ return err;
+
+ spin_lock_bh(&p->br->lock);
+ err = br_setport(p, tb);
+ spin_unlock_bh(&p->br->lock);
+ } else {
+ /* Binary compatability with old RSTP */
+ if (nla_len(protinfo) < sizeof(u8))
+ return -EINVAL;
+
+ spin_lock_bh(&p->br->lock);
+ err = br_set_port_state(p, nla_get_u8(protinfo));
+ spin_unlock_bh(&p->br->lock);
+ }
if (err)
- return err;
-
- spin_lock_bh(&p->br->lock);
- err = br_setport(p, tb);
- spin_unlock_bh(&p->br->lock);
- } else {
- /* Binary compatability with old RSTP */
- if (nla_len(protinfo) < sizeof(u8))
- return -EINVAL;
+ goto out;
+ }
- spin_lock_bh(&p->br->lock);
- err = br_set_port_state(p, nla_get_u8(protinfo));
- spin_unlock_bh(&p->br->lock);
+ if (afspec) {
+ err = br_afspec((struct net_bridge *)netdev_priv(dev), p,
+ afspec);
}
if (err == 0)
br_ifinfo_notify(RTM_NEWLINK, p);
+out:
return err;
}
--
1.7.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH V2 08/12] bridge: Add vlan support to static neighbors
2012-12-18 19:00 [PATCH V2 00/12] Add basic VLAN support to bridges Vlad Yasevich
` (6 preceding siblings ...)
2012-12-18 19:00 ` [PATCH V2 07/12] bridge: Add netlink interface to configure vlans on bridge ports Vlad Yasevich
@ 2012-12-18 19:00 ` Vlad Yasevich
2012-12-18 19:01 ` [PATCH V2 09/12] bridge: Add the ability to configure untagged vlans Vlad Yasevich
` (5 subsequent siblings)
13 siblings, 0 replies; 35+ messages in thread
From: Vlad Yasevich @ 2012-12-18 19:00 UTC (permalink / raw)
To: netdev; +Cc: shemminger, davem, or.gerlitz, jhs, mst
When a user adds bridge neighbors, allow him to specify VLAN id.
If the VLAN id is not specified, the neighbor will be added
for VLANs currently in the ports filter list. If the list is
empty, we use vlan 0 and only add 1 entry.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +-
drivers/net/macvlan.c | 2 +-
drivers/net/vxlan.c | 3 +-
include/linux/netdevice.h | 1 +
include/uapi/linux/neighbour.h | 1 +
net/bridge/br_fdb.c | 139 ++++++++++++++++++++++---
net/bridge/br_private.h | 2 +-
net/core/rtnetlink.c | 24 +++--
8 files changed, 146 insertions(+), 28 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 20a5af6..ec97efe 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6985,7 +6985,7 @@ static int ixgbe_ndo_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
return err;
}
-static int ixgbe_ndo_fdb_del(struct ndmsg *ndm,
+static int ixgbe_ndo_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
struct net_device *dev,
const unsigned char *addr)
{
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 68a43fe..480189c 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -565,7 +565,7 @@ static int macvlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
return err;
}
-static int macvlan_fdb_del(struct ndmsg *ndm,
+static int macvlan_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
struct net_device *dev,
const unsigned char *addr)
{
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 3b3fdf6..09340c9 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -392,7 +392,8 @@ static int vxlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
}
/* Delete entry (via netlink) */
-static int vxlan_fdb_delete(struct ndmsg *ndm, struct net_device *dev,
+static int vxlan_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
+ struct net_device *dev,
const unsigned char *addr)
{
struct vxlan_dev *vxlan = netdev_priv(dev);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index c6a14d4..3ee7a80 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -996,6 +996,7 @@ struct net_device_ops {
const unsigned char *addr,
u16 flags);
int (*ndo_fdb_del)(struct ndmsg *ndm,
+ struct nlattr *tb[],
struct net_device *dev,
const unsigned char *addr);
int (*ndo_fdb_dump)(struct sk_buff *skb,
diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
index 275e5d6..adb068c 100644
--- a/include/uapi/linux/neighbour.h
+++ b/include/uapi/linux/neighbour.h
@@ -20,6 +20,7 @@ enum {
NDA_LLADDR,
NDA_CACHEINFO,
NDA_PROBES,
+ NDA_VLAN,
__NDA_MAX
};
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index a244efc..f1199e4 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -506,6 +506,10 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
ci.ndm_refcnt = 0;
if (nla_put(skb, NDA_CACHEINFO, sizeof(ci), &ci))
goto nla_put_failure;
+
+ if (nla_put(skb, NDA_VLAN, sizeof(u16), &fdb->vlan_id))
+ goto nla_put_failure;
+
return nlmsg_end(skb, nlh);
nla_put_failure:
@@ -517,6 +521,7 @@ static inline size_t fdb_nlmsg_size(void)
{
return NLMSG_ALIGN(sizeof(struct ndmsg))
+ nla_total_size(ETH_ALEN) /* NDA_LLADDR */
+ + nla_total_size(sizeof(u16)) /* NDA_VLAN */
+ nla_total_size(sizeof(struct nda_cacheinfo));
}
@@ -618,19 +623,55 @@ static int fdb_add_entry(struct net_bridge_port *source, const __u8 *addr,
return 0;
}
+static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge_port *p,
+ const unsigned char *addr, u16 nlh_flags, u16 vid)
+{
+ int err = 0;
+
+ if (ndm->ndm_flags & NTF_USE) {
+ rcu_read_lock();
+ br_fdb_update(p->br, p, addr, vid);
+ rcu_read_unlock();
+ } else {
+ spin_lock_bh(&p->br->hash_lock);
+ err = fdb_add_entry(p, addr, ndm->ndm_state,
+ nlh_flags, vid);
+ spin_unlock_bh(&p->br->hash_lock);
+ }
+
+ return err;
+}
+
/* Add new permanent fdb entry with RTM_NEWNEIGH */
int br_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
struct net_device *dev,
const unsigned char *addr, u16 nlh_flags)
{
struct net_bridge_port *p;
+ struct net_port_vlan *pve;
int err = 0;
+ unsigned short vid = BR_INVALID_VID;
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;
}
+ if (tb[NDA_VLAN]) {
+ if (nla_len(tb[NDA_VLAN]) != sizeof(unsigned short)) {
+ pr_info("bridge: RTM_NEWNEIGH with invalid vlan\n");
+ return -EINVAL;
+ }
+
+ vid = nla_get_u16(tb[NDA_VLAN]);
+
+ if (vid > VLAN_N_VID) {
+ pr_info("bridge: RTM_NEWNEIGH with invalid vlan id %d\n",
+ vid);
+ return -EINVAL;
+ }
+ }
+
p = br_port_get_rtnl(dev);
if (p == NULL) {
pr_info("bridge: RTM_NEWNEIGH %s not a bridge port\n",
@@ -638,27 +679,45 @@ int br_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
return -EINVAL;
}
- if (ndm->ndm_flags & NTF_USE) {
- rcu_read_lock();
- br_fdb_update(p->br, p, addr, 0);
- rcu_read_unlock();
+ if (vid != BR_INVALID_VID) {
+ pve = nbp_vlan_find(p, vid);
+ if (!pve) {
+ pr_info("bridge: RTM_NEWNEIGH with unconfigured "
+ "vlan %d on port %s\n", vid, dev->name);
+ return -EINVAL;
+ }
+
+ /* VID was specified, so use it. */
+ err = __br_fdb_add(ndm, p, addr, nlh_flags, vid);
} else {
- spin_lock_bh(&p->br->hash_lock);
- err = fdb_add_entry(p, addr, ndm->ndm_state, nlh_flags,
- 0);
- spin_unlock_bh(&p->br->hash_lock);
+ if (list_empty(&p->vlan_list)) {
+ err = __br_fdb_add(ndm, p, addr, nlh_flags, 0);
+ goto out;
+ }
+
+ /* We have vlans configured on this port and user didn't
+ * specify a VLAN. To be nice, add/update entry for every
+ * vlan on this port.
+ */
+ list_for_each_entry_rcu(pve, &p->vlan_list, list) {
+ err = __br_fdb_add(ndm, p, addr, nlh_flags, pve->vid);
+ if (err)
+ goto out;
+ }
}
+out:
return err;
}
-static int fdb_delete_by_addr(struct net_bridge_port *p, const u8 *addr)
+static int fdb_delete_by_addr(struct net_bridge_port *p, const u8 *addr,
+ u16 vlan)
{
struct net_bridge *br = p->br;
- struct hlist_head *head = &br->hash[br_mac_hash(addr, 0)];
+ struct hlist_head *head = &br->hash[br_mac_hash(addr, vlan)];
struct net_bridge_fdb_entry *fdb;
- fdb = fdb_find(head, addr, 0);
+ fdb = fdb_find(head, addr, vlan);
if (!fdb)
return -ENOENT;
@@ -666,13 +725,42 @@ static int fdb_delete_by_addr(struct net_bridge_port *p, const u8 *addr)
return 0;
}
+static int __br_fdb_delete(struct net_bridge_port *p,
+ const unsigned char *addr, u16 vid)
+{
+ int err;
+
+ spin_lock_bh(&p->br->hash_lock);
+ err = fdb_delete_by_addr(p, addr, vid);
+ spin_unlock_bh(&p->br->hash_lock);
+
+ return err;
+}
+
/* Remove neighbor entry with RTM_DELNEIGH */
-int br_fdb_delete(struct ndmsg *ndm, struct net_device *dev,
+int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
+ struct net_device *dev,
const unsigned char *addr)
{
struct net_bridge_port *p;
+ struct net_port_vlan *pve;
int err;
+ unsigned short vid = BR_INVALID_VID;
+ if (tb[NDA_VLAN]) {
+ if (nla_len(tb[NDA_VLAN]) != sizeof(unsigned short)) {
+ pr_info("bridge: RTM_NEWNEIGH with invalid vlan\n");
+ return -EINVAL;
+ }
+
+ vid = nla_get_u16(tb[NDA_VLAN]);
+
+ if (vid > VLAN_N_VID) {
+ pr_info("bridge: RTM_NEWNEIGH with invalid vlan id %d\n",
+ vid);
+ return -EINVAL;
+ }
+ }
p = br_port_get_rtnl(dev);
if (p == NULL) {
pr_info("bridge: RTM_DELNEIGH %s not a bridge port\n",
@@ -680,9 +768,30 @@ int br_fdb_delete(struct ndmsg *ndm, struct net_device *dev,
return -EINVAL;
}
- spin_lock_bh(&p->br->hash_lock);
- err = fdb_delete_by_addr(p, addr);
- spin_unlock_bh(&p->br->hash_lock);
+ if (vid != BR_INVALID_VID) {
+ pve = nbp_vlan_find(p, vid);
+ if (!pve) {
+ pr_info("bridge: RTM_DELNEIGH with unconfigured "
+ "vlan %d on port %s\n", vid, dev->name);
+ return -EINVAL;
+ }
+ err = __br_fdb_delete(p, addr, vid);
+ } else {
+ if (list_empty(&p->vlan_list)) {
+ err = __br_fdb_delete(p, addr, 0);
+ goto out;
+ }
+
+ /* We have vlans configured on this port and user didn't
+ * specify a VLAN. To be nice, add/update entry for every
+ * vlan on this port.
+ */
+ err = -ENOENT;
+ list_for_each_entry_rcu(pve, &p->vlan_list, list) {
+ err &= __br_fdb_delete(p, addr, pve->vid);
+ }
+ }
+out:
return err;
}
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 2569afb..cc75212 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -407,7 +407,7 @@ extern void br_fdb_update(struct net_bridge *br,
const unsigned char *addr,
u16 vid);
-extern int br_fdb_delete(struct ndmsg *ndm,
+extern int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
struct net_device *dev,
const unsigned char *addr);
extern int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[],
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 1868625..8352302 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2125,7 +2125,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
{
struct net *net = sock_net(skb->sk);
struct ndmsg *ndm;
- struct nlattr *llattr;
+ struct nlattr *tb[NDA_MAX+1];
struct net_device *dev;
int err = -EINVAL;
__u8 *addr;
@@ -2133,8 +2133,9 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
if (!capable(CAP_NET_ADMIN))
return -EPERM;
- if (nlmsg_len(nlh) < sizeof(*ndm))
- return -EINVAL;
+ err = nlmsg_parse(nlh, sizeof(*ndm), tb, NDA_MAX, NULL);
+ if (err < 0)
+ return err;
ndm = nlmsg_data(nlh);
if (ndm->ndm_ifindex == 0) {
@@ -2148,13 +2149,17 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
return -ENODEV;
}
- llattr = nlmsg_find_attr(nlh, sizeof(*ndm), NDA_LLADDR);
- if (llattr == NULL || nla_len(llattr) != ETH_ALEN) {
- pr_info("PF_BRIGDE: RTM_DELNEIGH with invalid address\n");
+ if (!tb[NDA_LLADDR] || nla_len(tb[NDA_LLADDR]) != ETH_ALEN) {
+ pr_info("PF_BRIDGE: RTM_DELNEIGH with invalid address\n");
+ return -EINVAL;
+ }
+
+ addr = nla_data(tb[NDA_LLADDR]);
+ if (!is_valid_ether_addr(addr)) {
+ pr_info("PF_BRIDGE: RTM_DELNEIGH with invalid ether address\n");
return -EINVAL;
}
- addr = nla_data(llattr);
err = -EOPNOTSUPP;
/* Support fdb on master device the net/bridge default case */
@@ -2163,7 +2168,8 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
struct net_device *master = dev->master;
if (master->netdev_ops->ndo_fdb_del)
- err = master->netdev_ops->ndo_fdb_del(ndm, dev, addr);
+ err = master->netdev_ops->ndo_fdb_del(ndm, tb,
+ dev, addr);
if (err)
goto out;
@@ -2173,7 +2179,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
/* Embedded bridge, macvlan, and any other device support */
if ((ndm->ndm_flags & NTF_SELF) && dev->netdev_ops->ndo_fdb_del) {
- err = dev->netdev_ops->ndo_fdb_del(ndm, dev, addr);
+ err = dev->netdev_ops->ndo_fdb_del(ndm, tb, dev, addr);
if (!err) {
rtnl_fdb_notify(dev, addr, RTM_DELNEIGH);
--
1.7.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH V2 09/12] bridge: Add the ability to configure untagged vlans
2012-12-18 19:00 [PATCH V2 00/12] Add basic VLAN support to bridges Vlad Yasevich
` (7 preceding siblings ...)
2012-12-18 19:00 ` [PATCH V2 08/12] bridge: Add vlan support to static neighbors Vlad Yasevich
@ 2012-12-18 19:01 ` Vlad Yasevich
2012-12-18 23:01 ` Michael S. Tsirkin
2012-12-18 23:04 ` Michael S. Tsirkin
2012-12-18 19:01 ` [PATCH V2 10/12] bridge: Implement untagged vlan handling Vlad Yasevich
` (4 subsequent siblings)
13 siblings, 2 replies; 35+ messages in thread
From: Vlad Yasevich @ 2012-12-18 19:01 UTC (permalink / raw)
To: netdev; +Cc: shemminger, davem, or.gerlitz, jhs, mst
A user may designate a certain vlan as untagged. This means that
any ingress frame is assigned to this vlan and any forwarding decisions
are made with this vlan in mind. On egress, any frames tagged/labeled
with untagged vlan have the vlan tag removed and are send as regular
ethernet frames.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
include/uapi/linux/if_bridge.h | 3 +
net/bridge/br_if.c | 146 +++++++++++++++++++++++++++++++++++++---
net/bridge/br_netlink.c | 6 +-
net/bridge/br_private.h | 2 +
4 files changed, 144 insertions(+), 13 deletions(-)
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index d0b4f5c..988d858 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -127,6 +127,9 @@ enum {
BR_VLAN_DEL,
};
+#define BRIDGE_VLAN_INFO_MASTER 1
+#define BRIDGE_VLAN_INFO_UNTAGGED 2
+
struct bridge_vlan_info {
u16 op_code;
u16 flags;
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 57bbb35..14563fb 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -108,6 +108,34 @@ static void br_vlan_put(struct net_bridge_vlan *vlan)
br_vlan_destroy(vlan);
}
+/* Must be protected by RTNL */
+static void br_vlan_add_untagged(struct net_bridge *br,
+ struct net_bridge_vlan *vlan)
+{
+ ASSERT_RTNL();
+ if (br->untagged == vlan)
+ return;
+ else if (br->untagged) {
+ /* Untagged vlan is already set on the master,
+ * so drop the ref since we'll be replacing it.
+ */
+ br_vlan_put(br->untagged);
+ }
+ br_vlan_hold(vlan);
+ rcu_assign_pointer(br->untagged, vlan);
+}
+
+/* Must be protected by RTNL */
+static void br_vlan_del_untagged(struct net_bridge *br,
+ struct net_bridge_vlan *vlan)
+{
+ ASSERT_RTNL();
+ if (br->untagged == vlan) {
+ br_vlan_put(vlan);
+ rcu_assign_pointer(br->untagged, NULL);
+ }
+}
+
struct net_bridge_vlan *br_vlan_find(struct net_bridge *br, u16 vid)
{
struct net_bridge_vlan *vlan;
@@ -132,7 +160,7 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
vlan = br_vlan_find(br, vid);
if (vlan)
- return vlan;
+ goto untagged;
vlan = kzalloc(sizeof(struct net_bridge_vlan), GFP_KERNEL);
if (!vlan)
@@ -141,7 +169,7 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
vlan->vid = vid;
atomic_set(&vlan->refcnt, 1);
- if (flags & BRIDGE_FLAGS_SELF) {
+ if (flags & BRIDGE_VLAN_INFO_MASTER) {
/* Set bit 0 that is associated with the bridge master
* device. Port numbers start with 1.
*/
@@ -149,15 +177,24 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
}
hlist_add_head_rcu(&vlan->hlist, &br->vlan_hlist[br_vlan_hash(vid)]);
+
+untagged:
+ if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
+ br_vlan_add_untagged(br, vlan);
+
return vlan;
}
/* Must be protected by RTNL */
-static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
+static void br_vlan_del(struct net_bridge *br, struct net_bridge_vlan *vlan,
+ u16 flags)
{
ASSERT_RTNL();
- if (flags & BRIDGE_FLAGS_SELF) {
+ if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
+ br_vlan_del_untagged(br, vlan);
+
+ if (flags & BRIDGE_VLAN_INFO_MASTER) {
/* Clear bit 0 that is associated with the bridge master
* device.
*/
@@ -172,6 +209,14 @@ static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
vlan->vid = BR_INVALID_VID;
+ /* If, for whatever reason, bridge still has a ref on this vlan
+ * through the @untagged pointer, drop that ref and clear untagged.
+ */
+ if (br->untagged == vlan) {
+ br_vlan_put(vlan);
+ rcu_assign_pointer(br->untagged, NULL);
+ }
+
/* Drop the self-ref to trigger descrution. */
br_vlan_put(vlan);
}
@@ -187,7 +232,7 @@ int br_vlan_delete(struct net_bridge *br, u16 vid, u16 flags)
if (!vlan)
return -ENOENT;
- br_vlan_del(vlan, flags);
+ br_vlan_del(br, vlan, flags);
return 0;
}
@@ -204,7 +249,9 @@ static void br_vlan_flush(struct net_bridge *br)
for (i = 0; i < BR_VID_HASH_SIZE; i++) {
hlist_for_each_entry_safe(vlan, node, tmp,
&br->vlan_hlist[i], hlist) {
- br_vlan_del(vlan, BRIDGE_FLAGS_SELF);
+ br_vlan_del(br, vlan,
+ (BRIDGE_VLAN_INFO_MASTER |
+ BRIDGE_VLAN_INFO_UNTAGGED));
}
}
}
@@ -224,10 +271,70 @@ struct net_port_vlan *nbp_vlan_find(const struct net_bridge_port *p, u16 vid)
return NULL;
}
+static int nbp_vlan_add_untagged(struct net_bridge_port *p,
+ struct net_bridge_vlan *vlan,
+ u16 flags)
+{
+ struct net_device *dev = p->dev;
+
+ if (p->untagged) {
+ /* Port already has untagged vlan set. Drop the ref
+ * to the old one since we'll be replace it.
+ */
+ br_vlan_put(p->untagged);
+ } else {
+ int err;
+
+ /* Add vid 0 to filter if filter is available. */
+ if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
+ dev->netdev_ops->ndo_vlan_rx_add_vid &&
+ dev->netdev_ops->ndo_vlan_rx_kill_vid) {
+ err = dev->netdev_ops->ndo_vlan_rx_add_vid(dev, 0);
+ if (err)
+ return err;
+ }
+ }
+
+ /* This VLAN is handled as untagged/native. Save an
+ * additional ref.
+ */
+ br_vlan_hold(vlan);
+ rcu_assign_pointer(p->untagged, vlan);
+
+ return 0;
+}
+
+static void nbp_vlan_delete_untagged(struct net_bridge_port *p,
+ struct net_bridge_vlan *vlan)
+{
+ if (p->untagged != vlan)
+ return;
+
+ /* Remove VLAN from the device filter if it is supported. */
+ if ((p->dev->features & NETIF_F_HW_VLAN_FILTER) &&
+ p->dev->netdev_ops->ndo_vlan_rx_kill_vid) {
+ int err;
+
+ err = p->dev->netdev_ops->ndo_vlan_rx_kill_vid(p->dev, 0);
+ if (err) {
+ pr_warn("failed to kill vid %d for device %s\n",
+ vlan->vid, p->dev->name);
+ }
+ }
+
+ /* If this VLAN is currently functioning as untagged, clear it.
+ * It's safe to drop the refcount, since the vlan is still held
+ * by the port.
+ */
+ br_vlan_put(vlan);
+ rcu_assign_pointer(p->untagged, NULL);
+
+}
+
/* Must be protected by RTNL */
int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
{
- struct net_port_vlan *pve;
+ struct net_port_vlan *pve = NULL;
struct net_bridge_vlan *vlan;
struct net_device *dev = p->dev;
int err;
@@ -275,11 +382,21 @@ int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
set_bit(p->port_no, vlan->port_bitmap);
list_add_tail_rcu(&pve->list, &p->vlan_list);
+
+ if (flags & BRIDGE_VLAN_INFO_UNTAGGED) {
+ err = nbp_vlan_add_untagged(p, vlan, flags);
+ if (err)
+ goto del_vlan;
+ }
+
return 0;
clean_up:
kfree(pve);
- br_vlan_del(vlan, flags);
+ br_vlan_del(p->br, vlan, flags);
+ return err;
+del_vlan:
+ nbp_vlan_delete(p, vid, flags);
return err;
}
@@ -296,6 +413,9 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
if (!pve)
return -ENOENT;
+ if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
+ nbp_vlan_delete_untagged(p, pve->vlan);
+
/* Remove VLAN from the device filter if it is supported. */
if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
dev->netdev_ops->ndo_vlan_rx_kill_vid) {
@@ -306,6 +426,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
pr_warn("failed to kill vid %d for device %s\n",
vid, dev->name);
}
+
pve->vid = BR_INVALID_VID;
vlan = pve->vlan;
@@ -316,7 +437,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
list_del_rcu(&pve->list);
kfree_rcu(pve, rcu);
- br_vlan_del(vlan, flags);
+ br_vlan_del(p->br, vlan, flags);
return 0;
}
@@ -328,8 +449,11 @@ static void nbp_vlan_flush(struct net_bridge_port *p)
ASSERT_RTNL();
- list_for_each_entry_safe(pve, tmp, &p->vlan_list, list)
- nbp_vlan_delete(p, pve->vid, BRIDGE_FLAGS_SELF);
+ list_for_each_entry_safe(pve, tmp, &p->vlan_list, list) {
+ nbp_vlan_delete(p, pve->vid,
+ (BRIDGE_VLAN_INFO_MASTER |
+ BRIDGE_VLAN_INFO_UNTAGGED));
+ }
}
static void release_nbp(struct kobject *kobj)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 9cf2879..1b302ce 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -199,7 +199,8 @@ static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
if (p)
err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
else {
- u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
+ u16 flags = vinfo->flags |
+ BRIDGE_VLAN_INFO_MASTER;
if (!br_vlan_add(br, vinfo->vid, flags))
err = -ENOMEM;
}
@@ -210,7 +211,8 @@ static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
err = nbp_vlan_delete(p, vinfo->vid,
vinfo->flags);
else {
- u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
+ u16 flags = vinfo->flags |
+ BRIDGE_VLAN_INFO_MASTER;
err = br_vlan_delete(br, vinfo->vid, flags);
}
break;
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index cc75212..9328463 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -179,6 +179,7 @@ struct net_bridge_port
struct netpoll *np;
#endif
struct list_head vlan_list;
+ struct net_bridge_vlan __rcu *untagged;
};
#define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT)
@@ -298,6 +299,7 @@ struct net_bridge
struct timer_list gc_timer;
struct kobject *ifobj;
struct hlist_head vlan_hlist[BR_VID_HASH_SIZE];
+ struct net_bridge_vlan __rcu *untagged;
};
struct br_input_skb_cb {
--
1.7.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH V2 10/12] bridge: Implement untagged vlan handling
2012-12-18 19:00 [PATCH V2 00/12] Add basic VLAN support to bridges Vlad Yasevich
` (8 preceding siblings ...)
2012-12-18 19:01 ` [PATCH V2 09/12] bridge: Add the ability to configure untagged vlans Vlad Yasevich
@ 2012-12-18 19:01 ` Vlad Yasevich
2012-12-18 19:01 ` [PATCH V2 11/12] bridge: Dump vlan information from a bridge port Vlad Yasevich
` (3 subsequent siblings)
13 siblings, 0 replies; 35+ messages in thread
From: Vlad Yasevich @ 2012-12-18 19:01 UTC (permalink / raw)
To: netdev; +Cc: shemminger, davem, or.gerlitz, jhs, mst
When an untagged frame arrives on a port that has untagged vlan set,
the frame is assigned to the untagged VLAN. It will then procede
through the forwarding/egress process with the VLAN id set to the
untagged VLAN.
At egress, a frame with a VLAN id matching untagged vid will have its
vlan header stripped off.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
net/bridge/br_device.c | 25 +++++++-
net/bridge/br_forward.c | 134 ++++++++++++++++++++++++++++++++++++++++++++-
net/bridge/br_input.c | 46 ++++++++++++---
net/bridge/br_multicast.c | 37 +++++++-----
net/bridge/br_private.h | 20 ++++--
5 files changed, 224 insertions(+), 38 deletions(-)
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 1f9d0f9..37441b10 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -31,7 +31,8 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
struct net_bridge_mdb_entry *mdst;
struct br_cpu_netstats *brstats = this_cpu_ptr(br->stats);
struct net_bridge_vlan *vlan;
- u16 vid;
+ struct br_input_skb_cb *brcb;
+ u16 vid = 0;
rcu_read_lock();
#ifdef CONFIG_BRIDGE_NETFILTER
@@ -47,15 +48,31 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
brstats->tx_bytes += skb->len;
u64_stats_update_end(&brstats->syncp);
- BR_INPUT_SKB_CB(skb)->brdev = dev;
+ brcb = BR_INPUT_SKB_CB(skb);
+ memset(brcb, 0, sizeof(struct br_input_skb_cb));
+ brcb->brdev = dev;
+
+ if (br_get_vlan(skb, &vid)) {
+ u16 untagged_vid;
+
+ /* Untagged frame. See if there is an untagged VLAN
+ * configured.
+ */
+ if ((vlan = rcu_dereference(br->untagged)) != NULL &&
+ (untagged_vid = vlan->vid) != BR_INVALID_VID) {
+ __vlan_hwaccel_put_tag(skb, untagged_vid);
+ brcb->untagged = 1;
+ goto skip_lookup;
+ }
+ }
/* Any vlan transmitted by the bridge itself is permitted.
* Try to cache the vlan in the CB to speed up forwarding.
*/
- vid = br_get_vlan(skb);
vlan = br_vlan_find(br, vid);
+skip_lookup:
if (vlan)
- BR_INPUT_SKB_CB(skb)->vlan = vlan;
+ brcb->vlan = vlan;
skb_reset_mac_header(skb);
skb_pull(skb, ETH_HLEN);
diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 4ae5f55..bcd16e8 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -31,7 +31,7 @@ static inline bool br_allowed_egress(const struct net_bridge_port *p,
{
struct net_port_vlan *pve;
struct net_bridge_vlan *vlan = NULL;
- u16 vid;
+ u16 vid = 0;
if (list_empty(&p->vlan_list))
return true;
@@ -49,14 +49,134 @@ static inline bool br_allowed_egress(const struct net_bridge_port *p,
/* We don't have cached vlan information, so we need to do
* it the hard way.
*/
- vid = br_get_vlan(skb);
+ br_get_vlan(skb, &vid);
pve = nbp_vlan_find(p, vid);
- if (pve)
+ if (pve) {
+ BR_INPUT_SKB_CB(skb)->vlan = pve->vlan;
return true;
+ }
return false;
}
+/* Almost an exact copy of vlan_untag. Needed here since it's not exported
+ * and not available if vlan support isn't built in.
+ */
+static struct sk_buff *br_vlan_untag(struct sk_buff *skb)
+{
+ struct vlan_hdr *vhdr;
+
+ if (skb->protocol != htons(ETH_P_8021Q)) {
+ skb->vlan_tci = 0;
+ return skb;
+ }
+
+ skb = skb_share_check(skb, GFP_ATOMIC);
+ if (unlikely(!skb))
+ goto err_free;
+
+ if (unlikely(!pskb_may_pull(skb, VLAN_HLEN)))
+ goto err_free;
+
+ vhdr = (struct vlan_hdr *)skb->data;
+ skb_pull_rcsum(skb, VLAN_HLEN);
+ vlan_set_encap_proto(skb, vhdr);
+
+ if (skb_cow(skb, skb_headroom(skb)) < 0)
+ goto err_free;
+
+ memmove(skb->data - ETH_HLEN, skb->data - VLAN_ETH_HLEN, 2 * ETH_ALEN);
+ skb->mac_header += VLAN_HLEN;
+ skb->vlan_tci = 0;
+
+ skb_reset_network_header(skb);
+ skb_reset_transport_header(skb);
+ skb_reset_mac_len(skb);
+
+ return skb;
+
+err_free:
+ kfree_skb(skb);
+ return NULL;
+}
+
+struct sk_buff *br_handle_vlan(const struct net_bridge *br,
+ const struct net_bridge_port *p,
+ struct sk_buff *skb)
+{
+ struct net_bridge_vlan *untag_vlan;
+ struct net_bridge_vlan *skb_vlan = BR_INPUT_SKB_CB(skb)->vlan;
+
+ if (p) {
+ /* If there are no vlans defined on this port, there
+ * is nothing to do
+ */
+ if (list_empty(&p->vlan_list))
+ goto out;
+
+ untag_vlan = rcu_dereference(p->untagged);
+ } else if (br)
+ untag_vlan = rcu_dereference(br->untagged);
+ else {
+ kfree_skb(skb);
+ goto out;
+ }
+
+ if (BR_INPUT_SKB_CB(skb)->untagged) {
+ /* Frame arrived on an untagged vlan. If it is leaving
+ * on untagged interface, remove the tag we added.
+ */
+ if (skb_vlan && untag_vlan == skb_vlan) {
+ skb->vlan_tci = 0;
+ goto out;
+ }
+
+ /* Frame leaving on a tagged vlan. If output device is the
+ * bridge, we need to add the VLAN header. If we sending to
+ * port, we let dev_hard_start_xmit() add the header.
+ */
+ if (br && !p) {
+ /* vlan_put_tag expects skb->data to point to mac
+ * header.
+ */
+ skb_push(skb, ETH_HLEN);
+ skb = __vlan_put_tag(skb, skb->vlan_tci);
+ if (!skb)
+ goto out;
+ /* put skb->data back to where it was */
+ skb_pull(skb, ETH_HLEN);
+ skb->vlan_tci = 0;
+ }
+ } else {
+ /* Here we consider the frame tagged */
+ if (!skb_vlan && br) {
+ /* Bridge output device is special in that it doesn't
+ * do any egress filtering. This means that we need
+ * to check against untagged vlan a little differently.
+ */
+ u16 vid = 0;
+
+ br_get_vlan(skb, &vid);
+ if (untag_vlan && vid == untag_vlan->vid) {
+ /* VLAN is untagged on the bridge, strip */
+ skb = br_vlan_untag(skb);
+ }
+ } else {
+ /* Port egress check will find us the right vlan in
+ * case we couldn't find one on ingress.
+ * Now, if the egress vlan is "untagged/native",
+ * strip the tag. Otherwise, don't do anything.
+ */
+ if (untag_vlan && untag_vlan->vid != BR_INVALID_VID &&
+ skb_vlan == untag_vlan)
+ skb = br_vlan_untag(skb);
+ }
+ }
+
+out:
+ return skb;
+}
+
/* Don't forward packets to originating port or forwarding diasabled */
static inline int should_deliver(const struct net_bridge_port *p,
const struct sk_buff *skb)
@@ -97,6 +217,10 @@ static void __br_deliver(const struct net_bridge_port *to, struct sk_buff *skb)
{
skb->dev = to->dev;
+ skb = br_handle_vlan(NULL, to, skb);
+ if (!skb)
+ return;
+
if (unlikely(netpoll_tx_running(to->br->dev))) {
if (packet_length(skb) > skb->dev->mtu && !skb_is_gso(skb))
kfree_skb(skb);
@@ -120,6 +244,10 @@ static void __br_forward(const struct net_bridge_port *to, struct sk_buff *skb)
return;
}
+ skb = br_handle_vlan(NULL, to, skb);
+ if (!skb)
+ return;
+
indev = skb->dev;
skb->dev = to->dev;
skb_forward_csum(skb);
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index e51eb24..1ff7f2c 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -35,6 +35,10 @@ static int br_pass_frame_up(struct sk_buff *skb)
brstats->rx_bytes += skb->len;
u64_stats_update_end(&brstats->syncp);
+ skb = br_handle_vlan(br, NULL, skb);
+ if (!skb)
+ return NET_RX_DROP;
+
indev = skb->dev;
skb->dev = brdev;
@@ -43,11 +47,11 @@ static int br_pass_frame_up(struct sk_buff *skb)
}
static bool br_allowed_ingress(struct net_bridge_port *p, struct sk_buff *skb,
- u16 vid)
+ u16 *vid)
{
struct net_port_vlan *pve;
-
- BR_INPUT_SKB_CB(skb)->vlan = NULL;
+ struct net_bridge_vlan *vlan;
+ struct br_input_skb_cb *brcb = BR_INPUT_SKB_CB(skb);
/* If there are no vlan in the permitted list, all packets are
* permitted.
@@ -55,7 +59,27 @@ static bool br_allowed_ingress(struct net_bridge_port *p, struct sk_buff *skb,
if (list_empty(&p->vlan_list))
return true;
- pve = nbp_vlan_find(p, vid);
+ if (br_get_vlan(skb, vid)) {
+ u16 untagged_vid;
+ /* Frame did not have a tag. See if untagged vlan is set
+ * on this port.
+ */
+ if ((vlan = rcu_dereference(p->untagged)) == NULL ||
+ (untagged_vid = vlan->vid) == BR_INVALID_VID)
+ return false;
+
+ /* Untagged vlan is set on this port. Any untagged ingress
+ * frame is considered to belong to the untagged vlan, so
+ * mark it as such.
+ */
+ __vlan_hwaccel_put_tag(skb, untagged_vid);
+ brcb->vlan = vlan;
+ brcb->untagged = 1;
+ return true;
+ }
+
+ /* Frame has a valid vlan tag. Find the VLAN it belongs to. */
+ pve = nbp_vlan_find(p, *vid);
if (pve) {
BR_INPUT_SKB_CB(skb)->vlan = pve->vlan;
return true;
@@ -73,13 +97,15 @@ int br_handle_frame_finish(struct sk_buff *skb)
struct net_bridge_fdb_entry *dst;
struct net_bridge_mdb_entry *mdst;
struct sk_buff *skb2;
- u16 vid;
+ struct br_input_skb_cb *brcb = BR_INPUT_SKB_CB(skb);
+ u16 vid = 0;
if (!p || p->state == BR_STATE_DISABLED)
goto drop;
- vid = br_get_vlan(skb);
- if (!br_allowed_ingress(p, skb, vid))
+ memset(brcb, 0, sizeof(struct br_input_skb_cb));
+
+ if (!br_allowed_ingress(p, skb, &vid))
goto drop;
/* insert into forwarding database after filtering to avoid spoofing */
@@ -93,7 +119,7 @@ int br_handle_frame_finish(struct sk_buff *skb)
if (p->state == BR_STATE_LEARNING)
goto drop;
- BR_INPUT_SKB_CB(skb)->brdev = br->dev;
+ brcb->brdev = br->dev;
/* The packet skb2 goes to the local host (NULL to skip). */
skb2 = NULL;
@@ -148,8 +174,10 @@ drop:
static int br_handle_local_finish(struct sk_buff *skb)
{
struct net_bridge_port *p = br_port_get_rcu(skb->dev);
+ u16 vid = 0;
- br_fdb_update(p->br, p, eth_hdr(skb)->h_source, br_get_vlan(skb));
+ br_get_vlan(skb, &vid);
+ br_fdb_update(p->br, p, eth_hdr(skb)->h_source, vid);
return 0; /* process further */
}
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 072aa2d..a7fc2c7 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -905,6 +905,7 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br,
int type;
int err = 0;
__be32 group;
+ u16 vid = 0;
if (!pskb_may_pull(skb, sizeof(*ih)))
return -EINVAL;
@@ -940,8 +941,8 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br,
continue;
}
- err = br_ip4_multicast_add_group(br, port, group,
- br_get_vlan(skb));
+ br_get_vlan(skb, &vid);
+ err = br_ip4_multicast_add_group(br, port, group, vid);
if (err)
break;
}
@@ -960,6 +961,7 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
int len;
int num;
int err = 0;
+ u16 vid = 0;
if (!pskb_may_pull(skb, sizeof(*icmp6h)))
return -EINVAL;
@@ -1001,8 +1003,9 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
continue;
}
+ br_get_vlan(skb, &vid);
err = br_ip6_multicast_add_group(br, port, &grec->grec_mca,
- br_get_vlan(skb));
+ vid);
if (!err)
break;
}
@@ -1086,6 +1089,7 @@ static int br_ip4_multicast_query(struct net_bridge *br,
unsigned long now = jiffies;
__be32 group;
int err = 0;
+ u16 vid = 0;
spin_lock(&br->multicast_lock);
if (!netif_running(br->dev) ||
@@ -1120,8 +1124,8 @@ static int br_ip4_multicast_query(struct net_bridge *br,
if (!group)
goto out;
- mp = br_mdb_ip4_get(mlock_dereference(br->mdb, br), group,
- br_get_vlan(skb));
+ br_get_vlan(skb, &vid);
+ mp = br_mdb_ip4_get(mlock_dereference(br->mdb, br), group, vid);
if (!mp)
goto out;
@@ -1162,6 +1166,7 @@ static int br_ip6_multicast_query(struct net_bridge *br,
unsigned long now = jiffies;
const struct in6_addr *group = NULL;
int err = 0;
+ u16 vid = 0;
spin_lock(&br->multicast_lock);
if (!netif_running(br->dev) ||
@@ -1193,8 +1198,8 @@ static int br_ip6_multicast_query(struct net_bridge *br,
if (!group)
goto out;
- mp = br_mdb_ip6_get(mlock_dereference(br->mdb, br), group,
- br_get_vlan(skb));
+ br_get_vlan(skb, &vid);
+ mp = br_mdb_ip6_get(mlock_dereference(br->mdb, br), group, vid);
if (!mp)
goto out;
@@ -1343,6 +1348,7 @@ static int br_multicast_ipv4_rcv(struct net_bridge *br,
unsigned int len;
unsigned int offset;
int err;
+ u16 vid = 0;
/* We treat OOM as packet loss for now. */
if (!pskb_may_pull(skb, sizeof(*iph)))
@@ -1410,8 +1416,8 @@ static int br_multicast_ipv4_rcv(struct net_bridge *br,
case IGMP_HOST_MEMBERSHIP_REPORT:
case IGMPV2_HOST_MEMBERSHIP_REPORT:
BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
- err = br_ip4_multicast_add_group(br, port, ih->group,
- br_get_vlan(skb2));
+ vid = br_get_vlan(skb2, &vid);
+ err = br_ip4_multicast_add_group(br, port, ih->group, vid);
break;
case IGMPV3_HOST_MEMBERSHIP_REPORT:
err = br_ip4_multicast_igmp3_report(br, port, skb2);
@@ -1420,8 +1426,8 @@ static int br_multicast_ipv4_rcv(struct net_bridge *br,
err = br_ip4_multicast_query(br, port, skb2);
break;
case IGMP_HOST_LEAVE_MESSAGE:
- br_ip4_multicast_leave_group(br, port, ih->group,
- br_get_vlan(skb2));
+ vid = br_get_vlan(skb2, &vid);
+ br_ip4_multicast_leave_group(br, port, ih->group, vid);
break;
}
@@ -1446,6 +1452,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
unsigned int len;
int offset;
int err;
+ u16 vid = 0;
if (!pskb_may_pull(skb, sizeof(*ip6h)))
return -EINVAL;
@@ -1541,8 +1548,8 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
}
mld = (struct mld_msg *)skb_transport_header(skb2);
BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
- err = br_ip6_multicast_add_group(br, port, &mld->mld_mca,
- br_get_vlan(skb2));
+ br_get_vlan(skb2, &vid);
+ err = br_ip6_multicast_add_group(br, port, &mld->mld_mca, vid);
break;
}
case ICMPV6_MLD2_REPORT:
@@ -1559,8 +1566,8 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
goto out;
}
mld = (struct mld_msg *)skb_transport_header(skb2);
- br_ip6_multicast_leave_group(br, port, &mld->mld_mca,
- br_get_vlan(skb2));
+ br_get_vlan(skb2, &vid);
+ br_ip6_multicast_leave_group(br, port, &mld->mld_mca, vid);
}
}
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 9328463..6f662a4 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -198,18 +198,20 @@ static inline struct net_bridge_port *br_port_get_rtnl(struct net_device *dev)
rtnl_dereference(dev->rx_handler_data) : NULL;
}
-static inline u16 br_get_vlan(const struct sk_buff *skb)
+static inline int br_get_vlan(const struct sk_buff *skb, u16 *tag)
{
- u16 tag;
+ int rc = 0;
- if (vlan_tx_tag_present(skb))
- return vlan_tx_tag_get(skb) & VLAN_VID_MASK;
+ if (vlan_tx_tag_present(skb)) {
+ *tag = vlan_tx_tag_get(skb) & VLAN_VID_MASK;
+ return rc;
+ }
/* Untagged and VLAN 0 traffic is handled the same way */
- if (vlan_get_tag(skb, &tag))
- return 0;
+ rc = vlan_get_tag(skb, tag);
+ *tag = *tag & VLAN_VID_MASK;
- return tag & VLAN_VID_MASK;
+ return rc;
}
struct br_cpu_netstats {
@@ -305,6 +307,7 @@ struct net_bridge
struct br_input_skb_cb {
struct net_device *brdev;
struct net_bridge_vlan *vlan;
+ int untagged;
#ifdef CONFIG_BRIDGE_IGMP_SNOOPING
int igmp;
int mrouters_only;
@@ -431,6 +434,9 @@ extern int br_forward_finish(struct sk_buff *skb);
extern void br_flood_deliver(struct net_bridge *br, struct sk_buff *skb);
extern void br_flood_forward(struct net_bridge *br, struct sk_buff *skb,
struct sk_buff *skb2);
+extern struct sk_buff *br_handle_vlan(const struct net_bridge *br,
+ const struct net_bridge_port *p,
+ struct sk_buff *skb);
/* br_if.c */
extern void br_port_carrier_check(struct net_bridge_port *p);
--
1.7.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH V2 11/12] bridge: Dump vlan information from a bridge port
2012-12-18 19:00 [PATCH V2 00/12] Add basic VLAN support to bridges Vlad Yasevich
` (9 preceding siblings ...)
2012-12-18 19:01 ` [PATCH V2 10/12] bridge: Implement untagged vlan handling Vlad Yasevich
@ 2012-12-18 19:01 ` Vlad Yasevich
2012-12-18 19:01 ` [PATCH V2 12/12] bridge: Add vlan support for local fdb entries Vlad Yasevich
` (2 subsequent siblings)
13 siblings, 0 replies; 35+ messages in thread
From: Vlad Yasevich @ 2012-12-18 19:01 UTC (permalink / raw)
To: netdev; +Cc: shemminger, davem, or.gerlitz, jhs, mst
Using the RTM_GETLINK dump the vlan filter list of a given
bridge port. The information depends on setting the filter
flag similar to how nic VF info is dumped.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 +-
include/linux/netdevice.h | 3 +-
include/uapi/linux/if_bridge.h | 1 +
include/uapi/linux/rtnetlink.h | 1 +
net/bridge/br_if.c | 2 +
net/bridge/br_netlink.c | 69 ++++++++++++++++++++++--
net/bridge/br_private.h | 3 +-
net/core/rtnetlink.c | 16 ++++--
8 files changed, 85 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index ec97efe..2f2a8e0 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7062,7 +7062,8 @@ static int ixgbe_ndo_bridge_setlink(struct net_device *dev,
}
static int ixgbe_ndo_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
- struct net_device *dev)
+ struct net_device *dev,
+ u32 filter_mask)
{
struct ixgbe_adapter *adapter = netdev_priv(dev);
u16 mode;
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3ee7a80..d93c47c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1008,7 +1008,8 @@ struct net_device_ops {
struct nlmsghdr *nlh);
int (*ndo_bridge_getlink)(struct sk_buff *skb,
u32 pid, u32 seq,
- struct net_device *dev);
+ struct net_device *dev,
+ u32 filter_mask);
};
/*
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 988d858..6b63e3b 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -115,6 +115,7 @@ enum {
IFLA_BRIDGE_FLAGS,
IFLA_BRIDGE_MODE,
IFLA_BRIDGE_VLAN_INFO,
+ IFLA_BRIDGE_VLAN,
__IFLA_BRIDGE_MAX,
};
#define IFLA_BRIDGE_MAX (__IFLA_BRIDGE_MAX - 1)
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 354a1e7..f20654a 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -624,6 +624,7 @@ struct tcamsg {
/* New extended info filters for IFLA_EXT_MASK */
#define RTEXT_FILTER_VF (1 << 0)
+#define RTEXT_FILTER_BRVLAN (1 << 1)
/* End of information exported to user level */
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 14563fb..3b67ab7 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -382,6 +382,7 @@ int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
set_bit(p->port_no, vlan->port_bitmap);
list_add_tail_rcu(&pve->list, &p->vlan_list);
+ p->num_vlans++;
if (flags & BRIDGE_VLAN_INFO_UNTAGGED) {
err = nbp_vlan_add_untagged(p, vlan, flags);
@@ -435,6 +436,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
br_vlan_put(vlan);
list_del_rcu(&pve->list);
+ p->num_vlans--;
kfree_rcu(pve, rcu);
br_vlan_del(p->br, vlan, flags);
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 1b302ce..b1b7c70 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -65,8 +65,10 @@ static int br_port_fill_attrs(struct sk_buff *skb,
* Create one netlink message for one interface
* Contains port and master info as well as carrier and bridge state.
*/
-static int br_fill_ifinfo(struct sk_buff *skb, const struct net_bridge_port *port,
- u32 pid, u32 seq, int event, unsigned int flags)
+static int br_fill_ifinfo(struct sk_buff *skb,
+ const struct net_bridge_port *port,
+ u32 pid, u32 seq, int event, unsigned int flags,
+ u32 filter_mask)
{
const struct net_bridge *br = port->br;
const struct net_device *dev = port->dev;
@@ -108,6 +110,27 @@ static int br_fill_ifinfo(struct sk_buff *skb, const struct net_bridge_port *por
nla_nest_end(skb, nest);
}
+ /* Check if the VID information is requested */
+ if (filter_mask & RTEXT_FILTER_BRVLAN) {
+ struct nlattr *af;
+ struct net_port_vlan *pve;
+
+ if (list_empty(&port->vlan_list))
+ goto done;
+
+ af = nla_nest_start(skb, IFLA_AF_SPEC | NLA_F_NESTED);
+ if (!af)
+ goto nla_put_failure;
+
+ list_for_each_entry_rcu(pve, &port->vlan_list, list) {
+ if (nla_put_u16(skb, IFLA_BRIDGE_VLAN, pve->vid))
+ goto nla_put_failure;
+ }
+
+ nla_nest_end(skb, af);
+ }
+
+done:
return nlmsg_end(skb, nlh);
nla_put_failure:
@@ -134,7 +157,7 @@ void br_ifinfo_notify(int event, struct net_bridge_port *port)
if (skb == NULL)
goto errout;
- err = br_fill_ifinfo(skb, port, 0, 0, event, 0);
+ err = br_fill_ifinfo(skb, port, 0, 0, event, 0, 0);
if (err < 0) {
/* -EMSGSIZE implies BUG in br_nlmsg_size() */
WARN_ON(err == -EMSGSIZE);
@@ -152,7 +175,7 @@ errout:
* Dump information about all ports, in response to GETLINK
*/
int br_getlink(struct sk_buff *skb, u32 pid, u32 seq,
- struct net_device *dev)
+ struct net_device *dev, u32 filter_mask)
{
int err = 0;
struct net_bridge_port *port = br_port_get_rcu(dev);
@@ -161,7 +184,8 @@ int br_getlink(struct sk_buff *skb, u32 pid, u32 seq,
if (!port)
goto out;
- err = br_fill_ifinfo(skb, port, pid, seq, RTM_NEWLINK, NLM_F_MULTI);
+ err = br_fill_ifinfo(skb, port, pid, seq, RTM_NEWLINK, NLM_F_MULTI,
+ filter_mask);
out:
return err;
}
@@ -364,6 +388,23 @@ static int br_validate(struct nlattr *tb[], struct nlattr *data[])
return 0;
}
+static size_t br_get_link_af_size(const struct net_device *dev)
+{
+ struct net_bridge_port *p;
+
+ p = br_port_get_rcu(dev);
+ if (!p)
+ return 0;
+
+ /* Each VLAN is returned as a short in IFLA_BRIDGE_VLAN attr */
+ return p->num_vlans * nla_total_size(2);
+}
+
+struct rtnl_af_ops br_af_ops = {
+ .family = AF_BRIDGE,
+ .get_link_af_size = br_get_link_af_size,
+};
+
struct rtnl_link_ops br_link_ops __read_mostly = {
.kind = "bridge",
.priv_size = sizeof(struct net_bridge),
@@ -374,11 +415,27 @@ struct rtnl_link_ops br_link_ops __read_mostly = {
int __init br_netlink_init(void)
{
- return rtnl_link_register(&br_link_ops);
+ int err;
+
+ err = rtnl_af_register(&br_af_ops);
+ if (err < 0)
+ goto err2;
+
+ err = rtnl_link_register(&br_link_ops);
+ if (err < 0)
+ goto err1;
+
+ return 0;
+
+err2:
+ rtnl_af_unregister(&br_af_ops);
+err1:
+ return err;
}
void __exit br_netlink_fini(void)
{
+ rtnl_af_unregister(&br_af_ops);
rtnl_link_unregister(&br_link_ops);
rtnl_unregister_all(PF_BRIDGE);
}
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 6f662a4..00e07c8 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -178,6 +178,7 @@ struct net_bridge_port
#ifdef CONFIG_NET_POLL_CONTROLLER
struct netpoll *np;
#endif
+ u16 num_vlans;
struct list_head vlan_list;
struct net_bridge_vlan __rcu *untagged;
};
@@ -618,7 +619,7 @@ extern void br_netlink_fini(void);
extern void br_ifinfo_notify(int event, struct net_bridge_port *port);
extern int br_setlink(struct net_device *dev, struct nlmsghdr *nlmsg);
extern int br_getlink(struct sk_buff *skb, u32 pid, u32 seq,
- struct net_device *dev);
+ struct net_device *dev, u32 filter_mask);
#ifdef CONFIG_SYSFS
/* br_sysfs_if.c */
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 8352302..208965f 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2326,6 +2326,13 @@ static int rtnl_bridge_getlink(struct sk_buff *skb, struct netlink_callback *cb)
int idx = 0;
u32 portid = NETLINK_CB(cb->skb).portid;
u32 seq = cb->nlh->nlmsg_seq;
+ struct nlattr *extfilt;
+ u32 filter_mask = 0;
+
+ extfilt = nlmsg_find_attr(cb->nlh, sizeof(struct rtgenmsg),
+ IFLA_EXT_MASK);
+ if (extfilt)
+ filter_mask = nla_get_u32(extfilt);
rcu_read_lock();
for_each_netdev_rcu(net, dev) {
@@ -2335,14 +2342,15 @@ static int rtnl_bridge_getlink(struct sk_buff *skb, struct netlink_callback *cb)
if (master && master->netdev_ops->ndo_bridge_getlink) {
if (idx >= cb->args[0] &&
master->netdev_ops->ndo_bridge_getlink(
- skb, portid, seq, dev) < 0)
+ skb, portid, seq, dev, filter_mask) < 0)
break;
idx++;
}
if (ops->ndo_bridge_getlink) {
if (idx >= cb->args[0] &&
- ops->ndo_bridge_getlink(skb, portid, seq, dev) < 0)
+ ops->ndo_bridge_getlink(skb, portid, seq, dev,
+ filter_mask) < 0)
break;
idx++;
}
@@ -2383,14 +2391,14 @@ static int rtnl_bridge_notify(struct net_device *dev, u16 flags)
if ((!flags || (flags & BRIDGE_FLAGS_MASTER)) &&
master && master->netdev_ops->ndo_bridge_getlink) {
- err = master->netdev_ops->ndo_bridge_getlink(skb, 0, 0, dev);
+ err = master->netdev_ops->ndo_bridge_getlink(skb, 0, 0, dev, 0);
if (err < 0)
goto errout;
}
if ((flags & BRIDGE_FLAGS_SELF) &&
dev->netdev_ops->ndo_bridge_getlink) {
- err = dev->netdev_ops->ndo_bridge_getlink(skb, 0, 0, dev);
+ err = dev->netdev_ops->ndo_bridge_getlink(skb, 0, 0, dev, 0);
if (err < 0)
goto errout;
}
--
1.7.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH V2 12/12] bridge: Add vlan support for local fdb entries
2012-12-18 19:00 [PATCH V2 00/12] Add basic VLAN support to bridges Vlad Yasevich
` (10 preceding siblings ...)
2012-12-18 19:01 ` [PATCH V2 11/12] bridge: Dump vlan information from a bridge port Vlad Yasevich
@ 2012-12-18 19:01 ` Vlad Yasevich
2012-12-18 22:32 ` [PATCH V2 00/12] Add basic VLAN support to bridges Jiri Pirko
2012-12-19 8:10 ` Shmulik Ladkani
13 siblings, 0 replies; 35+ messages in thread
From: Vlad Yasevich @ 2012-12-18 19:01 UTC (permalink / raw)
To: netdev; +Cc: shemminger, davem, or.gerlitz, jhs, mst
When VLAN is added to the port, a local fdb entry for that port
(the entry with the mac address of the port) is added for that
VLAN. This way we can correctly determine if the traffic
is for the bridge itself. If the address of the port changes,
we try to change all the local fdb entries we have for that port.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
net/bridge/br_fdb.c | 66 ++++++++++++++++++++++++++++++++++-------------
net/bridge/br_if.c | 28 +++++++++++++++++++-
net/bridge/br_private.h | 4 ++-
3 files changed, 78 insertions(+), 20 deletions(-)
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index f1199e4..0666295 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -28,7 +28,7 @@
static struct kmem_cache *br_fdb_cache __read_mostly;
static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
- const unsigned char *addr);
+ const unsigned char *addr, u16 vid);
static void fdb_notify(struct net_bridge *br,
const struct net_bridge_fdb_entry *, int);
@@ -92,6 +92,7 @@ static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f)
void br_fdb_changeaddr(struct net_bridge_port *p, const unsigned char *newaddr)
{
struct net_bridge *br = p->br;
+ int no_vlan = list_empty(&p->vlan_list);
int i;
spin_lock_bh(&br->hash_lock);
@@ -106,10 +107,12 @@ void br_fdb_changeaddr(struct net_bridge_port *p, const unsigned char *newaddr)
if (f->dst == p && f->is_local) {
/* maybe another port has same hw addr? */
struct net_bridge_port *op;
+ u16 vid = f->vlan_id;
list_for_each_entry(op, &br->port_list, list) {
if (op != p &&
ether_addr_equal(op->dev->dev_addr,
- f->addr.addr)) {
+ f->addr.addr) &&
+ nbp_vlan_find(op, vid)) {
f->dst = op;
goto insert;
}
@@ -117,27 +120,55 @@ void br_fdb_changeaddr(struct net_bridge_port *p, const unsigned char *newaddr)
/* delete old one */
fdb_delete(br, f);
- goto insert;
+insert:
+ /* insert new address, may fail if invalid
+ * address or dup.
+ */
+ fdb_insert(br, p, newaddr, vid);
+
+ /* if this port has no vlan information
+ * configured, we can safely be done at
+ * this point.
+ */
+ if (no_vlan)
+ goto done;
}
}
}
- insert:
- /* insert new address, may fail if invalid address or dup. */
- fdb_insert(br, p, newaddr);
+done:
spin_unlock_bh(&br->hash_lock);
}
void br_fdb_change_mac_address(struct net_bridge *br, const u8 *newaddr)
{
struct net_bridge_fdb_entry *f;
+ struct net_bridge_vlan *vlan;
+ struct hlist_node *node;
+ int i;
/* If old entry was unassociated with any port, then delete it. */
f = __br_fdb_get(br, br->dev->dev_addr, 0);
if (f && f->is_local && !f->dst)
fdb_delete(br, f);
- fdb_insert(br, NULL, newaddr);
+ fdb_insert(br, NULL, newaddr, 0);
+
+ /* Now remove and add entries for every VLAN configured on the
+ * bridge.
+ */
+ for (i = 0; i < BR_VID_HASH_SIZE; i++) {
+ hlist_for_each_entry_rcu(vlan, node,
+ &br->vlan_hlist[i], hlist) {
+ if (vlan->vid == BR_INVALID_VID || vlan->vid == 0)
+ continue;
+
+ f = __br_fdb_get(br, br->dev->dev_addr, vlan->vid);
+ if (f && f->is_local && !f->dst)
+ fdb_delete(br, f);
+ fdb_insert(br, NULL, newaddr, vlan->vid);
+ }
+ }
}
void br_fdb_cleanup(unsigned long _data)
@@ -380,15 +411,15 @@ static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
}
static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
- const unsigned char *addr)
+ const unsigned char *addr, u16 vid)
{
- struct hlist_head *head = &br->hash[br_mac_hash(addr, 0)];
+ struct hlist_head *head = &br->hash[br_mac_hash(addr, vid)];
struct net_bridge_fdb_entry *fdb;
if (!is_valid_ether_addr(addr))
return -EINVAL;
- fdb = fdb_find(head, addr, 0);
+ fdb = fdb_find(head, addr, vid);
if (fdb) {
/* it is okay to have multiple ports with same
* address, just use the first one.
@@ -401,7 +432,7 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
fdb_delete(br, fdb);
}
- fdb = fdb_create(head, source, addr, 0);
+ fdb = fdb_create(head, source, addr, vid);
if (!fdb)
return -ENOMEM;
@@ -412,12 +443,12 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
/* Add entry for local address of interface */
int br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
- const unsigned char *addr)
+ const unsigned char *addr, u16 vid)
{
int ret;
spin_lock_bh(&br->hash_lock);
- ret = fdb_insert(br, source, addr);
+ ret = fdb_insert(br, source, addr, vid);
spin_unlock_bh(&br->hash_lock);
return ret;
}
@@ -710,10 +741,9 @@ out:
return err;
}
-static int fdb_delete_by_addr(struct net_bridge_port *p, const u8 *addr,
- u16 vlan)
+int fdb_delete_by_addr(struct net_bridge *br, const u8 *addr,
+ u16 vlan)
{
- struct net_bridge *br = p->br;
struct hlist_head *head = &br->hash[br_mac_hash(addr, vlan)];
struct net_bridge_fdb_entry *fdb;
@@ -721,7 +751,7 @@ static int fdb_delete_by_addr(struct net_bridge_port *p, const u8 *addr,
if (!fdb)
return -ENOENT;
- fdb_delete(p->br, fdb);
+ fdb_delete(br, fdb);
return 0;
}
@@ -731,7 +761,7 @@ static int __br_fdb_delete(struct net_bridge_port *p,
int err;
spin_lock_bh(&p->br->hash_lock);
- err = fdb_delete_by_addr(p, addr, vid);
+ err = fdb_delete_by_addr(p->br, addr, vid);
spin_unlock_bh(&p->br->hash_lock);
return err;
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 3b67ab7..041a3c8 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -178,6 +178,11 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
hlist_add_head_rcu(&vlan->hlist, &br->vlan_hlist[br_vlan_hash(vid)]);
+ if (br_fdb_insert(br, NULL, br->dev->dev_addr, vid)) {
+ br_err(br,
+ "failed insert local address bridge forwarding table\n");
+ }
+
untagged:
if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
br_vlan_add_untagged(br, vlan);
@@ -207,6 +212,12 @@ static void br_vlan_del(struct net_bridge *br, struct net_bridge_vlan *vlan,
if (!bitmap_empty(vlan->port_bitmap, PORT_BITMAP_LEN))
return;
+ if (vlan->vid) {
+ spin_lock_bh(&br->hash_lock);
+ fdb_delete_by_addr(br, br->dev->dev_addr, vlan->vid);
+ spin_unlock_bh(&br->hash_lock);
+ }
+
vlan->vid = BR_INVALID_VID;
/* If, for whatever reason, bridge still has a ref on this vlan
@@ -390,6 +401,12 @@ int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
goto del_vlan;
}
+ if (br_fdb_insert(p->br, p, dev->dev_addr, vid)) {
+ br_err(p->br,
+ "failed insert local address bridge forwarding table\n");
+ goto del_vlan;
+ }
+
return 0;
clean_up:
@@ -428,6 +445,15 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
vid, dev->name);
}
+ if (vid) {
+ /* If the VID !=0 remove fdb for this vid. VID 0 is special
+ * in that it's the default and is always there in the fdb.
+ */
+ spin_lock_bh(&p->br->hash_lock);
+ fdb_delete_by_addr(p->br, p->dev->dev_addr, vid);
+ spin_unlock_bh(&p->br->hash_lock);
+ }
+
pve->vid = BR_INVALID_VID;
vlan = pve->vlan;
@@ -773,7 +799,7 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
dev_set_mtu(br->dev, br_min_mtu(br));
- if (br_fdb_insert(br, p, dev->dev_addr))
+ if (br_fdb_insert(br, p, dev->dev_addr, 0))
netdev_err(dev, "failed insert local address bridge forwarding table\n");
kobject_uevent(&p->kobj, KOBJ_ADD);
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 00e07c8..495ec5a 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -407,11 +407,13 @@ extern int br_fdb_fillbuf(struct net_bridge *br, void *buf,
unsigned long count, unsigned long off);
extern int br_fdb_insert(struct net_bridge *br,
struct net_bridge_port *source,
- const unsigned char *addr);
+ const unsigned char *addr,
+ u16 vid);
extern void br_fdb_update(struct net_bridge *br,
struct net_bridge_port *source,
const unsigned char *addr,
u16 vid);
+extern int fdb_delete_by_addr(struct net_bridge *br, const u8 *addr, u16 vid);
extern int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
struct net_device *dev,
--
1.7.7.6
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH V2 01/12] bridge: Add vlan filtering infrastructure
2012-12-18 19:00 ` [PATCH V2 01/12] bridge: Add vlan filtering infrastructure Vlad Yasevich
@ 2012-12-18 21:13 ` Eric Dumazet
2012-12-18 21:26 ` Vlad Yasevich
0 siblings, 1 reply; 35+ messages in thread
From: Eric Dumazet @ 2012-12-18 21:13 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev, shemminger, davem, or.gerlitz, jhs, mst
On Tue, 2012-12-18 at 14:00 -0500, Vlad Yasevich wrote:
> +static void br_vlan_destroy(struct net_bridge_vlan *vlan)
> +{
> + if (!bitmap_empty(vlan->port_bitmap, PORT_BITMAP_LEN)) {
> + pr_err("Attempt to delete a VLAN %d from the bridge with "
> + "non-empty port bitmap (%p)\n", vlan->vid, vlan);
> + BUG();
> + }
> +
> + hlist_del_rcu(&vlan->hlist);
> + synchronize_net();
> + kfree_rcu(vlan, rcu);
> +}
Not clear why you both use synchronize_net() and kfree_rcu()
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH V2 01/12] bridge: Add vlan filtering infrastructure
2012-12-18 21:13 ` Eric Dumazet
@ 2012-12-18 21:26 ` Vlad Yasevich
0 siblings, 0 replies; 35+ messages in thread
From: Vlad Yasevich @ 2012-12-18 21:26 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, shemminger, davem, or.gerlitz, jhs, mst
On 12/18/2012 04:13 PM, Eric Dumazet wrote:
> On Tue, 2012-12-18 at 14:00 -0500, Vlad Yasevich wrote:
>
>> +static void br_vlan_destroy(struct net_bridge_vlan *vlan)
>> +{
>> + if (!bitmap_empty(vlan->port_bitmap, PORT_BITMAP_LEN)) {
>> + pr_err("Attempt to delete a VLAN %d from the bridge with "
>> + "non-empty port bitmap (%p)\n", vlan->vid, vlan);
>> + BUG();
>> + }
>> +
>> + hlist_del_rcu(&vlan->hlist);
>> + synchronize_net();
>> + kfree_rcu(vlan, rcu);
>> +}
>
> Not clear why you both use synchronize_net() and kfree_rcu()
>
I think this was a left-over from old code. I think this was
originally here to mimic del_nbp behavior, but I don't think
it's really needed as kfree_rcu should wait until all current
rcu section have completed.
-vlad
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH V2 00/12] Add basic VLAN support to bridges
2012-12-18 19:00 [PATCH V2 00/12] Add basic VLAN support to bridges Vlad Yasevich
` (11 preceding siblings ...)
2012-12-18 19:01 ` [PATCH V2 12/12] bridge: Add vlan support for local fdb entries Vlad Yasevich
@ 2012-12-18 22:32 ` Jiri Pirko
2012-12-18 22:46 ` Vlad Yasevich
2012-12-19 8:10 ` Shmulik Ladkani
13 siblings, 1 reply; 35+ messages in thread
From: Jiri Pirko @ 2012-12-18 22:32 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev, shemminger, davem, or.gerlitz, jhs, mst
I see that this patchset replicates a lot of code which is already
present in net/8021q/ or include/linux/if_vlan.h. I think it would
be nice to move this code into some "common" place, wouldn't it?
Jiri
Tue, Dec 18, 2012 at 08:00:51PM CET, vyasevic@redhat.com wrote:
>This series of patches provides an ability to add VLANs to the bridge
>ports. This is similar to what can be found in most switches. The bridge
>port may have any number of VLANs added to it including vlan 0 priority tagged
>traffic. When vlans are added to the port, only traffic tagged with particular
>vlan will forwarded over this port. Additionally, vlan ids are added to FDB
>entries and become part of the lookup. This way we correctly identify the FDB
>entry.
>
>A single vlan may also be designated as untagged. Any untagged traffic
>recieved by the port will be assigned to this vlan. Any traffic exiting
>the port with a VID matching the untagged vlan will exit untagged (the
>bridge will strip the vlan header). This is similar to "Native Vlan" support
>available in most switches.
>
>The default behavior ofthe bridge is unchanged if no vlans have been
>configured.
>
>Changes since v1:
> - Fixed some forwarding bugs.
> - Add vlan to local fdb entries. New local entries are created per vlan
> to facilite correct forwarding to bridge interface.
> - Allow configuration of vlans directly on the bridge master device
> in addition to ports.
>
>Changes since rfc v2:
> - Per-port vlan bitmap is gone and is replaced with a vlan list.
> - Added bridge vlan list, which is referenced by each port. Entries in
> the birdge vlan list have port bitmap that shows which port are parts
> of which vlan.
> - Netlink API changes.
> - Dropped sysfs support for now. If people think this is really usefull,
> can add it back.
> - Support for native/untagged vlans.
>
>Changes since rfc v1:
> - Comments addressed regarding formatting and RCU usage
> - iocts have been removed and changed over the netlink interface.
> - Added support of user added ndb entries.
> - changed sysfs interface to export a bitmap. Also added a write interface.
> I am not sure how much I like it, but it made my testing easier/faster. I
> might change the write interface to take text instead of binary.
>
>
>Vlad Yasevich (12):
> bridge: Add vlan filtering infrastructure
> bridge: Validate that vlan is permitted on ingress
> bridge: Verify that a vlan is allowed to egress on give port
> bridge: Cache vlan in the cb for faster egress lookup.
> bridge: Add vlan to unicast fdb entries
> bridge: Add vlan id to multicast groups
> bridge: Add netlink interface to configure vlans on bridge ports
> bridge: Add vlan support to static neighbors
> bridge: Add the ability to configure untagged vlans
> bridge: Implement untagged vlan handling
> bridge: Dump vlan information from a bridge port
> bridge: Add vlan support for local fdb entries
>
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 +-
> drivers/net/macvlan.c | 2 +-
> drivers/net/vxlan.c | 3 +-
> include/linux/netdevice.h | 4 +-
> include/uapi/linux/if_bridge.h | 23 ++-
> include/uapi/linux/neighbour.h | 1 +
> include/uapi/linux/rtnetlink.h | 1 +
> net/bridge/br_device.c | 34 ++-
> net/bridge/br_fdb.c | 253 ++++++++++++---
> net/bridge/br_forward.c | 160 ++++++++++
> net/bridge/br_if.c | 404 ++++++++++++++++++++++++-
> net/bridge/br_input.c | 65 ++++-
> net/bridge/br_multicast.c | 71 +++--
> net/bridge/br_netlink.c | 178 ++++++++++--
> net/bridge/br_private.h | 71 ++++-
> net/core/rtnetlink.c | 40 ++-
> 16 files changed, 1190 insertions(+), 125 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
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH V2 00/12] Add basic VLAN support to bridges
2012-12-18 22:32 ` [PATCH V2 00/12] Add basic VLAN support to bridges Jiri Pirko
@ 2012-12-18 22:46 ` Vlad Yasevich
2012-12-19 8:27 ` Jiri Pirko
0 siblings, 1 reply; 35+ messages in thread
From: Vlad Yasevich @ 2012-12-18 22:46 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, shemminger, davem, or.gerlitz, jhs, mst
On 12/18/2012 05:32 PM, Jiri Pirko wrote:
>
>
> I see that this patchset replicates a lot of code which is already
> present in net/8021q/ or include/linux/if_vlan.h. I think it would
> be nice to move this code into some "common" place, wouldn't it?
>
The only replication that I am aware of is in br_vlan_untag(). I
thought about pulling that piece out, but I think there is a reason
why it's not available when 801q support isn't turned on. I noted that
openvswitch implemented its own vlan header manipulation functions as well.
What else are you seeing that's duplicate?
-vlad
> Jiri
>
> Tue, Dec 18, 2012 at 08:00:51PM CET, vyasevic@redhat.com wrote:
>> This series of patches provides an ability to add VLANs to the bridge
>> ports. This is similar to what can be found in most switches. The bridge
>> port may have any number of VLANs added to it including vlan 0 priority tagged
>> traffic. When vlans are added to the port, only traffic tagged with particular
>> vlan will forwarded over this port. Additionally, vlan ids are added to FDB
>> entries and become part of the lookup. This way we correctly identify the FDB
>> entry.
>>
>> A single vlan may also be designated as untagged. Any untagged traffic
>> recieved by the port will be assigned to this vlan. Any traffic exiting
>> the port with a VID matching the untagged vlan will exit untagged (the
>> bridge will strip the vlan header). This is similar to "Native Vlan" support
>> available in most switches.
>>
>> The default behavior ofthe bridge is unchanged if no vlans have been
>> configured.
>>
>> Changes since v1:
>> - Fixed some forwarding bugs.
>> - Add vlan to local fdb entries. New local entries are created per vlan
>> to facilite correct forwarding to bridge interface.
>> - Allow configuration of vlans directly on the bridge master device
>> in addition to ports.
>>
>> Changes since rfc v2:
>> - Per-port vlan bitmap is gone and is replaced with a vlan list.
>> - Added bridge vlan list, which is referenced by each port. Entries in
>> the birdge vlan list have port bitmap that shows which port are parts
>> of which vlan.
>> - Netlink API changes.
>> - Dropped sysfs support for now. If people think this is really usefull,
>> can add it back.
>> - Support for native/untagged vlans.
>>
>> Changes since rfc v1:
>> - Comments addressed regarding formatting and RCU usage
>> - iocts have been removed and changed over the netlink interface.
>> - Added support of user added ndb entries.
>> - changed sysfs interface to export a bitmap. Also added a write interface.
>> I am not sure how much I like it, but it made my testing easier/faster. I
>> might change the write interface to take text instead of binary.
>>
>>
>> Vlad Yasevich (12):
>> bridge: Add vlan filtering infrastructure
>> bridge: Validate that vlan is permitted on ingress
>> bridge: Verify that a vlan is allowed to egress on give port
>> bridge: Cache vlan in the cb for faster egress lookup.
>> bridge: Add vlan to unicast fdb entries
>> bridge: Add vlan id to multicast groups
>> bridge: Add netlink interface to configure vlans on bridge ports
>> bridge: Add vlan support to static neighbors
>> bridge: Add the ability to configure untagged vlans
>> bridge: Implement untagged vlan handling
>> bridge: Dump vlan information from a bridge port
>> bridge: Add vlan support for local fdb entries
>>
>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 +-
>> drivers/net/macvlan.c | 2 +-
>> drivers/net/vxlan.c | 3 +-
>> include/linux/netdevice.h | 4 +-
>> include/uapi/linux/if_bridge.h | 23 ++-
>> include/uapi/linux/neighbour.h | 1 +
>> include/uapi/linux/rtnetlink.h | 1 +
>> net/bridge/br_device.c | 34 ++-
>> net/bridge/br_fdb.c | 253 ++++++++++++---
>> net/bridge/br_forward.c | 160 ++++++++++
>> net/bridge/br_if.c | 404 ++++++++++++++++++++++++-
>> net/bridge/br_input.c | 65 ++++-
>> net/bridge/br_multicast.c | 71 +++--
>> net/bridge/br_netlink.c | 178 ++++++++++--
>> net/bridge/br_private.h | 71 ++++-
>> net/core/rtnetlink.c | 40 ++-
>> 16 files changed, 1190 insertions(+), 125 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
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH V2 09/12] bridge: Add the ability to configure untagged vlans
2012-12-18 19:01 ` [PATCH V2 09/12] bridge: Add the ability to configure untagged vlans Vlad Yasevich
@ 2012-12-18 23:01 ` Michael S. Tsirkin
2012-12-18 23:03 ` Vlad Yasevich
2012-12-18 23:04 ` Michael S. Tsirkin
1 sibling, 1 reply; 35+ messages in thread
From: Michael S. Tsirkin @ 2012-12-18 23:01 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev, shemminger, davem, or.gerlitz, jhs
On Tue, Dec 18, 2012 at 02:01:00PM -0500, Vlad Yasevich wrote:
> A user may designate a certain vlan as untagged. This means that
> any ingress frame is assigned to this vlan and any forwarding decisions
> are made with this vlan in mind. On egress, any frames tagged/labeled
> with untagged vlan have the vlan tag removed and are send as regular
> ethernet frames.
>
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> ---
> include/uapi/linux/if_bridge.h | 3 +
> net/bridge/br_if.c | 146 +++++++++++++++++++++++++++++++++++++---
> net/bridge/br_netlink.c | 6 +-
> net/bridge/br_private.h | 2 +
> 4 files changed, 144 insertions(+), 13 deletions(-)
>
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index d0b4f5c..988d858 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -127,6 +127,9 @@ enum {
> BR_VLAN_DEL,
> };
>
> +#define BRIDGE_VLAN_INFO_MASTER 1
> +#define BRIDGE_VLAN_INFO_UNTAGGED 2
> +
> struct bridge_vlan_info {
> u16 op_code;
> u16 flags;
> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> index 57bbb35..14563fb 100644
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -108,6 +108,34 @@ static void br_vlan_put(struct net_bridge_vlan *vlan)
> br_vlan_destroy(vlan);
> }
>
> +/* Must be protected by RTNL */
> +static void br_vlan_add_untagged(struct net_bridge *br,
> + struct net_bridge_vlan *vlan)
> +{
> + ASSERT_RTNL();
> + if (br->untagged == vlan)
> + return;
> + else if (br->untagged) {
> + /* Untagged vlan is already set on the master,
> + * so drop the ref since we'll be replacing it.
> + */
> + br_vlan_put(br->untagged);
> + }
> + br_vlan_hold(vlan);
> + rcu_assign_pointer(br->untagged, vlan);
Is there a reason for rcu here but not else where? If all users are under
rtnl you can just assign in a simple way.
If not then rcu_dereference_protected would be more appropriate.
> +}
> +
> +/* Must be protected by RTNL */
> +static void br_vlan_del_untagged(struct net_bridge *br,
> + struct net_bridge_vlan *vlan)
> +{
> + ASSERT_RTNL();
> + if (br->untagged == vlan) {
> + br_vlan_put(vlan);
> + rcu_assign_pointer(br->untagged, NULL);
> + }
> +}
> +
> struct net_bridge_vlan *br_vlan_find(struct net_bridge *br, u16 vid)
> {
> struct net_bridge_vlan *vlan;
> @@ -132,7 +160,7 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
>
> vlan = br_vlan_find(br, vid);
> if (vlan)
> - return vlan;
> + goto untagged;
>
> vlan = kzalloc(sizeof(struct net_bridge_vlan), GFP_KERNEL);
> if (!vlan)
> @@ -141,7 +169,7 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
> vlan->vid = vid;
> atomic_set(&vlan->refcnt, 1);
>
> - if (flags & BRIDGE_FLAGS_SELF) {
> + if (flags & BRIDGE_VLAN_INFO_MASTER) {
> /* Set bit 0 that is associated with the bridge master
> * device. Port numbers start with 1.
> */
> @@ -149,15 +177,24 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
> }
>
> hlist_add_head_rcu(&vlan->hlist, &br->vlan_hlist[br_vlan_hash(vid)]);
> +
> +untagged:
> + if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
> + br_vlan_add_untagged(br, vlan);
> +
> return vlan;
> }
>
> /* Must be protected by RTNL */
> -static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
> +static void br_vlan_del(struct net_bridge *br, struct net_bridge_vlan *vlan,
> + u16 flags)
> {
> ASSERT_RTNL();
>
> - if (flags & BRIDGE_FLAGS_SELF) {
> + if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
> + br_vlan_del_untagged(br, vlan);
> +
> + if (flags & BRIDGE_VLAN_INFO_MASTER) {
> /* Clear bit 0 that is associated with the bridge master
> * device.
> */
> @@ -172,6 +209,14 @@ static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
>
> vlan->vid = BR_INVALID_VID;
>
> + /* If, for whatever reason, bridge still has a ref on this vlan
> + * through the @untagged pointer, drop that ref and clear untagged.
> + */
> + if (br->untagged == vlan) {
> + br_vlan_put(vlan);
> + rcu_assign_pointer(br->untagged, NULL);
> + }
> +
> /* Drop the self-ref to trigger descrution. */
> br_vlan_put(vlan);
> }
> @@ -187,7 +232,7 @@ int br_vlan_delete(struct net_bridge *br, u16 vid, u16 flags)
> if (!vlan)
> return -ENOENT;
>
> - br_vlan_del(vlan, flags);
> + br_vlan_del(br, vlan, flags);
> return 0;
> }
>
> @@ -204,7 +249,9 @@ static void br_vlan_flush(struct net_bridge *br)
> for (i = 0; i < BR_VID_HASH_SIZE; i++) {
> hlist_for_each_entry_safe(vlan, node, tmp,
> &br->vlan_hlist[i], hlist) {
> - br_vlan_del(vlan, BRIDGE_FLAGS_SELF);
> + br_vlan_del(br, vlan,
> + (BRIDGE_VLAN_INFO_MASTER |
> + BRIDGE_VLAN_INFO_UNTAGGED));
> }
> }
> }
> @@ -224,10 +271,70 @@ struct net_port_vlan *nbp_vlan_find(const struct net_bridge_port *p, u16 vid)
> return NULL;
> }
>
> +static int nbp_vlan_add_untagged(struct net_bridge_port *p,
> + struct net_bridge_vlan *vlan,
> + u16 flags)
> +{
> + struct net_device *dev = p->dev;
> +
> + if (p->untagged) {
> + /* Port already has untagged vlan set. Drop the ref
> + * to the old one since we'll be replace it.
> + */
> + br_vlan_put(p->untagged);
> + } else {
> + int err;
> +
> + /* Add vid 0 to filter if filter is available. */
> + if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
> + dev->netdev_ops->ndo_vlan_rx_add_vid &&
> + dev->netdev_ops->ndo_vlan_rx_kill_vid) {
> + err = dev->netdev_ops->ndo_vlan_rx_add_vid(dev, 0);
> + if (err)
> + return err;
> + }
> + }
> +
> + /* This VLAN is handled as untagged/native. Save an
> + * additional ref.
> + */
> + br_vlan_hold(vlan);
> + rcu_assign_pointer(p->untagged, vlan);
> +
> + return 0;
> +}
> +
> +static void nbp_vlan_delete_untagged(struct net_bridge_port *p,
> + struct net_bridge_vlan *vlan)
> +{
> + if (p->untagged != vlan)
> + return;
> +
> + /* Remove VLAN from the device filter if it is supported. */
> + if ((p->dev->features & NETIF_F_HW_VLAN_FILTER) &&
> + p->dev->netdev_ops->ndo_vlan_rx_kill_vid) {
> + int err;
> +
> + err = p->dev->netdev_ops->ndo_vlan_rx_kill_vid(p->dev, 0);
> + if (err) {
> + pr_warn("failed to kill vid %d for device %s\n",
> + vlan->vid, p->dev->name);
> + }
> + }
> +
> + /* If this VLAN is currently functioning as untagged, clear it.
> + * It's safe to drop the refcount, since the vlan is still held
> + * by the port.
> + */
> + br_vlan_put(vlan);
> + rcu_assign_pointer(p->untagged, NULL);
> +
> +}
> +
> /* Must be protected by RTNL */
> int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
> {
> - struct net_port_vlan *pve;
> + struct net_port_vlan *pve = NULL;
> struct net_bridge_vlan *vlan;
> struct net_device *dev = p->dev;
> int err;
> @@ -275,11 +382,21 @@ int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
> set_bit(p->port_no, vlan->port_bitmap);
>
> list_add_tail_rcu(&pve->list, &p->vlan_list);
> +
> + if (flags & BRIDGE_VLAN_INFO_UNTAGGED) {
> + err = nbp_vlan_add_untagged(p, vlan, flags);
> + if (err)
> + goto del_vlan;
> + }
> +
> return 0;
>
> clean_up:
> kfree(pve);
> - br_vlan_del(vlan, flags);
> + br_vlan_del(p->br, vlan, flags);
> + return err;
> +del_vlan:
> + nbp_vlan_delete(p, vid, flags);
> return err;
> }
>
> @@ -296,6 +413,9 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
> if (!pve)
> return -ENOENT;
>
> + if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
> + nbp_vlan_delete_untagged(p, pve->vlan);
> +
> /* Remove VLAN from the device filter if it is supported. */
> if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
> dev->netdev_ops->ndo_vlan_rx_kill_vid) {
> @@ -306,6 +426,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
> pr_warn("failed to kill vid %d for device %s\n",
> vid, dev->name);
> }
> +
> pve->vid = BR_INVALID_VID;
>
> vlan = pve->vlan;
> @@ -316,7 +437,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
> list_del_rcu(&pve->list);
> kfree_rcu(pve, rcu);
>
> - br_vlan_del(vlan, flags);
> + br_vlan_del(p->br, vlan, flags);
>
> return 0;
> }
> @@ -328,8 +449,11 @@ static void nbp_vlan_flush(struct net_bridge_port *p)
>
> ASSERT_RTNL();
>
> - list_for_each_entry_safe(pve, tmp, &p->vlan_list, list)
> - nbp_vlan_delete(p, pve->vid, BRIDGE_FLAGS_SELF);
> + list_for_each_entry_safe(pve, tmp, &p->vlan_list, list) {
> + nbp_vlan_delete(p, pve->vid,
> + (BRIDGE_VLAN_INFO_MASTER |
> + BRIDGE_VLAN_INFO_UNTAGGED));
> + }
> }
>
> static void release_nbp(struct kobject *kobj)
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 9cf2879..1b302ce 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -199,7 +199,8 @@ static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
> if (p)
> err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
> else {
> - u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
> + u16 flags = vinfo->flags |
> + BRIDGE_VLAN_INFO_MASTER;
> if (!br_vlan_add(br, vinfo->vid, flags))
> err = -ENOMEM;
> }
> @@ -210,7 +211,8 @@ static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
> err = nbp_vlan_delete(p, vinfo->vid,
> vinfo->flags);
> else {
> - u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
> + u16 flags = vinfo->flags |
> + BRIDGE_VLAN_INFO_MASTER;
> err = br_vlan_delete(br, vinfo->vid, flags);
> }
> break;
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index cc75212..9328463 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -179,6 +179,7 @@ struct net_bridge_port
> struct netpoll *np;
> #endif
> struct list_head vlan_list;
> + struct net_bridge_vlan __rcu *untagged;
> };
>
> #define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT)
> @@ -298,6 +299,7 @@ struct net_bridge
> struct timer_list gc_timer;
> struct kobject *ifobj;
> struct hlist_head vlan_hlist[BR_VID_HASH_SIZE];
> + struct net_bridge_vlan __rcu *untagged;
> };
>
> struct br_input_skb_cb {
> --
> 1.7.7.6
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH V2 09/12] bridge: Add the ability to configure untagged vlans
2012-12-18 23:01 ` Michael S. Tsirkin
@ 2012-12-18 23:03 ` Vlad Yasevich
2012-12-18 23:10 ` Michael S. Tsirkin
0 siblings, 1 reply; 35+ messages in thread
From: Vlad Yasevich @ 2012-12-18 23:03 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, shemminger, davem, or.gerlitz, jhs
On 12/18/2012 06:01 PM, Michael S. Tsirkin wrote:
> On Tue, Dec 18, 2012 at 02:01:00PM -0500, Vlad Yasevich wrote:
>> A user may designate a certain vlan as untagged. This means that
>> any ingress frame is assigned to this vlan and any forwarding decisions
>> are made with this vlan in mind. On egress, any frames tagged/labeled
>> with untagged vlan have the vlan tag removed and are send as regular
>> ethernet frames.
>>
>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>> ---
>> include/uapi/linux/if_bridge.h | 3 +
>> net/bridge/br_if.c | 146 +++++++++++++++++++++++++++++++++++++---
>> net/bridge/br_netlink.c | 6 +-
>> net/bridge/br_private.h | 2 +
>> 4 files changed, 144 insertions(+), 13 deletions(-)
>>
>> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
>> index d0b4f5c..988d858 100644
>> --- a/include/uapi/linux/if_bridge.h
>> +++ b/include/uapi/linux/if_bridge.h
>> @@ -127,6 +127,9 @@ enum {
>> BR_VLAN_DEL,
>> };
>>
>> +#define BRIDGE_VLAN_INFO_MASTER 1
>> +#define BRIDGE_VLAN_INFO_UNTAGGED 2
>> +
>> struct bridge_vlan_info {
>> u16 op_code;
>> u16 flags;
>> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
>> index 57bbb35..14563fb 100644
>> --- a/net/bridge/br_if.c
>> +++ b/net/bridge/br_if.c
>> @@ -108,6 +108,34 @@ static void br_vlan_put(struct net_bridge_vlan *vlan)
>> br_vlan_destroy(vlan);
>> }
>>
>> +/* Must be protected by RTNL */
>> +static void br_vlan_add_untagged(struct net_bridge *br,
>> + struct net_bridge_vlan *vlan)
>> +{
>> + ASSERT_RTNL();
>> + if (br->untagged == vlan)
>> + return;
>> + else if (br->untagged) {
>> + /* Untagged vlan is already set on the master,
>> + * so drop the ref since we'll be replacing it.
>> + */
>> + br_vlan_put(br->untagged);
>> + }
>> + br_vlan_hold(vlan);
>> + rcu_assign_pointer(br->untagged, vlan);
>
> Is there a reason for rcu here but not else where? If all users are under
> rtnl you can just assign in a simple way.
> If not then rcu_dereference_protected would be more appropriate.
Everywhere that the pointer changes rcu_assign_pointer is used.
Now, if we hold an RTNL, we can technically read the pointer with rcu
since it's guaranteed not to change since it only changes under RTNL.
I'll check that this is consistent.
If I access the pointer without rtnl, it's always inside rcu critical
section and with rcu_dereference().
I thought those were the basic rules of rcu. Did that change?
-vlad
>
>> +}
>> +
>> +/* Must be protected by RTNL */
>> +static void br_vlan_del_untagged(struct net_bridge *br,
>> + struct net_bridge_vlan *vlan)
>> +{
>> + ASSERT_RTNL();
>> + if (br->untagged == vlan) {
>> + br_vlan_put(vlan);
>> + rcu_assign_pointer(br->untagged, NULL);
>> + }
>> +}
>> +
>> struct net_bridge_vlan *br_vlan_find(struct net_bridge *br, u16 vid)
>> {
>> struct net_bridge_vlan *vlan;
>> @@ -132,7 +160,7 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
>>
>> vlan = br_vlan_find(br, vid);
>> if (vlan)
>> - return vlan;
>> + goto untagged;
>>
>> vlan = kzalloc(sizeof(struct net_bridge_vlan), GFP_KERNEL);
>> if (!vlan)
>> @@ -141,7 +169,7 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
>> vlan->vid = vid;
>> atomic_set(&vlan->refcnt, 1);
>>
>> - if (flags & BRIDGE_FLAGS_SELF) {
>> + if (flags & BRIDGE_VLAN_INFO_MASTER) {
>> /* Set bit 0 that is associated with the bridge master
>> * device. Port numbers start with 1.
>> */
>> @@ -149,15 +177,24 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
>> }
>>
>> hlist_add_head_rcu(&vlan->hlist, &br->vlan_hlist[br_vlan_hash(vid)]);
>> +
>> +untagged:
>> + if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
>> + br_vlan_add_untagged(br, vlan);
>> +
>> return vlan;
>> }
>>
>> /* Must be protected by RTNL */
>> -static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
>> +static void br_vlan_del(struct net_bridge *br, struct net_bridge_vlan *vlan,
>> + u16 flags)
>> {
>> ASSERT_RTNL();
>>
>> - if (flags & BRIDGE_FLAGS_SELF) {
>> + if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
>> + br_vlan_del_untagged(br, vlan);
>> +
>> + if (flags & BRIDGE_VLAN_INFO_MASTER) {
>> /* Clear bit 0 that is associated with the bridge master
>> * device.
>> */
>> @@ -172,6 +209,14 @@ static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
>>
>> vlan->vid = BR_INVALID_VID;
>>
>> + /* If, for whatever reason, bridge still has a ref on this vlan
>> + * through the @untagged pointer, drop that ref and clear untagged.
>> + */
>> + if (br->untagged == vlan) {
>> + br_vlan_put(vlan);
>> + rcu_assign_pointer(br->untagged, NULL);
>> + }
>> +
>> /* Drop the self-ref to trigger descrution. */
>> br_vlan_put(vlan);
>> }
>> @@ -187,7 +232,7 @@ int br_vlan_delete(struct net_bridge *br, u16 vid, u16 flags)
>> if (!vlan)
>> return -ENOENT;
>>
>> - br_vlan_del(vlan, flags);
>> + br_vlan_del(br, vlan, flags);
>> return 0;
>> }
>>
>> @@ -204,7 +249,9 @@ static void br_vlan_flush(struct net_bridge *br)
>> for (i = 0; i < BR_VID_HASH_SIZE; i++) {
>> hlist_for_each_entry_safe(vlan, node, tmp,
>> &br->vlan_hlist[i], hlist) {
>> - br_vlan_del(vlan, BRIDGE_FLAGS_SELF);
>> + br_vlan_del(br, vlan,
>> + (BRIDGE_VLAN_INFO_MASTER |
>> + BRIDGE_VLAN_INFO_UNTAGGED));
>> }
>> }
>> }
>> @@ -224,10 +271,70 @@ struct net_port_vlan *nbp_vlan_find(const struct net_bridge_port *p, u16 vid)
>> return NULL;
>> }
>>
>> +static int nbp_vlan_add_untagged(struct net_bridge_port *p,
>> + struct net_bridge_vlan *vlan,
>> + u16 flags)
>> +{
>> + struct net_device *dev = p->dev;
>> +
>> + if (p->untagged) {
>> + /* Port already has untagged vlan set. Drop the ref
>> + * to the old one since we'll be replace it.
>> + */
>> + br_vlan_put(p->untagged);
>> + } else {
>> + int err;
>> +
>> + /* Add vid 0 to filter if filter is available. */
>> + if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
>> + dev->netdev_ops->ndo_vlan_rx_add_vid &&
>> + dev->netdev_ops->ndo_vlan_rx_kill_vid) {
>> + err = dev->netdev_ops->ndo_vlan_rx_add_vid(dev, 0);
>> + if (err)
>> + return err;
>> + }
>> + }
>> +
>> + /* This VLAN is handled as untagged/native. Save an
>> + * additional ref.
>> + */
>> + br_vlan_hold(vlan);
>> + rcu_assign_pointer(p->untagged, vlan);
>> +
>> + return 0;
>> +}
>> +
>> +static void nbp_vlan_delete_untagged(struct net_bridge_port *p,
>> + struct net_bridge_vlan *vlan)
>> +{
>> + if (p->untagged != vlan)
>> + return;
>> +
>> + /* Remove VLAN from the device filter if it is supported. */
>> + if ((p->dev->features & NETIF_F_HW_VLAN_FILTER) &&
>> + p->dev->netdev_ops->ndo_vlan_rx_kill_vid) {
>> + int err;
>> +
>> + err = p->dev->netdev_ops->ndo_vlan_rx_kill_vid(p->dev, 0);
>> + if (err) {
>> + pr_warn("failed to kill vid %d for device %s\n",
>> + vlan->vid, p->dev->name);
>> + }
>> + }
>> +
>> + /* If this VLAN is currently functioning as untagged, clear it.
>> + * It's safe to drop the refcount, since the vlan is still held
>> + * by the port.
>> + */
>> + br_vlan_put(vlan);
>> + rcu_assign_pointer(p->untagged, NULL);
>> +
>> +}
>> +
>> /* Must be protected by RTNL */
>> int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
>> {
>> - struct net_port_vlan *pve;
>> + struct net_port_vlan *pve = NULL;
>> struct net_bridge_vlan *vlan;
>> struct net_device *dev = p->dev;
>> int err;
>> @@ -275,11 +382,21 @@ int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
>> set_bit(p->port_no, vlan->port_bitmap);
>>
>> list_add_tail_rcu(&pve->list, &p->vlan_list);
>> +
>> + if (flags & BRIDGE_VLAN_INFO_UNTAGGED) {
>> + err = nbp_vlan_add_untagged(p, vlan, flags);
>> + if (err)
>> + goto del_vlan;
>> + }
>> +
>> return 0;
>>
>> clean_up:
>> kfree(pve);
>> - br_vlan_del(vlan, flags);
>> + br_vlan_del(p->br, vlan, flags);
>> + return err;
>> +del_vlan:
>> + nbp_vlan_delete(p, vid, flags);
>> return err;
>> }
>>
>> @@ -296,6 +413,9 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
>> if (!pve)
>> return -ENOENT;
>>
>> + if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
>> + nbp_vlan_delete_untagged(p, pve->vlan);
>> +
>> /* Remove VLAN from the device filter if it is supported. */
>> if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
>> dev->netdev_ops->ndo_vlan_rx_kill_vid) {
>> @@ -306,6 +426,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
>> pr_warn("failed to kill vid %d for device %s\n",
>> vid, dev->name);
>> }
>> +
>> pve->vid = BR_INVALID_VID;
>>
>> vlan = pve->vlan;
>> @@ -316,7 +437,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
>> list_del_rcu(&pve->list);
>> kfree_rcu(pve, rcu);
>>
>> - br_vlan_del(vlan, flags);
>> + br_vlan_del(p->br, vlan, flags);
>>
>> return 0;
>> }
>> @@ -328,8 +449,11 @@ static void nbp_vlan_flush(struct net_bridge_port *p)
>>
>> ASSERT_RTNL();
>>
>> - list_for_each_entry_safe(pve, tmp, &p->vlan_list, list)
>> - nbp_vlan_delete(p, pve->vid, BRIDGE_FLAGS_SELF);
>> + list_for_each_entry_safe(pve, tmp, &p->vlan_list, list) {
>> + nbp_vlan_delete(p, pve->vid,
>> + (BRIDGE_VLAN_INFO_MASTER |
>> + BRIDGE_VLAN_INFO_UNTAGGED));
>> + }
>> }
>>
>> static void release_nbp(struct kobject *kobj)
>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>> index 9cf2879..1b302ce 100644
>> --- a/net/bridge/br_netlink.c
>> +++ b/net/bridge/br_netlink.c
>> @@ -199,7 +199,8 @@ static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
>> if (p)
>> err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
>> else {
>> - u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
>> + u16 flags = vinfo->flags |
>> + BRIDGE_VLAN_INFO_MASTER;
>> if (!br_vlan_add(br, vinfo->vid, flags))
>> err = -ENOMEM;
>> }
>> @@ -210,7 +211,8 @@ static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
>> err = nbp_vlan_delete(p, vinfo->vid,
>> vinfo->flags);
>> else {
>> - u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
>> + u16 flags = vinfo->flags |
>> + BRIDGE_VLAN_INFO_MASTER;
>> err = br_vlan_delete(br, vinfo->vid, flags);
>> }
>> break;
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index cc75212..9328463 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>> @@ -179,6 +179,7 @@ struct net_bridge_port
>> struct netpoll *np;
>> #endif
>> struct list_head vlan_list;
>> + struct net_bridge_vlan __rcu *untagged;
>> };
>>
>> #define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT)
>> @@ -298,6 +299,7 @@ struct net_bridge
>> struct timer_list gc_timer;
>> struct kobject *ifobj;
>> struct hlist_head vlan_hlist[BR_VID_HASH_SIZE];
>> + struct net_bridge_vlan __rcu *untagged;
>> };
>>
>> struct br_input_skb_cb {
>> --
>> 1.7.7.6
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH V2 09/12] bridge: Add the ability to configure untagged vlans
2012-12-18 19:01 ` [PATCH V2 09/12] bridge: Add the ability to configure untagged vlans Vlad Yasevich
2012-12-18 23:01 ` Michael S. Tsirkin
@ 2012-12-18 23:04 ` Michael S. Tsirkin
2012-12-19 1:06 ` Vlad Yasevich
1 sibling, 1 reply; 35+ messages in thread
From: Michael S. Tsirkin @ 2012-12-18 23:04 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev, shemminger, davem, or.gerlitz, jhs
On Tue, Dec 18, 2012 at 02:01:00PM -0500, Vlad Yasevich wrote:
> A user may designate a certain vlan as untagged. This means that
> any ingress frame is assigned to this vlan and any forwarding decisions
> are made with this vlan in mind. On egress, any frames tagged/labeled
> with untagged vlan have the vlan tag removed and are send as regular
> ethernet frames.
>
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> ---
> include/uapi/linux/if_bridge.h | 3 +
> net/bridge/br_if.c | 146 +++++++++++++++++++++++++++++++++++++---
> net/bridge/br_netlink.c | 6 +-
> net/bridge/br_private.h | 2 +
> 4 files changed, 144 insertions(+), 13 deletions(-)
>
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index d0b4f5c..988d858 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -127,6 +127,9 @@ enum {
> BR_VLAN_DEL,
> };
>
> +#define BRIDGE_VLAN_INFO_MASTER 1
> +#define BRIDGE_VLAN_INFO_UNTAGGED 2
> +
> struct bridge_vlan_info {
> u16 op_code;
> u16 flags;
> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> index 57bbb35..14563fb 100644
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -108,6 +108,34 @@ static void br_vlan_put(struct net_bridge_vlan *vlan)
> br_vlan_destroy(vlan);
> }
>
> +/* Must be protected by RTNL */
> +static void br_vlan_add_untagged(struct net_bridge *br,
> + struct net_bridge_vlan *vlan)
> +{
> + ASSERT_RTNL();
> + if (br->untagged == vlan)
> + return;
> + else if (br->untagged) {
> + /* Untagged vlan is already set on the master,
> + * so drop the ref since we'll be replacing it.
> + */
> + br_vlan_put(br->untagged);
> + }
> + br_vlan_hold(vlan);
> + rcu_assign_pointer(br->untagged, vlan);
> +}
> +
> +/* Must be protected by RTNL */
> +static void br_vlan_del_untagged(struct net_bridge *br,
> + struct net_bridge_vlan *vlan)
> +{
> + ASSERT_RTNL();
> + if (br->untagged == vlan) {
> + br_vlan_put(vlan);
> + rcu_assign_pointer(br->untagged, NULL);
> + }
> +}
> +
> struct net_bridge_vlan *br_vlan_find(struct net_bridge *br, u16 vid)
> {
> struct net_bridge_vlan *vlan;
> @@ -132,7 +160,7 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
>
> vlan = br_vlan_find(br, vid);
> if (vlan)
> - return vlan;
> + goto untagged;
>
> vlan = kzalloc(sizeof(struct net_bridge_vlan), GFP_KERNEL);
> if (!vlan)
> @@ -141,7 +169,7 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
> vlan->vid = vid;
> atomic_set(&vlan->refcnt, 1);
>
> - if (flags & BRIDGE_FLAGS_SELF) {
> + if (flags & BRIDGE_VLAN_INFO_MASTER) {
> /* Set bit 0 that is associated with the bridge master
> * device. Port numbers start with 1.
> */
> @@ -149,15 +177,24 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
> }
>
> hlist_add_head_rcu(&vlan->hlist, &br->vlan_hlist[br_vlan_hash(vid)]);
> +
> +untagged:
> + if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
> + br_vlan_add_untagged(br, vlan);
> +
> return vlan;
> }
>
> /* Must be protected by RTNL */
> -static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
> +static void br_vlan_del(struct net_bridge *br, struct net_bridge_vlan *vlan,
> + u16 flags)
> {
> ASSERT_RTNL();
>
> - if (flags & BRIDGE_FLAGS_SELF) {
> + if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
> + br_vlan_del_untagged(br, vlan);
> +
> + if (flags & BRIDGE_VLAN_INFO_MASTER) {
> /* Clear bit 0 that is associated with the bridge master
> * device.
> */
> @@ -172,6 +209,14 @@ static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
>
> vlan->vid = BR_INVALID_VID;
>
> + /* If, for whatever reason, bridge still has a ref on this vlan
> + * through the @untagged pointer, drop that ref and clear untagged.
> + */
> + if (br->untagged == vlan) {
> + br_vlan_put(vlan);
> + rcu_assign_pointer(br->untagged, NULL);
Is something doing an rcu sync after this point?
If yes maybe add a comment saying where it is, if not - some rcu read
side could still be using it through this pointer.
> + }
> +
> /* Drop the self-ref to trigger descrution. */
> br_vlan_put(vlan);
> }
> @@ -187,7 +232,7 @@ int br_vlan_delete(struct net_bridge *br, u16 vid, u16 flags)
> if (!vlan)
> return -ENOENT;
>
> - br_vlan_del(vlan, flags);
> + br_vlan_del(br, vlan, flags);
> return 0;
> }
>
> @@ -204,7 +249,9 @@ static void br_vlan_flush(struct net_bridge *br)
> for (i = 0; i < BR_VID_HASH_SIZE; i++) {
> hlist_for_each_entry_safe(vlan, node, tmp,
> &br->vlan_hlist[i], hlist) {
> - br_vlan_del(vlan, BRIDGE_FLAGS_SELF);
> + br_vlan_del(br, vlan,
> + (BRIDGE_VLAN_INFO_MASTER |
> + BRIDGE_VLAN_INFO_UNTAGGED));
> }
> }
> }
> @@ -224,10 +271,70 @@ struct net_port_vlan *nbp_vlan_find(const struct net_bridge_port *p, u16 vid)
> return NULL;
> }
>
> +static int nbp_vlan_add_untagged(struct net_bridge_port *p,
> + struct net_bridge_vlan *vlan,
> + u16 flags)
> +{
> + struct net_device *dev = p->dev;
> +
> + if (p->untagged) {
> + /* Port already has untagged vlan set. Drop the ref
> + * to the old one since we'll be replace it.
> + */
> + br_vlan_put(p->untagged);
> + } else {
> + int err;
> +
> + /* Add vid 0 to filter if filter is available. */
> + if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
> + dev->netdev_ops->ndo_vlan_rx_add_vid &&
> + dev->netdev_ops->ndo_vlan_rx_kill_vid) {
> + err = dev->netdev_ops->ndo_vlan_rx_add_vid(dev, 0);
> + if (err)
> + return err;
> + }
> + }
> +
> + /* This VLAN is handled as untagged/native. Save an
> + * additional ref.
> + */
> + br_vlan_hold(vlan);
> + rcu_assign_pointer(p->untagged, vlan);
> +
> + return 0;
> +}
> +
> +static void nbp_vlan_delete_untagged(struct net_bridge_port *p,
> + struct net_bridge_vlan *vlan)
> +{
> + if (p->untagged != vlan)
> + return;
> +
> + /* Remove VLAN from the device filter if it is supported. */
> + if ((p->dev->features & NETIF_F_HW_VLAN_FILTER) &&
> + p->dev->netdev_ops->ndo_vlan_rx_kill_vid) {
> + int err;
> +
> + err = p->dev->netdev_ops->ndo_vlan_rx_kill_vid(p->dev, 0);
> + if (err) {
> + pr_warn("failed to kill vid %d for device %s\n",
> + vlan->vid, p->dev->name);
> + }
> + }
> +
> + /* If this VLAN is currently functioning as untagged, clear it.
> + * It's safe to drop the refcount, since the vlan is still held
> + * by the port.
> + */
> + br_vlan_put(vlan);
> + rcu_assign_pointer(p->untagged, NULL);
> +
> +}
> +
> /* Must be protected by RTNL */
> int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
> {
> - struct net_port_vlan *pve;
> + struct net_port_vlan *pve = NULL;
> struct net_bridge_vlan *vlan;
> struct net_device *dev = p->dev;
> int err;
> @@ -275,11 +382,21 @@ int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
> set_bit(p->port_no, vlan->port_bitmap);
>
> list_add_tail_rcu(&pve->list, &p->vlan_list);
> +
> + if (flags & BRIDGE_VLAN_INFO_UNTAGGED) {
> + err = nbp_vlan_add_untagged(p, vlan, flags);
> + if (err)
> + goto del_vlan;
> + }
> +
> return 0;
>
> clean_up:
> kfree(pve);
> - br_vlan_del(vlan, flags);
> + br_vlan_del(p->br, vlan, flags);
> + return err;
> +del_vlan:
> + nbp_vlan_delete(p, vid, flags);
> return err;
> }
>
> @@ -296,6 +413,9 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
> if (!pve)
> return -ENOENT;
>
> + if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
> + nbp_vlan_delete_untagged(p, pve->vlan);
> +
> /* Remove VLAN from the device filter if it is supported. */
> if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
> dev->netdev_ops->ndo_vlan_rx_kill_vid) {
> @@ -306,6 +426,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
> pr_warn("failed to kill vid %d for device %s\n",
> vid, dev->name);
> }
> +
> pve->vid = BR_INVALID_VID;
>
> vlan = pve->vlan;
> @@ -316,7 +437,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
> list_del_rcu(&pve->list);
> kfree_rcu(pve, rcu);
>
> - br_vlan_del(vlan, flags);
> + br_vlan_del(p->br, vlan, flags);
>
> return 0;
> }
> @@ -328,8 +449,11 @@ static void nbp_vlan_flush(struct net_bridge_port *p)
>
> ASSERT_RTNL();
>
> - list_for_each_entry_safe(pve, tmp, &p->vlan_list, list)
> - nbp_vlan_delete(p, pve->vid, BRIDGE_FLAGS_SELF);
> + list_for_each_entry_safe(pve, tmp, &p->vlan_list, list) {
> + nbp_vlan_delete(p, pve->vid,
> + (BRIDGE_VLAN_INFO_MASTER |
> + BRIDGE_VLAN_INFO_UNTAGGED));
> + }
> }
>
> static void release_nbp(struct kobject *kobj)
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 9cf2879..1b302ce 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -199,7 +199,8 @@ static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
> if (p)
> err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
> else {
> - u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
> + u16 flags = vinfo->flags |
> + BRIDGE_VLAN_INFO_MASTER;
> if (!br_vlan_add(br, vinfo->vid, flags))
> err = -ENOMEM;
> }
> @@ -210,7 +211,8 @@ static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
> err = nbp_vlan_delete(p, vinfo->vid,
> vinfo->flags);
> else {
> - u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
> + u16 flags = vinfo->flags |
> + BRIDGE_VLAN_INFO_MASTER;
> err = br_vlan_delete(br, vinfo->vid, flags);
> }
> break;
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index cc75212..9328463 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -179,6 +179,7 @@ struct net_bridge_port
> struct netpoll *np;
> #endif
> struct list_head vlan_list;
> + struct net_bridge_vlan __rcu *untagged;
> };
>
> #define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT)
> @@ -298,6 +299,7 @@ struct net_bridge
> struct timer_list gc_timer;
> struct kobject *ifobj;
> struct hlist_head vlan_hlist[BR_VID_HASH_SIZE];
> + struct net_bridge_vlan __rcu *untagged;
> };
>
> struct br_input_skb_cb {
> --
> 1.7.7.6
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH V2 09/12] bridge: Add the ability to configure untagged vlans
2012-12-18 23:03 ` Vlad Yasevich
@ 2012-12-18 23:10 ` Michael S. Tsirkin
2012-12-19 14:50 ` Vlad Yasevich
0 siblings, 1 reply; 35+ messages in thread
From: Michael S. Tsirkin @ 2012-12-18 23:10 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev, shemminger, davem, or.gerlitz, jhs
On Tue, Dec 18, 2012 at 06:03:25PM -0500, Vlad Yasevich wrote:
> On 12/18/2012 06:01 PM, Michael S. Tsirkin wrote:
> >On Tue, Dec 18, 2012 at 02:01:00PM -0500, Vlad Yasevich wrote:
> >>A user may designate a certain vlan as untagged. This means that
> >>any ingress frame is assigned to this vlan and any forwarding decisions
> >>are made with this vlan in mind. On egress, any frames tagged/labeled
> >>with untagged vlan have the vlan tag removed and are send as regular
> >>ethernet frames.
> >>
> >>Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> >>---
> >> include/uapi/linux/if_bridge.h | 3 +
> >> net/bridge/br_if.c | 146 +++++++++++++++++++++++++++++++++++++---
> >> net/bridge/br_netlink.c | 6 +-
> >> net/bridge/br_private.h | 2 +
> >> 4 files changed, 144 insertions(+), 13 deletions(-)
> >>
> >>diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> >>index d0b4f5c..988d858 100644
> >>--- a/include/uapi/linux/if_bridge.h
> >>+++ b/include/uapi/linux/if_bridge.h
> >>@@ -127,6 +127,9 @@ enum {
> >> BR_VLAN_DEL,
> >> };
> >>
> >>+#define BRIDGE_VLAN_INFO_MASTER 1
> >>+#define BRIDGE_VLAN_INFO_UNTAGGED 2
> >>+
> >> struct bridge_vlan_info {
> >> u16 op_code;
> >> u16 flags;
> >>diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> >>index 57bbb35..14563fb 100644
> >>--- a/net/bridge/br_if.c
> >>+++ b/net/bridge/br_if.c
> >>@@ -108,6 +108,34 @@ static void br_vlan_put(struct net_bridge_vlan *vlan)
> >> br_vlan_destroy(vlan);
> >> }
> >>
> >>+/* Must be protected by RTNL */
> >>+static void br_vlan_add_untagged(struct net_bridge *br,
> >>+ struct net_bridge_vlan *vlan)
> >>+{
> >>+ ASSERT_RTNL();
> >>+ if (br->untagged == vlan)
> >>+ return;
> >>+ else if (br->untagged) {
> >>+ /* Untagged vlan is already set on the master,
> >>+ * so drop the ref since we'll be replacing it.
> >>+ */
> >>+ br_vlan_put(br->untagged);
> >>+ }
> >>+ br_vlan_hold(vlan);
> >>+ rcu_assign_pointer(br->untagged, vlan);
> >
> >Is there a reason for rcu here but not else where? If all users are under
> >rtnl you can just assign in a simple way.
> >If not then rcu_dereference_protected would be more appropriate.
>
> Everywhere that the pointer changes rcu_assign_pointer is used.
>
> Now, if we hold an RTNL, we can technically read the pointer with
> rcu since it's guaranteed not to change since it only changes under
> RTNL.
> I'll check that this is consistent.
Check what rcu_dereference_protected does. It's really just
an explicit way to say "this is accessed without rcu because I have
this lock".
> If I access the pointer without rtnl, it's always inside rcu
> critical section and with rcu_dereference().
>
> I thought those were the basic rules of rcu. Did that change?
>
> -vlad
> >
> >>+}
> >>+
> >>+/* Must be protected by RTNL */
> >>+static void br_vlan_del_untagged(struct net_bridge *br,
> >>+ struct net_bridge_vlan *vlan)
> >>+{
> >>+ ASSERT_RTNL();
> >>+ if (br->untagged == vlan) {
> >>+ br_vlan_put(vlan);
> >>+ rcu_assign_pointer(br->untagged, NULL);
> >>+ }
> >>+}
> >>+
> >> struct net_bridge_vlan *br_vlan_find(struct net_bridge *br, u16 vid)
> >> {
> >> struct net_bridge_vlan *vlan;
> >>@@ -132,7 +160,7 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
> >>
> >> vlan = br_vlan_find(br, vid);
> >> if (vlan)
> >>- return vlan;
> >>+ goto untagged;
> >>
> >> vlan = kzalloc(sizeof(struct net_bridge_vlan), GFP_KERNEL);
> >> if (!vlan)
> >>@@ -141,7 +169,7 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
> >> vlan->vid = vid;
> >> atomic_set(&vlan->refcnt, 1);
> >>
> >>- if (flags & BRIDGE_FLAGS_SELF) {
> >>+ if (flags & BRIDGE_VLAN_INFO_MASTER) {
> >> /* Set bit 0 that is associated with the bridge master
> >> * device. Port numbers start with 1.
> >> */
> >>@@ -149,15 +177,24 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
> >> }
> >>
> >> hlist_add_head_rcu(&vlan->hlist, &br->vlan_hlist[br_vlan_hash(vid)]);
> >>+
> >>+untagged:
> >>+ if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
> >>+ br_vlan_add_untagged(br, vlan);
> >>+
> >> return vlan;
> >> }
> >>
> >> /* Must be protected by RTNL */
> >>-static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
> >>+static void br_vlan_del(struct net_bridge *br, struct net_bridge_vlan *vlan,
> >>+ u16 flags)
> >> {
> >> ASSERT_RTNL();
> >>
> >>- if (flags & BRIDGE_FLAGS_SELF) {
> >>+ if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
> >>+ br_vlan_del_untagged(br, vlan);
> >>+
> >>+ if (flags & BRIDGE_VLAN_INFO_MASTER) {
> >> /* Clear bit 0 that is associated with the bridge master
> >> * device.
> >> */
> >>@@ -172,6 +209,14 @@ static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
> >>
> >> vlan->vid = BR_INVALID_VID;
> >>
> >>+ /* If, for whatever reason, bridge still has a ref on this vlan
> >>+ * through the @untagged pointer, drop that ref and clear untagged.
> >>+ */
> >>+ if (br->untagged == vlan) {
> >>+ br_vlan_put(vlan);
> >>+ rcu_assign_pointer(br->untagged, NULL);
> >>+ }
> >>+
> >> /* Drop the self-ref to trigger descrution. */
> >> br_vlan_put(vlan);
> >> }
> >>@@ -187,7 +232,7 @@ int br_vlan_delete(struct net_bridge *br, u16 vid, u16 flags)
> >> if (!vlan)
> >> return -ENOENT;
> >>
> >>- br_vlan_del(vlan, flags);
> >>+ br_vlan_del(br, vlan, flags);
> >> return 0;
> >> }
> >>
> >>@@ -204,7 +249,9 @@ static void br_vlan_flush(struct net_bridge *br)
> >> for (i = 0; i < BR_VID_HASH_SIZE; i++) {
> >> hlist_for_each_entry_safe(vlan, node, tmp,
> >> &br->vlan_hlist[i], hlist) {
> >>- br_vlan_del(vlan, BRIDGE_FLAGS_SELF);
> >>+ br_vlan_del(br, vlan,
> >>+ (BRIDGE_VLAN_INFO_MASTER |
> >>+ BRIDGE_VLAN_INFO_UNTAGGED));
> >> }
> >> }
> >> }
> >>@@ -224,10 +271,70 @@ struct net_port_vlan *nbp_vlan_find(const struct net_bridge_port *p, u16 vid)
> >> return NULL;
> >> }
> >>
> >>+static int nbp_vlan_add_untagged(struct net_bridge_port *p,
> >>+ struct net_bridge_vlan *vlan,
> >>+ u16 flags)
> >>+{
> >>+ struct net_device *dev = p->dev;
> >>+
> >>+ if (p->untagged) {
> >>+ /* Port already has untagged vlan set. Drop the ref
> >>+ * to the old one since we'll be replace it.
> >>+ */
> >>+ br_vlan_put(p->untagged);
> >>+ } else {
> >>+ int err;
> >>+
> >>+ /* Add vid 0 to filter if filter is available. */
> >>+ if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
> >>+ dev->netdev_ops->ndo_vlan_rx_add_vid &&
> >>+ dev->netdev_ops->ndo_vlan_rx_kill_vid) {
> >>+ err = dev->netdev_ops->ndo_vlan_rx_add_vid(dev, 0);
> >>+ if (err)
> >>+ return err;
> >>+ }
> >>+ }
> >>+
> >>+ /* This VLAN is handled as untagged/native. Save an
> >>+ * additional ref.
> >>+ */
> >>+ br_vlan_hold(vlan);
> >>+ rcu_assign_pointer(p->untagged, vlan);
> >>+
> >>+ return 0;
> >>+}
> >>+
> >>+static void nbp_vlan_delete_untagged(struct net_bridge_port *p,
> >>+ struct net_bridge_vlan *vlan)
> >>+{
> >>+ if (p->untagged != vlan)
> >>+ return;
> >>+
> >>+ /* Remove VLAN from the device filter if it is supported. */
> >>+ if ((p->dev->features & NETIF_F_HW_VLAN_FILTER) &&
> >>+ p->dev->netdev_ops->ndo_vlan_rx_kill_vid) {
> >>+ int err;
> >>+
> >>+ err = p->dev->netdev_ops->ndo_vlan_rx_kill_vid(p->dev, 0);
> >>+ if (err) {
> >>+ pr_warn("failed to kill vid %d for device %s\n",
> >>+ vlan->vid, p->dev->name);
> >>+ }
> >>+ }
> >>+
> >>+ /* If this VLAN is currently functioning as untagged, clear it.
> >>+ * It's safe to drop the refcount, since the vlan is still held
> >>+ * by the port.
> >>+ */
> >>+ br_vlan_put(vlan);
> >>+ rcu_assign_pointer(p->untagged, NULL);
> >>+
> >>+}
> >>+
> >> /* Must be protected by RTNL */
> >> int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
> >> {
> >>- struct net_port_vlan *pve;
> >>+ struct net_port_vlan *pve = NULL;
> >> struct net_bridge_vlan *vlan;
> >> struct net_device *dev = p->dev;
> >> int err;
> >>@@ -275,11 +382,21 @@ int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
> >> set_bit(p->port_no, vlan->port_bitmap);
> >>
> >> list_add_tail_rcu(&pve->list, &p->vlan_list);
> >>+
> >>+ if (flags & BRIDGE_VLAN_INFO_UNTAGGED) {
> >>+ err = nbp_vlan_add_untagged(p, vlan, flags);
> >>+ if (err)
> >>+ goto del_vlan;
> >>+ }
> >>+
> >> return 0;
> >>
> >> clean_up:
> >> kfree(pve);
> >>- br_vlan_del(vlan, flags);
> >>+ br_vlan_del(p->br, vlan, flags);
> >>+ return err;
> >>+del_vlan:
> >>+ nbp_vlan_delete(p, vid, flags);
> >> return err;
> >> }
> >>
> >>@@ -296,6 +413,9 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
> >> if (!pve)
> >> return -ENOENT;
> >>
> >>+ if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
> >>+ nbp_vlan_delete_untagged(p, pve->vlan);
> >>+
> >> /* Remove VLAN from the device filter if it is supported. */
> >> if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
> >> dev->netdev_ops->ndo_vlan_rx_kill_vid) {
> >>@@ -306,6 +426,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
> >> pr_warn("failed to kill vid %d for device %s\n",
> >> vid, dev->name);
> >> }
> >>+
> >> pve->vid = BR_INVALID_VID;
> >>
> >> vlan = pve->vlan;
> >>@@ -316,7 +437,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
> >> list_del_rcu(&pve->list);
> >> kfree_rcu(pve, rcu);
> >>
> >>- br_vlan_del(vlan, flags);
> >>+ br_vlan_del(p->br, vlan, flags);
> >>
> >> return 0;
> >> }
> >>@@ -328,8 +449,11 @@ static void nbp_vlan_flush(struct net_bridge_port *p)
> >>
> >> ASSERT_RTNL();
> >>
> >>- list_for_each_entry_safe(pve, tmp, &p->vlan_list, list)
> >>- nbp_vlan_delete(p, pve->vid, BRIDGE_FLAGS_SELF);
> >>+ list_for_each_entry_safe(pve, tmp, &p->vlan_list, list) {
> >>+ nbp_vlan_delete(p, pve->vid,
> >>+ (BRIDGE_VLAN_INFO_MASTER |
> >>+ BRIDGE_VLAN_INFO_UNTAGGED));
> >>+ }
> >> }
> >>
> >> static void release_nbp(struct kobject *kobj)
> >>diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> >>index 9cf2879..1b302ce 100644
> >>--- a/net/bridge/br_netlink.c
> >>+++ b/net/bridge/br_netlink.c
> >>@@ -199,7 +199,8 @@ static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
> >> if (p)
> >> err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
> >> else {
> >>- u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
> >>+ u16 flags = vinfo->flags |
> >>+ BRIDGE_VLAN_INFO_MASTER;
> >> if (!br_vlan_add(br, vinfo->vid, flags))
> >> err = -ENOMEM;
> >> }
> >>@@ -210,7 +211,8 @@ static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
> >> err = nbp_vlan_delete(p, vinfo->vid,
> >> vinfo->flags);
> >> else {
> >>- u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
> >>+ u16 flags = vinfo->flags |
> >>+ BRIDGE_VLAN_INFO_MASTER;
> >> err = br_vlan_delete(br, vinfo->vid, flags);
> >> }
> >> break;
> >>diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> >>index cc75212..9328463 100644
> >>--- a/net/bridge/br_private.h
> >>+++ b/net/bridge/br_private.h
> >>@@ -179,6 +179,7 @@ struct net_bridge_port
> >> struct netpoll *np;
> >> #endif
> >> struct list_head vlan_list;
> >>+ struct net_bridge_vlan __rcu *untagged;
> >> };
> >>
> >> #define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT)
> >>@@ -298,6 +299,7 @@ struct net_bridge
> >> struct timer_list gc_timer;
> >> struct kobject *ifobj;
> >> struct hlist_head vlan_hlist[BR_VID_HASH_SIZE];
> >>+ struct net_bridge_vlan __rcu *untagged;
> >> };
> >>
> >> struct br_input_skb_cb {
> >>--
> >>1.7.7.6
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH V2 09/12] bridge: Add the ability to configure untagged vlans
2012-12-18 23:04 ` Michael S. Tsirkin
@ 2012-12-19 1:06 ` Vlad Yasevich
0 siblings, 0 replies; 35+ messages in thread
From: Vlad Yasevich @ 2012-12-19 1:06 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, shemminger, davem, or.gerlitz, jhs
On 12/18/2012 06:04 PM, Michael S. Tsirkin wrote:
> On Tue, Dec 18, 2012 at 02:01:00PM -0500, Vlad Yasevich wrote:
>> A user may designate a certain vlan as untagged. This means that
>> any ingress frame is assigned to this vlan and any forwarding decisions
>> are made with this vlan in mind. On egress, any frames tagged/labeled
>> with untagged vlan have the vlan tag removed and are send as regular
>> ethernet frames.
>>
>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>> ---
>> include/uapi/linux/if_bridge.h | 3 +
>> net/bridge/br_if.c | 146 +++++++++++++++++++++++++++++++++++++---
>> net/bridge/br_netlink.c | 6 +-
>> net/bridge/br_private.h | 2 +
>> 4 files changed, 144 insertions(+), 13 deletions(-)
>>
>> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
>> index d0b4f5c..988d858 100644
>> --- a/include/uapi/linux/if_bridge.h
>> +++ b/include/uapi/linux/if_bridge.h
>> @@ -127,6 +127,9 @@ enum {
>> BR_VLAN_DEL,
>> };
>>
>> +#define BRIDGE_VLAN_INFO_MASTER 1
>> +#define BRIDGE_VLAN_INFO_UNTAGGED 2
>> +
>> struct bridge_vlan_info {
>> u16 op_code;
>> u16 flags;
>> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
>> index 57bbb35..14563fb 100644
>> --- a/net/bridge/br_if.c
>> +++ b/net/bridge/br_if.c
>> @@ -108,6 +108,34 @@ static void br_vlan_put(struct net_bridge_vlan *vlan)
>> br_vlan_destroy(vlan);
>> }
>>
>> +/* Must be protected by RTNL */
>> +static void br_vlan_add_untagged(struct net_bridge *br,
>> + struct net_bridge_vlan *vlan)
>> +{
>> + ASSERT_RTNL();
>> + if (br->untagged == vlan)
>> + return;
>> + else if (br->untagged) {
>> + /* Untagged vlan is already set on the master,
>> + * so drop the ref since we'll be replacing it.
>> + */
>> + br_vlan_put(br->untagged);
>> + }
>> + br_vlan_hold(vlan);
>> + rcu_assign_pointer(br->untagged, vlan);
>> +}
>> +
>> +/* Must be protected by RTNL */
>> +static void br_vlan_del_untagged(struct net_bridge *br,
>> + struct net_bridge_vlan *vlan)
>> +{
>> + ASSERT_RTNL();
>> + if (br->untagged == vlan) {
>> + br_vlan_put(vlan);
>> + rcu_assign_pointer(br->untagged, NULL);
>> + }
>> +}
>> +
>> struct net_bridge_vlan *br_vlan_find(struct net_bridge *br, u16 vid)
>> {
>> struct net_bridge_vlan *vlan;
>> @@ -132,7 +160,7 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
>>
>> vlan = br_vlan_find(br, vid);
>> if (vlan)
>> - return vlan;
>> + goto untagged;
>>
>> vlan = kzalloc(sizeof(struct net_bridge_vlan), GFP_KERNEL);
>> if (!vlan)
>> @@ -141,7 +169,7 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
>> vlan->vid = vid;
>> atomic_set(&vlan->refcnt, 1);
>>
>> - if (flags & BRIDGE_FLAGS_SELF) {
>> + if (flags & BRIDGE_VLAN_INFO_MASTER) {
>> /* Set bit 0 that is associated with the bridge master
>> * device. Port numbers start with 1.
>> */
>> @@ -149,15 +177,24 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
>> }
>>
>> hlist_add_head_rcu(&vlan->hlist, &br->vlan_hlist[br_vlan_hash(vid)]);
>> +
>> +untagged:
>> + if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
>> + br_vlan_add_untagged(br, vlan);
>> +
>> return vlan;
>> }
>>
>> /* Must be protected by RTNL */
>> -static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
>> +static void br_vlan_del(struct net_bridge *br, struct net_bridge_vlan *vlan,
>> + u16 flags)
>> {
>> ASSERT_RTNL();
>>
>> - if (flags & BRIDGE_FLAGS_SELF) {
>> + if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
>> + br_vlan_del_untagged(br, vlan);
>> +
>> + if (flags & BRIDGE_VLAN_INFO_MASTER) {
>> /* Clear bit 0 that is associated with the bridge master
>> * device.
>> */
>> @@ -172,6 +209,14 @@ static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
>>
>> vlan->vid = BR_INVALID_VID;
>>
>> + /* If, for whatever reason, bridge still has a ref on this vlan
>> + * through the @untagged pointer, drop that ref and clear untagged.
>> + */
>> + if (br->untagged == vlan) {
>> + br_vlan_put(vlan);
>> + rcu_assign_pointer(br->untagged, NULL);
>
> Is something doing an rcu sync after this point?
> If yes maybe add a comment saying where it is, if not - some rcu read
> side could still be using it through this pointer.
yes, at this point, all references from the ports have been dropped and
we are trying to destroy the element. The put below will trigger the
destruction path which will do kfree_rcu(). Thus there is a grace
period...
-vlad
>
>> + }
>> +
>> /* Drop the self-ref to trigger descrution. */
>> br_vlan_put(vlan);
>> }
>> @@ -187,7 +232,7 @@ int br_vlan_delete(struct net_bridge *br, u16 vid, u16 flags)
>> if (!vlan)
>> return -ENOENT;
>>
>> - br_vlan_del(vlan, flags);
>> + br_vlan_del(br, vlan, flags);
>> return 0;
>> }
>>
>> @@ -204,7 +249,9 @@ static void br_vlan_flush(struct net_bridge *br)
>> for (i = 0; i < BR_VID_HASH_SIZE; i++) {
>> hlist_for_each_entry_safe(vlan, node, tmp,
>> &br->vlan_hlist[i], hlist) {
>> - br_vlan_del(vlan, BRIDGE_FLAGS_SELF);
>> + br_vlan_del(br, vlan,
>> + (BRIDGE_VLAN_INFO_MASTER |
>> + BRIDGE_VLAN_INFO_UNTAGGED));
>> }
>> }
>> }
>> @@ -224,10 +271,70 @@ struct net_port_vlan *nbp_vlan_find(const struct net_bridge_port *p, u16 vid)
>> return NULL;
>> }
>>
>> +static int nbp_vlan_add_untagged(struct net_bridge_port *p,
>> + struct net_bridge_vlan *vlan,
>> + u16 flags)
>> +{
>> + struct net_device *dev = p->dev;
>> +
>> + if (p->untagged) {
>> + /* Port already has untagged vlan set. Drop the ref
>> + * to the old one since we'll be replace it.
>> + */
>> + br_vlan_put(p->untagged);
>> + } else {
>> + int err;
>> +
>> + /* Add vid 0 to filter if filter is available. */
>> + if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
>> + dev->netdev_ops->ndo_vlan_rx_add_vid &&
>> + dev->netdev_ops->ndo_vlan_rx_kill_vid) {
>> + err = dev->netdev_ops->ndo_vlan_rx_add_vid(dev, 0);
>> + if (err)
>> + return err;
>> + }
>> + }
>> +
>> + /* This VLAN is handled as untagged/native. Save an
>> + * additional ref.
>> + */
>> + br_vlan_hold(vlan);
>> + rcu_assign_pointer(p->untagged, vlan);
>> +
>> + return 0;
>> +}
>> +
>> +static void nbp_vlan_delete_untagged(struct net_bridge_port *p,
>> + struct net_bridge_vlan *vlan)
>> +{
>> + if (p->untagged != vlan)
>> + return;
>> +
>> + /* Remove VLAN from the device filter if it is supported. */
>> + if ((p->dev->features & NETIF_F_HW_VLAN_FILTER) &&
>> + p->dev->netdev_ops->ndo_vlan_rx_kill_vid) {
>> + int err;
>> +
>> + err = p->dev->netdev_ops->ndo_vlan_rx_kill_vid(p->dev, 0);
>> + if (err) {
>> + pr_warn("failed to kill vid %d for device %s\n",
>> + vlan->vid, p->dev->name);
>> + }
>> + }
>> +
>> + /* If this VLAN is currently functioning as untagged, clear it.
>> + * It's safe to drop the refcount, since the vlan is still held
>> + * by the port.
>> + */
>> + br_vlan_put(vlan);
>> + rcu_assign_pointer(p->untagged, NULL);
>> +
>> +}
>> +
>> /* Must be protected by RTNL */
>> int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
>> {
>> - struct net_port_vlan *pve;
>> + struct net_port_vlan *pve = NULL;
>> struct net_bridge_vlan *vlan;
>> struct net_device *dev = p->dev;
>> int err;
>> @@ -275,11 +382,21 @@ int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
>> set_bit(p->port_no, vlan->port_bitmap);
>>
>> list_add_tail_rcu(&pve->list, &p->vlan_list);
>> +
>> + if (flags & BRIDGE_VLAN_INFO_UNTAGGED) {
>> + err = nbp_vlan_add_untagged(p, vlan, flags);
>> + if (err)
>> + goto del_vlan;
>> + }
>> +
>> return 0;
>>
>> clean_up:
>> kfree(pve);
>> - br_vlan_del(vlan, flags);
>> + br_vlan_del(p->br, vlan, flags);
>> + return err;
>> +del_vlan:
>> + nbp_vlan_delete(p, vid, flags);
>> return err;
>> }
>>
>> @@ -296,6 +413,9 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
>> if (!pve)
>> return -ENOENT;
>>
>> + if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
>> + nbp_vlan_delete_untagged(p, pve->vlan);
>> +
>> /* Remove VLAN from the device filter if it is supported. */
>> if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
>> dev->netdev_ops->ndo_vlan_rx_kill_vid) {
>> @@ -306,6 +426,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
>> pr_warn("failed to kill vid %d for device %s\n",
>> vid, dev->name);
>> }
>> +
>> pve->vid = BR_INVALID_VID;
>>
>> vlan = pve->vlan;
>> @@ -316,7 +437,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
>> list_del_rcu(&pve->list);
>> kfree_rcu(pve, rcu);
>>
>> - br_vlan_del(vlan, flags);
>> + br_vlan_del(p->br, vlan, flags);
>>
>> return 0;
>> }
>> @@ -328,8 +449,11 @@ static void nbp_vlan_flush(struct net_bridge_port *p)
>>
>> ASSERT_RTNL();
>>
>> - list_for_each_entry_safe(pve, tmp, &p->vlan_list, list)
>> - nbp_vlan_delete(p, pve->vid, BRIDGE_FLAGS_SELF);
>> + list_for_each_entry_safe(pve, tmp, &p->vlan_list, list) {
>> + nbp_vlan_delete(p, pve->vid,
>> + (BRIDGE_VLAN_INFO_MASTER |
>> + BRIDGE_VLAN_INFO_UNTAGGED));
>> + }
>> }
>>
>> static void release_nbp(struct kobject *kobj)
>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>> index 9cf2879..1b302ce 100644
>> --- a/net/bridge/br_netlink.c
>> +++ b/net/bridge/br_netlink.c
>> @@ -199,7 +199,8 @@ static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
>> if (p)
>> err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
>> else {
>> - u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
>> + u16 flags = vinfo->flags |
>> + BRIDGE_VLAN_INFO_MASTER;
>> if (!br_vlan_add(br, vinfo->vid, flags))
>> err = -ENOMEM;
>> }
>> @@ -210,7 +211,8 @@ static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
>> err = nbp_vlan_delete(p, vinfo->vid,
>> vinfo->flags);
>> else {
>> - u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
>> + u16 flags = vinfo->flags |
>> + BRIDGE_VLAN_INFO_MASTER;
>> err = br_vlan_delete(br, vinfo->vid, flags);
>> }
>> break;
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index cc75212..9328463 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>> @@ -179,6 +179,7 @@ struct net_bridge_port
>> struct netpoll *np;
>> #endif
>> struct list_head vlan_list;
>> + struct net_bridge_vlan __rcu *untagged;
>> };
>>
>> #define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT)
>> @@ -298,6 +299,7 @@ struct net_bridge
>> struct timer_list gc_timer;
>> struct kobject *ifobj;
>> struct hlist_head vlan_hlist[BR_VID_HASH_SIZE];
>> + struct net_bridge_vlan __rcu *untagged;
>> };
>>
>> struct br_input_skb_cb {
>> --
>> 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
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH V2 00/12] Add basic VLAN support to bridges
2012-12-18 19:00 [PATCH V2 00/12] Add basic VLAN support to bridges Vlad Yasevich
` (12 preceding siblings ...)
2012-12-18 22:32 ` [PATCH V2 00/12] Add basic VLAN support to bridges Jiri Pirko
@ 2012-12-19 8:10 ` Shmulik Ladkani
2012-12-19 14:13 ` Vlad Yasevich
13 siblings, 1 reply; 35+ messages in thread
From: Shmulik Ladkani @ 2012-12-19 8:10 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev, shemminger, davem, or.gerlitz, jhs, mst
Thanks Vlad,
On Tue, 18 Dec 2012 14:00:51 -0500 Vlad Yasevich <vyasevic@redhat.com> wrote:
> A single vlan may also be designated as untagged. Any untagged traffic
> recieved by the port will be assigned to this vlan.
Why the "untagged vlan" is per-bridge global?
Usually, 802.1q switches define the PVID (port's VID) which controls
the value of VID, in case ingress frame is either untagged or
priority-tagged (per port configuration).
This gives greater flexibility.
> Any traffic exiting
> the port with a VID matching the untagged vlan will exit untagged (the
> bridge will strip the vlan header). This is similar to "Native Vlan" support
> available in most switches.
802.1q switches usually allow conifguring per-vlan, per-port
tagged/untagged egress policy: each vid has its port membership map and
an accompanying port egress-policy map.
This gives great flexibility defining all sorts of configurations.
Personally, I'd prefer a fully flexible vlan bridge allowing all sorts
of configurations (as available in 802.1q switches).
What's the reason limiting such configurations?
Regards,
Shmulik
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH V2 00/12] Add basic VLAN support to bridges
2012-12-18 22:46 ` Vlad Yasevich
@ 2012-12-19 8:27 ` Jiri Pirko
2012-12-19 16:25 ` Vlad Yasevich
2012-12-19 17:04 ` Thomas Graf
0 siblings, 2 replies; 35+ messages in thread
From: Jiri Pirko @ 2012-12-19 8:27 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev, shemminger, davem, or.gerlitz, jhs, mst
Tue, Dec 18, 2012 at 11:46:21PM CET, vyasevic@redhat.com wrote:
>On 12/18/2012 05:32 PM, Jiri Pirko wrote:
>>
>>
>>I see that this patchset replicates a lot of code which is already
>>present in net/8021q/ or include/linux/if_vlan.h. I think it would
>>be nice to move this code into some "common" place, wouldn't it?
>>
>
>The only replication that I am aware of is in br_vlan_untag(). I
>thought about pulling that piece out, but I think there is a reason
>why it's not available when 801q support isn't turned on. I noted that
>openvswitch implemented its own vlan header manipulation functions as well.
openvswitch should use the "common" code as well.
>
>What else are you seeing that's duplicate?
For example I spotted check of ndo_vlan_rx_[add/kill]_vid and
NETIF_F_HW_VLAN_FILTER and ndo_vlan_rx_[add/kill]_vid call
>
>-vlad
>
>>Jiri
>>
>>Tue, Dec 18, 2012 at 08:00:51PM CET, vyasevic@redhat.com wrote:
>>>This series of patches provides an ability to add VLANs to the bridge
>>>ports. This is similar to what can be found in most switches. The bridge
>>>port may have any number of VLANs added to it including vlan 0 priority tagged
>>>traffic. When vlans are added to the port, only traffic tagged with particular
>>>vlan will forwarded over this port. Additionally, vlan ids are added to FDB
>>>entries and become part of the lookup. This way we correctly identify the FDB
>>>entry.
>>>
>>>A single vlan may also be designated as untagged. Any untagged traffic
>>>recieved by the port will be assigned to this vlan. Any traffic exiting
>>>the port with a VID matching the untagged vlan will exit untagged (the
>>>bridge will strip the vlan header). This is similar to "Native Vlan" support
>>>available in most switches.
>>>
>>>The default behavior ofthe bridge is unchanged if no vlans have been
>>>configured.
>>>
>>>Changes since v1:
>>>- Fixed some forwarding bugs.
>>>- Add vlan to local fdb entries. New local entries are created per vlan
>>> to facilite correct forwarding to bridge interface.
>>>- Allow configuration of vlans directly on the bridge master device
>>> in addition to ports.
>>>
>>>Changes since rfc v2:
>>>- Per-port vlan bitmap is gone and is replaced with a vlan list.
>>>- Added bridge vlan list, which is referenced by each port. Entries in
>>> the birdge vlan list have port bitmap that shows which port are parts
>>> of which vlan.
>>>- Netlink API changes.
>>>- Dropped sysfs support for now. If people think this is really usefull,
>>> can add it back.
>>>- Support for native/untagged vlans.
>>>
>>>Changes since rfc v1:
>>>- Comments addressed regarding formatting and RCU usage
>>>- iocts have been removed and changed over the netlink interface.
>>>- Added support of user added ndb entries.
>>>- changed sysfs interface to export a bitmap. Also added a write interface.
>>> I am not sure how much I like it, but it made my testing easier/faster. I
>>> might change the write interface to take text instead of binary.
>>>
>>>
>>>Vlad Yasevich (12):
>>> bridge: Add vlan filtering infrastructure
>>> bridge: Validate that vlan is permitted on ingress
>>> bridge: Verify that a vlan is allowed to egress on give port
>>> bridge: Cache vlan in the cb for faster egress lookup.
>>> bridge: Add vlan to unicast fdb entries
>>> bridge: Add vlan id to multicast groups
>>> bridge: Add netlink interface to configure vlans on bridge ports
>>> bridge: Add vlan support to static neighbors
>>> bridge: Add the ability to configure untagged vlans
>>> bridge: Implement untagged vlan handling
>>> bridge: Dump vlan information from a bridge port
>>> bridge: Add vlan support for local fdb entries
>>>
>>>drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 +-
>>>drivers/net/macvlan.c | 2 +-
>>>drivers/net/vxlan.c | 3 +-
>>>include/linux/netdevice.h | 4 +-
>>>include/uapi/linux/if_bridge.h | 23 ++-
>>>include/uapi/linux/neighbour.h | 1 +
>>>include/uapi/linux/rtnetlink.h | 1 +
>>>net/bridge/br_device.c | 34 ++-
>>>net/bridge/br_fdb.c | 253 ++++++++++++---
>>>net/bridge/br_forward.c | 160 ++++++++++
>>>net/bridge/br_if.c | 404 ++++++++++++++++++++++++-
>>>net/bridge/br_input.c | 65 ++++-
>>>net/bridge/br_multicast.c | 71 +++--
>>>net/bridge/br_netlink.c | 178 ++++++++++--
>>>net/bridge/br_private.h | 71 ++++-
>>>net/core/rtnetlink.c | 40 ++-
>>>16 files changed, 1190 insertions(+), 125 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
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH V2 00/12] Add basic VLAN support to bridges
2012-12-19 8:10 ` Shmulik Ladkani
@ 2012-12-19 14:13 ` Vlad Yasevich
2012-12-19 19:37 ` Shmulik Ladkani
0 siblings, 1 reply; 35+ messages in thread
From: Vlad Yasevich @ 2012-12-19 14:13 UTC (permalink / raw)
To: Shmulik Ladkani; +Cc: netdev, shemminger, davem, or.gerlitz, jhs, mst
On 12/19/2012 03:10 AM, Shmulik Ladkani wrote:
> Thanks Vlad,
>
> On Tue, 18 Dec 2012 14:00:51 -0500 Vlad Yasevich <vyasevic@redhat.com> wrote:
>> A single vlan may also be designated as untagged. Any untagged traffic
>> recieved by the port will be assigned to this vlan.
>
> Why the "untagged vlan" is per-bridge global?
> Usually, 802.1q switches define the PVID (port's VID) which controls
> the value of VID, in case ingress frame is either untagged or
> priority-tagged (per port configuration).
> This gives greater flexibility.
It's not. There is a per port untagged pointer where you can designate
which VLAN is untagged/native on a port. The bride interface itself
can also function as a port, so it gets its own untagged pointer so
it can behave similar to port.
>
>> Any traffic exiting
>> the port with a VID matching the untagged vlan will exit untagged (the
>> bridge will strip the vlan header). This is similar to "Native Vlan" support
>> available in most switches.
>
> 802.1q switches usually allow conifguring per-vlan, per-port
> tagged/untagged egress policy: each vid has its port membership map and
> an accompanying port egress-policy map.
> This gives great flexibility defining all sorts of configurations.
Right, and that's what's provided here.
* Each VLAN has port membership map (net_bridge_vlan.portgroup).
* Each port has a list of vlans configured as well
(net_port_vlan.vlan_list).
* Each port also has a single vlan that can be untagged
(net_bridge_port.untagged).
* The bridge also has a single untagged vlan (net_bridge.untagged)
The limitation (in switches as well) is that only a single VLAN
may be untagged on any 1 port. If you have more then 1, you don't know
which VLAN the untagged traffic belongs to.
>
> Personally, I'd prefer a fully flexible vlan bridge allowing all sorts
> of configurations (as available in 802.1q switches).
>
> What's the reason limiting such configurations?
So, what do you see that's missing?
-vlad
>
> Regards,
> Shmulik
> --
> 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
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH V2 09/12] bridge: Add the ability to configure untagged vlans
2012-12-18 23:10 ` Michael S. Tsirkin
@ 2012-12-19 14:50 ` Vlad Yasevich
0 siblings, 0 replies; 35+ messages in thread
From: Vlad Yasevich @ 2012-12-19 14:50 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, shemminger, davem, or.gerlitz, jhs
On 12/18/2012 06:10 PM, Michael S. Tsirkin wrote:
> On Tue, Dec 18, 2012 at 06:03:25PM -0500, Vlad Yasevich wrote:
>> On 12/18/2012 06:01 PM, Michael S. Tsirkin wrote:
>>> On Tue, Dec 18, 2012 at 02:01:00PM -0500, Vlad Yasevich wrote:
>>>> A user may designate a certain vlan as untagged. This means that
>>>> any ingress frame is assigned to this vlan and any forwarding decisions
>>>> are made with this vlan in mind. On egress, any frames tagged/labeled
>>>> with untagged vlan have the vlan tag removed and are send as regular
>>>> ethernet frames.
>>>>
>>>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>>>> ---
>>>> include/uapi/linux/if_bridge.h | 3 +
>>>> net/bridge/br_if.c | 146 +++++++++++++++++++++++++++++++++++++---
>>>> net/bridge/br_netlink.c | 6 +-
>>>> net/bridge/br_private.h | 2 +
>>>> 4 files changed, 144 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
>>>> index d0b4f5c..988d858 100644
>>>> --- a/include/uapi/linux/if_bridge.h
>>>> +++ b/include/uapi/linux/if_bridge.h
>>>> @@ -127,6 +127,9 @@ enum {
>>>> BR_VLAN_DEL,
>>>> };
>>>>
>>>> +#define BRIDGE_VLAN_INFO_MASTER 1
>>>> +#define BRIDGE_VLAN_INFO_UNTAGGED 2
>>>> +
>>>> struct bridge_vlan_info {
>>>> u16 op_code;
>>>> u16 flags;
>>>> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
>>>> index 57bbb35..14563fb 100644
>>>> --- a/net/bridge/br_if.c
>>>> +++ b/net/bridge/br_if.c
>>>> @@ -108,6 +108,34 @@ static void br_vlan_put(struct net_bridge_vlan *vlan)
>>>> br_vlan_destroy(vlan);
>>>> }
>>>>
>>>> +/* Must be protected by RTNL */
>>>> +static void br_vlan_add_untagged(struct net_bridge *br,
>>>> + struct net_bridge_vlan *vlan)
>>>> +{
>>>> + ASSERT_RTNL();
>>>> + if (br->untagged == vlan)
>>>> + return;
>>>> + else if (br->untagged) {
>>>> + /* Untagged vlan is already set on the master,
>>>> + * so drop the ref since we'll be replacing it.
>>>> + */
>>>> + br_vlan_put(br->untagged);
>>>> + }
>>>> + br_vlan_hold(vlan);
>>>> + rcu_assign_pointer(br->untagged, vlan);
>>>
>>> Is there a reason for rcu here but not else where? If all users are under
>>> rtnl you can just assign in a simple way.
>>> If not then rcu_dereference_protected would be more appropriate.
>>
>> Everywhere that the pointer changes rcu_assign_pointer is used.
>>
>> Now, if we hold an RTNL, we can technically read the pointer with
>> rcu since it's guaranteed not to change since it only changes under
>> RTNL.
>> I'll check that this is consistent.
>
> Check what rcu_dereference_protected does. It's really just
> an explicit way to say "this is accessed without rcu because I have
> this lock".
Looks like the helper rtnl_dereference() already does what I need. I'll
use that.
Thanks
-vlad
>
>> If I access the pointer without rtnl, it's always inside rcu
>> critical section and with rcu_dereference().
>>
>> I thought those were the basic rules of rcu. Did that change?
>>
>> -vlad
>
>
>
>>>
>>>> +}
>>>> +
>>>> +/* Must be protected by RTNL */
>>>> +static void br_vlan_del_untagged(struct net_bridge *br,
>>>> + struct net_bridge_vlan *vlan)
>>>> +{
>>>> + ASSERT_RTNL();
>>>> + if (br->untagged == vlan) {
>>>> + br_vlan_put(vlan);
>>>> + rcu_assign_pointer(br->untagged, NULL);
>>>> + }
>>>> +}
>>>> +
>>>> struct net_bridge_vlan *br_vlan_find(struct net_bridge *br, u16 vid)
>>>> {
>>>> struct net_bridge_vlan *vlan;
>>>> @@ -132,7 +160,7 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
>>>>
>>>> vlan = br_vlan_find(br, vid);
>>>> if (vlan)
>>>> - return vlan;
>>>> + goto untagged;
>>>>
>>>> vlan = kzalloc(sizeof(struct net_bridge_vlan), GFP_KERNEL);
>>>> if (!vlan)
>>>> @@ -141,7 +169,7 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
>>>> vlan->vid = vid;
>>>> atomic_set(&vlan->refcnt, 1);
>>>>
>>>> - if (flags & BRIDGE_FLAGS_SELF) {
>>>> + if (flags & BRIDGE_VLAN_INFO_MASTER) {
>>>> /* Set bit 0 that is associated with the bridge master
>>>> * device. Port numbers start with 1.
>>>> */
>>>> @@ -149,15 +177,24 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
>>>> }
>>>>
>>>> hlist_add_head_rcu(&vlan->hlist, &br->vlan_hlist[br_vlan_hash(vid)]);
>>>> +
>>>> +untagged:
>>>> + if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
>>>> + br_vlan_add_untagged(br, vlan);
>>>> +
>>>> return vlan;
>>>> }
>>>>
>>>> /* Must be protected by RTNL */
>>>> -static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
>>>> +static void br_vlan_del(struct net_bridge *br, struct net_bridge_vlan *vlan,
>>>> + u16 flags)
>>>> {
>>>> ASSERT_RTNL();
>>>>
>>>> - if (flags & BRIDGE_FLAGS_SELF) {
>>>> + if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
>>>> + br_vlan_del_untagged(br, vlan);
>>>> +
>>>> + if (flags & BRIDGE_VLAN_INFO_MASTER) {
>>>> /* Clear bit 0 that is associated with the bridge master
>>>> * device.
>>>> */
>>>> @@ -172,6 +209,14 @@ static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
>>>>
>>>> vlan->vid = BR_INVALID_VID;
>>>>
>>>> + /* If, for whatever reason, bridge still has a ref on this vlan
>>>> + * through the @untagged pointer, drop that ref and clear untagged.
>>>> + */
>>>> + if (br->untagged == vlan) {
>>>> + br_vlan_put(vlan);
>>>> + rcu_assign_pointer(br->untagged, NULL);
>>>> + }
>>>> +
>>>> /* Drop the self-ref to trigger descrution. */
>>>> br_vlan_put(vlan);
>>>> }
>>>> @@ -187,7 +232,7 @@ int br_vlan_delete(struct net_bridge *br, u16 vid, u16 flags)
>>>> if (!vlan)
>>>> return -ENOENT;
>>>>
>>>> - br_vlan_del(vlan, flags);
>>>> + br_vlan_del(br, vlan, flags);
>>>> return 0;
>>>> }
>>>>
>>>> @@ -204,7 +249,9 @@ static void br_vlan_flush(struct net_bridge *br)
>>>> for (i = 0; i < BR_VID_HASH_SIZE; i++) {
>>>> hlist_for_each_entry_safe(vlan, node, tmp,
>>>> &br->vlan_hlist[i], hlist) {
>>>> - br_vlan_del(vlan, BRIDGE_FLAGS_SELF);
>>>> + br_vlan_del(br, vlan,
>>>> + (BRIDGE_VLAN_INFO_MASTER |
>>>> + BRIDGE_VLAN_INFO_UNTAGGED));
>>>> }
>>>> }
>>>> }
>>>> @@ -224,10 +271,70 @@ struct net_port_vlan *nbp_vlan_find(const struct net_bridge_port *p, u16 vid)
>>>> return NULL;
>>>> }
>>>>
>>>> +static int nbp_vlan_add_untagged(struct net_bridge_port *p,
>>>> + struct net_bridge_vlan *vlan,
>>>> + u16 flags)
>>>> +{
>>>> + struct net_device *dev = p->dev;
>>>> +
>>>> + if (p->untagged) {
>>>> + /* Port already has untagged vlan set. Drop the ref
>>>> + * to the old one since we'll be replace it.
>>>> + */
>>>> + br_vlan_put(p->untagged);
>>>> + } else {
>>>> + int err;
>>>> +
>>>> + /* Add vid 0 to filter if filter is available. */
>>>> + if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
>>>> + dev->netdev_ops->ndo_vlan_rx_add_vid &&
>>>> + dev->netdev_ops->ndo_vlan_rx_kill_vid) {
>>>> + err = dev->netdev_ops->ndo_vlan_rx_add_vid(dev, 0);
>>>> + if (err)
>>>> + return err;
>>>> + }
>>>> + }
>>>> +
>>>> + /* This VLAN is handled as untagged/native. Save an
>>>> + * additional ref.
>>>> + */
>>>> + br_vlan_hold(vlan);
>>>> + rcu_assign_pointer(p->untagged, vlan);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void nbp_vlan_delete_untagged(struct net_bridge_port *p,
>>>> + struct net_bridge_vlan *vlan)
>>>> +{
>>>> + if (p->untagged != vlan)
>>>> + return;
>>>> +
>>>> + /* Remove VLAN from the device filter if it is supported. */
>>>> + if ((p->dev->features & NETIF_F_HW_VLAN_FILTER) &&
>>>> + p->dev->netdev_ops->ndo_vlan_rx_kill_vid) {
>>>> + int err;
>>>> +
>>>> + err = p->dev->netdev_ops->ndo_vlan_rx_kill_vid(p->dev, 0);
>>>> + if (err) {
>>>> + pr_warn("failed to kill vid %d for device %s\n",
>>>> + vlan->vid, p->dev->name);
>>>> + }
>>>> + }
>>>> +
>>>> + /* If this VLAN is currently functioning as untagged, clear it.
>>>> + * It's safe to drop the refcount, since the vlan is still held
>>>> + * by the port.
>>>> + */
>>>> + br_vlan_put(vlan);
>>>> + rcu_assign_pointer(p->untagged, NULL);
>>>> +
>>>> +}
>>>> +
>>>> /* Must be protected by RTNL */
>>>> int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
>>>> {
>>>> - struct net_port_vlan *pve;
>>>> + struct net_port_vlan *pve = NULL;
>>>> struct net_bridge_vlan *vlan;
>>>> struct net_device *dev = p->dev;
>>>> int err;
>>>> @@ -275,11 +382,21 @@ int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
>>>> set_bit(p->port_no, vlan->port_bitmap);
>>>>
>>>> list_add_tail_rcu(&pve->list, &p->vlan_list);
>>>> +
>>>> + if (flags & BRIDGE_VLAN_INFO_UNTAGGED) {
>>>> + err = nbp_vlan_add_untagged(p, vlan, flags);
>>>> + if (err)
>>>> + goto del_vlan;
>>>> + }
>>>> +
>>>> return 0;
>>>>
>>>> clean_up:
>>>> kfree(pve);
>>>> - br_vlan_del(vlan, flags);
>>>> + br_vlan_del(p->br, vlan, flags);
>>>> + return err;
>>>> +del_vlan:
>>>> + nbp_vlan_delete(p, vid, flags);
>>>> return err;
>>>> }
>>>>
>>>> @@ -296,6 +413,9 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
>>>> if (!pve)
>>>> return -ENOENT;
>>>>
>>>> + if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
>>>> + nbp_vlan_delete_untagged(p, pve->vlan);
>>>> +
>>>> /* Remove VLAN from the device filter if it is supported. */
>>>> if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
>>>> dev->netdev_ops->ndo_vlan_rx_kill_vid) {
>>>> @@ -306,6 +426,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
>>>> pr_warn("failed to kill vid %d for device %s\n",
>>>> vid, dev->name);
>>>> }
>>>> +
>>>> pve->vid = BR_INVALID_VID;
>>>>
>>>> vlan = pve->vlan;
>>>> @@ -316,7 +437,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
>>>> list_del_rcu(&pve->list);
>>>> kfree_rcu(pve, rcu);
>>>>
>>>> - br_vlan_del(vlan, flags);
>>>> + br_vlan_del(p->br, vlan, flags);
>>>>
>>>> return 0;
>>>> }
>>>> @@ -328,8 +449,11 @@ static void nbp_vlan_flush(struct net_bridge_port *p)
>>>>
>>>> ASSERT_RTNL();
>>>>
>>>> - list_for_each_entry_safe(pve, tmp, &p->vlan_list, list)
>>>> - nbp_vlan_delete(p, pve->vid, BRIDGE_FLAGS_SELF);
>>>> + list_for_each_entry_safe(pve, tmp, &p->vlan_list, list) {
>>>> + nbp_vlan_delete(p, pve->vid,
>>>> + (BRIDGE_VLAN_INFO_MASTER |
>>>> + BRIDGE_VLAN_INFO_UNTAGGED));
>>>> + }
>>>> }
>>>>
>>>> static void release_nbp(struct kobject *kobj)
>>>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>>>> index 9cf2879..1b302ce 100644
>>>> --- a/net/bridge/br_netlink.c
>>>> +++ b/net/bridge/br_netlink.c
>>>> @@ -199,7 +199,8 @@ static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
>>>> if (p)
>>>> err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
>>>> else {
>>>> - u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
>>>> + u16 flags = vinfo->flags |
>>>> + BRIDGE_VLAN_INFO_MASTER;
>>>> if (!br_vlan_add(br, vinfo->vid, flags))
>>>> err = -ENOMEM;
>>>> }
>>>> @@ -210,7 +211,8 @@ static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
>>>> err = nbp_vlan_delete(p, vinfo->vid,
>>>> vinfo->flags);
>>>> else {
>>>> - u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
>>>> + u16 flags = vinfo->flags |
>>>> + BRIDGE_VLAN_INFO_MASTER;
>>>> err = br_vlan_delete(br, vinfo->vid, flags);
>>>> }
>>>> break;
>>>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>>>> index cc75212..9328463 100644
>>>> --- a/net/bridge/br_private.h
>>>> +++ b/net/bridge/br_private.h
>>>> @@ -179,6 +179,7 @@ struct net_bridge_port
>>>> struct netpoll *np;
>>>> #endif
>>>> struct list_head vlan_list;
>>>> + struct net_bridge_vlan __rcu *untagged;
>>>> };
>>>>
>>>> #define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT)
>>>> @@ -298,6 +299,7 @@ struct net_bridge
>>>> struct timer_list gc_timer;
>>>> struct kobject *ifobj;
>>>> struct hlist_head vlan_hlist[BR_VID_HASH_SIZE];
>>>> + struct net_bridge_vlan __rcu *untagged;
>>>> };
>>>>
>>>> struct br_input_skb_cb {
>>>> --
>>>> 1.7.7.6
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH V2 00/12] Add basic VLAN support to bridges
2012-12-19 8:27 ` Jiri Pirko
@ 2012-12-19 16:25 ` Vlad Yasevich
2012-12-19 17:04 ` Thomas Graf
1 sibling, 0 replies; 35+ messages in thread
From: Vlad Yasevich @ 2012-12-19 16:25 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, shemminger, davem, or.gerlitz, jhs, mst
On 12/19/2012 03:27 AM, Jiri Pirko wrote:
> Tue, Dec 18, 2012 at 11:46:21PM CET, vyasevic@redhat.com wrote:
>> On 12/18/2012 05:32 PM, Jiri Pirko wrote:
>>>
>>>
>>> I see that this patchset replicates a lot of code which is already
>>> present in net/8021q/ or include/linux/if_vlan.h. I think it would
>>> be nice to move this code into some "common" place, wouldn't it?
>>>
>>
>> The only replication that I am aware of is in br_vlan_untag(). I
>> thought about pulling that piece out, but I think there is a reason
>> why it's not available when 801q support isn't turned on. I noted that
>> openvswitch implemented its own vlan header manipulation functions as well.
>
> openvswitch should use the "common" code as well.
>
>>
>> What else are you seeing that's duplicate?
>
> For example I spotted check of ndo_vlan_rx_[add/kill]_vid and
> NETIF_F_HW_VLAN_FILTER and ndo_vlan_rx_[add/kill]_vid call
Ahh yes.... I can make that generic. Thanks
-vlad
>
>
>>
>> -vlad
>>
>>> Jiri
>>>
>>> Tue, Dec 18, 2012 at 08:00:51PM CET, vyasevic@redhat.com wrote:
>>>> This series of patches provides an ability to add VLANs to the bridge
>>>> ports. This is similar to what can be found in most switches. The bridge
>>>> port may have any number of VLANs added to it including vlan 0 priority tagged
>>>> traffic. When vlans are added to the port, only traffic tagged with particular
>>>> vlan will forwarded over this port. Additionally, vlan ids are added to FDB
>>>> entries and become part of the lookup. This way we correctly identify the FDB
>>>> entry.
>>>>
>>>> A single vlan may also be designated as untagged. Any untagged traffic
>>>> recieved by the port will be assigned to this vlan. Any traffic exiting
>>>> the port with a VID matching the untagged vlan will exit untagged (the
>>>> bridge will strip the vlan header). This is similar to "Native Vlan" support
>>>> available in most switches.
>>>>
>>>> The default behavior ofthe bridge is unchanged if no vlans have been
>>>> configured.
>>>>
>>>> Changes since v1:
>>>> - Fixed some forwarding bugs.
>>>> - Add vlan to local fdb entries. New local entries are created per vlan
>>>> to facilite correct forwarding to bridge interface.
>>>> - Allow configuration of vlans directly on the bridge master device
>>>> in addition to ports.
>>>>
>>>> Changes since rfc v2:
>>>> - Per-port vlan bitmap is gone and is replaced with a vlan list.
>>>> - Added bridge vlan list, which is referenced by each port. Entries in
>>>> the birdge vlan list have port bitmap that shows which port are parts
>>>> of which vlan.
>>>> - Netlink API changes.
>>>> - Dropped sysfs support for now. If people think this is really usefull,
>>>> can add it back.
>>>> - Support for native/untagged vlans.
>>>>
>>>> Changes since rfc v1:
>>>> - Comments addressed regarding formatting and RCU usage
>>>> - iocts have been removed and changed over the netlink interface.
>>>> - Added support of user added ndb entries.
>>>> - changed sysfs interface to export a bitmap. Also added a write interface.
>>>> I am not sure how much I like it, but it made my testing easier/faster. I
>>>> might change the write interface to take text instead of binary.
>>>>
>>>>
>>>> Vlad Yasevich (12):
>>>> bridge: Add vlan filtering infrastructure
>>>> bridge: Validate that vlan is permitted on ingress
>>>> bridge: Verify that a vlan is allowed to egress on give port
>>>> bridge: Cache vlan in the cb for faster egress lookup.
>>>> bridge: Add vlan to unicast fdb entries
>>>> bridge: Add vlan id to multicast groups
>>>> bridge: Add netlink interface to configure vlans on bridge ports
>>>> bridge: Add vlan support to static neighbors
>>>> bridge: Add the ability to configure untagged vlans
>>>> bridge: Implement untagged vlan handling
>>>> bridge: Dump vlan information from a bridge port
>>>> bridge: Add vlan support for local fdb entries
>>>>
>>>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 +-
>>>> drivers/net/macvlan.c | 2 +-
>>>> drivers/net/vxlan.c | 3 +-
>>>> include/linux/netdevice.h | 4 +-
>>>> include/uapi/linux/if_bridge.h | 23 ++-
>>>> include/uapi/linux/neighbour.h | 1 +
>>>> include/uapi/linux/rtnetlink.h | 1 +
>>>> net/bridge/br_device.c | 34 ++-
>>>> net/bridge/br_fdb.c | 253 ++++++++++++---
>>>> net/bridge/br_forward.c | 160 ++++++++++
>>>> net/bridge/br_if.c | 404 ++++++++++++++++++++++++-
>>>> net/bridge/br_input.c | 65 ++++-
>>>> net/bridge/br_multicast.c | 71 +++--
>>>> net/bridge/br_netlink.c | 178 ++++++++++--
>>>> net/bridge/br_private.h | 71 ++++-
>>>> net/core/rtnetlink.c | 40 ++-
>>>> 16 files changed, 1190 insertions(+), 125 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
>>
> --
> 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
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH V2 00/12] Add basic VLAN support to bridges
2012-12-19 8:27 ` Jiri Pirko
2012-12-19 16:25 ` Vlad Yasevich
@ 2012-12-19 17:04 ` Thomas Graf
2012-12-19 17:11 ` Vlad Yasevich
1 sibling, 1 reply; 35+ messages in thread
From: Thomas Graf @ 2012-12-19 17:04 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Vlad Yasevich, netdev, shemminger, davem, or.gerlitz, jhs, mst
On 12/19/12 at 09:27am, Jiri Pirko wrote:
> Tue, Dec 18, 2012 at 11:46:21PM CET, vyasevic@redhat.com wrote:
> >On 12/18/2012 05:32 PM, Jiri Pirko wrote:
> >>
> >>
> >>I see that this patchset replicates a lot of code which is already
> >>present in net/8021q/ or include/linux/if_vlan.h. I think it would
> >>be nice to move this code into some "common" place, wouldn't it?
> >>
> >
> >The only replication that I am aware of is in br_vlan_untag(). I
> >thought about pulling that piece out, but I think there is a reason
> >why it's not available when 801q support isn't turned on. I noted that
> >openvswitch implemented its own vlan header manipulation functions as well.
>
> openvswitch should use the "common" code as well.
I was just about to mention this. This overlaps with openvswitch
in functionality which I have absoluetely no objections against
but code reuse should come to focus in order to avoid having to
fix bugs twice.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH V2 00/12] Add basic VLAN support to bridges
2012-12-19 17:04 ` Thomas Graf
@ 2012-12-19 17:11 ` Vlad Yasevich
2012-12-19 17:19 ` Jiri Pirko
2012-12-19 17:20 ` Thomas Graf
0 siblings, 2 replies; 35+ messages in thread
From: Vlad Yasevich @ 2012-12-19 17:11 UTC (permalink / raw)
To: Thomas Graf; +Cc: Jiri Pirko, netdev, shemminger, davem, or.gerlitz, jhs, mst
On 12/19/2012 12:04 PM, Thomas Graf wrote:
> On 12/19/12 at 09:27am, Jiri Pirko wrote:
>> Tue, Dec 18, 2012 at 11:46:21PM CET, vyasevic@redhat.com wrote:
>>> On 12/18/2012 05:32 PM, Jiri Pirko wrote:
>>>>
>>>>
>>>> I see that this patchset replicates a lot of code which is already
>>>> present in net/8021q/ or include/linux/if_vlan.h. I think it would
>>>> be nice to move this code into some "common" place, wouldn't it?
>>>>
>>>
>>> The only replication that I am aware of is in br_vlan_untag(). I
>>> thought about pulling that piece out, but I think there is a reason
>>> why it's not available when 801q support isn't turned on. I noted that
>>> openvswitch implemented its own vlan header manipulation functions as well.
>>
>> openvswitch should use the "common" code as well.
>
> I was just about to mention this. This overlaps with openvswitch
> in functionality which I have absoluetely no objections against
> but code reuse should come to focus in order to avoid having to
> fix bugs twice.
>
Could we consolidate the code after this is accepted and all the parties
can agree on the consolidation? I'd really like to keep this series
as minimally invasive as possible.
Thanks
-vlad
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH V2 00/12] Add basic VLAN support to bridges
2012-12-19 17:11 ` Vlad Yasevich
@ 2012-12-19 17:19 ` Jiri Pirko
2012-12-19 17:20 ` Thomas Graf
1 sibling, 0 replies; 35+ messages in thread
From: Jiri Pirko @ 2012-12-19 17:19 UTC (permalink / raw)
To: Vlad Yasevich
Cc: Thomas Graf, netdev, shemminger, davem, or.gerlitz, jhs, mst
Wed, Dec 19, 2012 at 06:11:45PM CET, vyasevic@redhat.com wrote:
>On 12/19/2012 12:04 PM, Thomas Graf wrote:
>>On 12/19/12 at 09:27am, Jiri Pirko wrote:
>>>Tue, Dec 18, 2012 at 11:46:21PM CET, vyasevic@redhat.com wrote:
>>>>On 12/18/2012 05:32 PM, Jiri Pirko wrote:
>>>>>
>>>>>
>>>>>I see that this patchset replicates a lot of code which is already
>>>>>present in net/8021q/ or include/linux/if_vlan.h. I think it would
>>>>>be nice to move this code into some "common" place, wouldn't it?
>>>>>
>>>>
>>>>The only replication that I am aware of is in br_vlan_untag(). I
>>>>thought about pulling that piece out, but I think there is a reason
>>>>why it's not available when 801q support isn't turned on. I noted that
>>>>openvswitch implemented its own vlan header manipulation functions as well.
>>>
>>>openvswitch should use the "common" code as well.
>>
>>I was just about to mention this. This overlaps with openvswitch
>>in functionality which I have absoluetely no objections against
>>but code reuse should come to focus in order to avoid having to
>>fix bugs twice.
>>
>
>Could we consolidate the code after this is accepted and all the parties
>can agree on the consolidation? I'd really like to keep this series
>as minimally invasive as possible.
That sounds good to me.
>
>Thanks
>-vlad
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH V2 00/12] Add basic VLAN support to bridges
2012-12-19 17:11 ` Vlad Yasevich
2012-12-19 17:19 ` Jiri Pirko
@ 2012-12-19 17:20 ` Thomas Graf
1 sibling, 0 replies; 35+ messages in thread
From: Thomas Graf @ 2012-12-19 17:20 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: Jiri Pirko, netdev, shemminger, davem, or.gerlitz, jhs, mst
On 12/19/12 at 12:11pm, Vlad Yasevich wrote:
> Could we consolidate the code after this is accepted and all the parties
> can agree on the consolidation? I'd really like to keep this series
> as minimally invasive as possible.
Sure, this was just a general remark on general future direction :)
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH V2 00/12] Add basic VLAN support to bridges
2012-12-19 14:13 ` Vlad Yasevich
@ 2012-12-19 19:37 ` Shmulik Ladkani
2012-12-19 20:03 ` Vlad Yasevich
0 siblings, 1 reply; 35+ messages in thread
From: Shmulik Ladkani @ 2012-12-19 19:37 UTC (permalink / raw)
To: vyasevic; +Cc: netdev, shemminger, davem, or.gerlitz, jhs, mst
Hi Vlad,
On Wed, 19 Dec 2012 09:13:10 -0500 Vlad Yasevich <vyasevic@redhat.com> wrote:
> > Why the "untagged vlan" is per-bridge global?
> It's not. There is a per port untagged pointer where you can designate
> which VLAN is untagged/native on a port.
Ok (misinterpreted the text in the cover letter).
> > 802.1q switches usually allow conifguring per-vlan, per-port
> > tagged/untagged egress policy: each vid has its port membership map and
> > an accompanying port egress-policy map.
> > This gives great flexibility defining all sorts of configurations.
>
> Right, and that's what's provided here.
> * Each VLAN has port membership map (net_bridge_vlan.portgroup).
> * Each port has a list of vlans configured as well
> (net_port_vlan.vlan_list).
> * Each port also has a single vlan that can be untagged
> (net_bridge_port.untagged).
> * The bridge also has a single untagged vlan (net_bridge.untagged)
>
> The limitation (in switches as well) is that only a single VLAN
> may be untagged on any 1 port.
Switches usually allow you to configure each port's egress policy per
vlan, and allow you to configure multiple vlans to _egress_ untagged
on a port.
> If you have more then 1, you don't know
> which VLAN the untagged traffic belongs to.
The port's PVID uniquely determines VID to associate with the frame
during _ingress_ on that port - in the case frame arrived untagged.
This is unrelated to whether a frame having a specific VID would _egress_
tagged or untagged on that port.
> > Personally, I'd prefer a fully flexible vlan bridge allowing all sorts
> > of configurations (as available in 802.1q switches).
> >
> > What's the reason limiting such configurations?
>
> So, what do you see that's missing?
I suspect some configurations wouldn't be possible at current design,
please let me know what do you think.
Take for example such a configuration:
+---+---+
| | |
p1 p2 p3
A bridge with 3 ports: p1 p2 p3.
(the bridge interface itself is irrelevant for this discussion)
This setup expects all traffic at those ports to be untagged.
I'd like p1 to be bridged to p2, but not to p3.
I'd like p3 to be bridged to p2, but not to p1.
Set the following:
p1 - PVID is 1
member of VLANs 1(egress untagged), 2(egress untagged)
p3 - PVID is 3
member of VLANs 3(egress untagged), 2(egress untagged)
p2 - PVID is 2
member of VLANs 1,2,3 (all egress untagged)
Or putting it differently:
VLAN 1 ports membership: p1, p2
egress policy: U U
VLAN 2 ports membership: p1, p2, p3
egress policy: U U U
VLAN 3 ports membership: p2, p3
egress policy: U U
Now, untagged traffic arriving at p1 will be assigned its PVID, which
is 1, an thus forced to egress via p2 (the only other member of VLAN 1).
Upon egress on p2, it will egress untagged.
(similarly for ingress in p3, only egress at p2 is possible, again
untagged).
If untagged traffic arrives at p2, it will be assigned its PVID, which
is 2, and thus it may egress on either p1 or p3 (they are both members
of VLAN 2).
Actual port of egress is according to FDB match.
Again egress would be untagged.
This setup might sound exotic, but similar patterns may be used in
reality.
Although the example is synthetic and simplistic, still it demonstrates
the vast flexibility allowed by a 802.1q bridge.
The bridge constructs needed for supporting such setups are:
- per port: PVID
- per VLAN: port membership map
- per VLAN: port egress policy map
I agree, tools other than a vlan bridge may implement such setups, but
using the vlan bridge would be preferred, mainly due to the simplicity.
Regards,
Shmulik
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH V2 00/12] Add basic VLAN support to bridges
2012-12-19 19:37 ` Shmulik Ladkani
@ 2012-12-19 20:03 ` Vlad Yasevich
2012-12-19 22:59 ` Vlad Yasevich
2012-12-20 7:00 ` Shmulik Ladkani
0 siblings, 2 replies; 35+ messages in thread
From: Vlad Yasevich @ 2012-12-19 20:03 UTC (permalink / raw)
To: Shmulik Ladkani; +Cc: netdev, shemminger, davem, or.gerlitz, jhs, mst
On 12/19/2012 02:37 PM, Shmulik Ladkani wrote:
> Hi Vlad,
>
> On Wed, 19 Dec 2012 09:13:10 -0500 Vlad Yasevich <vyasevic@redhat.com> wrote:
>>> Why the "untagged vlan" is per-bridge global?
>> It's not. There is a per port untagged pointer where you can designate
>> which VLAN is untagged/native on a port.
>
> Ok (misinterpreted the text in the cover letter).
>
>>> 802.1q switches usually allow conifguring per-vlan, per-port
>>> tagged/untagged egress policy: each vid has its port membership map and
>>> an accompanying port egress-policy map.
>>> This gives great flexibility defining all sorts of configurations.
>>
>> Right, and that's what's provided here.
>> * Each VLAN has port membership map (net_bridge_vlan.portgroup).
>> * Each port has a list of vlans configured as well
>> (net_port_vlan.vlan_list).
>> * Each port also has a single vlan that can be untagged
>> (net_bridge_port.untagged).
>> * The bridge also has a single untagged vlan (net_bridge.untagged)
>>
>> The limitation (in switches as well) is that only a single VLAN
>> may be untagged on any 1 port.
>
> Switches usually allow you to configure each port's egress policy per
> vlan, and allow you to configure multiple vlans to _egress_ untagged
> on a port.
>
>> If you have more then 1, you don't know
>> which VLAN the untagged traffic belongs to.
>
> The port's PVID uniquely determines VID to associate with the frame
> during _ingress_ on that port - in the case frame arrived untagged.
>
> This is unrelated to whether a frame having a specific VID would _egress_
> tagged or untagged on that port.
>
Ahh... I see what you mean. You would like to separate
ingress policy and egress policy with regard to how tags are applied...
I haven't seen that type of config before.
I did say "Basic VLAN support". :)
In this set of patches ingress and egress policies are hardcoded the same...
So, consider that what I am calling "untagged" in this series is
really vlan associated with PVID. To change the egress policy, we
could add an untagged bitmap into the vlan. Then the bitmap from the
vlan would determine the egress policy. If the port is in the "tagged"
bitmap, frame leaves tagged. If the port is in the "untagged" bitmap,
frame leaves untagged.
The code to make this would would be simple enough. The more
interesting part would be the configuration :)
>>> Personally, I'd prefer a fully flexible vlan bridge allowing all sorts
>>> of configurations (as available in 802.1q switches).
>>>
>>> What's the reason limiting such configurations?
>>
>> So, what do you see that's missing?
>
[ snip good example ]
>
> The bridge constructs needed for supporting such setups are:
> - per port: PVID
> - per VLAN: port membership map
> - per VLAN: port egress policy map
Ok, so from above, membership map is the exiting port_bitmap. Egress
policy map could be new untagged_bitmap. We wouldn't need a tagged
policy map since a port can't be "in egress policy, but not in
membership map".
Membership port_bitmap is consulted on egress for basic forward/drop
decision (just as it is now). Egress policy (untagged bitmap) is
consulted to see how the forwarding is done.
Sounds about right? If so, I could probably work something up.
Will probably leave the configuration for later as that might take a bit
longer to figure out.
-vlad
>
> I agree, tools other than a vlan bridge may implement such setups, but
> using the vlan bridge would be preferred, mainly due to the simplicity.
>
> Regards,
> Shmulik
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH V2 00/12] Add basic VLAN support to bridges
2012-12-19 20:03 ` Vlad Yasevich
@ 2012-12-19 22:59 ` Vlad Yasevich
2012-12-20 7:00 ` Shmulik Ladkani
1 sibling, 0 replies; 35+ messages in thread
From: Vlad Yasevich @ 2012-12-19 22:59 UTC (permalink / raw)
To: vyasevic; +Cc: Shmulik Ladkani, netdev, shemminger, davem, or.gerlitz, jhs, mst
On 12/19/2012 03:03 PM, Vlad Yasevich wrote:
> On 12/19/2012 02:37 PM, Shmulik Ladkani wrote:
>> Hi Vlad,
>>
>> On Wed, 19 Dec 2012 09:13:10 -0500 Vlad Yasevich <vyasevic@redhat.com>
>> wrote:
>>>> Why the "untagged vlan" is per-bridge global?
>>> It's not. There is a per port untagged pointer where you can designate
>>> which VLAN is untagged/native on a port.
>>
>> Ok (misinterpreted the text in the cover letter).
>>
>>>> 802.1q switches usually allow conifguring per-vlan, per-port
>>>> tagged/untagged egress policy: each vid has its port membership map and
>>>> an accompanying port egress-policy map.
>>>> This gives great flexibility defining all sorts of configurations.
>>>
>>> Right, and that's what's provided here.
>>> * Each VLAN has port membership map (net_bridge_vlan.portgroup).
>>> * Each port has a list of vlans configured as well
>>> (net_port_vlan.vlan_list).
>>> * Each port also has a single vlan that can be untagged
>>> (net_bridge_port.untagged).
>>> * The bridge also has a single untagged vlan (net_bridge.untagged)
>>>
>>> The limitation (in switches as well) is that only a single VLAN
>>> may be untagged on any 1 port.
>>
>> Switches usually allow you to configure each port's egress policy per
>> vlan, and allow you to configure multiple vlans to _egress_ untagged
>> on a port.
>>
>>> If you have more then 1, you don't know
>>> which VLAN the untagged traffic belongs to.
>>
>> The port's PVID uniquely determines VID to associate with the frame
>> during _ingress_ on that port - in the case frame arrived untagged.
>>
>> This is unrelated to whether a frame having a specific VID would _egress_
>> tagged or untagged on that port.
>>
>
>
> Ahh... I see what you mean. You would like to separate
> ingress policy and egress policy with regard to how tags are applied...
> I haven't seen that type of config before.
>
> I did say "Basic VLAN support". :)
>
> In this set of patches ingress and egress policies are hardcoded the
> same...
>
> So, consider that what I am calling "untagged" in this series is
> really vlan associated with PVID. To change the egress policy, we
> could add an untagged bitmap into the vlan. Then the bitmap from the
> vlan would determine the egress policy. If the port is in the "tagged"
> bitmap, frame leaves tagged. If the port is in the "untagged" bitmap,
> frame leaves untagged.
>
> The code to make this would would be simple enough. The more
> interesting part would be the configuration :)
Actually, this looks much simpler then I originally thought. I think I
might have something half-baked tomorrow.
-vlad
>
>
>>>> Personally, I'd prefer a fully flexible vlan bridge allowing all sorts
>>>> of configurations (as available in 802.1q switches).
>>>>
>>>> What's the reason limiting such configurations?
>>>
>>> So, what do you see that's missing?
>>
>
> [ snip good example ]
>
>>
>> The bridge constructs needed for supporting such setups are:
>> - per port: PVID
>> - per VLAN: port membership map
>> - per VLAN: port egress policy map
>
> Ok, so from above, membership map is the exiting port_bitmap. Egress
> policy map could be new untagged_bitmap. We wouldn't need a tagged
> policy map since a port can't be "in egress policy, but not in
> membership map".
>
> Membership port_bitmap is consulted on egress for basic forward/drop
> decision (just as it is now). Egress policy (untagged bitmap) is
> consulted to see how the forwarding is done.
>
> Sounds about right? If so, I could probably work something up.
> Will probably leave the configuration for later as that might take a bit
> longer to figure out.
>
> -vlad
>
>>
>> I agree, tools other than a vlan bridge may implement such setups, but
>> using the vlan bridge would be preferred, mainly due to the simplicity.
>>
>> Regards,
>> Shmulik
>>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH V2 00/12] Add basic VLAN support to bridges
2012-12-19 20:03 ` Vlad Yasevich
2012-12-19 22:59 ` Vlad Yasevich
@ 2012-12-20 7:00 ` Shmulik Ladkani
1 sibling, 0 replies; 35+ messages in thread
From: Shmulik Ladkani @ 2012-12-20 7:00 UTC (permalink / raw)
To: vyasevic; +Cc: netdev, shemminger, davem, or.gerlitz, jhs, mst
Hi Vlad,
On Wed, 19 Dec 2012 15:03:36 -0500 Vlad Yasevich <vyasevic@redhat.com> wrote:
> > The port's PVID uniquely determines VID to associate with the frame
> > during _ingress_ on that port - in the case frame arrived untagged.
> >
> > This is unrelated to whether a frame having a specific VID would _egress_
> > tagged or untagged on that port.
>
> Ahh... I see what you mean. You would like to separate
> ingress policy and egress policy with regard to how tags are applied...
Exactly.
Those are two different things; sometimes their configuration collide,
sometimes not.
> > The bridge constructs needed for supporting such setups are:
> > - per port: PVID
> > - per VLAN: port membership map
> > - per VLAN: port egress policy map
>
> Ok, so from above, membership map is the exiting port_bitmap.
Ok.
> Egress policy map could be new untagged_bitmap. We wouldn't need a tagged
> policy map since a port can't be "in egress policy, but not in
> membership map".
Yes, that is correct.
However I wouldn't call it "untagged_bitmap".
The name might suggest that "egress untagged" is an anomaly, where
"normal" behavior is egress tagged.
But as said, both are valid, its just a matter of configuration.
You basically need one more bit for each member port, stating
egress tagged/untagged.
> Sounds about right? If so, I could probably work something up.
Yes, looking forward to review the code.
P.S.
Sorry for late spotting this; I don't follow net-dev regularly.
I hope to take a look at the code soon, see if I have any meaningful
comments.
Regards,
Shmulik
^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2012-12-20 7:08 UTC | newest]
Thread overview: 35+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-18 19:00 [PATCH V2 00/12] Add basic VLAN support to bridges Vlad Yasevich
2012-12-18 19:00 ` [PATCH V2 01/12] bridge: Add vlan filtering infrastructure Vlad Yasevich
2012-12-18 21:13 ` Eric Dumazet
2012-12-18 21:26 ` Vlad Yasevich
2012-12-18 19:00 ` [PATCH V2 02/12] bridge: Validate that vlan is permitted on ingress Vlad Yasevich
2012-12-18 19:00 ` [PATCH V2 03/12] bridge: Verify that a vlan is allowed to egress on give port Vlad Yasevich
2012-12-18 19:00 ` [PATCH V2 04/12] bridge: Cache vlan in the cb for faster egress lookup Vlad Yasevich
2012-12-18 19:00 ` [PATCH V2 05/12] bridge: Add vlan to unicast fdb entries Vlad Yasevich
2012-12-18 19:00 ` [PATCH V2 06/12] bridge: Add vlan id to multicast groups Vlad Yasevich
2012-12-18 19:00 ` [PATCH V2 07/12] bridge: Add netlink interface to configure vlans on bridge ports Vlad Yasevich
2012-12-18 19:00 ` [PATCH V2 08/12] bridge: Add vlan support to static neighbors Vlad Yasevich
2012-12-18 19:01 ` [PATCH V2 09/12] bridge: Add the ability to configure untagged vlans Vlad Yasevich
2012-12-18 23:01 ` Michael S. Tsirkin
2012-12-18 23:03 ` Vlad Yasevich
2012-12-18 23:10 ` Michael S. Tsirkin
2012-12-19 14:50 ` Vlad Yasevich
2012-12-18 23:04 ` Michael S. Tsirkin
2012-12-19 1:06 ` Vlad Yasevich
2012-12-18 19:01 ` [PATCH V2 10/12] bridge: Implement untagged vlan handling Vlad Yasevich
2012-12-18 19:01 ` [PATCH V2 11/12] bridge: Dump vlan information from a bridge port Vlad Yasevich
2012-12-18 19:01 ` [PATCH V2 12/12] bridge: Add vlan support for local fdb entries Vlad Yasevich
2012-12-18 22:32 ` [PATCH V2 00/12] Add basic VLAN support to bridges Jiri Pirko
2012-12-18 22:46 ` Vlad Yasevich
2012-12-19 8:27 ` Jiri Pirko
2012-12-19 16:25 ` Vlad Yasevich
2012-12-19 17:04 ` Thomas Graf
2012-12-19 17:11 ` Vlad Yasevich
2012-12-19 17:19 ` Jiri Pirko
2012-12-19 17:20 ` Thomas Graf
2012-12-19 8:10 ` Shmulik Ladkani
2012-12-19 14:13 ` Vlad Yasevich
2012-12-19 19:37 ` Shmulik Ladkani
2012-12-19 20:03 ` Vlad Yasevich
2012-12-19 22:59 ` Vlad Yasevich
2012-12-20 7:00 ` Shmulik Ladkani
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).