query: tcp_sock tcp_header_len calculations (re-sent)

PATCH Superseded William Allen Simpson 1 participant 10 messages Jan 10, 2010 – Jan 20, 2010

Subsystems (3)

Networking Network drivers TCP

lore

Share & tools

lore

William Allen Simpson

author

2010-01-10 12:06 UTC #0 lore raw

Apparently, nobody on the network developers list knows about this. I've stumbled upon a completely undocumented and incomprehensible usage for tcp_header_len. Is whomever wrote this still around?

linux/tcp.h documents this as:
...
u16 tcp_header_len; /* Bytes of tcp header to send */
...

So far, so good. But it's clearly *not* correct in tcp_output.c:

tcp_connect_init()
...

	tp->tcp_header_len = sizeof(struct tcphdr) +
		(sysctl_tcp_timestamps ? TCPOLEN_TSTAMP_ALIGNED : 0);

#ifdef CONFIG_TCP_MD5SIG
	if (tp->af_specific->md5_lookup(sk, sk) != NULL)
		tp->tcp_header_len += TCPOLEN_MD5SIG_ALIGNED;

#endif
...

This combination is actually *impossible* -- current options code *never* allows both authentication and timestamps, doing SACK instead:

tcp_syn_options()
...

	if (likely(sysctl_tcp_timestamps && *md5 == NULL)) {
		opts->options |= OPTION_TS;

...

tcp_synack_options()
...

		/* We can't fit any SACK blocks in a packet with MD5 + TS
		 * options. There was discussion about disabling SACK
		 * rather than TS in order to fit in better with old,
		 * buggy kernels, but that was deemed to be unnecessary.
		 */
		doing_ts &= !ireq->sack_ok;

...

Thus, tcp_header_len has the wrong value, resulting in underestimation for MSS. But even worse usage in minisocks.c:

tcp_create_openreq_child()
...

		if (newtp->rx_opt.tstamp_ok) {
			newtp->rx_opt.ts_recent = req->ts_recent;
			newtp->rx_opt.ts_recent_stamp = get_seconds();
			newtp->tcp_header_len = sizeof(struct tcphdr) + TCPOLEN_TSTAMP_ALIGNED;
		} else {
			newtp->rx_opt.ts_recent_stamp = 0;
			newtp->tcp_header_len = sizeof(struct tcphdr);
		}
#ifdef CONFIG_TCP_MD5SIG
		newtp->md5sig_info = NULL;	/*XXX*/
#endif
		if (skb->len >= TCP_MSS_DEFAULT + newtp->tcp_header_len)
			newicsk->icsk_ack.last_seg_size = skb->len - newtp->tcp_header_len;

...

This takes an *output* estimation, and then compares it to (and subtracts from) skb->len, which is *input* length. What's supposed to happen here?

Shouldn't this simply use the real input tcp_hdrlen()?

William Allen Simpson

author

2010-01-15 14:16 UTC #1 lore raw

PATCH 1/1 tcp: input header length, prediction, and timestamp bugs

Fix incorrect header prediction flags documentation.

Don't use output calculated tp->tcp_header_len for input decisions. While the output header is usually the same as the input (same options in both directions), that's a poor assumption. In particular, Sack will be different. Newer options are not guaranteed.

Moreover, in the fast path, that only saved a shift or two. The other efficiencies in this patch more than make up the difference.

Instead, use tp->rx_opt.tstamp_ok to accurately predict header length.

Likewise, use tp->rx_opt.tstamp_ok for received MSS calculations.

Don't use "sizeof(struct tcphdr) + TCPOLEN_TSTAMP_ALIGNED" to guess that the timestamp is present. This may have been OK in the days with fewer possible options, but various combinations of newer options may yield the same header length.

Instead, use tp->rx_opt.saw_tstamp to determine a timestamp is present. (This bug is in 3 places.)

There's no need to test buffer length against header length, already checked by tcp_v[4,6]_rcv(). Straighten code for minor efficiency gain.

Don't overload tp->tcp_header_len by temporarily storing predicted header length. It's wiped later by the output code, and very confusing.

Instead, the simpler __tcp_fast_path_header_length() can also be used for the same purpose, at the cost of a branch, but the substantial savings of removing the extra store and re-load.

Finally, don't use TCPOLEN_MD5SIG_ALIGNED in the tcp_minisock.c last_seg_size calculations. It's not used in all other places, and may result in a too short estimate.

Instead, use the same calculations as tp->advmss in tcp_input.c.

Stand-alone patch, originally developed for TCPCT.

Requires:
net: tcp_header_len_th and tcp_option_len_th
tcp: harmonize tcp_vx_rcv header length assumptions

Signed-off-by [email protected]

---
 include/linux/tcp.h      |    6 ++-
 include/net/tcp.h        |    9 ++++-
 net/ipv4/tcp_input.c     |   92 ++++++++++++++++------------------------------
 net/ipv4/tcp_minisocks.c |   10 ++---
 4 files changed, 49 insertions(+), 68 deletions(-)

include/linux/tcp.h +5 -1

index 74728f7..2987ee8 100644
@@ -301,7 +301,11 @@ struct tcp_sock {
 
 /*
  *	Header prediction flags
- *	0x5?10 << 16 + snd_wnd in net byte order
+ *	S << 28 + TCP_FLAG_ACK + snd_wnd, in net byte order
+ *		(PSH flag is ignored)
+ *		S is 5 (no options), or 8 (timestamp aligned)
+ *	otherwise, 0 to turn it off -- for instance, when there are
+ *	holes in receive space.
  */
 	__be32	pred_flags;

include/net/tcp.h +8 -1

index 34f5cc2..30817b1 100644
@@ -533,9 +533,16 @@ static inline u32 __tcp_set_rto(const struct tcp_sock *tp)
 	return (tp->srtt >> 3) + tp->rttvar;
 }
 
+static inline u16 __tcp_fast_path_header_length(const struct tcp_sock *tp)
+{
+	return tp->rx_opt.tstamp_ok
+		? sizeof(struct tcphdr) + TCPOLEN_TSTAMP_ALIGNED
+		: sizeof(struct tcphdr);
+}
+
 static inline void __tcp_fast_path_on(struct tcp_sock *tp, u32 snd_wnd)
 {
-	tp->pred_flags = htonl((tp->tcp_header_len << 26) |
+	tp->pred_flags = htonl((__tcp_fast_path_header_length(tp) << (28 - 2)) |
 			       ntohl(TCP_FLAG_ACK) |
 			       snd_wnd);
 }

net/ipv4/tcp_input.c +32 ~~-60~~

150 lines not on this page · raw message

net/ipv4/tcp_minisocks.c +4 -6

index f206ee5..d57a7da 100644
@@ -387,6 +387,7 @@ struct sock *tcp_create_openreq_child(struct sock *sk, struct request_sock *req,
 		struct tcp_sock *newtp = tcp_sk(newsk);
 		struct tcp_sock *oldtp = tcp_sk(sk);
 		struct tcp_cookie_values *oldcvp = oldtp->cookie_values;
+		int lss = skb->len - sizeof(struct tcphdr);
 
 		/* TCP Cookie Transactions require space for the cookie pair,
 		 * as it differs for each connection.  There is no need to

@@ -490,18 +491,15 @@ struct sock *tcp_create_openreq_child(struct sock *sk, struct request_sock *req,
 		if (newtp->rx_opt.tstamp_ok) {
 			newtp->rx_opt.ts_recent = req->ts_recent;
 			newtp->rx_opt.ts_recent_stamp = get_seconds();
-			newtp->tcp_header_len = sizeof(struct tcphdr) + TCPOLEN_TSTAMP_ALIGNED;
+			lss -= TCPOLEN_TSTAMP_ALIGNED;
 		} else {
 			newtp->rx_opt.ts_recent_stamp = 0;
-			newtp->tcp_header_len = sizeof(struct tcphdr);
 		}
 #ifdef CONFIG_TCP_MD5SIG
 		newtp->md5sig_info = NULL;	/*XXX*/
-		if (newtp->af_specific->md5_lookup(sk, newsk))
-			newtp->tcp_header_len += TCPOLEN_MD5SIG_ALIGNED;
 #endif
-		if (skb->len >= TCP_MSS_DEFAULT + newtp->tcp_header_len)
-			newicsk->icsk_ack.last_seg_size = skb->len - newtp->tcp_header_len;
+		if (lss >= TCP_MSS_DEFAULT)
+			newicsk->icsk_ack.last_seg_size = lss;
 		newtp->rx_opt.mss_clamp = req->mss;
 		TCP_ECN_openreq_child(newtp, req);

Signature · 1 line

1.6.3.3

William Allen Simpson

author

2010-01-15 19:12 UTC #2 lore raw

Re: [PATCH] tcp: input header length, prediction, and timestamp bugs

Quoted 6 lines · William Allen Simpson

Finally, don't use TCPOLEN_MD5SIG_ALIGNED in the tcp_minisock.c last_seg_size calculations. It's not used in all other places, and may result in a too short estimate.
Instead, use the same calculations as tp->advmss in tcp_input.c.

This part really shouldn't have been included in this patch, as it depends on similar changes to tcp_output.c. My mistake cutting the patches apart from the bigger testing patch.

I'll send a new version promptly.

William Allen Simpson

author

2010-01-15 19:20 UTC #3 lore raw

PATCH v2 1/1 tcp: input header length, prediction, and timestamp bugs

In reply to William Allen Simpson

Fix incorrect header prediction flags documentation.

Moreover, in the fast path, that only saved a shift or two. The other efficiencies in this patch more than make up the difference.

Instead, use tp->rx_opt.tstamp_ok to accurately predict header length.

Likewise, use tp->rx_opt.tstamp_ok for received MSS calculations.

Instead, use tp->rx_opt.saw_tstamp to determine a timestamp is present.

There's no need to test buffer length against header length, already checked by tcp_v[4,6]_rcv(). Straighten code for minor efficiency gain.

Stand-alone patch, originally developed for TCPCT.

Requires:
net: tcp_header_len_th and tcp_option_len_th
tcp: harmonize tcp_vx_rcv header length assumptions

Signed-off-by [email protected]

---
 include/linux/tcp.h  |    6 +++-
 include/net/tcp.h    |    9 +++++-
 net/ipv4/tcp_input.c |   84 +++++++++++++++++++-------------------------------
 3 files changed, 45 insertions(+), 54 deletions(-)

include/linux/tcp.h +5 -1

index 74728f7..2987ee8 100644
@@ -301,7 +301,11 @@ struct tcp_sock {
 
 /*
  *	Header prediction flags
- *	0x5?10 << 16 + snd_wnd in net byte order
+ *	S << 28 + TCP_FLAG_ACK + snd_wnd, in net byte order
+ *		(PSH flag is ignored)
+ *		S is 5 (no options), or 8 (timestamp aligned)
+ *	otherwise, 0 to turn it off -- for instance, when there are
+ *	holes in receive space.
  */
 	__be32	pred_flags;

include/net/tcp.h +8 -1

index 34f5cc2..30817b1 100644
@@ -533,9 +533,16 @@ static inline u32 __tcp_set_rto(const struct tcp_sock *tp)
 	return (tp->srtt >> 3) + tp->rttvar;
 }
 
+static inline u16 __tcp_fast_path_header_length(const struct tcp_sock *tp)
+{
+	return tp->rx_opt.tstamp_ok
+		? sizeof(struct tcphdr) + TCPOLEN_TSTAMP_ALIGNED
+		: sizeof(struct tcphdr);
+}
+
 static inline void __tcp_fast_path_on(struct tcp_sock *tp, u32 snd_wnd)
 {
-	tp->pred_flags = htonl((tp->tcp_header_len << 26) |
+	tp->pred_flags = htonl((__tcp_fast_path_header_length(tp) << (28 - 2)) |
 			       ntohl(TCP_FLAG_ACK) |
 			       snd_wnd);
 }

net/ipv4/tcp_input.c +32 ~~-52~~

128 lines not on this page · raw message

Signature · 1 line

1.6.3.3

William Allen Simpson

author

2010-01-19 17:35 UTC #4 lore raw

Re: [PATCH v2] tcp: input header length, prediction, and timestamp bugs

Over the weekend, I've been reviewing the .lst output that Andi explained, and I've found a few interesting things.

1) The 4.4 compiler isn't very smart about shifts:

static inline unsigned int tcp_header_len_th(const struct tcphdr *th)

{
	return th->doff * 4;
c04ea93e:	0f b6 47 0c          	movzbl 0xc(%edi),%eax
c04ea942:	c0 e8 04             	shr    $0x4,%al
c04ea945:	0f b6 c0             	movzbl %al,%eax
c04ea948:	c1 e0 02             	shl    $0x2,%eax

That could easily be done as shr $0x2 instead.

2) This "fast path" code is under quite a bit of register pressure on the 32-bit i386. There's a lot of saving and re-loading.

3) Particularly, the seldom used *th and len parameters are saved and reloaded in the very beginning of the fast path, really wasting time.

4) Since both *th and len are actually indexed loads from *skb (which the compiler keeps in a register most of the time), doing indexed loads from the stack (%ebp) is the same, so they shouldn't be sent as parameters at all!

5) There's already code, added back in 2006 for DMA, that directly references skb->len instead of the len parameter. Probably lack of header documentation, so the coder failed to notice:

				if (copied_early)
					tcp_cleanup_rbuf(sk, skb->len);
