Re: [PATCH net-next v5 6/6] net: xilinx: axienet: Add statistics support for XXV ethernet

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] net: xilinx: axienet: wrong RX bad-FCS offset in
  axienet_xxv_get_stats64()
--

commit 460172ab6de68521ff2c9e9d0c5b3adf14de408f
Author: Suraj Gupta <suraj.gupta2@amd.com>

net: xilinx: axienet: Add statistics support for XXV ethernet

This commit adds hardware statistics collection and reporting for the
Xilinx 10G/25G (XXV) Ethernet MAC via ndo_get_stats64() and ethtool
statistics callbacks, gated on the "xlnx,has-statistics-counters"
devicetree property.

> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_xxv.c b/drivers/net/ethernet/xilinx/xilinx_axienet_xxv.c
> index 3c204dbbf6aec..83782bfbce4ce 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_xxv.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_xxv.c
[ ... ]
> @@ -335,6 +411,204 @@ static int axienet_10g25g_clk_init(struct axienet_local *lp)
>  	return 0;
>  }
[ ... ]
> +static void axienet_xxv_get_stats64(struct axienet_local *lp,
> +				    struct rtnl_link_stats64 *stats)
> +{
> +	unsigned int start;
> +	u64 tx_hw_errors;
> +
> +	do {
> +		start = read_seqcount_begin(&lp->hw_stats_seqcount);
> +		stats->rx_crc_errors =
> +			lp->xxv_stat_base[XXV_STAT_RX_PACKET_BAD_FCS];
> +		/* Both in-range length errors and frame-too-long (oversize)
> +		 * frames are IEEE 802.3 length errors, so fold them together
> +		 * into rx_length_errors; rx_over_errors is reserved for receiver
> +		 * FIFO/ring overflow, which the XXV MAC does not expose.
> +		 */
> +		stats->rx_length_errors =
> +			lp->xxv_stat_base[XXV_STAT_RX_INRANGEERR] +
> +			lp->xxv_stat_base[XXV_STAT_RX_OVERSIZE];
> +		stats->rx_errors = lp->xxv_stat_base[XXV_STAT_RX_UNDERSIZE] +
> +				   lp->xxv_stat_base[XXV_STAT_RX_FRAGMENT] +
> +				   lp->xxv_stat_base[XXV_STAT_RX_JABBER] +
> +				   stats->rx_crc_errors +
> +				   stats->rx_length_errors;

[ ... ]

> +static void axienet_xxv_get_eth_mac_stats(struct axienet_local *lp,
> +					  struct ethtool_eth_mac_stats *mac_stats)
> +{
> +	unsigned int start;
> +
> +	do {
> +		start = read_seqcount_begin(&lp->hw_stats_seqcount);
> +		mac_stats->FramesTransmittedOK =
> +			lp->xxv_stat_base[XXV_STAT_TX_TOTAL_GOOD_PACKETS];
> +		mac_stats->FramesReceivedOK =
> +			lp->xxv_stat_base[XXV_STAT_RX_TOTAL_GOOD_PACKETS];
> +		mac_stats->OctetsTransmittedOK =
> +			lp->xxv_stat_base[XXV_STAT_TX_TOTAL_GOOD_BYTES];
> +		mac_stats->OctetsReceivedOK =
> +			lp->xxv_stat_base[XXV_STAT_RX_TOTAL_GOOD_BYTES];
> +		mac_stats->FrameCheckSequenceErrors =
> +			lp->xxv_stat_base[XXV_STAT_RX_PACKET_BAD_FCS];

[ ... ]

> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_xxv.h b/drivers/net/ethernet/xilinx/xilinx_axienet_xxv.h
> index 6530344e40c7f..83682d1524ede 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_xxv.h
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_xxv.h
> @@ -66,6 +66,36 @@ struct axienet_config;
[ ... ]
> +#define XXV_STAT_RX_UNDERSIZE_OFFSET		0x00000898
> +#define XXV_STAT_RX_FRAGMENT_OFFSET		0x000008A0
> +#define XXV_STAT_RX_OVERSIZE_OFFSET		0x000008A8
> +#define XXV_STAT_RX_JABBER_OFFSET		0x000008B8
> +#define XXV_STAT_RX_PACKET_BAD_FCS_OFFSET	0x000008C8

[Severity: Medium]
Should this read STAT_RX_BAD_FCS at offset 0x08C0 instead of
STAT_RX_PACKET_BAD_FCS at 0x08C8?

Per PG210, STAT_RX_PACKET_BAD_FCS (0x08C8) counts all received packets with
bad FCS regardless of length, which includes frames shorter than 64 bytes
with bad FCS (STAT_RX_FRAGMENT at 0x08A0) and frames longer than the configured
maximum length with bad FCS (STAT_RX_JABBER at 0x08B8).  Using 0x08C8
causes axienet_xxv_get_stats64() to double-count received fragments and
jabbers in stats->rx_errors, and includes undersize and oversize bad-FCS
frames in stats->rx_crc_errors and mac_stats->FrameCheckSequenceErrors,
whereas STAT_RX_BAD_FCS (0x08C0) counts only valid-length frames with bad
FCS.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114182947.2908628-1-suraj.gupta2@amd.com?part=6