Discussion:
[PATCH net-next V6 01/14] vlan: wrap hw-acceleration calls in separate functions.
(too old to reply)
Vlad Yasevich
2013-01-16 18:17:56 UTC
Permalink
Wrap VLAN hardware acceleration calls into separate functions. This way
other code can re-use it.

Signed-off-by: Vlad Yasevich <***@redhat.com>
---
include/linux/if_vlan.h | 21 ++++++++++++
net/8021q/vlan.c | 4 +--
net/8021q/vlan_core.c | 81 +++++++++++++++++++++++++++++++++++++---------
3 files changed, 87 insertions(+), 19 deletions(-)

diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index d06cc5c..b658eda 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -93,6 +93,10 @@ extern u16 vlan_dev_vlan_id(const struct net_device *dev);
extern bool vlan_do_receive(struct sk_buff **skb);
extern struct sk_buff *vlan_untag(struct sk_buff *skb);

+extern bool vlan_hw_buggy(const struct net_device *dev);
+extern int vlan_vid_add_hw(struct net_device *dev, unsigned short vid);
+extern int vlan_vid_del_hw(struct net_device *dev, unsigned short vid);
+
extern int vlan_vid_add(struct net_device *dev, unsigned short vid);
extern void vlan_vid_del(struct net_device *dev, unsigned short vid);

@@ -155,6 +159,23 @@ static inline bool vlan_uses_dev(const struct net_device *dev)
{
return false;
}
+
+static inline bool vlan_hw_buggy(const struct net_device *dev)
+{
+ return false;
+}
+
+static inline int vlan_vid_add_hw(struct net_device *dev,
+ unsigned short vid)
+{
+ return 0;
+}
+
+static inline int vlan_vid_del_hw(struct net_device *dev,
+ unsigned short vid)
+{
+ return 0;
+}
#endif

/**
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index babfde9..540d759 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -117,15 +117,13 @@ void unregister_vlan_dev(struct net_device *dev, struct list_head *head)
int vlan_check_real_dev(struct net_device *real_dev, u16 vlan_id)
{
const char *name = real_dev->name;
- const struct net_device_ops *ops = real_dev->netdev_ops;

if (real_dev->features & NETIF_F_VLAN_CHALLENGED) {
pr_info("VLANs not supported on %s\n", name);
return -EOPNOTSUPP;
}

- if ((real_dev->features & NETIF_F_HW_VLAN_FILTER) &&
- (!ops->ndo_vlan_rx_add_vid || !ops->ndo_vlan_rx_kill_vid)) {
+ if (vlan_hw_buggy(real_dev)) {
pr_info("Device %s has buggy VLAN hw accel\n", name);
return -EOPNOTSUPP;
}
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 380440b..d044dd3 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -145,6 +145,63 @@ err_free:
return NULL;
}

+/**
+ * vlan_hw_buggy - Check to see if VLAN hw acceleration is supported.
+ * @dev: netdevice of the lowerdev/hw nic
+ *
+ * Checks to see if HW and driver report VLAN acceleration correctly.
+ */
+bool vlan_hw_buggy(const struct net_device *dev)
+{
+ const struct net_device_ops *ops = dev->netdev_ops;
+
+ if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
+ (!ops->ndo_vlan_rx_add_vid || !ops->ndo_vlan_rx_kill_vid))
+ return true;
+
+ return false;
+}
+EXPORT_SYMBOL(vlan_hw_buggy);
+
+/**
+ * vlan_vid_add_hw - Add the VLAN vid to the HW filter
+ * @dev: netdevice of the lowerdev/hw nic
+ * @vid: vlan id.
+ *
+ * Inserts the vid into the HW vlan filter table if hw supports it.
+ */
+int vlan_vid_add_hw(struct net_device *dev, unsigned short vid)
+{
+ const struct net_device_ops *ops = dev->netdev_ops;
+ int err = 0;
+
+ if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
+ ops->ndo_vlan_rx_add_vid)
+ err = ops->ndo_vlan_rx_add_vid(dev, vid);
+
+ return err;
+}
+EXPORT_SYMBOL(vlan_vid_add_hw);
+
+/**
+ * vlan_vid_del_hw - Delete the VLAN vid from the HW filter
+ * @dev: netdevice of the lowerdev/hw nic
+ * @vid: vlan id.
+ *
+ * Delete the vid from the HW vlan filter table if hw supports it.
+ */
+int vlan_vid_del_hw(struct net_device *dev, unsigned short vid)
+{
+ const struct net_device_ops *ops = dev->netdev_ops;
+ int err = 0;
+
+ if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
+ ops->ndo_vlan_rx_kill_vid)
+ err = ops->ndo_vlan_rx_add_vid(dev, vid);
+
+ return err;
+}
+EXPORT_SYMBOL(vlan_vid_del_hw);

/*
* vlan info and vid list
@@ -216,7 +273,6 @@ static int __vlan_vid_add(struct vlan_info *vlan_info, unsigned short vid,
struct vlan_vid_info **pvid_info)
{
struct net_device *dev = vlan_info->real_dev;
- const struct net_device_ops *ops = dev->netdev_ops;
struct vlan_vid_info *vid_info;
int err;

@@ -224,13 +280,10 @@ static int __vlan_vid_add(struct vlan_info *vlan_info, unsigned short vid,
if (!vid_info)
return -ENOMEM;

- if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
- ops->ndo_vlan_rx_add_vid) {
- err = ops->ndo_vlan_rx_add_vid(dev, vid);
- if (err) {
- kfree(vid_info);
- return err;
- }
+ err = vlan_vid_add_hw(dev, vid);
+ if (err) {
+ kfree(vid_info);
+ return err;
}
list_add(&vid_info->list, &vlan_info->vid_list);
vlan_info->nr_vids++;
@@ -278,17 +331,13 @@ static void __vlan_vid_del(struct vlan_info *vlan_info,
struct vlan_vid_info *vid_info)
{
struct net_device *dev = vlan_info->real_dev;
- const struct net_device_ops *ops = dev->netdev_ops;
unsigned short vid = vid_info->vid;
int err;

- if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
- ops->ndo_vlan_rx_kill_vid) {
- err = ops->ndo_vlan_rx_kill_vid(dev, vid);
- if (err) {
- pr_warn("failed to kill vid %d for device %s\n",
- vid, dev->name);
- }
+ err = vlan_vid_del_hw(dev, vid);
+ if (err) {
+ pr_warn("failed to kill vid %d for device %s\n",
+ vid, dev->name);
}
list_del(&vid_info->list);
kfree(vid_info);
--
1.7.7.6
Vlad Yasevich
2013-01-16 18:18:01 UTC
Permalink
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 <***@redhat.com>
---
include/linux/netdevice.h | 2 +
include/uapi/linux/if_bridge.h | 9 +++
net/bridge/br_device.c | 1 +
net/bridge/br_if.c | 1 +
net/bridge/br_netlink.c | 134 ++++++++++++++++++++++++++++++++++-----
net/bridge/br_private.h | 1 +
net/core/rtnetlink.c | 72 +++++++++++++++++++++
7 files changed, 202 insertions(+), 18 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 0209ac3..3a68563 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1017,6 +1017,8 @@ struct net_device_ops {
int (*ndo_bridge_getlink)(struct sk_buff *skb,
u32 pid, u32 seq,
struct net_device *dev);
+ int (*ndo_bridge_dellink)(struct net_device *dev,
+ struct nlmsghdr *nlh);
int (*ndo_change_carrier)(struct net_device *dev,
bool new_carrier);
};
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 5db2975..3ca9817 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -108,15 +108,24 @@ 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)

+#define BRIDGE_VLAN_INFO_MASTER (1<<0) /* Operate on Bridge device as well */
+
+struct bridge_vlan_info {
+ u16 flags;
+ u16 vid;
+};
+
/* Bridge multicast database attributes
* [MDBA_MDB] = {
* [MDBA_MDB_ENTRY] = {
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 6b8f816..0802f86 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -317,6 +317,7 @@ static const struct net_device_ops br_netdev_ops = {
.ndo_fdb_dump = br_fdb_dump,
.ndo_bridge_getlink = br_getlink,
.ndo_bridge_setlink = br_setlink,
+ .ndo_bridge_dellink = br_dellink,
};

static void br_dev_free(struct net_device *dev)
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 2427a21..48dcd6c 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 caa3f69..75f368c 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"
@@ -119,10 +120,14 @@ nla_put_failure:
*/
void br_ifinfo_notify(int event, struct net_bridge_port *port)
{
- struct net *net = dev_net(port->dev);
+ struct net *net;
struct sk_buff *skb;
int err = -ENOBUFS;

+ if (!port)
+ return;
+
+ net = dev_net(port->dev);
br_debug(port->br, "port %u(%s) event %d\n",
(unsigned int)port->port_no, port->dev->name, event);

@@ -144,6 +149,7 @@ errout:
rtnl_set_sk_err(net, RTNLGRP_LINK, err);
}

+
/*
* Dump information about all ports, in response to GETLINK
*/
@@ -162,6 +168,59 @@ 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,
+ int cmd)
+{
+ struct nlattr *tb[IFLA_BRIDGE_MAX+1];
+ int err = 0;
+
+ err = nla_parse_nested(tb, IFLA_BRIDGE_MAX, af_spec, ifla_br_policy);
+ if (err)
+ return err;
+
+ if (tb[IFLA_BRIDGE_VLAN_INFO]) {
+ struct net_port_vlans *v;
+ struct bridge_vlan_info *vinfo;
+
+ vinfo = nla_data(tb[IFLA_BRIDGE_VLAN_INFO]);
+
+ if (vinfo->vid >= VLAN_N_VID)
+ return -EINVAL;
+
+ v = (p) ? &p->vlan_info : &br->vlan_info;
+
+ switch (cmd) {
+ case RTM_SETLINK:
+ err = nbp_vlan_add(v, vinfo->vid);
+ if (err)
+ break;
+ if (p && (vinfo->flags & BRIDGE_VLAN_INFO_MASTER)) {
+ err = nbp_vlan_add(&p->br->vlan_info,
+ vinfo->vid);
+ }
+ break;
+
+ case RTM_DELLINK:
+ nbp_vlan_delete(v, vinfo->vid);
+ if (p && (vinfo->flags & BRIDGE_VLAN_INFO_MASTER))
+ nbp_vlan_delete(&p->br->vlan_info, vinfo->vid);
+
+ 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 },
@@ -241,6 +300,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;
@@ -248,38 +308,76 @@ 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, RTM_SETLINK);
}

if (err == 0)
br_ifinfo_notify(RTM_NEWLINK, p);

+out:
return err;
}

