linux-kernel - Re: [net-next RFC PATCH 0/6] Add support for qca8k mdio rw in Ethernet packet

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:   Wed, 8 Dec 2021 02:15:09 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     Ansuel Smith <ansuelsmth@...il.com>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [net-next RFC PATCH 0/6] Add support for qca8k mdio rw in
 Ethernet packet

On Wed, Dec 08, 2021 at 01:47:36AM +0200, Vladimir Oltean wrote:
> On Tue, Dec 07, 2021 at 11:22:41PM +0100, Andrew Lunn wrote:
> > > I like the idea of tagger-owend per-switch-tree private data.
> > > Do we really need to hook logic?
> > 
> > We have two different things here.
> > 
> > 1) The tagger needs somewhere to store its own private data.
> > 2) The tagger needs to share state with the switch driver.
> > 
> > We can probably have the DSA core provide 1). Add the size to
> > dsa_device_ops structure, and provide helpers to go from either a
> > master or a slave netdev to the private data.
> 
> We cannot "add the size to the dsa_device_ops structure", because it is
> a singleton (const struct). It is not replicated at all, not per port,
> not per switch, not per tree, but global to the kernel. Not to mention
> const. Nobody needs state as shared as that.

What i'm suggesting is 

static const struct dsa_device_ops edsa_netdev_ops = {
        .name     = "edsa",
        .proto    = DSA_TAG_PROTO_EDSA,
        .xmit     = edsa_xmit,
        .rcv      = edsa_rcv,
        .needed_headroom = EDSA_HLEN,
	.priv_size = 42;
};

The priv_size indicates that an instance of this tagger needs 42 bytes
of private data. More likely it will be a sizeof(struct dsa_priv), but
that is a detail.

When a master is setup and the tagger instantiated, 42 bytes of memory
will be allocated and put somewhere it can be found via a helper.

This is not meant for shared state between the tagger and the switch
driver, this is private to the tagger. As such it is less likely to be
dependent on the number of ports etc. It is somewhere to store an skb
pointer, maybe a sequence number for the management message expected
as a reply from the switch etc.

   Andrew