public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Sabrina Dubroca <sd@queasysnail.net>
To: Rishikesh Jethwani <rjethwani@purestorage.com>
Cc: netdev@vger.kernel.org, saeedm@nvidia.com, tariqt@nvidia.com,
	mbloch@nvidia.com, borisp@nvidia.com, john.fastabend@gmail.com,
	kuba@kernel.org, davem@davemloft.net, pabeni@redhat.com,
	edumazet@google.com, leon@kernel.org
Subject: Re: [RFC PATCH v7 1/4] tls: add TLS 1.3 hardware offload support
Date: Mon, 23 Feb 2026 15:34:00 +0100	[thread overview]
Message-ID: <aZxlWCVi4RZhRWB4@krikkit> (raw)
In-Reply-To: <20260205231558.139818-2-rjethwani@purestorage.com>

The subject prefix needs to contain "PATCH net-next" to tell which
tree this applies to (net-next since it's a new feature).

See https://docs.kernel.org/process/maintainer-netdev.html


2026-02-05, 16:15:55 -0700, Rishikesh Jethwani wrote:
> Add TLS 1.3 support to the kernel TLS hardware offload infrastructure,
> enabling hardware acceleration for TLS 1.3 connections on capable NICs.
> 
> TLS 1.3 differs from TLS 1.2 in several key ways that affect hardware
> offload:
> 
> 1. Content type handling: TLS 1.3 encrypts the content type as part of
>    the ciphertext (inner content type), with the outer header always
>    showing application_data. Added content type byte appending in
>    tls_device_record_close() before the authentication tag.
> 
> 2. Version validation: Extended tls_set_device_offload() and
>    tls_set_device_offload_rx() to accept TLS_1_3_VERSION in addition
>    to TLS_1_2_VERSION.
> 
> 3. Software fallback: Updated tls_device_fallback.c to handle TLS 1.3
>    IV construction (XOR with sequence number instead of explicit IV)
>    and version-specific AAD sizes (5 bytes for TLS 1.3 vs 13 bytes for
>    TLS 1.2).
> 
> 4. Reencrypt path: Modified tls_device_reencrypt() to use
>    prot->prepend_size and prot->tag_size instead of hardcoded
>    TLS 1.2 values.
> 
> 5. Memory fallback: Pre-populate dummy_page with identity mapping for
>    all 256 byte values, enabling safe fallback when memory allocation
>    fails during TLS 1.3 content type appending.
> 
> 6. Rekey handling: HW offload key update (rekey) is not yet supported.
>    Added upfront checks to reject rekey on HW offload connections with
>    -EOPNOTSUPP. For SW connections, rekey skips device offload attempts
>    and goes directly to software path.

Really not a fan of the LLM output. I don't think most of this needs
to be in the commit message, or at least it should be much less
verbose.

Most of the comments in this series are also LLM verbosity, and
entirely unnecessary.


> Tested on Mellanox ConnectX-6 Dx (Crypto Enabled) with TLS 1.3 AES-GCM-128
> and AES-GCM-256 cipher suites.
> 
> Signed-off-by: Rishikesh Jethwani <rjethwani@purestorage.com>
> ---
>  net/tls/tls_device.c          | 57 ++++++++++++++++++----
>  net/tls/tls_device_fallback.c | 36 ++++++++++----
>  net/tls/tls_main.c            | 89 +++++++++++++++++++++--------------
>  3 files changed, 129 insertions(+), 53 deletions(-)
> 
> diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
> index 82ea407e520a..459963c254f4 100644
> --- a/net/tls/tls_device.c
> +++ b/net/tls/tls_device.c
> @@ -319,6 +319,33 @@ static void tls_device_record_close(struct sock *sk,
>  	struct tls_prot_info *prot = &ctx->prot_info;
>  	struct page_frag dummy_tag_frag;
>  
> +	/* TLS 1.3: append content type byte before tag.
> +	 * Record structure: [Header (5)] + [Ciphertext + ContentType (1)]
> +	 * + [Tag (16)]
> +	 * The content type is encrypted with the ciphertext for authentication.
> +	 */
> +	if (prot->version == TLS_1_3_VERSION) {
> +		struct page_frag dummy_content_type_frag;
> +		struct page_frag *content_type_pfrag = pfrag;
> +
> +		if (unlikely(pfrag->size - pfrag->offset < prot->tail_size) &&
> +		    !skb_page_frag_refill(prot->tail_size, pfrag,
> +					  sk->sk_allocation)) {
> +			/* Out of memory: use pre-populated dummy_page */

nit: unnecessary comment

Maybe a silly question but if we have this fallback to the dummy_page,
why even bother trying to allocate?

> +			dummy_content_type_frag.page = dummy_page;
> +			dummy_content_type_frag.offset = record_type;
> +			content_type_pfrag = &dummy_content_type_frag;
> +		} else {
> +			/* Write content type to current pfrag */

nit: unnecessary comment

> +			unsigned char *content_type_addr;
> +
> +			content_type_addr = page_address(pfrag->page) +
> +					    pfrag->offset;
> +			*content_type_addr = record_type;
> +		}
> +		tls_append_frag(record, content_type_pfrag, prot->tail_size);
> +	}
> +
>  	/* append tag
>  	 * device will fill in the tag, we just need to append a placeholder
>  	 * use socket memory to improve coalescing (re-using a single buffer
> @@ -335,7 +362,7 @@ static void tls_device_record_close(struct sock *sk,
>  
>  	/* fill prepend */
>  	tls_fill_prepend(ctx, skb_frag_address(&record->frags[0]),
> -			 record->len - prot->overhead_size,
> +			 (record->len - prot->overhead_size) + prot->tail_size,

nit: why the parens?

>  			 record_type);
>  }
>  
> @@ -883,6 +910,7 @@ static int
>  tls_device_reencrypt(struct sock *sk, struct tls_context *tls_ctx)
>  {
>  	struct tls_sw_context_rx *sw_ctx = tls_sw_ctx_rx(tls_ctx);
> +	struct tls_prot_info *prot = &tls_ctx->prot_info;
>  	const struct tls_cipher_desc *cipher_desc;
>  	int err, offset, copy, data_len, pos;
>  	struct sk_buff *skb, *skb_iter;
> @@ -894,7 +922,7 @@ tls_device_reencrypt(struct sock *sk, struct tls_context *tls_ctx)
>  	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,
> +	orig_buf = kmalloc(rxm->full_len + prot->prepend_size,
>  			   sk->sk_allocation);
>  	if (!orig_buf)
>  		return -ENOMEM;
> @@ -909,9 +937,8 @@ tls_device_reencrypt(struct sock *sk, struct tls_context *tls_ctx)
>  	offset = rxm->offset;
>  
>  	sg_init_table(sg, 1);
> -	sg_set_buf(&sg[0], buf,
> -		   rxm->full_len + TLS_HEADER_SIZE + cipher_desc->iv);
> -	err = skb_copy_bits(skb, offset, buf, TLS_HEADER_SIZE + cipher_desc->iv);
> +	sg_set_buf(&sg[0], buf, rxm->full_len + prot->prepend_size);
> +	err = skb_copy_bits(skb, offset, buf, prot->prepend_size);
>  	if (err)
>  		goto free_buf;
>  
> @@ -922,7 +949,7 @@ tls_device_reencrypt(struct sock *sk, struct tls_context *tls_ctx)
>  	else
>  		err = 0;
>  
> -	data_len = rxm->full_len - cipher_desc->tag;
> +	data_len = rxm->full_len - prot->tag_size;

Does this actually need to be changed? prot->tag_size doesn't vary
based on TLS version.

>  
>  	if (skb_pagelen(skb) > offset) {
>  		copy = min_t(int, skb_pagelen(skb) - offset, data_len);
> @@ -1089,7 +1116,8 @@ int tls_set_device_offload(struct sock *sk)
>  	}
>  
>  	crypto_info = &ctx->crypto_send.info;
> -	if (crypto_info->version != TLS_1_2_VERSION) {
> +	if (crypto_info->version != TLS_1_2_VERSION &&
> +	    crypto_info->version != TLS_1_3_VERSION) {

You can get rid of this whole version check, it was only needed
because HW offload supported less than SW. tls_main already ensures
that the version is 1.2 or 1.3.

>  		rc = -EOPNOTSUPP;
>  		goto release_netdev;
>  	}
> @@ -1196,7 +1224,8 @@ int tls_set_device_offload_rx(struct sock *sk, struct tls_context *ctx)
>  	struct net_device *netdev;
>  	int rc = 0;
>  
> -	if (ctx->crypto_recv.info.version != TLS_1_2_VERSION)
> +	if (ctx->crypto_recv.info.version != TLS_1_2_VERSION &&
> +	    ctx->crypto_recv.info.version != TLS_1_3_VERSION)

And same here.

But on the topic of version checks, Jakub asked you previously [1] to
make sure all drivers reject 1.3, and your reply only mentioned 3
drivers. We have 5 in tree (and the 2 your reply didn't mention don't
seem to have a version check, based on a quick look).

[1] https://lore.kernel.org/netdev/20260105172745.5cc67e79@kernel.org/


[...]
> diff --git a/net/tls/tls_device_fallback.c b/net/tls/tls_device_fallback.c
> index 03d508a45aae..c8d18cce80ed 100644
> --- a/net/tls/tls_device_fallback.c
> +++ b/net/tls/tls_device_fallback.c
> @@ -55,7 +55,7 @@ static int tls_enc_record(struct aead_request *aead_req,
>  	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;
> +	buf_size = prot->prepend_size;
>  	len = min_t(int, *in_len, buf_size);
>  
>  	memcpy_from_scatterwalk(buf, in, len);
> @@ -66,16 +66,24 @@ static int tls_enc_record(struct aead_request *aead_req,
>  		return 0;
>  
>  	len = buf[4] | (buf[3] << 8);
> -	len -= cipher_desc->iv;
> +	if (prot->version != TLS_1_3_VERSION)
> +		len -= cipher_desc->iv;
>  
>  	tls_make_aad(aad, len - cipher_desc->tag, (char *)&rcd_sn, buf[0], prot);
>  
> -	memcpy(iv + cipher_desc->salt, buf + TLS_HEADER_SIZE, cipher_desc->iv);
> +	/* For TLS 1.2, copy explicit IV from record header.
> +	 * For TLS 1.3, IV was already set up and we XOR with sequence number.
> +	 */
> +	if (prot->version == TLS_1_3_VERSION)
> +		tls_xor_iv_with_seq(prot, iv, (char *)&rcd_sn);

tls_sw calls this unconditionally. I think it would make sense to be
consistent for the whole iv generation.

> +	else
> +		memcpy(iv + cipher_desc->salt, buf + TLS_HEADER_SIZE,
> +		       cipher_desc->iv);
>  
>  	sg_init_table(sg_in, ARRAY_SIZE(sg_in));
>  	sg_init_table(sg_out, ARRAY_SIZE(sg_out));
> -	sg_set_buf(sg_in, aad, TLS_AAD_SPACE_SIZE);
> -	sg_set_buf(sg_out, aad, TLS_AAD_SPACE_SIZE);
> +	sg_set_buf(sg_in, aad, prot->aad_size);
> +	sg_set_buf(sg_out, aad, prot->aad_size);
>  	scatterwalk_get_sglist(in, sg_in + 1);
>  	scatterwalk_get_sglist(out, sg_out + 1);
>  
> @@ -112,7 +120,6 @@ static void tls_init_aead_request(struct aead_request *aead_req,
>  				  struct crypto_aead *aead)
>  {
>  	aead_request_set_tfm(aead_req, aead);
> -	aead_request_set_ad(aead_req, TLS_AAD_SPACE_SIZE);

Then please get rid of this function entirely. We don't need a wrapper
for a single call to the crypto API.

>  }
>  
>  static struct aead_request *tls_alloc_aead_request(struct crypto_aead *aead,
> @@ -303,9 +310,9 @@ static struct sk_buff *tls_enc_skb(struct tls_context *tls_ctx,
>  {
>  	struct tls_offload_context_tx *ctx = tls_offload_ctx_tx(tls_ctx);
>  	int tcp_payload_offset = skb_tcp_all_headers(skb);
> +	void *buf, *iv, *aad, *dummy_buf, *salt, *iv_src;
>  	int payload_len = skb->len - tcp_payload_offset;
>  	const struct tls_cipher_desc *cipher_desc;
> -	void *buf, *iv, *aad, *dummy_buf, *salt;
>  	struct aead_request *aead_req;
>  	struct sk_buff *nskb = NULL;
>  	int buf_len;
> @@ -317,7 +324,11 @@ static struct sk_buff *tls_enc_skb(struct tls_context *tls_ctx,
>  	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 +
> +	/* Set AAD size based on TLS version */

another unnecessary comment.

[...]
> +		/* HW rekey not yet supported */

(here too)

[...]
> +		/* Only try HW offload on initial setup, not rekey */

(here too)

-- 
Sabrina

  reply	other threads:[~2026-02-23 14:34 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-05 23:15 [RFC PATCH v7 0/4] tls: Add TLS 1.3 hardware offload support Rishikesh Jethwani
2026-02-05 23:15 ` [RFC PATCH v7 1/4] tls: add " Rishikesh Jethwani
2026-02-23 14:34   ` Sabrina Dubroca [this message]
2026-02-23 21:33     ` Rishikesh Jethwani
2026-02-05 23:15 ` [RFC PATCH v7 2/4] tls: add hardware offload key update support Rishikesh Jethwani
2026-02-23 14:40   ` Sabrina Dubroca
2026-02-05 23:15 ` [RFC PATCH v7 3/4] mlx5: TLS 1.3 hardware offload support Rishikesh Jethwani
2026-02-05 23:15 ` [RFC PATCH v7 4/4] selftests: drivers: net: hw: add TLS hardware offload test Rishikesh Jethwani

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aZxlWCVi4RZhRWB4@krikkit \
    --to=sd@queasysnail.net \
    --cc=borisp@nvidia.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=kuba@kernel.org \
    --cc=leon@kernel.org \
    --cc=mbloch@nvidia.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=rjethwani@purestorage.com \
    --cc=saeedm@nvidia.com \
    --cc=tariqt@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox