On Mon, Nov 08, 2021 at 09:11:10PM +0100, Marek Behún wrote: > On Mon, 8 Nov 2021 20:53:36 +0100 > Andrew Lunn <andrew@xxxxxxx> wrote: > > > > 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. > > > > 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. > > I am working with Marvell PHYs and Marvell switches. I am aware of the > problem that SW netdev does not work for switches because we don't have > uptodate data. > > It seems to me that yes, if the user wants to blink the LEDs on > activity on switch port, the netdev trigger should only work in offload > mode and only on LEDs that are connected to switch pins, unless we > implement a mechanism to get statistics at lest 10 times a second > (which could maybe be done on marvell switches by reading the registers > via ethernet frames instead of MDIO). > > Marvell switches don't seem to support rx only / tx only activity > blinking, only rx/tx. I think this could be solved by making rx/tx > sysfs files for netdev trigger behave so that writing to one would also > write to another. > > But since currently netdev trigger does not work for these switches, I > think making it so that it works at least to some fashion (in HW > supported modes) would still be better that current situation. > > I will try to look into this, maybe work together with Ansuel. > > Marek I'm polishing some api with Andrew comments and will try to extend netdev trigger while still proposing the additional trigger. Better discuss in v3. I think the main target here is first have a good generic solution for offload and then actually thinking about working on the triggers/offload part. -- Ansuel