+/* Delete port information */
+int br_dellink(struct net_device *dev, struct nlmsghdr *nlh)
+{
+ struct ifinfomsg *ifm;
+ struct nlattr *afspec;
+ struct net_bridge_port *p;
+ int err;
+
+ ifm = nlmsg_data(nlh);
+
+ afspec = nlmsg_find_attr(nlh, sizeof(*ifm), IFLA_AF_SPEC);
+ if (!afspec)
+ return 0;
+
+ p = br_port_get_rtnl(dev);
+ /* We want to accept dev as bridge itself as well */
+ if (!p && !(dev->priv_flags & IFF_EBRIDGE))
+ return -EINVAL;
+
+ err = br_afspec((struct net_bridge *)netdev_priv(dev), p,
+ afspec, RTM_DELLINK);
+
+ return err;
+}
static int br_validate(struct nlattr *tb[], struct nlattr *data[])
{
if (tb[IFLA_ADDRESS]) {
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 7960681..1e7a202 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -718,6 +718,7 @@ extern int br_netlink_init(void);
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_dellink(struct net_device *dev, struct nlmsghdr *nlmsg);
extern int br_getlink(struct sk_buff *skb, u32 pid, u32 seq,
struct net_device *dev);

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 9a419b0..1aba89f 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2470,6 +2470,77 @@ out:
return err;
}

+static int rtnl_bridge_dellink(struct sk_buff *skb, struct nlmsghdr *nlh,
+ void *arg)
+{
+ struct net *net = sock_net(skb->sk);
+ struct ifinfomsg *ifm;
+ struct net_device *dev;
+ struct nlattr *br_spec, *attr = NULL;
+ int rem, err = -EOPNOTSUPP;
+ u16 oflags, flags = 0;
+ bool have_flags = false;
+
+ if (nlmsg_len(nlh) < sizeof(*ifm))
+ return -EINVAL;
+
+ ifm = nlmsg_data(nlh);
+ if (ifm->ifi_family != AF_BRIDGE)
+ return -EPFNOSUPPORT;
+
+ dev = __dev_get_by_index(net, ifm->ifi_index);
+ if (!dev) {
+ pr_info("PF_BRIDGE: RTM_SETLINK with unknown ifindex\n");
+ return -ENODEV;
+ }
+
+ br_spec = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_AF_SPEC);
+ if (br_spec) {
+ nla_for_each_nested(attr, br_spec, rem) {
+ if (nla_type(attr) == IFLA_BRIDGE_FLAGS) {
+ have_flags = true;
+ flags = nla_get_u16(attr);
+ break;
+ }
+ }
+ }
+
+ oflags = flags;
+
+ if (!flags || (flags & BRIDGE_FLAGS_MASTER)) {
+ struct net_device *br_dev = netdev_master_upper_dev_get(dev);
+
+ if (!br_dev || !br_dev->netdev_ops->ndo_bridge_dellink) {
+ err = -EOPNOTSUPP;
+ goto out;
+ }
+
+ err = br_dev->netdev_ops->ndo_bridge_dellink(dev, nlh);
+ if (err)
+ goto out;
+
+ flags &= ~BRIDGE_FLAGS_MASTER;
+ }
+
+ if ((flags & BRIDGE_FLAGS_SELF)) {
+ if (!dev->netdev_ops->ndo_bridge_dellink)
+ err = -EOPNOTSUPP;
+ else
+ err = dev->netdev_ops->ndo_bridge_dellink(dev, nlh);
+
+ if (!err)
+ flags &= ~BRIDGE_FLAGS_SELF;
+ }
+
+ if (have_flags)
+ memcpy(nla_data(attr), &flags, sizeof(flags));
+ /* Generate event to notify upper layer of bridge change */
+ if (!err)
+ err = rtnl_bridge_notify(dev, oflags);
+out:
+ return err;
+}
+
/* Protected by RTNL sempahore. */
static struct rtattr **rta_buf;
static int rtattr_max;
@@ -2653,6 +2724,7 @@ void __init rtnetlink_init(void)
rtnl_register(PF_BRIDGE, RTM_GETNEIGH, NULL, rtnl_fdb_dump, NULL);

rtnl_register(PF_BRIDGE, RTM_GETLINK, NULL, rtnl_bridge_getlink, NULL);
+ rtnl_register(PF_BRIDGE, RTM_DELLINK, rtnl_bridge_dellink, NULL, NULL);
rtnl_register(PF_BRIDGE, RTM_SETLINK, rtnl_bridge_setlink, NULL, NULL);
}
--
1.7.7.6
Cong Wang
2013-01-17 04:54:37 UTC
Permalink
Post by Vlad Yasevich
+
+struct bridge_vlan_info {
+ u16 flags;
+ u16 vid;
+};
I think you have to use __u16 in UAPI headers.
David Miller
2013-01-17 05:52:43 UTC
Permalink
From: Cong Wang <***@gmail.com>
Date: Thu, 17 Jan 2013 04:54:37 +0000 (UTC)
Post by Cong Wang
Post by Vlad Yasevich
+
+struct bridge_vlan_info {
+ u16 flags;
+ u16 vid;
+};
I think you have to use __u16 in UAPI headers.
That's right.
Vlad Yasevich
2013-01-16 18:17:59 UTC
Permalink
When bridge forwards a frame, make sure that a frame is allowed
to egress on that port.

Signed-off-by: Vlad Yasevich <***@redhat.com>
---
net/bridge/br_forward.c | 1 +
net/bridge/br_input.c | 10 ++++++++++
net/bridge/br_private.h | 9 +++++++++
net/bridge/br_vlan.c | 18 ++++++++++++++++++
4 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 02015a5..d740b52 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -31,6 +31,7 @@ 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->vlan_info, skb) &&
p->state == BR_STATE_FORWARDING);
}

diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index cc959fa..9a1c4e3 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -35,6 +35,16 @@ static int br_pass_frame_up(struct sk_buff *skb)
brstats->rx_bytes += skb->len;
u64_stats_update_end(&brstats->syncp);

+ /* Bridge is just like any other port. Make sure the
+ * packet is allowed except in promisc modue when someone
+ * may be running packet capture.
+ */
+ if (!(brdev->flags & IFF_PROMISC) &&
+ !br_allowed_egress(&br->vlan_info, skb)) {
+ kfree_skb(skb);
+ return NET_RX_DROP;
+ }
+
indev = skb->dev;
skb->dev = brdev;

diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 7018546..39e70af 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -583,6 +583,8 @@ static inline bool br_multicast_is_router(struct net_bridge *br)
/* br_vlan.c */
#ifdef CONFIG_BRIDGE_VLAN_FILTERING
extern bool br_allowed_ingress(struct net_port_vlans *v, struct sk_buff *skb);
+extern bool br_allowed_egress(const struct net_port_vlans *v,
+ const struct sk_buff *skb);
extern struct net_bridge_vlan *br_vlan_find(struct net_bridge *br, u16 vid);
extern void br_vlan_flush(struct net_bridge *br);
extern int nbp_vlan_add(struct net_port_vlans *v, u16 vid);
@@ -598,6 +600,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;

@@ -611,6 +614,12 @@ static inline bool br_allowed_ingress(struct net_port_vlans *v,
return true;
}

+static inline bool br_allowed_egress(const struct net_port_vlans *v,
+ const struct sk_buff *skb)
+{
+ return true;
+}
+
static inline struct net_bridge_vlan *br_vlan_find(struct net_bridge *br,
u16 vid)
{
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 72b4892..a4f7c15 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -25,6 +25,24 @@ bool br_allowed_ingress(struct net_port_vlans *v, struct sk_buff *skb)
return false;
}

+/* Called under RCU. */
+bool br_allowed_egress(const struct net_port_vlans *v,
+ const struct sk_buff *skb)
+{
+ struct net_port_vlan *pve;
+ u16 vid;
+
+ if (list_empty(&v->vlan_list))
+ return true;
+
+ vid = br_get_vlan(skb);
+ pve = nbp_vlan_find(v, vid);
+ if (pve)
+ return true;
+
+ return false;
+}
+
static void br_vlan_destroy(struct net_bridge_vlan *vlan)
{
if (!bitmap_empty(vlan->port_bitmap, PORT_BITMAP_LEN)) {
--
1.7.7.6
Vlad Yasevich
2013-01-16 18:18:03 UTC
Permalink
This patch implements the ingress/egress policy. At ingress,
any untagged traffic is assigned to the PVID. Any tagged
traffic is filtered according to membership bitmap. The vlan
is cached in the skb.

At egress, if we haven't cached the vlan (traffic arrived on vlan
unaware interface), we try to find the vlan for egress port and
cache it. If the egress port is vlan unaware, packet is forwared
as is.

When we do have the vlan cached, we check to see the "untagged"
bitmap to see if the traffic is supposed to leave untagged. Otherwise
the traffic will leave with the vlan tag set.

Signed-off-by: Vlad Yasevich <***@redhat.com>
---
net/8021q/vlan_core.c | 1 +
net/bridge/br_device.c | 4 +-
net/bridge/br_forward.c | 8 ++++
net/bridge/br_input.c | 8 +++-
net/bridge/br_private.h | 34 +++++++++++----
net/bridge/br_vlan.c | 105 ++++++++++++++++++++++++++++++++++++++++++----
6 files changed, 138 insertions(+), 22 deletions(-)

diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index d044dd3..99dffa6 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -144,6 +144,7 @@ err_free:
kfree_skb(skb);
return NULL;
}
+EXPORT_SYMBOL(vlan_untag);

/**
* vlan_hw_buggy - Check to see if VLAN hw acceleration is supported.
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 0802f86..2d5b63d 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);
+ u16 vid = 0;

rcu_read_lock();
#ifdef CONFIG_BRIDGE_NETFILTER
@@ -45,7 +46,8 @@ 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);

- if (!br_allowed_ingress(&br->vlan_info, skb))
+ memset(BR_INPUT_SKB_CB(skb), 0, sizeof(struct br_input_skb_cb));
+ if (!br_allowed_ingress(&br->vlan_info, skb, &vid))
goto out;

BR_INPUT_SKB_CB(skb)->brdev = dev;
diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index d740b52..6dc9f39 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -64,6 +64,10 @@ int br_forward_finish(struct sk_buff *skb)

static void __br_deliver(const struct net_bridge_port *to, struct sk_buff *skb)
{
+ skb = br_handle_vlan(&to->vlan_info, skb);
+ if (!skb)
+ return;
+
skb->dev = to->dev;

if (unlikely(netpoll_tx_running(to->br->dev))) {
@@ -89,6 +93,10 @@ static void __br_forward(const struct net_bridge_port *to, struct sk_buff *skb)
return;
}

+ skb = br_handle_vlan(&to->vlan_info, 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 9a1c4e3..10f8467 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -45,6 +45,10 @@ static int br_pass_frame_up(struct sk_buff *skb)
return NET_RX_DROP;
}

+ skb = br_handle_vlan(&br->vlan_info, skb);
+ if (!skb)
+ return NET_RX_DROP;
+
indev = skb->dev;
skb->dev = brdev;

@@ -61,11 +65,13 @@ 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 = 0;

if (!p || p->state == BR_STATE_DISABLED)
goto drop;

- if (!br_allowed_ingress(&p->vlan_info, skb))
+ memset(BR_INPUT_SKB_CB(skb), 0, sizeof(struct br_input_skb_cb));
+ if (!br_allowed_ingress(&p->vlan_info, skb, &vid))
goto drop;

/* insert into forwarding database after filtering to avoid spoofing */
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index ecf5f34..573dfab 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -76,6 +76,7 @@ struct net_bridge_vlan {
struct rcu_head rcu;
u16 vid;
unsigned long port_bitmap[PORT_BITMAP_LEN];
+ unsigned long untagged_bitmap[PORT_BITMAP_LEN];
};

struct net_port_vlan {
@@ -318,6 +319,7 @@ static inline struct net_bridge *vlans_to_bridge(struct net_port_vlans *vlans)
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;
@@ -584,9 +586,12 @@ static inline bool br_multicast_is_router(struct net_bridge *br)

/* br_vlan.c */
#ifdef CONFIG_BRIDGE_VLAN_FILTERING
-extern bool br_allowed_ingress(struct net_port_vlans *v, struct sk_buff *skb);
+extern bool br_allowed_ingress(struct net_port_vlans *v, struct sk_buff *skb,
+ u16 *vid);
extern bool br_allowed_egress(const struct net_port_vlans *v,
const struct sk_buff *skb);
+extern struct sk_buff *br_handle_vlan(const struct net_port_vlans *v,
+ struct sk_buff *skb);
extern struct net_bridge_vlan *br_vlan_find(struct net_bridge *br, u16 vid);
extern void br_vlan_flush(struct net_bridge *br);
extern int nbp_vlan_add(struct net_port_vlans *v, u16 vid, u16 flags);
@@ -595,23 +600,26 @@ extern struct net_port_vlan *nbp_vlan_find(const struct net_port_vlans *v,
u16 vid);
extern void nbp_vlan_flush(struct net_port_vlans *vlans);

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

#else
static inline bool br_allowed_ingress(struct net_port_vlans *v,
- struct sk_buff *skb)
+ struct sk_buff *skb,
+ u16 *vid)
{
return true;
}
@@ -622,6 +630,12 @@ static inline bool br_allowed_egress(const struct net_port_vlans *v,
return true;
}

