Re: [net-next,v2,7/8] net: wangxun: add Tx timeout process

[Jakub Kicinski <kuba@kernel.org>]

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

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: wangxun: add Tx timeout process

Implement .ndo_tx_timeout to handle Tx side timeout event. When Tx timeout
event occur, it will triger driver into reset process.

> diff --git a/drivers/net/ethernet/wangxun/libwx/Makefile b/drivers/net/ethernet/wangxun/libwx/Makefile
> index a71b0ad77de3..c8724bb129aa 100644
> --- a/drivers/net/ethernet/wangxun/libwx/Makefile
> +++ b/drivers/net/ethernet/wangxun/libwx/Makefile
> @@ -4,5 +4,5 @@
>
>  obj-$(CONFIG_LIBWX) += libwx.o
>
> -libwx-objs := wx_hw.o wx_lib.o wx_ethtool.o wx_ptp.o wx_mbx.o wx_sriov.o
> +libwx-objs := wx_hw.o wx_lib.o wx_ethtool.o wx_ptp.o wx_mbx.o wx_sriov.o wx_err.o
>  libwx-objs += wx_vf.o wx_vf_lib.o wx_vf_common.o

[ ... ]

> diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
> index 4dd7de79431c..0ed33480b4f4 100644
> --- a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
> +++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c

[ ... ]

> @@ -779,9 +803,11 @@ static int ngbe_probe(struct pci_dev *pdev,
>  	eth_hw_addr_set(netdev, wx->mac.perm_addr);
>  	wx_mac_set_default_filter(wx, wx->mac.perm_addr);
>
> +	ngbe_init_service(wx);
> +
>  	err = wx_init_interrupt_scheme(wx);
>  	if (err)
> -		goto err_free_mac_table;
> +		goto err_cancel_service;
>
>  	/* phy Interface Configuration */
>  	err = ngbe_mdio_init(wx);
> @@ -801,6 +827,9 @@ static int ngbe_probe(struct pci_dev *pdev,
>  	wx_control_hw(wx, false);
>  err_clear_interrupt_scheme:
>  	wx_clear_interrupt_scheme(wx);
> +err_cancel_service:
> +	timer_delete_sync(&wx->service_timer);
> +	cancel_work_sync(&wx->service_task);
>  err_free_mac_table:
>  	kfree(wx->rss_key);
>  	kfree(wx->mac_table);
> @@ -826,6 +855,8 @@ static void ngbe_remove(struct pci_dev *pdev)
>  	struct wx *wx = pci_get_drvdata(pdev);
>  	struct net_device *netdev;
>
> +	cancel_work_sync(&wx->service_task);
> +

Should timer_delete_sync(&wx->service_timer) be called before
cancel_work_sync(&wx->service_task) in ngbe_remove()?

The service timer is self-rearming and calls mod_timer() to reschedule
itself and wx_service_event_schedule() to queue work every 2 seconds:

drivers/net/ethernet/wangxun/libwx/wx_lib.c:wx_service_timer()
{
	...
	wx_service_event_schedule(wx);
	mod_timer(&wx->service_timer, ...);
}

If the timer fires after cancel_work_sync returns, it can queue new work
that runs concurrently with or after device teardown. The probe error path
shows the correct order:

ngbe_probe() error path:
err_cancel_service:
	timer_delete_sync(&wx->service_timer);
	cancel_work_sync(&wx->service_task);

This stops the timer first to prevent new work from being queued, then
cancels any pending work.
-- 
pw-bot: cr