On Wed, Dec 08, 2021 at 02:32:22PM +0200, Vladimir Oltean wrote:

On Wed, Dec 08, 2021 at 04:40:32AM +0100, Ansuel Smith wrote:

quoted

I still have to find a solution to a slowdown problem and this is where
I would love to get some hint.
Currently I still didn't find a good way to understand when the tagger
starts to accept packets and because of this the initial setup is slow
as every completion timeouts. Am I missing something or is there a way
to check for this?
After the initial slowdown, as soon as the cpu port is ready and starts
to accept packet, every transaction is near instant and no completion
timeouts.

My guess is that the problem with the initial slowdown is that you try
to use the Ethernet based register access before things are set up:
before the master is up and ready, before the switch is minimally set
up, etc.

I think what this Ethernet-based register access technique needs to be
more reliable is a notification about the DSA master going up or down.
Otherwise it won't be very efficient at all, to wait for every single
Ethernet access attempt to time out before attempting a direct MDIO
access.

Yes that is the main problem. My idea would be a notification fired as
soon as the tagger starts to send/process packet. That way we should be
certain that Ethernet mdio is ready. (then use a bool to comunicate
that the tagger is ready? And a dsa driver would use that or a helper to
understand what is the correct I/O path to use? I would love to remove
all these extra check and make something more direct but I think it
would spam the dsa ops even more)

The timeout has to stay anyway to prevent any type of breakage by the
Ethernet mdio not working.

But there are some problems with offering a "master_going_up/master_going_down"
set of callbacks. Specifically, we could easily hook into the NETDEV_PRE_UP/
NETDEV_GOING_DOWN netdev notifiers and transform these into DSA switch
API calls. The goal would be for the qca8k tagger to mark the
Ethernet-based register access method as available/unavailable, and in
the regmap implementation, to use that or the other. DSA would then also
be responsible for calling "master_going_up" when the switch ports and
master are sufficiently initialized that traffic should be possible.
But that first "master_going_up" notification is in fact the most
problematic one, because we may not receive a NETDEV_PRE_UP event,
because the DSA master may already be up when we probe our switch tree.
This would be a bit finicky to get right. We may, for instance, hold
rtnl_lock for the entirety of dsa_tree_setup_master(). This will block
potentially concurrent netdevice notifiers handled by dsa_slave_nb.
And while holding rtnl_lock() and immediately after each dsa_master_setup(),
we may check whether master->flags & IFF_UP is true, and if it is,
synthesize a call to ds->ops->master_going_up(). We also need to do the
reverse in dsa_tree_teardown_master().

Should we care about holding the lock for that much time? Will do some
test hoping the IFF_UP is sufficient to make the Ethernet mdio work.

-- 
	Ansuel