+static inline struct sk_buff *br_handle_vlan(const struct net_port_vlans *v,
+ struct sk_buff *skb)
+{
+ return skb;
+}
+
static inline struct net_bridge_vlan *br_vlan_find(struct net_bridge *br,
u16 vid)
{
@@ -653,7 +667,7 @@ static inline void nbp_vlan_flush(struct net_port_vlans *vlans)
{
}

-static inline u16 br_get_vlan(const struct sk_buff *skb)
+static inline u16 br_get_vlan(const struct sk_buff *skb, u16 *tag)
{
return 0;
}
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index f6dc8a7..cbaa6b5 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -6,12 +6,9 @@
#include "br_private.h"

/* Called under RCU */
-bool br_allowed_ingress(struct net_port_vlans *v, struct sk_buff *skb)
+bool br_allowed_ingress(struct net_port_vlans *v, struct sk_buff *skb, u16 *vid)
{
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.
@@ -19,8 +16,28 @@ bool br_allowed_ingress(struct net_port_vlans *v, struct sk_buff *skb)
if (list_empty(&v->vlan_list))
return true;

- vid = br_get_vlan(skb);
- pve = nbp_vlan_find(v, vid);
+ if (br_get_vlan(skb, vid)) {
+ struct net_bridge_vlan *pvlan = rcu_dereference(v->pvlan);
+ u16 pvid;
+
+ /* Frame did not have a tag. See if pvid is set
+ * on this port. That tells us which vlan untagged
+ * traffic belongs to.
+ */
+ if (!pvlan || (pvid = pvlan->vid) == BR_INVALID_VID)
+ return false;
+
+ /* PVID is set on this port. Any untagged ingress
+ * frame is considered to belong to this vlan.
+ */
+ __vlan_hwaccel_put_tag(skb, pvid);
+ BR_INPUT_SKB_CB(skb)->vlan = pvlan;
+ BR_INPUT_SKB_CB(skb)->untagged = 1;
+ return true;
+ }
+
+ /* Frame had a valid vlan tag. Find the VLAN it belongs to */
+ pve = nbp_vlan_find(v, *vid);
if (pve) {
BR_INPUT_SKB_CB(skb)->vlan = rcu_dereference(pve->vlan);
return true;
@@ -35,7 +52,7 @@ bool br_allowed_egress(const struct net_port_vlans *v,
{
struct net_port_vlan *pve;
struct net_bridge_vlan *vlan = NULL;
- u16 vid;
+ u16 vid = 0;

if (list_empty(&v->vlan_list))
return true;
@@ -51,16 +68,84 @@ bool br_allowed_egress(const struct net_port_vlans *v,
}

/* We don't have cached vlan information, so we need to do
- * it the hard way.
+ * it the hard way. Cache the vlan so we can check egress
+ * policy later.
*/
- vid = br_get_vlan(skb);
+ br_get_vlan(skb, &vid);
pve = nbp_vlan_find(v, vid);
- if (pve)
+ if (pve) {
+ BR_INPUT_SKB_CB(skb)->vlan = rcu_dereference(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)
+{
+ if (skb->protocol != htons(ETH_P_8021Q)) {
+ skb->vlan_tci = 0;
+ return skb;
+ }
+
+ skb->vlan_tci = 0;
+ skb = vlan_untag(skb);
+ if (skb)
+ skb->vlan_tci = 0;
+
+ return skb;
+}
+
+struct sk_buff *br_handle_vlan(const struct net_port_vlans *pv,
+ struct sk_buff *skb)
+{
+ struct net_bridge_vlan *skb_vlan = BR_INPUT_SKB_CB(skb)->vlan;
+
+ /* If there is no vlan policy on the egress port,
+ * leave according to ingress policy.
+ */
+ if (list_empty(&pv->vlan_list)) {
+ if (BR_INPUT_SKB_CB(skb)->untagged)
+ skb->vlan_tci = 0;
+ goto out;
+ }
+
+ /* Egress policy is provided by the vlan bitmaps. We
+ * are here since we are allowed to egress according to
+ * the vlan membership. Now consult the untagged bitmap to
+ * see if we should be leaving untagged.
+ */
+ if (test_bit(pv->port_idx, skb_vlan->untagged_bitmap))
+ skb = br_vlan_untag(skb);
+ else {
+ /* Egress policy says "send tagged". If output device is the
+ * bridge, we need to add the VLAN header ourselves since we'll
+ * be going through the RX path. Sending to ports puts
+ * the frame on the TX path and we let dev_hard_start_xmit()
+ * add the header.
+ */
+ if (skb->protocol != htons(ETH_P_8021Q) &&
+ pv->port_idx == 0) {
+ /* 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;
+ }
+ }
+
+out:
+ return skb;
+}
+
static void br_vlan_destroy(struct net_bridge_vlan *vlan)
{
if (!bitmap_empty(vlan->port_bitmap, PORT_BITMAP_LEN)) {
--
1.7.7.6
Vlad Yasevich
2013-01-16 18:18:02 UTC
Permalink
A user may designate a certain vlan as PVID. This means that
any ingress frame that does not contain a vlan tag is assigned to
this vlan and any forwarding decisions are made with this vlan in mind.

Signed-off-by: Vlad Yasevich <***@redhat.com>
---
include/uapi/linux/if_bridge.h | 1 +
net/bridge/br_netlink.c | 4 +-
net/bridge/br_private.h | 5 ++-
net/bridge/br_vlan.c | 70 +++++++++++++++++++++++++++++++++++++++-
4 files changed, 75 insertions(+), 5 deletions(-)

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 3ca9817..c6c30e2 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -120,6 +120,7 @@ enum {
#define IFLA_BRIDGE_MAX (__IFLA_BRIDGE_MAX - 1)

#define BRIDGE_VLAN_INFO_MASTER (1<<0) /* Operate on Bridge device as well */
+#define BRIDGE_VLAN_INFO_PVID (1<<1) /* VLAN is PVID, ingress untagged */

struct bridge_vlan_info {
u16 flags;
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 75f368c..1dbfcf3 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -200,12 +200,12 @@ static int br_afspec(struct net_bridge *br,

switch (cmd) {
case RTM_SETLINK:
- err = nbp_vlan_add(v, vinfo->vid);
+ err = nbp_vlan_add(v, vinfo->vid, vinfo->flags);
if (err)
break;
if (p && (vinfo->flags & BRIDGE_VLAN_INFO_MASTER)) {
err = nbp_vlan_add(&p->br->vlan_info,
- vinfo->vid);
+ vinfo->vid, vinfo->flags);
}
break;

diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 1e7a202..ecf5f34 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -88,6 +88,7 @@ struct net_port_vlan {
struct net_port_vlans {
u16 port_idx;
struct list_head vlan_list;
+ struct net_bridge_vlan __rcu *pvlan;
};

struct net_bridge_fdb_entry
@@ -588,7 +589,7 @@ extern bool br_allowed_egress(const struct net_port_vlans *v,
const struct sk_buff *skb);
extern struct net_bridge_vlan *br_vlan_find(struct net_bridge *br, u16 vid);
extern void br_vlan_flush(struct net_bridge *br);
-extern int nbp_vlan_add(struct net_port_vlans *v, u16 vid);
+extern int nbp_vlan_add(struct net_port_vlans *v, u16 vid, u16 flags);
extern int nbp_vlan_delete(struct net_port_vlans *v, u16 vid);
extern struct net_port_vlan *nbp_vlan_find(const struct net_port_vlans *v,
u16 vid);
@@ -631,7 +632,7 @@ static inline void br_vlan_flush(struct net_bridge *br)
{
}

-static inline int nbp_vlan_add(struct net_port_vlans *v, u16 vid)
+static inline int nbp_vlan_add(struct net_port_vlans *v, u16 vid, u16 flags)
{
return -EINVAL;
}
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 01ebfbc..f6dc8a7 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -157,6 +157,56 @@ void br_vlan_flush(struct net_bridge *br)
}
}

+static int nbp_vlan_add_pvid(struct net_port_vlans *v,
+ struct net_bridge_vlan *vlan)
+{
+ struct net_bridge_vlan *pvlan = rtnl_dereference(v->pvlan);
+
+ if (pvlan == vlan)
+ return 0;
+ else if (pvlan) {
+ /* PVID is already set. Drop the ref
+ * to the old one since we'll be replace it.
+ */
+ br_vlan_put(pvlan);
+ } else if (v->port_idx) {
+ struct net_device *dev = vlans_to_port(v)->dev;
+
+ /* Add vid 0 to filter if filter is available. */
+ if (!vlan_hw_buggy(dev)) {
+ int err = vlan_vid_add_hw(dev, 0);
+ if (err)
+ return err;
+ }
+ }
+
+ br_vlan_hold(vlan);
+ rcu_assign_pointer(v->pvlan, vlan);
+ return 0;
+}
+
+static void nbp_vlan_delete_pvid(struct net_port_vlans *v,
+ struct net_bridge_vlan *vlan)
+{
+ struct net_bridge_vlan *pvlan = rtnl_dereference(v->pvlan);
+
+ if (pvlan != vlan)
+ return;
+
+ if (v->port_idx &&
+ vlan_vid_del_hw(vlans_to_port(v)->dev, 0)) {
+ pr_warn("failed to kill vid 0 for device %s\n",
+ vlans_to_port(v)->dev->name);
+ }
+
+ /* If this VLAN is currently functioning as pvlan, clear it.
+ * It's safe to drop the refcount, since the vlan is still held
+ * by the pve->vlan pointer.
+ */
+ br_vlan_put(vlan);
+ rcu_assign_pointer(v->pvlan, NULL);
+}
+
struct net_port_vlan *nbp_vlan_find(const struct net_port_vlans *v, u16 vid)
{
struct net_port_vlan *pve;
@@ -173,7 +223,7 @@ struct net_port_vlan *nbp_vlan_find(const struct net_port_vlans *v, u16 vid)
}

/* Must be protected by RTNL */
-int nbp_vlan_add(struct net_port_vlans *v, u16 vid)
+int nbp_vlan_add(struct net_port_vlans *v, u16 vid, u16 flags)
{
struct net_port_vlan *pve = NULL;
struct net_bridge_vlan *vlan;
@@ -222,12 +272,22 @@ int nbp_vlan_add(struct net_port_vlans *v, u16 vid)
set_bit(v->port_idx, vlan->port_bitmap);

list_add_tail_rcu(&pve->list, &v->vlan_list);
+
+ if (flags & BRIDGE_VLAN_INFO_PVID) {
+ err = nbp_vlan_add_pvid(v, vlan);
+ if (err)
+ goto del_vlan;
+ }
+
return 0;

clean_up:
kfree(pve);
br_vlan_del(vlan);
return err;
+del_vlan:
+ nbp_vlan_delete(v, vid);
+ return err;
}

/* Must be protected by RTNL */
@@ -235,13 +295,21 @@ int nbp_vlan_delete(struct net_port_vlans *v, u16 vid)
{
struct net_port_vlan *pve;
struct net_bridge_vlan *vlan;
+ struct net_bridge *br;

ASSERT_RTNL();

+ if (v->port_idx)
+ br = vlans_to_port(v)->br;
+ else
+ br = vlans_to_bridge(v);
+
pve = nbp_vlan_find(v, vid);
if (!pve)
return -ENOENT;

+ nbp_vlan_delete_pvid(v, pve->vlan);
+
if (v->port_idx) {
/* A valid port index means this is a port.
* Remove VLAN from the port device filter if it is supported.
--
1.7.7.6
Vlad Yasevich
2013-01-16 18:18:04 UTC
Permalink
Add an ability to configure "untagged" egress policy to
the VLAN information of the bridge. The policy is by a
flag and is represented as a port bitmap per vlan. Frames
that leave a ports in "untagged" policy bitmap would egress
the port without VLAN header.

Signed-off-by: Vlad Yasevich <***@redhat.com>
---
include/uapi/linux/if_bridge.h | 1 +
net/bridge/br_vlan.c | 49 +++++++++++++++++++++++++++++++--------
2 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index c6c30e2..f1bf8d3 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -121,6 +121,7 @@ enum {

#define BRIDGE_VLAN_INFO_MASTER (1<<0) /* Operate on Bridge device as well */
#define BRIDGE_VLAN_INFO_PVID (1<<1) /* VLAN is PVID, ingress untagged */
+#define BRIDGE_VLAN_INFO_UNTAGGED (1<<2) /* VLAN egresses untagged */

struct bridge_vlan_info {
u16 flags;
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index cbaa6b5..58cfbb4 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -292,6 +292,32 @@ static void nbp_vlan_delete_pvid(struct net_port_vlans *v,
rcu_assign_pointer(v->pvlan, NULL);
}

+static int nbp_vlan_do_flags(struct net_port_vlans *v,
+ struct net_bridge_vlan *vlan,
+ u16 flags)
+{
+ int err = 0;
+
+ if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
+ set_bit(v->port_idx, vlan->untagged_bitmap);
+
+ if (flags & BRIDGE_VLAN_INFO_PVID)
+ err = nbp_vlan_add_pvid(v, vlan);
+
+ return err;
+}
+
+static void nbp_vlan_undo_flags(struct net_port_vlans *v,
+ struct net_bridge_vlan *vlan,
+ u16 flags)
+{
+ if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
+ clear_bit(v->port_idx, vlan->untagged_bitmap);
+
+ if (flags & BRIDGE_VLAN_INFO_PVID)
+ nbp_vlan_delete_pvid(v, vlan);
+}
+
struct net_port_vlan *nbp_vlan_find(const struct net_port_vlans *v, u16 vid)
{
struct net_port_vlan *pve;
@@ -326,10 +352,14 @@ int nbp_vlan_add(struct net_port_vlans *v, u16 vid, u16 flags)
return -ENOMEM;

/* Check to see if this port is already part of the vlan. If
- * it is, there is nothing more to do.
+ * it is, handle any flags and return.
*/
- if (test_bit(v->port_idx, vlan->port_bitmap))
- return -EEXIST;
+ if (test_bit(v->port_idx, vlan->port_bitmap)) {
+ err = nbp_vlan_do_flags(v, vlan, flags);
+ if (err)
+ nbp_vlan_undo_flags(v, vlan, flags);
+ return err;
+ }

/* Create port vlan, link it to bridge vlan list, and add port the
* portgroup.
@@ -358,11 +388,9 @@ int nbp_vlan_add(struct net_port_vlans *v, u16 vid, u16 flags)

list_add_tail_rcu(&pve->list, &v->vlan_list);

- if (flags & BRIDGE_VLAN_INFO_PVID) {
- err = nbp_vlan_add_pvid(v, vlan);
- if (err)
- goto del_vlan;
- }
+ err = nbp_vlan_do_flags(v, vlan, flags);
+ if (err)
+ goto del_vlan;

return 0;

@@ -393,7 +421,9 @@ int nbp_vlan_delete(struct net_port_vlans *v, u16 vid)
if (!pve)
return -ENOENT;

- nbp_vlan_delete_pvid(v, pve->vlan);
+ vlan = rtnl_dereference(pve->vlan);
+ nbp_vlan_undo_flags(v, vlan,
+ BRIDGE_VLAN_INFO_PVID | BRIDGE_VLAN_INFO_UNTAGGED);

if (v->port_idx) {
/* A valid port index means this is a port.
@@ -407,7 +437,6 @@ int nbp_vlan_delete(struct net_port_vlans *v, u16 vid)
}
pve->vid = BR_INVALID_VID;

- vlan = rtnl_dereference(pve->vlan);
rcu_assign_pointer(pve->vlan, NULL);
clear_bit(v->port_idx, vlan->port_bitmap);
br_vlan_put(vlan);
--
1.7.7.6
Vlad Yasevich
2013-01-16 18:18:09 UTC
Permalink
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 <***@redhat.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 +-
include/linux/netdevice.h | 3 +-
include/uapi/linux/rtnetlink.h | 1 +
net/bridge/br_netlink.c | 111 ++++++++++++++++++++++---
net/bridge/br_private.h | 3 +-
net/bridge/br_vlan.c | 2 +
net/core/rtnetlink.c | 16 +++-
7 files changed, 121 insertions(+), 18 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 eb7a45f..178d297 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1017,7 +1017,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);
int (*ndo_bridge_dellink)(struct net_device *dev,
struct nlmsghdr *nlh);
int (*ndo_change_carrier)(struct net_device *dev,
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 7a5eb19..7a2144e 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -630,6 +630,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_netlink.c b/net/bridge/br_netlink.c
index 1dbfcf3..4007813 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -65,15 +65,21 @@ 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_device *dev)
{
- const struct net_bridge *br = port->br;
- const struct net_device *dev = port->dev;
+ const struct net_bridge *br;
struct ifinfomsg *hdr;
struct nlmsghdr *nlh;
u8 operstate = netif_running(dev) ? dev->operstate : IF_OPER_DOWN;

+ if (port)
+ br = port->br;
+ else
+ br = netdev_priv(dev);
+
br_debug(br, "br_fill_info event %d port %s master %s\n",
event, dev->name, br->dev->name);

@@ -99,7 +105,7 @@ static int br_fill_ifinfo(struct sk_buff *skb, const struct net_bridge_port *por
nla_put_u32(skb, IFLA_LINK, dev->iflink)))
goto nla_put_failure;

- if (event == RTM_NEWLINK) {
+ if (event == RTM_NEWLINK && port) {
struct nlattr *nest
= nla_nest_start(skb, IFLA_PROTINFO | NLA_F_NESTED);

@@ -108,6 +114,52 @@ 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;
+ const struct net_port_vlans *v;
+ struct bridge_vlan_info vinfo;
+ struct net_bridge_vlan *vlan;
+ u16 pvid = BR_INVALID_VID;
+
+ if (port)
+ v = &port->vlan_info;
+ else
+ v = &br->vlan_info;
+
+ if (list_empty(&v->vlan_list))
+ goto done;
+
+ vlan = rtnl_dereference(v->pvlan);
+ if (vlan)
+ pvid = vlan->vid;
+
+ af = nla_nest_start(skb, IFLA_AF_SPEC);
+ if (!af)
+ goto nla_put_failure;
+
+ list_for_each_entry_rcu(pve, &v->vlan_list, list) {
+ if (pve->vid == BR_INVALID_VID)
+ continue;
+
+ vinfo.vid = pve->vid;
+ vinfo.flags = 0;
+ if (vinfo.vid == pvid)
+ vinfo.flags |= BRIDGE_VLAN_INFO_PVID;
+ vlan = rtnl_dereference(pve->vlan);
+ if (test_bit(v->port_idx, vlan->untagged_bitmap))
+ vinfo.flags |= BRIDGE_VLAN_INFO_UNTAGGED;
+
+ if (nla_put(skb, IFLA_BRIDGE_VLAN_INFO,
+ sizeof(vinfo), &vinfo))
+ goto nla_put_failure;
+ }
+
+ nla_nest_end(skb, af);
+ }
+
+done:
return nlmsg_end(skb, nlh);

nla_put_failure:
@@ -135,7 +187,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, port->dev);
if (err < 0) {
/* -EMSGSIZE implies BUG in br_nlmsg_size() */
WARN_ON(err == -EMSGSIZE);
@@ -154,16 +206,17 @@ 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);

- /* not a bridge port */
- if (!port)
+ /* not a bridge port and */
+ if (!port && !(filter_mask & RTEXT_FILTER_BRVLAN))
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, dev);
out:
return err;
}
@@ -390,6 +443,26 @@ 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_port_vlans *pv;
+
+ if (br_port_exists(dev))
+ pv = &(br_port_get_rcu(dev))->vlan_info;
+ else if (dev->priv_flags & IFF_EBRIDGE)
+ pv = &((struct net_bridge *)netdev_priv(dev))->vlan_info;
+ else
+ return 0;
+
+ /* Each VLAN is returned in bridge_vlan_info along with flags */
+ return pv->num_vlans * nla_total_size(sizeof(struct bridge_vlan_info));
+}
+
+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),
@@ -400,10 +473,26 @@ 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)
+ goto out;
+
+ err = rtnl_link_register(&br_link_ops);
+ if (err)
+ goto out_af;
+
+ return 0;
+
+out_af:
+ rtnl_af_unregister(&br_af_ops);
+out:
+ return err;
}

void __exit br_netlink_fini(void)
{
+ rtnl_af_unregister(&br_af_ops);
rtnl_link_unregister(&br_link_ops);
}
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 46f8bdc..2dd91a5 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -89,6 +89,7 @@ struct net_port_vlan {

struct net_port_vlans {
u16 port_idx;
+ u16 num_vlans;
struct list_head vlan_list;
struct net_bridge_vlan __rcu *pvlan;
};
@@ -741,7 +742,7 @@ 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_dellink(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/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 40ef79f..efb3534 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -387,6 +387,7 @@ int nbp_vlan_add(struct net_port_vlans *v, u16 vid, u16 flags)
set_bit(v->port_idx, vlan->port_bitmap);

list_add_tail_rcu(&pve->list, &v->vlan_list);
+ v->num_vlans++;

err = nbp_vlan_do_flags(v, vlan, flags);
if (err)
@@ -463,6 +464,7 @@ int nbp_vlan_delete(struct net_port_vlans *v, u16 vid)
br_vlan_put(vlan);

list_del_rcu(&pve->list);
+ v->num_vlans--;
kfree_rcu(pve, rcu);

br_vlan_del(vlan);
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 87b07d8..257b73e 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 (br_dev && br_dev->netdev_ops->ndo_bridge_getlink) {
if (idx >= cb->args[0] &&
br_dev->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)) &&
br_dev && br_dev->netdev_ops->ndo_bridge_getlink) {
- err = br_dev->netdev_ops->ndo_bridge_getlink(skb, 0, 0, dev);
+ err = br_dev->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
Vlad Yasevich
2013-01-16 18:18:07 UTC
Permalink
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 <***@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 | 142 ++++++++++++++++++++++---
net/bridge/br_private.h | 2 +-
net/core/rtnetlink.c | 23 +++--
8 files changed, 146 insertions(+), 30 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 1047e58..3e16d74 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -564,7 +564,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 40f2cc1..c72540b 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 3a68563..eb7a45f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1005,6 +1005,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..90d0bc9 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,41 +679,87 @@ 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->vlan_info, 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_info.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_info.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 *br, 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;

- fdb_delete(p->br, fdb);
+ fdb_delete(br, fdb);
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->br, 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 +767,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->vlan_info, 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_info.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_info.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 908a72a..287374c 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -426,7 +426,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 1aba89f..87b07d8 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2122,7 +2122,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;
@@ -2130,8 +2130,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) {
@@ -2145,13 +2146,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 */
@@ -2161,7 +2166,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
const struct net_device_ops *ops = br_dev->netdev_ops;

if (ops->ndo_fdb_del)
- err = ops->ndo_fdb_del(ndm, dev, addr);
+ err = ops->ndo_fdb_del(ndm, tb, dev, addr);

if (err)
goto out;
@@ -2171,7 +2176,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
Cong Wang
2013-01-17 05:16:02 UTC
Permalink
Post by Vlad Yasevich
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index a244efc..90d0bc9 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;
Why not just nla_put_u16()?
Post by Vlad Yasevich
+
+ /* 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_info.vlan_list, list) {
+ err &= __br_fdb_delete(p, addr, pve->vid);
+ }
'&=' or '|='?
Vlad Yasevich
2013-01-16 18:18:08 UTC
Permalink
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 <***@redhat.com>
---
net/bridge/br_fdb.c | 62 +++++++++++++++++++++++++++++++++++-----------
net/bridge/br_if.c | 2 +-
net/bridge/br_private.h | 4 ++-
net/bridge/br_vlan.c | 25 +++++++++++++++++-
4 files changed, 74 insertions(+), 19 deletions(-)

diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 90d0bc9..942fe49 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_info.vlan_list);
int i;

spin_lock_bh(&br->hash_lock);
@@ -106,10 +107,13 @@ 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->vlan_info,
+ vid)) {
f->dst = op;
goto insert;
}
@@ -117,27 +121,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 +412,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 +433,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 +444,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,8 +742,8 @@ out:
return err;
}

-static int fdb_delete_by_addr(struct net_bridge *br, const u8 *addr,
- u16 vlan)
+int fdb_delete_by_addr(struct net_bridge *br, const u8 *addr,
+ u16 vlan)
{
struct hlist_head *head = &br->hash[br_mac_hash(addr, vlan)];
struct net_bridge_fdb_entry *fdb;
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 48dcd6c..45b4ee6 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -400,7 +400,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 287374c..46f8bdc 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -420,11 +420,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,
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 58cfbb4..40ef79f 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -392,6 +392,14 @@ int nbp_vlan_add(struct net_port_vlans *v, u16 vid, u16 flags)
if (err)
goto del_vlan;

+ err = br_fdb_insert(br, p,
+ (p ? p->dev->dev_addr : br->dev->dev_addr), vid);
+ if (err) {
+ br_err(br,
+ "failed insert local address bridge forwarding table\n");
+ goto del_vlan;
+ }
+
return 0;

clean_up:
@@ -409,13 +417,17 @@ int nbp_vlan_delete(struct net_port_vlans *v, u16 vid)
struct net_port_vlan *pve;
struct net_bridge_vlan *vlan;
struct net_bridge *br;
+ unsigned char *addr;

ASSERT_RTNL();

- if (v->port_idx)
+ if (v->port_idx) {
br = vlans_to_port(v)->br;
- else
+ addr = vlans_to_port(v)->dev->dev_addr;
+ } else {
br = vlans_to_bridge(v);
+ addr = br->dev->dev_addr;
+ }

pve = nbp_vlan_find(v, vid);
if (!pve)
@@ -435,6 +447,15 @@ int nbp_vlan_delete(struct net_port_vlans *v, u16 vid)
pr_warn("failed to kill vid %d for device %s\n",
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(&br->hash_lock);
+ fdb_delete_by_addr(br, addr, vid);
+ spin_unlock_bh(&br->hash_lock);
+ }
pve->vid = BR_INVALID_VID;

rcu_assign_pointer(pve->vlan, NULL);
--
1.7.7.6
Vlad Yasevich
2013-01-16 18:18:05 UTC
Permalink
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 <***@redhat.com>
---
include/uapi/linux/if_bridge.h | 2 +-
net/bridge/br_device.c | 2 +-
net/bridge/br_fdb.c | 70 +++++++++++++++++++++++----------------
net/bridge/br_input.c | 9 +++--
net/bridge/br_private.h | 7 +++-
5 files changed, 54 insertions(+), 36 deletions(-)

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index f1bf8d3..8e1fc51 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 2d5b63d..1d42695 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -72,7 +72,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 10f8467..e138037 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -76,7 +76,7 @@ int br_handle_frame_finish(struct sk_buff *skb)

/* 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))
@@ -111,7 +111,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;
@@ -139,8 +140,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, &vid);
+ br_fdb_update(p->br, p, eth_hdr(skb)->h_source, vid);
return 0; /* process further */
}

diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 573dfab..8a30c03 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -103,6 +103,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 {
@@ -411,7 +412,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);
@@ -420,7 +422,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
Vlad Yasevich
2013-01-16 18:18:06 UTC
Permalink
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 <***@redhat.com>
---
net/bridge/br_multicast.c | 69 ++++++++++++++++++++++++++++++++-------------
net/bridge/br_private.h | 1 +
2 files changed, 50 insertions(+), 20 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 5391ca4..b3e3289 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -39,6 +39,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;
@@ -50,16 +52,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

@@ -68,10 +73,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;
@@ -101,24 +106,27 @@ struct net_bridge_mdb_entry *br_mdb_ip_get(struct net_bridge_mdb_htable *mdb,
}

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);
}
@@ -694,7 +702,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;

@@ -703,6 +712,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);
}
@@ -710,7 +720,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;

@@ -719,6 +730,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);
}
@@ -895,10 +907,12 @@ 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;

+ br_get_vlan(skb, &vid);
ih = igmpv3_report_hdr(skb);
num = ntohs(ih->ngrec);
len = sizeof(*ih);
@@ -930,7 +944,7 @@ 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, vid);
if (err)
break;
}
@@ -949,10 +963,12 @@ 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;

