Re: [patch net-next] virtio_net: add support for Byte Queue Limits
On Fri, Jun 07, 2024 at 01:30:34PM +0200, Jiri Pirko wrote: > Fri, Jun 07, 2024 at 12:23:37PM CEST, m...@redhat.com wrote: > >On Fri, Jun 07, 2024 at 11:57:37AM +0200, Jiri Pirko wrote: > >> >True. Personally, I would like to just drop orphan mode. But I'm not > >> >sure others are happy with this. > >> > >> How about to do it other way around. I will take a stab at sending patch > >> removing it. If anyone is against and has solid data to prove orphan > >> mode is needed, let them provide those. > > > >Break it with no warning and see if anyone complains? > > This is now what I suggested at all. > > >No, this is not how we handle userspace compatibility, normally. > > Sure. > > Again: > > I would send orphan removal patch containing: > 1) no module options removal. Warn if someone sets it up > 2) module option to disable napi is ignored > 3) orphan mode is removed from code > > There is no breakage. Only, hypotetically performance downgrade in some > hypotetical usecase nobody knows of. Performance is why people use virtio. It's as much a breakage as any other bug. The main difference is, with other types of breakage, they are typically binary and we can not tolerate them at all. A tiny, negligeable performance regression might be tolarable if it brings other benefits. I very much doubt avoiding interrupts is negligeable though. And making code simpler isn't a big benefit, users do not care. > My point was, if someone presents > solid data to prove orphan is needed during the patch review, let's toss > out the patch. > > Makes sense? It's not hypothetical - if anything, it's hypothetical that performance does not regress. And we just got a report from users that see a regression without. So, not really. > > > > >-- > >MST > >
Re: [PATCH net-next] virtio_net: Add TX stop and wake counters
On Sat, 8 Jun 2024 08:41:44 +0800 Jason Xing wrote: > > > Sorry to revive this thread. I wonder why not use this patch like mlnx > > > driver > > > does instead of adding statistics into the yaml file? Are we gradually > > > using or > > > adding more fields into the yaml file to replace the 'ethtool -S' command? > > > > > > > It's trivial to have the stats in ethtool as well. But I noticed > > the stats series intentionally removed some stats from ethtool. So > > I didn't put it both places. > > Thank you for the reply. I thought there was some particular reason > :-) Yes, we don't want duplication. We have a long standing (and documented) policy against duplicating normal stats in custom stat APIs, otherwise vendors pile everything into the custom stats.
Re: [PATCH net-next] virtio_net: Add TX stop and wake counters
On Tue, Jun 11, 2024 at 1:57 AM Jakub Kicinski wrote: > > On Sat, 8 Jun 2024 08:41:44 +0800 Jason Xing wrote: > > > > Sorry to revive this thread. I wonder why not use this patch like mlnx > > > > driver > > > > does instead of adding statistics into the yaml file? Are we gradually > > > > using or > > > > adding more fields into the yaml file to replace the 'ethtool -S' > > > > command? > > > > > > > > > > It's trivial to have the stats in ethtool as well. But I noticed > > > the stats series intentionally removed some stats from ethtool. So > > > I didn't put it both places. > > > > Thank you for the reply. I thought there was some particular reason > > :-) > > Yes, we don't want duplication. We have a long standing (and > documented) policy against duplicating normal stats in custom stat > APIs, otherwise vendors pile everything into the custom stats. Thanks, Jakub. I see :)
