Messages in this thread | | | Date | Wed, 8 Dec 2021 15:33:27 +0100 | From | Ansuel Smith <> | Subject | Re: [net-next RFC PATCH v2 0/8] Add support for qca8k mdio rw in Ethernet packet |
| |
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: > > 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
| |