* Question: net/mlx5e kTLS TX TIS reuse vs in-flight WQEs
@ 2026-10-22 18:17 Rishikesh Jethwani
2026-11-03 14:45 ` Tariq Toukan
0 siblings, 1 reply; 9+ messages in thread
From: Rishikesh Jethwani @ 2026-10-22 18:17 UTC (permalink / raw)
To: tariqt
Cc: Saeed Mahameed, Mark Bloch, Gal Pressman, Jakub Kicinski,
Boris Pismenny, Sabrina Dubroca, John Fastabend, netdev
Hi Tariq,
Could a DUMP/SEND WQE from an old owner on SQ A read or update TIS
state after SQ B has reinitialised that TIS for a new owner?
The existing TIS pool predates live rekey. My TLS 1.3 TX KeyUpdate
series (v17 [1]; v18 in preparation) calls tls_dev_del() and then
tls_dev_add() on a live socket, and the question arises through this
sequence:
1. A spurious RTO retransmit of old-key data is posted on SQ A.
2. While that data remains unacknowledged, setsockopt(TLS_TX) starts
rekey and installs the rekey validator, which stops admitting new
plaintext to hardware offload. An ACK covering all remaining
old-key data marks rekey ready before the driver observes the
retransmit's SQ completion. A subsequent sendmsg() enters rekey
completion.
3. Rekey completion calls synchronize_net(), which waits for CPU
transmit paths, not SQ completions, so a WQE on a stalled or
backlogged SQ A can still be pending. mlx5e_ktls_del_tx() then
releases the DEK and immediately calls pool_push(). DEK
reclamation uses batched SYNC_CRYPTO; returning the TIS does not
wait for synchronization covering its old DEK.
4. Before re-adding the offload, TLS allocates the new SW-fallback
AEAD. In that interval another connection's mlx5e_ktls_add_tx()
can pop the returned TIS and use it on SQ B. The same socket can
also reach SQ B on re-add: with the retransmit queue empty, TCP
sets ooo_okay even while the retransmitted skb still holds
sk_wmem_alloc, so queue selection may move the flow.
mlx5e_ktls_handle_tx_skb() posts the new owner's initial
parameters with skip_static_post=false, fence_first_post=false:
static parameters unfenced, progress parameters fenced, then a
fence NOP.
An unobserved completion does not by itself mean the WQE still accesses
the TIS; that is the hardware ordering question. The same question
applies on SQ A: resync fences its first parameter update, initial
setup does not, and I do not know what either guarantees against
earlier WQEs that reference the TIS.
The normal close path has additional ordering. tls_dev_del() runs
from work queued by tls_device_sk_destruct(), which itself runs from
__sk_free(), i.e. only once sk_wmem_alloc reaches zero. For
transmitted mlx5 skbs, SQ completion calls napi_consume_skb(), which
invokes tcp_wfree() to release that accounting, and parameter and DUMP
WQEs preceding a posted SEND on the same SQ are retired by that SEND's
completion. For successful transmissions, this gives close-time
ordering without an explicit drain in del_tx. WQEs posted without a
subsequent SEND, and SQ error/reset paths, need separate lifetime
guarantees. Live rekey does not wait for socket destruction.
Was the recycling fence relying on this socket-lifetime ordering, or
does it independently protect against outstanding work on another SQ?
c4dfe704f53f ("net/mlx5e: kTLS, Recycle objects of device-offloaded
TLS TX connections") says the fence ensures context modification does
not interfere with existing traffic. Does that cover old SEND, DUMP
and parameter WQEs on another SQ, including work the device has not
yet executed?
If reuse must wait, would retaining the TIS and DEK until all
referencing work has completed or been definitively stopped suffice,
including during SQ error/reset?
I have not reproduced a failure; the reuse is visible in the driver,
but I have not established the hardware ordering guarantee. A PRM
reference, or a statement of which completion must precede reuse,
would settle it.
Thanks,
Rishikesh
[1] https://lore.kernel.org/netdev/20260917224355.2288021-1-rjethwani@purestorage.com/
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: Question: net/mlx5e kTLS TX TIS reuse vs in-flight WQEs 2026-10-22 18:17 Question: net/mlx5e kTLS TX TIS reuse vs in-flight WQEs Rishikesh Jethwani @ 2026-11-03 14:45 ` Tariq Toukan 2026-11-04 5:01 ` Rishikesh Jethwani 0 siblings, 1 reply; 9+ messages in thread From: Tariq Toukan @ 2026-11-03 14:45 UTC (permalink / raw) To: Rishikesh Jethwani, tariqt Cc: Saeed Mahameed, Mark Bloch, Gal Pressman, Jakub Kicinski, Boris Pismenny, Sabrina Dubroca, John Fastabend, netdev On 22/10/2026 21:17, Rishikesh Jethwani wrote: > Hi Tariq, > Hi Rishikesh, Good analysis, and thanks for the detailed write-up. > Could a DUMP/SEND WQE from an old owner on SQ A read or update TIS > state after SQ B has reinitialised that TIS for a new owner? > Short answer: Yes, in principle it could, once del runs on a live socket as in your series. Today, only parameter/DUMP WQEs of a dropped skb can be left behind (item 1 below): a pending SEND holds sk_wmem_alloc, so the socket is not destructed before it completes. The TIS is a function-global object, not bound to an SQ: a WQE that references its TISN reads/updates its state whenever that WQE executes, from whichever SQ it was posted on, and the hardware provides no cross-SQ ordering for such accesses. Details below, and at the end a proposal for the way forward, on which I'd appreciate your feedback. > The existing TIS pool predates live rekey. My TLS 1.3 TX KeyUpdate > series (v17 [1]; v18 in preparation) calls tls_dev_del() and then > tls_dev_add() on a live socket, and the question arises through this > sequence: > > 1. A spurious RTO retransmit of old-key data is posted on SQ A. > 2. While that data remains unacknowledged, setsockopt(TLS_TX) starts > rekey and installs the rekey validator, which stops admitting new > plaintext to hardware offload. An ACK covering all remaining > old-key data marks rekey ready before the driver observes the > retransmit's SQ completion. A subsequent sendmsg() enters rekey > completion. > 3. Rekey completion calls synchronize_net(), which waits for CPU > transmit paths, not SQ completions, so a WQE on a stalled or > backlogged SQ A can still be pending. mlx5e_ktls_del_tx() then > releases the DEK and immediately calls pool_push(). DEK > reclamation uses batched SYNC_CRYPTO; returning the TIS does not > wait for synchronization covering its old DEK. > 4. Before re-adding the offload, TLS allocates the new SW-fallback > AEAD. In that interval another connection's mlx5e_ktls_add_tx() > can pop the returned TIS and use it on SQ B. The same socket can > also reach SQ B on re-add: with the retransmit queue empty, TCP > sets ooo_okay even while the retransmitted skb still holds > sk_wmem_alloc, so queue selection may move the flow. > mlx5e_ktls_handle_tx_skb() posts the new owner's initial > parameters with skip_static_post=false, fence_first_post=false: > static parameters unfenced, progress parameters fenced, then a > fence NOP. > Your reading of the driver is accurate on all points. In particular, mlx5e_ktls_del_tx() returns the TIS to the pool with no completion tracking. The same goes for the DEK: the batched SYNC_CRYPTO guards against stale cached keys when a DEK object is reused, but nothing waits for pending WQEs before that, and without bulk DEK support the DEK is destroyed right away. So the DEK needs the same lifetime guarantee as the TIS. The same-socket exposure is also broader than the ooo_okay disjunct: the RTO and PLB rehash paths reset the dst, which clears the cached TX queue, and txq_reselection_ms lets a busy flow re-pick its queue periodically, both regardless of ooo_okay. So one connection's WQEs can be in flight on two SQs today, even without rekey. > An unobserved completion does not by itself mean the WQE still accesses > the TIS; that is the hardware ordering question. The same question > applies on SQ A: resync fences its first parameter update, initial > setup does not, and I do not know what either guarantees against > earlier WQEs that reference the TIS. > The fence modes in the WQE ctrl segment (FM field; the parameter WQEs use Initiator_Small_Fence) are all defined in terms of previous WQEs on the same SQ: a fenced WQE waits until earlier WQEs on that SQ are no longer in execution. Even the strongest mode (Strong_Ordering) orders only against previous WQEs of the same SQ. No fence mode, and no other WQE-level primitive, orders against outstanding work on another SQ. So within one SQ, the fenced first parameter WQE of a resync does guarantee that earlier WQEs referencing the TIS are done executing before the context is modified. The unfenced static parameters of the initial setup are safe only because, under the current lifetime contract, the previous owner's WQEs are known to be complete by the time the TIS leaves the pool. Across SQs, nothing is ordered. > The normal close path has additional ordering. tls_dev_del() runs > from work queued by tls_device_sk_destruct(), which itself runs from > __sk_free(), i.e. only once sk_wmem_alloc reaches zero. For > transmitted mlx5 skbs, SQ completion calls napi_consume_skb(), which > invokes tcp_wfree() to release that accounting, and parameter and DUMP > WQEs preceding a posted SEND on the same SQ are retired by that SEND's > completion. For successful transmissions, this gives close-time > ordering without an explicit drain in del_tx. WQEs posted without a > subsequent SEND, and SQ error/reset paths, need separate lifetime > guarantees. Live rekey does not wait for socket destruction. > Correct, including the completion detail: parameter and DUMP WQEs do not request CQEs of their own, so they are retired only when a later CQE-generating WQE on the same SQ completes. And you have a point about WQEs posted without a subsequent SEND: the error paths of mlx5e_ktls_handle_tx_skb(), and a DMA mapping failure of the SEND itself in mlx5e_sq_xmit_wqe(), drop the skb after parameter/DUMP WQEs were posted for it. Its wmem is then released while those WQEs are still pending. That is a narrow, pre-existing hole in the destruct-time argument, independent of rekey. SQ error/reset is covered: mlx5e_free_txqsq_descs() runs only once the SQ was moved out of RDY, when the hardware is done with it. > Was the recycling fence relying on this socket-lifetime ordering, or > does it independently protect against outstanding work on another SQ? > c4dfe704f53f ("net/mlx5e: kTLS, Recycle objects of device-offloaded > TLS TX connections") says the fence ensures context modification does > not interfere with existing traffic. Does that cover old SEND, DUMP > and parameter WQEs on another SQ, including work the device has not > yet executed? > It relies on the socket-lifetime ordering. The fence in c4dfe704f53f addresses the same-SQ pipelining: the new owner's data must not be transmitted before the context update has completed, and the update must not interfere with WQEs in execution on that SQ. It does not, and cannot, cover WQEs pending on another SQ; those were assumed complete by the destruct-time contract you describe, or stopped by the channel teardown. Live rekey via del+add breaks that assumption. > If reuse must wait, would retaining the TIS and DEK until all > referencing work has completed or been definitively stopped suffice, > including during SQ error/reset? > Yes, that is the right condition: every WQE referencing the TIS (or the DEK) has either completed - a CQE of a later WQE on the same SQ retires it, as the completions of an SQ are in order - or its SQ was moved out of RDY and its descriptors were reclaimed in mlx5e_free_txqsq_descs(), which covers the error/recovery and teardown paths. As for how to get there, here is what I have in mind, in three parts. Feedback is welcome on all of them, and in particular on the third, as it affects your series. 1. Pre-existing, independent of rekey: the stranded WQEs above. Rather than adding completion tracking for the TIS, I'd restore the socket-lifetime contract in the driver: an skb dropped after WQEs were posted for it is not freed right away, but held by a NOP that requests a completion, so that the skb and its wmem are released only once all WQEs posted ahead of it are retired (or the SQ is reset). mlx5e_ktls_del_tx() stays as is. I intend to send this as a fix to net. 2. Pre-existing, and a dependency of rekey: TX queue stability. I'd like to address the queue moves above in the stack, opt-in: a driver whose TX offload keeps per-connection state updated by queue execution declares it with a tlsdev_ops flag, and for sockets offloaded to it, the TX queue is re-selected only once sk_wmem_alloc has drained. That is, ooo_okay keeps only its wmem condition, and while wmem is held, a dst reset does not clear the cached queue and txq_reselection_ms does not expire it. With (1), a drained wmem means that no work referencing the connection state is pending on any SQ. txhash rethink and PLB still fire, and their network-level repathing (e.g. the IPv6 flowlabel) is unaffected; only the local queue switch waits until the driver drains. Other drivers, and non-offloaded sockets, keep today's behavior. 3. Rekey. Releasing the TIS of a live connection to the pool, as del+add does, requires that no old-key work is still pending, which the driver cannot cheaply tell. I see two directions: a. Keep the TIS, and replace the key in place through a new tlsdev op (say, tls_dev_rekey()). With (2), the new parameters are posted on the same SQ as all old-key WQEs, fenced, so they take effect only after them, and the pool is not involved. The driver keeps the replaced key until it is no longer in use. For that, the op would be called only once all data sent with the current key was acknowledged, which, as I understand it, your rekey completion already waits for, and which is needed anyway, as the device cannot retransmit old-key data after the switch. b. Keep del+add, and have the TLS core call tls_dev_del() only once sk_wmem_alloc has drained. With (1), that guarantees no pending work references the TIS or the DEK. The cost is a potentially long wait on a backlogged or paused queue, during which new data is held back or sent in SW. Tracking SEND completions per connection in the driver would also work, but means per-packet accounting for a rare event. I lean towards (a). Would it fit your series, or do you see reasons to keep del+add? > I have not reproduced a failure; the reuse is visible in the driver, > but I have not established the hardware ordering guarantee. A PRM > reference, or a statement of which completion must precede reuse, > would settle it. > Thanks, Tariq > Thanks, > Rishikesh > > [1] https://lore.kernel.org/netdev/20260917224355.2288021-1-rjethwani@purestorage.com/ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Question: net/mlx5e kTLS TX TIS reuse vs in-flight WQEs 2026-11-03 14:45 ` Tariq Toukan @ 2026-11-04 5:01 ` Rishikesh Jethwani 2026-11-05 9:25 ` Tariq Toukan 0 siblings, 1 reply; 9+ messages in thread From: Rishikesh Jethwani @ 2026-11-04 5:01 UTC (permalink / raw) To: Tariq Toukan Cc: Saeed Mahameed, Mark Bloch, Gal Pressman, Jakub Kicinski, Boris Pismenny, Sabrina Dubroca, John Fastabend, netdev On Tue, Nov 3, 2026 at 6:45 AM Tariq Toukan <tariqt@nvidia.com> wrote: > > > > On 22/10/2026 21:17, Rishikesh Jethwani wrote: > > Hi Tariq, > > Hi Tariq, Thanks, this answers the cross-SQ ordering question. Short version: I agree with (1), and (3a) fits the series; I prefer it. For (2), I agree with the goal, but wmem-gated reselection alone does not keep a connection's work on one SQ in mlx5e; the holes are listed below, with two ways to close them, and I'd like your preference. Two hardware questions here, and a third on initial setup under (1): - Does a completed DESTROY_SQ, or finding an SQ in ERR, guarantee that none of its WQEs can still access a TIS or DEK? And can DESTROY_SQ fail while the function is healthy? In mlx5e_close_txqsq(), the void mlx5e_destroy_sq() wrapper discards the destroy status, and mlx5e_free_txqsq_descs() then frees descriptors, advancing sq->cc without CQEs, so the "moved out of RDY" argument depends on the answer. - DEK release in mlx5_crypto_dek_destroy() has two paths. Without a DEK pool (dek_priv == NULL) the object is destroyed right away; with one, the DEK is pushed back to the pool, its id can be handed to another connection after the batched SYNC_CRYPTO, and the object itself is destroyed later from destroy_work. After all WQEs referencing a DEK have retired, is either path, including reuse of a pooled id by another connection, safe while an idle TIS still names that DEK, before replacement static parameters are posted? Or is today's release only safe because of the destruct-time ordering? > Hi Rishikesh, > > Good analysis, and thanks for the detailed write-up. > > > Could a DUMP/SEND WQE from an old owner on SQ A read or update TIS > > state after SQ B has reinitialised that TIS for a new owner? > > > > Short answer: Yes, in principle it could, once del runs on a live socket > as in your series. Today, only parameter/DUMP WQEs of a dropped skb can > be left behind (item 1 below): a pending SEND holds sk_wmem_alloc, so > the socket is not destructed before it completes. > > The TIS is a function-global object, not bound to an SQ: a WQE that > references its TISN reads/updates its state whenever that WQE executes, > from whichever SQ it was posted on, and the hardware provides no > cross-SQ ordering for such accesses. Details below, and at the end a > proposal for the way forward, on which I'd appreciate your feedback. > > > The existing TIS pool predates live rekey. My TLS 1.3 TX KeyUpdate > > series (v17 [1]; v18 in preparation) calls tls_dev_del() and then > > tls_dev_add() on a live socket, and the question arises through this > > sequence: > > > > 1. A spurious RTO retransmit of old-key data is posted on SQ A. > > 2. While that data remains unacknowledged, setsockopt(TLS_TX) starts > > rekey and installs the rekey validator, which stops admitting new > > plaintext to hardware offload. An ACK covering all remaining > > old-key data marks rekey ready before the driver observes the > > retransmit's SQ completion. A subsequent sendmsg() enters rekey > > completion. > > 3. Rekey completion calls synchronize_net(), which waits for CPU > > transmit paths, not SQ completions, so a WQE on a stalled or > > backlogged SQ A can still be pending. mlx5e_ktls_del_tx() then > > releases the DEK and immediately calls pool_push(). DEK > > reclamation uses batched SYNC_CRYPTO; returning the TIS does not > > wait for synchronization covering its old DEK. > > 4. Before re-adding the offload, TLS allocates the new SW-fallback > > AEAD. In that interval another connection's mlx5e_ktls_add_tx() > > can pop the returned TIS and use it on SQ B. The same socket can > > also reach SQ B on re-add: with the retransmit queue empty, TCP > > sets ooo_okay even while the retransmitted skb still holds > > sk_wmem_alloc, so queue selection may move the flow. > > mlx5e_ktls_handle_tx_skb() posts the new owner's initial > > parameters with skip_static_post=false, fence_first_post=false: > > static parameters unfenced, progress parameters fenced, then a > > fence NOP. > > > > Your reading of the driver is accurate on all points. In particular, > mlx5e_ktls_del_tx() returns the TIS to the pool with no completion > tracking. The same goes for the DEK: the batched SYNC_CRYPTO guards > against stale cached keys when a DEK object is reused, but nothing > waits for pending WQEs before that, and without bulk DEK support the > DEK is destroyed right away. So the DEK needs the same lifetime > guarantee as the TIS. > > The same-socket exposure is also broader than the ooo_okay disjunct: > the RTO and PLB rehash paths reset the dst, which clears the cached > TX queue, and txq_reselection_ms lets a busy flow re-pick its queue > periodically, both regardless of ooo_okay. So one connection's WQEs > can be in flight on two SQs today, even without rekey. > > > An unobserved completion does not by itself mean the WQE still accesses > > the TIS; that is the hardware ordering question. The same question > > applies on SQ A: resync fences its first parameter update, initial > > setup does not, and I do not know what either guarantees against > > earlier WQEs that reference the TIS. > > > > The fence modes in the WQE ctrl segment (FM field; the parameter WQEs > use Initiator_Small_Fence) are all defined in terms of previous WQEs > on the same SQ: a fenced WQE waits until earlier WQEs on that SQ are > no longer in execution. Even the strongest mode (Strong_Ordering) > orders only against previous WQEs of the same SQ. No fence mode, and > no other WQE-level primitive, orders against outstanding work on > another SQ. > > So within one SQ, the fenced first parameter WQE of a resync does > guarantee that earlier WQEs referencing the TIS are done executing > before the context is modified. The unfenced static parameters of the > initial setup are safe only because, under the current lifetime > contract, the previous owner's WQEs are known to be complete by the > time the TIS leaves the pool. Across SQs, nothing is ordered. > > > The normal close path has additional ordering. tls_dev_del() runs > > from work queued by tls_device_sk_destruct(), which itself runs from > > __sk_free(), i.e. only once sk_wmem_alloc reaches zero. For > > transmitted mlx5 skbs, SQ completion calls napi_consume_skb(), which > > invokes tcp_wfree() to release that accounting, and parameter and DUMP > > WQEs preceding a posted SEND on the same SQ are retired by that SEND's > > completion. For successful transmissions, this gives close-time > > ordering without an explicit drain in del_tx. WQEs posted without a > > subsequent SEND, and SQ error/reset paths, need separate lifetime > > guarantees. Live rekey does not wait for socket destruction. > > > > Correct, including the completion detail: parameter and DUMP WQEs do > not request CQEs of their own, so they are retired only when a later > CQE-generating WQE on the same SQ completes. And you have a point > about WQEs posted without a subsequent SEND: the error paths of > mlx5e_ktls_handle_tx_skb(), and a DMA mapping failure of the SEND > itself in mlx5e_sq_xmit_wqe(), drop the skb after parameter/DUMP WQEs > were posted for it. Its wmem is then released while those WQEs are > still pending. That is a narrow, pre-existing hole in the > destruct-time argument, independent of rekey. SQ error/reset is > covered: mlx5e_free_txqsq_descs() runs only once the SQ was moved out > of RDY, when the hardware is done with it. > > > Was the recycling fence relying on this socket-lifetime ordering, or > > does it independently protect against outstanding work on another SQ? > > c4dfe704f53f ("net/mlx5e: kTLS, Recycle objects of device-offloaded > > TLS TX connections") says the fence ensures context modification does > > not interfere with existing traffic. Does that cover old SEND, DUMP > > and parameter WQEs on another SQ, including work the device has not > > yet executed? > > > > It relies on the socket-lifetime ordering. The fence in c4dfe704f53f > addresses the same-SQ pipelining: the new owner's data must not be > transmitted before the context update has completed, and the update > must not interfere with WQEs in execution on that SQ. It does not, and > cannot, cover WQEs pending on another SQ; those were assumed complete > by the destruct-time contract you describe, or stopped by the channel > teardown. Live rekey via del+add breaks that assumption. > > > If reuse must wait, would retaining the TIS and DEK until all > > referencing work has completed or been definitively stopped suffice, > > including during SQ error/reset? > > > > Yes, that is the right condition: every WQE referencing the TIS (or > the DEK) has either completed - a CQE of a later WQE on the same SQ > retires it, as the completions of an SQ are in order - or its SQ was > moved out of RDY and its descriptors were reclaimed in > mlx5e_free_txqsq_descs(), which covers the error/recovery and > teardown paths. > > As for how to get there, here is what I have in mind, in three parts. > Feedback is welcome on all of them, and in particular on the third, > as it affects your series. > > 1. Pre-existing, independent of rekey: the stranded WQEs above. > Rather than adding completion tracking for the TIS, I'd restore the > socket-lifetime contract in the driver: an skb dropped after WQEs > were posted for it is not freed right away, but held by a NOP that > requests a completion, so that the skb and its wmem are released > only once all WQEs posted ahead of it are retired (or the SQ is > reset). mlx5e_ktls_del_tx() stays as is. I intend to send this as a > fix to net. Agreed. Three spots for it: on SEND DMA failure, mlx5e_sq_xmit_wqe() and mlx5e_sq_xmit_mpwqe() already post a NOP through mlx5e_tx_flush() but free the skb first, so the skb should be held instead; that NOP asks for a completion because mlx5e_notify_hw() sets MLX5_WQE_CTRL_CQ_UPDATE, not mlx5e_post_nop(). mlx5e_ktls_handle_tx_skb()'s err_out needs the same hold-NOP and a doorbell, since it can leave parameter/DUMP WQEs posted without one. The MPWQE case is reachable too: when TX MPWQE is enabled, mlx5e_sq_xmit_mpwqe() takes any linear, untagged, non-GSO skb with inline mode NONE, which includes a pass-through skb that posted pending parameters and took SKIP_NO_DATA, since it leaves with no TISN. One correction to "SQ error/reset is covered": error-CQE recovery never runs mlx5e_free_txqsq_descs(). mlx5e_tx_reporter_err_cqe_recover() waits in mlx5e_wait_for_sq_flush() for cc to reach pc, then zeroes both counters, so on success every WQE has been retired by an error CQE and the held skb is released normally. If that wait times out, the SQ stays in ERR with its queue stopped but NAPI still polling, so unretired skbs stay held until a later completion or close. Timeout recovery is different: mlx5e_safe_reopen_channels() closes the old SQs and reclaims their descriptors in mlx5e_free_txqsq_descs(). In all of them the hold-NOP from (1) eventually gets its skb released; whether the device has stopped touching the TIS by then is the first question above. A related recovery gap exists today without rekey: if a connection's initial static parameters are flushed without executing, recovery does not re-arm ctx_post_pending. A retransmit of the same record can also skip static parameters, so software does not guarantee that the new context gets installed. What does the hardware do with a SEND naming a TIS whose static parameters never executed, recycled or freshly created? The rekey contract below handles the corresponding replacement case; initial setup may need the same treatment. When do you expect to post (1)? I'll base the series on it and can test on ConnectX-6 Dx before you post. > 2. Pre-existing, and a dependency of rekey: TX queue stability. I'd > like to address the queue moves above in the stack, opt-in: a > driver whose TX offload keeps per-connection state updated by queue > execution declares it with a tlsdev_ops flag, and for sockets > offloaded to it, the TX queue is re-selected only once > sk_wmem_alloc has drained. That is, ooo_okay keeps only its wmem > condition, and while wmem is held, a dst reset does not clear the > cached queue and txq_reselection_ms does not expire it. With (1), a > drained wmem means that no work referencing the connection state is > pending on any SQ. txhash rethink and PLB still fire, and their > network-level repathing (e.g. the IPv6 flowlabel) is unaffected; > only the local queue switch waits until the driver drains. Other > drivers, and non-offloaded sockets, keep today's behavior. Gating ooo_okay, the dst-reset queue clear and txq_reselection_ms on wmem covers every path through sk_tx_queue_mapping. In mlx5e the SQ can still change while that mapping stays put, or bypass it: - Channel replacement with tis_tir_td_order destroys the old channels only after the new ones are active (3b88a535a8e1). mlx5e_deactivate_txqsq() posts a final NOP, but nothing waits for its CQE, and mlx5e_poll_tx_cq() returns early once ENABLED is cleared. The deactivated SQ can still hold old-key WQEs (e.g. under PFC pause) while the same txq index already names the new SQ, so (3a)'s "same SQ as all old-key WQEs" holds only once the old SQ is destroyed, which is the DESTROY_SQ question above. - With num_tcs > 1, mlx5e_select_queue() adds a priority offset after netdev_pick_tx() (DSCP under MLX5_QPTS_TRUST_DSCP, else VLAN prio; HTB by skb->priority), so IP_TOS/IPV6_TCLASS or SO_PRIORITY changes move the SQ. - Egress skbedit queue_mapping bypasses ndo_select_queue(), and SKBEDIT_F_TXQ_SKBHASH can move a flow after a txhash rethink. - A queue-count change: netdev_pick_tx() ignores a cached index >= real_num_tx_queues whatever wmem says. Separately, "a drained wmem means that no work referencing the connection state is pending on any SQ" has a pre-existing hole: netem with delay, jitter or rate calls skb_orphan_partial(), which keeps skb->sk but swaps tcp_wfree for sock_efree, releasing the wmem charge while the skb is parked. can_skb_orphan_partial() refuses decrypted skbs and anything not owned through sock_wfree/tcp_wfree, so two kinds of tcp_wfree skb get through: - plain TCP data queued before setsockopt(TLS_TX); at dequeue it ends before the start marker, and mlx5e_ktls_handle_tx_skb() posts the pending parameter WQEs naming the new TIS and takes the MLX5E_KTLS_SYNC_SKIP_NO_DATA path; - new-key ciphertext the SW context sent during rekey, an ordinary tcp_wfree skb not marked decrypted; dequeued after rekey completion, it passes through tls_validate_xmit_skb() on the device or on a bond master, and ends before the completion's marker. The socket reference held by skb_set_owner_sk_safe() still delays destruct, so your destruct-time argument in (1) is unaffected; only the wmem gate in (2) can open early. Both orphan paths release the charge, but (2) reads a drained charge as no outstanding work; the partial orphan keeps the reference and drops the charge, so the two come apart. Both kinds of skb above can be handled in the driver alone: mlx5e can skip the context WQEs for an skb with skb->destructor != tcp_wfree (the test tcp_gso_segment() already uses), which covers the pre-install plaintext, whose orphan decision netem_enqueue() made before the socket was offloaded, and the parked rekey ciphertext alike. Both already go out as they are; the pending parameters just wait for the next skb. No record data awaiting HW encryption reaches that path: a decrypted skb is never partially orphaned, and a fully orphaned decrypted one is dropped in validate_xmit_skb() before any driver sees it. Refusing partial orphan in can_skb_orphan_partial() would also close the rekey case, but needs a core-to-TLS hook and still misses the pre-install one. So (2) as proposed is a good stability hint, but not proof. I see two ways to get correctness: i. Close the holes individually, without a new per-packet lock: the driver check above, keeping the priority offset and skbedit on the pinned queue for offloaded sockets, and, while kTLS TX connections are live, closing the old channels before the switch; mlx5e can tell that today only by subtracting tx_tls_del from tx_tls_ctx, so it needs a proper count. That last part gives back the downtime 3b88a535a8e1 saved, and still relies on your answer to the first question; (i) only moves that dependency earlier. It also spreads the invariant across the stack and the driver, and a future queue-selection path can silently break it. ii. A driver-side owner check where WQEs are posted: priv_tx records its owner SQ as (txq index, generation) and the last WQE counter it posted there; a post on another SQ is admitted only once the owner's completions cover that counter or its generation is retired (by a completed DESTROY_SQ or recovery, if the first question above confirms that suffices), and is otherwise SW-encrypted with tls_encrypt_skb(), as chcr, nfp and funeth do. Completion progress stays per SQ, so this is not per-connection completion accounting, but it does need a per-connection lock on the kTLS xmit path, which is uncontended unless the flow moves. I'd measure it before posting. I lean towards (ii) for robustness, with (2) as the hint that keeps it off the slow path, but I'd rather hear your view on the hot-path cost first. > 3. Rekey. Releasing the TIS of a live connection to the pool, as > del+add does, requires that no old-key work is still pending, which > the driver cannot cheaply tell. I see two directions: > > a. Keep the TIS, and replace the key in place through a new tlsdev > op (say, tls_dev_rekey()). With (2), the new parameters are > posted on the same SQ as all old-key WQEs, fenced, so they take > effect only after them, and the pool is not involved. The driver > keeps the replaced key until it is no longer in use. For that, > the op would be called only once all data sent with the current > key was acknowledged, which, as I understand it, your rekey > completion already waits for, and which is needed anyway, as the > device cannot retransmit old-key data after the switch. > > b. Keep del+add, and have the TLS core call tls_dev_del() only once > sk_wmem_alloc has drained. With (1), that guarantees no pending > work references the TIS or the DEK. The cost is a potentially > long wait on a backlogged or paused queue, during which new data > is held back or sent in SW. > > Tracking SEND completions per connection in the driver would also > work, but means per-packet accounting for a rare event. > > I lean towards (a). Would it fit your series, or do you see reasons > to keep del+add? (3a) fits, and I prefer it. sendmsg keeps sending SW ciphertext during rekey, which holds wmem, so (3b) can wait indefinitely unless the sender stalls. With this series, mlx5e is the only in-tree driver accepting TLS 1.3 crypto_info, so the core can require the new op for TX rekey and leave other devices in SW. Proposed contract: int (*tls_dev_rekey)(struct net_device *netdev, struct sock *sk, enum tls_offload_ctx_dir direction, struct tls_crypto_info *crypto_info, u32 start_offload_tcp_sn); - direction is kept for symmetry with tls_dev_add()/tls_dev_del(); only TX uses it for now, RX stays on del+add. - Called from tls_device_complete_rekey() in process context, under tx_lock, the socket lock and device_offload_lock (read), where tls_dev_del() is called today. All old-key records are ACKed and synchronize_net() has drained in-progress transmissions; skbs still in a qdisc are revalidated at dequeue by the rekey validator. crypto_info carries the new key and the live record sequence; start_offload_tcp_sn is write_seq. - The core builds the new fallback AEAD before the call (today it is built between del and add) and commits it, with the record marker, before restoring the HW validator. Until then the driver sees no new offload plaintext. - The driver keeps the TIS, allocates the new DEK and does all sleeping firmware work first, then publishes key, expected sequence and pending parameters. The re-post covers both static and progress parameters, with the fence on the first WQE (unlike today's initial post, where the static params go out unfenced). - The old DEK is freed from process context once a successful CQE covers the fenced replacement, subject to your answer to the second question above. - Retirement of the SQ only proves the old work stopped, not that the replacement took effect. If the SQ errors or closes before a successful CQE covers the replacement, the driver forces a resync on the next offloaded skb regardless of expected_seq: fenced static and progress params for that skb's record, with the skip-static shortcut bypassed, and a DUMP of any record prefix before the SEND. A plain repost is not enough, since expected_seq has already advanced and the next skb can match it while starting mid-record. - On failure, the old context and key stay intact, nothing is posted, and traffic stays in SW until teardown or a later KeyUpdate. If this direction works for you, I'll write mlx5e_ktls_rekey_tx() and whichever (2) option you prefer on top of your fix, and test them on the same hardware. Thanks, Rishikesh > > > I have not reproduced a failure; the reuse is visible in the driver, > > but I have not established the hardware ordering guarantee. A PRM > > reference, or a statement of which completion must precede reuse, > > would settle it. > > > > Thanks, > Tariq > > > Thanks, > > Rishikesh > > > > [1] https://lore.kernel.org/netdev/20260917224355.2288021-1-rjethwani@purestorage.com/ > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Question: net/mlx5e kTLS TX TIS reuse vs in-flight WQEs 2026-11-04 5:01 ` Rishikesh Jethwani @ 2026-11-05 9:25 ` Tariq Toukan 2026-11-06 19:58 ` Rishikesh Jethwani 0 siblings, 1 reply; 9+ messages in thread From: Tariq Toukan @ 2026-11-05 9:25 UTC (permalink / raw) To: Rishikesh Jethwani Cc: Saeed Mahameed, Mark Bloch, Gal Pressman, Jakub Kicinski, Boris Pismenny, Sabrina Dubroca, John Fastabend, netdev On 04/11/2026 7:01, Rishikesh Jethwani wrote: > Thanks, this answers the cross-SQ ordering question. Short version: > I agree with (1), and (3a) fits the series; I prefer it. For (2), I > agree with the goal, but wmem-gated reselection alone does not keep a > connection's work on one SQ in mlx5e; the holes are listed below, > with two ways to close them, and I'd like your preference. Hi Rishikesh, Thanks, very thorough again. Short version from my side: I'd go with (i), closing the holes, rather than (ii), and with (3a) along the lines of your contract. As the queue moves corrupt records today, regardless of rekey, I'd send (2) and the mlx5e side of (i) to net as fixes; I could reproduce the corruption on ConnectX-6 Dx. Details inline. Feedback is welcome, especially where it touches your series. > Two hardware questions here, and a third on initial setup under (1): > > - Does a completed DESTROY_SQ, or finding an SQ in ERR, guarantee > that none of its WQEs can still access a TIS or DEK? And can > DESTROY_SQ fail while the function is healthy? [...] > - DEK release in mlx5_crypto_dek_destroy() has two paths. Without [...] > After all WQEs referencing a DEK have retired, is either path, > including reuse of a pooled id by another connection, safe while > an idle TIS still names that DEK, before replacement static > parameters are posted? Or is today's release only safe because > of the destruct-time ordering? Good questions. I'll take them to our HW/FW folks, and get back to you once I have answers I can back up. Meanwhile, what I propose below relies only on what the close path already relies on today: a CQE retires all earlier WQEs of its SQ, and once an SQ is destroyed, none of its WQEs executes. >> 1. Pre-existing, independent of rekey: the stranded WQEs above. [...] > Agreed. Three spots for it: on SEND DMA failure, mlx5e_sq_xmit_wqe() and > mlx5e_sq_xmit_mpwqe() already post a NOP through mlx5e_tx_flush() but > free the skb first, so the skb should be held instead; that NOP asks for > a completion because mlx5e_notify_hw() sets MLX5_WQE_CTRL_CQ_UPDATE, not > mlx5e_post_nop(). mlx5e_ktls_handle_tx_skb()'s err_out needs the same > hold-NOP and a doorbell, since it can leave parameter/DUMP WQEs posted > without one. The MPWQE case is reachable too: when TX MPWQE is enabled, > mlx5e_sq_xmit_mpwqe() takes any linear, untagged, non-GSO skb with > inline mode NONE, which includes a pass-through skb that posted pending > parameters and took SKIP_NO_DATA, since it leaves with no TISN. Right, the patch covers the same three spots: mlx5e_tx_flush() takes the skb and attaches it to its NOP, which requests a completion through mlx5e_notify_hw(), and both SEND error paths hand it the skbs of kTLS sockets, MPWQE included. > One correction to "SQ error/reset is covered": error-CQE recovery never > runs mlx5e_free_txqsq_descs(). mlx5e_tx_reporter_err_cqe_recover() > waits in mlx5e_wait_for_sq_flush() for cc to reach pc, then zeroes both > counters, so on success every WQE has been retired by an error CQE and > the held skb is released normally. [...] Correct, thanks. > A related recovery gap exists today without rekey: if a connection's > initial static parameters are flushed without executing, recovery does > not re-arm ctx_post_pending. A retransmit of the same record can also > skip static parameters, so software does not guarantee that the new > context gets installed. What does the hardware do with a SEND naming a > TIS whose static parameters never executed, recycled or freshly created? > The rekey contract below handles the corresponding replacement case; > initial setup may need the same treatment. Agreed, and it is wider than the static params: any lost WQE leaves the TIS state off from what the driver tracks. WQEs are lost when an skb is dropped after its kTLS WQEs were posted (or after expected_seq advanced past it), when an SQ error flushes them, and when an SQ is closed with WQEs pending. The driver holds the skb in each of these, so I'd mark its connection right there, and have the next skb of a marked connection resync fully: static params regardless of rec_seq, progress params, and a DUMP of the record prefix. That covers initial setup, and the replacement case of your contract, with no rekey-specific code. I'd send it to net together with (1). > When do you expect to post (1)? I'll base the series on it and can > test on ConnectX-6 Dx before you post. After internal review and testing; I can share the patches as RFCs in the meantime. (1) goes to net together with the resync fix above and the fixes for (2) below: the stack part, the mlx5e queue pin, the channel replacement overlap, and the pass-through skbs. I tested them on ConnectX-6 Dx, and testing on your side is very welcome, especially with your series on top. >> 2. Pre-existing, and a dependency of rekey: TX queue stability. I'd [...] > Gating ooo_okay, the dst-reset queue clear and txq_reselection_ms > on wmem covers every path through sk_tx_queue_mapping. In mlx5e the > SQ can still change while that mapping stays put, or bypass it: > > - Channel replacement with tis_tir_td_order destroys the old > channels only after the new ones are active (3b88a535a8e1). > mlx5e_deactivate_txqsq() posts a final NOP, but nothing waits for > its CQE, and mlx5e_poll_tx_cq() returns early once ENABLED is > cleared. The deactivated SQ can still hold old-key WQEs (e.g. > under PFC pause) while the same txq index already names the new > SQ, so (3a)'s "same SQ as all old-key WQEs" holds only once the > old SQ is destroyed, which is the DESTROY_SQ question above. Right, and that overlap is a bug of 3b88a535a8e1 on its own. I'd fix it by draining the old SQs before activating the new channels: their NAPI is already disabled, so the switch reaps their completions directly, and the old channels are still closed after the activation, which keeps the downtime saving. If they don't drain in time (e.g. under a long pause), it warns and falls back to closing them first. No kTLS-specific count is needed. > - With num_tcs > 1, mlx5e_select_queue() adds a priority offset > after netdev_pick_tx() (DSCP under MLX5_QPTS_TRUST_DSCP, else VLAN > prio; HTB by skb->priority), so IP_TOS/IPV6_TCLASS or SO_PRIORITY > changes move the SQ. mlx5e_select_queue() can keep the final queue of such sockets: cache it in sk_tx_queue_mapping, and reuse it as long as ooo_okay is clear. A priority change then takes effect once the connection has nothing in flight, which for a busy bulk flow can take a while; that's the price of keeping the order. > - Egress skbedit queue_mapping bypasses ndo_select_queue(), and > SKBEDIT_F_TXQ_SKBHASH can move a flow after a txhash rethink. I'd document it as not supported with this kind of offload, and have the driver count it (see below). > - A queue-count change: netdev_pick_tx() ignores a cached index > >= real_num_tx_queues whatever wmem says. In mlx5e that comes only with a channel replacement, which the drain covers, or with an HTB leaf deletion, which closes the leaf SQ before its queue is removed. > Separately, "a drained wmem means that no work referencing the > connection state is pending on any SQ" has a pre-existing hole: > netem with delay, jitter or rate calls skb_orphan_partial(), which > keeps skb->sk but swaps tcp_wfree for sock_efree, releasing the wmem > charge while the skb is parked. [...] > Both kinds of skb above can be handled in the driver alone: mlx5e can > skip the context WQEs for an skb with skb->destructor != tcp_wfree (the > test tcp_gso_segment() already uses), which covers the pre-install > plaintext, whose orphan decision netem_enqueue() made before the socket > was offloaded, and the parked rekey ciphertext alike. [...] Agreed that it belongs in the driver. Rather than keying on the destructor, I wrote it so that nothing is posted for an skb that doesn't use the context: the pending params are posted only once the skb is known to need them (in order, or resynced, where they fold into the full resync), and stay pending on SKIP_NO_DATA. A pass-through skb then touches nothing, wherever it was parked. Does that cover both of your cases? > So (2) as proposed is a good stability hint, but not proof. I see > two ways to get correctness: > > i. Close the holes individually, without a new per-packet lock: the [...] > ii. A driver-side owner check where WQEs are posted: priv_tx [...] > I lean towards (ii) for robustness, with (2) as the hint that keeps > it off the slow path, but I'd rather hear your view on the hot-path > cost first. I'd prefer (i). With the drain, it no longer needs to close channels first or to count connections, and the rest is small and local. (ii) adds a lock and a SW encryption fallback to the mlx5e xmit path, for a case that shouldn't happen once the holes are closed; a fallback that hardly ever runs is also hard to keep correct. And as these moves corrupt records today, without rekey, I'd send (2) and the mlx5e queue pin to net as fixes. On ConnectX-6 Dx with many channels, kTLS TX records already fail decryption at the receiver with the default txq_reselection_ms, and forcing moves breaks most connections; with the net fixes, the same runs are clean. As for a future path silently breaking the invariant, the driver can catch that cheaply: it remembers the SQ that a connection last posted on, and when the next skb arrives on another SQ while ooo_okay is clear and the old SQ is still live, it bumps a counter and warns once. That is a pointer compare per skb, and it turns a silent corruption into a visible one. > (3a) fits, and I prefer it. sendmsg keeps sending SW ciphertext during > rekey, which holds wmem, so (3b) can wait indefinitely unless the > sender stalls. With this series, mlx5e is the only in-tree driver > accepting TLS 1.3 crypto_info, so the core can require the new op for > TX rekey and leave other devices in SW. > > Proposed contract: > > int (*tls_dev_rekey)(struct net_device *netdev, struct sock *sk, > enum tls_offload_ctx_dir direction, > struct tls_crypto_info *crypto_info, > u32 start_offload_tcp_sn); Looks good to me. > - The old DEK is freed from process context once a successful > CQE covers the fenced replacement, subject to your answer to the > second question above. Agreed, and it's simpler than I first thought: the driver doesn't need to track completions per connection. When it posts the fenced replacement, it can hand the old DEK to that WQE's descriptor, the way DUMP WQEs hold their page refs today, and release it once the SQ retires that WQE. Completions being in order, that covers every old-key WQE posted before it on the SQ. A flush or an SQ close retires it too; the TIS may then still name the old DEK until the resync installs the new one, which is your second question again, and the same as an idle TIS in the pool today. The release runs from NAPI, so the destroy itself would be deferred to a work item. > - Retirement of the SQ only proves the old work stopped, not that > the replacement took effect. If the SQ errors or closes before > a successful CQE covers the replacement, the driver forces a > resync on the next offloaded skb regardless of expected_seq: [...] That's the resync fix above, which isn't specific to rekey. One question on the core side: once the HW validator is restored, can a decrypted old-key skb still be queued, e.g. a spurious retransmit parked in a qdisc, and reach the driver? It would end before the new marker, so mlx5e would take SKIP_NO_DATA with a decrypted skb, which today hits WARN_ON_ONCE() and drops it. > If this direction works for you, I'll write mlx5e_ktls_rekey_tx() > and whichever (2) option you prefer on top of your fix, and test > them on the same hardware. Sounds good. (2) is covered by the net fixes, so please go ahead with the op and mlx5e_ktls_rekey_tx() on top of them, and I'll review. The RFCs mentioned above should let you start before the fixes land. Thanks, Tariq ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Question: net/mlx5e kTLS TX TIS reuse vs in-flight WQEs 2026-11-05 9:25 ` Tariq Toukan @ 2026-11-06 19:58 ` Rishikesh Jethwani 2026-11-08 19:45 ` Tariq Toukan 0 siblings, 1 reply; 9+ messages in thread From: Rishikesh Jethwani @ 2026-11-06 19:58 UTC (permalink / raw) To: Tariq Toukan Cc: Saeed Mahameed, Mark Bloch, Gal Pressman, Jakub Kicinski, Boris Pismenny, Sabrina Dubroca, John Fastabend, netdev On Thu, Nov 5, 2026 at 1:26 AM Tariq Toukan <tariqt@nvidia.com> wrote: > > > On 04/11/2026 7:01, Rishikesh Jethwani wrote: > > Thanks, this answers the cross-SQ ordering question. Short version: > > I agree with (1), and (3a) fits the series; I prefer it. For (2), I > > agree with the goal, but wmem-gated reselection alone does not keep a > > connection's work on one SQ in mlx5e; the holes are listed below, > > with two ways to close them, and I'd like your preference. > > Hi Rishikesh, Hi Tariq, Thanks. Agreed on (3a), (i) and the net fixes, with one condition on (i) below. Answers to your two questions are below, plus what rekey needs from the fixes before I build on them. > Thanks, very thorough again. Short version from my side: I'd go with > (i), closing the holes, rather than (ii), and with (3a) along the lines > of your contract. As the queue moves corrupt records today, regardless > of rekey, I'd send (2) and the mlx5e side of (i) to net as fixes; I > could reproduce the corruption on ConnectX-6 Dx. Details inline. > Feedback is welcome, especially where it touches your series. > > > Two hardware questions here, and a third on initial setup under (1): > > > > - Does a completed DESTROY_SQ, or finding an SQ in ERR, guarantee > > that none of its WQEs can still access a TIS or DEK? And can > > DESTROY_SQ fail while the function is healthy? > [...] > > - DEK release in mlx5_crypto_dek_destroy() has two paths. Without > [...] > > After all WQEs referencing a DEK have retired, is either path, > > including reuse of a pooled id by another connection, safe while > > an idle TIS still names that DEK, before replacement static > > parameters are posted? Or is today's release only safe because > > of the destruct-time ordering? > > Good questions. I'll take them to our HW/FW folks, and get back to you > once I have answers I can back up. Meanwhile, what I propose below > relies only on what the close path already relies on today: a CQE > retires all earlier WQEs of its SQ, and once an SQ is destroyed, none > of its WQEs executes. Thanks. Until then, the two hardware questions remain open, including whether DESTROY_SQ can fail; the destroy wrapper discards the status. One correction to my last mail: recovery clears RECOVERING even on a flush timeout, so the queue can wake once stop_room frees. > >> 1. Pre-existing, independent of rekey: the stranded WQEs above. > [...] > > Agreed. Three spots for it: on SEND DMA failure, mlx5e_sq_xmit_wqe() and > > mlx5e_sq_xmit_mpwqe() already post a NOP through mlx5e_tx_flush() but > > free the skb first, so the skb should be held instead; that NOP asks for > > a completion because mlx5e_notify_hw() sets MLX5_WQE_CTRL_CQ_UPDATE, not > > mlx5e_post_nop(). mlx5e_ktls_handle_tx_skb()'s err_out needs the same > > hold-NOP and a doorbell, since it can leave parameter/DUMP WQEs posted > > without one. The MPWQE case is reachable too: when TX MPWQE is enabled, > > mlx5e_sq_xmit_mpwqe() takes any linear, untagged, non-GSO skb with > > inline mode NONE, which includes a pass-through skb that posted pending > > parameters and took SKIP_NO_DATA, since it leaves with no TISN. > > Right, the patch covers the same three spots: mlx5e_tx_flush() takes > the skb and attaches it to its NOP, which requests a completion through > mlx5e_notify_hw(), and both SEND error paths hand it the skbs of kTLS > sockets, MPWQE included. > > > One correction to "SQ error/reset is covered": error-CQE recovery never > > runs mlx5e_free_txqsq_descs(). mlx5e_tx_reporter_err_cqe_recover() > > waits in mlx5e_wait_for_sq_flush() for cc to reach pc, then zeroes both > > counters, so on success every WQE has been retired by an error CQE and > > the held skb is released normally. > [...] > > Correct, thanks. > > > A related recovery gap exists today without rekey: if a connection's > > initial static parameters are flushed without executing, recovery does > > not re-arm ctx_post_pending. A retransmit of the same record can also > > skip static parameters, so software does not guarantee that the new > > context gets installed. What does the hardware do with a SEND naming a > > TIS whose static parameters never executed, recycled or freshly created? > > The rekey contract below handles the corresponding replacement case; > > initial setup may need the same treatment. > > Agreed, and it is wider than the static params: any lost WQE leaves the > TIS state off from what the driver tracks. WQEs are lost when an skb is > dropped after its kTLS WQEs were posted (or after expected_seq advanced > past it), when an SQ error flushes them, and when an SQ is closed with > WQEs pending. The driver holds the skb in each of these, so I'd mark > its connection right there, and have the next skb of a marked > connection resync fully: static params regardless of rec_seq, progress > params, and a DUMP of the record prefix. That covers initial setup, and > the replacement case of your contract, with no rekey-specific code. I'd > send it to net together with (1). Agreed. What rekey needs from that resync fix: a lost initialization or replacement always triggers a full resync on the next skb that needs the TLS context, and a connection marked again while the driver is consuming an earlier mark stays marked (test-and-clear, not clear-after). > > When do you expect to post (1)? I'll base the series on it and can > > test on ConnectX-6 Dx before you post. > > After internal review and testing; I can share the patches as RFCs in > the meantime. (1) goes to net together with the resync fix above and > the fixes for (2) below: the stack part, the mlx5e queue pin, the > channel replacement overlap, and the pass-through skbs. I tested them > on ConnectX-6 Dx, and testing on your side is very welcome, especially > with your series on top. > > >> 2. Pre-existing, and a dependency of rekey: TX queue stability. I'd > [...] > > Gating ooo_okay, the dst-reset queue clear and txq_reselection_ms > > on wmem covers every path through sk_tx_queue_mapping. In mlx5e the > > SQ can still change while that mapping stays put, or bypass it: > > > > - Channel replacement with tis_tir_td_order destroys the old > > channels only after the new ones are active (3b88a535a8e1). > > mlx5e_deactivate_txqsq() posts a final NOP, but nothing waits for > > its CQE, and mlx5e_poll_tx_cq() returns early once ENABLED is > > cleared. The deactivated SQ can still hold old-key WQEs (e.g. > > under PFC pause) while the same txq index already names the new > > SQ, so (3a)'s "same SQ as all old-key WQEs" holds only once the > > old SQ is destroyed, which is the DESTROY_SQ question above. > > Right, and that overlap is a bug of 3b88a535a8e1 on its own. I'd fix it > by draining the old SQs before activating the new channels: their NAPI > is already disabled, so the switch reaps their completions directly, > and the old channels are still closed after the activation, which keeps > the downtime saving. If they don't drain in time (e.g. under a long > pause), it warns and falls back to closing them first. No kTLS-specific > count is needed. > > > - With num_tcs > 1, mlx5e_select_queue() adds a priority offset > > after netdev_pick_tx() (DSCP under MLX5_QPTS_TRUST_DSCP, else VLAN > > prio; HTB by skb->priority), so IP_TOS/IPV6_TCLASS or SO_PRIORITY > > changes move the SQ. > > mlx5e_select_queue() can keep the final queue of such sockets: cache it > in sk_tx_queue_mapping, and reuse it as long as ooo_okay is clear. A > priority change then takes effect once the connection has nothing in > flight, which for a busy bulk flow can take a while; that's the price > of keeping the order. > > > - Egress skbedit queue_mapping bypasses ndo_select_queue(), and > > SKBEDIT_F_TXQ_SKBHASH can move a flow after a txhash rethink. > > I'd document it as not supported with this kind of offload, and have > the driver count it (see below). > > > - A queue-count change: netdev_pick_tx() ignores a cached index > > >= real_num_tx_queues whatever wmem says. > > In mlx5e that comes only with a channel replacement, which the drain > covers, or with an HTB leaf deletion, which closes the leaf SQ before > its queue is removed. > > > Separately, "a drained wmem means that no work referencing the > > connection state is pending on any SQ" has a pre-existing hole: > > netem with delay, jitter or rate calls skb_orphan_partial(), which > > keeps skb->sk but swaps tcp_wfree for sock_efree, releasing the wmem > > charge while the skb is parked. > [...] > > Both kinds of skb above can be handled in the driver alone: mlx5e can > > skip the context WQEs for an skb with skb->destructor != tcp_wfree (the > > test tcp_gso_segment() already uses), which covers the pre-install > > plaintext, whose orphan decision netem_enqueue() made before the socket > > was offloaded, and the parked rekey ciphertext alike. > [...] > > Agreed that it belongs in the driver. Rather than keying on the > destructor, I wrote it so that nothing is posted for an skb that > doesn't use the context: the pending params are posted only once the > skb is known to need them (in order, or resynced, where they fold into > the full resync), and stay pending on SKIP_NO_DATA. A pass-through skb > then touches nothing, wherever it was parked. Does that cover both of > your cases? Within the sequence window, yes. Outside it, a partially orphaned pass-through skb can be held past half the sequence space and alias expected_seq or a live record. An explicit bypass right after the datalen check in mlx5e_ktls_handle_tx_skb() closes that without a sequence comparison: if (!skb_is_decrypted(skb)) return true; All kTLS HW data is sent with MSG_SENDPAGE_DECRYPTED, and decrypted skbs can't be partially orphaned, so every skb that reaches the context then holds wmem. > > So (2) as proposed is a good stability hint, but not proof. I see > > two ways to get correctness: > > > > i. Close the holes individually, without a new per-packet lock: the > [...] > > ii. A driver-side owner check where WQEs are posted: priv_tx > [...] > > I lean towards (ii) for robustness, with (2) as the hint that keeps > > it off the slow path, but I'd rather hear your view on the hot-path > > cost first. > > I'd prefer (i). With the drain, it no longer needs to close channels > first or to count connections, and the rest is small and local. (ii) > adds a lock and a SW encryption fallback to the mlx5e xmit path, for a > case that shouldn't happen once the holes are closed; a fallback that > hardly ever runs is also hard to keep correct. > > And as these moves corrupt records today, without rekey, I'd send (2) > and the mlx5e queue pin to net as fixes. On ConnectX-6 Dx with many > channels, kTLS TX records already fail decryption at the receiver with > the default txq_reselection_ms, and forcing moves breaks most > connections; with the net fixes, the same runs are clean. > > As for a future path silently breaking the invariant, the driver can > catch that cheaply: it remembers the SQ that a connection last posted > on, and when the next skb arrives on another SQ while ooo_okay is clear > and the old SQ is still live, it bumps a counter and warns once. That > is a pointer compare per skb, and it turns a silent corruption into a > visible one. I'm happy to use (i), but warning/counting alone does not cover the forced-queue case: a detected conflict has to prevent the conflicting hardware access, since old TIS accesses must have retired before another SQ can modify the context. Dropping before any TLS WQE or context-state change would preserve safety without SW crypto. TCP can recover unacknowledged data through retransmission; conflicting retransmits keep being dropped until the old accesses have retired. I'll leave the mechanism to your patches. Points they need to cover: 1. Egress skbedit never calls ndo_select_queue(), so a counter and a warning still let conflicting WQEs reach two SQs. 2. sk_tx_queue_mapping is shared by every device on the route: if the route passes through another device for a while, its netdev_pick_tx() can overwrite the cached queue, and back on mlx5e the flow moves while the old SQ still has work. ooo_okay alone isn't proof either: TCP also sets it when the rtx queue is empty, and on a bond the stamp can be stale by the time the lower device selects. bond_select_queue() ignores sk_tx_queue_mapping, and the slave's netdev_pick_tx() runs only after the bond's qdisc dequeues, so an ooo_okay skb that was overtaken (e.g. by a later skb in a higher prio band after an SO_PRIORITY change) can re-pick while that later skb is still outstanding on the old SQ. For this bond example, re-check wmem at selection: allow the move only if sk_wmem_alloc_get(sk) <= skb->truesize while the skb still holds its own charge, and only at zero wmem for an uncharged one. Across transmit paths, the migration check and queue publication also need serialization. 3. The drain has to cover the QoS/HTB SQs too, and reap deactivated SQs although mlx5e_poll_tx_cq() returns early once ENABLED is clear; a missing or replaced txq2sq entry isn't evidence of retirement. On a drain timeout, the fallback must close the old SQs first (which keeps the guarantee, modulo DESTROY_SQ), never activate with them alive. On the price you mention for priority changes, agreed. Please document in Documentation/networking/tls-offload.rst that pinning delays priority/DSCP/HTB-driven TX queue changes until migration is safe, potentially indefinitely under sustained traffic. > > (3a) fits, and I prefer it. sendmsg keeps sending SW ciphertext during > > rekey, which holds wmem, so (3b) can wait indefinitely unless the > > sender stalls. With this series, mlx5e is the only in-tree driver > > accepting TLS 1.3 crypto_info, so the core can require the new op for > > TX rekey and leave other devices in SW. > > > > Proposed contract: > > > > int (*tls_dev_rekey)(struct net_device *netdev, struct sock *sk, > > enum tls_offload_ctx_dir direction, > > struct tls_crypto_info *crypto_info, > > u32 start_offload_tcp_sn); > > Looks good to me. > > > - The old DEK is freed from process context once a successful > > CQE covers the fenced replacement, subject to your answer to the > > second question above. > > Agreed, and it's simpler than I first thought: the driver doesn't need > to track completions per connection. When it posts the fenced > replacement, it can hand the old DEK to that WQE's descriptor, the way > DUMP WQEs hold their page refs today, and release it once the SQ > retires that WQE. Completions being in order, that covers every old-key > WQE posted before it on the SQ. A flush or an SQ close retires it too; > the TIS may then still name the old DEK until the resync installs the > new one, which is your second question again, and the same as an idle > TIS in the pool today. The release runs from NAPI, so the destroy > itself would be deferred to a work item. Nice, that's simpler. On the core side, the op's contract: the callback prepares the new key and pending parameters but posts no TLS WQEs for this connection. After it returns success, the core commits the new fallback AEAD and start marker, then restores the HW validator, as v18's del+add path already does. Replacement parameters are posted when the next skb needing the TLS context reaches the driver. On failure, the callback releases its new allocations and preserves the existing context for outstanding work and eventual tls_dev_del(); the core commits nothing and TX stays in software. Callbacks are serialized per connection, but another may arrive before the previous replacement is posted. On success, the driver must clean up any superseded, never-posted key. On the driver side, the DEK lifetime requirement: the old DEK stays alive until a completion proves old-key work has retired; a successful CQE covering the fenced replacement does that. Whether a flush, an SQ close, or deletion back to the TIS pool may release a DEK the TIS still names depends on your hardware answers; if not, retention has to extend through the pool, which is pre-existing and not rekey specific. > > - Retirement of the SQ only proves the old work stopped, not that > > the replacement took effect. If the SQ errors or closes before > > a successful CQE covers the replacement, the driver forces a > > resync on the next offloaded skb regardless of expected_seq: > [...] > > That's the resync fix above, which isn't specific to rekey. > > One question on the core side: once the HW validator is restored, can > a decrypted old-key skb still be queued, e.g. a spurious retransmit > parked in a qdisc, and reach the driver? It would end before the new > marker, so mlx5e would take SKIP_NO_DATA with a decrypted skb, which > today hits WARN_ON_ONCE() and drops it. It can be queued, but within the sequence window the core drops it first, in two v18 patches that aren't upstream yet. "tls: device: add TX KeyUpdate support" sets TLS_TX_REKEY_FLOOR before restoring tls_validate_xmit_skb(), which then drops any payload skb whose end_seq is not after snd_una; completion runs only once every old-key record is ACKed. "net: sched: re-validate decrypted skbs on requeue" makes an skb parked on the requeue list go through the current validator again. Outside that window, the floor does not reliably identify old-key clones. Once snd_una advances at least 2^31 bytes past a parked clone's end_seq, the comparison can reverse; that progress can begin before rekey completion. Later traffic must overtake the parked clone, for example if a classful qdisc lets a newly selected priority class run ahead. Record lookup also uses 32-bit TCP sequence numbers today without rekey, so a sufficiently delayed clone can alias a live record there too. The floor catches old-key clones within its comparison window, but does not remove that ambiguity. I'll look at closing this in the core separately. > > If this direction works for you, I'll write mlx5e_ktls_rekey_tx() > > and whichever (2) option you prefer on top of your fix, and test > > them on the same hardware. > > Sounds good. (2) is covered by the net fixes, so please go ahead with > the op and mlx5e_ktls_rekey_tx() on top of them, and I'll review. The > RFCs mentioned above should let you start before the fixes land. Please Cc me on the RFCs, and let me know which fixes the rekey patches should depend on. I'll implement the core op, its contract, the v18 integration and mlx5e_ktls_rekey_tx() on top of them, and post the patches together as an RFC for your review. I'll keep the flush/close DEK release and pool-lifetime questions explicit until your HW/FW answers settle them. Devices without the callback keep TX in software after a KeyUpdate. > Thanks, > Tariq Thanks, Rishikesh ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Question: net/mlx5e kTLS TX TIS reuse vs in-flight WQEs 2026-11-06 19:58 ` Rishikesh Jethwani @ 2026-11-08 19:45 ` Tariq Toukan 2026-11-09 9:21 ` Tariq Toukan 2026-11-09 23:21 ` Rishikesh Jethwani 0 siblings, 2 replies; 9+ messages in thread From: Tariq Toukan @ 2026-11-08 19:45 UTC (permalink / raw) To: Rishikesh Jethwani Cc: Saeed Mahameed, Mark Bloch, Gal Pressman, Jakub Kicinski, Boris Pismenny, Sabrina Dubroca, John Fastabend, netdev On 06/11/2026 21:58, Rishikesh Jethwani wrote: > Thanks. Agreed on (3a), (i) and the net fixes, with one condition on > (i) below. Hi Rishikesh, Thanks. I took your points into the patches, details inline. I'm re-running the ConnectX-6 Dx tests with the changes, and will share the RFCs once they pass. > Thanks. Until then, the two hardware questions remain open, including > whether DESTROY_SQ can fail; the destroy wrapper discards the status. DESTROY_SQ doesn't fail while the function is healthy. If it does, something went badly wrong in the firmware. > One correction to my last mail: recovery clears RECOVERING even on a > flush timeout, so the queue can wake once stop_room frees. Thanks. WQEs posted to that SQ then complete with error CQEs, which mark their connections for a full resync, as in the first error. > Agreed. What rekey needs from that resync fix: a lost initialization > or replacement always triggers a full resync on the next skb that > needs the TLS context, and a connection marked again while the driver > is consuming an earlier mark stays marked (test-and-clear, not > clear-after). Both hold now. The mark stays until an skb that uses the context consumes it, and it is taken with a test-and-clear before the resync is posted, so a loss reported meanwhile marks the connection again. > An explicit bypass right after the > datalen check in mlx5e_ktls_handle_tx_skb() closes that without a > sequence comparison: > > if (!skb_is_decrypted(skb)) > return true; Agreed, it's in the pass-through patch, with credit. > I'm happy to use (i), but warning/counting alone does not cover the > forced-queue case: a detected conflict has to prevent the conflicting > hardware access, since old TIS accesses must have retired before > another SQ can modify the context. Dropping before any TLS WQE or > context-state change would preserve safety without SW crypto. Agreed, the counter is now a drop. Each connection records the TX queue that it posts on. An skb on another TX queue moves the connection only if no other skb of its socket is in flight: an skb is freed only once the WQEs posted for it completed, or were destroyed with their SQ, so nothing of the connection is then pending on any SQ. Otherwise the skb is dropped before anything is posted, and counted, and TCP retransmits it. The move is made under the old TX queue's lock, taken with a trylock, under which the connection posts there. That serializes the check and the publication with that path, and leaves the connection's datapath with a compare of the queue index. This covers your points 1 and 2 where the WQEs are posted, wherever the queue came from. For 2, the mlx5e pin now also re-checks wmem at selection as you suggest, so a stale ooo_okay doesn't lead to drops in the first place. Does that match what you had in mind? > 3. The drain has to cover the QoS/HTB SQs too, and reap deactivated > SQs although mlx5e_poll_tx_cq() returns early once ENABLED is > clear; a missing or replaced txq2sq entry isn't evidence of > retirement. On a drain timeout, the fallback must close the old > SQs first (which keeps the guarantee, modulo DESTROY_SQ), never > activate with them alive. That's how the drain works: it covers the QoS SQs, reaps the deactivated SQs directly, and on a timeout closes the old channels before activating the new ones. The drop above doesn't rely on txq2sq either: an SQ that is deactivated alone, e.g. on an HTB leaf delete, keeps its skbs until it is destroyed, so their socket keeps its wmem until then. > On the price you mention for priority changes, agreed. Please > document in Documentation/networking/tls-offload.rst that pinning > delays priority/DSCP/HTB-driven TX queue changes until migration is > safe, potentially indefinitely under sustained traffic. Done. > Nice, that's simpler. On the core side, the op's contract: the > callback prepares the new key and pending parameters but posts no TLS > WQEs for this connection. [...] > Callbacks are serialized per connection, but another may arrive > before the previous replacement is posted. On success, the driver > must clean up any superseded, never-posted key. Agreed on the contract. > Record lookup also uses 32-bit TCP sequence numbers > today without rekey, so a sufficiently delayed clone can alias a live > record there too. The floor catches old-key clones within its > comparison window, but does not remove that ambiguity. I'll look at > closing this in the core separately. Thanks. On the driver side, the bypass above keeps plaintext skbs out of the record lookup, so it's down to the decrypted clones. > Please Cc me on the RFCs, and let me know which fixes the rekey > patches should depend on. Will do. The rekey patches should go on top of these net fixes: net/mlx5e: kTLS, Hold a dropped skb until its posted WQEs complete net/mlx5e: kTLS, Resync TX fully after its WQEs were lost net/mlx5e: kTLS, Post TX context params only for skbs that use them net/mlx5e: Drain old TX SQs before activating new channels net: tls: keep the TX queue of a device-offloaded socket stable net/mlx5e: kTLS, Keep the TX queue of an offloaded socket stable The drop goes to net-next separately, as "net/mlx5e: kTLS, Keep a TX connection on one SQ at a time"; the rekey patches don't depend on it. Thanks, Tariq ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Question: net/mlx5e kTLS TX TIS reuse vs in-flight WQEs 2026-11-08 19:45 ` Tariq Toukan @ 2026-11-09 9:21 ` Tariq Toukan 2026-11-09 23:21 ` Rishikesh Jethwani 1 sibling, 0 replies; 9+ messages in thread From: Tariq Toukan @ 2026-11-09 9:21 UTC (permalink / raw) To: Rishikesh Jethwani Cc: Saeed Mahameed, Mark Bloch, Gal Pressman, Jakub Kicinski, Boris Pismenny, Sabrina Dubroca, John Fastabend, netdev On 08/11/2026 21:45, Tariq Toukan wrote: > I'm re-running the ConnectX-6 Dx tests with the changes, and will > share the RFCs once they pass. They pass, with one change to the drop below. > An skb on another TX queue moves the connection only if no other skb > of its socket is in flight This also dropped on legitimate stack moves: the next skbs, queued behind the first one on the new TX queue, hold sk_wmem_alloc when it reaches the driver. So a move that the TX queue selection allows, only while nothing else of the socket is in flight, now hands over the new TX queue right there, and the check at xmit is left for skbs that bypass the selection, like tc skbedit. No drops without forced moves now. I'll share my patches shortly. Thanks, Tariq ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Question: net/mlx5e kTLS TX TIS reuse vs in-flight WQEs 2026-11-08 19:45 ` Tariq Toukan 2026-11-09 9:21 ` Tariq Toukan @ 2026-11-09 23:21 ` Rishikesh Jethwani 2026-11-10 14:45 ` Tariq Toukan 1 sibling, 1 reply; 9+ messages in thread From: Rishikesh Jethwani @ 2026-11-09 23:21 UTC (permalink / raw) To: Tariq Toukan Cc: Saeed Mahameed, Mark Bloch, Gal Pressman, Jakub Kicinski, Boris Pismenny, Sabrina Dubroca, John Fastabend, netdev On Sun, Nov 8, 2026 at 11:45 AM Tariq Toukan <tariqt@nvidia.com> wrote: > > > On 06/11/2026 21:58, Rishikesh Jethwani wrote: > > Thanks. Agreed on (3a), (i) and the net fixes, with one condition on > > (i) below. > > Hi Rishikesh, Hi Tariq, Thanks. I've now reviewed the seven-patch RFC [1]. The held-skb, full-resync and pass-through changes cover the points I raised. I've replied on patches 7 and 4 with two remaining holes in the one-SQ guarantee: the selection-time ownership transfer, and the PTP SQs in the drain. > Thanks. I took your points into the patches, details inline. I'm > re-running the ConnectX-6 Dx tests with the changes, and will share the > RFCs once they pass. > > > Thanks. Until then, the two hardware questions remain open, including > > whether DESTROY_SQ can fail; the destroy wrapper discards the status. > > DESTROY_SQ doesn't fail while the function is healthy. If it does, > something went badly wrong in the firmware. Thanks for clarifying the healthy-function case. I'll continue to use CQEs or successful SQ destruction as the retirement evidence. The DEK question is still open: once all old-key WQEs have retired, may the old DEK be destroyed or returned for reuse while an idle TIS still names it? This needs to cover both standalone DEKs and pooled IDs reused by another connection. A flushed replacement need not have installed the new key, and deletion can return the still-bound TIS to its pool. The resync fix restores the context on its next use, but does not by itself establish that releasing the old DEK before then is safe. Please keep that HW/FW answer explicit. Until it is settled, I cannot treat a flush/close alone as sufficient to release a still-named DEK; the conservative alternative is to retain the possible bindings until a successful replacement or TIS destruction, including through pooling. > > One correction to my last mail: recovery clears RECOVERING even on a > > flush timeout, so the queue can wake once stop_room frees. > > Thanks. WQEs posted to that SQ then complete with error CQEs, which mark > their connections for a full resync, as in the first error. > > > Agreed. What rekey needs from that resync fix: a lost initialization > > or replacement always triggers a full resync on the next skb that > > needs the TLS context, and a connection marked again while the driver > > is consuming an earlier mark stays marked (test-and-clear, not > > clear-after). > > Both hold now. The mark stays until an skb that uses the context > consumes it, and it is taken with a test-and-clear before the resync is > posted, so a loss reported meanwhile marks the connection again. > > > An explicit bypass right after the > > datalen check in mlx5e_ktls_handle_tx_skb() closes that without a > > sequence comparison: > > > > if (!skb_is_decrypted(skb)) > > return true; > > Agreed, it's in the pass-through patch, with credit. > > > I'm happy to use (i), but warning/counting alone does not cover the > > forced-queue case: a detected conflict has to prevent the conflicting > > hardware access, since old TIS accesses must have retired before > > another SQ can modify the context. Dropping before any TLS WQE or > > context-state change would preserve safety without SW crypto. > > Agreed, the counter is now a drop. Each connection records the TX queue > that it posts on. An skb on another TX queue moves the connection only > if no other skb of its socket is in flight: an skb is freed only once > the WQEs posted for it completed, or were destroyed with their SQ, so > nothing of the connection is then pending on any SQ. Otherwise the skb > is dropped before anything is posted, and counted, and TCP retransmits > it. > > The move is made under the old TX queue's lock, taken with a trylock, > under which the connection posts there. That serializes the check and > the publication with that path, and leaves the connection's datapath > with a compare of the queue index. > > This covers your points 1 and 2 where the WQEs are posted, wherever the > queue came from. For 2, the mlx5e pin now also re-checks wmem at > selection as you suggest, so a stale ooo_okay doesn't lead to drops in > the first place. Does that match what you had in mind? > > > 3. The drain has to cover the QoS/HTB SQs too, and reap deactivated > > SQs although mlx5e_poll_tx_cq() returns early once ENABLED is > > clear; a missing or replaced txq2sq entry isn't evidence of > > retirement. On a drain timeout, the fallback must close the old > > SQs first (which keeps the guarantee, modulo DESTROY_SQ), never > > activate with them alive. > > That's how the drain works: it covers the QoS SQs, reaps the deactivated > SQs directly, and on a timeout closes the old channels before activating > the new ones. The drop above doesn't rely on txq2sq either: an SQ that > is deactivated alone, e.g. on an HTB leaf delete, keeps its skbs until > it is destroyed, so their socket keeps its wmem until then. > > > On the price you mention for priority changes, agreed. Please > > document in Documentation/networking/tls-offload.rst that pinning > > delays priority/DSCP/HTB-driven TX queue changes until migration is > > safe, potentially indefinitely under sustained traffic. > > Done. > > > Nice, that's simpler. On the core side, the op's contract: the > > callback prepares the new key and pending parameters but posts no TLS > > WQEs for this connection. > > [...] > > > Callbacks are serialized per connection, but another may arrive > > before the previous replacement is posted. On success, the driver > > must clean up any superseded, never-posted key. > > Agreed on the contract. Agreed. I'll replace v18's live TX del+add with the new callback and implement mlx5e_ktls_rekey_tx() on the revised fixes. That includes cleanup of superseded, never-posted keys, and explicit ownership of retiring DEKs through WQE completion and deferred destruction. Devices without the callback will stay in software after a KeyUpdate. The delayed decrypted-clone case remains a separate core follow-up. The pass-through bypass closes the unmarked-skb case, but the existing 32-bit record lookup can still alias a later record, even without rekey. Rekey adds the cross-key case; the floor only covers delays within its comparison window. The fix I'm considering needs TCP support and a per-skb check in the driver's in-order TX path too; I'll Cc you when I post it. > > Record lookup also uses 32-bit TCP sequence numbers > > today without rekey, so a sufficiently delayed clone can alias a live > > record there too. The floor catches old-key clones within its > > comparison window, but does not remove that ambiguity. I'll look at > > closing this in the core separately. > > Thanks. On the driver side, the bypass above keeps plaintext skbs out of > the record lookup, so it's down to the decrypted clones. > > > Please Cc me on the RFCs, and let me know which fixes the rekey > > patches should depend on. > > Will do. The rekey patches should go on top of these net fixes: > > net/mlx5e: kTLS, Hold a dropped skb until its posted WQEs complete > net/mlx5e: kTLS, Resync TX fully after its WQEs were lost > net/mlx5e: kTLS, Post TX context params only for skbs that use them > net/mlx5e: Drain old TX SQs before activating new channels > net: tls: keep the TX queue of a device-offloaded socket stable > net/mlx5e: kTLS, Keep the TX queue of an offloaded socket stable > > The drop goes to net-next separately, as "net/mlx5e: kTLS, Keep a TX > connection on one SQ at a time"; the rekey patches don't depend on it. I do need patch 7, with the selection-time fix from my reply to it, as a rekey dependency. Patches 1-6 alone do not cover egress skbedit or a cached queue overwritten by another device on the route. A fenced replacement on SQ B cannot order itself against old-key WQEs still on SQ A. Keeping patch 7 in net-next is fine; I'll base rekey on it as well as the net fixes. The RFC cover letter's "builds on this series" matches that dependency. > Thanks, > Tariq Thanks, Rishikesh [1] https://lore.kernel.org/netdev/20261109114734.2087878-1-tariqt@nvidia.com/ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Question: net/mlx5e kTLS TX TIS reuse vs in-flight WQEs 2026-11-09 23:21 ` Rishikesh Jethwani @ 2026-11-10 14:45 ` Tariq Toukan 0 siblings, 0 replies; 9+ messages in thread From: Tariq Toukan @ 2026-11-10 14:45 UTC (permalink / raw) To: Rishikesh Jethwani, Tariq Toukan Cc: Saeed Mahameed, Mark Bloch, Gal Pressman, Jakub Kicinski, Boris Pismenny, Sabrina Dubroca, John Fastabend, netdev On 10/11/2026 1:21, Rishikesh Jethwani wrote: > Thanks. I've now reviewed the seven-patch RFC [1]. The held-skb, > full-resync and pass-through changes cover the points I raised. I've > replied on patches 7 and 4 with two remaining holes in the one-SQ > guarantee: the selection-time ownership transfer, and the PTP SQs in > the drain. Thanks for the review. Agreed on both, replied there. > The DEK question is still open: once all old-key WQEs have retired, may > the old DEK be destroyed or returned for reuse while an idle TIS still > names it? This needs to cover both standalone DEKs and pooled IDs reused > by another connection. A flushed replacement need not have installed > the new key, and deletion can return the still-bound TIS to its pool. > The resync fix restores the context on its next use, but does not by > itself establish that releasing the old DEK before then is safe. > > Please keep that HW/FW answer explicit. Until it is settled, I cannot > treat a flush/close alone as sufficient to release a still-named DEK; > the conservative alternative is to retain the possible bindings until > a successful replacement or TIS destruction, including through pooling. Double checking. Will update. > Agreed. I'll replace v18's live TX del+add with the new callback and > implement mlx5e_ktls_rekey_tx() on the revised fixes. Sounds good. > The delayed decrypted-clone case remains a separate core follow-up. [...] > I'll Cc you when I post it. Thanks, please do. > I do need patch 7, with the selection-time fix from my reply to it, as > a rekey dependency. Patches 1-6 alone do not cover egress skbedit or a > cached queue overwritten by another device on the route. A fenced > replacement on SQ B cannot order itself against old-key WQEs still on > SQ A. Keeping patch 7 in net-next is fine; I'll base rekey on it as > well as the net fixes. Right, I missed those cases. Rekey depends on patch 7 too, and v2 of the RFC will carry the selection-time fix. Thanks, Tariq ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-11-10 14:45 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-22 18:17 Question: net/mlx5e kTLS TX TIS reuse vs in-flight WQEs Rishikesh Jethwani 2026-11-03 14:45 ` Tariq Toukan 2026-11-04 5:01 ` Rishikesh Jethwani 2026-11-05 9:25 ` Tariq Toukan 2026-11-06 19:58 ` Rishikesh Jethwani 2026-11-08 19:45 ` Tariq Toukan 2026-11-09 9:21 ` Tariq Toukan 2026-11-09 23:21 ` Rishikesh Jethwani 2026-11-10 14:45 ` Tariq Toukan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox