All of lore.kernel.org
 help / color / mirror / Atom feed
From: Herbert Xu <herbert@gondor.apana.org.au>
To: Jarek Poplawski <jarkao2@gmail.com>
Cc: David Miller <davem@davemloft.net>,
	netdev@vger.kernel.org, Patrick McHardy <kaber@trash.net>
Subject: Re: [PATCH] net: Fix vlan_gro_frags vs netpoll and bonding paths
Date: Sat, 28 Aug 2010 08:13:37 +0800	[thread overview]
Message-ID: <20100828001337.GA1955@gondor.apana.org.au> (raw)
In-Reply-To: <20100827205042.GA13844@del.dom.local>

On Fri, Aug 27, 2010 at 10:50:42PM +0200, Jarek Poplawski wrote:
> 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.

Right, thanks for catching this.

> 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);

But this code should go into the netpoll (i.e., slow-path) case
only so as not to impede performance.

Also, we need to fix this for the non-VLAN case as well.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

  reply	other threads:[~2010-08-28  0:13 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-27 20:50 [PATCH] net: Fix vlan_gro_frags vs netpoll and bonding paths Jarek Poplawski
2010-08-28  0:13 ` Herbert Xu [this message]
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=20100828001337.GA1955@gondor.apana.org.au \
    --to=herbert@gondor.apana.org.au \
    --cc=davem@davemloft.net \
    --cc=jarkao2@gmail.com \
    --cc=kaber@trash.net \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.