+ br_get_vlan(skb, &vid);
icmp6h = icmp6_hdr(skb);
num = ntohs(icmp6h->icmp6_dataun.un_data16[1]);
len = sizeof(*icmp6h);
@@ -990,7 +1006,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,
+ vid);
if (!err)
break;
}
@@ -1074,6 +1091,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) ||
@@ -1108,7 +1126,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, &vid);
+ mp = br_mdb_ip4_get(mlock_dereference(br->mdb, br), group, vid);
if (!mp)
goto out;

@@ -1149,6 +1168,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) ||
@@ -1180,7 +1200,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, &vid);
+ mp = br_mdb_ip6_get(mlock_dereference(br->mdb, br), group, vid);
if (!mp)
goto out;

@@ -1286,7 +1307,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;

@@ -1295,6 +1317,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);
}
@@ -1302,7 +1325,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;

@@ -1311,6 +1335,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);
}
@@ -1326,6 +1351,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)))
@@ -1386,6 +1412,7 @@ static int br_multicast_ipv4_rcv(struct net_bridge *br,

err = 0;

+ br_get_vlan(skb2, &vid);
BR_INPUT_SKB_CB(skb)->igmp = 1;
ih = igmp_hdr(skb2);

@@ -1393,7 +1420,7 @@ 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, vid);
break;
case IGMPV3_HOST_MEMBERSHIP_REPORT:
err = br_ip4_multicast_igmp3_report(br, port, skb2);
@@ -1402,7 +1429,7 @@ 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, vid);
break;
}

