From: William Allen Simpson <william.allen.simpson@gmail.com>
To: Linux Kernel Network Developers <netdev@vger.kernel.org>
Cc: Michael Chan <mchan@broadcom.com>
Subject: Re: [PATCH v2 RFC 2/5] TCPCT part 2b: remove old tcp_optlen function
Date: Thu, 31 Dec 2009 14:44:21 -0500 [thread overview]
Message-ID: <4B3CFF15.7070308@gmail.com> (raw)
In-Reply-To: <4B3CFC73.3070204@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 653 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.
No semantic changes. The drivers should function exactly as existing.
Untested. No response from testers in 9+ weeks.
Requires:
TCPCT part 2a: TCP header pointer functions
Signed-off-by: William.Allen.Simpson@gmail.com
CC: Michael Chan <mchan@broadcom.com>
---
drivers/net/bnx2.c | 26 +++++++++++++--------
drivers/net/tg3.c | 59 ++++++++++++++++++++++++++-------------------------
include/linux/tcp.h | 5 ----
3 files changed, 46 insertions(+), 44 deletions(-)
[-- Attachment #2: TCPCT+2b2.patch --]
[-- Type: text/plain, Size: 6942 bytes --]
diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 65df1de..3fc4912 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -6352,6 +6352,11 @@ bnx2_vlan_rx_register(struct net_device *dev, struct vlan_group *vlgrp)
/* Called with netif_tx_lock.
* bnx2_tx_int() runs without netif_tx_lock unless it needs to call
* netif_wake_queue().
+ *
+ * Quoth David Miller:
+ * This is transmit, and the packets can only come from the Linux TCP stack,
+ * not some external entity. ... adding [length] checks to drivers would be
+ * just a lot of rediculious bloat. [sic]
*/
static netdev_tx_t
bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)
@@ -6397,18 +6402,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;
@@ -6421,11 +6425,13 @@ 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);
+ /* assumes positive ip_opt_words without checking */
+ int opt_words = ip_opt_words + tcp_opt_words;
+
+ if (opt_words > 0)
+ vlan_tag_flags |= opt_words << 8;
}
} else
mss = 0;
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 3a74d21..31179db 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -5421,6 +5421,11 @@ static void tg3_set_txd(struct tg3_napi *tnapi, int entry,
/* hard_start_xmit for devices that don't have any bugs and
* support TG3_FLG2_HW_TSO_2 and TG3_FLG2_HW_TSO_3 only.
+ *
+ * Quoth David Miller:
+ * This is transmit, and the packets can only come from the Linux TCP stack,
+ * not some external entity. ... adding [length] checks to drivers would be
+ * just a lot of rediculious bloat. [sic]
*/
static netdev_tx_t tg3_start_xmit(struct sk_buff *skb,
struct net_device *dev)
@@ -5458,7 +5463,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) &&
@@ -5466,18 +5471,16 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb,
dev_kfree_skb(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) {
@@ -5491,7 +5494,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)
@@ -5624,6 +5627,11 @@ tg3_tso_bug_end:
/* hard_start_xmit for devices that have the 4G bug and/or 40-bit bug and
* support TG3_FLG2_HW_TSO_1 or firmware TSO only.
+ *
+ * Quoth David Miller:
+ * This is transmit, and the packets can only come from the Linux TCP stack,
+ * not some external entity. ... adding [length] checks to drivers would be
+ * just a lot of rediculious bloat. [sic]
*/
static netdev_tx_t tg3_start_xmit_dma_bug(struct sk_buff *skb,
struct net_device *dev)
@@ -5665,18 +5673,18 @@ 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)) {
dev_kfree_skb(skb);
goto out_unlock;
}
+ th = tcp_hdr(skb);
+ hdr_len = ip_hdrlen(skb) + tcp_header_len_th(th);
- tcp_opt_len = tcp_optlen(skb);
- ip_tcp_len = ip_hdrlen(skb) + sizeof(struct tcphdr);
-
- 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));
@@ -5688,13 +5696,14 @@ 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);
+
+ opt_bytes = hdr_len - sizeof(*iph) - sizeof(*th);
+ /* assumes positive opt_bytes without checking */
if (tp->tg3_flags2 & TG3_FLG2_HW_TSO_3) {
mss |= (hdr_len & 0xc) << 12;
@@ -5705,19 +5714,11 @@ static netdev_tx_t tg3_start_xmit_dma_bug(struct sk_buff *skb,
mss |= hdr_len << 9;
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);
}
}
#if TG3_VLAN_TAG_USED
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index d0133cf..74728f7 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;
-}
-
/* Length of fixed header 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-31 19:44 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-12-31 19:33 [PATCH v2 0/5 RFC] TCPCT part 2: cleanup after part 1 William Allen Simpson
2009-12-31 19:38 ` [PATCH v2 RFC 1/5] TCPCT part 2a: TCP header pointer functions William Allen Simpson
2009-12-31 19:44 ` William Allen Simpson [this message]
2009-12-31 19:52 ` [PATCH v2 RFC 3/5] TCPCT part 2c: accept SYNACK data William Allen Simpson
2009-12-31 19:58 ` [PATCH v2 RFC 4/5] TCPCT part 2d: cleanup tcp_parse_options William Allen Simpson
2009-12-31 20:06 ` [PATCH v2 RFC 5/5] TCPCT part 2e: parse cookie pair and 64-bit timestamp 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=4B3CFF15.7070308@gmail.com \
--to=william.allen.simpson@gmail.com \
--cc=mchan@broadcom.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).