On Wed, Dec 08, 2021 at 01:20:20AM +0200, Vladimir Oltean wrote:

On Wed, Dec 08, 2021 at 12:05:11AM +0100, Ansuel Smith wrote:

quoted

Hm. Interesting idea. So qca8k would provide the way to parse the packet
and made the request. The tagger would just detect the packet and
execute the dedicated function.
About mib considering the driver autocast counter for every port and
every packet have the relevant port to it (set in the qca tag), the
idea was to put a big array and directly write the data. The ethtool
function will then just read the data and report it. (or even work
directly on the ethtool data array).

Apart from the fact that you'd be running inside the priv->rw_reg_ack_handler()
which runs in softirq context (so you need spinlocks to serialize with
the code that runs in process and/or workqueue context), you have access
to all the data structures from the switch driver that you're used to.
So you could copy from the void *buf into something owned by struct
qca8k_priv *priv, sure.

quoted

quoted

  My current idea is maybe not ideal and a bit fuzzy, because the switch
  driver would need to be aware of the fact that the tagger private data
  is in dp->priv, and some code in one folder needs to be in sync with
  some code in another folder. But at least it should be safer this way,
  because we are in more control over the exact connection that's being
  made.

- to avoid leaking memory, we also need to patch dsa_tree_put() to issue
  a disconnect event on unbind.

- the tagging protocol driver would always need to NULL-check the
  function pointer before dereferencing it, because it may connect to a
  switch driver that doesn't set them up (dsa_loop):

	struct qca8k_tagger_private *priv = dp->priv;

	if (priv->rw_reg_ack_handler)
		priv->rw_reg_ack_handler(dp, skb_mac_header(skb));

Ok so your idea is to make the driver the one controlling ""everything""
and keep the tagger as dummy as possible. That would also remove all the
need to put stuff in the global include dir. Looks complex but handy. We
still need to understand the state part. Any hint about that?

In the mean time I will try implement this.

What do you mean exactly by understanding the state?

I was referring to the "shared state" problem but you already answer
that in the prev email.

-- 
	Ansuel