On 10/7/2026 11:54 PM, Tian Xun Ng wrote:
On 10/7/2026 10:07 AM, Tantilov, Emil S wrote:
I did a rough draft, keeping your logic and I think we
should be able to avoid the new flags by introducing a global variable
adapter->vc_xn_timeout_ms and set it based on the flow we're in
Thanks, that works well. v3 is ready along those lines: no new flags,
adapter->vc_xn_timeout_ms set to the default at probe and to 500 ms in
idpf_shutdown() right before IDPF_REMOVE_IN_PROG, and idpf_send_mb_msg()
caps each transaction with it and sets it to 0 after the first
shortened transaction times out, so the rest fail at once.
Backported to our 6.17 kernel (two arm64 servers, two PFs each, IOMMU
translating): no faults in 30 warm reboots. The slowest teardown
message is still DEALLOC_VECTORS at ~300 ms, sent after DESTROY_VPORT;
the queue-related ones took at most 158 ms. With the driver made to
skip sending the teardown messages, the first one timed out at ~500 ms,
the rest failed immediately, and every reboot faulted again.
Before I post it, two things it exposes that I would like your view on:
1. Setting IDPF_REMOVE_IN_PROG on shutdown also moves
libie_ctlq_xn_shutdown() after idpf_ptp_release() and
idpf_deinit_task(), as on remove. mbx_task keeps running in that
window, so an async PTP Tx timestamp reply can reach
idpf_ptp_get_tx_tstamp_async_handler() after
idpf_ptp_release_vport_tstamp() has freed tx_tstamp_caps. Remove has
the same window today; shutdown would now have it too. An unmatched
If I understand correctly, this is referring to possible UAF in the ptp
release logic ... I can't say for sure, since I don't have your patch,
but I think it is a valid concern. Your previous patch would have the
same effect, so I don't believe this to be new (as result of the
refactor) and would be an existing bug in idpf_remove().
BTW, I should mention here, you need to set IDPF_REMOVE_IN_PROG before
cancelling the tasks in idpf_shutdown() to make sure the event task is
not re-queued.
VIRTCHNL2_OP_EVENT (link change) has a similar window against the
vport being freed, which exists on shutdown already.
One way to close it without new flags: once IDPF_REMOVE_IN_PROG is
set, cancel_delayed_work_sync() and requeue mbx_task, and have the
PTP callback and idpf_recv_event_msg() return early when the bit is
set; matched replies are still consumed, so the teardown is not
affected. I can send that as a prerequisite patch, but I cannot
exercise the PTP part here (PTP init returns -EOPNOTSUPP on our
parts). Would you prefer that, or to handle it separately?
2. A transaction that is already waiting when shutdown starts keeps its
own timeout. For example a GET_STATS from stats_task, which is only
cancelled in idpf_deinit_task(), can still hold shutdown for up to
60 s with a dead control plane, where the early xn shutdown used to
wake it. Fixing that would need a way in libie to expire waiting
transactions, so I would note it in the commit message rather than
claim a hard bound. Does that sound right?
Yea, I think solving this issue in general would probably require
changes in controlq. For the purpose of this patch limiting the delay
within reason for the more practical cases is good enough.
Another thing to note, in your previous patch you had a comment:
+ /* Destroy the vports while the mailbox is still up, so that the
+ * device stops its queues before the next kernel reuses their memory.
+ * A reset does not clear them on every device.
+ */
The last sentence is incorrect - a reset must clear the vport(s) state.
This patch is still warranted as we would like the driver to gracefully
signal the destruction of vports, however documenting FW bug as a fact
is not OK.
Thanks,
Emil
Thanks,
Tian Xun
