Netdev List
 help / color / mirror / Atom feed
From: Tariq Toukan <tariqt@nvidia.com>
To: Rishikesh Jethwani <rjethwani@purestorage.com>, tariqt@nvidia.com
Cc: Saeed Mahameed <saeedm@nvidia.com>,
	Mark Bloch <mbloch@nvidia.com>, Gal Pressman <gal@nvidia.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Boris Pismenny <borisp@nvidia.com>,
	Sabrina Dubroca <sd@queasysnail.net>,
	John Fastabend <john.fastabend@gmail.com>,
	netdev@vger.kernel.org
Subject: Re: Question: net/mlx5e kTLS TX TIS reuse vs in-flight WQEs
Date: Tue, 3 Nov 2026 16:45:22 +0200	[thread overview]
Message-ID: <fc0da060-5bf5-4289-8814-d25e6c422673@nvidia.com> (raw)
In-Reply-To: <20261022181751.1349984-1-rjethwani@purestorage.com>



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/


  reply	other threads:[~2026-11-03 14:45 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=fc0da060-5bf5-4289-8814-d25e6c422673@nvidia.com \
    --to=tariqt@nvidia.com \
    --cc=borisp@nvidia.com \
    --cc=gal@nvidia.com \
    --cc=john.fastabend@gmail.com \
    --cc=kuba@kernel.org \
    --cc=mbloch@nvidia.com \
    --cc=netdev@vger.kernel.org \
    --cc=rjethwani@purestorage.com \
    --cc=saeedm@nvidia.com \
    --cc=sd@queasysnail.net \
    /path/to/YOUR_REPLY

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

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