[net-next,0/8] Improve GbEth performance on Renesas RZ/G2L and related SoCs

Message ID 20240131170523.30048-1-paul.barker.ct@bp.renesas.com
Headers
Series Improve GbEth performance on Renesas RZ/G2L and related SoCs |

Message

Paul Barker Jan. 31, 2024, 5:05 p.m. UTC
  This series aims to improve peformance of the GbEth IP in the Renesas
RZ/G2L SoC family and the RZ/G3S SoC, which use the ravb driver. Along
the way, we do some refactoring and ensure that napi_complete_done() is
used in accordance with the NAPI documentation.

Performance improvment mainly comes from enabling SW IRQ Coalescing for
all SoCs using the GbEth IP, and NAPI Threaded mode for single core SoCs
using the GbEth IP. These can be enabled/disabled at runtime via sysfs,
but our goal is to set sensible defaults which get good performance on
the affected SoCs.

Changes are made specific to the GbEth IP, avoiding potential impact on
the other Renesas R-Car based SoCs which also use the ravb driver. This
follows the principle of only submitting patches that we can fully test.

The performance impact of this series on iperf3 testing is as follows:
  * RZ/G2L Ethernet throughput is unchanged, but CPU usage drops:
      * Bidirectional and TCP RX: 6.5% less CPU usage
      * UDP RX: 10% less CPU usage

  * RZ/G2UL and RZ/G3S Ethernet throughput is increased for all test
    cases except UDP TX, which suffers a slight loss:
      * TCP TX: 32% more throughput
      * TCP RX: 11% more throughput
      * UDP TX: 10% less throughput
      * UDP RX: 10183% more throughput - the previous throughput of
        1.06Mbps is what prompted this work.

  * RZ/G2N CPU usage and Ethernet throughput is unchanged (tested as a
    representative of the SoCs which use the R-Car based RAVB IP).

This series depends on:
  * "net: ravb: Let IP-specific receive function to interrogate descriptors" v5
    https://lore.kernel.org/all/20240131084133.1671440-2-claudiu.beznea.uj@bp.renesas.com/

To get the results shown above, you'll also need:
  * "topology: Set capacity_freq_ref in all cases"
    https://lore.kernel.org/all/20240117190545.596057-1-vincent.guittot@linaro.org/

  * "ravb: Add Rx checksum offload support" v2
    https://lore.kernel.org/all/20240124102115.132154-2-biju.das.jz@bp.renesas.com/

  * "ravb: Add Tx checksum offload support" v2
    https://lore.kernel.org/all/20240124102115.132154-3-biju.das.jz@bp.renesas.com/

Work in this area will continue, in particular we expect to improve
TCP/UDP RX performance further with future changes to RX buffer
handling.

Paul Barker (8):
  net: ravb: Split R-Car & GbEth poll functions
  net: ravb: Simplify GbEth poll & receive functions
  net: ravb: Count packets in GbEth RX (not descriptors)
  net: ravb: Always process TX descriptor ring in GbEth poll
  net: ravb: Always update error counters
  net: ravb: Align GbEth poll function with NAPI docs
  net: ravb: Enable SW IRQ Coalescing for GbEth
  net: ravb: Use NAPI threaded mode on 1-core CPUs with GbEth IP

 drivers/net/ethernet/renesas/ravb.h      |   3 +-
 drivers/net/ethernet/renesas/ravb_main.c | 103 ++++++++++++++++-------
 2 files changed, 74 insertions(+), 32 deletions(-)
  

Comments

Andrew Lunn Jan. 31, 2024, 6:26 p.m. UTC | #1
> Changes are made specific to the GbEth IP, avoiding potential impact on
> the other Renesas R-Car based SoCs which also use the ravb driver. This
> follows the principle of only submitting patches that we can fully test.
 
Are you saying that Renesas does not have access to all Renesas RDKs?

I don't particularly like the way your first patch makes a copy of
shared functions. Is it not likely that R-Car would also benefit from
this?

	Andrew
  
Sergey Shtylyov Feb. 1, 2024, 7:22 p.m. UTC | #2
On 1/31/24 8:05 PM, Paul Barker wrote:

> This series aims to improve peformance of the GbEth IP in the Renesas

   Performance.

> RZ/G2L SoC family and the RZ/G3S SoC, which use the ravb driver. Along
> the way, we do some refactoring and ensure that napi_complete_done() is
> used in accordance with the NAPI documentation.
> 
> Performance improvment mainly comes from enabling SW IRQ Coalescing for

   Improvement.

> all SoCs using the GbEth IP, and NAPI Threaded mode for single core SoCs
> using the GbEth IP. These can be enabled/disabled at runtime via sysfs,
> but our goal is to set sensible defaults which get good performance on
> the affected SoCs.
> 
> Changes are made specific to the GbEth IP, avoiding potential impact on
> the other Renesas R-Car based SoCs which also use the ravb driver. This
> follows the principle of only submitting patches that we can fully test.
> 
> The performance impact of this series on iperf3 testing is as follows:
>   * RZ/G2L Ethernet throughput is unchanged, but CPU usage drops:
>       * Bidirectional and TCP RX: 6.5% less CPU usage
>       * UDP RX: 10% less CPU usage
> 
>   * RZ/G2UL and RZ/G3S Ethernet throughput is increased for all test
>     cases except UDP TX, which suffers a slight loss:
>       * TCP TX: 32% more throughput
>       * TCP RX: 11% more throughput
>       * UDP TX: 10% less throughput
>       * UDP RX: 10183% more throughput - the previous throughput of

   10183%, really? 8-)

