mbox series

[net-next,v2,0/2] Add HW checksum offload support for RZ/G2L GbEthernet IP

Message ID 20240124102115.132154-1-biju.das.jz@bp.renesas.com (mailing list archive)
Headers show
Series Add HW checksum offload support for RZ/G2L GbEthernet IP | expand

Message

Biju Das Jan. 24, 2024, 10:21 a.m. UTC
This patch series aims to add HW checksum offload supported by TOE module
found on the RZ/G2L Gb ethernet IP.

The TOE has hw support for calculating IP header and TCP/UDP/ICMP checksum
for both IPV4 and IPV6.

For Rx, the result of checksum calculation is attached to last 4byte
of ethernet frames. First 2bytes is result of IPV4 header checksum
and next 2 bytes is TCP/UDP/ICMP.

If frame does not have error "0000" attached to checksum calculation
result. For unsupported frames "ffff" is attached to checksum calculation
result. Cases like IPV6, IPV4 header is always set to "FFFF".

For Tx, the result of checksum calculation is set to the checksum field of
each IPv4 Header/TCP/UDP/ICMP of ethernet frames. For the unsupported
frames, those fields are not changed. If a transmission frame is an UDP
frame of IPv4 and its checksum value in the UDP header field is H’0000,
TOE does not calculate checksum for UDP part of this frame as it is
optional function as per standards.

Add Tx/Rx checksum offload supported by TOE for IPV4 and TCP/UDP protocols.

Results of iperf3 in Mbps

RZ/V2L:
TCP(Tx/Rx) results with checksum offload Enabled:	{921,932}
TCP(Tx/Rx) results with checksum offload Disabled:	{867,612}

UDP(Tx/Rx) results with checksum offload Enabled:	{950,946}
UDP(Tx/Rx) results with checksum offload Disabled:	{952,920}

RZ/G2L:
TCP(Tx/Rx) results with checksum offload Enabled:	{920,936}
TCP(Tx/Rx) results with checksum offload Disabled:	{871,626}

UDP(Tx/Rx) results with checksum offload Enabled:	{953,950}
UDP(Tx/Rx) results with checksum offload Disabled:	{954,920}

RZ/G2LC:
TCP(Tx/Rx) results with checksum offload Enabled:	{927,936}
TCP(Tx/Rx) results with checksum offload Disabled:	{889,626}

UDP(Tx/Rx) results with checksum offload Enabled:	{950,946}
UDP(Tx/Rx) results with checksum offload Disabled:	{949,944}

v1->v2:
 * Updated covering letter and results
 * Fixed the sparse warnings for patch#1 by replacing __sum16->__wsum.

Note:
 This patches are tested with [1] without the CPU performance is not good 
 [1] https://lore.kernel.org/all/20240117190545.596057-1-vincent.guittot@linaro.org/

Biju Das (2):
  ravb: Add Rx checksum offload support
  ravb: Add Tx checksum offload support

 drivers/net/ethernet/renesas/ravb.h      |  35 ++++++
 drivers/net/ethernet/renesas/ravb_main.c | 137 ++++++++++++++++++++++-
 2 files changed, 170 insertions(+), 2 deletions(-)

Comments

Sergey Shtylyov Jan. 25, 2024, 7:10 p.m. UTC | #1
Hello!

On 1/24/24 1:21 PM, Biju Das wrote:

> This patch series aims to add HW checksum offload supported by TOE module
> found on the RZ/G2L Gb ethernet IP.

   Your previous try was back in 2021, still the cover letter has the same
