netdev - RE: [PATCH v3 net-next 2/2] ravb: Add Tx checksum offload support for GbEth

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID:
 <TYCPR01MB11269851FEFD6FA7EEBCE724786412@TYCPR01MB11269.jpnprd01.prod.outlook.com>
Date: Sat, 3 Feb 2024 14:02:57 +0000
From: Biju Das <biju.das.jz@...renesas.com>
To: Sergey Shtylyov <s.shtylyov@....ru>, biju.das.au <biju.das.au@...il.com>
CC: "David S. Miller" <davem@...emloft.net>, Eric Dumazet
	<edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, Claudiu Beznea <claudiu.beznea.uj@...renesas.com>,
	Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>, Wolfram Sang
	<wsa+renesas@...g-engineering.com>, nikita.yoush
	<nikita.yoush@...entembedded.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "linux-renesas-soc@...r.kernel.org"
	<linux-renesas-soc@...r.kernel.org>, Geert Uytterhoeven
	<geert+renesas@...der.be>, Prabhakar Mahadev Lad
	<prabhakar.mahadev-lad.rj@...renesas.com>
Subject: RE: [PATCH v3 net-next 2/2] ravb: Add Tx checksum offload support for
 GbEth

Hi Sergey,

> -----Original Message-----
> From: Sergey Shtylyov <s.shtylyov@....ru>
> Sent: Friday, February 2, 2024 8:00 PM
> Subject: Re: [PATCH v3 net-next 2/2] ravb: Add Tx checksum offload support
> for GbEth
> 
> On 2/2/24 12:13 AM, Biju Das wrote:
> [...]
> 
> >>> TOE has hardware support for calculating IP header and TCP/UDP/ICMP
> >>> checksum for both IPv4 and IPv6.
> >>>
> >>> Add Tx checksum offload supported by TOE for IPv4 and TCP/UDP.
> >>>
> >>> 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 UDPv4 frame and its checksum value in the UDP header
> >>> field is 0x0000, TOE does not calculate checksum for UDP part of
> >>> this frame as it is optional function as per standards.
> >>>
> >>> We can test this functionality by the below commands
> >>>
> >>> ethtool -K eth0 tx on --> to turn on Tx checksum offload ethtool -K
> >>> eth0 tx off --> to turn off Tx checksum offload
> >>>
> >>> Signed-off-by: Biju Das <biju.das.jz@...renesas.com>
> [...]
> 
> >>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> >>> b/drivers/net/ethernet/renesas/ravb_main.c
> >>> index c4dc6ec54287..042dc565d1a5 100644
> >>> --- a/drivers/net/ethernet/renesas/ravb_main.c
> >>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> [...]
> >>> @@ -1986,6 +1999,35 @@ static void ravb_tx_timeout_work(struct
> >>> work_struct *work)
> [...]
> >>> +     case IPPROTO_ICMP:
> >>> +             fallthrough;
> >>
> >>    You don't even need fallthrough, actually...
> >
> > Clang Compiler will complain.
> >
> 
>    That's not like our case. If clang can't compile:
> 
> 	case IPPROTO_ICMP:
> 	default:
> 		return false;
> 
> it's seriously broken!
> 
> >>    But why do you return false for ICMP? Isn't it supported by TOE?
> >
> > It is supported by the hardware, but not the network subsystem.
> 
>    Then I don't think we should set CSR1.TICMP{4|6}, so TOE wouldn't try
> to replace the checksum in the ICMP packets...

OK will drop ICMP and send v4.

Cheers,
Biju