Re: PATCH 2.4.0.9.2: export ethtool interface

2000-09-20 Thread Andrew Morton

Donald Becker wrote:
> 
> On Tue, 19 Sep 2000, Andrew Morton wrote:
> 
> > > This patch, against 2.4.0-test9-pre2, moves ethtool.h from the private
> > > domain of the sparc ports into include/linux.  This publishes an
> ...
> > This is good. It would be useful to have this in place ASAP so driver
> > authors have something to look at and to work against.
> 
> You all know my opinion on this interface: it is bad.

Yes and no.  I assume you refer to the ability to force link speed,
duplex, etc?  This is a broken way to fix broken hardware/software and I
wouldn't expect many drivers to implement this part.

But the ability to talk to the driver directly, to have a uniform
interface to query the link state, the link settings, to be able to tune
driver-specific settings, to fetch driver-specific performance data and
to dynamically change the driver debug level would be pretty useful.

> > * There don't seem to be enough port types and transceiver types.
> > 10base2? BNC? 100baseTx/100baseFX?
> 
> 1Mbps HomePNA? 10Mbps HomePNA?  (Check the control words for the PNA spec.)
> Various 802.11?
> 10Gb Ethernet?  Have you looked at gigabit autonegotiation?

Not at all, and I suspect that an attempt to define all this stuff
up-front will come unstuck.

> If you are proposing a new interface (and obviously tossing the
> existing MII-MDIO emulation that has existed for a few years) you should at
> least support current hardware.

Well, if the driver supports SIOCGMIIPHY then the userland support
library should be able to sort these things out.  As far as reporting on
interface settings, I'd view this proposal as allowing individual
drivers to fill in any gaps.  But the proposal goes further than this.


Anyway.  Jeff, this smells to me like 2.5-food.  This interface won't be
proven and stabilised until a couple of dissimilar drivers have actually
used it, and the userland stuff is implemented.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: PATCH 2.4.0.9.2: export ethtool interface

2000-09-19 Thread Donald Becker

On Tue, 19 Sep 2000, Andrew Morton wrote:

> > This patch, against 2.4.0-test9-pre2, moves ethtool.h from the private
> > domain of the sparc ports into include/linux.  This publishes an
...
> This is good. It would be useful to have this in place ASAP so driver
> authors have something to look at and to work against.

You all know my opinion on this interface: it is bad.

> * I added SUPPORTED_MAC_FLOWCTRL/ADVERTISED_MAC_FLOWCTRL for advertising
> of 802.3x MAC-layer flow control.

There are two elements of this: Rx and Tx flow control.  We might support Rx
flow control, but not generate flow control packets.  Or perhaps the
inverse.

> * There don't seem to be enough port types and transceiver types. 
> 10base2? BNC? 100baseTx/100baseFX?

1Mbps HomePNA? 10Mbps HomePNA?  (Check the control words for the PNA spec.)
Various 802.11?
10Gb Ethernet?  Have you looked at gigabit autonegotiation?

If you are proposing a new interface (and obviously tossing the
existing MII-MDIO emulation that has existed for a few years) you should at
least support current hardware.


Donald Becker   [EMAIL PROTECTED]
Scyld Computing Corporation http://www.scyld.com
410 Severn Ave. Suite 210   Beowulf-II Cluster Distribution
Annapolis MD 21403

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/



Re: PATCH 2.4.0.9.2: export ethtool interface

2000-09-19 Thread Andrew Morton

Jeff Garzik wrote:
> 
> This patch, against 2.4.0-test9-pre2, moves ethtool.h from the private
> domain of the sparc ports into include/linux.  This publishes an
> existing interface, and has been discussed before.  (search past lkml
> subject headers for "media tool" and "ethtool")
> 
> This updated patch should take some of the past threads into account.
> The differences from the current sparc ethtool.h are:
> 
> * bitmask for advertising as well as supported media types
> * interrupt mitigation hints max{tx,rx}pkt (Jes suggestion)
> * four reserved u32 slots for future expansion
> * assigned an ioctl: SIOCETHTOOL (0x8944)
> 

This is good. It would be useful to have this in place ASAP so driver
authors have something to look at and to work against.

I've made some edits (revised patch attached here).

* With this interface I don't see a way in which one can force some
settings but leave others unforced.  Suppose, for example, that I wish
to force 10mbps but to leave the setting of the transceiver type,
duplex, etc alone (ie: up to the driver/hardware to determine the value
of these).

If you do an ETHTOOL_SSET/modify/ETHTOOL_SGET on the interface you'll
end up *forcing* all these settings into whatever state they happened to
be in when you did the read, no?

So I added the `force_mask' which should be tested by the ETHTOOL_SSET
handler to decide which fields are actually being forced.

When the driver receives a ETHTOOL_SGET it should populate `force_mask'
to indicate to the caller which fields are currently forced.

