On Fri, 2021-01-08 at 10:57 +0200, Vladimir Oltean wrote: > On Thu, Jan 07, 2021 at 07:59:37PM -0800, Saeed Mahameed wrote: > > On Thu, 2021-01-07 at 13:58 +0100, Eric Dumazet wrote: > > > On Thu, Jan 7, 2021 at 12:33 PM Vladimir Oltean < > > > olte...@gmail.com> > > > wrote:
... > > > > > > > There is an effort initiated by Jakub to standardize the > > > > ethtool > > > > statistics. My objection was that you can't expect that to > > > > happen > > > > unless > > > > dev_get_stats is sleepable just like ethtool -S is. So I think > > > > the > > > > same > > > > reasoning should apply to ethtool -S too, really. > > > > > > I think we all agree on the principles, once we make sure to not > > > add more pressure on RTNL. It seems you addressed our feedback, > > > all > > > is fine. > > > > > > > Eric, about two years ago you were totally against sleeping in > > ndo_get_stats, what happened ? :) > > https://lore.kernel.org/netdev/4cc44e85-cb5e-502c-30f3-c6ea564fe...@gmail.com/ > > I believe that what is different this time is that DSA switches are > typically connected over a slow and bottlenecked bus (so periodic > driver-level readouts would only make things worse for phc2sys and > such other latency-sensitive programs), plus they are offloading > interfaces for forwarding (so software-based counters could never be > accurate). Support those, and supporting firmware-based high-speed > devices will come as a nice side-effect. > FWIW that discussion took place here: > https://patchwork.ozlabs.org/project/netdev/patch/20201125193740.36825-3-george.mccollis...@gmail.com/ > I understand the motivation and I agree with the concept, hence my patchset :) > > My approach to solve this was much simpler and didn't require a > > new > > mutex nor RTNL lock, all i did is to reduce the rcu critical > > section to > > not include the call to the driver by simply holding the netdev via > > dev_hold() > > I feel this is a call for the bonding maintainers to make. If they're > willing to replace rtnl_dereference with bond_dereference throughout > the > whole driver, and reduce other guys' amount of work when other NDOs > start losing the rtnl_mutex too, then I can't see what's wrong with > my > approach (despite not being "as simple"). If they think that update- > side > protection of the slaves array is just fine the way it is, then I > suppose that RCU protection + dev_hold is indeed all that I can do. To be honest i haven't really looked at your patches, I just quickly went through them to get an idea of what you did, but let me take a more careful look and will give my ultimate feedback.
