netdev - Re: [net-next,v1,3/7] net: wangxun: move reusable PCI driver ops functions into libwx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260204022237.1572988-1-kuba@kernel.org>
Date: Tue,  3 Feb 2026 18:22:37 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: jiawenwu@...stnetic.com
Cc: Jakub Kicinski <kuba@...nel.org>,
	mengyuanlou@...-swift.com,
	horms@...nel.org,
	netdev@...r.kernel.org,
	edumazet@...gle.com,
	davem@...emloft.net,
	andrew+netdev@...n.ch,
	pabeni@...hat.com
Subject: Re: [net-next,v1,3/7] net: wangxun: move reusable PCI driver ops functions into libwx

This is an AI-generated review of your patch. The human sending this
email says "Sending this out without checking since you have to
repost. Please note that the AI code review may be wrong especially
around defensive programming (which we recommend against)."

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: wangxun: move reusable PCI driver ops functions into libwx

This commit refactors PCI driver ops functions (suspend, resume, shutdown)
from ngbe_main.c into the shared libwx library, adding function pointers
wx->close_suspend() and adjusting wx->do_reset() to support this.

> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_lib.c b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
> --- a/drivers/net/ethernet/wangxun/libwx/wx_lib.c
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_lib.c

[ ... ]

> +int wx_resume(struct pci_dev *pdev)
> +{
> +	struct net_device *netdev;
> +	struct wx *wx;
> +	u32 err;
> +
> +	wx = pci_get_drvdata(pdev);
> +	netdev = wx->netdev;
> +
> +	err = pci_enable_device_mem(pdev);
> +	if (err) {
> +		wx_err(wx, "Cannot enable PCI device from suspend\n");
> +		return err;
> +	}
> +	pci_set_master(pdev);
> +	device_wakeup_disable(&pdev->dev);
> +
> +	wx->do_reset(netdev, false);
> +	rtnl_lock();
> +
> +	err = wx_init_interrupt_scheme(wx);
> +	if (!err && netif_running(netdev))
> +		err = netdev->netdev_ops->ndo_open(netdev);
> +	if (!err)
> +		netif_device_attach(netdev);
> +	rtnl_unlock();
> +
> +	return 0;
> +}

Does wx_resume() need to return the error value stored in err?

The function assigns errors from wx_init_interrupt_scheme() and ndo_open()
to the err variable, and correctly skips netif_device_attach() when errors
occur, but then unconditionally returns 0 instead of returning err.

The early error handling for pci_enable_device_mem() correctly returns err,
but errors from the later calls are lost. This would tell the PM core that
resume succeeded when it actually failed.
-- 
pw-bot: cr