netdev - Re: [PATCH net-next v2 02/12] iavf: kill "legacy-rx" for good

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: <b169b20e-f782-6723-9697-215258d6f3a3@intel.com>
Date: Tue, 30 May 2023 18:22:07 +0200
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Alexander H Duyck <alexander.duyck@...il.com>
CC: "David S. Miller" <davem@...emloft.net>, Eric Dumazet
	<edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
	Magnus Karlsson <magnus.karlsson@...el.com>, Michal Kubiak
	<michal.kubiak@...el.com>, Larysa Zaremba <larysa.zaremba@...el.com>, "Jesper
 Dangaard Brouer" <hawk@...nel.org>, Ilias Apalodimas
	<ilias.apalodimas@...aro.org>, Christoph Hellwig <hch@....de>, Paul Menzel
	<pmenzel@...gen.mpg.de>, <netdev@...r.kernel.org>,
	<intel-wired-lan@...ts.osuosl.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next v2 02/12] iavf: kill "legacy-rx" for good

From: Alexander H Duyck <alexander.duyck@...il.com>
Date: Tue, 30 May 2023 08:29:10 -0700

> On Thu, 2023-05-25 at 14:57 +0200, Alexander Lobakin wrote:
>> Ever since build_skb() became stable, the old way with allocating an skb
>> for storing the headers separately, which will be then copied manually,
>> was slower, less flexible and thus obsolete.

[...]

> The "legacy-rx" was never about performance. It was mostly about
> providing a fall back in the event of an unexpected behavior. Keep in
> mind that in order to enable this we are leaving the page mapped and
> syncing it multiple times. In order to enable support for this we had
> to add several new items that I had deemed to be a bit risky such as
> support for DMA pages that were synced by the driver instead of on
> map/unmap and the use of the build_skb logic.

And now pretty much every driver doesn't do map/unmap each time and
keeps its mapping for a very long time. And now Chris noticed that and
says it's not supposed to be used like that :D

> 
> My main concern was that if we ever ran into  header corruption we
> could switch this on and then the pages would only be writable by the
> device.

[...]

> Since it has been about 6 years without any issues I would say we are
> safe to remove it.
> 
> Reviewed-by: Alexander Duyck <alexanderduyck@...com>

Thanks!
Olek