From 92186c1455a2d3563dcea58a6f4729d518b5be50 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 6 Feb 2025 20:17:24 -0800 Subject: [PATCH] scsi: iscsi_tcp: Switch to using the crc32c library Now that the crc32c() library function directly takes advantage of architecture-specific optimizations, it is unnecessary to go through the crypto API. Just use crc32c(). This is much simpler, and it improves performance due to eliminating the crypto API overhead. Signed-off-by: Eric Biggers Link: https://lore.kernel.org/r/20250207041724.70733-1-ebiggers@kernel.org Signed-off-by: Martin K. Petersen --- drivers/scsi/Kconfig | 2 +- drivers/scsi/iscsi_tcp.c | 60 ++++-------------------- drivers/scsi/iscsi_tcp.h | 4 +- drivers/scsi/libiscsi_tcp.c | 91 ++++++++++++++++--------------------- include/scsi/libiscsi_tcp.h | 16 +++---- 5 files changed, 57 insertions(+), 116 deletions(-) diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig index c749d376159a..5a3c670aec27 100644 --- a/drivers/scsi/Kconfig +++ b/drivers/scsi/Kconfig @@ -303,9 +303,9 @@ if SCSI_LOWLEVEL && SCSI config ISCSI_TCP tristate "iSCSI Initiator over TCP/IP" depends on SCSI && INET + select CRC32 select CRYPTO select CRYPTO_MD5 - select CRYPTO_CRC32C select SCSI_ISCSI_ATTRS help The iSCSI Driver provides a host with the ability to access storage diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c index e81f60985193..7b4fe0e6afb2 100644 --- a/drivers/scsi/iscsi_tcp.c +++ b/drivers/scsi/iscsi_tcp.c @@ -17,7 +17,6 @@ * Zhenyu Wang */ -#include #include #include #include @@ -468,8 +467,7 @@ static void iscsi_sw_tcp_send_hdr_prep(struct iscsi_conn *conn, void *hdr, * sufficient room. */ if (conn->hdrdgst_en) { - iscsi_tcp_dgst_header(tcp_sw_conn->tx_hash, hdr, hdrlen, - hdr + hdrlen); + iscsi_tcp_dgst_header(hdr, hdrlen, hdr + hdrlen); hdrlen += ISCSI_DIGEST_SIZE; } @@ -494,7 +492,7 @@ iscsi_sw_tcp_send_data_prep(struct iscsi_conn *conn, struct scatterlist *sg, { struct iscsi_tcp_conn *tcp_conn = conn->dd_data; struct iscsi_sw_tcp_conn *tcp_sw_conn = tcp_conn->dd_data; - struct ahash_request *tx_hash = NULL; + u32 *tx_crcp = NULL; unsigned int hdr_spec_len; ISCSI_SW_TCP_DBG(conn, "offset=%d, datalen=%d %s\n", offset, len, @@ -507,11 +505,10 @@ iscsi_sw_tcp_send_data_prep(struct iscsi_conn *conn, struct scatterlist *sg, WARN_ON(iscsi_padded(len) != iscsi_padded(hdr_spec_len)); if (conn->datadgst_en) - tx_hash = tcp_sw_conn->tx_hash; + tx_crcp = &tcp_sw_conn->tx_crc; return iscsi_segment_seek_sg(&tcp_sw_conn->out.data_segment, - sg, count, offset, len, - NULL, tx_hash); + sg, count, offset, len, NULL, tx_crcp); } static void @@ -520,7 +517,7 @@ iscsi_sw_tcp_send_linear_data_prep(struct iscsi_conn *conn, void *data, { struct iscsi_tcp_conn *tcp_conn = conn->dd_data; struct iscsi_sw_tcp_conn *tcp_sw_conn = tcp_conn->dd_data; - struct ahash_request *tx_hash = NULL; + u32 *tx_crcp = NULL; unsigned int hdr_spec_len; ISCSI_SW_TCP_DBG(conn, "datalen=%zd %s\n", len, conn->datadgst_en ? @@ -532,10 +529,10 @@ iscsi_sw_tcp_send_linear_data_prep(struct iscsi_conn *conn, void *data, WARN_ON(iscsi_padded(len) != iscsi_padded(hdr_spec_len)); if (conn->datadgst_en) - tx_hash = tcp_sw_conn->tx_hash; + tx_crcp = &tcp_sw_conn->tx_crc; iscsi_segment_init_linear(&tcp_sw_conn->out.data_segment, - data, len, NULL, tx_hash); + data, len, NULL, tx_crcp); } static int iscsi_sw_tcp_pdu_init(struct iscsi_task *task, @@ -583,7 +580,6 @@ iscsi_sw_tcp_conn_create(struct iscsi_cls_session *cls_session, struct iscsi_cls_conn *cls_conn; struct iscsi_tcp_conn *tcp_conn; struct iscsi_sw_tcp_conn *tcp_sw_conn; - struct crypto_ahash *tfm; cls_conn = iscsi_tcp_conn_setup(cls_session, sizeof(*tcp_sw_conn), conn_idx); @@ -596,37 +592,9 @@ iscsi_sw_tcp_conn_create(struct iscsi_cls_session *cls_session, tcp_sw_conn->queue_recv = iscsi_recv_from_iscsi_q; mutex_init(&tcp_sw_conn->sock_lock); - - tfm = crypto_alloc_ahash("crc32c", 0, CRYPTO_ALG_ASYNC); - if (IS_ERR(tfm)) - goto free_conn; - - tcp_sw_conn->tx_hash = ahash_request_alloc(tfm, GFP_KERNEL); - if (!tcp_sw_conn->tx_hash) - goto free_tfm; - ahash_request_set_callback(tcp_sw_conn->tx_hash, 0, NULL, NULL); - - tcp_sw_conn->rx_hash = ahash_request_alloc(tfm, GFP_KERNEL); - if (!tcp_sw_conn->rx_hash) - goto free_tx_hash; - ahash_request_set_callback(tcp_sw_conn->rx_hash, 0, NULL, NULL); - - tcp_conn->rx_hash = tcp_sw_conn->rx_hash; + tcp_conn->rx_crcp = &tcp_sw_conn->rx_crc; return cls_conn; - -free_tx_hash: - ahash_request_free(tcp_sw_conn->tx_hash); -free_tfm: - crypto_free_ahash(tfm); -free_conn: - iscsi_conn_printk(KERN_ERR, conn, - "Could not create connection due to crc32c " - "loading error. Make sure the crc32c " - "module is built as a module or into the " - "kernel\n"); - iscsi_tcp_conn_teardown(cls_conn); - return NULL; } static void iscsi_sw_tcp_release_conn(struct iscsi_conn *conn) @@ -664,20 +632,8 @@ static void iscsi_sw_tcp_release_conn(struct iscsi_conn *conn) static void iscsi_sw_tcp_conn_destroy(struct iscsi_cls_conn *cls_conn) { struct iscsi_conn *conn = cls_conn->dd_data; - struct iscsi_tcp_conn *tcp_conn = conn->dd_data; - struct iscsi_sw_tcp_conn *tcp_sw_conn = tcp_conn->dd_data; iscsi_sw_tcp_release_conn(conn); - - ahash_request_free(tcp_sw_conn->rx_hash); - if (tcp_sw_conn->tx_hash) { - struct crypto_ahash *tfm; - - tfm = crypto_ahash_reqtfm(tcp_sw_conn->tx_hash); - ahash_request_free(tcp_sw_conn->tx_hash); - crypto_free_ahash(tfm); - } - iscsi_tcp_conn_teardown(cls_conn); } diff --git a/drivers/scsi/iscsi_tcp.h b/drivers/scsi/iscsi_tcp.h index 89a6fc552f0b..c3e5d9fa6add 100644 --- a/drivers/scsi/iscsi_tcp.h +++ b/drivers/scsi/iscsi_tcp.h @@ -41,8 +41,8 @@ struct iscsi_sw_tcp_conn { void (*old_write_space)(struct sock *); /* data and header digests */ - struct ahash_request *tx_hash; /* CRC32C (Tx) */ - struct ahash_request *rx_hash; /* CRC32C (Rx) */ + u32 tx_crc; /* CRC32C (Tx) */ + u32 rx_crc; /* CRC32C (Rx) */ /* MIB custom statistics */ uint32_t sendpage_failures_cnt; diff --git a/drivers/scsi/libiscsi_tcp.c b/drivers/scsi/libiscsi_tcp.c index c182aa83f2c9..e90805ba868f 100644 --- a/drivers/scsi/libiscsi_tcp.c +++ b/drivers/scsi/libiscsi_tcp.c @@ -15,7 +15,7 @@ * Zhenyu Wang */ -#include +#include #include #include #include @@ -168,7 +168,7 @@ iscsi_tcp_segment_splice_digest(struct iscsi_segment *segment, void *digest) segment->size = ISCSI_DIGEST_SIZE; segment->copied = 0; segment->sg = NULL; - segment->hash = NULL; + segment->crcp = NULL; } /** @@ -191,29 +191,27 @@ int iscsi_tcp_segment_done(struct iscsi_tcp_conn *tcp_conn, struct iscsi_segment *segment, int recv, unsigned copied) { - struct scatterlist sg; unsigned int pad; ISCSI_DBG_TCP(tcp_conn->iscsi_conn, "copied %u %u size %u %s\n", segment->copied, copied, segment->size, recv ? "recv" : "xmit"); - if (segment->hash && copied) { - /* - * If a segment is kmapd we must unmap it before sending - * to the crypto layer since that will try to kmap it again. - */ - iscsi_tcp_segment_unmap(segment); - - if (!segment->data) { - sg_init_table(&sg, 1); - sg_set_page(&sg, sg_page(segment->sg), copied, - segment->copied + segment->sg_offset + - segment->sg->offset); - } else - sg_init_one(&sg, segment->data + segment->copied, - copied); - ahash_request_set_crypt(segment->hash, &sg, NULL, copied); - crypto_ahash_update(segment->hash); + if (segment->crcp && copied) { + if (segment->data) { + *segment->crcp = crc32c(*segment->crcp, + segment->data + segment->copied, + copied); + } else { + const void *data; + + data = kmap_local_page(sg_page(segment->sg)); + *segment->crcp = crc32c(*segment->crcp, + data + segment->copied + + segment->sg_offset + + segment->sg->offset, + copied); + kunmap_local(data); + } } segment->copied += copied; @@ -258,10 +256,8 @@ int iscsi_tcp_segment_done(struct iscsi_tcp_conn *tcp_conn, * Set us up for transferring the data digest. hdr digest * is completely handled in hdr done function. */ - if (segment->hash) { - ahash_request_set_crypt(segment->hash, NULL, - segment->digest, 0); - crypto_ahash_final(segment->hash); + if (segment->crcp) { + put_unaligned_le32(~*segment->crcp, segment->digest); iscsi_tcp_segment_splice_digest(segment, recv ? segment->recv_digest : segment->digest); return 0; @@ -282,8 +278,7 @@ EXPORT_SYMBOL_GPL(iscsi_tcp_segment_done); * given buffer, and returns the number of bytes * consumed, which can actually be less than @len. * - * If hash digest is enabled, the function will update the - * hash while copying. + * If CRC is enabled, the function will update the CRC while copying. * Combining these two operations doesn't buy us a lot (yet), * but in the future we could implement combined copy+crc, * just way we do for network layer checksums. @@ -311,14 +306,10 @@ iscsi_tcp_segment_recv(struct iscsi_tcp_conn *tcp_conn, } inline void -iscsi_tcp_dgst_header(struct ahash_request *hash, const void *hdr, - size_t hdrlen, unsigned char digest[ISCSI_DIGEST_SIZE]) +iscsi_tcp_dgst_header(const void *hdr, size_t hdrlen, + unsigned char digest[ISCSI_DIGEST_SIZE]) { - struct scatterlist sg; - - sg_init_one(&sg, hdr, hdrlen); - ahash_request_set_crypt(hash, &sg, digest, hdrlen); - crypto_ahash_digest(hash); + put_unaligned_le32(~crc32c(~0, hdr, hdrlen), digest); } EXPORT_SYMBOL_GPL(iscsi_tcp_dgst_header); @@ -343,24 +334,23 @@ iscsi_tcp_dgst_verify(struct iscsi_tcp_conn *tcp_conn, */ static inline void __iscsi_segment_init(struct iscsi_segment *segment, size_t size, - iscsi_segment_done_fn_t *done, struct ahash_request *hash) + iscsi_segment_done_fn_t *done, u32 *crcp) { memset(segment, 0, sizeof(*segment)); segment->total_size = size; segment->done = done; - if (hash) { - segment->hash = hash; - crypto_ahash_init(hash); + if (crcp) { + segment->crcp = crcp; + *crcp = ~0; } } inline void iscsi_segment_init_linear(struct iscsi_segment *segment, void *data, - size_t size, iscsi_segment_done_fn_t *done, - struct ahash_request *hash) + size_t size, iscsi_segment_done_fn_t *done, u32 *crcp) { - __iscsi_segment_init(segment, size, done, hash); + __iscsi_segment_init(segment, size, done, crcp); segment->data = data; segment->size = size; } @@ -370,13 +360,12 @@ inline int iscsi_segment_seek_sg(struct iscsi_segment *segment, struct scatterlist *sg_list, unsigned int sg_count, unsigned int offset, size_t size, - iscsi_segment_done_fn_t *done, - struct ahash_request *hash) + iscsi_segment_done_fn_t *done, u32 *crcp) { struct scatterlist *sg; unsigned int i; - __iscsi_segment_init(segment, size, done, hash); + __iscsi_segment_init(segment, size, done, crcp); for_each_sg(sg_list, sg, sg_count, i) { if (offset < sg->length) { iscsi_tcp_segment_init_sg(segment, sg, offset); @@ -393,7 +382,7 @@ EXPORT_SYMBOL_GPL(iscsi_segment_seek_sg); * iscsi_tcp_hdr_recv_prep - prep segment for hdr reception * @tcp_conn: iscsi connection to prep for * - * This function always passes NULL for the hash argument, because when this + * This function always passes NULL for the crcp argument, because when this * function is called we do not yet know the final size of the header and want * to delay the digest processing until we know that. */ @@ -434,15 +423,15 @@ static void iscsi_tcp_data_recv_prep(struct iscsi_tcp_conn *tcp_conn) { struct iscsi_conn *conn = tcp_conn->iscsi_conn; - struct ahash_request *rx_hash = NULL; + u32 *rx_crcp = NULL; if (conn->datadgst_en && !(conn->session->tt->caps & CAP_DIGEST_OFFLOAD)) - rx_hash = tcp_conn->rx_hash; + rx_crcp = tcp_conn->rx_crcp; iscsi_segment_init_linear(&tcp_conn->in.segment, conn->data, tcp_conn->in.datalen, - iscsi_tcp_data_recv_done, rx_hash); + iscsi_tcp_data_recv_done, rx_crcp); } /** @@ -730,7 +719,7 @@ iscsi_tcp_hdr_dissect(struct iscsi_conn *conn, struct iscsi_hdr *hdr) if (tcp_conn->in.datalen) { struct iscsi_tcp_task *tcp_task = task->dd_data; - struct ahash_request *rx_hash = NULL; + u32 *rx_crcp = NULL; struct scsi_data_buffer *sdb = &task->sc->sdb; /* @@ -743,7 +732,7 @@ iscsi_tcp_hdr_dissect(struct iscsi_conn *conn, struct iscsi_hdr *hdr) */ if (conn->datadgst_en && !(conn->session->tt->caps & CAP_DIGEST_OFFLOAD)) - rx_hash = tcp_conn->rx_hash; + rx_crcp = tcp_conn->rx_crcp; ISCSI_DBG_TCP(conn, "iscsi_tcp_begin_data_in( " "offset=%d, datalen=%d)\n", @@ -756,7 +745,7 @@ iscsi_tcp_hdr_dissect(struct iscsi_conn *conn, struct iscsi_hdr *hdr) tcp_task->data_offset, tcp_conn->in.datalen, iscsi_tcp_process_data_in, - rx_hash); + rx_crcp); spin_unlock(&conn->session->back_lock); return rc; } @@ -878,7 +867,7 @@ iscsi_tcp_hdr_recv_done(struct iscsi_tcp_conn *tcp_conn, return 0; } - iscsi_tcp_dgst_header(tcp_conn->rx_hash, hdr, + iscsi_tcp_dgst_header(hdr, segment->total_copied - ISCSI_DIGEST_SIZE, segment->digest); diff --git a/include/scsi/libiscsi_tcp.h b/include/scsi/libiscsi_tcp.h index 7c8ba9d7378b..ef53d4bea28a 100644 --- a/include/scsi/libiscsi_tcp.h +++ b/include/scsi/libiscsi_tcp.h @@ -15,7 +15,6 @@ struct iscsi_tcp_conn; struct iscsi_segment; struct sk_buff; -struct ahash_request; typedef int iscsi_segment_done_fn_t(struct iscsi_tcp_conn *, struct iscsi_segment *); @@ -27,7 +26,7 @@ struct iscsi_segment { unsigned int total_size; unsigned int total_copied; - struct ahash_request *hash; + u32 *crcp; unsigned char padbuf[ISCSI_PAD_LEN]; unsigned char recv_digest[ISCSI_DIGEST_SIZE]; unsigned char digest[ISCSI_DIGEST_SIZE]; @@ -61,8 +60,8 @@ struct iscsi_tcp_conn { * stop to terminate */ /* control data */ struct iscsi_tcp_recv in; /* TCP receive context */ - /* CRC32C (Rx) LLD should set this is they do not offload */ - struct ahash_request *rx_hash; + /* CRC32C (Rx) LLD should set this if they do not offload */ + u32 *rx_crcp; }; struct iscsi_tcp_task { @@ -99,18 +98,15 @@ extern void iscsi_tcp_segment_unmap(struct iscsi_segment *segment); extern void iscsi_segment_init_linear(struct iscsi_segment *segment, void *data, size_t size, - iscsi_segment_done_fn_t *done, - struct ahash_request *hash); + iscsi_segment_done_fn_t *done, u32 *crcp); extern int iscsi_segment_seek_sg(struct iscsi_segment *segment, struct scatterlist *sg_list, unsigned int sg_count, unsigned int offset, size_t size, - iscsi_segment_done_fn_t *done, - struct ahash_request *hash); + iscsi_segment_done_fn_t *done, u32 *crcp); /* digest helpers */ -extern void iscsi_tcp_dgst_header(struct ahash_request *hash, const void *hdr, - size_t hdrlen, +extern void iscsi_tcp_dgst_header(const void *hdr, size_t hdrlen, unsigned char digest[ISCSI_DIGEST_SIZE]); extern struct iscsi_cls_conn * iscsi_tcp_conn_setup(struct iscsi_cls_session *cls_session, int dd_data_size, -- 2.50.1