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

[Satish Kharat <satishkh@cisco.com>]

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 <horms@kernel.org> wrote:
> On Sat, Oct 03, 2026 at 07:50:54PM +0000,
> netdev-bot+sashiko@kernel.org wrote:
> > 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!
>
> > - [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