All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/8] bridge: new security features
@ 2012-10-30  0:57 Stephen Hemminger
  2012-10-30  0:57 ` [PATCH net-next 1/8] bridge: bridge port parameters over netlink Stephen Hemminger
                   ` (7 more replies)
  0 siblings, 8 replies; 18+ messages in thread
From: Stephen Hemminger @ 2012-10-30  0:57 UTC (permalink / raw)
  To: davem; +Cc: netdev

This set implements some new API's and features to allow limiting
reception of Spanning Tree Protocol packets.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH net-next 1/8] bridge: bridge port parameters over netlink
  2012-10-30  0:57 [PATCH net-next 0/8] bridge: new security features Stephen Hemminger
@ 2012-10-30  0:57 ` Stephen Hemminger
  2012-10-30 18:48   ` Tommy S. Christensen
  2012-10-31 14:01   ` John Fastabend
  2012-10-30  0:57 ` [PATCH net-next 2/8] bridge: add template for bridge port flags Stephen Hemminger
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 18+ messages in thread
From: Stephen Hemminger @ 2012-10-30  0:57 UTC (permalink / raw)
  To: davem; +Cc: netdev

[-- Attachment #1: bridge-port-flags-netlink.patch --]
[-- Type: text/plain, Size: 8141 bytes --]

Expose bridge port parameters over netlink. By switching 
from a single byet to a nested message, this can be used for
other bridge parameters.

Although, this changes IFLA_PROTINFO attribute from one byte to a full nested
set of attributes; it is safe for applications because the
old message used IFLA_PROTINFO and new one uses
 IFLA_PROTINFO | NLA_F_NESTED.

The code still accepts to old format requests, and therefore stays
compatiable with user mode RSTP daemon. Since the type field
for nested and unnested attributes are different, and the old
code in libnetlink doesn't do the mask, it is also safe to use
with old versions of bridge monitor command.

Note: although mode is only a boolean, treating it as a
full byte since in the future someone will probably want to add more
values (like macvlan has).

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

---
 include/uapi/linux/if_link.h |   10 ++
 net/bridge/br_netlink.c      |  145 ++++++++++++++++++++++++++++++++-----------
 2 files changed, 119 insertions(+), 36 deletions(-)

--- a/include/uapi/linux/if_link.h	2012-10-29 15:59:40.289955276 -0700
+++ b/include/uapi/linux/if_link.h	2012-10-29 16:10:15.263578236 -0700
@@ -205,6 +205,21 @@ enum {
 
 #define IFLA_INET6_MAX	(__IFLA_INET6_MAX - 1)
 
+enum {
+	BRIDGE_MODE_UNSPEC,
+	BRIDGE_MODE_HAIRPIN,
+};
+
+enum {
+	IFLA_BRPORT_UNSPEC,
+	IFLA_BRPORT_STATE,	/* Spanning tree state     */
+	IFLA_BRPORT_PRIORITY,	/* "             priority  */
+	IFLA_BRPORT_COST,	/* "             cost      */
+	IFLA_BRPORT_MODE,	/* mode (hairpin)          */
+	__IFLA_BRPORT_MAX
+};
+#define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
+
 struct ifla_cacheinfo {
 	__u32	max_reasm_len;
 	__u32	tstamp;		/* ipv6InterfaceTable updated timestamp */
--- a/net/bridge/br_netlink.c	2012-10-29 15:59:40.289955276 -0700
+++ b/net/bridge/br_netlink.c	2012-10-29 16:14:21.069109611 -0700
@@ -20,16 +20,39 @@
 #include "br_private.h"
 #include "br_private_stp.h"
 
+static inline size_t br_port_info_size(void)
+{
+	return nla_total_size(1)	/* IFLA_BRPORT_STATE  */
+		+ nla_total_size(2)	/* IFLA_BRPORT_PRIORITY */
+		+ nla_total_size(4)	/* IFLA_BRPORT_COST */
+		+ nla_total_size(1)	/* IFLA_BRPORT_MODE */
+		+ 0;
+}
+
 static inline size_t br_nlmsg_size(void)
 {
 	return NLMSG_ALIGN(sizeof(struct ifinfomsg))
-	       + nla_total_size(IFNAMSIZ) /* IFLA_IFNAME */
-	       + nla_total_size(MAX_ADDR_LEN) /* IFLA_ADDRESS */
-	       + nla_total_size(4) /* IFLA_MASTER */
-	       + nla_total_size(4) /* IFLA_MTU */
-	       + nla_total_size(4) /* IFLA_LINK */
-	       + nla_total_size(1) /* IFLA_OPERSTATE */
-	       + nla_total_size(1); /* IFLA_PROTINFO */
+		+ nla_total_size(IFNAMSIZ) /* IFLA_IFNAME */
+		+ nla_total_size(MAX_ADDR_LEN) /* IFLA_ADDRESS */
+		+ nla_total_size(4) /* IFLA_MASTER */
+		+ nla_total_size(4) /* IFLA_MTU */
+		+ nla_total_size(4) /* IFLA_LINK */
+		+ nla_total_size(1) /* IFLA_OPERSTATE */
+		+ nla_total_size(br_port_info_size()); /* IFLA_PROTINFO */
+}
+
+static int br_port_fill_attrs(struct sk_buff *skb,
+			      const struct net_bridge_port *p)
+{
+	u8 mode = !!(p->flags & BR_HAIRPIN_MODE);
+
+	if (nla_put_u8(skb, IFLA_BRPORT_STATE, p->state) ||
+	    nla_put_u16(skb, IFLA_BRPORT_PRIORITY, p->priority) ||
+	    nla_put_u32(skb, IFLA_BRPORT_COST, p->path_cost) ||
+	    nla_put_u8(skb, IFLA_BRPORT_MODE, mode))
+		return -EMSGSIZE;
+
+	return 0;
 }
 
 /*
@@ -67,10 +90,18 @@ static int br_fill_ifinfo(struct sk_buff
 	    (dev->addr_len &&
 	     nla_put(skb, IFLA_ADDRESS, dev->addr_len, dev->dev_addr)) ||
 	    (dev->ifindex != dev->iflink &&
-	     nla_put_u32(skb, IFLA_LINK, dev->iflink)) ||
-	    (event == RTM_NEWLINK &&
-	     nla_put_u8(skb, IFLA_PROTINFO, port->state)))
+	     nla_put_u32(skb, IFLA_LINK, dev->iflink)))
 		goto nla_put_failure;
+
+	if (event == RTM_NEWLINK) {
+		struct nlattr *nest
+			= nla_nest_start(skb, IFLA_PROTINFO | NLA_F_NESTED);
+
+		if (nest == NULL || br_port_fill_attrs(skb, port) < 0)
+			goto nla_put_failure;
+		nla_nest_end(skb, nest);
+	}
+
 	return nlmsg_end(skb, nlh);
 
 nla_put_failure:
@@ -140,60 +171,127 @@ skip:
 	return skb->len;
 }
 
-/*
- * Change state of port (ie from forwarding to blocking etc)
+static const struct nla_policy ifla_brport_policy[IFLA_BRPORT_MAX + 1] = {
+	[IFLA_BRPORT_STATE]	= { .type = NLA_U8 },
+	[IFLA_BRPORT_COST]	= { .type = NLA_U16 },
+	[IFLA_BRPORT_PRIORITY]	= { .type = NLA_U32 },
+	[IFLA_BRPORT_MODE]	= { .type = NLA_U8 },
+};
+
+/* Change state of port (ie from forwarding to blocking etc)
  * Used by spanning tree in user space.
  */
-static int br_rtm_setlink(struct sk_buff *skb,  struct nlmsghdr *nlh, void *arg)
+static int br_rtm_set_port_state(struct net_bridge_port *p, u8 state)
+{
+	if (state > BR_STATE_BLOCKING)
+		return -EINVAL;
+
+	/* if kernel STP is running, don't allow changes */
+	if (p->br->stp_enabled == BR_KERNEL_STP)
+		return -EBUSY;
+
+	if (!netif_running(p->dev) ||
+	    (!netif_carrier_ok(p->dev) && state != BR_STATE_DISABLED))
+		return -ENETDOWN;
+
+	p->state = state;
+	br_log_state(p);
+	br_port_state_selection(p->br);
+	return 0;
+}
+
+static void br_rtm_setflag(struct net_bridge_port *p, struct nlattr *tb[],
+			   int attrtype, unsigned long mask)
+{
+	if (tb[attrtype]) {
+		u8 flag = nla_get_u8(tb[attrtype]);
+		if (flag)
+			p->flags |= mask;
+		else
+			p->flags &= ~mask;
+	}
+}
+
+/* Process bridge protocol info on port */
+static int br_rtm_protinfo(struct net_bridge_port *p, struct nlattr *tb[])
+{
+	int err;
+
+	br_rtm_setflag(p, tb, IFLA_BRPORT_MODE, BR_HAIRPIN_MODE);
+
+	if (tb[IFLA_BRPORT_COST]) {
+		err = br_stp_set_path_cost(p, nla_get_u32(tb[IFLA_BRPORT_COST]));
+		if (err)
+			return err;
+	}
+
+	if (tb[IFLA_BRPORT_PRIORITY]) {
+		err = br_stp_set_port_priority(p, nla_get_u16(tb[IFLA_BRPORT_PRIORITY]));
+		if (err)
+			return err;
+	}
+
+	if (tb[IFLA_BRPORT_STATE]) {
+		err = br_rtm_set_port_state(p, nla_get_u8(tb[IFLA_BRPORT_STATE]));
+		if (err)
+			return err;
+	}
+	return 0;
+}
+
+/* Change state and parameters on port. */
+static int br_rtm_setlink(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
 {
 	struct net *net = sock_net(skb->sk);
 	struct ifinfomsg *ifm;
-	struct nlattr *protinfo;
+	struct nlattr *pinfo;
 	struct net_device *dev;
 	struct net_bridge_port *p;
-	u8 new_state;
-
-	if (nlmsg_len(nlh) < sizeof(*ifm))
-		return -EINVAL;
+	struct nlattr *tb[IFLA_BRPORT_MAX];
+	int err;
 
 	ifm = nlmsg_data(nlh);
 	if (ifm->ifi_family != AF_BRIDGE)
 		return -EPFNOSUPPORT;
 
-	protinfo = nlmsg_find_attr(nlh, sizeof(*ifm), IFLA_PROTINFO);
-	if (!protinfo || nla_len(protinfo) < sizeof(u8))
-		return -EINVAL;
-
-	new_state = nla_get_u8(protinfo);
-	if (new_state > BR_STATE_BLOCKING)
-		return -EINVAL;
+	pinfo = nlmsg_find_attr(nlh, sizeof(*ifm), IFLA_PROTINFO);
+	if (!pinfo)
+		return 0;
 
 	dev = __dev_get_by_index(net, ifm->ifi_index);
-	if (!dev)
+	if (!dev) {
+		pr_debug("br_rtm_setlink: ifindex %u not found\n",
+			 ifm->ifi_index);
 		return -ENODEV;
+	}
 
 	p = br_port_get_rtnl(dev);
-	if (!p)
+	if (!p) {
+		pr_debug("br_rtm_setlink: device %s not a bridge port\n",
+			 dev->name);
 		return -EINVAL;
+	}
 
-	/* if kernel STP is running, don't allow changes */
-	if (p->br->stp_enabled == BR_KERNEL_STP)
-		return -EBUSY;
-
-	if (!netif_running(dev) ||
-	    (!netif_carrier_ok(dev) && new_state != BR_STATE_DISABLED))
-		return -ENETDOWN;
-
-	p->state = new_state;
-	br_log_state(p);
-
-	spin_lock_bh(&p->br->lock);
-	br_port_state_selection(p->br);
-	spin_unlock_bh(&p->br->lock);
+	if (pinfo->nla_type & NLA_F_NESTED) {
+		err = nla_parse_nested(tb, IFLA_BRPORT_MAX,
+				       pinfo, ifla_brport_policy);
+		if (err)
+			return err;
+
+		spin_lock_bh(&p->br->lock);
+		err = br_rtm_protinfo(p, tb);
+		spin_unlock_bh(&p->br->lock);
+	} else {
+		/* Binary compatability with old RSTP */
+		spin_lock_bh(&p->br->lock);
+		err = br_rtm_set_port_state(p, nla_get_u8(pinfo));
+		spin_unlock_bh(&p->br->lock);
+	}
 
-	br_ifinfo_notify(RTM_NEWLINK, p);
+	if (err == 0)
+		br_ifinfo_notify(RTM_NEWLINK, p);
 
-	return 0;
+	return err;
 }
 
 static int br_validate(struct nlattr *tb[], struct nlattr *data[])

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH net-next 2/8] bridge: add template for bridge port flags
  2012-10-30  0:57 [PATCH net-next 0/8] bridge: new security features Stephen Hemminger
  2012-10-30  0:57 ` [PATCH net-next 1/8] bridge: bridge port parameters over netlink Stephen Hemminger
@ 2012-10-30  0:57 ` Stephen Hemminger
  2012-10-30  0:57 ` [PATCH net-next 3/8] bridge: implement BPDU blocking Stephen Hemminger
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Stephen Hemminger @ 2012-10-30  0:57 UTC (permalink / raw)
  To: davem; +Cc: netdev

[-- Attachment #1: brport-flags-sysfs.patch --]
[-- Type: text/plain, Size: 1902 bytes --]

Provide macro to build sysfs data structures and functions
for accessing flag bits.  If flag bits change do netlink
notification.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>


--- a/net/bridge/br_sysfs_if.c	2012-10-29 16:40:07.741155824 -0700
+++ b/net/bridge/br_sysfs_if.c	2012-10-29 16:50:49.950706115 -0700
@@ -34,6 +34,28 @@ const struct brport_attribute brport_att
 	.store	= _store,					\
 };
 
+#define BRPORT_ATTR_FLAG(_name, _mask)				\
+static ssize_t show_##_name(struct net_bridge_port *p, char *buf) \
+{								\
+	return sprintf(buf, "%d\n", !!(p->flags & _mask));	\
+}								\
+static int store_##_name(struct net_bridge_port *p, unsigned long v) \
+{								\
+	unsigned long flags = p->flags;				\
+	if (v)							\
+		flags |= _mask;					\
+	else							\
+		flags &= ~_mask;				\
+	if (flags != p->flags) {				\
+		p->flags = flags;				\
+		br_ifinfo_notify(RTM_NEWLINK, p);		\
+	}							\
+	return 0;						\
+}								\
+static BRPORT_ATTR(_name, S_IRUGO | S_IWUSR,			\
+		   show_##_name, store_##_name)
+
+
 static ssize_t show_path_cost(struct net_bridge_port *p, char *buf)
 {
 	return sprintf(buf, "%d\n", p->path_cost);
@@ -133,21 +155,7 @@ static int store_flush(struct net_bridge
 }
 static BRPORT_ATTR(flush, S_IWUSR, NULL, store_flush);
 
-static ssize_t show_hairpin_mode(struct net_bridge_port *p, char *buf)
-{
-	int hairpin_mode = (p->flags & BR_HAIRPIN_MODE) ? 1 : 0;
-	return sprintf(buf, "%d\n", hairpin_mode);
-}
-static int store_hairpin_mode(struct net_bridge_port *p, unsigned long v)
-{
-	if (v)
-		p->flags |= BR_HAIRPIN_MODE;
-	else
-		p->flags &= ~BR_HAIRPIN_MODE;
-	return 0;
-}
-static BRPORT_ATTR(hairpin_mode, S_IRUGO | S_IWUSR,
-		   show_hairpin_mode, store_hairpin_mode);
+BRPORT_ATTR_FLAG(hairpin_mode, BR_HAIRPIN_MODE);
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 static ssize_t show_multicast_router(struct net_bridge_port *p, char *buf)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH net-next 3/8] bridge: implement BPDU blocking
  2012-10-30  0:57 [PATCH net-next 0/8] bridge: new security features Stephen Hemminger
  2012-10-30  0:57 ` [PATCH net-next 1/8] bridge: bridge port parameters over netlink Stephen Hemminger
  2012-10-30  0:57 ` [PATCH net-next 2/8] bridge: add template for bridge port flags Stephen Hemminger
@ 2012-10-30  0:57 ` Stephen Hemminger
  2012-10-31  2:38   ` Cong Wang
  2012-10-30  0:57 ` [PATCH net-next 4/8] bridge: add root port blocking Stephen Hemminger
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Stephen Hemminger @ 2012-10-30  0:57 UTC (permalink / raw)
  To: davem; +Cc: netdev

[-- Attachment #1: bridge-bpdu-guard.patch --]
[-- Type: text/plain, Size: 4296 bytes --]

This is Linux bridge implementation of STP protection
(Cisco BPDU guard/Juniper BPDU block). BPDU block disables
the bridge port if a STP BPDU packet is received.

Why would you want to do this?
If running Spanning Tree on bridge, hostile devices on the network
may send BPDU and cause network failure. Enabling bpdu block
will detect and stop this.

How to recover the port?
The port will be restarted if link is brought down, or
removed and reattached.  For example:
 # ip li set dev eth0 down; ip li set dev eth0 up

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

---
 include/uapi/linux/if_link.h |    1 +
 net/bridge/br_netlink.c      |    7 ++++++-
 net/bridge/br_private.h      |    1 +
 net/bridge/br_stp_bpdu.c     |    7 +++++++
 net/bridge/br_sysfs_if.c     |    2 ++
 5 files changed, 17 insertions(+), 1 deletion(-)

--- a/net/bridge/br_private.h	2012-10-29 17:07:08.005982838 -0700
+++ b/net/bridge/br_private.h	2012-10-29 17:37:20.959775346 -0700
@@ -135,6 +135,7 @@ struct net_bridge_port
 
 	unsigned long 			flags;
 #define BR_HAIRPIN_MODE		0x00000001
+#define BR_BPDU_GUARD           0x00000002
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 	u32				multicast_startup_queries_sent;
--- a/net/bridge/br_stp_bpdu.c	2012-10-29 17:07:08.005982838 -0700
+++ b/net/bridge/br_stp_bpdu.c	2012-10-29 17:31:14.327457434 -0700
@@ -170,6 +170,13 @@ void br_stp_rcv(const struct stp_proto *
 	if (!ether_addr_equal(dest, br->group_addr))
 		goto out;
 
+	if (p->flags & BR_BPDU_GUARD) {
+		br_notice(br, "BPDU received on blocked port %u(%s)\n",
+			  (unsigned int) p->port_no, p->dev->name);
+		br_stp_disable_port(p);
+		goto out;
+	}
+
 	buf = skb_pull(skb, 3);
 
 	if (buf[0] == BPDU_TYPE_CONFIG) {
--- a/net/bridge/br_sysfs_if.c	2012-10-29 17:19:30.562525344 -0700
+++ b/net/bridge/br_sysfs_if.c	2012-10-29 17:37:38.207602125 -0700
@@ -156,6 +156,7 @@ static int store_flush(struct net_bridge
 static BRPORT_ATTR(flush, S_IWUSR, NULL, store_flush);
 
 BRPORT_ATTR_FLAG(hairpin_mode, BR_HAIRPIN_MODE);
+BRPORT_ATTR_FLAG(bpdu_guard, BR_BPDU_GUARD);
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 static ssize_t show_multicast_router(struct net_bridge_port *p, char *buf)
@@ -189,6 +190,7 @@ static const struct brport_attribute *br
 	&brport_attr_hold_timer,
 	&brport_attr_flush,
 	&brport_attr_hairpin_mode,
+	&brport_attr_bpdu_block,
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 	&brport_attr_multicast_router,
 #endif
--- a/include/uapi/linux/if_link.h	2012-10-29 17:07:14.713915471 -0700
+++ b/include/uapi/linux/if_link.h	2012-10-29 17:37:20.959775346 -0700
@@ -216,6 +216,7 @@ enum {
 	IFLA_BRPORT_PRIORITY,	/* "             priority  */
 	IFLA_BRPORT_COST,	/* "             cost      */
 	IFLA_BRPORT_MODE,	/* mode (hairpin)          */
+	IFLA_BRPORT_GUARD,	/* bpdu guard              */
 	__IFLA_BRPORT_MAX
 };
 #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
--- a/net/bridge/br_netlink.c	2012-10-29 17:09:23.520621865 -0700
+++ b/net/bridge/br_netlink.c	2012-10-29 17:37:20.959775346 -0700
@@ -26,6 +26,7 @@ static inline size_t br_port_info_size(v
 		+ nla_total_size(2)	/* IFLA_BRPORT_PRIORITY */
 		+ nla_total_size(4)	/* IFLA_BRPORT_COST */
 		+ nla_total_size(1)	/* IFLA_BRPORT_MODE */
+		+ nla_total_size(1)	/* IFLA_BRPORT_GUARD */
 		+ 0;
 }
 
@@ -49,7 +50,8 @@ static int br_port_fill_attrs(struct sk_
 	if (nla_put_u8(skb, IFLA_BRPORT_STATE, p->state) ||
 	    nla_put_u16(skb, IFLA_BRPORT_PRIORITY, p->priority) ||
 	    nla_put_u32(skb, IFLA_BRPORT_COST, p->path_cost) ||
-	    nla_put_u8(skb, IFLA_BRPORT_MODE, mode))
+	    nla_put_u8(skb, IFLA_BRPORT_MODE, mode) ||
+	    nla_put_u8(skb, IFLA_BRPORT_GUARD, !!(p->flags & BR_BPDU_GUARD)))
 		return -EMSGSIZE;
 
 	return 0;
@@ -176,6 +178,7 @@ static const struct nla_policy ifla_brpo
 	[IFLA_BRPORT_COST]	= { .type = NLA_U16 },
 	[IFLA_BRPORT_PRIORITY]	= { .type = NLA_U32 },
 	[IFLA_BRPORT_MODE]	= { .type = NLA_U8 },
+	[IFLA_BRPORT_GUARD]	= { .type = NLA_U8 },
 };
 
 /* Change state of port (ie from forwarding to blocking etc)
@@ -218,6 +221,7 @@ static int br_rtm_protinfo(struct net_br
 	int err;
 
 	br_rtm_setflag(p, tb, IFLA_BRPORT_MODE, BR_HAIRPIN_MODE);
+	br_rtm_setflag(p, tb, IFLA_BRPORT_GUARD, BR_BPDU_GUARD);
 
 	if (tb[IFLA_BRPORT_COST]) {
 		err = br_stp_set_path_cost(p, nla_get_u32(tb[IFLA_BRPORT_COST]));

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH net-next 4/8] bridge: add root port blocking
  2012-10-30  0:57 [PATCH net-next 0/8] bridge: new security features Stephen Hemminger
                   ` (2 preceding siblings ...)
  2012-10-30  0:57 ` [PATCH net-next 3/8] bridge: implement BPDU blocking Stephen Hemminger
@ 2012-10-30  0:57 ` Stephen Hemminger
  2012-10-30  0:57 ` [PATCH net-next 5/8] bridge: add bpdu filter Stephen Hemminger
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Stephen Hemminger @ 2012-10-30  0:57 UTC (permalink / raw)
  To: davem; +Cc: netdev

[-- Attachment #1: bridge-root-block.patch --]
[-- Type: text/plain, Size: 4600 bytes --]

This is Linux bridge implementation of root port guard.
If BPDU is received from a leaf (edge) port, it should not
be elected as root port.

Why would you want to do this?
If using STP on a bridge and the downstream bridges are not fully
trusted; this prevents a hostile guest for rerouting traffic.

Why not just use netfilter?
Netfilter does not track of follow spanning tree decisions.
It would be difficult and error prone to try and mirror STP
resolution in netfilter module.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>


---
 include/uapi/linux/if_link.h |    1 +
 net/bridge/br_netlink.c      |    6 +++++-
 net/bridge/br_private.h      |    1 +
 net/bridge/br_stp.c          |   22 +++++++++++++++++++++-
 net/bridge/br_sysfs_if.c     |    2 ++
 5 files changed, 30 insertions(+), 2 deletions(-)

--- a/net/bridge/br_private.h	2012-10-29 17:39:57.902199174 -0700
+++ b/net/bridge/br_private.h	2012-10-29 17:40:20.373973489 -0700
@@ -136,6 +136,7 @@ struct net_bridge_port
 	unsigned long 			flags;
 #define BR_HAIRPIN_MODE		0x00000001
 #define BR_BPDU_GUARD           0x00000002
+#define BR_ROOT_BLOCK		0x00000004
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 	u32				multicast_startup_queries_sent;
--- a/net/bridge/br_stp.c	2012-10-29 17:39:57.902199174 -0700
+++ b/net/bridge/br_stp.c	2012-10-29 17:40:20.373973489 -0700
@@ -100,6 +100,21 @@ static int br_should_become_root_port(co
 	return 0;
 }
 
+static void br_root_port_block(const struct net_bridge *br,
+			       struct net_bridge_port *p)
+{
+
+	br_notice(br, "port %u(%s) tried to become root port (blocked)",
+		  (unsigned int) p->port_no, p->dev->name);
+
+	p->state = BR_STATE_LISTENING;
+	br_log_state(p);
+	br_ifinfo_notify(RTM_NEWLINK, p);
+
+	if (br->forward_delay > 0)
+		mod_timer(&p->forward_delay_timer, jiffies + br->forward_delay);
+}
+
 /* called under bridge lock */
 static void br_root_selection(struct net_bridge *br)
 {
@@ -107,7 +122,12 @@ static void br_root_selection(struct net
 	u16 root_port = 0;
 
 	list_for_each_entry(p, &br->port_list, list) {
-		if (br_should_become_root_port(p, root_port))
+		if (!br_should_become_root_port(p, root_port))
+			continue;
+
+		if (p->flags & BR_ROOT_BLOCK)
+			br_root_port_block(br, p);
+		else
 			root_port = p->port_no;
 	}
 
--- a/net/bridge/br_sysfs_if.c	2012-10-29 17:40:15.230025149 -0700
+++ b/net/bridge/br_sysfs_if.c	2012-10-29 17:40:37.869797779 -0700
@@ -157,6 +157,7 @@ static BRPORT_ATTR(flush, S_IWUSR, NULL,
 
 BRPORT_ATTR_FLAG(hairpin_mode, BR_HAIRPIN_MODE);
 BRPORT_ATTR_FLAG(bpdu_guard, BR_BPDU_GUARD);
+BRPORT_ATTR_FLAG(root_block, BR_ROOT_BLOCK);
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 static ssize_t show_multicast_router(struct net_bridge_port *p, char *buf)
@@ -191,6 +192,7 @@ static const struct brport_attribute *br
 	&brport_attr_flush,
 	&brport_attr_hairpin_mode,
 	&brport_attr_bpdu_guard,
+	&brport_attr_root_block,
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 	&brport_attr_multicast_router,
 #endif
--- a/include/uapi/linux/if_link.h	2012-10-29 17:39:57.902199174 -0700
+++ b/include/uapi/linux/if_link.h	2012-10-29 17:40:20.373973489 -0700
@@ -217,6 +217,7 @@ enum {
 	IFLA_BRPORT_COST,	/* "             cost      */
 	IFLA_BRPORT_MODE,	/* mode (hairpin)          */
 	IFLA_BRPORT_GUARD,	/* bpdu guard              */
+	IFLA_BRPORT_PROTECT,	/* root port protection    */
 	__IFLA_BRPORT_MAX
 };
 #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
--- a/net/bridge/br_netlink.c	2012-10-29 17:39:57.902199174 -0700
+++ b/net/bridge/br_netlink.c	2012-10-29 17:40:20.373973489 -0700
@@ -27,6 +27,7 @@ static inline size_t br_port_info_size(v
 		+ nla_total_size(4)	/* IFLA_BRPORT_COST */
 		+ nla_total_size(1)	/* IFLA_BRPORT_MODE */
 		+ nla_total_size(1)	/* IFLA_BRPORT_GUARD */
+		+ nla_total_size(1)	/* IFLA_BRPORT_PROTECT */
 		+ 0;
 }
 
@@ -51,7 +52,8 @@ static int br_port_fill_attrs(struct sk_
 	    nla_put_u16(skb, IFLA_BRPORT_PRIORITY, p->priority) ||
 	    nla_put_u32(skb, IFLA_BRPORT_COST, p->path_cost) ||
 	    nla_put_u8(skb, IFLA_BRPORT_MODE, mode) ||
-	    nla_put_u8(skb, IFLA_BRPORT_GUARD, !!(p->flags & BR_BPDU_GUARD)))
+	    nla_put_u8(skb, IFLA_BRPORT_GUARD, !!(p->flags & BR_BPDU_GUARD)) ||
+	    nla_put_u8(skb, IFLA_BRPORT_PROTECT, !!(p->flags & BR_ROOT_BLOCK)))
 		return -EMSGSIZE;
 
 	return 0;
@@ -179,6 +181,7 @@ static const struct nla_policy ifla_brpo
 	[IFLA_BRPORT_PRIORITY]	= { .type = NLA_U32 },
 	[IFLA_BRPORT_MODE]	= { .type = NLA_U8 },
 	[IFLA_BRPORT_GUARD]	= { .type = NLA_U8 },
+	[IFLA_BRPORT_PROTECT]	= { .type = NLA_U8 },
 };
 
 /* Change state of port (ie from forwarding to blocking etc)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH net-next 5/8] bridge: add bpdu filter
  2012-10-30  0:57 [PATCH net-next 0/8] bridge: new security features Stephen Hemminger
                   ` (3 preceding siblings ...)
  2012-10-30  0:57 ` [PATCH net-next 4/8] bridge: add root port blocking Stephen Hemminger
@ 2012-10-30  0:57 ` Stephen Hemminger
  2012-10-30  0:57 ` [PATCH net-next 6/8] tun: implement byte queue limits Stephen Hemminger
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Stephen Hemminger @ 2012-10-30  0:57 UTC (permalink / raw)
  To: davem; +Cc: netdev

[-- Attachment #1: bridge-filter.patch --]
[-- Type: text/plain, Size: 3300 bytes --]

BPDU filter allows spanning tree to be disabled for a bridge
but not send or receive BPDU packets on a specific port. A common
usage of this to turn on spanning tree (so that bridge can talk
to other bridges), but turn off STP packets to leaf virtual devices.

This could be done with ebtables, but that adds another set of layers
which hurts performance and is much more difficult to manage.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/net/bridge/br_private.h	2012-10-29 17:40:20.373973489 -0700
+++ b/net/bridge/br_private.h	2012-10-29 17:40:48.945686542 -0700
@@ -137,6 +137,7 @@ struct net_bridge_port
 #define BR_HAIRPIN_MODE		0x00000001
 #define BR_BPDU_GUARD           0x00000002
 #define BR_ROOT_BLOCK		0x00000004
+#define BR_BPDU_FILTER		0x00000008
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 	u32				multicast_startup_queries_sent;
--- a/net/bridge/br_stp_bpdu.c	2012-10-29 17:39:53.910239263 -0700
+++ b/net/bridge/br_stp_bpdu.c	2012-10-29 17:40:48.945686542 -0700
@@ -78,6 +78,9 @@ void br_send_config_bpdu(struct net_brid
 	if (p->br->stp_enabled != BR_KERNEL_STP)
 		return;
 
+	if (p->flags & BR_BPDU_FILTER)
+		return;
+
 	buf[0] = 0;
 	buf[1] = 0;
 	buf[2] = 0;
@@ -123,6 +126,9 @@ void br_send_tcn_bpdu(struct net_bridge_
 	if (p->br->stp_enabled != BR_KERNEL_STP)
 		return;
 
+	if (p->flags & BR_BPDU_FILTER)
+		return;
+
 	buf[0] = 0;
 	buf[1] = 0;
 	buf[2] = 0;
@@ -170,6 +176,9 @@ void br_stp_rcv(const struct stp_proto *
 	if (!ether_addr_equal(dest, br->group_addr))
 		goto out;
 
+	if (p->flags & BR_BPDU_FILTER)
+		goto out;
+
 	if (p->flags & BR_BPDU_GUARD) {
 		br_notice(br, "BPDU received on blocked port %u(%s)\n",
 			  (unsigned int) p->port_no, p->dev->name);
--- a/net/bridge/br_sysfs_if.c	2012-10-29 17:40:37.869797779 -0700
+++ b/net/bridge/br_sysfs_if.c	2012-10-29 17:40:48.945686542 -0700
@@ -158,6 +158,7 @@ static BRPORT_ATTR(flush, S_IWUSR, NULL,
 BRPORT_ATTR_FLAG(hairpin_mode, BR_HAIRPIN_MODE);
 BRPORT_ATTR_FLAG(bpdu_guard, BR_BPDU_GUARD);
 BRPORT_ATTR_FLAG(root_block, BR_ROOT_BLOCK);
+BRPORT_ATTR_FLAG(bpdu_filter, BR_BPDU_FILTER);
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 static ssize_t show_multicast_router(struct net_bridge_port *p, char *buf)
@@ -193,6 +194,7 @@ static const struct brport_attribute *br
 	&brport_attr_hairpin_mode,
 	&brport_attr_bpdu_guard,
 	&brport_attr_root_block,
+	&brport_attr_bpdu_filter,
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 	&brport_attr_multicast_router,
 #endif
--- a/include/uapi/linux/if_link.h	2012-10-29 17:40:20.373973489 -0700
+++ b/include/uapi/linux/if_link.h	2012-10-29 17:40:48.945686542 -0700
@@ -218,6 +218,7 @@ enum {
 	IFLA_BRPORT_MODE,	/* mode (hairpin)          */
 	IFLA_BRPORT_GUARD,	/* bpdu guard              */
 	IFLA_BRPORT_PROTECT,	/* root port protection    */
+	IFLA_BRPORT_BLOCK,	/* bpdu filter             */
 	__IFLA_BRPORT_MAX
 };
 #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
--- a/net/bridge/br_netlink.c	2012-10-29 17:40:20.373973489 -0700
+++ b/net/bridge/br_netlink.c	2012-10-29 17:40:48.945686542 -0700
@@ -28,6 +28,7 @@ static inline size_t br_port_info_size(v
 		+ nla_total_size(1)	/* IFLA_BRPORT_MODE */
 		+ nla_total_size(1)	/* IFLA_BRPORT_GUARD */
 		+ nla_total_size(1)	/* IFLA_BRPORT_PROTECT */
+		+ nla_total_size(1)	/* IFLA_BRPORT_BLOCK */
 		+ 0;
 }
 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH net-next 6/8] tun: implement byte queue limits
  2012-10-30  0:57 [PATCH net-next 0/8] bridge: new security features Stephen Hemminger
                   ` (4 preceding siblings ...)
  2012-10-30  0:57 ` [PATCH net-next 5/8] bridge: add bpdu filter Stephen Hemminger
@ 2012-10-30  0:57 ` Stephen Hemminger
  2012-10-30  1:09   ` Stephen Hemminger
  2012-10-30  0:57 ` [PATCH net-next 7/8] virtio: make some structures const Stephen Hemminger
  2012-10-30  0:57 ` [PATCH net-next 8/8] iproute2: handle new bridge PROTINFO format Stephen Hemminger
  7 siblings, 1 reply; 18+ messages in thread
From: Stephen Hemminger @ 2012-10-30  0:57 UTC (permalink / raw)
  To: davem; +Cc: netdev

[-- Attachment #1: tap-bql.patch --]
[-- Type: text/plain, Size: 697 bytes --]

Add byte queue limits for tun device transmit.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/drivers/net/tun.c	2012-10-26 14:29:51.172647696 -0700
+++ b/drivers/net/tun.c	2012-10-26 14:31:46.663487824 -0700
@@ -421,6 +421,9 @@ static netdev_tx_t tun_net_xmit(struct s
 		goto drop;
 	skb_orphan(skb);
 
+	/* Update byte queue limits */
+	netdev_sent_queue(dev, skb->len);
+
 	/* Enqueue packet */
 	skb_queue_tail(&tun->socket.sk->sk_receive_queue, skb);
 
@@ -991,6 +994,8 @@ static ssize_t tun_do_read(struct tun_st
 		netif_wake_queue(tun->dev);
 
 		ret = tun_put_user(tun, skb, iv, len);
+
+		netdev_completed_queue(tun->dev, 1, skb->len);
 		kfree_skb(skb);
 		break;
 	}

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH net-next 7/8] virtio: make some structures const
  2012-10-30  0:57 [PATCH net-next 0/8] bridge: new security features Stephen Hemminger
                   ` (5 preceding siblings ...)
  2012-10-30  0:57 ` [PATCH net-next 6/8] tun: implement byte queue limits Stephen Hemminger
@ 2012-10-30  0:57 ` Stephen Hemminger
  2012-10-30  1:09   ` Stephen Hemminger
  2012-10-30  0:57 ` [PATCH net-next 8/8] iproute2: handle new bridge PROTINFO format Stephen Hemminger
  7 siblings, 1 reply; 18+ messages in thread
From: Stephen Hemminger @ 2012-10-30  0:57 UTC (permalink / raw)
  To: davem; +Cc: netdev

[-- Attachment #1: virtio-const.patch --]
[-- Type: text/plain, Size: 979 bytes --]

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>


--- a/drivers/net/virtio_net.c	2012-10-26 12:27:23.911918635 -0700
+++ b/drivers/net/virtio_net.c	2012-10-26 12:52:28.632806729 -0700
@@ -1274,12 +1274,12 @@ static int virtnet_restore(struct virtio
 }
 #endif
 
-static struct virtio_device_id id_table[] = {
+static const struct virtio_device_id id_table[] = {
 	{ VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID },
 	{ 0 },
 };
 
-static unsigned int features[] = {
+static const unsigned int features[] = {
 	VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM,
 	VIRTIO_NET_F_GSO, VIRTIO_NET_F_MAC,
 	VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6,
@@ -1290,7 +1290,7 @@ static unsigned int features[] = {
 	VIRTIO_NET_F_GUEST_ANNOUNCE,
 };
 
-static struct virtio_driver virtio_net_driver = {
+static struct virtio_driver virtio_net_driver __read_mostly = {
 	.feature_table = features,
 	.feature_table_size = ARRAY_SIZE(features),
 	.driver.name =	KBUILD_MODNAME,

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH net-next 8/8] iproute2: handle new bridge PROTINFO format
  2012-10-30  0:57 [PATCH net-next 0/8] bridge: new security features Stephen Hemminger
                   ` (6 preceding siblings ...)
  2012-10-30  0:57 ` [PATCH net-next 7/8] virtio: make some structures const Stephen Hemminger
@ 2012-10-30  0:57 ` Stephen Hemminger
  7 siblings, 0 replies; 18+ messages in thread
From: Stephen Hemminger @ 2012-10-30  0:57 UTC (permalink / raw)
  To: davem; +Cc: netdev

[-- Attachment #1: virtio-bql.patch --]
[-- Type: text/plain, Size: 3886 bytes --]

Support nested attributes over netlink for bridge port state
---
Patch against current -git version of iproute2

 bridge/link.c           |   56 ++++++++++++++++++++++++++++++++++++++++++-----
 include/linux/if_link.h |   18 +++++++++++++++
 lib/libnetlink.c        |    6 +++--
 3 files changed, 72 insertions(+), 8 deletions(-)

diff --git a/bridge/link.c b/bridge/link.c
index edb6fbf..4ae256b 100644
--- a/bridge/link.c
+++ b/bridge/link.c
@@ -69,6 +69,49 @@ static void print_operstate(FILE *f, __u8 state)
 		fprintf(f, "state %s ", oper_states[state]);
 }
 
+static void print_stpstate(FILE *fp, __u8 state)
+{
+	if (state <= BR_STATE_BLOCKING)
+		fprintf(fp, "state %s", port_states[state]);
+	else
+		fprintf(fp, "state (%d)", state);
+}
+
+static void print_protinfo(FILE *f, struct rtattr *pinfo)
+{
+	struct rtattr *pb[IFLA_BRPORT_MAX+1];
+
+	parse_rtattr_nested(pb, IFLA_BRPORT_MAX, pinfo);
+
+	if (pb[IFLA_BRPORT_STATE])
+		print_stpstate(f, rta_getattr_u8(pb[IFLA_BRPORT_STATE]));
+
+	if (pb[IFLA_BRPORT_PRIORITY])
+		fprintf(f, " priority %u",
+			rta_getattr_u16(pb[IFLA_BRPORT_PRIORITY]));
+
+	if (pb[IFLA_BRPORT_COST])
+		fprintf(f, " cost %u",
+			rta_getattr_u32(pb[IFLA_BRPORT_COST]));
+
+
+	if (pb[IFLA_BRPORT_MODE]) {
+		__u8 mode = rta_getattr_u8(pb[IFLA_BRPORT_MODE]);
+		if (mode == BRIDGE_MODE_HAIRPIN)
+			fprintf(f, " hairpin");
+	}
+
+	if (pb[IFLA_BRPORT_BLOCK] && rta_getattr_u8(pb[IFLA_BRPORT_BLOCK]))
+		fprintf(f, " bpdu blocked");
+
+	if (pb[IFLA_BRPORT_GUARD] && rta_getattr_u8(pb[IFLA_BRPORT_GUARD]))
+		fprintf(f, " bpdu guard");
+
+	if (pb[IFLA_BRPORT_PROTECT] && rta_getattr_u8(pb[IFLA_BRPORT_PROTECT]))
+		fprintf(f, " rootprotect");
+
+}
+
 int print_linkinfo(const struct sockaddr_nl *who,
 		   struct nlmsghdr *n, void *arg)
 {
@@ -124,13 +167,14 @@ int print_linkinfo(const struct sockaddr_nl *who,
 			if_indextoname(rta_getattr_u32(tb[IFLA_MASTER]), b1));
 
 	if (tb[IFLA_PROTINFO]) {
-		__u8 state = rta_getattr_u8(tb[IFLA_PROTINFO]);
-		if (state <= BR_STATE_BLOCKING)
-			fprintf(fp, "state %s", port_states[state]);
-		else
-			fprintf(fp, "state (%d)", state);
-	}
+		struct rtattr *pinfo = tb[IFLA_PROTINFO];
 
+		/* Handle both new (nested) and bare format */
+		if (pinfo->rta_type & NLA_F_NESTED)
+			print_protinfo(fp, pinfo);
+ 		else
+			print_stpstate(fp, rta_getattr_u8(tb[IFLA_PROTINFO]));
+	}
 
 	fprintf(fp, "\n");
 	fflush(fp);
diff --git a/include/linux/if_link.h b/include/linux/if_link.h
index 012d95a..cb5e8a7 100644
--- a/include/linux/if_link.h
+++ b/include/linux/if_link.h
@@ -203,6 +203,24 @@ enum {
 
 #define IFLA_INET6_MAX	(__IFLA_INET6_MAX - 1)
 
+enum {
+	BRIDGE_MODE_UNSPEC,
+	BRIDGE_MODE_HAIRPIN,
+};
+
+enum {
+	IFLA_BRPORT_UNSPEC,
+	IFLA_BRPORT_STATE,	/* Spanning tree state     */
+	IFLA_BRPORT_PRIORITY,	/* "             priority  */
+	IFLA_BRPORT_COST,	/* "             cost      */
+	IFLA_BRPORT_MODE,	/* mode (hairpin)          */
+	IFLA_BRPORT_GUARD,	/* bpdu guard              */
+	IFLA_BRPORT_PROTECT,	/* root port protection    */
+	IFLA_BRPORT_BLOCK,	/* bpdu filter             */
+	__IFLA_BRPORT_MAX
+};
+#define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
+
 struct ifla_cacheinfo {
 	__u32	max_reasm_len;
 	__u32	tstamp;		/* ipv6InterfaceTable updated timestamp */
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 8e8c8b9..08dadf9 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -653,9 +653,11 @@ int rta_addattr_l(struct rtattr *rta, int maxlen, int type,
 int parse_rtattr(struct rtattr *tb[], int max, struct rtattr *rta, int len)
 {
 	memset(tb, 0, sizeof(struct rtattr *) * (max + 1));
+
 	while (RTA_OK(rta, len)) {
-		if ((rta->rta_type <= max) && (!tb[rta->rta_type]))
-			tb[rta->rta_type] = rta;
+		__u16 rta_type = rta->rta_type & NLA_TYPE_MASK;
+		if (rta_type <= max && !tb[rta_type])
+			tb[rta_type] = rta;
 		rta = RTA_NEXT(rta,len);
 	}
 	if (len)
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next 7/8] virtio: make some structures const
  2012-10-30  0:57 ` [PATCH net-next 7/8] virtio: make some structures const Stephen Hemminger
@ 2012-10-30  1:09   ` Stephen Hemminger
  0 siblings, 0 replies; 18+ messages in thread
From: Stephen Hemminger @ 2012-10-30  1:09 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: davem, netdev

On Mon, 29 Oct 2012 17:57:38 -0700
Stephen Hemminger <shemminger@vyatta.com> wrote:

> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> 

This one is unrelated, you can ignore it if you want.
Just part of the overall testing pile.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next 6/8] tun: implement byte queue limits
  2012-10-30  0:57 ` [PATCH net-next 6/8] tun: implement byte queue limits Stephen Hemminger
@ 2012-10-30  1:09   ` Stephen Hemminger
  0 siblings, 0 replies; 18+ messages in thread
From: Stephen Hemminger @ 2012-10-30  1:09 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: davem, netdev

On Mon, 29 Oct 2012 17:57:37 -0700
Stephen Hemminger <shemminger@vyatta.com> wrote:

> Add byte queue limits for tun device transmit.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Unrelated patch, that was being tested at same time.
Ignore it.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next 1/8] bridge: bridge port parameters over netlink
  2012-10-30  0:57 ` [PATCH net-next 1/8] bridge: bridge port parameters over netlink Stephen Hemminger
@ 2012-10-30 18:48   ` Tommy S. Christensen
  2012-10-30 21:00     ` Stephen Hemminger
  2012-10-31 14:01   ` John Fastabend
  1 sibling, 1 reply; 18+ messages in thread
From: Tommy S. Christensen @ 2012-10-30 18:48 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: davem, netdev

On 10/30/2012 01:57 AM, Stephen Hemminger wrote:
>
> -/*
> - * Change state of port (ie from forwarding to blocking etc)
> +static const struct nla_policy ifla_brport_policy[IFLA_BRPORT_MAX + 1] = {
> +	[IFLA_BRPORT_STATE]	= { .type = NLA_U8 },
> +	[IFLA_BRPORT_COST]	= { .type = NLA_U16 },
> +	[IFLA_BRPORT_PRIORITY]	= { .type = NLA_U32 },
> +	[IFLA_BRPORT_MODE]	= { .type = NLA_U8 },
> +};
> +

Hey Stephen

It seems you've swapped the sizes of COST and PRIORITY in this part.

-Tommy

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next 1/8] bridge: bridge port parameters over netlink
  2012-10-30 18:48   ` Tommy S. Christensen
@ 2012-10-30 21:00     ` Stephen Hemminger
  0 siblings, 0 replies; 18+ messages in thread
From: Stephen Hemminger @ 2012-10-30 21:00 UTC (permalink / raw)
  To: Tommy S. Christensen; +Cc: davem, netdev

On Tue, 30 Oct 2012 19:48:53 +0100
"Tommy S. Christensen" <tsc@gurgi.dk> wrote:

> On 10/30/2012 01:57 AM, Stephen Hemminger wrote:
> >
> > -/*
> > - * Change state of port (ie from forwarding to blocking etc)
> > +static const struct nla_policy ifla_brport_policy[IFLA_BRPORT_MAX + 1] = {
> > +	[IFLA_BRPORT_STATE]	= { .type = NLA_U8 },
> > +	[IFLA_BRPORT_COST]	= { .type = NLA_U16 },
> > +	[IFLA_BRPORT_PRIORITY]	= { .type = NLA_U32 },
> > +	[IFLA_BRPORT_MODE]	= { .type = NLA_U8 },
> > +};
> > +
> 
> Hey Stephen
> 
> It seems you've swapped the sizes of COST and PRIORITY in this part.
> 
> -Tommy
> 

Ok, will fix in next version.

One alternative I considered would be to use previously unused IFLA_PRIORITY and IFLA_COST
which would be more generic. But these fields only really matter to bridge.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next 3/8] bridge: implement BPDU blocking
  2012-10-30  0:57 ` [PATCH net-next 3/8] bridge: implement BPDU blocking Stephen Hemminger
@ 2012-10-31  2:38   ` Cong Wang
  2012-10-31 20:57     ` Stephen Hemminger
  0 siblings, 1 reply; 18+ messages in thread
From: Cong Wang @ 2012-10-31  2:38 UTC (permalink / raw)
  To: netdev

On Tue, 30 Oct 2012 at 00:57 GMT, Stephen Hemminger <shemminger@vyatta.com> wrote:
> --- a/net/bridge/br_stp_bpdu.c	2012-10-29 17:07:08.005982838 -0700
> +++ b/net/bridge/br_stp_bpdu.c	2012-10-29 17:31:14.327457434 -0700
> @@ -170,6 +170,13 @@ void br_stp_rcv(const struct stp_proto *
>  	if (!ether_addr_equal(dest, br->group_addr))
>  		goto out;
>  
> +	if (p->flags & BR_BPDU_GUARD) {
> +		br_notice(br, "BPDU received on blocked port %u(%s)\n",
> +			  (unsigned int) p->port_no, p->dev->name);

net_ratelimit() ?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next 1/8] bridge: bridge port parameters over netlink
  2012-10-30  0:57 ` [PATCH net-next 1/8] bridge: bridge port parameters over netlink Stephen Hemminger
  2012-10-30 18:48   ` Tommy S. Christensen
@ 2012-10-31 14:01   ` John Fastabend
  2012-10-31 21:30     ` Stephen Hemminger
  1 sibling, 1 reply; 18+ messages in thread
From: John Fastabend @ 2012-10-31 14:01 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: davem, netdev

On 10/29/2012 5:57 PM, Stephen Hemminger wrote:
> Expose bridge port parameters over netlink. By switching
> from a single byet to a nested message, this can be used for
> other bridge parameters.
>
> Although, this changes IFLA_PROTINFO attribute from one byte to a full nested
> set of attributes; it is safe for applications because the
> old message used IFLA_PROTINFO and new one uses
>   IFLA_PROTINFO | NLA_F_NESTED.
>
> The code still accepts to old format requests, and therefore stays
> compatiable with user mode RSTP daemon. Since the type field
> for nested and unnested attributes are different, and the old
> code in libnetlink doesn't do the mask, it is also safe to use
> with old versions of bridge monitor command.
>
> Note: although mode is only a boolean, treating it as a
> full byte since in the future someone will probably want to add more
> values (like macvlan has).
>
> Signed-off-by: Stephen Hemminger<shemminger@vyatta.com>
>
> ---

Stephen,

Did you see these two patches

http://patchwork.ozlabs.org/patch/193900/
http://patchwork.ozlabs.org/patch/193901/

here I added nested bridge attributes to IFLA_AF_SPEC and pass them down
to the drivers as needed. Should we merge these two sets so that we have
only a single nested set of bridge attributes? Either in IFLA_AF_SPEC or
IFLA_PROTINFO.

The attributes in this patch are port specifics and the one in the
patches listed above are bridge specific so in this sense perhaps
its OK to keep them separate. I'm not sure it matters much either
way but thought I would mention it.

Also I suspect these two series will have conflicts but I haven't tried
yet.

Thanks,
John

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next 3/8] bridge: implement BPDU blocking
  2012-10-31  2:38   ` Cong Wang
@ 2012-10-31 20:57     ` Stephen Hemminger
  0 siblings, 0 replies; 18+ messages in thread
From: Stephen Hemminger @ 2012-10-31 20:57 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev

On Wed, 31 Oct 2012 02:38:46 +0000 (UTC)
Cong Wang <xiyou.wangcong@gmail.com> wrote:

> On Tue, 30 Oct 2012 at 00:57 GMT, Stephen Hemminger <shemminger@vyatta.com> wrote:
> > --- a/net/bridge/br_stp_bpdu.c	2012-10-29 17:07:08.005982838 -0700
> > +++ b/net/bridge/br_stp_bpdu.c	2012-10-29 17:31:14.327457434 -0700
> > @@ -170,6 +170,13 @@ void br_stp_rcv(const struct stp_proto *
> >  	if (!ether_addr_equal(dest, br->group_addr))
> >  		goto out;
> >  
> > +	if (p->flags & BR_BPDU_GUARD) {
> > +		br_notice(br, "BPDU received on blocked port %u(%s)\n",
> > +			  (unsigned int) p->port_no, p->dev->name);
> 
> net_ratelimit() ?
> 

Not necessary since this causes port of bridge to become disabled;
which blocks all further packets (including STP) until manually reset.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next 1/8] bridge: bridge port parameters over netlink
  2012-10-31 14:01   ` John Fastabend
@ 2012-10-31 21:30     ` Stephen Hemminger
  2012-11-01  1:33       ` John Fastabend
  0 siblings, 1 reply; 18+ messages in thread
From: Stephen Hemminger @ 2012-10-31 21:30 UTC (permalink / raw)
  To: John Fastabend; +Cc: davem, netdev

On Wed, 31 Oct 2012 07:01:49 -0700
John Fastabend <john.r.fastabend@intel.com> wrote:

> On 10/29/2012 5:57 PM, Stephen Hemminger wrote:
> > Expose bridge port parameters over netlink. By switching
> > from a single byet to a nested message, this can be used for
> > other bridge parameters.
> >
> > Although, this changes IFLA_PROTINFO attribute from one byte to a full nested
> > set of attributes; it is safe for applications because the
> > old message used IFLA_PROTINFO and new one uses
> >   IFLA_PROTINFO | NLA_F_NESTED.
> >
> > The code still accepts to old format requests, and therefore stays
> > compatiable with user mode RSTP daemon. Since the type field
> > for nested and unnested attributes are different, and the old
> > code in libnetlink doesn't do the mask, it is also safe to use
> > with old versions of bridge monitor command.
> >
> > Note: although mode is only a boolean, treating it as a
> > full byte since in the future someone will probably want to add more
> > values (like macvlan has).
> >
> > Signed-off-by: Stephen Hemminger<shemminger@vyatta.com>
> >
> > ---
> 
> Stephen,
> 
> Did you see these two patches
> 
> http://patchwork.ozlabs.org/patch/193900/
> http://patchwork.ozlabs.org/patch/193901/
> 
> here I added nested bridge attributes to IFLA_AF_SPEC and pass them down
> to the drivers as needed. Should we merge these two sets so that we have
> only a single nested set of bridge attributes? Either in IFLA_AF_SPEC or
> IFLA_PROTINFO.
> 
> The attributes in this patch are port specifics and the one in the
> patches listed above are bridge specific so in this sense perhaps
> its OK to keep them separate. I'm not sure it matters much either
> way but thought I would mention it.
> 
> Also I suspect these two series will have conflicts but I haven't tried
> yet.
> 
> Thanks,
> John

This is an area where there is no clear choice.
I would like to keep AF_UNSPEC for non-protocol stuff,
that is why I targeted PF_BRIDGE:IFLA_PROTINFO.

Other alternative would be to add sysctl which is less message
based, but is more general. (ie. /default and /all are available).

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next 1/8] bridge: bridge port parameters over netlink
  2012-10-31 21:30     ` Stephen Hemminger
@ 2012-11-01  1:33       ` John Fastabend
  0 siblings, 0 replies; 18+ messages in thread
From: John Fastabend @ 2012-11-01  1:33 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: davem, netdev

[...]

>> here I added nested bridge attributes to IFLA_AF_SPEC and pass them down
>> to the drivers as needed. Should we merge these two sets so that we have
>> only a single nested set of bridge attributes? Either in IFLA_AF_SPEC or
>> IFLA_PROTINFO.
>>

[...]

>
> This is an area where there is no clear choice.
> I would like to keep AF_UNSPEC for non-protocol stuff,
> that is why I targeted PF_BRIDGE:IFLA_PROTINFO.
>

Works for me.

> Other alternative would be to add sysctl which is less message
> based, but is more general. (ie. /default and /all are available).
>

I would just assume use netlink seeing this is common for all the other
networking bits and any networking app is already likely to be handling
these events.

.John

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2012-11-01  1:33 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-30  0:57 [PATCH net-next 0/8] bridge: new security features Stephen Hemminger
2012-10-30  0:57 ` [PATCH net-next 1/8] bridge: bridge port parameters over netlink Stephen Hemminger
2012-10-30 18:48   ` Tommy S. Christensen
2012-10-30 21:00     ` Stephen Hemminger
2012-10-31 14:01   ` John Fastabend
2012-10-31 21:30     ` Stephen Hemminger
2012-11-01  1:33       ` John Fastabend
2012-10-30  0:57 ` [PATCH net-next 2/8] bridge: add template for bridge port flags Stephen Hemminger
2012-10-30  0:57 ` [PATCH net-next 3/8] bridge: implement BPDU blocking Stephen Hemminger
2012-10-31  2:38   ` Cong Wang
2012-10-31 20:57     ` Stephen Hemminger
2012-10-30  0:57 ` [PATCH net-next 4/8] bridge: add root port blocking Stephen Hemminger
2012-10-30  0:57 ` [PATCH net-next 5/8] bridge: add bpdu filter Stephen Hemminger
2012-10-30  0:57 ` [PATCH net-next 6/8] tun: implement byte queue limits Stephen Hemminger
2012-10-30  1:09   ` Stephen Hemminger
2012-10-30  0:57 ` [PATCH net-next 7/8] virtio: make some structures const Stephen Hemminger
2012-10-30  1:09   ` Stephen Hemminger
2012-10-30  0:57 ` [PATCH net-next 8/8] iproute2: handle new bridge PROTINFO format Stephen Hemminger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.