All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Duyck <alexander.duyck@gmail.com>
To: intel-wired-lan@osuosl.org
Subject: [Intel-wired-lan] [Patch net] igb: pass the correct maxlen for eth_get_headlen()
Date: Wed, 22 Apr 2015 12:43:45 -0700	[thread overview]
Message-ID: <5537F9F1.1080008@gmail.com> (raw)
In-Reply-To: <1429724760-10075-1-git-send-email-xiyou.wangcong@gmail.com>

On 04/22/2015 10:45 AM, Cong Wang wrote:
> The second parameter of eth_get_headlen() is the length of
> the frame buffer, not the header length of skb.
>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
>  drivers/net/ethernet/intel/igb/igb_main.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index a0a9b1f..7b3a370 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -6852,7 +6852,9 @@ static void igb_pull_tail(struct igb_ring *rx_ring,
>  	/* we need the header to contain the greater of either ETH_HLEN or
>  	 * 60 bytes if the skb->len is less than 60 for skb_pad.
>  	 */
> -	pull_len = eth_get_headlen(va, IGB_RX_HDR_LEN);
> +	pull_len = eth_get_headlen(va, skb_frag_size(frag));
> +	if (unlikely(pull_len > IGB_RX_HDR_LEN))
> +		pull_len = IGB_RX_HDR_LEN;
>  
>  	/* align pull length to size of long to optimize memcpy performance */
>  	skb_copy_to_linear_data(skb, va, ALIGN(pull_len, sizeof(long)));

You have this part right.  The length represents the maximum length we
are willing to traverse in the buffer.  So if we 100% want to get the
entire header regardless of what we can copy into then we could follow
your approach.  However, since the allocated space in the skb is only
IGB_RX_HDR_LEN we only really want to traverse up to that length.  Then
that is all we copy out of the header.

As a result we don't need the extra code for putting the upper limit on
pull_len since that is factored in by passing IGB_RX_HDR_LEN as the
maximum traversal length.

- Alex

WARNING: multiple messages have this Message-ID (diff)
From: Alexander Duyck <alexander.duyck@gmail.com>
To: Cong Wang <xiyou.wangcong@gmail.com>, netdev@vger.kernel.org
Cc: intel-wired-lan@lists.osuosl.org,
	Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Subject: Re: [Patch net] igb: pass the correct maxlen for eth_get_headlen()
Date: Wed, 22 Apr 2015 12:43:45 -0700	[thread overview]
Message-ID: <5537F9F1.1080008@gmail.com> (raw)
In-Reply-To: <1429724760-10075-1-git-send-email-xiyou.wangcong@gmail.com>

On 04/22/2015 10:45 AM, Cong Wang wrote:
> The second parameter of eth_get_headlen() is the length of
> the frame buffer, not the header length of skb.
>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
>  drivers/net/ethernet/intel/igb/igb_main.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index a0a9b1f..7b3a370 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -6852,7 +6852,9 @@ static void igb_pull_tail(struct igb_ring *rx_ring,
>  	/* we need the header to contain the greater of either ETH_HLEN or
>  	 * 60 bytes if the skb->len is less than 60 for skb_pad.
>  	 */
> -	pull_len = eth_get_headlen(va, IGB_RX_HDR_LEN);
> +	pull_len = eth_get_headlen(va, skb_frag_size(frag));
> +	if (unlikely(pull_len > IGB_RX_HDR_LEN))
> +		pull_len = IGB_RX_HDR_LEN;
>  
>  	/* align pull length to size of long to optimize memcpy performance */
>  	skb_copy_to_linear_data(skb, va, ALIGN(pull_len, sizeof(long)));

You have this part right.  The length represents the maximum length we
are willing to traverse in the buffer.  So if we 100% want to get the
entire header regardless of what we can copy into then we could follow
your approach.  However, since the allocated space in the skb is only
IGB_RX_HDR_LEN we only really want to traverse up to that length.  Then
that is all we copy out of the header.

As a result we don't need the extra code for putting the upper limit on
pull_len since that is factored in by passing IGB_RX_HDR_LEN as the
maximum traversal length.

- Alex

  parent reply	other threads:[~2015-04-22 19:43 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-22 17:45 [Intel-wired-lan] [Patch net] igb: pass the correct maxlen for eth_get_headlen() Cong Wang
2015-04-22 17:45 ` Cong Wang
2015-04-22 17:45 ` [Intel-wired-lan] [Patch net] fm10k: " Cong Wang
2015-04-22 17:45   ` Cong Wang
2015-04-22 17:45 ` [Intel-wired-lan] [Patch net] ixgbe: " Cong Wang
2015-04-22 17:45   ` Cong Wang
2015-04-22 17:46 ` [Intel-wired-lan] [Patch net] ixgbevf: " Cong Wang
2015-04-22 17:46   ` Cong Wang
2015-04-22 19:43 ` Alexander Duyck [this message]
2015-04-22 19:43   ` [Patch net] igb: " Alexander Duyck
2015-04-22 20:14   ` [Intel-wired-lan] " Cong Wang
2015-04-22 20:14     ` Cong Wang
2015-04-22 20:21     ` [Intel-wired-lan] " Alexander Duyck
2015-04-22 20:21       ` Alexander Duyck
2015-04-22 20:33       ` [Intel-wired-lan] " Cong Wang
2015-04-22 20:33         ` Cong Wang
2015-04-22 21:42         ` [Intel-wired-lan] " Alexander Duyck
2015-04-22 21:42           ` Alexander Duyck
2015-04-22 21:56           ` [Intel-wired-lan] " Cong Wang
2015-04-22 21:56             ` Cong Wang
2015-04-22 22:34             ` [Intel-wired-lan] " Alexander Duyck
2015-04-22 22:34               ` Alexander Duyck
2015-04-22 23:23               ` [Intel-wired-lan] " Cong Wang
2015-04-22 23:23                 ` Cong Wang
2015-04-23  3:40                 ` [Intel-wired-lan] " Alexander Duyck
2015-04-23  3:40                   ` Alexander Duyck
2015-04-23 18:06                   ` [Intel-wired-lan] " Cong Wang
2015-04-23 18:06                     ` Cong Wang
2015-04-23 18:40                     ` [Intel-wired-lan] " Alexander Duyck
2015-04-23 18:40                       ` Alexander Duyck
2015-04-23 19:14                       ` [Intel-wired-lan] " Cong Wang
2015-04-23 19:14                         ` Cong Wang
2015-04-23 19:30                 ` [Intel-wired-lan] " Cong Wang
2015-04-23 19:30                   ` Cong Wang
2015-04-23 19:45                   ` [Intel-wired-lan] " Alexander Duyck
2015-04-23 19:45                     ` Alexander Duyck
2015-04-23 22:07                     ` Cong Wang
2015-04-23 22:07                       ` Cong Wang
2015-04-25  1:07 ` Jeff Kirsher
2015-04-25  1:07   ` Jeff Kirsher

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=5537F9F1.1080008@gmail.com \
    --to=alexander.duyck@gmail.com \
    --cc=intel-wired-lan@osuosl.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.