All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleksij Rempel <o.rempel@pengutronix.de>
To: Lukas Wunner <lukas@wunner.de>
Cc: Andrew Lunn <andrew@lunn.ch>, Oliver Neukum <oneukum@suse.com>,
	Oleksij Rempel <linux@rempel-privat.de>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Heiner Kallweit <hkallweit1@gmail.com>
Subject: Re: ordering of call to unbind() in usbnet_disconnect
Date: Sun, 27 Mar 2022 10:37:02 +0200	[thread overview]
Message-ID: <20220327083702.GC27264@pengutronix.de> (raw)
In-Reply-To: <20220326130430.GD31022@wunner.de>

On Sat, Mar 26, 2022 at 02:04:30PM +0100, Lukas Wunner wrote:
> On Sat, Mar 26, 2022 at 01:49:28PM +0100, Andrew Lunn wrote:
> > On Sat, Mar 26, 2022 at 01:39:29PM +0100, Lukas Wunner wrote:
> > > On Mon, Mar 21, 2022 at 02:10:27PM +0100, Andrew Lunn wrote:
> > > > There are two patterns in use at the moment:
> > > > 
> > > > 1) The phy is attached in open() and detached in close(). There is no
> > > >    danger of the netdev disappearing at this time.
> > > > 
> > > > 2) The PHY is attached during probe, and detached during release.
> > > > 
> > > > This second case is what is being used here in the USB code. This is
> > > > also a common pattern for complex devices. In probe, you get all the
> > > > components of a complex devices, stitch them together and then
> > > > register the composite device. During release, you unregister the
> > > > composite device, and then release all the components. Since this is a
> > > > natural model, i think it should work.
> > > 
> > > I've gone through all drivers and noticed that some of them use a variation
> > > of pattern 2 which looks fishy:
> > > 
> > > On probe, they first attach the PHY, then register the netdev.
> > > On remove, they detach the PHY, then unregister the netdev.
> > > 
> > > Is it legal to detach the PHY from a registered (potentially running)
> > > netdev? It looks wrong to me.
> > 
> > I think the network stack guarantee that the close() method is called
> > before unregister completes. It is a common pattern to attach the PHY
> > in open() and detach it in close(). The stack itself should not be
> > using the PHY when it is down, the exception being IOCTL handlers
> > which people often get wrong.
> 
> But the PHY is detached from a *running* netdev *before* that netdev
> is unregistered (and closed).  Is that really legal?

IMO, it reflects, more or less, the reality of devices with SFP modules.
The PHY can be physically removed from running netdev. At same time,
netdev should be registered and visible for the user, even if PHY is not
physically attached.

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

  reply	other threads:[~2022-03-27  8:37 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-10 11:25 ordering of call to unbind() in usbnet_disconnect Oliver Neukum
2022-03-10 11:38 ` Oleksij Rempel
2022-03-14 18:42   ` Lukas Wunner
2022-03-14 19:14     ` Andrew Lunn
2022-03-15  5:44       ` Oleksij Rempel
2022-03-15  8:32         ` Lukas Wunner
2022-03-15 11:38           ` Oleksij Rempel
2022-03-15 13:28             ` Andrew Lunn
2022-03-17 15:53               ` Oliver Neukum
2022-03-17 21:03                 ` Lukas Wunner
2022-03-21 10:17                 ` Lukas Wunner
2022-03-21 10:43                   ` Oleksij Rempel
2022-03-31  9:35                   ` Oliver Neukum
2022-03-21 10:02               ` Lukas Wunner
2022-03-21 13:10                 ` Andrew Lunn
2022-03-26 12:39                   ` Lukas Wunner
2022-03-26 12:49                     ` Andrew Lunn
2022-03-26 13:04                       ` Lukas Wunner
2022-03-27  8:37                         ` Oleksij Rempel [this message]
2022-03-31  9:20                           ` Oliver Neukum
2022-03-31  9:30                             ` Lukas Wunner
2022-03-31  9:59                               ` Oliver Neukum
2022-03-31 11:22                                 ` Lukas Wunner
2022-03-26 12:25             ` Lukas Wunner
2022-03-26 12:44               ` Andrew Lunn
2022-03-26 13:01                 ` Lukas Wunner

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=20220327083702.GC27264@pengutronix.de \
    --to=o.rempel@pengutronix.de \
    --cc=andrew@lunn.ch \
    --cc=hkallweit1@gmail.com \
    --cc=linux@rempel-privat.de \
    --cc=lukas@wunner.de \
    --cc=netdev@vger.kernel.org \
    --cc=oneukum@suse.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.