[PATCH 3/3] bridge: Add support for VLANs
Tobias Jungel
tobias.jungel at bisdn.de
Wed Nov 25 07:47:29 PST 2015
some more comments inline here as well.
/tobi
On Di, 2015-11-24 at 11:56 -0800, David Ahern wrote:
> Signed-off-by: Wilson Kok <wkok at cumulusnetworks.com>
> Signed-off-by: David Ahern <dsa at cumulusnetworks.com>
> ---
> include/linux-private/linux/if_bridge.h | 14 +++
> include/linux-private/linux/rtnetlink.h | 6 +
> include/netlink/route/link/bridge.h | 21 ++++
> lib/route/link/bridge.c | 203
> ++++++++++++++++++++++++++++++++
> lib/route/neigh.c | 9 +-
> 5 files changed, 251 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux-private/linux/if_bridge.h b/include/linux-
> private/linux/if_bridge.h
> index 5db297514aec..810d3489c090 100644
> --- a/include/linux-private/linux/if_bridge.h
> +++ b/include/linux-private/linux/if_bridge.h
> @@ -108,15 +108,29 @@ 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 */
> +#define BRIDGE_VLAN_INFO_PVID (1<<1) /* VLAN is PVID,
> ingress untagged */
> +#define BRIDGE_VLAN_INFO_UNTAGGED (1<<2) /* VLAN
> egresses untagged */
> +#define BRIDGE_VLAN_INFO_RANGE_BEGIN (1<<3) /* VLAN is start
> of vlan range */
> +#define BRIDGE_VLAN_INFO_RANGE_END (1<<4) /* VLAN is end of
> vlan range */
> +#define BRIDGE_VLAN_INFO_BRENTRY (1<<5) /* Global bridge VLAN
> entry */
> +
> +struct bridge_vlan_info {
> + __u16 flags;
> + __u16 vid;
> +};
> +
> /* Bridge multicast database attributes
> * [MDBA_MDB] = {
> * [MDBA_MDB_ENTRY] = {
> diff --git a/include/linux-private/linux/rtnetlink.h b/include/linux-
> private/linux/rtnetlink.h
> index 2363c18ed62c..f3393d634343 100644
> --- a/include/linux-private/linux/rtnetlink.h
> +++ b/include/linux-private/linux/rtnetlink.h
> @@ -600,6 +600,12 @@ struct tcamsg {
> #define TCA_ACT_TAB 1 /* attr type must be >=1 */
> #define TCAA_MAX 1
>
> +/* New extended info filters for IFLA_EXT_MASK */
> +#define RTEXT_FILTER_VF (1 << 0)
> +#define RTEXT_FILTER_BRVLAN (1 << 1)
> +#define RTEXT_FILTER_BRVLAN_COMPRESSED (1 << 2)
> +#define RTEXT_FILTER_SKIP_STATS (1 << 3)
> +
> /* End of information exported to user level */
>
> #endif /* __LINUX_RTNETLINK_H */
> diff --git a/include/netlink/route/link/bridge.h
> b/include/netlink/route/link/bridge.h
> index 16a4505f8b74..5dc6a367595c 100644
> --- a/include/netlink/route/link/bridge.h
> +++ b/include/netlink/route/link/bridge.h
> @@ -19,6 +19,22 @@
> extern "C" {
> #endif
>
> +/* maximum vlan id */
> +#define VLAN_N_VID 4096
> +
> +#define BITS_PER_BYTE 8
> +#define BITS_PER_LONG (BITS_PER_BYTE *
> sizeof(unsigned long))
> +#define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
> +#define BITS_TO_LONGS(nr) DIV_ROUND_UP(nr, BITS_PER_LONG)
> +#define BR_VLAN_BITMAP_LEN BITS_TO_LONGS(VLAN_N_VID)
> +
> +struct bridge_vlan
> +{
> + uint16_t pvid;
> + unsigned long vlan_bitmap[BR_VLAN_BITMAP_LEN];
> + unsigned long untagged_bitmap[BR_VLAN_BITMAP_LEN];
> +};
> +
Fine with me, even if I personnaly would rather opt for the kernels
compressed data structure, but this is probably just a matter of taste.
> /**
> * Bridge flags
> * @ingroup bridge
> @@ -52,6 +68,11 @@ extern char * rtnl_link_bridge_flags2str(int, char
> *, size_t);
> extern int rtnl_link_bridge_str2flags(const char *);
>
> extern int rtnl_link_bridge_add(struct nl_sock *sk, const
> char *name);
> +
> +extern int rtnl_link_bridge_pvid(struct rtnl_link *link);
> +extern int rtnl_link_bridge_has_vlan(struct rtnl_link *link);
> +
> +extern struct bridge_vlan *rtnl_bridge_get_port_vlan(struct
> rtnl_link *link);
> #ifdef __cplusplus
> }
> #endif
> diff --git a/lib/route/link/bridge.c b/lib/route/link/bridge.c
> index 544f02cd379b..d965a40469a5 100644
> --- a/lib/route/link/bridge.c
> +++ b/lib/route/link/bridge.c
> @@ -25,11 +25,14 @@
> #include <netlink-private/route/link/api.h>
> #include <linux/if_bridge.h>
>
> +#define VLAN_VID_MASK 0x0fff /* VLAN Identifier */
> +
> /** @cond SKIP */
> #define BRIDGE_ATTR_PORT_STATE (1 << 0)
> #define BRIDGE_ATTR_PRIORITY (1 << 1)
> #define BRIDGE_ATTR_COST (1 << 2)
> #define BRIDGE_ATTR_FLAGS (1 << 3)
> +#define BRIDGE_ATTR_PORT_VLAN (1 << 4)
>
> #define PRIV_FLAG_NEW_ATTRS (1 << 0)
>
> @@ -42,8 +45,49 @@ struct bridge_data
> uint32_t b_flags;
> uint32_t b_flags_mask;
> uint32_t ce_mask; /* HACK to support attr
> macros */
> + struct bridge_vlan vlan_info;
> };
>
> +static void set_bit(unsigned nr, unsigned long *addr)
> +{
> + unsigned long mask, *p;
> +
> + if (nr < VLAN_N_VID) {
> + mask = (1UL << (nr % BITS_PER_LONG));
> + p = addr + nr / BITS_PER_LONG;
> + *p |= mask;
> + }
> +}
> +
> +#if 0
> +static void clear_bit(unsigned nr, unsigned long *addr)
> +{
> + unsigned long mask, *p;
> +
> + if (nr < VLAN_N_VID) {
> + mask = (1UL << (nr % BITS_PER_LONG));
> + p = addr + nr / BITS_PER_LONG;
> + *p &= ~mask;
> + }
> +}
> +#endif
> +
> +static int find_next_bit(int i, unsigned long x)
> +{
> + int j;
> +
> + if (i >= (int) sizeof(x) * 8)
> + return -1;
> +
> + /* find first bit */
> + if (i < 0)
> + return __builtin_ffsl(x);
> +
> + /* mask off prior finds to get next */
> + j = __builtin_ffsl(x >> i);
> + return j ? j += i : 0;
> +}
> +
> static struct rtnl_link_af_ops bridge_ops;
>
> #define IS_BRIDGE_LINK_ASSERT(link) \
> @@ -141,6 +185,112 @@ static int bridge_parse_protinfo(struct
> rtnl_link *link, struct nlattr *attr,
> return 0;
> }
>
> +static int bridge_parse_af(struct rtnl_link *link, struct nlattr
> *attr,
> + void *data)
> +{
> + struct bridge_data *bd = data;
> + struct bridge_vlan_info *vinfo = NULL;
> +
> + if (nla_type(attr) != IFLA_BRIDGE_VLAN_INFO)
> + return 0;
> +
> + if (nla_len(attr) != sizeof(struct bridge_vlan_info))
> + return -EINVAL;
> +
> + vinfo = nla_data(attr);
> + if (!vinfo->vid || vinfo->vid >= VLAN_VID_MASK)
> + return -EINVAL;
> +
> + if (vinfo->flags & BRIDGE_VLAN_INFO_RANGE_BEGIN) {
> + NL_DBG(1, "Unexpected BRIDGE_VLAN_INFO_RANGE_BEGIN
> flag; can not handle it.\n");
> + return -EINVAL;
afaik RTEXT_FILTER_BRVLAN_COMPRESSED is the default for the kernel to
send notifications, so I see this rather mandatory to have.
> + }
> +
> + if (vinfo->flags & BRIDGE_VLAN_INFO_PVID)
> + bd->vlan_info.pvid = vinfo->vid;
> +
> + if (vinfo->flags & BRIDGE_VLAN_INFO_UNTAGGED)
> + set_bit(vinfo->vid, bd->vlan_info.untagged_bitmap);
> +
> + set_bit(vinfo->vid, bd->vlan_info.vlan_bitmap);
> +
> + bd->ce_mask |= BRIDGE_ATTR_PORT_VLAN;
> +
> + return 0;
> +}
> +
> +static int bridge_get_af(struct nl_msg *msg)
> +{
> + __u32 ext_filter_mask = RTEXT_FILTER_BRVLAN;
> +
> + return nla_put(msg, IFLA_EXT_MASK, sizeof(ext_filter_mask),
> &ext_filter_mask);
> +}
> +
> +static void walk_bitmap(struct nl_dump_params *p, unsigned long *b,
> int nr)
> +{
> + int i = -1, j, k;
> + int start = -1, prev = -1;
> + int done, found = 0;
> +
> + for (k = 0; k < nr; k++) {
> + int base_bit;
> + unsigned long a = b[k];
> +
> + base_bit = k * BITS_PER_LONG;
> + i = -1;
> + done = 0;
> + while (!done) {
> + j = find_next_bit(i, a);
> + if (j > 0) {
> + /* first hit of any bit */
> + if (start < 0 && prev < 0) {
> + start = prev = j - 1 +
> base_bit;
> + goto next;
> + }
> + /* this bit is a continuation of
> prior bits */
> + if (j - 2 + base_bit == prev) {
> + prev++;
> + goto next;
> + }
> + } else
> + done = 1;
> +
> + if (start >= 0) {
> + found++;
> + if (done && k < nr - 1)
> + break;
> +
> + nl_dump(p, " %d", start);
> + if (start != prev)
> + nl_dump(p, "-%d", prev);
> +
> + if (done)
> + break;
> + }
> + if (j > 0)
> + start = prev = j - 1 + base_bit;
> +next:
> + i = j;
> + }
> + }
> + if (!found)
> + nl_dump(p, " <none>");
> +
> + return;
> +}
> +
> +static void rtnl_link_bridge_dump_vlans(struct nl_dump_params *p,
> + struct bridge_data *bd)
> +{
> + nl_dump(p, "pvid %u", bd->vlan_info.pvid);
> +
> + nl_dump(p, " all vlans:");
> + walk_bitmap(p, &bd->vlan_info.vlan_bitmap[0],
> BR_VLAN_BITMAP_LEN);
> +
> + nl_dump(p, " untagged vlans:");
> + walk_bitmap(p, &bd->vlan_info.untagged_bitmap[0],
> BR_VLAN_BITMAP_LEN);
> +}
> +
> static void bridge_dump_details(struct rtnl_link *link,
> struct nl_dump_params *p, void
> *data)
> {
> @@ -157,6 +307,17 @@ static void bridge_dump_details(struct rtnl_link
> *link,
> if (bd->ce_mask & BRIDGE_ATTR_COST)
> nl_dump(p, "cost %u ", bd->b_cost);
>
> + if (bd->ce_mask & BRIDGE_ATTR_PORT_VLAN)
> + rtnl_link_bridge_dump_vlans(p, bd);
> +
> + if (bd->ce_mask & BRIDGE_ATTR_FLAGS) {
> + char buf[256];
> +
> + rtnl_link_bridge_flags2str(bd->b_flags & bd-
> >b_flags_mask,
> + buf, sizeof(buf));
> + nl_dump(p, "%s", buf);
> + }
> +
> nl_dump(p, "\n");
> }
>
> @@ -171,6 +332,8 @@ static int bridge_compare(struct rtnl_link *_a,
> struct rtnl_link *_b,
> diff |= BRIDGE_DIFF(PORT_STATE, a->b_port_state != b-
> >b_port_state);
> diff |= BRIDGE_DIFF(PRIORITY, a->b_priority != b-
> >b_priority);
> diff |= BRIDGE_DIFF(COST, a->b_cost != b->b_cost);
> + diff |= BRIDGE_DIFF(PORT_VLAN, memcmp(&a->vlan_info, &b-
> >vlan_info,
> + sizeof(struct
> bridge_vlan)));
>
> if (flags & LOOSE_COMPARISON)
> diff |= BRIDGE_DIFF(FLAGS,
> @@ -503,6 +666,44 @@ int rtnl_link_bridge_str2flags(const char *name)
>
> /** @} */
>
> +int rtnl_link_bridge_pvid(struct rtnl_link *link)
this function seems to be public, so its missing in the .sym file.
> +{
> + struct bridge_data *bd = link->l_af_data[AF_BRIDGE];
> +
> + if (bd->ce_mask & BRIDGE_ATTR_PORT_VLAN)
> + return (int) bd->vlan_info.pvid;
> +
> + return -EINVAL;
> +}
> +
> +int rtnl_link_bridge_has_vlan(struct rtnl_link *link)
same as above
> +{
> + struct bridge_data *bd = link->l_af_data[AF_BRIDGE];
> + int i;
> +
> + if (bd->ce_mask & BRIDGE_ATTR_PORT_VLAN) {
> + if (bd->vlan_info.pvid)
> + return 1;
> +
> + for (i = 0; i < BR_VLAN_BITMAP_LEN; ++i) {
> + if (bd->vlan_info.vlan_bitmap[i] ||
> + bd->vlan_info.untagged_bitmap[i])
> + return 1;
> + }
> + }
> + return 0;
> +}
> +
> +struct bridge_vlan *rtnl_bridge_get_port_vlan(struct rtnl_link
> *link)
same as above and I guess the function should be renamed to
rtnl_link_bridge_get_port_vlan
> +{
> + struct bridge_data *data = link->l_af_data[AF_BRIDGE];
> +
> + if (data && (data->ce_mask & BRIDGE_ATTR_PORT_VLAN))
> + return &data->vlan_info;
> +
> + return NULL;
> +}
> +
> static struct rtnl_link_af_ops bridge_ops = {
> .ao_family = AF_BRIDGE,
> .ao_alloc = &bridge_alloc,
> @@ -511,6 +712,8 @@ static struct rtnl_link_af_ops bridge_ops = {
> .ao_parse_protinfo = &bridge_parse_protinfo,
> .ao_dump[NL_DUMP_DETAILS] = &bridge_dump_details,
> .ao_compare = &bridge_compare,
> + .ao_parse_af = &bridge_parse_af,
> + .ao_get_af = &bridge_get_af,
> };
>
> static void __init bridge_init(void)
> diff --git a/lib/route/neigh.c b/lib/route/neigh.c
> index 6059e7f4d8f3..436d766a6a11 100644
> --- a/lib/route/neigh.c
> +++ b/lib/route/neigh.c
> @@ -210,6 +210,7 @@ static void neigh_keygen(struct nl_object *obj,
> uint32_t *hashkey,
> struct neigh_hash_key {
> uint32_t n_family;
> uint32_t n_ifindex;
> + uint16_t n_vlan;
> char n_addr[0];
> } __attribute__((packed)) *nkey;
> #ifdef NL_DEBUG
> @@ -234,6 +235,7 @@ static void neigh_keygen(struct nl_object *obj,
> uint32_t *hashkey,
> }
> nkey->n_family = neigh->n_family;
> if (neigh->n_family == AF_BRIDGE) {
> + nkey->n_vlan = neigh->n_vlan;
> if (neigh->n_flags & NTF_SELF)
> nkey->n_ifindex = neigh->n_ifindex;
> else
> @@ -316,9 +318,9 @@ static uint32_t neigh_id_attrs_get(struct
> nl_object *obj)
>
> if (neigh->n_family == AF_BRIDGE) {
> if (neigh->n_flags & NTF_SELF)
> - return (NEIGH_ATTR_LLADDR |
> NEIGH_ATTR_FAMILY | NEIGH_ATTR_IFINDEX);
> + return (NEIGH_ATTR_LLADDR |
> NEIGH_ATTR_FAMILY | NEIGH_ATTR_IFINDEX | NEIGH_ATTR_VLAN);
> else
> - return (NEIGH_ATTR_LLADDR |
> NEIGH_ATTR_FAMILY | NEIGH_ATTR_MASTER);
> + return (NEIGH_ATTR_LLADDR |
> NEIGH_ATTR_FAMILY | NEIGH_ATTR_MASTER | NEIGH_ATTR_VLAN);
> } else
> return (NEIGH_ATTR_IFINDEX | NEIGH_ATTR_DST |
> NEIGH_ATTR_FAMILY);
> }
> @@ -472,6 +474,9 @@ static void neigh_dump_line(struct nl_object *a,
> struct nl_dump_params *p)
> nl_dump(p, "lladdr %s ",
> nl_addr2str(n->n_lladdr, lladdr,
> sizeof(lladdr)));
>
> + if (n->ce_mask & NEIGH_ATTR_VLAN)
> + nl_dump(p, "vlan %d ", n->n_vlan);
> +
> rtnl_neigh_state2str(n->n_state, state, sizeof(state));
> rtnl_neigh_flags2str(n->n_flags, flags, sizeof(flags));
>
More information about the libnl
mailing list