netdev - Re: [net-next RFC PATCH 3/4] net: dsa: qca8k: rework mib autocast handling

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:   Mon, 18 Jul 2022 19:20:07 +0200
From:   Christian Marangi <ansuelsmth@...il.com>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Russell King <linux@...linux.org.uk>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jens Axboe <axboe@...nel.dk>, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [net-next RFC PATCH 3/4] net: dsa: qca8k: rework mib autocast
 handling

On Mon, Jul 18, 2022 at 08:27:12PM +0300, Vladimir Oltean wrote:
> On Sat, Jul 16, 2022 at 07:49:57PM +0200, Christian Marangi wrote:
> > In preparation for code split, move the autocast mib function used to
> > receive mib data from eth packet in priv struct and use that in
> > get_ethtool_stats instead of referencing the function directly. This is
> > needed as the get_ethtool_stats function will be moved to a common file.
> > 
> > Signed-off-by: Christian Marangi <ansuelsmth@...il.com>
> > ---
> 
> Can this change be deferred until there actually appears a second
> implementation of (*autocast_mib)?
>

Mhhh it would be problematic since I would like to move the ethtools
stats function to common code and keep the autocast_mib handler in the
qca8k specific code.

An alternative would be to keep the entire ethtool stats function in
qca8k specific code but it needs to be moved anyway.

This change is required as probably ipq4019 mmio will be faster to
access mib data than using the autocast way.

Tell me how to proceed. Think to skip this we have to leave ethtool
stats function in qca8k specific code and move it later?

> > diff --git a/drivers/net/dsa/qca/qca8k.h b/drivers/net/dsa/qca/qca8k.h
> > index 22ece14e06dc..a306638a7100 100644
> > --- a/drivers/net/dsa/qca/qca8k.h
> > +++ b/drivers/net/dsa/qca/qca8k.h
> > @@ -403,6 +403,7 @@ struct qca8k_priv {
> >  	struct qca8k_mdio_cache mdio_cache;
> >  	struct qca8k_pcs pcs_port_0;
> >  	struct qca8k_pcs pcs_port_6;
> > +	int (*autocast_mib)(struct dsa_switch *ds, int port, u64 *data);
> 
> Typically we hold function pointers in separate read-only structures rather
> than in the stateful private structure of the driver, see struct sja1105_info,
> struct felix_info, struct mv88e6xxx_info and mv88e6xxx_ops, struct b53_io_ops,
> etc etc.
> 

Oh ok it's just match data. We should already have something like that
in qca8k but I wasn't aware of the _info suffix. If we decide to keep
this, can i allign the match struct we use in qca8k to the new pattern
and add the function pointer there?

> >  };
> >  
> >  struct qca8k_mib_desc {
> > -- 
> > 2.36.1
> > 
> 

-- 
	Ansuel