netdev - Re: [net-next PATCH 13/15] eth: fbnic: add basic Rx handling

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: <008a9e73-16a4-4d45-9559-0df7a08e9855@intel.com>
Date: Tue, 16 Apr 2024 16:05:41 +0200
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Alexander Duyck <alexander.duyck@...il.com>
CC: Jakub Kicinski <kuba@...nel.org>, Yunsheng Lin <linyunsheng@...wei.com>,
	<netdev@...r.kernel.org>, Alexander Duyck <alexanderduyck@...com>,
	<davem@...emloft.net>, <pabeni@...hat.com>
Subject: Re: [net-next PATCH 13/15] eth: fbnic: add basic Rx handling

From: Alexander Duyck <alexander.duyck@...il.com>
Date: Mon, 15 Apr 2024 11:03:13 -0700

> On Mon, Apr 15, 2024 at 10:11 AM Jakub Kicinski <kuba@...nel.org> wrote:
>>
>> On Mon, 15 Apr 2024 08:03:38 -0700 Alexander Duyck wrote:
>>>>> The advantage of being a purpose built driver is that we aren't
>>>>> running on any architectures where the PAGE_SIZE > 4K. If it came to
>>>>
>>>> I am not sure if 'being a purpose built driver' argument is strong enough
>>>> here, at least the Kconfig does not seems to be suggesting it is a purpose
>>>> built driver, perhaps add a 'depend on' to suggest that?
>>>
>>> I'm not sure if you have been following the other threads. One of the
>>> general thoughts of pushback against this driver was that Meta is
>>> currently the only company that will have possession of this NIC. As
>>> such Meta will be deciding what systems it goes into and as a result
>>> of that we aren't likely to be running it on systems with 64K pages.
>>
>> Didn't take long for this argument to float to the surface..
> 
> This wasn't my full argument. You truncated the part where I
> specifically called out that it is hard to justify us pushing a
> proprietary API that is only used by our driver.
> 
>> We tried to write some rules with Paolo but haven't published them, yet.
>> Here is one that may be relevant:
>>
>>   3. External contributions
>>   -------------------------
>>
>>   Owners of drivers for private devices must not exhibit a stronger
>>   sense of ownership or push back on accepting code changes from
>>   members of the community. 3rd party contributions should be evaluated
>>   and eventually accepted, or challenged only on technical arguments
>>   based on the code itself. In particular, the argument that the owner
>>   is the only user and therefore knows best should not be used.
>>
>> Not exactly a contribution, but we predicted the "we know best"
>> tone of the argument :(
> 
> The "we know best" is more of an "I know best" as someone who has
> worked with page pool and the page fragment API since well before it
> existed. My push back is based on the fact that we don't want to

I still strongly believe Jesper-style arguments like "I've been working
with this for aeons", "I invented the Internet", "I was born 3 decades
before this API was introduced" are not valid arguments.

> allocate fragments, we want to allocate pages and fragment them
> ourselves after the fact. As such it doesn't make much sense to add an
> API that will have us trying to use the page fragment API which holds
> onto the page when the expectation is that we will take the whole
> thing and just fragment it ourselves.

[...]

Re "this HW works only on x86, why bother" -- I still believe there
shouldn't be any hardcodes in any driver based on the fact that the HW
is deployed only on particular systems. Page sizes, Endianness,
32/64-bit... It's not difficult to make a driver look like it's
universal and could work anywhere, really.

Thanks,
Olek