Re: [PATCH net-next v01 2/7] hinic3: Add ethtool statistic ops

[Ioana Ciornei]

On Fri, Mar 13, 2026 at 03:54:09PM +0800, Fan Gong wrote:
>   Add PF/VF statistics functions in TX and RX processing.
>   Implement following ethtool callback function:
> .get_sset_count
> .get_ethtool_stats
> .get_strings
> .get_eth_phy_stats
> .get_eth_mac_stats
> .get_eth_ctrl_stats
> .get_rmon_stats
> .get_pause_stats
> 
>   These callbacks allow users to utilize ethtool for detailed
> TX and RX netdev stats monitoring.
> 
> Co-developed-by: Zhu Yikai <zhuyikai1@xxxxxxxxxxxxxx>
> Signed-off-by: Zhu Yikai <zhuyikai1@xxxxxxxxxxxxxx>
> Signed-off-by: Fan Gong <gongfan1@xxxxxxxxxx>
> ---

(...)

> +static void hinic3_get_eth_mac_stats(struct net_device *netdev,
> +				     struct ethtool_eth_mac_stats *mac_stats)
> +{
> +	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
> +	struct mag_cmd_port_stats *ps;
> +	int err;
> +
> +	ps = kmalloc_obj(*ps);
> +	if (!ps)
> +		return;
> +
> +	err = hinic3_get_phy_port_stats(nic_dev->hwdev, ps);
> +	if (err) {
> +		kfree(ps);
> +		netdev_err(netdev, "Failed to get eth mac stats from fw\n");
> +		return;
> +	}
> +
> +	mac_stats->FramesTransmittedOK = ps->mac_tx_uni_pkt_num +
> +					 ps->mac_tx_multi_pkt_num +
> +					 ps->mac_tx_broad_pkt_num;

Why do you do this instead of using ps->mac_tx_good_pkt_num which, at
least to an external observer, looks to be FramesTransmittedOK's
matching counter.


> +	mac_stats->FramesReceivedOK = ps->mac_rx_uni_pkt_num +
> +				      ps->mac_rx_multi_pkt_num +
> +				      ps->mac_rx_broad_pkt_num;

The same comment here with mac_rx_good_pkt_num.

> +	mac_stats->FrameCheckSequenceErrors = ps->mac_rx_fcs_err_pkt_num;
> +	mac_stats->OctetsTransmittedOK = ps->mac_tx_total_oct_num;
> +	mac_stats->OctetsReceivedOK = ps->mac_rx_total_oct_num;
> +	mac_stats->MulticastFramesXmittedOK = ps->mac_tx_multi_pkt_num;
> +	mac_stats->BroadcastFramesXmittedOK = ps->mac_tx_broad_pkt_num;
> +	mac_stats->MulticastFramesReceivedOK = ps->mac_rx_multi_pkt_num;
> +	mac_stats->BroadcastFramesReceivedOK = ps->mac_rx_broad_pkt_num;
> +	mac_stats->InRangeLengthErrors = ps->mac_rx_fcs_err_pkt_num;

You assign mac_rx_fcs_err_pkt_num to InRangeLengthErrors. This doesn't
seem right.

> +	mac_stats->OutOfRangeLengthField = ps->mac_rx_undersize_pkt_num +
> +					   ps->mac_rx_undermin_pkt_num +
> +					   ps->mac_rx_oversize_pkt_num +
> +					   ps->mac_rx_jabber_pkt_num;
> +	mac_stats->FrameTooLongErrors = ps->mac_rx_oversize_pkt_num +
> +					ps->mac_rx_jabber_pkt_num;

If your MAC does not export certain counters listed in the IEEE 802.3
MAC statistics (30.3.1.1.*) I think it would be best to just not set
them up in the ethtooo_eth_mac_stats structure.

Ioana