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

From: Vladimir Oltean
Date: Tue Dec 07 2021 - 18:47:42 EST


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.

Given this, do you have objections to the sja1105_port->data model for
shared state?

> 2) is harder. But as far as i know, we have an 1:N setup. One switch
> driver can use N tag drivers. So we need the switch driver to be sure
> the tag driver is what it expects. We keep the shared state in the tag
> driver, so it always has valid data, but when the switch driver wants
> to get a pointer to it, it needs to pass a enum dsa_tag_protocol and
> if it does not match, the core should return -EINVAL or similar.

In my proposal, the tagger will allocate the memory from its side of the
->connect() call. So regardless of whether the switch driver side
connects or not, the memory inside dp->priv is there for the tagger to
use. The switch can access it or it can ignore it.