issues (hm, I didn't point out those back then).

> The TOE has hw support for calculating IP header and TCP/UDP/ICMP checksum
> for both IPV4 and IPV6.
> 
> For Rx, the result of checksum calculation is attached to last 4byte
> of ethernet frames.

   "For Rx, the 4-byte result of checksum calculation is attached to the
Ethernet frames", you wanted to say?

> First 2bytes is result of IPV4 header checksum
> and next 2 bytes is TCP/UDP/ICMP.

   TCP/UDP/ICMP checksum, you mean?

> If frame does not have error "0000" attached to checksum calculation

   "If a frame does not have error, 0x0000 is attached as a checksum
calculation result", you wanted to say?

> result. For unsupported frames "ffff" is attached to checksum calculation

   s/to/as/, again?

> result. Cases like IPV6, IPV4 header is always set to "FFFF".

   In case of an IPv6 packet, IPv4 checksum is always set to 0xFFFF",
you wanted to say?

> For Tx, the result of checksum calculation is set to the checksum field of
> each IPv4 Header/TCP/UDP/ICMP of ethernet frames. For the unsupported
> frames, those fields are not changed. If a transmission frame is an UDP
> frame of IPv4 and its checksum value in the UDP header field is H’0000,

   I think you can call it just UDPv4...

> TOE does not calculate checksum for UDP part of this frame as it is
> optional function as per standards.
> 
> Add Tx/Rx checksum offload supported by TOE for IPV4 and TCP/UDP protocols.
> 
> Results of iperf3 in Mbps
> 
> RZ/V2L:
> TCP(Tx/Rx) results with checksum offload Enabled:	{921,932}
> TCP(Tx/Rx) results with checksum offload Disabled:	{867,612}
> 
> UDP(Tx/Rx) results with checksum offload Enabled:	{950,946}
> UDP(Tx/Rx) results with checksum offload Disabled:	{952,920}
> 
> RZ/G2L:
> TCP(Tx/Rx) results with checksum offload Enabled:	{920,936}
> TCP(Tx/Rx) results with checksum offload Disabled:	{871,626}
> 
> UDP(Tx/Rx) results with checksum offload Enabled:	{953,950}
> UDP(Tx/Rx) results with checksum offload Disabled:	{954,920}
> 
> RZ/G2LC:
> TCP(Tx/Rx) results with checksum offload Enabled:	{927,936}
> TCP(Tx/Rx) results with checksum offload Disabled:	{889,626}
> 
> UDP(Tx/Rx) results with checksum offload Enabled:	{950,946}
> UDP(Tx/Rx) results with checksum offload Disabled:	{949,944}

   Too many figures, I think... :-)
   How RZ/G2L SoC is different from RZ/G2LC?

> v1->v2:
>  * Updated covering letter and results
>  * Fixed the sparse warnings for patch#1 by replacing __sum16->__wsum.
> 
> Note:
>  This patches are tested with [1] without the CPU performance is not good

   Without CPU? I guess the performance would be 0. Seriously, this is
hardly parseable... :-)
 
>  [1] https://lore.kernel.org/all/20240117190545.596057-1-vincent.guittot@linaro.org/
> 
> Biju Das (2):
>   ravb: Add Rx checksum offload support
>   ravb: Add Tx checksum offload support

   These summaries sound like you're adding checksum offload support for
all supported SoCs while you only do that for those having GbEther...

[...]

MBR, Sergey
Sergey Shtylyov Jan. 25, 2024, 7:11 p.m. UTC | #2
Hello!

On 1/24/24 1:21 PM, Biju Das wrote:

> This patch series aims to add HW checksum offload supported by TOE module
> found on the RZ/G2L Gb ethernet IP.

   Your previous try was back in 2021, still the cover letter has the same
issues (hm, I didn't point out those back then).

> The TOE has hw support for calculating IP header and TCP/UDP/ICMP checksum
> for both IPV4 and IPV6.
> 
> For Rx, the result of checksum calculation is attached to last 4byte
> of ethernet frames.

   "For Rx, the 4-byte result of checksum calculation is attached to the
Ethernet frames", you wanted to say?

> First 2bytes is result of IPV4 header checksum
> and next 2 bytes is TCP/UDP/ICMP.

   TCP/UDP/ICMP checksum, you mean?

> If frame does not have error "0000" attached to checksum calculation

   "If a frame does not have checksum error, 0x0000 is attached as
a checksum calculation result", you wanted to say?

> result. For unsupported frames "ffff" is attached to checksum calculation

   s/to/as/, again?

> result. Cases like IPV6, IPV4 header is always set to "FFFF".

   In case of an IPv6 packet, IPv4 checksum is always set to 0xFFFF",
you wanted to say?

> For Tx, the result of checksum calculation is set to the checksum field of
> each IPv4 Header/TCP/UDP/ICMP of ethernet frames. For the unsupported
> frames, those fields are not changed. If a transmission frame is an UDP
> frame of IPv4 and its checksum value in the UDP header field is H’0000,

   I think you can call it just UDPv4...

> TOE does not calculate checksum for UDP part of this frame as it is
> optional function as per standards.
> 
> Add Tx/Rx checksum offload supported by TOE for IPV4 and TCP/UDP protocols.
> 
> Results of iperf3 in Mbps
> 
> RZ/V2L:
> TCP(Tx/Rx) results with checksum offload Enabled:	{921,932}
> TCP(Tx/Rx) results with checksum offload Disabled:	{867,612}
> 
> UDP(Tx/Rx) results with checksum offload Enabled:	{950,946}
> UDP(Tx/Rx) results with checksum offload Disabled:	{952,920}
> 
> RZ/G2L:
> TCP(Tx/Rx) results with checksum offload Enabled:	{920,936}
> TCP(Tx/Rx) results with checksum offload Disabled:	{871,626}
> 
> UDP(Tx/Rx) results with checksum offload Enabled:	{953,950}
> UDP(Tx/Rx) results with checksum offload Disabled:	{954,920}
> 
> RZ/G2LC:
> TCP(Tx/Rx) results with checksum offload Enabled:	{927,936}
> TCP(Tx/Rx) results with checksum offload Disabled:	{889,626}
> 
> UDP(Tx/Rx) results with checksum offload Enabled:	{950,946}
> UDP(Tx/Rx) results with checksum offload Disabled:	{949,944}

   Too many figures, I think... :-)
   How RZ/G2L SoC is different from RZ/G2LC?

