On Sat, Jun 03, 2006 at 07:51:23PM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@ wrote:
>
> I think that people will start thinking why we cannot
> skb_pull(skb, len) if skb_headlen(skb) == len; some comment needed...
Good idea. Here is a better one.
[TCP]: Avoid skb_pull if possible when trimming head
Trimming the head of an skb by calling skb_pull can cause the packet
to become unaligned if the length pulled is odd. Since the length is
entirely arbitrary for a FIN packet carrying data, this is actually
quite common.
Unaligned data is not the end of the world, but we should avoid it if
it's easily done. In this case it is trivial. Since we're discarding
all of the head data it doesn't matter whether we move skb->data forward
or back.
However, it is still possible to have unaligned skb->data in general.
So network drivers should be prepared to handle it instead of crashing.
This patch also adds an unlikely marking on len < headlen since partial
ACKs on head data are extremely rare in the wild. As the return value
of __pskb_trim_head is no longer ever NULL that has been removed.
Signed-off-by: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]>
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 743016b..f33c9dd 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -642,7 +642,7 @@ int tcp_fragment(struct sock *sk, struct
* eventually). The difference is that pulled data not copied, but
* immediately discarded.
*/
-static unsigned char *__pskb_trim_head(struct sk_buff *skb, int len)
+static void __pskb_trim_head(struct sk_buff *skb, int len)
{
int i, k, eat;
@@ -667,7 +667,6 @@ static unsigned char *__pskb_trim_head(s
skb->tail = skb->data;
skb->data_len -= len;
skb->len = skb->data_len;
- return skb->tail;
}
int tcp_trim_head(struct sock *sk, struct sk_buff *skb, u32 len)
@@ -676,12 +675,11 @@ int tcp_trim_head(struct sock *sk, struc
pskb_expand_head(skb, 0, 0, GFP_ATOMIC))
return -ENOMEM;
- if (len <= skb_headlen(skb)) {
+ /* If len == headlen, we avoid __skb_pull to preserve alignment. */
+ if (unlikely(len < skb_headlen(skb)))
__skb_pull(skb, len);
- } else {
- if (__pskb_trim_head(skb, len-skb_headlen(skb)) == NULL)
- return -ENOMEM;
- }
+ else
+ __pskb_trim_head(skb, len - skb_headlen(skb));
TCP_SKB_CB(skb)->seq += len;
skb->ip_summed = CHECKSUM_HW;
