From: William Allen Simpson <william.allen.simpson@gmail.com>
To: Linux Kernel Network Developers <netdev@vger.kernel.org>
Subject: Re: [net-next-2.6 PATCH v9 2/2] TCPCT part 1i: remove old tcp_optlen() function
Date: Thu, 03 Dec 2009 18:54:41 -0500 [thread overview]
Message-ID: <4B184FC1.8020805@gmail.com> (raw)
In-Reply-To: <4B184A20.4050005@gmail.com>
[-- 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
next prev parent reply other threads:[~2009-12-03 23:54 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-12-03 23:30 [net-next-2.6 PATCH v9 0/2] TCPCT part 1: cleanup William Allen Simpson
2009-12-03 23:40 ` [net-next-2.6 PATCH v9 0/2] TCPCT part 1h: accept SYNACK data William Allen Simpson
2009-12-03 23:54 ` William Allen Simpson [this message]
2009-12-03 23:56 ` [net-next-2.6 PATCH v9 2/2] TCPCT part 1i: remove old tcp_optlen() function David Miller
2009-12-04 12:06 ` William Allen Simpson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4B184FC1.8020805@gmail.com \
--to=william.allen.simpson@gmail.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).