Re: [PATCH net-next v10 5/5] net: wangxun: add pcie error handler

On Thu, Jul 16, 2026 at 02:55:30PM +0800, Jiawen Wu wrote:

On Tue, Jul 14, 2026 5:11 PM, Simon Horman wrote:

quoted

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---

--- Patch [5]: [PATCH net-next v10 5/5] net: wangxun: add pcie error handler ---

commit 91baa4e77d07ab6772569a7d37ed86cfda625c00
Author: Jiawen Wu [off-list ref]
Subject: net: wangxun: add pcie error handler

This commit adds PCIe error handling support (AER) to the wangxun drivers
(ngbe and txgbe). It detects hardware errors, attempts to soft quiesce,
and registers standard PCI error handler callbacks.

quoted

diff --git a/drivers/net/ethernet/wangxun/libwx/wx_err.c b/drivers/net/ethernet/wangxun/libwx/wx_err.c
index ee27f96735dc7..0cb99e456a1c9 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_err.c
+++ b/drivers/net/ethernet/wangxun/libwx/wx_err.c

[ ... ]

quoted

+static void wx_io_resume(struct pci_dev *pdev)
+{
+	struct wx *wx = pci_get_drvdata(pdev);
+	struct net_device *netdev;
+	int err;
+
+	netdev = wx->netdev;
+	rtnl_lock();
+	if (netif_running(netdev)) {
+		err = netdev->netdev_ops->ndo_open(netdev);

[Severity: High]
Does this sequence risk corrupting the hw_tc_lock seqlock?

During AER recovery, wx_io_slot_reset() calls down_suspend(), which bypasses
wx_ptp_stop(). This leaves the PTP clock registered and potentially in use by
userspace.

When wx_io_resume() then calls ndo_open(), this triggers txgbe_up() or
ngbe_up(), which in turn calls wx_ptp_init(). Since wx_ptp_init()
unconditionally calls seqlock_init(&wx->hw_tc_lock), won't this corrupt the
lock if userspace is actively issuing PTP ioctls?

PTP clock was destroyed in wx_ptp_quiesce() before V8 patch set.  The
piece of code was removed based on the Sashiko's comment.  I think it
should be added back because the order of the calls in wx_soft_quiesce()
has been changed.

Thanks.

quoted

quoted

+		if (err) {
+			wx_err(wx, "Failed to open netdev after reset\n");

[ ... ]

quoted

diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
index 92895f5035118..56d4b63387fd6 100644
--- a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
+++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c

[ ... ]

quoted

@@ -413,6 +428,9 @@ static void ngbe_disable_device(struct wx *wx)

 static void ngbe_reset(struct wx *wx)
 {
+	if (test_bit(WX_FLAG_NEED_PCIE_RECOVERY, wx->flags))
+		return;
+
 	wx_flush_sw_mac_table(wx);

[Severity: High]
Is the hardware properly reinitialized here after an AER slot reset?

In wx_io_slot_reset(), the hardware needs to be restarted from scratch.
While txgbe_reset() calls txgbe_reset_hw(), ngbe_reset() only flushes the
software MAC tables and skips calling ngbe_reset_hw().

Because this patch registers wx_err_handler for the ngbe driver, won't this
leave the hardware uninitialized and the interface broken after a PCIe slot
reset?

I think I have answered this question many times and also described
ngbe's lack of functionality in the commit and cover-letter messages.

Sorry about that, I missed that this is already covered.

...