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: <20250314111801.2oela3qoi5qrl6el@skbuf>
Date: Fri, 14 Mar 2025 13:18:01 +0200
From: Vladimir Oltean <vladimir.oltean@....com>
To: Wei Fang <wei.fang@....com>
Cc: Claudiu Manoil <claudiu.manoil@....com>,
	Clark Wang <xiaoning.wang@....com>,
	"andrew+netdev@...n.ch" <andrew+netdev@...n.ch>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"edumazet@...gle.com" <edumazet@...gle.com>,
	"kuba@...nel.org" <kuba@...nel.org>,
	"pabeni@...hat.com" <pabeni@...hat.com>,
	"christophe.leroy@...roup.eu" <christophe.leroy@...roup.eu>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"imx@...ts.linux.dev" <imx@...ts.linux.dev>,
	"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v4 net-next 02/14] net: enetc: add command BD ring
 support for i.MX95 ENETC

On Fri, Mar 14, 2025 at 06:51:06AM +0200, Wei Fang wrote:
> > I don't understand the need for si->ops->setup_cbdr() and
> > si->ops->teardown_cbdr()?
> > Doesn't every call site know which kind of SI it is dealing with, and thus it can
> > appropriately call the direct symbol?
> > - the v1 PSI and the VSI call enetc_setup_cbdr() and enetc_teardown_cbdr()
> > - the v4 PSI calls enetc4_setup_cbdr() and enetc4_teardown_cbdr()
> 
> Yes, for PSI we can use directly call these functions because they are different
> drivers, but for VSI, v1 and v4 will use the same driver. I just want the PSI and
> VSI to be consistent. If you don't like this, I can remove these interfaces from
> the patch set, and add vf_setup_cbdr and vf_teardown_cbdr in the future when
> I add the VF support for ENETC v4.

It's not that I don't like them, the point is rather simple: as far as
this patch set is concerned, converting direct function calls to
indirect ones is an unfinished idea. It needs to be evaluated in full
context, which is not present here - as you say, v4 VSIs need to be
further modified to call a different set of operations - but right now,
they call a single set of CBDR functions. Changes which require
subsequent patch sets in order to make any sense at all are discouraged.

Given the fact that the PSI code paths still don't benefit from an
indirect function call in any way, I would in principle recommend to
keep calling their CBDR methods directly. For VSIs I don't know which is
preferable (if-else vs function pointer), I need to see that code first.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