net/bridge: Add 'hairpin' port forwarding mode

PATCH Applied Fischer, Anna 4 participants 10 messages Aug 13, 2009 – Aug 17, 2009

Subsystems (4)

Networking Network drivers virtio core Ethernet bridge

lore

Share & tools

lore

Fischer, Anna

author

2009-08-13 16:55 UTC #0 lore raw

PATCH 1/1 net/bridge: Add 'hairpin' port forwarding mode

This patch adds a 'hairpin' (also called 'reflective relay') mode port configuration to the Linux Ethernet bridge kernel module. A bridge supporting hairpin forwarding mode can send frames back out through the port the frame was received on.

Hairpin mode is required to support basic VEPA (Virtual Ethernet Port Aggregator) capabilities.

You can find additional information on VEPA here:
http://tech.groups.yahoo.com/group/evb/
http://www.ieee802.org/1/files/public/docs2009/new-hudson-vepa_seminar-20090514d.pdf
http://www.internet2.edu/presentations/jt2009jul/20090719-congdon.pdf

An additional patch 'bridge-utils: Add 'hairpin' port forwarding mode' is provided to allow configuring hairpin mode from userspace tools.

Signed-off-by Paul Congdon <[email protected]>

Signed-off-by Anna Fischer <[email protected]>

---

 net/bridge/br_forward.c  |    3 ++-
 net/bridge/br_if.c       |    1 +
 net/bridge/br_private.h  |    3 +++
 net/bridge/br_sysfs_if.c |   17 +++++++++++++++++
 4 files changed, 23 insertions(+), 1 deletions(-)

net/bridge/br_forward.c +2 -1

index d2c27c8..bc1704a 100644
@@ -22,7 +22,8 @@
 static inline int should_deliver(const struct net_bridge_port *p,
 				 const struct sk_buff *skb)
 {
-	return (skb->dev != p->dev && p->state == BR_STATE_FORWARDING);
+	return (((p->flags & BR_HAIRPIN_MODE) || skb->dev != p->dev) &&
+		p->state == BR_STATE_FORWARDING);
 }
 
 static inline unsigned packet_length(const struct sk_buff *skb)

net/bridge/br_if.c +1 -0

index eb404dc..e486f1f 100644
@@ -256,6 +256,7 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
 	p->path_cost = port_cost(dev);
 	p->priority = 0x8000 >> BR_PORT_BITS;
 	p->port_no = index;
+	p->flags = 0;
 	br_init_port(p);
 	p->state = BR_STATE_DISABLED;
 	br_stp_port_timer_init(p);

net/bridge/br_private.h +3 -0

index d5b5537..8319247 100644
@@ -81,6 +81,9 @@ struct net_bridge_port
 	struct timer_list		message_age_timer;
 	struct kobject			kobj;
 	struct rcu_head			rcu;
+
+	unsigned long 			flags;
+#define BR_HAIRPIN_MODE		0x00000001
 };
 
 struct net_bridge

net/bridge/br_sysfs_if.c +17 -0

index 4a3cdf8..820643a 100644
@@ -143,6 +143,22 @@ static ssize_t store_flush(struct net_bridge_port *p, unsigned long v)
 }
 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 ssize_t 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);
