[net-next-2.6 PATCH v9 0/2] TCPCT part 1: cleanup

[net-next-2.6 PATCH v9 0/2] TCPCT part 1: cleanup

[William Allen Simpson]

Some portions that were removed during the various patch splits,
last seen in v4 and v6 respectively.

Re: [net-next-2.6 PATCH v9 0/2] TCPCT part 1h: accept SYNACK data

[William Allen Simpson]

[-- Attachment #1: Type: text/plain, Size: 778 bytes --]

When accompanied by cookie option, Initiator (client) queues incoming
SYNACK transaction data.

This is a straightforward re-implementation of an earlier (year-old)
patch that no longer applies cleanly, with permission of the original
author (Adam Langley).  The patch was previously reviewed:

    http://thread.gmane.org/gmane.linux.network/102586

Also, redefine two TCP header functions to accept TCP header pointer.
When subtracting, return signed int to allow error checking.

These functions will also be used in subsequent patches that implement
additional features.

Signed-off-by: William.Allen.Simpson@gmail.com
---
  include/linux/tcp.h  |   12 ++++++++++++
  net/ipv4/tcp_input.c |   25 ++++++++++++++++++++++++-
  2 files changed, 36 insertions(+), 1 deletions(-)

[-- Attachment #2: TCPCT+1h9.patch --]
[-- Type: text/plain, Size: 2443 bytes --]

diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 7fee8a4..54ef984 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -223,6 +223,18 @@ static inline unsigned int tcp_optlen(const struct sk_buff *skb)
 	return (tcp_hdr(skb)->doff - 5) * 4;
 }
 
+/* Fixed portion plus standard options. */
+static inline unsigned int tcp_header_len_th(const struct tcphdr *th)
+{
+	return th->doff * 4;
+}
+
+/* Standard options only.  When doff is bad, this could be negative. */
+static inline int tcp_option_len_th(const struct tcphdr *th)
+{
+	return (int)(th->doff * 4) - sizeof(*th);
+}
+
 /* This defines a selective acknowledgement block. */
 struct tcp_sack_block_wire {
 	__be32	start_seq;
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 57ae96a..8089424 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5393,6 +5393,12 @@ discard:
 	return 0;
 }
 
+/*
+ * Returns:
+ *	+1 on reset,
+ *	0 success and/or SYNACK data,
+ *	-1 on discard.
+ */
 static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
 					 struct tcphdr *th, unsigned len)
 {
@@ -5402,6 +5408,7 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
 	struct dst_entry *dst = __sk_dst_get(sk);
 	struct tcp_cookie_values *cvp = tp->cookie_values;
 	int saved_clamp = tp->rx_opt.mss_clamp;
+	int queued = 0;
 
 	tcp_parse_options(skb, &tp->rx_opt, &hash_location, 0, dst);
 
@@ -5523,6 +5530,19 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
 				       hash_location, cookie_size);
 				cvp->cookie_pair_size = cookie_pair_size;
 			}
+
+			queued = skb->len - tcp_header_len_th(th);
+			if (queued > 0) {
+				/* Queue incoming transaction data. */
+				__skb_pull(skb, tcp_header_len_th(th));
+				__skb_queue_tail(&sk->sk_receive_queue, skb);
+				skb_set_owner_r(skb, sk);
+				sk->sk_data_ready(sk, 0);
+				cvp->s_data_in = 1; /* true */
+				tp->rcv_nxt = TCP_SKB_CB(skb)->end_seq;
+				tp->rcv_wup = TCP_SKB_CB(skb)->end_seq;
+				tp->copied_seq = TCP_SKB_CB(skb)->seq + 1;
+			}
 		}
 
 		smp_mb();
@@ -5576,11 +5596,14 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
 						  TCP_DELACK_MAX, TCP_RTO_MAX);
 
 discard:
-			__kfree_skb(skb);
+			if (queued <= 0)
+				__kfree_skb(skb);
 			return 0;
 		} else {
 			tcp_send_ack(sk);
 		}
+		if (queued > 0)
+			return 0;
 		return -1;
 	}
 
-- 
1.6.3.3

Re: [net-next-2.6 PATCH v9 2/2] TCPCT part 1i: remove old tcp_optlen() function

[William Allen Simpson]

