On 6/19/24 11:01, Przemek Kitszel wrote: > On 6/19/24 09:16, Omer Shpigelman wrote: >> On 6/18/24 17:19, Andrew Lunn wrote: > > [...] > >>>>>> +module_param(poll_enable, bool, 0444); >>>>>> +MODULE_PARM_DESC(poll_enable, >>>>>> + "Enable Rx polling rather than IRQ + NAPI (0 = no, 1 = yes, default: no)"); >>>>> >>>>> Module parameters are not liked. This probably needs to go away. >>>>> >>>> >>>> I see that various vendors under net/ethernet/* use module parameters. >>>> Can't we add another one? >>> >>> Look at the history of those module parameters. Do you see many added >>> in the last year? 5 years? >>> >> >> I didn't check that prior to my submit. Regarding this "no new module >> parameters allowed" rule, is that documented anywhere? if not, is that the >> common practice? not to try to do something that was not done recently? >> how "recently" is defined? >> I just want to clarify this because it's hard to handle these submissions >> when we write some code based on existing examples but then we are >> rejected because "we don't do that here anymore". >> I want to avoid future cases of this mismatch. >> > > best way is to read netdev ML, that way you will learn what interfaces > are frowned upon and which are outright banned, sometimes you could > judge yourself knowing which interfaces are most developed recently > > in this module params example - they were introduced to allow init phase > configuration of the device, that could not be postponed, what in the > general case sounds like a workaround; hardest cases include huge swaths > of (physical continuous) memory to be allocated, but for that there are > now device tree binding solutions; more typical cases for networking are > resolved via devlink reload > > devlink parms are also the thing that should be used as a default for > new parameters, the best if given parameter is not driver specific quirk > > poll_enable sounds like something that should be a common param, > but you have to better describe what you mean there > (see napi_poll(), "Enable Rx polling" would mean to use that as default, > do you mean busy polling or what?) Yes, busy polling. But never mind, I was informed that NAPI must be used so apparently I'll need to anyway remove this polling mode and its module parameter.