* [PATCH 2/2] via-rhine: Assign random MAC address if necessary
From: Joe Perches @ 2011-04-17 0:15 UTC (permalink / raw)
To: Alexandru Gagniuc, Roger Luethi; +Cc: David S. Miller, netdev, linux-kernel
In-Reply-To: <cover.1302998994.git.joe@perches.com>
Roger Luethi has had several reports of Rhine NICs providing
an invalid MAC address. If so, assign a random MAC address so
the hardware can still be used.
Tested as a standalone interface, as carrier for ppp, and as a
bonding slave.
Original-patch-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/via-rhine.c | 12 +++++++-----
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/net/via-rhine.c b/drivers/net/via-rhine.c
index 40f394c..7f23ab9 100644
--- a/drivers/net/via-rhine.c
+++ b/drivers/net/via-rhine.c
@@ -838,13 +838,15 @@ static int __devinit rhine_init_one(struct pci_dev *pdev,
for (i = 0; i < 6; i++)
dev->dev_addr[i] = ioread8(ioaddr + StationAddr + i);
- memcpy(dev->perm_addr, dev->dev_addr, dev->addr_len);
- if (!is_valid_ether_addr(dev->perm_addr)) {
- rc = -EIO;
- dev_err(&pdev->dev, "Invalid MAC address\n");
- goto err_out_unmap;
+ if (!is_valid_ether_addr(dev->dev_addr)) {
+ /* Report it and use a random ethernet address instead */
+ netdev_err(dev, "Invalid MAC address: %pM\n", dev->dev_addr);
+ random_ether_addr(dev->dev_addr);
+ netdev_info(dev, "Using random MAC address: %pM\n",
+ dev->dev_addr);
}
+ memcpy(dev->perm_addr, dev->dev_addr, dev->addr_len);
/* For Rhine-I/II, phy_id is loaded from EEPROM */
if (!phy_id)
--
1.7.4.2.g597a6.dirty
^ permalink raw reply related
* [PATCH 1/2] via_rhine: Use netdev_<level> and pr_<level>
From: Joe Perches @ 2011-04-17 0:15 UTC (permalink / raw)
To: Alexandru Gagniuc, Roger Luethi; +Cc: David S. Miller, netdev, linux-kernel
In-Reply-To: <cover.1302998994.git.joe@perches.com>
Use the more current logging styles.
Add #define DEBUG to make netdev_dbg always active.
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/via-rhine.c | 230 ++++++++++++++++++++++-------------------------
1 files changed, 108 insertions(+), 122 deletions(-)
diff --git a/drivers/net/via-rhine.c b/drivers/net/via-rhine.c
index 0422a79..40f394c 100644
--- a/drivers/net/via-rhine.c
+++ b/drivers/net/via-rhine.c
@@ -29,6 +29,8 @@
*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#define DRV_NAME "via-rhine"
#define DRV_VERSION "1.5.0"
#define DRV_RELDATE "2010-10-09"
@@ -37,6 +39,7 @@
/* A few user-configurable values.
These may be modified when a driver module is loaded. */
+#define DEBUG
static int debug = 1; /* 1 normal messages, 0 quiet .. 7 verbose. */
static int max_interrupt_work = 20;
@@ -111,8 +114,7 @@ static const int multicast_filter_limit = 32;
/* These identify the driver base version and may not be removed. */
static const char version[] __devinitconst =
- KERN_INFO DRV_NAME ".c:v1.10-LK" DRV_VERSION " " DRV_RELDATE
- " Written by Donald Becker\n";
+ "v1.10-LK" DRV_VERSION " " DRV_RELDATE " Written by Donald Becker";
/* This driver was written to use PCI memory space. Some early versions
of the Rhine may only work correctly with I/O space accesses. */
@@ -495,14 +497,15 @@ static void rhine_set_vlan_cam_mask(void __iomem *ioaddr, u32 mask);
static void rhine_init_cam_filter(struct net_device *dev);
static void rhine_update_vcam(struct net_device *dev);
-#define RHINE_WAIT_FOR(condition) do { \
- int i=1024; \
- while (!(condition) && --i) \
- ; \
- if (debug > 1 && i < 512) \
- printk(KERN_INFO "%s: %4d cycles used @ %s:%d\n", \
- DRV_NAME, 1024-i, __func__, __LINE__); \
-} while(0)
+#define RHINE_WAIT_FOR(condition) \
+do { \
+ int i = 1024; \
+ while (!(condition) && --i) \
+ ; \
+ if (debug > 1 && i < 512) \
+ pr_info("%4d cycles used @ %s:%d\n", \
+ 1024 - i, __func__, __LINE__); \
+} while (0)
static inline u32 get_intr_status(struct net_device *dev)
{
@@ -571,8 +574,8 @@ static void rhine_power_init(struct net_device *dev)
default:
reason = "Unknown";
}
- printk(KERN_INFO "%s: Woke system up. Reason: %s.\n",
- DRV_NAME, reason);
+ netdev_info(dev, "Woke system up. Reason: %s\n",
+ reason);
}
}
}
@@ -586,8 +589,7 @@ static void rhine_chip_reset(struct net_device *dev)
IOSYNC;
if (ioread8(ioaddr + ChipCmd1) & Cmd1Reset) {
- printk(KERN_INFO "%s: Reset not complete yet. "
- "Trying harder.\n", DRV_NAME);
+ netdev_info(dev, "Reset not complete yet. Trying harder.\n");
/* Force reset */
if (rp->quirks & rqForceReset)
@@ -598,9 +600,9 @@ static void rhine_chip_reset(struct net_device *dev)
}
if (debug > 1)
- printk(KERN_INFO "%s: Reset %s.\n", dev->name,
- (ioread8(ioaddr + ChipCmd1) & Cmd1Reset) ?
- "failed" : "succeeded");
+ netdev_info(dev, "Reset %s\n",
+ (ioread8(ioaddr + ChipCmd1) & Cmd1Reset) ?
+ "failed" : "succeeded");
}
#ifdef USE_MMIO
@@ -728,9 +730,7 @@ static int __devinit rhine_init_one(struct pci_dev *pdev,
/* when built into the kernel, we only print version if device is found */
#ifndef MODULE
- static int printed_version;
- if (!printed_version++)
- printk(version);
+ pr_info_once("%s\n", version);
#endif
io_size = 256;
@@ -765,8 +765,8 @@ static int __devinit rhine_init_one(struct pci_dev *pdev,
/* this should always be supported */
rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
if (rc) {
- printk(KERN_ERR "32-bit PCI DMA addresses not supported by "
- "the card!?\n");
+ dev_err(&pdev->dev,
+ "32-bit PCI DMA addresses not supported by the card!?\n");
goto err_out;
}
@@ -774,7 +774,7 @@ static int __devinit rhine_init_one(struct pci_dev *pdev,
if ((pci_resource_len(pdev, 0) < io_size) ||
(pci_resource_len(pdev, 1) < io_size)) {
rc = -EIO;
- printk(KERN_ERR "Insufficient PCI resources, aborting\n");
+ dev_err(&pdev->dev, "Insufficient PCI resources, aborting\n");
goto err_out;
}
@@ -786,7 +786,7 @@ static int __devinit rhine_init_one(struct pci_dev *pdev,
dev = alloc_etherdev(sizeof(struct rhine_private));
if (!dev) {
rc = -ENOMEM;
- printk(KERN_ERR "alloc_etherdev failed\n");
+ dev_err(&pdev->dev, "alloc_etherdev failed\n");
goto err_out;
}
SET_NETDEV_DEV(dev, &pdev->dev);
@@ -804,8 +804,9 @@ static int __devinit rhine_init_one(struct pci_dev *pdev,
ioaddr = pci_iomap(pdev, bar, io_size);
if (!ioaddr) {
rc = -EIO;
- printk(KERN_ERR "ioremap failed for device %s, region 0x%X "
- "@ 0x%lX\n", pci_name(pdev), io_size, memaddr);
+ dev_err(&pdev->dev,
+ "ioremap failed for device %s, region 0x%X @ 0x%lX\n",
+ pci_name(pdev), io_size, memaddr);
goto err_out_free_res;
}
@@ -820,8 +821,9 @@ static int __devinit rhine_init_one(struct pci_dev *pdev,
unsigned char b = readb(ioaddr+reg);
if (a != b) {
rc = -EIO;
- printk(KERN_ERR "MMIO do not match PIO [%02x] "
- "(%02x != %02x)\n", reg, a, b);
+ dev_err(&pdev->dev,
+ "MMIO do not match PIO [%02x] (%02x != %02x)\n",
+ reg, a, b);
goto err_out_unmap;
}
}
@@ -840,7 +842,7 @@ static int __devinit rhine_init_one(struct pci_dev *pdev,
if (!is_valid_ether_addr(dev->perm_addr)) {
rc = -EIO;
- printk(KERN_ERR "Invalid MAC address\n");
+ dev_err(&pdev->dev, "Invalid MAC address\n");
goto err_out_unmap;
}
@@ -878,14 +880,14 @@ static int __devinit rhine_init_one(struct pci_dev *pdev,
if (rc)
goto err_out_unmap;
- printk(KERN_INFO "%s: VIA %s at 0x%lx, %pM, IRQ %d.\n",
- dev->name, name,
+ netdev_info(dev, "VIA %s at 0x%lx, %pM, IRQ %d\n",
+ name,
#ifdef USE_MMIO
- memaddr,
+ memaddr,
#else
- (long)ioaddr,
+ (long)ioaddr,
#endif
- dev->dev_addr, pdev->irq);
+ dev->dev_addr, pdev->irq);
pci_set_drvdata(pdev, dev);
@@ -896,11 +898,11 @@ static int __devinit rhine_init_one(struct pci_dev *pdev,
mdio_write(dev, phy_id, MII_BMCR, mii_cmd);
if (mii_status != 0xffff && mii_status != 0x0000) {
rp->mii_if.advertising = mdio_read(dev, phy_id, 4);
- printk(KERN_INFO "%s: MII PHY found at address "
- "%d, status 0x%4.4x advertising %4.4x "
- "Link %4.4x.\n", dev->name, phy_id,
- mii_status, rp->mii_if.advertising,
- mdio_read(dev, phy_id, 5));
+ netdev_info(dev,
+ "MII PHY found at address %d, status 0x%04x advertising %04x Link %04x\n",
+ phy_id,
+ mii_status, rp->mii_if.advertising,
+ mdio_read(dev, phy_id, 5));
/* set IFF_RUNNING */
if (mii_status & BMSR_LSTATUS)
@@ -912,8 +914,7 @@ static int __devinit rhine_init_one(struct pci_dev *pdev,
}
rp->mii_if.phy_id = phy_id;
if (debug > 1 && avoid_D3)
- printk(KERN_INFO "%s: No D3 power state at shutdown.\n",
- dev->name);
+ netdev_info(dev, "No D3 power state at shutdown\n");
return 0;
@@ -938,7 +939,7 @@ static int alloc_ring(struct net_device* dev)
TX_RING_SIZE * sizeof(struct tx_desc),
&ring_dma);
if (!ring) {
- printk(KERN_ERR "Could not allocate DMA memory.\n");
+ netdev_err(dev, "Could not allocate DMA memory\n");
return -ENOMEM;
}
if (rp->quirks & rqRhineI) {
@@ -1098,8 +1099,8 @@ static void rhine_check_media(struct net_device *dev, unsigned int init_media)
iowrite8(ioread8(ioaddr + ChipCmd1) & ~Cmd1FDuplex,
ioaddr + ChipCmd1);
if (debug > 1)
- printk(KERN_INFO "%s: force_media %d, carrier %d\n", dev->name,
- rp->mii_if.force_media, netif_carrier_ok(dev));
+ netdev_info(dev, "force_media %d, carrier %d\n",
+ rp->mii_if.force_media, netif_carrier_ok(dev));
}
/* Called after status of force_media possibly changed */
@@ -1113,9 +1114,8 @@ static void rhine_set_carrier(struct mii_if_info *mii)
else /* Let MMI library update carrier status */
rhine_check_media(mii->dev, 0);
if (debug > 1)
- printk(KERN_INFO "%s: force_media %d, carrier %d\n",
- mii->dev->name, mii->force_media,
- netif_carrier_ok(mii->dev));
+ netdev_info(mii->dev, "force_media %d, carrier %d\n",
+ mii->force_media, netif_carrier_ok(mii->dev));
}
/**
@@ -1402,8 +1402,7 @@ static int rhine_open(struct net_device *dev)
return rc;
if (debug > 1)
- printk(KERN_DEBUG "%s: rhine_open() irq %d.\n",
- dev->name, rp->pdev->irq);
+ netdev_dbg(dev, "%s() irq %d\n", __func__, rp->pdev->irq);
rc = alloc_ring(dev);
if (rc) {
@@ -1415,10 +1414,9 @@ static int rhine_open(struct net_device *dev)
rhine_chip_reset(dev);
init_registers(dev);
if (debug > 2)
- printk(KERN_DEBUG "%s: Done rhine_open(), status %4.4x "
- "MII status: %4.4x.\n",
- dev->name, ioread16(ioaddr + ChipCmd),
- mdio_read(dev, rp->mii_if.phy_id, MII_BMSR));
+ netdev_dbg(dev, "%s() Done - status %04x MII status: %04x\n",
+ __func__, ioread16(ioaddr + ChipCmd),
+ mdio_read(dev, rp->mii_if.phy_id, MII_BMSR));
netif_start_queue(dev);
@@ -1461,10 +1459,9 @@ static void rhine_tx_timeout(struct net_device *dev)
struct rhine_private *rp = netdev_priv(dev);
void __iomem *ioaddr = rp->base;
- printk(KERN_WARNING "%s: Transmit timed out, status %4.4x, PHY status "
- "%4.4x, resetting...\n",
- dev->name, ioread16(ioaddr + IntrStatus),
- mdio_read(dev, rp->mii_if.phy_id, MII_BMSR));
+ netdev_warn(dev, "Transmit timed out, status %04x, PHY status %04x, resetting...\n",
+ ioread16(ioaddr + IntrStatus),
+ mdio_read(dev, rp->mii_if.phy_id, MII_BMSR));
schedule_work(&rp->reset_task);
}
@@ -1551,8 +1548,8 @@ static netdev_tx_t rhine_start_tx(struct sk_buff *skb,
spin_unlock_irqrestore(&rp->lock, flags);
if (debug > 4) {
- printk(KERN_DEBUG "%s: Transmit frame #%d queued in slot %d.\n",
- dev->name, rp->cur_tx-1, entry);
+ netdev_dbg(dev, "Transmit frame #%d queued in slot %d\n",
+ rp->cur_tx-1, entry);
}
return NETDEV_TX_OK;
}
@@ -1578,8 +1575,8 @@ static irqreturn_t rhine_interrupt(int irq, void *dev_instance)
IOSYNC;
if (debug > 4)
- printk(KERN_DEBUG "%s: Interrupt, status %8.8x.\n",
- dev->name, intr_status);
+ netdev_dbg(dev, "Interrupt, status %08x\n",
+ intr_status);
if (intr_status & (IntrRxDone | IntrRxErr | IntrRxDropped |
IntrRxWakeUp | IntrRxEmpty | IntrRxNoBuf)) {
@@ -1597,9 +1594,9 @@ static irqreturn_t rhine_interrupt(int irq, void *dev_instance)
RHINE_WAIT_FOR(!(ioread8(ioaddr+ChipCmd) & CmdTxOn));
if (debug > 2 &&
ioread8(ioaddr+ChipCmd) & CmdTxOn)
- printk(KERN_WARNING "%s: "
- "rhine_interrupt() Tx engine "
- "still on.\n", dev->name);
+ netdev_warn(dev,
+ "%s: Tx engine still on\n",
+ __func__);
}
rhine_tx(dev);
}
@@ -1611,16 +1608,15 @@ static irqreturn_t rhine_interrupt(int irq, void *dev_instance)
rhine_error(dev, intr_status);
if (--boguscnt < 0) {
- printk(KERN_WARNING "%s: Too much work at interrupt, "
- "status=%#8.8x.\n",
- dev->name, intr_status);
+ netdev_warn(dev, "Too much work at interrupt, status=%#08x\n",
+ intr_status);
break;
}
}
if (debug > 3)
- printk(KERN_DEBUG "%s: exiting interrupt, status=%8.8x.\n",
- dev->name, ioread16(ioaddr + IntrStatus));
+ netdev_dbg(dev, "exiting interrupt, status=%08x\n",
+ ioread16(ioaddr + IntrStatus));
return IRQ_RETVAL(handled);
}
@@ -1637,15 +1633,14 @@ static void rhine_tx(struct net_device *dev)
while (rp->dirty_tx != rp->cur_tx) {
txstatus = le32_to_cpu(rp->tx_ring[entry].tx_status);
if (debug > 6)
- printk(KERN_DEBUG "Tx scavenge %d status %8.8x.\n",
- entry, txstatus);
+ netdev_dbg(dev, "Tx scavenge %d status %08x\n",
+ entry, txstatus);
if (txstatus & DescOwn)
break;
if (txstatus & 0x8000) {
if (debug > 1)
- printk(KERN_DEBUG "%s: Transmit error, "
- "Tx status %8.8x.\n",
- dev->name, txstatus);
+ netdev_dbg(dev, "Transmit error, Tx status %08x\n",
+ txstatus);
dev->stats.tx_errors++;
if (txstatus & 0x0400)
dev->stats.tx_carrier_errors++;
@@ -1668,9 +1663,9 @@ static void rhine_tx(struct net_device *dev)
else
dev->stats.collisions += txstatus & 0x0F;
if (debug > 6)
- printk(KERN_DEBUG "collisions: %1.1x:%1.1x\n",
- (txstatus >> 3) & 0xF,
- txstatus & 0xF);
+ netdev_dbg(dev, "collisions: %1.1x:%1.1x\n",
+ (txstatus >> 3) & 0xF,
+ txstatus & 0xF);
dev->stats.tx_bytes += rp->tx_skbuff[entry]->len;
dev->stats.tx_packets++;
}
@@ -1714,9 +1709,9 @@ static int rhine_rx(struct net_device *dev, int limit)
int entry = rp->cur_rx % RX_RING_SIZE;
if (debug > 4) {
- printk(KERN_DEBUG "%s: rhine_rx(), entry %d status %8.8x.\n",
- dev->name, entry,
- le32_to_cpu(rp->rx_head_desc->rx_status));
+ netdev_dbg(dev, "%s(), entry %d status %08x\n",
+ __func__, entry,
+ le32_to_cpu(rp->rx_head_desc->rx_status));
}
/* If EOP is set on the next entry, it's a new packet. Send it up. */
@@ -1730,26 +1725,26 @@ static int rhine_rx(struct net_device *dev, int limit)
break;
if (debug > 4)
- printk(KERN_DEBUG "rhine_rx() status is %8.8x.\n",
- desc_status);
+ netdev_dbg(dev, "%s() status is %08x\n",
+ __func__, desc_status);
if ((desc_status & (RxWholePkt | RxErr)) != RxWholePkt) {
if ((desc_status & RxWholePkt) != RxWholePkt) {
- printk(KERN_WARNING "%s: Oversized Ethernet "
- "frame spanned multiple buffers, entry "
- "%#x length %d status %8.8x!\n",
- dev->name, entry, data_size,
- desc_status);
- printk(KERN_WARNING "%s: Oversized Ethernet "
- "frame %p vs %p.\n", dev->name,
- rp->rx_head_desc, &rp->rx_ring[entry]);
+ netdev_warn(dev,
+ "Oversized Ethernet frame spanned multiple buffers, "
+ "entry %#x length %d status %08x!\n",
+ entry, data_size,
+ desc_status);
+ netdev_warn(dev,
+ "Oversized Ethernet frame %p vs %p\n",
+ rp->rx_head_desc,
+ &rp->rx_ring[entry]);
dev->stats.rx_length_errors++;
} else if (desc_status & RxErr) {
/* There was a error. */
if (debug > 2)
- printk(KERN_DEBUG "rhine_rx() Rx "
- "error was %8.8x.\n",
- desc_status);
+ netdev_dbg(dev, "%s() Rx error was %08x\n",
+ __func__, desc_status);
dev->stats.rx_errors++;
if (desc_status & 0x0030)
dev->stats.rx_length_errors++;
@@ -1791,9 +1786,7 @@ static int rhine_rx(struct net_device *dev, int limit)
} else {
skb = rp->rx_skbuff[entry];
if (skb == NULL) {
- printk(KERN_ERR "%s: Inconsistent Rx "
- "descriptor chain.\n",
- dev->name);
+ netdev_err(dev, "Inconsistent Rx descriptor chain\n");
break;
}
rp->rx_skbuff[entry] = NULL;
@@ -1886,9 +1879,8 @@ static void rhine_restart_tx(struct net_device *dev) {
else {
/* This should never happen */
if (debug > 1)
- printk(KERN_WARNING "%s: rhine_restart_tx() "
- "Another error occurred %8.8x.\n",
- dev->name, intr_status);
+ netdev_warn(dev, "%s() Another error occurred %08x\n",
+ __func__, intr_status);
}
}
@@ -1909,21 +1901,19 @@ static void rhine_error(struct net_device *dev, int intr_status)
}
if (intr_status & IntrTxAborted) {
if (debug > 1)
- printk(KERN_INFO "%s: Abort %8.8x, frame dropped.\n",
- dev->name, intr_status);
+ netdev_info(dev, "Abort %08x, frame dropped\n",
+ intr_status);
}
if (intr_status & IntrTxUnderrun) {
if (rp->tx_thresh < 0xE0)
BYTE_REG_BITS_SET((rp->tx_thresh += 0x20), 0x80, ioaddr + TxConfig);
if (debug > 1)
- printk(KERN_INFO "%s: Transmitter underrun, Tx "
- "threshold now %2.2x.\n",
- dev->name, rp->tx_thresh);
+ netdev_info(dev, "Transmitter underrun, Tx threshold now %02x\n",
+ rp->tx_thresh);
}
if (intr_status & IntrTxDescRace) {
if (debug > 2)
- printk(KERN_INFO "%s: Tx descriptor write-back race.\n",
- dev->name);
+ netdev_info(dev, "Tx descriptor write-back race\n");
}
if ((intr_status & IntrTxError) &&
(intr_status & (IntrTxAborted |
@@ -1932,9 +1922,8 @@ static void rhine_error(struct net_device *dev, int intr_status)
BYTE_REG_BITS_SET((rp->tx_thresh += 0x20), 0x80, ioaddr + TxConfig);
}
if (debug > 1)
- printk(KERN_INFO "%s: Unspecified error. Tx "
- "threshold now %2.2x.\n",
- dev->name, rp->tx_thresh);
+ netdev_info(dev, "Unspecified error. Tx threshold now %02x\n",
+ rp->tx_thresh);
}
if (intr_status & (IntrTxAborted | IntrTxUnderrun | IntrTxDescRace |
IntrTxError))
@@ -1944,8 +1933,8 @@ static void rhine_error(struct net_device *dev, int intr_status)
IntrTxError | IntrTxAborted | IntrNormalSummary |
IntrTxDescRace)) {
if (debug > 1)
- printk(KERN_ERR "%s: Something Wicked happened! "
- "%8.8x.\n", dev->name, intr_status);
+ netdev_err(dev, "Something Wicked happened! %08x\n",
+ intr_status);
}
spin_unlock(&rp->lock);
@@ -2145,9 +2134,8 @@ static int rhine_close(struct net_device *dev)
spin_lock_irq(&rp->lock);
if (debug > 1)
- printk(KERN_DEBUG "%s: Shutting down ethercard, "
- "status was %4.4x.\n",
- dev->name, ioread16(ioaddr + ChipCmd));
+ netdev_dbg(dev, "Shutting down ethercard, status was %04x\n",
+ ioread16(ioaddr + ChipCmd));
/* Switch to loopback mode to avoid hardware races. */
iowrite8(rp->tx_thresh | 0x02, ioaddr + TxConfig);
@@ -2265,12 +2253,12 @@ static int rhine_resume(struct pci_dev *pdev)
return 0;
if (request_irq(dev->irq, rhine_interrupt, IRQF_SHARED, dev->name, dev))
- printk(KERN_ERR "via-rhine %s: request_irq failed\n", dev->name);
+ netdev_err(dev, "request_irq failed\n");
ret = pci_set_power_state(pdev, PCI_D0);
if (debug > 1)
- printk(KERN_INFO "%s: Entering power state D0 %s (%d).\n",
- dev->name, ret ? "failed" : "succeeded", ret);
+ netdev_info(dev, "Entering power state D0 %s (%d)\n",
+ ret ? "failed" : "succeeded", ret);
pci_restore_state(pdev);
@@ -2326,17 +2314,15 @@ static int __init rhine_init(void)
{
/* when a module, this is printed whether or not devices are found in probe */
#ifdef MODULE
- printk(version);
+ pr_info("%s\n", version);
#endif
if (dmi_check_system(rhine_dmi_table)) {
/* these BIOSes fail at PXE boot if chip is in D3 */
avoid_D3 = 1;
- printk(KERN_WARNING "%s: Broken BIOS detected, avoid_D3 "
- "enabled.\n",
- DRV_NAME);
+ pr_warn("Broken BIOS detected, avoid_D3 enabled\n");
}
else if (avoid_D3)
- printk(KERN_INFO "%s: avoid_D3 set.\n", DRV_NAME);
+ pr_info("avoid_D3 set\n");
return pci_register_driver(&rhine_driver);
}
--
1.7.4.2.g597a6.dirty
^ permalink raw reply related
* [PATCH 0/2] via-rhine: Logging cleanups and use random MAC address
From: Joe Perches @ 2011-04-17 0:15 UTC (permalink / raw)
To: Alexandru Gagniuc, netdev; +Cc: Roger Luethi, David S. Miller, linux-kernel
In-Reply-To: <4DAA12C0.7030106@gmail.com>
Alexandru's patch didn't apply so use the
current logging standards and fix the whitespace
in Alexandru's patch.
Joe Perches (2):
via_rhine: Use netdev_<level> and pr_<level>
via-rhine: Assign random MAC address if necessary
drivers/net/via-rhine.c | 240 ++++++++++++++++++++++-------------------------
1 files changed, 114 insertions(+), 126 deletions(-)
--
1.7.4.2.g597a6.dirty
^ permalink raw reply
* Re: [PATCH] via-rhine: do not freak out due to invalid MAC address
From: David Miller @ 2011-04-17 0:11 UTC (permalink / raw)
To: mr.nuke.me; +Cc: netdev, rl
In-Reply-To: <4DAA12C0.7030106@gmail.com>
From: "Alex G." <mr.nuke.me@gmail.com>
Date: Sun, 17 Apr 2011 01:05:52 +0300
> via-rhine drops out of the init code if the hardware provides an invalid
> MAC address. Roger Luethi has had several reports of Rhine NICs doing just
> that. The hardware still works, though; assigning a random MAC address
> allows the NIC to be used as usual. Tested as a standalone interface,
> as carrier for ppp, and as bonding slave.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Your patch is corrupted, your email client turned tab characters
into spaces, which makes your patch unusable.
Please read Documentation/email-clients.txt to learn how to fix
this, try sending the patch to yourself and testing that you
can actually apply the patch as you receive it, and then make a
full, clean, resubmission of the fixed patch.
Thanks.
^ permalink raw reply
* Re: [PATCH] via-rhine: do not freak out due to invalid MAC address
From: Ben Hutchings @ 2011-04-17 0:08 UTC (permalink / raw)
To: Alex G.; +Cc: netdev, Roger Luethi, David S. Miller
In-Reply-To: <4DAA12C0.7030106@gmail.com>
On Sun, 2011-04-17 at 01:05 +0300, Alex G. wrote:
> via-rhine drops out of the init code if the hardware provides an invalid
> MAC address. Roger Luethi has had several reports of Rhine NICs doing just
> that. The hardware still works, though; assigning a random MAC address
> allows the NIC to be used as usual. Tested as a standalone interface,
> as carrier for ppp, and as bonding slave.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
>
> diff --git a/drivers/net/via-rhine.c b/drivers/net/via-rhine.c
> index 0422a79..e7e0fa9 100644
> --- a/drivers/net/via-rhine.c
> +++ b/drivers/net/via-rhine.c
> @@ -836,13 +836,18 @@ static int __devinit rhine_init_one(struct pci_dev *pdev,
>
> for (i = 0; i < 6; i++)
> dev->dev_addr[i] = ioread8(ioaddr + StationAddr + i);
> - memcpy(dev->perm_addr, dev->dev_addr, dev->addr_len);
>
> - if (!is_valid_ether_addr(dev->perm_addr)) {
> - rc = -EIO;
> - printk(KERN_ERR "Invalid MAC address\n");
> - goto err_out_unmap;
> + if (!is_valid_ether_addr(dev->dev_addr)) {
> + printk(KERN_ERR "via-rhine: Invalid MAC address: %pM. \n",
> + dev->dev_addr);
> + /* The device may still be used normally if a valid MAC is
> + * configured
> + */
> + random_ether_addr(dev->dev_addr);
> + printk(KERN_ERR "via-rhine: Using randomly generated address:"
> + "%pM instead. \n", dev->dev_addr);
> }
> + memcpy(dev->perm_addr, dev->dev_addr, dev->addr_len);
I don't think you should hide the original address (presumably the
'permanent' address from EEPORM) in this way. How about changing the
initial loop to initialise dev->perm_addr, then either copying that to
dev->dev_addr or generating a random address in dev->dev_addr?
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* [PATCH v3] net: cxgb4{,vf}: convert to hw_features
From: Michał Mirosław @ 2011-04-16 23:05 UTC (permalink / raw)
To: netdev; +Cc: Dimitris Michailidis, Casey Leedom
In-Reply-To: <20110416165226.B881F13A65@rere.qmqm.pl>
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
v3: cxgb4vf: fix hw/vlan_features values again
v2: cxgb4: remove now unneeded variable in t4_ethrx_handler()
cxgb4vf: fix hw/vlan_features values
drivers/net/cxgb4/cxgb4.h | 6 ---
drivers/net/cxgb4/cxgb4_main.c | 72 ++++++++---------------------------
drivers/net/cxgb4/sge.c | 4 +-
drivers/net/cxgb4vf/adapter.h | 6 ---
drivers/net/cxgb4vf/cxgb4vf_main.c | 57 ++++------------------------
drivers/net/cxgb4vf/sge.c | 4 +-
6 files changed, 28 insertions(+), 121 deletions(-)
diff --git a/drivers/net/cxgb4/cxgb4.h b/drivers/net/cxgb4/cxgb4.h
index 01d49ea..bc9982a 100644
--- a/drivers/net/cxgb4/cxgb4.h
+++ b/drivers/net/cxgb4/cxgb4.h
@@ -290,7 +290,6 @@ struct port_info {
u8 port_id;
u8 tx_chan;
u8 lport; /* associated offload logical port */
- u8 rx_offload; /* CSO, etc */
u8 nqsets; /* # of qsets */
u8 first_qset; /* index of first qset */
u8 rss_mode;
@@ -298,11 +297,6 @@ struct port_info {
u16 *rss;
};
-/* port_info.rx_offload flags */
-enum {
- RX_CSO = 1 << 0,
-};
-
struct dentry;
struct work_struct;
diff --git a/drivers/net/cxgb4/cxgb4_main.c b/drivers/net/cxgb4/cxgb4_main.c
index 0af9c9f..bdc868c 100644
--- a/drivers/net/cxgb4/cxgb4_main.c
+++ b/drivers/net/cxgb4/cxgb4_main.c
@@ -1531,24 +1531,6 @@ static int set_pauseparam(struct net_device *dev,
return 0;
}
-static u32 get_rx_csum(struct net_device *dev)
-{
- struct port_info *p = netdev_priv(dev);
-
- return p->rx_offload & RX_CSO;
-}
-
-static int set_rx_csum(struct net_device *dev, u32 data)
-{
- struct port_info *p = netdev_priv(dev);
-
- if (data)
- p->rx_offload |= RX_CSO;
- else
- p->rx_offload &= ~RX_CSO;
- return 0;
-}
-
static void get_sge_param(struct net_device *dev, struct ethtool_ringparam *e)
{
const struct port_info *pi = netdev_priv(dev);
@@ -1870,36 +1852,20 @@ static int set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
return err;
}
-#define TSO_FLAGS (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_TSO_ECN)
-
-static int set_tso(struct net_device *dev, u32 value)
-{
- if (value)
- dev->features |= TSO_FLAGS;
- else
- dev->features &= ~TSO_FLAGS;
- return 0;
-}
-
-static int set_flags(struct net_device *dev, u32 flags)
+static int cxgb_set_features(struct net_device *dev, u32 features)
{
+ const struct port_info *pi = netdev_priv(dev);
+ u32 changed = dev->features ^ features;
int err;
- unsigned long old_feat = dev->features;
- err = ethtool_op_set_flags(dev, flags, ETH_FLAG_RXHASH |
- ETH_FLAG_RXVLAN | ETH_FLAG_TXVLAN);
- if (err)
- return err;
+ if (!(changed & NETIF_F_HW_VLAN_RX))
+ return 0;
- if ((old_feat ^ dev->features) & NETIF_F_HW_VLAN_RX) {
- const struct port_info *pi = netdev_priv(dev);
-
- err = t4_set_rxmode(pi->adapter, pi->adapter->fn, pi->viid, -1,
- -1, -1, -1, !!(flags & ETH_FLAG_RXVLAN),
- true);
- if (err)
- dev->features = old_feat;
- }
+ err = t4_set_rxmode(pi->adapter, pi->adapter->fn, pi->viid, -1,
+ -1, -1, -1,
+ !!(features & NETIF_F_HW_VLAN_RX), true);
+ if (unlikely(err))
+ dev->features = features ^ NETIF_F_HW_VLAN_RX;
return err;
}
@@ -2010,10 +1976,6 @@ static struct ethtool_ops cxgb_ethtool_ops = {
.set_eeprom = set_eeprom,
.get_pauseparam = get_pauseparam,
.set_pauseparam = set_pauseparam,
- .get_rx_csum = get_rx_csum,
- .set_rx_csum = set_rx_csum,
- .set_tx_csum = ethtool_op_set_tx_ipv6_csum,
- .set_sg = ethtool_op_set_sg,
.get_link = ethtool_op_get_link,
.get_strings = get_strings,
.set_phys_id = identify_port,
@@ -2024,8 +1986,6 @@ static struct ethtool_ops cxgb_ethtool_ops = {
.get_regs = get_regs,
.get_wol = get_wol,
.set_wol = set_wol,
- .set_tso = set_tso,
- .set_flags = set_flags,
.get_rxnfc = get_rxnfc,
.get_rxfh_indir = get_rss_table,
.set_rxfh_indir = set_rss_table,
@@ -2882,6 +2842,7 @@ static const struct net_device_ops cxgb4_netdev_ops = {
.ndo_get_stats64 = cxgb_get_stats,
.ndo_set_rx_mode = cxgb_set_rxmode,
.ndo_set_mac_address = cxgb_set_mac_addr,
+ .ndo_set_features = cxgb_set_features,
.ndo_validate_addr = eth_validate_addr,
.ndo_do_ioctl = cxgb_ioctl,
.ndo_change_mtu = cxgb_change_mtu,
@@ -3564,6 +3525,7 @@ static void free_some_resources(struct adapter *adapter)
t4_fw_bye(adapter, adapter->fn);
}
+#define TSO_FLAGS (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_TSO_ECN)
#define VLAN_FEAT (NETIF_F_SG | NETIF_F_IP_CSUM | TSO_FLAGS | \
NETIF_F_IPV6_CSUM | NETIF_F_HIGHDMA)
@@ -3665,14 +3627,14 @@ static int __devinit init_one(struct pci_dev *pdev,
pi = netdev_priv(netdev);
pi->adapter = adapter;
pi->xact_addr_filt = -1;
- pi->rx_offload = RX_CSO;
pi->port_id = i;
netdev->irq = pdev->irq;
- netdev->features |= NETIF_F_SG | TSO_FLAGS;
- netdev->features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
- netdev->features |= NETIF_F_GRO | NETIF_F_RXHASH | highdma;
- netdev->features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
+ netdev->hw_features = NETIF_F_SG | TSO_FLAGS |
+ NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
+ NETIF_F_RXCSUM | NETIF_F_RXHASH |
+ NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
+ netdev->features |= netdev->hw_features | highdma;
netdev->vlan_features = netdev->features & VLAN_FEAT;
netdev->netdev_ops = &cxgb4_netdev_ops;
diff --git a/drivers/net/cxgb4/sge.c b/drivers/net/cxgb4/sge.c
index 311471b..75a4b0f 100644
--- a/drivers/net/cxgb4/sge.c
+++ b/drivers/net/cxgb4/sge.c
@@ -1556,7 +1556,6 @@ int t4_ethrx_handler(struct sge_rspq *q, const __be64 *rsp,
{
bool csum_ok;
struct sk_buff *skb;
- struct port_info *pi;
const struct cpl_rx_pkt *pkt;
struct sge_eth_rxq *rxq = container_of(q, struct sge_eth_rxq, rspq);
@@ -1584,10 +1583,9 @@ int t4_ethrx_handler(struct sge_rspq *q, const __be64 *rsp,
if (skb->dev->features & NETIF_F_RXHASH)
skb->rxhash = (__force u32)pkt->rsshdr.hash_val;
- pi = netdev_priv(skb->dev);
rxq->stats.pkts++;
- if (csum_ok && (pi->rx_offload & RX_CSO) &&
+ if (csum_ok && (q->netdev->features & NETIF_F_RXCSUM) &&
(pkt->l2info & htonl(RXF_UDP | RXF_TCP))) {
if (!pkt->ip_frag) {
skb->ip_summed = CHECKSUM_UNNECESSARY;
diff --git a/drivers/net/cxgb4vf/adapter.h b/drivers/net/cxgb4vf/adapter.h
index 4766b41..4fd821a 100644
--- a/drivers/net/cxgb4vf/adapter.h
+++ b/drivers/net/cxgb4vf/adapter.h
@@ -97,17 +97,11 @@ struct port_info {
u16 rss_size; /* size of VI's RSS table slice */
u8 pidx; /* index into adapter port[] */
u8 port_id; /* physical port ID */
- u8 rx_offload; /* CSO, etc. */
u8 nqsets; /* # of "Queue Sets" */
u8 first_qset; /* index of first "Queue Set" */
struct link_config link_cfg; /* physical port configuration */
};
-/* port_info.rx_offload flags */
-enum {
- RX_CSO = 1 << 0,
-};
-
/*
* Scatter Gather Engine resources for the "adapter". Our ingress and egress
* queues are organized into "Queue Sets" with one ingress and one egress
diff --git a/drivers/net/cxgb4vf/cxgb4vf_main.c b/drivers/net/cxgb4vf/cxgb4vf_main.c
index c662679..8cf9890 100644
--- a/drivers/net/cxgb4vf/cxgb4vf_main.c
+++ b/drivers/net/cxgb4vf/cxgb4vf_main.c
@@ -1326,30 +1326,6 @@ static void cxgb4vf_get_pauseparam(struct net_device *dev,
}
/*
- * Return whether RX Checksum Offloading is currently enabled for the device.
- */
-static u32 cxgb4vf_get_rx_csum(struct net_device *dev)
-{
- struct port_info *pi = netdev_priv(dev);
-
- return (pi->rx_offload & RX_CSO) != 0;
-}
-
-/*
- * Turn RX Checksum Offloading on or off for the device.
- */
-static int cxgb4vf_set_rx_csum(struct net_device *dev, u32 csum)
-{
- struct port_info *pi = netdev_priv(dev);
-
- if (csum)
- pi->rx_offload |= RX_CSO;
- else
- pi->rx_offload &= ~RX_CSO;
- return 0;
-}
-
-/*
* Identify the port by blinking the port's LED.
*/
static int cxgb4vf_phys_id(struct net_device *dev,
@@ -1569,18 +1545,6 @@ static void cxgb4vf_get_wol(struct net_device *dev,
*/
#define TSO_FLAGS (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_TSO_ECN)
-/*
- * Set TCP Segmentation Offloading feature capabilities.
- */
-static int cxgb4vf_set_tso(struct net_device *dev, u32 tso)
-{
- if (tso)
- dev->features |= TSO_FLAGS;
- else
- dev->features &= ~TSO_FLAGS;
- return 0;
-}
-
static struct ethtool_ops cxgb4vf_ethtool_ops = {
.get_settings = cxgb4vf_get_settings,
.get_drvinfo = cxgb4vf_get_drvinfo,
@@ -1591,10 +1555,6 @@ static struct ethtool_ops cxgb4vf_ethtool_ops = {
.get_coalesce = cxgb4vf_get_coalesce,
.set_coalesce = cxgb4vf_set_coalesce,
.get_pauseparam = cxgb4vf_get_pauseparam,
- .get_rx_csum = cxgb4vf_get_rx_csum,
- .set_rx_csum = cxgb4vf_set_rx_csum,
- .set_tx_csum = ethtool_op_set_tx_ipv6_csum,
- .set_sg = ethtool_op_set_sg,
.get_link = ethtool_op_get_link,
.get_strings = cxgb4vf_get_strings,
.set_phys_id = cxgb4vf_phys_id,
@@ -1603,7 +1563,6 @@ static struct ethtool_ops cxgb4vf_ethtool_ops = {
.get_regs_len = cxgb4vf_get_regs_len,
.get_regs = cxgb4vf_get_regs,
.get_wol = cxgb4vf_get_wol,
- .set_tso = cxgb4vf_set_tso,
};
/*
@@ -2638,19 +2597,19 @@ static int __devinit cxgb4vf_pci_probe(struct pci_dev *pdev,
* it.
*/
pi->xact_addr_filt = -1;
- pi->rx_offload = RX_CSO;
netif_carrier_off(netdev);
netdev->irq = pdev->irq;
- netdev->features = (NETIF_F_SG | TSO_FLAGS |
- NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
- NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX |
- NETIF_F_GRO);
+ netdev->hw_features = NETIF_F_SG | TSO_FLAGS |
+ NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
+ NETIF_F_HW_VLAN_TX | NETIF_F_RXCSUM;
+ netdev->vlan_features = NETIF_F_SG | TSO_FLAGS |
+ NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
+ NETIF_F_HIGHDMA;
+ netdev->features = netdev->hw_features |
+ NETIF_F_HW_VLAN_RX;
if (pci_using_dac)
netdev->features |= NETIF_F_HIGHDMA;
- netdev->vlan_features =
- (netdev->features &
- ~(NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX));
#ifdef HAVE_NET_DEVICE_OPS
netdev->netdev_ops = &cxgb4vf_netdev_ops;
diff --git a/drivers/net/cxgb4vf/sge.c b/drivers/net/cxgb4vf/sge.c
index bb65121..5182960 100644
--- a/drivers/net/cxgb4vf/sge.c
+++ b/drivers/net/cxgb4vf/sge.c
@@ -1555,8 +1555,8 @@ int t4vf_ethrx_handler(struct sge_rspq *rspq, const __be64 *rsp,
pi = netdev_priv(skb->dev);
rxq->stats.pkts++;
- if (csum_ok && (pi->rx_offload & RX_CSO) && !pkt->err_vec &&
- (be32_to_cpu(pkt->l2info) & (RXF_UDP|RXF_TCP))) {
+ if (csum_ok && (rspq->netdev->features & NETIF_F_RXCSUM) &&
+ !pkt->err_vec && (be32_to_cpu(pkt->l2info) & (RXF_UDP|RXF_TCP))) {
if (!pkt->ip_frag)
skb->ip_summed = CHECKSUM_UNNECESSARY;
else {
--
1.7.2.5
^ permalink raw reply related
* Re: [PATCH v2] net: cxgb4{,vf}: convert to hw_features
From: Michał Mirosław @ 2011-04-16 23:04 UTC (permalink / raw)
To: Dimitris Michailidis; +Cc: netdev, Casey Leedom
In-Reply-To: <4DA9DEBF.8050300@chelsio.com>
On Sat, Apr 16, 2011 at 11:23:59AM -0700, Dimitris Michailidis wrote:
> Michał Mirosław wrote:
>> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
>> ---
>> v2: cxgb4: remove now unneeded variable in t4_ethrx_handler()
>> cxgb4vf: fix hw/vlan_features values
>
> Thanks for doing this. The cxgb4 part looks good. For cxgb4vf I have a
> comment below.
>
>> @@ -2638,19 +2597,18 @@ static int __devinit cxgb4vf_pci_probe(struct pci_dev *pdev,
>> - netdev->features = (NETIF_F_SG | TSO_FLAGS |
>> - NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
>> - NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX |
>> - NETIF_F_GRO);
>> + netdev->hw_features = NETIF_F_SG | TSO_FLAGS | NETIF_F_RXCSUM |
>> + NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
>> + netdev->vlan_features = NETIF_F_SG | TSO_FLAGS |
>> + NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
>> + NETIF_F_HW_VLAN_TX | NETIF_F_HIGHDMA;
>
> This NETIF_F_HW_VLAN_TX looks misplaced. Did you mean to add it to
> hw_features rather than vlan_features?
Yes. You're right! [v3 following]
Best Regards,
Michał Mirosław
^ permalink raw reply
* [PATCH] via-rhine: do not freak out due to invalid MAC address
From: Alex G. @ 2011-04-16 22:05 UTC (permalink / raw)
To: netdev; +Cc: Roger Luethi, David S. Miller
via-rhine drops out of the init code if the hardware provides an invalid
MAC address. Roger Luethi has had several reports of Rhine NICs doing just
that. The hardware still works, though; assigning a random MAC address
allows the NIC to be used as usual. Tested as a standalone interface,
as carrier for ppp, and as bonding slave.
Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
diff --git a/drivers/net/via-rhine.c b/drivers/net/via-rhine.c
index 0422a79..e7e0fa9 100644
--- a/drivers/net/via-rhine.c
+++ b/drivers/net/via-rhine.c
@@ -836,13 +836,18 @@ static int __devinit rhine_init_one(struct pci_dev *pdev,
for (i = 0; i < 6; i++)
dev->dev_addr[i] = ioread8(ioaddr + StationAddr + i);
- memcpy(dev->perm_addr, dev->dev_addr, dev->addr_len);
- if (!is_valid_ether_addr(dev->perm_addr)) {
- rc = -EIO;
- printk(KERN_ERR "Invalid MAC address\n");
- goto err_out_unmap;
+ if (!is_valid_ether_addr(dev->dev_addr)) {
+ printk(KERN_ERR "via-rhine: Invalid MAC address: %pM. \n",
+ dev->dev_addr);
+ /* The device may still be used normally if a valid MAC is
+ * configured
+ */
+ random_ether_addr(dev->dev_addr);
+ printk(KERN_ERR "via-rhine: Using randomly generated address:"
+ "%pM instead. \n", dev->dev_addr);
}
+ memcpy(dev->perm_addr, dev->dev_addr, dev->addr_len);
/* For Rhine-I/II, phy_id is loaded from EEPROM */
if (!phy_id)
^ permalink raw reply related
* Re: [PATCH v2] net: cxgb4{,vf}: convert to hw_features
From: Dimitris Michailidis @ 2011-04-16 18:23 UTC (permalink / raw)
To: Michał Mirosław; +Cc: netdev, Casey Leedom
In-Reply-To: <20110416165226.B881F13A65@rere.qmqm.pl>
Michał Mirosław wrote:
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
> v2: cxgb4: remove now unneeded variable in t4_ethrx_handler()
> cxgb4vf: fix hw/vlan_features values
Thanks for doing this. The cxgb4 part looks good. For cxgb4vf I have a
comment below.
> @@ -2638,19 +2597,18 @@ static int __devinit cxgb4vf_pci_probe(struct pci_dev *pdev,
> - netdev->features = (NETIF_F_SG | TSO_FLAGS |
> - NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
> - NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX |
> - NETIF_F_GRO);
> + netdev->hw_features = NETIF_F_SG | TSO_FLAGS | NETIF_F_RXCSUM |
> + NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
> + netdev->vlan_features = NETIF_F_SG | TSO_FLAGS |
> + NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
> + NETIF_F_HW_VLAN_TX | NETIF_F_HIGHDMA;
This NETIF_F_HW_VLAN_TX looks misplaced. Did you mean to add it to
hw_features rather than vlan_features?
^ permalink raw reply
* Re: [PATCH] net: cxgb4{,vf}: convert to hw_features
From: Michał Mirosław @ 2011-04-16 16:56 UTC (permalink / raw)
To: Dimitris Michailidis; +Cc: netdev, Casey Leedom
In-Reply-To: <4DA895DF.1090809@chelsio.com>
On Fri, Apr 15, 2011 at 12:00:47PM -0700, Dimitris Michailidis wrote:
> Michał Mirosław wrote:
[...]
>> + netdev->hw_features = NETIF_F_SG | TSO_FLAGS |
>> + NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
>> + NETIF_F_RXCSUM | NETIF_F_RXHASH |
>> + NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
>> + netdev->features |= netdev->hw_features | highdma;
>> netdev->vlan_features = netdev->features & VLAN_FEAT;
> Here vlan_features does not include NETIF_F_RXCSUM but the cxgb4vf bits
> below do include it. I looked at some other drivers and saw again some
> include it and some don't. The core VLAN code handles NETIF_F_RXCSUM on
> its own. Is there some rule for whether drivers should set it in their
> vlan_features or not?
Since NETIF_F_RXCSUM is ignored in vlan_features I'd rather see it not set.
(fixed in v2)
>> diff --git a/drivers/net/cxgb4/sge.c b/drivers/net/cxgb4/sge.c
>> index 311471b..e8f6f8e 100644
>> --- a/drivers/net/cxgb4/sge.c
>> +++ b/drivers/net/cxgb4/sge.c
>> @@ -1587,7 +1587,7 @@ int t4_ethrx_handler(struct sge_rspq *q, const __be64 *rsp,
>> pi = netdev_priv(skb->dev);
>> rxq->stats.pkts++;
>> - if (csum_ok && (pi->rx_offload & RX_CSO) &&
>> + if (csum_ok && (q->netdev->features & NETIF_F_RXCSUM) &&
>> (pkt->l2info & htonl(RXF_UDP | RXF_TCP))) {
>> if (!pkt->ip_frag) {
>> skb->ip_summed = CHECKSUM_UNNECESSARY;
> With this change variable 'pi' can be removed but I can do this cleanup
> after this patch goes in.
I've included it in v2.
>> diff --git a/drivers/net/cxgb4vf/cxgb4vf_main.c b/drivers/net/cxgb4vf/cxgb4vf_main.c
>> index c662679..04a5c2d 100644
>> --- a/drivers/net/cxgb4vf/cxgb4vf_main.c
>> +++ b/drivers/net/cxgb4vf/cxgb4vf_main.c
>> @@ -2638,14 +2597,13 @@ static int __devinit cxgb4vf_pci_probe(struct pci_dev *pdev,
>> * it.
>> */
>> pi->xact_addr_filt = -1;
>> - pi->rx_offload = RX_CSO;
>> netif_carrier_off(netdev);
>> netdev->irq = pdev->irq;
>> - netdev->features = (NETIF_F_SG | TSO_FLAGS |
>> - NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
>> - NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX |
>> - NETIF_F_GRO);
>> + netdev->hw_features = NETIF_F_SG | TSO_FLAGS | NETIF_F_RXCSUM |
>> + NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
>> + NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
>> + netdev->features = netdev->hw_features;
>> if (pci_using_dac)
>> netdev->features |= NETIF_F_HIGHDMA;
>> netdev->vlan_features =
> cxgb4vf does not implement toggling of NETIF_F_HW_VLAN_RX so I think the
> flag should be set in features but not hw_features or maybe the driver
> needs to handle it in fix_features?
Fixed in v2.
Best Regards,
Michał Mirosław
^ permalink raw reply
* [PATCH v2] net: cxgb4{,vf}: convert to hw_features
From: Michał Mirosław @ 2011-04-16 16:52 UTC (permalink / raw)
To: netdev; +Cc: Dimitris Michailidis, Casey Leedom
In-Reply-To: <20110415145050.6A43913A6A@rere.qmqm.pl>
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
v2: cxgb4: remove now unneeded variable in t4_ethrx_handler()
cxgb4vf: fix hw/vlan_features values
drivers/net/cxgb4/cxgb4.h | 6 ---
drivers/net/cxgb4/cxgb4_main.c | 72 ++++++++---------------------------
drivers/net/cxgb4/sge.c | 4 +-
drivers/net/cxgb4vf/adapter.h | 6 ---
drivers/net/cxgb4vf/cxgb4vf_main.c | 56 +++------------------------
drivers/net/cxgb4vf/sge.c | 4 +-
6 files changed, 27 insertions(+), 121 deletions(-)
diff --git a/drivers/net/cxgb4/cxgb4.h b/drivers/net/cxgb4/cxgb4.h
index 01d49ea..bc9982a 100644
--- a/drivers/net/cxgb4/cxgb4.h
+++ b/drivers/net/cxgb4/cxgb4.h
@@ -290,7 +290,6 @@ struct port_info {
u8 port_id;
u8 tx_chan;
u8 lport; /* associated offload logical port */
- u8 rx_offload; /* CSO, etc */
u8 nqsets; /* # of qsets */
u8 first_qset; /* index of first qset */
u8 rss_mode;
@@ -298,11 +297,6 @@ struct port_info {
u16 *rss;
};
-/* port_info.rx_offload flags */
-enum {
- RX_CSO = 1 << 0,
-};
-
struct dentry;
struct work_struct;
diff --git a/drivers/net/cxgb4/cxgb4_main.c b/drivers/net/cxgb4/cxgb4_main.c
index 0af9c9f..bdc868c 100644
--- a/drivers/net/cxgb4/cxgb4_main.c
+++ b/drivers/net/cxgb4/cxgb4_main.c
@@ -1531,24 +1531,6 @@ static int set_pauseparam(struct net_device *dev,
return 0;
}
-static u32 get_rx_csum(struct net_device *dev)
-{
- struct port_info *p = netdev_priv(dev);
-
- return p->rx_offload & RX_CSO;
-}
-
-static int set_rx_csum(struct net_device *dev, u32 data)
-{
- struct port_info *p = netdev_priv(dev);
-
- if (data)
- p->rx_offload |= RX_CSO;
- else
- p->rx_offload &= ~RX_CSO;
- return 0;
-}
-
static void get_sge_param(struct net_device *dev, struct ethtool_ringparam *e)
{
const struct port_info *pi = netdev_priv(dev);
@@ -1870,36 +1852,20 @@ static int set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
return err;
}
-#define TSO_FLAGS (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_TSO_ECN)
-
-static int set_tso(struct net_device *dev, u32 value)
-{
- if (value)
- dev->features |= TSO_FLAGS;
- else
- dev->features &= ~TSO_FLAGS;
- return 0;
-}
-
-static int set_flags(struct net_device *dev, u32 flags)
+static int cxgb_set_features(struct net_device *dev, u32 features)
{
+ const struct port_info *pi = netdev_priv(dev);
+ u32 changed = dev->features ^ features;
int err;
- unsigned long old_feat = dev->features;
- err = ethtool_op_set_flags(dev, flags, ETH_FLAG_RXHASH |
- ETH_FLAG_RXVLAN | ETH_FLAG_TXVLAN);
- if (err)
- return err;
+ if (!(changed & NETIF_F_HW_VLAN_RX))
+ return 0;
- if ((old_feat ^ dev->features) & NETIF_F_HW_VLAN_RX) {
- const struct port_info *pi = netdev_priv(dev);
-
- err = t4_set_rxmode(pi->adapter, pi->adapter->fn, pi->viid, -1,
- -1, -1, -1, !!(flags & ETH_FLAG_RXVLAN),
- true);
- if (err)
- dev->features = old_feat;
- }
+ err = t4_set_rxmode(pi->adapter, pi->adapter->fn, pi->viid, -1,
+ -1, -1, -1,
+ !!(features & NETIF_F_HW_VLAN_RX), true);
+ if (unlikely(err))
+ dev->features = features ^ NETIF_F_HW_VLAN_RX;
return err;
}
@@ -2010,10 +1976,6 @@ static struct ethtool_ops cxgb_ethtool_ops = {
.set_eeprom = set_eeprom,
.get_pauseparam = get_pauseparam,
.set_pauseparam = set_pauseparam,
- .get_rx_csum = get_rx_csum,
- .set_rx_csum = set_rx_csum,
- .set_tx_csum = ethtool_op_set_tx_ipv6_csum,
- .set_sg = ethtool_op_set_sg,
.get_link = ethtool_op_get_link,
.get_strings = get_strings,
.set_phys_id = identify_port,
@@ -2024,8 +1986,6 @@ static struct ethtool_ops cxgb_ethtool_ops = {
.get_regs = get_regs,
.get_wol = get_wol,
.set_wol = set_wol,
- .set_tso = set_tso,
- .set_flags = set_flags,
.get_rxnfc = get_rxnfc,
.get_rxfh_indir = get_rss_table,
.set_rxfh_indir = set_rss_table,
@@ -2882,6 +2842,7 @@ static const struct net_device_ops cxgb4_netdev_ops = {
.ndo_get_stats64 = cxgb_get_stats,
.ndo_set_rx_mode = cxgb_set_rxmode,
.ndo_set_mac_address = cxgb_set_mac_addr,
+ .ndo_set_features = cxgb_set_features,
.ndo_validate_addr = eth_validate_addr,
.ndo_do_ioctl = cxgb_ioctl,
.ndo_change_mtu = cxgb_change_mtu,
@@ -3564,6 +3525,7 @@ static void free_some_resources(struct adapter *adapter)
t4_fw_bye(adapter, adapter->fn);
}
+#define TSO_FLAGS (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_TSO_ECN)
#define VLAN_FEAT (NETIF_F_SG | NETIF_F_IP_CSUM | TSO_FLAGS | \
NETIF_F_IPV6_CSUM | NETIF_F_HIGHDMA)
@@ -3665,14 +3627,14 @@ static int __devinit init_one(struct pci_dev *pdev,
pi = netdev_priv(netdev);
pi->adapter = adapter;
pi->xact_addr_filt = -1;
- pi->rx_offload = RX_CSO;
pi->port_id = i;
netdev->irq = pdev->irq;
- netdev->features |= NETIF_F_SG | TSO_FLAGS;
- netdev->features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
- netdev->features |= NETIF_F_GRO | NETIF_F_RXHASH | highdma;
- netdev->features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
+ netdev->hw_features = NETIF_F_SG | TSO_FLAGS |
+ NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
+ NETIF_F_RXCSUM | NETIF_F_RXHASH |
+ NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
+ netdev->features |= netdev->hw_features | highdma;
netdev->vlan_features = netdev->features & VLAN_FEAT;
netdev->netdev_ops = &cxgb4_netdev_ops;
diff --git a/drivers/net/cxgb4/sge.c b/drivers/net/cxgb4/sge.c
index 311471b..75a4b0f 100644
--- a/drivers/net/cxgb4/sge.c
+++ b/drivers/net/cxgb4/sge.c
@@ -1556,7 +1556,6 @@ int t4_ethrx_handler(struct sge_rspq *q, const __be64 *rsp,
{
bool csum_ok;
struct sk_buff *skb;
- struct port_info *pi;
const struct cpl_rx_pkt *pkt;
struct sge_eth_rxq *rxq = container_of(q, struct sge_eth_rxq, rspq);
@@ -1584,10 +1583,9 @@ int t4_ethrx_handler(struct sge_rspq *q, const __be64 *rsp,
if (skb->dev->features & NETIF_F_RXHASH)
skb->rxhash = (__force u32)pkt->rsshdr.hash_val;
- pi = netdev_priv(skb->dev);
rxq->stats.pkts++;
- if (csum_ok && (pi->rx_offload & RX_CSO) &&
+ if (csum_ok && (q->netdev->features & NETIF_F_RXCSUM) &&
(pkt->l2info & htonl(RXF_UDP | RXF_TCP))) {
if (!pkt->ip_frag) {
skb->ip_summed = CHECKSUM_UNNECESSARY;
diff --git a/drivers/net/cxgb4vf/adapter.h b/drivers/net/cxgb4vf/adapter.h
index 4766b41..4fd821a 100644
--- a/drivers/net/cxgb4vf/adapter.h
+++ b/drivers/net/cxgb4vf/adapter.h
@@ -97,17 +97,11 @@ struct port_info {
u16 rss_size; /* size of VI's RSS table slice */
u8 pidx; /* index into adapter port[] */
u8 port_id; /* physical port ID */
- u8 rx_offload; /* CSO, etc. */
u8 nqsets; /* # of "Queue Sets" */
u8 first_qset; /* index of first "Queue Set" */
struct link_config link_cfg; /* physical port configuration */
};
-/* port_info.rx_offload flags */
-enum {
- RX_CSO = 1 << 0,
-};
-
/*
* Scatter Gather Engine resources for the "adapter". Our ingress and egress
* queues are organized into "Queue Sets" with one ingress and one egress
diff --git a/drivers/net/cxgb4vf/cxgb4vf_main.c b/drivers/net/cxgb4vf/cxgb4vf_main.c
index c662679..4577f9b 100644
--- a/drivers/net/cxgb4vf/cxgb4vf_main.c
+++ b/drivers/net/cxgb4vf/cxgb4vf_main.c
@@ -1326,30 +1326,6 @@ static void cxgb4vf_get_pauseparam(struct net_device *dev,
}
/*
- * Return whether RX Checksum Offloading is currently enabled for the device.
- */
-static u32 cxgb4vf_get_rx_csum(struct net_device *dev)
-{
- struct port_info *pi = netdev_priv(dev);
-
- return (pi->rx_offload & RX_CSO) != 0;
-}
-
-/*
- * Turn RX Checksum Offloading on or off for the device.
- */
-static int cxgb4vf_set_rx_csum(struct net_device *dev, u32 csum)
-{
- struct port_info *pi = netdev_priv(dev);
-
- if (csum)
- pi->rx_offload |= RX_CSO;
- else
- pi->rx_offload &= ~RX_CSO;
- return 0;
-}
-
-/*
* Identify the port by blinking the port's LED.
*/
static int cxgb4vf_phys_id(struct net_device *dev,
@@ -1569,18 +1545,6 @@ static void cxgb4vf_get_wol(struct net_device *dev,
*/
#define TSO_FLAGS (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_TSO_ECN)
-/*
- * Set TCP Segmentation Offloading feature capabilities.
- */
-static int cxgb4vf_set_tso(struct net_device *dev, u32 tso)
-{
- if (tso)
- dev->features |= TSO_FLAGS;
- else
- dev->features &= ~TSO_FLAGS;
- return 0;
-}
-
static struct ethtool_ops cxgb4vf_ethtool_ops = {
.get_settings = cxgb4vf_get_settings,
.get_drvinfo = cxgb4vf_get_drvinfo,
@@ -1591,10 +1555,6 @@ static struct ethtool_ops cxgb4vf_ethtool_ops = {
.get_coalesce = cxgb4vf_get_coalesce,
.set_coalesce = cxgb4vf_set_coalesce,
.get_pauseparam = cxgb4vf_get_pauseparam,
- .get_rx_csum = cxgb4vf_get_rx_csum,
- .set_rx_csum = cxgb4vf_set_rx_csum,
- .set_tx_csum = ethtool_op_set_tx_ipv6_csum,
- .set_sg = ethtool_op_set_sg,
.get_link = ethtool_op_get_link,
.get_strings = cxgb4vf_get_strings,
.set_phys_id = cxgb4vf_phys_id,
@@ -1603,7 +1563,6 @@ static struct ethtool_ops cxgb4vf_ethtool_ops = {
.get_regs_len = cxgb4vf_get_regs_len,
.get_regs = cxgb4vf_get_regs,
.get_wol = cxgb4vf_get_wol,
- .set_tso = cxgb4vf_set_tso,
};
/*
@@ -2638,19 +2597,18 @@ static int __devinit cxgb4vf_pci_probe(struct pci_dev *pdev,
* it.
*/
pi->xact_addr_filt = -1;
- pi->rx_offload = RX_CSO;
netif_carrier_off(netdev);
netdev->irq = pdev->irq;
- netdev->features = (NETIF_F_SG | TSO_FLAGS |
- NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
- NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX |
- NETIF_F_GRO);
+ netdev->hw_features = NETIF_F_SG | TSO_FLAGS | NETIF_F_RXCSUM |
+ NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
+ netdev->vlan_features = NETIF_F_SG | TSO_FLAGS |
+ NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
+ NETIF_F_HW_VLAN_TX | NETIF_F_HIGHDMA;
+ netdev->features = netdev->hw_features |
+ NETIF_F_HW_VLAN_RX;
if (pci_using_dac)
netdev->features |= NETIF_F_HIGHDMA;
- netdev->vlan_features =
- (netdev->features &
- ~(NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX));
#ifdef HAVE_NET_DEVICE_OPS
netdev->netdev_ops = &cxgb4vf_netdev_ops;
diff --git a/drivers/net/cxgb4vf/sge.c b/drivers/net/cxgb4vf/sge.c
index bb65121..5182960 100644
--- a/drivers/net/cxgb4vf/sge.c
+++ b/drivers/net/cxgb4vf/sge.c
@@ -1555,8 +1555,8 @@ int t4vf_ethrx_handler(struct sge_rspq *rspq, const __be64 *rsp,
pi = netdev_priv(skb->dev);
rxq->stats.pkts++;
- if (csum_ok && (pi->rx_offload & RX_CSO) && !pkt->err_vec &&
- (be32_to_cpu(pkt->l2info) & (RXF_UDP|RXF_TCP))) {
+ if (csum_ok && (rspq->netdev->features & NETIF_F_RXCSUM) &&
+ !pkt->err_vec && (be32_to_cpu(pkt->l2info) & (RXF_UDP|RXF_TCP))) {
if (!pkt->ip_frag)
skb->ip_summed = CHECKSUM_UNNECESSARY;
else {
--
1.7.2.5
^ permalink raw reply related
* Re: [RFC net-next] bonding: notify when bonding device address changes
From: Stephen Hemminger @ 2011-04-16 16:18 UTC (permalink / raw)
To: Jay Vosburgh
Cc: Nicolas de Pesloüan, Michał Górny, netdev, roy,
Andy Gospodarek
In-Reply-To: <11728.1302895704@death>
On Fri, 15 Apr 2011 12:28:24 -0700
Jay Vosburgh <fubar@us.ibm.com> wrote:
> Stephen Hemminger <shemminger@vyatta.com> wrote:
>
> >When a device changes its hardware address, it needs to call the network
> >device notifiers to inform protocols.
> >
> >Compile tested only.
I did not audit that all the call sites have the proper locking.
When calling notifier, no bridge locks should be held and RTNL mutex
should be held.
^ permalink raw reply
* Re: net: Automatic IRQ siloing for network devices
From: Stephen Hemminger @ 2011-04-16 16:17 UTC (permalink / raw)
To: Neil Horman; +Cc: Ben Hutchings, netdev, davem
In-Reply-To: <20110416015938.GB2200@neilslaptop.think-freely.org>
On Fri, 15 Apr 2011 21:59:38 -0400
Neil Horman <nhorman@tuxdriver.com> wrote:
> On Fri, Apr 15, 2011 at 11:54:29PM +0100, Ben Hutchings wrote:
> > On Fri, 2011-04-15 at 16:17 -0400, Neil Horman wrote:
> > > Automatic IRQ siloing for network devices
> > >
> > > At last years netconf:
> > > http://vger.kernel.org/netconf2010.html
> > >
> > > Tom Herbert gave a talk in which he outlined some of the things we can do to
> > > improve scalability and througput in our network stack
> > >
> > > One of the big items on the slides was the notion of siloing irqs, which is the
> > > practice of setting irq affinity to a cpu or cpu set that was 'close' to the
> > > process that would be consuming data. The idea was to ensure that a hard irq
> > > for a nic (and its subsequent softirq) would execute on the same cpu as the
> > > process consuming the data, increasing cache hit rates and speeding up overall
> > > throughput.
> > >
> > > I had taken an idea away from that talk, and have finally gotten around to
> > > implementing it. One of the problems with the above approach is that its all
> > > quite manual. I.e. to properly enact this siloiong, you have to do a few things
> > > by hand:
> > >
> > > 1) decide which process is the heaviest user of a given rx queue
> > > 2) restrict the cpus which that task will run on
> > > 3) identify the irq which the rx queue in (1) maps to
> > > 4) manually set the affinity for the irq in (3) to cpus which match the cpus in
> > > (2)
> > [...]
> >
> > This presumably works well with small numbers of flows and/or large
> > numbers of queues. You could scale it up somewhat by manipulating the
> > device's flow hash indirection table, but that usually only has 128
> > entries. (Changing the indirection table is currently quite expensive,
> > though that could be changed.)
> >
> > I see RFS and accelerated RFS as the only reasonable way to scale to
> > large numbers of flows. And as part of accelerated RFS, I already did
> > the work for mapping CPUs to IRQs (note, not the other way round). If
> > IRQ affinity keeps changing then it will significantly undermine the
> > usefulness of hardware flow steering.
> >
> > Now I'm not saying that your approach is useless. There is more
> > hardware out there with flow hashing than with flow steering, and there
> > are presumably many systems with small numbers of active flows. But I
> > think we need to avoid having two features that conflict and a
> > requirement for administrators to make a careful selection between them.
> >
> > Ben.
> >
> I hear what your saying and I agree, theres no point in having features work
> against each other. That said, I'm not sure I agree that these features have to
> work against one another, nor does a sysadmin need to make a choice between the
> two. Note the third patch in this series. Making this work requires that
> network drivers wanting to participate in this affinity algorithm opt in by
> using the request_net_irq macro to attach the interrupt to the rfs affinity code
> that I added. Theres no reason that a driver which supports hardware that still
> uses flow steering can't opt out of this algorithm, and as a result irqbalance
> will still treat those interrupts as it normally does. And for those drivers
> which do opt in, irqbalance can take care of affinity assignment, using the
> provided hint. No need for sysadmin intervention.
>
> I'm sure there can be improvements made to this code, but I think theres less
> conflict between the work you've done and this code than there appears to be at
> first blush.
>
My gut feeling is that:
* kernel should default to a simple static sane irq policy without user
space. This is especially true for multi-queue devices where the default
puts all IRQ's on one cpu.
* irqbalance should do a one-shot rearrangement at boot up. It should rearrange
when new IRQ's are requested. The kernel should have capablity to notify
userspace (uevent?) when IRQ's are added or removed.
* Let scheduler make decisions about migrating processes (rather than let irqbalance
migrate IRQ's).
* irqbalance should not do the hacks it does to try and guess at network traffic.
--
^ permalink raw reply
* [PATCH net-next] bridge: fix accidental creation of sysfs directory
From: Stephen Hemminger @ 2011-04-16 16:09 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Commit bb900b27a2f49b37bc38c08e656ea13048fee13b introduced a bug in net-next
because of a typo in notifier. Every device would have the sysfs
bridge directory (and files).
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/bridge/br_notify.c 2011-04-16 08:56:06.130218417 -0700
+++ b/net/bridge/br_notify.c 2011-04-16 08:59:30.703509541 -0700
@@ -37,7 +37,7 @@ static int br_device_event(struct notifi
int err;
/* register of bridge completed, add sysfs entries */
- if ((dev->priv_flags && IFF_EBRIDGE) && event == NETDEV_REGISTER) {
+ if ((dev->priv_flags & IFF_EBRIDGE) && event == NETDEV_REGISTER) {
br_sysfs_addbr(dev);
return NOTIFY_DONE;
}
^ permalink raw reply
* Re: r8169 misleading firmware error messages
From: Ben Hutchings @ 2011-04-16 15:34 UTC (permalink / raw)
To: Fejes József; +Cc: François Romieu, netdev
In-Reply-To: <4DA9864E.2070405@joco.name>
[-- Attachment #1: Type: text/plain, Size: 1240 bytes --]
On Sat, 2011-04-16 at 14:06 +0200, Fejes József wrote:
[...]
> I took a deeper look. It seems to me that the firmware files are not the
> usual microcode type that the device can't function without, it just
> sets up some registers, which supposedly already contain some sensible
> values, so it's more like patching.
Some of the R8169 variants have a microcontroller in the PHY running
firmware that is initially loaded from non-volatile memory (maybe
eFuse?). These blobs contain bug fixes for the original PHY firmware.
> That explains why this device still
> works without the firmware. So my actual question is this: what do I
> gain if I use the firmware, what do I lose if I don't?
[...]
The original firmware apparently is unable to establish a stable link
against some link partners.
This warning:
> W: Possible missing firmware /lib/firmware/rtl_nic/rtl8168d-2.fw for
> module r8169
> W: Possible missing firmware /lib/firmware/rtl_nic/rtl8168d-1.fw for
> module r8169
is purely based on the MODULE_FIRMWARE annotations, which do not
distinguish which devices might require which files.
Ben.
--
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: r8169 misleading firmware error messages
From: Fejes József @ 2011-04-16 12:06 UTC (permalink / raw)
To: François Romieu; +Cc: netdev
In-Reply-To: <20110416110454.GC17833@electric-eye.fr.zoreil.com>
>> I see there's a condition, if an rtl_readphy returns a wrong value, it
>> doesn't even try to load the firmware and just prints the message.
>> Although this wouldn't explain why the error message disappeared
>> when the files were there.
>
> I don't get your point.
I meant this:
2233 if ((rtl_readphy(tp, 0x06) != 0xbf00) ||
2234 (rtl_apply_firmware(tp, FIRMWARE_8168D_1) < 0)) {
2235 netif_warn(tp, probe, tp->dev, "unable to apply
firmware patch\n");
2236 }
So if a user sees this warning, they don't know if the right firmware is
missing or something else is wrong with the device and it doesn't even
try to load the firmware.
>
>> Clearly, my device works without these firmware files. If my device
>> works better with them, or if there are other similar devices which
>> require it, I think there should be a configuration option to
>> disable this firmware stuff and its benefits altogether so that it
>> doesn't even report that it needs it.
>
> The driver uses netif_warn, not netif_err.
>
> I think the driver is nevrotic enough as is and I will not add
> what I consider a silly if not unusable configuration option.
>
> Feel free to send patches if you think they really add some
> value.
>
I took a deeper look. It seems to me that the firmware files are not the
usual microcode type that the device can't function without, it just
sets up some registers, which supposedly already contain some sensible
values, so it's more like patching. That explains why this device still
works without the firmware. So my actual question is this: what do I
gain if I use the firmware, what do I lose if I don't? Since the
previous kernel version, either something pretty important was moved out
of the code into the firmware file, in which case it's a bad idea not to
use them, or they add some features to an already perfectly working
device, in which case I thought it could be a good idea to make it a
configuration option. I'd just like to understand because I couldn't
find any documentation, I don't mean to question your decisions.
^ permalink raw reply
* Re: [PATCH 2/3] net: Add net device irq siloing feature
From: Neil Horman @ 2011-04-16 11:55 UTC (permalink / raw)
To: Eric Dumazet
Cc: Stephen Hemminger, netdev, davem, Dimitris Michailidis,
Thomas Gleixner, David Howells, Tom Herbert, Ben Hutchings
In-Reply-To: <1302934897.2792.6.camel@edumazet-laptop>
On Sat, Apr 16, 2011 at 08:21:37AM +0200, Eric Dumazet wrote:
> Le vendredi 15 avril 2011 à 21:52 -0700, Stephen Hemminger a écrit :
> > > On Fri, Apr 15, 2011 at 11:49:03PM +0100, Ben Hutchings wrote:
> > > > On Fri, 2011-04-15 at 16:17 -0400, Neil Horman wrote:
> > > > > Using the irq affinity infrastrucuture, we can now allow net
> > > > > devices to call
> > > > > request_irq using a new wrapper function (request_net_irq), which
> > > > > will attach a
> > > > > common affinty_update handler to each requested irq. This affinty
> > > > > update mechanism correlates each tracked irq to the flow(s) that
> > > > > said irq processes
> > > > > most frequently. The highest traffic flow is noted, marked and
> > > > > exported to user
> > > > > space via the affinity_hint proc file for each irq. In this way,
> > > > > utilities like
> > > > > irqbalance are able to determine which cpu is recieving the most
> > > > > data from each
> > > > > rx queue on a given NIC, and set irq affinity accordingly.
> > > > [...]
> > > >
> > > > Is irqbalance expected to poll the affinity hints? How often?
> > > >
> > > Yes, its done just that for quite some time. Intel added that ability
> > > at the
> > > same time they added the affinity_hint proc file. Irqbalance polls the
> > > affinity_hint file at the same time it rebalances all irqs (every 10
> > > seconds). If the affinity_hint is non-zero, irqbalance just copies it
> > > to smp_affinity for
> > > the same irq. Up until now thats been just about dead code because
> > > only ixgbe
> > > sets affinity_hint. Thats why I added the affinity_alg file, so
> > > irqbalance could do something more intellegent than just a blind copy.
> > > With the patch that
> > > I referenced I added code to irqbalance to allow it to preform
> > > different balancing methods based on the output of affinity_alg.
> > > Neil
> >
> > I hate the way more and more interfaces are becoming device driver
> > specific. It makes it impossible to build sane management infrastructure
> > and causes lots of customer and service complaints.
> >
>
> For me, the whole problem is the paradigm that we adapt IRQ to CPU were
> applications _were_ running in last seconds, while process scheduler
> might perform other choices, ie migrate task to cpu where IRQ was
> happening (the cpu calling wakeups)
>
> We can add logic to each layer, and yet not gain perfect behavior.
>
> Some kind of cooperation is neeed.
>
> Irqbalance for example is of no use in the case of a network flood
> happening on your machine, because we enter NAPI mode for several
> minutes on a single cpu. We'll need to add special logic in NAPI loop to
> force an exit to reschedule an IRQ (so that another cpu can take it)
>
Would you consider an approach whereby we, instead of updating irq affinity to
match the process that consumes data from a given irq, bias the scheduler such
that process which consume data from a given irq not be moved away from the same
core/l2 cache being fed by that flow? Do you have a suggestion for how best to
communicate that to the scheduler? It would seem that interrogating the RFS
table from the scheduler might not be well received.
Best
Neil
>
>
>
^ permalink raw reply
* Re: r8169 misleading firmware error messages
From: François Romieu @ 2011-04-16 11:04 UTC (permalink / raw)
To: Fejes József; +Cc: netdev
In-Reply-To: <4DA952D5.7030805@joco.name>
On Sat, Apr 16, 2011 at 10:27:01AM +0200, Fejes József wrote:
[...]
> So then I installed the rtl_nic firmware files, and the error
> messages were gone.
>
> How do I know if it actually tries to load the firmware at all?
Currently: remove it and the driver will tell you that it could not
load the firmware.
It's a bit rough. I'll report something through ethtool (-i).
> I see there's a condition, if an rtl_readphy returns a wrong value, it
> doesn't even try to load the firmware and just prints the message.
> Although this wouldn't explain why the error message disappeared
> when the files were there.
I don't get your point.
> Clearly, my device works without these firmware files. If my device
> works better with them, or if there are other similar devices which
> require it, I think there should be a configuration option to
> disable this firmware stuff and its benefits altogether so that it
> doesn't even report that it needs it.
The driver uses netif_warn, not netif_err.
I think the driver is nevrotic enough as is and I will not add
what I consider a silly if not unusable configuration option.
Feel free to send patches if you think they really add some
value.
--
Ueimor
^ permalink raw reply
* Re: [PATCH] r8169: Be verbose when unable to load fw patch
From: François Romieu @ 2011-04-16 11:03 UTC (permalink / raw)
To: David Miller; +Cc: bp, linux-kernel, borislav.petkov, netdev
In-Reply-To: <20110413.105737.39178380.davem@davemloft.net>
[...]
> Ok. So will you submit this yourself later or should I just
> apply this myself directly to net-2.6?
Please apply it directly.
Thanks.
--
Ueimor
^ permalink raw reply
* Re: The bonding driver should notify userspace of MAC address change
From: Nicolas de Pesloüan @ 2011-04-16 9:07 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: Michał Górny, netdev, roy, Andy Gospodarek
In-Reply-To: <16422.1302903932@death>
Le 15/04/2011 23:45, Jay Vosburgh a écrit :
> Nicolas de Pesloüan<nicolas.2p.debian@gmail.com> wrote:
>
>> Agreed.
>>
>>> Is there some race window there between the register and the
>>> netif_carrier_off?
>>
>> It might be that dhcpd does not wait for link to be up before starting to send DHCP requests.
>
> It looks like it's not related to carrier state at all:
>
> #212: dhcpcd requires restart to get an IP address for bonded interface
> -----------------------+-----------------
> Reporter: mgorny@… | Owner: roy
> Type: defect | Status: new
> Priority: major | Milestone:
> Component: dhcpcd | Version: 5.1
> Resolution: | Keywords:
> -----------------------+-----------------
>
> Comment (by roy):
>
> Sorry, the above isn't too clear.
>
> dhcpcd will read the hardware address when the interface is marked IFF_UP
> or when given RTM_NEWLINK with ifi->ifi_change = ~0U, the latter being
> sent by some drivers to tell userland that an interface characteristic has
> changed - like say a hardware address - if the driver supports such a
> change whilst still up. Normal behaviour is to mark device as DOWN before
> changing hardware address. bonding does this whilst marked UP, hence this
> issue.
>
> carrier going up / down is just that, it's not a signal to re-read the
> interface characteristics.
>
>
> Now this confuses me again; I thought that running the dhcp
> client (dhcpcd) over bonding has worked for years, although I've not
> personally tried it recently. Perhaps it varies by distro. In any
> event, this behavior of bonding (setting the bond's MAC without a
> down/up flip) has never been different in my memory.
I use dhcpcd over bonding everyday on my laptop (debian/testing), for several years.
bond0 uses eth0 and wlan0 as slaves. The time to get a DHCP reply vary, mostly because the time to
register to the wifi AP is not perfectly stable. Sometimes (not very often), I need to ifdown/ifup
to get a DHCP reply. This might be due to dhcpcd reading the MAC too early and getting an all zero
MAC. I need to try and double check this, but as it happen early in the boot process and not very
often, it will be hard to diagnose.
That being said, sending a DHCP request with an all zero source MAC sounds very strange to me.
The RFC for DHCP (RFC2131) states that a DHCP server must be prepared to broadcast the reply if the
client hardware address is null.
If 'giaddr' is zero and 'ciaddr' is zero, and the broadcast bit is
set, then the server broadcasts DHCPOFFER and DHCPACK messages to
0xffffffff. If the broadcast bit is not set and 'giaddr' is zero and
'ciaddr' is zero, then the server unicasts DHCPOFFER and DHCPACK
messages to the client's hardware address and 'yiaddr' address. In
all cases, when 'giaddr' is zero, the server broadcasts any DHCPNAK
messages to 0xffffffff.
But a DHCP server must store a client identifier associated with a given dynamic IP address for the
duration of the lease, to be able to give the same IP address to the same client if this client
restarts while the lease is still valid.
If the client hardware address is all zero and the client configuration does not provide some other
user defined identifier, then the client identifier is not unique and the DHCP server should - I
think - ignore the request.
So, except if the local dhcpcd configuration contains an user defined identifier, I think dhcpcd
should wait for the MAC address to be not null before sending any requests.
Nicolas.
> I've not yet dug down to see if NETDEV_CHANGEADDR will result in
> an RTM_NEWLINK to user space. At first glance it doesn't look like it.
>
> When bonding goes link up, however, I think linkwatch_do_dev
> will issue an RTM_NEWLINK (via a call to netdev_state_change), or,
> alternately, dev_change_flags will do it at IFF_UP time.
>
> -J
>
> ---
> -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
>
^ permalink raw reply
* r8169 misleading firmware error messages
From: Fejes József @ 2011-04-16 8:27 UTC (permalink / raw)
To: netdev
Hi,
I'm using Linux 2.6.38.3 with a Realtek 8169 card, ID: 10ec:8168
(integrated into an Intel D945GSEJT mobo). It never needed any firmware
files, and when I was using 2.6.37, it never complained about it either.
Now it does complain (but it still works perfectly):
r8169 0000:01:00.0: eth0: unable to apply firmware patch
Also when I do initramfs stuff:
W: Possible missing firmware /lib/firmware/rtl_nic/rtl8168d-2.fw for
module r8169
W: Possible missing firmware /lib/firmware/rtl_nic/rtl8168d-1.fw for
module r8169
So then I installed the rtl_nic firmware files, and the error messages
were gone.
How do I know if it actually tries to load the firmware at all? I see
there's a condition, if an rtl_readphy returns a wrong value, it doesn't
even try to load the firmware and just prints the message. Although this
wouldn't explain why the error message disappeared when the files were
there.
Clearly, my device works without these firmware files. If my device
works better with them, or if there are other similar devices which
require it, I think there should be a configuration option to disable
this firmware stuff and its benefits altogether so that it doesn't even
report that it needs it.
Best regards,
--
Fejes József
http://joco.name
^ permalink raw reply
* Re: [PATCH 2/3] net: Add net device irq siloing feature
From: Eric Dumazet @ 2011-04-16 6:21 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Neil Horman, netdev, davem, Dimitris Michailidis, Thomas Gleixner,
David Howells, Tom Herbert, Ben Hutchings
In-Reply-To: <367764507.40661.1302929544356.JavaMail.root@tahiti.vyatta.com>
Le vendredi 15 avril 2011 à 21:52 -0700, Stephen Hemminger a écrit :
> > On Fri, Apr 15, 2011 at 11:49:03PM +0100, Ben Hutchings wrote:
> > > On Fri, 2011-04-15 at 16:17 -0400, Neil Horman wrote:
> > > > Using the irq affinity infrastrucuture, we can now allow net
> > > > devices to call
> > > > request_irq using a new wrapper function (request_net_irq), which
> > > > will attach a
> > > > common affinty_update handler to each requested irq. This affinty
> > > > update mechanism correlates each tracked irq to the flow(s) that
> > > > said irq processes
> > > > most frequently. The highest traffic flow is noted, marked and
> > > > exported to user
> > > > space via the affinity_hint proc file for each irq. In this way,
> > > > utilities like
> > > > irqbalance are able to determine which cpu is recieving the most
> > > > data from each
> > > > rx queue on a given NIC, and set irq affinity accordingly.
> > > [...]
> > >
> > > Is irqbalance expected to poll the affinity hints? How often?
> > >
> > Yes, its done just that for quite some time. Intel added that ability
> > at the
> > same time they added the affinity_hint proc file. Irqbalance polls the
> > affinity_hint file at the same time it rebalances all irqs (every 10
> > seconds). If the affinity_hint is non-zero, irqbalance just copies it
> > to smp_affinity for
> > the same irq. Up until now thats been just about dead code because
> > only ixgbe
> > sets affinity_hint. Thats why I added the affinity_alg file, so
> > irqbalance could do something more intellegent than just a blind copy.
> > With the patch that
> > I referenced I added code to irqbalance to allow it to preform
> > different balancing methods based on the output of affinity_alg.
> > Neil
>
> I hate the way more and more interfaces are becoming device driver
> specific. It makes it impossible to build sane management infrastructure
> and causes lots of customer and service complaints.
>
For me, the whole problem is the paradigm that we adapt IRQ to CPU were
applications _were_ running in last seconds, while process scheduler
might perform other choices, ie migrate task to cpu where IRQ was
happening (the cpu calling wakeups)
We can add logic to each layer, and yet not gain perfect behavior.
Some kind of cooperation is neeed.
Irqbalance for example is of no use in the case of a network flood
happening on your machine, because we enter NAPI mode for several
minutes on a single cpu. We'll need to add special logic in NAPI loop to
force an exit to reschedule an IRQ (so that another cpu can take it)
^ permalink raw reply
* Re: [PATCH 2/3] net: Add net device irq siloing feature
From: Stephen Hemminger @ 2011-04-16 4:52 UTC (permalink / raw)
To: Neil Horman
Cc: netdev, davem, Dimitris Michailidis, Thomas Gleixner,
David Howells, Eric Dumazet, Tom Herbert, Ben Hutchings
In-Reply-To: <20110416014547.GA2200@neilslaptop.think-freely.org>
> On Fri, Apr 15, 2011 at 11:49:03PM +0100, Ben Hutchings wrote:
> > On Fri, 2011-04-15 at 16:17 -0400, Neil Horman wrote:
> > > Using the irq affinity infrastrucuture, we can now allow net
> > > devices to call
> > > request_irq using a new wrapper function (request_net_irq), which
> > > will attach a
> > > common affinty_update handler to each requested irq. This affinty
> > > update mechanism correlates each tracked irq to the flow(s) that
> > > said irq processes
> > > most frequently. The highest traffic flow is noted, marked and
> > > exported to user
> > > space via the affinity_hint proc file for each irq. In this way,
> > > utilities like
> > > irqbalance are able to determine which cpu is recieving the most
> > > data from each
> > > rx queue on a given NIC, and set irq affinity accordingly.
> > [...]
> >
> > Is irqbalance expected to poll the affinity hints? How often?
> >
> Yes, its done just that for quite some time. Intel added that ability
> at the
> same time they added the affinity_hint proc file. Irqbalance polls the
> affinity_hint file at the same time it rebalances all irqs (every 10
> seconds). If the affinity_hint is non-zero, irqbalance just copies it
> to smp_affinity for
> the same irq. Up until now thats been just about dead code because
> only ixgbe
> sets affinity_hint. Thats why I added the affinity_alg file, so
> irqbalance could do something more intellegent than just a blind copy.
> With the patch that
> I referenced I added code to irqbalance to allow it to preform
> different balancing methods based on the output of affinity_alg.
> Neil
I hate the way more and more interfaces are becoming device driver
specific. It makes it impossible to build sane management infrastructure
and causes lots of customer and service complaints.
^ permalink raw reply
* Re: [PATCH net-next-2.6 3/3] bonding,ipv4,ipv6,vlan: Handle NETDEV_BONDING_FAILOVER like NETDEV_NOTIFY_PEERS
From: Ben Hutchings @ 2011-04-16 2:53 UTC (permalink / raw)
To: Brian Haley
Cc: Jay Vosburgh, David Miller, Andy Gospodarek, Patrick McHardy,
netdev
In-Reply-To: <4DA8F627.5090109@hp.com>
On Fri, 2011-04-15 at 21:51 -0400, Brian Haley wrote:
> On 04/15/2011 08:30 PM, Jay Vosburgh wrote:
> > Ben Hutchings <bhutchings@solarflare.com> wrote:
> >
> >> It is undesirable for the bonding driver to be poking into higher
> >> level protocols, and notifiers provide a way to avoid that. This does
> >> mean removing the ability to configure reptitition of gratuitous ARPs
> >> and unsolicited NAs.
> >
> > In principle I think this is a good thing (getting rid of some
> > of those dependencies, duplicated code, etc).
> >
> > However, the removal of the multiple grat ARP and NAs may be an
> > issue for some users. I don't know that we can just remove this (along
> > with its API) without going through the feature removal process.
>
> Right, I don't know how many people are using these, they might not be
> happy, especially since specifying an unknown parameter will cause a
> module load to fail:
>
> --> modprobe bonding foobar=27
> FATAL: Error inserting bonding (/lib/modules/2.6.32-31-generic/kernel/drivers/net/bonding/bonding.ko): Unknown symbol in module, or unknown parameter (see dmesg)
>
> When these params are stuffed in /etc/modprobe.d/options, a reboot to
> a kernel without them will cause some swearing :)
Yes, this is a problem.
If the feature of repeated advertismenets is implemented in ipv4/ipv6
then we could propagate the parameters there, logging a warning, up to
some deprecation deadline.
> BTW, if this is accepted you need to update the documentation as well.
>
> > As I recall, the multiple gratuitous ARP stuff was added for
> > Infiniband, because it is dependent on the grat ARP for a smooth
> > failover.
> >
> > There is also currently logic to check the linkwatch link state
> > to wait for the link to go up prior to sending a grat ARP; this is also
> > for IB.
> >
> > Brian Haley added the unsolicited NAs; I've added him to the cc
> > so perhaps he (or somebody else) can comment on the necessity of keeping
> > the ability to send multiple NAs.
>
> I added it because in an IPv6-only environment I was seeing really long
> failover times on bonds. I believe this was a customer-reported issue, so
> there *might* be someone setting it, but I think my testing always showed
> one was enough to wake-up the switch.
>
> Is it useful to call netdev_bonding_change() multiple times from within
> bond_change_active_slave(), like MAX(arp, na) times?
We can't wait around to do that. And it would be abusing the notifier
based on assumptions about what other components do, which I'm trying to
get away from.
> One comment below...
[...]
> Does your change also cover this case with multiple VLAN IDs? Is that covered in
> the vlan.c code below?
[...]
Should do. I didn't specifically test multiple VLAN IDs but I'm looping
over them.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH 1/3] irq: Add registered affinity guidance infrastructure
From: Neil Horman @ 2011-04-16 2:11 UTC (permalink / raw)
To: Thomas Gleixner
Cc: netdev, davem, nhorman, Dimitris Michailidis, David Howells,
Eric Dumazet, Tom Herbert
In-Reply-To: <alpine.LFD.2.00.1104160144370.2744@localhost6.localdomain6>
On Sat, Apr 16, 2011 at 02:22:58AM +0200, Thomas Gleixner wrote:
> On Fri, 15 Apr 2011, Neil Horman wrote:
>
> > From: nhorman <nhorman@devel2.think-freely.org>
> >
> > This patch adds the needed data to the irq_desc struct, as well as the needed
> > API calls to allow the requester of an irq to register a handler function to
> > determine the affinity_hint of that irq when queried from user space.
>
> This changelog simply sucks. It does not explain the rationale for
> this churn at all.
>
It seems pretty clear to me. It allows a common function to update the value of
affinity_hint when its queried.
> Which problem is it solving?
> Why are the current interfaces not sufficient?
Did you read the initial post that I sent with it? Apparently not. Apologies,
its seems my git-send-email didn't cc everyone I expected it to:
http://marc.info/?l=linux-netdev&m=130291921026187&w=2
I'll skip the rest of your your email, and just try to turn some of your rant
into something more acceptible to you.
Neil
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox