Re: [PATCH v1 net-next] net: lan743x: configure interrupt moderation timers based on speed

[Andrew Lunn <andrew@lunn.ch>]

On Tue, May 06, 2025 at 04:02:30AM +0000, Thangaraj.S@microchip.com wrote:
> Hi Andrew,
> Thanks for reviewing the patch,
> 
> On Mon, 2025-05-05 at 14:15 +0200, Andrew Lunn wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> > 
> > On Mon, May 05, 2025 at 12:59:43PM +0530, Thangaraj Samynathan wrote:
> > > Configures the interrupt moderation timer value to 64us for 2.5G,
> > > 150us for 1G, 330us for 10/100M. Earlier this was 400us for all
> > > speeds. This improvess UDP TX and Bidirectional performance to
> > > 2.3Gbps from 1.4Gbps in 2.5G. These values are derived after
> > > experimenting with different values.
> > 
> > It would be good to also implement:
> > 
> >        ethtool -c|--show-coalesce devname
> > 
> >        ethtool -C|--coalesce devname [adaptive-rx on|off] [adaptive-
> > tx on|off]
> >               [rx-usecs N] [rx-frames N] [rx-usecs-irq N] [rx-frames-
> > irq N]
> >               [tx-usecs N] [tx-frames N] [tx-usecs-irq N] [tx-frames-
> > irq N]
> >               [stats-block-usecs N] [pkt-rate-low N] [rx-usecs-low N]
> >               [rx-frames-low N] [tx-usecs-low N] [tx-frames-low N]
> >               [pkt-rate-high N] [rx-usecs-high N] [rx-frames-high N]
> >               [tx-usecs-high N] [tx-frames-high N] [sample-interval
> > N]
> >               [cqe-mode-rx on|off] [cqe-mode-tx on|off] [tx-aggr-max-
> > bytes N]
> >               [tx-aggr-max-frames N] [tx-aggr-time-usecs N]
> > 
> > so the user can configure it. Sometimes lower power is more important
> > than high speed.
> > 
> >         Andrew
> 
> We've tuned the interrupt moderation values based on testing to improve
> performance. For now, we’ll keep these fixed values optimized for
> performance across all speeds. That said, we agree that adding ethtool
> -c/-C support would provide valuable flexibility for users to balance
> power and performance, and we’ll consider implementing that in a future
> update.

As you said, you have optimised for performance. That might cause
regressions for some users. We try to avoid regressions, and if
somebody does report a regression, we will have to revert this change.
If you were to implement this ethtool option, we are a lot less likely
to make a revert, we can instruct the user how to set the coalesce for
there use case.

	Andrew