Re: [PATCH iwl-net v2] idpf: keep the mailbox up while tearing down vports on shutdown

["Tantilov, Emil S" <emil.s.tantilov@intel.com>]

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