From: Tian Xun Ng <luckilystar08@gmail.com>
To: emil.s.tantilov@intel.com
Cc: Tian Xun Ng <luckilystar08@gmail.com>,
intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
anthony.l.nguyen@intel.com, przemyslaw.kitszel@intel.com,
aleksander.lobakin@intel.com, aleksandr.loktionov@intel.com,
andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com,
Tian Xun Ng <tianxun.ng@bytedance.com>
Subject: Re: [PATCH iwl-net 1/2] idpf: keep the mailbox up while tearing down vports on shutdown
Date: Mon, 21 Sep 2026 11:27:32 +0800 [thread overview]
Message-ID: <20260921032827.32485-2-luckilystar08@gmail.com> (raw)
In-Reply-To: <45ffbd03-653c-46fc-b500-07e9b99f295f@intel.com>
On 9/18/2026, Tantilov, Emil S wrote:
> Actually we can't wait on shutdown. If the MBX is defunct, like CP is
> down or unresponsive, the shutdown will hang for a very long time. This
> is the reason why we wanted to avoid communication on shutdown. Have you
> tested the shutdown after stopping the control plane?
No, I have not, and I cannot on this platform: the control plane sits
behind the device and I have no way to stop it from the host. So I have
to take your point as given, and as written the patch is not acceptable:
each teardown transaction uses IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC (60 s),
and the teardown per vport is disable_vport, disable_queues (which also
waits for the SW marker) and destroy_vport. With a dead CP and two vports
that is minutes of hang on every reboot, to remove a fault that only
shows up on warm reboots. That trade is wrong.
What I would like to propose for v2 is to keep the teardown but bound it:
a shutdown-specific timeout, on the order of a second or two, used for
those three transactions when the driver is shutting down. If the CP
answers, the device is told to stop its queues and the stray writes go
away; if it does not, shutdown loses a bounded couple of seconds instead
of minutes. Does that direction look acceptable to you, and is there a
timeout value you would consider safe? If you would rather not have any
mailbox traffic on shutdown at all, then I think the fix has to come from
the device side instead, and I would rather know that before sending v2.
> This logic already exists in the reset handling, there should be no need
> to replicate it here. Do you have a trace and/or exact scenario that
> leads to remove being called while in a reset, but MBX is still alive?
No, I do not have such a trace. I added idpf_is_reset_detected() defensively
rather than from an observed case, and I will drop it in v2.
For the record, what we do see without any of this, on arm64 with two idpf
functions: after a warm reboot the device still has its queues enabled with
the previous kernel's ring addresses, and the first queue reconfiguration in
the next boot makes it write SW_MARKER completions into memory that kernel
has already reused. 20 of 20 warm reboots on an unpatched control node, none
in 151 with the teardown messages delivered.
Thanks for the review.
next prev parent reply other threads:[~2026-09-21 3:28 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-17 10:52 [PATCH iwl-net 0/2] idpf: stop stray device writes after a warm reboot Tian Xun Ng
2026-09-17 10:52 ` [PATCH iwl-net 1/2] idpf: keep the mailbox up while tearing down vports on shutdown Tian Xun Ng
2026-09-18 15:45 ` Loktionov, Aleksandr
2026-09-18 17:59 ` Tantilov, Emil S
2026-09-21 3:27 ` Tian Xun Ng [this message]
2026-09-21 21:06 ` Tantilov, Emil S
2026-10-06 10:56 ` Tian Xun Ng
2026-10-06 20:03 ` Tantilov, Emil S
2026-09-17 10:52 ` [PATCH iwl-net 2/2] idpf: reset the function " Tian Xun Ng
2026-09-18 15:46 ` Loktionov, Aleksandr
2026-09-18 17:54 ` Tantilov, Emil S
2026-09-21 3:27 ` Tian Xun Ng
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=20260921032827.32485-2-luckilystar08@gmail.com \
--to=luckilystar08@gmail.com \
--cc=aleksander.lobakin@intel.com \
--cc=aleksandr.loktionov@intel.com \
--cc=andrew+netdev@lunn.ch \
--cc=anthony.l.nguyen@intel.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=emil.s.tantilov@intel.com \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=przemyslaw.kitszel@intel.com \
--cc=tianxun.ng@bytedance.com \
/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