Re: [PATCH v9 5/6] tls: add hardware offload key update support

[Sabrina Dubroca <sd@queasysnail.net>]

2026-03-20, 17:57:05 -0600, Rishikesh Jethwani wrote:
> Add TLS KeyUpdate (rekey) support for hardware offload connections.
> 
> When tls_dev_add() fails during hardware rekey, the connection
> gracefully degrades to software encryption/decryption while
> maintaining TLS_HW configuration.
> 
> Tested on Mellanox ConnectX-6 Dx (Crypto Enabled) with multiple
> TLS 1.3 key update cycles.

Now your commit messages have gone from "overly detailed descriptions
of every minor change" to not describing anything about a pretty
complex patch. This is also not helpful for reviewers.

Something like an overview of the parts that you're adding (flags,
extra SW context, saved sequence numbers, etc) and how they come
together to make the key transition work would probably help us figure
out what this patch does and if it does it correctly.

[short example: during the key transition a temporary SW ctx is used
because [...]. clean_acked keeps track of ACKs and sets the READY flag
when [...], then the next sendmsg call will complete the rekey in the
offload ctx and tear down the temporary SW ctx. etc]



Only a few comments on the rekey completion for now, I'm still looking
at the patch.

[...]
> +static int tls_device_complete_rekey(struct sock *sk, struct tls_context *ctx)
> +{
[...]
> +	rc = netdev->tlsdev_ops->tls_dev_add(netdev, sk, TLS_OFFLOAD_CTX_DIR_TX,
> +					     &offload_ctx->rekey_crypto_send.info,
> +					     tcp_sk(sk)->write_seq);
> +
[...]
> +	/* following this assignment tls_is_skb_tx_device_offloaded
> +	 * will return true and the context might be accessed
> +	 * by the netdev's xmit function.
> +	 */
> +	smp_store_release(&sk->sk_validate_xmit_skb, tls_validate_xmit_skb);
> +
> +	tls_sw_release_resources_tx(sk);

The write_seq we passed to tls_dev_add could be wrong if this ends up
pushing encrypted data on the socket?

> +	ctx->rekey_sw_ctx = NULL;
> +	ctx->rekey_cipher_ctx = NULL;
> +
> +	return 0;


[...]
> @@ -187,6 +404,32 @@ static void tls_tcp_clean_acked(struct sock *sk, u32 acked_seq)
>  	}
>  
>  	ctx->unacked_record_sn += deleted_records;
> +
> +	/* Track ACKs to determine when HW rekey can complete:
> +	 * complete_seq captures write_seq when old-key data is ACKed,
> +	 * REKEY_READY is set once all pending data (including any new) is
> +	 * ACKed.
> +	 */
> +	if (test_bit(TLS_TX_REKEY_PENDING, &tls_ctx->flags) &&
> +	    !test_bit(TLS_TX_REKEY_FAILED, &tls_ctx->flags)) {
> +		u32 boundary_seq = READ_ONCE(tls_ctx->rekey_boundary_seq);
> +		u32 complete_seq = READ_ONCE(tls_ctx->rekey_complete_seq);
> +
> +		if (!before(acked_seq, boundary_seq) && complete_seq == 0) {
> +			complete_seq = tcp_sk(sk)->write_seq;
> +			WRITE_ONCE(tls_ctx->rekey_complete_seq, complete_seq);
> +		}
> +
> +		if (complete_seq != 0) {
> +			u32 current_write_seq = tcp_sk(sk)->write_seq;
> +
> +			if (before(complete_seq, current_write_seq))
> +				WRITE_ONCE(tls_ctx->rekey_complete_seq, current_write_seq);
> +			else if (!before(acked_seq, complete_seq))
> +				set_bit(TLS_TX_REKEY_READY, &tls_ctx->flags);
> +		}

This is a really tricky part. AFAIU the rekey will only complete if
the peer manages to ACK fast enough, so that we don't already have
more data already in flight? And if not, we stay in SW mode "forever"?

Could we move to REKEY_READY as soon as acked >= boundary, and let
closing the SW context handle all the data that was sent since the
rekey was requested? Then we have a situation similar to the initial
setup, with some data floating in the queues that doesn't need to be
encrypted (in the initial setup case, because it's pre-encryption;
here, because it went through SW encrypt), and HW taking over after
that.

-- 
Sabrina