Re: [PATCH net-next v2 2/5] ionic: Get "link_down_count" ext link stat from firmware

[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.
---
ionic: Get "link_down_count" ext link stat from firmware

This commit replaces the driver-calculated software link down count with the
hardware count provided by the firmware in the extended link statistics. It
updates the ethtool callback to read the counter from the device's port_info
and removes the software tracking completely.

> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> index 78a802eb159f0..af0c4cc8ad8e4 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> @@ -117,7 +117,8 @@ static void ionic_get_link_ext_stats(struct net_device *netdev,
>  	struct ionic_lif *lif = netdev_priv(netdev);
>  
>  	if (lif->ionic->pdev->is_physfn)
> -		stats->link_down_events = lif->link_down_count;
> +		stats->link_down_events =
> +			    lif->ionic->idev.port_info->status.link_down_count;

Could this lead to a NULL pointer dereference?

If a PCI error triggers device recovery via .reset_prepare, the device stops
but the net_device remains registered. If the subsequent ionic_setup_one()
fails during .reset_done, port_info is explicitly freed and set to NULL.
Calling ethtool in this state would crash the kernel.

Other callbacks like ionic_get_link_ksettings() guard against this:

	if (!idev->port_info) {
		netdev_err(netdev, "port_info not initialized\n");
		return -EOPNOTSUPP;
	}

Does link_down_count also need a byte-order conversion?

The firmware-provided field link_down_count in struct ionic_port_status is
defined as a little-endian 16-bit integer (__le16). Assigning it directly
to stats->link_down_events without wrapping it in le16_to_cpu() will result
in incorrect statistic values on big-endian architectures.