[-- Attachment #1: Type: text/plain, Size: 666 bytes --]

In the only two existing files using the old tcp_optlen() function,
clean up confusing and inconsistent mixing of both byte and word
offsets.  Document assumptions.  David Miller's review sayeth:

    This is transmit, and the packets can only come from the Linux
    TCP stack, not some external entity.

Untested.  No response from testers in 6+ weeks.

Signed-off-by: William.Allen.Simpson@gmail.com
CC: Michael Chan <mchan@broadcom.com>
---
  drivers/net/bnx2.c  |   22 +++++++++++++---------
  drivers/net/tg3.c   |   51 +++++++++++++++++++++++----------------------------
  include/linux/tcp.h |    5 -----
  3 files changed, 36 insertions(+), 42 deletions(-)

[-- Attachment #2: TCPCT+1i9.patch --]
[-- Type: text/plain, Size: 5567 bytes --]

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 4cae2a8..5b9c41b 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -6354,18 +6354,17 @@ bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 #endif
 	if ((mss = skb_shinfo(skb)->gso_size)) {
-		u32 tcp_opt_len;
-		struct iphdr *iph;
+		struct tcphdr *th = tcp_hdr(skb);
+		int tcp_opt_words = th->doff - (sizeof(*th) >> 2);
 
+		/* assumes positive tcp_opt_words without checking */
 		vlan_tag_flags |= TX_BD_FLAGS_SW_LSO;
 
-		tcp_opt_len = tcp_optlen(skb);
-
 		if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6) {
 			u32 tcp_off = skb_transport_offset(skb) -
 				      sizeof(struct ipv6hdr) - ETH_HLEN;
 
-			vlan_tag_flags |= ((tcp_opt_len >> 2) << 8) |
+			vlan_tag_flags |= (tcp_opt_words << 8) |
 					  TX_BD_FLAGS_SW_FLAGS;
 			if (likely(tcp_off == 0))
 				vlan_tag_flags &= ~TX_BD_FLAGS_TCP6_OFF0_MSK;
@@ -6378,10 +6377,15 @@ bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)
 				mss |= (tcp_off & 0xc) << TX_BD_TCP6_OFF2_SHL;
 			}
 		} else {
-			iph = ip_hdr(skb);
-			if (tcp_opt_len || (iph->ihl > 5)) {
-				vlan_tag_flags |= ((iph->ihl - 5) +
-						   (tcp_opt_len >> 2)) << 8;
+			struct iphdr *iph = ip_hdr(skb);
+			int ip_opt_words = iph->ihl - (sizeof(*iph) >> 2);
+			int opt_words;
+
+			/* assumes positive ip_opt_words without checking */
+			opt_words = ip_opt_words + tcp_opt_words;
+
+			if (opt_words > 0) {
+				vlan_tag_flags |= opt_words << 8;
 			}
 		}
 	} else
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 302ea0b..f4c4173 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -5459,7 +5459,7 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb,
 	base_flags = 0;
 	mss = 0;
 	if ((mss = skb_shinfo(skb)->gso_size) != 0) {
-		int tcp_opt_len, ip_tcp_len;
+		struct tcphdr *th;
 		u32 hdrlen;
 
 		if (skb_header_cloned(skb) &&
@@ -5468,17 +5468,16 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb,
 			goto out_unlock;
 		}
 
+		th = tcp_hdr(skb);
+
 		if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6)
 			hdrlen = skb_headlen(skb) - ETH_HLEN;
 		else {
 			struct iphdr *iph = ip_hdr(skb);
 
-			tcp_opt_len = tcp_optlen(skb);
-			ip_tcp_len = ip_hdrlen(skb) + sizeof(struct tcphdr);
-
+			hdrlen = ip_hdrlen(skb) + tcp_header_len_th(th);
+			iph->tot_len = htons(mss + hdrlen);
 			iph->check = 0;
-			iph->tot_len = htons(mss + ip_tcp_len + tcp_opt_len);
-			hdrlen = ip_tcp_len + tcp_opt_len;
 		}
 
 		if (tp->tg3_flags2 & TG3_FLG2_HW_TSO_3) {
@@ -5492,7 +5491,7 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb,
 		base_flags |= (TXD_FLAG_CPU_PRE_DMA |
 			       TXD_FLAG_CPU_POST_DMA);
 
-		tcp_hdr(skb)->check = 0;
+		th->check = 0;
 
 	}
 	else if (skb->ip_summed == CHECKSUM_PARTIAL)
@@ -5666,7 +5665,9 @@ static netdev_tx_t tg3_start_xmit_dma_bug(struct sk_buff *skb,
 
 	if ((mss = skb_shinfo(skb)->gso_size) != 0) {
 		struct iphdr *iph;
-		u32 tcp_opt_len, ip_tcp_len, hdr_len;
+		struct tcphdr *th;
+		u32 hdr_len;
+		int opt_bytes;
 
 		if (skb_header_cloned(skb) &&
 		    pskb_expand_head(skb, 0, 0, GFP_ATOMIC)) {
@@ -5674,10 +5675,9 @@ static netdev_tx_t tg3_start_xmit_dma_bug(struct sk_buff *skb,
 			goto out_unlock;
 		}
 
-		tcp_opt_len = tcp_optlen(skb);
-		ip_tcp_len = ip_hdrlen(skb) + sizeof(struct tcphdr);
+		th = tcp_hdr(skb);
+		hdr_len = ip_hdrlen(skb) + tcp_header_len_th(th);
 
-		hdr_len = ip_tcp_len + tcp_opt_len;
 		if (unlikely((ETH_HLEN + hdr_len) > 80) &&
 			     (tp->tg3_flags2 & TG3_FLG2_TSO_BUG))
 			return (tg3_tso_bug(tp, skb));
@@ -5689,35 +5689,30 @@ static netdev_tx_t tg3_start_xmit_dma_bug(struct sk_buff *skb,
 		iph->check = 0;
 		iph->tot_len = htons(mss + hdr_len);
 		if (tp->tg3_flags2 & TG3_FLG2_HW_TSO) {
-			tcp_hdr(skb)->check = 0;
+			th->check = 0;
 			base_flags &= ~TXD_FLAG_TCPUDP_CSUM;
 		} else
-			tcp_hdr(skb)->check = ~csum_tcpudp_magic(iph->saddr,
-								 iph->daddr, 0,
-								 IPPROTO_TCP,
-								 0);
+			th->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr,
+						       0, IPPROTO_TCP, 0);
+
+		/* assumes positive without checking */
+		opt_bytes = hdr_len - sizeof(*iph) - sizeof(*th);
 
 		if (tp->tg3_flags2 & TG3_FLG2_HW_TSO_3) {
 			mss |= (hdr_len & 0xc) << 12;
 			if (hdr_len & 0x10)
 				base_flags |= 0x00000010;
 			base_flags |= (hdr_len & 0x3e0) << 5;
-		} else if (tp->tg3_flags2 & TG3_FLG2_HW_TSO_2)
+		} else if (tp->tg3_flags2 & TG3_FLG2_HW_TSO_2) {
 			mss |= hdr_len << 9;
-		else if ((tp->tg3_flags2 & TG3_FLG2_HW_TSO_1) ||
+		} else if ((tp->tg3_flags2 & TG3_FLG2_HW_TSO_1) ||
 			 GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5705) {
-			if (tcp_opt_len || iph->ihl > 5) {
-				int tsflags;
-
-				tsflags = (iph->ihl - 5) + (tcp_opt_len >> 2);
-				mss |= (tsflags << 11);
+			if (opt_bytes > 0) {
+				mss |= opt_bytes << (11 - 2);
 			}
 		} else {
-			if (tcp_opt_len || iph->ihl > 5) {
-				int tsflags;
-
-				tsflags = (iph->ihl - 5) + (tcp_opt_len >> 2);
-				base_flags |= tsflags << 12;
+			if (opt_bytes > 0) {
+				base_flags |= opt_bytes << (12 - 2);
 			}
 		}
 	}
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 54ef984..67f7354 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -218,11 +218,6 @@ static inline unsigned int tcp_hdrlen(const struct sk_buff *skb)
 	return tcp_hdr(skb)->doff * 4;
 }
 
-static inline unsigned int tcp_optlen(const struct sk_buff *skb)
-{
-	return (tcp_hdr(skb)->doff - 5) * 4;
-}
-
 /* Fixed portion plus standard options. */
 static inline unsigned int tcp_header_len_th(const struct tcphdr *th)
 {
-- 
1.6.3.3

Re: [net-next-2.6 PATCH v9 2/2] TCPCT part 1i: remove old tcp_optlen() function

[David Miller]

These changes are probably fine but are too late for the current
merge window, please resubmit them (with any requested changes made
by reviewers) for the next merge window.

Thanks.

Re: [net-next-2.6 PATCH v9 2/2] TCPCT part 1i: remove old tcp_optlen() function

[William Allen Simpson]

David Miller wrote:
> These changes are probably fine but are too late for the current
> merge window, please resubmit them (with any requested changes made
> by reviewers) for the next merge window.
> 
> Thanks.
> 
OK.  These were split from the main part 1 sequence (looking back, both
by Ilpo's review at different times), because he objected to "cleanup"
patches being included with "new" features.  They've both been in 4 to 6
previous patch variants.

(More review would be good, and I'd be more comfortable with review by
somebody that could actually test this patch.  That's why the CC:
according to Documentation/SubmittingPatches part 13.)

I'm a bit unclear on "merge window", and am unable to find information
in the Documentation tree.  I see that there was a message that came in
the middle of my patch submission:

   Re: README: net-next-2.6 plans...

Trying to understand the process:  When I first started sending patches
in early October against the -2.6 tree, I was told privately to make
them against net-next-2.6 instead.  Does this mean these "cleanup"
patches should be made for -2.6 now?