Re: [PATCH 2/3] Seperate DSACK from SACK fast path
From: Baruch Even [EMAIL PROTECTED] Date: Fri, 02 Feb 2007 16:41:21 +0200 Move DSACK code outside the SACK fast-path checking code. If the DSACK determined that the information was too old we stayed with a partial cache copied. Most likely this matters very little since the next packet will not be DSACK and we will find it in the cache. but it's still not good form and there is little reason to couple the two checks. Since the SACK receive cache doesn't need the data to be in host order we also remove the ntohl in the checking loop. Signed-Off-By: Baruch Even [EMAIL PROTECTED] Thanks for fixing up the endianness annotations, applied. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Seperate DSACK from SACK fast path
From: Baruch Even [EMAIL PROTECTED]
Date: Mon, 29 Jan 2007 09:13:44 +0200
+ for (i = 0; i num_sacks; i++) {
+ __be32 start_seq = sp[i].start_seq;
+ __be32 end_seq = sp[i].end_seq;
This is not sufficient, you have to also fix up the type
of the recv_sack_cache[] array entries in struct tcp_sock
to be struct tcp_sack_block_wire.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Seperate DSACK from SACK fast path
Baruch Even [EMAIL PROTECTED] wrote:
Since the SACK receive cache doesn't need the data to be in host
order we also remove the ntohl in the checking loop.
...
- for (i = 0; i num_sacks; i++) {
- __u32 start_seq = ntohl(sp[i].start_seq);
- __u32 end_seq = ntohl(sp[i].end_seq);
+ for (i = 0; i num_sacks; i++) {
+ __u32 start_seq = sp[i].start_seq;
+ __u32 end_seq = sp[i].end_seq;
Yes. The only comparison we do with recv_sack_cache entries is != and
that works for net-endian just fine.
In that case you need to use __be32 before Al Viro starts coming after
you :)
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Seperate DSACK from SACK fast path
From: Herbert Xu [EMAIL PROTECTED]
Date: Mon, 29 Jan 2007 16:06:07 +1100
Baruch Even [EMAIL PROTECTED] wrote:
Since the SACK receive cache doesn't need the data to be in host
order we also remove the ntohl in the checking loop.
...
- for (i = 0; i num_sacks; i++) {
- __u32 start_seq = ntohl(sp[i].start_seq);
- __u32 end_seq = ntohl(sp[i].end_seq);
+ for (i = 0; i num_sacks; i++) {
+ __u32 start_seq = sp[i].start_seq;
+ __u32 end_seq = sp[i].end_seq;
Yes. The only comparison we do with recv_sack_cache entries is != and
that works for net-endian just fine.
In that case you need to use __be32 before Al Viro starts coming after
you :)
Good catch, Baruch please fix this up :-)
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] Seperate DSACK from SACK fast path
Move DSACK code outside the SACK fast-path checking code. If the DSACK
determined that the information was too old we stayed with a partial cache
copied. Most likely this matters very little since the next packet will not be
DSACK and we will find it in the cache. but it's still not good form and there
is little reason to couple the two checks.
Since the SACK receive cache doesn't need the data to be in host order we also
remove the ntohl in the checking loop.
Signed-Off-By: Baruch Even [EMAIL PROTECTED]
Index: 2.6-rc6/net/ipv4/tcp_input.c
===
--- 2.6-rc6.orig/net/ipv4/tcp_input.c 2007-01-27 15:06:30.0 +0200
+++ 2.6-rc6/net/ipv4/tcp_input.c2007-01-27 15:59:15.0 +0200
@@ -948,16 +948,43 @@
tp-fackets_out = 0;
prior_fackets = tp-fackets_out;
+ /* Check for D-SACK. */
+ if (before(ntohl(sp[0].start_seq), TCP_SKB_CB(ack_skb)-ack_seq)) {
+ dup_sack = 1;
+ tp-rx_opt.sack_ok |= 4;
+ NET_INC_STATS_BH(LINUX_MIB_TCPDSACKRECV);
+ } else if (num_sacks 1
+ !after(ntohl(sp[0].end_seq), ntohl(sp[1].end_seq))
+ !before(ntohl(sp[0].start_seq),
ntohl(sp[1].start_seq))) {
+ dup_sack = 1;
+ tp-rx_opt.sack_ok |= 4;
+ NET_INC_STATS_BH(LINUX_MIB_TCPDSACKOFORECV);
+ }
+
+ /* D-SACK for already forgotten data...
+* Do dumb counting. */
+ if (dup_sack
+ !after(ntohl(sp[0].end_seq), prior_snd_una)
+ after(ntohl(sp[0].end_seq), tp-undo_marker))
+ tp-undo_retrans--;
+
+ /* Eliminate too old ACKs, but take into
+* account more or less fresh ones, they can
+* contain valid SACK info.
+*/
+ 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 */
flag = 1;
- for (i = 0; i num_sacks; i++) {
- __u32 start_seq = ntohl(sp[i].start_seq);
- __u32 end_seq = ntohl(sp[i].end_seq);
+ for (i = 0; i num_sacks; i++) {
+ __u32 start_seq = sp[i].start_seq;
+ __u32 end_seq = sp[i].end_seq;
- if (i == 0){
+ if (i == 0) {
if (tp-recv_sack_cache[i].start_seq != start_seq)
flag = 0;
} else {
@@ -967,37 +994,6 @@
}
tp-recv_sack_cache[i].start_seq = start_seq;
tp-recv_sack_cache[i].end_seq = end_seq;
-
- /* Check for D-SACK. */
- if (i == 0) {
- u32 ack = TCP_SKB_CB(ack_skb)-ack_seq;
-
- if (before(start_seq, ack)) {
- dup_sack = 1;
- tp-rx_opt.sack_ok |= 4;
- NET_INC_STATS_BH(LINUX_MIB_TCPDSACKRECV);
- } else if (num_sacks 1
- !after(end_seq, ntohl(sp[1].end_seq))
- !before(start_seq, ntohl(sp[1].start_seq))) {
- dup_sack = 1;
- tp-rx_opt.sack_ok |= 4;
- NET_INC_STATS_BH(LINUX_MIB_TCPDSACKOFORECV);
- }
-
- /* D-SACK for already forgotten data...
-* Do dumb counting. */
- if (dup_sack
- !after(end_seq, prior_snd_una)
- after(end_seq, tp-undo_marker))
- tp-undo_retrans--;
-
- /* Eliminate too old ACKs, but take into
-* account more or less fresh ones, they can
-* contain valid SACK info.
-*/
- if (before(ack, prior_snd_una - tp-max_window))
- return 0;
- }
}
if (flag)
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Seperate DSACK from SACK fast path
From: Baruch Even [EMAIL PROTECTED]
Date: Sat, 27 Jan 2007 18:49:49 +0200
Since the SACK receive cache doesn't need the data to be in host
order we also remove the ntohl in the checking loop.
...
- for (i = 0; i num_sacks; i++) {
- __u32 start_seq = ntohl(sp[i].start_seq);
- __u32 end_seq = ntohl(sp[i].end_seq);
+ for (i = 0; i num_sacks; i++) {
+ __u32 start_seq = sp[i].start_seq;
+ __u32 end_seq = sp[i].end_seq;
...
}
tp-recv_sack_cache[i].start_seq = start_seq;
tp-recv_sack_cache[i].end_seq = end_seq;
Ok, and now the sack cache and the real sack blocks are
stored in net-endian and this works out because we only
make direct equality comparisons with the recv_sack_cache[]
entry values?
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Seperate DSACK from SACK fast path
* David Miller [EMAIL PROTECTED] [070128 06:06]:
From: Baruch Even [EMAIL PROTECTED]
Date: Sat, 27 Jan 2007 18:49:49 +0200
Since the SACK receive cache doesn't need the data to be in host
order we also remove the ntohl in the checking loop.
...
- for (i = 0; i num_sacks; i++) {
- __u32 start_seq = ntohl(sp[i].start_seq);
- __u32 end_seq = ntohl(sp[i].end_seq);
+ for (i = 0; i num_sacks; i++) {
+ __u32 start_seq = sp[i].start_seq;
+ __u32 end_seq = sp[i].end_seq;
...
}
tp-recv_sack_cache[i].start_seq = start_seq;
tp-recv_sack_cache[i].end_seq = end_seq;
Ok, and now the sack cache and the real sack blocks are
stored in net-endian and this works out because we only
make direct equality comparisons with the recv_sack_cache[]
entry values?
Yes. The only comparison we do with recv_sack_cache entries is != and
that works for net-endian just fine.
The only reason recv_sack_cache was in host-order before that was that
start_seq and end_seq were used to do more before/after comparisons for
DSACK.
Baruch
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
