On Mon, Nov 08, 2021 at 08:53:36PM +0100, Andrew Lunn wrote:

quoted

I guess I will have to work on this again ASAP or we will end up with
solution that I don't like.

Nonetheless, what is your opinion about offloading netdev trigger vs
introducing another trigger?

It is a solution that fits the general pattern, do it in software, and
offload it if possible.

However, i'm not sure the software solution actually works very well.
At least for switches. The two DSA drivers which implement
get_stats64() simply copy the cached statistics. The XRS700X updates
its cached values every 3000ms. The ar9331 is the same. Those are the
only two switch drivers which implement get_stats64 and none implement
get_stats. There was also was an issue that get_stats64() cannot
perform blocking calls. I don't remember if that was fixed, but if
not, get_stats64() is going to be pretty useless on switches.

No it wasn't, I lost the interest.
I feel pretty uneasy hooking up .ndo_get_stats64() to my switches, and
basically opening the flood gates for random processes and kernel
threads to send SPI transactions back and forth like it's nothing.
Latency for programs like ptp4l and phc2sys is actually more important.

We also need to handle drivers which don't actually implement
dev_get_stats(). That probably means only supporting offloads, all
modes which cannot be offloaded need to be rejected. This is pretty
much the same case of software control of the LEDs is not possible.
Unfortunately, dev_get_stats() does not return -EOPNOTSUPP, you need
to look at dev->netdev_ops->ndo_get_stats64 and
dev->netdev_ops->ndo_get_stats.

Are you working on Marvell switches? Have you implemented
get_stats64() for mv88e6xxx? How often do you poll the hardware for
the stats?

Given this, i think we need to bias the API so that it very likely
ends up offloading, if offloading is available.

     Andrew

We could use some of the newer stats APIs exposed by Jakub if
.ndo_get_stats64 is not implemented, like ethtool_ops->get_eth_mac_stats.
Although.. see above.