Netdev List
 help / color / mirror / Atom feed
* [PATCH] sb1250-mac: Driver model & phylib update
From: Maciej W. Rozycki @ 2007-09-21 11:52 UTC (permalink / raw)
  To: Andrew Morton, Jeff Garzik; +Cc: netdev, linux-mips, linux-kernel

 A driver model and phylib update.  It includes the following changes:

1. Removal of unused module options.

2. Phylib support and the resulting removal of generic bits for handling
   the PHY.

3. Proper reserving of device resources and using ioremap()ped handles
   to access MAC registers rather than platform-specific macros.

4. Handling of the device using the driver model.

Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
---
 This is a resubmission and this patch has waited for some time now as the 
original submission triggered some concerns on the way phylib handled 
interrupt-driven PHY units.  The issues were resolved a while ago already, 
but I was distracted by some other work, mainly in the toolchain area, so 
this got postponed this far.  For some background information -- as a 
reminder -- here is the original long explanation I gave back then:

 "Here is a set of patches that update the sb1250-mac driver used for the 
onchip Gigabit Ethernet interfaces of the Broadcom SiByte family of SOCs 
including the BCM1250 and a couple of other members.  These are used, 
among others, on various Broadcom evaluation boards together with Broadcom 
Gigabit Ethernet PHY chips.  Changes include porting the driver to the 
driver model as a platform device, support for phylib, including the 
BCM54xx PHYs in the interrupt mode, proper resource managment and a couple 
of minor clean-ups.

 Apart from changes to networking code, there are a few required in the
architecture-specific areas and therefore I am sending these changes to
Ralf and the linux-mips list as well.  It might also involve a few more
interested parties in the discussion.

 The changes were tested with a Broadcom SWARM board, which includes a
BCM1250 part which has 3 MAC units on chip, of which 2 are usable, with
BCM5421 PHY chips attached (both wired to the same interrupt line, which
made testing whether IRQ sharing works properly in phylib possible).
Link partners included a 1000base and a 100base interface doing
autonegotiation as well as a 10base one doing none.

 Other Broadcom boards that I know of may have these or BCM5411 or BCM5461
chips.  The lack of documentation or at least actual pieces of hardware
makes the use of interrupts impossible for all but the SWARM, the Sentosa
and the Shorty (with the latter unsupported by Linux)."

 All the bits except from the mentioned architecture-specific fix -- the 
firmware of the SWARM gets the polarity of the PHY IRQ line wrong, which 
has to be reprogrammed in the SOC -- have found their way into Linux 
already.  The polarity fix is not critical for the update provided here as 
the update has been written such that with the current state of affairs 
the driver will use the polled mode of phylib operation.

 I will provide the architecture-specific fix later on, probably once this 
update has propagated back to the linux-mips.org tree (suggestions as to 
why I should do otherwise certainly welcome).  This way if any problems 
are seen due to the switch of the PHY to the interrupt-driven mode, they 
can be sorted out independently of this change.

 This change applies to the current netdev-2.6.git#upstream tree, on top 
of "patch-netdev-2.6.23-rc6-20070920-sb1250-mac-typedef-9" submitted 
yesterday.

 Please apply.

  Maciej

patch-netdev-2.6.23-rc6-20070920-sb1250-mac-29
diff -up --recursive --new-file linux-netdev-2.6.23-rc6-20070920.macro/drivers/net/Kconfig linux-netdev-2.6.23-rc6-20070920/drivers/net/Kconfig
--- linux-netdev-2.6.23-rc6-20070920.macro/drivers/net/Kconfig	2007-09-20 17:55:14.000000000 +0000
+++ linux-netdev-2.6.23-rc6-20070920/drivers/net/Kconfig	2007-09-20 20:31:42.000000000 +0000
@@ -2126,6 +2126,7 @@ config R8169_VLAN
 config SB1250_MAC
 	tristate "SB1250 Gigabit Ethernet support"
 	depends on SIBYTE_SB1xxx_SOC
+	select PHYLIB
 	---help---
 	  This driver supports Gigabit Ethernet interfaces based on the
 	  Broadcom SiByte family of System-On-a-Chip parts.  They include
diff -up --recursive --new-file linux-netdev-2.6.23-rc6-20070920.macro/drivers/net/sb1250-mac.c linux-netdev-2.6.23-rc6-20070920/drivers/net/sb1250-mac.c
--- linux-netdev-2.6.23-rc6-20070920.macro/drivers/net/sb1250-mac.c	2007-09-20 18:09:18.000000000 +0000
+++ linux-netdev-2.6.23-rc6-20070920/drivers/net/sb1250-mac.c	2007-09-20 21:06:55.000000000 +0000
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2001,2002,2003,2004 Broadcom Corporation
+ * Copyright (c) 2006, 2007  Maciej W. Rozycki
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -18,7 +19,12 @@
  *
  * This driver is designed for the Broadcom SiByte SOC built-in
  * Ethernet controllers. Written by Mitch Lichtenberg at Broadcom Corp.
+ *
+ * Updated to the driver model and the PHY abstraction layer
+ * by Maciej W. Rozycki.
  */
+
+#include <linux/bug.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/string.h>
@@ -32,9 +38,15 @@
 #include <linux/skbuff.h>
 #include <linux/init.h>
 #include <linux/bitops.h>
-#include <asm/processor.h>		/* Processor type for cache alignment. */
-#include <asm/io.h>
+#include <linux/err.h>
+#include <linux/ethtool.h>
+#include <linux/mii.h>
+#include <linux/phy.h>
+#include <linux/platform_device.h>
+
 #include <asm/cache.h>
+#include <asm/io.h>
+#include <asm/processor.h>	/* Processor type for cache alignment. */
 
 /* This is only here until the firmware is ready.  In that case,
    the firmware leaves the ethernet address in the register for us. */
@@ -48,7 +60,7 @@
 
 /* These identify the driver base version and may not be removed. */
 #if 0
-static char version1[] __devinitdata =
+static char version1[] __initdata =
 "sb1250-mac.c:1.00 1/11/2001 Written by Mitch Lichtenberg\n";
 #endif
 
@@ -57,8 +69,6 @@ static char version1[] __devinitdata =
 
 #define CONFIG_SBMAC_COALESCE
 
-#define MAX_UNITS 4		/* More are supported, limit only on options */
-
 /* Time in jiffies before concluding the transmitter is hung. */
 #define TX_TIMEOUT  (2*HZ)
 
@@ -74,26 +84,6 @@ static int debug = 1;
 module_param(debug, int, S_IRUGO);
 MODULE_PARM_DESC(debug, "Debug messages");
 
-/* mii status msgs */
-static int noisy_mii = 1;
-module_param(noisy_mii, int, S_IRUGO);
-MODULE_PARM_DESC(noisy_mii, "MII status messages");
-
-/* Used to pass the media type, etc.
-   Both 'options[]' and 'full_duplex[]' should exist for driver
-   interoperability.
-   The media type is usually passed in 'options[]'.
-*/
-#ifdef MODULE
-static int options[MAX_UNITS] = {-1, -1, -1, -1};
-module_param_array(options, int, NULL, S_IRUGO);
-MODULE_PARM_DESC(options, "1-" __MODULE_STRING(MAX_UNITS));
-
-static int full_duplex[MAX_UNITS] = {-1, -1, -1, -1};
-module_param_array(full_duplex, int, NULL, S_IRUGO);
-MODULE_PARM_DESC(full_duplex, "1-" __MODULE_STRING(MAX_UNITS));
-#endif
-
 #ifdef CONFIG_SBMAC_COALESCE
 static int int_pktcnt_tx = 255;
 module_param(int_pktcnt_tx, int, S_IRUGO);
