* [PATCH 4/7] [TCP]: Extract tcp_match_queue_to_sack from sacktag code
From: Ilpo Järvinen @ 2007-10-11 11:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev, TAKANO Ryousei
In-Reply-To: <11921028673528-git-send-email-ilpo.jarvinen@helsinki.fi>
This is necessary for upcoming DSACK bugfix. Reduces sacktag
length which is not very sad thing at all... :-)
Notice that there's a need to handle out-of-mem at caller's
place.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
net/ipv4/tcp_input.c | 54 ++++++++++++++++++++++++++++++++-----------------
1 files changed, 35 insertions(+), 19 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 5004704..bd18c25 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1181,6 +1181,38 @@ static int tcp_check_dsack(struct tcp_sock *tp, struct sk_buff *ack_skb,
return dup_sack;
}
+/* Check if skb is fully within the SACK block. In presence of GSO skbs,
+ * the incoming SACK may not exactly match but we can find smaller MSS
+ * aligned portion of it that matches. Therefore we might need to fragment
+ * which may fail and creates some hassle (caller must handle error case
+ * returns).
+ */
+int tcp_match_skb_to_sack(struct sock *sk, struct sk_buff *skb,
+ u32 start_seq, u32 end_seq)
+{
+ int in_sack, err;
+ unsigned int pkt_len;
+
+ in_sack = !after(start_seq, TCP_SKB_CB(skb)->seq) &&
+ !before(end_seq, TCP_SKB_CB(skb)->end_seq);
+
+ if (tcp_skb_pcount(skb) > 1 && !in_sack &&
+ after(TCP_SKB_CB(skb)->end_seq, start_seq)) {
+
+ 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;
+ err = tcp_fragment(sk, skb, pkt_len, skb_shinfo(skb)->gso_size);
+ if (err < 0)
+ return err;
+ }
+
+ return in_sack;
+}
+
static int
tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_una)
{
@@ -1333,25 +1365,9 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
if (!before(TCP_SKB_CB(skb)->seq, end_seq))
break;
- in_sack = !after(start_seq, TCP_SKB_CB(skb)->seq) &&
- !before(end_seq, TCP_SKB_CB(skb)->end_seq);
-
- if (tcp_skb_pcount(skb) > 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;
- }
+ in_sack = tcp_match_skb_to_sack(sk, skb, start_seq, end_seq);
+ if (in_sack < 0)
+ break;
fack_count += tcp_skb_pcount(skb);
--
1.5.0.6
^ permalink raw reply related
* [PATCH 1/7] [TCP]: Add bytes_acked (ABC) clearing to FRTO too
From: Ilpo Järvinen @ 2007-10-11 11:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev, TAKANO Ryousei
In-Reply-To: <1192102867479-git-send-email-ilpo.jarvinen@helsinki.fi>
I was reading tcp_enter_loss while looking for Cedric's bug and
noticed bytes_acked adjustment is missing from FRTO side.
Since bytes_acked will only be used in tcp_cong_avoid, I think
it's safe to assume RTO would be spurious. During FRTO cwnd
will be not controlled by tcp_cong_avoid and if FRTO calls for
conventional recovery, cwnd is adjusted and the result of wrong
assumption is cleared from bytes_acked. If RTO was in fact
spurious, we did normal ABC already and can continue without
any additional adjustments.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
net/ipv4/tcp_input.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 101054c..e40857e 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1687,6 +1687,7 @@ static void tcp_enter_frto_loss(struct sock *sk, int allowed_segments, int flag)
tp->snd_cwnd_cnt = 0;
tp->snd_cwnd_stamp = tcp_time_stamp;
tp->frto_counter = 0;
+ tp->bytes_acked = 0;
tp->reordering = min_t(unsigned int, tp->reordering,
sysctl_tcp_reordering);
@@ -2824,6 +2825,7 @@ static void tcp_conservative_spur_to_response(struct tcp_sock *tp)
{
tp->snd_cwnd = min(tp->snd_cwnd, tp->snd_ssthresh);
tp->snd_cwnd_cnt = 0;
+ tp->bytes_acked = 0;
TCP_ECN_queue_cwr(tp);
tcp_moderate_cwnd(tp);
}
--
1.5.0.6
^ permalink raw reply related
* [PATCH net-2.6 0/7] TCP: small changes/fixes + lost_retrans brokeness fix
From: Ilpo Järvinen @ 2007-10-11 11:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev, TAKANO Ryousei
Some small and simple changes. Minor testing done, until I got
one LostRetrans counted, which turned out to be hard task than
I thought (without specifically arranged scenario, just some
basic netem delay & drops).
Lost_retrans handling fix is now tested, I fixed one incorrectly
placed if from the lost_retrans_low patch after reading it
through.
...Recount removal patch could be more elegant in the way it
plays around with the tcp_clear_retrans, but I just wasn't able
to figure out how to do that nicely enough...
Those cleanups are preparation for DSACK fix, which is yet to
be done but IMHO they won't hurt if applied alone either.
Dave, please apply at will. FYI, I'll post also the SACK rewrite
patchset rebased after this (built on top of these).
--
i.
^ permalink raw reply
* [PATCH 2/7] [TCP]: Fix mark_head_lost to ignore R-bit when trying to mark L
From: Ilpo Järvinen @ 2007-10-11 11:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev, TAKANO Ryousei
In-Reply-To: <11921028673599-git-send-email-ilpo.jarvinen@helsinki.fi>
This condition (plain R) can arise at least in recovery that
is triggered after tcp_undo_loss. There isn't any reason why
they should not be marked as lost, not marking makes in_flight
estimator to return too large values.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
net/ipv4/tcp_input.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index e40857e..c827285 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1987,7 +1987,7 @@ static void tcp_mark_head_lost(struct sock *sk,
cnt += tcp_skb_pcount(skb);
if (cnt > packets || after(TCP_SKB_CB(skb)->end_seq, high_seq))
break;
- if (!(TCP_SKB_CB(skb)->sacked&TCPCB_TAGBITS)) {
+ if (!(TCP_SKB_CB(skb)->sacked & (TCPCB_SACKED_ACKED|TCPCB_LOST))) {
TCP_SKB_CB(skb)->sacked |= TCPCB_LOST;
tp->lost_out += tcp_skb_pcount(skb);
tcp_verify_retransmit_hint(tp, skb);
--
1.5.0.6
^ permalink raw reply related
* [PATCH 3/7] [TCP]: Kill almost unused variable pcount from sacktag
From: Ilpo Järvinen @ 2007-10-11 11:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev, TAKANO Ryousei
In-Reply-To: <1192102867725-git-send-email-ilpo.jarvinen@helsinki.fi>
It's on the way for future cutting of that function.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
net/ipv4/tcp_input.c | 9 +++------
1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index c827285..5004704 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1314,7 +1314,7 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
flag |= FLAG_DATA_LOST;
tcp_for_write_queue_from(skb, sk) {
- int in_sack, pcount;
+ int in_sack;
u8 sacked;
if (skb == tcp_send_head(sk))
@@ -1336,9 +1336,7 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
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 &&
+ if (tcp_skb_pcount(skb) > 1 && !in_sack &&
after(TCP_SKB_CB(skb)->end_seq, start_seq)) {
unsigned int pkt_len;
@@ -1353,10 +1351,9 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
TCP_SKB_CB(skb)->seq);
if (tcp_fragment(sk, skb, pkt_len, skb_shinfo(skb)->gso_size))
break;
- pcount = tcp_skb_pcount(skb);
}
- fack_count += pcount;
+ fack_count += tcp_skb_pcount(skb);
sacked = TCP_SKB_CB(skb)->sacked;
--
1.5.0.6
^ permalink raw reply related
* [PATCH 6/7] [TCP]: Fix lost_retrans loop vs fastpath problems
From: Ilpo Järvinen @ 2007-10-11 11:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev, TAKANO Ryousei
In-Reply-To: <11921028672853-git-send-email-ilpo.jarvinen@helsinki.fi>
Detection implemented with lost_retrans must work also when
fastpath is taken, yet most of the queue is skipped including
(very likely) those retransmitted skb's we're interested in.
This problem appeared when the hints got added, which removed
a need to always walk over the whole write queue head.
Therefore decicion for the lost_retrans worker loop entry must
be separated from the sacktag processing more than it was
necessary before.
It turns out to be problematic to optimize the worker loop
very heavily because ack_seqs of skb may have a number of
discontinuity points. Maybe similar approach as currently is
implemented could be attempted but that's becoming more and
more complex because the trend is towards less skb walking
in sacktag marker. Trying a simple work until all rexmitted
skbs heve been processed approach.
Maybe after(highest_sack_end_seq, tp->high_seq) checking is not
sufficiently accurate and causes entry too often in no-work-to-do
cases. Since that's not known, I've separated solution to that
from this patch.
Noticed because of report against a related problem from TAKANO
Ryousei <takano@axe-inc.co.jp>. He also provided a patch to
that part of the problem. This patch includes solution to it
(though this patch has to use somewhat different placement).
TAKANO's description and patch is available here:
http://marc.info/?l=linux-netdev&m=119149311913288&w=2
...In short, TAKANO's problem is that end_seq the loop is using
not necessarily the largest SACK block's end_seq because the
current ACK may still have higher SACK blocks which are later
by the loop.
Cc: TAKANO Ryousei <takano@axe-inc.co.jp>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
net/ipv4/tcp_input.c | 37 ++++++++++++++++++++++---------------
1 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index a066039..d5e0fcc 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1109,27 +1109,34 @@ static int tcp_is_sackblock_valid(struct tcp_sock *tp, int is_dsack,
/* Check for lost retransmit. This superb idea is borrowed from "ratehalving".
* Event "C". Later note: FACK people cheated me again 8), we have to account
* for reordering! Ugly, but should help.
+ *
+ * Search retransmitted skbs from write_queue that were sent when snd_nxt was
+ * less than what is now known to be received by the other end (derived from
+ * SACK blocks by the caller).
*/
-static int tcp_mark_lost_retrans(struct sock *sk, u32 lost_retrans)
+static int tcp_mark_lost_retrans(struct sock *sk, u32 received_upto)
{
struct tcp_sock *tp = tcp_sk(sk);
struct sk_buff *skb;
int flag = 0;
+ int cnt = 0;
tcp_for_write_queue(skb, sk) {
u32 ack_seq = TCP_SKB_CB(skb)->ack_seq;
if (skb == tcp_send_head(sk))
break;
- if (after(TCP_SKB_CB(skb)->seq, lost_retrans))
+ if (cnt == tp->retrans_out)
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, ack_seq) &&
+ if (!(TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_RETRANS))
+ continue;
+
+ if (after(received_upto, ack_seq) &&
(tcp_is_fack(tp) ||
- !before(lost_retrans,
+ !before(received_upto,
ack_seq + tp->reordering * tp->mss_cache))) {
TCP_SKB_CB(skb)->sacked &= ~TCPCB_SACKED_RETRANS;
tp->retrans_out -= tcp_skb_pcount(skb);
@@ -1143,6 +1150,8 @@ static int tcp_mark_lost_retrans(struct sock *sk, u32 lost_retrans)
flag |= FLAG_DATA_SACKED;
NET_INC_STATS_BH(LINUX_MIB_TCPLOSTRETRANSMIT);
}
+ } else {
+ cnt += tcp_skb_pcount(skb);
}
}
return flag;
@@ -1225,7 +1234,7 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
int num_sacks = (ptr[1] - TCPOLEN_SACK_BASE)>>3;
int reord = tp->packets_out;
int prior_fackets;
- u32 lost_retrans = 0;
+ u32 highest_sack_end_seq = 0;
int flag = 0;
int found_dup_sack = 0;
int cached_fack_count;
@@ -1396,11 +1405,6 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
continue;
}
- 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;
-
if (!in_sack)
continue;
@@ -1454,9 +1458,10 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
if (fack_count > tp->fackets_out)
tp->fackets_out = fack_count;
- if (after(TCP_SKB_CB(skb)->seq,
- tp->highest_sack))
+ if (after(TCP_SKB_CB(skb)->seq, tp->highest_sack)) {
tp->highest_sack = TCP_SKB_CB(skb)->seq;
+ highest_sack_end_seq = TCP_SKB_CB(skb)->end_seq;
+ }
} else {
if (dup_sack && (sacked&TCPCB_RETRANS))
reord = min(fack_count, reord);
@@ -1476,8 +1481,10 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
}
}
- if (lost_retrans && icsk->icsk_ca_state == TCP_CA_Recovery)
- flag |= tcp_mark_lost_retrans(sk, lost_retrans);
+ if (tp->retrans_out && highest_sack_end_seq &&
+ after(highest_sack_end_seq, tp->high_seq) &&
+ icsk->icsk_ca_state == TCP_CA_Recovery)
+ flag |= tcp_mark_lost_retrans(sk, highest_sack_end_seq);
tcp_verify_left_out(tp);
--
1.5.0.6
^ permalink raw reply related
* [PATCH 5/7] [TCP]: No need to re-count fackets_out/sacked_out at RTO
From: Ilpo Järvinen @ 2007-10-11 11:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev, TAKANO Ryousei
In-Reply-To: <11921028673030-git-send-email-ilpo.jarvinen@helsinki.fi>
Both sacked_out and fackets_out are directly known from how
parameter. Since fackets_out is accurate, there's no need for
recounting (sacked_out was previously unnecessarily counted
in the loop anyway).
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
net/ipv4/tcp_input.c | 26 ++++++++++++++++----------
1 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index bd18c25..a066039 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1711,18 +1711,23 @@ static void tcp_enter_frto_loss(struct sock *sk, int allowed_segments, int flag)
tcp_clear_retrans_hints_partial(tp);
}
-void tcp_clear_retrans(struct tcp_sock *tp)
+static void tcp_clear_retrans_partial(struct tcp_sock *tp)
{
tp->retrans_out = 0;
-
- tp->fackets_out = 0;
- tp->sacked_out = 0;
tp->lost_out = 0;
tp->undo_marker = 0;
tp->undo_retrans = 0;
}
+void tcp_clear_retrans(struct tcp_sock *tp)
+{
+ tcp_clear_retrans_partial(tp);
+
+ tp->fackets_out = 0;
+ tp->sacked_out = 0;
+}
+
/* Enter Loss state. If "how" is not zero, forget all SACK information
* and reset tags completely, otherwise preserve SACKs. If receiver
* dropped its ofo queue, we will know this due to reneging detection.
@@ -1732,7 +1737,6 @@ void tcp_enter_loss(struct sock *sk, int how)
const struct inet_connection_sock *icsk = inet_csk(sk);
struct tcp_sock *tp = tcp_sk(sk);
struct sk_buff *skb;
- int cnt = 0;
/* Reduce ssthresh if it has not yet been made inside this window. */
if (icsk->icsk_ca_state <= TCP_CA_Disorder || tp->snd_una == tp->high_seq ||
@@ -1746,7 +1750,10 @@ void tcp_enter_loss(struct sock *sk, int how)
tp->snd_cwnd_stamp = tcp_time_stamp;
tp->bytes_acked = 0;
- tcp_clear_retrans(tp);
+ tcp_clear_retrans_partial(tp);
+
+ if (tcp_is_reno(tp))
+ tcp_reset_reno_sack(tp);
if (!how) {
/* Push undo marker, if it was plain RTO and nothing
@@ -1754,13 +1761,15 @@ void tcp_enter_loss(struct sock *sk, int how)
tp->undo_marker = tp->snd_una;
tcp_clear_retrans_hints_partial(tp);
} else {
+ tp->sacked_out = 0;
+ tp->fackets_out = 0;
tcp_clear_all_retrans_hints(tp);
}
tcp_for_write_queue(skb, sk) {
if (skb == tcp_send_head(sk))
break;
- cnt += tcp_skb_pcount(skb);
+
if (TCP_SKB_CB(skb)->sacked&TCPCB_RETRANS)
tp->undo_marker = 0;
TCP_SKB_CB(skb)->sacked &= (~TCPCB_TAGBITS)|TCPCB_SACKED_ACKED;
@@ -1768,9 +1777,6 @@ void tcp_enter_loss(struct sock *sk, int how)
TCP_SKB_CB(skb)->sacked &= ~TCPCB_SACKED_ACKED;
TCP_SKB_CB(skb)->sacked |= TCPCB_LOST;
tp->lost_out += tcp_skb_pcount(skb);
- } else {
- tp->sacked_out += tcp_skb_pcount(skb);
- tp->fackets_out = cnt;
}
}
tcp_verify_left_out(tp);
--
1.5.0.6
^ permalink raw reply related
* [PATCH 7/7] [TCP]: Limit processing lost_retrans loop to work-to-do cases
From: Ilpo Järvinen @ 2007-10-11 11:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev, TAKANO Ryousei
In-Reply-To: <11921028671668-git-send-email-ilpo.jarvinen@helsinki.fi>
This addition of lost_retrans_low to tcp_sock might be
unnecessary, it's not clear how often lost_retrans worker is
executed when there wasn't work to do.
Cc: TAKANO Ryousei <takano@axe-inc.co.jp>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
include/linux/tcp.h | 2 ++
net/ipv4/tcp_input.c | 14 +++++++++++---
net/ipv4/tcp_output.c | 2 ++
3 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 9ff456e..c5b94c1 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -348,6 +348,8 @@ struct tcp_sock {
int lost_cnt_hint;
int retransmit_cnt_hint;
+ u32 lost_retrans_low; /* Sent seq after any rxmit (lowest) */
+
u16 advmss; /* Advertised MSS */
u16 prior_ssthresh; /* ssthresh saved at recovery start */
u32 lost_out; /* Lost packets */
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index d5e0fcc..0a42e93 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1112,7 +1112,8 @@ static int tcp_is_sackblock_valid(struct tcp_sock *tp, int is_dsack,
*
* Search retransmitted skbs from write_queue that were sent when snd_nxt was
* less than what is now known to be received by the other end (derived from
- * SACK blocks by the caller).
+ * SACK blocks by the caller). Also calculate the lowest snd_nxt among the
+ * remaining retransmitted skbs to avoid some costly processing per ACKs.
*/
static int tcp_mark_lost_retrans(struct sock *sk, u32 received_upto)
{
@@ -1120,6 +1121,7 @@ static int tcp_mark_lost_retrans(struct sock *sk, u32 received_upto)
struct sk_buff *skb;
int flag = 0;
int cnt = 0;
+ u32 new_low_seq = 0;
tcp_for_write_queue(skb, sk) {
u32 ack_seq = TCP_SKB_CB(skb)->ack_seq;
@@ -1151,9 +1153,15 @@ static int tcp_mark_lost_retrans(struct sock *sk, u32 received_upto)
NET_INC_STATS_BH(LINUX_MIB_TCPLOSTRETRANSMIT);
}
} else {
+ if (!new_low_seq || before(ack_seq, new_low_seq))
+ new_low_seq = ack_seq;
cnt += tcp_skb_pcount(skb);
}
}
+
+ if (tp->retrans_out)
+ tp->lost_retrans_low = new_low_seq;
+
return flag;
}
@@ -1481,8 +1489,8 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
}
}
- if (tp->retrans_out && highest_sack_end_seq &&
- after(highest_sack_end_seq, tp->high_seq) &&
+ if (tp->retrans_out &&
+ after(highest_sack_end_seq, tp->lost_retrans_low) &&
icsk->icsk_ca_state == TCP_CA_Recovery)
flag |= tcp_mark_lost_retrans(sk, highest_sack_end_seq);
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 5329675..324b420 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1914,6 +1914,8 @@ int tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb)
printk(KERN_DEBUG "retrans_out leaked.\n");
}
#endif
+ if (!tp->retrans_out)
+ tp->lost_retrans_low = tp->snd_nxt;
TCP_SKB_CB(skb)->sacked |= TCPCB_RETRANS;
tp->retrans_out += tcp_skb_pcount(skb);
--
1.5.0.6
^ permalink raw reply related
* [RFC PATCH net-2.6] TCP: SACKtag rewrite
From: Ilpo Järvinen @ 2007-10-11 12:21 UTC (permalink / raw)
To: David Miller; +Cc: netdev, TAKANO Ryousei, --cc
Dave (& others?),
Here's the rebased SACKtag rewrite series rebased for you,
applies on top of the seven patch series I sent earlier
today (or on top of net-2.6 as soon as Dave picks them up
and pushes out).
Some trivial helper patches first (two first ones from
tcp-2.6), the most interesting bits are in the fifth patch.
I'll recode the included DSACK handling, it will be much
cleaner then so please ignore that mess for now...
This time only compile tested but should be still runnable
because I tested an earlier version and diff against that
looked sane.
--
i.
^ permalink raw reply
* [PATCH 3/6] [TCP]: Convert highest_sack to sk_buff to allow direct access
From: Ilpo Järvinen @ 2007-10-11 12:21 UTC (permalink / raw)
To: David Miller; +Cc: netdev, TAKANO Ryousei, --cc
In-Reply-To: <11921053181212-git-send-email-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 | 11 ++++++-----
net/ipv4/tcp_output.c | 19 ++++++++++---------
4 files changed, 33 insertions(+), 16 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index c5b94c1..0ec6bb6 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 92049e6..aead90a 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1306,6 +1306,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 9d3d390..39d6a6a 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1238,10 +1238,11 @@ int tcp_match_skb_to_sack(struct sock *sk, struct sk_buff *skb,
return in_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. */
@@ -1321,8 +1322,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;
state->highest_sack_end_seq = TCP_SKB_CB(skb)->end_seq;
}
} else {
@@ -1363,7 +1364,7 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
if (!tp->sacked_out) {
if (WARN_ON(tp->fackets_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);
@@ -1497,7 +1498,7 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
fack_count += tcp_skb_pcount(skb);
- 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 324b420..9603de8 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 || tcp_is_reno(tp))
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,9 +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 (tcp_is_sack(tp) && tp->sacked_out &&
- (TCP_SKB_CB(skb)->seq == tp->highest_sack))
- tp->highest_sack = TCP_SKB_CB(buff)->seq;
+ if (tcp_is_sack(tp) && 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;
@@ -772,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. */
@@ -1720,7 +1721,7 @@ static void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *skb, int m
tcp_skb_pcount(next_skb) != 1);
if (WARN_ON(tcp_is_sack(tp) && tp->sacked_out &&
- (TCP_SKB_CB(next_skb)->seq == tp->highest_sack)))
+ (next_skb == tp->highest_sack)))
return;
/* Ok. We will be able to collapse the packet. */
@@ -1755,7 +1756,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, next_skb, tcp_skb_pcount(next_skb));
+ tcp_adjust_fackets_out(sk, next_skb, tcp_skb_pcount(next_skb));
tp->packets_out -= tcp_skb_pcount(next_skb);
/* changed transmit queue under us so clear hints */
@@ -2036,7 +2037,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
* [PATCH 2/6] [TCP]: Create tcp_sacktag_one().
From: Ilpo Järvinen @ 2007-10-11 12:21 UTC (permalink / raw)
To: David Miller; +Cc: netdev, TAKANO Ryousei, --cc, David S.Miller
In-Reply-To: <11921053181239-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 | 205 ++++++++++++++++++++++++++------------------------
1 files changed, 106 insertions(+), 99 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 1f6cbce..9d3d390 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1238,6 +1238,110 @@ int tcp_match_skb_to_sack(struct sock *sk, struct sk_buff *skb,
return in_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 (!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;
+ state->highest_sack_end_seq = TCP_SKB_CB(skb)->end_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)
{
@@ -1370,7 +1474,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;
- u8 sacked;
if (skb == tcp_send_head(sk))
break;
@@ -1394,104 +1497,8 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
fack_count += tcp_skb_pcount(skb);
- 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 (!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;
- state.highest_sack_end_seq = TCP_SKB_CB(skb)->end_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/6] [TCP]: Create tcp_sacktag_state.
From: Ilpo Järvinen @ 2007-10-11 12:21 UTC (permalink / raw)
To: David Miller; +Cc: netdev, TAKANO Ryousei, --cc, David S.Miller
In-Reply-To: <11921053182989-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 | 82 +++++++++++++++++++++++++++-----------------------
1 files changed, 44 insertions(+), 38 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 0a42e93..1f6cbce 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1165,6 +1165,14 @@ static int tcp_mark_lost_retrans(struct sock *sk, u32 received_upto)
return flag;
}
+struct tcp_sacktag_state {
+ unsigned int flag;
+ int reord;
+ int prior_fackets;
+ u32 highest_sack_end_seq;
+ 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,
u32 prior_snd_una)
@@ -1240,26 +1248,23 @@ 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 highest_sack_end_seq = 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) {
if (WARN_ON(tp->fackets_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
@@ -1272,18 +1277,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;
@@ -1294,8 +1299,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;
@@ -1313,17 +1318,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;
@@ -1332,12 +1334,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.highest_sack_end_seq = 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) {
@@ -1360,7 +1366,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;
@@ -1371,7 +1377,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;
}
@@ -1401,12 +1407,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. */
@@ -1435,8 +1441,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;
@@ -1452,15 +1458,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)
@@ -1468,11 +1474,11 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
if (after(TCP_SKB_CB(skb)->seq, tp->highest_sack)) {
tp->highest_sack = TCP_SKB_CB(skb)->seq;
- highest_sack_end_seq = TCP_SKB_CB(skb)->end_seq;
+ state.highest_sack_end_seq = TCP_SKB_CB(skb)->end_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
@@ -1490,15 +1496,15 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
}
if (tp->retrans_out &&
- after(highest_sack_end_seq, tp->lost_retrans_low) &&
+ after(state.highest_sack_end_seq, tp->lost_retrans_low) &&
icsk->icsk_ca_state == TCP_CA_Recovery)
- flag |= tcp_mark_lost_retrans(sk, highest_sack_end_seq);
+ state.flag |= tcp_mark_lost_retrans(sk, state.highest_sack_end_seq);
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);
@@ -1506,7 +1512,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;
}
/* If we receive more dupacks than we expected counting segments
--
1.5.0.6
^ permalink raw reply related
* [PATCH 4/6] [TCP]: Earlier SACK block verification & simplify access to them
From: Ilpo Järvinen @ 2007-10-11 12:21 UTC (permalink / raw)
To: David Miller; +Cc: netdev, TAKANO Ryousei, --cc
In-Reply-To: <11921053182585-git-send-email-ilpo.jarvinen@helsinki.fi>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
include/linux/tcp.h | 2 +-
net/ipv4/tcp_input.c | 75 ++++++++++++++++++++++++++++++-------------------
2 files changed, 47 insertions(+), 30 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 0ec6bb6..3e412f2 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
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 39d6a6a..c260642 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1350,9 +1350,11 @@ 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 tcp_sack_block_wire *sp_wire = (struct tcp_sack_block_wire *)(ptr+2);
+ struct tcp_sack_block sp[4];
struct sk_buff *cached_skb;
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;
@@ -1367,7 +1369,7 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
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, ack_skb, sp_wire, num_sacks, prior_snd_una);
if (found_dup_sack)
state.flag |= FLAG_DSACKING_ACK;
@@ -1378,14 +1380,46 @@ 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;
+ used_sacks = 0;
+ for (i = 0; i < num_sacks; i++) {
+ int dup_sack = !i && found_dup_sack;
+
+ 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));
+
+ 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);
+ else
+ NET_INC_STATS_BH(LINUX_MIB_TCPDSACKIGNOREDOLD);
+ } else {
+ /* Don't count olds caused by ACK reordering */
+ if ((TCP_SKB_CB(ack_skb)->ack_seq != tp->snd_una) &&
+ !after(sp[used_sacks].end_seq, tp->snd_una))
+ continue;
+ NET_INC_STATS_BH(LINUX_MIB_TCPSACKDISCARD);
+ }
+ continue;
+ }
+
+ /* Ignore very old stuff early */
+ if (!after(sp[used_sacks].end_seq, prior_snd_una))
+ continue;
+
+ used_sacks++;
+ }
+
/* 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;
- for (i = 0; i < num_sacks; i++) {
- __be32 start_seq = sp[i].start_seq;
- __be32 end_seq = sp[i].end_seq;
+ for (i = 0; i < used_sacks; i++) {
+ u32 start_seq = sp[i].start_seq;
+ u32 end_seq = sp[i].end_seq;
if (i == 0) {
if (tp->recv_sack_cache[i].start_seq != start_seq)
@@ -1406,17 +1440,16 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
state.first_sack_index = 0;
if (force_one_sack)
- num_sacks = 1;
+ used_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 (i = used_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;
+ if (after(sp[j].start_seq, sp[j+1].start_seq)) {
+ struct tcp_sack_block tmp;
tmp = sp[j];
sp[j] = sp[j+1];
@@ -1443,29 +1476,13 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
state.prior_fackets = tp->fackets_out;
state.highest_sack_end_seq = 0;
- for (i=0; i<num_sacks; i++, sp++) {
+ for (i=0; i< used_sacks; i++) {
struct sk_buff *skb;
- __u32 start_seq = ntohl(sp->start_seq);
- __u32 end_seq = ntohl(sp->end_seq);
+ u32 start_seq = sp[i].start_seq;
+ u32 end_seq = sp[i].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 (dup_sack) {
- if (!tp->undo_marker)
- NET_INC_STATS_BH(LINUX_MIB_TCPDSACKIGNOREDNOUNDO);
- else
- NET_INC_STATS_BH(LINUX_MIB_TCPDSACKIGNOREDOLD);
- } 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))
- continue;
- NET_INC_STATS_BH(LINUX_MIB_TCPSACKDISCARD);
- }
- continue;
- }
-
skb = cached_skb;
fack_count = cached_fack_count;
--
1.5.0.6
^ permalink raw reply related
* [PATCH 6/6] [TCP]: Track sacktag (DEVEL PATCH)
From: Ilpo Järvinen @ 2007-10-11 12:21 UTC (permalink / raw)
To: David Miller; +Cc: netdev, TAKANO Ryousei, --cc
In-Reply-To: <11921053181933-git-send-email-ilpo.jarvinen@helsinki.fi>
This is not intented to go to mainline, provided just for those
who are interested enough about the algorithm internals during
a test.
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 e5b05b0..9909178 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 c13f1af..02b34a5 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1382,6 +1382,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;
}
@@ -1405,6 +1409,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;
}
@@ -1535,9 +1540,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))
@@ -1550,41 +1568,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];
+ }
if (tp->retrans_out &&
after(state.highest_sack_end_seq, tp->lost_retrans_low) &&
--
1.5.0.6
^ permalink raw reply related
* [RFC PATCH 5/6] [TCP]: Rewrite sack_recv_cache (WIP)
From: Ilpo Järvinen @ 2007-10-11 12:21 UTC (permalink / raw)
To: David Miller; +Cc: netdev, TAKANO Ryousei, --cc
In-Reply-To: <11921053184007-git-send-email-ilpo.jarvinen@helsinki.fi>
Key points of this patch are:
- In case new SACK information is advance only type, no skb
processing below previously discovered highest point is done
- Optimize cases below highest point too since there's no need
to always go up to highest point (which is very likely still
present in that SACK), this is not entirely true though
because I'm dropping the fastpath_skb_hint which could
previously optimize those cases even better. Whether that
significant, I'm not too sure.
Corrently it will provide skipping by walking. Combined with
RB-tree, all skipping would become fast too regardless of window
size (can be done incrementally later).
Previously a number of cases in TCP SACK processing fails to
take advantage of costly stored information in sack_recv_cache.
Most importantly expected events such as cumulative ACK and new
hole ACKs. Processing on such ACKs result in rather long walks
building up latencies (which easily gets nasty when window is
large). Those latencies are often completely unnecessary
compared with the amount of _new_ information received, usually
for cumulative ACK there's no new information at all, yet TCP
walked whole queue unnecessary potentially taking a number of
costly cache misses on the way, etc.!
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
and use direct access to highest SACKed skb as replacement.
Effectively "special cased" fastpath is dropped. This change
adds some complexity to introduce better coveraged "fastpath",
though the added complexity should make TCP behave more cache
friendly.
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 (if necessary at all). In addition, whenever
possible, TCP fast-forwards to highest_sack skb that was made
available by an earlier patch. 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
"special case".
DSACKs are special case that must always be walked.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
include/linux/tcp.h | 3 -
include/net/tcp.h | 1 -
net/ipv4/tcp_input.c | 253 ++++++++++++++++++++++++++++++------------------
net/ipv4/tcp_output.c | 14 +---
4 files changed, 159 insertions(+), 112 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 3e412f2..91c4430 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -343,10 +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; /* Lags behind by current skb's pcount
- * compared to respective fackets_out */
int lost_cnt_hint;
int retransmit_cnt_hint;
diff --git a/include/net/tcp.h b/include/net/tcp.h
index aead90a..cff2d86 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 c260642..c13f1af 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1170,10 +1170,14 @@ struct tcp_sacktag_state {
int reord;
int prior_fackets;
u32 highest_sack_end_seq;
- int first_sack_index;
+ int fack_count;
+ 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)
{
@@ -1183,6 +1187,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) {
@@ -1192,6 +1198,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);
}
@@ -1343,6 +1351,88 @@ 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;
+
+ 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 = tcp_match_skb_to_sack(sk, skb, start_seq, end_seq);
+ if (in_sack < 0)
+ break;
+
+ state->fack_count += tcp_skb_pcount(skb);
+
+ 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)
{
@@ -1352,16 +1442,17 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
TCP_SKB_CB(ack_skb)->sacked);
struct tcp_sack_block_wire *sp_wire = (struct tcp_sack_block_wire *)(ptr+2);
struct tcp_sack_block sp[4];
- struct sk_buff *cached_skb;
+ 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) {
if (WARN_ON(tp->fackets_out))
@@ -1369,7 +1460,7 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
tp->highest_sack = tcp_write_queue_head(sk);
}
- found_dup_sack = tcp_check_dsack(tp, ack_skb, sp_wire, 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;
@@ -1412,113 +1503,88 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
used_sacks++;
}
- /* 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;
- for (i = 0; i < used_sacks; i++) {
- u32 start_seq = sp[i].start_seq;
- u32 end_seq = sp[i].end_seq;
-
- 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;
- }
-
- state.first_sack_index = 0;
- if (force_one_sack)
- used_sacks = 1;
- else {
- int j;
- tp->fastpath_skb_hint = NULL;
-
- /* 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;
-
- 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;
- }
+ /* 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;
+ tmp = sp[j];
+ sp[j] = sp[j+1];
+ sp[j+1] = tmp;
}
}
}
- /* 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.fack_count = 0;
state.highest_sack_end_seq = 0;
- for (i=0; i< used_sacks; i++) {
- struct sk_buff *skb;
+ skb = tcp_write_queue_head(sk);
+ i = 0;
+
+ 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++;
+ }
+
+ while (i < used_sacks) {
u32 start_seq = sp[i].start_seq;
u32 end_seq = sp[i].end_seq;
- int fack_count;
- int dup_sack = (found_dup_sack && (i == state.first_sack_index));
-
- 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;
- tcp_for_write_queue_from(skb, sk) {
- int in_sack;
+ /* Skip too early cached blocks */
+ while (tcp_sack_cache_ok(tp, cache) &&
+ !before(start_seq, cache->end_seq))
+ cache++;
- if (skb == tcp_send_head(sk))
- break;
+ 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;
+ }
- 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;
+ 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;
+ }
- /* 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;
-
- in_sack = tcp_match_skb_to_sack(sk, skb, start_seq, end_seq);
- if (in_sack < 0)
- break;
-
- fack_count += tcp_skb_pcount(skb);
+ skb = tcp_sacktag_skip(skb, sk, &state, start_seq);
+ skb = tcp_sacktag_walk(skb, sk, &state, start_seq, end_seq, 0);
+ i++;
+ }
- tcp_sacktag_one(skb, sk, &state, in_sack,
- dup_sack, fack_count, end_seq);
- }
+ /* 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];
if (tp->retrans_out &&
after(state.highest_sack_end_seq, tp->lost_retrans_low) &&
@@ -2746,9 +2812,6 @@ static int tcp_clean_rtx_queue(struct sock *sk, s32 *seq_rtt_p)
tcp_rearm_rto(sk);
tp->fackets_out -= min(pkts_acked, tp->fackets_out);
- /* hint's skb might be NULL but we don't need to care */
- tp->fastpath_cnt_hint -= min_t(u32, pkts_acked,
- tp->fastpath_cnt_hint);
if (tcp_is_reno(tp))
tcp_remove_reno_sacks(sk, pkts_acked);
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 9603de8..1225f91 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
@@ -1761,11 +1754,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;
- tp->fastpath_cnt_hint -= tcp_skb_pcount(skb);
- }
sk_stream_free_skb(sk, next_skb);
}
--
1.5.0.6
^ permalink raw reply related
* [PATCH respin] ucc_geth: fix module removal
From: Anton Vorontsov @ 2007-10-11 12:48 UTC (permalink / raw)
To: netdev; +Cc: leoli, linuxppc-dev, linux-kernel
- uccf should be set to NULL to not double-free memory on
subsequent calls;
- ind_hash_q and group_hash_q lists should be initialized in the
probe() function, instead of struct_init() (called by open()),
otherwise there will be an oops if ucc_geth_driver removed
prior 'ifconfig ethX up';
- add unregister_netdev();
- reorder geth_remove() steps.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/net/ucc_geth.c | 17 ++++++++++-------
1 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index 7dedc96..18a6f48 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -2080,8 +2080,10 @@ static void ucc_geth_memclean(struct ucc_geth_private *ugeth)
if (!ugeth)
return;
- if (ugeth->uccf)
+ if (ugeth->uccf) {
ucc_fast_free(ugeth->uccf);
+ ugeth->uccf = NULL;
+ }
if (ugeth->p_thread_data_tx) {
qe_muram_free(ugeth->thread_dat_tx_offset);
@@ -2312,10 +2314,6 @@ static int ucc_struct_init(struct ucc_geth_private *ugeth)
ug_info = ugeth->ug_info;
uf_info = &ug_info->uf_info;
- /* Create CQs for hash tables */
- INIT_LIST_HEAD(&ugeth->group_hash_q);
- INIT_LIST_HEAD(&ugeth->ind_hash_q);
-
if (!((uf_info->bd_mem_part == MEM_PART_SYSTEM) ||
(uf_info->bd_mem_part == MEM_PART_MURAM))) {
if (netif_msg_probe(ugeth))
@@ -3949,6 +3947,10 @@ static int ucc_geth_probe(struct of_device* ofdev, const struct of_device_id *ma
ugeth = netdev_priv(dev);
spin_lock_init(&ugeth->lock);
+ /* Create CQs for hash tables */
+ INIT_LIST_HEAD(&ugeth->group_hash_q);
+ INIT_LIST_HEAD(&ugeth->ind_hash_q);
+
dev_set_drvdata(device, dev);
/* Set the dev->base_addr to the gfar reg region */
@@ -4002,9 +4004,10 @@ static int ucc_geth_remove(struct of_device* ofdev)
struct net_device *dev = dev_get_drvdata(device);
struct ucc_geth_private *ugeth = netdev_priv(dev);
- dev_set_drvdata(device, NULL);
- ucc_geth_memclean(ugeth);
+ unregister_netdev(dev);
free_netdev(dev);
+ ucc_geth_memclean(ugeth);
+ dev_set_drvdata(device, NULL);
return 0;
}
--
1.5.0.6
^ permalink raw reply related
* [PATCH] ucc_geth: add support for netpoll
From: Anton Vorontsov @ 2007-10-11 12:48 UTC (permalink / raw)
To: netdev; +Cc: leoli, linuxppc-dev, linux-kernel
This patch adds netpoll support for the QE UCC Gigabit Ethernet
driver. The approach is very similar to the gianfar driver.
Tested using netconsole.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/net/ucc_geth.c | 19 +++++++++++++++++++
1 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index 18a6f48..06807ce 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -3691,6 +3691,22 @@ static irqreturn_t ucc_geth_irq_handler(int irq, void *info)
return IRQ_HANDLED;
}
+#ifdef CONFIG_NET_POLL_CONTROLLER
+/*
+ * Polling 'interrupt' - used by things like netconsole to send skbs
+ * without having to re-enable interrupts. It's not called while
+ * the interrupt routine is executing.
+ */
+static void ucc_netpoll(struct net_device *dev)
+{
+ struct ucc_geth_private *ugeth = netdev_priv(dev);
+
+ disable_irq(ugeth->ug_info->uf_info.irq);
+ ucc_geth_irq_handler(ugeth->ug_info->uf_info.irq, dev);
+ enable_irq(ugeth->ug_info->uf_info.irq);
+}
+#endif /* CONFIG_NET_POLL_CONTROLLER */
+
/* Called when something needs to use the ethernet device */
/* Returns 0 for success. */
static int ucc_geth_open(struct net_device *dev)
@@ -3969,6 +3985,9 @@ static int ucc_geth_probe(struct of_device* ofdev, const struct of_device_id *ma
dev->poll = ucc_geth_poll;
dev->weight = UCC_GETH_DEV_WEIGHT;
#endif /* CONFIG_UGETH_NAPI */
+#ifdef CONFIG_NET_POLL_CONTROLLER
+ dev->poll_controller = ucc_netpoll;
+#endif
dev->stop = ucc_geth_close;
dev->get_stats = ucc_geth_get_stats;
// dev->change_mtu = ucc_geth_change_mtu;
--
1.5.0.6
^ permalink raw reply related
* [PATCH respin] phy: implement release function
From: Anton Vorontsov @ 2007-10-11 12:49 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel
Lately I've got this nice badness on mdio bus removal:
Device 'e0103120:06' does not have a release() function, it is broken and must be fixed.
------------[ cut here ]------------
Badness at drivers/base/core.c:107
NIP: c015c1a8 LR: c015c1a8 CTR: c0157488
REGS: c34bdcf0 TRAP: 0700 Not tainted (2.6.23-rc5-g9ebadfbb-dirty)
MSR: 00029032 <EE,ME,IR,DR> CR: 24088422 XER: 00000000
...
[c34bdda0] [c015c1a8] device_release+0x78/0x80 (unreliable)
[c34bddb0] [c01354cc] kobject_cleanup+0x80/0xbc
[c34bddd0] [c01365f0] kref_put+0x54/0x6c
[c34bdde0] [c013543c] kobject_put+0x24/0x34
[c34bddf0] [c015c384] put_device+0x1c/0x2c
[c34bde00] [c0180e84] mdiobus_unregister+0x2c/0x58
...
Though actually there is nothing broken, it just device
subsystem core expects another "pattern" of resource managment.
This patch implement phy device's release function, thus
we're getting rid of this badness.
Also small hidden bug fixed, hope none other introduced. ;-)
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Acked-by: Andy Fleming <afleming@freescale.com>
---
drivers/net/phy/mdio_bus.c | 9 +++++----
drivers/net/phy/phy_device.c | 13 +++++++++++++
include/linux/phy.h | 1 +
3 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index fc2f0e6..c30196d 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -91,9 +91,12 @@ int mdiobus_register(struct mii_bus *bus)
err = device_register(&phydev->dev);
- if (err)
+ if (err) {
printk(KERN_ERR "phy %d failed to register\n",
i);
+ phy_device_free(phydev);
+ phydev = NULL;
+ }
}
bus->phy_map[i] = phydev;
@@ -110,10 +113,8 @@ void mdiobus_unregister(struct mii_bus *bus)
int i;
for (i = 0; i < PHY_MAX_ADDR; i++) {
- if (bus->phy_map[i]) {
+ if (bus->phy_map[i])
device_unregister(&bus->phy_map[i]->dev);
- kfree(bus->phy_map[i]);
- }
}
}
EXPORT_SYMBOL(mdiobus_unregister);
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 49328e0..8e25bf7 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -44,6 +44,17 @@ static struct phy_driver genphy_driver;
extern int mdio_bus_init(void);
extern void mdio_bus_exit(void);
+void phy_device_free(struct phy_device *phydev)
+{
+ kfree(phydev);
+}
+EXPORT_SYMBOL(phy_device_free);
+
+static void phy_device_release(struct device *dev)
+{
+ phy_device_free(to_phy_device(dev));
+}
+
struct phy_device* phy_device_create(struct mii_bus *bus, int addr, int phy_id)
{
struct phy_device *dev;
@@ -54,6 +65,8 @@ struct phy_device* phy_device_create(struct mii_bus *bus, int addr, int phy_id)
if (NULL == dev)
return (struct phy_device*) PTR_ERR((void*)-ENOMEM);
+ dev->dev.release = phy_device_release;
+
dev->speed = 0;
dev->duplex = -1;
dev->pause = dev->asym_pause = 0;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 2a65978..9ec1363 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -398,6 +398,7 @@ int phy_mii_ioctl(struct phy_device *phydev,
int phy_start_interrupts(struct phy_device *phydev);
void phy_print_status(struct phy_device *phydev);
struct phy_device* phy_device_create(struct mii_bus *bus, int addr, int phy_id);
+void phy_device_free(struct phy_device *phydev);
extern struct bus_type mdio_bus_type;
#endif /* __PHY_H */
--
1.5.0.6
^ permalink raw reply related
* Re: [RFC/PATCH 4/4] UDP memory usage accounting (take 4): memory limitation
From: Satoshi OSHIMA @ 2007-10-11 12:51 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Andi Kleen, David Miller, Evgeniy Polyakov, Herbert Xu, netdev,
?? ??, Yumiko SUGITA, ??@RedHat
In-Reply-To: <20071005085257.24b2e5cc@freepuppy.rosehill>
Hi Stephen,
Thank you for your comment.
>> {
>> + .ctl_name = NET_UDP_MEM,
>> + .procname = "udp_mem",
>> + .data = &sysctl_udp_mem,
>> + .maxlen = sizeof(sysctl_udp_mem),
>> + .mode = 0644,
>> + .proc_handler = &proc_dointvec
>> + },
>> + {
>> .ctl_name = NET_TCP_APP_WIN,
>> .procname = "tcp_app_win",
>> .data = &sysctl_tcp_app_win,
>
> if you use &proc_dointvec_minmax, then you could inforce min/max
> values for udp_mem for the sysctl
udp_mem has two meanings:
* turn off this limitation function (currently udp_mem<=4096)
* limit udp memory (currently udp_mem>4096)
To realize this, udp_mem is evaluated whether udp_mem equals
4096 or smaller in UDP and IP layers.
If udp_mem has proc_dointvec_minmax or dedicated proc handler,
turn off check must be done in UDP and IP layers. This means
there is no reduction of the check in UDP and IP layers.
If you pointed out that minus value of udp_mem is strange,
I agree. I'll fix it.
How about this?
min=4096 (and turn off limitation)
udp_mem>4096 (and turn on limitation)
Satoshi Oshima
^ permalink raw reply
* Regression in net-2.6.24?
From: TAKANO Ryousei @ 2007-10-11 13:51 UTC (permalink / raw)
To: netdev; +Cc: davem, ilpo.jarvinen, shemminger
In-Reply-To: <Pine.LNX.4.64.0710111300280.23026@kivilampi-30.cs.helsinki.fi>
From: "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi>
Subject: Re: [RFC PATCH] [TCP]: Fix lost_retrans loop vs fastpath problems
Date: Thu, 11 Oct 2007 13:12:49 +0300 (EEST)
> > But, I got
> > a kernel panic at net_rx_action() in our experimental setting.
> > I am using the net-2.6.24 kernel _without_ this patch.
> > (I will post a bug report separately).
>
> ...Please do. :-)
>
I have got a kernel panic, when I run the Iperf benchmark in the following
setting. Each node has x86_64 dual processors and 2 tg3 NICs (BCM95704A6).
If the router does not regulate the bandwidth, a kernel panic does not occur.
Node A ----> Router -------> Delay -------> Node B
(Policing rate: emulator
500Mbps) (RTT: 20ms)
Ggit-bisect told me that the following commit causes a regression:
commit bea3348eef27e6044b6161fd04c3152215f96411
Author: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Wed Oct 3 16:41:36 2007 -0700
[NET]: Make NAPI polling independent of struct net_device objects.
Here is Oops message:
Unable to handle kernel paging request at 0000000000100108 RIP:
[<ffffffff80421d59>] net_rx_action+0x169/0x1c0
PGD f384d067 PUD 0
Oops: 0002 [1] SMP
CPU 0
Modules linked in: 8021q tcp_bic netconsole evdev joydev sg st sr_mod ohci_hcd i2c_amd756 i2c_amd8111 i2c_core ipv6 tg3 usbhid usbcore ff_memless dm_mod ext3 jbd sata_sil libata sd_mod scsi_mod
Pid: 0, comm: swapper Not tainted 2.6.23-rc9 #2
RIP: 0010:[<ffffffff80421d59>] [<ffffffff80421d59>] net_rx_action+0x169/0x1c0
RSP: 0018:ffffffff80847eb0 EFLAGS: 00010046
RAX: 0000000000200200 RBX: ffff8100f63ef750 RCX: 0000000000000246
RDX: 0000000000100100 RSI: ffffc20002e60204 RDI: ffff8100f63ef6c0
RBP: ffffffff80847ef0 R08: ffff810178b72fd8 R09: 0000000000000001
R10: 0000000000000003 R11: 0000000000000000 R12: 0000000000000040
R13: 0000000000000040 R14: ffff810003f51640 R15: 0000000000000000
FS: 0000000040331960(0000) GS:ffffffff805d7000(0000) knlGS:0000000000000000
CS: 0010 DS: 0018 ES: 0018 CR0: 000000008005003b
CR2: 0000000000100108 CR3: 00000000f45b6000 CR4: 00000000000006e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process swapper (pid: 0, threadinfo ffffffff806de000, task ffffffff8058b540)
Stack: ffffffff80847eb0 000000ec80847eb0 00000001000b17ec 0000000000000009
ffffffff805e10b0 ffffffff8083eee0 0000000000000000 0000000000000009
ffffffff80847f30 ffffffff8023d935 0000000000000000 0000000000000046
Call Trace:
<IRQ> [<ffffffff8023d935>] __do_softirq+0x75/0x100
[<ffffffff8020cb8c>] call_softirq+0x1c/0x30
[<ffffffff8020dd89>] do_softirq+0x49/0xb0
[<ffffffff8023d4e5>] irq_exit+0x45/0x50
[<ffffffff8020e053>] do_IRQ+0x83/0x100
[<ffffffff8020a310>] default_idle+0x0/0x50
[<ffffffff8020bf11>] ret_from_intr+0x0/0xb
<EOI> [<ffffffff8020a33d>] default_idle+0x2d/0x50
[<ffffffff8020aa72>] enter_idle+0x22/0x30
[<ffffffff8020ab0c>] cpu_idle+0x8c/0xd0
[<ffffffff804af6ec>] rest_init+0x5c/0x70
[<ffffffff806e8baf>] start_kernel+0x28f/0x300
[<ffffffff806e8140>] _sinittext+0x140/0x180
Code: 48 89 42 08 48 89 10 49 8b 46 08 4c 89 33 49 89 5e 08 48 89
RIP [<ffffffff80421d59>] net_rx_action+0x169/0x1c0
RSP <ffffffff80847eb0>
CR2: 0000000000100108
Kernel panic - not syncing: Aiee, killing interrupt handler!
RIP points at __list_del (net_rx_action -> list_move_tail -> __list_del).
$ objdump -S net/core/dev.o | less
<snip>
static inline void __list_del(struct list_head * prev, struct list_head * next)
{
b42: 48 8b 43 08 mov 0x8(%rbx),%rax
b46: 48 8b 13 mov (%rbx),%rdx
next->prev = prev;
b49: 48 89 42 08 mov %rax,0x8(%rdx) <====
prev->next = next;
b4d: 48 89 10 mov %rdx,(%rax)
b50: 49 8b 46 08 mov 0x8(%r14),%rax
b54: 4c 89 33 mov %r14,(%rbx)
b57: 49 89 5e 08 mov %rbx,0x8(%r14)
b5b: 48 89 18 mov %rbx,(%rax)
What is happen and what information do you need to fix it?
Thanks in advance,
Ryousei Takano
^ permalink raw reply
* Re: [PATCH] IB/ipoib: Bound the net device to the ipoib_neigh structue
From: Moni Shoua @ 2007-10-11 14:48 UTC (permalink / raw)
To: Roland Dreier, Jay Vosburgh, jeff
Cc: David Miller, ogerlitz, netdev, Moni Levy
In-Reply-To: <aday7eaddod.fsf@cisco.com>
Roland Dreier wrote:
> > I also ran a test for the code in the branch of 2.6.24 and found a problem.
> > I see that ifconfig down doesn't return (for IPoIB interfaces) and it's stuck in napi_disable() in the kernel (any idea why?)
>
> For what it's worth, I took the upstream 2.6.23 git tree and merged in
> Dave's latest net-2.6.24 tree and my latest for-2.6.24 tree and tried
> that. I brought up an IPoIB interface, sent a few pings, and did
> ifconfig down, and it worked fine.
>
> Can you try the same thing without the bonding patches to see if your
> setup works OK too?
>
> Also can you give more details about what you do to get ifconfig down stuck?
>
> - R.
Without bonding ifconfig down works fine.
It happens only when ib interfaces are slaves of a bonding device.
I thought before that the stuck is in napi_disable() but it's almost right.
I put prints before and after call to napi_disable and see that it is called twice.
I'll try to investigate in this direction.
ib0: stopping interface
ib0: before napi_disable
ib0: after napi_disable
ib0: downing ib_dev
ib0: All sends and receives done.
ib0: stopping interface
ib0: before napi_disable
There is also a dump of the kernel log after 'echo t > /proc/sysrq-trigger' (for ifconfig)
SysRq : Show State
ifconfig S 0000000000000000 0 6311 6099
ffff810034f49d18 0000000000000086 0000000000000000 ffffffffffffffff
ffff810037e747c0 ffff810037e747c0 000000013481e000 ffff81003a851a78
ffff81003a851840 000000003b0c8c00 0000000000000000 00000000802358ee
Call Trace:
[<ffffffff8023cc89>] lock_timer_base+0x24/0x49
[<ffffffff80403754>] schedule_timeout+0x8a/0xad
[<ffffffff8023d241>] process_timeout+0x0/0x5
[<ffffffff8023d6ec>] msleep_interruptible+0x11/0x39
[<ffffffff884081a7>] :ib_ipoib:ipoib_stop+0x64/0x12c
[<ffffffff8039fc07>] dev_close+0x3e/0x56
[<ffffffff803a1c31>] dev_change_flags+0xa7/0x15f
[<ffffffff803e5bee>] devinet_ioctl+0x293/0x5ed
[<ffffffff803e775b>] inet_ioctl+0x7f/0x9d
[<ffffffff80395b2e>] sock_ioctl+0x0/0x1fe
[<ffffffff80395d08>] sock_ioctl+0x1da/0x1fe
[<ffffffff802947d9>] do_ioctl+0x29/0x6f
[<ffffffff80294a75>] vfs_ioctl+0x256/0x267
[<ffffffff80294adf>] sys_ioctl+0x59/0x7a
[<ffffffff8020bc0e>] system_call+0x7e/0x83
^ permalink raw reply
* [PATCH]race between open and disconnect in irda-usb
From: Oliver Neukum @ 2007-10-11 15:45 UTC (permalink / raw)
To: Jean Tourrilhes, Nick Fedchik, netdev, USB development list
Hi,
it seems to me that irda_usb_net_open() must set self->netopen
under spinlock or disconnect() may fail to kill all URBs.
Signed-off-by: Oliver Neukum <oneukum@suse.de>
Regards
Oliver
----
--- a/drivers/net/irda/irda-usb.c 2007-10-11 17:24:35.000000000 +0200
+++ b/drivers/net/irda/irda-usb.c 2007-10-11 17:30:30.000000000 +0200
@@ -1168,6 +1168,7 @@ static int stir421x_patch_device(struct
static int irda_usb_net_open(struct net_device *netdev)
{
struct irda_usb_cb *self;
+ unsigned long flags;
char hwname[16];
int i;
@@ -1177,13 +1178,16 @@ static int irda_usb_net_open(struct net_
self = (struct irda_usb_cb *) netdev->priv;
IRDA_ASSERT(self != NULL, return -1;);
+ spin_lock_irqsave(&self->lock, flags);
/* Can only open the device if it's there */
if(!self->present) {
+ spin_unlock_irqrestore(&self->lock, flags);
IRDA_WARNING("%s(), device not present!\n", __FUNCTION__);
return -1;
}
if(self->needspatch) {
+ spin_unlock_irqrestore(&self->lock, flags);
IRDA_WARNING("%s(), device needs patch\n", __FUNCTION__) ;
return -EIO ;
}
@@ -1198,6 +1202,7 @@ static int irda_usb_net_open(struct net_
/* To do *before* submitting Rx urbs and starting net Tx queue
* Jean II */
self->netopen = 1;
+ spin_unlock_irqrestore(&self->lock, flags);
/*
* Now that everything should be initialized properly,
-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
^ permalink raw reply
* Re: e100 problems in .23rc8 ?
From: Kok, Auke @ 2007-10-11 16:10 UTC (permalink / raw)
To: Herbert Xu, Dave Jones; +Cc: Kok, Auke, netdev, esandeen, dmack
In-Reply-To: <20071011012520.GA13160@gondor.apana.org.au>
Herbert Xu wrote:
> On Wed, Oct 10, 2007 at 08:36:38PM -0400, Dave Jones wrote:
>> The e1000 changes you reference above, is this the changeset you mean?
>>
>> commit 416b5d10afdc797c21c457ade3714e8f2f75edd9
>> Author: Auke Kok <auke-jan.h.kok@intel.com>
>> Date: Fri Jun 1 10:22:39 2007 -0700
>>
>> e1000: disable polling before registering netdevice
>
> Yep.
this patch actually called napi_disable() in the probe routine which was wrong,
but e100 does not do that. Nonetheless e100 doesn't call netif_carrier_off() and
netif_stop_queue(), so to make e100 the same as e1000 we should probably do this,
see below.
Dave, can you see if this resolves the issue for you? If so then we might want to
push this to -stable.
Auke
---
e100: disable netdevice explicitly to avoid rx irq oops
Several reported OOPS messages suggest that e100 has a race that was fixed in
e1000 before where incoming interrupts trigger an OOPS immediately after probe()
finishes.
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
diff --git a/drivers/net/e100.c b/drivers/net/e100.c
index 280313b..ded5f68 100644
--- a/drivers/net/e100.c
+++ b/drivers/net/e100.c
@@ -2682,6 +2682,10 @@ static int __devinit e100_probe(struct pci_dev *pdev,
if (err)
DPRINTK(PROBE, ERR, "Error clearing wake event\n");
+ /* tell the stack to leave us alone until e100_open() is called */
+ netif_carrier_off(netdev);
+ netif_stop_queue(netdev);
+
strcpy(netdev->name, "eth%d");
if((err = register_netdev(netdev))) {
DPRINTK(PROBE, ERR, "Cannot register net device, aborting.\n");
^ permalink raw reply related
* Re: [PATCH]race between open and disconnect in irda-usb
From: Jean Tourrilhes @ 2007-10-11 16:23 UTC (permalink / raw)
To: Oliver Neukum, Samuel Ortiz; +Cc: Nick Fedchik, netdev, USB development list
In-Reply-To: <200710111745.28594.oliver@neukum.org>
On Thu, Oct 11, 2007 at 05:45:28PM +0200, Oliver Neukum wrote:
> Hi,
>
Hi,
You need to cc Samuel Ortiz and the IrDA mailing list if you
want any meaningful response.
> it seems to me that irda_usb_net_open() must set self->netopen
> under spinlock or disconnect() may fail to kill all URBs.
I believe to patch is correct, but in practice I fail to see
how it could happen. Anyway, Samuel will have the final word.
> Signed-off-by: Oliver Neukum <oneukum@suse.de>
>
> Regards
> Oliver
Have fun...
Jean
> ----
>
> --- a/drivers/net/irda/irda-usb.c 2007-10-11 17:24:35.000000000 +0200
> +++ b/drivers/net/irda/irda-usb.c 2007-10-11 17:30:30.000000000 +0200
> @@ -1168,6 +1168,7 @@ static int stir421x_patch_device(struct
> static int irda_usb_net_open(struct net_device *netdev)
> {
> struct irda_usb_cb *self;
> + unsigned long flags;
> char hwname[16];
> int i;
>
> @@ -1177,13 +1178,16 @@ static int irda_usb_net_open(struct net_
> self = (struct irda_usb_cb *) netdev->priv;
> IRDA_ASSERT(self != NULL, return -1;);
>
> + spin_lock_irqsave(&self->lock, flags);
> /* Can only open the device if it's there */
> if(!self->present) {
> + spin_unlock_irqrestore(&self->lock, flags);
> IRDA_WARNING("%s(), device not present!\n", __FUNCTION__);
> return -1;
> }
>
> if(self->needspatch) {
> + spin_unlock_irqrestore(&self->lock, flags);
> IRDA_WARNING("%s(), device needs patch\n", __FUNCTION__) ;
> return -EIO ;
> }
> @@ -1198,6 +1202,7 @@ static int irda_usb_net_open(struct net_
> /* To do *before* submitting Rx urbs and starting net Tx queue
> * Jean II */
> self->netopen = 1;
> + spin_unlock_irqrestore(&self->lock, flags);
>
> /*
> * Now that everything should be initialized properly,
>
^ permalink raw reply
* Re: [PATCH] rtnl: Simplify ASSERT_RTNL
From: Eric W. Biederman @ 2007-10-11 16:33 UTC (permalink / raw)
To: Herbert Xu; +Cc: David Miller, netdev, Patrick McHardy
In-Reply-To: <20071011082845.GA16527@gondor.apana.org.au>
Herbert Xu <herbert@gondor.apana.org.au> writes:
> Well thanks to that warning we're on our way of improving the
> code that triggered it in such a way that this warning will soon
> go silent.
>
> That's precisely the reason why I object to having this warning
> removed. Now you have a good point that this warning doesn't
> trigger all the time. The fix to that is to *make* it trigger
> always, not removing it.
I'm almost convinced but.
Where people deliberately use convoluted locking is where we
most need things like ASSERT_RTNL.
Having ASSERT_RTNL warn if you were sleeping does not seem
intuitive from the name.
This instance of convoluted locking seems like a complete
one off to me, and if it will warn about other constructs
currently in the kernel it seems wrong.
Frankly I don't feel comfortable adding the check because I can't
defend the presence of might_sleep() in ASSERT_RTNL. If I can't
understand a change well enough to defend it I will not take
responsibility for it, and I will not add my Signed-off-by to it.
The patch I wrote was trivial a trivial optimization and obviously
correct. Adding the might_sleep() and the patch becomes the start
of a crusade for better code that I don't believe in.
So I would rather forget this patch then make that one line addition.
Thanks,
Eric
^ 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