RE: [PATCH net-next 04/10] RDMA/siw: use skb_crc32c() instead of __skb_checksum()

[Bernard Metzler]

> -----Original Message-----
> From: Eric Biggers <ebiggers@xxxxxxxxxx>
> Sent: Sunday, May 11, 2025 2:41 AM
> To: netdev@xxxxxxxxxxxxxxx
> Cc: linux-nvme@xxxxxxxxxxxxxxxxxxx; linux-sctp@xxxxxxxxxxxxxxx; linux-
> rdma@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Daniel Borkmann
> <daniel@xxxxxxxxxxxxx>; Marcelo Ricardo Leitner
> <marcelo.leitner@xxxxxxxxx>; Sagi Grimberg <sagi@xxxxxxxxxxx>; Ard
> Biesheuvel <ardb@xxxxxxxxxx>
> Subject: [EXTERNAL] [PATCH net-next 04/10] RDMA/siw: use skb_crc32c()
> instead of __skb_checksum()
> 
> From: Eric Biggers <ebiggers@xxxxxxxxxx>
> 
> Instead of calling __skb_checksum() with a skb_checksum_ops struct that
> does CRC32C, just call the new function skb_crc32c().  This is faster
> and simpler.
> 
> Signed-off-by: Eric Biggers <ebiggers@xxxxxxxxxx>
> ---
>  drivers/infiniband/sw/siw/Kconfig |  1 +
>  drivers/infiniband/sw/siw/siw.h   | 22 +---------------------
>  2 files changed, 2 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/siw/Kconfig
> b/drivers/infiniband/sw/siw/Kconfig
> index ae4a953e2a03..186f182b80e7 100644
> --- a/drivers/infiniband/sw/siw/Kconfig
> +++ b/drivers/infiniband/sw/siw/Kconfig
> @@ -1,10 +1,11 @@
>  config RDMA_SIW
>  	tristate "Software RDMA over TCP/IP (iWARP) driver"
>  	depends on INET && INFINIBAND
>  	depends on INFINIBAND_VIRT_DMA
>  	select CRC32
> +	select NET_CRC32C
>  	help
>  	This driver implements the iWARP RDMA transport over
>  	the Linux TCP/IP network stack. It enables a system with a
>  	standard Ethernet adapter to interoperate with a iWARP
>  	adapter or with another system running the SIW driver.
> diff --git a/drivers/infiniband/sw/siw/siw.h
> b/drivers/infiniband/sw/siw/siw.h
> index 385067e07faf..d9e5a2e4c471 100644
> --- a/drivers/infiniband/sw/siw/siw.h
> +++ b/drivers/infiniband/sw/siw/siw.h
> @@ -691,33 +691,13 @@ static inline void siw_crc_oneshot(const void *data,
> size_t len, u8 out[4])
>  	siw_crc_init(&crc);
>  	siw_crc_update(&crc, data, len);
>  	return siw_crc_final(&crc, out);
>  }
> 
> -static inline __wsum siw_csum_update(const void *buff, int len, __wsum
> sum)
> -{
> -	return (__force __wsum)crc32c((__force __u32)sum, buff, len);
> -}
> -
> -static inline __wsum siw_csum_combine(__wsum csum, __wsum csum2, int
> offset,
> -				      int len)
> -{
> -	return (__force __wsum)crc32c_combine((__force __u32)csum,
> -					      (__force __u32)csum2, len);
> -}
> -
>  static inline void siw_crc_skb(struct siw_rx_stream *srx, unsigned int
> len)
>  {
> -	const struct skb_checksum_ops siw_cs_ops = {
> -		.update = siw_csum_update,
> -		.combine = siw_csum_combine,
> -	};
> -	__wsum crc = (__force __wsum)srx->mpa_crc;
> -
> -	crc = __skb_checksum(srx->skb, srx->skb_offset, len, crc,
> -			     &siw_cs_ops);
> -	srx->mpa_crc = (__force u32)crc;
> +	srx->mpa_crc = skb_crc32c(srx->skb, srx->skb_offset, len, srx-
> >mpa_crc);
>  }
> 
>  #define siw_dbg(ibdev, fmt, ...)
> \
>  	ibdev_dbg(ibdev, "%s: " fmt, __func__, ##__VA_ARGS__)
> 
> --
> 2.49.0
> 

Thanks Eric!
Works fine. Correct checksum tested against siw and cxgb4 peers.

Reviewed-by: Bernard Metzler <bmt@xxxxxxxxxxxxxx>