Re: [PATCH net-next v3 1/3] ethtool: add PHY receive frame and byte totals

[James Hilliard]

On Fri, Nov 13, 2026 at 11:42 AM Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx> wrote:
>
> On Tue, Nov 10, 2026 at 05:42:33PM +0100, Andrew Lunn wrote:
> > On Mon, Nov 09, 2026 at 07:48:32PM -0700, James Hilliard wrote:
> > > The generic PHY receive packet and byte counters describe successfully
> > > received traffic. Some PHYs instead count all received frames and bytes,
> > > including frames with errors. A frame error count cannot recover the
> > > number of successfully received bytes because damaged frames have
> > > different lengths.
> > >
> > > Add optional receive-total counters to ethtool_phy_stats and the phydev
> > > statistics group. Count frame bytes including the Ethernet header, padding
> > > and FCS, and excluding the preamble and start frame delimiter. These
> > > counters describe reception at the PHY, before MAC or host filtering.
> > >
> > > Append the new attributes so existing statistic IDs and their meanings
> > > remain unchanged. Publish the names through the existing statistics string
> > > set, and retain the unset-counter convention so drivers can report either
> > > set of counters independently.
> > >
> > > Signed-off-by: James Hilliard <james.hilliard1@xxxxxxxxx>
> > > ---
> > >  Documentation/networking/ethtool-netlink.rst | 16 +++++++++++++++-
> > >  include/linux/ethtool.h                      |  6 ++++++
> > >  include/uapi/linux/ethtool_netlink.h         |  2 ++
> > >  net/ethtool/stats.c                          |  8 +++++++-
> > >  4 files changed, 30 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
> > > index eba226c79ec3..5528d0f7afdd 100644
> > > --- a/Documentation/networking/ethtool-netlink.rst
> > > +++ b/Documentation/networking/ethtool-netlink.rst
> > > @@ -1665,7 +1665,7 @@ the ``ETHTOOL_A_STATS_GROUPS`` bitset. Currently defined values are:
> > >   ETHTOOL_STATS_ETH_PHY  eth-phy  Basic IEEE 802.3 PHY statistics (30.3.2.1.*)
> > >   ETHTOOL_STATS_ETH_CTRL eth-ctrl Basic IEEE 802.3 MAC Ctrl statistics (30.3.3.*)
> > >   ETHTOOL_STATS_RMON     rmon     RMON (RFC 2819) statistics
> > > - ETHTOOL_STATS_PHY      phy      Additional PHY statistics, not defined by IEEE
> > > + ETHTOOL_STATS_PHY      phydev   Additional PHY statistics, not defined by IEEE
> > >   ====================== ======== ===============================================
> > >
> > >  Each group should have a corresponding ``ETHTOOL_A_STATS_GRP`` in the reply.
> > > @@ -1683,6 +1683,20 @@ by ``ETHTOOL_A_STATS_GRP_SS_ID``. Complex statistics (such as RMON histogram
> > >  entries) are also listed inside ``ETHTOOL_A_STATS_GRP`` and do not have
> > >  a string defined in the string set.
> > >
> > > +The ``phydev`` group provides separate counters for successfully received
> > > +traffic and total received traffic.
> >
> > You need to be careful with naming here. This is a
> > kAPI. phydev/phylib/phylink are implementation details, a set of
> > helpers which some MAC drivers use to implement PHYs. But MAC driver
> > can implement PHYs in different ways, and don't need to use a phydev.
> >
> > ``ETHTOOL_A_STATS_PHY_RX_PKTS``
> > > +(``RxFrames``) and ``ETHTOOL_A_STATS_PHY_RX_BYTES`` (``RxOctets``) count
> > > +successfully received frames and bytes. ``ETHTOOL_A_STATS_PHY_RX_PKTS_TOTAL``
> > > +(``RxFramesTotal``) and ``ETHTOOL_A_STATS_PHY_RX_BYTES_TOTAL`` (``RxOctetsTotal``)
> > > +include frames with errors. The total byte count includes the Ethernet header,
> > > +padding and FCS, but excludes the preamble and start frame delimiter.
> >
> > And what would you call it when the counter does include the preamble
> > and start frame delimiter?
> >
> > Take a step back. Just because this PHY has these counters, do we need
> > to support them? And what happens when the next PHY comes along with
> > slightly different counters? Do we add them as well?
> >
> > It is vendor SDK syndrome. The hardware can do this, so lets expose
> > it.
> >
> > What is your use case here which justifies these specific counters.
>
> I have the same question. James, your answer mentions traffic monitoring
> and error diagnosis, but not a concrete problem on AC200/AC300 that these
> totals would help with. My guess is a receive problem that another driver
> for this PHY already works around.
>
> Rockchip's vendor driver for the same PHY ID (RK630 and RV1106) reads the
> packet and CRC error counters from page 9, registers 0x1b/0x1c and
> 0x1d/0x1e:
>
> https://github.com/rockchip-linux/kernel/blob/eeb355823a794954737fd75820dfccdab4c57af6/drivers/net/phy/rk630phy.c#L430
>
> The vendor driver does not export them. It polls them to tune the
> receiver. If there is a signal but no link, it switches to a second
> analog setting and checks again. It switches settings in two more cases:
> too few frames arrive, or more than 15% of the frames have a CRC error.

Allwinner applies efuse-derived trim on both AC200 and AC300 alongside
hard-coded PHY register settings. AC200 uses one fixed set of settings;
AC300 selects between two sets using efuse bit 9. These are applied at
initialization, rather than adjusted from live packet/error statistics.

> Its comments call this "Long network cable parameters". It also changes
> the RX detect threshold depending on the temperature.
>
> Do you see a problem like this on your AC200/AC300 boards? If so, can
> you describe it? Then we can decide whether the driver should work
> around that problem.

I haven't reproduced that problem or tested long cables and temperature
extremes. Temporary register changes on AC200 confirmed that candidate
tuning fields are writable, but not that their analog meanings match
Rockchip's. Adaptive tuning could be relevant future work, but I don't
yet have evidence that ACx00 needs the Rockchip workaround.

>
> Oleksij