Date:   Wed, 8 Dec 2021 15:33:27 +0100
From:   Ansuel Smith <ansuelsmth@...il.com>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org
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

Powered by blists - more mailing lists