Re: [patch 06/23] Fix SKB_WITH_OVERHEAD calculations.

!MAILaRCHIVE_VOTE_RePLACE
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: <herbert@...>
Cc: <gregkh@...>, <linux-kernel@...>, <stable@...>, <jmforbes@...>, <zwane@...>, <tytso@...>, <rdunlap@...>, <davej@...>, <chuckw@...>, <reviews@...>, <mkrufky@...>, <cebbert@...>, <cavokz@...>, <torvalds@...>, <akpm@...>, <alan@...>, <bunk@...>
Date: Thursday, November 15, 2007 - 3:00 am

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 15 Nov 2007 14:29:39 +0800


I was just about to mention this and what I think we should do
instead is keep the SKB_WITH_OVERHEAD calculation fix in there
(it fixes a serious bug which users are hitting) and add the
fix for the second bug fix this round as well.

Therefore, Greg please keep the patch add the following one as it will
address all of the issues.

[TCP]: Fix size calculation in sk_stream_alloc_pskb

[ Upstream commit: fb93134dfc2a6e6fbedc7c270a31da03fce88db9 ]

We round up the header size in sk_stream_alloc_pskb so that
TSO packets get zero tail room.  Unfortunately this rounding
up is not coordinated with the select_size() function used by
TCP to calculate the second parameter of sk_stream_alloc_pskb.

As a result, we may allocate more than a page of data in the
non-TSO case when exactly one page is desired.

In fact, rounding up the head room is detrimental in the non-TSO
case because it makes memory that would otherwise be available to
the payload head room.  TSO doesn't need this either, all it wants
is the guarantee that there is no tail room.

So this patch fixes this by adjusting the skb_reserve call so that
exactly the requested amount (which all callers have calculated in
a precise way) is made available as tail room.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/include/net/sock.h b/include/net/sock.h
index 5504fb9..567e468 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1235,14 +1235,16 @@ static inline struct sk_buff *sk_stream_alloc_pskb(struct sock *sk,
 						   gfp_t gfp)
 {
 	struct sk_buff *skb;
-	int hdr_len;
 
-	hdr_len = SKB_DATA_ALIGN(sk->sk_prot->max_header);
-	skb = alloc_skb_fclone(size + hdr_len, gfp);
+	skb = alloc_skb_fclone(size + sk->sk_prot->max_header, gfp);
 	if (skb) {
 		skb->truesize += mem;
 		if (sk_stream_wmem_schedule(sk, skb->truesize)) {
-			skb_reserve(skb, hdr_len);
+			/*
+			 * Make sure that we have exactly size bytes
+			 * available to the caller, no more, no less.
+			 */
+			skb_reserve(skb, skb_tailroom(skb) - size);
 			return skb;
 		}
 		__kfree_skb(skb);
-
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
[patch 17/23] ieee80211: fix TKIP QoS bug, Greg KH, (Thu Nov 15, 2:20 am)
[patch 15/23] Fix netlink timeouts., Greg KH, (Thu Nov 15, 2:20 am)
[patch 12/23] Fix VLAN address syncing., Greg KH, (Thu Nov 15, 2:20 am)
[patch 10/23] Fix TEQL oops., Greg KH, (Thu Nov 15, 2:20 am)
[patch 05/23] Fix 9P protocol build, Greg KH, (Thu Nov 15, 2:20 am)
Re: [patch 06/23] Fix SKB_WITH_OVERHEAD calculations., David Miller, (Thu Nov 15, 3:00 am)