netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 1/3] tipc: cosmetic: clean up comments and break a long line
@ 2013-05-01 22:04 Gerlando Falauto
  2013-05-01 22:04 ` [PATCH v5 2/3] tipc: tipc_bcbearer_send(): simplify bearer selection Gerlando Falauto
  2013-05-01 22:04 ` [PATCH v5 3/3] tipc: pskb_copy() buffers when sending on more than one bearer Gerlando Falauto
  0 siblings, 2 replies; 3+ messages in thread
From: Gerlando Falauto @ 2013-05-01 22:04 UTC (permalink / raw)
  To: netdev; +Cc: jon.maloy, erik.hugne, ying.xue, holger.brunck, Gerlando Falauto

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
Changes from v4:
* rebased on net-next
Changes from v3:
* Added "and break a long line" to the commit message
Changes from v2:
* Split cosmetic (this patch) from functional changes (next patch)

 net/tipc/bcast.c |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index 25e159c..0e2f432 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -584,8 +584,7 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
 {
 	int bp_index;
 
-	/*
-	 * Prepare broadcast link message for reliable transmission,
+	/* Prepare broadcast link message for reliable transmission,
 	 * if first time trying to send it;
 	 * preparation is skipped for broadcast link protocol messages
 	 * since they are sent in an unreliable manner and don't need it
@@ -613,11 +612,12 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
 		struct tipc_bearer *s = bcbearer->bpairs[bp_index].secondary;
 
 		if (!p)
-			break;	/* no more bearers to try */
+			break; /* No more bearers to try */
 
-		tipc_nmap_diff(&bcbearer->remains, &p->nodes, &bcbearer->remains_new);
+		tipc_nmap_diff(&bcbearer->remains, &p->nodes,
+			       &bcbearer->remains_new);
 		if (bcbearer->remains_new.count == bcbearer->remains.count)
-			continue;	/* bearer pair doesn't add anything */
+			continue; /* Nothing added by bearer pair */
 
 		if (!tipc_bearer_blocked(p))
 			tipc_bearer_send(p, buf, &p->bcast_addr);
@@ -628,13 +628,14 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
 			/* unable to send on either bearer */
 			continue;
 
+		/* Swap bearers for next packet */
 		if (s) {
 			bcbearer->bpairs[bp_index].primary = s;
 			bcbearer->bpairs[bp_index].secondary = p;
 		}
 
 		if (bcbearer->remains_new.count == 0)
-			break;	/* all targets reached */
+			break; /* All targets reached */
 
 		bcbearer->remains = bcbearer->remains_new;
 	}
-- 
1.7.10.1

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

* [PATCH v5 2/3] tipc: tipc_bcbearer_send(): simplify bearer selection
  2013-05-01 22:04 [PATCH v5 1/3] tipc: cosmetic: clean up comments and break a long line Gerlando Falauto
@ 2013-05-01 22:04 ` Gerlando Falauto
  2013-05-01 22:04 ` [PATCH v5 3/3] tipc: pskb_copy() buffers when sending on more than one bearer Gerlando Falauto
  1 sibling, 0 replies; 3+ messages in thread
