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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Sergey,

> -----Original Message-----
> From: Sergey Shtylyov <s.shtylyov@xxxxxx>
> 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@xxxxxxxxxxxxxx>
> [...]
> 
> >>> 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





[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux