[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YbAL5pP6IrN1ey5e@lunn.ch>
Date: Wed, 8 Dec 2021 02:35:34 +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:14:49AM +0200, Vladimir Oltean wrote:
> On Tue, Dec 07, 2021 at 11:54:07PM +0100, Andrew Lunn wrote:
> > > I considered a simplified form like this, but I think the tagger private
> > > data will still stay in dp->priv, only its ownership will change.
> >
> > Isn't dp a port structure. So there is one per port?
>
> Yes, but dp->priv is a pointer. The thing it points to may not
> necessarily be per port.
>
> > This is where i think we need to separate shared state from tagger
> > private data. Probably tagger private data is not per port. Shared
> > state between the switch driver and the tagger maybe is per port?
>
> I don't know whether there's such a big difference between
> "shared state" vs "private data"?
The difference is to do with stopping the kernel exploding when the
switch driver is not using the tagger it expects.
Anything which is private to the tagger is not a problem. Only the
tagger uses it, so it cannot be wrong.
Anything which is shared between the tagger and the switch driver we
have to be careful about. We are just passing void * pointers
about. There is no type checking. If i'm correct about the 1:N
relationship, we can store shared state in the tagger. The tagger
should be O.K, because it only ever needs to deal with one format of
shared state. The switch driver needs to handle N different formats of
shared state, depending on which of the N different taggers are in
operation. Ideally, when it asks for the void * pointer for shared
information, some sort of checking is performed to ensure the void *
is what the switch driver actually expects. Maybe it needs to pass the
tag driver it thinks it is talking to, or as well as getting the void
* back, it also gets the tag enum and it verifies it actually knows
about that tag driver.
Andrew
Powered by blists - more mailing lists