From: Jeff Garzik <firstname.lastname@example.org> To: Donald Becker <email@example.com> Cc: Bogdan Costescu <firstname.lastname@example.org>, Linux Kernel Mailing List <email@example.com>, firstname.lastname@example.org, "David S. Miller" <email@example.com>, Linus Torvalds <firstname.lastname@example.org> Subject: Re: PATCH: ethtool MII helpers Date: Wed, 13 Jun 2001 16:24:53 -0400 [thread overview] Message-ID: <3B27CC15.2B92E71A@mandrakesoft.com> (raw) In-Reply-To: <Pine.LNX.email@example.com> Donald Becker wrote: > I was on vacation, and thus didn't have the opportunity to comment earlier. Thanks a bunch for your comments here. > On Tue, 12 Jun 2001, Jeff Garzik wrote: > > > > - You are proposing some caching for the MII registers. I suppose that you > > > would like to have this code also working with whatever caching will be > > > done for MII access that was recently discussed. Wouldn't this produce > > > double caching under some circumstances ? > > > > You misunderstood the code. The "caching" here is whatever is -already- > > being done by the driver. Many Becker-style drivers cache the > > advertising value. If such a driver uses the ethtool MII code, that is > > one less MII read that needs to occur. > > That's not the way I read the code. It appears to cache various MII > management registers. I still think there is a misunderstanding here, brought about my short explanation and lack of docs.. The key here is the lifetime of the cache. Without extra work on the part of the driver author, the data in struct ethtool_mii_info only exists for a single ioctl call. ethtool_mii_info is a container, not a data cache. So, if you already have MII register cached somewhere, like advertising, or you perform MII reads before calling ethtool_mii_[gs]set, then those values are "cached" in the sense that mii.c will not re-read the register values. Since MII reads are not the quickest operations in the world, I preferred to be flexible in allowing what will occur before and after the ethtool_mii_[gs]set call. > Caching almost any MII register, except the ID registers, may be > invalid. Agreed. I even said this in an MII thread on lkml a couple weeks ago ;-) > Caching and ioctl() rate-limiting are both a problem for a program I use > frequently. It monitors the transceiver to report the timing and state > transitions of autonegotiation. It internally handles polling rate > limiting by backing off the poll rate when nothing is happening. But > when something happens, it polls every timer tick for the next 30 > ticks. Unfortunately that is at loggerheads with the potential for a bunch of people to soak the system with unpriveleged MII reads via ioctl. That is the core problem, and caching or rate-limiting is only a suggested solution. I could forget about rate-limiting if we required CAP_NET_ADMIN and/or CAP_RAW_IO for all these ioctls, but that might cause complaints too.. > > David's argument is for ethtool, which originally comes out of the sparc > > port (see include/asm-sparc/ethtool.h in older trees), and has been > > around for a while, but doesn't enjoy the massive deployment that the > > MII ioctls enjoy. We have control over the ethtool API, and we can > > correct its deficiencies, whereas any MII spec deficiencies must be > > worked out inside the driver. > > You should first understand what MII management registers provide before > deciding that you can do better. There are some design uglinesses, > but it was put together by people that lived and breathed transceivers. > It has been proven over six or seven years or use with no incompatible > changes to the original software interface definition. > > >... > > Further, for the userland ethtool program, support for MII ioctls will > > be added soon, so that there will be no need for additional mii-tool or > > mii-diag tools. > > This could be easily reversed: the additional ethtool program was not > needed in the first place. > > > > This is nice, but I would like to able to restart autonegotiation even > > > without changing any of the advertised capabilities. If I missed this > > > possibility, please point me to it... > > > > no, that is a capability which needs to be added to ethtool. > > ETHTOOL_RENEG or ETHTOOL_ANRESTART or something. Basically kick the > > link state machine, whether such a state machine is in the driver or in > > the MII phy. That's the one big thing that mii-tool can do that ethtool > > cannot, AFAICS. > > An additional capability of the MII ioctl() is that it permits sending > "next page" extended information to the link partner. [move this down here] > This message covers > Why caching MII values doesn't work. [responded above] > Why extended MII values are useful. Ok, thanks, agreed. About the larger issue of why ethtool exists, I wonder about things like: how do the MII ioctls cover things like switching transceivers? supporting aui/10b2? supporting sym phys? ethtool is not just about 10/100 media. It's a general software diagnostics utility and tuning tool for your ethernet driver. The same kernel interface and the same userland program will allow me to associate an ethernet interface with a driver and bus location, adjust media settings, adjust interrupt mitigation settings, or perhaps even perform a driver-specific duty. I am very much convinced that the extended MII ioctls are useful, and would even support codifying them in sockios.h, using the SIOCMII* names you are already using. However I see the MII ioctls as a tuning tool for a specific (though large) subset of hardware. I am still not comfortable with considering the MII ioctls as the standard for communication between the kernel and userland... Tangent, to close on a more concrete technical note. The MII ioctls in their current form are not completely portable. For DaveM and others doing 32-bit userland on 64-bit kernel, you have to pass through ioctl translation layer. Not only will the SIOCMIIxxx ioctls need to be made official, but the structure which has so far been implicitly defined (u16* data) in the ioctls would need to be explicitly defined, in some central location. Regards, Jeff -- Jeff Garzik | Andre the Giant has a posse. Building 1024 | MandrakeSoft |
next prev parent reply other threads:[~2001-06-13 20:25 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2001-06-10 17:34 Jeff Garzik 2001-06-11 5:59 ` Pekka Savola 2001-06-11 6:10 ` Keith Owens 2001-06-12 17:09 ` Bogdan Costescu 2001-06-12 17:40 ` Jeff Garzik 2001-06-12 18:40 ` PATCH: ethtool MII helpers (vers 2) Jeff Garzik 2001-06-13 15:29 ` PATCH: ethtool MII helpers Bogdan Costescu 2001-06-13 16:56 ` Donald Becker 2001-06-13 20:24 ` Jeff Garzik [this message] 2001-06-15 17:22 ` Bogdan Costescu 2001-06-22 5:10 ` Chris Wedgwood 2001-06-22 5:24 ` Jeff Garzik 2001-06-22 5:34 ` Chris Wedgwood 2001-06-22 5:58 ` Jeff Garzik
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=3B27CC15.2B92E71A@mandrakesoft.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: PATCH: ethtool MII helpers' \ /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
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).