> v1->v2:
>  * Updated covering letter and results
>  * Fixed the sparse warnings for patch#1 by replacing __sum16->__wsum.
> 
> Note:
>  This patches are tested with [1] without the CPU performance is not good

   Without CPU? I guess the performance would be 0. Seriously, this is
hardly parseable... :-)
 
>  [1] https://lore.kernel.org/all/20240117190545.596057-1-vincent.guittot@linaro.org/
> 
> Biju Das (2):
>   ravb: Add Rx checksum offload support
>   ravb: Add Tx checksum offload support

   These summaries sound like you're adding checksum offload support for
all supported SoCs while you only do that for those having GbEther...

[...]

MBR, Sergey
Biju Das Jan. 25, 2024, 10:08 p.m. UTC | #3
Hello Sergey,

Thanks for the feedback.

> -----Original Message-----
> From: Sergey Shtylyov <s.shtylyov@omp.ru>
> Sent: Thursday, January 25, 2024 7:11 PM
> Subject: Re: [PATCH net-next v2 0/2] Add HW checksum offload support for
> RZ/G2L GbEthernet IP
> 
> Hello!
> 
> On 1/24/24 1:21 PM, Biju Das wrote:
> 
> > This patch series aims to add HW checksum offload supported by TOE
> > module found on the RZ/G2L Gb ethernet IP.
> 
>    Your previous try was back in 2021, still the cover letter has the same
> issues (hm, I didn't point out those back then).

Thanks for correcting my bad English.

> 
> > The TOE has hw support for calculating IP header and TCP/UDP/ICMP
> > checksum for both IPV4 and IPV6.
> >
> > For Rx, the result of checksum calculation is attached to last 4byte
> > of ethernet frames.
> 
>    "For Rx, the 4-byte result of checksum calculation is attached to the
> Ethernet frames", you wanted to say?

Ok.

> 
> > First 2bytes is result of IPV4 header checksum and next 2 bytes is
> > TCP/UDP/ICMP.
> 
>    TCP/UDP/ICMP checksum, you mean?

Yes.

> 
> > If frame does not have error "0000" attached to checksum calculation
> 
>    "If a frame does not have error, 0x0000 is attached as a checksum
> calculation result", you wanted to say?
> 
> > result. For unsupported frames "ffff" is attached to checksum
> > calculation
> 
>    s/to/as/, again?

OK.

> 
> > result. Cases like IPV6, IPV4 header is always set to "FFFF".
> 
>    In case of an IPv6 packet, IPv4 checksum is always set to 0xFFFF", you
> wanted to say?
> 
> > For Tx, the result of checksum calculation is set to the checksum
> > field of each IPv4 Header/TCP/UDP/ICMP of ethernet frames. For the
> > unsupported frames, those fields are not changed. If a transmission
> > frame is an UDP frame of IPv4 and its checksum value in the UDP header
> > field is H'0000,
> 
>    I think you can call it just UDPv4...

OK.

> 
> > TOE does not calculate checksum for UDP part of this frame as it is
> > optional function as per standards.
> >
> > Add Tx/Rx checksum offload supported by TOE for IPV4 and TCP/UDP
> protocols.
> >
> > Results of iperf3 in Mbps
> >
> > RZ/V2L:
> > TCP(Tx/Rx) results with checksum offload Enabled:	{921,932}
> > TCP(Tx/Rx) results with checksum offload Disabled:	{867,612}
> >
> > UDP(Tx/Rx) results with checksum offload Enabled:	{950,946}
> > UDP(Tx/Rx) results with checksum offload Disabled:	{952,920}
> >
> > RZ/G2L:
> > TCP(Tx/Rx) results with checksum offload Enabled:	{920,936}
> > TCP(Tx/Rx) results with checksum offload Disabled:	{871,626}
> >
> > UDP(Tx/Rx) results with checksum offload Enabled:	{953,950}
> > UDP(Tx/Rx) results with checksum offload Disabled:	{954,920}
> >
> > RZ/G2LC:
> > TCP(Tx/Rx) results with checksum offload Enabled:	{927,936}
> > TCP(Tx/Rx) results with checksum offload Disabled:	{889,626}
> >
> > UDP(Tx/Rx) results with checksum offload Enabled:	{950,946}
> > UDP(Tx/Rx) results with checksum offload Disabled:	{949,944}
> 
>    Too many figures, I think... :-)
>    How RZ/G2L SoC is different from RZ/G2LC?