@@ -1427,6 +1454,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;
@@ -1510,6 +1538,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,

err = 0;

+ br_get_vlan(skb, &vid);
BR_INPUT_SKB_CB(skb)->igmp = 1;

switch (icmp6_type) {
@@ -1522,7 +1551,7 @@ 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, vid);
break;
}
case ICMPV6_MLD2_REPORT:
@@ -1539,7 +1568,7 @@ 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, vid);
}
}

diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 8a30c03..908a72a 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
Vlad Yasevich
2013-01-16 18:17:57 UTC
Permalink
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 <***@redhat.com>
---
net/bridge/Kconfig | 14 +++
net/bridge/Makefile | 2 +
net/bridge/br_device.c | 5 +
net/bridge/br_if.c | 4 +
net/bridge/br_private.h | 95 ++++++++++++++++++++
net/bridge/br_vlan.c | 223 +++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 343 insertions(+), 0 deletions(-)
create mode 100644 net/bridge/br_vlan.c

diff --git a/net/bridge/Kconfig b/net/bridge/Kconfig
index 6dee7bf..c0283c1 100644
--- a/net/bridge/Kconfig
+++ b/net/bridge/Kconfig
@@ -46,3 +46,17 @@ config BRIDGE_IGMP_SNOOPING
Say N to exclude this support and reduce the binary size.

If unsure, say Y.
+
+config BRIDGE_VLAN_FILTERING
+ bool "VLAN filtering"
+ depends on BRIDGE
+ depends on VLAN_8021Q
+ default y
+ ---help---
+ If you say Y here, then the Ethernet bridge will be able selectively
+ receive and forward traffic based on VLAN information in the packet
+ any VLAN information configured on the bridge port or bridge device.
+
+ Say N to exclude this support and reduce the binary size.
+
+ If unsure, say Y.
diff --git a/net/bridge/Makefile b/net/bridge/Makefile
index e859098..e85498b2f 100644
--- a/net/bridge/Makefile
+++ b/net/bridge/Makefile
@@ -14,4 +14,6 @@ bridge-$(CONFIG_BRIDGE_NETFILTER) += br_netfilter.o

bridge-$(CONFIG_BRIDGE_IGMP_SNOOPING) += br_multicast.o br_mdb.o

+bridge-$(CONFIG_BRIDGE_VLAN_FILTERING) += br_vlan.o
+
obj-$(CONFIG_BRIDGE_NF_EBTABLES) += netfilter/
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index e1bc090..b64b5c8 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -331,6 +331,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);
@@ -353,6 +354,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;
@@ -367,6 +370,8 @@ void br_dev_setup(struct net_device *dev)
br->bridge_hello_time = br->hello_time = 2 * HZ;
br->bridge_forward_delay = br->forward_delay = 15 * HZ;
br->ageing_time = 300 * HZ;
+ br->vlan_info.port_idx = 0;
+ INIT_LIST_HEAD(&br->vlan_info.vlan_list);

br_netfilter_rtable_init(br);
br_stp_timer_init(br);
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 2148d47..2427a21 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -139,6 +139,7 @@ static void del_nbp(struct net_bridge_port *p)

br_ifinfo_notify(RTM_DELLINK, p);

+ nbp_vlan_flush(&p->vlan_info);
br_fdb_delete_by_port(br, p, 1);

list_del_rcu(&p->list);
@@ -170,6 +171,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 +224,8 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
p->flags = 0;
br_init_port(p);
p->state = BR_STATE_DISABLED;
+ p->vlan_info.port_idx = index;
+ INIT_LIST_HEAD(&p->vlan_info.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 8d83be5..615f10c 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,31 @@ struct br_ip
__be16 proto;
};

+#define BR_INVALID_VID (1<<15)
+
+#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 __rcu *vlan;
+ struct rcu_head rcu;
+ u16 vid;
+};
+
+struct net_port_vlans {
+ u16 port_idx;
+ struct list_head vlan_list;
+};
+
struct net_bridge_fdb_entry
{
struct hlist_node hlist;
@@ -156,6 +183,7 @@ struct net_bridge_port
#ifdef CONFIG_NET_POLL_CONTROLLER
struct netpoll *np;
#endif
+ struct net_port_vlans vlan_info;
};

#define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT)
@@ -174,6 +202,16 @@ static inline struct net_bridge_port *br_port_get_rtnl(struct net_device *dev)
rtnl_dereference(dev->rx_handler_data) : NULL;
}

+static inline struct net_bridge_port *vlans_to_port(struct net_port_vlans *vlans)
+{
+ struct net_bridge_port *p = NULL;
+
+ if (vlans->port_idx)
+ p = container_of((vlans), struct net_bridge_port, vlan_info);
+
+ return p;
+}
+
struct br_cpu_netstats {
u64 rx_packets;
u64 rx_bytes;
@@ -260,8 +298,22 @@ 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 net_port_vlans vlan_info;
};

+static inline struct net_bridge *vlans_to_bridge(struct net_port_vlans *vlans)
+{
+ struct net_bridge *br;
+
+ if (!vlans->port_idx)
+ br = container_of((vlans), struct net_bridge, vlan_info);
+ else
+ br = vlans_to_port(vlans)->br;
+
+ return br;
+}
+
struct br_input_skb_cb {
struct net_device *brdev;
#ifdef CONFIG_BRIDGE_IGMP_SNOOPING
@@ -528,6 +580,49 @@ static inline bool br_multicast_is_router(struct net_bridge *br)
}
#endif

