Re: [PATCH net 1/2] net: ngbe: propagate resume errors to the PM core

Hi,

Thanks for the review. Point by point:

[High] ngbe_resume() still discards the return value of ngbe_reset_hw()

Agreed. ngbe_reset_hw() can genuinely fail (the wx_stop_adapter() error
exit, or the LAN reset read_poll_timeout() expiring), and on those exits
ngbe_reset_misc(), wx_clear_hw_cntrs(), wx_get_mac_addr() and
wx_init_rx_addrs() are all skipped; ngbe_probe() treats the very same
call as fatal. I will send a follow-up patch propagating that error as
well, so that the whole tail of ngbe_resume() reports failures to the
PM core.

[Medium] "the device can be re-probed"

Agreed, that sentence overstates the effect: device_resume()
(drivers/base/power/main.c) only records the failure (async_error,
dpm_save_failed_dev()) and logs it via pm_dev_err(); there is no
unbind, re-probe or retry. What the patch actually buys is that the
failure is reported to the PM core instead of being silently swallowed;
the user-visible state of the netdev itself is unchanged. Sorry for the
imprecise wording.

[High, pre-existing] detached netdev with __LINK_STATE_START still set
after a failed resume

Acknowledged. One detail from reading the code: the WX_STATE_RES_FREED
guard cannot fire in this scenario. That bit is only set by
ngbe_down_suspend(), which is reachable only via wx->down_suspend from
the PCI error-recovery path (libwx/wx_err.c); the normal suspend path
goes through ngbe_dev_shutdown() -> ngbe_close() and never sets it, so
the bit is clear here and the double teardown (including the memset()
of already-freed descriptor rings) is indeed reachable. It is
pre-existing and larger than this series; I plan to look at the
failed-open unwind separately.

[Medium, pre-existing] wxvf_resume()

Acknowledged -- the same swallowed-error pattern is there, with an
unconditional netif_device_attach() on top. It is shared by ngbevf and
txgbevf, so it deserves its own patch rather than being folded into
this series.

[Medium, pre-existing] ngbe_open() error path frees rings while the
hardware is still armed

Acknowledged; see my reply on 2/2, the same reasoning applies here.

To sum up: I do not plan a v2 of this series. Patch 1/2 does what its
commit message promises for the two error paths it touches, the
ngbe_reset_hw() gap is a separate fix, and the remaining findings are
pre-existing. I will follow up with patches for the ngbe_reset_hw()
propagation and the failed-open unwind.

Thanks,
Zhang Yunfei