Thread (13 messages) 13 messages, 3 authors, 6d ago

Re: [PATCH net-next v2 3/6] enic: recover V2 VF mailbox when PF state is unknown

flat view

From: Satish Kharat <satishkh@cisco.com>
Date: 2026-10-06 20:41:38
Also in: lkml

Hi Simon,

Thanks for taking a look, and apologies for not responding to the Sashiko report sooner.

I agree that both High findings are valid. They are related consequences of the reset and error-unwind handling in this revision: the netdev can remain administratively running after the datapath has been partially stopped, and vf_datapath_open is not sufficient to represent and safely recover that state.

I have reviewed the remaining findings as well and am preparing a new
revision that addresses the valid, non-pre-existing issues. I will also reply to the Sashiko report with an issue-by-issue disposition for every Medium-or-higher finding, including concrete explanations for findings that do not require a code change.

Please consider v2 superseded. I will post v3 after the updated implementation has completed review and testing.

Thanks,
Satish

pw-bot: cr

On Tue, Oct 6, 2026 at 5:48 PM Simon Horman [off-list ref] wrote:
On Sat, Oct 03, 2026 at 07:50:54PM +0000,
netdev-bot+sashiko@kernel.org wrote:
quoted
Thank you for your contribution! Sashiko AI review found 11 potential
issue(s) to consider.

Critical: 0 · High: 2 · Medium: 6 · Low: 3

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] enic_reset() now has two new error exits, taken when
  enic_dev_soft_reset() fails or when the newly added vnic_dev_init()…
- [High] The patch adds vf_datapath_open so that "a failed internal reset
  cannot stop it twice".
Hi Satish,

There is an expectation that the author responds to these messages
in order for the patch-set to proceed.

In order to move things a long a bit I've taken a brief look at the two
issues above (only) and they seem both related to each other and
legitimate problems.

Please also look over these and other issues reported by
netdev-bot+sashiko@kernel.org for this patchset-set.

For those that are non-issues, please indicate that,
e.g. by responding to the bot on the ML.

And please post an updated version that addresses the real,
non-pre-existing, issues.

Thanks!
quoted
- [Medium] The commit message says the timed-out DMA mapping is left "for
  admin-channel teardown to reclaim safely".
- [Medium] vf_mbox_recovery_active is a plain bool.
- [Medium] vf_mbox_recovery_active stays true for the whole run of the
  reset worker, including the time after enic_open() has committed…
- [Medium] The patch makes several admin-RQ drop paths trigger full VF
  recovery (RX quarantine, carrier off, sends disabled, a soft reset…
- [Medium] The new V2 VF error exit in enic_open() (`goto
  err_out_dev_disable`) runs after enic_dev_enable() has succeeded, when…
- [Medium] The new vnic_dev_init() call in enic_reset() issues CMD_INIT,
  or on older firmware CMD_INIT_v1 + CMD_GET_MAC_ADDR + CMD_ADDR_ADD,…
- [Low] Concern: the patch has no Fixes: tag even though it fixes V2
  mailbox behaviour introduced by commit 1f0c856b596337.
- [Low] Concern: the commit message and comments assume a VF MAC/receive-
  filter mailbox protocol and a station/filter replay that do not exist…
- [Low] Concern: the new enic_reset_addr_lists() calls in enic_open() and
  enic_admin_chan_reopen() run __dev_uc_unsync()/__dev_mc_unsync()…
--
pw-bot: changes-requested
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help