* I added SUPPORTED_MAC_FLOWCTRL/ADVERTISED_MAC_FLOWCTRL for advertising
of 802.3x MAC-layer flow control.

* I added the `flow_ctrl' field to say whether we want to enable/disable
this.

Other comments:

* There don't seem to be enough port types and transceiver types. 
10base2? BNC? 100baseTx/100baseFX?

* Regarding the ADVERTISED bits.  Are these designed to read back what
we're currently advertising, or are they designed to allow us to force
what is to be advertised?  Both, I guess.

* maxtxpkt and maxrxpkt seem highly specific to particular hardware and
particular drivers.  Plus there are a large range of other
driver-specific performance tunables which are typically controlled by
module parms.  Why pick these two?

It may be better to bury this sort of thing into a device-specific
anonymous field.  Shrug - don't bother: I wouldn't expect anyone to
really use these things because it's such a PITA to tear down all the
driver data structures and rebuild them.

* Are you really, really sure about the alignment issues with struct
ethtool_cmd?  It strikes me that burying a short and five chars in among
a bunch of longs is asking for trouble.  Suppose the caller is using a
different compiler?  If you're not sure, make 'em all u32 :)



On a related topic, it offends me that when you unplug the RJ45 from a
win2k machine it pops up a little box telling you your network has
disappeared.  We can't do that.

At present, SIOCGIFFLAGS returns an amalgam of __LINK_STATE_START and
__LINK_STATE_NOCARRIER in IFF_RUNNING.  We need to separate these out so
applications can query __LINK_STATE_NOCARRIER independently.  We also
need a select()able interface so changes in carrier can be monitored.

The users of a select()able carrier detect would be desktop UIs and the
high-availability guys.  The latter is a guess - I don't really know if
people who are concerned about failover would find a carrier-lost
indication useful.  TBD.

--- linux-2.4.0-test9-pre4/include/linux/ethtool.h  Thu Jan  1 10:00:00 1970
+++ linux-akpm/include/linux/ethtool.h  Tue Sep 19 23:09:38 2000
@@ -0,0 +1,114 @@
+/* $Id: ethtool.h,v 1.1.186.1 2000/09/16 20:13:14 jgarzik Exp $
+ * ethtool.h: Defines for Linux ethtool.
+ *
+ * Copyright (C) 1998 David S. Miller ([EMAIL PROTECTED])
+ */
+
+#ifndef _LINUX_ETHTOOL_H
+#define _LINUX_ETHTOOL_H
+
+/* This should work for both 32 and 64 bit userland. */
+struct ethtool_cmd {
+   u32 cmd;
+   u32 force_mask; /* Which fieldswe wish to force */
+   u32 supported;  /* Features this interface supports */
+   u32 advertising;/* Features this interface advertises */
+   u16 speed;  /* The forced speed, 10Mb, 100Mb, gigabit */
+   u8  duplex; /* Duplex, half or full */
+   u8  flow_ctrl;  /* 802.3x flow control */
+   u8  port;   /* Which connector port */
+   u8  phy_address;
+   u8  transceiver;/* Which tranceiver to use */
+   u8  autoneg;/* Enable or disable autonegotiation */
+   u32 maxtxpkt;   /* Tx pkts before generating tx int */
+   u32 maxrxpkt;   /* Rx pkts before generating rx int */
+   u32 reserved[4];
+};
+
+
+/* CMDs currently supported */
+#define ETHTOOL_GSET   0x0001 /* Get settings, non-privileged. */
+#define ETHTOOL_SSET   0x0002 /* Set settings, privileged. */
+
+/* compatibilit

PATCH 2.4.0.9.2: export ethtool interface

2000-09-18 Thread Jeff Garzik

This patch, against 2.4.0-test9-pre2, moves ethtool.h from the private
domain of the sparc ports into include/linux.  This publishes an
existing interface, and has been discussed before.  (search past lkml
subject headers for "media tool" and "ethtool")

This updated patch should take some of the past threads into account. 
The differences from the current sparc ethtool.h are:

* bitmask for advertising as well as supported media types
* interrupt mitigation hints max{tx,rx}pkt (Jes suggestion)
* four reserved u32 slots for future expansion
* assigned an ioctl: SIOCETHTOOL (0x8944)

Questions, comments, and feedback welcome.  If there are no objections,
I'll submit to DaveM for inclusion in 2.4.0-test9-whatever, after this
current round of discussion.

Jeff

diff -urN vanilla/linux-2.4.0-test9-pre2/include/linux/ethtool.h 
linux_2_3/include/linux/ethtool.h
--- vanilla/linux-2.4.0-test9-pre2/include/linux/ethtool.h  Wed Dec 31 19:00:00 
1969
+++ linux_2_3/include/linux/ethtool.h   Sun Sep 17 17:29:10 2000
@@ -0,0 +1,96 @@
+/* $Id: ethtool.h,v 1.1.186.1 2000/09/16 20:13:14 jgarzik Exp $
+ * ethtool.h: Defines for Linux ethtool.
+ *
+ * Copyright (C) 1998 David S. Miller ([EMAIL PROTECTED])
+ */
+
+#ifndef _LINUX_ETHTOOL_H
+#define _LINUX_ETHTOOL_H
+
+
+/* This should work for both 32 and 64 bit userland. */
+struct ethtool_cmd {
+   u32 cmd;
+   u32 supported;  /* Features this interface supports */
+   u32 advertising;/* Features this interface advertises */
+   u16 speed;  /* The forced speed, 10Mb, 100Mb, gigabit */
+   u8  duplex; /* Duplex, half or full */
+   u8  port;   /* Which connector port */
+   u8  phy_address;
+   u8  transceiver;/* Which tranceiver to use */
+   u8  autoneg;/* Enable or disable autonegotiation */
+   u32 maxtxpkt;   /* Tx pkts before generating tx int */
+   u32 maxrxpkt;   /* Rx pkts before generating rx int */
+   u32 reserved[4];
+};
+
+
+/* CMDs currently supported */
+#define ETHTOOL_GSET   0x0001 /* Get settings, non-privileged. */
+#define ETHTOOL_SSET   0x0002 /* Set settings, privileged. */
+
+/* compatibility with older code */
+#define SPARC_ETH_GSET ETHTOOL_GSET
+#define SPARC_ETH_SSET ETHTOOL_SSET
+
+/* Indicates what features are supported by the interface. */
+#define SUPPORTED_10baseT_Half (1 << 0)
+#define SUPPORTED_10baseT_Full (1 << 1)
+#define SUPPORTED_100baseT_Half(1 << 2)
+#define SUPPORTED_100baseT_Full(1 << 3)
+#define SUPPORTED_1000baseT_Half   (1 << 4)
+#define SUPPORTED_1000baseT_Full   (1 << 5)
+#define SUPPORTED_Autoneg  (1 << 6)
+#define SUPPORTED_TP   (1 << 7)
+#define SUPPORTED_AUI  (1 << 8)
+#define SUPPORTED_MII  (1 << 9)
+#define SUPPORTED_FIBRE(1 << 10)
+
+/* Indicates what features are advertised by the interface. */
+#define ADVERTISED_10baseT_Half(1 << 0)
+#define ADVERTISED_10baseT_Full(1 << 1)
+#define ADVERTISED_100baseT_Half   (1 << 2)
+#define ADVERTISED_100baseT_Full   (1 << 3)
+#define ADVERTISED_1000baseT_Half  (1 << 4)
+#define ADVERTISED_1000baseT_Full  (1 << 5)
+#define ADVERTISED_Autoneg (1 << 6)
+#define ADVERTISED_TP  (1 << 7)
+#define ADVERTISED_AUI (1 << 8)
+#define ADVERTISED_MII (1 << 9)
+#define ADVERTISED_FIBRE   (1 << 10)
+
+/* The following are all involved in forcing a particular link
+ * mode for the device for setting things.  When getting the
+ * devices settings, these indicate the current mode and whether
+ * it was foced up into this mode or autonegotiated.
+ */
+
+/* The forced speed, 10Mb, 100Mb, gigabit. */
+#define SPEED_10   10
+#define SPEED_100  100
+#define SPEED_1000 1000
+
+/* Duplex, half or full. */
+#define DUPLEX_HALF0x00
+#define DUPLEX_FULL0x01
+
+/* Which connector port. */
+#define PORT_TP0x00
+#define PORT_AUI   0x01
+#define PORT_MII   0x02
+#define PORT_FIBRE 0x03
+
+/* Which tranceiver to use. */
+#define XCVR_INTERNAL  0x00
+#define XCVR_EXTERNAL  0x01
+#define XCVR_DUMMY10x02
+#define XCVR_DUMMY20x03
+#define XCVR_DUMMY30x04
+
+/* Enable or disable autonegotiation.  If this is set to enable,
+ * the forced link modes above are completely ignored.
+ */
+#define AUTONEG_DISABLE0x00
+#define AUTONEG_ENABLE 0x01
+
+#endif /* _LINUX_ETHTOOL_H */
diff -urN vanilla/linux-2.4.0-test9-pre2/drivers/net/sunhme.c 
linux_2_3/drivers/net/sunhme.c
--- vanilla/linux-2.4.0-test9-pre2/drivers/net/sunhme.c Thu Sep  7 11:32:00 2000
+++ linux_2_3/drivers/net/sun