linux-kernel - Re: [PATCH net-next v1 1/7] net: stmmac: xgmac: drop incomplete FPE implementation

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: <20240709171018.7tifdirqjhq6cohy@skbuf>
Date: Tue, 9 Jul 2024 20:10:18 +0300
From: Vladimir Oltean <olteanv@...il.com>
To: Andrew Lunn <andrew@...n.ch>, Furong Xu <0x1207@...il.com>
Cc: Jianheng Zhang <Jianheng.Zhang@...opsys.com>,
	"David S. Miller" <davem@...emloft.net>,
	Alexandre Torgue <alexandre.torgue@...s.st.com>,
	Jose Abreu <joabreu@...opsys.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Maxime Coquelin <mcoquelin.stm32@...il.com>,
	Joao Pinto <jpinto@...opsys.com>, netdev@...r.kernel.org,
	linux-stm32@...md-mailman.stormreply.com,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	xfr@...look.com, rock.xu@....com
Subject: Re: [PATCH net-next v1 1/7] net: stmmac: xgmac: drop incomplete FPE
 implementation

Hi Andrew, Furong,

On Tue, Jul 09, 2024 at 03:16:35PM +0200, Andrew Lunn wrote:
> On Tue, Jul 09, 2024 at 04:21:19PM +0800, Furong Xu wrote:
> > The FPE support for xgmac is incomplete, drop it temporarily.
> > Once FPE implementation is refactored, xgmac support will be added.
> 
> This is a pretty unusual thing to do. What does the current
> implementation do? Is there enough for it to actually work? If i was
> doing a git bisect and landed on this patch, could i find my
> networking is broken?
> 
> More normal is to build a new implementation by the side, and then
> swap to it.
> 
> 	Andrew
> 

There were 2 earlier attempts from Jianheng Zhang @ Synopsys to add FPE
support to new hardware.

I told him that the #1 priority should be to move the stmmac driver over
to the new standard API which uses ethtool + tc.
https://lore.kernel.org/netdev/CY5PR12MB63726FED738099761A9B81E7BF8FA@CY5PR12MB6372.namprd12.prod.outlook.com/
https://lore.kernel.org/netdev/CY5PR12MB63727C24923AE855CFF0D425BF8EA@CY5PR12MB6372.namprd12.prod.outlook.com/

I'm not sure what happened in the meantime. Jianheng must have faced
some issue, because he never came back.

I did comment this at the time:

| Even this very patch is slightly strange - it is not brand new hardware
| support, but it fills in some more FPE ops in dwxlgmac2_ops - when
| dwxgmac3_fpe_configure() was already there. So this suggests the
| existing support was incomplete. How complete is it now? No way to tell.
| There is a selftest to tell, but we can't run it because the driver
| doesn't integrate with those kernel APIs.

So it is relatively known that the support is incomplete. But I still
think we should push for more reviewer insight into this driver by
having access to a selftest to get a clearer picture of how it behaves.
For that, we need the compliance to the common API.

Otherwise, I completely agree to the idea that any development should be
done incrementally on top of whatever already exists, instead of putting
a curtain on and then taking it back off.