From: Jarek Poplawski <jarkao2@gmail•com>
To: David Miller <davem@davemloft•net>
Cc: netdev@vger•kernel.org, Herbert Xu <herbert@gondor•apana.org.au>,
Patrick McHardy <kaber@trash•net>
Subject: [PATCH] net: Fix vlan_gro_frags vs netpoll and bonding paths
Date: Fri, 27 Aug 2010 22:50:42 +0200 [thread overview]
Message-ID: <20100827205042.GA13844@del.dom.local> (raw)
After positive netpoll_rx_on() check in vlan_gro_receive() there is
skipped part of the "common" GRO_NORMAL path, especially "pull:" in
dev_gro_receive(), where at least eth header should be copied for
entirely paged skbs. So, eth_type_trans() can read zeroed header only.
Alas moving the netpoll_rx_on() check isn't enough here because a bit
later, in vlan_gro_common(), skb_bond_should_drop() is called, which
depends on skb->protocol and skb->pkt_type data, and can also change
eth_hdr h_dest address (which is overwritten later, btw).
To fix both problems this patch completes copying and verifying of eth
header from the fragment in vlan_gro_receive(). For that purpose the
"pull:" part of dev_gro_receive() was separated into an inline as
skb_gro_pull_in(), and there was added a "lighter" version of
napi_frags_finish(). No gro path except vlan_gro_frags() should be
affected by these changes.
Signed-off-by: Jarek Poplawski <jarkao2@gmail•com>
---
include/linux/netdevice.h | 33 ++++++++++++++++++++++++++++++
net/8021q/vlan_core.c | 16 ++++++++++----
net/core/dev.c | 48 +++++++++++++++++++++++++-------------------
3 files changed, 71 insertions(+), 26 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 46c36ff..ad59f76 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1339,6 +1339,36 @@ static inline void skb_gro_pull(struct sk_buff *skb, unsigned int len)
NAPI_GRO_CB(skb)->data_offset += len;
}
+/*
+ * Complete pulling of the header(s) from the fragment
+ */
+static inline void skb_gro_pull_in(struct sk_buff *skb)
+{
+ int grow;
+
+ if (skb_headlen(skb) >= skb_gro_offset(skb))
+ return;
+
+ grow = skb_gro_offset(skb) - skb_headlen(skb);
+
+ BUG_ON(skb->end - skb->tail < grow);
+
+ memcpy(skb_tail_pointer(skb), NAPI_GRO_CB(skb)->frag0, grow);
+
+ skb->tail += grow;
+ skb->data_len -= grow;
+
+ skb_shinfo(skb)->frags[0].page_offset += grow;
+ skb_shinfo(skb)->frags[0].size -= grow;
+
+ if (unlikely(!skb_shinfo(skb)->frags[0].size)) {
+ put_page(skb_shinfo(skb)->frags[0].page);
+ memmove(skb_shinfo(skb)->frags,
+ skb_shinfo(skb)->frags + 1,
+ --skb_shinfo(skb)->nr_frags * sizeof(skb_frag_t));
+ }
+}
+
static inline void *skb_gro_header_fast(struct sk_buff *skb,
unsigned int offset)
{
@@ -1701,6 +1731,9 @@ extern struct sk_buff * napi_get_frags(struct napi_struct *napi);
extern gro_result_t napi_frags_finish(struct napi_struct *napi,
struct sk_buff *skb,
gro_result_t ret);
+extern gro_result_t __napi_frags_finish(struct napi_struct *napi,
+ struct sk_buff *skb,
+ gro_result_t ret);
extern struct sk_buff * napi_frags_skb(struct napi_struct *napi);
extern gro_result_t napi_gro_frags(struct napi_struct *napi);
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 01ddb04..58289fe 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -139,13 +139,19 @@ gro_result_t vlan_gro_frags(struct napi_struct *napi, struct vlan_group *grp,
if (!skb)
return GRO_DROP;
- if (netpoll_rx_on(skb)) {
- skb->protocol = eth_type_trans(skb, skb->dev);
+ /*
+ * Complete the eth header here, mainly for skb_bond_should_drop(),
+ * and for netpoll_rx_on() btw.
+ */
+ skb_gro_pull_in(skb);
+ skb->protocol = eth_type_trans(skb, skb->dev);
+ skb_gro_pull(skb, -ETH_HLEN);
+
+ if (netpoll_rx_on(skb))
return vlan_hwaccel_receive_skb(skb, grp, vlan_tci)
? GRO_DROP : GRO_NORMAL;
- }
- return napi_frags_finish(napi, skb,
- vlan_gro_common(napi, grp, vlan_tci, skb));
+ return __napi_frags_finish(napi, skb,
+ vlan_gro_common(napi, grp, vlan_tci, skb));
}
EXPORT_SYMBOL(vlan_gro_frags);
diff --git a/net/core/dev.c b/net/core/dev.c
index 3721fbb..cdc0be5 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3126,27 +3126,7 @@ enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff *skb)
ret = GRO_HELD;
pull:
- if (skb_headlen(skb) < skb_gro_offset(skb)) {
- int grow = skb_gro_offset(skb) - skb_headlen(skb);
-
- BUG_ON(skb->end - skb->tail < grow);
-
- memcpy(skb_tail_pointer(skb), NAPI_GRO_CB(skb)->frag0, grow);
-
- skb->tail += grow;
- skb->data_len -= grow;
-
- skb_shinfo(skb)->frags[0].page_offset += grow;
- skb_shinfo(skb)->frags[0].size -= grow;
-
- if (unlikely(!skb_shinfo(skb)->frags[0].size)) {
- put_page(skb_shinfo(skb)->frags[0].page);
- memmove(skb_shinfo(skb)->frags,
- skb_shinfo(skb)->frags + 1,
- --skb_shinfo(skb)->nr_frags * sizeof(skb_frag_t));
- }
- }
-
+ skb_gro_pull_in(skb);
ok:
return ret;
@@ -3267,6 +3247,32 @@ gro_result_t napi_frags_finish(struct napi_struct *napi, struct sk_buff *skb,
}
EXPORT_SYMBOL(napi_frags_finish);
+/*
+ * The lighter version of napi_frags_finish() without eth_type_trans() etc.
+ */
+gro_result_t __napi_frags_finish(struct napi_struct *napi, struct sk_buff *skb,
+ gro_result_t ret)
+{
+ switch (ret) {
+ case GRO_NORMAL:
+ if (netif_receive_skb(skb))
+ ret = GRO_DROP;
+ break;
+
+ case GRO_DROP:
+ case GRO_MERGED_FREE:
+ napi_reuse_skb(napi, skb);
+ break;
+
+ case GRO_HELD:
+ case GRO_MERGED:
+ break;
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL(__napi_frags_finish);
+
struct sk_buff *napi_frags_skb(struct napi_struct *napi)
{
struct sk_buff *skb = napi->skb;
next reply other threads:[~2010-08-27 20:50 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-08-27 20:50 Jarek Poplawski [this message]
2010-08-28 0:13 ` [PATCH] net: Fix vlan_gro_frags vs netpoll and bonding paths Herbert Xu
2010-08-28 9:44 ` Jarek Poplawski
2010-08-28 10:54 ` [RFC] gro: Is it ok to share a single napi from several devs ? Eric Dumazet
2010-08-28 14:31 ` Jarek Poplawski
2010-08-28 14:48 ` Eric Dumazet
2010-08-28 15:16 ` Jarek Poplawski
2010-08-28 17:14 ` Stephen Hemminger
2010-08-28 21:41 ` David Miller
2010-08-28 22:31 ` Stephen Hemminger
2010-08-28 22:33 ` David Miller
2010-08-29 9:59 ` Jarek Poplawski
2010-08-29 17:06 ` David Miller
2010-08-29 18:39 ` Eric Dumazet
2010-08-30 6:42 ` Jarek Poplawski
2010-08-30 15:57 ` Stephen Hemminger
2010-08-30 16:50 ` David Miller
2010-08-30 17:51 ` [PATCH] sky2: don't do GRO on second port Stephen Hemminger
2010-08-30 19:09 ` Jarek Poplawski
2010-09-01 21:51 ` David Miller
2010-09-01 21:55 ` Stephen Hemminger
2010-09-02 9:18 ` Jarek Poplawski
2010-09-02 12:53 ` Jarek Poplawski
2010-09-02 16:30 ` David Miller
2010-09-02 16:48 ` Jarek Poplawski
2010-09-02 8:33 ` Jarek Poplawski
2010-09-02 9:31 ` Eric Dumazet
2010-09-02 9:55 ` Jarek Poplawski
2010-09-02 10:41 ` Eric Dumazet
2010-09-02 11:02 ` Jarek Poplawski
2010-09-02 12:09 ` Eric Dumazet
2010-09-02 12:28 ` Jarek Poplawski
2010-09-02 17:08 ` David Miller
2010-09-02 21:26 ` Herbert Xu
2010-09-03 5:23 ` Eric Dumazet
2010-09-02 9:32 ` Jarek Poplawski
2010-08-30 18:36 ` [RFC] gro: Is it ok to share a single napi from several devs ? Jarek Poplawski
2010-08-30 19:59 ` [RFC] netpoll: " Eric Dumazet
2010-08-30 20:12 ` Stephen Hemminger
2010-08-30 20:19 ` Eric Dumazet
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=20100827205042.GA13844@del.dom.local \
--to=jarkao2@gmail$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=herbert@gondor$(echo .)apana.org.au \
--cc=kaber@trash$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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