>         1.06Mbps is what prompted this work.
> 
>   * RZ/G2N CPU usage and Ethernet throughput is unchanged (tested as a
>     representative of the SoCs which use the R-Car based RAVB IP).
> 
> This series depends on:
>   * "net: ravb: Let IP-specific receive function to interrogate descriptors" v5
>     https://lore.kernel.org/all/20240131084133.1671440-2-claudiu.beznea.uj@bp.renesas.com/

   So this is based on yet unmerged patch?
   Dave, Jakub, et al., the series should be considered RFC then.

> To get the results shown above, you'll also need:
>   * "topology: Set capacity_freq_ref in all cases"
>     https://lore.kernel.org/all/20240117190545.596057-1-vincent.guittot@linaro.org/
> 
>   * "ravb: Add Rx checksum offload support" v2
>     https://lore.kernel.org/all/20240124102115.132154-2-biju.das.jz@bp.renesas.com/
> 
>   * "ravb: Add Tx checksum offload support" v2
>     https://lore.kernel.org/all/20240124102115.132154-3-biju.das.jz@bp.renesas.com/

   Those 2 are not finalized yet...

> Work in this area will continue, in particular we expect to improve
> TCP/UDP RX performance further with future changes to RX buffer
> handling.
> 
> Paul Barker (8):
>   net: ravb: Split R-Car & GbEth poll functions
>   net: ravb: Simplify GbEth poll & receive functions
>   net: ravb: Count packets in GbEth RX (not descriptors)
>   net: ravb: Always process TX descriptor ring in GbEth poll
>   net: ravb: Always update error counters
>   net: ravb: Align GbEth poll function with NAPI docs
>   net: ravb: Enable SW IRQ Coalescing for GbEth
>   net: ravb: Use NAPI threaded mode on 1-core CPUs with GbEth IP
> 
>  drivers/net/ethernet/renesas/ravb.h      |   3 +-
>  drivers/net/ethernet/renesas/ravb_main.c | 103 ++++++++++++++++-------
>  2 files changed, 74 insertions(+), 32 deletions(-)

MBR, Sergey
  
Paul Barker Feb. 2, 2024, 9:39 a.m. UTC | #3
On 31/01/2024 18:26, Andrew Lunn wrote:
>> Changes are made specific to the GbEth IP, avoiding potential impact on
>> the other Renesas R-Car based SoCs which also use the ravb driver. This
>> follows the principle of only submitting patches that we can fully test.
>  
> Are you saying that Renesas does not have access to all Renesas RDKs?
> 
> I don't particularly like the way your first patch makes a copy of
> shared functions. Is it not likely that R-Car would also benefit from
> this?

We have the required RDKs. For the R-Car based SoCs, we need to confirm
that gPTP still works if we change the poll/receive code paths - this
will require an AVB-capable network switch and additional time to test.
So our plan was to handle the GbEth code paths first without affecting
R-Car, then follow up with another patch set for the R-Car code paths
when we've done the required tests.

I discussed this with our team, and we're happy to do this in one go for
both R-Car and GbEth code paths if that's preferred. I'll send the
patches as an RFC (as Sergey has commented it should be an RFC anyway as
it depends on an unmerged patch) and we'll do the gPTP test with a
couple of R-Car boards.

Thanks,
Paul
  
Andrew Lunn Feb. 2, 2024, 1:30 p.m. UTC | #4
On Fri, Feb 02, 2024 at 09:39:42AM +0000, Paul Barker wrote:
> On 31/01/2024 18:26, Andrew Lunn wrote:
> >> Changes are made specific to the GbEth IP, avoiding potential impact on
> >> the other Renesas R-Car based SoCs which also use the ravb driver. This
> >> follows the principle of only submitting patches that we can fully test.
> >  
> > Are you saying that Renesas does not have access to all Renesas RDKs?
> > 
> > I don't particularly like the way your first patch makes a copy of
> > shared functions. Is it not likely that R-Car would also benefit from
> > this?
> 
> We have the required RDKs. For the R-Car based SoCs, we need to confirm
> that gPTP still works if we change the poll/receive code paths - this
> will require an AVB-capable network switch and additional time to test.
> So our plan was to handle the GbEth code paths first without affecting
> R-Car, then follow up with another patch set for the R-Car code paths
> when we've done the required tests.
> 
> I discussed this with our team, and we're happy to do this in one go for
> both R-Car and GbEth code paths if that's preferred.

Hi Paul

I think it would be simpler, since you would then need to recombine
the code paths you have just split. Its better to not split them in
the first place if possible.

    Andrew