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:

quoted

On Tue, Dec 07, 2021 at 11:54:07PM +0100, Andrew Lunn wrote:

quoted

quoted

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.

quoted

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