+/* br_vlan.c */
+#ifdef CONFIG_BRIDGE_VLAN_FILTERING
+extern struct net_bridge_vlan *br_vlan_find(struct net_bridge *br, u16 vid);
+extern void br_vlan_flush(struct net_bridge *br);
+extern int nbp_vlan_add(struct net_port_vlans *v, u16 vid);
+extern int nbp_vlan_delete(struct net_port_vlans *v, u16 vid);
+extern struct net_port_vlan *nbp_vlan_find(const struct net_port_vlans *v,
+ u16 vid);
+extern void nbp_vlan_flush(struct net_port_vlans *vlans);
+#else
+static inline struct net_bridge_vlan *br_vlan_find(struct net_bridge *br,
+ u16 vid)
+{
+ return NULL;
+}
+
+static inline void br_vlan_flush(struct net_bridge *br)
+{
+}
+
+static inline int nbp_vlan_add(struct net_port_vlans *v, u16 vid)
+{
+ return -EINVAL;
+}
+
+static inline int nbp_vlan_delete(struct net_port_vlans *v, u16 vid)
+{
+ return -EINVAL;
+}
+
+static inline struct net_port_vlan *nbp_vlan_find(
+ const struct net_port_vlans *v,
+ u16 vid)
+{
+ return NULL;
+}
+
+static inline void nbp_vlan_flush(struct net_port_vlans *vlans)
+{
+}
+
+#endif
+
/* br_netfilter.c */
#ifdef CONFIG_BRIDGE_NETFILTER
extern int br_netfilter_init(void);
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
new file mode 100644
index 0000000..816e85e
--- /dev/null
+++ b/net/bridge/br_vlan.c
@@ -0,0 +1,223 @@
+#include <linux/kernel.h>
+#include <linux/netdevice.h>
+#include <linux/rtnetlink.h>
+#include <linux/slab.h>
+
+#include "br_private.h"
+
+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);
+ 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 */
+static struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid)
+{
+ 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);
+
+ 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)
+{
+ ASSERT_RTNL();
+
+ /* 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 destruction. */
+ br_vlan_put(vlan);
+}
+
+void br_vlan_flush(struct net_bridge *br)
+{
+ struct net_bridge_vlan *vlan;
+ struct hlist_node *node;
+ struct hlist_node *tmp;
+ int i;
+
+ nbp_vlan_flush(&br->vlan_info);
+
+ /* 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);
+ }
+ }
+}
+
+struct net_port_vlan *nbp_vlan_find(const struct net_port_vlans *v, 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, &v->vlan_list, list) {
+ if (pve->vid == vid)
+ return pve;
+ }
+
+ return NULL;
+}
+
+/* Must be protected by RTNL */
+int nbp_vlan_add(struct net_port_vlans *v, u16 vid)
+{
+ struct net_port_vlan *pve = NULL;
+ struct net_bridge_vlan *vlan;
+ struct net_bridge *br = vlans_to_bridge(v);
+ struct net_bridge_port *p = vlans_to_port(v);
+ int err;
+
+ ASSERT_RTNL();
+
+ /* Find a vlan in the bridge vlan list. If it isn't there,
+ * create it
+ */
+ vlan = br_vlan_add(br, vid);
+ 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(v->port_idx, 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 (p && !vlan_hw_buggy(p->dev)) {
+ err = vlan_vid_add_hw(p->dev, vid);
+ if (err)
+ goto clean_up;
+ }
+
+ pve->vid = vid;
+ rcu_assign_pointer(pve->vlan, vlan);
+ br_vlan_hold(vlan);
+ set_bit(v->port_idx, vlan->port_bitmap);
+
+ list_add_tail_rcu(&pve->list, &v->vlan_list);
+ return 0;
+
+clean_up:
+ kfree(pve);
+ br_vlan_del(vlan);
+ return err;
+}
+
+/* Must be protected by RTNL */
+int nbp_vlan_delete(struct net_port_vlans *v, u16 vid)
+{
+ struct net_port_vlan *pve;
+ struct net_bridge_vlan *vlan;
+
+ ASSERT_RTNL();
+
+ pve = nbp_vlan_find(v, vid);
+ if (!pve)
+ return -ENOENT;
+
+ if (v->port_idx) {
+ /* A valid port index means this is a port.
+ * Remove VLAN from the port device filter if it is supported.
+ */
+ struct net_device *dev = vlans_to_port(v)->dev;
+
+ if (vlan_vid_del_hw(dev, vid))
+ pr_warn("failed to kill vid %d for device %s\n",
+ vid, dev->name);
+ }
+ pve->vid = BR_INVALID_VID;
+
+ vlan = rtnl_dereference(pve->vlan);
+ rcu_assign_pointer(pve->vlan, NULL);
+ clear_bit(v->port_idx, vlan->port_bitmap);
+ br_vlan_put(vlan);
+
+ list_del_rcu(&pve->list);
+ kfree_rcu(pve, rcu);
+
+ br_vlan_del(vlan);
+
+ return 0;
+}
+
+void nbp_vlan_flush(struct net_port_vlans *vlans)
+{
+ struct net_port_vlan *pve;
+ struct net_port_vlan *tmp;
+
+ ASSERT_RTNL();
+
+ list_for_each_entry_safe(pve, tmp, &vlans->vlan_list, list)
+ nbp_vlan_delete(vlans, pve->vid);
+}
--
1.7.7.6
Cong Wang
2013-01-17 04:47:37 UTC
Permalink
Post by Vlad Yasevich
+config BRIDGE_VLAN_FILTERING
+ bool "VLAN filtering"
+ depends on BRIDGE
+ depends on VLAN_8021Q
+ default y
This should default to n, otherwise Linus will complain.
Post by Vlad Yasevich
+ ---help---
+ If you say Y here, then the Ethernet bridge will be able selectively
+ receive and forward traffic based on VLAN information in the packet
+ any VLAN information configured on the bridge port or bridge device.
_and_ any VLAN information...? Missed 'and'?
Michał Mirosław
2013-01-18 01:57:14 UTC
Permalink
2013/1/16 Vlad Yasevich <***@redhat.com>:
[...]
Post by Vlad Yasevich
--- /dev/null
+++ b/net/bridge/br_vlan.c
[...]
Post by Vlad Yasevich
+struct net_port_vlan *nbp_vlan_find(const struct net_port_vlans *v, =
u16 vid)
Post by Vlad Yasevich
+{
+ struct net_port_vlan *pve;
+
+ /* Must be done either in rcu critical section or with RTNL h=
eld */
Post by Vlad Yasevich
+ WARN_ON_ONCE(!rcu_read_lock_held() && !rtnl_is_locked());
+
+ list_for_each_entry_rcu(pve, &v->vlan_list, list) {
+ if (pve->vid =3D=3D vid)
+ return pve;
+ }
+
+ return NULL;
+}
This looks expensive - it's O(n) with n =3D number of configured VLANs =
on a port.
And this is called for every packet. The bridge already has a hash of V=
LAN
structures found by br_vlan_find(). You could add a second bitmap there
(eg. ingres_ports[]) and check port's bit instead of walking the list.
You would use a bit more memory (64 bytes minus the removed list-head)
per configured VLAN but save some cycles in hot path.

Best Regards,
Micha=B3 Miros=B3aw
Vlad Yasevich
2013-01-20 17:59:22 UTC
Permalink
[...]
Post by Vlad Yasevich
--- /dev/null
+++ b/net/bridge/br_vlan.c
[...]
Post by Vlad Yasevich
+struct net_port_vlan *nbp_vlan_find(const struct net_port_vlans *v,=
u16 vid)
Post by Vlad Yasevich
+{
+ struct net_port_vlan *pve;
+
+ /* Must be done either in rcu critical section or with RTNL =
held */
Post by Vlad Yasevich
+ WARN_ON_ONCE(!rcu_read_lock_held() && !rtnl_is_locked());
+
+ list_for_each_entry_rcu(pve, &v->vlan_list, list) {
+ if (pve->vid =3D=3D vid)
+ return pve;
+ }
+
+ return NULL;
+}
This looks expensive - it's O(n) with n =3D number of configured VLAN=
s on a port.
And this is called for every packet. The bridge already has a hash of=
VLAN
structures found by br_vlan_find(). You could add a second bitmap the=
re
(eg. ingres_ports[]) and check port's bit instead of walking the list=
=2E
You would use a bit more memory (64 bytes minus the removed list-head=
)
per configured VLAN but save some cycles in hot path.
Technically wouldn't even need another bitmap as an existing membership=
=20
bitmap would cover this case. I did some profiling and the list is=20
faster for 3 vlans per port. Hash is faster for more then 3 vlans.

I can easily switch to hash if that is what others think.

-vlad
Best Regards,
Micha=B3 Miros=B3aw
Stephen Hemminger
2013-01-20 19:38:25 UTC
Permalink
On Sun, 20 Jan 2013 12:59:22 -0500
[...]
Post by Vlad Yasevich
--- /dev/null
+++ b/net/bridge/br_vlan.c
[...]
Post by Vlad Yasevich
+struct net_port_vlan *nbp_vlan_find(const struct net_port_vlans *=
v, u16 vid)
Post by Vlad Yasevich
+{
+ struct net_port_vlan *pve;
+
+ /* Must be done either in rcu critical section or with RTN=
L held */
Post by Vlad Yasevich
+ WARN_ON_ONCE(!rcu_read_lock_held() && !rtnl_is_locked());
+
+ list_for_each_entry_rcu(pve, &v->vlan_list, list) {
+ if (pve->vid =3D=3D vid)
+ return pve;
+ }
+
+ return NULL;
+}
This looks expensive - it's O(n) with n =3D number of configured VL=
ANs on a port.
And this is called for every packet. The bridge already has a hash =
of VLAN
structures found by br_vlan_find(). You could add a second bitmap t=
here
(eg. ingres_ports[]) and check port's bit instead of walking the li=
st.
You would use a bit more memory (64 bytes minus the removed list-he=
ad)
per configured VLAN but save some cycles in hot path.
=20
Technically wouldn't even need another bitmap as an existing membersh=
ip=20
bitmap would cover this case. I did some profiling and the list is=20
faster for 3 vlans per port. Hash is faster for more then 3 vlans.
=20
I can easily switch to hash if that is what others think.
=20
-vlad
Let's assume the people that really want this feature are using a lot
of vlan's. i.e n =3D 1000 or so. A bitmap is O(1). Any hash list would
incur a just a big memory penalty for the list head. In other words
a full bitmap is 4096 bits =3D 512 bytes. If you use hash list,
then the equivalent memory size would be only 64 list heads, therefore
a bitmap is a better choice than a hlist.
Vlad Yasevich
2013-01-21 01:50:59 UTC
Permalink
Post by Stephen Hemminger
On Sun, 20 Jan 2013 12:59:22 -0500
[...]
Post by Vlad Yasevich
--- /dev/null
+++ b/net/bridge/br_vlan.c
[...]
Post by Vlad Yasevich
+struct net_port_vlan *nbp_vlan_find(const struct net_port_vlans *=
v, u16 vid)
Post by Stephen Hemminger
Post by Vlad Yasevich
+{
+ struct net_port_vlan *pve;
+
+ /* Must be done either in rcu critical section or with RTN=
L held */
Post by Stephen Hemminger
Post by Vlad Yasevich
+ WARN_ON_ONCE(!rcu_read_lock_held() && !rtnl_is_locked());
+
+ list_for_each_entry_rcu(pve, &v->vlan_list, list) {
+ if (pve->vid =3D=3D vid)
+ return pve;
+ }
+
+ return NULL;
+}
This looks expensive - it's O(n) with n =3D number of configured VL=
ANs on a port.
Post by Stephen Hemminger
And this is called for every packet. The bridge already has a hash =
of VLAN
Post by Stephen Hemminger
structures found by br_vlan_find(). You could add a second bitmap t=
here
Post by Stephen Hemminger
(eg. ingres_ports[]) and check port's bit instead of walking the li=
st.
Post by Stephen Hemminger
You would use a bit more memory (64 bytes minus the removed list-he=
ad)
Post by Stephen Hemminger
per configured VLAN but save some cycles in hot path.
Technically wouldn't even need another bitmap as an existing members=
hip
Post by Stephen Hemminger
bitmap would cover this case. I did some profiling and the list is
faster for 3 vlans per port. Hash is faster for more then 3 vlans.
I can easily switch to hash if that is what others think.
-vlad
Let's assume the people that really want this feature are using a lot
of vlan's. i.e n =3D 1000 or so. A bitmap is O(1). Any hash list woul=
d
Post by Stephen Hemminger
incur a just a big memory penalty for the list head. In other words
a full bitmap is 4096 bits =3D 512 bytes. If you use hash list,
then the equivalent memory size would be only 64 list heads, therefor=
e
Post by Stephen Hemminger
a bitmap is a better choice than a hlist.
This was the approach taken in the RFC v1 of this series. What I found=
=20
was that while it worked very well as far as speed goes, it was a bit=20
cumbersome to extend it to support pvids and it would completely fall
on its face for egress policy that Shmulik is suggesting. So any kinds=
=20
of extensions to it were tough to do.

This is why I went with the list. Interestingly enough, VLAN=20
implementation in the kernel is a list and noone is complaining that it=
=20
is really slow on the fast path.

-vlad
Shmulik Ladkani
2013-01-21 11:45:34 UTC
Permalink
Hi Vlad,
Post by Vlad Yasevich
Post by Stephen Hemminger
Let's assume the people that really want this feature are using a lot
of vlan's. i.e n = 1000 or so. A bitmap is O(1). Any hash list would
incur a just a big memory penalty for the list head. In other words
a full bitmap is 4096 bits = 512 bytes. If you use hash list,
then the equivalent memory size would be only 64 list heads, therefore
a bitmap is a better choice than a hlist.
This was the approach taken in the RFC v1 of this series. What I found
was that while it worked very well as far as speed goes, it was a bit
cumbersome to extend it to support pvids and it would completely fall
on its face for egress policy that Shmulik is suggesting. So any kinds
of extensions to it were tough to do.
I don't see why this is the case.

How about (sketch only, names questionable...):

struct net_bridge {
+ unsigned long vlan_port_membership_bitmap[VLAN_N_VID][PORT_BITMAP_LEN];
+ unsigned long vlan_port_egress_policy_bitmap[VLAN_N_VID][PORT_BITMAP_LEN];
}

(can be alloc'ed instead of the arrays being part of the struct)

struct net_bridge_port {
+ u16 pvid;
};

Allows O(1) to the query "is port P member of vlan V".
Allows O(1) to the query "should vlan V egress tagged/untagged on port P".

I guess this might simplify the data structures involved, avoiding the
refcounts, etc...

The penaties are:
- memory
- aesthetics (?)
- inefficient if query is "give me the entire list of VLANs port P is
member of". But do we have such a query in bridge's code?

You say it went cumbersome. Am I missing something?

BTW, altenatively, you may:

struct net_bridge_port {
+ unsigned long vlan_membership_bitmap[BITS_TO_LONGS(VLAN_N_VID)];
+ unsigned long vlan_egress_policy_bitmap[BITS_TO_LONGS(VLAN_N_VID)];
+ u16 pvid;
};

Which also allows O(1) to "is port 'nbp' member of vlan V".

Difference:
- For the membership structure:
former (within net_bridge) uses 4096 * BR_MAX_PORTS bits,
latter (within net_bridge_port) uses NumOfNBPs * 4096 bits
- better aesthetics (?)

Regards,
Shmulik
Vlad Yasevich
2013-01-22 14:31:43 UTC
Permalink
Post by Shmulik Ladkani
Hi Vlad,
Post by Vlad Yasevich
Post by Stephen Hemminger
Let's assume the people that really want this feature are using a lot
of vlan's. i.e n = 1000 or so. A bitmap is O(1). Any hash list would
incur a just a big memory penalty for the list head. In other words
a full bitmap is 4096 bits = 512 bytes. If you use hash list,
then the equivalent memory size would be only 64 list heads, therefore
a bitmap is a better choice than a hlist.
This was the approach taken in the RFC v1 of this series. What I found
was that while it worked very well as far as speed goes, it was a bit
cumbersome to extend it to support pvids and it would completely fall
on its face for egress policy that Shmulik is suggesting. So any kinds
of extensions to it were tough to do.
I don't see why this is the case.
struct net_bridge {
+ unsigned long vlan_port_membership_bitmap[VLAN_N_VID][PORT_BITMAP_LEN];
+ unsigned long vlan_port_egress_policy_bitmap[VLAN_N_VID][PORT_BITMAP_LEN];
}
(can be alloc'ed instead of the arrays being part of the struct)
struct net_bridge_port {
+ u16 pvid;
};
Allows O(1) to the query "is port P member of vlan V".
Allows O(1) to the query "should vlan V egress tagged/untagged on port P".
I guess this might simplify the data structures involved, avoiding the
refcounts, etc...
- memory
- aesthetics (?)
- inefficient if query is "give me the entire list of VLANs port P is
member of". But do we have such a query in bridge's code?
Yes. When a mac address is added to a port without an explicit vlan tag
we try to add it for every vlan available on the port.

Also, in the API, the user may request vlans configured on a port.
Post by Shmulik Ladkani
You say it went cumbersome. Am I missing something?
struct net_bridge_port {
+ unsigned long vlan_membership_bitmap[BITS_TO_LONGS(VLAN_N_VID)];
+ unsigned long vlan_egress_policy_bitmap[BITS_TO_LONGS(VLAN_N_VID)];
+ u16 pvid;
};
Which also allows O(1) to "is port 'nbp' member of vlan V".
This is what the earlier RFC patches did. You are paying a large memory
penalty and carrying a mostly empty bitmap when only a small number of
vlans is used.

If someone decides that they'd like priority support, you'd need another
array or list to hold priority values.

-vlad
Post by Shmulik Ladkani
former (within net_bridge) uses 4096 * BR_MAX_PORTS bits,
latter (within net_bridge_port) uses NumOfNBPs * 4096 bits
- better aesthetics (?)
Regards,
Shmulik
Shmulik Ladkani
2013-01-22 15:55:24 UTC
Permalink
Thanks Vlad,
Post by Vlad Yasevich
Post by Shmulik Ladkani
I guess this might simplify the data structures involved, avoiding the
refcounts, etc...
- memory
- aesthetics (?)
- inefficient if query is "give me the entire list of VLANs port P is
member of". But do we have such a query in bridge's code?
Yes. When a mac address is added to a port without an explicit vlan tag
we try to add it for every vlan available on the port.
I see.
Can't this be bypassed by adding a _single_ FDB entry whose VID value
denotes "member of ANY vlan" (value outside the valid 0-4095 range)?
Post by Vlad Yasevich
Also, in the API, the user may request vlans configured on a port.
Personally I'd pay the penalty implementing this specific user request
in an inefficeint way, to acheive overall simplicity in core bridge
code.
But that's just my humble opinion, maybe others might spot drawbacks
taking this approach.

BTW, went through the ML, couldn't find the reason why dropped the
per-port vlan bitmap and replaced with a vlan list (after your RFC v2
patches). Care to explain what was your motivation?

Regards,
Shmulik
Vlad Yasevich
2013-01-22 16:27:01 UTC
Permalink
Post by Shmulik Ladkani
Thanks Vlad,
Post by Vlad Yasevich
Post by Shmulik Ladkani
I guess this might simplify the data structures involved, avoiding the
refcounts, etc...
- memory
- aesthetics (?)
- inefficient if query is "give me the entire list of VLANs port P is
member of". But do we have such a query in bridge's code?
Yes. When a mac address is added to a port without an explicit vlan tag
we try to add it for every vlan available on the port.
I see.
Can't this be bypassed by adding a _single_ FDB entry whose VID value
denotes "member of ANY vlan" (value outside the valid 0-4095 range)?
Post by Vlad Yasevich
Also, in the API, the user may request vlans configured on a port.
Personally I'd pay the penalty implementing this specific user request
in an inefficeint way, to acheive overall simplicity in core bridge
code.
But that's just my humble opinion, maybe others might spot drawbacks
taking this approach.
BTW, went through the ML, couldn't find the reason why dropped the
per-port vlan bitmap and replaced with a vlan list (after your RFC v2
patches). Care to explain what was your motivation?
I wanted to reduce the memory footprint and make it a bit more
extensible so if priority was ever added, it would be very simple to do.
I also had to play some ugly memory barrier games to make it less racy.
I thought that the list/hash code was cleaner.

-vlad
Post by Shmulik Ladkani
Regards,
Shmulik
Vlad Yasevich
2013-01-22 17:32:45 UTC
Permalink
I appreciate all the work on this. And at this point it may seem like it
will never get in mainline. As far as I am concerned these are the key
1. VLAN filtering on both ingress and egress using same table
Can you elaborate a bit more what you mean by this?
2. O(1) based bit map table
3. netlink based configuration and dump (per port). Okay to just provide the
bitmap
4. kernel feature should be optional
5. default behavior has to be to allow all VLAN's for backwards compatibility.
I think with the exception of 2, all of the above requirements are met
by the current code. As for #2, a hash approach provides most of the
performance gain and allows for extensibility.
I am currently removing the list from all the hot paths, and we could
make the hash wider if you want to make each hlist shorter.
* cleaning up kernel VLAN API's. Eliminate vlan_buggy() and any/all cases
where error is not detected until first packet
I don't think we can truly do that since non-vlans frames should work on
vlan_buggy interfaces.

I could also remove all the HW filtering calls out of the code since
they are not truly needed now due to running in promisc mode.

-vlad
* sysfs API's - sysfs is not good for binary (like bitmap), and doing
a list
* split ingress/egress
* future extensibility ideas - do it right now, then do the next step;
don't get ahead of yourself.
Shmulik Ladkani
2013-01-20 21:38:51 UTC
Permalink
Hi Vlad,
Post by Vlad Yasevich
@@ -156,6 +183,7 @@ struct net_bridge_port
#ifdef CONFIG_NET_POLL_CONTROLLER
struct netpoll *np;
#endif
+ struct net_port_vlans vlan_info;
(here and at 'struct net_bridge' as well)

Not sure what the policy is; Isn't it preferred to enclose the new
fields under CONFIG_BRIDGE_VLAN_FILTERING?
Post by Vlad Yasevich
+static inline struct net_bridge *vlans_to_bridge(struct net_port_vlans *vlans)
+{
+ struct net_bridge *br;
+
+ if (!vlans->port_idx)
+ br = container_of((vlans), struct net_bridge, vlan_info);
+ else
+ br = vlans_to_port(vlans)->br;
+
+ return br;
+}
Guess it would simplify things if the bridge "master port" had an 'nbp'
representation of its own ;-)
Post by Vlad Yasevich
+extern struct net_bridge_vlan *br_vlan_find(struct net_bridge *br, u16 vid);
Seems 'br_vlan_find' can be declared static within br_vlan.c.
Post by Vlad Yasevich
+extern void br_vlan_flush(struct net_bridge *br);
According to your preference, consider s/br_vlan_flush/br_vlans_flush/
since it better suggest acting on all bridge's vlans.
Post by Vlad Yasevich
+extern void nbp_vlan_flush(struct net_port_vlans *vlans);
According to your preference, consider s/nbp_vlan_flush/nbp_vlans_flush/
since it better suggest acting on all port's vlans.
Post by Vlad Yasevich
+void br_vlan_flush(struct net_bridge *br)
+{
+ struct net_bridge_vlan *vlan;
+ struct hlist_node *node;
+ struct hlist_node *tmp;
+ int i;
+
+ nbp_vlan_flush(&br->vlan_info);
+
+ /* Make sure that there are no vlans left in the bridge after
+ * all the ports have been removed.
+ */
Improper indent.
Post by Vlad Yasevich
+ 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);
Can there be any vlans left at that point? Shouldn't del_nbp() on all
ports take care of that?
Also, if there _were_ any vlans left (whose bitmap isn't cleared),
'br_vlan_del' won't do a thing.
Am I missing something?

Regards,
Shmulik
Vlad Yasevich
2013-01-21 01:56:08 UTC
Permalink
Post by Shmulik Ladkani
Hi Vlad,
Post by Vlad Yasevich
@@ -156,6 +183,7 @@ struct net_bridge_port
#ifdef CONFIG_NET_POLL_CONTROLLER
struct netpoll *np;
#endif
+ struct net_port_vlans vlan_info;
(here and at 'struct net_bridge' as well)
Not sure what the policy is; Isn't it preferred to enclose the new
fields under CONFIG_BRIDGE_VLAN_FILTERING?
Good catch. Missed this one.
Post by Shmulik Ladkani
Post by Vlad Yasevich
+static inline struct net_bridge *vlans_to_bridge(struct net_port_vlans *vlans)
+{
+ struct net_bridge *br;
+
+ if (!vlans->port_idx)
+ br = container_of((vlans), struct net_bridge, vlan_info);
+ else
+ br = vlans_to_port(vlans)->br;
+
+ return br;
+}
Guess it would simplify things if the bridge "master port" had an 'nbp'
representation of its own ;-)
Yes that would simplify things for this case, but it will also add a lot
of state that's not necessary. I considered doing this, but the master
device doesn't really act as port in on things, just some.
Post by Shmulik Ladkani
Post by Vlad Yasevich
+extern struct net_bridge_vlan *br_vlan_find(struct net_bridge *br, u16 vid);
Seems 'br_vlan_find' can be declared static within br_vlan.c.
Will check.
Post by Shmulik Ladkani
Post by Vlad Yasevich
+extern void br_vlan_flush(struct net_bridge *br);
According to your preference, consider s/br_vlan_flush/br_vlans_flush/
since it better suggest acting on all bridge's vlans.
Post by Vlad Yasevich
+extern void nbp_vlan_flush(struct net_port_vlans *vlans);
According to your preference, consider s/nbp_vlan_flush/nbp_vlans_flush/
since it better suggest acting on all port's vlans.
Post by Vlad Yasevich
+void br_vlan_flush(struct net_bridge *br)
+{
+ struct net_bridge_vlan *vlan;
+ struct hlist_node *node;
+ struct hlist_node *tmp;
+ int i;
+
+ nbp_vlan_flush(&br->vlan_info);
+
+ /* Make sure that there are no vlans left in the bridge after
+ * all the ports have been removed.
+ */
Improper indent.
will fix
Post by Shmulik Ladkani
Post by Vlad Yasevich
+ 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);
Can there be any vlans left at that point? Shouldn't del_nbp() on all
ports take care of that?
I think this function might have been a leftover from prior series when
bridge didn't have its vlan list. With the new series, I don't this it
is needed. I'll double-check.

Thanks
-vlad
Post by Shmulik Ladkani
Also, if there _were_ any vlans left (whose bitmap isn't cleared),
'br_vlan_del' won't do a thing.
Am I missing something?
Regards,
Shmulik
Vlad Yasevich
2013-01-16 18:17:58 UTC
Permalink
When a frame arrives on a port or transmitted by the bridge,
if we have VLANs configured, validate that a given VLAN is allowed
to enter the bridge.

Signed-off-by: Vlad Yasevich <***@redhat.com>
---
net/bridge/br_device.c | 3 +++
net/bridge/br_input.c | 4 ++++
net/bridge/br_private.h | 25 +++++++++++++++++++++++++
net/bridge/br_vlan.c | 20 ++++++++++++++++++++
4 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index b64b5c8..6b8f816 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -45,6 +45,9 @@ 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);

+ if (!br_allowed_ingress(&br->vlan_info, skb))
+ goto out;
+
BR_INPUT_SKB_CB(skb)->brdev = dev;

skb_reset_mac_header(skb);
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 4b34207..cc959fa 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 */
@@ -54,6 +55,9 @@ int br_handle_frame_finish(struct sk_buff *skb)
if (!p || p->state == BR_STATE_DISABLED)
goto drop;

+ if (!br_allowed_ingress(&p->vlan_info, 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 615f10c..7018546 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -582,6 +582,7 @@ static inline bool br_multicast_is_router(struct net_bridge *br)

/* br_vlan.c */
#ifdef CONFIG_BRIDGE_VLAN_FILTERING
+extern bool br_allowed_ingress(struct net_port_vlans *v, struct sk_buff *skb);
extern struct net_bridge_vlan *br_vlan_find(struct net_bridge *br, u16 vid);
extern void br_vlan_flush(struct net_bridge *br);
extern int nbp_vlan_add(struct net_port_vlans *v, u16 vid);
@@ -589,7 +590,27 @@ extern int nbp_vlan_delete(struct net_port_vlans *v, u16 vid);
extern struct net_port_vlan *nbp_vlan_find(const struct net_port_vlans *v,
u16 vid);
extern void nbp_vlan_flush(struct net_port_vlans *vlans);
+
+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;
+}
+
#else
+static inline bool br_allowed_ingress(struct net_port_vlans *v,
+ struct sk_buff *skb)
+{
+ return true;
+}
+
static inline struct net_bridge_vlan *br_vlan_find(struct net_bridge *br,
u16 vid)
{
@@ -621,6 +642,10 @@ static inline void nbp_vlan_flush(struct net_port_vlans *vlans)
{
}

+static inline u16 br_get_vlan(const struct sk_buff *skb)
+{
+ return 0;
+}
#endif

/* br_netfilter.c */
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 816e85e..72b4892 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -5,6 +5,26 @@

#include "br_private.h"

+/* Called under RCU */
+bool br_allowed_ingress(struct net_port_vlans *v, 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(&v->vlan_list))
+ return true;
+
+ vid = br_get_vlan(skb);
+ pve = nbp_vlan_find(v, vid);
+ if (pve)
+ return true;
+
+ return false;
+}
+
static void br_vlan_destroy(struct net_bridge_vlan *vlan)
{
if (!bitmap_empty(vlan->port_bitmap, PORT_BITMAP_LEN)) {
--
1.7.7.6
Shmulik Ladkani
2013-01-20 22:27:10 UTC
Permalink
Hi Vlad,
Post by Vlad Yasevich
@@ -45,6 +45,9 @@ 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);
+ if (!br_allowed_ingress(&br->vlan_info, skb))
+ goto out;
+
Shouldn't you consume the 'skb' in case "not allowed"? the 'out' label
doesn't take care of that.
Post by Vlad Yasevich
+bool br_allowed_ingress(struct net_port_vlans *v, 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(&v->vlan_list))
+ return true;
Rethinking this, after discussed at [1].
The above means the port having no vlans is actually a member of every
possible vlan.
IMO it might not be what users expect, and may complicate things.

Maybe we should adapt a simpler approach:
If the bridge is a vlan enabled bridge, and the port is not a member of
the given vid, drop.
If the bridge is "vlan disabled", then all packets are permitted.

Regards,
Shmulik

[1]
http://marc.info/?l=linux-netdev&m=135602065425514&w=2
Vlad Yasevich
2013-01-21 01:58:50 UTC
Permalink
Post by Shmulik Ladkani
Hi Vlad,
Post by Vlad Yasevich
@@ -45,6 +45,9 @@ 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);
+ if (!br_allowed_ingress(&br->vlan_info, skb))
+ goto out;
+
Shouldn't you consume the 'skb' in case "not allowed"? the 'out' label
doesn't take care of that.
Post by Vlad Yasevich
+bool br_allowed_ingress(struct net_port_vlans *v, 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(&v->vlan_list))
+ return true;
Rethinking this, after discussed at [1].
The above means the port having no vlans is actually a member of every
possible vlan.
IMO it might not be what users expect, and may complicate things.
If the bridge is a vlan enabled bridge, and the port is not a member of
the given vid, drop.
If the bridge is "vlan disabled", then all packets are permitted.
this is hybrid configuration where some ports have vlan filtering
enabled while others do not.

Currently the default is to act as a non-vlan bridge when there is no
configuration. I'll consider adding a configuration nob to switch the
behavior so that strict filtering can be enforced.

-vlad
Post by Shmulik Ladkani
Regards,
Shmulik
[1]
http://marc.info/?l=linux-netdev&m=135602065425514&w=2
Vlad Yasevich
2013-01-16 18:18:00 UTC
Permalink
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 <***@redhat.com>
---
net/bridge/br_private.h | 1 +
net/bridge/br_vlan.c | 20 +++++++++++++++++++-
2 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 39e70af..7960681 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -316,6 +316,7 @@ static inline struct net_bridge *vlans_to_bridge(struct net_port_vlans *vlans)

struct br_input_skb_cb {
struct net_device *brdev;
+ struct net_bridge_vlan *vlan;
#ifdef CONFIG_BRIDGE_IGMP_SNOOPING
int igmp;
int mrouters_only;
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index a4f7c15..01ebfbc 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -11,6 +11,8 @@ bool br_allowed_ingress(struct net_port_vlans *v, 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.
*/
@@ -19,8 +21,10 @@ bool br_allowed_ingress(struct net_port_vlans *v, struct sk_buff *skb)

vid = br_get_vlan(skb);
pve = nbp_vlan_find(v, vid);
- if (pve)
+ if (pve) {
+ BR_INPUT_SKB_CB(skb)->vlan = rcu_dereference(pve->vlan);
return true;
+ }

return false;
}
@@ -30,11 +34,25 @@ bool br_allowed_egress(const struct net_port_vlans *v,
const struct sk_buff *skb)
{
struct net_port_vlan *pve;
+ struct net_bridge_vlan *vlan = NULL;
u16 vid;

if (list_empty(&v->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(v->port_idx, 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(v, vid);
if (pve)
--
1.7.7.6
Michał Mirosław
2013-01-16 22:00:54 UTC
Permalink
Wrap VLAN hardware acceleration calls into separate functions. This =
way
other code can re-use it.
Maybe instead of jumping aroung bugs that are easily fixed just don't
let them in? See untested patch below. Affected drivers can be either
fixed or have NETIF_F_HW_VLAN_FILTER removed from advertised features.

--cut-here--

net: disallow drivers with buggy VLAN accel to register_netdevice()

Signed-off-by: Micha=C5=82 Miros=C5=82aw <mirq-***@rere.qmqm.pl>
---
net/8021q/vlan.c | 7 -------
net/8021q/vlan_core.c | 6 ++----
net/core/dev.c | 9 +++++++++
3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index babfde9..addc578 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -117,19 +117,12 @@ void unregister_vlan_dev(struct net_device *dev, =
struct list_head *head)
int vlan_check_real_dev(struct net_device *real_dev, u16 vlan_id)
{
const char *name =3D real_dev->name;
- const struct net_device_ops *ops =3D real_dev->netdev_ops;
=20
if (real_dev->features & NETIF_F_VLAN_CHALLENGED) {
pr_info("VLANs not supported on %s\n", name);
return -EOPNOTSUPP;
}
=20
- if ((real_dev->features & NETIF_F_HW_VLAN_FILTER) &&
- (!ops->ndo_vlan_rx_add_vid || !ops->ndo_vlan_rx_kill_vid)) {
- pr_info("Device %s has buggy VLAN hw accel\n", name);
- return -EOPNOTSUPP;
- }
-
if (vlan_find_dev(real_dev, vlan_id) !=3D NULL)
return -EEXIST;
=20
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 380440b..71b64fd 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -224,8 +224,7 @@ static int __vlan_vid_add(struct vlan_info *vlan_in=
fo, unsigned short vid,
if (!vid_info)
return -ENOMEM;
=20
- if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
- ops->ndo_vlan_rx_add_vid) {
+ if (dev->features & NETIF_F_HW_VLAN_FILTER) {
err =3D ops->ndo_vlan_rx_add_vid(dev, vid);
if (err) {
kfree(vid_info);
@@ -282,8 +281,7 @@ static void __vlan_vid_del(struct vlan_info *vlan_i=
nfo,
unsigned short vid =3D vid_info->vid;
int err;
=20
- if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
- ops->ndo_vlan_rx_kill_vid) {
+ if (dev->features & NETIF_F_HW_VLAN_FILTER) {
err =3D ops->ndo_vlan_rx_kill_vid(dev, vid);
if (err) {
pr_warn("failed to kill vid %d for device %s\n",
diff --git a/net/core/dev.c b/net/core/dev.c
index b6d2b32..f87cdc2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6211,6 +6211,15 @@ int register_netdevice(struct net_device *dev)
}
}
=20
+ if (((dev->hw_features & NETIF_F_HW_VLAN_FILTER) ||
+ (dev->features & NETIF_F_HW_VLAN_FILTER)) &&
+ (!dev->netdev_ops->ndo_vlan_rx_add_vid ||
+ !dev->netdev_ops->ndo_vlan_rx_kill_vid)) {
+ netdev_WARN(dev, "Buggy VLAN acceleration in driver!\n");
+ ret =3D -EINVAL;
+ goto err_uninit;
+ }
+
ret =3D -EBUSY;
if (!dev->ifindex)
dev->ifindex =3D dev_new_index(net);
--=20
1.7.10.4
Michał Mirosław
2013-01-16 22:03:17 UTC
Permalink
(resend with proper To:)
Wrap VLAN hardware acceleration calls into separate functions. This =
way
other code can re-use it.
Maybe instead of jumping aroung bugs that are easily fixed just don't
let them in? See untested patch below. Affected drivers can be either
fixed or have NETIF_F_HW_VLAN_FILTER removed from advertised features.

--cut-here--

net: disallow drivers with buggy VLAN accel to register_netdevice()

Signed-off-by: Micha=B3 Miros=B3aw <mirq-***@rere.qmqm.pl>
---
net/8021q/vlan.c | 7 -------
net/8021q/vlan_core.c | 6 ++----
net/core/dev.c | 9 +++++++++
3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index babfde9..addc578 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -117,19 +117,12 @@ void unregister_vlan_dev(struct net_device *dev, =
struct list_head *head)
int vlan_check_real_dev(struct net_device *real_dev, u16 vlan_id)
{
const char *name =3D real_dev->name;
- const struct net_device_ops *ops =3D real_dev->netdev_ops;
=20
if (real_dev->features & NETIF_F_VLAN_CHALLENGED) {
pr_info("VLANs not supported on %s\n", name);
return -EOPNOTSUPP;
}
=20
- if ((real_dev->features & NETIF_F_HW_VLAN_FILTER) &&
- (!ops->ndo_vlan_rx_add_vid || !ops->ndo_vlan_rx_kill_vid)) {
- pr_info("Device %s has buggy VLAN hw accel\n", name);
- return -EOPNOTSUPP;
- }
-
if (vlan_find_dev(real_dev, vlan_id) !=3D NULL)
return -EEXIST;
=20
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 380440b..71b64fd 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -224,8 +224,7 @@ static int __vlan_vid_add(struct vlan_info *vlan_in=
fo, unsigned short vid,
if (!vid_info)
return -ENOMEM;
=20
- if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
- ops->ndo_vlan_rx_add_vid) {
+ if (dev->features & NETIF_F_HW_VLAN_FILTER) {
err =3D ops->ndo_vlan_rx_add_vid(dev, vid);
if (err) {
kfree(vid_info);
@@ -282,8 +281,7 @@ static void __vlan_vid_del(struct vlan_info *vlan_i=
nfo,
unsigned short vid =3D vid_info->vid;
int err;
=20
- if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
- ops->ndo_vlan_rx_kill_vid) {
+ if (dev->features & NETIF_F_HW_VLAN_FILTER) {
err =3D ops->ndo_vlan_rx_kill_vid(dev, vid);
if (err) {
pr_warn("failed to kill vid %d for device %s\n",
diff --git a/net/core/dev.c b/net/core/dev.c
index b6d2b32..f87cdc2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6211,6 +6211,15 @@ int register_netdevice(struct net_device *dev)
}
}
=20
+ if (((dev->hw_features & NETIF_F_HW_VLAN_FILTER) ||
+ (dev->features & NETIF_F_HW_VLAN_FILTER)) &&
+ (!dev->netdev_ops->ndo_vlan_rx_add_vid ||
+ !dev->netdev_ops->ndo_vlan_rx_kill_vid)) {
+ netdev_WARN(dev, "Buggy VLAN acceleration in driver!\n");
+ ret =3D -EINVAL;
+ goto err_uninit;
+ }
+
ret =3D -EBUSY;
if (!dev->ifindex)
dev->ifindex =3D dev_new_index(net);
--=20
1.7.10.4


----- End forwarded message -----
Continue reading on narkive:
Loading...