linux-kernel - Re: [PATCH net-next 3/5] net: ipqess: introduce the Qualcomm IPQESS driver

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]

Date:   Tue, 24 Oct 2023 11:16:27 +0200 (CEST)
From:   Romain Gantois <romain.gantois@...tlin.com>
To:     Andrew Lunn <andrew@...n.ch>
cc:     Romain Gantois <romain.gantois@...tlin.com>, davem@...emloft.net,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Jakub Kicinski <kuba@...nel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        thomas.petazzoni@...tlin.com,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        linux-arm-kernel@...ts.infradead.org,
        Vladimir Oltean <vladimir.oltean@....com>,
        Luka Perkov <luka.perkov@...tura.hr>,
        Robert Marko <robert.marko@...tura.hr>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Maxime Chevallier <maxime.chevallier@...tlin.com>
Subject: Re: [PATCH net-next 3/5] net: ipqess: introduce the Qualcomm IPQESS
 driver

Hello Andrew,

On Mon, 23 Oct 2023, Andrew Lunn wrote:

[...]
> > +	struct qca8k_priv *priv = port->sw->priv;
> > +	struct ipqess_port_dump_ctx dump = {
> > +		.dev = dev,
> > +		.skb = skb,
> > +		.cb = cb,
> > +		.idx = *idx,
> > +	};
> 
> And with a little bit of refactoring, you should be able to use the
> core of qca8k_port_fdb_dump(). All that seems to differ is how you get
> to the struct qca8k_priv *priv.
> 
> That then makes me wounder if there is more code here which could be
> removed with a little refactoring of the DSA driver?

Yes, I think this should be possible for a few more functions, I'll look into it 
for the v2.

> > +static int ipqess_port_get_eeprom_len(struct net_device *dev)
> > +{
> > +	return 0;
> > +}
> 
> Is this actually useful? What does it default to if not provided? 42?

It's not, I'll remove it.

> 
> > +	for (c = 0; c < priv->info->mib_count; c++) {
> > +		mib = &ar8327_mib[c];
> > +		reg = QCA8K_PORT_MIB_COUNTER(port->index) + mib->offset;
> > +
> > +		ret = qca8k_read(priv, reg, &val);
> > +		if (ret < 0)
> > +			continue;
> 
> Given the switch is built in, is this fast? The 8k driver avoids doing
> register reads for this.

Sorry, I don't quite understand what you mean. Are you referring to the existing 
QCA8k DSA driver? From what I've seen, it calls qca8k_get_ethtool_stats defined 
in qca8k-common.c and this uses the same register read.

Thanks,

Romain