Just want to share with the wider community how the HW checksum is
improving the performance of various SoCs in the RZ/G2L family.

and the results show improved performance on all 3 SoCs.
> 
> > v1->v2:
> >  * Updated covering letter and results
> >  * Fixed the sparse warnings for patch#1 by replacing __sum16->__wsum.
> >
> > Note:
> >  This patches are tested with [1] without the CPU performance is not
> > good
> 
>    Without CPU? I guess the performance would be 0. Seriously, this is
> hardly parseable... :-)

without the patch [1] CPU performance is not good which impacts the
network throughput.

[1] https://lore.kernel.org/all/20240117190545.596057-1-vincent.guittot@linaro.org/

Cheers,
Biju
Sergey Shtylyov Jan. 26, 2024, 7:01 p.m. UTC | #4
On 1/26/24 1:08 AM, Biju Das wrote:
[...]

>> -----Original Message-----
>> From: Sergey Shtylyov <s.shtylyov@omp.ru>
>> Sent: Thursday, January 25, 2024 7:11 PM
>> Subject: Re: [PATCH net-next v2 0/2] Add HW checksum offload support for
>> RZ/G2L GbEthernet IP
>>
>> Hello!
>>
>> On 1/24/24 1:21 PM, Biju Das wrote:
>>
>>> This patch series aims to add HW checksum offload supported by TOE
>>> module found on the RZ/G2L Gb ethernet IP.
>>
>>    Your previous try was back in 2021, still the cover letter has the same
>> issues (hm, I didn't point out those back then).
> 
> Thanks for correcting my bad English.

   I don't think you were the author of the e.g. RZ/G2L User's Manual that
has the same wording... Or were you? :-)

[...]

>>> TOE does not calculate checksum for UDP part of this frame as it is
>>> optional function as per standards.
>>>
>>> Add Tx/Rx checksum offload supported by TOE for IPV4 and TCP/UDP
>> protocols.
>>>
>>> Results of iperf3 in Mbps
>>>
>>> RZ/V2L:
>>> TCP(Tx/Rx) results with checksum offload Enabled:	{921,932}
>>> TCP(Tx/Rx) results with checksum offload Disabled:	{867,612}
>>>
>>> UDP(Tx/Rx) results with checksum offload Enabled:	{950,946}
>>> UDP(Tx/Rx) results with checksum offload Disabled:	{952,920}
>>>
>>> RZ/G2L:
>>> TCP(Tx/Rx) results with checksum offload Enabled:	{920,936}
>>> TCP(Tx/Rx) results with checksum offload Disabled:	{871,626}
>>>
>>> UDP(Tx/Rx) results with checksum offload Enabled:	{953,950}
>>> UDP(Tx/Rx) results with checksum offload Disabled:	{954,920}
>>>
>>> RZ/G2LC:
>>> TCP(Tx/Rx) results with checksum offload Enabled:	{927,936}
>>> TCP(Tx/Rx) results with checksum offload Disabled:	{889,626}
>>>
>>> UDP(Tx/Rx) results with checksum offload Enabled:	{950,946}
>>> UDP(Tx/Rx) results with checksum offload Disabled:	{949,944}
>>
>>    Too many figures, I think... :-)
>>    How RZ/G2L SoC is different from RZ/G2LC?

   At least they are described by a single manual...

> Just want to share with the wider community how the HW checksum is
> improving the performance of various SoCs in the RZ/G2L family.
> 
> and the results show improved performance on all 3 SoCs.

   I guess RZ/V2L and RZ/G2L would've been enough... but I'm probably
quibbling... :-)

>>> v1->v2:
>>>  * Updated covering letter and results
>>>  * Fixed the sparse warnings for patch#1 by replacing __sum16->__wsum.
>>>
>>> Note:
>>>  This patches are tested with [1] without the CPU performance is not
>>> good
>>
>>    Without CPU? I guess the performance would be 0. Seriously, this is
>> hardly parseable... :-)
> 
> without the patch [1] CPU performance is not good which impacts the
> network throughput.
> 
> [1] https://lore.kernel.org/all/20240117190545.596057-1-vincent.guittot@linaro.org/

   Thanks, that's much better. :-)

> Cheers,
> Biju

MBR, Sergey