c04ead5d:	8b 56 50             	mov    0x50(%esi),%edx
c04ead60:	89 d8                	mov    %ebx,%eax
c04ead62:	e8 fc ff ff ff       	call   c04ead63 <tcp_rcv_established+0x633>

Therefore, I'll resubmit this patch, removing the existing len parameter. And maybe *th, too....

William Allen Simpson

author

2010-01-19 19:35 UTC #5 lore raw

Re: [PATCH v2] tcp: input header length, prediction, and timestamp bugs

Quoted 3 lines · William Allen Simpson

Therefore, I'll resubmit this patch, removing the existing len parameter. And maybe *th, too....

Just to quickly note that gcc 4.4 doesn't properly remember that it has already loaded *th with this rampant use of an inline function (unlike the older macro method):

c04ea739: 89 d3 mov %edx,%ebx

static inline struct tcphdr *tcp_hdr(const struct sk_buff *skb)

{
	return (struct tcphdr *)skb_transport_header(skb);
c04ea743:	8b 92 94 00 00 00    	mov    0x94(%edx),%edx
	 *
	 *	Our current scheme is not silly either but we take the
	 *	extra cost of the net_bh soft interrupt processing...
	 *	We do checksum and copy also but from device to kernel.
	 */
	if ((tcp_flag_word(tcp_hdr(skb)) & TCP_HP_BITS) == tp->pred_flags &&

...

Note that the index is in both %edx and %ebx, but it uses replaced %edx. Although by inspection that result stays in %edx, it reloaded twice more:

	res = tcp_validate_incoming(sk, skb, tcp_hdr(skb), 1);
c04ea78c:	8b 8b 94 00 00 00    	mov    0x94(%ebx),%ecx
c04ea792:	89 da                	mov    %ebx,%edx
c04ea794:	89 f0                	mov    %esi,%eax
c04ea796:	c7 04 24 01 00 00 00 	movl   $0x1,(%esp)
c04ea79d:	e8 0e c3 ff ff       	call   c04e6ab0 <tcp_validate_incoming>
	if (res <= 0)
c04ea7a2:	85 c0                	test   %eax,%eax
c04ea7a4:	0f 8e 8e 03 00 00    	jle    c04eab38 <tcp_rcv_established+0x408>

#else /* NET_SKBUFF_DATA_USES_OFFSET */

static inline unsigned char *skb_transport_header(const struct sk_buff *skb)

{
	return skb->transport_header;
c04ea7aa:	8b 83 94 00 00 00    	mov    0x94(%ebx),%eax
c04ea7b0:	f6 40 0d 10          	testb  $0x10,0xd(%eax)
c04ea7b4:	0f 85 5e 03 00 00    	jne    c04eab18 <tcp_rcv_established+0x3e8>

This doesn't happen with the parameter *th (undisturbed in %edi):

c04ea78a:	89 f9                	mov    %edi,%ecx
c04ea78c:	89 f2                	mov    %esi,%edx
c04ea78e:	89 d8                	mov    %ebx,%eax
c04ea790:	c7 04 24 01 00 00 00 	movl   $0x1,(%esp)
c04ea797:	e8 14 c3 ff ff       	call   c04e6ab0 <tcp_validate_incoming>
	if (res <= 0)
c04ea79c:	85 c0                	test   %eax,%eax
c04ea79e:	0f 8e 8c 03 00 00    	jle    c04eab30 <tcp_rcv_established+0x400>
		return -res;

step5:

	if (th->ack && tcp_ack(sk, skb, FLAG_SLOWPATH) < 0)
c04ea7a4:	f6 47 0d 10          	testb  $0x10,0xd(%edi)
c04ea7a8:	0f 85 62 03 00 00    	jne    c04eab10 <tcp_rcv_established+0x3e0>

Therefore, keeping the parameter *th.

William Allen Simpson

author

2010-01-19 19:58 UTC #6 lore raw

Re: [PATCH v2] tcp: input header length, prediction, and timestamp bugs

In reply to William Allen Simpson

Quoted 2 lines · William Allen Simpson

Therefore, I'll resubmit this patch, removing the existing len parameter.

And for record, there's a jtcp_rcv_established(), and the len isn't used there either (it uses skb->len instead).

I also discovered that the related tcp_rcv_state_process() and tcp_rcv_synsent_state_process() never uses their len parameter. So, I'll remove them, too.

William Allen Simpson

author

2010-01-19 20:17 UTC #7 lore raw

PATCH v3 1/1 tcp: input header length, prediction, and timestamp bugs

In reply to William Allen Simpson

Fix incorrect header prediction flags documentation.

Relieve register pressure in (the i386) fast path by accessing skb->len directly, instead of carrying a rarely used len parameter.

Eliminate unused len parameters in two other functions.

Moreover, in the fast path, that only saved a shift or two. The other efficiencies in this patch more than make up the difference.

Instead, use tp->rx_opt.tstamp_ok to accurately predict header length.

Likewise, use tp->rx_opt.tstamp_ok for received MSS calculations.

Instead, use tp->rx_opt.saw_tstamp to determine a timestamp is present.

There's no need to test buffer length against header length, already checked by tcp_v[4,6]_rcv(). Straighten code for minor efficiency gain.

Stand-alone patch, originally developed for TCPCT.

Requires:
net: tcp_header_len_th and tcp_option_len_th
tcp: harmonize tcp_vx_rcv header length assumptions

Signed-off-by [email protected]

---
 include/linux/tcp.h  |    6 +++-
 include/net/tcp.h    |   15 +++++---
 net/ipv4/tcp_input.c |   94 +++++++++++++++++++------------------------------
 net/ipv4/tcp_ipv4.c  |    4 +-
 net/ipv4/tcp_probe.c |    2 +-
 net/ipv6/tcp_ipv6.c  |    4 +-
 6 files changed, 57 insertions(+), 68 deletions(-)

include/linux/tcp.h +5 -1

index 74728f7..2987ee8 100644
@@ -301,7 +301,11 @@ struct tcp_sock {
 
 /*
  *	Header prediction flags
- *	0x5?10 << 16 + snd_wnd in net byte order
+ *	S << 28 + TCP_FLAG_ACK + snd_wnd, in net byte order
+ *		(PSH flag is ignored)
+ *		S is 5 (no options), or 8 (timestamp aligned)
+ *	otherwise, 0 to turn it off -- for instance, when there are
+ *	holes in receive space.
  */
 	__be32	pred_flags;

include/net/tcp.h +10 -5

index 34f5cc2..6b0d7e9 100644
@@ -310,13 +310,11 @@ extern int			tcp_ioctl(struct sock *sk,
 
 extern int			tcp_rcv_state_process(struct sock *sk, 
 						      struct sk_buff *skb,
-						      struct tcphdr *th,
-						      unsigned len);
+						      struct tcphdr *th);
 
 extern int			tcp_rcv_established(struct sock *sk, 
 						    struct sk_buff *skb,
-						    struct tcphdr *th, 
-						    unsigned len);
+						    struct tcphdr *th);
 
 extern void			tcp_rcv_space_adjust(struct sock *sk);

@@ -533,9 +531,16 @@ static inline u32 __tcp_set_rto(const struct tcp_sock *tp)
 	return (tp->srtt >> 3) + tp->rttvar;
 }
 
+static inline u16 __tcp_fast_path_header_length(const struct tcp_sock *tp)
+{
+	return tp->rx_opt.tstamp_ok
+		? sizeof(struct tcphdr) + TCPOLEN_TSTAMP_ALIGNED
+		: sizeof(struct tcphdr);
+}
+
 static inline void __tcp_fast_path_on(struct tcp_sock *tp, u32 snd_wnd)
 {
-	tp->pred_flags = htonl((tp->tcp_header_len << 26) |
+	tp->pred_flags = htonl((__tcp_fast_path_header_length(tp) << (28 - 2)) |
 			       ntohl(TCP_FLAG_ACK) |
 			       snd_wnd);
 }

net/ipv4/tcp_input.c +37 ~~-57~~

166 lines not on this page · raw message

net/ipv4/tcp_ipv4.c +2 -2

index 0a76e41..f999e06 100644
@@ -1551,7 +1551,7 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
 
 	if (sk->sk_state == TCP_ESTABLISHED) { /* Fast path */
 		TCP_CHECK_TIMER(sk);
-		if (tcp_rcv_established(sk, skb, tcp_hdr(skb), skb->len)) {
+		if (tcp_rcv_established(sk, skb, tcp_hdr(skb))) {
 			rsk = sk;
 			goto reset;
 		}

@@ -1578,7 +1578,7 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
 	}
 
 	TCP_CHECK_TIMER(sk);
-	if (tcp_rcv_state_process(sk, skb, tcp_hdr(skb), skb->len)) {
+	if (tcp_rcv_state_process(sk, skb, tcp_hdr(skb))) {
 		rsk = sk;
 		goto reset;
 	}

net/ipv4/tcp_probe.c +1 -1

index bb110c5..7bcd3ba 100644
@@ -88,7 +88,7 @@ static inline int tcp_probe_avail(void)
  * Note: arguments must match tcp_rcv_established()!
  */
 static int jtcp_rcv_established(struct sock *sk, struct sk_buff *skb,
-			       struct tcphdr *th, unsigned len)
+			       struct tcphdr *th)
 {
 	const struct tcp_sock *tp = tcp_sk(sk);
 	const struct inet_sock *inet = inet_sk(sk);

net/ipv6/tcp_ipv6.c +2 -2

index b76939a..3d08a4d 100644
@@ -1586,7 +1586,7 @@ static int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 
 	if (sk->sk_state == TCP_ESTABLISHED) { /* Fast path */
 		TCP_CHECK_TIMER(sk);
-		if (tcp_rcv_established(sk, skb, tcp_hdr(skb), skb->len))
+		if (tcp_rcv_established(sk, skb, tcp_hdr(skb)))
 			goto reset;
 		TCP_CHECK_TIMER(sk);
 		if (opt_skb)

@@ -1618,7 +1618,7 @@ static int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 	}
 
 	TCP_CHECK_TIMER(sk);
-	if (tcp_rcv_state_process(sk, skb, tcp_hdr(skb), skb->len))
+	if (tcp_rcv_state_process(sk, skb, tcp_hdr(skb)))
 		goto reset;
 	TCP_CHECK_TIMER(sk);
 	if (opt_skb)

Signature · 1 line

1.6.3.3

William Allen Simpson

author

2010-01-19 23:58 UTC #8 lore raw

Re: [PATCH v3] tcp: input header length, prediction, and timestamp bugs

Missing 1 part of the patch. My mistake cutting the patches apart from the bigger testing patch. I'll send a new version promptly.

William Allen Simpson

author

2010-01-20 00:01 UTC #9 lore raw

PATCH v4 1/1 tcp: input header length, prediction, and timestamp bugs

In reply to William Allen Simpson

Fix incorrect header prediction flags documentation.

Relieve register pressure in (the i386) fast path by accessing skb->len directly, instead of carrying a rarely used len parameter.

Eliminate unused len parameters in two other functions.

Moreover, in the fast path, that only saved a shift or two. The other efficiencies in this patch more than make up the difference.

Instead, use tp->rx_opt.tstamp_ok to accurately predict header length.

Likewise, use tp->rx_opt.tstamp_ok for received MSS calculations.

Instead, use tp->rx_opt.saw_tstamp to determine a timestamp is present.

There's no need to test buffer length against header length, already checked by tcp_v[4,6]_rcv(). Straighten code for minor efficiency gain.

Stand-alone patch, originally developed for TCPCT.

Requires:
net: tcp_header_len_th and tcp_option_len_th
tcp: harmonize tcp_vx_rcv header length assumptions

Signed-off-by [email protected]

---
 include/linux/tcp.h      |    6 ++-
 include/net/tcp.h        |   15 +++++--
 net/ipv4/tcp_input.c     |   94 ++++++++++++++++++----------------------------
 net/ipv4/tcp_ipv4.c      |    4 +-
 net/ipv4/tcp_minisocks.c |    3 +-
 net/ipv4/tcp_probe.c     |    2 +-
 net/ipv6/tcp_ipv6.c      |    4 +-
 7 files changed, 58 insertions(+), 70 deletions(-)

include/linux/tcp.h +5 -1

index 74728f7..2987ee8 100644
@@ -301,7 +301,11 @@ struct tcp_sock {
 
 /*
  *	Header prediction flags
- *	0x5?10 << 16 + snd_wnd in net byte order
+ *	S << 28 + TCP_FLAG_ACK + snd_wnd, in net byte order
+ *		(PSH flag is ignored)
+ *		S is 5 (no options), or 8 (timestamp aligned)
+ *	otherwise, 0 to turn it off -- for instance, when there are
+ *	holes in receive space.
  */
 	__be32	pred_flags;

include/net/tcp.h +10 -5

index 34f5cc2..6b0d7e9 100644
@@ -310,13 +310,11 @@ extern int			tcp_ioctl(struct sock *sk,
 
 extern int			tcp_rcv_state_process(struct sock *sk, 
 						      struct sk_buff *skb,
-						      struct tcphdr *th,
-						      unsigned len);
+						      struct tcphdr *th);
 
 extern int			tcp_rcv_established(struct sock *sk, 
 						    struct sk_buff *skb,
-						    struct tcphdr *th, 
-						    unsigned len);
+						    struct tcphdr *th);
 
 extern void			tcp_rcv_space_adjust(struct sock *sk);

@@ -533,9 +531,16 @@ static inline u32 __tcp_set_rto(const struct tcp_sock *tp)
 	return (tp->srtt >> 3) + tp->rttvar;
 }
 
+static inline u16 __tcp_fast_path_header_length(const struct tcp_sock *tp)
+{
+	return tp->rx_opt.tstamp_ok
+		? sizeof(struct tcphdr) + TCPOLEN_TSTAMP_ALIGNED
+		: sizeof(struct tcphdr);
+}
+
 static inline void __tcp_fast_path_on(struct tcp_sock *tp, u32 snd_wnd)
 {
-	tp->pred_flags = htonl((tp->tcp_header_len << 26) |
+	tp->pred_flags = htonl((__tcp_fast_path_header_length(tp) << (28 - 2)) |
 			       ntohl(TCP_FLAG_ACK) |
 			       snd_wnd);
 }

net/ipv4/tcp_input.c +37 ~~-57~~

166 lines not on this page · raw message

net/ipv4/tcp_ipv4.c +2 -2

index 0a76e41..f999e06 100644
@@ -1551,7 +1551,7 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
 
 	if (sk->sk_state == TCP_ESTABLISHED) { /* Fast path */
 		TCP_CHECK_TIMER(sk);
-		if (tcp_rcv_established(sk, skb, tcp_hdr(skb), skb->len)) {
+		if (tcp_rcv_established(sk, skb, tcp_hdr(skb))) {
 			rsk = sk;
 			goto reset;
 		}

@@ -1578,7 +1578,7 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
 	}
 
 	TCP_CHECK_TIMER(sk);
-	if (tcp_rcv_state_process(sk, skb, tcp_hdr(skb), skb->len)) {
+	if (tcp_rcv_state_process(sk, skb, tcp_hdr(skb))) {
 		rsk = sk;
 		goto reset;
 	}

net/ipv4/tcp_minisocks.c +1 -2

index f206ee5..37b7536 100644
@@ -718,8 +718,7 @@ int tcp_child_process(struct sock *parent, struct sock *child,
 	int state = child->sk_state;
 
 	if (!sock_owned_by_user(child)) {
-		ret = tcp_rcv_state_process(child, skb, tcp_hdr(skb),
-					    skb->len);
+		ret = tcp_rcv_state_process(child, skb, tcp_hdr(skb));
 		/* Wakeup parent, send SIGIO */
 		if (state == TCP_SYN_RECV && child->sk_state != state)
 			parent->sk_data_ready(parent, 0);

net/ipv4/tcp_probe.c +1 -1

index bb110c5..7bcd3ba 100644
@@ -88,7 +88,7 @@ static inline int tcp_probe_avail(void)
  * Note: arguments must match tcp_rcv_established()!
  */
 static int jtcp_rcv_established(struct sock *sk, struct sk_buff *skb,
-			       struct tcphdr *th, unsigned len)
+			       struct tcphdr *th)
 {
 	const struct tcp_sock *tp = tcp_sk(sk);
 	const struct inet_sock *inet = inet_sk(sk);

net/ipv6/tcp_ipv6.c +2 -2

index b76939a..3d08a4d 100644
@@ -1586,7 +1586,7 @@ static int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 
 	if (sk->sk_state == TCP_ESTABLISHED) { /* Fast path */
 		TCP_CHECK_TIMER(sk);
-		if (tcp_rcv_established(sk, skb, tcp_hdr(skb), skb->len))
+		if (tcp_rcv_established(sk, skb, tcp_hdr(skb)))
 			goto reset;
 		TCP_CHECK_TIMER(sk);
 		if (opt_skb)

@@ -1618,7 +1618,7 @@ static int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 	}
 
 	TCP_CHECK_TIMER(sk);
-	if (tcp_rcv_state_process(sk, skb, tcp_hdr(skb), skb->len))
+	if (tcp_rcv_state_process(sk, skb, tcp_hdr(skb)))
 		goto reset;
 	TCP_CHECK_TIMER(sk);
 	if (opt_skb)

Signature · 1 line

1.6.3.3