* [PATCH 3/5] [TCP]: Convert highest_sack to sk_buff to allow direct access
From: Ilpo Järvinen @ 2007-09-24 10:28 UTC (permalink / raw)
To: David Miller, Stephen Hemminger, SANGTAE HA, Tom Quetchenbach,
Baruch Even <bar
Cc: netdev, Ilpo Järvinen
In-Reply-To: <11906297271155-git-send-email-ilpo.jarvinen@helsinki.fi>
From: =?ISO-8859-1?q?Ilpo_J=E4rvinen?= <ilpo.jarvinen@helsinki.fi>
It is going to replace the sack fastpath hint quite soon... :-)
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
include/linux/tcp.h | 6 ++++--
include/net/tcp.h | 13 +++++++++++++
net/ipv4/tcp_input.c | 12 ++++++------
net/ipv4/tcp_output.c | 19 ++++++++++---------
4 files changed, 33 insertions(+), 17 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index f8cf090..1d6be2a 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -332,8 +332,10 @@ struct tcp_sock {
struct tcp_sack_block_wire recv_sack_cache[4];
- u32 highest_sack; /* Start seq of globally highest revd SACK
- * (validity guaranteed only if sacked_out > 0) */
+ struct sk_buff *highest_sack; /* highest skb with SACK received
+ * (validity guaranteed only if
+ * sacked_out > 0)
+ */
/* from STCP, retrans queue hinting */
struct sk_buff* lost_skb_hint;
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 991ccdc..8bc64b7 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1308,6 +1308,19 @@ static inline int tcp_write_queue_empty(struct sock *sk)
return skb_queue_empty(&sk->sk_write_queue);
}
+/* Start sequence of the highest skb with SACKed bit, valid only if
+ * sacked > 0 or when the caller has ensured validity by itself.
+ */
+static inline u32 tcp_highest_sack_seq(struct sock *sk)
+{
+ struct tcp_sock *tp = tcp_sk(sk);
+
+ if (WARN_ON(!tp->sacked_out &&
+ tp->highest_sack != tcp_write_queue_head(sk)))
+ return tp->snd_una;
+ return TCP_SKB_CB(tp->highest_sack)->seq;
+}
+
/* /proc */
enum tcp_seq_states {
TCP_SEQ_STATE_LISTENING,
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 76e9c9b..85dd4b0 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1143,10 +1143,11 @@ static int tcp_check_dsack(struct tcp_sock *tp, struct sk_buff *ack_skb,
return dup_sack;
}
-static void tcp_sacktag_one(struct sk_buff *skb, struct tcp_sock *tp,
+static void tcp_sacktag_one(struct sk_buff *skb, struct sock *sk,
struct tcp_sacktag_state *state, int in_sack,
int dup_sack, int fack_count, u32 end_seq)
{
+ struct tcp_sock *tp = tcp_sk(sk);
u8 sacked = TCP_SKB_CB(skb)->sacked;
/* Account D-SACK for retransmitted packet. */
@@ -1231,9 +1232,8 @@ static void tcp_sacktag_one(struct sk_buff *skb, struct tcp_sock *tp,
if (fack_count > tp->fackets_out)
tp->fackets_out = fack_count;
- if (after(TCP_SKB_CB(skb)->seq,
- tp->highest_sack))
- tp->highest_sack = TCP_SKB_CB(skb)->seq;
+ if (after(TCP_SKB_CB(skb)->seq, tcp_highest_sack_seq(sk)))
+ tp->highest_sack = skb;
} else {
if (dup_sack && (sacked&TCPCB_RETRANS))
state->reord = min(fack_count, state->reord);
@@ -1271,7 +1271,7 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
if (!tp->sacked_out) {
tp->fackets_out = 0;
- tp->highest_sack = tp->snd_una;
+ tp->highest_sack = tcp_write_queue_head(sk);
}
found_dup_sack = tcp_check_dsack(tp, ack_skb, sp, num_sacks, prior_snd_una);
@@ -1424,7 +1424,7 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
fack_count += pcount;
- tcp_sacktag_one(skb, tp, &state, in_sack,
+ tcp_sacktag_one(skb, sk, &state, in_sack,
dup_sack, fack_count, end_seq);
}
}
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 94c8011..fd51692 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -657,13 +657,15 @@ static void tcp_set_skb_tso_segs(struct sock *sk, struct sk_buff *skb, unsigned
* tweak SACK fastpath hint too as it would overwrite all changes unless
* hint is also changed.
*/
-static void tcp_adjust_fackets_out(struct tcp_sock *tp, struct sk_buff *skb,
+static void tcp_adjust_fackets_out(struct sock *sk, struct sk_buff *skb,
int decr)
{
+ struct tcp_sock *tp = tcp_sk(sk);
+
if (!tp->sacked_out)
return;
- if (!before(tp->highest_sack, TCP_SKB_CB(skb)->seq))
+ if (!before(tcp_highest_sack_seq(sk), TCP_SKB_CB(skb)->seq))
tp->fackets_out -= decr;
/* cnt_hint is "off-by-one" compared with fackets_out (see sacktag) */
@@ -712,8 +714,8 @@ int tcp_fragment(struct sock *sk, struct sk_buff *skb, u32 len, unsigned int mss
TCP_SKB_CB(buff)->end_seq = TCP_SKB_CB(skb)->end_seq;
TCP_SKB_CB(skb)->end_seq = TCP_SKB_CB(buff)->seq;
- if (tp->sacked_out && (TCP_SKB_CB(skb)->seq == tp->highest_sack))
- tp->highest_sack = TCP_SKB_CB(buff)->seq;
+ if (tp->sacked_out && (skb == tp->highest_sack))
+ tp->highest_sack = buff;
/* PSH and FIN should only be set in the second packet. */
flags = TCP_SKB_CB(skb)->flags;
@@ -771,7 +773,7 @@ int tcp_fragment(struct sock *sk, struct sk_buff *skb, u32 len, unsigned int mss
tcp_dec_pcount_approx_int(&tp->sacked_out, diff);
tcp_verify_left_out(tp);
}
- tcp_adjust_fackets_out(tp, skb, diff);
+ tcp_adjust_fackets_out(sk, skb, diff);
}
/* Link BUFF into the send queue. */
@@ -1718,8 +1720,7 @@ static void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *skb, int m
BUG_ON(tcp_skb_pcount(skb) != 1 ||
tcp_skb_pcount(next_skb) != 1);
- if (WARN_ON(tp->sacked_out &&
- (TCP_SKB_CB(next_skb)->seq == tp->highest_sack)))
+ if (WARN_ON(tp->sacked_out && (next_skb == tp->highest_sack)))
return;
/* Ok. We will be able to collapse the packet. */
@@ -1754,7 +1755,7 @@ static void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *skb, int m
if (tcp_is_reno(tp) && tp->sacked_out)
tcp_dec_pcount_approx(&tp->sacked_out, next_skb);
- tcp_adjust_fackets_out(tp, skb, tcp_skb_pcount(next_skb));
+ tcp_adjust_fackets_out(sk, skb, tcp_skb_pcount(next_skb));
tp->packets_out -= tcp_skb_pcount(next_skb);
/* changed transmit queue under us so clear hints */
@@ -2031,7 +2032,7 @@ void tcp_xmit_retransmit_queue(struct sock *sk)
break;
tp->forward_skb_hint = skb;
- if (after(TCP_SKB_CB(skb)->seq, tp->highest_sack))
+ if (after(TCP_SKB_CB(skb)->seq, tcp_highest_sack_seq(sk)))
break;
if (tcp_packets_in_flight(tp) >= tp->snd_cwnd)
--
1.5.0.6
^ permalink raw reply related
* [RFC PATCH net-2.6.24 0/5]: TCP sacktag cache usage recoded
From: Ilpo Järvinen @ 2007-09-24 10:28 UTC (permalink / raw)
To: David Miller, Stephen Hemminger, SANGTAE HA, Tom Quetchenbach,
Baruch Even <bar
Cc: netdev
Hi all,
After couple of wrong-wayed before/after()s and one infinite
loopy version, here's the current trial version of a sacktag
cache usage recode....
Two first patches come from tcp-2.6 (rebased and rotated).
This series apply cleanly only on top of the other three patch
series I posted earlier today. The last debug patch provides
some statistics for those interested enough.
Dave, please DO NOT apply! ...Some thoughts could be nice
though :-).
It should improve processing of such likely events as cumulative
ACKs and new forward holed SACK considerably because full walk
is not necessary anymore (old code could have been tweaked to
cover them but it's better to drop each special case handling
altogether and do a generic solution. Redundancy of fastpath
hints and highest_sack stuff is also addressed, however, it
might have slight effect as the hint could point to something
less than highest_sack occassionally, whether that's significant
remains to see... In all cases except hint below highest_sack,
the new solution should perform at least as well as the old code
(with a bit larger constant though, no additional cache misses
though) because the SACK blocks old code choose not to process
should all fall to LINUX_MIB_TCP_FULLSKIP category.
It's possible to improve it easily with RB-tree stuff though
this version is based on code using linked lists. I'm not yet
too sure that I got everything 100% correct as I "tweak"
start/end_seqs and exit skb loops a way that is prone to
off-by-one errors, could miss skb here and there. I'll probably
also recode dsack handling too to avoid recursion.
Stephen, Sangtae, others? experiencing those unexpected RTOs
during recovery of large windowed TCP, could you please give it
a try if it helps any...
--
i.
ps. Our net-2.6.24 (and mainline?) DSACK processing code could
be broken btw. DSACK in the middle of other SACK block during
in-order walk seems to not be processed at all as the earlier
(sorted) block caused skb to advance past it already? (Just
occurred to me, I'll see what I can do to that if I've enough
time).
^ permalink raw reply
* [PATCH 2/5] [TCP]: Create tcp_sacktag_one().
From: Ilpo Järvinen @ 2007-09-24 10:28 UTC (permalink / raw)
To: David Miller, Stephen Hemminger, SANGTAE HA, Tom Quetchenbach,
Baruch Even <bar
Cc: netdev, David S. Miller
In-Reply-To: <11906297272364-git-send-email-ilpo.jarvinen@helsinki.fi>
From: David S. Miller <davem@sunset.davemloft.net>
Worker function that implements the main logic of
the inner-most loop of tcp_sacktag_write_queue().
Signed-off-by: David S. Miller <davem@davemloft.net>
Acked-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
net/ipv4/tcp_input.c | 213 ++++++++++++++++++++++++++------------------------
1 files changed, 110 insertions(+), 103 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 04ff465..76e9c9b 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1143,6 +1143,114 @@ static int tcp_check_dsack(struct tcp_sock *tp, struct sk_buff *ack_skb,
return dup_sack;
}
+static void tcp_sacktag_one(struct sk_buff *skb, struct tcp_sock *tp,
+ struct tcp_sacktag_state *state, int in_sack,
+ int dup_sack, int fack_count, u32 end_seq)
+{
+ u8 sacked = TCP_SKB_CB(skb)->sacked;
+
+ /* Account D-SACK for retransmitted packet. */
+ if ((dup_sack && in_sack) &&
+ (sacked & TCPCB_RETRANS) &&
+ after(TCP_SKB_CB(skb)->end_seq, tp->undo_marker))
+ tp->undo_retrans--;
+
+ /* The frame is ACKed. */
+ if (!after(TCP_SKB_CB(skb)->end_seq, tp->snd_una)) {
+ if (sacked & TCPCB_RETRANS) {
+ if ((dup_sack && in_sack) &&
+ (sacked & TCPCB_SACKED_ACKED))
+ state->reord = min(fack_count, state->reord);
+ } else {
+ /* If it was in a hole, we detected reordering. */
+ if (fack_count < state->prior_fackets &&
+ !(sacked & TCPCB_SACKED_ACKED))
+ state->reord = min(fack_count, state->reord);
+ }
+
+ /* Nothing to do; acked frame is about to be dropped. */
+ return;
+ }
+
+ if ((sacked & TCPCB_SACKED_RETRANS) &&
+ after(end_seq, TCP_SKB_CB(skb)->ack_seq) &&
+ (!state->lost_retrans || after(end_seq, state->lost_retrans)))
+ state->lost_retrans = end_seq;
+
+ if (!in_sack)
+ return;
+
+ if (!(sacked & TCPCB_SACKED_ACKED)) {
+ if (sacked & TCPCB_SACKED_RETRANS) {
+ /* If the segment is not tagged as lost,
+ * we do not clear RETRANS, believing
+ * that retransmission is still in flight.
+ */
+ if (sacked & TCPCB_LOST) {
+ TCP_SKB_CB(skb)->sacked &=
+ ~(TCPCB_LOST|TCPCB_SACKED_RETRANS);
+ tp->lost_out -= tcp_skb_pcount(skb);
+ tp->retrans_out -= tcp_skb_pcount(skb);
+
+ /* clear lost hint */
+ tp->retransmit_skb_hint = NULL;
+ }
+ } else {
+ /* New sack for not retransmitted frame,
+ * which was in hole. It is reordering.
+ */
+ if (!(sacked & TCPCB_RETRANS) &&
+ fack_count < state->prior_fackets)
+ state->reord = min(fack_count, state->reord);
+
+ if (sacked & TCPCB_LOST) {
+ TCP_SKB_CB(skb)->sacked &= ~TCPCB_LOST;
+ tp->lost_out -= tcp_skb_pcount(skb);
+
+ /* clear lost hint */
+ tp->retransmit_skb_hint = NULL;
+ }
+ /* SACK enhanced F-RTO detection.
+ * Set flag if and only if non-rexmitted
+ * segments below frto_highmark are
+ * SACKed (RFC4138; Appendix B).
+ * Clearing correct due to in-order walk
+ */
+ if (after(end_seq, tp->frto_highmark)) {
+ state->flag &= ~FLAG_ONLY_ORIG_SACKED;
+ } else {
+ if (!(sacked & TCPCB_RETRANS))
+ state->flag |= FLAG_ONLY_ORIG_SACKED;
+ }
+ }
+
+ TCP_SKB_CB(skb)->sacked |= TCPCB_SACKED_ACKED;
+ state->flag |= FLAG_DATA_SACKED;
+ tp->sacked_out += tcp_skb_pcount(skb);
+
+ if (fack_count > tp->fackets_out)
+ tp->fackets_out = fack_count;
+
+ if (after(TCP_SKB_CB(skb)->seq,
+ tp->highest_sack))
+ tp->highest_sack = TCP_SKB_CB(skb)->seq;
+ } else {
+ if (dup_sack && (sacked&TCPCB_RETRANS))
+ state->reord = min(fack_count, state->reord);
+ }
+
+ /* D-SACK. We can detect redundant retransmission
+ * in S|R and plain R frames and clear it.
+ * undo_retrans is decreased above, L|R frames
+ * are accounted above as well.
+ */
+ if (dup_sack && (TCP_SKB_CB(skb)->sacked&TCPCB_SACKED_RETRANS)) {
+ TCP_SKB_CB(skb)->sacked &= ~TCPCB_SACKED_RETRANS;
+ tp->retrans_out -= tcp_skb_pcount(skb);
+ tp->retransmit_skb_hint = NULL;
+ }
+}
+
static int
tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_una)
{
@@ -1274,7 +1382,6 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
tcp_for_write_queue_from(skb, sk) {
int in_sack, pcount;
- u8 sacked;
if (skb == tcp_send_head(sk))
break;
@@ -1317,108 +1424,8 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
fack_count += pcount;
- sacked = TCP_SKB_CB(skb)->sacked;
-
- /* Account D-SACK for retransmitted packet. */
- if ((dup_sack && in_sack) &&
- (sacked & TCPCB_RETRANS) &&
- after(TCP_SKB_CB(skb)->end_seq, tp->undo_marker))
- tp->undo_retrans--;
-
- /* The frame is ACKed. */
- if (!after(TCP_SKB_CB(skb)->end_seq, tp->snd_una)) {
- if (sacked&TCPCB_RETRANS) {
- if ((dup_sack && in_sack) &&
- (sacked&TCPCB_SACKED_ACKED))
- state.reord = min(fack_count, state.reord);
- } else {
- /* If it was in a hole, we detected reordering. */
- if (fack_count < state.prior_fackets &&
- !(sacked&TCPCB_SACKED_ACKED))
- state.reord = min(fack_count, state.reord);
- }
-
- /* Nothing to do; acked frame is about to be dropped. */
- continue;
- }
-
- if ((sacked&TCPCB_SACKED_RETRANS) &&
- after(end_seq, TCP_SKB_CB(skb)->ack_seq) &&
- (!state.lost_retrans || after(end_seq, state.lost_retrans)))
- state.lost_retrans = end_seq;
-
- if (!in_sack)
- continue;
-
- if (!(sacked&TCPCB_SACKED_ACKED)) {
- if (sacked & TCPCB_SACKED_RETRANS) {
- /* If the segment is not tagged as lost,
- * we do not clear RETRANS, believing
- * that retransmission is still in flight.
- */
- if (sacked & TCPCB_LOST) {
- TCP_SKB_CB(skb)->sacked &= ~(TCPCB_LOST|TCPCB_SACKED_RETRANS);
- tp->lost_out -= tcp_skb_pcount(skb);
- tp->retrans_out -= tcp_skb_pcount(skb);
-
- /* clear lost hint */
- tp->retransmit_skb_hint = NULL;
- }
- } else {
- /* New sack for not retransmitted frame,
- * which was in hole. It is reordering.
- */
- if (!(sacked & TCPCB_RETRANS) &&
- fack_count < state.prior_fackets)
- state.reord = min(fack_count, state.reord);
-
- if (sacked & TCPCB_LOST) {
- TCP_SKB_CB(skb)->sacked &= ~TCPCB_LOST;
- tp->lost_out -= tcp_skb_pcount(skb);
-
- /* clear lost hint */
- tp->retransmit_skb_hint = NULL;
- }
- /* SACK enhanced F-RTO detection.
- * Set flag if and only if non-rexmitted
- * segments below frto_highmark are
- * SACKed (RFC4138; Appendix B).
- * Clearing correct due to in-order walk
- */
- if (after(end_seq, tp->frto_highmark)) {
- state.flag &= ~FLAG_ONLY_ORIG_SACKED;
- } else {
- if (!(sacked & TCPCB_RETRANS))
- state.flag |= FLAG_ONLY_ORIG_SACKED;
- }
- }
-
- TCP_SKB_CB(skb)->sacked |= TCPCB_SACKED_ACKED;
- state.flag |= FLAG_DATA_SACKED;
- tp->sacked_out += tcp_skb_pcount(skb);
-
- if (fack_count > tp->fackets_out)
- tp->fackets_out = fack_count;
-
- if (after(TCP_SKB_CB(skb)->seq,
- tp->highest_sack))
- tp->highest_sack = TCP_SKB_CB(skb)->seq;
- } else {
- if (dup_sack && (sacked&TCPCB_RETRANS))
- state.reord = min(fack_count, state.reord);
- }
-
- /* D-SACK. We can detect redundant retransmission
- * in S|R and plain R frames and clear it.
- * undo_retrans is decreased above, L|R frames
- * are accounted above as well.
- */
- if (dup_sack &&
- (TCP_SKB_CB(skb)->sacked&TCPCB_SACKED_RETRANS)) {
- TCP_SKB_CB(skb)->sacked &= ~TCPCB_SACKED_RETRANS;
- tp->retrans_out -= tcp_skb_pcount(skb);
- tp->retransmit_skb_hint = NULL;
- }
+ tcp_sacktag_one(skb, tp, &state, in_sack,
+ dup_sack, fack_count, end_seq);
}
}
--
1.5.0.6
^ permalink raw reply related
* [PATCH 1/5] [TCP]: Create tcp_sacktag_state.
From: Ilpo Järvinen @ 2007-09-24 10:28 UTC (permalink / raw)
To: David Miller, Stephen Hemminger, SANGTAE HA, Tom Quetchenbach,
Baruch Even <bar
Cc: netdev, David S. Miller
In-Reply-To: <11906297271684-git-send-email-ilpo.jarvinen@helsinki.fi>
From: David S. Miller <davem@davemloft.net>
It is difficult to break out the inner-logic of
tcp_sacktag_write_queue() into worker functions because
so many local variables get updated in-place.
Start to overcome this by creating a structure block
of state variables that can be passed around into
worker routines.
[I made minor tweaks due to rebase/reordering of stuff
in tcp-2.6 tree, and dropped found_dup_sack & dup_sack
from state -ij]
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
net/ipv4/tcp_input.c | 89 ++++++++++++++++++++++++++-----------------------
1 files changed, 47 insertions(+), 42 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 259f517..04ff465 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1102,6 +1102,13 @@ static int tcp_is_sackblock_valid(struct tcp_sock *tp, int is_dsack,
return !before(start_seq, end_seq - tp->max_window);
}
+struct tcp_sacktag_state {
+ unsigned int flag;
+ int reord;
+ int prior_fackets;
+ u32 lost_retrans;
+ int first_sack_index;
+};
static int tcp_check_dsack(struct tcp_sock *tp, struct sk_buff *ack_skb,
struct tcp_sack_block_wire *sp, int num_sacks,
@@ -1146,25 +1153,22 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
struct tcp_sack_block_wire *sp = (struct tcp_sack_block_wire *)(ptr+2);
struct sk_buff *cached_skb;
int num_sacks = (ptr[1] - TCPOLEN_SACK_BASE)>>3;
- int reord = tp->packets_out;
- int prior_fackets;
- u32 lost_retrans = 0;
- int flag = 0;
- int found_dup_sack = 0;
+ struct tcp_sacktag_state state;
+ int found_dup_sack;
int cached_fack_count;
int i;
- int first_sack_index;
+ int force_one_sack;
+
+ state.flag = 0;
if (!tp->sacked_out) {
tp->fackets_out = 0;
tp->highest_sack = tp->snd_una;
}
- prior_fackets = tp->fackets_out;
- found_dup_sack = tcp_check_dsack(tp, ack_skb, sp,
- num_sacks, prior_snd_una);
+ found_dup_sack = tcp_check_dsack(tp, ack_skb, sp, num_sacks, prior_snd_una);
if (found_dup_sack)
- flag |= FLAG_DSACKING_ACK;
+ state.flag |= FLAG_DSACKING_ACK;
/* Eliminate too old ACKs, but take into
* account more or less fresh ones, they can
@@ -1177,18 +1181,18 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
* if the only SACK change is the increase of the end_seq of
* the first block then only apply that SACK block
* and use retrans queue hinting otherwise slowpath */
- flag = 1;
+ force_one_sack = 1;
for (i = 0; i < num_sacks; i++) {
__be32 start_seq = sp[i].start_seq;
__be32 end_seq = sp[i].end_seq;
if (i == 0) {
if (tp->recv_sack_cache[i].start_seq != start_seq)
- flag = 0;
+ force_one_sack = 0;
} else {
if ((tp->recv_sack_cache[i].start_seq != start_seq) ||
(tp->recv_sack_cache[i].end_seq != end_seq))
- flag = 0;
+ force_one_sack = 0;
}
tp->recv_sack_cache[i].start_seq = start_seq;
tp->recv_sack_cache[i].end_seq = end_seq;
@@ -1199,8 +1203,8 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
tp->recv_sack_cache[i].end_seq = 0;
}
- first_sack_index = 0;
- if (flag)
+ state.first_sack_index = 0;
+ if (force_one_sack)
num_sacks = 1;
else {
int j;
@@ -1218,17 +1222,14 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
sp[j+1] = tmp;
/* Track where the first SACK block goes to */
- if (j == first_sack_index)
- first_sack_index = j+1;
+ if (j == state.first_sack_index)
+ state.first_sack_index = j+1;
}
}
}
}
- /* clear flag as used for different purpose in following code */
- flag = 0;
-
/* Use SACK fastpath hint if valid */
cached_skb = tp->fastpath_skb_hint;
cached_fack_count = tp->fastpath_cnt_hint;
@@ -1237,12 +1238,16 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
cached_fack_count = 0;
}
+ state.reord = tp->packets_out;
+ state.prior_fackets = tp->fackets_out;
+ state.lost_retrans = 0;
+
for (i=0; i<num_sacks; i++, sp++) {
struct sk_buff *skb;
__u32 start_seq = ntohl(sp->start_seq);
__u32 end_seq = ntohl(sp->end_seq);
int fack_count;
- int dup_sack = (found_dup_sack && (i == first_sack_index));
+ int dup_sack = (found_dup_sack && (i == state.first_sack_index));
if (!tcp_is_sackblock_valid(tp, dup_sack, start_seq, end_seq)) {
if (dup_sack) {
@@ -1265,7 +1270,7 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
/* Event "B" in the comment above. */
if (after(end_seq, tp->high_seq))
- flag |= FLAG_DATA_LOST;
+ state.flag |= FLAG_DATA_LOST;
tcp_for_write_queue_from(skb, sk) {
int in_sack, pcount;
@@ -1276,7 +1281,7 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
cached_skb = skb;
cached_fack_count = fack_count;
- if (i == first_sack_index) {
+ if (i == state.first_sack_index) {
tp->fastpath_skb_hint = skb;
tp->fastpath_cnt_hint = fack_count;
}
@@ -1325,12 +1330,12 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
if (sacked&TCPCB_RETRANS) {
if ((dup_sack && in_sack) &&
(sacked&TCPCB_SACKED_ACKED))
- reord = min(fack_count, reord);
+ state.reord = min(fack_count, state.reord);
} else {
/* If it was in a hole, we detected reordering. */
- if (fack_count < prior_fackets &&
+ if (fack_count < state.prior_fackets &&
!(sacked&TCPCB_SACKED_ACKED))
- reord = min(fack_count, reord);
+ state.reord = min(fack_count, state.reord);
}
/* Nothing to do; acked frame is about to be dropped. */
@@ -1339,8 +1344,8 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
if ((sacked&TCPCB_SACKED_RETRANS) &&
after(end_seq, TCP_SKB_CB(skb)->ack_seq) &&
- (!lost_retrans || after(end_seq, lost_retrans)))
- lost_retrans = end_seq;
+ (!state.lost_retrans || after(end_seq, state.lost_retrans)))
+ state.lost_retrans = end_seq;
if (!in_sack)
continue;
@@ -1364,8 +1369,8 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
* which was in hole. It is reordering.
*/
if (!(sacked & TCPCB_RETRANS) &&
- fack_count < prior_fackets)
- reord = min(fack_count, reord);
+ fack_count < state.prior_fackets)
+ state.reord = min(fack_count, state.reord);
if (sacked & TCPCB_LOST) {
TCP_SKB_CB(skb)->sacked &= ~TCPCB_LOST;
@@ -1381,15 +1386,15 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
* Clearing correct due to in-order walk
*/
if (after(end_seq, tp->frto_highmark)) {
- flag &= ~FLAG_ONLY_ORIG_SACKED;
+ state.flag &= ~FLAG_ONLY_ORIG_SACKED;
} else {
if (!(sacked & TCPCB_RETRANS))
- flag |= FLAG_ONLY_ORIG_SACKED;
+ state.flag |= FLAG_ONLY_ORIG_SACKED;
}
}
TCP_SKB_CB(skb)->sacked |= TCPCB_SACKED_ACKED;
- flag |= FLAG_DATA_SACKED;
+ state.flag |= FLAG_DATA_SACKED;
tp->sacked_out += tcp_skb_pcount(skb);
if (fack_count > tp->fackets_out)
@@ -1400,7 +1405,7 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
tp->highest_sack = TCP_SKB_CB(skb)->seq;
} else {
if (dup_sack && (sacked&TCPCB_RETRANS))
- reord = min(fack_count, reord);
+ state.reord = min(fack_count, state.reord);
}
/* D-SACK. We can detect redundant retransmission
@@ -1423,20 +1428,20 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
* we have to account for reordering! Ugly,
* but should help.
*/
- if (lost_retrans && icsk->icsk_ca_state == TCP_CA_Recovery) {
+ if (state.lost_retrans && icsk->icsk_ca_state == TCP_CA_Recovery) {
struct sk_buff *skb;
tcp_for_write_queue(skb, sk) {
if (skb == tcp_send_head(sk))
break;
- if (after(TCP_SKB_CB(skb)->seq, lost_retrans))
+ if (after(TCP_SKB_CB(skb)->seq, state.lost_retrans))
break;
if (!after(TCP_SKB_CB(skb)->end_seq, tp->snd_una))
continue;
if ((TCP_SKB_CB(skb)->sacked&TCPCB_SACKED_RETRANS) &&
- after(lost_retrans, TCP_SKB_CB(skb)->ack_seq) &&
+ after(state.lost_retrans, TCP_SKB_CB(skb)->ack_seq) &&
(tcp_is_fack(tp) ||
- !before(lost_retrans,
+ !before(state.lost_retrans,
TCP_SKB_CB(skb)->ack_seq + tp->reordering *
tp->mss_cache))) {
TCP_SKB_CB(skb)->sacked &= ~TCPCB_SACKED_RETRANS;
@@ -1448,7 +1453,7 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
if (!(TCP_SKB_CB(skb)->sacked&(TCPCB_LOST|TCPCB_SACKED_ACKED))) {
tp->lost_out += tcp_skb_pcount(skb);
TCP_SKB_CB(skb)->sacked |= TCPCB_LOST;
- flag |= FLAG_DATA_SACKED;
+ state.flag |= FLAG_DATA_SACKED;
NET_INC_STATS_BH(LINUX_MIB_TCPLOSTRETRANSMIT);
}
}
@@ -1457,9 +1462,9 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
tcp_verify_left_out(tp);
- if ((reord < tp->fackets_out) && icsk->icsk_ca_state != TCP_CA_Loss &&
+ if ((state.reord < tp->fackets_out) && icsk->icsk_ca_state != TCP_CA_Loss &&
(!tp->frto_highmark || after(tp->snd_una, tp->frto_highmark)))
- tcp_update_reordering(sk, ((tp->fackets_out + 1) - reord), 0);
+ tcp_update_reordering(sk, ((tp->fackets_out + 1) - state.reord), 0);
#if FASTRETRANS_DEBUG > 0
BUG_TRAP((int)tp->sacked_out >= 0);
@@ -1467,7 +1472,7 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
BUG_TRAP((int)tp->retrans_out >= 0);
BUG_TRAP((int)tcp_packets_in_flight(tp) >= 0);
#endif
- return flag;
+ return state.flag;
}
/* F-RTO can only be used if TCP has never retransmitted anything other than
--
1.5.0.6
^ permalink raw reply related
* [DEVELOPER PATCH 5/5] [TCP]: Track sacktag
From: Ilpo Järvinen @ 2007-09-24 10:28 UTC (permalink / raw)
To: David Miller, Stephen Hemminger, SANGTAE HA, Tom Quetchenbach,
Baruch Even <bar
Cc: netdev, Ilpo Järvinen
In-Reply-To: <1190629727956-git-send-email-ilpo.jarvinen@helsinki.fi>
From: =?ISO-8859-1?q?Ilpo_J=E4rvinen?= <ilpo.jarvinen@helsinki.fi>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
include/linux/snmp.h | 20 +++++++++++++++++++
net/ipv4/proc.c | 20 +++++++++++++++++++
net/ipv4/tcp_input.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++--
3 files changed, 89 insertions(+), 3 deletions(-)
diff --git a/include/linux/snmp.h b/include/linux/snmp.h
index 89f0c2b..42b8c07 100644
--- a/include/linux/snmp.h
+++ b/include/linux/snmp.h
@@ -214,6 +214,26 @@ enum
LINUX_MIB_TCPDSACKIGNOREDOLD, /* TCPSACKIgnoredOld */
LINUX_MIB_TCPDSACKIGNOREDNOUNDO, /* TCPSACKIgnoredNoUndo */
LINUX_MIB_TCPSPURIOUSRTOS, /* TCPSpuriousRTOs */
+ LINUX_MIB_TCP_SACKTAG,
+ LINUX_MIB_TCP_SACK0,
+ LINUX_MIB_TCP_SACK1,
+ LINUX_MIB_TCP_SACK2,
+ LINUX_MIB_TCP_SACK3,
+ LINUX_MIB_TCP_SACK4,
+ LINUX_MIB_TCP_WALKEDSKBS,
+ LINUX_MIB_TCP_WALKEDDSACKS,
+ LINUX_MIB_TCP_SKIPPEDSKBS,
+ LINUX_MIB_TCP_NOCACHE,
+ LINUX_MIB_TCP_FULLWALK,
+ LINUX_MIB_TCP_HEADWALK,
+ LINUX_MIB_TCP_TAILWALK,
+ LINUX_MIB_TCP_FULLSKIP,
+ LINUX_MIB_TCP_TAILSKIP,
+ LINUX_MIB_TCP_HEADSKIP,
+ LINUX_MIB_TCP_FULLSKIP_TOHIGH,
+ LINUX_MIB_TCP_TAILSKIP_TOHIGH,
+ LINUX_MIB_TCP_NEWSKIP,
+ LINUX_MIB_TCP_CACHEREMAINING,
__LINUX_MIB_MAX
};
diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index 9dee70e..4808f82 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -246,6 +246,26 @@ static const struct snmp_mib snmp4_net_list[] = {
SNMP_MIB_ITEM("TCPDSACKIgnoredOld", LINUX_MIB_TCPDSACKIGNOREDOLD),
SNMP_MIB_ITEM("TCPDSACKIgnoredNoUndo", LINUX_MIB_TCPDSACKIGNOREDNOUNDO),
SNMP_MIB_ITEM("TCPSpuriousRTOs", LINUX_MIB_TCPSPURIOUSRTOS),
+ SNMP_MIB_ITEM("TCP_SACKTAG", LINUX_MIB_TCP_SACKTAG),
+ SNMP_MIB_ITEM("TCP_SACK0", LINUX_MIB_TCP_SACK0),
+ SNMP_MIB_ITEM("TCP_SACK1", LINUX_MIB_TCP_SACK1),
+ SNMP_MIB_ITEM("TCP_SACK2", LINUX_MIB_TCP_SACK2),
+ SNMP_MIB_ITEM("TCP_SACK3", LINUX_MIB_TCP_SACK3),
+ SNMP_MIB_ITEM("TCP_SACK4", LINUX_MIB_TCP_SACK4),
+ SNMP_MIB_ITEM("TCP_WALKEDSKBS", LINUX_MIB_TCP_WALKEDSKBS),
+ SNMP_MIB_ITEM("TCP_WALKEDDSACKS", LINUX_MIB_TCP_WALKEDDSACKS),
+ SNMP_MIB_ITEM("TCP_SKIPPEDSKBS", LINUX_MIB_TCP_SKIPPEDSKBS),
+ SNMP_MIB_ITEM("TCP_NOCACHE", LINUX_MIB_TCP_NOCACHE),
+ SNMP_MIB_ITEM("TCP_FULLWALK", LINUX_MIB_TCP_FULLWALK),
+ SNMP_MIB_ITEM("TCP_HEADWALK", LINUX_MIB_TCP_HEADWALK),
+ SNMP_MIB_ITEM("TCP_TAILWALK", LINUX_MIB_TCP_TAILWALK),
+ SNMP_MIB_ITEM("TCP_FULLSKIP", LINUX_MIB_TCP_FULLSKIP),
+ SNMP_MIB_ITEM("TCP_TAILSKIP", LINUX_MIB_TCP_TAILSKIP),
+ SNMP_MIB_ITEM("TCP_HEADSKIP", LINUX_MIB_TCP_HEADSKIP),
+ SNMP_MIB_ITEM("TCP_FULLSKIP_TOHIGH", LINUX_MIB_TCP_FULLSKIP_TOHIGH),
+ SNMP_MIB_ITEM("TCP_TAILSKIP_TOHIGH", LINUX_MIB_TCP_TAILSKIP_TOHIGH),
+ SNMP_MIB_ITEM("TCP_NEWSKIP", LINUX_MIB_TCP_NEWSKIP),
+ SNMP_MIB_ITEM("TCP_CACHEREMAINING", LINUX_MIB_TCP_CACHEREMAINING),
SNMP_MIB_SENTINEL
};
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 9dfdd67..38045c8 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1306,6 +1306,10 @@ static struct sk_buff *tcp_sacktag_walk(struct sk_buff *skb, struct sock *sk,
tcp_sacktag_one(skb, sk, state, in_sack, dup_sack,
state->fack_count, end_seq);
+
+ NET_INC_STATS_BH(LINUX_MIB_TCP_WALKEDSKBS);
+ if (dup_sack)
+ NET_INC_STATS_BH(LINUX_MIB_TCP_WALKEDDSACKS);
}
return skb;
}
@@ -1329,6 +1333,7 @@ static struct sk_buff *tcp_sacktag_skip(struct sk_buff *skb, struct sock *sk,
skb = tcp_sacktag_walk(skb, sk, state, state->dup_start,
state->dup_end, 1);
}
+ NET_INC_STATS_BH(LINUX_MIB_TCP_SKIPPEDSKBS);
}
return skb;
}
@@ -1458,9 +1463,22 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
cache++;
}
+ NET_INC_STATS_BH(LINUX_MIB_TCP_SACKTAG);
+ switch (used_sacks) {
+ case 0: NET_INC_STATS_BH(LINUX_MIB_TCP_SACK0); break;
+ case 1: NET_INC_STATS_BH(LINUX_MIB_TCP_SACK1); break;
+ case 2: NET_INC_STATS_BH(LINUX_MIB_TCP_SACK2); break;
+ case 3: NET_INC_STATS_BH(LINUX_MIB_TCP_SACK3); break;
+ case 4: NET_INC_STATS_BH(LINUX_MIB_TCP_SACK4); break;
+ }
+
+ if (!tcp_sack_cache_ok(tp, cache))
+ NET_INC_STATS_BH(LINUX_MIB_TCP_NOCACHE);
+
while (i < used_sacks) {
u32 start_seq = sp[i].start_seq;
u32 end_seq = sp[i].end_seq;
+ int fullwalk = 0;
/* Event "B" in the comment above. */
if (after(end_seq, tp->high_seq))
@@ -1473,41 +1491,69 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
if (tcp_sack_cache_ok(tp, cache)) {
if (after(end_seq, cache->start_seq)) {
+ int headskip = 0;
+
if (before(start_seq, cache->start_seq)) {
skb = tcp_sacktag_skip(skb, sk, &state, start_seq);
skb = tcp_sacktag_walk(skb, sk, &state, start_seq, cache->start_seq, 0);
- }
+ NET_INC_STATS_BH(LINUX_MIB_TCP_HEADWALK);
+ } else
+ headskip = 1;
+
/* Rest of the block already fully processed? */
if (!after(end_seq, cache->end_seq)) {
i++;
+ if (headskip)
+ NET_INC_STATS_BH(LINUX_MIB_TCP_FULLSKIP);
+ else
+ NET_INC_STATS_BH(LINUX_MIB_TCP_TAILSKIP);
continue;
}
+
if (TCP_SKB_CB(tp->highest_sack)->end_seq != cache->end_seq) {
skb = tcp_sacktag_skip(skb, sk, &state, cache->end_seq);
cache++;
+ if (headskip)
+ NET_INC_STATS_BH(LINUX_MIB_TCP_HEADSKIP);
continue;
}
skb = tcp_sacktag_skip_to_highsack(skb, sk, &state, cache);
- }
+ if (headskip)
+ NET_INC_STATS_BH(LINUX_MIB_TCP_FULLSKIP_TOHIGH);
+ else
+ NET_INC_STATS_BH(LINUX_MIB_TCP_TAILSKIP_TOHIGH);
+ } else
+ fullwalk = 1;
} else if (!before(start_seq, tcp_highest_sack_seq(sk)) &&
before(TCP_SKB_CB(skb)->seq, tcp_highest_sack_seq(sk))) {
skb = tcp_write_queue_next(sk, tp->highest_sack);
state.fack_count = tp->fackets_out;
+ NET_INC_STATS_BH(LINUX_MIB_TCP_NEWSKIP);
+ fullwalk = 1;
}
skb = tcp_sacktag_skip(skb, sk, &state, start_seq);
skb = tcp_sacktag_walk(skb, sk, &state, start_seq, end_seq, 0);
+ if (fullwalk)
+ NET_INC_STATS_BH(LINUX_MIB_TCP_FULLWALK);
+ else
+ NET_INC_STATS_BH(LINUX_MIB_TCP_TAILWALK);
i++;
}
+ if (tcp_sack_cache_ok(tp, cache))
+ NET_INC_STATS_BH(LINUX_MIB_TCP_CACHEREMAINING);
+
/* Clear the head of the cache sack blocks so we can skip it next time */
for (i = 0; i < ARRAY_SIZE(tp->recv_sack_cache) - used_sacks; i++) {
tp->recv_sack_cache[i].start_seq = 0;
tp->recv_sack_cache[i].end_seq = 0;
}
- for (j = 0; j < used_sacks; j++)
+ for (j = 0; j < used_sacks; j++) {
+ WARN_ON(i >= ARRAY_SIZE(tp->recv_sack_cache));
tp->recv_sack_cache[i++] = sp[j];
+ }
/* Check for lost retransmit. This superb idea is
* borrowed from "ratehalving". Event "C".
--
1.5.0.6
^ permalink raw reply related
* [RFC PATCH 4/5] [TCP]: Rewrite sack_recv_cache (WIP)
From: Ilpo Järvinen @ 2007-09-24 10:28 UTC (permalink / raw)
To: David Miller, Stephen Hemminger, SANGTAE HA, Tom Quetchenbach,
Baruch Even <bar
Cc: netdev, Ilpo Järvinen
In-Reply-To: <11906297274028-git-send-email-ilpo.jarvinen@helsinki.fi>
From: =?ISO-8859-1?q?Ilpo_J=E4rvinen?= <ilpo.jarvinen@helsinki.fi>
Previously a number of cases in TCP SACK processing fail to take
advantage of costly stored information in sack_recv_cache. Most
importantly expected events such as cumulative ACK, new hole
ACKs and first ACK after RTO fall to this category. Processing
on such ACKs result in rather long walks building up latencies
(which easily gets nasty when window is large), which are
completely unnecessary, usually no new information was gathered
except the new SACK block above the hole in the respective case.
Since the inclusion of highest_sack, there's a lot information
that is very likely redundant (SACK fastpath hint stuff,
fackets_out, highest_sack), though there's no ultimate guarantee
that they'll remain the same whole the time (in all unearthly
scenarios). Take advantage of this too and drop fastpath hint.
Effectively this drops "special cased" fastpath. This change
adds some complexity to introduce better coveraged "fastpath".
The current ACK's SACK blocks are compared against each cached
block individially and only ranges that are new are then scanned
by the high constant walk. For other parts of write queue, even
when in previously known part of the SACK blocks, a faster skip
function is used. In addition, whenever possible, TCP
fast-forwards to highest_sack skb that was made available
earlier. In typical case, no other things but this fast-forward
and mandatory markings after that occur making the access
pattern quite similar to the former fastpath. DSACKs are special
case that must always be walked.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
include/linux/tcp.h | 4 +-
include/net/tcp.h | 1 -
net/ipv4/tcp_input.c | 320 ++++++++++++++++++++++++++++++------------------
net/ipv4/tcp_output.c | 12 +--
4 files changed, 202 insertions(+), 135 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 1d6be2a..8d91eac 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -330,7 +330,7 @@ struct tcp_sock {
struct tcp_sack_block duplicate_sack[1]; /* D-SACK block */
struct tcp_sack_block selective_acks[4]; /* The SACKS themselves*/
- struct tcp_sack_block_wire recv_sack_cache[4];
+ struct tcp_sack_block recv_sack_cache[4];
struct sk_buff *highest_sack; /* highest skb with SACK received
* (validity guaranteed only if
@@ -343,9 +343,7 @@ struct tcp_sock {
struct sk_buff *scoreboard_skb_hint;
struct sk_buff *retransmit_skb_hint;
struct sk_buff *forward_skb_hint;
- struct sk_buff *fastpath_skb_hint;
- int fastpath_cnt_hint;
int lost_cnt_hint;
int retransmit_cnt_hint;
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 8bc64b7..d5def9b 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1078,7 +1078,6 @@ static inline void tcp_clear_retrans_hints_partial(struct tcp_sock *tp)
static inline void tcp_clear_all_retrans_hints(struct tcp_sock *tp)
{
tcp_clear_retrans_hints_partial(tp);
- tp->fastpath_skb_hint = NULL;
}
/* MD5 Signature */
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 85dd4b0..9dfdd67 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1106,11 +1106,15 @@ struct tcp_sacktag_state {
unsigned int flag;
int reord;
int prior_fackets;
+ int fack_count;
u32 lost_retrans;
- int first_sack_index;
+ u32 dup_start;
+ u32 dup_end;
};
-static int tcp_check_dsack(struct tcp_sock *tp, struct sk_buff *ack_skb,
+static int tcp_check_dsack(struct tcp_sock *tp,
+ struct tcp_sacktag_state *state,
+ struct sk_buff *ack_skb,
struct tcp_sack_block_wire *sp, int num_sacks,
u32 prior_snd_una)
{
@@ -1120,6 +1124,8 @@ static int tcp_check_dsack(struct tcp_sock *tp, struct sk_buff *ack_skb,
if (before(start_seq_0, TCP_SKB_CB(ack_skb)->ack_seq)) {
dup_sack = 1;
+ state->dup_start = start_seq_0;
+ state->dup_end = end_seq_0;
tcp_dsack_seen(tp);
NET_INC_STATS_BH(LINUX_MIB_TCPDSACKRECV);
} else if (num_sacks > 1) {
@@ -1129,6 +1135,8 @@ static int tcp_check_dsack(struct tcp_sock *tp, struct sk_buff *ack_skb,
if (!after(end_seq_0, end_seq_1) &&
!before(start_seq_0, start_seq_1)) {
dup_sack = 1;
+ state->dup_start = start_seq_1;
+ state->dup_end = end_seq_1;
tcp_dsack_seen(tp);
NET_INC_STATS_BH(LINUX_MIB_TCPDSACKOFORECV);
}
@@ -1251,6 +1259,104 @@ static void tcp_sacktag_one(struct sk_buff *skb, struct sock *sk,
}
}
+static struct sk_buff *tcp_sacktag_walk(struct sk_buff *skb, struct sock *sk,
+ struct tcp_sacktag_state *state,
+ u32 start_seq, u32 end_seq,
+ int dup_sack)
+{
+ tcp_for_write_queue_from(skb, sk) {
+ int in_sack, pcount;
+
+ if (skb == tcp_send_head(sk))
+ break;
+
+ /* The retransmission queue is always in order, so
+ * we can short-circuit the walk early.
+ */
+ if (!before(TCP_SKB_CB(skb)->seq, end_seq))
+ break;
+
+ if (dup_sack)
+ state->dup_start = 0;
+
+ if (state->dup_start && !before(TCP_SKB_CB(skb)->seq, state->dup_start))
+ tcp_sacktag_walk(skb, sk, state, state->dup_start, state->dup_end, 1);
+
+ in_sack = !after(start_seq, TCP_SKB_CB(skb)->seq) &&
+ !before(end_seq, TCP_SKB_CB(skb)->end_seq);
+
+ pcount = tcp_skb_pcount(skb);
+
+ if (pcount > 1 && !in_sack &&
+ after(TCP_SKB_CB(skb)->end_seq, start_seq)) {
+ unsigned int pkt_len;
+
+ in_sack = !after(start_seq, TCP_SKB_CB(skb)->seq);
+
+ if (!in_sack)
+ pkt_len = (start_seq - TCP_SKB_CB(skb)->seq);
+ else
+ pkt_len = (end_seq - TCP_SKB_CB(skb)->seq);
+ if (tcp_fragment(sk, skb, pkt_len, skb_shinfo(skb)->gso_size))
+ break;
+ pcount = tcp_skb_pcount(skb);
+ }
+
+ state->fack_count += pcount;
+
+ tcp_sacktag_one(skb, sk, state, in_sack, dup_sack,
+ state->fack_count, end_seq);
+ }
+ return skb;
+}
+
+/* Avoid all extra work that is being done by sacktag while walking in
+ * a normal way
+ */
+static struct sk_buff *tcp_sacktag_skip(struct sk_buff *skb, struct sock *sk,
+ struct tcp_sacktag_state *state,
+ u32 skip_to_seq)
+{
+ tcp_for_write_queue_from(skb, sk) {
+ if (skb == tcp_send_head(sk))
+ break;
+
+ if (before(TCP_SKB_CB(skb)->end_seq, skip_to_seq))
+ break;
+
+ /* DSACKs must always be processed */
+ if (state->dup_start && !before(TCP_SKB_CB(skb)->seq, state->dup_start)) {
+ skb = tcp_sacktag_walk(skb, sk, state, state->dup_start,
+ state->dup_end, 1);
+ }
+ }
+ return skb;
+}
+
+/* We have better entry point available */
+static struct sk_buff *tcp_sacktag_skip_to_highsack(struct sk_buff *skb,
+ struct sock *sk,
+ struct tcp_sacktag_state *state,
+ struct tcp_sack_block *cache)
+{
+ struct tcp_sock *tp = tcp_sk(sk);
+
+ if (state->dup_start && after(state->dup_start, cache->start_seq) &&
+ before(state->dup_start, TCP_SKB_CB(tp->highest_sack)->end_seq)) {
+ skb = tcp_sacktag_skip(skb, sk, state, state->dup_start);
+ tcp_sacktag_walk(skb, sk, state, state->dup_start, state->dup_end, 1);
+ }
+ skb = tcp_write_queue_next(sk, tp->highest_sack);
+ state->fack_count = tp->fackets_out;
+
+ return skb;
+}
+
+static int tcp_sack_cache_ok(struct tcp_sock *tp, struct tcp_sack_block *cache)
+{
+ return cache < tp->recv_sack_cache + ARRAY_SIZE(tp->recv_sack_cache);
+}
+
static int
tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_una)
{
@@ -1258,23 +1364,26 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
struct tcp_sock *tp = tcp_sk(sk);
unsigned char *ptr = (skb_transport_header(ack_skb) +
TCP_SKB_CB(ack_skb)->sacked);
- struct tcp_sack_block_wire *sp = (struct tcp_sack_block_wire *)(ptr+2);
- struct sk_buff *cached_skb;
+ struct tcp_sack_block_wire *sp_wire = (struct tcp_sack_block_wire *)(ptr+2);
+ struct tcp_sack_block sp[4];
+ struct tcp_sack_block *cache;
int num_sacks = (ptr[1] - TCPOLEN_SACK_BASE)>>3;
+ int used_sacks;
struct tcp_sacktag_state state;
int found_dup_sack;
- int cached_fack_count;
- int i;
- int force_one_sack;
+ struct sk_buff *skb;
+ int i, j;
state.flag = 0;
+ state.dup_start = 0;
+ state.dup_end = 0;
if (!tp->sacked_out) {
tp->fackets_out = 0;
tp->highest_sack = tcp_write_queue_head(sk);
}
- found_dup_sack = tcp_check_dsack(tp, ack_skb, sp, num_sacks, prior_snd_una);
+ found_dup_sack = tcp_check_dsack(tp, &state, ack_skb, sp_wire, num_sacks, prior_snd_una);
if (found_dup_sack)
state.flag |= FLAG_DSACKING_ACK;
@@ -1285,79 +1394,16 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
if (before(TCP_SKB_CB(ack_skb)->ack_seq, prior_snd_una - tp->max_window))
return 0;
- /* SACK fastpath:
- * if the only SACK change is the increase of the end_seq of
- * the first block then only apply that SACK block
- * and use retrans queue hinting otherwise slowpath */
- force_one_sack = 1;
+ used_sacks = 0;
for (i = 0; i < num_sacks; i++) {
- __be32 start_seq = sp[i].start_seq;
- __be32 end_seq = sp[i].end_seq;
+ int dup_sack = !i && found_dup_sack;
- if (i == 0) {
- if (tp->recv_sack_cache[i].start_seq != start_seq)
- force_one_sack = 0;
- } else {
- if ((tp->recv_sack_cache[i].start_seq != start_seq) ||
- (tp->recv_sack_cache[i].end_seq != end_seq))
- force_one_sack = 0;
- }
- tp->recv_sack_cache[i].start_seq = start_seq;
- tp->recv_sack_cache[i].end_seq = end_seq;
- }
- /* Clear the rest of the cache sack blocks so they won't match mistakenly. */
- for (; i < ARRAY_SIZE(tp->recv_sack_cache); i++) {
- tp->recv_sack_cache[i].start_seq = 0;
- tp->recv_sack_cache[i].end_seq = 0;
- }
+ sp[used_sacks].start_seq = ntohl(get_unaligned(&sp_wire[i].start_seq));
+ sp[used_sacks].end_seq = ntohl(get_unaligned(&sp_wire[i].end_seq));
- state.first_sack_index = 0;
- if (force_one_sack)
- num_sacks = 1;
- else {
- int j;
- tp->fastpath_skb_hint = NULL;
-
- /* order SACK blocks to allow in order walk of the retrans queue */
- for (i = num_sacks-1; i > 0; i--) {
- for (j = 0; j < i; j++){
- if (after(ntohl(sp[j].start_seq),
- ntohl(sp[j+1].start_seq))){
- struct tcp_sack_block_wire tmp;
-
- tmp = sp[j];
- sp[j] = sp[j+1];
- sp[j+1] = tmp;
-
- /* Track where the first SACK block goes to */
- if (j == state.first_sack_index)
- state.first_sack_index = j+1;
- }
-
- }
- }
- }
-
- /* Use SACK fastpath hint if valid */
- cached_skb = tp->fastpath_skb_hint;
- cached_fack_count = tp->fastpath_cnt_hint;
- if (!cached_skb) {
- cached_skb = tcp_write_queue_head(sk);
- cached_fack_count = 0;
- }
-
- state.reord = tp->packets_out;
- state.prior_fackets = tp->fackets_out;
- state.lost_retrans = 0;
-
- for (i=0; i<num_sacks; i++, sp++) {
- struct sk_buff *skb;
- __u32 start_seq = ntohl(sp->start_seq);
- __u32 end_seq = ntohl(sp->end_seq);
- int fack_count;
- int dup_sack = (found_dup_sack && (i == state.first_sack_index));
-
- if (!tcp_is_sackblock_valid(tp, dup_sack, start_seq, end_seq)) {
+ if (!tcp_is_sackblock_valid(tp, dup_sack,
+ sp[used_sacks].start_seq,
+ sp[used_sacks].end_seq)) {
if (dup_sack) {
if (!tp->undo_marker)
NET_INC_STATS_BH(LINUX_MIB_TCPDSACKIGNOREDNOUNDO);
@@ -1366,68 +1412,102 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
} else {
/* Don't count olds caused by ACK reordering */
if ((TCP_SKB_CB(ack_skb)->ack_seq != tp->snd_una) &&
- !after(end_seq, tp->snd_una))
+ !after(sp[used_sacks].end_seq, tp->snd_una))
continue;
NET_INC_STATS_BH(LINUX_MIB_TCPSACKDISCARD);
}
continue;
}
- skb = cached_skb;
- fack_count = cached_fack_count;
-
- /* Event "B" in the comment above. */
- if (after(end_seq, tp->high_seq))
- state.flag |= FLAG_DATA_LOST;
+ /* Ignore very old stuff early */
+ if (!after(sp[used_sacks].end_seq, prior_snd_una))
+ continue;
- tcp_for_write_queue_from(skb, sk) {
- int in_sack, pcount;
+ used_sacks++;
+ }
- if (skb == tcp_send_head(sk))
- break;
+ /* order SACK blocks to allow in order walk of the retrans queue */
+ for (i = used_sacks-1; i > 0; i--) {
+ for (j = 0; j < i; j++){
+ if (after(sp[j].start_seq, sp[j+1].start_seq)) {
+ struct tcp_sack_block tmp;
- cached_skb = skb;
- cached_fack_count = fack_count;
- if (i == state.first_sack_index) {
- tp->fastpath_skb_hint = skb;
- tp->fastpath_cnt_hint = fack_count;
+ tmp = sp[j];
+ sp[j] = sp[j+1];
+ sp[j+1] = tmp;
}
+ }
+ }
- /* The retransmission queue is always in order, so
- * we can short-circuit the walk early.
- */
- if (!before(TCP_SKB_CB(skb)->seq, end_seq))
- break;
+ state.reord = tp->packets_out;
+ state.prior_fackets = tp->fackets_out;
+ state.lost_retrans = 0;
+ state.fack_count = 0;
- in_sack = !after(start_seq, TCP_SKB_CB(skb)->seq) &&
- !before(end_seq, TCP_SKB_CB(skb)->end_seq);
+ skb = tcp_write_queue_head(sk);
+ i = 0;
- pcount = tcp_skb_pcount(skb);
+ if (!tp->sacked_out) {
+ /* It's already past, so skip checking against it */
+ cache = tp->recv_sack_cache + ARRAY_SIZE(tp->recv_sack_cache);
+ } else {
+ cache = tp->recv_sack_cache;
+ /* Skip empty blocks in at head of the cache */
+ while (tcp_sack_cache_ok(tp, cache) && !cache->start_seq &&
+ !cache->end_seq)
+ cache++;
+ }
- if (pcount > 1 && !in_sack &&
- after(TCP_SKB_CB(skb)->end_seq, start_seq)) {
- unsigned int pkt_len;
+ while (i < used_sacks) {
+ u32 start_seq = sp[i].start_seq;
+ u32 end_seq = sp[i].end_seq;
- in_sack = !after(start_seq,
- TCP_SKB_CB(skb)->seq);
+ /* Event "B" in the comment above. */
+ if (after(end_seq, tp->high_seq))
+ state.flag |= FLAG_DATA_LOST;
- if (!in_sack)
- pkt_len = (start_seq -
- TCP_SKB_CB(skb)->seq);
- else
- pkt_len = (end_seq -
- TCP_SKB_CB(skb)->seq);
- if (tcp_fragment(sk, skb, pkt_len, skb_shinfo(skb)->gso_size))
- break;
- pcount = tcp_skb_pcount(skb);
- }
+ /* Skip too early cached blocks */
+ while (tcp_sack_cache_ok(tp, cache) &&
+ !before(start_seq, cache->end_seq))
+ cache++;
- fack_count += pcount;
+ if (tcp_sack_cache_ok(tp, cache)) {
+ if (after(end_seq, cache->start_seq)) {
+ if (before(start_seq, cache->start_seq)) {
+ skb = tcp_sacktag_skip(skb, sk, &state, start_seq);
+ skb = tcp_sacktag_walk(skb, sk, &state, start_seq, cache->start_seq, 0);
+ }
+ /* Rest of the block already fully processed? */
+ if (!after(end_seq, cache->end_seq)) {
+ i++;
+ continue;
+ }
+ if (TCP_SKB_CB(tp->highest_sack)->end_seq != cache->end_seq) {
+ skb = tcp_sacktag_skip(skb, sk, &state, cache->end_seq);
+ cache++;
+ continue;
+ }
- tcp_sacktag_one(skb, sk, &state, in_sack,
- dup_sack, fack_count, end_seq);
+ skb = tcp_sacktag_skip_to_highsack(skb, sk, &state, cache);
+ }
+ } else if (!before(start_seq, tcp_highest_sack_seq(sk)) &&
+ before(TCP_SKB_CB(skb)->seq, tcp_highest_sack_seq(sk))) {
+ skb = tcp_write_queue_next(sk, tp->highest_sack);
+ state.fack_count = tp->fackets_out;
}
+
+ skb = tcp_sacktag_skip(skb, sk, &state, start_seq);
+ skb = tcp_sacktag_walk(skb, sk, &state, start_seq, end_seq, 0);
+ i++;
+ }
+
+ /* Clear the head of the cache sack blocks so we can skip it next time */
+ for (i = 0; i < ARRAY_SIZE(tp->recv_sack_cache) - used_sacks; i++) {
+ tp->recv_sack_cache[i].start_seq = 0;
+ tp->recv_sack_cache[i].end_seq = 0;
}
+ for (j = 0; j < used_sacks; j++)
+ tp->recv_sack_cache[i++] = sp[j];
/* Check for lost retransmit. This superb idea is
* borrowed from "ratehalving". Event "C".
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index fd51692..4cfda16 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -653,9 +653,7 @@ static void tcp_set_skb_tso_segs(struct sock *sk, struct sk_buff *skb, unsigned
}
/* When a modification to fackets out becomes necessary, we need to check
- * skb is counted to fackets_out or not. Another important thing is to
- * tweak SACK fastpath hint too as it would overwrite all changes unless
- * hint is also changed.
+ * skb is counted to fackets_out or not.
*/
static void tcp_adjust_fackets_out(struct sock *sk, struct sk_buff *skb,
int decr)
@@ -667,11 +665,6 @@ static void tcp_adjust_fackets_out(struct sock *sk, struct sk_buff *skb,
if (!before(tcp_highest_sack_seq(sk), TCP_SKB_CB(skb)->seq))
tp->fackets_out -= decr;
-
- /* cnt_hint is "off-by-one" compared with fackets_out (see sacktag) */
- if (tp->fastpath_skb_hint != NULL &&
- after(TCP_SKB_CB(tp->fastpath_skb_hint)->seq, TCP_SKB_CB(skb)->seq))
- tp->fastpath_cnt_hint -= decr;
}
/* Function to create two new TCP segments. Shrinks the given segment
@@ -1760,9 +1753,6 @@ static void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *skb, int m
/* changed transmit queue under us so clear hints */
tcp_clear_retrans_hints_partial(tp);
- /* manually tune sacktag skb hint */
- if (tp->fastpath_skb_hint == next_skb)
- tp->fastpath_skb_hint = skb;
sk_stream_free_skb(sk, next_skb);
}
--
1.5.0.6
^ permalink raw reply related
* Re: 2.6.23-rc6-mm1: Build failures on ppc64_defconfig
From: Mel Gorman @ 2007-09-24 11:12 UTC (permalink / raw)
To: Satyam Sharma
Cc: Andrew Morton, Linux Kernel Mailing List, linuxppc-dev,
Linux Netdev Mailing List, jeff
In-Reply-To: <alpine.LFD.0.999.0709221221500.22517@enigma.security.iitk.ac.in>
On (22/09/07 12:24), Satyam Sharma didst pronounce:
>
>
> On Thu, 20 Sep 2007, Satyam Sharma wrote:
> >
> > BTW ppc64_defconfig didn't quite like 2.6.23-rc6-mm1 either ...
> > IIRC I got build failures in:
>
> > drivers/net/spider_net.c
>
>
> [PATCH -mm] spider_net: Misc build fixes after recent netdev stats changes
>
> Unbreak the following:
>
> drivers/net/spider_net.c: In function 'spider_net_release_tx_chain':
> drivers/net/spider_net.c:818: error: 'dev' undeclared (first use in this function)
> drivers/net/spider_net.c:818: error: (Each undeclared identifier is reported only once
> drivers/net/spider_net.c:818: error: for each function it appears in.)
> drivers/net/spider_net.c: In function 'spider_net_xmit':
> drivers/net/spider_net.c:922: error: 'dev' undeclared (first use in this function)
> drivers/net/spider_net.c: In function 'spider_net_pass_skb_up':
> drivers/net/spider_net.c:1018: error: 'dev' undeclared (first use in this function)
> drivers/net/spider_net.c: In function 'spider_net_decode_one_descr':
> drivers/net/spider_net.c:1215: error: 'dev' undeclared (first use in this function)
> make[2]: *** [drivers/net/spider_net.o] Error 1
>
> Signed-off-by: Satyam Sharma <satyam@infradead.org>
I've confirmed that this patch fixes the build error in question.
Acked-by: Mel Gorman <mel@csn.ul.ie>
>
> ---
>
> drivers/net/spider_net.c | 24 +++++++++++-------------
> 1 file changed, 11 insertions(+), 13 deletions(-)
>
> diff -ruNp a/drivers/net/spider_net.c b/drivers/net/spider_net.c
> --- a/drivers/net/spider_net.c 2007-09-22 06:26:39.000000000 +0530
> +++ b/drivers/net/spider_net.c 2007-09-22 12:12:23.000000000 +0530
> @@ -795,6 +795,7 @@ spider_net_set_low_watermark(struct spid
> static int
> spider_net_release_tx_chain(struct spider_net_card *card, int brutal)
> {
> + struct net_device *dev = card->netdev;
> struct spider_net_descr_chain *chain = &card->tx_chain;
> struct spider_net_descr *descr;
> struct spider_net_hw_descr *hwdescr;
> @@ -919,7 +920,7 @@ spider_net_xmit(struct sk_buff *skb, str
> spider_net_release_tx_chain(card, 0);
>
> if (spider_net_prepare_tx_descr(card, skb) != 0) {
> - dev->stats.tx_dropped++;
> + netdev->stats.tx_dropped++;
> netif_stop_queue(netdev);
> return NETDEV_TX_BUSY;
> }
> @@ -979,16 +980,12 @@ static void
> spider_net_pass_skb_up(struct spider_net_descr *descr,
> struct spider_net_card *card)
> {
> - struct spider_net_hw_descr *hwdescr= descr->hwdescr;
> - struct sk_buff *skb;
> - struct net_device *netdev;
> - u32 data_status, data_error;
> -
> - data_status = hwdescr->data_status;
> - data_error = hwdescr->data_error;
> - netdev = card->netdev;
> + struct spider_net_hw_descr *hwdescr = descr->hwdescr;
> + struct sk_buff *skb = descr->skb;
> + struct net_device *netdev = card->netdev;
> + u32 data_status = hwdescr->data_status;
> + u32 data_error = hwdescr->data_error;
>
> - skb = descr->skb;
> skb_put(skb, hwdescr->valid_size);
>
> /* the card seems to add 2 bytes of junk in front
> @@ -1015,8 +1012,8 @@ spider_net_pass_skb_up(struct spider_net
> }
>
> /* update netdevice statistics */
> - dev->stats.rx_packets++;
> - dev->stats.rx_bytes += skb->len;
> + netdev->stats.rx_packets++;
> + netdev->stats.rx_bytes += skb->len;
>
> /* pass skb up to stack */
> netif_receive_skb(skb);
> @@ -1184,6 +1181,7 @@ static int spider_net_resync_tail_ptr(st
> static int
> spider_net_decode_one_descr(struct spider_net_card *card)
> {
> + struct net_device *dev = card->netdev;
> struct spider_net_descr_chain *chain = &card->rx_chain;
> struct spider_net_descr *descr = chain->tail;
> struct spider_net_hw_descr *hwdescr = descr->hwdescr;
> @@ -1210,7 +1208,7 @@ spider_net_decode_one_descr(struct spide
> (status == SPIDER_NET_DESCR_PROTECTION_ERROR) ||
> (status == SPIDER_NET_DESCR_FORCE_END) ) {
> if (netif_msg_rx_err(card))
> - dev_err(&card->netdev->dev,
> + dev_err(&dev->dev,
> "dropping RX descriptor with state %d\n", status);
> dev->stats.rx_dropped++;
> goto bad_desc;
>
--
--
Mel Gorman
Part-time Phd Student Linux Technology Center
University of Limerick IBM Dublin Software Lab
^ permalink raw reply
* Re: pktgen question
From: Steve Wise @ 2007-09-24 13:54 UTC (permalink / raw)
To: Ben Greear; +Cc: hadi, Evgeniy Polyakov, netdev, Robert Olsson
In-Reply-To: <46F75968.4010307@candelatech.com>
I think pktgen should be cloning the skbs using skb_clone(). Then it
will work for all devices, eh?
Ben Greear wrote:
> jamal wrote:
>> On Sun, 2007-23-09 at 12:55 -0500, Steve Wise wrote:
>>
>>
>>> Its a hack that breaks cxgb3 because cxgb3 uses the skb->cb area for
>>> each skb passed down. So cxgb3 is at fault then? IE a driver cannot
>>> use the skb->cb field if the users count is > 1? Or maybe a driver
>>> can _never_ use the cb field?
>>>
>>
>> any layer can use the cb structure whichever way they wish. There are
>> violations, e.g:
>> the vlan code also uses the cb field to pass vlan details for hardware
>> acceleration. How does pktgen affect it though, clone() will just copy
>> the cb field and pktgen doesnt touch it.
>> In retrospect, pktgen may have to use clone - ccing Robert Olsson.
>>
> Pktgen abuses the ref count, as far as I can tell. It works with most
> ethernet drivers,
> but that multi-pkt will also fail with things like vlans because the
> skb->dev is changed
> as it is transmitted (to the lower-level device).
>
> I'd say just don't use the multi-pkt with pktgen on devices that can't
> handle it.
>
> Thanks,
> Ben
>
>
^ permalink raw reply
* Re: [ofa-general] Re: [PATCH V5 2/11] IB/ipoib: Notify the world before doing unregister
From: Moni Shoua @ 2007-09-24 13:56 UTC (permalink / raw)
To: Roland Dreier; +Cc: netdev, Jay Vosburgh, OpenFabrics General
In-Reply-To: <adar6kpwdax.fsf@cisco.com>
Roland Dreier wrote:
> > The action in bonding to a detach of slave is to unregister the master (see patch 10).
> > This can't be done from the context of unregister_netdevice itself (it is protected by rtnl_lock).
>
> I'm confused. Your patch has:
>
> > + ipoib_slave_detach(cpriv->dev);
> > unregister_netdev(cpriv->dev);
>
> And ipoib_slave_detach() is:
>
> > +static inline void ipoib_slave_detach(struct net_device *dev)
> > +{
> > + rtnl_lock();
> > + netdev_slave_detach(dev);
> > + rtnl_unlock();
> > +}
>
> so you are calling netdev_slave_detach() with the rtnl lock held.
> Why can't you make the same call from the start of unregister_netdevice()?
>
> Anyway, if the rtnl lock is a problem, can you just add the call to
> netdev_slave_detach() to unregister_netdev() before it takes the rtnl lock?
>
> - R.
>
Your comment made me do a little rethinking.
In bonding, device is released by calling unregister_netdevice() that doesn't
take the rtnl_lock (unlike unregister_netdev() that does). I guess that this made me
confused to think that this is not possible. So, I guess I could put
the detach notification in unregister_netedev() and the reaction to the notification
in the bonding driver would not block.
However, I looked one more time at the code of unregister_netdevice() and found out that
nothing prevents from calling unregister_netdevice() again when the notification NETDEV_GOING_DOWN
is sent. I tried that and it works.
I have a new set of patches without sending a slave detach and I will send it soon.
Thanks for the comment Roland. It makes this patch simpler.
I'd also like to give a credit to Jay for the idea of using NETDEV_GOING_DOWN notification
instead of NETDEV_CHANGE+IFF_SLAVE_DETACH. He suggested it a while ago but I wrongly thought that
it wouldn't work.
^ permalink raw reply
* Re: pktgen question
From: Ben Greear @ 2007-09-24 14:39 UTC (permalink / raw)
To: Steve Wise; +Cc: hadi, Evgeniy Polyakov, netdev, Robert Olsson
In-Reply-To: <46F7C185.1030202@opengridcomputing.com>
Steve Wise wrote:
> I think pktgen should be cloning the skbs using skb_clone(). Then it
> will work for all devices, eh?
That might work, but it would decrease performance slightly (or,
increase CPU load at least).
Maybe a new option: multi_clone
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH] sb1250-mac: Driver model & phylib update
From: Maciej W. Rozycki @ 2007-09-24 14:41 UTC (permalink / raw)
To: Andrew Morton; +Cc: Jeff Garzik, netdev, linux-mips, linux-kernel
In-Reply-To: <20070921124409.7f3d122b.akpm@linux-foundation.org>
On Fri, 21 Sep 2007, Andrew Morton wrote:
> > A driver model and phylib update.
>
> akpm:/usr/src/25> diffstat patches/git-net.patch | tail -n 1
> 1013 files changed, 187667 insertions(+), 23587 deletions(-)
>
> Sorry, but raising networking patches against Linus's crufty
> old mainline tree just isn't viable at present.
Well, this is against Jeff's netdev-2.6 tree which hopefully is not as
crufty as Linus's old mainline; if it is not possible to queue this change
for 2.6.25 or suchlike, then I will try to resubmit later. Thanks for
your attention though.
Maciej
^ permalink raw reply
* Re: pktgen question
From: Steve Wise @ 2007-09-24 15:00 UTC (permalink / raw)
To: Ben Greear; +Cc: hadi, Evgeniy Polyakov, netdev, Robert Olsson
In-Reply-To: <46F7CC09.6070603@candelatech.com>
Ben Greear wrote:
> Steve Wise wrote:
>> I think pktgen should be cloning the skbs using skb_clone(). Then it
>> will work for all devices, eh?
> That might work, but it would decrease performance slightly (or,
> increase CPU load at least).
Perf-wise, you could clone the skbs up front, then deliver them to the
nic in a tight loop. This would mitigate the added overhead introduced
by calling skb_clone() in the loop doing transmits...
>
> Maybe a new option: multi_clone
>
If the current code is busted, I think it should be fixed.
My 2 cents.
Steve.
^ permalink raw reply
* [PATCH V6 0/9] net/bonding: ADD IPoIB support for the bonding driver
From: Moni Shoua @ 2007-09-24 15:27 UTC (permalink / raw)
To: Roland Dreier, Jay Vosburgh, netdev; +Cc: OpenFabrics General
This patch series is the sixth version (see below link to V5) of the
suggested changes to the bonding driver so it would be able to support
non ARPHRD_ETHER netdevices for its High-Availability (active-backup) mode.
Patches 1-8 were originally submitted in V5 and patch 9 is an addition by Jay.
Major changes from the previous version:
----------------------------------------
1. Remove the patches to net/core. Bonding will use the NETDEV_GOING_DOWN notification
instead of NETDEV_CHANGE+IFF_SLAVE_DETACH. This reduces the amount of patches from 11
to 9.
Links to earlier discussion:
----------------------------
1. A discussion in netdev about bonding support for IPoIB.
http://lists.openwall.net/netdev/2006/11/30/46
2. V5 series
http://lists.openfabrics.org/pipermail/general/2007-September/040996.html
^ permalink raw reply
* [ofa-general] [PATCH V6 1/9] IB/ipoib: Bound the net device to the ipoib_neigh structue
From: Moni Shoua @ 2007-09-24 15:29 UTC (permalink / raw)
To: Roland Dreier, Jay Vosburgh; +Cc: netdev, OpenFabrics General
In-Reply-To: <46F7D770.4090500@voltaire.com>
IPoIB uses a two layer neighboring scheme, such that for each struct neighbour
whose device is an ipoib one, there is a struct ipoib_neigh buddy which is
created on demand at the tx flow by an ipoib_neigh_alloc(skb->dst->neighbour)
call.
When using the bonding driver, neighbours are created by the net stack on behalf
of the bonding (master) device. On the tx flow the bonding code gets an skb such
that skb->dev points to the master device, it changes this skb to point on the
slave device and calls the slave hard_start_xmit function.
Under this scheme, ipoib_neigh_destructor assumption that for each struct
neighbour it gets, n->dev is an ipoib device and hence netdev_priv(n->dev)
can be casted to struct ipoib_dev_priv is buggy.
To fix it, this patch adds a dev field to struct ipoib_neigh which is used
instead of the struct neighbour dev one, when n->dev->flags has the
IFF_MASTER bit set.
Signed-off-by: Moni Shoua <monis at voltaire.com>
Signed-off-by: Or Gerlitz <ogerlitz at voltaire.com>
---
drivers/infiniband/ulp/ipoib/ipoib.h | 4 +++-
drivers/infiniband/ulp/ipoib/ipoib_main.c | 24 +++++++++++++++---------
drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 3 ++-
3 files changed, 20 insertions(+), 11 deletions(-)
Index: net-2.6/drivers/infiniband/ulp/ipoib/ipoib.h
===================================================================
--- net-2.6.orig/drivers/infiniband/ulp/ipoib/ipoib.h 2007-09-18 17:08:53.245849217 +0200
+++ net-2.6/drivers/infiniband/ulp/ipoib/ipoib.h 2007-09-18 17:09:26.534874404 +0200
@@ -328,6 +328,7 @@ struct ipoib_neigh {
struct sk_buff_head queue;
struct neighbour *neighbour;
+ struct net_device *dev;
struct list_head list;
};
@@ -344,7 +345,8 @@ static inline struct ipoib_neigh **to_ip
INFINIBAND_ALEN, sizeof(void *));
}
-struct ipoib_neigh *ipoib_neigh_alloc(struct neighbour *neigh);
+struct ipoib_neigh *ipoib_neigh_alloc(struct neighbour *neigh,
+ struct net_device *dev);
void ipoib_neigh_free(struct net_device *dev, struct ipoib_neigh *neigh);
extern struct workqueue_struct *ipoib_workqueue;
Index: net-2.6/drivers/infiniband/ulp/ipoib/ipoib_main.c
===================================================================
--- net-2.6.orig/drivers/infiniband/ulp/ipoib/ipoib_main.c 2007-09-18 17:08:53.245849217 +0200
+++ net-2.6/drivers/infiniband/ulp/ipoib/ipoib_main.c 2007-09-18 17:23:54.725744661 +0200
@@ -511,7 +511,7 @@ static void neigh_add_path(struct sk_buf
struct ipoib_path *path;
struct ipoib_neigh *neigh;
- neigh = ipoib_neigh_alloc(skb->dst->neighbour);
+ neigh = ipoib_neigh_alloc(skb->dst->neighbour, skb->dev);
if (!neigh) {
++priv->stats.tx_dropped;
dev_kfree_skb_any(skb);
@@ -830,6 +830,13 @@ static void ipoib_neigh_cleanup(struct n
unsigned long flags;
struct ipoib_ah *ah = NULL;
+ neigh = *to_ipoib_neigh(n);
+ if (neigh) {
+ priv = netdev_priv(neigh->dev);
+ ipoib_dbg(priv, "neigh_destructor for bonding device: %s\n",
+ n->dev->name);
+ } else
+ return;
ipoib_dbg(priv,
"neigh_cleanup for %06x " IPOIB_GID_FMT "\n",
IPOIB_QPN(n->ha),
@@ -837,13 +844,10 @@ static void ipoib_neigh_cleanup(struct n
spin_lock_irqsave(&priv->lock, flags);
- neigh = *to_ipoib_neigh(n);
- if (neigh) {
- if (neigh->ah)
- ah = neigh->ah;
- list_del(&neigh->list);
- ipoib_neigh_free(n->dev, neigh);
- }
+ if (neigh->ah)
+ ah = neigh->ah;
+ list_del(&neigh->list);
+ ipoib_neigh_free(n->dev, neigh);
spin_unlock_irqrestore(&priv->lock, flags);
@@ -851,7 +855,8 @@ static void ipoib_neigh_cleanup(struct n
ipoib_put_ah(ah);
}
-struct ipoib_neigh *ipoib_neigh_alloc(struct neighbour *neighbour)
+struct ipoib_neigh *ipoib_neigh_alloc(struct neighbour *neighbour,
+ struct net_device *dev)
{
struct ipoib_neigh *neigh;
@@ -860,6 +865,7 @@ struct ipoib_neigh *ipoib_neigh_alloc(st
return NULL;
neigh->neighbour = neighbour;
+ neigh->dev = dev;
*to_ipoib_neigh(neighbour) = neigh;
skb_queue_head_init(&neigh->queue);
ipoib_cm_set(neigh, NULL);
Index: net-2.6/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
===================================================================
--- net-2.6.orig/drivers/infiniband/ulp/ipoib/ipoib_multicast.c 2007-09-18 17:08:53.245849217 +0200
+++ net-2.6/drivers/infiniband/ulp/ipoib/ipoib_multicast.c 2007-09-18 17:09:26.536874045 +0200
@@ -727,7 +727,8 @@ out:
if (skb->dst &&
skb->dst->neighbour &&
!*to_ipoib_neigh(skb->dst->neighbour)) {
- struct ipoib_neigh *neigh = ipoib_neigh_alloc(skb->dst->neighbour);
+ struct ipoib_neigh *neigh = ipoib_neigh_alloc(skb->dst->neighbour,
+ skb->dev);
if (neigh) {
kref_get(&mcast->ah->ref);
^ permalink raw reply
* [ofa-general] [PATCH V6 2/9] IB/ipoib: Verify address handle validity on send
From: Moni Shoua @ 2007-09-24 15:30 UTC (permalink / raw)
To: Roland Dreier, Jay Vosburgh; +Cc: netdev, OpenFabrics General
In-Reply-To: <46F7D770.4090500@voltaire.com>
When the bonding device senses a carrier loss of its active slave it replaces
that slave with a new one. In between the times when the carrier of an IPoIB
device goes down and ipoib_neigh is destroyed, it is possible that the
bonding driver will send a packet on a new slave that uses an old ipoib_neigh.
This patch detects and prevents this from happenning.
Signed-off-by: Moni Shoua <monis at voltaire.com>
Signed-off-by: Or Gerlitz <ogerlitz at voltaire.com>
---
drivers/infiniband/ulp/ipoib/ipoib_main.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
Index: net-2.6/drivers/infiniband/ulp/ipoib/ipoib_main.c
===================================================================
--- net-2.6.orig/drivers/infiniband/ulp/ipoib/ipoib_main.c 2007-09-18 17:09:26.535874225 +0200
+++ net-2.6/drivers/infiniband/ulp/ipoib/ipoib_main.c 2007-09-18 17:10:22.375853147 +0200
@@ -686,9 +686,10 @@ static int ipoib_start_xmit(struct sk_bu
goto out;
}
} else if (neigh->ah) {
- if (unlikely(memcmp(&neigh->dgid.raw,
+ if (unlikely((memcmp(&neigh->dgid.raw,
skb->dst->neighbour->ha + 4,
- sizeof(union ib_gid)))) {
+ sizeof(union ib_gid))) ||
+ (neigh->dev != dev))) {
spin_lock(&priv->lock);
/*
* It's safe to call ipoib_put_ah() inside
^ permalink raw reply
* [ofa-general] [PATCH V6 3/9] net/bonding: Enable bonding to enslave non ARPHRD_ETHER
From: Moni Shoua @ 2007-09-24 15:32 UTC (permalink / raw)
To: Roland Dreier, Jay Vosburgh; +Cc: netdev, OpenFabrics General
In-Reply-To: <46F7D770.4090500@voltaire.com>
This patch changes some of the bond netdevice attributes and functions
to be that of the active slave for the case of the enslaved device not being
of ARPHRD_ETHER type. Basically it overrides those setting done by ether_setup(),
which are netdevice **type** dependent and hence might be not appropriate for
devices of other types. It also enforces mutual exclusion on bonding slaves
from dissimilar ether types, as was concluded over the v1 discussion.
IPoIB (see Documentation/infiniband/ipoib.txt) MAC address is made of a 3 bytes
IB QP (Queue Pair) number and 16 bytes IB port GID (Global ID) of the port this
IPoIB device is bounded to. The QP is a resource created by the IB HW and the
GID is an identifier burned into the HCA (i have omitted here some details which
are not important for the bonding RFC).
Signed-off-by: Moni Shoua <monis at voltaire.com>
Signed-off-by: Or Gerlitz <ogerlitz at voltaire.com>
---
drivers/net/bonding/bond_main.c | 39 +++++++++++++++++++++++++++++++++++++++
1 files changed, 39 insertions(+)
Index: net-2.6/drivers/net/bonding/bond_main.c
===================================================================
--- net-2.6.orig/drivers/net/bonding/bond_main.c 2007-08-15 10:08:59.000000000 +0300
+++ net-2.6/drivers/net/bonding/bond_main.c 2007-08-15 10:54:13.424688411 +0300
@@ -1237,6 +1237,26 @@ static int bond_compute_features(struct
return 0;
}
+
+static void bond_setup_by_slave(struct net_device *bond_dev,
+ struct net_device *slave_dev)
+{
+ bond_dev->hard_header = slave_dev->hard_header;
+ bond_dev->rebuild_header = slave_dev->rebuild_header;
+ bond_dev->hard_header_cache = slave_dev->hard_header_cache;
+ bond_dev->header_cache_update = slave_dev->header_cache_update;
+ bond_dev->hard_header_parse = slave_dev->hard_header_parse;
+
+ bond_dev->neigh_setup = slave_dev->neigh_setup;
+
+ bond_dev->type = slave_dev->type;
+ bond_dev->hard_header_len = slave_dev->hard_header_len;
+ bond_dev->addr_len = slave_dev->addr_len;
+
+ memcpy(bond_dev->broadcast, slave_dev->broadcast,
+ slave_dev->addr_len);
+}
+
/* enslave device <slave> to bond device <master> */
int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
{
@@ -1311,6 +1331,25 @@ int bond_enslave(struct net_device *bond
goto err_undo_flags;
}
+ /* set bonding device ether type by slave - bonding netdevices are
+ * created with ether_setup, so when the slave type is not ARPHRD_ETHER
+ * there is a need to override some of the type dependent attribs/funcs.
+ *
+ * bond ether type mutual exclusion - don't allow slaves of dissimilar
+ * ether type (eg ARPHRD_ETHER and ARPHRD_INFINIBAND) share the same bond
+ */
+ if (bond->slave_cnt == 0) {
+ if (slave_dev->type != ARPHRD_ETHER)
+ bond_setup_by_slave(bond_dev, slave_dev);
+ } else if (bond_dev->type != slave_dev->type) {
+ printk(KERN_ERR DRV_NAME ": %s ether type (%d) is different "
+ "from other slaves (%d), can not enslave it.\n",
+ slave_dev->name,
+ slave_dev->type, bond_dev->type);
+ res = -EINVAL;
+ goto err_undo_flags;
+ }
+
if (slave_dev->set_mac_address == NULL) {
printk(KERN_ERR DRV_NAME
": %s: Error: The slave device you specified does "
^ permalink raw reply
* [ofa-general] [PATCH V6 4/9] net/bonding: Enable bonding to enslave netdevices not supporting set_mac_address()
From: Moni Shoua @ 2007-09-24 15:36 UTC (permalink / raw)
To: Roland Dreier, Jay Vosburgh; +Cc: netdev, OpenFabrics General
In-Reply-To: <46F7D770.4090500@voltaire.com>
This patch allows for enslaving netdevices which do not support
the set_mac_address() function. In that case the bond mac address is the one
of the active slave, where remote peers are notified on the mac address
(neighbour) change by Gratuitous ARP sent by bonding when fail-over occurs
(this is already done by the bonding code).
Signed-off-by: Moni Shoua <monis at voltaire.com>
Signed-off-by: Or Gerlitz <ogerlitz at voltaire.com>
---
drivers/net/bonding/bond_main.c | 87 +++++++++++++++++++++++++++-------------
drivers/net/bonding/bonding.h | 1
2 files changed, 60 insertions(+), 28 deletions(-)
Index: net-2.6/drivers/net/bonding/bond_main.c
===================================================================
--- net-2.6.orig/drivers/net/bonding/bond_main.c 2007-08-15 10:54:13.000000000 +0300
+++ net-2.6/drivers/net/bonding/bond_main.c 2007-08-15 10:54:41.971632881 +0300
@@ -1095,6 +1095,14 @@ void bond_change_active_slave(struct bon
if (new_active) {
bond_set_slave_active_flags(new_active);
}
+
+ /* when bonding does not set the slave MAC address, the bond MAC
+ * address is the one of the active slave.
+ */
+ if (new_active && !bond->do_set_mac_addr)
+ memcpy(bond->dev->dev_addr, new_active->dev->dev_addr,
+ new_active->dev->addr_len);
+
bond_send_gratuitous_arp(bond);
}
}
@@ -1351,13 +1359,22 @@ int bond_enslave(struct net_device *bond
}
if (slave_dev->set_mac_address == NULL) {
- printk(KERN_ERR DRV_NAME
- ": %s: Error: The slave device you specified does "
- "not support setting the MAC address. "
- "Your kernel likely does not support slave "
- "devices.\n", bond_dev->name);
- res = -EOPNOTSUPP;
- goto err_undo_flags;
+ if (bond->slave_cnt == 0) {
+ printk(KERN_WARNING DRV_NAME
+ ": %s: Warning: The first slave device you "
+ "specified does not support setting the MAC "
+ "address. This bond MAC address would be that "
+ "of the active slave.\n", bond_dev->name);
+ bond->do_set_mac_addr = 0;
+ } else if (bond->do_set_mac_addr) {
+ printk(KERN_ERR DRV_NAME
+ ": %s: Error: The slave device you specified "
+ "does not support setting the MAC addres,."
+ "but this bond uses this practice. \n"
+ , bond_dev->name);
+ res = -EOPNOTSUPP;
+ goto err_undo_flags;
+ }
}
new_slave = kzalloc(sizeof(struct slave), GFP_KERNEL);
@@ -1378,16 +1395,18 @@ int bond_enslave(struct net_device *bond
*/
memcpy(new_slave->perm_hwaddr, slave_dev->dev_addr, ETH_ALEN);
- /*
- * Set slave to master's mac address. The application already
- * set the master's mac address to that of the first slave
- */
- memcpy(addr.sa_data, bond_dev->dev_addr, bond_dev->addr_len);
- addr.sa_family = slave_dev->type;
- res = dev_set_mac_address(slave_dev, &addr);
- if (res) {
- dprintk("Error %d calling set_mac_address\n", res);
- goto err_free;
+ if (bond->do_set_mac_addr) {
+ /*
+ * Set slave to master's mac address. The application already
+ * set the master's mac address to that of the first slave
+ */
+ memcpy(addr.sa_data, bond_dev->dev_addr, bond_dev->addr_len);
+ addr.sa_family = slave_dev->type;
+ res = dev_set_mac_address(slave_dev, &addr);
+ if (res) {
+ dprintk("Error %d calling set_mac_address\n", res);
+ goto err_free;
+ }
}
res = netdev_set_master(slave_dev, bond_dev);
@@ -1612,9 +1631,11 @@ err_close:
dev_close(slave_dev);
err_restore_mac:
- memcpy(addr.sa_data, new_slave->perm_hwaddr, ETH_ALEN);
- addr.sa_family = slave_dev->type;
- dev_set_mac_address(slave_dev, &addr);
+ if (bond->do_set_mac_addr) {
+ memcpy(addr.sa_data, new_slave->perm_hwaddr, ETH_ALEN);
+ addr.sa_family = slave_dev->type;
+ dev_set_mac_address(slave_dev, &addr);
+ }
err_free:
kfree(new_slave);
@@ -1792,10 +1813,12 @@ int bond_release(struct net_device *bond
/* close slave before restoring its mac address */
dev_close(slave_dev);
- /* restore original ("permanent") mac address */
- memcpy(addr.sa_data, slave->perm_hwaddr, ETH_ALEN);
- addr.sa_family = slave_dev->type;
- dev_set_mac_address(slave_dev, &addr);
+ if (bond->do_set_mac_addr) {
+ /* restore original ("permanent") mac address */
+ memcpy(addr.sa_data, slave->perm_hwaddr, ETH_ALEN);
+ addr.sa_family = slave_dev->type;
+ dev_set_mac_address(slave_dev, &addr);
+ }
slave_dev->priv_flags &= ~(IFF_MASTER_8023AD | IFF_MASTER_ALB |
IFF_SLAVE_INACTIVE | IFF_BONDING |
@@ -1882,10 +1905,12 @@ static int bond_release_all(struct net_d
/* close slave before restoring its mac address */
dev_close(slave_dev);
- /* restore original ("permanent") mac address*/
- memcpy(addr.sa_data, slave->perm_hwaddr, ETH_ALEN);
- addr.sa_family = slave_dev->type;
- dev_set_mac_address(slave_dev, &addr);
+ if (bond->do_set_mac_addr) {
+ /* restore original ("permanent") mac address*/
+ memcpy(addr.sa_data, slave->perm_hwaddr, ETH_ALEN);
+ addr.sa_family = slave_dev->type;
+ dev_set_mac_address(slave_dev, &addr);
+ }
slave_dev->priv_flags &= ~(IFF_MASTER_8023AD | IFF_MASTER_ALB |
IFF_SLAVE_INACTIVE);
@@ -3922,6 +3947,9 @@ static int bond_set_mac_address(struct n
dprintk("bond=%p, name=%s\n", bond, (bond_dev ? bond_dev->name : "None"));
+ if (!bond->do_set_mac_addr)
+ return -EOPNOTSUPP;
+
if (!is_valid_ether_addr(sa->sa_data)) {
return -EADDRNOTAVAIL;
}
@@ -4312,6 +4340,9 @@ static int bond_init(struct net_device *
bond_create_proc_entry(bond);
#endif
+ /* set do_set_mac_addr to true on startup */
+ bond->do_set_mac_addr = 1;
+
list_add_tail(&bond->bond_list, &bond_dev_list);
return 0;
Index: net-2.6/drivers/net/bonding/bonding.h
===================================================================
--- net-2.6.orig/drivers/net/bonding/bonding.h 2007-08-15 10:08:58.000000000 +0300
+++ net-2.6/drivers/net/bonding/bonding.h 2007-08-15 10:55:34.359354833 +0300
@@ -185,6 +185,7 @@ struct bonding {
struct timer_list mii_timer;
struct timer_list arp_timer;
s8 kill_timers;
+ s8 do_set_mac_addr;
struct net_device_stats stats;
#ifdef CONFIG_PROC_FS
struct proc_dir_entry *proc_entry;
^ permalink raw reply
* [ofa-general] [PATCH V6 5/9] net/bonding: Enable IP multicast for bonding IPoIB devices
From: Moni Shoua @ 2007-09-24 15:37 UTC (permalink / raw)
To: Roland Dreier, Jay Vosburgh; +Cc: netdev, OpenFabrics General
In-Reply-To: <46F7D770.4090500@voltaire.com>
Allow to enslave devices when the bonding device is not up. Over the discussion
held at the previous post this seemed to be the most clean way to go, where it
is not expected to cause instabilities.
Normally, the bonding driver is UP before any enslavement takes place.
Once a netdevice is UP, the network stack acts to have it join some multicast groups
(eg the all-hosts 224.0.0.1). Now, since ether_setup() have set the bonding device
type to be ARPHRD_ETHER and address len to be ETHER_ALEN, the net core code
computes a wrong multicast link address. This is b/c ip_eth_mc_map() is called
where for multicast joins taking place after the enslavement another ip_xxx_mc_map()
is called (eg ip_ib_mc_map() when the bond type is ARPHRD_INFINIBAND)
Signed-off-by: Moni Shoua <monis at voltaire.com>
Signed-off-by: Or Gerlitz <ogerlitz at voltaire.com>
---
drivers/net/bonding/bond_main.c | 5 +++--
drivers/net/bonding/bond_sysfs.c | 6 ++----
2 files changed, 5 insertions(+), 6 deletions(-)
Index: net-2.6/drivers/net/bonding/bond_main.c
===================================================================
--- net-2.6.orig/drivers/net/bonding/bond_main.c 2007-08-15 10:54:41.000000000 +0300
+++ net-2.6/drivers/net/bonding/bond_main.c 2007-08-15 10:55:48.431862446 +0300
@@ -1285,8 +1285,9 @@ int bond_enslave(struct net_device *bond
/* bond must be initialized by bond_open() before enslaving */
if (!(bond_dev->flags & IFF_UP)) {
- dprintk("Error, master_dev is not up\n");
- return -EPERM;
+ printk(KERN_WARNING DRV_NAME
+ " %s: master_dev is not up in bond_enslave\n",
+ bond_dev->name);
}
/* already enslaved */
Index: net-2.6/drivers/net/bonding/bond_sysfs.c
===================================================================
--- net-2.6.orig/drivers/net/bonding/bond_sysfs.c 2007-08-15 10:08:58.000000000 +0300
+++ net-2.6/drivers/net/bonding/bond_sysfs.c 2007-08-15 10:55:48.432862269 +0300
@@ -266,11 +266,9 @@ static ssize_t bonding_store_slaves(stru
/* Quick sanity check -- is the bond interface up? */
if (!(bond->dev->flags & IFF_UP)) {
- printk(KERN_ERR DRV_NAME
- ": %s: Unable to update slaves because interface is down.\n",
+ printk(KERN_WARNING DRV_NAME
+ ": %s: doing slave updates when interface is down.\n",
bond->dev->name);
- ret = -EPERM;
- goto out;
}
/* Note: We can't hold bond->lock here, as bond_create grabs it. */
^ permalink raw reply
* Re: pktgen question
From: Ben Greear @ 2007-09-24 15:37 UTC (permalink / raw)
To: Steve Wise; +Cc: hadi, Evgeniy Polyakov, netdev, Robert Olsson
In-Reply-To: <46F7D12A.7020607@opengridcomputing.com>
Steve Wise wrote:
>
> Ben Greear wrote:
>> Steve Wise wrote:
>>> I think pktgen should be cloning the skbs using skb_clone(). Then
>>> it will work for all devices, eh?
>> That might work, but it would decrease performance slightly (or,
>> increase CPU load at least).
>
> Perf-wise, you could clone the skbs up front, then deliver them to the
> nic in a tight loop. This would mitigate the added overhead
> introduced by calling skb_clone() in the loop doing transmits...
That only works if you are sending a small number of skbs. You can't
pre-clone several minutes worth of 10Gbe traffic
with any normal amount of RAM.
>>
>> Maybe a new option: multi_clone
>>
>
> If the current code is busted, I think it should be fixed.
Well, it works fine when used correctly :)
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* [PATCH V6 6/9] net/bonding: Handlle wrong assumptions that slave is always an Ethernet device
From: Moni Shoua @ 2007-09-24 15:40 UTC (permalink / raw)
To: Roland Dreier, Jay Vosburgh; +Cc: netdev, OpenFabrics General
In-Reply-To: <46F7D770.4090500@voltaire.com>
bonding sometimes uses Ethernet constants (such as MTU and address length) which
are not good when it enslaves non Ethernet devices (such as InfiniBand).
Signed-off-by: Moni Shoua <monis at voltaire.com>
---
drivers/net/bonding/bond_main.c | 3 ++-
drivers/net/bonding/bond_sysfs.c | 10 ++++++++--
drivers/net/bonding/bonding.h | 1 +
3 files changed, 11 insertions(+), 3 deletions(-)
Index: net-2.6/drivers/net/bonding/bond_main.c
===================================================================
--- net-2.6.orig/drivers/net/bonding/bond_main.c 2007-09-24 12:52:33.000000000 +0200
+++ net-2.6/drivers/net/bonding/bond_main.c 2007-09-24 12:57:33.411459811 +0200
@@ -1224,7 +1224,8 @@ static int bond_compute_features(struct
struct slave *slave;
struct net_device *bond_dev = bond->dev;
unsigned long features = bond_dev->features;
- unsigned short max_hard_header_len = ETH_HLEN;
+ unsigned short max_hard_header_len = max((u16)ETH_HLEN,
+ bond_dev->hard_header_len);
int i;
features &= ~(NETIF_F_ALL_CSUM | BOND_VLAN_FEATURES);
Index: net-2.6/drivers/net/bonding/bond_sysfs.c
===================================================================
--- net-2.6.orig/drivers/net/bonding/bond_sysfs.c 2007-09-24 12:55:09.000000000 +0200
+++ net-2.6/drivers/net/bonding/bond_sysfs.c 2007-09-24 13:00:23.752680721 +0200
@@ -260,6 +260,7 @@ static ssize_t bonding_store_slaves(stru
char command[IFNAMSIZ + 1] = { 0, };
char *ifname;
int i, res, found, ret = count;
+ u32 original_mtu;
struct slave *slave;
struct net_device *dev = NULL;
struct bonding *bond = to_bond(d);
@@ -325,6 +326,7 @@ static ssize_t bonding_store_slaves(stru
}
/* Set the slave's MTU to match the bond */
+ original_mtu = dev->mtu;
if (dev->mtu != bond->dev->mtu) {
if (dev->change_mtu) {
res = dev->change_mtu(dev,
@@ -339,6 +341,9 @@ static ssize_t bonding_store_slaves(stru
}
rtnl_lock();
res = bond_enslave(bond->dev, dev);
+ bond_for_each_slave(bond, slave, i)
+ if (strnicmp(slave->dev->name, ifname, IFNAMSIZ) == 0)
+ slave->original_mtu = original_mtu;
rtnl_unlock();
if (res) {
ret = res;
@@ -351,6 +356,7 @@ static ssize_t bonding_store_slaves(stru
bond_for_each_slave(bond, slave, i)
if (strnicmp(slave->dev->name, ifname, IFNAMSIZ) == 0) {
dev = slave->dev;
+ original_mtu = slave->original_mtu;
break;
}
if (dev) {
@@ -365,9 +371,9 @@ static ssize_t bonding_store_slaves(stru
}
/* set the slave MTU to the default */
if (dev->change_mtu) {
- dev->change_mtu(dev, 1500);
+ dev->change_mtu(dev, original_mtu);
} else {
- dev->mtu = 1500;
+ dev->mtu = original_mtu;
}
}
else {
Index: net-2.6/drivers/net/bonding/bonding.h
===================================================================
--- net-2.6.orig/drivers/net/bonding/bonding.h 2007-09-24 12:55:09.000000000 +0200
+++ net-2.6/drivers/net/bonding/bonding.h 2007-09-24 12:57:33.412459636 +0200
@@ -156,6 +156,7 @@ struct slave {
s8 link; /* one of BOND_LINK_XXXX */
s8 state; /* one of BOND_STATE_XXXX */
u32 original_flags;
+ u32 original_mtu;
u32 link_failure_count;
u16 speed;
u8 duplex;
^ permalink raw reply
* Re: pktgen question
From: Robert Olsson @ 2007-09-24 15:42 UTC (permalink / raw)
To: Steve Wise; +Cc: Ben Greear, hadi, Evgeniy Polyakov, netdev, Robert Olsson
In-Reply-To: <46F7C185.1030202@opengridcomputing.com>
Hi,
Steve Wise writes:
> I think pktgen should be cloning the skbs using skb_clone(). Then it
> will work for all devices, eh?
pktgen assumes for "fastpath" sending exclusive ownership of
the skb. And does a skb_get to avoid final skb destruction so
the same skb can be sent over and over. The idea is to avoid
memory allocation and keep things in cache to give very high
packet rates with identical packets.
I
But if you need to alter the packet then the skb_get trick can't
be done. And you have to turn off "fastpath" with clone_skb
> Perf-wise, you could clone the skbs up front, then deliver them to the
> nic in a tight loop. This would mitigate the added overhead introduced
> by calling skb_clone() in the loop doing transmits...
Sure it's can be done. It could replay sequences etc but it will not
beat the skb_get trick in sending identical packets. It has been
proposed before but I've avoided such efforts to keep things relatively
small and simple. Really pktgen should be reworked to have s small
skim in kernel and move the rest of the stuff to userland.
Cheers.
--ro
^ permalink raw reply
* [ofa-general] PATCH V6 7/9] net/bonding: Delay sending of gratuitous ARP to avoid failure
From: Moni Shoua @ 2007-09-24 15:46 UTC (permalink / raw)
To: Roland Dreier, Jay Vosburgh; +Cc: netdev, OpenFabrics General
In-Reply-To: <46F7D770.4090500@voltaire.com>
Delay sending a gratuitous_arp when LINK_STATE_LINKWATCH_PENDING bit
in dev->state field is on. This improves the chances for the arp packet to
be transmitted.
Signed-off-by: Moni Shoua <monis at voltaire.com>
---
drivers/net/bonding/bond_main.c | 24 +++++++++++++++++++++---
drivers/net/bonding/bonding.h | 1 +
2 files changed, 22 insertions(+), 3 deletions(-)
Index: net-2.6/drivers/net/bonding/bond_main.c
===================================================================
--- net-2.6.orig/drivers/net/bonding/bond_main.c 2007-08-15 10:56:33.000000000 +0300
+++ net-2.6/drivers/net/bonding/bond_main.c 2007-08-15 11:04:37.221123652 +0300
@@ -1102,8 +1102,14 @@ void bond_change_active_slave(struct bon
if (new_active && !bond->do_set_mac_addr)
memcpy(bond->dev->dev_addr, new_active->dev->dev_addr,
new_active->dev->addr_len);
-
- bond_send_gratuitous_arp(bond);
+ if (bond->curr_active_slave &&
+ test_bit(__LINK_STATE_LINKWATCH_PENDING,
+ &bond->curr_active_slave->dev->state)) {
+ dprintk("delaying gratuitous arp on %s\n",
+ bond->curr_active_slave->dev->name);
+ bond->send_grat_arp = 1;
+ } else
+ bond_send_gratuitous_arp(bond);
}
}
@@ -2083,6 +2089,17 @@ void bond_mii_monitor(struct net_device
* program could monitor the link itself if needed.
*/
+ if (bond->send_grat_arp) {
+ if (bond->curr_active_slave && test_bit(__LINK_STATE_LINKWATCH_PENDING,
+ &bond->curr_active_slave->dev->state))
+ dprintk("Needs to send gratuitous arp but not yet\n");
+ else {
+ dprintk("sending delayed gratuitous arp on on %s\n",
+ bond->curr_active_slave->dev->name);
+ bond_send_gratuitous_arp(bond);
+ bond->send_grat_arp = 0;
+ }
+ }
read_lock(&bond->curr_slave_lock);
oldcurrent = bond->curr_active_slave;
read_unlock(&bond->curr_slave_lock);
@@ -2484,7 +2501,7 @@ static void bond_send_gratuitous_arp(str
if (bond->master_ip) {
bond_arp_send(slave->dev, ARPOP_REPLY, bond->master_ip,
- bond->master_ip, 0);
+ bond->master_ip, 0);
}
list_for_each_entry(vlan, &bond->vlan_list, vlan_list) {
@@ -4293,6 +4310,7 @@ static int bond_init(struct net_device *
bond->current_arp_slave = NULL;
bond->primary_slave = NULL;
bond->dev = bond_dev;
+ bond->send_grat_arp = 0;
INIT_LIST_HEAD(&bond->vlan_list);
/* Initialize the device entry points */
Index: net-2.6/drivers/net/bonding/bonding.h
===================================================================
--- net-2.6.orig/drivers/net/bonding/bonding.h 2007-08-15 10:56:33.000000000 +0300
+++ net-2.6/drivers/net/bonding/bonding.h 2007-08-15 11:05:41.516451497 +0300
@@ -187,6 +187,7 @@ struct bonding {
struct timer_list arp_timer;
s8 kill_timers;
s8 do_set_mac_addr;
+ s8 send_grat_arp;
struct net_device_stats stats;
#ifdef CONFIG_PROC_FS
struct proc_dir_entry *proc_entry;
^ permalink raw reply
* [PATCH V6 8/9] net/bonding: Destroy bonding master when last slave is gone
From: Moni Shoua @ 2007-09-24 15:47 UTC (permalink / raw)
To: Roland Dreier, Jay Vosburgh; +Cc: netdev, OpenFabrics General
In-Reply-To: <46F7D770.4090500@voltaire.com>
When bonding enslaves non Ethernet devices it takes pointers to functions
in the module that owns the slaves. In this case it becomes unsafe
to keep the bonding master registered after last slave was unenslaved
because we don't know if the pointers are still valid. Destroying the bond when slave_cnt is zero
ensures that these functions be used anymore.
Signed-off-by: Moni Shoua <monis at voltaire.com>
---
drivers/net/bonding/bond_main.c | 37 +++++++++++++++++++++++++++++++++++++
drivers/net/bonding/bond_sysfs.c | 9 +++++----
drivers/net/bonding/bonding.h | 3 +++
3 files changed, 45 insertions(+), 4 deletions(-)
Index: net-2.6/drivers/net/bonding/bond_main.c
===================================================================
--- net-2.6.orig/drivers/net/bonding/bond_main.c 2007-09-24 14:01:24.055441842 +0200
+++ net-2.6/drivers/net/bonding/bond_main.c 2007-09-24 14:05:05.658979207 +0200
@@ -1256,6 +1256,7 @@ static int bond_compute_features(struct
static void bond_setup_by_slave(struct net_device *bond_dev,
struct net_device *slave_dev)
{
+ struct bonding *bond = bond_dev->priv;
bond_dev->hard_header = slave_dev->hard_header;
bond_dev->rebuild_header = slave_dev->rebuild_header;
bond_dev->hard_header_cache = slave_dev->hard_header_cache;
@@ -1270,6 +1271,7 @@ static void bond_setup_by_slave(struct n
memcpy(bond_dev->broadcast, slave_dev->broadcast,
slave_dev->addr_len);
+ bond->setup_by_slave = 1;
}
/* enslave device <slave> to bond device <master> */
@@ -1838,6 +1840,35 @@ int bond_release(struct net_device *bond
}
/*
+* Destroy a bonding device.
+* Must be under rtnl_lock when this function is called.
+*/
+void bond_destroy(struct bonding *bond)
+{
+ bond_deinit(bond->dev);
+ bond_destroy_sysfs_entry(bond);
+ unregister_netdevice(bond->dev);
+}
+
+/*
+* First release a slave and than destroy the bond if no more slaves iare left.
+* Must be under rtnl_lock when this function is called.
+*/
+int bond_release_and_destroy(struct net_device *bond_dev, struct net_device *slave_dev)
+{
+ struct bonding *bond = bond_dev->priv;
+ int ret;
+
+ ret = bond_release(bond_dev, slave_dev);
+ if ((ret == 0) && (bond->slave_cnt == 0)) {
+ printk(KERN_INFO DRV_NAME " %s: destroying bond %s.\n",
+ bond_dev->name);
+ bond_destroy(bond);
+ }
+ return ret;
+}
+
+/*
* This function releases all slaves.
*/
static int bond_release_all(struct net_device *bond_dev)
@@ -3337,6 +3368,11 @@ static int bond_slave_netdev_event(unsig
* ... Or is it this?
*/
break;
+ case NETDEV_GOING_DOWN:
+ dprintk("slave %s is going down\n", slave_dev->name);
+ if (bond->setup_by_slave)
+ bond_release_and_destroy(bond_dev, slave_dev);
+ break;
case NETDEV_CHANGEMTU:
/*
* TODO: Should slaves be allowed to
@@ -4311,6 +4347,7 @@ static int bond_init(struct net_device *
bond->primary_slave = NULL;
bond->dev = bond_dev;
bond->send_grat_arp = 0;
+ bond->setup_by_slave = 0;
INIT_LIST_HEAD(&bond->vlan_list);
/* Initialize the device entry points */
Index: net-2.6/drivers/net/bonding/bonding.h
===================================================================
--- net-2.6.orig/drivers/net/bonding/bonding.h 2007-09-24 14:01:24.055441842 +0200
+++ net-2.6/drivers/net/bonding/bonding.h 2007-09-24 14:01:24.627340013 +0200
@@ -188,6 +188,7 @@ struct bonding {
s8 kill_timers;
s8 do_set_mac_addr;
s8 send_grat_arp;
+ s8 setup_by_slave;
struct net_device_stats stats;
#ifdef CONFIG_PROC_FS
struct proc_dir_entry *proc_entry;
@@ -295,6 +296,8 @@ static inline void bond_unset_master_alb
struct vlan_entry *bond_next_vlan(struct bonding *bond, struct vlan_entry *curr);
int bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb, struct net_device *slave_dev);
int bond_create(char *name, struct bond_params *params, struct bonding **newbond);
+void bond_destroy(struct bonding *bond);
+int bond_release_and_destroy(struct net_device *bond_dev, struct net_device *slave_dev);
void bond_deinit(struct net_device *bond_dev);
int bond_create_sysfs(void);
void bond_destroy_sysfs(void);
Index: net-2.6/drivers/net/bonding/bond_sysfs.c
===================================================================
--- net-2.6.orig/drivers/net/bonding/bond_sysfs.c 2007-09-24 14:01:23.523536550 +0200
+++ net-2.6/drivers/net/bonding/bond_sysfs.c 2007-09-24 14:01:24.628339835 +0200
@@ -164,9 +164,7 @@ static ssize_t bonding_store_bonds(struc
printk(KERN_INFO DRV_NAME
": %s is being deleted...\n",
bond->dev->name);
- bond_deinit(bond->dev);
- bond_destroy_sysfs_entry(bond);
- unregister_netdevice(bond->dev);
+ bond_destroy(bond);
rtnl_unlock();
goto out;
}
@@ -363,7 +361,10 @@ static ssize_t bonding_store_slaves(stru
printk(KERN_INFO DRV_NAME ": %s: Removing slave %s\n",
bond->dev->name, dev->name);
rtnl_lock();
- res = bond_release(bond->dev, dev);
+ if (bond->setup_by_slave)
+ res = bond_release_and_destroy(bond->dev, dev);
+ else
+ res = bond_release(bond->dev, dev);
rtnl_unlock();
if (res) {
ret = res;
^ permalink raw reply
* [PATCH 9/9] bonding: Optionally allow ethernet slaves to keep own MAC
From: Moni Shoua @ 2007-09-24 15:49 UTC (permalink / raw)
To: Roland Dreier, Jay Vosburgh; +Cc: netdev, OpenFabrics General
In-Reply-To: <46F7D770.4090500@voltaire.com>
Update the "don't change MAC of slaves" functionality added in
previous changes to be a generic option, rather than something tied to IB
devices, as it's occasionally useful for regular ethernet devices as well.
Adds "fail_over_mac" option (which is automatically enabled for IB
slaves), applicable only to active-backup mode.
Includes documentation update.
Updates bonding driver version to 3.2.0.
Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
---
Documentation/networking/bonding.txt | 33 +++++++++++++++++++
drivers/net/bonding/bond_main.c | 57 +++++++++++++++++++++------------
drivers/net/bonding/bond_sysfs.c | 49 +++++++++++++++++++++++++++++
drivers/net/bonding/bonding.h | 6 ++--
4 files changed, 121 insertions(+), 24 deletions(-)
diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
index 1da5666..1134062 100644
--- a/Documentation/networking/bonding.txt
+++ b/Documentation/networking/bonding.txt
@@ -281,6 +281,39 @@ downdelay
will be rounded down to the nearest multiple. The default
value is 0.
+fail_over_mac
+
+ Specifies whether active-backup mode should set all slaves to
+ the same MAC address (the traditional behavior), or, when
+ enabled, change the bond's MAC address when changing the
+ active interface (i.e., fail over the MAC address itself).
+
+ Fail over MAC is useful for devices that cannot ever alter
+ their MAC address, or for devices that refuse incoming
+ broadcasts with their own source MAC (which interferes with
+ the ARP monitor).
+
+ The down side of fail over MAC is that every device on the
+ network must be updated via gratuitous ARP, vs. just updating
+ a switch or set of switches (which often takes place for any
+ traffic, not just ARP traffic, if the switch snoops incoming
+ traffic to update its tables) for the traditional method. If
+ the gratuitous ARP is lost, communication may be disrupted.
+
+ When fail over MAC is used in conjuction with the mii monitor,
+ devices which assert link up prior to being able to actually
+ transmit and receive are particularly susecptible to loss of
+ the gratuitous ARP, and an appropriate updelay setting may be
+ required.
+
+ A value of 0 disables fail over MAC, and is the default. A
+ value of 1 enables fail over MAC. This option is enabled
+ automatically if the first slave added cannot change its MAC
+ address. This option may be modified via sysfs only when no
+ slaves are present in the bond.
+
+ This option was added in bonding version 3.2.0.
+
lacp_rate
Option specifying the rate in which we'll ask our link partner
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 77caca3..c01ff9d 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -97,6 +97,7 @@ static char *xmit_hash_policy = NULL;
static int arp_interval = BOND_LINK_ARP_INTERV;
static char *arp_ip_target[BOND_MAX_ARP_TARGETS] = { NULL, };
static char *arp_validate = NULL;
+static int fail_over_mac = 0;
struct bond_params bonding_defaults;
module_param(max_bonds, int, 0);
@@ -130,6 +131,8 @@ module_param_array(arp_ip_target, charp, NULL, 0);
MODULE_PARM_DESC(arp_ip_target, "arp targets in n.n.n.n form");
module_param(arp_validate, charp, 0);
MODULE_PARM_DESC(arp_validate, "validate src/dst of ARP probes: none (default), active, backup or all");
+module_param(fail_over_mac, int, 0);
+MODULE_PARM_DESC(fail_over_mac, "For active-backup, do not set all slaves to the same MAC. 0 of off (default), 1 for on.");
/*----------------------------- Global variables ----------------------------*/
@@ -1099,7 +1102,7 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
/* when bonding does not set the slave MAC address, the bond MAC
* address is the one of the active slave.
*/
- if (new_active && !bond->do_set_mac_addr)
+ if (new_active && bond->params.fail_over_mac)
memcpy(bond->dev->dev_addr, new_active->dev->dev_addr,
new_active->dev->addr_len);
if (bond->curr_active_slave &&
@@ -1371,16 +1374,16 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
if (slave_dev->set_mac_address == NULL) {
if (bond->slave_cnt == 0) {
printk(KERN_WARNING DRV_NAME
- ": %s: Warning: The first slave device you "
- "specified does not support setting the MAC "
- "address. This bond MAC address would be that "
- "of the active slave.\n", bond_dev->name);
- bond->do_set_mac_addr = 0;
- } else if (bond->do_set_mac_addr) {
+ ": %s: Warning: The first slave device "
+ "specified does not support setting the MAC "
+ "address. Enabling the fail_over_mac option.",
+ bond_dev->name);
+ bond->params.fail_over_mac = 1;
+ } else if (!bond->params.fail_over_mac) {
printk(KERN_ERR DRV_NAME
- ": %s: Error: The slave device you specified "
- "does not support setting the MAC addres,."
- "but this bond uses this practice. \n"
+ ": %s: Error: The slave device specified "
+ "does not support setting the MAC address, "
+ "but fail_over_mac is not enabled.\n"
, bond_dev->name);
res = -EOPNOTSUPP;
goto err_undo_flags;
@@ -1405,7 +1408,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
*/
memcpy(new_slave->perm_hwaddr, slave_dev->dev_addr, ETH_ALEN);
- if (bond->do_set_mac_addr) {
+ if (!bond->params.fail_over_mac) {
/*
* Set slave to master's mac address. The application already
* set the master's mac address to that of the first slave
@@ -1641,7 +1644,7 @@ err_close:
dev_close(slave_dev);
err_restore_mac:
- if (bond->do_set_mac_addr) {
+ if (!bond->params.fail_over_mac) {
memcpy(addr.sa_data, new_slave->perm_hwaddr, ETH_ALEN);
addr.sa_family = slave_dev->type;
dev_set_mac_address(slave_dev, &addr);
@@ -1823,7 +1826,7 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
/* close slave before restoring its mac address */
dev_close(slave_dev);
- if (bond->do_set_mac_addr) {
+ if (!bond->params.fail_over_mac) {
/* restore original ("permanent") mac address */
memcpy(addr.sa_data, slave->perm_hwaddr, ETH_ALEN);
addr.sa_family = slave_dev->type;
@@ -1944,7 +1947,7 @@ static int bond_release_all(struct net_device *bond_dev)
/* close slave before restoring its mac address */
dev_close(slave_dev);
- if (bond->do_set_mac_addr) {
+ if (!bond->params.fail_over_mac) {
/* restore original ("permanent") mac address*/
memcpy(addr.sa_data, slave->perm_hwaddr, ETH_ALEN);
addr.sa_family = slave_dev->type;
@@ -3066,9 +3069,15 @@ static void bond_info_show_master(struct seq_file *seq)
curr = bond->curr_active_slave;
read_unlock(&bond->curr_slave_lock);
- seq_printf(seq, "Bonding Mode: %s\n",
+ seq_printf(seq, "Bonding Mode: %s",
bond_mode_name(bond->params.mode));
+ if (bond->params.mode == BOND_MODE_ACTIVEBACKUP &&
+ bond->params.fail_over_mac)
+ seq_printf(seq, " (fail_over_mac)");
+
+ seq_printf(seq, "\n");
+
if (bond->params.mode == BOND_MODE_XOR ||
bond->params.mode == BOND_MODE_8023AD) {
seq_printf(seq, "Transmit Hash Policy: %s (%d)\n",
@@ -4008,8 +4017,12 @@ static int bond_set_mac_address(struct net_device *bond_dev, void *addr)
dprintk("bond=%p, name=%s\n", bond, (bond_dev ? bond_dev->name : "None"));
- if (!bond->do_set_mac_addr)
- return -EOPNOTSUPP;
+ /*
+ * If fail_over_mac is enabled, do nothing and return success.
+ * Returning an error causes ifenslave to fail.
+ */
+ if (bond->params.fail_over_mac)
+ return 0;
if (!is_valid_ether_addr(sa->sa_data)) {
return -EADDRNOTAVAIL;
@@ -4402,10 +4415,6 @@ static int bond_init(struct net_device *bond_dev, struct bond_params *params)
#ifdef CONFIG_PROC_FS
bond_create_proc_entry(bond);
#endif
-
- /* set do_set_mac_addr to true on startup */
- bond->do_set_mac_addr = 1;
-
list_add_tail(&bond->bond_list, &bond_dev_list);
return 0;
@@ -4739,6 +4748,11 @@ static int bond_check_params(struct bond_params *params)
primary = NULL;
}
+ if (fail_over_mac && (bond_mode != BOND_MODE_ACTIVEBACKUP))
+ printk(KERN_WARNING DRV_NAME
+ ": Warning: fail_over_mac only affects "
+ "active-backup mode.\n");
+
/* fill params struct with the proper values */
params->mode = bond_mode;
params->xmit_policy = xmit_hashtype;
@@ -4750,6 +4764,7 @@ static int bond_check_params(struct bond_params *params)
params->use_carrier = use_carrier;
params->lacp_fast = lacp_fast;
params->primary[0] = 0;
+ params->fail_over_mac = fail_over_mac;
if (primary) {
strncpy(params->primary, primary, IFNAMSIZ);
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 71db5d9..a907b68 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -567,6 +567,54 @@ static ssize_t bonding_store_arp_validate(struct device *d,
static DEVICE_ATTR(arp_validate, S_IRUGO | S_IWUSR, bonding_show_arp_validate, bonding_store_arp_validate);
/*
+ * Show and store fail_over_mac. User only allowed to change the
+ * value when there are no slaves.
+ */
+static ssize_t bonding_show_fail_over_mac(struct device *d, struct device_attribute *attr, char *buf)
+{
+ struct bonding *bond = to_bond(d);
+
+ return sprintf(buf, "%d\n", bond->params.fail_over_mac) + 1;
+}
+
+static ssize_t bonding_store_fail_over_mac(struct device *d, struct device_attribute *attr, const char *buf, size_t count)
+{
+ int new_value;
+ int ret = count;
+ struct bonding *bond = to_bond(d);
+
+ if (bond->slave_cnt != 0) {
+ printk(KERN_ERR DRV_NAME
+ ": %s: Can't alter fail_over_mac with slaves in bond.\n",
+ bond->dev->name);
+ ret = -EPERM;
+ goto out;
+ }
+
+ if (sscanf(buf, "%d", &new_value) != 1) {
+ printk(KERN_ERR DRV_NAME
+ ": %s: no fail_over_mac value specified.\n",
+ bond->dev->name);
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if ((new_value == 0) || (new_value == 1)) {
+ bond->params.fail_over_mac = new_value;
+ printk(KERN_INFO DRV_NAME ": %s: Setting fail_over_mac to %d.\n",
+ bond->dev->name, new_value);
+ } else {
+ printk(KERN_INFO DRV_NAME
+ ": %s: Ignoring invalid fail_over_mac value %d.\n",
+ bond->dev->name, new_value);
+ }
+out:
+ return ret;
+}
+
+static DEVICE_ATTR(fail_over_mac, S_IRUGO | S_IWUSR, bonding_show_fail_over_mac, bonding_store_fail_over_mac);
+
+/*
* Show and set the arp timer interval. There are two tricky bits
* here. First, if ARP monitoring is activated, then we must disable
* MII monitoring. Second, if the ARP timer isn't running, we must
@@ -1390,6 +1438,7 @@ static DEVICE_ATTR(ad_partner_mac, S_IRUGO, bonding_show_ad_partner_mac, NULL);
static struct attribute *per_bond_attrs[] = {
&dev_attr_slaves.attr,
&dev_attr_mode.attr,
+ &dev_attr_fail_over_mac.attr,
&dev_attr_arp_validate.attr,
&dev_attr_arp_interval.attr,
&dev_attr_arp_ip_target.attr,
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index ed0f587..9d6153e 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -22,8 +22,8 @@
#include "bond_3ad.h"
#include "bond_alb.h"
-#define DRV_VERSION "3.1.3"
-#define DRV_RELDATE "June 13, 2007"
+#define DRV_VERSION "3.2.0"
+#define DRV_RELDATE "September 13, 2007"
#define DRV_NAME "bonding"
#define DRV_DESCRIPTION "Ethernet Channel Bonding Driver"
@@ -128,6 +128,7 @@ struct bond_params {
int arp_interval;
int arp_validate;
int use_carrier;
+ int fail_over_mac;
int updelay;
int downdelay;
int lacp_fast;
@@ -186,7 +187,6 @@ struct bonding {
struct timer_list mii_timer;
struct timer_list arp_timer;
s8 kill_timers;
- s8 do_set_mac_addr;
s8 send_grat_arp;
s8 setup_by_slave;
struct net_device_stats stats;
-- 1.5.2-rc2.GIT
^ permalink raw reply related
* Re: [PATCH V6 5/9] net/bonding: Enable IP multicast for bonding IPoIB devices
From: Stephen Hemminger @ 2007-09-24 16:04 UTC (permalink / raw)
To: Moni Shoua; +Cc: Roland Dreier, Jay Vosburgh, netdev, OpenFabrics General
In-Reply-To: <46F7D99C.3030602@voltaire.com>
On Mon, 24 Sep 2007 17:37:00 +0200
Moni Shoua <monis@voltaire.com> wrote:
> Allow to enslave devices when the bonding device is not up. Over the discussion
> held at the previous post this seemed to be the most clean way to go, where it
> is not expected to cause instabilities.
>
> Normally, the bonding driver is UP before any enslavement takes place.
> Once a netdevice is UP, the network stack acts to have it join some multicast groups
> (eg the all-hosts 224.0.0.1). Now, since ether_setup() have set the bonding device
> type to be ARPHRD_ETHER and address len to be ETHER_ALEN, the net core code
> computes a wrong multicast link address. This is b/c ip_eth_mc_map() is called
> where for multicast joins taking place after the enslavement another ip_xxx_mc_map()
> is called (eg ip_ib_mc_map() when the bond type is ARPHRD_INFINIBAND)
>
> Signed-off-by: Moni Shoua <monis at voltaire.com>
> Signed-off-by: Or Gerlitz <ogerlitz at voltaire.com>
> ---
> drivers/net/bonding/bond_main.c | 5 +++--
> drivers/net/bonding/bond_sysfs.c | 6 ++----
> 2 files changed, 5 insertions(+), 6 deletions(-)
>
> Index: net-2.6/drivers/net/bonding/bond_main.c
> ===================================================================
> --- net-2.6.orig/drivers/net/bonding/bond_main.c 2007-08-15 10:54:41.000000000 +0300
> +++ net-2.6/drivers/net/bonding/bond_main.c 2007-08-15 10:55:48.431862446 +0300
> @@ -1285,8 +1285,9 @@ int bond_enslave(struct net_device *bond
>
> /* bond must be initialized by bond_open() before enslaving */
> if (!(bond_dev->flags & IFF_UP)) {
> - dprintk("Error, master_dev is not up\n");
> - return -EPERM;
> + printk(KERN_WARNING DRV_NAME
> + " %s: master_dev is not up in bond_enslave\n",
> + bond_dev->name);
> }
>
> /* already enslaved */
> Index: net-2.6/drivers/net/bonding/bond_sysfs.c
> ===================================================================
> --- net-2.6.orig/drivers/net/bonding/bond_sysfs.c 2007-08-15 10:08:58.000000000 +0300
> +++ net-2.6/drivers/net/bonding/bond_sysfs.c 2007-08-15 10:55:48.432862269 +0300
> @@ -266,11 +266,9 @@ static ssize_t bonding_store_slaves(stru
>
> /* Quick sanity check -- is the bond interface up? */
> if (!(bond->dev->flags & IFF_UP)) {
> - printk(KERN_ERR DRV_NAME
> - ": %s: Unable to update slaves because interface is down.\n",
> + printk(KERN_WARNING DRV_NAME
> + ": %s: doing slave updates when interface is down.\n",
> bond->dev->name);
> - ret = -EPERM;
> - goto out;
> }
>
Please get rid of the warning. Make bonding work correctly and allow enslave/remove
of device when bonding is down.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox