From owner-freebsd-net@FreeBSD.ORG Wed Jan 25 20:36:44 2012 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 5085A1065673 for ; Wed, 25 Jan 2012 20:36:44 +0000 (UTC) (envelope-from marius@alchemy.franken.de) Received: from alchemy.franken.de (alchemy.franken.de [194.94.249.214]) by mx1.freebsd.org (Postfix) with ESMTP id 80CEC8FC18 for ; Wed, 25 Jan 2012 20:36:42 +0000 (UTC) Received: from alchemy.franken.de (localhost [127.0.0.1]) by alchemy.franken.de (8.14.4/8.14.4/ALCHEMY.FRANKEN.DE) with ESMTP id q0PKGIDF017723; Wed, 25 Jan 2012 21:16:18 +0100 (CET) (envelope-from marius@alchemy.franken.de) Received: (from marius@localhost) by alchemy.franken.de (8.14.4/8.14.4/Submit) id q0PKGICs017722; Wed, 25 Jan 2012 21:16:18 +0100 (CET) (envelope-from marius) Date: Wed, 25 Jan 2012 21:16:18 +0100 From: Marius Strobl To: Stefan Bethke Message-ID: <20120125201618.GO44286@alchemy.franken.de> References: <600A8C6C-DAB4-4E22-A034-38224017166B@lassitu.de> <20111213025041.GF3705@michelle.cdnetworks.com> <45B0B859-207C-4F02-A28F-7E34B775A273@lassitu.de> <20111213185348.GA7546@michelle.cdnetworks.com> <20111214011657.GH36635@alchemy.franken.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i Cc: YongHyeon PYUN , FreeBSD Net Subject: Re: "ifconfig media off"? X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 25 Jan 2012 20:36:44 -0000 On Sat, Jan 21, 2012 at 12:58:08AM +0100, Stefan Bethke wrote: > Am 14.12.2011 um 02:16 schrieb Marius Strobl: > > > On Tue, Dec 13, 2011 at 10:53:48AM -0800, YongHyeon PYUN wrote: > >> On Tue, Dec 13, 2011 at 11:04:51AM +0100, Stefan Bethke wrote: > >>> Am 13.12.2011 um 03:50 schrieb YongHyeon PYUN: > >>> > >>>> On Tue, Dec 13, 2011 at 12:56:22AM +0100, Stefan Bethke wrote: > >>>>> I'm currently writing a driver to configure an ethernet switch chip (see TL-WR1043ND on -embedded). > >>>>> > >>>>> I noticed that there doesn't seem to be a way to power down a phy right now through the ifconfig media command. > >>>>> > >>>>> Would there be objections to extend the media subtype definitions to include an "off", "poweroff" or "down" media subtype, and add code to the relevant phy drivers to power down the phy for this media subtype? > >>>>> > >>>>> The difference between media subtype "none" and this new one would be that there will be no link, even if there is a physical connection. With media subtype "none", a 10 MBit/s half-duplex connection is established, potentially confusing the remote end about the availability of this link. On the local side, the link is down, so no packets are exchanged. > >>>>> > >>>> > >>>> I think "none" means "isolated" so should have no established link > >>>> and probably you can also power down the PHY. > >>>> I vaguely guess the PHY of switch chip does not correctly support > >>>> isolated mode so you may have wanted to power down. > >>> > >>> > >>> After looking at the code a bit more, I think the common code just doesn't set the BMCR_PDOWN (but clears it when bringing up the PHY). > >>> > >> > >> Yes, and most PHYs could be powered down when BMCR_ISO is chosen. > >> I'm not sure whether this could be applied to hardwares that > >> support multiple PHYs(i.e. internal and external transceivers) > >> though. Marius may have some opinions on this(CCed). > >> However powering down PHY with BMCR_ISO looks natural to me. > >> > >>> Index: sys/dev/mii/mii_physubr.c > >>> =================================================================== > >>> --- sys/dev/mii/mii_physubr.c (revision 228402) > >>> +++ sys/dev/mii/mii_physubr.c (working copy) > >>> @@ -58,7 +58,7 @@ > >>> */ > >>> static const struct mii_media mii_media_table[MII_NMEDIA] = { > >>> /* None */ > >>> - { BMCR_ISO, ANAR_CSMA, > >>> + { BMCR_ISO | BMCR_PDOWN, ANAR_CSMA, > >>> 0, }, > >>> > >>> /* 10baseT */ > >>> > >>> I've opened kern/163240. > >>> http://www.freebsd.org/cgi/query-pr.cgi?pr=163240 > > I'd like to revisit this. Just to reiterate my motivation for the change: I want to be able to indicate to the remote end that my station is not active. With the PHY just isolated from the MII, the link stays up and functional (and even autoneg continues to work), so the remote has no indication that it's just shouting into a void. Yes, I understand the motivation and generally agree that this should be implemented. IMO the above is just a quick-hack though and no proper solution, on the other hand I neither see a need to grown an "off" media for this. > > > I don't think powering down the PHY along with IFM_NONE especially > > in that way is a good idea for several reasons: > > - It's incomplete as not all PHY drivers use mii_phy_add_media()/ > > mii_phy_setmedia(). > > - Even for those that do IFM_NONE isn't added when the PHY driver > > sets MIIF_NOISOLATE (for some PHYs BMCR_ISO either just doesn't > > work as especially the built-in ones probably have been designed > > with only single-PHY configurations in mind or even wedges the > > chip up to the point that even a reset doesn't get it working > > again). In general though, BMCR_ISO and BMCR_PDOWN are orthogonal > > (even in IEEE 802.3-2008 as far as I can see), i.e. while BMCR_ISO > > might be broken, BMCR_PDOWN could work (actually I'd expect > > BMCR_PDOWN to be less fragile than BMCR_ISO). > > I didn't expect my suggestion to be the be-all end-all, only a quick and easy way to allow compliant PHYs to be powered down, and I'm not sure why a "complete" solution is required. I'd assume that PHYs setting MIIF_NOISOLATE have specific requirements, so it's OK to not have the power-down option available there. (Plus I don't have hardware I could test that case on). I wouldn't call it "specific requirements". The PHY drivers I've flagged with MIIF_NOISOLATE so far fall into one of two categories: a) Setting BMCR_ISO just doesn't have any effect and the PHY happily continues to pass traffic. Setting MIIF_NOISOLATE in this case is done in order to not add an non-working "none" media. b) Upon setting BMCR_ISO the hardware wedges up to a way that a power-cycle is required in order to get it into a working state again. MIIF_NOISOLATE is set here in order to protect the users from such an experience when configuring "none" (think production machines). Some PHYs affected by these quirks are integrated ones with no external MII bus on the MAC so it's actually sort of okay that you can't isolate the integrated PHY but unfortunately some are also stand-alone PHYs. Based on that I expect the same issues with power down as seen with isolation with the two quirk sets not necessarily being correlated. Actually, I know for a fact that at least some Broadcom PHYs are affected by power down issues. Also, see the following if if_age.c: /* * No PME capability, PHY power down. * XXX * Due to an unknown reason powering down PHY resulted * in unexpected results such as inaccessbility of * hardware of freshly rebooted system. Disable * powering down PHY until I got more information for * Attansic/Atheros PHY hardwares. */ #ifdef notyet age_miibus_writereg(sc->age_dev, sc->age_phyaddr, MII_BMCR, BMCR_PDOWN); #endif That's why I don't want to make BMCR_PDOWN available as soon as MIIF_NOISOLATE is not set and generally don't want to combine BMCR_PDOWN with BMCR_ISO. > > > - There should be a way to let a PHY driver do something different > > than setting BMCR_PDOWN when it's told to power down as f.e. at > > least some Broadcom PHYs support a "super isolation" mode. > > Cool, but how does my suggestion stop anyone from implementing that? Why not solve it properly in the first place? I agree that a possibility to power down PHYs should be implemented but given that we've lived for so long without it I don't see a need to get a quick-hack in as fast as possible. > > > - As you already mentioned in general there can be multiple PHYs > > on one MII bus and I'd expect "power down that interface" to > > mean to power all of them down (granted, MACs with multiple > > PHYs are rather uncommon in reality but they do exist, f.e. > > Allied Telesis has several card models that use two PHYs per > > MAC). > > Since there can be only one active PHY at a time, it could make sense to also set PDOWN in mii_mediachg(). How about a new flag MIIF_PDOWN that the phy driver can set to indicate that it would rather be shut off than just isolated? Right now I only care about ukphy anyway. Yes, something like a MIIF_PDOWN (or probably better a MIIF_NOPDWON to be in line with MIIF_NOISOLATE) is needed in order to deal with broken PHYs. On the other hand IEEE 802.3-2008 says that non-active PHYs should just be isolated, not powered down or both. Given that multi-PHY configurations are actually very rare in practice I don't see a good reason to violate the standard and power down non-active PHYs in mii_mediachg(). For the embedded world with multiple (fake) stuff hanging around on the MDIO bus you need to already prevent mii_mediachg() from isolating all but one "PHY" anyway. > > > In generall I agree that there should be a way to power down PHYs > > though. While there might be some merit in additionally adding that > > as a media option I think that the most intuitive way would be for > > `ifconfig foo0 down` to also power down the PHYs on that interface. > > In order implement that the basic way seems to be to make the > > various foo_stop() methods of MAC drivers to call mii_down() (as > > in NetBSD) and to make mii_phy_down() (or a replacement) actually > > do something, with PHY drivers being allowed to opt-out with > > something like a MIIF_NOPOWERDOWN. What I'm not sure about is how > > to implement power down in the individual drivers while keeping > > code duplication for the common case low, i.e. I'm not sure whether > > the NetBSD way of having a MII_DOWN case in the service routines > > is the way to go. > > I'm hesitant to tie IFF_UP to link state. Is there precedent for that? My gut feeling is that IFF_UP has always meant that the interface has been brought up administratively; bringing an interface down "only" means that the system should not transmit on it and discard received frames. > Without investigating all the Ethernet drivers that don't use miibus(4) I'd say yes, in FreeBSD administratively down currently means what you describe. However, the question is whether that behavior makes any sense at all. So far I couldn't think of a single usecase where the current behavior of keeping up the link but ignoring all the traffic when an interface is set to down would be needed (and if there actually is one the same effect likely could be achieved by just up'ing the interface but not setting an address. On the other hand, tying power down and therefore the link state (if the PHY allows to ...) as the advantage of: - no need to grow a hack or another media to power down a PHY, - save power in the default case, - being in line what administratively up/down means on network gear, - doing the right thing when you want to bring a link of an LACP channel up/down, - not reporting false media when the interface is down (this is PHY-dependent though). I'm interested to hear about usecases where the current behavior of keeping up the link when the interface is down actually is required though. Marius