[PATCH net-next 00/14] net: tls: various code cleanups and improvements

[PATCH net-next 00/14] net: tls: various code cleanups and improvements

[Sabrina Dubroca]

This series contains multiple cleanups and simplifications for the
config code of both TLS_SW and TLS_HW.

It also modifies the chcr_ktls driver to use driver_state like all
other drivers, so that we can then make driver_state fixed size
instead of a flex array always allocated to that same fixed size. As
reported by Gustavo A. R. Silva, the way chcr_ktls misuses
driver_state irritates GCC [1].

Patches 1 and 2 are follow-ups to my previous cipher_desc series.

[1] https://lore.kernel.org/netdev/ZRvzdlvlbX4+eIln@work/

Sabrina Dubroca (14):
  tls: get salt using crypto_info_salt in tls_enc_skb
  tls: drop unnecessary cipher_type checks in tls offload
  tls: store rec_seq directly within cipher_context
  tls: rename MAX_IV_SIZE to TLS_MAX_IV_SIZE
  tls: store iv directly within cipher_context
  tls: extract context alloc/initialization out of tls_set_sw_offload
  tls: move tls_prot_info initialization out of tls_set_sw_offload
  tls: also use init_prot_info in tls_set_device_offload
  tls: add a helper to allocate/initialize offload_ctx_tx
  tls: remove tls_context argument from tls_set_sw_offload
  tls: remove tls_context argument from tls_set_device_offload
  tls: validate crypto_info in a separate helper
  chcr_ktls: use tls_offload_context_tx and driver_state like other
    drivers
  tls: use fixed size for tls_offload_context_{tx,rx}.driver_state

 .../chelsio/inline_crypto/ch_ktls/chcr_ktls.c |  43 ++--
 .../chelsio/inline_crypto/ch_ktls/chcr_ktls.h |  36 +++-
 include/net/tls.h                             |  21 +-
 net/tls/tls.h                                 |  12 +-
 net/tls/tls_device.c                          | 101 ++++-----
 net/tls/tls_device_fallback.c                 |  23 +-
 net/tls/tls_main.c                            |  62 +++---
 net/tls/tls_sw.c                              | 198 +++++++++---------
 8 files changed, 244 insertions(+), 252 deletions(-)

-- 
2.42.0

[PATCH net-next 01/14] tls: get salt using crypto_info_salt in tls_enc_skb

[Sabrina Dubroca]

