Re: [PATCH 3/6] RDMA/rxe: switch to using the crc32 library

[Zhu Yanjun <yanjun.zhu@linux.dev>]

在 2025/1/27 23:38, Eric Biggers 写道:
> From: Eric Biggers <ebiggers@google.com>
> 
> Now that the crc32() library function takes advantage of
> architecture-specific optimizations, it is unnecessary to go through the
> crypto API.  Just use crc32().  This is much simpler, and it improves
> performance due to eliminating the crypto API overhead.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>

After the patchset is applied, if the RXE can connect to MLX RDMA NIC 
successfully, I am fine with it.

Reviewed-by: Zhu Yanjun <yanjun.zhu@linux.dev>

Zhu Yanjun

> ---
>   drivers/infiniband/sw/rxe/Kconfig     |  3 +-
>   drivers/infiniband/sw/rxe/rxe.c       |  3 --
>   drivers/infiniband/sw/rxe/rxe.h       |  1 -
>   drivers/infiniband/sw/rxe/rxe_icrc.c  | 61 ++-------------------------
>   drivers/infiniband/sw/rxe/rxe_loc.h   |  1 -
>   drivers/infiniband/sw/rxe/rxe_req.c   |  1 -
>   drivers/infiniband/sw/rxe/rxe_verbs.c |  4 --
>   drivers/infiniband/sw/rxe/rxe_verbs.h |  1 -
>   8 files changed, 5 insertions(+), 70 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/Kconfig b/drivers/infiniband/sw/rxe/Kconfig
> index 06b8dc5093f77..c180e7ebcfc5b 100644
> --- a/drivers/infiniband/sw/rxe/Kconfig
> +++ b/drivers/infiniband/sw/rxe/Kconfig
> @@ -2,12 +2,11 @@
>   config RDMA_RXE
>   	tristate "Software RDMA over Ethernet (RoCE) driver"
>   	depends on INET && PCI && INFINIBAND
>   	depends on INFINIBAND_VIRT_DMA
>   	select NET_UDP_TUNNEL
> -	select CRYPTO
> -	select CRYPTO_CRC32
> +	select CRC32
>   	help
>   	This driver implements the InfiniBand RDMA transport over
>   	the Linux network stack. It enables a system with a
>   	standard Ethernet adapter to interoperate with a RoCE
>   	adapter or with another system running the RXE driver.
> diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c
> index 1ba4a0c8726ae..f8ac79ef70143 100644
> --- a/drivers/infiniband/sw/rxe/rxe.c
> +++ b/drivers/infiniband/sw/rxe/rxe.c
> @@ -29,13 +29,10 @@ void rxe_dealloc(struct ib_device *ib_dev)
>   	rxe_pool_cleanup(&rxe->mr_pool);
>   	rxe_pool_cleanup(&rxe->mw_pool);
>   
>   	WARN_ON(!RB_EMPTY_ROOT(&rxe->mcg_tree));
>   
> -	if (rxe->tfm)
> -		crypto_free_shash(rxe->tfm);
> -
>   	mutex_destroy(&rxe->usdev_lock);
>   }
>   
>   /* initialize rxe device parameters */
>   static void rxe_init_device_param(struct rxe_dev *rxe)
> diff --git a/drivers/infiniband/sw/rxe/rxe.h b/drivers/infiniband/sw/rxe/rxe.h
> index fe7f970667325..8db65731499d0 100644
> --- a/drivers/infiniband/sw/rxe/rxe.h
> +++ b/drivers/infiniband/sw/rxe/rxe.h
> @@ -19,11 +19,10 @@
>   #include <rdma/ib_pack.h>
>   #include <rdma/ib_smi.h>
>   #include <rdma/ib_umem.h>
>   #include <rdma/ib_cache.h>
>   #include <rdma/ib_addr.h>
> -#include <crypto/hash.h>
>   
>   #include "rxe_net.h"
>   #include "rxe_opcode.h"
>   #include "rxe_hdr.h"
>   #include "rxe_param.h"
> diff --git a/drivers/infiniband/sw/rxe/rxe_icrc.c b/drivers/infiniband/sw/rxe/rxe_icrc.c
> index c7b0b4673b959..63d03f0f71e38 100644
> --- a/drivers/infiniband/sw/rxe/rxe_icrc.c
> +++ b/drivers/infiniband/sw/rxe/rxe_icrc.c
> @@ -7,62 +7,10 @@
>   #include <linux/crc32.h>
>   
>   #include "rxe.h"
>   #include "rxe_loc.h"
>   
> -/**
> - * rxe_icrc_init() - Initialize crypto function for computing crc32
> - * @rxe: rdma_rxe device object
> - *
> - * Return: 0 on success else an error
> - */
> -int rxe_icrc_init(struct rxe_dev *rxe)
> -{
> -	struct crypto_shash *tfm;
> -
> -	tfm = crypto_alloc_shash("crc32", 0, 0);
> -	if (IS_ERR(tfm)) {
> -		rxe_dbg_dev(rxe, "failed to init crc32 algorithm err: %ld\n",
> -			       PTR_ERR(tfm));
> -		return PTR_ERR(tfm);
> -	}
> -
> -	rxe->tfm = tfm;
> -
> -	return 0;
> -}
> -
> -/**
> - * rxe_crc32() - Compute cumulative crc32 for a contiguous segment
> - * @rxe: rdma_rxe device object
> - * @crc: starting crc32 value from previous segments
> - * @next: starting address of current segment
> - * @len: length of current segment
> - *
> - * Return: the cumulative crc32 checksum
> - */
> -static u32 rxe_crc32(struct rxe_dev *rxe, u32 crc, void *next, size_t len)
> -{
> -	u32 icrc;
> -	int err;
> -
> -	SHASH_DESC_ON_STACK(shash, rxe->tfm);
> -
> -	shash->tfm = rxe->tfm;
> -	*(u32 *)shash_desc_ctx(shash) = crc;
> -	err = crypto_shash_update(shash, next, len);
> -	if (unlikely(err)) {
> -		rxe_dbg_dev(rxe, "failed crc calculation, err: %d\n", err);
> -		return crc32_le(crc, next, len);
> -	}
> -
> -	icrc = *(u32 *)shash_desc_ctx(shash);
> -	barrier_data(shash_desc_ctx(shash));
> -
> -	return icrc;
> -}
> -
>   /**
>    * rxe_icrc() - Compute the ICRC of a packet
>    * @skb: packet buffer
>    * @pkt: packet information
>    *
> @@ -117,19 +65,18 @@ static u32 rxe_icrc(struct sk_buff *skb, struct rxe_pkt_info *pkt)
>   
>   	/* exclude bth.resv8a */
>   	bth->qpn |= cpu_to_be32(~BTH_QPN_MASK);
>   
>   	/* Update the CRC with the first part of the headers. */
> -	crc = rxe_crc32(pkt->rxe, crc, pshdr, hdr_size + RXE_BTH_BYTES);
> +	crc = crc32(crc, pshdr, hdr_size + RXE_BTH_BYTES);
>   
>   	/* Update the CRC with the remainder of the headers. */
> -	crc = rxe_crc32(pkt->rxe, crc, pkt->hdr + RXE_BTH_BYTES,
> -			rxe_opcode[pkt->opcode].length - RXE_BTH_BYTES);
> +	crc = crc32(crc, pkt->hdr + RXE_BTH_BYTES,
> +		    rxe_opcode[pkt->opcode].length - RXE_BTH_BYTES);
>   
>   	/* Update the CRC with the payload. */
> -	crc = rxe_crc32(pkt->rxe, crc, payload_addr(pkt),
> -			payload_size(pkt) + bth_pad(pkt));
> +	crc = crc32(crc, payload_addr(pkt), payload_size(pkt) + bth_pad(pkt));
>   
>   	/* Invert the CRC and return it. */
>   	return ~crc;
>   }
>   
> diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
> index ded46119151bb..c57ab8975c5d1 100644
> --- a/drivers/infiniband/sw/rxe/rxe_loc.h
> +++ b/drivers/infiniband/sw/rxe/rxe_loc.h
> @@ -166,11 +166,10 @@ int rxe_completer(struct rxe_qp *qp);
>   int rxe_requester(struct rxe_qp *qp);
>   int rxe_sender(struct rxe_qp *qp);
>   int rxe_receiver(struct rxe_qp *qp);
>   
>   /* rxe_icrc.c */
> -int rxe_icrc_init(struct rxe_dev *rxe);
>   int rxe_icrc_check(struct sk_buff *skb, struct rxe_pkt_info *pkt);
>   void rxe_icrc_generate(struct sk_buff *skb, struct rxe_pkt_info *pkt);
>   
>   void rxe_resp_queue_pkt(struct rxe_qp *qp, struct sk_buff *skb);
>   
> diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
> index 87a02f0deb000..9d0392df8a92f 100644
> --- a/drivers/infiniband/sw/rxe/rxe_req.c
> +++ b/drivers/infiniband/sw/rxe/rxe_req.c
> @@ -3,11 +3,10 @@
>    * Copyright (c) 2016 Mellanox Technologies Ltd. All rights reserved.
>    * Copyright (c) 2015 System Fabric Works, Inc. All rights reserved.
>    */
>   
>   #include <linux/skbuff.h>
> -#include <crypto/hash.h>
>   
>   #include "rxe.h"
>   #include "rxe_loc.h"
>   #include "rxe_queue.h"
>   
> diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
> index 6152a0fdfc8ca..c05379f8b4f57 100644
> --- a/drivers/infiniband/sw/rxe/rxe_verbs.c
> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
> @@ -1531,14 +1531,10 @@ int rxe_register_device(struct rxe_dev *rxe, const char *ibdev_name,
>   	ib_set_device_ops(dev, &rxe_dev_ops);
>   	err = ib_device_set_netdev(&rxe->ib_dev, ndev, 1);
>   	if (err)
>   		return err;
>   
> -	err = rxe_icrc_init(rxe);
> -	if (err)
> -		return err;
> -
>   	err = ib_register_device(dev, ibdev_name, NULL);
>   	if (err)
>   		rxe_dbg_dev(rxe, "failed with error %d\n", err);
>   
>   	/*
> diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h
> index 6573ceec0ef58..6e31134a5fa5b 100644
> --- a/drivers/infiniband/sw/rxe/rxe_verbs.h
> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.h
> @@ -400,11 +400,10 @@ struct rxe_dev {
>   	u64			mmap_offset;
>   
>   	atomic64_t		stats_counters[RXE_NUM_OF_COUNTERS];
>   
>   	struct rxe_port		port;
> -	struct crypto_shash	*tfm;
>   };
>   
>   static inline struct net_device *rxe_ib_device_get_netdev(struct ib_device *dev)
>   {
>   	return ib_device_get_netdev(dev, RXE_PORT);