linux-kernel - Re: [PATCH 09/15] net: hbl_en: add habanalabs Ethernet driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <280848c5-6721-47f4-8b39-2ecd4851da4f@habana.ai>
Date: Wed, 19 Jun 2024 12:15:58 +0000
From: Omer Shpigelman <oshpigelman@...ana.ai>
To: Przemek Kitszel <przemyslaw.kitszel@...el.com>,
        Andrew Lunn
	<andrew@...n.ch>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "ogabbay@...nel.org" <ogabbay@...nel.org>,
        Zvika Yehudai <zyehudai@...ana.ai>
Subject: Re: [PATCH 09/15] net: hbl_en: add habanalabs Ethernet driver

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.