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

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

[Tian Xun Ng]

From: Tian Xun Ng <tianxun.ng@bytedance.com>

Since commit 4c9106f4906a ("idpf: fix adapter NULL pointer dereference
on reboot"), idpf_shutdown() calls idpf_vc_core_deinit() directly instead
of idpf_remove(), so IDPF_REMOVE_IN_PROG is not set on shutdown.
idpf_vc_core_deinit() uses that flag to decide when to shut the virtchnl
transaction manager down. Without it, libie_ctlq_xn_shutdown() runs
before idpf_deinit_task() tears the vports down, so every message sent
during that teardown (disable vport, disable queues, destroy vport)
fails at once. The device is never told to stop its queues and keeps
them enabled, with the ring addresses of the kernel that is going away.

After a warm reboot, the first queue reconfiguration of a port that has
not been opened yet (udev setting the MTU) sends VIRTCHNL2_OP_DEL_QUEUES.
The device then drains the queues it still considers live and writes
SW_MARKER TX completions to the previous kernel's completion rings. With
the IOMMU translating, those writes fault about 19 s into every such
boot:

  arm-smmu-v3 arm-smmu-v3.12.auto: event: F_TRANSLATION client: 0006:01:00.0 sid: 0x30100 ssid: 0x0 iova: 0x3ef60840 ipa: 0x0

In IOMMU pass-through mode they land on pages the new kernel has already
reused, which shows up as "pagealloc: memory corruption" with
page_poison=1 and as crashes in unrelated code without it.

A function reset on shutdown does not help on these devices: with a PF
reset issued in idpf_shutdown() and PFGEN_RSTAT polled until the reset
completed, every warm reboot still faulted, and the reset the next
kernel issues at probe does not clear the queues either. Delivering the
teardown messages does.

Set IDPF_REMOVE_IN_PROG in idpf_shutdown(), as idpf_remove() does, so
that the vports are destroyed while the mailbox is still up. So that an
unresponsive control plane cannot hold up a reboot, give each mailbox
transaction during shutdown a 2 s timeout instead of 60 s, and fail the
remaining ones at once after the first timeout. If the function has
already been reset, for instance a VF whose PF went down first,
idpf_is_reset_detected() fails them without waiting.

Tested on two arm64 (64K pages) servers, each with two idpf PFs, with
this change backported to a 6.17 kernel and the IOMMU translating: no
faults in 25 warm reboots, against a fault on every warm reboot
without it. Each PF's teardown took about 1.2 s for six mailbox
transactions; the slowest, VIRTCHNL2_OP_DEALLOC_VECTORS, took about
300 ms. To stand in for a control plane that does not reply, the driver
was also built to skip sending mailbox messages during shutdown: the
first transaction timed out after 2 s, the rest failed at once, the
teardown took 2.8 s per PF, and every reboot faulted again.

Fixes: 4c9106f4906a ("idpf: fix adapter NULL pointer dereference on reboot")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Tian Xun Ng <tianxun.ng@bytedance.com>
---
v2:
- Drop patch 2 (function reset on shutdown). On a VF it queued
  VIRTCHNL2_OP_RESET_VF and then freed the mailbox, and on these devices
  a PF reset does not clear the stale queues anyway (numbers in the v1
  thread).
- Set IDPF_REMOVE_IN_PROG in idpf_shutdown() instead of changing the
  condition in idpf_vc_core_deinit(); no new idpf_is_reset_detected()
  call (Emil).
- Bound the shutdown teardown: 2 s per transaction instead of 60 s, and
  fail the remaining ones at once after the first timeout (Emil). 2 s is
  about six times the slowest teardown transaction measured here.
- No VF special case, unlike what I suggested in the thread: a VF whose
  PF has been reset already fails fast through the existing
  idpf_is_reset_detected() check in idpf_send_mb_msg(), and a VF with a
  working mailbox leaves queues behind on a warm reboot just like a PF.
  Not tested on a VF; I have no VF setup.
- Compile-tested on net-queue dev-queue (W=1, allmodconfig and
  allyesconfig, no new warnings); runtime-tested as the 6.17 backport
  described above.

v1: https://lore.kernel.org/all/20260917105205.37561-1-luckilystar08@gmail.com/

 drivers/net/ethernet/intel/idpf/idpf.h        |  4 +++
 drivers/net/ethernet/intel/idpf/idpf_main.c   |  7 +++++
 .../net/ethernet/intel/idpf/idpf_virtchnl.c   | 28 ++++++++++++++++---
 .../net/ethernet/intel/idpf/idpf_virtchnl.h   |  1 +
 4 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/idpf/idpf.h b/drivers/net/ethernet/intel/idpf/idpf.h
index 470bc23c8..5d2e8a346 100644
--- a/drivers/net/ethernet/intel/idpf/idpf.h
+++ b/drivers/net/ethernet/intel/idpf/idpf.h
@@ -88,6 +88,8 @@ enum idpf_state {
  * @IDPF_REMOVE_IN_PROG: Driver remove in progress
  * @IDPF_MB_INTR_MODE: Mailbox in interrupt mode
  * @IDPF_VC_CORE_INIT: virtchnl core has been init
+ * @IDPF_SHUTDOWN_IN_PROG: Driver shutdown in progress
+ * @IDPF_SHUTDOWN_XN_TIMEOUT: A mailbox transaction timed out during shutdown
  * @IDPF_FLAGS_NBITS: Must be last
  */
 enum idpf_flags {
@@ -97,6 +99,8 @@ enum idpf_flags {
 	IDPF_REMOVE_IN_PROG,
 	IDPF_MB_INTR_MODE,
 	IDPF_VC_CORE_INIT,
+	IDPF_SHUTDOWN_IN_PROG,
+	IDPF_SHUTDOWN_XN_TIMEOUT,
 	IDPF_FLAGS_NBITS,
 };
 
diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c b/drivers/net/ethernet/intel/idpf/idpf_main.c
index 129bccaa6..fa27ee1cd 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_main.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_main.c
@@ -196,6 +196,13 @@ static void idpf_shutdown(struct pci_dev *pdev)
 
 	cancel_delayed_work_sync(&adapter->serv_task);
 	cancel_delayed_work_sync(&adapter->vc_event_task);
+
+	/* 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.
+	 */
+	set_bit(IDPF_SHUTDOWN_IN_PROG, adapter->flags);
+	set_bit(IDPF_REMOVE_IN_PROG, adapter->flags);
 	idpf_vc_core_deinit(adapter);
 	idpf_deinit_dflt_mbx(adapter);
 
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
index 1caf52706..5f5d72671 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
@@ -190,22 +190,38 @@ static void idpf_prepare_ptp_mb_msg(struct idpf_adapter *adapter, u32 op,
  * Cleanup the mailbox queue entries of the previously sent message to
  * unmap and release the buffer.
  *
+ * During shutdown each transaction gets a short timeout, and once one of
+ * them times out the rest fail at once, so that an unresponsive control
+ * plane cannot hold up a reboot.
+ *
  * Return: 0 if the request was successful, -%EBUSY if reset is detected
- *	   or Tx control queue is full, other negative error code on failure.
+ *	   or Tx control queue is full, -%ETIMEDOUT if a transaction already
+ *	   timed out during shutdown, other negative error code on failure.
  */
 int idpf_send_mb_msg(struct idpf_adapter *adapter,
 		     struct libie_ctlq_xn_send_params *xn_params,
 		     void *send_buf, size_t send_buf_size)
 {
+	bool shutdown = test_bit(IDPF_SHUTDOWN_IN_PROG, adapter->flags);
 	struct libie_ctlq_msg ctlq_msg = {};
+	int err = 0;
 
-	if (idpf_is_reset_detected(adapter)) {
+	if (idpf_is_reset_detected(adapter))
+		err = -EBUSY;
+	else if (shutdown && test_bit(IDPF_SHUTDOWN_XN_TIMEOUT, adapter->flags))
+		err = -ETIMEDOUT;
+
+	if (err) {
 		if (!libie_cp_can_send_onstack(send_buf_size))
 			kfree(send_buf);
 
-		return -EBUSY;
+		return err;
 	}
 
+	if (shutdown)
+		xn_params->timeout_ms = min_t(u64, xn_params->timeout_ms,
+					      IDPF_VC_XN_SHUTDOWN_TIMEOUT_MSEC);
+
 	idpf_prepare_ptp_mb_msg(adapter, xn_params->chnl_opcode, &ctlq_msg);
 	xn_params->ctlq_msg = ctlq_msg.opcode ? &ctlq_msg : NULL;
 
@@ -217,7 +233,11 @@ int idpf_send_mb_msg(struct idpf_adapter *adapter,
 
 	idpf_mb_clean(xn_params->ctlq, false);
 
-	return libie_ctlq_xn_send(xn_params);
+	err = libie_ctlq_xn_send(xn_params);
+	if (err == -ETIMEDOUT && shutdown)
+		set_bit(IDPF_SHUTDOWN_XN_TIMEOUT, adapter->flags);
+
+	return err;
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
index 5d27805ff..b0809d149 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
@@ -7,6 +7,7 @@
 #include <linux/net/intel/virtchnl2.h>
 
 #define IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC	(60 * 1000)
+#define IDPF_VC_XN_SHUTDOWN_TIMEOUT_MSEC	2000
 
 struct idpf_adapter;
 struct idpf_netdev_priv;
-- 
2.50.1 (Apple Git-155)

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

[Tantilov, Emil S]

On 10/6/2026 10:57 PM, Tian Xun Ng wrote:
> From: Tian Xun Ng <tianxun.ng@bytedance.com>
> 
> Since commit 4c9106f4906a ("idpf: fix adapter NULL pointer dereference
> on reboot"), idpf_shutdown() calls idpf_vc_core_deinit() directly instead
> of idpf_remove(), so IDPF_REMOVE_IN_PROG is not set on shutdown.
> idpf_vc_core_deinit() uses that flag to decide when to shut the virtchnl
> transaction manager down. Without it, libie_ctlq_xn_shutdown() runs
> before idpf_deinit_task() tears the vports down, so every message sent
> during that teardown (disable vport, disable queues, destroy vport)
> fails at once. The device is never told to stop its queues and keeps
> them enabled, with the ring addresses of the kernel that is going away.
> 
> After a warm reboot, the first queue reconfiguration of a port that has
> not been opened yet (udev setting the MTU) sends VIRTCHNL2_OP_DEL_QUEUES.
> The device then drains the queues it still considers live and writes
> SW_MARKER TX completions to the previous kernel's completion rings. With
> the IOMMU translating, those writes fault about 19 s into every such
> boot:
> 
>    arm-smmu-v3 arm-smmu-v3.12.auto: event: F_TRANSLATION client: 0006:01:00.0 sid: 0x30100 ssid: 0x0 iova: 0x3ef60840 ipa: 0x0
> 
> In IOMMU pass-through mode they land on pages the new kernel has already
> reused, which shows up as "pagealloc: memory corruption" with
> page_poison=1 and as crashes in unrelated code without it.
> 
> A function reset on shutdown does not help on these devices: with a PF
> reset issued in idpf_shutdown() and PFGEN_RSTAT polled until the reset
> completed, every warm reboot still faulted, and the reset the next
> kernel issues at probe does not clear the queues either. Delivering the
> teardown messages does.
> 
> Set IDPF_REMOVE_IN_PROG in idpf_shutdown(), as idpf_remove() does, so
> that the vports are destroyed while the mailbox is still up. So that an
> unresponsive control plane cannot hold up a reboot, give each mailbox
> transaction during shutdown a 2 s timeout instead of 60 s, and fail the
> remaining ones at once after the first timeout. If the function has
> already been reset, for instance a VF whose PF went down first,
> idpf_is_reset_detected() fails them without waiting.
> 
> Tested on two arm64 (64K pages) servers, each with two idpf PFs, with
> this change backported to a 6.17 kernel and the IOMMU translating: no
> faults in 25 warm reboots, against a fault on every warm reboot
> without it. Each PF's teardown took about 1.2 s for six mailbox
> transactions; the slowest, VIRTCHNL2_OP_DEALLOC_VECTORS, took about
> 300 ms. To stand in for a control plane that does not reply, the driver

I'd probably bump this to 500ms to be sure. It's good that you measured 
the slowest in your environment, but there is no guarantee that would be 
the same elsewhere.

> was also built to skip sending mailbox messages during shutdown: the
> first transaction timed out after 2 s, the rest failed at once, the
> teardown took 2.8 s per PF, and every reboot faulted again.
> 
> Fixes: 4c9106f4906a ("idpf: fix adapter NULL pointer dereference on reboot")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Tian Xun Ng <tianxun.ng@bytedance.com>
> ---
> v2:
> - Drop patch 2 (function reset on shutdown). On a VF it queued
>    VIRTCHNL2_OP_RESET_VF and then freed the mailbox, and on these devices
>    a PF reset does not clear the stale queues anyway (numbers in the v1
>    thread).
> - Set IDPF_REMOVE_IN_PROG in idpf_shutdown() instead of changing the
>    condition in idpf_vc_core_deinit(); no new idpf_is_reset_detected()
>    call (Emil).
> - Bound the shutdown teardown: 2 s per transaction instead of 60 s, and
>    fail the remaining ones at once after the first timeout (Emil). 2 s is
>    about six times the slowest teardown transaction measured here.
> - No VF special case, unlike what I suggested in the thread: a VF whose
>    PF has been reset already fails fast through the existing
>    idpf_is_reset_detected() check in idpf_send_mb_msg(), and a VF with a
>    working mailbox leaves queues behind on a warm reboot just like a PF.
>    Not tested on a VF; I have no VF setup.
> - Compile-tested on net-queue dev-queue (W=1, allmodconfig and
>    allyesconfig, no new warnings); runtime-tested as the 6.17 backport
>    described above.
> 
> v1: https://lore.kernel.org/all/20260917105205.37561-1-luckilystar08@gmail.com/
> 
>   drivers/net/ethernet/intel/idpf/idpf.h        |  4 +++
>   drivers/net/ethernet/intel/idpf/idpf_main.c   |  7 +++++
>   .../net/ethernet/intel/idpf/idpf_virtchnl.c   | 28 ++++++++++++++++---
>   .../net/ethernet/intel/idpf/idpf_virtchnl.h   |  1 +
>   4 files changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf.h b/drivers/net/ethernet/intel/idpf/idpf.h
> index 470bc23c8..5d2e8a346 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf.h
> +++ b/drivers/net/ethernet/intel/idpf/idpf.h
> @@ -88,6 +88,8 @@ enum idpf_state {
>    * @IDPF_REMOVE_IN_PROG: Driver remove in progress
>    * @IDPF_MB_INTR_MODE: Mailbox in interrupt mode
>    * @IDPF_VC_CORE_INIT: virtchnl core has been init
> + * @IDPF_SHUTDOWN_IN_PROG: Driver shutdown in progress
> + * @IDPF_SHUTDOWN_XN_TIMEOUT: A mailbox transaction timed out during shutdown
>    * @IDPF_FLAGS_NBITS: Must be last
>    */
>   enum idpf_flags {
> @@ -97,6 +99,8 @@ enum idpf_flags {
>   	IDPF_REMOVE_IN_PROG,
>   	IDPF_MB_INTR_MODE,
>   	IDPF_VC_CORE_INIT,
> +	IDPF_SHUTDOWN_IN_PROG,
> +	IDPF_SHUTDOWN_XN_TIMEOUT,

In general we try to avoid adding new flags as they tend to spread 
accross the code in weird ways. I think we can replicate the logic in 
your patch without the need for introducing new flags.

>   	IDPF_FLAGS_NBITS,
>   };
>   
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c b/drivers/net/ethernet/intel/idpf/idpf_main.c
> index 129bccaa6..fa27ee1cd 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_main.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_main.c
> @@ -196,6 +196,13 @@ static void idpf_shutdown(struct pci_dev *pdev)
>   
>   	cancel_delayed_work_sync(&adapter->serv_task);
>   	cancel_delayed_work_sync(&adapter->vc_event_task);
> +
> +	/* 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.
> +	 */
> +	set_bit(IDPF_SHUTDOWN_IN_PROG, adapter->flags);
> +	set_bit(IDPF_REMOVE_IN_PROG, adapter->flags);

Case in point, now we have an overlap with SHUTDOWN_IN_PROG and 
REMOVE_IN_PROG, with the shutdown logic depending on the side effects of 
REMOVE_IN_PROG. 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, like in 
this spot instead of the above, something like:

+	adapter->vc_xn_timeout_ms = IDPF_VC_XN_SHUTDOWN_TIMEOUT_MSEC;
+	set_bit(IDPF_REMOVE_IN_PROG, adapter->flags);

followed with the error logic in idpf_send_mb_msg() that allows us to 
bail early.

Thanks,
Emil

>   	idpf_vc_core_deinit(adapter);
>   	idpf_deinit_dflt_mbx(adapter);
>   
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> index 1caf52706..5f5d72671 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> @@ -190,22 +190,38 @@ static void idpf_prepare_ptp_mb_msg(struct idpf_adapter *adapter, u32 op,
>    * Cleanup the mailbox queue entries of the previously sent message to
>    * unmap and release the buffer.
>    *
> + * During shutdown each transaction gets a short timeout, and once one of
> + * them times out the rest fail at once, so that an unresponsive control
> + * plane cannot hold up a reboot.
> + *
>    * Return: 0 if the request was successful, -%EBUSY if reset is detected
> - *	   or Tx control queue is full, other negative error code on failure.
> + *	   or Tx control queue is full, -%ETIMEDOUT if a transaction already
> + *	   timed out during shutdown, other negative error code on failure.
>    */
>   int idpf_send_mb_msg(struct idpf_adapter *adapter,
>   		     struct libie_ctlq_xn_send_params *xn_params,
>   		     void *send_buf, size_t send_buf_size)
>   {
> +	bool shutdown = test_bit(IDPF_SHUTDOWN_IN_PROG, adapter->flags);
>   	struct libie_ctlq_msg ctlq_msg = {};
> +	int err = 0;
>   
> -	if (idpf_is_reset_detected(adapter)) {
> +	if (idpf_is_reset_detected(adapter))
> +		err = -EBUSY;
> +	else if (shutdown && test_bit(IDPF_SHUTDOWN_XN_TIMEOUT, adapter->flags))
> +		err = -ETIMEDOUT;
> +
> +	if (err) {
>   		if (!libie_cp_can_send_onstack(send_buf_size))
>   			kfree(send_buf);
>   
> -		return -EBUSY;
> +		return err;
>   	}
>   
> +	if (shutdown)
> +		xn_params->timeout_ms = min_t(u64, xn_params->timeout_ms,
> +					      IDPF_VC_XN_SHUTDOWN_TIMEOUT_MSEC);
> +
>   	idpf_prepare_ptp_mb_msg(adapter, xn_params->chnl_opcode, &ctlq_msg);
>   	xn_params->ctlq_msg = ctlq_msg.opcode ? &ctlq_msg : NULL;
>   
> @@ -217,7 +233,11 @@ int idpf_send_mb_msg(struct idpf_adapter *adapter,
>   
>   	idpf_mb_clean(xn_params->ctlq, false);
>   
> -	return libie_ctlq_xn_send(xn_params);
> +	err = libie_ctlq_xn_send(xn_params);
> +	if (err == -ETIMEDOUT && shutdown)
> +		set_bit(IDPF_SHUTDOWN_XN_TIMEOUT, adapter->flags);
> +
> +	return err;
>   }
>   
>   /**
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
> index 5d27805ff..b0809d149 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
> +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
> @@ -7,6 +7,7 @@
>   #include <linux/net/intel/virtchnl2.h>
>   
>   #define IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC	(60 * 1000)
> +#define IDPF_VC_XN_SHUTDOWN_TIMEOUT_MSEC	2000
>   
>   struct idpf_adapter;
>   struct idpf_netdev_priv;

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

[Tian Xun Ng]

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
   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?

Thanks,
Tian Xun

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

[Tantilov, Emil S]

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