@@ -112,6 +102,7 @@ module_param(int_timeout_rx, int, S_IRUG
 MODULE_PARM_DESC(int_timeout_rx, "RX timeout value");
 #endif
 
+#include <asm/sibyte/board.h>
 #include <asm/sibyte/sb1250.h>
 #if defined(CONFIG_SIBYTE_BCM1x55) || defined(CONFIG_SIBYTE_BCM1x80)
 #include <asm/sibyte/bcm1480_regs.h>
@@ -135,22 +126,43 @@ MODULE_PARM_DESC(int_timeout_rx, "RX tim
 #error invalid SiByte MAC configuation
 #endif
 
+#ifdef K_INT_PHY
+#define SBMAC_PHY_INT			K_INT_PHY
+#else
+#define SBMAC_PHY_INT			PHY_POLL
+#endif
+
 /**********************************************************************
  *  Simple types
  ********************************************************************* */
 
+enum sbmac_speed {
+	sbmac_speed_none = 0,
+	sbmac_speed_10 = SPEED_10,
+	sbmac_speed_100 = SPEED_100,
+	sbmac_speed_1000 = SPEED_1000,
+};
 
-enum sbmac_speed { sbmac_speed_auto, sbmac_speed_10,
-		   sbmac_speed_100, sbmac_speed_1000 };
-
-enum sbmac_duplex { sbmac_duplex_auto, sbmac_duplex_half,
-		    sbmac_duplex_full };
+enum sbmac_duplex {
+	sbmac_duplex_none = -1,
+	sbmac_duplex_half = DUPLEX_HALF,
+	sbmac_duplex_full = DUPLEX_FULL,
+};
 
-enum sbmac_fc { sbmac_fc_auto, sbmac_fc_disabled, sbmac_fc_frame,
-		sbmac_fc_collision, sbmac_fc_carrier } sbmac_fc_t;
+enum sbmac_fc {
+	sbmac_fc_none,
+	sbmac_fc_disabled,
+	sbmac_fc_frame,
+	sbmac_fc_collision,
+	sbmac_fc_carrier,
+};
 
-enum sbmac_state { sbmac_state_uninit, sbmac_state_off, sbmac_state_on,
-		   sbmac_state_broken };
+enum sbmac_state {
+	sbmac_state_uninit,
+	sbmac_state_off,
+	sbmac_state_on,
+	sbmac_state_broken,
+};
 
 
 /**********************************************************************
@@ -244,18 +256,14 @@ struct sbmac_softc {
 	 */
 	struct net_device	*sbm_dev;	/* pointer to linux device */
 	struct napi_struct	napi;
+	struct phy_device	*phy_dev;	/* the associated PHY device */
+	struct mii_bus		mii_bus;	/* the MII bus */
+	int			phy_irq[PHY_MAX_ADDR];
 	spinlock_t		sbm_lock;	/* spin lock */
-	struct timer_list	sbm_timer;	/* for monitoring MII */
 	int			sbm_devflags;	/* current device flags */
 
-	int			sbm_phy_oldbmsr;
-	int			sbm_phy_oldanlpar;
-	int			sbm_phy_oldk1stsr;
-	int			sbm_phy_oldlinkstat;
 	int			sbm_buffersize;
 
-	unsigned char		sbm_phys[2];
-
 	/*
 	 * Controller-specific things
 	 */
@@ -274,6 +282,8 @@ struct sbmac_softc {
 	enum sbmac_speed	sbm_speed;	/* current speed */
 	enum sbmac_duplex	sbm_duplex;	/* current duplex */
 	enum sbmac_fc		sbm_fc;		/* cur. flow control setting */
+	int			sbm_pause;	/* current pause setting */
+	int			sbm_link;	/* current link state */
 
 	unsigned char		sbm_hwaddr[ETHER_ADDR_LEN];
 
@@ -313,36 +323,37 @@ static uint64_t sbmac_addr2reg(unsigned 
 static irqreturn_t sbmac_intr(int irq, void *dev_instance);
 static int sbmac_start_tx(struct sk_buff *skb, struct net_device *dev);
 static void sbmac_setmulti(struct sbmac_softc *sc);
-static int sbmac_init(struct net_device *dev, int idx);
+static int sbmac_init(struct platform_device *pldev, long long base);
 static int sbmac_set_speed(struct sbmac_softc *s, enum sbmac_speed speed);
 static int sbmac_set_duplex(struct sbmac_softc *s, enum sbmac_duplex duplex,
 			    enum sbmac_fc fc);
 
 static int sbmac_open(struct net_device *dev);
-static void sbmac_timer(unsigned long data);
 static void sbmac_tx_timeout (struct net_device *dev);
 static void sbmac_set_rx_mode(struct net_device *dev);
 static int sbmac_mii_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
 static int sbmac_close(struct net_device *dev);
 static int sbmac_poll(struct napi_struct *napi, int budget);
 
-static int sbmac_mii_poll(struct sbmac_softc *s, int noisy);
+static void sbmac_mii_poll(struct net_device *dev);
 static int sbmac_mii_probe(struct net_device *dev);
 
-static void sbmac_mii_sync(struct sbmac_softc *s);
-static void sbmac_mii_senddata(struct sbmac_softc *s, unsigned int data,
+static void sbmac_mii_sync(void __iomem *sbm_mdio);
+static void sbmac_mii_senddata(void __iomem *sbm_mdio, unsigned int data,
 			       int bitcnt);
-static unsigned int sbmac_mii_read(struct sbmac_softc *s, int phyaddr,
-				   int regidx);
-static void sbmac_mii_write(struct sbmac_softc *s, int phyaddr, int regidx,
-			    unsigned int regval);
+static int sbmac_mii_read(struct mii_bus *bus, int phyaddr, int regidx);
+static int sbmac_mii_write(struct mii_bus *bus, int phyaddr, int regidx,
+			   u16 val);
 
 
 /**********************************************************************
  *  Globals
  ********************************************************************* */
 
-static uint64_t sbmac_orig_hwaddr[MAX_UNITS];
+static char sbmac_string[] = "sb1250-mac";
+static char sbmac_pretty[] = "SB1250 MAC";
+
+static char sbmac_mdio_string[] = "sb1250-mac-mdio";
 
 
 /**********************************************************************
@@ -354,185 +365,66 @@ static uint64_t sbmac_orig_hwaddr[MAX_UN
 #define	MII_COMMAND_WRITE	0x01
 #define	MII_COMMAND_ACK		0x02
 
-#define BMCR_RESET     0x8000
-#define BMCR_LOOPBACK  0x4000
-#define BMCR_SPEED0    0x2000
-#define BMCR_ANENABLE  0x1000
-#define BMCR_POWERDOWN 0x0800
-#define BMCR_ISOLATE   0x0400
-#define BMCR_RESTARTAN 0x0200
-#define BMCR_DUPLEX    0x0100
-#define BMCR_COLTEST   0x0080
-#define BMCR_SPEED1    0x0040
-#define BMCR_SPEED1000	BMCR_SPEED1
-#define BMCR_SPEED100	BMCR_SPEED0
-#define BMCR_SPEED10 	0
-
-#define BMSR_100BT4	0x8000
-#define BMSR_100BT_FDX	0x4000
-#define BMSR_100BT_HDX  0x2000
-#define BMSR_10BT_FDX   0x1000
-#define BMSR_10BT_HDX   0x0800
-#define BMSR_100BT2_FDX 0x0400
-#define BMSR_100BT2_HDX 0x0200
-#define BMSR_1000BT_XSR	0x0100
-#define BMSR_PRESUP	0x0040
-#define BMSR_ANCOMPLT	0x0020
-#define BMSR_REMFAULT	0x0010
-#define BMSR_AUTONEG	0x0008
-#define BMSR_LINKSTAT	0x0004
-#define BMSR_JABDETECT	0x0002
-#define BMSR_EXTCAPAB	0x0001
-
-#define PHYIDR1 	0x2000
-#define PHYIDR2		0x5C60
-
-#define ANAR_NP		0x8000
-#define ANAR_RF		0x2000
-#define ANAR_ASYPAUSE	0x0800
-#define ANAR_PAUSE	0x0400
-#define ANAR_T4		0x0200
-#define ANAR_TXFD	0x0100
-#define ANAR_TXHD	0x0080
-#define ANAR_10FD	0x0040
-#define ANAR_10HD	0x0020
-#define ANAR_PSB	0x0001
-
-#define ANLPAR_NP	0x8000
-#define ANLPAR_ACK	0x4000
-#define ANLPAR_RF	0x2000
-#define ANLPAR_ASYPAUSE	0x0800
-#define ANLPAR_PAUSE	0x0400
-#define ANLPAR_T4	0x0200
-#define ANLPAR_TXFD	0x0100
-#define ANLPAR_TXHD	0x0080
-#define ANLPAR_10FD	0x0040
-#define ANLPAR_10HD	0x0020
-#define ANLPAR_PSB	0x0001	/* 802.3 */
-
-#define ANER_PDF	0x0010
-#define ANER_LPNPABLE	0x0008
-#define ANER_NPABLE	0x0004
-#define ANER_PAGERX	0x0002
-#define ANER_LPANABLE	0x0001
-
-#define ANNPTR_NP	0x8000
-#define ANNPTR_MP	0x2000
-#define ANNPTR_ACK2	0x1000
-#define ANNPTR_TOGTX	0x0800
-#define ANNPTR_CODE	0x0008
-
-#define ANNPRR_NP	0x8000
-#define ANNPRR_MP	0x2000
-#define ANNPRR_ACK3	0x1000
-#define ANNPRR_TOGTX	0x0800
-#define ANNPRR_CODE	0x0008
-
-#define K1TCR_TESTMODE	0x0000
-#define K1TCR_MSMCE	0x1000
-#define K1TCR_MSCV	0x0800
-#define K1TCR_RPTR	0x0400
-#define K1TCR_1000BT_FDX 0x200
-#define K1TCR_1000BT_HDX 0x100
-
-#define K1STSR_MSMCFLT	0x8000
-#define K1STSR_MSCFGRES	0x4000
-#define K1STSR_LRSTAT	0x2000
-#define K1STSR_RRSTAT	0x1000
-#define K1STSR_LP1KFD	0x0800
-#define K1STSR_LP1KHD   0x0400
-#define K1STSR_LPASMDIR	0x0200
-
-#define K1SCR_1KX_FDX	0x8000
-#define K1SCR_1KX_HDX	0x4000
-#define K1SCR_1KT_FDX	0x2000
-#define K1SCR_1KT_HDX	0x1000
-
-#define STRAP_PHY1	0x0800
-#define STRAP_NCMODE	0x0400
-#define STRAP_MANMSCFG	0x0200
-#define STRAP_ANENABLE	0x0100
-#define STRAP_MSVAL	0x0080
-#define STRAP_1KHDXADV	0x0010
-#define STRAP_1KFDXADV	0x0008
-#define STRAP_100ADV	0x0004
-#define STRAP_SPEEDSEL	0x0000
-#define STRAP_SPEED100	0x0001
-
-#define PHYSUP_SPEED1000 0x10
-#define PHYSUP_SPEED100  0x08
-#define PHYSUP_SPEED10   0x00
-#define PHYSUP_LINKUP	 0x04
-#define PHYSUP_FDX       0x02
-
-#define	MII_BMCR	0x00 	/* Basic mode control register (rw) */
-#define	MII_BMSR	0x01	/* Basic mode status register (ro) */
-#define	MII_PHYIDR1	0x02
-#define	MII_PHYIDR2	0x03
-
-#define MII_K1STSR	0x0A	/* 1K Status Register (ro) */
-#define	MII_ANLPAR	0x05	/* Autonegotiation lnk partner abilities (rw) */
-
-
 #define M_MAC_MDIO_DIR_OUTPUT	0		/* for clarity */
 
 #define ENABLE 		1
 #define DISABLE		0
 
 /**********************************************************************
- *  SBMAC_MII_SYNC(s)
+ *  SBMAC_MII_SYNC(sbm_mdio)
  *
  *  Synchronize with the MII - send a pattern of bits to the MII
  *  that will guarantee that it is ready to accept a command.
  *
  *  Input parameters:
- *  	   s - sbmac structure
+ *  	   sbm_mdio - address of the MAC's MDIO register
  *
  *  Return value:
  *  	   nothing
  ********************************************************************* */
 
-static void sbmac_mii_sync(struct sbmac_softc *s)
+static void sbmac_mii_sync(void __iomem *sbm_mdio)
 {
 	int cnt;
 	uint64_t bits;
 	int mac_mdio_genc;
 
-	mac_mdio_genc = __raw_readq(s->sbm_mdio) & M_MAC_GENC;
+	mac_mdio_genc = __raw_readq(sbm_mdio) & M_MAC_GENC;
 
 	bits = M_MAC_MDIO_DIR_OUTPUT | M_MAC_MDIO_OUT;
 
-	__raw_writeq(bits | mac_mdio_genc, s->sbm_mdio);
+	__raw_writeq(bits | mac_mdio_genc, sbm_mdio);
 
 	for (cnt = 0; cnt < 32; cnt++) {
-		__raw_writeq(bits | M_MAC_MDC | mac_mdio_genc, s->sbm_mdio);
-		__raw_writeq(bits | mac_mdio_genc, s->sbm_mdio);
+		__raw_writeq(bits | M_MAC_MDC | mac_mdio_genc, sbm_mdio);
+		__raw_writeq(bits | mac_mdio_genc, sbm_mdio);
 	}
 }
 
 /**********************************************************************
- *  SBMAC_MII_SENDDATA(s,data,bitcnt)
+ *  SBMAC_MII_SENDDATA(sbm_mdio, data, bitcnt)
  *
  *  Send some bits to the MII.  The bits to be sent are right-
  *  justified in the 'data' parameter.
  *
  *  Input parameters:
- *  	   s - sbmac structure
- *  	   data - data to send
- *  	   bitcnt - number of bits to send
+ *  	   sbm_mdio - address of the MAC's MDIO register
+ *  	   data     - data to send
+ *  	   bitcnt   - number of bits to send
  ********************************************************************* */
 
-static void sbmac_mii_senddata(struct sbmac_softc *s,unsigned int data, int bitcnt)
+static void sbmac_mii_senddata(void __iomem *sbm_mdio, unsigned int data,
+			       int bitcnt)
 {
 	int i;
 	uint64_t bits;
 	unsigned int curmask;
 	int mac_mdio_genc;
 
-	mac_mdio_genc = __raw_readq(s->sbm_mdio) & M_MAC_GENC;
+	mac_mdio_genc = __raw_readq(sbm_mdio) & M_MAC_GENC;
 
 	bits = M_MAC_MDIO_DIR_OUTPUT;
-	__raw_writeq(bits | mac_mdio_genc, s->sbm_mdio);
+	__raw_writeq(bits | mac_mdio_genc, sbm_mdio);
 
 	curmask = 1 << (bitcnt - 1);
 
@@ -540,9 +432,9 @@ static void sbmac_mii_senddata(struct sb
 		if (data & curmask)
 			bits |= M_MAC_MDIO_OUT;
 		else bits &= ~M_MAC_MDIO_OUT;
-		__raw_writeq(bits | mac_mdio_genc, s->sbm_mdio);
-		__raw_writeq(bits | M_MAC_MDC | mac_mdio_genc, s->sbm_mdio);
-		__raw_writeq(bits | mac_mdio_genc, s->sbm_mdio);
+		__raw_writeq(bits | mac_mdio_genc, sbm_mdio);
+		__raw_writeq(bits | M_MAC_MDC | mac_mdio_genc, sbm_mdio);
+		__raw_writeq(bits | mac_mdio_genc, sbm_mdio);
 		curmask >>= 1;
 	}
 }
@@ -550,21 +442,22 @@ static void sbmac_mii_senddata(struct sb
 
 
 /**********************************************************************
- *  SBMAC_MII_READ(s,phyaddr,regidx)
- *
+ *  SBMAC_MII_READ(bus, phyaddr, regidx)
  *  Read a PHY register.
  *
  *  Input parameters:
- *  	   s - sbmac structure
+ *  	   bus     - MDIO bus handle
  *  	   phyaddr - PHY's address
- *  	   regidx = index of register to read
+ *  	   regnum  - index of register to read
  *
  *  Return value:
- *  	   value read, or 0 if an error occurred.
+ *  	   value read, or 0xffff if an error occurred.
  ********************************************************************* */
 
-static unsigned int sbmac_mii_read(struct sbmac_softc *s,int phyaddr,int regidx)
+static int sbmac_mii_read(struct mii_bus *bus, int phyaddr, int regidx)
 {
+	struct sbmac_softc *sc = (struct sbmac_softc *)bus->priv;
+	void __iomem *sbm_mdio = sc->sbm_mdio;
 	int idx;
 	int error;
 	int regval;
@@ -574,8 +467,7 @@ static unsigned int sbmac_mii_read(struc
 	 * Synchronize ourselves so that the PHY knows the next
 	 * thing coming down is a command
 	 */
-
-	sbmac_mii_sync(s);
+	sbmac_mii_sync(sbm_mdio);
 
 	/*
 	 * Send the data to the PHY.  The sequence is
@@ -584,37 +476,37 @@ static unsigned int sbmac_mii_read(struc
 	 * the PHY addr (5 bits)
 	 * the register index (5 bits)
 	 */
+	sbmac_mii_senddata(sbm_mdio, MII_COMMAND_START, 2);
+	sbmac_mii_senddata(sbm_mdio, MII_COMMAND_READ, 2);
+	sbmac_mii_senddata(sbm_mdio, phyaddr, 5);
+	sbmac_mii_senddata(sbm_mdio, regidx, 5);
 
-	sbmac_mii_senddata(s,MII_COMMAND_START, 2);
-	sbmac_mii_senddata(s,MII_COMMAND_READ, 2);
-	sbmac_mii_senddata(s,phyaddr, 5);
-	sbmac_mii_senddata(s,regidx, 5);
-
-	mac_mdio_genc = __raw_readq(s->sbm_mdio) & M_MAC_GENC;
+	mac_mdio_genc = __raw_readq(sbm_mdio) & M_MAC_GENC;
 
 	/*
 	 * Switch the port around without a clock transition.
 	 */
-	__raw_writeq(M_MAC_MDIO_DIR_INPUT | mac_mdio_genc, s->sbm_mdio);
+	__raw_writeq(M_MAC_MDIO_DIR_INPUT | mac_mdio_genc, sbm_mdio);
 
 	/*
 	 * Send out a clock pulse to signal we want the status
 	 */
-
-	__raw_writeq(M_MAC_MDIO_DIR_INPUT | M_MAC_MDC | mac_mdio_genc, s->sbm_mdio);
-	__raw_writeq(M_MAC_MDIO_DIR_INPUT | mac_mdio_genc, s->sbm_mdio);
+	__raw_writeq(M_MAC_MDIO_DIR_INPUT | M_MAC_MDC | mac_mdio_genc,
+		     sbm_mdio);
+	__raw_writeq(M_MAC_MDIO_DIR_INPUT | mac_mdio_genc, sbm_mdio);
 
 	/*
 	 * If an error occurred, the PHY will signal '1' back
 	 */
-	error = __raw_readq(s->sbm_mdio) & M_MAC_MDIO_IN;
+	error = __raw_readq(sbm_mdio) & M_MAC_MDIO_IN;
 
 	/*
 	 * Issue an 'idle' clock pulse, but keep the direction
 	 * the same.
 	 */
-	__raw_writeq(M_MAC_MDIO_DIR_INPUT | M_MAC_MDC | mac_mdio_genc, s->sbm_mdio);
-	__raw_writeq(M_MAC_MDIO_DIR_INPUT | mac_mdio_genc, s->sbm_mdio);
+	__raw_writeq(M_MAC_MDIO_DIR_INPUT | M_MAC_MDC | mac_mdio_genc,
+		     sbm_mdio);
+	__raw_writeq(M_MAC_MDIO_DIR_INPUT | mac_mdio_genc, sbm_mdio);
 
 	regval = 0;
 
@@ -622,55 +514,60 @@ static unsigned int sbmac_mii_read(struc
 		regval <<= 1;
 
 		if (error == 0) {
-			if (__raw_readq(s->sbm_mdio) & M_MAC_MDIO_IN)
+			if (__raw_readq(sbm_mdio) & M_MAC_MDIO_IN)
 				regval |= 1;
 		}
 
-		__raw_writeq(M_MAC_MDIO_DIR_INPUT|M_MAC_MDC | mac_mdio_genc, s->sbm_mdio);
-		__raw_writeq(M_MAC_MDIO_DIR_INPUT | mac_mdio_genc, s->sbm_mdio);
+		__raw_writeq(M_MAC_MDIO_DIR_INPUT | M_MAC_MDC | mac_mdio_genc,
+			     sbm_mdio);
+		__raw_writeq(M_MAC_MDIO_DIR_INPUT | mac_mdio_genc, sbm_mdio);
 	}
 
 	/* Switch back to output */
-	__raw_writeq(M_MAC_MDIO_DIR_OUTPUT | mac_mdio_genc, s->sbm_mdio);
+	__raw_writeq(M_MAC_MDIO_DIR_OUTPUT | mac_mdio_genc, sbm_mdio);
 
 	if (error == 0)
 		return regval;
-	return 0;
+	return 0xffff;
 }
 
 
 /**********************************************************************
- *  SBMAC_MII_WRITE(s,phyaddr,regidx,regval)
+ *  SBMAC_MII_WRITE(bus, phyaddr, regidx, regval)
  *
  *  Write a value to a PHY register.
  *
  *  Input parameters:
- *  	   s - sbmac structure
+ *  	   bus     - MDIO bus handle
  *  	   phyaddr - PHY to use
- *  	   regidx - register within the PHY
- *  	   regval - data to write to register
+ *  	   regidx  - register within the PHY
+ *  	   regval  - data to write to register
  *
  *  Return value:
- *  	   nothing
+ *  	   0 for success
  ********************************************************************* */
 
-static void sbmac_mii_write(struct sbmac_softc *s,int phyaddr,int regidx,
-			    unsigned int regval)
+static int sbmac_mii_write(struct mii_bus *bus, int phyaddr, int regidx,
+			   u16 regval)
 {
+	struct sbmac_softc *sc = (struct sbmac_softc *)bus->priv;
+	void __iomem *sbm_mdio = sc->sbm_mdio;
 	int mac_mdio_genc;
 
-	sbmac_mii_sync(s);
+	sbmac_mii_sync(sbm_mdio);
 
-	sbmac_mii_senddata(s,MII_COMMAND_START,2);
-	sbmac_mii_senddata(s,MII_COMMAND_WRITE,2);
-	sbmac_mii_senddata(s,phyaddr, 5);
-	sbmac_mii_senddata(s,regidx, 5);
-	sbmac_mii_senddata(s,MII_COMMAND_ACK,2);
-	sbmac_mii_senddata(s,regval,16);
+	sbmac_mii_senddata(sbm_mdio, MII_COMMAND_START, 2);
+	sbmac_mii_senddata(sbm_mdio, MII_COMMAND_WRITE, 2);
+	sbmac_mii_senddata(sbm_mdio, phyaddr, 5);
+	sbmac_mii_senddata(sbm_mdio, regidx, 5);
+	sbmac_mii_senddata(sbm_mdio, MII_COMMAND_ACK, 2);
+	sbmac_mii_senddata(sbm_mdio, regval, 16);
 
-	mac_mdio_genc = __raw_readq(s->sbm_mdio) & M_MAC_GENC;
+	mac_mdio_genc = __raw_readq(sbm_mdio) & M_MAC_GENC;
 
-	__raw_writeq(M_MAC_MDIO_DIR_OUTPUT | mac_mdio_genc, s->sbm_mdio);
+	__raw_writeq(M_MAC_MDIO_DIR_OUTPUT | mac_mdio_genc, sbm_mdio);
+
+	return 0;
 }
 
 
@@ -713,27 +610,27 @@ static void sbdma_initctx(struct sbmacdm
 	s->sbe_idx =(s->sbm_base - A_MAC_BASE_0)/MAC_SPACING;
 #endif
 
-	__raw_writeq(0, IOADDR(A_MAC_REGISTER(s->sbe_idx, R_MAC_RMON_TX_BYTES)));
-	__raw_writeq(0, IOADDR(A_MAC_REGISTER(s->sbe_idx, R_MAC_RMON_COLLISIONS)));
-	__raw_writeq(0, IOADDR(A_MAC_REGISTER(s->sbe_idx, R_MAC_RMON_LATE_COL)));
-	__raw_writeq(0, IOADDR(A_MAC_REGISTER(s->sbe_idx, R_MAC_RMON_EX_COL)));
-	__raw_writeq(0, IOADDR(A_MAC_REGISTER(s->sbe_idx, R_MAC_RMON_FCS_ERROR)));
-	__raw_writeq(0, IOADDR(A_MAC_REGISTER(s->sbe_idx, R_MAC_RMON_TX_ABORT)));
-	__raw_writeq(0, IOADDR(A_MAC_REGISTER(s->sbe_idx, R_MAC_RMON_TX_BAD)));
-	__raw_writeq(0, IOADDR(A_MAC_REGISTER(s->sbe_idx, R_MAC_RMON_TX_GOOD)));
-	__raw_writeq(0, IOADDR(A_MAC_REGISTER(s->sbe_idx, R_MAC_RMON_TX_RUNT)));
-	__raw_writeq(0, IOADDR(A_MAC_REGISTER(s->sbe_idx, R_MAC_RMON_TX_OVERSIZE)));
-	__raw_writeq(0, IOADDR(A_MAC_REGISTER(s->sbe_idx, R_MAC_RMON_RX_BYTES)));
-	__raw_writeq(0, IOADDR(A_MAC_REGISTER(s->sbe_idx, R_MAC_RMON_RX_MCAST)));
-	__raw_writeq(0, IOADDR(A_MAC_REGISTER(s->sbe_idx, R_MAC_RMON_RX_BCAST)));
-	__raw_writeq(0, IOADDR(A_MAC_REGISTER(s->sbe_idx, R_MAC_RMON_RX_BAD)));
-	__raw_writeq(0, IOADDR(A_MAC_REGISTER(s->sbe_idx, R_MAC_RMON_RX_GOOD)));
-	__raw_writeq(0, IOADDR(A_MAC_REGISTER(s->sbe_idx, R_MAC_RMON_RX_RUNT)));
-	__raw_writeq(0, IOADDR(A_MAC_REGISTER(s->sbe_idx, R_MAC_RMON_RX_OVERSIZE)));
-	__raw_writeq(0, IOADDR(A_MAC_REGISTER(s->sbe_idx, R_MAC_RMON_RX_FCS_ERROR)));
-	__raw_writeq(0, IOADDR(A_MAC_REGISTER(s->sbe_idx, R_MAC_RMON_RX_LENGTH_ERROR)));
-	__raw_writeq(0, IOADDR(A_MAC_REGISTER(s->sbe_idx, R_MAC_RMON_RX_CODE_ERROR)));
-	__raw_writeq(0, IOADDR(A_MAC_REGISTER(s->sbe_idx, R_MAC_RMON_RX_ALIGN_ERROR)));
+	__raw_writeq(0, s->sbm_base + R_MAC_RMON_TX_BYTES);
+	__raw_writeq(0, s->sbm_base + R_MAC_RMON_COLLISIONS);
+	__raw_writeq(0, s->sbm_base + R_MAC_RMON_LATE_COL);
+	__raw_writeq(0, s->sbm_base + R_MAC_RMON_EX_COL);
+	__raw_writeq(0, s->sbm_base + R_MAC_RMON_FCS_ERROR);
+	__raw_writeq(0, s->sbm_base + R_MAC_RMON_TX_ABORT);
+	__raw_writeq(0, s->sbm_base + R_MAC_RMON_TX_BAD);
+	__raw_writeq(0, s->sbm_base + R_MAC_RMON_TX_GOOD);
+	__raw_writeq(0, s->sbm_base + R_MAC_RMON_TX_RUNT);
+	__raw_writeq(0, s->sbm_base + R_MAC_RMON_TX_OVERSIZE);
+	__raw_writeq(0, s->sbm_base + R_MAC_RMON_RX_BYTES);
+	__raw_writeq(0, s->sbm_base + R_MAC_RMON_RX_MCAST);
+	__raw_writeq(0, s->sbm_base + R_MAC_RMON_RX_BCAST);
+	__raw_writeq(0, s->sbm_base + R_MAC_RMON_RX_BAD);
+	__raw_writeq(0, s->sbm_base + R_MAC_RMON_RX_GOOD);
+	__raw_writeq(0, s->sbm_base + R_MAC_RMON_RX_RUNT);
+	__raw_writeq(0, s->sbm_base + R_MAC_RMON_RX_OVERSIZE);
+	__raw_writeq(0, s->sbm_base + R_MAC_RMON_RX_FCS_ERROR);
+	__raw_writeq(0, s->sbm_base + R_MAC_RMON_RX_LENGTH_ERROR);
+	__raw_writeq(0, s->sbm_base + R_MAC_RMON_RX_CODE_ERROR);
+	__raw_writeq(0, s->sbm_base + R_MAC_RMON_RX_ALIGN_ERROR);
 
 	/*
 	 * initialize register pointers
@@ -953,7 +850,7 @@ static int sbdma_add_rcvbuffer(struct sb
 	if (sb == NULL) {
 		sb_new = dev_alloc_skb(ENET_PACKET_SIZE + SMP_CACHE_BYTES * 2 + ETHER_ALIGN);
 		if (sb_new == NULL) {
-			printk(KERN_INFO "%s: sk_buff allocation failed\n",
+			pr_info("%s: sk_buff allocation failed\n",
 			       d->sbdma_eth->sbm_dev->name);
 			return -ENOBUFS;
 		}
@@ -1472,14 +1369,6 @@ static int sbmac_initctx(struct sbmac_so
 	s->sbm_imr       = s->sbm_base + R_MAC_INT_MASK;
 	s->sbm_mdio      = s->sbm_base + R_MAC_MDIO;
 
-	s->sbm_phys[0]   = 1;
-	s->sbm_phys[1]   = 0;
-
-	s->sbm_phy_oldbmsr = 0;
-	s->sbm_phy_oldanlpar = 0;
-	s->sbm_phy_oldk1stsr = 0;
-	s->sbm_phy_oldlinkstat = 0;
-
 	/*
 	 * Initialize the DMA channels.  Right now, only one per MAC is used
 	 * Note: Only do this _once_, as it allocates memory from the kernel!
@@ -1494,14 +1383,6 @@ static int sbmac_initctx(struct sbmac_so
 
 	s->sbm_state = sbmac_state_off;
 
-	/*
-	 * Initial speed is (XXX TEMP) 10MBit/s HDX no FC
-	 */
-
-	s->sbm_speed = sbmac_speed_10;
-	s->sbm_duplex = sbmac_duplex_half;
-	s->sbm_fc = sbmac_fc_disabled;
-
 	return 0;
 }
 
@@ -2008,8 +1889,6 @@ static int sbmac_set_speed(struct sbmac_
 		cfg |= V_MAC_SPEED_SEL_1000MBPS | M_MAC_BURST_EN;
 		break;
 
-	case sbmac_speed_auto:		/* XXX not implemented */
-		/* fall through */
 	default:
 		return 0;
 	}
@@ -2083,8 +1962,6 @@ static int sbmac_set_duplex(struct sbmac
 			cfg |= M_MAC_HDX_EN | V_MAC_FC_CMD_ENAB_FALSECARR;
 			break;
 
-		case sbmac_fc_auto:		/* XXX not implemented */
-			/* fall through */
 		case sbmac_fc_frame:		/* not valid in half duplex */
 		default:			/* invalid selection */
 			return 0;
@@ -2103,15 +1980,12 @@ static int sbmac_set_duplex(struct sbmac
 
 		case sbmac_fc_collision:	/* not valid in full duplex */
 		case sbmac_fc_carrier:		/* not valid in full duplex */
-		case sbmac_fc_auto:		/* XXX not implemented */
-			/* fall through */
 		default:
 			return 0;
 		}
 		break;
-	case sbmac_duplex_auto:
-		/* XXX not implemented */
-		break;
+	default:
+		return 0;
 	}
 
 	/*
@@ -2390,7 +2264,7 @@ static int sb1250_change_mtu(struct net_
 	if (new_mtu >  ENET_PACKET_SIZE)
 		return -EINVAL;
 	_dev->mtu = new_mtu;
-	printk(KERN_INFO "changing the mtu to %d\n", new_mtu);
+	pr_info("changing the mtu to %d\n", new_mtu);
 	return 0;
 }
 
@@ -2406,20 +2280,17 @@ static int sb1250_change_mtu(struct net_
  *  	   status
  ********************************************************************* */
 
-static int sbmac_init(struct net_device *dev, int idx)
+static int sbmac_init(struct platform_device *pldev, long long base)
 {
-	struct sbmac_softc *sc;
+	struct net_device *dev = pldev->dev.driver_data;
+	int idx = pldev->id;
+	struct sbmac_softc *sc = netdev_priv(dev);
 	unsigned char *eaddr;
 	uint64_t ea_reg;
 	int i;
 	int err;
 	DECLARE_MAC_BUF(mac);
 
-	sc = netdev_priv(dev);
-
-	/* Determine controller base address */
-
-	sc->sbm_base = IOADDR(dev->base_addr);
 	sc->sbm_dev = dev;
 	sc->sbe_idx = idx;
 
@@ -2476,43 +2347,55 @@ static int sbmac_init(struct net_device 
 	dev->poll_controller = sbmac_netpoll;
 #endif
 
+	dev->irq		= UNIT_INT(idx);
+
 	/* This is needed for PASS2 for Rx H/W checksum feature */
 	sbmac_set_iphdr_offset(sc);
 
 	err = register_netdev(dev);
-	if (err)
-		goto out_uninit;
-
-	if (sc->rx_hw_checksum == ENABLE) {
-		printk(KERN_INFO "%s: enabling TCP rcv checksum\n",
-			sc->sbm_dev->name);
+	if (err) {
+		printk(KERN_ERR "%s.%d: unable to register netdev\n",
+		       sbmac_string, idx);
+		sbmac_uninitctx(sc);
+		return err;
 	}
 
+	pr_info("%s.%d: registered as %s\n", sbmac_string, idx, dev->name);
+
+	if (sc->rx_hw_checksum == ENABLE)
+		pr_info("%s: enabling TCP rcv checksum\n", dev->name);
+
 	/*
 	 * Display Ethernet address (this is called during the config
 	 * process so we need to finish off the config message that
 	 * was being displayed)
 	 */
-	printk(KERN_INFO
-	       "%s: SiByte Ethernet at 0x%08lX, address: %s\n",
-	       dev->name, dev->base_addr, print_mac(mac, eaddr));
+	pr_info("%s: SiByte Ethernet at 0x%08Lx, address: %s\n",
+	       dev->name, base, print_mac(mac, eaddr));
 
-	return 0;
+	sc->mii_bus.name = sbmac_mdio_string;
+	sc->mii_bus.id = idx;
+	sc->mii_bus.priv = sc;
+	sc->mii_bus.read = sbmac_mii_read;
+	sc->mii_bus.write = sbmac_mii_write;
+	sc->mii_bus.irq = sc->phy_irq;
+	for (i = 0; i < PHY_MAX_ADDR; ++i)
+		sc->mii_bus.irq[i] = SBMAC_PHY_INT;
 
-out_uninit:
-	sbmac_uninitctx(sc);
+	sc->mii_bus.dev = &pldev->dev;
+	dev_set_drvdata(&pldev->dev, &sc->mii_bus);
 
-	return err;
+	return 0;
 }
 
 
 static int sbmac_open(struct net_device *dev)
 {
 	struct sbmac_softc *sc = netdev_priv(dev);
+	int err;
 
-	if (debug > 1) {
-		printk(KERN_DEBUG "%s: sbmac_open() irq %d.\n", dev->name, dev->irq);
-	}
+	if (debug > 1)
+		pr_debug("%s: sbmac_open() irq %d.\n", dev->name, dev->irq);
 
 	/*
 	 * map/route interrupt (clear status first, in case something
@@ -2521,25 +2404,35 @@ static int sbmac_open(struct net_device 
 	 */
 
 	__raw_readq(sc->sbm_isr);
-	if (request_irq(dev->irq, &sbmac_intr, IRQF_SHARED, dev->name, dev))
-		return -EBUSY;
+	err = request_irq(dev->irq, &sbmac_intr, IRQF_SHARED, dev->name, dev);
+	if (err) {
+		printk(KERN_ERR "%s: unable to get IRQ %d\n", dev->name,
+		       dev->irq);
+		goto out_err;
+	}
 
 	/*
-	 * Probe phy address
+	 * Probe PHY address
 	 */
-
-	if(sbmac_mii_probe(dev) == -1) {
-		printk("%s: failed to probe PHY.\n", dev->name);
-		return -EINVAL;
+	err = mdiobus_register(&sc->mii_bus);
+	if (err) {
+		printk(KERN_ERR "%s: unable to register MDIO bus\n",
+		       dev->name);
+		goto out_unirq;
 	}
 
-	napi_enable(&sc->napi);
+	sc->sbm_speed = sbmac_speed_none;
+	sc->sbm_duplex = sbmac_duplex_none;
+	sc->sbm_fc = sbmac_fc_none;
+	sc->sbm_pause = -1;
+	sc->sbm_link = 0;
 
 	/*
-	 * Configure default speed
+	 * Attach to the PHY
 	 */
-
-	sbmac_mii_poll(sc,noisy_mii);
+	err = sbmac_mii_probe(dev);
+	if (err)
+		goto out_unregister;
 
 	/*
 	 * Turn on the channel
@@ -2547,200 +2440,133 @@ static int sbmac_open(struct net_device 
 
 	sbmac_set_channel_state(sc,sbmac_state_on);
 
-	/*
-	 * XXX Station address is in dev->dev_addr
-	 */
-
-	if (dev->if_port == 0)
-		dev->if_port = 0;
-
 	netif_start_queue(dev);
 
 	sbmac_set_rx_mode(dev);
 
-	/* Set the timer to check for link beat. */
-	init_timer(&sc->sbm_timer);
-	sc->sbm_timer.expires = jiffies + 2 * HZ/100;
-	sc->sbm_timer.data = (unsigned long)dev;
-	sc->sbm_timer.function = &sbmac_timer;
-	add_timer(&sc->sbm_timer);
+	phy_start(sc->phy_dev);
+
+	napi_enable(&sc->napi);
 
 	return 0;
+
+out_unregister:
+	mdiobus_unregister(&sc->mii_bus);
+
+out_unirq:
+	free_irq(dev->irq, dev);
+
+out_err:
+	return err;
 }
 
 static int sbmac_mii_probe(struct net_device *dev)
 {
+	struct sbmac_softc *sc = netdev_priv(dev);
+	struct phy_device *phy_dev;
 	int i;
-	struct sbmac_softc *s = netdev_priv(dev);
-	u16 bmsr, id1, id2;
-	u32 vendor, device;
-
-	for (i=1; i<31; i++) {
-	bmsr = sbmac_mii_read(s, i, MII_BMSR);
-		if (bmsr != 0) {
-			s->sbm_phys[0] = i;
-			id1 = sbmac_mii_read(s, i, MII_PHYIDR1);
-			id2 = sbmac_mii_read(s, i, MII_PHYIDR2);
-			vendor = ((u32)id1 << 6) | ((id2 >> 10) & 0x3f);
-			device = (id2 >> 4) & 0x3f;
-
-			printk(KERN_INFO "%s: found phy %d, vendor %06x part %02x\n",
-				dev->name, i, vendor, device);
-			return i;
-		}
-	}
-	return -1;
-}
-
 
-static int sbmac_mii_poll(struct sbmac_softc *s,int noisy)
-{
-    int bmsr,bmcr,k1stsr,anlpar;
-    int chg;
-    char buffer[100];
-    char *p = buffer;
-
-    /* Read the mode status and mode control registers. */
-    bmsr = sbmac_mii_read(s,s->sbm_phys[0],MII_BMSR);
-    bmcr = sbmac_mii_read(s,s->sbm_phys[0],MII_BMCR);
-
-    /* get the link partner status */
-    anlpar = sbmac_mii_read(s,s->sbm_phys[0],MII_ANLPAR);
-
-    /* if supported, read the 1000baseT register */
-    if (bmsr & BMSR_1000BT_XSR) {
-	k1stsr = sbmac_mii_read(s,s->sbm_phys[0],MII_K1STSR);
-	}
-    else {
-	k1stsr = 0;
+	for (i = 0; i < PHY_MAX_ADDR; i++) {
+		phy_dev = sc->mii_bus.phy_map[i];
+		if (phy_dev)
+			break;
 	}
+	if (!phy_dev) {
+		printk(KERN_ERR "%s: no PHY found\n", dev->name);
+		return -ENXIO;
+	}
+
+	phy_dev = phy_connect(dev, phy_dev->dev.bus_id, &sbmac_mii_poll, 0,
+			      PHY_INTERFACE_MODE_GMII);
+	if (IS_ERR(phy_dev)) {
+		printk(KERN_ERR "%s: could not attach to PHY\n", dev->name);
+		return PTR_ERR(phy_dev);
+	}
+
+	/* Remove any features not supported by the controller */
+	phy_dev->supported &= SUPPORTED_10baseT_Half |
+			      SUPPORTED_10baseT_Full |
+			      SUPPORTED_100baseT_Half |
+			      SUPPORTED_100baseT_Full |
+			      SUPPORTED_1000baseT_Half |
+			      SUPPORTED_1000baseT_Full |
+			      SUPPORTED_Autoneg |
+			      SUPPORTED_MII |
+			      SUPPORTED_Pause |
+			      SUPPORTED_Asym_Pause;
+	phy_dev->advertising = phy_dev->supported;
+
+	pr_info("%s: attached PHY driver [%s] (mii_bus:phy_addr=%s, irq=%d)\n",
+		dev->name, phy_dev->drv->name,
+		phy_dev->dev.bus_id, phy_dev->irq);
 
-    chg = 0;
+	sc->phy_dev = phy_dev;
 
-    if ((bmsr & BMSR_LINKSTAT) == 0) {
-	/*
-	 * If link status is down, clear out old info so that when
-	 * it comes back up it will force us to reconfigure speed
-	 */
-	s->sbm_phy_oldbmsr = 0;
-	s->sbm_phy_oldanlpar = 0;
-	s->sbm_phy_oldk1stsr = 0;
 	return 0;
-	}
-
-    if ((s->sbm_phy_oldbmsr != bmsr) ||
-	(s->sbm_phy_oldanlpar != anlpar) ||
-	(s->sbm_phy_oldk1stsr != k1stsr)) {
-	if (debug > 1) {
-	    printk(KERN_DEBUG "%s: bmsr:%x/%x anlpar:%x/%x  k1stsr:%x/%x\n",
-	       s->sbm_dev->name,
-	       s->sbm_phy_oldbmsr,bmsr,
-	       s->sbm_phy_oldanlpar,anlpar,
-	       s->sbm_phy_oldk1stsr,k1stsr);
-	    }
-	s->sbm_phy_oldbmsr = bmsr;
-	s->sbm_phy_oldanlpar = anlpar;
-	s->sbm_phy_oldk1stsr = k1stsr;
-	chg = 1;
-	}
-
-    if (chg == 0)
-	    return 0;
-
-    p += sprintf(p,"Link speed: ");
-
-    if (k1stsr & K1STSR_LP1KFD) {
-	s->sbm_speed = sbmac_speed_1000;
-	s->sbm_duplex = sbmac_duplex_full;
-	s->sbm_fc = sbmac_fc_frame;
-	p += sprintf(p,"1000BaseT FDX");
-	}
-    else if (k1stsr & K1STSR_LP1KHD) {
-	s->sbm_speed = sbmac_speed_1000;
-	s->sbm_duplex = sbmac_duplex_half;
-	s->sbm_fc = sbmac_fc_disabled;
-	p += sprintf(p,"1000BaseT HDX");
-	}
-    else if (anlpar & ANLPAR_TXFD) {
-	s->sbm_speed = sbmac_speed_100;
-	s->sbm_duplex = sbmac_duplex_full;
-	s->sbm_fc = (anlpar & ANLPAR_PAUSE) ? sbmac_fc_frame : sbmac_fc_disabled;
-	p += sprintf(p,"100BaseT FDX");
-	}
-    else if (anlpar & ANLPAR_TXHD) {
-	s->sbm_speed = sbmac_speed_100;
-	s->sbm_duplex = sbmac_duplex_half;
-	s->sbm_fc = sbmac_fc_disabled;
-	p += sprintf(p,"100BaseT HDX");
-	}
-    else if (anlpar & ANLPAR_10FD) {
-	s->sbm_speed = sbmac_speed_10;
-	s->sbm_duplex = sbmac_duplex_full;
-	s->sbm_fc = sbmac_fc_frame;
-	p += sprintf(p,"10BaseT FDX");
-	}
-    else if (anlpar & ANLPAR_10HD) {
-	s->sbm_speed = sbmac_speed_10;
-	s->sbm_duplex = sbmac_duplex_half;
-	s->sbm_fc = sbmac_fc_collision;
-	p += sprintf(p,"10BaseT HDX");
-	}
-    else {
-	p += sprintf(p,"Unknown");
-	}
-
-    if (noisy) {
-	    printk(KERN_INFO "%s: %s\n",s->sbm_dev->name,buffer);
-	    }
-
-    return 1;
 }
 
 
-static void sbmac_timer(unsigned long data)
+static void sbmac_mii_poll(struct net_device *dev)
 {
-	struct net_device *dev = (struct net_device *)data;
 	struct sbmac_softc *sc = netdev_priv(dev);
-	int next_tick = HZ;
-	int mii_status;
-
-	spin_lock_irq (&sc->sbm_lock);
-
-	/* make IFF_RUNNING follow the MII status bit "Link established" */
-	mii_status = sbmac_mii_read(sc, sc->sbm_phys[0], MII_BMSR);
+	struct phy_device *phy_dev = sc->phy_dev;
+	unsigned long flags;
+	enum sbmac_fc fc;
+	int link_chg, speed_chg, duplex_chg, pause_chg, fc_chg;
 
-	if ( (mii_status & BMSR_LINKSTAT) != (sc->sbm_phy_oldlinkstat) ) {
-    	        sc->sbm_phy_oldlinkstat = mii_status & BMSR_LINKSTAT;
-		if (mii_status & BMSR_LINKSTAT) {
-			netif_carrier_on(dev);
-		}
-		else {
-			netif_carrier_off(dev);
+	link_chg = (sc->sbm_link != phy_dev->link);
+	speed_chg = (sc->sbm_speed != phy_dev->speed);
+	duplex_chg = (sc->sbm_duplex != phy_dev->duplex);
+	pause_chg = (sc->sbm_pause != phy_dev->pause);
+
+	if (!link_chg && !speed_chg && !duplex_chg && !pause_chg)
+		return;					/* Hmmm... */
+
+	if (!phy_dev->link) {
+		if (link_chg) {
+			sc->sbm_link = phy_dev->link;
+			sc->sbm_speed = sbmac_speed_none;
+			sc->sbm_duplex = sbmac_duplex_none;
+			sc->sbm_fc = sbmac_fc_disabled;
+			sc->sbm_pause = -1;
+			pr_info("%s: link unavailable\n", dev->name);
 		}
+		return;
 	}
 
-	/*
-	 * Poll the PHY to see what speed we should be running at
-	 */
+	if (phy_dev->duplex == DUPLEX_FULL) {
+		if (phy_dev->pause)
+			fc = sbmac_fc_frame;
+		else
+			fc = sbmac_fc_disabled;
+	} else
+		fc = sbmac_fc_collision;
+	fc_chg = (sc->sbm_fc != fc);
 
-	if (sbmac_mii_poll(sc,noisy_mii)) {
-		if (sc->sbm_state != sbmac_state_off) {
-			/*
-			 * something changed, restart the channel
-			 */
-			if (debug > 1) {
-				printk("%s: restarting channel because speed changed\n",
-				       sc->sbm_dev->name);
-			}
-			sbmac_channel_stop(sc);
-			sbmac_channel_start(sc);
-		}
-	}
+	pr_info("%s: link available: %dbase-%cD\n", dev->name, phy_dev->speed,
+		phy_dev->duplex == DUPLEX_FULL ? 'F' : 'H');
 
-	spin_unlock_irq (&sc->sbm_lock);
+	spin_lock_irqsave(&sc->sbm_lock, flags);
+
+	sc->sbm_speed = phy_dev->speed;
+	sc->sbm_duplex = phy_dev->duplex;
+	sc->sbm_fc = fc;
+	sc->sbm_pause = phy_dev->pause;
+	sc->sbm_link = phy_dev->link;
 
-	sc->sbm_timer.expires = jiffies + next_tick;
-	add_timer(&sc->sbm_timer);
+	if ((speed_chg || duplex_chg || fc_chg) &&
+	    sc->sbm_state != sbmac_state_off) {
+		/*
+		 * something changed, restart the channel
+		 */
+		if (debug > 1)
+			pr_debug("%s: restarting channel "
+				 "because PHY state changed\n", dev->name);
+		sbmac_channel_stop(sc);
+		sbmac_channel_start(sc);
+	}
+
+	spin_unlock_irqrestore(&sc->sbm_lock, flags);
 }
 
 
@@ -2793,64 +2619,34 @@ static void sbmac_set_rx_mode(struct net
 static int sbmac_mii_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 {
 	struct sbmac_softc *sc = netdev_priv(dev);
-	u16 *data = (u16 *)&rq->ifr_ifru;
-	unsigned long flags;
-	int retval;
-
-	spin_lock_irqsave(&sc->sbm_lock, flags);
-	retval = 0;
 
-	switch(cmd) {
-	case SIOCDEVPRIVATE:		/* Get the address of the PHY in use. */
-		data[0] = sc->sbm_phys[0] & 0x1f;
-		/* Fall Through */
-	case SIOCDEVPRIVATE+1:		/* Read the specified MII register. */
-		data[3] = sbmac_mii_read(sc, data[0] & 0x1f, data[1] & 0x1f);
-		break;
-	case SIOCDEVPRIVATE+2:		/* Write the specified MII register */
-		if (!capable(CAP_NET_ADMIN)) {
-			retval = -EPERM;
-			break;
-		}
-		if (debug > 1) {
-		    printk(KERN_DEBUG "%s: sbmac_mii_ioctl: write %02X %02X %02X\n",dev->name,
-		       data[0],data[1],data[2]);
-		    }
-		sbmac_mii_write(sc, data[0] & 0x1f, data[1] & 0x1f, data[2]);
-		break;
-	default:
-		retval = -EOPNOTSUPP;
-	}
+	if (!netif_running(dev) || !sc->phy_dev)
+		return -EINVAL;
 
-	spin_unlock_irqrestore(&sc->sbm_lock, flags);
-	return retval;
+	return phy_mii_ioctl(sc->phy_dev, if_mii(rq), cmd);
 }
 
 static int sbmac_close(struct net_device *dev)
 {
 	struct sbmac_softc *sc = netdev_priv(dev);
-	unsigned long flags;
-	int irq;
 
 	napi_disable(&sc->napi);
 
-	sbmac_set_channel_state(sc,sbmac_state_off);
+	phy_stop(sc->phy_dev);
 
-	del_timer_sync(&sc->sbm_timer);
-
-	spin_lock_irqsave(&sc->sbm_lock, flags);
+	sbmac_set_channel_state(sc, sbmac_state_off);
 
 	netif_stop_queue(dev);
 
-	if (debug > 1) {
-		printk(KERN_DEBUG "%s: Shutting down ethercard\n",dev->name);
-	}
+	if (debug > 1)
+		pr_debug("%s: Shutting down ethercard\n", dev->name);
 
-	spin_unlock_irqrestore(&sc->sbm_lock, flags);
+	phy_disconnect(sc->phy_dev);
+	sc->phy_dev = NULL;
+
+	mdiobus_unregister(&sc->mii_bus);
 
-	irq = dev->irq;
-	synchronize_irq(irq);
-	free_irq(irq, dev);
+	free_irq(dev->irq, dev);
 
 	sbdma_emptyring(&(sc->sbm_txdma));
 	sbdma_emptyring(&(sc->sbm_rxdma));
@@ -2883,54 +2679,195 @@ static int sbmac_poll(struct napi_struct
 	return work_done;
 }
 
+
+static int __init sbmac_probe(struct platform_device *pldev)
+{
+	struct net_device *dev;
+	struct sbmac_softc *sc;
+	void __iomem *sbm_base;
+	struct resource *res;
+	u64 sbmac_orig_hwaddr;
+	int err;
+
+	res = platform_get_resource(pldev, IORESOURCE_MEM, 0);
+	BUG_ON(!res);
+	sbm_base = ioremap_nocache(res->start, res->end - res->start + 1);
+	if (!sbm_base) {
+		printk(KERN_ERR "%s: unable to map device registers\n",
+		       pldev->dev.bus_id);
+		err = -ENOMEM;
+		goto out_out;
+	}
+
+	/*
+	 * The R_MAC_ETHERNET_ADDR register will be set to some nonzero
+	 * value for us by the firmware if we're going to use this MAC.
+	 * If we find a zero, skip this MAC.
+	 */
+	sbmac_orig_hwaddr = __raw_readq(sbm_base + R_MAC_ETHERNET_ADDR);
+	pr_debug("%s: %sconfiguring MAC at 0x%08Lx\n", pldev->dev.bus_id,
+		 sbmac_orig_hwaddr ? "" : "not ", (long long)res->start);
+	if (sbmac_orig_hwaddr == 0) {
+		err = 0;
+		goto out_unmap;
+	}
+
+	/*
+	 * Okay, cool.  Initialize this MAC.
+	 */
+	dev = alloc_etherdev(sizeof(struct sbmac_softc));
+	if (!dev) {
+		printk(KERN_ERR "%s: unable to allocate etherdev\n",
+		       pldev->dev.bus_id);
+		err = -ENOMEM;
+		goto out_unmap;
+	}
+
+	pldev->dev.driver_data = dev;
+	SET_NETDEV_DEV(dev, &pldev->dev);
+
+	sc = netdev_priv(dev);
+	sc->sbm_base = sbm_base;
+
+	err = sbmac_init(pldev, res->start);
+	if (err)
+		goto out_kfree;
+
+	return 0;
+
+out_kfree:
+	free_netdev(dev);
+	__raw_writeq(sbmac_orig_hwaddr, sbm_base + R_MAC_ETHERNET_ADDR);
+
+out_unmap:
+	iounmap(sbm_base);
+
+out_out:
+	return err;
+}
+
+static int __exit sbmac_remove(struct platform_device *pldev)
+{
+	struct net_device *dev = pldev->dev.driver_data;
+	struct sbmac_softc *sc = netdev_priv(dev);
+
+	unregister_netdev(dev);
+	sbmac_uninitctx(sc);
+	iounmap(sc->sbm_base);
+	free_netdev(dev);
+
+	return 0;
+}
+
+
+static struct platform_device **sbmac_pldev;
+static int sbmac_max_units;
+
 #if defined(SBMAC_ETH0_HWADDR) || defined(SBMAC_ETH1_HWADDR) || defined(SBMAC_ETH2_HWADDR) || defined(SBMAC_ETH3_HWADDR)
-static void
-sbmac_setup_hwaddr(int chan,char *addr)
+static void __init sbmac_setup_hwaddr(int idx, char *addr)
 {
+	void __iomem *sbm_base;
+	unsigned long start, end;
 	uint8_t eaddr[6];
 	uint64_t val;
-	unsigned long port;
 
-	port = A_MAC_CHANNEL_BASE(chan);
-	sbmac_parse_hwaddr(addr,eaddr);
+	if (idx >= sbmac_max_units)
+		return;
+
+	start = A_MAC_CHANNEL_BASE(idx);
+	end = A_MAC_CHANNEL_BASE(idx + 1) - 1;
+
+	sbm_base = ioremap_nocache(start, end - start + 1);
+	if (!sbm_base) {
+		printk(KERN_ERR "%s: unable to map device registers\n",
+		       sbmac_string);
+		return;
+	}
+
+	sbmac_parse_hwaddr(addr, eaddr);
 	val = sbmac_addr2reg(eaddr);
-	__raw_writeq(val, IOADDR(port+R_MAC_ETHERNET_ADDR));
-	val = __raw_readq(IOADDR(port+R_MAC_ETHERNET_ADDR));
+	__raw_writeq(val, sbm_base + R_MAC_ETHERNET_ADDR);
+	val = __raw_readq(sbm_base + R_MAC_ETHERNET_ADDR);
+
+	iounmap(sbm_base);
 }
 #endif
 
-static struct net_device *dev_sbmac[MAX_UNITS];
+static int __init sbmac_platform_probe_one(int idx)
+{
+	struct platform_device *pldev;
+	struct {
+		struct resource r;
+		char name[strlen(sbmac_pretty) + 4];
+	} *res;
+	int err;
 
-static int __init
-sbmac_init_module(void)
+	res = kzalloc(sizeof(*res), GFP_KERNEL);
+	if (!res) {
+		printk(KERN_ERR "%s.%d: unable to allocate memory\n",
+		       sbmac_string, idx);
+		err = -ENOMEM;
+		goto out_err;
+	}
+
+	/*
+	 * This is the base address of the MAC.
+	 */
+	snprintf(res->name, sizeof(res->name), "%s %d", sbmac_pretty, idx);
+	res->r.name = res->name;
+	res->r.flags = IORESOURCE_MEM;
+	res->r.start = A_MAC_CHANNEL_BASE(idx);
+	res->r.end = A_MAC_CHANNEL_BASE(idx + 1) - 1;
+
+	pldev = platform_device_register_simple(sbmac_string, idx, &res->r, 1);
+	if (IS_ERR(pldev)) {
+		printk(KERN_ERR "%s.%d: unable to register platform device\n",
+		       sbmac_string, idx);
+		err = PTR_ERR(pldev);
+		goto out_kfree;
+	}
+
+	if (!pldev->dev.driver) {
+		err = 0;		/* No hardware at this address. */
+		goto out_unregister;
+	}
+
+	sbmac_pldev[idx] = pldev;
+	return 0;
+
+out_unregister:
+	platform_device_unregister(pldev);
+
+out_kfree:
+	kfree(res);
+
+out_err:
+	return err;
+}
+
+static void __init sbmac_platform_probe(void)
 {
-	int idx;
-	struct net_device *dev;
-	unsigned long port;
-	int chip_max_units;
+	int i;
 
 	/* Set the number of available units based on the SOC type.  */
 	switch (soc_type) {
 	case K_SYS_SOC_TYPE_BCM1250:
 	case K_SYS_SOC_TYPE_BCM1250_ALT:
-		chip_max_units = 3;
+		sbmac_max_units = 3;
 		break;
 	case K_SYS_SOC_TYPE_BCM1120:
 	case K_SYS_SOC_TYPE_BCM1125:
 	case K_SYS_SOC_TYPE_BCM1125H:
-	case K_SYS_SOC_TYPE_BCM1250_ALT2: /* Hybrid */
-		chip_max_units = 2;
+	case K_SYS_SOC_TYPE_BCM1250_ALT2:	/* Hybrid */
+		sbmac_max_units = 2;
 		break;
 	case K_SYS_SOC_TYPE_BCM1x55:
 	case K_SYS_SOC_TYPE_BCM1x80:
-		chip_max_units = 4;
+		sbmac_max_units = 4;
 		break;
 	default:
-		chip_max_units = 0;
-		break;
+		return;				/* none */
 	}
-	if (chip_max_units > MAX_UNITS)
-		chip_max_units = MAX_UNITS;
 
 	/*
 	 * For bringup when not using the firmware, we can pre-fill
@@ -2938,89 +2875,71 @@ sbmac_init_module(void)
 	 * specified in this file (or maybe from the config file?)
 	 */
 #ifdef SBMAC_ETH0_HWADDR
-	if (chip_max_units > 0)
-	  sbmac_setup_hwaddr(0,SBMAC_ETH0_HWADDR);
+	sbmac_setup_hwaddr(0, SBMAC_ETH0_HWADDR);
 #endif
 #ifdef SBMAC_ETH1_HWADDR
-	if (chip_max_units > 1)
-	  sbmac_setup_hwaddr(1,SBMAC_ETH1_HWADDR);
+	sbmac_setup_hwaddr(1, SBMAC_ETH1_HWADDR);
 #endif
 #ifdef SBMAC_ETH2_HWADDR
-	if (chip_max_units > 2)
-	  sbmac_setup_hwaddr(2,SBMAC_ETH2_HWADDR);
+	sbmac_setup_hwaddr(2, SBMAC_ETH2_HWADDR);
 #endif
 #ifdef SBMAC_ETH3_HWADDR
-	if (chip_max_units > 3)
-	  sbmac_setup_hwaddr(3,SBMAC_ETH3_HWADDR);
+	sbmac_setup_hwaddr(3, SBMAC_ETH3_HWADDR);
 #endif
 
+	sbmac_pldev = kcalloc(sbmac_max_units, sizeof(*sbmac_pldev),
+			      GFP_KERNEL);
+	if (!sbmac_pldev) {
+		printk(KERN_ERR "%s: unable to allocate memory\n",
+		       sbmac_string);
+		return;
+	}
+
 	/*
 	 * Walk through the Ethernet controllers and find
 	 * those who have their MAC addresses set.
 	 */
-	for (idx = 0; idx < chip_max_units; idx++) {
+	for (i = 0; i < sbmac_max_units; i++)
+		if (sbmac_platform_probe_one(i))
+			break;
+}
 
-	        /*
-	         * This is the base address of the MAC.
-		 */
 
-	        port = A_MAC_CHANNEL_BASE(idx);
+static void __exit sbmac_platform_cleanup(void)
+{
+	int i;
 
-		/*
-		 * The R_MAC_ETHERNET_ADDR register will be set to some nonzero
-		 * value for us by the firmware if we are going to use this MAC.
-		 * If we find a zero, skip this MAC.
-		 */
+	for (i = 0; i < sbmac_max_units; i++)
+		platform_device_unregister(sbmac_pldev[i]);
+	kfree(sbmac_pldev);
+}
 
-		sbmac_orig_hwaddr[idx] = __raw_readq(IOADDR(port+R_MAC_ETHERNET_ADDR));
-		if (sbmac_orig_hwaddr[idx] == 0) {
-			printk(KERN_DEBUG "sbmac: not configuring MAC at "
-			       "%lx\n", port);
-		    continue;
-		}
 
-		/*
-		 * Okay, cool.  Initialize this MAC.
-		 */
+static struct platform_driver sbmac_driver = {
+	.probe = sbmac_probe,
+	.remove = __exit_p(sbmac_remove),
+	.driver = {
+		.name = sbmac_string,
+	},
+};
 
-		dev = alloc_etherdev(sizeof(struct sbmac_softc));
-		if (!dev)
-			return -ENOMEM;
-
-		printk(KERN_DEBUG "sbmac: configuring MAC at %lx\n", port);
-
-		dev->irq = UNIT_INT(idx);
-		dev->base_addr = port;
-		dev->mem_end = 0;
-		if (sbmac_init(dev, idx)) {
-			port = A_MAC_CHANNEL_BASE(idx);
-			__raw_writeq(sbmac_orig_hwaddr[idx], IOADDR(port+R_MAC_ETHERNET_ADDR));
-			free_netdev(dev);
-			continue;
-		}
-		dev_sbmac[idx] = dev;
-	}
-	return 0;
-}
+static int __init sbmac_init_module(void)
+{
+	int err;
 
+	err = platform_driver_register(&sbmac_driver);
+	if (err)
+		return err;
 
-static void __exit
-sbmac_cleanup_module(void)
-{
-	struct net_device *dev;
-	int idx;
+	sbmac_platform_probe();
 
-	for (idx = 0; idx < MAX_UNITS; idx++) {
-		struct sbmac_softc *sc;
-		dev = dev_sbmac[idx];
-		if (!dev)
-			continue;
+	return err;
+}
 
-		sc = netdev_priv(dev);
-		unregister_netdev(dev);
-		sbmac_uninitctx(sc);
-		free_netdev(dev);
-	}
+static void __exit sbmac_cleanup_module(void)
+{
+	sbmac_platform_cleanup();
+	platform_driver_unregister(&sbmac_driver);
 }
 
 module_init(sbmac_init_module);

^ permalink raw reply

* Re: change the way e1000 is handling short VLAN frames
From: jamal @ 2007-09-21 11:59 UTC (permalink / raw)
  To: Emil Micek; +Cc: netdev, auke-jan.h.kok
In-Reply-To: <1190359905.3551.28.camel@magi.nadr>

On Fri, 2007-21-09 at 09:31 +0200, Emil Micek wrote:
> Hello list,
> 
> I'd like to change behaviour of e1000 module when transmiting short
> ethernet frames (shorter then 64 bytes) trough VLAN interface.
> 
> Current e1000 (according to our observations) first appends 4 bytes of
> VLAN tag and then pads the frame to 64 bytes with zeroes if necessary
> before transmiting it.

Which would make it a bug. AFAIK, the minimum VLAN tagged packet going
out is 68 bytes.

> I'd like to change it so it first pads the frame to 64 B and appends the
> VLAN tag afterwards, so the resulting VLAN frame would be 68 bytes
> minimum.

If you were able to figure this much out: What have you tried and what
do you still need help on? CCing some of the e1000 people(done) will
probably help you get this resolved faster.

cheers,
jamal


^ permalink raw reply

* Re: [LARTC] ifb and ppp
From: Patrick McHardy @ 2007-09-21 11:56 UTC (permalink / raw)
  To: hadi; +Cc: Frithjof Hammer, Linux Netdev List
In-Reply-To: <1190373789.4261.16.camel@localhost>

jamal wrote:
> On Thu, 2007-20-09 at 17:26 +0200, Patrick McHardy wrote:
> 
>> I don't see a good solution for this that
>>allows to keep the iptables rules, I'd suggest to switch to ematches.
> 
> 
> One approach could be to use ipt action:
> 
> -------------------
> tc filter add dev ppp0 parent ffff: protocol ip u32 match u32/ematch
> some match flowid 1:1 action ipt -j mark --set-mark 1
> ..
> ...
> ....
> iptables here to use the marks ...


This doesn't help much since he uses the iptables marks for
classification on the ifb device, so he might as well just
classify directly using u32. I think it would be nice to
have an ematch equivalent to the ipt action for matches.
Should be pretty easy to write (slightly above 60 seconds
according to the documentation :)).

^ permalink raw reply

* Re: [LARTC] ifb and ppp
From: jamal @ 2007-09-21 12:14 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: Frithjof Hammer, Linux Netdev List
In-Reply-To: <46F3B16F.1060909@trash.net>

On Fri, 2007-21-09 at 13:56 +0200, Patrick McHardy wrote:

> This doesn't help much since he uses the iptables marks for
> classification on the ifb device, so he might as well just
> classify directly using u32. 

true.

> I think it would be nice to
> have an ematch equivalent to the ipt action for matches.

yes, that would help in his case.

> Should be pretty easy to write 
> (slightly above 60 seconds
> according to the documentation :)).

Ah, i hadnt paid attention to that before;-> I'd like to use
that whole thing on a tshirt.

cheers,
jamal


^ permalink raw reply

* Re: wrong arp query with policy routing
From: jamal @ 2007-09-21 12:25 UTC (permalink / raw)
  To: Marco Berizzi; +Cc: Chuck Ebbert, netdev
In-Reply-To: <BAY103-DAV1014588479A3FF5B66EEDDB2BB0@phx.gbl>


On Fri, 2007-21-09 at 11:08 +0200, Marco Berizzi wrote:

> thanks for the reply.
> I have tried to 'echo 1 > /proc/sys/net/ipv4/conf/eth0',
> but the 'arp whos-has' behaviour doesn't change.
> Other hints?

Give a man a fish and he'll eat for a day....

Documentation/networking/ip-sysctl.txt

cheers,
jamal




^ permalink raw reply

* [RFC/PATCH 0/3] UDP memory usage accounting
From: Satoshi OSHIMA @ 2007-09-21 12:18 UTC (permalink / raw)
  To: netdev; +Cc: haoki, 吉藤 英明

This patch set try to introduce memory usage accounting for
UDP(currently ipv4 only).

Currently, memory usage of UDP can be observed as the sam of
usage of tx_queue and rx_queue. But I believe that the system
wide accounting is usefull when heavy loaded condition.

In the next step, I would like to add memory usage quota
for UDP to avoid unlimited memory consumption problem
under DDOS attack.

This patch set is for 2.6.23-rc7.

Unfortunately, I don't have any NIC with UFO.
So this patch set is not tested with UFO supported
device.

I appreciate your comment/test/feedback.

Satoshi Oshima

^ permalink raw reply

* [RFC/PATCH 2/3] UDP memory usage accounting: accounting unit and variable
From: Satoshi OSHIMA @ 2007-09-21 12:26 UTC (permalink / raw)
  To: netdev; +Cc: haoki, 吉藤 英明

This patch introduces global variable for UDP memory accounting.
The unit is page.


signed-off-by: Satoshi Oshima <satoshi.oshima.fk@hitachi.com>
signed-off-by: Hideo Aoki <haoki@redhat.com>

Index: 2.6.23-rc3-udp_limit/include/net/sock.h
===================================================================
--- 2.6.23-rc3-udp_limit.orig/include/net/sock.h
+++ 2.6.23-rc3-udp_limit/include/net/sock.h
@@ -723,6 +723,13 @@ static inline int sk_stream_wmem_schedul
sk_stream_mem_schedule(sk, size, 0);
}

+#define SK_DATAGRAM_MEM_QUANTUM ((int)PAGE_SIZE)
+
+static inline int sk_datagram_pages(int amt)
+{
+ return DIV_ROUND_UP(amt, SK_DATAGRAM_MEM_QUANTUM);
+}
+
/* Used by processes to "lock" a socket state, so that
* interrupts and bottom half handlers won't change it
* from under us. It essentially blocks any incoming
Index: 2.6.23-rc3-udp_limit/include/net/udp.h
===================================================================
--- 2.6.23-rc3-udp_limit.orig/include/net/udp.h
+++ 2.6.23-rc3-udp_limit/include/net/udp.h
@@ -65,6 +65,8 @@ extern rwlock_t udp_hash_lock;

extern struct proto udp_prot;

+extern atomic_t udp_memory_allocated;
+
struct sk_buff;

/*
Index: 2.6.23-rc3-udp_limit/net/ipv4/proc.c
===================================================================
--- 2.6.23-rc3-udp_limit.orig/net/ipv4/proc.c
+++ 2.6.23-rc3-udp_limit/net/ipv4/proc.c
@@ -66,7 +66,8 @@ static int sockstat_seq_show(struct seq_
fold_prot_inuse(&tcp_prot), atomic_read(&tcp_orphan_count),
tcp_death_row.tw_count, atomic_read(&tcp_sockets_allocated),
atomic_read(&tcp_memory_allocated));
- seq_printf(seq, "UDP: inuse %d\n", fold_prot_inuse(&udp_prot));
+ seq_printf(seq, "UDP: inuse %d mem %d\n", fold_prot_inuse(&udp_prot),
+ atomic_read(&udp_memory_allocated));
seq_printf(seq, "UDPLITE: inuse %d\n", fold_prot_inuse(&udplite_prot));
seq_printf(seq, "RAW: inuse %d\n", fold_prot_inuse(&raw_prot));
seq_printf(seq, "FRAG: inuse %d memory %d\n", ip_frag_nqueues,
Index: 2.6.23-rc3-udp_limit/net/ipv4/udp.c
===================================================================
--- 2.6.23-rc3-udp_limit.orig/net/ipv4/udp.c
+++ 2.6.23-rc3-udp_limit/net/ipv4/udp.c
@@ -113,6 +113,10 @@ DEFINE_SNMP_STAT(struct udp_mib, udp_sta
struct hlist_head udp_hash[UDP_HTABLE_SIZE];
DEFINE_RWLOCK(udp_hash_lock);

+atomic_t udp_memory_allocated;
+
+EXPORT_SYMBOL(udp_memory_allocated);
+
static int udp_port_rover;

static inline int __udp_lib_lport_inuse(__u16 num, struct hlist_head
udptable[])


^ permalink raw reply

* [RFC/PATCH 3/3] UDP memory usage accounting: measurement
From: Satoshi OSHIMA @ 2007-09-21 12:28 UTC (permalink / raw)
  To: netdev; +Cc: haoki, 吉藤 英明

This patch introduces memory usage measurement for UDP.


signed-off-by: Satoshi Oshima <satoshi.oshima.fk@hitachi.com>
signed-off-by: Hideo Aoki <haoki@redhat.com>

Index: 2.6.23-rc7-udp_limit/net/ipv4/ip_output.c
===================================================================
--- 2.6.23-rc7-udp_limit.orig/net/ipv4/ip_output.c
+++ 2.6.23-rc7-udp_limit/net/ipv4/ip_output.c
@@ -743,6 +743,8 @@ static inline int ip_ufo_append_data(str
 		/* specify the length of each IP datagram fragment*/
 		skb_shinfo(skb)->gso_size = mtu - fragheaderlen;
 		skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
+		atomic_add(sk_datagram_pages(skb->truesize),
+			   sk->sk_prot->memory_allocated);
 		__skb_queue_tail(&sk->sk_write_queue, skb);
 
 		return 0;
@@ -924,6 +926,9 @@ alloc_new_skb:
 			}
 			if (skb == NULL)
 				goto error;
+			if (sk->sk_protocol == IPPROTO_UDP)
+				atomic_add(sk_datagram_pages(skb->truesize),
+					   sk->sk_prot->memory_allocated);
 
 			/*
 			 *	Fill in the control structures
@@ -1023,6 +1028,8 @@ alloc_new_skb:
 				frag = &skb_shinfo(skb)->frags[i];
 				skb->truesize += PAGE_SIZE;
 				atomic_add(PAGE_SIZE, &sk->sk_wmem_alloc);
+				if (sk->sk_protocol == IPPROTO_UDP)
+					atomic_inc(sk->sk_prot->memory_allocated);
 			} else {
 				err = -EMSGSIZE;
 				goto error;
@@ -1123,7 +1130,9 @@ ssize_t	ip_append_page(struct sock *sk, 
 			if (unlikely(!skb)) {
 				err = -ENOBUFS;
 				goto error;
-			}
+			} else if (sk->sk_protocol == IPPROTO_UDP)
+				atomic_add(sk_datagram_pages(skb->truesize),
+					   sk->sk_prot->memory_allocated);
 
 			/*
 			 *	Fill in the control structures
@@ -1152,6 +1161,8 @@ ssize_t	ip_append_page(struct sock *sk, 
 			/*
 			 * Put the packet on the pending queue.
 			 */
+			atomic_add(sk_datagram_pages(skb->truesize),
+				   sk->sk_prot->memory_allocated);
 			__skb_queue_tail(&sk->sk_write_queue, skb);
 			continue;
 		}
@@ -1202,13 +1213,14 @@ int ip_push_pending_frames(struct sock *
 	struct iphdr *iph;
 	__be16 df = 0;
 	__u8 ttl;
-	int err = 0;
+	int err = 0, send_page_size;
 
 	if ((skb = __skb_dequeue(&sk->sk_write_queue)) == NULL)
 		goto out;
 	tail_skb = &(skb_shinfo(skb)->frag_list);
 
 	/* move skb->data to ip header from ext header */
+	send_page_size = sk_datagram_pages(skb->truesize);
 	if (skb->data < skb_network_header(skb))
 		__skb_pull(skb, skb_network_offset(skb));
 	while ((tmp_skb = __skb_dequeue(&sk->sk_write_queue)) != NULL) {
@@ -1218,6 +1230,7 @@ int ip_push_pending_frames(struct sock *
 		skb->len += tmp_skb->len;
 		skb->data_len += tmp_skb->len;
 		skb->truesize += tmp_skb->truesize;
+		send_page_size += sk_datagram_pages(tmp_skb->truesize);
 		__sock_put(tmp_skb->sk);
 		tmp_skb->destructor = NULL;
 		tmp_skb->sk = NULL;
@@ -1269,6 +1282,8 @@ int ip_push_pending_frames(struct sock *
 	/* Netfilter gets whole the not fragmented skb. */
 	err = NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, skb, NULL,
 		      skb->dst->dev, dst_output);
+	if (sk->sk_protocol == IPPROTO_UDP)
+		atomic_sub(send_page_size, sk->sk_prot->memory_allocated);
 	if (err) {
 		if (err > 0)
 			err = inet->recverr ? net_xmit_errno(err) : 0;
@@ -1299,8 +1314,12 @@ void ip_flush_pending_frames(struct sock
 	struct inet_sock *inet = inet_sk(sk);
 	struct sk_buff *skb;
 
-	while ((skb = __skb_dequeue_tail(&sk->sk_write_queue)) != NULL)
+	while ((skb = __skb_dequeue_tail(&sk->sk_write_queue)) != NULL) {
+		if (sk->sk_protocol == IPPROTO_UDP)
+			atomic_sub(sk_datagram_pages(skb->truesize),
+				   sk->sk_prot->memory_allocated);
 		kfree_skb(skb);
+	}
 
 	inet->cork.flags &= ~IPCORK_OPT;
 	kfree(inet->cork.opt);
Index: 2.6.23-rc7-udp_limit/net/ipv4/udp.c
===================================================================
--- 2.6.23-rc7-udp_limit.orig/net/ipv4/udp.c
+++ 2.6.23-rc7-udp_limit/net/ipv4/udp.c
@@ -887,6 +887,9 @@ try_again:
 		err = ulen;
 
 out_free:
+	atomic_sub(sk_datagram_pages(skb->truesize),
+		   sk->sk_prot->memory_allocated);
+
 	skb_free_datagram(sk, skb);
 out:
 	return err;
@@ -1019,6 +1022,9 @@ int udp_queue_rcv_skb(struct sock * sk, 
 		goto drop;
 	}
 
+	atomic_add(sk_datagram_pages(skb->truesize), 
+		   sk->sk_prot->memory_allocated);
+
 	UDP_INC_STATS_BH(UDP_MIB_INDATAGRAMS, up->pcflag);
 	return 0;
 
@@ -1443,6 +1449,7 @@ struct proto udp_prot = {
 	.hash		   = udp_lib_hash,
 	.unhash		   = udp_lib_unhash,
 	.get_port	   = udp_v4_get_port,
+	.memory_allocated  = &udp_memory_allocated,
 	.obj_size	   = sizeof(struct udp_sock),
 #ifdef CONFIG_COMPAT
 	.compat_setsockopt = compat_udp_setsockopt,


^ permalink raw reply

* [RFC/PATCH 1/3] UDP memory usage accounting: fix send buffer check
From: Satoshi OSHIMA @ 2007-09-21 12:22 UTC (permalink / raw)
  To: netdev; +Cc: haoki, 吉藤 英明

This patch introduces sndbuf size check before
memory allcation for send buffer.


signed-off-by: Satoshi Oshima <satoshi.oshima.fk@hitachi.com>
signed-off-by: Hideo Aoki <haoki@redhat.com>

Index: 2.6.23-rc7-udp_limit/net/ipv4/ip_output.c
===================================================================
--- 2.6.23-rc7-udp_limit.orig/net/ipv4/ip_output.c
+++ 2.6.23-rc7-udp_limit/net/ipv4/ip_output.c
@@ -1004,6 +1004,11 @@ alloc_new_skb:
 					frag = &skb_shinfo(skb)->frags[i];
 				}
 			} else if (i < MAX_SKB_FRAGS) {
+				if (atomic_read(&sk->sk_wmem_alloc) + PAGE_SIZE
+				    > 2 * sk->sk_sndbuf) {
+					err = -ENOBUFS;
+					goto error;
+				}
 				if (copy > PAGE_SIZE)
 					copy = PAGE_SIZE;
 				page = alloc_pages(sk->sk_allocation, 0);


^ permalink raw reply

* Re: [RFC PATCH] 2.6.22.6 netfilter: sk_setup_caps in ip_make_route_harder
From: Patrick McHardy @ 2007-09-21 12:39 UTC (permalink / raw)
  To: lepton; +Cc: Netfilter Development Mailinglist, Linux Netdev List
In-Reply-To: <20070919155554.GA11710@router.lepton.home>

lepton wrote:
> Yes, you are right.
> What do you think about this:
> For all packets can be sent out, we just disable 
> all things in sk_route_caps in ip_route_me_harder


Whats the point of doing that? Is rerouting breaking anything for you?

^ permalink raw reply

* Re: change the way e1000 is handling short VLAN frames
From: Emil Micek @ 2007-09-21 12:50 UTC (permalink / raw)
  To: hadi; +Cc: auke-jan.h.kok@intel.com, netdev mailing list
In-Reply-To: <1190375946.4261.42.camel@localhost>

On Fri, 2007-09-21 at 07:59 -0400, jamal wrote:
> > Current e1000 (according to our observations) first appends 4 bytes of
> > VLAN tag and then pads the frame to 64 bytes with zeroes if necessary
> > before transmiting it.
> 
> Which would make it a bug. AFAIK, the minimum VLAN tagged packet going
> out is 68 bytes.
Are you sure about this? We did some extensive testing capturing VLAN
frames going out of our Intel PRO/1000 MT Dual Port Server Adapter
(82571EB chipset) and we definitely captured VLAN frames shorter then 68
bytes. Here is text dump of such frame:
http://www.tkrjasek.cz/mie/cmts_tests/between_linux_and_cmts.txt
(note that wireshark captures the frames without 4 bytes FCS (frame
check sequence) so the above packet is in fact 66 bytes long).

What is the right behaviour according to specification? In iee802.3,
minFrameSize is 64bytes. I've never seen any document which'd say that
VLAN frames should be 68 bytes minimum.


> > I'd like to change it so it first pads the frame to 64 B and appends the
> > VLAN tag afterwards, so the resulting VLAN frame would be 68 bytes
> > minimum.
> 
> If you were able to figure this much out: What have you tried and what
> do you still need help on? CCing some of the e1000 people(done) will
> probably help you get this resolved faster.
Well to make long story short. While testing VLAN capabilities of our C3
CMTS, we discovered that some frames are corupted after passing trough
the CMTS. We discovered, that affected frames are VLAN frames 64 - 67
bytes long. 
We tried several NIC's in our linux box mainly with e1000 driver but
also some NIC's from 3com, broadcom and Planet. All of them suffer from
this issue except NIC from Planet (RTL-8169 chipset), which pads VLAN
frames to 68 bytes. The other cards pad VLAN frames only to 64 bytes. 
We also tried to tag the traffic on some VLAN enabled switches (d-link,
edge-core, cisco catalyst) and they also pad VLAN frames to 68 bytes.

Now i'm little confused by your reply, becaouse you write, that VLAN
frames generated by e1000 should be 68 bytes minimum, which contradicts
witch our observations.

thanks for your reply
Emil


^ permalink raw reply

* Re: [PATCH] PHYLIB: IRQ event workqueue handling fixes
From: Maciej W. Rozycki @ 2007-09-21 12:51 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Andy Fleming, Jeff Garzik, netdev, linux-kernel
In-Reply-To: <20070920165348.0b25be3d.akpm@linux-foundation.org>

On Thu, 20 Sep 2007, Andrew Morton wrote:

> You always put boring, crappy, insufficient text in the for-the-changelog
> section and interesting, useful, sufficient text in the not-for-the-changelog
> section.

 I'll swap the sections in the future then. ;-)  Frankly I was not sure 
whether the changelog was happy about being fed with lengthy explanations 
and it has not spoken out.

 I have to admit this is a habit carried over from the FSF-style ChangeLog 
-- where the enforced rule is actually *not* to provide any explanation 
for why changes are done and only describe what has been modified (with 
any discussion around archived in the respective mailing list).

> But you can't fool me!  I have an editor and I fix it up.

 Thank you and sorry for the extra work I caused you -- I shall keep your 
suggestion in mind in the future.

  Maciej

^ permalink raw reply

* Re: [PATCH 2/7] CAN: Add PF_CAN core module
From: Patrick McHardy @ 2007-09-21 12:47 UTC (permalink / raw)
  To: Urs Thuermann
  Cc: netdev, David Miller, Thomas Gleixner, Oliver Hartkopp,
	Oliver Hartkopp, Urs Thuermann
In-Reply-To: <20070920184532.3795.2@janus.isnogud.escape.de>

Urs Thuermann wrote:
> +static int can_create(struct net *net, struct socket *sock, int protocol)
> +{
> +	...
> +
> +	spin_lock(&proto_tab_lock);
> +	cp = proto_tab[protocol];
> +	if (cp && !try_module_get(cp->prot->owner))
> +		cp = NULL;
> +	spin_unlock(&proto_tab_lock);
> +
> +	/* check for success and correct type */
> +	if (!cp || cp->type != sock->type) {
> +		ret = -EPROTONOSUPPORT;
> +		goto errout;
> +	}
> +
> +	if (cp->capability >= 0 && !capable(cp->capability)) {
> +		ret = -EPERM;
> +		goto errout;
> +	}
> +
> +	sock->ops = cp->ops;


You drop the module reference again when leaving this function.
So sock->ops might contain a stale pointer if the module is
unloaded after this. You need to either keep the module reference
while the socket is alive or remove stale references when
unregistering the protocol.


^ permalink raw reply

* Re: [PATCH 3/7] CAN: Add raw protocol
From: Patrick McHardy @ 2007-09-21 12:49 UTC (permalink / raw)
  To: Urs Thuermann
  Cc: netdev, David Miller, Thomas Gleixner, Oliver Hartkopp,
	Oliver Hartkopp, Urs Thuermann
In-Reply-To: <20070920184533.3795.3@janus.isnogud.escape.de>

Urs Thuermann wrote:
> +config CAN_RAW_USER
> +	bool "Allow non-root users to access Raw CAN Protocol sockets"


If you plan to remove this option, it should happen before merging
since it affects userspace visible behaviour.


^ permalink raw reply

* Re: wrong arp query with policy routing
From: Marco Berizzi @ 2007-09-21 12:56 UTC (permalink / raw)
  To: hadi; +Cc: Chuck Ebbert, netdev
In-Reply-To: <1190377540.4261.58.camel@localhost>

jamal wrote:

> On Fri, 2007-21-09 at 11:08 +0200, Marco Berizzi wrote:
>
> > thanks for the reply.
> > I have tried to 'echo 1 > /proc/sys/net/ipv4/conf/eth0',
> > but the 'arp whos-has' behaviour doesn't change.
> > Other hints?
>
> Give a man a fish and he'll eat for a day....
>
> Documentation/networking/ip-sysctl.txt

Thanks for the tip Jamal.
I have done a 'grep -i arp' on that file and
arp_announce should do the trick. But I always
get the same behaviour, after:

echo 2 > /proc/sys/net/ipv4/conf/eth0/arp_announce
echo 2 > /proc/sys/net/ipv4/conf/all/arp_announce

Here is:
14:43:26.096574 00:01:02:ad:db:82 > ff:ff:ff:ff:ff:ff, ethertype ARP
(0x0806),
length 42: arp who-has 1.1.1.254 tell 172.16.1.247

Here is my 'ip a s' output:
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast
qlen 1000
    link/ether 00:01:02:ad:db:82 brd ff:ff:ff:ff:ff:ff
    inet 172.16.1.247/23 brd 172.16.1.255 scope global eth0
    inet 1.1.1.1/32 scope global eth0




^ permalink raw reply

* Re: [RFC/PATCH 0/3] UDP memory usage accounting
From: Evgeniy Polyakov @ 2007-09-21 12:58 UTC (permalink / raw)
  To: Satoshi OSHIMA; +Cc: netdev, haoki, 吉藤 英明
In-Reply-To: <46F3B67F.4030007@hitachi.com>

Hi.

On Fri, Sep 21, 2007 at 09:18:07PM +0900, Satoshi OSHIMA (satoshi.oshima.fk@hitachi.com) wrote:
> This patch set try to introduce memory usage accounting for
> UDP(currently ipv4 only).
> 
> Currently, memory usage of UDP can be observed as the sam of
> usage of tx_queue and rx_queue. But I believe that the system
> wide accounting is usefull when heavy loaded condition.
> 
> In the next step, I would like to add memory usage quota
> for UDP to avoid unlimited memory consumption problem
> under DDOS attack.

Could you please desribed such attack in more details?
Each UDP socket has its queue length which can not be exceeded
(roughly), no new sockets are created when remote side sends a packet
(like after special steps in TCP), so where is possibility to eat all
the mem?

> This patch set is for 2.6.23-rc7.

I seriously doubt you want to put udp specific hacks and zillions of
atomic ops all around the code just to know exact number of bytes eaten
for UDP.

Please use udp specific code (like udp_sendmsg()) for proper accounting
if you need that, but not hacks in generic ip code.

-- 
	Evgeniy Polyakov

^ permalink raw reply

* Re: [RFC/PATCH 2/3] UDP memory usage accounting: accounting unit and variable
From: Andi Kleen @ 2007-09-21 13:10 UTC (permalink / raw)
  To: Satoshi OSHIMA; +Cc: netdev, haoki, 吉藤英明
In-Reply-To: <46F3B88D.1030601@hitachi.com>

Satoshi OSHIMA <satoshi.oshima.fk@hitachi.com> writes:

> This patch introduces global variable for UDP memory accounting.
> The unit is page.

The global variable doesn't seem to be very MP scalable, especially
if you change it for each packet. This will be a very hot cache line,
in the worst case bouncing around a large machine.

Possible alternatives:
- Per CPU variables
- You only change the global on socket creation time (by pre allocating a large
amount) or when the system comes under memory pressure.
- Batching of the global updates for multiple packets [that's a variant
of the previous one, might be still too costly though]

Also for such variables it's usually good to cache line pad them on SMP
to avoid false sharing with something else.

-Andi

^ permalink raw reply

* Re: wrong arp query with policy routing
From: Marco Berizzi @ 2007-09-21 13:30 UTC (permalink / raw)
  To: hadi; +Cc: Chuck Ebbert, netdev
In-Reply-To: <BAY103-DAV13C9EFE297F41FFC4007EAB2BB0@phx.gbl>

Marco Berizzi wrote:

>     inet 1.1.1.1/32 scope global eth0
                   ^^
Sorry, my fault.
Apologies for all the noise.



^ permalink raw reply

* Re: change the way e1000 is handling short VLAN frames
From: jamal @ 2007-09-21 13:37 UTC (permalink / raw)
  To: Emil Micek; +Cc: auke-jan.h.kok@intel.com, netdev mailing list, Ben Greear
In-Reply-To: <1190379015.3551.57.camel@magi.nadr>

On Fri, 2007-21-09 at 14:50 +0200, Emil Micek wrote:
> On Fri, 2007-09-21 at 07:59 -0400, jamal wrote:

> > Which would make it a bug. AFAIK, the minimum VLAN tagged packet going
> > out is 68 bytes.
> Are you sure about this? 

This is what i have always seen. Double checked with google and she gave
me many a url, example:
http://www.cisco.com/warp/public/473/741_4.html
I think you can pretty much trust cisco on something as basic as this.
Also:
http://www.techfest.com/networking/lan/ethernet2.htm

> We did some extensive testing capturing VLAN
> frames going out of our Intel PRO/1000 MT Dual Port Server Adapter
> (82571EB chipset) and we definitely captured VLAN frames shorter then 68
> bytes. Here is text dump of such frame:
> http://www.tkrjasek.cz/mie/cmts_tests/between_linux_and_cmts.txt

Do you have access to another NIC? a tg3 based one will be good to test
for hardware level tagging.

> (note that wireshark captures the frames without 4 bytes FCS (frame
> check sequence) so the above packet is in fact 66 bytes long).
> 
> What is the right behaviour according to specification? In iee802.3,
> minFrameSize is 64bytes. I've never seen any document which'd say that
> VLAN frames should be 68 bytes minimum.

Refer to above.

> Now i'm little confused by your reply, becaouse you write, that VLAN
> frames generated by e1000 should be 68 bytes minimum, which contradicts
> witch our observations.

Miscommunication ;-> I said the packets going out should be 68B
otherwise it is a bug. if e1000 is sending 64B out, it is a bug.

I think any other NIC that doesnt do hardware based vlan tags would be
good to try as well because it uses the software stack to do the
tagging.

cheers,
jamal


^ permalink raw reply

* Re: [PATCH 1/2] David Miller's rbtree patches for 2.6.22.6
From: Ilpo Järvinen @ 2007-09-21 13:37 UTC (permalink / raw)
  To: Tom Quetchenbach; +Cc: netdev
In-Reply-To: <46F1D00B.6030108@gmail.com>

On Wed, 19 Sep 2007, Tom Quetchenbach wrote:

> Patch 1: David Miller's red-black tree code, tweaked for 2.6.22.6,
> with some bugfixes

It would help if you would leave the original changes as is (rb-tree and 
fack_count separated) and add your work on top of that...

> diff -ur linux-2.6.22.6/include/net/tcp.h linux-2.6.22.6-rbtree-davem-fixed/include/net/tcp.h
> --- linux-2.6.22.6/include/net/tcp.h	2007-08-30 23:21:01.000000000 -0700
> +++ linux-2.6.22.6-rbtree-davem-fixed/include/net/tcp.h	2007-09-19 17:36:07.000000000 -0700
> @@ -540,6 +540,7 @@
>  	__u32		seq;		/* Starting sequence number	*/
>  	__u32		end_seq;	/* SEQ + FIN + SYN + datalen	*/
>  	__u32		when;		/* used to compute rtt's	*/
> +	unsigned int	fack_count;	/* speed up SACK processing	*/
>  	__u8		flags;		/* TCP header flags.		*/
>  
>  	/* NOTE: These must match up to the flags byte in a
> @@ -1043,12 +1044,12 @@
>  }
>  
>  /*from STCP */
> -static inline void clear_all_retrans_hints(struct tcp_sock *tp){
> +static inline void clear_all_retrans_hints(struct tcp_sock *tp)
> +{

Unrelated change, please don't do that. Besides, it's already fixed in 
net-2.6.24.

>  	tp->lost_skb_hint = NULL;
>  	tp->scoreboard_skb_hint = NULL;
>  	tp->retransmit_skb_hint = NULL;
>  	tp->forward_skb_hint = NULL;
> -	tp->fastpath_skb_hint = NULL;
>  }
>  
>  /* MD5 Signature */
> @@ -1227,9 +1229,61 @@
>  	sk->sk_send_head = NULL;
>  }
>  
> +static inline struct sk_buff *tcp_write_queue_find(struct sock *sk, __u32 seq)
> +{
> +	struct rb_node *rb_node = tcp_sk(sk)->write_queue_rb.rb_node;
> +	struct sk_buff *skb = NULL;
> +
> +	while (rb_node) {
> +		struct sk_buff *tmp = rb_entry(rb_node,struct sk_buff,rb);
> +		if (TCP_SKB_CB(tmp)->end_seq > seq) {

This is old and buggy version of the rb-tree code. Get the latest rb-tree 
patch from tcp-2.6 tree.

> +			skb = tmp;
> +			if (TCP_SKB_CB(tmp)->seq <= seq)

...fixed in tcp-2.6.

> +				break;
> +			rb_node = rb_node->rb_left;
> +		} else
> +			rb_node = rb_node->rb_right;
> +
> +	}
> +	return skb;
> +}
> +
> +static inline void tcp_rb_insert(struct sk_buff *skb, struct rb_root *root)
> +{
> +	struct rb_node **rb_link, *rb_parent;
> +	__u32 seq = TCP_SKB_CB(skb)->seq;
> +
> +	rb_link = &root->rb_node;
> +	rb_parent = NULL;
> +	while (*rb_link != NULL) {
> +		struct sk_buff *tmp = rb_entry(*rb_link,struct sk_buff,rb);
> +		rb_parent = *rb_link;
> +		if (TCP_SKB_CB(tmp)->end_seq > seq) {
> +			BUG_ON(TCP_SKB_CB(tmp)->seq <= seq);

...these are broken as well.

>
> +			rb_link = &rb_parent->rb_left;
> +		} else {
> +			rb_link = &rb_parent->rb_right;
> +		}
> +	}
> +	rb_link_node(&skb->rb, rb_parent, rb_link);
> +	rb_insert_color(&skb->rb, root);
> +}
> +
> +static inline void tcp_rb_unlink(struct sk_buff *skb, struct rb_root *root)
> +{
> +	rb_erase(&skb->rb, root);
> +}
> +
>  static inline void __tcp_add_write_queue_tail(struct sock *sk, struct sk_buff *skb)
>  {
> +	struct sk_buff *tail = tcp_write_queue_tail(sk);
> +	unsigned int fc = 0;
> +
> +	if (tail)
> +		fc = TCP_SKB_CB(tail)->fack_count + tcp_skb_pcount(tail);
> +	TCP_SKB_CB(skb)->fack_count = fc;
>  	__skb_queue_tail(&sk->sk_write_queue, skb);
> +	tcp_rb_insert(skb, &tcp_sk(sk)->write_queue_rb);
>  }
>  
>  static inline void tcp_add_write_queue_tail(struct sock *sk, struct sk_buff *skb)
> diff -ur linux-2.6.22.6/net/ipv4/tcp_input.c linux-2.6.22.6-rbtree-davem-fixed/net/ipv4/tcp_input.c
> --- linux-2.6.22.6/net/ipv4/tcp_input.c	2007-08-30 23:21:01.000000000 -0700
> +++ linux-2.6.22.6-rbtree-davem-fixed/net/ipv4/tcp_input.c	2007-09-13 18:23:16.000000000 -0700
> @@ -947,14 +947,13 @@
>  	unsigned char *ptr = (skb_transport_header(ack_skb) +
>  			      TCP_SKB_CB(ack_skb)->sacked);
>  	struct tcp_sack_block_wire *sp = (struct tcp_sack_block_wire *)(ptr+2);
> -	struct sk_buff *cached_skb;
>  	int num_sacks = (ptr[1] - TCPOLEN_SACK_BASE)>>3;
>  	int reord = tp->packets_out;
>  	int prior_fackets;
>  	u32 lost_retrans = 0;
>  	int flag = 0;
>  	int found_dup_sack = 0;
> -	int cached_fack_count;
> +	int fack_count_base;
>  	int i;
>  	int first_sack_index;
>  
> @@ -1020,7 +1019,6 @@
>  		num_sacks = 1;
>  	else {
>  		int j;
> -		tp->fastpath_skb_hint = NULL;
>  
>  		/* order SACK blocks to allow in order walk of the retrans queue */
>  		for (i = num_sacks-1; i > 0; i--) {
> @@ -1045,14 +1043,7 @@
>  	/* clear flag as used for different purpose in following code */
>  	flag = 0;
>  
> -	/* Use SACK fastpath hint if valid */
> -	cached_skb = tp->fastpath_skb_hint;
> -	cached_fack_count = tp->fastpath_cnt_hint;
> -	if (!cached_skb) {
> -		cached_skb = tcp_write_queue_head(sk);
> -		cached_fack_count = 0;
> -	}
> -
> +	fack_count_base = TCP_SKB_CB(tcp_write_queue_head(sk))->fack_count;
>  	for (i=0; i<num_sacks; i++, sp++) {
>  		struct sk_buff *skb;
>  		__u32 start_seq = ntohl(sp->start_seq);
> @@ -1060,8 +1051,10 @@
>  		int fack_count;
>  		int dup_sack = (found_dup_sack && (i == first_sack_index));
>  
> -		skb = cached_skb;
> -		fack_count = cached_fack_count;
> +		skb = tcp_write_queue_find(sk, start_seq);
> +		if (!skb)
> +			continue;

In net-2.6.24 we validate SACK blocks early. ...This is not a working 
solution anyway since tcp_write_queue_find(end_seq) might be valid don't 
you think (though with validator it should only happen when dup_sack is 
set)? For non-DSACK cases, a better alternative is like this:

if (WARN_ON(skb == NULL))
	continue;

> +		fack_count = TCP_SKB_CB(skb)->fack_count - fack_count_base;
>  
>  		/* Event "B" in the comment above. */
>  		if (after(end_seq, tp->high_seq))

-- 
 i.

^ permalink raw reply

* Re: change the way e1000 is handling short VLAN frames
From: jamal @ 2007-09-21 13:46 UTC (permalink / raw)
  To: Emil Micek; +Cc: auke-jan.h.kok@intel.com, netdev mailing list, Ben Greear
In-Reply-To: <1190381841.4231.10.camel@localhost>

On Fri, 2007-21-09 at 09:37 -0400, jamal wrote:
> On Fri, 2007-21-09 at 14:50 +0200, Emil Micek wrote:
> > On Fri, 2007-09-21 at 07:59 -0400, jamal wrote:
> 
> > > Which would make it a bug. AFAIK, the minimum VLAN tagged packet going
> > > out is 68 bytes.
> > Are you sure about this? 
> 
> This is what i have always seen. 

If the first email caused confusion - so will this ;->
I have never used e1000 for vlan tagging. My view is that if it emits
(instead of 68B) 64B packets with tags then it is buggy and needs to be
fixed. Hope that makes sense.


cheers,
jamal


^ permalink raw reply

* Re: [PATCH 2/2] David Miller's rbtree patches for 2.6.22.6
From: Ilpo Järvinen @ 2007-09-21 13:48 UTC (permalink / raw)
  To: Tom Quetchenbach; +Cc: Netdev
In-Reply-To: <46F1D063.6030005@gmail.com>

On Wed, 19 Sep 2007, Tom Quetchenbach wrote:

> Patch 2: fixes to fack_counts and enhancement of SACK fast path

...Usually these are not combined in patches but a separate patch per 
change.

> diff -ur linux-2.6.22.6-rbtree-davem-fixed/include/net/tcp.h linux-2.6.22.6-rbtree-tomq/include/net/tcp.h
> --- linux-2.6.22.6-rbtree-davem-fixed/include/net/tcp.h	2007-09-19 17:36:07.000000000 -0700
> +++ linux-2.6.22.6-rbtree-tomq/include/net/tcp.h	2007-09-19 12:22:06.000000000 -0700
> @@ -1213,6 +1213,11 @@

...Btw, please always use -p to diff as well as it helps review :-).

>  	sk->sk_send_head = tcp_write_queue_next(sk, skb);
>  	if (sk->sk_send_head == (struct sk_buff *)&sk->sk_write_queue)
>  		sk->sk_send_head = NULL;
> +	else
> +		/* update fack_count of send_head. Since we've sent skb already,
> + 		 * its packet count must be set by now. */
> +		TCP_SKB_CB(sk->sk_send_head)->fack_count =
> +			TCP_SKB_CB(skb)->fack_count + tcp_skb_pcount(skb);
>
>  	/* Don't override Nagle indefinately with F-RTO */
>  	if (tp->frto_counter == 2)
>  		tp->frto_counter = 3;
> @@ -1310,19 +1315,22 @@
>  /* An insert into the middle of the write queue causes the fack
>   * counts in subsequent packets to become invalid, fix them up.
>   */
> -static inline void tcp_reset_fack_counts(struct sock *sk, struct sk_buff *first)
> +static inline void tcp_reset_fack_counts(struct sock *sk, struct sk_buff *skb)
>  {
> -	struct sk_buff *prev = first->prev;
> +	struct sk_buff *prev = skb->prev;
>  	unsigned int fc = 0;
>  
>  	if (prev != (struct sk_buff *) &sk->sk_write_queue)
>  		fc = TCP_SKB_CB(prev)->fack_count + tcp_skb_pcount(prev);
>  
> -	while (first != (struct sk_buff *)&sk->sk_write_queue) {
> -		TCP_SKB_CB(first)->fack_count = fc;
> +	while (skb != (struct sk_buff *)&sk->sk_write_queue) {
> +		if (TCP_SKB_CB(skb)->fack_count == fc || !tcp_skb_pcount(skb))

What is this !tcp_skb_pcount(skb) for???? I think what we really want here 
is this: 	|| (skb == tcp_send_head(sk))

> +			break;
>
> -		fc += tcp_skb_pcount(first);
> -		first = first->next;
> +		TCP_SKB_CB(skb)->fack_count = fc;
> +
> +		fc += tcp_skb_pcount(skb);
> +		skb = skb->next;
>  	}
>  }
>  
> diff -ur linux-2.6.22.6-rbtree-davem-fixed/net/ipv4/tcp_input.c linux-2.6.22.6-rbtree-tomq/net/ipv4/tcp_input.c
> --- linux-2.6.22.6-rbtree-davem-fixed/net/ipv4/tcp_input.c	2007-09-13 18:23:16.000000000 -0700
> +++ linux-2.6.22.6-rbtree-tomq/net/ipv4/tcp_input.c	2007-09-19 12:27:42.000000000 -0700
> @@ -956,6 +956,7 @@
>  	int fack_count_base;
>  	int i;
>  	int first_sack_index;
> +	u32 prev_end_seq = 0;
>  
>  	if (!tp->sacked_out)
>  		tp->fackets_out = 0;
> @@ -1000,6 +1001,7 @@
>  		if (i == 0) {
>  			if (tp->recv_sack_cache[i].start_seq != start_seq)
>  				flag = 0;
> +			prev_end_seq = ntohl(tp->recv_sack_cache[i].end_seq);
>  		} else {
>  			if ((tp->recv_sack_cache[i].start_seq != start_seq) ||
>  			    (tp->recv_sack_cache[i].end_seq != end_seq))
> @@ -1016,9 +1018,16 @@
>  
>  	first_sack_index = 0;
>  	if (flag)
> +		/* all that has changed is end of first SACK block. So all we
> + 		 * need to do is tag those skbs that were'nt tagged last time. */

IMHO, a bit verbose comment. Besides, we already tell above that what SACK 
fastpath is all about...

>  		num_sacks = 1;
>  	else {
>  		int j;
> +		
> +		/* more than just end of first SACK block has changed; invalidate
> +		 * prev_end_seq */
> +
> +		prev_end_seq = 0;

Don't tell what's obvious from code as well.

>  		/* order SACK blocks to allow in order walk of the retrans queue */
>  		for (i = num_sacks-1; i > 0; i--) {
> @@ -1051,6 +1060,8 @@
>  		int fack_count;
>  		int dup_sack = (found_dup_sack && (i == first_sack_index));
>  
> +		if (prev_end_seq) start_seq = prev_end_seq;
> +

...We never add statements on the same line after if statements (see 
Documentation/CodingStyle)



-- 
 i.

^ permalink raw reply

* Re: [RFC PATCH] 2.6.22.6 netfilter: sk_setup_caps in ip_make_route_harder
From: lepton @ 2007-09-21 14:07 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: netfilter-devel, netdev
In-Reply-To: <46F3BB9E.4000105@trash.net>

Yes.
My situation is:
The default route device is a e1000 network card that can do TSO.
So the tcp stack will try send big skb to netfilter frame work.
But after rerouting, the packtes will go out from a device  that
can not do TSO. the packet is just get dropped..

I thinks if we can't get a way to tell tcp stack the sk_route_caps of 
the real out device, we can just disable all things for safety.

On Fri, Sep 21, 2007 at 02:39:58PM +0200, Patrick McHardy wrote:
> lepton wrote:
> > Yes, you are right.
> > What do you think about this:
> > For all packets can be sent out, we just disable 
> > all things in sk_route_caps in ip_route_me_harder
> 
> 
> Whats the point of doing that? Is rerouting breaking anything for you?

^ permalink raw reply

* Re: [PATCH 0/2] David Miller's rbtree patches for 2.6.22.6
From: Ilpo Järvinen @ 2007-09-21 14:08 UTC (permalink / raw)
  To: Tom Quetchenbach; +Cc: Netdev
In-Reply-To: <46F1CF35.3030606@gmail.com>

On Wed, 19 Sep 2007, Tom Quetchenbach wrote:

> Here are a couple of patches against 2.6.22.6. The first one is just
> David's patches tweaked for 2.6.22.6, with a couple of minor bugfixes to
> get it to compile and not crash.

Why did you combine original patches to a single larger one, I think Dave 
made them separate on purpose.

> (I also changed
> __tcp_insert_write_queue_tail() to set the fack_count of the new packet
> to the fack_count of the tail plus the packet count of the tail, not the
> packet count of the new skb, because I think that's how it was intended
> to be. Right?

I think I noticed similar "off-by-pcount" error when I looked that long 
time ago, so I guess you're correct. We're only interested in delta 
of it anyway and add the current skb's pcount to it (which is not 
fixed until tcp_fragment in sacktag is past).

> In the second patch there are a couple of significant changes.  One is
> (as Baruch suggested) to modify the existing SACK fast path so that we
> don't tag packets we've already tagged when we advance by a packet.

This solution would still spend extensive amount of time in processing  
loop, whenever recv_sack_cache fast-path is not taken, that is, e.g. when 
cumulative ACK after retransmissions arrive or new hole becomes visible 
(which are not very exceptional events after all :-)). In the cumulative 
ACK case especially, this processing is very likely _fully_ wasted 
walking.

So there is still room for large improvements. I've made an improved 
version of the current sacktag walk couple of days ago (it's in a state 
where it didn't crash but likely very buggy still), I'll post it here 
soon... Idea is embed recv_sack_cache checking fully into the walking 
loop. By doing that an previously known work is not duplicated. The patch
is currently against non-rbtree stuff but incorporating rb-tree things on 
top of it should be very trivial and synergy benefits with rbtree should 
be considerable because non-rbtree has to do "fast walk" skipping for skbs 
that are under highest_sack which is prone to cache misses. 

> The other issue is that the cached fack_counts seem to be wrong, because
> they're set when we insert into the queue, but tcp_set_tso_segs() is
> called later, just before we send, so all the fack_counts are zero. My
> solution was to set the fack_count when we advance the send_head.

I think it's better solution anyway, since we might have to do 
reset_fack_counts() in between and there's no need to update past 
sk_send_head.

> Also I
> changed tcp_reset_fack_counts() so that it exits when it hits an skb
> whose tcp_skb_pcount() is zero

Do you mind to explain what's the purpose of that?

> or whose fack_count is already correct.
> (This really helps when TSO is on, since there's lots of inserting into
> the middle of the queue.)

Good point.

> Please let me know how I can help get this tested and debugged.

Most network development happens against latest net-2.6(.x) trees. 
In addition, there's experimental tcp-2.6 tree but it's currently a bit 
outdated already (and DaveM is very busy with the phenomenal merge 
they're doing for 2.6.24 :-) so it's not too likely to be updated very 
soon).

...Anyway, thanks for your interest in these things.


-- 
 i.

^ permalink raw reply

* Re: e1000 driver and samba
From: L F @ 2007-09-21 14:08 UTC (permalink / raw)
  To: Bill Fink; +Cc: netdev
In-Reply-To: <20070919225105.1e80e91d.billfink@mindspring.com>

On 9/19/07, Bill Fink <billfink@mindspring.com> wrote:
> Just my personal opinion, but unless you want to do more testing,
> since you now seem to have a working setup, I would tend to leave
> it the way it is.
Quite sensible, yes. Performance even seems to be good - I am getting
40-40MBps reads and 24-26MBps writes - so it'll stay the way it is.

> By any chance did you happen to power cycle some equipment in this
> process that you didn't previously power cycle during earlier testing
> and debugging?  If so, perhaps that hardware had somehow gotten into
> a funky state, and the power cycling might have cleared it up.
Not that I am aware of: one of the first things that I did - and
repeated basically every step of the way - was to powercycle the two
switches, following the same line of reasoning you did. The clients
were turned off every night and turned back on every morning and the
WAN Comcast CPE wasn't touched for the duration. The only thing that
did change is that in an impetus of efficiency or perhaps desperation
I changed that cable too (to CAT6, 3' long), but I can't imagine that
would affect the LAN side of operations.
Thanks again - to everyone - for the help. I am still puzzled, but at
least I am puzzled with a consistent situation.
To Mr. Romieu: the patch you provided seems to work, in that 'regular'
loads don't trip samba up. I have to check the CRCs, though.

>                                                 -Bill

Luigi Fabio

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox