[PATCH net-next] iwlwifi: dont pull too much payload in skb head

[PATCH net-next] iwlwifi: dont pull too much payload in skb head

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

Since merge window is now pretty close, I would prefer David applies
this directly in net-next, if you dont mind, as this patch is more a
core network issue than an iwlwifi one.

Thanks !

[PATCH net-next] iwlwifi: dont pull too much payload in skb head

As iwlwifi use fat skbs, it should not pull too much data in skb->head,
and particularly no tcp data payload, or splice() is slower, and TCP
coalescing is disabled. Copying payload to userland also involves at
least two copies (part from header, part from fragment)

Each layer will pull its header from the fragment as needed.

(on 64bit arches, skb_tailroom(skb) at this point is 192 bytes)

With this patch applied, I have a major reduction of collapsed/pruned
TCP packets, a nice increase of TCPRcvCoalesce counter, and overall
better Internet User experience.

Small packets are still using a fragless skb, so that page can be reused
by the driver.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Johannes Berg <johannes.berg@intel.com>
Cc: Wey-Yi Guy <wey-yi.w.guy@intel.com>
---
 drivers/net/wireless/iwlwifi/iwl-agn-rx.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-rx.c b/drivers/net/wireless/iwlwifi/iwl-agn-rx.c
index 18a3837..403de96 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-rx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-rx.c
@@ -759,7 +759,12 @@ static void iwlagn_pass_packet_to_mac80211(struct iwl_priv *priv,
 		IWL_ERR(priv, "alloc_skb failed\n");
 		return;
 	}
-	hdrlen = min_t(unsigned int, len, skb_tailroom(skb));
+	/* If frame is small enough to fit in skb->head, pull it completely.
+	 * If not, only pull ieee80211_hdr so that splice() or TCP coalesce
+	 * are more efficient.
+	 */
+	hdrlen = (len <= skb_tailroom(skb)) ? len : sizeof(*hdr);
+
 	memcpy(skb_put(skb, hdrlen), hdr, hdrlen);
 	fraglen = len - hdrlen;
 

RE: [PATCH net-next] iwlwifi: dont pull too much payload in skb head

[Berg, Johannes]

> Since merge window is now pretty close, I would prefer David applies this
> directly in net-next, if you dont mind, as this patch is more a core network issue
> than an iwlwifi one.
> 
> Thanks !

Sure, good with me, I don't think we have colliding patches.

Reviewed-by: Johannes Berg <johannes.berg@intel.com>

> As iwlwifi use fat skbs, it should not pull too much data in skb->head, and
> particularly no tcp data payload, or splice() is slower, and TCP coalescing is
> disabled. Copying payload to userland also involves at least two copies (part
> from header, part from fragment)
> 
> Each layer will pull its header from the fragment as needed.
> 
> (on 64bit arches, skb_tailroom(skb) at this point is 192 bytes)
> 
> With this patch applied, I have a major reduction of collapsed/pruned TCP
> packets, a nice increase of TCPRcvCoalesce counter, and overall better Internet
> User experience.
> 
> Small packets are still using a fragless skb, so that page can be reused by the
> driver.

We may want to move this code into mac80211 later though since it also has an if (pull in everything, even reallocating if necessary, if it's a management frame), but that can wait, I think we're the only driver using paged RX.

johannes

PS: sorry about the footer -- unfortunately I haven't managed to convince IT to remove it on my @intel address
--------------------------------------------------------------------------------------
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland 
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer
Registergericht: Muenchen HRB 47456 
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

RE: [PATCH net-next] iwlwifi: dont pull too much payload in skb head

[Eric Dumazet]

On Fri, 2012-05-18 at 14:59 +0000, Berg, Johannes wrote:
> > Since merge window is now pretty close, I would prefer David applies this
> > directly in net-next, if you dont mind, as this patch is more a core network issue
> > than an iwlwifi one.
> > 
> > Thanks !
> 
> Sure, good with me, I don't think we have colliding patches.
> 
> Reviewed-by: Johannes Berg <johannes.berg@intel.com>

Thanks

> We may want to move this code into mac80211 later though since it also
> has an if (pull in everything, even reallocating if necessary, if it's
> a management frame), but that can wait, I think we're the only driver
> using paged RX.

This is OK, these frames wont be injected in linux IP/TCP stack.

Or maybe you would like an optimized version of skb_header_pointer(),
avoiding the copy if the whole blob can be part of _one_ fragment ?

Re: [PATCH net-next] iwlwifi: dont pull too much payload in skb head

[David Miller]

From: "Berg, Johannes" <johannes.berg@intel.com>
Date: Fri, 18 May 2012 14:59:04 +0000

>> Since merge window is now pretty close, I would prefer David applies this
>> directly in net-next, if you dont mind, as this patch is more a core network issue
>> than an iwlwifi one.
>> 
>> Thanks !
> 
> Sure, good with me, I don't think we have colliding patches.
> 
> Reviewed-by: Johannes Berg <johannes.berg@intel.com>

Applied.

RE: [PATCH net-next] iwlwifi: dont pull too much payload in skb head

[Berg, Johannes]

> > We may want to move this code into mac80211 later though since it also
> > has an if (pull in everything, even reallocating if necessary, if it's
> > a management frame), but that can wait, I think we're the only driver
> > using paged RX.
> 
> This is OK, these frames wont be injected in linux IP/TCP stack.

Right.

> Or maybe you would like an optimized version of skb_header_pointer(),
> avoiding the copy if the whole blob can be part of _one_ fragment ?

Hmm. I guess that would work, but I'm not sure it's worth the effort since there typically aren't many management frames. We'd have to replace all skb->data, the entire mac80211 assumes that management frames are linear. 

johannes
-- 

--------------------------------------------------------------------------------------
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland 
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer
Registergericht: Muenchen HRB 47456 
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052