Messages in this thread | | | Date | Wed, 8 Dec 2021 04:39:46 +0100 | From | Ansuel Smith <> | Subject | Re: [net-next RFC PATCH 0/6] Add support for qca8k mdio rw in Ethernet packet |
| |
On Wed, Dec 08, 2021 at 02:35:34AM +0100, Andrew Lunn wrote: > 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
I'm sending v2 with Vladimir suggestion so we can start working on that. Hope with a some split code it would be easier to find the problem with this and find a way to correctly validate the shared data between tagger and dsa driver. (you will probably have to rewrite this also for v2 and sorry for this)
-- Ansuel
| |