Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: "Russell King (Oracle)" <linux@armlinux.org.uk>,
	Vladimir Oltean <vladimir.oltean@nxp.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH net-next 0/3] Make the PHY library stop being so greedy when binding the generic PHY driver
Date: Fri, 3 Sep 2021 19:21:19 +0200	[thread overview]
Message-ID: <YTJZj/Js+nmDTG0y@lunn.ch> (raw)
In-Reply-To: <20210903162253.5utsa45zy6h4v76t@skbuf>

On Fri, Sep 03, 2021 at 07:22:53PM +0300, Vladimir Oltean wrote:
> [ trimming the CC list, I'm sure most people don't care, if they do,
>   they can watch the mailing list ]
> 
> On Thu, Sep 02, 2021 at 09:29:05PM +0100, Russell King (Oracle) wrote:
> > On Thu, Sep 02, 2021 at 11:21:24PM +0300, Vladimir Oltean wrote:
> > > On Thu, Sep 02, 2021 at 09:03:01PM +0100, Russell King (Oracle) wrote:
> > > > # systemctl list-dependencies networking.service
> > > > networking.service
> > > >   ├─ifupdown-pre.service
> > > >   ├─system.slice
> > > >   └─network.target
> > > > # systemctl list-dependencies ifupdown-pre.service
> > > > ifupdown-pre.service
> > > >   ├─system.slice
> > > >   └─systemd-udevd.service
> > > > 
> > > > Looking in the service files for a better idea:
> > > > 
> > > > networking.service:
> > > > Requires=ifupdown-pre.service
> > > > Wants=network.target
> > > > After=local-fs.target network-pre.target apparmor.service systemd-sysctl.service systemd-modules-load.service ifupdown-pre.service
> > > > Before=network.target shutdown.target network-online.target
> > > > 
> > > > ifupdown-pre.service:
> > > > Wants=systemd-udevd.service
> > > > After=systemd-udev-trigger.service
> > > > Before=network.target
> > > > 
> > > > So, the dependency you mention is already present. As is a dependency
> > > > on udev. The problem is udev does all the automatic module loading
> > > > asynchronously and in a multithreaded way.
> > > > 
> > > > I don't think there's a way to make systemd wait for all module loads
> > > > to complete.
> > > 
> > > So ifupdown-pre.service has a call to "udevadm settle". This "watches
> > > the udev event queue, and exits if all current events are handled",
> > > according to the man page. But which current events? ifupdown-pre.service
> > > does not have the dependency on systemd-modules-load.service, just
> > > networking.service does. So maybe ifupdown-pre.service does not wait for
> > > DSA to finish initializing, then it tells networking.service that all is ok.
> > 
> > ifupdown-pre.service does have a call to udevadm settle, and that
> > does get called from what I can tell.
> > 
> > systemd-modules-load.service is an entire red herring. The only
> > module listed in the various modules-load.d directories is "tun"
> > for openvpn (which isn't currently being used.)
> > 
> > As I've already told you (and you seem to have ignored), DSA gets
> > loaded by udev, not by systemd-modules-load.service.
> > systemd-modules-load.service is irrelevant to my situation.
> > 
> > I think there's a problem with "and exits if all current events are
> > handled" - does that mean it's fired off a modprobe process which
> > is in progress, or does that mean that the modprobe process has
> > completed.
> > 
> > Given that we can see that ifup is being run while the DSA module is
> > still in the middle of probing, the latter interpretation can not be
> > true - unless systemd is ignoring the dependencies. Or just in
> > general, systemd being systemd (I have very little faith in systemd
> > behaving as it should.)
> 
> So I've set a fresh installation of Debian Buster on my Turris MOX,
> which has 3 mv88e6xxx switches, and I've put the mv88e6xxx driver inside
> the rootfs as a module to be loaded by udev based on modaliases just
> like you've said.  Additionally, the PHY driver is also a module.
> The kernel is built straight from the v5.13 tag, absolutely no changes.
> 
> Literally the only changes I've done to this system are:
> 1. install bridge-utils
> 2. create this file, it is sourced by /etc/network/interfaces:
> root@debian:~# cat /etc/network/interfaces.d/bridge
> auto br0
> iface br0 inet manual
>         bridge_ports lan1 lan2 lan3 lan4 lan5 lan6 lan7 lan8 lan9 lan10 lan11 lan12 lan13 lan14 lan15 lan16 lan17 lan18 lan19 lan20 lan21 lan22 lan23 lan24 sfp
>         bridge_maxwait 0

Hi Russell

Do you have

auto brdsl

in your /etc/network/interfaces?

Looking at /lib/udev/bridge-network-interface it seems it will only do
hotplug of interfaces if auto is set on the bridge interface. Without
auto, it only does coldplug. So late appearing switch ports won't get
added.

      Andrew

  reply	other threads:[~2021-09-03 17:21 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-01 22:50 [RFC PATCH net-next 0/3] Make the PHY library stop being so greedy when binding the generic PHY driver Vladimir Oltean
2021-09-01 22:50 ` [RFC PATCH net-next 1/3] net: phy: don't bind genphy in phy_attach_direct if the specific driver defers probe Vladimir Oltean
2021-09-02  5:43   ` Greg Kroah-Hartman
2021-09-02 10:11     ` Vladimir Oltean
2021-09-02 10:37       ` Greg Kroah-Hartman
2021-09-02 11:17         ` Vladimir Oltean
2021-09-02 14:37     ` Rafael J. Wysocki
2021-09-02 18:50   ` Russell King (Oracle)
2021-09-02 19:23     ` Vladimir Oltean
2021-09-02 19:51     ` Andrew Lunn
2021-09-02 20:33       ` Florian Fainelli
2021-09-02 21:33         ` Russell King (Oracle)
2021-09-02 21:39           ` Vladimir Oltean
2021-09-02 22:24             ` Russell King (Oracle)
2021-09-02 22:45               ` Vladimir Oltean
2021-09-02 23:02                 ` Andrew Lunn
2021-09-02 23:26                   ` Vladimir Oltean
2021-09-03  0:04                     ` Russell King (Oracle)
2021-09-03 20:48                       ` Vladimir Oltean
2021-09-03 22:06                         ` Russell King (Oracle)
2021-09-04 21:59                           ` Vladimir Oltean
2021-09-04 23:25                             ` Russell King (Oracle)
2021-09-05  0:41                               ` Vladimir Oltean
2021-09-03  9:27               ` Ioana Ciornei
2021-09-01 22:50 ` [RFC PATCH net-next 2/3] net: dsa: destroy the phylink instance on any error in dsa_slave_phy_setup Vladimir Oltean
2021-09-02 12:25   ` Russell King (Oracle)
2021-09-02 23:21   ` Florian Fainelli
2021-09-01 22:50 ` [RFC PATCH net-next 3/3] net: dsa: allow the phy_connect() call to return -EPROBE_DEFER Vladimir Oltean
2021-09-02 12:19 ` [RFC PATCH net-next 0/3] Make the PHY library stop being so greedy when binding the generic PHY driver Russell King (Oracle)
2021-09-02 12:35   ` Vladimir Oltean
2021-09-02 12:59     ` Vladimir Oltean
2021-09-02 13:26     ` Russell King (Oracle)
2021-09-02 15:23       ` Vladimir Oltean
2021-09-02 16:31         ` Russell King (Oracle)
2021-09-02 17:10           ` Vladimir Oltean
2021-09-02 17:50             ` Russell King (Oracle)
2021-09-02 19:05               ` Vladimir Oltean
2021-09-02 20:03                 ` Russell King (Oracle)
2021-09-02 20:21                   ` Vladimir Oltean
2021-09-02 20:29                     ` Russell King (Oracle)
2021-09-03 16:22                       ` Vladimir Oltean
2021-09-03 17:21                         ` Andrew Lunn [this message]
2021-09-03 18:58                           ` Russell King (Oracle)
2021-09-03 19:56                             ` Andrew Lunn
2021-09-03 20:08                               ` Russell King (Oracle)
2021-09-03 18:54                         ` Russell King (Oracle)
2021-09-03 20:11                           ` Vladimir Oltean
2021-09-02 20:07     ` Andrew Lunn
2021-09-02 20:32       ` Vladimir Oltean
2021-09-02 21:39         ` Russell King (Oracle)
2021-09-02 22:05 ` Vladimir Oltean
2021-09-02 23:29   ` Saravana Kannan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YTJZj/Js+nmDTG0y@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=f.fainelli@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=vivien.didelot@gmail.com \
    --cc=vladimir.oltean@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).