From: Gerlando Falauto @ 2013-05-01 22:04 UTC (permalink / raw)
  To: netdev; +Cc: jon.maloy, erik.hugne, ying.xue, holger.brunck, Gerlando Falauto

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
Changes from v4:
* Rebased on net-next 
* fixed bcast_addr (no longer within b->media)
Changes from v3: NONE
Changes from v2:
* Fixed non-bisectability; was tipc_bearer_send(b, buf, &s->media...)

 net/tipc/bcast.c |   18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index 0e2f432..d9d848d 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -610,23 +610,23 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
 	for (bp_index = 0; bp_index < MAX_BEARERS; bp_index++) {
 		struct tipc_bearer *p = bcbearer->bpairs[bp_index].primary;
 		struct tipc_bearer *s = bcbearer->bpairs[bp_index].secondary;
+		struct tipc_bearer *b = p;
 
 		if (!p)
 			break; /* No more bearers to try */
 
-		tipc_nmap_diff(&bcbearer->remains, &p->nodes,
+		if (tipc_bearer_blocked(p)) {
+			if (!s || tipc_bearer_blocked(s))
+				continue; /* Can't use either bearer */
+			b = s;
+		}
+
+		tipc_nmap_diff(&bcbearer->remains, &b->nodes,
 			       &bcbearer->remains_new);
 		if (bcbearer->remains_new.count == bcbearer->remains.count)
 			continue; /* Nothing added by bearer pair */
 
-		if (!tipc_bearer_blocked(p))
-			tipc_bearer_send(p, buf, &p->bcast_addr);
-		else if (s && !tipc_bearer_blocked(s))
-			/* unable to send on primary bearer */
-			tipc_bearer_send(s, buf, &s->bcast_addr);
-		else
-			/* unable to send on either bearer */
-			continue;
+		tipc_bearer_send(b, buf, &b->bcast_addr);
 
 		/* Swap bearers for next packet */
 		if (s) {
-- 
1.7.10.1

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

* [PATCH v5 3/3] tipc: pskb_copy() buffers when sending on more than one bearer
  2013-05-01 22:04 [PATCH v5 1/3] tipc: cosmetic: clean up comments and break a long line Gerlando Falauto
  2013-05-01 22:04 ` [PATCH v5 2/3] tipc: tipc_bcbearer_send(): simplify bearer selection Gerlando Falauto
@ 2013-05-01 22:04 ` Gerlando Falauto
  1 sibling, 0 replies; 3+ messages in thread
From: Gerlando Falauto @ 2013-05-01 22:04 UTC (permalink / raw)
  To: netdev; +Cc: jon.maloy, erik.hugne, ying.xue, holger.brunck, Gerlando Falauto

When sending packets, TIPC bearers use skb_clone() before writing their
hardware header. This will however NOT copy the data buffer.
So when the same packet is sent over multiple bearers (to reach multiple
nodes), the same socket buffer data will be treated by multiple
tipc_media drivers which will write their own hardware header through
dev_hard_header().
Most of the time this is not a problem, because by the time the
packet is processed by the second media, it has already been sent over
the first one. However, when the first transmission is delayed (e.g.
because of insufficient bandwidth or through a shaper), the next bearer
will overwrite the hardware header, resulting in the packet being sent:
a) with the wrong source address, when bearers of the same type,
e.g. ethernet, are involved
b) with a completely corrupt header, or even dropped, when bearers of
different types are involved.

So when the same socket buffer is to be sent multiple times, send a
pskb_copy() instead (from the second instance on), and release it
afterwards (the bearer will skb_clone() it anyway).

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
Changes from v4:
* Rebase on net-next
* Fixed bcast_addr (no longer within b->media)
Changes from v3: NONE
Changes from v2:
* Rebased

 net/tipc/bcast.c |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index d9d848d..e5f3da5 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -611,6 +611,7 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
 		struct tipc_bearer *p = bcbearer->bpairs[bp_index].primary;
 		struct tipc_bearer *s = bcbearer->bpairs[bp_index].secondary;
 		struct tipc_bearer *b = p;
+		struct sk_buff *tbuf;
 
 		if (!p)
 			break; /* No more bearers to try */
@@ -626,7 +627,17 @@ static int tipc_bcbearer_send(struct sk_buff *buf,
 		if (bcbearer->remains_new.count == bcbearer->remains.count)
 			continue; /* Nothing added by bearer pair */
 
-		tipc_bearer_send(b, buf, &b->bcast_addr);
+		if (bp_index == 0) {
+			/* Use original buffer for first bearer */
+			tipc_bearer_send(b, buf, &b->bcast_addr);
+		} else {
+			/* Avoid concurrent buffer access */
+			tbuf = pskb_copy(buf, GFP_ATOMIC);
+			if (!tbuf)
+				break;
+			tipc_bearer_send(b, tbuf, &b->bcast_addr);
+			kfree_skb(tbuf); /* Bearer keeps a clone */
+		}
 
 		/* Swap bearers for next packet */
 		if (s) {
-- 
1.7.10.1

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

end of thread, other threads:[~2013-05-01 22:05 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-01 22:04 [PATCH v5 1/3] tipc: cosmetic: clean up comments and break a long line Gerlando Falauto
2013-05-01 22:04 ` [PATCH v5 2/3] tipc: tipc_bcbearer_send(): simplify bearer selection Gerlando Falauto
2013-05-01 22:04 ` [PATCH v5 3/3] tipc: pskb_copy() buffers when sending on more than one bearer Gerlando Falauto

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).