From: Simon Horman <horms@verge.net.au>
To: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Cc: netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
linux-renesas-soc@vger.kernel.org, linux-sh@vger.kernel.org
Subject: Re: [PATCH 2/7] sh_eth: RX checksum offload support
Date: Tue, 29 Jan 2019 08:58:10 +0100 [thread overview]
Message-ID: <20190129075810.rqeekk5j4npjh6wf@verge.net.au> (raw)
In-Reply-To: <d95fff4f-7750-87c4-3664-eb5efb593b75@cogentembedded.com>
Hi Sergei,
On Mon, Jan 28, 2019 at 06:45:26PM +0300, Sergei Shtylyov wrote:
> Hello!
>
> On 01/28/2019 03:18 PM, Simon Horman wrote:
>
> >> Add support for the RX checksum offload. This is enabled by default and
> >> may be disabled and re-enabled using 'ethtool':
> >>
> >> # ethtool -K eth0 rx {on|off}
> >>
> >> Some Ether MACs provide a simple checksumming scheme which appears to be
> >> completely compatible with CHECKSUM_COMPLETE: sum of all packet data after
> >> the L2 header is appended to packet data; this may be trivially read by
> >> the driver and used to update the skb accordingly. The same checksumming
> >> scheme is implemented in the EtherAVB MACs and now supported by tha 'ravb'
> >> driver.
> >>
> >> In terms of performance, throughput is close to gigabit line rate with the
> >> RX checksum offload both enabled and disabled. The 'perf' output, however,
> >> appears to indicate that significantly less time is spent in do_csum() --
> >> this is as expected.
> >
> > Nice.
> >
> > FYI, this seems similar to what I observed for RAVB, perhaps on H3 I don't
> > exactly recall. On E3, which has less CPU power, I recently observed that
> > with rx-csum enabled I can achieve gigabit line rate, but with rx-csum
> > disabled throughput is significantly lower. I.e. on that system throughput
> > is CPU bound with 1500 byte packets unless rx-csum enabled.
>
> Unfortunately, we can't teset these patches on the other gen3 boards. ISTR
> you have RZ/A1H board... if it's still with you, I'd appreciate testing.
Unfortunately, as of a few weeks ago, I no longer have that board.
> > Next point:
> >
> > 2da64300fbc ("ravb: expand rx descriptor data to accommodate hw checksum")
> > is fresh in my mind and I wonder if mdp->rx_buf_sz needs to grow to ensure
> > that there is always enough space for the csum.
>
> Well, if you look at sh_eth_ring_init(), you'll see that the driver reserves
> plenty of space at the end the RX buffers.
Yes, I see that. And I assume that was enough space before this patch.
But is it still enough space now that 2 bytes are needed for the hardware csum?
2 bytes that might have previously been used as packet data in some
circumstances.
> > In particular, have you
> > tested this with MTU-size frames with VLANs. (My test is to run iperf3 over
> > a VLAN netdev, netperf over a VLAN netdev would likely work just as well.)
>
> Could you refresh me on how to bring up a VLAN on a given interface?
You will need a kernel with CONFIG_VLAN_8021Q enabled.
Then you can do something like this:
ip link add link eth0 name eth0.1 type vlan id 1
ip addr add 10.1.1.100/24 dev eth0.1
ip link set dev eth0.1 up
> [...]
> >> The above results collected on the R-Car V3H Starter Kit board.
> >>
> >> Based on the commit 4d86d3818627 ("ravb: RX checksum offload")...
> >>
> >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> [...]
>
> MBR, Sergei
>
next prev parent reply other threads:[~2019-01-29 7:58 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-27 17:33 [PATCH 0/7] sh_eth: implement simple RX checksum offload Sergei Shtylyov
2019-01-27 17:36 ` [PATCH 1/7] sh_eth: rename sh_eth_cpu_data::hw_checksum Sergei Shtylyov
2019-01-28 9:21 ` Geert Uytterhoeven
2019-01-28 11:08 ` Sergei Shtylyov
2019-01-28 19:15 ` David Miller
2019-01-27 17:37 ` [PATCH 2/7] sh_eth: RX checksum offload support Sergei Shtylyov
2019-01-28 12:18 ` Simon Horman
2019-01-28 15:45 ` Sergei Shtylyov
2019-01-29 7:58 ` Simon Horman [this message]
2019-01-29 15:43 ` Sergei Shtylyov
2019-01-30 10:06 ` Simon Horman
2019-01-27 17:38 ` [PATCH 3/7] sh_eth: offload RX checksum on R7S72100 Sergei Shtylyov
2019-01-28 12:20 ` Simon Horman
2019-01-28 15:21 ` Sergei Shtylyov
2019-01-29 8:00 ` Simon Horman
2019-01-29 10:37 ` Sergei Shtylyov
2019-01-29 15:02 ` Chris Brandt
2019-01-29 16:03 ` Sergei Shtylyov
2019-01-30 10:08 ` Simon Horman
2019-01-27 17:39 ` [PATCH 4/7] sh_eth: offload RX checksum on R8A7740 Sergei Shtylyov
2019-01-29 18:20 ` Geert Uytterhoeven
2019-01-31 10:52 ` Sergei Shtylyov
2019-01-31 11:11 ` Geert Uytterhoeven
2019-01-27 17:40 ` [PATCH 5/7] sh_eth: offload RX checksum on R8A77980 Sergei Shtylyov
2019-01-27 17:41 ` [PATCH 6/7] sh_eth: offload RX checksum on SH7734 Sergei Shtylyov
2019-01-27 17:42 ` [PATCH 7/7] sh_eth: offload RX checksum on SH7763 Sergei Shtylyov
2019-02-04 11:55 ` Rob Landley
2019-02-04 15:17 ` Sergei Shtylyov
2019-01-27 17:52 ` [PATCH 0/7] sh_eth: implement simple RX checksum offload Heiner Kallweit
2019-01-29 11:06 ` 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=20190129075810.rqeekk5j4npjh6wf@verge.net.au \
--to=horms@verge.net.au \
--cc=davem@davemloft.net \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=linux-sh@vger.kernel.org \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).