All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Horman <horms@verge.net.au>
To: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Cc: David Miller <davem@davemloft.net>,
	Magnus Damm <magnus.damm@gmail.com>,
	netdev@vger.kernel.org, linux-renesas-soc@vger.kernel.org
Subject: Re: [PATCH net] ravb: expand rx descriptor data to accommodate hw checksum
Date: Fri, 11 Jan 2019 14:35:03 +0100	[thread overview]
Message-ID: <20190111133502.dma5hft5wgowdg6l@verge.net.au> (raw)
In-Reply-To: <3a5c3a17-a410-fba7-32eb-a3495c5378de@cogentembedded.com>

On Thu, Jan 10, 2019 at 06:52:51PM +0300, Sergei Shtylyov wrote:
> Hello!
> 
> On 01/10/2019 05:02 PM, Simon Horman wrote:
> 
> > EtherAVB may provide a checksum of packet data appended to packet data. In
> > order to allow this checksum to be received by the host descriptor data
> > needs to be enlarged by 2 bytes to accommodate the checksum.
> > 
> > In the case of MTU-sized packets without a VLAN tag the
> > checksum were already accommodated by virtue of the space reserved for the
> > VLAN tag. However, a packet of MTU-size with a  VLAN tag consumed all
> > packet data space provided by a descriptor leaving no space for the
> > trailing checksum.
> 
>    Wait! The gen3 manual is rather clear about the auto-checksumming not working
> right in the presence of the VLAN tag. Where do you check for that case?

In my testing on E3 this works correctly. Which portion of
the manual are you referring to?

> 
> > This was not detected by the driver which incorrectly used the last two
> > bytes of packet data as the checksum and truncate the packet by two bytes.
> > This resulted all such packets being dropped.
> > 
> > A work around is to disable rx checksum offload
> >  # ethtool -K eth0 rx off
> > 
> > This patch resolves this problem by increasing the size available for
> > packet data in rx descriptors by two bytes. It also introduces
> > RAVB_CSUM_LEN to make things a little clearer than "2" sprinkled lightly
> > over the driver.
> 
>    What about using sizeof(__sum16) instead? That type is declared in
> <linux/types.h> and used in 'struct iphdr'...

As in the following?

#define RAVB_CSUM_LEN sizeof(__sum16)

> 
> > 
> > Tested on R-Car E3 (r8a77990) ES1.0 based Ebisu-4D board
> > 
> > Fixes: 4d86d3818627 ("ravb: RX checksum offload")
> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> [...]
> 
> MBR, Sergei
> 

  reply	other threads:[~2019-01-11 13:35 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-10 14:02 [PATCH net] ravb: expand rx descriptor data to accommodate hw checksum Simon Horman
2019-01-10 15:52 ` Sergei Shtylyov
2019-01-11 13:35   ` Simon Horman [this message]
2019-01-11 15:22     ` Sergei Shtylyov
2019-01-11 15:48       ` Simon Horman
2019-01-11 16:00         ` Sergei Shtylyov
2019-01-18 14:58         ` Sergei Shtylyov

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=20190111133502.dma5hft5wgowdg6l@verge.net.au \
    --to=horms@verge.net.au \
    --cc=davem@davemloft.net \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=magnus.damm@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=sergei.shtylyov@cogentembedded.com \
    /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.