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