+
 static struct brport_attribute *brport_attrs[] = {
 	&brport_attr_path_cost,
 	&brport_attr_priority,

@@ -159,6 +175,7 @@ static struct brport_attribute *brport_attrs[] = {
 	&brport_attr_forward_delay_timer,
 	&brport_attr_hold_timer,
 	&brport_attr_flush,
+	&brport_attr_hairpin_mode,
 	NULL
 };

Arnd Bergmann

Acked-by on v1 1/1

2009-08-13 17:06 UTC #1 lore raw

Quoted 13 lines · Anna

This patch adds a 'hairpin' (also called 'reflective relay') mode port configuration to the Linux Ethernet bridge kernel module. A bridge supporting hairpin forwarding mode can send frames back out through the port the frame was received on.
Hairpin mode is required to support basic VEPA (Virtual Ethernet Port Aggregator) capabilities.
You can find additional information on VEPA here:
http://tech.groups.yahoo.com/group/evb/
http://www.ieee802.org/1/files/public/docs2009/new-hudson-vepa_seminar-20090514d.pdf
http://www.internet2.edu/presentations/jt2009jul/20090719-congdon.pdf

Quoted 5 lines

An additional patch 'bridge-utils: Add 'hairpin' port forwarding mode' is provided to allow configuring hairpin mode from userspace tools.
Signed-off-by Paul Congdon <[email protected]>
Signed-off-by Anna Fischer <[email protected]>

Acked-by Arnd Bergmann <[email protected]>

Arnd Bergmann

Acked-by on v1 1/1

2009-08-13 17:06 UTC #2 lore raw

In reply to Fischer, Anna

Quoted 13 lines · Anna

This patch adds a 'hairpin' (also called 'reflective relay') mode port configuration to the Linux Ethernet bridge kernel module. A bridge supporting hairpin forwarding mode can send frames back out through the port the frame was received on.
Hairpin mode is required to support basic VEPA (Virtual Ethernet Port Aggregator) capabilities.
You can find additional information on VEPA here:
http://tech.groups.yahoo.com/group/evb/
http://www.ieee802.org/1/files/public/docs2009/new-hudson-vepa_seminar-20090514d.pdf
http://www.internet2.edu/presentations/jt2009jul/20090719-congdon.pdf

Quoted 5 lines

An additional patch 'bridge-utils: Add 'hairpin' port forwarding mode' is provided to allow configuring hairpin mode from userspace tools.
Signed-off-by Paul Congdon <[email protected]>
Signed-off-by Anna Fischer <[email protected]>

Acked-by Arnd Bergmann <[email protected]>

Stephen Hemminger

2009-08-13 18:07 UTC #3 lore raw

In reply to Fischer, Anna

net/bridge/br_if.c static struct net_bridge_port *new_nbp(struct net_bridge *br, · line 262

Quoted 13 lines

net/bridge/br_if.c +1 -0

index eb404dc..e486f1f 100644
@@ -256,6 +256,7 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
 	p->path_cost = port_cost(dev);
 	p->priority = 0x8000 >> BR_PORT_BITS;
 	p->port_no = index;
+	p->flags = 0;
 	br_init_port(p);
 	p->state = BR_STATE_DISABLED;
 	br_stp_port_timer_init(p);

Minor nit. this is unnecessary since the structure is allocated with kzalloc.

Stephen Hemminger

2009-08-13 18:07 UTC #4 lore raw

In reply to Fischer, Anna

net/bridge/br_if.c static struct net_bridge_port *new_nbp(struct net_bridge *br, · line 262

Quoted 13 lines

net/bridge/br_if.c +1 -0

index eb404dc..e486f1f 100644
@@ -256,6 +256,7 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br,
 	p->path_cost = port_cost(dev);
 	p->priority = 0x8000 >> BR_PORT_BITS;
 	p->port_no = index;
+	p->flags = 0;
 	br_init_port(p);
 	p->state = BR_STATE_DISABLED;
 	br_stp_port_timer_init(p);

Minor nit. this is unnecessary since the structure is allocated with kzalloc.

David Miller

maintainer Applied

2009-08-13 23:27 UTC #5 lore raw

In reply to Fischer, Anna

From: "Fischer, Anna" <[email protected]>
Date: Thu, 13 Aug 2009 16:55:16 +0000

Quoted 13 lines

This patch adds a 'hairpin' (also called 'reflective relay') mode port configuration to the Linux Ethernet bridge kernel module. A bridge supporting hairpin forwarding mode can send frames back out through the port the frame was received on.
Hairpin mode is required to support basic VEPA (Virtual Ethernet Port Aggregator) capabilities.
You can find additional information on VEPA here:
http://tech.groups.yahoo.com/group/evb/
http://www.ieee802.org/1/files/public/docs2009/new-hudson-vepa_seminar-20090514d.pdf
http://www.internet2.edu/presentations/jt2009jul/20090719-congdon.pdf

Quoted 5 lines

An additional patch 'bridge-utils: Add 'hairpin' port forwarding mode' is provided to allow configuring hairpin mode from userspace tools.
Signed-off-by Paul Congdon <[email protected]>
Signed-off-by Anna Fischer <[email protected]>

Applied to net-next-2.6

Stephen Hemminger

2009-08-14 21:41 UTC #6 lore raw

RFC 1/1 bridge: prevent hairpin and STP problems?

Do we need to add this to block Spanning Tree from being enabled with hairpin mode? I am not sure what the exact usage of hairpin mode and if it is possible to create loops and get STP confusion.

For comment only, do not apply as is.

net/bridge/br_ioctl.c +1 -2

@@ -259,8 +259,7 @@ static int old_dev_ioctl(struct net_devi
 		if (!capable(CAP_NET_ADMIN))
 			return -EPERM;
 
-		br_stp_set_enabled(br, args[1]);
-		return 0;
+		return br_stp_set_enabled(br, args[1]);
 
 	case BRCTL_SET_BRIDGE_PRIORITY:
 		if (!capable(CAP_NET_ADMIN))

net/bridge/br_stp_if.c +10 -1

@@ -160,17 +160,26 @@ static void br_stp_stop(struct net_bridg
 	br->stp_enabled = BR_NO_STP;
 }
 
-void br_stp_set_enabled(struct net_bridge *br, unsigned long val)
+int br_stp_set_enabled(struct net_bridge *br, unsigned long val)
 {
 	ASSERT_RTNL();
 
 	if (val) {
+		struct net_bridge_port *p;
+		list_for_each_entry_rcu(p, &br->port_list, list) {
+			if (p->flags & BR_HAIRPIN_MODE)
+				return -EINVAL;
+		}
+
+
 		if (br->stp_enabled == BR_NO_STP)
 			br_stp_start(br);
 	} else {
 		if (br->stp_enabled != BR_NO_STP)
 			br_stp_stop(br);
 	}
+
+	return 0;
 }
 
 /* called under bridge lock */

net/bridge/br_sysfs_br.c +4 -2

@@ -164,6 +164,7 @@ static ssize_t store_stp_state(struct de
 	struct net_bridge *br = to_bridge(d);
 	char *endp;
 	unsigned long val;
+	int ret;
 
 	if (!capable(CAP_NET_ADMIN))
 		return -EPERM;

@@ -174,10 +175,11 @@ static ssize_t store_stp_state(struct de
 
 	if (!rtnl_trylock())
 		return restart_syscall();
-	br_stp_set_enabled(br, val);
+
+	ret = br_stp_set_enabled(br, val);
 	rtnl_unlock();
 
-	return len;
+	return (ret == 0) ? len : ret;
 }
 static DEVICE_ATTR(stp_state, S_IRUGO | S_IWUSR, show_stp_state,
 		   store_stp_state);

net/bridge/br_sysfs_if.c +5 -2

@@ -150,10 +150,13 @@ static ssize_t show_hairpin_mode(struct 
 }
 static ssize_t store_hairpin_mode(struct net_bridge_port *p, unsigned long v)
 {
-	if (v)
+	if (!v)
+		p->flags &= ~BR_HAIRPIN_MODE;
+	else if (p->br->stp_enabled == BR_NO_STP)
 		p->flags |= BR_HAIRPIN_MODE;
 	else
-		p->flags &= ~BR_HAIRPIN_MODE;
+		return -EINVAL;
+
 	return 0;
 }
 static BRPORT_ATTR(hairpin_mode, S_IRUGO | S_IWUSR,

net/bridge/br_private.h +1 -1

@@ -218,7 +218,7 @@ extern void br_become_designated_port(st
 /* br_stp_if.c */
 extern void br_stp_enable_bridge(struct net_bridge *br);
 extern void br_stp_disable_bridge(struct net_bridge *br);
-extern void br_stp_set_enabled(struct net_bridge *br, unsigned long val);
+extern int br_stp_set_enabled(struct net_bridge *br, unsigned long val);
 extern void br_stp_enable_port(struct net_bridge_port *p);
 extern void br_stp_disable_port(struct net_bridge_port *p);
 extern void br_stp_recalculate_bridge_id(struct net_bridge *br);

Fischer, Anna

author

2009-08-17 21:16 UTC #7 lore raw

RE: [RFC] bridge: prevent hairpin and STP problems?

Quoted 7 lines

Subject: [RFC] bridge: prevent hairpin and STP problems?
Do we need to add this to block Spanning Tree from being enabled with hairpin mode? I am not sure what the exact usage of hairpin mode and if it is possible to create loops and get STP confusion.
For comment only, do not apply as is.

Your patch disables STP on the whole bridge if one or more ports are set to hairpin mode.

However, I don't really see that this is necessary.

A hairpin mode port should not reflect BPDUs, because otherwise the connected port would think it has detected a loop. The hairpin mode port should still be able to generate BPDUs though, and in any case the bridge should still be able to run STP.

The hairpin patch we submitted reflects packets on the forwarding / data path whereas BPDUs are processed with a separate hook, so we should not be reflecting BPDUs back out of a hairpin mode port.

Anna

Stephen Hemminger

2009-08-17 22:37 UTC #8 lore raw

Re: [RFC] bridge: prevent hairpin and STP problems?

Quoted 13 lines · Anna

Quoted 7 lines
Subject: [RFC] bridge: prevent hairpin and STP problems?
Do we need to add this to block Spanning Tree from being enabled with hairpin mode? I am not sure what the exact usage of hairpin mode and if it is possible to create loops and get STP confusion.
For comment only, do not apply as is.
Your patch disables STP on the whole bridge if one or more ports are set to hairpin mode.
However, I don't really see that this is necessary.

Quoted 8 lines

A hairpin mode port should not reflect BPDUs, because otherwise the connected port would think it has detected a loop. The hairpin mode port should still be able to generate BPDUs though, and in any case the bridge should still be able to run STP.
The hairpin patch we submitted reflects packets on the forwarding / data path whereas BPDUs are processed with a separate hook, so we should not be reflecting BPDUs back out of a hairpin mode port.

So if user is using hairpin properly, the STP would work. In fact it would be a good thing since it would detect looping configurations.

David Miller

maintainer Applied

2009-08-13 23:27 UTC #9 lore raw

In reply to Fischer, Anna

From: "Fischer, Anna" <[email protected]>
Date: Thu, 13 Aug 2009 16:55:16 +0000

Quoted 13 lines

This patch adds a 'hairpin' (also called 'reflective relay') mode port configuration to the Linux Ethernet bridge kernel module. A bridge supporting hairpin forwarding mode can send frames back out through the port the frame was received on.
Hairpin mode is required to support basic VEPA (Virtual Ethernet Port Aggregator) capabilities.
You can find additional information on VEPA here:
http://tech.groups.yahoo.com/group/evb/
http://www.ieee802.org/1/files/public/docs2009/new-hudson-vepa_seminar-20090514d.pdf
http://www.internet2.edu/presentations/jt2009jul/20090719-congdon.pdf

Quoted 5 lines

An additional patch 'bridge-utils: Add 'hairpin' port forwarding mode' is provided to allow configuring hairpin mode from userspace tools.
Signed-off-by Paul Congdon <[email protected]>
Signed-off-by Anna Fischer <[email protected]>

Applied to net-next-2.6