I skipped this conversion in my previous series.

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 net/tls/tls_device_fallback.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/net/tls/tls_device_fallback.c b/net/tls/tls_device_fallback.c
index 1d743f310f4f..b4a65f53d9c0 100644
--- a/net/tls/tls_device_fallback.c
+++ b/net/tls/tls_device_fallback.c
@@ -340,10 +340,7 @@ static struct sk_buff *tls_enc_skb(struct tls_context *tls_ctx,
 
 	switch (tls_ctx->crypto_send.info.cipher_type) {
 	case TLS_CIPHER_AES_GCM_128:
-		salt = tls_ctx->crypto_send.aes_gcm_128.salt;
-		break;
 	case TLS_CIPHER_AES_GCM_256:
-		salt = tls_ctx->crypto_send.aes_gcm_256.salt;
 		break;
 	default:
 		goto free_req;
@@ -356,6 +353,7 @@ static struct sk_buff *tls_enc_skb(struct tls_context *tls_ctx,
 		goto free_req;
 
 	iv = buf;
+	salt = crypto_info_salt(&tls_ctx->crypto_send.info, cipher_desc);
 	memcpy(iv, salt, cipher_desc->salt);
 	aad = buf + cipher_desc->salt + cipher_desc->iv;
 	dummy_buf = aad + TLS_AAD_SPACE_SIZE;
-- 
2.42.0

[PATCH net-next 02/14] tls: drop unnecessary cipher_type checks in tls offload

[Sabrina Dubroca]

We should never reach tls_device_reencrypt, tls_enc_record, or
tls_enc_skb with a cipher_type that can't be offloaded. Replace those
checks with a DEBUG_NET_WARN_ON_ONCE, and use cipher_desc instead of
hard-coding offloadable cipher types.

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 net/tls/tls_device.c          |  8 +-------
 net/tls/tls_device_fallback.c | 17 +++--------------
 2 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index 8c94c926606a..fbd687a0c66f 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -891,14 +891,8 @@ tls_device_reencrypt(struct sock *sk, struct tls_context *tls_ctx)
 	struct strp_msg *rxm;
 	char *orig_buf, *buf;
 
-	switch (tls_ctx->crypto_recv.info.cipher_type) {
-	case TLS_CIPHER_AES_GCM_128:
-	case TLS_CIPHER_AES_GCM_256:
-		break;
-	default:
-		return -EINVAL;
-	}
 	cipher_desc = get_cipher_desc(tls_ctx->crypto_recv.info.cipher_type);
+	DEBUG_NET_WARN_ON_ONCE(!cipher_desc || !cipher_desc->offloadable);
 
 	rxm = strp_msg(tls_strp_msg(sw_ctx));
 	orig_buf = kmalloc(rxm->full_len + TLS_HEADER_SIZE + cipher_desc->iv,
diff --git a/net/tls/tls_device_fallback.c b/net/tls/tls_device_fallback.c
index b4a65f53d9c0..1d2b4d83ccab 100644
--- a/net/tls/tls_device_fallback.c
+++ b/net/tls/tls_device_fallback.c
@@ -62,14 +62,8 @@ static int tls_enc_record(struct aead_request *aead_req,
 	u16 len;
 	int rc;
 
-	switch (prot->cipher_type) {
-	case TLS_CIPHER_AES_GCM_128:
-	case TLS_CIPHER_AES_GCM_256:
-		break;
-	default:
-		return -EINVAL;
-	}
 	cipher_desc = get_cipher_desc(prot->cipher_type);
+	DEBUG_NET_WARN_ON_ONCE(!cipher_desc || !cipher_desc->offloadable);
 
 	buf_size = TLS_HEADER_SIZE + cipher_desc->iv;
 	len = min_t(int, *in_len, buf_size);
@@ -338,14 +332,9 @@ static struct sk_buff *tls_enc_skb(struct tls_context *tls_ctx,
 	if (!aead_req)
 		return NULL;
 
-	switch (tls_ctx->crypto_send.info.cipher_type) {
-	case TLS_CIPHER_AES_GCM_128:
-	case TLS_CIPHER_AES_GCM_256:
-		break;
-	default:
-		goto free_req;
-	}
 	cipher_desc = get_cipher_desc(tls_ctx->crypto_send.info.cipher_type);
+	DEBUG_NET_WARN_ON_ONCE(!cipher_desc || !cipher_desc->offloadable);
+
 	buf_len = cipher_desc->salt + cipher_desc->iv + TLS_AAD_SPACE_SIZE +
 		  sync_size + cipher_desc->tag;
 	buf = kmalloc(buf_len, GFP_ATOMIC);
-- 
2.42.0

[PATCH net-next 03/14] tls: store rec_seq directly within cipher_context

[Sabrina Dubroca]

TLS_MAX_REC_SEQ_SIZE is 8B, we don't get anything by using kmalloc.

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 include/net/tls.h    |  2 +-
 net/tls/tls_device.c | 11 ++---------
 net/tls/tls_main.c   |  1 -
 net/tls/tls_sw.c     | 13 ++-----------
 4 files changed, 5 insertions(+), 22 deletions(-)

diff --git a/include/net/tls.h b/include/net/tls.h
index a2b44578dcb7..f3f22b08af26 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -194,7 +194,7 @@ enum tls_context_flags {
 
 struct cipher_context {
 	char *iv;
-	char *rec_seq;
+	char rec_seq[TLS_MAX_REC_SEQ_SIZE];
 };
 
 union tls_crypto_context {
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index fbd687a0c66f..525d7b813869 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -58,7 +58,6 @@ static void tls_device_free_ctx(struct tls_context *ctx)
 {
 	if (ctx->tx_conf == TLS_HW) {
 		kfree(tls_offload_ctx_tx(ctx));
-		kfree(ctx->tx.rec_seq);
 		kfree(ctx->tx.iv);
 	}
 
@@ -1098,16 +1097,12 @@ int tls_set_device_offload(struct sock *sk, struct tls_context *ctx)
 	memcpy(ctx->tx.iv + cipher_desc->salt, iv, cipher_desc->iv);
 
 	prot->rec_seq_size = cipher_desc->rec_seq;
-	ctx->tx.rec_seq = kmemdup(rec_seq, cipher_desc->rec_seq, GFP_KERNEL);
-	if (!ctx->tx.rec_seq) {
-		rc = -ENOMEM;
-		goto free_iv;
-	}
+	memcpy(ctx->tx.rec_seq, rec_seq, cipher_desc->rec_seq);
 
 	start_marker_record = kmalloc(sizeof(*start_marker_record), GFP_KERNEL);
 	if (!start_marker_record) {
 		rc = -ENOMEM;
-		goto free_rec_seq;
+		goto free_iv;
 	}
 
 	offload_ctx = kzalloc(TLS_OFFLOAD_CONTEXT_SIZE_TX, GFP_KERNEL);
@@ -1192,8 +1187,6 @@ int tls_set_device_offload(struct sock *sk, struct tls_context *ctx)
 	ctx->priv_ctx_tx = NULL;
 free_marker_record:
 	kfree(start_marker_record);
-free_rec_seq:
-	kfree(ctx->tx.rec_seq);
 free_iv:
 	kfree(ctx->tx.iv);
 release_netdev:
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index 02f583ff9239..f705d812fc36 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -344,7 +344,6 @@ static void tls_sk_proto_cleanup(struct sock *sk,
 
 	/* We need these for tls_sw_fallback handling of other packets */
 	if (ctx->tx_conf == TLS_SW) {
-		kfree(ctx->tx.rec_seq);
 		kfree(ctx->tx.iv);
 		tls_sw_release_resources_tx(sk);
 		TLS_DEC_STATS(sock_net(sk), LINUX_MIB_TLSCURRTXSW);
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 270712b8d391..93d40c9a6823 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -2467,7 +2467,6 @@ void tls_sw_release_resources_rx(struct sock *sk)
 	struct tls_context *tls_ctx = tls_get_ctx(sk);
 	struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
 
-	kfree(tls_ctx->rx.rec_seq);
 	kfree(tls_ctx->rx.iv);
 
 	if (ctx->aead_recv) {
@@ -2692,19 +2691,14 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
 	prot->rec_seq_size = cipher_desc->rec_seq;
 	memcpy(cctx->iv, salt, cipher_desc->salt);
 	memcpy(cctx->iv + cipher_desc->salt, iv, cipher_desc->iv);
-
-	cctx->rec_seq = kmemdup(rec_seq, cipher_desc->rec_seq, GFP_KERNEL);
-	if (!cctx->rec_seq) {
-		rc = -ENOMEM;
-		goto free_iv;
-	}
+	memcpy(cctx->rec_seq, rec_seq, cipher_desc->rec_seq);
 
 	if (!*aead) {
 		*aead = crypto_alloc_aead(cipher_desc->cipher_name, 0, 0);
 		if (IS_ERR(*aead)) {
 			rc = PTR_ERR(*aead);
 			*aead = NULL;
-			goto free_rec_seq;
+			goto free_iv;
 		}
 	}
 
@@ -2736,9 +2730,6 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
 free_aead:
 	crypto_free_aead(*aead);
 	*aead = NULL;
-free_rec_seq:
-	kfree(cctx->rec_seq);
-	cctx->rec_seq = NULL;
 free_iv:
 	kfree(cctx->iv);
 	cctx->iv = NULL;
-- 
2.42.0

[PATCH net-next 04/14] tls: rename MAX_IV_SIZE to TLS_MAX_IV_SIZE

[Sabrina Dubroca]

It's defined in include/net/tls.h, avoid using an overly generic name.

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 include/net/tls.h             | 2 +-
 net/tls/tls.h                 | 2 +-
 net/tls/tls_device_fallback.c | 2 +-
 net/tls/tls_main.c            | 2 +-
 net/tls/tls_sw.c              | 6 +++---
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/net/tls.h b/include/net/tls.h
index f3f22b08af26..5200ce27db91 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -61,7 +61,7 @@ struct tls_rec;
 
 #define TLS_AAD_SPACE_SIZE		13
 
-#define MAX_IV_SIZE			16
+#define TLS_MAX_IV_SIZE			16
 #define TLS_TAG_SIZE			16
 #define TLS_MAX_REC_SEQ_SIZE		8
 #define TLS_MAX_AAD_SIZE		TLS_AAD_SPACE_SIZE
diff --git a/net/tls/tls.h b/net/tls/tls.h
index 28a8c0e80e3c..16830aa2d6ec 100644
--- a/net/tls/tls.h
+++ b/net/tls/tls.h
@@ -127,7 +127,7 @@ struct tls_rec {
 	struct sock *sk;
 
 	char aad_space[TLS_AAD_SPACE_SIZE];
-	u8 iv_data[MAX_IV_SIZE];
+	u8 iv_data[TLS_MAX_IV_SIZE];
 	struct aead_request aead_req;
 	u8 aead_req_ctx[];
 };
diff --git a/net/tls/tls_device_fallback.c b/net/tls/tls_device_fallback.c
index 1d2b4d83ccab..4e7228f275fa 100644
--- a/net/tls/tls_device_fallback.c
+++ b/net/tls/tls_device_fallback.c
@@ -54,7 +54,7 @@ static int tls_enc_record(struct aead_request *aead_req,
 			  struct scatter_walk *out, int *in_len,
 			  struct tls_prot_info *prot)
 {
-	unsigned char buf[TLS_HEADER_SIZE + MAX_IV_SIZE];
+	unsigned char buf[TLS_HEADER_SIZE + TLS_MAX_IV_SIZE];
 	const struct tls_cipher_desc *cipher_desc;
 	struct scatterlist sg_in[3];
 	struct scatterlist sg_out[3];
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index f705d812fc36..58f13660fe6b 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -59,7 +59,7 @@ enum {
 };
 
 #define CHECK_CIPHER_DESC(cipher,ci)				\
-	static_assert(cipher ## _IV_SIZE <= MAX_IV_SIZE);		\
+	static_assert(cipher ## _IV_SIZE <= TLS_MAX_IV_SIZE);		\
 	static_assert(cipher ## _REC_SEQ_SIZE <= TLS_MAX_REC_SEQ_SIZE);	\
 	static_assert(cipher ## _TAG_SIZE == TLS_TAG_SIZE);		\
 	static_assert(sizeof_field(struct ci, iv) == cipher ## _IV_SIZE);	\
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 93d40c9a6823..5b6175f9b9a6 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -60,7 +60,7 @@ struct tls_decrypt_arg {
 
 struct tls_decrypt_ctx {
 	struct sock *sk;
-	u8 iv[MAX_IV_SIZE];
+	u8 iv[TLS_MAX_IV_SIZE];
 	u8 aad[TLS_MAX_AAD_SIZE];
 	u8 tail;
 	struct scatterlist sg[];
@@ -2319,7 +2319,7 @@ int tls_rx_msg_size(struct tls_strparser *strp, struct sk_buff *skb)
 {
 	struct tls_context *tls_ctx = tls_get_ctx(strp->sk);
 	struct tls_prot_info *prot = &tls_ctx->prot_info;
-	char header[TLS_HEADER_SIZE + MAX_IV_SIZE];
+	char header[TLS_HEADER_SIZE + TLS_MAX_IV_SIZE];
 	size_t cipher_overhead;
 	size_t data_len = 0;
 	int ret;
@@ -2669,7 +2669,7 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
 	}
 
 	/* Sanity-check the sizes for stack allocations. */
-	if (nonce_size > MAX_IV_SIZE || prot->aad_size > TLS_MAX_AAD_SIZE) {
+	if (nonce_size > TLS_MAX_IV_SIZE || prot->aad_size > TLS_MAX_AAD_SIZE) {
 		rc = -EINVAL;
 		goto free_priv;
 	}
-- 
2.42.0

[PATCH net-next 05/14] tls: store iv directly within cipher_context

[Sabrina Dubroca]

TLS_MAX_IV_SIZE + TLS_MAX_SALT_SIZE is 20B, we don't get much benefit
in cipher_context's size and can simplify the init code a bit.

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 include/net/tls.h    |  3 ++-
 net/tls/tls_device.c | 13 ++-----------
 net/tls/tls_main.c   |  2 +-
 net/tls/tls_sw.c     | 13 ++-----------
 4 files changed, 7 insertions(+), 24 deletions(-)

diff --git a/include/net/tls.h b/include/net/tls.h
index 5200ce27db91..28cc40d7b945 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -62,6 +62,7 @@ struct tls_rec;
 #define TLS_AAD_SPACE_SIZE		13
 
 #define TLS_MAX_IV_SIZE			16
+#define TLS_MAX_SALT_SIZE		4
 #define TLS_TAG_SIZE			16
 #define TLS_MAX_REC_SEQ_SIZE		8
 #define TLS_MAX_AAD_SIZE		TLS_AAD_SPACE_SIZE
@@ -193,7 +194,7 @@ enum tls_context_flags {
 };
 
 struct cipher_context {
-	char *iv;
+	char iv[TLS_MAX_IV_SIZE + TLS_MAX_SALT_SIZE];
 	char rec_seq[TLS_MAX_REC_SEQ_SIZE];
 };
 
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index 525d7b813869..0981496c6294 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -56,10 +56,8 @@ static struct page *dummy_page;
 
 static void tls_device_free_ctx(struct tls_context *ctx)
 {
-	if (ctx->tx_conf == TLS_HW) {
+	if (ctx->tx_conf == TLS_HW)
 		kfree(tls_offload_ctx_tx(ctx));
-		kfree(ctx->tx.iv);
-	}
 
 	if (ctx->rx_conf == TLS_HW)
 		kfree(tls_offload_ctx_rx(ctx));
@@ -1088,11 +1086,6 @@ int tls_set_device_offload(struct sock *sk, struct tls_context *ctx)
 	prot->overhead_size = prot->prepend_size + prot->tag_size;
 	prot->iv_size = cipher_desc->iv;
 	prot->salt_size = cipher_desc->salt;
-	ctx->tx.iv = kmalloc(cipher_desc->iv + cipher_desc->salt, GFP_KERNEL);
-	if (!ctx->tx.iv) {
-		rc = -ENOMEM;
-		goto release_netdev;
-	}
 
 	memcpy(ctx->tx.iv + cipher_desc->salt, iv, cipher_desc->iv);
 
@@ -1102,7 +1095,7 @@ int tls_set_device_offload(struct sock *sk, struct tls_context *ctx)
 	start_marker_record = kmalloc(sizeof(*start_marker_record), GFP_KERNEL);
 	if (!start_marker_record) {
 		rc = -ENOMEM;
-		goto free_iv;
+		goto release_netdev;
 	}
 
 	offload_ctx = kzalloc(TLS_OFFLOAD_CONTEXT_SIZE_TX, GFP_KERNEL);
@@ -1187,8 +1180,6 @@ int tls_set_device_offload(struct sock *sk, struct tls_context *ctx)
 	ctx->priv_ctx_tx = NULL;
 free_marker_record:
 	kfree(start_marker_record);
-free_iv:
-	kfree(ctx->tx.iv);
 release_netdev:
 	dev_put(netdev);
 	return rc;
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index 58f13660fe6b..b91524ac1009 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -60,6 +60,7 @@ enum {
 
 #define CHECK_CIPHER_DESC(cipher,ci)				\
 	static_assert(cipher ## _IV_SIZE <= TLS_MAX_IV_SIZE);		\
+	static_assert(cipher ## _SALT_SIZE <= TLS_MAX_SALT_SIZE);		\
 	static_assert(cipher ## _REC_SEQ_SIZE <= TLS_MAX_REC_SEQ_SIZE);	\
 	static_assert(cipher ## _TAG_SIZE == TLS_TAG_SIZE);		\
 	static_assert(sizeof_field(struct ci, iv) == cipher ## _IV_SIZE);	\
@@ -344,7 +345,6 @@ static void tls_sk_proto_cleanup(struct sock *sk,
 
 	/* We need these for tls_sw_fallback handling of other packets */
 	if (ctx->tx_conf == TLS_SW) {
-		kfree(ctx->tx.iv);
 		tls_sw_release_resources_tx(sk);
 		TLS_DEC_STATS(sock_net(sk), LINUX_MIB_TLSCURRTXSW);
 	} else if (ctx->tx_conf == TLS_HW) {
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 5b6175f9b9a6..c3da937b8207 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -2467,8 +2467,6 @@ void tls_sw_release_resources_rx(struct sock *sk)
 	struct tls_context *tls_ctx = tls_get_ctx(sk);
 	struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
 
-	kfree(tls_ctx->rx.iv);
-
 	if (ctx->aead_recv) {
 		__skb_queue_purge(&ctx->rx_list);
 		crypto_free_aead(ctx->aead_recv);
@@ -2682,11 +2680,7 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
 			      prot->tag_size + prot->tail_size;
 	prot->iv_size = cipher_desc->iv;
 	prot->salt_size = cipher_desc->salt;
-	cctx->iv = kmalloc(cipher_desc->iv + cipher_desc->salt, GFP_KERNEL);
-	if (!cctx->iv) {
-		rc = -ENOMEM;
-		goto free_priv;
-	}
+
 	/* Note: 128 & 256 bit salt are the same size */
 	prot->rec_seq_size = cipher_desc->rec_seq;
 	memcpy(cctx->iv, salt, cipher_desc->salt);
@@ -2698,7 +2692,7 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
 		if (IS_ERR(*aead)) {
 			rc = PTR_ERR(*aead);
 			*aead = NULL;
-			goto free_iv;
+			goto free_priv;
 		}
 	}
 
@@ -2730,9 +2724,6 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
 free_aead:
 	crypto_free_aead(*aead);
 	*aead = NULL;
-free_iv:
-	kfree(cctx->iv);
-	cctx->iv = NULL;
 free_priv:
 	if (tx) {
 		kfree(ctx->priv_ctx_tx);
-- 
2.42.0

[PATCH net-next 06/14] tls: extract context alloc/initialization out of tls_set_sw_offload

[Sabrina Dubroca]

Simplify tls_set_sw_offload a bit.

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 net/tls/tls_sw.c | 86 ++++++++++++++++++++++++++++--------------------
 1 file changed, 51 insertions(+), 35 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index c3da937b8207..b5428f543d17 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -2578,6 +2578,48 @@ void tls_update_rx_zc_capable(struct tls_context *tls_ctx)
 		tls_ctx->prot_info.version != TLS_1_3_VERSION;
 }
 
+static struct tls_sw_context_tx *init_ctx_tx(struct tls_context *ctx, struct sock *sk)
+{
+	struct tls_sw_context_tx *sw_ctx_tx;
+
+	if (!ctx->priv_ctx_tx) {
+		sw_ctx_tx = kzalloc(sizeof(*sw_ctx_tx), GFP_KERNEL);
+		if (!sw_ctx_tx)
+			return NULL;
+	} else {
+		sw_ctx_tx = ctx->priv_ctx_tx;
+	}
+
+	crypto_init_wait(&sw_ctx_tx->async_wait);
+	spin_lock_init(&sw_ctx_tx->encrypt_compl_lock);
+	INIT_LIST_HEAD(&sw_ctx_tx->tx_list);
+	INIT_DELAYED_WORK(&sw_ctx_tx->tx_work.work, tx_work_handler);
+	sw_ctx_tx->tx_work.sk = sk;
+
+	return sw_ctx_tx;
+}
+
+static struct tls_sw_context_rx *init_ctx_rx(struct tls_context *ctx)
+{
+	struct tls_sw_context_rx *sw_ctx_rx;
+
+	if (!ctx->priv_ctx_rx) {
+		sw_ctx_rx = kzalloc(sizeof(*sw_ctx_rx), GFP_KERNEL);
+		if (!sw_ctx_rx)
+			return NULL;
+	} else {
+		sw_ctx_rx = ctx->priv_ctx_rx;
+	}
+
+	crypto_init_wait(&sw_ctx_rx->async_wait);
+	spin_lock_init(&sw_ctx_rx->decrypt_compl_lock);
+	init_waitqueue_head(&sw_ctx_rx->wq);
+	skb_queue_head_init(&sw_ctx_rx->rx_list);
+	skb_queue_head_init(&sw_ctx_rx->async_hold);
+
+	return sw_ctx_rx;
+}
+
 int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
 {
 	struct tls_context *tls_ctx = tls_get_ctx(sk);
@@ -2599,48 +2641,22 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
 	}
 
 	if (tx) {
-		if (!ctx->priv_ctx_tx) {
-			sw_ctx_tx = kzalloc(sizeof(*sw_ctx_tx), GFP_KERNEL);
-			if (!sw_ctx_tx) {
-				rc = -ENOMEM;
-				goto out;
-			}
-			ctx->priv_ctx_tx = sw_ctx_tx;
-		} else {
-			sw_ctx_tx =
-				(struct tls_sw_context_tx *)ctx->priv_ctx_tx;
-		}
-	} else {
-		if (!ctx->priv_ctx_rx) {
-			sw_ctx_rx = kzalloc(sizeof(*sw_ctx_rx), GFP_KERNEL);
-			if (!sw_ctx_rx) {
-				rc = -ENOMEM;
-				goto out;
-			}
-			ctx->priv_ctx_rx = sw_ctx_rx;
-		} else {
-			sw_ctx_rx =
-				(struct tls_sw_context_rx *)ctx->priv_ctx_rx;
-		}
-	}
+		ctx->priv_ctx_tx = init_ctx_tx(ctx, sk);
+		if (!ctx->priv_ctx_tx)
+			return -ENOMEM;
 
-	if (tx) {
-		crypto_init_wait(&sw_ctx_tx->async_wait);
-		spin_lock_init(&sw_ctx_tx->encrypt_compl_lock);
+		sw_ctx_tx = ctx->priv_ctx_tx;
 		crypto_info = &ctx->crypto_send.info;
 		cctx = &ctx->tx;
 		aead = &sw_ctx_tx->aead_send;
-		INIT_LIST_HEAD(&sw_ctx_tx->tx_list);
-		INIT_DELAYED_WORK(&sw_ctx_tx->tx_work.work, tx_work_handler);
-		sw_ctx_tx->tx_work.sk = sk;
 	} else {
-		crypto_init_wait(&sw_ctx_rx->async_wait);
-		spin_lock_init(&sw_ctx_rx->decrypt_compl_lock);
-		init_waitqueue_head(&sw_ctx_rx->wq);
+		ctx->priv_ctx_rx = init_ctx_rx(ctx);
+		if (!ctx->priv_ctx_rx)
+			return -ENOMEM;
+
+		sw_ctx_rx = ctx->priv_ctx_rx;
 		crypto_info = &ctx->crypto_recv.info;
 		cctx = &ctx->rx;
-		skb_queue_head_init(&sw_ctx_rx->rx_list);
-		skb_queue_head_init(&sw_ctx_rx->async_hold);
 		aead = &sw_ctx_rx->aead_recv;
 	}
 
-- 
2.42.0

[PATCH net-next 07/14] tls: move tls_prot_info initialization out of tls_set_sw_offload

[Sabrina Dubroca]

Simplify tls_set_sw_offload, and allow reuse for the tls_device code.

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 net/tls/tls_sw.c | 62 ++++++++++++++++++++++++++----------------------
 1 file changed, 34 insertions(+), 28 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index b5428f543d17..b8e89bbb4a49 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -2620,6 +2620,37 @@ static struct tls_sw_context_rx *init_ctx_rx(struct tls_context *ctx)
 	return sw_ctx_rx;
 }
 
+static int init_prot_info(struct tls_prot_info *prot,
+			  const struct tls_crypto_info *crypto_info,
+			  const struct tls_cipher_desc *cipher_desc)
+{
+	u16 nonce_size = cipher_desc->nonce;
+
+	if (crypto_info->version == TLS_1_3_VERSION) {
+		nonce_size = 0;
+		prot->aad_size = TLS_HEADER_SIZE;
+		prot->tail_size = 1;
+	} else {
+		prot->aad_size = TLS_AAD_SPACE_SIZE;
+		prot->tail_size = 0;
+	}
+
+	/* Sanity-check the sizes for stack allocations. */
+	if (nonce_size > TLS_MAX_IV_SIZE || prot->aad_size > TLS_MAX_AAD_SIZE)
+		return -EINVAL;
+
+	prot->version = crypto_info->version;
+	prot->cipher_type = crypto_info->cipher_type;
+	prot->prepend_size = TLS_HEADER_SIZE + nonce_size;
+	prot->tag_size = cipher_desc->tag;
+	prot->overhead_size = prot->prepend_size + prot->tag_size + prot->tail_size;
+	prot->iv_size = cipher_desc->iv;
+	prot->salt_size = cipher_desc->salt;
+	prot->rec_seq_size = cipher_desc->rec_seq;
+
+	return 0;
+}
+
 int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
 {
 	struct tls_context *tls_ctx = tls_get_ctx(sk);
@@ -2632,7 +2663,6 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
 	struct crypto_tfm *tfm;
 	char *iv, *rec_seq, *key, *salt;
 	const struct tls_cipher_desc *cipher_desc;
-	u16 nonce_size;
 	int rc = 0;
 
 	if (!ctx) {
@@ -2666,39 +2696,15 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
 		goto free_priv;
 	}
 
-	nonce_size = cipher_desc->nonce;
+	rc = init_prot_info(prot, crypto_info, cipher_desc);
+	if (rc)
+		goto free_priv;
 
 	iv = crypto_info_iv(crypto_info, cipher_desc);
 	key = crypto_info_key(crypto_info, cipher_desc);
 	salt = crypto_info_salt(crypto_info, cipher_desc);
 	rec_seq = crypto_info_rec_seq(crypto_info, cipher_desc);
 
-	if (crypto_info->version == TLS_1_3_VERSION) {
-		nonce_size = 0;
-		prot->aad_size = TLS_HEADER_SIZE;
-		prot->tail_size = 1;
-	} else {
-		prot->aad_size = TLS_AAD_SPACE_SIZE;
-		prot->tail_size = 0;
-	}
-
-	/* Sanity-check the sizes for stack allocations. */
-	if (nonce_size > TLS_MAX_IV_SIZE || prot->aad_size > TLS_MAX_AAD_SIZE) {
-		rc = -EINVAL;
-		goto free_priv;
-	}
-
-	prot->version = crypto_info->version;
-	prot->cipher_type = crypto_info->cipher_type;
-	prot->prepend_size = TLS_HEADER_SIZE + nonce_size;
-	prot->tag_size = cipher_desc->tag;
-	prot->overhead_size = prot->prepend_size +
-			      prot->tag_size + prot->tail_size;
-	prot->iv_size = cipher_desc->iv;
-	prot->salt_size = cipher_desc->salt;
-
-	/* Note: 128 & 256 bit salt are the same size */
-	prot->rec_seq_size = cipher_desc->rec_seq;
 	memcpy(cctx->iv, salt, cipher_desc->salt);
 	memcpy(cctx->iv + cipher_desc->salt, iv, cipher_desc->iv);
 	memcpy(cctx->rec_seq, rec_seq, cipher_desc->rec_seq);
-- 
2.42.0

[PATCH net-next 08/14] tls: also use init_prot_info in tls_set_device_offload

[Sabrina Dubroca]

Most values are shared. Nonce size turns out to be equal to IV size
for all offloadable ciphers.

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 net/tls/tls.h        |  4 ++++
 net/tls/tls_device.c | 14 ++++----------
 net/tls/tls_sw.c     | 14 ++++++++++----
 3 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/net/tls/tls.h b/net/tls/tls.h
index 16830aa2d6ec..756ed6cbc3df 100644
--- a/net/tls/tls.h
+++ b/net/tls/tls.h
@@ -142,6 +142,10 @@ void update_sk_prot(struct sock *sk, struct tls_context *ctx);
 int wait_on_pending_writer(struct sock *sk, long *timeo);
 void tls_err_abort(struct sock *sk, int err);
 
+int init_prot_info(struct tls_prot_info *prot,
+		   const struct tls_crypto_info *crypto_info,
+		   const struct tls_cipher_desc *cipher_desc,
+		   int mode);
 int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx);
 void tls_update_rx_zc_capable(struct tls_context *tls_ctx);
 void tls_sw_strparser_arm(struct sock *sk, struct tls_context *ctx);
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index 0981496c6294..3d73dd97e903 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -1076,20 +1076,14 @@ int tls_set_device_offload(struct sock *sk, struct tls_context *ctx)
 		goto release_netdev;
 	}
 
+	rc = init_prot_info(prot, crypto_info, cipher_desc, TLS_HW);
+	if (rc)
+		goto release_netdev;
+
 	iv = crypto_info_iv(crypto_info, cipher_desc);
 	rec_seq = crypto_info_rec_seq(crypto_info, cipher_desc);
 
-	prot->version = crypto_info->version;
-	prot->cipher_type = crypto_info->cipher_type;
-	prot->prepend_size = TLS_HEADER_SIZE + cipher_desc->iv;
-	prot->tag_size = cipher_desc->tag;
-	prot->overhead_size = prot->prepend_size + prot->tag_size;
-	prot->iv_size = cipher_desc->iv;
-	prot->salt_size = cipher_desc->salt;
-
 	memcpy(ctx->tx.iv + cipher_desc->salt, iv, cipher_desc->iv);
-
-	prot->rec_seq_size = cipher_desc->rec_seq;
 	memcpy(ctx->tx.rec_seq, rec_seq, cipher_desc->rec_seq);
 
 	start_marker_record = kmalloc(sizeof(*start_marker_record), GFP_KERNEL);
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index b8e89bbb4a49..0995d3d14f4b 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -2620,9 +2620,10 @@ static struct tls_sw_context_rx *init_ctx_rx(struct tls_context *ctx)
 	return sw_ctx_rx;
 }
 
-static int init_prot_info(struct tls_prot_info *prot,
-			  const struct tls_crypto_info *crypto_info,
-			  const struct tls_cipher_desc *cipher_desc)
+int init_prot_info(struct tls_prot_info *prot,
+		   const struct tls_crypto_info *crypto_info,
+		   const struct tls_cipher_desc *cipher_desc,
+		   int mode)
 {
 	u16 nonce_size = cipher_desc->nonce;
 
@@ -2635,6 +2636,11 @@ static int init_prot_info(struct tls_prot_info *prot,
 		prot->tail_size = 0;
 	}
 
+	if (mode == TLS_HW) {
+		prot->aad_size = 0;
+		prot->tail_size = 0;
+	}
+
 	/* Sanity-check the sizes for stack allocations. */
 	if (nonce_size > TLS_MAX_IV_SIZE || prot->aad_size > TLS_MAX_AAD_SIZE)
 		return -EINVAL;
@@ -2696,7 +2702,7 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
 		goto free_priv;
 	}
 
-	rc = init_prot_info(prot, crypto_info, cipher_desc);
+	rc = init_prot_info(prot, crypto_info, cipher_desc, TLS_SW);
 	if (rc)
 		goto free_priv;
 
-- 
2.42.0

[PATCH net-next 09/14] tls: add a helper to allocate/initialize offload_ctx_tx

[Sabrina Dubroca]

Simplify tls_set_device_offload a bit.

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 net/tls/tls_device.c | 39 +++++++++++++++++++++++++--------------
 1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index 3d73dd97e903..0184426251b0 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -1033,6 +1033,30 @@ static void tls_device_attach(struct tls_context *ctx, struct sock *sk,
 	}
 }
 
+static struct tls_offload_context_tx *alloc_offload_ctx_tx(struct tls_context *ctx)
+{
+	struct tls_offload_context_tx *offload_ctx;
+	__be64 rcd_sn;
+
+	offload_ctx = kzalloc(TLS_OFFLOAD_CONTEXT_SIZE_TX, GFP_KERNEL);
+	if (!offload_ctx)
+		return NULL;
+
+	INIT_WORK(&offload_ctx->destruct_work, tls_device_tx_del_task);
+	INIT_LIST_HEAD(&offload_ctx->records_list);
+	spin_lock_init(&offload_ctx->lock);
+	sg_init_table(offload_ctx->sg_tx_data,
+		      ARRAY_SIZE(offload_ctx->sg_tx_data));
+
+	/* start at rec_seq - 1 to account for the start marker record */
+	memcpy(&rcd_sn, ctx->tx.rec_seq, sizeof(rcd_sn));
+	offload_ctx->unacked_record_sn = be64_to_cpu(rcd_sn) - 1;
+
+	offload_ctx->ctx = ctx;
+
+	return offload_ctx;
+}
+
 int tls_set_device_offload(struct sock *sk, struct tls_context *ctx)
 {
 	struct tls_context *tls_ctx = tls_get_ctx(sk);
@@ -1044,7 +1068,6 @@ int tls_set_device_offload(struct sock *sk, struct tls_context *ctx)
 	struct net_device *netdev;
 	char *iv, *rec_seq;
 	struct sk_buff *skb;
-	__be64 rcd_sn;
 	int rc;
 
 	if (!ctx)
@@ -1092,7 +1115,7 @@ int tls_set_device_offload(struct sock *sk, struct tls_context *ctx)
 		goto release_netdev;
 	}
 
-	offload_ctx = kzalloc(TLS_OFFLOAD_CONTEXT_SIZE_TX, GFP_KERNEL);
+	offload_ctx = alloc_offload_ctx_tx(ctx);
 	if (!offload_ctx) {
 		rc = -ENOMEM;
 		goto free_marker_record;
@@ -1102,22 +1125,10 @@ int tls_set_device_offload(struct sock *sk, struct tls_context *ctx)
 	if (rc)
 		goto free_offload_ctx;
 
-	/* start at rec_seq - 1 to account for the start marker record */
-	memcpy(&rcd_sn, ctx->tx.rec_seq, sizeof(rcd_sn));
-	offload_ctx->unacked_record_sn = be64_to_cpu(rcd_sn) - 1;
-
 	start_marker_record->end_seq = tcp_sk(sk)->write_seq;
 	start_marker_record->len = 0;
 	start_marker_record->num_frags = 0;
-
-	INIT_WORK(&offload_ctx->destruct_work, tls_device_tx_del_task);
-	offload_ctx->ctx = ctx;
-
-	INIT_LIST_HEAD(&offload_ctx->records_list);
 	list_add_tail(&start_marker_record->list, &offload_ctx->records_list);
-	spin_lock_init(&offload_ctx->lock);
-	sg_init_table(offload_ctx->sg_tx_data,
-		      ARRAY_SIZE(offload_ctx->sg_tx_data));
 
 	clean_acked_data_enable(inet_csk(sk), &tls_icsk_clean_acked);
 	ctx->push_pending_record = tls_device_push_pending_record;
-- 
2.42.0

[PATCH net-next 10/14] tls: remove tls_context argument from tls_set_sw_offload

[Sabrina Dubroca]

It's not really needed since we end up refetching it as tls_ctx. We
can also remove the NULL check, since we have already dereferenced ctx
in do_tls_setsockopt_conf.

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 net/tls/tls.h        |  2 +-
 net/tls/tls_device.c |  2 +-
 net/tls/tls_main.c   |  4 ++--
 net/tls/tls_sw.c     | 18 ++++++++----------
 4 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/net/tls/tls.h b/net/tls/tls.h
index 756ed6cbc3df..d9e8cd73b20e 100644
--- a/net/tls/tls.h
+++ b/net/tls/tls.h
@@ -146,7 +146,7 @@ int init_prot_info(struct tls_prot_info *prot,
 		   const struct tls_crypto_info *crypto_info,
 		   const struct tls_cipher_desc *cipher_desc,
 		   int mode);
-int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx);
+int tls_set_sw_offload(struct sock *sk, int tx);
 void tls_update_rx_zc_capable(struct tls_context *tls_ctx);
 void tls_sw_strparser_arm(struct sock *sk, struct tls_context *ctx);
 void tls_sw_strparser_done(struct tls_context *tls_ctx);
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index 0184426251b0..1dc217870f9d 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -1233,7 +1233,7 @@ int tls_set_device_offload_rx(struct sock *sk, struct tls_context *ctx)
 	context->resync_nh_reset = 1;
 
 	ctx->priv_ctx_rx = context;
-	rc = tls_set_sw_offload(sk, ctx, 0);
+	rc = tls_set_sw_offload(sk, 0);
 	if (rc)
 		goto release_ctx;
 
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index b91524ac1009..6c5e0cad89e8 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -663,7 +663,7 @@ static int do_tls_setsockopt_conf(struct sock *sk, sockptr_t optval,
 			TLS_INC_STATS(sock_net(sk), LINUX_MIB_TLSTXDEVICE);
 			TLS_INC_STATS(sock_net(sk), LINUX_MIB_TLSCURRTXDEVICE);
 		} else {
-			rc = tls_set_sw_offload(sk, ctx, 1);
+			rc = tls_set_sw_offload(sk, 1);
 			if (rc)
 				goto err_crypto_info;
 			TLS_INC_STATS(sock_net(sk), LINUX_MIB_TLSTXSW);
@@ -677,7 +677,7 @@ static int do_tls_setsockopt_conf(struct sock *sk, sockptr_t optval,
 			TLS_INC_STATS(sock_net(sk), LINUX_MIB_TLSRXDEVICE);
 			TLS_INC_STATS(sock_net(sk), LINUX_MIB_TLSCURRRXDEVICE);
 		} else {
-			rc = tls_set_sw_offload(sk, ctx, 0);
+			rc = tls_set_sw_offload(sk, 0);
 			if (rc)
 				goto err_crypto_info;
 			TLS_INC_STATS(sock_net(sk), LINUX_MIB_TLSRXSW);
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 0995d3d14f4b..0f6da4ce3ed7 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -2657,24 +2657,22 @@ int init_prot_info(struct tls_prot_info *prot,
 	return 0;
 }
 
-int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
+int tls_set_sw_offload(struct sock *sk, int tx)
 {
-	struct tls_context *tls_ctx = tls_get_ctx(sk);
-	struct tls_prot_info *prot = &tls_ctx->prot_info;
-	struct tls_crypto_info *crypto_info;
 	struct tls_sw_context_tx *sw_ctx_tx = NULL;
 	struct tls_sw_context_rx *sw_ctx_rx = NULL;
+	const struct tls_cipher_desc *cipher_desc;
+	struct tls_crypto_info *crypto_info;
+	char *iv, *rec_seq, *key, *salt;
 	struct cipher_context *cctx;
+	struct tls_prot_info *prot;
 	struct crypto_aead **aead;
+	struct tls_context *ctx;
 	struct crypto_tfm *tfm;
-	char *iv, *rec_seq, *key, *salt;
-	const struct tls_cipher_desc *cipher_desc;
 	int rc = 0;
 
-	if (!ctx) {
-		rc = -EINVAL;
-		goto out;
-	}
+	ctx = tls_get_ctx(sk);
+	prot = &ctx->prot_info;
 
 	if (tx) {
 		ctx->priv_ctx_tx = init_ctx_tx(ctx, sk);
-- 
2.42.0

[PATCH net-next 11/14] tls: remove tls_context argument from tls_set_device_offload

[Sabrina Dubroca]

It's not really needed since we end up refetching it as tls_ctx. We
can also remove the NULL check, since we have already dereferenced ctx
in do_tls_setsockopt_conf.

While at it, fix up the reverse xmas tree ordering.

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 net/tls/tls.h        |  4 ++--
 net/tls/tls_device.c | 14 +++++++-------
 net/tls/tls_main.c   |  2 +-
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/net/tls/tls.h b/net/tls/tls.h
index d9e8cd73b20e..478b2c0060aa 100644
--- a/net/tls/tls.h
+++ b/net/tls/tls.h
@@ -227,7 +227,7 @@ static inline bool tls_strp_msg_mixed_decrypted(struct tls_sw_context_rx *ctx)
 #ifdef CONFIG_TLS_DEVICE
 int tls_device_init(void);
 void tls_device_cleanup(void);
-int tls_set_device_offload(struct sock *sk, struct tls_context *ctx);
+int tls_set_device_offload(struct sock *sk);
 void tls_device_free_resources_tx(struct sock *sk);
 int tls_set_device_offload_rx(struct sock *sk, struct tls_context *ctx);
 void tls_device_offload_cleanup_rx(struct sock *sk);
@@ -238,7 +238,7 @@ static inline int tls_device_init(void) { return 0; }
 static inline void tls_device_cleanup(void) {}
 
 static inline int
-tls_set_device_offload(struct sock *sk, struct tls_context *ctx)
+tls_set_device_offload(struct sock *sk)
 {
 	return -EOPNOTSUPP;
 }
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index 1dc217870f9d..fe52765beaee 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -1057,21 +1057,21 @@ static struct tls_offload_context_tx *alloc_offload_ctx_tx(struct tls_context *c
 	return offload_ctx;
 }
 
-int tls_set_device_offload(struct sock *sk, struct tls_context *ctx)
+int tls_set_device_offload(struct sock *sk)
 {
-	struct tls_context *tls_ctx = tls_get_ctx(sk);
-	struct tls_prot_info *prot = &tls_ctx->prot_info;
-	const struct tls_cipher_desc *cipher_desc;
 	struct tls_record_info *start_marker_record;
 	struct tls_offload_context_tx *offload_ctx;
+	const struct tls_cipher_desc *cipher_desc;
 	struct tls_crypto_info *crypto_info;
+	struct tls_prot_info *prot;
 	struct net_device *netdev;
-	char *iv, *rec_seq;
+	struct tls_context *ctx;
 	struct sk_buff *skb;
+	char *iv, *rec_seq;
 	int rc;
 
-	if (!ctx)
-		return -EINVAL;
+	ctx = tls_get_ctx(sk);
+	prot = &ctx->prot_info;
 
 	if (ctx->priv_ctx_tx)
 		return -EEXIST;
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index 6c5e0cad89e8..a342853ab6ae 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -657,7 +657,7 @@ static int do_tls_setsockopt_conf(struct sock *sk, sockptr_t optval,
 	}
 
 	if (tx) {
-		rc = tls_set_device_offload(sk, ctx);
+		rc = tls_set_device_offload(sk);
 		conf = TLS_HW;
 		if (!rc) {
 			TLS_INC_STATS(sock_net(sk), LINUX_MIB_TLSTXDEVICE);
-- 
2.42.0

[PATCH net-next 12/14] tls: validate crypto_info in a separate helper

[Sabrina Dubroca]

Simplify do_tls_setsockopt_conf a bit.

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 net/tls/tls_main.c | 51 ++++++++++++++++++++++++----------------------
 1 file changed, 27 insertions(+), 24 deletions(-)

diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index a342853ab6ae..b125a08a618a 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -580,6 +580,31 @@ static int tls_getsockopt(struct sock *sk, int level, int optname,
 	return do_tls_getsockopt(sk, optname, optval, optlen);
 }
 
+static int validate_crypto_info(const struct tls_crypto_info *crypto_info,
+				const struct tls_crypto_info *alt_crypto_info)
+{
+	if (crypto_info->version != TLS_1_2_VERSION &&
+	    crypto_info->version != TLS_1_3_VERSION)
+		return -EINVAL;
+
+	switch (crypto_info->cipher_type) {
+	case TLS_CIPHER_ARIA_GCM_128:
+	case TLS_CIPHER_ARIA_GCM_256:
+		if (crypto_info->version != TLS_1_2_VERSION)
+			return -EINVAL;
+		break;
+	}
+
+	/* Ensure that TLS version and ciphers are same in both directions */
+	if (TLS_CRYPTO_INFO_READY(alt_crypto_info)) {
+		if (alt_crypto_info->version != crypto_info->version ||
+		    alt_crypto_info->cipher_type != crypto_info->cipher_type)
+			return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int do_tls_setsockopt_conf(struct sock *sk, sockptr_t optval,
 				  unsigned int optlen, int tx)
 {
@@ -611,21 +636,9 @@ static int do_tls_setsockopt_conf(struct sock *sk, sockptr_t optval,
 		goto err_crypto_info;
 	}
 
-	/* check version */
-	if (crypto_info->version != TLS_1_2_VERSION &&
-	    crypto_info->version != TLS_1_3_VERSION) {
-		rc = -EINVAL;
+	rc = validate_crypto_info(crypto_info, alt_crypto_info);
+	if (rc)
 		goto err_crypto_info;
-	}
-
-	/* Ensure that TLS version and ciphers are same in both directions */
-	if (TLS_CRYPTO_INFO_READY(alt_crypto_info)) {
-		if (alt_crypto_info->version != crypto_info->version ||
-		    alt_crypto_info->cipher_type != crypto_info->cipher_type) {
-			rc = -EINVAL;
-			goto err_crypto_info;
-		}
-	}
 
 	cipher_desc = get_cipher_desc(crypto_info->cipher_type);
 	if (!cipher_desc) {
@@ -633,16 +646,6 @@ static int do_tls_setsockopt_conf(struct sock *sk, sockptr_t optval,
 		goto err_crypto_info;
 	}
 
-	switch (crypto_info->cipher_type) {
-	case TLS_CIPHER_ARIA_GCM_128:
-	case TLS_CIPHER_ARIA_GCM_256:
-		if (crypto_info->version != TLS_1_2_VERSION) {
-			rc = -EINVAL;
-			goto err_crypto_info;
-		}
-		break;
-	}
-
 	if (optlen != cipher_desc->crypto_info) {
 		rc = -EINVAL;
 		goto err_crypto_info;
-- 
2.42.0

[PATCH net-next 13/14] chcr_ktls: use tls_offload_context_tx and driver_state like other drivers

[Sabrina Dubroca]

chcr_ktls uses the space reserved in driver_state by
tls_set_device_offload, but makes up into own wrapper around
tls_offload_context_tx instead of accessing driver_state via the
__tls_driver_ctx helper.

In this driver, driver_state is only used to store a pointer to a
larger context struct allocated by the driver.

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 .../chelsio/inline_crypto/ch_ktls/chcr_ktls.c | 43 ++++++++-----------
 .../chelsio/inline_crypto/ch_ktls/chcr_ktls.h | 36 ++++++++++++----
 2 files changed, 46 insertions(+), 33 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
index bcdc7fc2f427..6482728794dd 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
@@ -361,9 +361,7 @@ static void chcr_ktls_dev_del(struct net_device *netdev,
 			      struct tls_context *tls_ctx,
 			      enum tls_offload_ctx_dir direction)
 {
-	struct chcr_ktls_ofld_ctx_tx *tx_ctx =
-				chcr_get_ktls_tx_context(tls_ctx);
-	struct chcr_ktls_info *tx_info = tx_ctx->chcr_info;
+	struct chcr_ktls_info *tx_info = chcr_get_ktls_tx_info(tls_ctx);
 	struct ch_ktls_port_stats_debug *port_stats;
 	struct chcr_ktls_uld_ctx *u_ctx;
 
@@ -396,7 +394,7 @@ static void chcr_ktls_dev_del(struct net_device *netdev,
 	port_stats = &tx_info->adap->ch_ktls_stats.ktls_port[tx_info->port_id];
 	atomic64_inc(&port_stats->ktls_tx_connection_close);
 	kvfree(tx_info);
-	tx_ctx->chcr_info = NULL;
+	chcr_set_ktls_tx_info(tls_ctx, NULL);
 	/* release module refcount */
 	module_put(THIS_MODULE);
 }
@@ -417,7 +415,6 @@ static int chcr_ktls_dev_add(struct net_device *netdev, struct sock *sk,
 {
 	struct tls_context *tls_ctx = tls_get_ctx(sk);
 	struct ch_ktls_port_stats_debug *port_stats;
-	struct chcr_ktls_ofld_ctx_tx *tx_ctx;
 	struct chcr_ktls_uld_ctx *u_ctx;
 	struct chcr_ktls_info *tx_info;
 	struct dst_entry *dst;
@@ -427,8 +424,6 @@ static int chcr_ktls_dev_add(struct net_device *netdev, struct sock *sk,
 	u8 daaddr[16];
 	int ret = -1;
 
-	tx_ctx = chcr_get_ktls_tx_context(tls_ctx);
-
 	pi = netdev_priv(netdev);
 	adap = pi->adapter;
 	port_stats = &adap->ch_ktls_stats.ktls_port[pi->port_id];
@@ -440,7 +435,7 @@ static int chcr_ktls_dev_add(struct net_device *netdev, struct sock *sk,
 		goto out;
 	}
 
-	if (tx_ctx->chcr_info)
+	if (chcr_get_ktls_tx_info(tls_ctx))
 		goto out;
 
 	if (u_ctx && u_ctx->detach)
@@ -566,7 +561,7 @@ static int chcr_ktls_dev_add(struct net_device *netdev, struct sock *sk,
 		goto free_tid;
 
 	atomic64_inc(&port_stats->ktls_tx_ctx);
-	tx_ctx->chcr_info = tx_info;
+	chcr_set_ktls_tx_info(tls_ctx, tx_info);
 
 	return 0;
 
@@ -647,7 +642,7 @@ static int chcr_ktls_cpl_act_open_rpl(struct adapter *adap,
 {
 	const struct cpl_act_open_rpl *p = (void *)input;
 	struct chcr_ktls_info *tx_info = NULL;
-	struct chcr_ktls_ofld_ctx_tx *tx_ctx;
+	struct tls_offload_context_tx *tx_ctx;
 	struct chcr_ktls_uld_ctx *u_ctx;
 	unsigned int atid, tid, status;
 	struct tls_context *tls_ctx;
@@ -686,7 +681,7 @@ static int chcr_ktls_cpl_act_open_rpl(struct adapter *adap,
 		cxgb4_insert_tid(t, tx_info, tx_info->tid, tx_info->ip_family);
 		/* Adding tid */
 		tls_ctx = tls_get_ctx(tx_info->sk);
-		tx_ctx = chcr_get_ktls_tx_context(tls_ctx);
+		tx_ctx = tls_offload_ctx_tx(tls_ctx);
 		u_ctx = adap->uld[CXGB4_ULD_KTLS].handle;
 		if (u_ctx) {
 			ret = xa_insert_bh(&u_ctx->tid_list, tid, tx_ctx,
@@ -1924,7 +1919,7 @@ static int chcr_ktls_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	u32 tls_end_offset, tcp_seq, skb_data_len, skb_offset;
 	struct ch_ktls_port_stats_debug *port_stats;
-	struct chcr_ktls_ofld_ctx_tx *tx_ctx;
+	struct tls_offload_context_tx *tx_ctx;
 	struct ch_ktls_stats_debug *stats;
 	struct tcphdr *th = tcp_hdr(skb);
 	int data_len, qidx, ret = 0, mss;
@@ -1944,6 +1939,7 @@ static int chcr_ktls_xmit(struct sk_buff *skb, struct net_device *dev)
 	mss = skb_is_gso(skb) ? skb_shinfo(skb)->gso_size : data_len;
 
 	tls_ctx = tls_get_ctx(skb->sk);
+	tx_ctx = tls_offload_ctx_tx(tls_ctx);
 	tls_netdev = rcu_dereference_bh(tls_ctx->netdev);
 	/* Don't quit on NULL: if tls_device_down is running in parallel,
 	 * netdev might become NULL, even if tls_is_skb_tx_device_offloaded was
@@ -1952,8 +1948,7 @@ static int chcr_ktls_xmit(struct sk_buff *skb, struct net_device *dev)
 	if (unlikely(tls_netdev && tls_netdev != dev))
 		goto out;
 
-	tx_ctx = chcr_get_ktls_tx_context(tls_ctx);
-	tx_info = tx_ctx->chcr_info;
+	tx_info = chcr_get_ktls_tx_info(tls_ctx);
 
 	if (unlikely(!tx_info))
 		goto out;
@@ -1979,19 +1974,19 @@ static int chcr_ktls_xmit(struct sk_buff *skb, struct net_device *dev)
 	 * we will send the complete record again.
 	 */
 
-	spin_lock_irqsave(&tx_ctx->base.lock, flags);
+	spin_lock_irqsave(&tx_ctx->lock, flags);
 
 	do {
 
 		cxgb4_reclaim_completed_tx(adap, &q->q, true);
 		/* fetch the tls record */
-		record = tls_get_record(&tx_ctx->base, tcp_seq,
+		record = tls_get_record(tx_ctx, tcp_seq,
 					&tx_info->record_no);
 		/* By the time packet reached to us, ACK is received, and record
 		 * won't be found in that case, handle it gracefully.
 		 */
 		if (unlikely(!record)) {
-			spin_unlock_irqrestore(&tx_ctx->base.lock, flags);
+			spin_unlock_irqrestore(&tx_ctx->lock, flags);
 			atomic64_inc(&port_stats->ktls_tx_drop_no_sync_data);
 			goto out;
 		}
@@ -2015,7 +2010,7 @@ static int chcr_ktls_xmit(struct sk_buff *skb, struct net_device *dev)
 						      tls_end_offset !=
 						      record->len);
 			if (ret) {
-				spin_unlock_irqrestore(&tx_ctx->base.lock,
+				spin_unlock_irqrestore(&tx_ctx->lock,
 						       flags);
 				goto out;
 			}
@@ -2046,7 +2041,7 @@ static int chcr_ktls_xmit(struct sk_buff *skb, struct net_device *dev)
 				/* free the refcount taken earlier */
 				if (tls_end_offset < data_len)
 					dev_kfree_skb_any(skb);
-				spin_unlock_irqrestore(&tx_ctx->base.lock, flags);
+				spin_unlock_irqrestore(&tx_ctx->lock, flags);
 				goto out;
 			}
 
@@ -2082,7 +2077,7 @@ static int chcr_ktls_xmit(struct sk_buff *skb, struct net_device *dev)
 
 		/* if any failure, come out from the loop. */
 		if (ret) {
-			spin_unlock_irqrestore(&tx_ctx->base.lock, flags);
+			spin_unlock_irqrestore(&tx_ctx->lock, flags);
 			if (th->fin)
 				dev_kfree_skb_any(skb);
 
@@ -2097,7 +2092,7 @@ static int chcr_ktls_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	} while (data_len > 0);
 
-	spin_unlock_irqrestore(&tx_ctx->base.lock, flags);
+	spin_unlock_irqrestore(&tx_ctx->lock, flags);
 	atomic64_inc(&port_stats->ktls_tx_encrypted_packets);
 	atomic64_add(skb_data_len, &port_stats->ktls_tx_encrypted_bytes);
 
@@ -2185,17 +2180,17 @@ static void clear_conn_resources(struct chcr_ktls_info *tx_info)
 static void ch_ktls_reset_all_conn(struct chcr_ktls_uld_ctx *u_ctx)
 {
 	struct ch_ktls_port_stats_debug *port_stats;
-	struct chcr_ktls_ofld_ctx_tx *tx_ctx;
+	struct tls_offload_context_tx *tx_ctx;
 	struct chcr_ktls_info *tx_info;
 	unsigned long index;
 
 	xa_for_each(&u_ctx->tid_list, index, tx_ctx) {
-		tx_info = tx_ctx->chcr_info;
+		tx_info = __chcr_get_ktls_tx_info(tx_ctx);
 		clear_conn_resources(tx_info);
 		port_stats = &tx_info->adap->ch_ktls_stats.ktls_port[tx_info->port_id];
 		atomic64_inc(&port_stats->ktls_tx_connection_close);
 		kvfree(tx_info);
-		tx_ctx->chcr_info = NULL;
+		memset(tx_ctx->driver_state, 0, TLS_DRIVER_STATE_SIZE_TX);
 		/* release module refcount */
 		module_put(THIS_MODULE);
 	}
diff --git a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.h b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.h
index 10572dc55365..dbbba92bf540 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.h
+++ b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.h
@@ -67,8 +67,7 @@ struct chcr_ktls_info {
 	bool pending_close;
 };
 
-struct chcr_ktls_ofld_ctx_tx {
-	struct tls_offload_context_tx base;
+struct chcr_ktls_ctx_tx {
 	struct chcr_ktls_info *chcr_info;
 };
 
@@ -79,14 +78,33 @@ struct chcr_ktls_uld_ctx {
 	bool detach;
 };
 
-static inline struct chcr_ktls_ofld_ctx_tx *
-chcr_get_ktls_tx_context(struct tls_context *tls_ctx)
+static inline struct chcr_ktls_info *
+__chcr_get_ktls_tx_info(struct tls_offload_context_tx *octx)
 {
-	BUILD_BUG_ON(sizeof(struct chcr_ktls_ofld_ctx_tx) >
-		     TLS_OFFLOAD_CONTEXT_SIZE_TX);
-	return container_of(tls_offload_ctx_tx(tls_ctx),
-			    struct chcr_ktls_ofld_ctx_tx,
-			    base);
+	struct chcr_ktls_ctx_tx *priv_ctx;
+
+	BUILD_BUG_ON(sizeof(struct chcr_ktls_ctx_tx) > TLS_DRIVER_STATE_SIZE_TX);
+	priv_ctx = (struct chcr_ktls_ctx_tx *)octx->driver_state;
+	return priv_ctx->chcr_info;
+}
+
+static inline struct chcr_ktls_info *
+chcr_get_ktls_tx_info(struct tls_context *tls_ctx)
+{
+	struct chcr_ktls_ctx_tx *priv_ctx;
+
+	BUILD_BUG_ON(sizeof(struct chcr_ktls_ctx_tx) > TLS_DRIVER_STATE_SIZE_TX);
+	priv_ctx = (struct chcr_ktls_ctx_tx *)__tls_driver_ctx(tls_ctx, TLS_OFFLOAD_CTX_DIR_TX);
+	return priv_ctx->chcr_info;
+}
+
+static inline void
+chcr_set_ktls_tx_info(struct tls_context *tls_ctx, struct chcr_ktls_info *chcr_info)
+{
+	struct chcr_ktls_ctx_tx *priv_ctx;
+
+	priv_ctx = __tls_driver_ctx(tls_ctx, TLS_OFFLOAD_CTX_DIR_TX);
+	priv_ctx->chcr_info = chcr_info;
 }
 
 static inline int chcr_get_first_rx_qid(struct adapter *adap)
-- 
2.42.0

[PATCH net-next 14/14] tls: use fixed size for tls_offload_context_{tx,rx}.driver_state

[Sabrina Dubroca]

driver_state is a flex array, but is always allocated by the tls core
to a fixed size (TLS_DRIVER_STATE_SIZE_{TX,RX}). Simplify the code by
making that size explicit so that sizeof(struct
tls_offload_context_{tx,rx}) works.

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 include/net/tls.h    | 14 ++++----------
 net/tls/tls_device.c |  4 ++--
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/include/net/tls.h b/include/net/tls.h
index 28cc40d7b945..962f0c501111 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -150,6 +150,7 @@ struct tls_record_info {
 	skb_frag_t frags[MAX_SKB_FRAGS];
 };
 
+#define TLS_DRIVER_STATE_SIZE_TX	16
 struct tls_offload_context_tx {
 	struct crypto_aead *aead_send;
 	spinlock_t lock;	/* protects records list */
@@ -163,17 +164,13 @@ struct tls_offload_context_tx {
 	void (*sk_destruct)(struct sock *sk);
 	struct work_struct destruct_work;
 	struct tls_context *ctx;
-	u8 driver_state[] __aligned(8);
 	/* The TLS layer reserves room for driver specific state
 	 * Currently the belief is that there is not enough
 	 * driver specific state to justify another layer of indirection
 	 */
-#define TLS_DRIVER_STATE_SIZE_TX	16
+	u8 driver_state[TLS_DRIVER_STATE_SIZE_TX] __aligned(8);
 };
 
-#define TLS_OFFLOAD_CONTEXT_SIZE_TX                                            \
-	(sizeof(struct tls_offload_context_tx) + TLS_DRIVER_STATE_SIZE_TX)
-
 enum tls_context_flags {
 	/* tls_device_down was called after the netdev went down, device state
 	 * was released, and kTLS works in software, even though rx_conf is
@@ -303,6 +300,7 @@ struct tls_offload_resync_async {
 	u32 log[TLS_DEVICE_RESYNC_ASYNC_LOGMAX];
 };
 
+#define TLS_DRIVER_STATE_SIZE_RX	8
 struct tls_offload_context_rx {
 	/* sw must be the first member of tls_offload_context_rx */
 	struct tls_sw_context_rx sw;
@@ -326,17 +324,13 @@ struct tls_offload_context_rx {
 			struct tls_offload_resync_async *resync_async;
 		};
 	};
-	u8 driver_state[] __aligned(8);
 	/* The TLS layer reserves room for driver specific state
 	 * Currently the belief is that there is not enough
 	 * driver specific state to justify another layer of indirection
 	 */
-#define TLS_DRIVER_STATE_SIZE_RX	8
+	u8 driver_state[TLS_DRIVER_STATE_SIZE_RX] __aligned(8);
 };
 
-#define TLS_OFFLOAD_CONTEXT_SIZE_RX					\
-	(sizeof(struct tls_offload_context_rx) + TLS_DRIVER_STATE_SIZE_RX)
-
 struct tls_record_info *tls_get_record(struct tls_offload_context_tx *context,
 				       u32 seq, u64 *p_record_sn);
 
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index fe52765beaee..f01543557a60 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -1038,7 +1038,7 @@ static struct tls_offload_context_tx *alloc_offload_ctx_tx(struct tls_context *c
 	struct tls_offload_context_tx *offload_ctx;
 	__be64 rcd_sn;
 
-	offload_ctx = kzalloc(TLS_OFFLOAD_CONTEXT_SIZE_TX, GFP_KERNEL);
+	offload_ctx = kzalloc(sizeof(*offload_ctx), GFP_KERNEL);
 	if (!offload_ctx)
 		return NULL;
 
@@ -1225,7 +1225,7 @@ int tls_set_device_offload_rx(struct sock *sk, struct tls_context *ctx)
 		goto release_lock;
 	}
 
-	context = kzalloc(TLS_OFFLOAD_CONTEXT_SIZE_RX, GFP_KERNEL);
+	context = kzalloc(sizeof(*context), GFP_KERNEL);
 	if (!context) {
 		rc = -ENOMEM;
 		goto release_lock;
-- 
2.42.0

Re: [PATCH net-next 00/14] net: tls: various code cleanups and improvements

[Simon Horman]

+ "David S. Miller" <davem@davemloft.net>
  Paolo Abeni <pabeni@redhat.com>
  Eric Dumazet <edumazet@google.com>

On Mon, Oct 09, 2023 at 10:50:40PM +0200, Sabrina Dubroca wrote:
> This series contains multiple cleanups and simplifications for the
> config code of both TLS_SW and TLS_HW.
> 
> It also modifies the chcr_ktls driver to use driver_state like all
> other drivers, so that we can then make driver_state fixed size
> instead of a flex array always allocated to that same fixed size. As
> reported by Gustavo A. R. Silva, the way chcr_ktls misuses
> driver_state irritates GCC [1].
> 
> Patches 1 and 2 are follow-ups to my previous cipher_desc series.
> 
> [1] https://lore.kernel.org/netdev/ZRvzdlvlbX4+eIln@work/
> 
> Sabrina Dubroca (14):
>   tls: get salt using crypto_info_salt in tls_enc_skb
>   tls: drop unnecessary cipher_type checks in tls offload
>   tls: store rec_seq directly within cipher_context
>   tls: rename MAX_IV_SIZE to TLS_MAX_IV_SIZE
>   tls: store iv directly within cipher_context
>   tls: extract context alloc/initialization out of tls_set_sw_offload
>   tls: move tls_prot_info initialization out of tls_set_sw_offload
>   tls: also use init_prot_info in tls_set_device_offload
>   tls: add a helper to allocate/initialize offload_ctx_tx
>   tls: remove tls_context argument from tls_set_sw_offload
>   tls: remove tls_context argument from tls_set_device_offload
>   tls: validate crypto_info in a separate helper
>   chcr_ktls: use tls_offload_context_tx and driver_state like other
>     drivers
>   tls: use fixed size for tls_offload_context_{tx,rx}.driver_state
> 
>  .../chelsio/inline_crypto/ch_ktls/chcr_ktls.c |  43 ++--
>  .../chelsio/inline_crypto/ch_ktls/chcr_ktls.h |  36 +++-
>  include/net/tls.h                             |  21 +-
>  net/tls/tls.h                                 |  12 +-
>  net/tls/tls_device.c                          | 101 ++++-----
>  net/tls/tls_device_fallback.c                 |  23 +-
>  net/tls/tls_main.c                            |  62 +++---
>  net/tls/tls_sw.c                              | 198 +++++++++---------
>  8 files changed, 244 insertions(+), 252 deletions(-)

Thanks Sabrina,

this was a nice set of cleanups to read.

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net-next 00/14] net: tls: various code cleanups and improvements

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Mon,  9 Oct 2023 22:50:40 +0200 you wrote:
> This series contains multiple cleanups and simplifications for the
> config code of both TLS_SW and TLS_HW.
> 
> It also modifies the chcr_ktls driver to use driver_state like all
> other drivers, so that we can then make driver_state fixed size
> instead of a flex array always allocated to that same fixed size. As
> reported by Gustavo A. R. Silva, the way chcr_ktls misuses
> driver_state irritates GCC [1].
> 
> [...]

Here is the summary with links:
  - [net-next,01/14] tls: get salt using crypto_info_salt in tls_enc_skb
    https://git.kernel.org/netdev/net-next/c/3bab3ee0f95e
  - [net-next,02/14] tls: drop unnecessary cipher_type checks in tls offload
    https://git.kernel.org/netdev/net-next/c/8f1d532b4a49
  - [net-next,03/14] tls: store rec_seq directly within cipher_context
    https://git.kernel.org/netdev/net-next/c/6d5029e54700
  - [net-next,04/14] tls: rename MAX_IV_SIZE to TLS_MAX_IV_SIZE
    https://git.kernel.org/netdev/net-next/c/bee6b7b30706
  - [net-next,05/14] tls: store iv directly within cipher_context
    https://git.kernel.org/netdev/net-next/c/1c1cb3110d7e
  - [net-next,06/14] tls: extract context alloc/initialization out of tls_set_sw_offload
    https://git.kernel.org/netdev/net-next/c/615580cbc99a
  - [net-next,07/14] tls: move tls_prot_info initialization out of tls_set_sw_offload
    https://git.kernel.org/netdev/net-next/c/a9937816edde
  - [net-next,08/14] tls: also use init_prot_info in tls_set_device_offload
    https://git.kernel.org/netdev/net-next/c/1a074f7618e8
  - [net-next,09/14] tls: add a helper to allocate/initialize offload_ctx_tx
    https://git.kernel.org/netdev/net-next/c/013740799987
  - [net-next,10/14] tls: remove tls_context argument from tls_set_sw_offload
    https://git.kernel.org/netdev/net-next/c/b6a30ec9239a
  - [net-next,11/14] tls: remove tls_context argument from tls_set_device_offload
    https://git.kernel.org/netdev/net-next/c/4f4866991847
  - [net-next,12/14] tls: validate crypto_info in a separate helper
    https://git.kernel.org/netdev/net-next/c/1cf7fbcee60a
  - [net-next,13/14] chcr_ktls: use tls_offload_context_tx and driver_state like other drivers
    https://git.kernel.org/netdev/net-next/c/0700aa3a7503
  - [net-next,14/14] tls: use fixed size for tls_offload_context_{tx,rx}.driver_state
    https://git.kernel.org/netdev/net-next/c/9f0c8245516b

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net-next 08/14] tls: also use init_prot_info in tls_set_device_offload

[Jakub Kicinski]

On Mon,  9 Oct 2023 22:50:48 +0200 Sabrina Dubroca wrote:
> +	if (mode == TLS_HW) {
> +		prot->aad_size = 0;
> +		prot->tail_size = 0;
> +	}

Strange, tail_size doesn't matter because HW doesn't support TLS 1.3
but aad_size?  Is it overwritten by SW init or something?

Re: [PATCH net-next 00/14] net: tls: various code cleanups and improvements

[Jakub Kicinski]

On Mon,  9 Oct 2023 22:50:40 +0200 Sabrina Dubroca wrote:
> This series contains multiple cleanups and simplifications for the
> config code of both TLS_SW and TLS_HW.
> 
> It also modifies the chcr_ktls driver to use driver_state like all
> other drivers, so that we can then make driver_state fixed size
> instead of a flex array always allocated to that same fixed size. As
> reported by Gustavo A. R. Silva, the way chcr_ktls misuses
> driver_state irritates GCC [1].
> 
> Patches 1 and 2 are follow-ups to my previous cipher_desc series.

Nice cleanups FWIW! Sorry I didn't get to acking it in time :S

Re: [PATCH net-next 08/14] tls: also use init_prot_info in tls_set_device_offload

[Tariq Toukan]

On 09/10/2023 23:50, Sabrina Dubroca wrote:
> Most values are shared. Nonce size turns out to be equal to IV size
> for all offloadable ciphers.
> 
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> ---
>   net/tls/tls.h        |  4 ++++
>   net/tls/tls_device.c | 14 ++++----------
>   net/tls/tls_sw.c     | 14 ++++++++++----
>   3 files changed, 18 insertions(+), 14 deletions(-)
> 

Hi,

FYI, we caught some new failures in kTLS device-offload traffic, 
bisected to this patch.
We're trying to collect more info and analyze...

Regards,
Tariq

Re: [PATCH net-next 08/14] tls: also use init_prot_info in tls_set_device_offload

[Sabrina Dubroca]

2023-10-13, 14:23:07 -0700, Jakub Kicinski wrote:
> On Mon,  9 Oct 2023 22:50:48 +0200 Sabrina Dubroca wrote:
> > +	if (mode == TLS_HW) {
> > +		prot->aad_size = 0;
> > +		prot->tail_size = 0;
> > +	}
> 
> Strange, tail_size doesn't matter because HW doesn't support TLS 1.3
> but aad_size?  Is it overwritten by SW init or something?

For RX, yes, tls_set_device_offload_rx -> tls_set_sw_offload ->
  init_prot_info(mode=TLS_SW).
But aad_size is not used in tls_device_reencrypt, maybe because (for
both GCM variants)
    TLS_HEADER_SIZE + cipher_desc->iv == TLS_AAD_SPACE_SIZE

For TX, it looks like tls_device_fallback hardcodes TLS_AAD_SPACE_SIZE
where tls_sw would use prot->aad_size. tls_device doesn't use either.

Actually this patch is broken. If we set TLS_RX to TLS_SW first (for
example because we disabled tls-hw-rx-offload with ethtool), and
TLS_TX to TLS_HW second, that will set aad_size to 0, but RX needs it
to be set.

I'll send a fix to drop this hunk completely. Thanks for reviewing.

-- 
Sabrina