netdev - 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: <20240710155714.000010cb@gmail.com>
Date: Wed, 10 Jul 2024 15:57:14 +0800
From: Furong Xu <0x1207@...il.com>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Andrew Lunn <andrew@...n.ch>, 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 Vladimir

On Tue, 9 Jul 2024 20:10:18 +0300, Vladimir Oltean <olteanv@...il.com> wrote:
> 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.
> 

After some searching and learning about your commits for FPE using the
generic framework, I think it is clear enough to me to implement the new
standard driver interface which uses ethtool + tc, and then the refactor
of low level FPE function can follow.
Thanks.