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

From: Sabrina Dubroca <[email-masked]>
Date: Wed, 6 May 2026 12:37:35 +0200

2026-05-05, 10:40:41 +0200, Paolo Abeni wrote:
> On 4/29/26 8:10 PM, Rishikesh Jethwani wrote:
> > On TX, the NIC key cannot be replaced while HW-offloaded records
> > are still unacked. tls_dev_start_rekey() installs a temporary SW
> > context with the new key and redirects sendmsg through
> > tls_sw_sendmsg_locked. If no records are pending,
> > tls_dev_complete_rekey() runs inline during setsockopt; otherwise
> > clean_acked sets REKEY_READY once all old-key records are ACKed
> > and the next sendmsg completes the rekey, flushing SW records and
> > reinstalling HW offload at the current write_seq. A KeyUpdate
> > arriving while one is pending re-keys the SW AEAD in place; if the
> > HW reinstall fails the socket stays in SW mode (REKEY_FAILED).
> > 
> > On RX, the NIC may have already decrypted in-flight records with
> > the old key before the peer's KeyUpdate is parsed, so the old
> > AEAD, IV and rec_seq are retained on tls_offload_context_rx.
> > tls_check_pending_rekey() invokes tls_device_rx_del_key() to drop
> > the NIC key; otherwise post-KeyUpdate records (carrying new-key
> > wire encryption) would be XOR'd with the retired key.
> > tls_device_decrypted() classifies records by old_nic_boundary:
> > 
> >   - after the boundary: new-key record; drop the old key.
> >   - before, fully encrypted: advance old_rec_seq, let SW AEAD decrypt.
> >   - before, (partially) decrypted: reencrypt with the old key so SW
> >     AEAD can decrypt with the new key.
> > 
> > For mixed records skb->decrypted flags can be wrong (NIC clears
> > them on auth failure); on -EBADMSG, tls_rx_rekey_retry() toggles
> > those flags, decrements old_rec_seq to reuse the nonce, and
> > retries once (gated by old_key_reencrypted).
> > 
> > The new key's tls_dev_add is deferred until the old key is fully
> > consumed: tls_set_device_offload_rx() sets dev_add_pending while
> > old_aead_recv is retained, and tls_device_deferred_dev_add()
> > installs the new key once copied_seq crosses old_nic_boundary.
> > 
> > Tested on Mellanox ConnectX-6 Dx (Crypto Enabled) with multiple
> > TLS 1.3 TX and RX KeyUpdate cycles.
> > 
> > Signed-off-by: Rishikesh Jethwani <[email-masked]>
> > ---
> >  include/net/tls.h             |  84 +++-
> >  include/uapi/linux/snmp.h     |   2 +
> >  net/tls/tls.h                 |  29 +-
> >  net/tls/tls_device.c          | 753 +++++++++++++++++++++++++++++++---
> >  net/tls/tls_device_fallback.c |  24 ++
> >  net/tls/tls_main.c            |  92 +++--
> >  net/tls/tls_proc.c            |   2 +
> >  net/tls/tls_sw.c              |  76 +++-
> >  net/tls/trace.h               |  79 ++++
> 
> This patch is really big and complex and you should break it to help
> reviewers.
> 
> At very least you can split out the tracing bits and the trivial
> refactor moving around declaration and definitions to separate patches.

True, that would make review of the refactorings easier, and then the
changes for rekey would be more obvious.

Things like tls_sw_ctx_tx_init, adding/using tls_tx_cipher_ctx and
others helpers, function renames, splitting of
tls_set_device_offload*, maybe others.

Based on this, I won't review the whole rekey code this time around (I
think it will be much easier to follow the logic after the split), but
I have some comments that I'll send out today.

-- 
Sabrina