Re: [PATCH net-next 2/5] ionic: Report "link_down_events_phy" in ethtool statistics

[Eric Joyner <eric.joyner@amd.com>]

On 5/5/2026 4:21 PM, Jakub Kicinski wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> On Tue, 5 May 2026 12:53:26 -0700 Eric Joyner wrote:
>>> We have a standard stat for this:
>>>
>>> struct ethtool_link_ext_stats {
>>>         /* Custom Linux statistic for PHY level link down events.
>>>          * In a simpler world it should be equal to netdev->carrier_down_count
>>>          * unfortunately netdev also counts local reconfigurations which don't
>>>          * actually take the physical link down, not to mention NC-SI which,
>>>          * if present, keeps the link up regardless of host state.
>>>          * This statistic counts when PHY _actually_ went down, or lost link.
>>>          *
>>>          * Note that we need u64 for ethtool_stats_init() and comparisons
>>>          * to ETHTOOL_STAT_NOT_SET, but only u32 is exposed to the user.
>>>          */
>>>         u64 link_down_events;
>>> };
>>>
>>>
>>> IOW the definition of this stat is - ignoring asymetric link faults
>>> this counter should match between link partners.
>>
>> So, this is a little awkward to talk about -- we are already filling out that
>> field with a stat that's calculated by the driver; there's a task that monitors
>> link transitions and increments it.
>>
>> But, I'm not sure if that method exists because of older firmwares or cards not
>> tracking the link_down_event count for us. So, I didn't want to just overwrite
>> this stat with the value from firmware because then we'd lose the count on cards
>> running firmware that doesn't do that counting for us.
>>
>> So that's one reason why I put the stat in the generic ethtool stats output, and
>> gave it the slightly different name "link_down_events_phy" to distinguish it
>> (this also matches Mellanox's name for consistency) from this stat in the struct
>> you posted. Though reading the doc comment you posted, this new stat really does
>> belong there.
> 
> If the stat is currently filled in with something that does not work as
> documented it should be fine to drop it. The counter is optional and
> relatively recent. I think it's unlikely that any monitoring will have
> a hard dependency on it.

We did some testing internally, and it's safe to replace the link_down_events
stat with the FW generated stat instead of the one the driver calculates. It
gets populated and updated properly by the firmware on all of the hardware we
test on; I'll have that replaced in the v2.

- Eric