Netdev List
 help / color / mirror / Atom feed
* Re: [RFC] cirrus ep93xx ethernet driver
From: Lennert Buytenhek @ 2006-06-28  2:05 UTC (permalink / raw)
  To: netdev
In-Reply-To: <20060626025924.GF6826@xi.wantstofly.org>

On Mon, Jun 26, 2006 at 04:59:24AM +0200, Lennert Buytenhek wrote:

> The cirrus ep93xx is an ARM SoC that includes an ethernet MAC --
> this patch adds a driver for that ethernet MAC.

Attached is a new version that optimises interrupt handling somewhat.
Since we clear RX status as the first thing we do in the poll handler,
we might as well read the read-to-clear version of the interrupt status
register in the interrupt handler and avoid the explicit clear in the
poll handler.  This shaves close to a second off a 128M sendfile() test.

At ~40 seconds for a 128M sendfile (~3.2MB/sec), the network performance
of this CPU isn't impressive by any means, but given that the CPU only
runs at 200MHz and that the MAC doesn't do checksum offloading and insists
on 4-byte buffer alignment, we can't really do a whole lot better.  The
performance with this driver is still a good deal better than with the
vendor driver, though -- for this particular test (128M sendfile), the
vendor driver needs 1m21s.

Apart from that it still uses numeric chip register addresses, I'm
quite happy with the driver as it is, it survives heavy beating and
is pretty stable.

Index: linux-2.6.17-git10/drivers/net/arm/Kconfig
===================================================================
--- linux-2.6.17-git10.orig/drivers/net/arm/Kconfig
+++ linux-2.6.17-git10/drivers/net/arm/Kconfig
@@ -39,3 +39,10 @@ config ARM_AT91_ETHER
 	help
 	  If you wish to compile a kernel for the AT91RM9200 and enable
 	  ethernet support, then you should always answer Y to this.
+
+config EP93XX_ETH
+	tristate "EP93xx Ethernet support"
+	depends on NET_ETHERNET && ARM && ARCH_EP93XX
+	help
+	  This is a driver for the ethernet hardware included in EP93xx CPUs.
+	  Say Y if you are building a kernel for EP93xx based devices.
Index: linux-2.6.17-git10/drivers/net/arm/Makefile
===================================================================
--- linux-2.6.17-git10.orig/drivers/net/arm/Makefile
+++ linux-2.6.17-git10/drivers/net/arm/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_ARM_ETHERH)	+= etherh.o
 obj-$(CONFIG_ARM_ETHER3)	+= ether3.o
 obj-$(CONFIG_ARM_ETHER1)	+= ether1.o
 obj-$(CONFIG_ARM_AT91_ETHER)	+= at91_ether.o
+obj-$(CONFIG_EP93XX_ETH)	+= ep93xx_eth.o
Index: linux-2.6.17-git10/drivers/net/arm/ep93xx_eth.c
===================================================================
--- /dev/null
+++ linux-2.6.17-git10/drivers/net/arm/ep93xx_eth.c
@@ -0,0 +1,668 @@
+/*
+ * EP93xx ethernet network device driver
+ * Copyright (C) 2006 Lennert Buytenhek <buytenh@wantstofly.org>
+ * Dedicated to Marija Kulikova.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/config.h>
+#include <linux/dma-mapping.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/netdevice.h>
+#include <linux/etherdevice.h>
+#include <linux/init.h>
+#include <linux/moduleparam.h>
+#include <linux/platform_device.h>
+#include <asm/arch/ep93xx-regs.h>
+#include <asm/arch/platform.h>
+#include <asm/io.h>
+#include "ep93xx_eth.h"
+
+#define DRV_MODULE_VERSION	"0.1"
+
+#define RX_QUEUE_ENTRIES	64
+#define TX_QUEUE_ENTRIES	8
+
+struct ep93xx_descs
+{
+	struct ep93xx_rdesc	rdesc[RX_QUEUE_ENTRIES];
+	struct ep93xx_tdesc	tdesc[TX_QUEUE_ENTRIES];
+	struct ep93xx_rstat	rstat[RX_QUEUE_ENTRIES];
+	struct ep93xx_tstat	tstat[TX_QUEUE_ENTRIES];
+};
+
+struct ep93xx_priv
+{
+	struct resource		*res;
+	void			*base_addr;
+	int			irq;
+
+	struct ep93xx_descs	*descs;
+	dma_addr_t		descs_dma_addr;
+
+	void			*rx_buf[RX_QUEUE_ENTRIES];
+	void			*tx_buf[TX_QUEUE_ENTRIES];
+
+	int			rx_pointer;
+	int			tx_clean_pointer;
+	int			tx_pointer;
+	int			tx_pending;
+
+	struct net_device_stats	stats;
+};
+
+#define rdb(ep, off)		__raw_readb((ep)->base_addr + (off))
+#define rdw(ep, off)		__raw_readw((ep)->base_addr + (off))
+#define rdl(ep, off)		__raw_readl((ep)->base_addr + (off))
+#define wrb(ep, off, val)	__raw_writeb((val), (ep)->base_addr + (off))
+#define wrw(ep, off, val)	__raw_writew((val), (ep)->base_addr + (off))
+#define wrl(ep, off, val)	__raw_writel((val), (ep)->base_addr + (off))
+
+static int ep93xx_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	struct ep93xx_priv *ep = netdev_priv(dev);
+	int entry;
+
+	if (unlikely(skb->len) > PAGE_SIZE) {
+		ep->stats.tx_dropped++;
+		dev_kfree_skb(skb);
+		return 0;
+	}
+
+	entry = ep->tx_pointer;
+	ep->tx_pointer = (ep->tx_pointer + 1) % TX_QUEUE_ENTRIES;
+
+	ep->descs->tdesc[entry].tdesc1 =
+		TDESC1_EOF | (entry << 16) | (skb->len & 0xfff);
+	skb_copy_and_csum_dev(skb, ep->tx_buf[entry]);
+	dma_sync_single(NULL, ep->descs->tdesc[entry].buf_addr,
+				skb->len, DMA_TO_DEVICE);
+	dev_kfree_skb(skb);
+
+	dev->trans_start = jiffies;
+
+	local_irq_disable();
+	ep->tx_pending++;
+	if (ep->tx_pending == TX_QUEUE_ENTRIES)
+		netif_stop_queue(dev);
+	local_irq_enable();
+
+	wrl(ep, 0x00bc, 1);
+
+	return 0;
+}
+
+static int ep93xx_rx(struct net_device *dev, int *budget)
+{
+	struct ep93xx_priv *ep = netdev_priv(dev);
+	int tail_offset;
+	int rx_done;
+	int processed;
+
+	tail_offset = rdl(ep, 0x00a8) - ep->descs_dma_addr;
+
+	rx_done = 0;
+	processed = 0;
+	while (*budget > 0) {
+		int entry;
+		struct ep93xx_rstat *rstat;
+		u32 rstat0;
+		u32 rstat1;
+		int length;
+		struct sk_buff *skb;
+
+		entry = ep->rx_pointer;
+		rstat = ep->descs->rstat + entry;
+		if ((void *)rstat - (void *)ep->descs == tail_offset) {
+			rx_done = 1;
+			break;
+		}
+
+		rstat0 = rstat->rstat0;
+		rstat1 = rstat->rstat1;
+		rstat->rstat0 = 0;
+		rstat->rstat1 = 0;
+
+		if (!(rstat0 & RSTAT0_RFP)) {
+			printk(KERN_CRIT "ep93xx_rx: buffer not done "
+					 " %.8x %.8x\n", rstat0, rstat1);
+			BUG();
+		}
+		if (!(rstat0 & RSTAT0_EOF)) {
+			printk(KERN_CRIT "ep93xx_rx: not end-of-frame "
+					 " %.8x %.8x\n", rstat0, rstat1);
+			BUG();
+		}
+		if (!(rstat0 & RSTAT0_EOB)) {
+			printk(KERN_CRIT "ep93xx_rx: not end-of-buffer "
+					 " %.8x %.8x\n", rstat0, rstat1);
+			BUG();
+		}
+		if (!(rstat1 & RSTAT1_RFP)) {
+			printk(KERN_CRIT "ep93xx_rx: buffer1 not done "
+					 " %.8x %.8x\n", rstat0, rstat1);
+			BUG();
+		}
+		if ((rstat1 & RSTAT1_BUFFER_INDEX) >> 16 != entry) {
+			printk(KERN_INFO "ep93xx_rx: entry mismatch "
+					 " %.8x %.8x\n", rstat0, rstat1);
+			BUG();
+		}
+
+		if (!(rstat0 & RSTAT0_RWE)) {
+			printk(KERN_NOTICE "ep93xx_rx: receive error "
+					 " %.8x %.8x\n", rstat0, rstat1);
+
+			ep->stats.rx_errors++;
+			if (rstat0 & RSTAT0_OE)
+				ep->stats.rx_fifo_errors++;
+			if (rstat0 & RSTAT0_FE)
+				ep->stats.rx_frame_errors++;
+			if (rstat0 & (RSTAT0_RUNT | RSTAT0_EDATA))
+				ep->stats.rx_length_errors++;
+			if (rstat0 & RSTAT0_CRCE)
+				ep->stats.rx_crc_errors++;
+			goto err;
+		}
+
+		length = rstat1 & RSTAT1_FRAME_LENGTH;
+		if (length < 4 || length > PAGE_SIZE) {
+			printk(KERN_NOTICE "ep93xx_rx: invalid length "
+					 " %.8x %.8x\n", rstat0, rstat1);
+			goto err;
+		}
+
+		/* Strip FCS.  */
+		if (rstat0 & RSTAT0_CRCI)
+			length -= 4;
+
+		skb = dev_alloc_skb(length + 2);
+		if (likely(skb != NULL)) {
+			skb->dev = dev;
+			skb_reserve(skb, 2);
+			dma_sync_single(NULL, ep->descs->rdesc[entry].buf_addr,
+						length, DMA_FROM_DEVICE);
+			eth_copy_and_sum(skb, ep->rx_buf[entry], length, 0);
+			skb_put(skb, length);
+			skb->protocol = eth_type_trans(skb, dev);
+
+			dev->last_rx = jiffies;
+
+			netif_receive_skb(skb);
+
+			ep->stats.rx_packets++;
+			ep->stats.rx_bytes += length;
+		} else {
+			ep->stats.rx_dropped++;
+		}
+
+err:
+		ep->rx_pointer = (entry + 1) % RX_QUEUE_ENTRIES;
+		processed++;
+		dev->quota--;
+		(*budget)--;
+	}
+
+	if (processed) {
+		wrw(ep, 0x009c, processed);
+		wrw(ep, 0x00ac, processed);
+	}
+
+	return !rx_done;
+}
+
+static int ep93xx_have_more_rx(struct ep93xx_priv *ep)
+{
+	struct ep93xx_rstat *rstat;
+	int tail_offset;
+
+	rstat = ep->descs->rstat + ep->rx_pointer;
+	tail_offset = rdl(ep, 0x00a8) - ep->descs_dma_addr;
+
+	return !((void *)rstat - (void *)ep->descs == tail_offset);
+}
+
+static int ep93xx_poll(struct net_device *dev, int *budget)
+{
+	struct ep93xx_priv *ep = netdev_priv(dev);
+
+	/* @@@ Have to stop polling when device is downed while we
+	 * are polling.  */
+
+poll_some_more:
+	if (ep93xx_rx(dev, budget))
+		return 1;
+
+	netif_rx_complete(dev);
+
+	local_irq_disable();
+	wrl(ep, 0x0024, 0x0000000f);
+	if (ep93xx_have_more_rx(ep)) {
+		wrl(ep, 0x0024, 0x00000008);
+		wrl(ep, 0x0028, 0x00000004);
+		local_irq_enable();
+
+		if (netif_rx_reschedule(dev, 0))
+			goto poll_some_more;
+
+		return 0;
+	}
+	local_irq_enable();
+
+	return 0;
+}
+
+static void ep93xx_tx_complete(struct net_device *dev)
+{
+	struct ep93xx_priv *ep = netdev_priv(dev);
+	int tail_offset;
+	int wake;
+
+	tail_offset = rdl(ep, 0x00c8) - ep->descs_dma_addr;
+
+	wake = 0;
+	while (1) {
+		int entry;
+		struct ep93xx_tstat *tstat;
+		u32 tstat0;
+
+		entry = ep->tx_clean_pointer;
+		tstat = ep->descs->tstat + entry;
+		if ((void *)tstat - (void *)ep->descs == tail_offset)
+			break;
+
+		tstat0 = tstat->tstat0;
+		tstat->tstat0 = 0;
+
+		if (!(tstat0 & TSTAT0_TXFP)) {
+			printk(KERN_CRIT "ep93xx_tx_complete: buffer not done "
+					 " %.8x\n", tstat0);
+			BUG();
+		}
+		if (tstat0 & TSTAT0_FA) {
+			printk(KERN_CRIT "ep93xx_tx_complete: frame aborted "
+					 " %.8x\n", tstat0);
+			BUG();
+		}
+		if ((tstat0 & TSTAT0_BUFFER_INDEX) != entry) {
+			printk(KERN_INFO "ep93xx_tx_complete: entry mismatch "
+					 " %.8x\n", tstat0);
+			BUG();
+		}
+
+		if (tstat0 & TSTAT0_TXWE) {
+			int length = ep->descs->tdesc[entry].tdesc1 & 0xfff;
+
+			ep->stats.tx_packets++;
+			ep->stats.tx_bytes += length;
+		} else {
+			ep->stats.tx_errors++;
+		}
+		if (tstat0 & TSTAT0_LCRS)
+			ep->stats.tx_carrier_errors++;
+		if (tstat0 & TSTAT0_OW)
+			ep->stats.tx_window_errors++;
+		if (tstat0 & TSTAT0_TXU)
+			ep->stats.tx_fifo_errors++;
+		ep->stats.collisions += (tstat0 >> 16) & 0x1f;
+
+		ep->tx_clean_pointer = (entry + 1) % TX_QUEUE_ENTRIES;
+		if (ep->tx_pending == TX_QUEUE_ENTRIES)
+			wake = 1;
+		ep->tx_pending--;
+	}
+
+	if (wake)
+		netif_wake_queue(dev);
+}
+
+static irqreturn_t ep93xx_irq(int irq, void *dev_id, struct pt_regs *regs)
+{
+	struct net_device *dev = dev_id;
+	struct ep93xx_priv *ep = netdev_priv(dev);
+	u32 status;
+
+	status = rdl(ep, 0x002c);
+	if (status == 0)
+		return IRQ_NONE;
+
+	if (status & 0x00000004) {
+		if (likely(__netif_rx_schedule_prep(dev))) {
+			wrl(ep, 0x0024, 0x00000008);
+			__netif_rx_schedule(dev);
+		}
+	}
+
+	if (status & 0x00000008)
+		ep93xx_tx_complete(dev);
+
+	return IRQ_HANDLED;
+}
+
+static void ep93xx_free_buffers(struct ep93xx_priv *ep)
+{
+	int i;
+
+	for (i = 0; i < RX_QUEUE_ENTRIES; i++) {
+		dma_addr_t d;
+
+		d = ep->descs->rdesc[i].buf_addr;
+		if (d)
+			dma_unmap_single(NULL, d, PAGE_SIZE, DMA_FROM_DEVICE);
+
+		if (ep->rx_buf[i] != NULL)
+			free_page((unsigned long)ep->rx_buf[i]);
+	}
+
+	for (i = 0; i < TX_QUEUE_ENTRIES; i++) {
+		dma_addr_t d;
+
+		d = ep->descs->tdesc[i].buf_addr;
+		if (d)
+			dma_unmap_single(NULL, d, PAGE_SIZE, DMA_TO_DEVICE);
+
+		if (ep->tx_buf[i] != NULL)
+			free_page((unsigned long)ep->tx_buf[i]);
+	}
+
+	dma_free_coherent(NULL, sizeof(struct ep93xx_descs), ep->descs,
+							ep->descs_dma_addr);
+}
+
+static int ep93xx_alloc_buffers(struct ep93xx_priv *ep)
+{
+	int i;
+
+	ep->descs = dma_alloc_coherent(NULL, sizeof(struct ep93xx_descs),
+				&ep->descs_dma_addr, GFP_KERNEL | GFP_DMA);
+	if (ep->descs == NULL)
+		return 1;
+
+	for (i = 0; i < RX_QUEUE_ENTRIES; i++) {
+		void *page;
+		dma_addr_t d;
+
+		page = (void *)get_zeroed_page(GFP_KERNEL | GFP_DMA);
+		if (page == NULL)
+			goto err;
+
+		d = dma_map_single(NULL, page, PAGE_SIZE, DMA_FROM_DEVICE);
+		if (dma_mapping_error(d)) {
+			free_page((unsigned long)page);
+			goto err;
+		}
+
+		ep->rx_buf[i] = page;
+		ep->descs->rdesc[i].buf_addr = d;
+		ep->descs->rdesc[i].rdesc1 = (i << 16) | PAGE_SIZE;
+	}
+
+	for (i = 0; i < TX_QUEUE_ENTRIES; i++) {
+		void *page;
+		dma_addr_t d;
+
+		page = (void *)get_zeroed_page(GFP_KERNEL | GFP_DMA);
+		if (page == NULL)
+			goto err;
+
+		d = dma_map_single(NULL, page, PAGE_SIZE, DMA_TO_DEVICE);
+		if (dma_mapping_error(d)) {
+			free_page((unsigned long)page);
+			goto err;
+		}
+
+		ep->tx_buf[i] = page;
+		ep->descs->tdesc[i].buf_addr = d;
+	}
+
+	return 0;
+
+err:
+	ep93xx_free_buffers(ep);
+	return 1;
+}
+
+static void ep93xx_start_hw(struct net_device *dev)
+{
+	struct ep93xx_priv *ep = netdev_priv(dev);
+	unsigned long addr;
+
+	wrl(ep, 0x0020, 0x00000001);
+	while (rdl(ep, 0x0020) & 0x00000001)
+		;
+
+	/* Receive descriptor ring.  */
+	addr = ep->descs_dma_addr + offsetof(struct ep93xx_descs, rdesc);
+	wrl(ep, 0x0090, addr);
+	wrl(ep, 0x0098, addr);
+	wrw(ep, 0x0094, RX_QUEUE_ENTRIES * sizeof(struct ep93xx_rdesc));
+
+	/* Receive status ring.  */
+	addr = ep->descs_dma_addr + offsetof(struct ep93xx_descs, rstat);
+	wrl(ep, 0x00a0, addr);
+	wrl(ep, 0x00a8, addr);
+	wrw(ep, 0x00a4, RX_QUEUE_ENTRIES * sizeof(struct ep93xx_rstat));
+
+	/* Transmit descriptor ring.  */
+	addr = ep->descs_dma_addr + offsetof(struct ep93xx_descs, tdesc);
+	wrl(ep, 0x00b0, addr);
+	wrl(ep, 0x00b8, addr);
+	wrw(ep, 0x00b4, TX_QUEUE_ENTRIES * sizeof(struct ep93xx_tdesc));
+
+	/* Transmit status ring.  */
+	addr = ep->descs_dma_addr + offsetof(struct ep93xx_descs, tstat);
+	wrl(ep, 0x00c0, addr);
+	wrl(ep, 0x00c8, addr);
+	wrw(ep, 0x00c4, TX_QUEUE_ENTRIES * sizeof(struct ep93xx_tstat));
+
+	wrl(ep, 0x0080, 0x00000101);			// bmctl
+	wrl(ep, 0x0024, 0x0000000f);			// inten
+	wrl(ep, 0x0064, 0x00000000);			// gintmask
+	while (!(rdl(ep, 0x0084) & 0x00000008))
+		cpu_relax();
+	wrl(ep, 0x009c, RX_QUEUE_ENTRIES);
+	wrl(ep, 0x00ac, RX_QUEUE_ENTRIES);
+
+	wrb(ep, 0x0050, dev->dev_addr[0]);
+	wrb(ep, 0x0051, dev->dev_addr[1]);
+	wrb(ep, 0x0052, dev->dev_addr[2]);
+	wrb(ep, 0x0053, dev->dev_addr[3]);
+	wrb(ep, 0x0054, dev->dev_addr[4]);
+	wrb(ep, 0x0055, dev->dev_addr[5]);
+	wrl(ep, 0x004c, 0x00000000);
+
+	wrl(ep, 0x0000, 0x00073800);
+	wrl(ep, 0x0004, 0x00000001);
+}
+
+static void ep93xx_stop_hw(struct net_device *dev)
+{
+	struct ep93xx_priv *ep = netdev_priv(dev);
+
+	wrl(ep, 0x0020, 0x00000001);
+	while (rdl(ep, 0x0020) & 0x00000001)
+		;
+}
+
+static int ep93xx_open(struct net_device *dev)
+{
+	struct ep93xx_priv *ep = netdev_priv(dev);
+	int err;
+
+	if (ep93xx_alloc_buffers(ep))
+		return -ENOMEM;
+
+	ep93xx_start_hw(dev);
+	ep->rx_pointer = 0;
+	ep->tx_clean_pointer = 0;
+	ep->tx_pointer = 0;
+	ep->tx_pending = 0;
+
+	err = request_irq(ep->irq, ep93xx_irq, SA_SHIRQ, "ep93xx_eth", dev);
+	if (err) {
+		ep93xx_stop_hw(dev);
+		ep93xx_free_buffers(ep);
+		return err;
+	}
+
+	wrl(ep, 0x0064, 0x00008000);			// gintmask
+
+	netif_start_queue(dev);
+
+	return 0;
+}
+
+static int ep93xx_close(struct net_device *dev)
+{
+	struct ep93xx_priv *ep = netdev_priv(dev);
+
+	netif_stop_queue(dev);
+
+	wrl(ep, 0x0064, 0x00000000);			// gintmask
+	free_irq(ep->irq, dev);
+	ep93xx_stop_hw(dev);
+	ep93xx_free_buffers(ep);
+
+	return 0;
+}
+
+static struct net_device_stats *ep93xx_get_stats(struct net_device *dev)
+{
+	struct ep93xx_priv *ep = netdev_priv(dev);
+	return &(ep->stats);
+}
+
+struct net_device *ep93xx_dev_alloc(struct ep93xx_eth_data *data)
+{
+	struct net_device *dev;
+	struct ep93xx_priv *ep;
+
+	dev = alloc_etherdev(sizeof(struct ep93xx_priv));
+	if (dev == NULL)
+		return NULL;
+	ep = netdev_priv(dev);
+
+	memcpy(dev->dev_addr, data->dev_addr, ETH_ALEN);
+
+	dev->hard_start_xmit = ep93xx_xmit;
+	dev->poll = ep93xx_poll;
+	dev->open = ep93xx_open;
+	dev->stop = ep93xx_close;
+	dev->get_stats = ep93xx_get_stats;
+
+	dev->features |= NETIF_F_SG | NETIF_F_HW_CSUM;
+	dev->weight = 64;
+
+	return dev;
+}
+
+
+static int ep93xx_eth_remove(struct platform_device *pdev)
+{
+	struct net_device *dev;
+	struct ep93xx_priv *ep;
+
+	dev = platform_get_drvdata(pdev);
+	if (dev == NULL)
+		return 0;
+	platform_set_drvdata(pdev, NULL);
+
+	ep = netdev_priv(dev);
+
+	// @@@ force down
+	unregister_netdev(dev);
+	ep93xx_free_buffers(ep);
+
+	if (ep->base_addr != NULL)
+		iounmap(ep->base_addr);
+
+	if (ep->res != NULL) {
+		release_resource(ep->res);
+		kfree(ep->res);
+	}
+
+	free_netdev(dev);
+
+	return 0;
+}
+
+static int ep93xx_eth_probe(struct platform_device *pdev)
+{
+	struct ep93xx_eth_data *data;
+	struct net_device *dev;
+	struct ep93xx_priv *ep;
+	int err;
+
+	data = pdev->dev.platform_data;
+	if (pdev == NULL)
+		return -ENODEV;
+
+	printk(KERN_INFO "ep93xx-eth " DRV_MODULE_VERSION " loading\n");
+
+	dev = ep93xx_dev_alloc(data);
+	if (dev == NULL) {
+		err = -ENOMEM;
+		goto err_out;
+	}
+	ep = netdev_priv(dev);
+
+	platform_set_drvdata(pdev, dev);
+
+	ep->res = request_mem_region(pdev->resource[0].start,
+			pdev->resource[0].end - pdev->resource[0].start + 1,
+			pdev->dev.bus_id);
+	if (ep->res == NULL) {
+		dev_err(&pdev->dev, "Could not reserve memory region\n");
+		err = -ENOMEM;
+		goto err_out;
+	}
+
+	ep->base_addr = ioremap(pdev->resource[0].start,
+			pdev->resource[0].end - pdev->resource[0].start);
+	if (ep->base_addr == NULL) {
+		dev_err(&pdev->dev, "Failed to ioremap ethernet registers\n");
+		err = -EIO;
+		goto err_out;
+	}
+	ep->irq = pdev->resource[1].start;
+
+	err = register_netdev(dev);
+	if (err) {
+		dev_err(&pdev->dev, "Failed to register netdev\n");
+		goto err_out;
+	}
+
+	return 0;
+
+err_out:
+	ep93xx_eth_remove(pdev);
+	return err;
+}
+
+
+static struct platform_driver ep93xx_eth_driver = {
+	.probe		= ep93xx_eth_probe,
+	.remove		= ep93xx_eth_remove,
+	.driver		= {
+		.name	= "ep93xx-eth",
+	},
+};
+
+static int __init ep93xx_eth_init_module(void)
+{
+	return platform_driver_register(&ep93xx_eth_driver);
+}
+
+static void __exit ep93xx_eth_cleanup_module(void)
+{
+	platform_driver_unregister(&ep93xx_eth_driver);
+}
+
+module_init(ep93xx_eth_init_module);
+module_exit(ep93xx_eth_cleanup_module);
+MODULE_LICENSE("GPL");
Index: linux-2.6.17-git10/drivers/net/arm/ep93xx_eth.h
===================================================================
--- /dev/null
+++ linux-2.6.17-git10/drivers/net/arm/ep93xx_eth.h
@@ -0,0 +1,75 @@
+/*
+ * EP93xx ethernet network device driver
+ * Copyright (C) 2006 Lennert Buytenhek <buytenh@wantstofly.org>
+ * Dedicated to Marija Kulikova.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __EP93XX_ETH_H
+#define __EP93XX_ETH_H
+
+struct ep93xx_rdesc
+{
+	u32	buf_addr;
+	u32	rdesc1;
+};
+
+#define RDESC1_NSOF		0x80000000
+#define RDESC1_BUFFER_INDEX	0x7fff0000
+#define RDESC1_BUFFER_LENGTH	0x0000ffff
+
+struct ep93xx_rstat
+{
+	u32	rstat0;
+	u32	rstat1;
+};
+
+#define RSTAT0_RFP		0x80000000
+#define RSTAT0_RWE		0x40000000
+#define RSTAT0_EOF		0x20000000
+#define RSTAT0_EOB		0x10000000
+#define RSTAT0_AM		0x00c00000
+#define RSTAT0_RX_ERR		0x00200000
+#define RSTAT0_OE		0x00100000
+#define RSTAT0_FE		0x00080000
+#define RSTAT0_RUNT		0x00040000
+#define RSTAT0_EDATA		0x00020000
+#define RSTAT0_CRCE		0x00010000
+#define RSTAT0_CRCI		0x00008000
+#define RSTAT0_HTI		0x00003f00
+#define RSTAT1_RFP		0x80000000
+#define RSTAT1_BUFFER_INDEX	0x7fff0000
+#define RSTAT1_FRAME_LENGTH	0x0000ffff
+
+struct ep93xx_tdesc
+{
+	u32	buf_addr;
+	u32	tdesc1;
+};
+
+#define TDESC1_EOF		0x80000000
+#define TDESC1_BUFFER_INDEX	0x7fff0000
+#define TDESC1_BUFFER_ABORT	0x00008000
+#define TDESC1_BUFFER_LENGTH	0x00000fff
+
+struct ep93xx_tstat
+{
+	u32	tstat0;
+};
+
+#define TSTAT0_TXFP		0x80000000
+#define TSTAT0_TXWE		0x40000000
+#define TSTAT0_FA		0x20000000
+#define TSTAT0_LCRS		0x10000000
+#define TSTAT0_OW		0x04000000
+#define TSTAT0_TXU		0x02000000
+#define TSTAT0_ECOLL		0x01000000
+#define TSTAT0_NCOLL		0x001f0000
+#define TSTAT0_BUFFER_INDEX	0x00007fff
+
+
+#endif
Index: linux-2.6.17-git10/include/asm-arm/arch-ep93xx/ep93xx-regs.h
===================================================================
--- linux-2.6.17-git10.orig/include/asm-arm/arch-ep93xx/ep93xx-regs.h
+++ linux-2.6.17-git10/include/asm-arm/arch-ep93xx/ep93xx-regs.h
@@ -27,6 +27,7 @@
 #define EP93XX_DMA_BASE			(EP93XX_AHB_VIRT_BASE + 0x00000000)
 
 #define EP93XX_ETHERNET_BASE		(EP93XX_AHB_VIRT_BASE + 0x00010000)
+#define EP93XX_ETHERNET_PHYS_BASE	(EP93XX_AHB_PHYS_BASE + 0x00010000)
 
 #define EP93XX_USB_BASE			(EP93XX_AHB_VIRT_BASE + 0x00020000)
 #define EP93XX_USB_PHYS_BASE		(EP93XX_AHB_PHYS_BASE + 0x00020000)
Index: linux-2.6.17-git10/include/asm-arm/arch-ep93xx/platform.h
===================================================================
--- linux-2.6.17-git10.orig/include/asm-arm/arch-ep93xx/platform.h
+++ linux-2.6.17-git10/include/asm-arm/arch-ep93xx/platform.h
@@ -11,5 +11,10 @@ void ep93xx_init_devices(void);
 void ep93xx_clock_init(void);
 extern struct sys_timer ep93xx_timer;
 
+struct ep93xx_eth_data
+{
+	unsigned char	dev_addr[6];
+};
+
 
 #endif

^ permalink raw reply

* Re: [NET]: Added GSO header verification
From: Michael Chan @ 2006-06-28  2:07 UTC (permalink / raw)
  To: Herbert Xu; +Cc: David S. Miller, netdev
In-Reply-To: <20060627223120.GA19026@gondor.apana.org.au>

On Wed, 2006-06-28 at 08:31 +1000, Herbert Xu wrote:

> [NET]: Fix logical error in skb_gso_ok
> 
> The test in skb_gso_ok is backwards.  Noticed by Michael Chan
> <mchan@broadcom.com>.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Acked-by: Michael Chan <mchan@broadcom.com>


^ permalink raw reply

* Re: [Patch 1/1] AF_UNIX Datagram getpeersec (with latest updates)
From: James Morris @ 2006-06-28  2:13 UTC (permalink / raw)
  To: Xiaolan Zhang
  Cc: Andrew Morton, chrisw, Catherine Zhang, czhang.us,
	David S. Miller, George Wilson, Herbert Xu, latten, netdev,
	Stephen Smalley, Serge E Hallyn, tjaeger
In-Reply-To: <OF5E332833.A2E43B52-ON8525719B.0009D441-8525719B.000A510A@us.ibm.com>

On Tue, 27 Jun 2006, Xiaolan Zhang wrote:

> > Just one more thing, we don't need to export this function now.
> 
> You mean moving it to security/selinux/hooks.c and making it static?

Yep.

> I think conceptually this is where it should reside -- auditing system 
> might need it in the future, for example.

We can export it then.



- James
-- 
James Morris
<jmorris@namei.org>

^ permalink raw reply

* Re: [Patch 1/1] AF_UNIX Datagram getpeersec (with latest updates)
From: James Morris @ 2006-06-28  2:23 UTC (permalink / raw)
  To: Xiaolan Zhang
  Cc: Andrew Morton, chrisw, Catherine Zhang, czhang.us,
	David S. Miller, George Wilson, Herbert Xu, latten, netdev,
	Stephen Smalley, Serge E Hallyn, tjaeger
In-Reply-To: <Pine.LNX.4.64.0606272213240.7100@d.namei>

On Tue, 27 Jun 2006, James Morris wrote:

> > I think conceptually this is where it should reside -- auditing system 
> > might need it in the future, for example.
> 
> We can export it then.

To clarify, we can export it if the audit system needs it, in the future.



- James
-- 
James Morris
<jmorris@namei.org>

^ permalink raw reply

* Re: [Patch 1/1] AF_UNIX Datagram getpeersec (with latest updates)
From: Xiaolan Zhang @ 2006-06-28  2:45 UTC (permalink / raw)
  To: James Morris
  Cc: Andrew Morton, chrisw, Catherine Zhang, czhang.us,
	David S. Miller, George Wilson, Herbert Xu, latten, netdev,
	Stephen Smalley, Serge E Hallyn, tjaeger
In-Reply-To: <Pine.LNX.4.64.0606272213240.7100@d.namei>

Got it.  Will send a new patch soon.

Catherine

James Morris <jmorris@namei.org> wrote on 06/27/2006 10:13:48 PM:

> On Tue, 27 Jun 2006, Xiaolan Zhang wrote:
> 
> > > Just one more thing, we don't need to export this function now.
> > 
> > You mean moving it to security/selinux/hooks.c and making it static?
> 
> Yep.
> 
> > I think conceptually this is where it should reside -- auditing system 

> > might need it in the future, for example.
> 
> We can export it then.
> 
> 
> 
> - James
> -- 
> James Morris
> <jmorris@namei.org>


^ permalink raw reply

* Re: [PATCH Round 3 0/2][RFC] Network Event Notifier Mechanism
From: Herbert Xu @ 2006-06-28  2:54 UTC (permalink / raw)
  To: Steve Wise; +Cc: davem, netdev
In-Reply-To: <20060627205050.30723.25753.stgit@stevo-desktop>

Steve Wise <swise@opengridcomputing.com> wrote:
> 
> The reason these devices need update events is because they typically
> cache this information in hardware and need to be notified when this
> information has been updated.  For information on RDMA protocols, see:
> http://www.ietf.org/html.charters/rddp-charter.html.

Please give more specific reasons for needing these events because it
is certainly far from obvious from reading those documents.

Without reasons these invasive changes may turn out to be completely
inappropriate.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH Round 3 0/2][RFC] Network Event Notifier Mechanism
From: Herbert Xu @ 2006-06-28  3:04 UTC (permalink / raw)
  To: Steve Wise; +Cc: davem, netdev, Jeff Garzik
In-Reply-To: <E1FvQBu-00018u-00@gondolin.me.apana.org.au>

On Wed, Jun 28, 2006 at 12:54:10PM +1000, Herbert Xu wrote:
> 
> Please give more specific reasons for needing these events because it
> is certainly far from obvious from reading those documents.

Never mind, I've found your earlier messages on the list which explains
your reasons more clearly.  It would be nice if you could include those
explanations in your patch description.

BTW, does this mean that we're now comfortable with full TOE?

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* [PATCH]NET: Add ECN support for TSO
From: Michael Chan @ 2006-06-28  3:06 UTC (permalink / raw)
  To: davem, herbert; +Cc: netdev

In the current TSO implementation, NETIF_F_TSO and ECN cannot be
turned on together in a TCP connection.  The problem is that most
hardware that supports TSO does not handle CWR correctly if it is set
in the TSO packet.  Correct handling requires CWR to be set in the
first packet only if it is set in the TSO header.

This patch adds the ability to turn on NETIF_F_TSO and ECN using 
GSO if necessary to handle TSO packets with CWR set.  Hardware
that handles CWR correctly can turn on NETIF_F_TSO_ECN in the dev->
features flag.

All TSO packets with CWR set will have the SKB_GSO_TCPV4_ECN set.  If
the output device does not have the NETIF_F_TSO_ECN feature set, GSO
will split the packet up correctly with CWR only set in the first
segment.

It is further assumed that all hardware will handle ECE properly by
replicating the ECE flag in all segments.  If that is not the case, a
simple extension of the logic will be required.


Signed-off-by: Michael Chan <mchan@broadcom.com>


diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index efd1e2a..f393de2 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -316,6 +316,7 @@ struct net_device
 #define NETIF_F_TSO		(SKB_GSO_TCPV4 << NETIF_F_GSO_SHIFT)
 #define NETIF_F_UFO		(SKB_GSO_UDPV4 << NETIF_F_GSO_SHIFT)
 #define NETIF_F_GSO_ROBUST	(SKB_GSO_DODGY << NETIF_F_GSO_SHIFT)
+#define NETIF_F_TSO_ECN		(SKB_GSO_TCPV4_ECN << NETIF_F_GSO_SHIFT)
 
 #define NETIF_F_GEN_CSUM	(NETIF_F_NO_CSUM | NETIF_F_HW_CSUM)
 #define NETIF_F_ALL_CSUM	(NETIF_F_IP_CSUM | NETIF_F_GEN_CSUM)
@@ -1002,6 +1003,11 @@ static inline int netif_needs_gso(struct
 	return !skb_gso_ok(skb, dev->features);
 }
 
+static inline int tso_ecn_capable(unsigned long features)
+{
+	return ((features & NETIF_F_GSO) || (features & NETIF_F_TSO_ECN));
+}
+
 #endif /* __KERNEL__ */
 
 #endif	/* _LINUX_DEV_H */
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 5fb72da..e74c294 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -175,6 +175,9 @@ enum {
 
 	/* This indicates the skb is from an untrusted source. */
 	SKB_GSO_DODGY = 1 << 2,
+
+	/* This indicates the tcp segment has CWR set. */
+	SKB_GSO_TCPV4_ECN = 1 << 3,
 };
 
 /** 
diff --git a/include/net/sock.h b/include/net/sock.h
index 2d8d6ad..2c75172 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1033,7 +1033,8 @@ static inline void sk_setup_caps(struct 
 	if (sk->sk_route_caps & NETIF_F_GSO)
 		sk->sk_route_caps |= NETIF_F_TSO;
 	if (sk->sk_route_caps & NETIF_F_TSO) {
-		if (sock_flag(sk, SOCK_NO_LARGESEND) || dst->header_len)
+		if ((sock_flag(sk, SOCK_NO_LARGESEND) &&
+		    !tso_ecn_capable(sk->sk_route_caps)) || dst->header_len)
 			sk->sk_route_caps &= ~NETIF_F_TSO;
 		else 
 			sk->sk_route_caps |= NETIF_F_SG | NETIF_F_HW_CSUM;
diff --git a/include/net/tcp_ecn.h b/include/net/tcp_ecn.h
index c6b8439..871dca2 100644
--- a/include/net/tcp_ecn.h
+++ b/include/net/tcp_ecn.h
@@ -31,7 +31,8 @@ static inline void TCP_ECN_send_syn(stru
 				    struct sk_buff *skb)
 {
 	tp->ecn_flags = 0;
-	if (sysctl_tcp_ecn && !(sk->sk_route_caps & NETIF_F_TSO)) {
+	if (sysctl_tcp_ecn && (!(sk->sk_route_caps & NETIF_F_TSO) ||
+			       tso_ecn_capable(sk->sk_route_caps))) {
 		TCP_SKB_CB(skb)->flags |= TCPCB_FLAG_ECE|TCPCB_FLAG_CWR;
 		tp->ecn_flags = TCP_ECN_OK;
 		sock_set_flag(sk, SOCK_NO_LARGESEND);
@@ -56,6 +57,9 @@ static inline void TCP_ECN_send(struct s
 			if (tp->ecn_flags&TCP_ECN_QUEUE_CWR) {
 				tp->ecn_flags &= ~TCP_ECN_QUEUE_CWR;
 				skb->h.th->cwr = 1;
+				if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4)
+					skb_shinfo(skb)->gso_type |=
+						SKB_GSO_TCPV4_ECN;
 			}
 		} else {
 			/* ACK or retransmitted segment: clear ECT|CE */
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index bdd71db..c4a4dba 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2044,7 +2044,8 @@ struct sk_buff * tcp_make_synack(struct 
 	memset(th, 0, sizeof(struct tcphdr));
 	th->syn = 1;
 	th->ack = 1;
-	if (dst->dev->features&NETIF_F_TSO)
+	if ((dst->dev->features & NETIF_F_TSO) &&
+	    !tso_ecn_capable(dst->dev->features))
 		ireq->ecn_ok = 0;
 	TCP_ECN_make_synack(req, th);
 	th->source = inet_sk(sk)->sport;



^ permalink raw reply related

* [PATCH]bnx2: Add NETIF_F_TSO_ECN
From: Michael Chan @ 2006-06-28  3:07 UTC (permalink / raw)
  To: davem, herbert; +Cc: netdev

Add NETIF_F_TSO_ECN feature for all bnx2 hardware.

Signed-off-by: Michael Chan <mchan@broadcom.com>


diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 7635736..e89d5df 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -5128,6 +5128,16 @@ bnx2_set_rx_csum(struct net_device *dev,
 	return 0;
 }
 
+static int
+bnx2_set_tso(struct net_device *dev, u32 data)
+{
+	if (data)
+		dev->features |= NETIF_F_TSO | NETIF_F_TSO_ECN;
+	else
+		dev->features &= ~(NETIF_F_TSO | NETIF_F_TSO_ECN);
+	return 0;
+}
+
 #define BNX2_NUM_STATS 46
 
 static struct {
@@ -5445,7 +5455,7 @@ static struct ethtool_ops bnx2_ethtool_o
 	.set_sg			= ethtool_op_set_sg,
 #ifdef BCM_TSO
 	.get_tso		= ethtool_op_get_tso,
-	.set_tso		= ethtool_op_set_tso,
+	.set_tso		= bnx2_set_tso,
 #endif
 	.self_test_count	= bnx2_self_test_count,
 	.self_test		= bnx2_self_test,
@@ -5926,7 +5936,7 @@ bnx2_init_one(struct pci_dev *pdev, cons
 	dev->features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
 #endif
 #ifdef BCM_TSO
-	dev->features |= NETIF_F_TSO;
+	dev->features |= NETIF_F_TSO | NETIF_F_TSO_ECN;
 #endif
 
 	netif_carrier_off(bp->dev);



^ permalink raw reply related

* Re: [PATCH]NET: Add ECN support for TSO
From: Herbert Xu @ 2006-06-28  3:10 UTC (permalink / raw)
  To: Michael Chan; +Cc: davem, netdev
In-Reply-To: <1151464007.5124.13.camel@rh4>

On Tue, Jun 27, 2006 at 08:06:47PM -0700, Michael Chan wrote:
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 2d8d6ad..2c75172 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -1033,7 +1033,8 @@ static inline void sk_setup_caps(struct 
>  	if (sk->sk_route_caps & NETIF_F_GSO)
>  		sk->sk_route_caps |= NETIF_F_TSO;
>  	if (sk->sk_route_caps & NETIF_F_TSO) {
> -		if (sock_flag(sk, SOCK_NO_LARGESEND) || dst->header_len)
> +		if ((sock_flag(sk, SOCK_NO_LARGESEND) &&
> +		    !tso_ecn_capable(sk->sk_route_caps)) || dst->header_len)
>  			sk->sk_route_caps &= ~NETIF_F_TSO;

Why turn it off? With GSO in place the stack will handle it just fine
(even your description says so :)  We should instead remove all code
that turns off TSO/ECN when the other is present.

Otherwise the patch looks good.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH Round 3 0/2][RFC] Network Event Notifier Mechanism
From: Jeff Garzik @ 2006-06-28  3:24 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Steve Wise, davem, netdev
In-Reply-To: <20060628030400.GA4474@gondor.apana.org.au>

Herbert Xu wrote:
> On Wed, Jun 28, 2006 at 12:54:10PM +1000, Herbert Xu wrote:
>> Please give more specific reasons for needing these events because it
>> is certainly far from obvious from reading those documents.
> 
> Never mind, I've found your earlier messages on the list which explains
> your reasons more clearly.  It would be nice if you could include those
> explanations in your patch description.
> 
> BTW, does this mean that we're now comfortable with full TOE?

I don't see how that position has changed?

http://linux-net.osdl.org/index.php/TOE

	Jeff



^ permalink raw reply

* Re: [PATCH Round 3 0/2][RFC] Network Event Notifier Mechanism
From: Herbert Xu @ 2006-06-28  3:37 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Steve Wise, davem, netdev
In-Reply-To: <44A1F669.8070606@pobox.com>

On Tue, Jun 27, 2006 at 11:24:25PM -0400, Jeff Garzik wrote:
>
> I don't see how that position has changed?
> 
> http://linux-net.osdl.org/index.php/TOE

Well I must say that RDMA over TCP smells very much like TOE.  They've
got an ARP table, a routing table, and presumably a TCP stack.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [patch 2/6] [Network namespace] Network device sharing by view
From: Eric W. Biederman @ 2006-06-28  3:38 UTC (permalink / raw)
  To: Alexey Kuznetsov
  Cc: Dave Hansen, Herbert Poetzl, Ben Greear, Daniel Lezcano,
	Andrey Savochkin, linux-kernel, netdev, serue, clg, Andrew Morton,
	dev, devel, sam, viro, Alexey Kuznetsov
In-Reply-To: <20060627234210.GA1598@ms2.inr.ac.ru>

Alexey Kuznetsov <kuznet@ms2.inr.ac.ru> writes:

> Hello!
>
>> It may look weird, but do application really *need* to see eth0 rather
>> than eth858354?
>
> Applications do not care, humans do. :-)
>
> What's about applications they just need to see exactly the same device
> after migration. Not only name, but f.e. also its ifindex. If you do not
> create a separate namespace for netdevices, you will inevitably end up
> with some strange hack sort of VPIDs to translate (or to partition) ifindices
> or to tell that "ping -I eth858354 xxx" is too coimplicated application
> to survive migration.


Actually there are applications with peculiar licensing practices that
do look at devices like eth0 to verify you have the appropriate mac, and
do really weird things if you don't have an eth0.

Plus there are other cases where it can be simpler to hard code things
if it is allowable. (The human factor)  Otherwise your configuration
must be done through hotplug scripts.

But yes there are misguided applications that care.

Eric

^ permalink raw reply

* Re: [PATCH]NET: Add ECN support for TSO
From: Michael Chan @ 2006-06-28  3:40 UTC (permalink / raw)
  To: Herbert Xu; +Cc: davem, netdev
In-Reply-To: <20060628031052.GA4570@gondor.apana.org.au>

On Wed, 2006-06-28 at 13:10 +1000, Herbert Xu wrote:
> On Tue, Jun 27, 2006 at 08:06:47PM -0700, Michael Chan wrote:
> >
> > diff --git a/include/net/sock.h b/include/net/sock.h
> > index 2d8d6ad..2c75172 100644
> > --- a/include/net/sock.h
> > +++ b/include/net/sock.h
> > @@ -1033,7 +1033,8 @@ static inline void sk_setup_caps(struct 
> >  	if (sk->sk_route_caps & NETIF_F_GSO)
> >  		sk->sk_route_caps |= NETIF_F_TSO;
> >  	if (sk->sk_route_caps & NETIF_F_TSO) {
> > -		if (sock_flag(sk, SOCK_NO_LARGESEND) || dst->header_len)
> > +		if ((sock_flag(sk, SOCK_NO_LARGESEND) &&
> > +		    !tso_ecn_capable(sk->sk_route_caps)) || dst->header_len)
> >  			sk->sk_route_caps &= ~NETIF_F_TSO;
> 
> Why turn it off? With GSO in place the stack will handle it just fine
> (even your description says so :)  We should instead remove all code
> that turns off TSO/ECN when the other is present.
> 
We need to turn off NETIF_F_TSO for a connection that has negotiated to
turn on ECN if the output device cannot handle TSO and ECN.  In other
words, if the output device does not have either GSO or TSO_ECN feature
set.


^ permalink raw reply

* Re: [PATCH]NET: Add ECN support for TSO
From: Herbert Xu @ 2006-06-28  3:48 UTC (permalink / raw)
  To: Michael Chan; +Cc: davem, netdev
In-Reply-To: <1151466034.5124.18.camel@rh4>

On Tue, Jun 27, 2006 at 08:40:34PM -0700, Michael Chan wrote:
>
> We need to turn off NETIF_F_TSO for a connection that has negotiated to
> turn on ECN if the output device cannot handle TSO and ECN.  In other
> words, if the output device does not have either GSO or TSO_ECN feature
> set.

I think you're mixing up GSO the mechanism with GSO the flag.  The GSO
flag simply tells the TCP stack whether TSO should be used or not, even
if the hardware does not support TSO at all.  The GSO mechanism on the
other hand is ALWAYS present.  So regardless of the presence of the GSO
flag, you can always rely on the GSO mechanism to pick up the pieces (or
rather generate the pieces as the case may be :)

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [patch 2/6] [Network namespace] Network device sharing by view
From: Eric W. Biederman @ 2006-06-28  4:07 UTC (permalink / raw)
  To: Herbert Poetzl
  Cc: Kirill Korotaev, Daniel Lezcano, Andrey Savochkin, linux-kernel,
	netdev, serue, haveblue, clg, Andrew Morton, devel, sam, viro,
	Alexey Kuznetsov
In-Reply-To: <20060627230723.GC2612@MAIL.13thfloor.at>

Herbert Poetzl <herbert@13thfloor.at> writes:

> On Tue, Jun 27, 2006 at 10:29:39AM -0600, Eric W. Biederman wrote:
>> Herbert Poetzl <herbert@13thfloor.at> writes:
>
>> I watched the linux-vserver irc channel for a while and almost
>> every network problem was caused by the change in semantics 
>> vserver provides.
>
> the problem here is not the change in semantics compared
> to a real linux system (as there basically is none) but
> compared to _other_ technologies like UML or QEMU, which
> add the need for bridging and additional interfaces, while
> Linux-VServer only focuses on the IP layer ...

Not being able to bind to INADDR_ANY is a huge semantic change.
Unless things have changed recently you get that change when
you have two IP addresses in Linux-Vserver.

Talking to the outsider world through the loop back interface
is a noticeable semantics change.

Having to be careful of who uses INADDR_ANY on the host
when you have guests is essentially a semantics change.

Being able to talk to the outside world with a server
bound only to the loopback IP is a weird semantic
change.

And I suspect I missed something, it is weird peculiar and
I don't care to remember all of the exceptions.

Have a few more network interfaces for a layer 2 solution
is fundamental.  Believing without proof and after arguments
to the contrary that you have not contradicted that a layer 2
solution is inherently slower is non-productive.  Arguing
that a layer 2 only solution most prove itself on guest to guest
communication is also non-productive.

So just to sink one additional nail in the coffin of the silly
guest to guest communication issue.  For any two guests where
fast communication between them is really important I can run
an additional interface pair that requires no routing or bridging.
Given that the implementation of the tunnel device is essentially
the same as the loopback interface and that I make only one
trip through the network stack there will be no performance overhead.
Similarly for any critical guest communication to the outside world
I can give the guest a real network adapter.

That said I don't think those things will be necessary and that if
they are it is an optimization opportunity to make various bits
of the network stack faster.

Bridging or routing between guests is an exercise in simplicity
and control not a requirement.

>> In this case when you allow a guest more than one IP your hack 
>> while easy to maintain becomes much more complex. 
>
> why? a set of IPs is quite similar to a single IP (which
> is actually a subset), so no real change there, only
> IP_ANY means something different for a guest ...

Which simply filtering at bind time makes impossible.

With a guest with 4 IPs 
10.0.0.1 192.168.0.1 172.16.0.1 127.0.0.1
How do you make INADDR_ANY work with just filtering at bind time?

The host has at least the additional IPs.
10.0.0.2 192.168.0.2 172.16.0.2 127.0.0.1

Herbert I suspect we are talking about completely different
implementations otherwise I can't possibly see how we have
such different perceptions of their capabilities.

I am talking precisely about filter IP addresses at connect
or bind time that a guest can use.  Which as I recall is
what vserver implements.  If you are thinking of your ngnet
implementation that would explain things.

>> Especially as you address each case people care about one at a time.
>
> hmm?

Multiple IPs, IPv6, additional protocols, firewalls. etc.

>> In one shot this goes the entire way. Given how many people miss that
>> you do the work at layer 2 than at layer 3 I would not call this the
>> straight forward approach. The straight forward implementation yes,
>> but not the straight forward approach.
>
> seems I lost you here ...


>> > for example, you won't have multiple routing tables
>> > in a kernel where this feature is disabled, no?
>> > so why should it affect a guest, or require modified
>> > apps inside a guest when we would decide to provide
>> > only a single routing table?
>> >
>> >> From my POV, fully virtualized namespaces are the future. 
>> >
>> > the future is already there, it's called Xen or UML, or QEMU :)
>> 
>> Yep.  And now we need it to run fast.
>
> hmm, maybe you should try to optimize linux for Xen then,
> as I'm sure it will provide the optimal virtualization
> and has all the features folks are looking for (regarding
> virtualization)
>
> I thought we are trying to figure a light-weight subset
> of isolation and virtualization technologies and methods
> which make sense to have in mainline ...

And you presume doing things at layer 2 is more expensive than
layer 3.

>From what I have seen of layer 3 solutions it is a 
bloody maintenance nightmare, and an inflexible mess.

>> >> It is what makes virtualization solution usable (w/o apps
>> >> modifications), provides all the features and doesn't require much
>> >> efforts from people to be used.
>> >
>> > and what if they want to use virtualization inside
>> > their guests? where do you draw the line?
>> 
>> The implementation doesn't have any problems with guests inside
>> of guests.
>> 
>> The only reason to restrict guests inside of guests is because
>> the we aren't certain which permissions make sense.
>
> well, we have not even touched the permission issues yet

Agreed, permissions have not discussed but the point is that is the only
reason to keep from nesting the networking stack the way I have described
it.

Eric

^ permalink raw reply

* TOE, etc. (was Re: [PATCH Round 3 0/2][RFC] Network Event Notifier Mechanism)
From: Jeff Garzik @ 2006-06-28  4:18 UTC (permalink / raw)
  To: Herbert Xu, davem; +Cc: Steve Wise, netdev
In-Reply-To: <20060628033708.GA4922@gondor.apana.org.au>

Herbert Xu wrote:
> On Tue, Jun 27, 2006 at 11:24:25PM -0400, Jeff Garzik wrote:
>> I don't see how that position has changed?
>>
>> http://linux-net.osdl.org/index.php/TOE
> 
> Well I must say that RDMA over TCP smells very much like TOE.  They've
> got an ARP table, a routing table, and presumably a TCP stack.

A PCI device that presents itself as a SCSI controller, but under the 
hood is really iSCSI-over-TCP smells like TOE.  Running a virtualized 
Linux guest on top of a proprietary stack [which provides networking 
services to guests] also smells like TOE.  :)

If a TOE vendors wants to do TOE in a way that is transparent to the 
kernel, more power to them.  Such non-Linux TCP stack solutions still 
suffer many of the problems listed at the web page above, but at least 
they impose no burden on kernel maintenance.

i.e. we really _do not_ want to get into the habit of co-managing arp 
tables, routing tables, filtering rules, and dozens of other such 
resources with multiple remote, independent TCP stack.  We have enough 
complexity as it is today, coordinating between the random variations of 
SMP, uniprocessor, and NUMA machines out there.  Not to mention 
competing with under-the-hood firmware actions (ASF) on NICs.

As an aside, RDMA over TCP just seems silly.  TCP was _not_ meant to do 
the things that RDMA users want.  The infiniband/RDMA programming model 
is an ultra-low-latency polling model where one or two apps are allowed 
to completely consume the machine, either busy-waiting or processing 
messages.

Unfortunately I don't have more details, so you just get a generalized 
rant :)

	Jeff




^ permalink raw reply

* Re: Network namespaces a path to mergable code.
From: Eric W. Biederman @ 2006-06-28  4:20 UTC (permalink / raw)
  To: Andrey Savochkin
  Cc: dlezcano, linux-kernel, netdev, serue, haveblue, clg,
	Andrew Morton, dev, herbert, devel, sam, viro, Alexey Kuznetsov
In-Reply-To: <20060627215859.A20679@castle.nmd.msu.ru>

Andrey Savochkin <saw@swsoft.com> writes:

> Eric,
>
> On Tue, Jun 27, 2006 at 11:20:40AM -0600, Eric W. Biederman wrote:
>> 
>> Thinking about this I am going to suggest a slightly different direction
>> for get a patchset we can merge.
>> 
>> First we concentrate on the fundamentals.
>> - How we mark a device as belonging to a specific network namespace.
>> - How we mark a socket as belonging to a specific network namespace.
>
> I agree with the direction of your thoughts.
> I was trying to do a similar thing, define clear steps in network
> namespace merging.
>
> My first patchset covers devices but not sockets.
> The only difference from what you're suggesting is ipv4 routing.
> For me, it is not less important than devices and sockets.  May be even
> more important, since routing exposes design deficiencies less obvious at
> socket level.

I agree we need to do it.  I mostly want a base that allows us to 
not need to convert the whole network stack at once and still be able
to merge code all the way to the stable kernel.

The routing code is important for understanding design choices.  It
isn't important for merging if that makes sense.   

For everyone looking at routing choices the IPv6 routing table is
interesting because it does not use a hash table, and seems quite
possibly to be an equally fast structure that scales better.

There is something to think about there.

>> As part of the fundamentals we add a patch to the generic socket code
>> that by default will disable it for protocol families that do not indicate
>> support for handling network namespaces, on a non-default network namespace.
>
> Fine
>
> Can you summarize you objections against my way of handling devices, please?
> And what was the typo you referred to in your letter to Kirill Korotaev?

I have no fundamental objects to the content I have seen so far.

Please read the first email Kirill responded too.  I quoted a couple
of sections of code and described the bugs I saw with the patch.

All minor things.  The typo I was referring to was a section where the
original iteration was on an ifp variable and you called it dev
without changing the rest of the code in that section.  

The only big issue was that the patch too big, and should be split
into a patchset for better review.  One patch for the new functions,
and the an additional patch for each driver/subsystem hunk describing
why that chunk needed to be changed.

I'm still curious why many of those chunks can't use existing helper
functions, to be cleaned up.

Eric

^ permalink raw reply

* Re: TOE, etc. (was Re: [PATCH Round 3 0/2][RFC] Network Event Notifier Mechanism)
From: Herbert Xu @ 2006-06-28  4:29 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: davem, Steve Wise, netdev
In-Reply-To: <44A20311.5050301@pobox.com>

On Wed, Jun 28, 2006 at 12:18:25AM -0400, Jeff Garzik wrote:
> 
> A PCI device that presents itself as a SCSI controller, but under the 
> hood is really iSCSI-over-TCP smells like TOE.  Running a virtualized 
> Linux guest on top of a proprietary stack [which provides networking 
> services to guests] also smells like TOE.  :)

Agreed.  However, when they start adding hooks to the ARP table, the
routing table, and PMTU management, it begs the question what more is
there to add for TOE (well, user-space driven TOE at least)?
 
> Unfortunately I don't have more details, so you just get a generalized 
> rant :)

OK, the patch under discussion here adds hooks to all the stuff in the
previous paragraph for the purpose of RDMA over TCP (well I must say
that the exact RDMA application/hardware has never been clearly given
but this is what I can gather from the previous posts).

Put it another way, I think the dividing line between TOE and iSCSI or
virtualisation is exactly the interface between them and the Linux kernel.
If the interface is an existing one such as SCSI or standard IP then it's
OK.  However, when it starts poking in the guts of the Linux stack I'd say
that it has crossed the line.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: Network namespaces a path to mergable code.
From: Eric W. Biederman @ 2006-06-28  4:33 UTC (permalink / raw)
  To: Sam Vilain
  Cc: Andrey Savochkin, dlezcano, linux-kernel, netdev, serue, haveblue,
	clg, Andrew Morton, dev, herbert, devel, viro, Alexey Kuznetsov,
	Mark Huang
In-Reply-To: <44A1AF37.3070100@vilain.net>

Sam Vilain <sam@vilain.net> writes:

> It sounds then like it would be a good start to have general socket
> namespaces, if it would merge more easily - perhaps then network device
> namespaces would fall into place more easily.


I guess I really see both sockets and devices as the fundamental
entities of a network namespace.  Sockets need to be tagged because
in the general case there is no guarantee that a socket that you are
using was created in the network namespace of your current process.

In general it is possible to get file descriptors opened by someone
else because unix domain sockets allow file descriptor passing.  Similarly
I think there are cases in both unshare and fork that allows you to sockets
open before you entered a namespace.

Since you can't create a new socket in a different network namespace
I can't see any real problems with allowing them to be used, but they
are something to be careful about in container creation code.

Something to examine here is that if both network devices and sockets
are tagged does that still allow implicit network namespace passing.

Eric

^ permalink raw reply

* Re: [PATCH]NET: Add ECN support for TSO
From: Michael Chan @ 2006-06-28  4:37 UTC (permalink / raw)
  To: Herbert Xu; +Cc: davem, netdev
In-Reply-To: <20060628034823.GA5125@gondor.apana.org.au>

On Wed, 2006-06-28 at 13:48 +1000, Herbert Xu wrote:

> I think you're mixing up GSO the mechanism with GSO the flag.  The GSO
> flag simply tells the TCP stack whether TSO should be used or not, even
> if the hardware does not support TSO at all.  The GSO mechanism on the
> other hand is ALWAYS present.  So regardless of the presence of the GSO
> flag, you can always rely on the GSO mechanism to pick up the pieces (or
> rather generate the pieces as the case may be :)
> 
Thanks, that was my confusion.  Here's the revised patch:

[NET]: Add ECN support for TSO

In the current TSO implementation, NETIF_F_TSO and ECN cannot be
turned on together in a TCP connection.  The problem is that most
hardware that supports TSO does not handle CWR correctly if it is set
in the TSO packet.  Correct handling requires CWR to be set in the
first packet only if it is set in the TSO header.

This patch adds the ability to turn on NETIF_F_TSO and ECN using 
GSO if necessary to handle TSO packets with CWR set.  Hardware
that handles CWR correctly can turn on NETIF_F_TSO_ECN in the dev->
features flag.

All TSO packets with CWR set will have the SKB_GSO_TCPV4_ECN set.  If
the output device does not have the NETIF_F_TSO_ECN feature set, GSO
will split the packet up correctly with CWR only set in the first
segment.

With help from Herbert Xu <herbert@gondor.apana.org.au>.

Since ECN can always be enabled with TSO, the SOCK_NO_LARGESEND sock
flag is completely removed.

Signed-off-by: Michael Chan <mchan@broadcom.com>


diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 84b0f0d..a42a9f4 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -316,6 +316,7 @@ struct net_device
 #define NETIF_F_TSO		(SKB_GSO_TCPV4 << NETIF_F_GSO_SHIFT)
 #define NETIF_F_UFO		(SKB_GSO_UDPV4 << NETIF_F_GSO_SHIFT)
 #define NETIF_F_GSO_ROBUST	(SKB_GSO_DODGY << NETIF_F_GSO_SHIFT)
+#define NETIF_F_TSO_ECN		(SKB_GSO_TCPV4_ECN << NETIF_F_GSO_SHIFT)
 
 #define NETIF_F_GEN_CSUM	(NETIF_F_NO_CSUM | NETIF_F_HW_CSUM)
 #define NETIF_F_ALL_CSUM	(NETIF_F_IP_CSUM | NETIF_F_GEN_CSUM)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 5fb72da..e74c294 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -175,6 +175,9 @@ enum {
 
 	/* This indicates the skb is from an untrusted source. */
 	SKB_GSO_DODGY = 1 << 2,
+
+	/* This indicates the tcp segment has CWR set. */
+	SKB_GSO_TCPV4_ECN = 1 << 3,
 };
 
 /** 
diff --git a/include/net/sock.h b/include/net/sock.h
index 2d8d6ad..7136bae 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -383,7 +383,6 @@ enum sock_flags {
 	SOCK_USE_WRITE_QUEUE, /* whether to call sk->sk_write_space in sock_wfree */
 	SOCK_DBG, /* %SO_DEBUG setting */
 	SOCK_RCVTSTAMP, /* %SO_TIMESTAMP setting */
-	SOCK_NO_LARGESEND, /* whether to sent large segments or not */
 	SOCK_LOCALROUTE, /* route locally only, %SO_DONTROUTE setting */
 	SOCK_QUEUE_SHRUNK, /* write queue has been shrunk recently */
 };
@@ -1033,7 +1032,7 @@ static inline void sk_setup_caps(struct 
 	if (sk->sk_route_caps & NETIF_F_GSO)
 		sk->sk_route_caps |= NETIF_F_TSO;
 	if (sk->sk_route_caps & NETIF_F_TSO) {
-		if (sock_flag(sk, SOCK_NO_LARGESEND) || dst->header_len)
+		if (dst->header_len)
 			sk->sk_route_caps &= ~NETIF_F_TSO;
 		else 
 			sk->sk_route_caps |= NETIF_F_SG | NETIF_F_HW_CSUM;
diff --git a/include/net/tcp_ecn.h b/include/net/tcp_ecn.h
index c6b8439..7bb366f 100644
--- a/include/net/tcp_ecn.h
+++ b/include/net/tcp_ecn.h
@@ -31,10 +31,9 @@ static inline void TCP_ECN_send_syn(stru
 				    struct sk_buff *skb)
 {
 	tp->ecn_flags = 0;
-	if (sysctl_tcp_ecn && !(sk->sk_route_caps & NETIF_F_TSO)) {
+	if (sysctl_tcp_ecn) {
 		TCP_SKB_CB(skb)->flags |= TCPCB_FLAG_ECE|TCPCB_FLAG_CWR;
 		tp->ecn_flags = TCP_ECN_OK;
-		sock_set_flag(sk, SOCK_NO_LARGESEND);
 	}
 }
 
@@ -56,6 +55,9 @@ static inline void TCP_ECN_send(struct s
 			if (tp->ecn_flags&TCP_ECN_QUEUE_CWR) {
 				tp->ecn_flags &= ~TCP_ECN_QUEUE_CWR;
 				skb->h.th->cwr = 1;
+				if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4)
+					skb_shinfo(skb)->gso_type |=
+						SKB_GSO_TCPV4_ECN;
 			}
 		} else {
 			/* ACK or retransmitted segment: clear ECT|CE */
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 94fe5b1..7fa0b4a 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4178,8 +4178,6 @@ static int tcp_rcv_synsent_state_process
 		 */
 
 		TCP_ECN_rcv_synack(tp, th);
-		if (tp->ecn_flags&TCP_ECN_OK)
-			sock_set_flag(sk, SOCK_NO_LARGESEND);
 
 		tp->snd_wl1 = TCP_SKB_CB(skb)->seq;
 		tcp_ack(sk, skb, FLAG_SLOWPATH);
@@ -4322,8 +4320,6 @@ discard:
 		tp->max_window = tp->snd_wnd;
 
 		TCP_ECN_rcv_syn(tp, th);
-		if (tp->ecn_flags&TCP_ECN_OK)
-			sock_set_flag(sk, SOCK_NO_LARGESEND);
 
 		tcp_mtup_init(sk);
 		tcp_sync_mss(sk, icsk->icsk_pmtu_cookie);
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 2b9b7f6..54b2ef7 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -440,8 +440,6 @@ struct sock *tcp_create_openreq_child(st
 			newicsk->icsk_ack.last_seg_size = skb->len - newtp->tcp_header_len;
 		newtp->rx_opt.mss_clamp = req->mss;
 		TCP_ECN_openreq_child(newtp, req);
-		if (newtp->ecn_flags&TCP_ECN_OK)
-			sock_set_flag(newsk, SOCK_NO_LARGESEND);
 
 		TCP_INC_STATS_BH(TCP_MIB_PASSIVEOPENS);
 	}
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index bdd71db..5a7cb4a 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2044,8 +2044,6 @@ struct sk_buff * tcp_make_synack(struct 
 	memset(th, 0, sizeof(struct tcphdr));
 	th->syn = 1;
 	th->ack = 1;
-	if (dst->dev->features&NETIF_F_TSO)
-		ireq->ecn_ok = 0;
 	TCP_ECN_make_synack(req, th);
 	th->source = inet_sk(sk)->sport;
 	th->dest = ireq->rmt_port;



^ permalink raw reply related

* Re: TOE, etc. (was Re: [PATCH Round 3 0/2][RFC] Network Event Notifier Mechanism)
From: Jeff Garzik @ 2006-06-28  4:40 UTC (permalink / raw)
  To: Herbert Xu; +Cc: davem, Steve Wise, netdev
In-Reply-To: <20060628042959.GA5561@gondor.apana.org.au>

Herbert Xu wrote:
> On Wed, Jun 28, 2006 at 12:18:25AM -0400, Jeff Garzik wrote:
>> A PCI device that presents itself as a SCSI controller, but under the 
>> hood is really iSCSI-over-TCP smells like TOE.  Running a virtualized 
>> Linux guest on top of a proprietary stack [which provides networking 
>> services to guests] also smells like TOE.  :)
> 
> Agreed.  However, when they start adding hooks to the ARP table, the
> routing table, and PMTU management, it begs the question what more is
> there to add for TOE (well, user-space driven TOE at least)?

Well, you've always been able to implement userspace (or otherwise 
completely-virtualized) network stack.  tuntap and the packet socket 
enable that, if nothing else.  But, like you characterize below, those 
are existing, well-defined, easily contained interfaces.


> Put it another way, I think the dividing line between TOE and iSCSI or
> virtualisation is exactly the interface between them and the Linux kernel.
> If the interface is an existing one such as SCSI or standard IP then it's
> OK.  However, when it starts poking in the guts of the Linux stack I'd say
> that it has crossed the line.

Strongly agreed.

	Jeff



^ permalink raw reply

* Re: [PATCH]NET: Add ECN support for TSO
From: Herbert Xu @ 2006-06-28  4:42 UTC (permalink / raw)
  To: Michael Chan; +Cc: davem, netdev
In-Reply-To: <1151469421.3502.3.camel@rh4>

On Tue, Jun 27, 2006 at 09:37:01PM -0700, Michael Chan wrote:
> 
> Signed-off-by: Michael Chan <mchan@broadcom.com>

Looks good to me too!

> @@ -56,6 +55,9 @@ static inline void TCP_ECN_send(struct s
>  			if (tp->ecn_flags&TCP_ECN_QUEUE_CWR) {
>  				tp->ecn_flags &= ~TCP_ECN_QUEUE_CWR;
>  				skb->h.th->cwr = 1;
> +				if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4)
> +					skb_shinfo(skb)->gso_type |=
> +						SKB_GSO_TCPV4_ECN;

As a byte-pincher I must suggest that you turn this check into something
like 

				if (skb_shinfo(skb)->gso_type)

or even

				if (skb_shinfo(skb)->gso_size)

:)

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: TOE, etc.
From: David Miller @ 2006-06-28  4:43 UTC (permalink / raw)
  To: herbert; +Cc: jgarzik, swise, netdev
In-Reply-To: <20060628042959.GA5561@gondor.apana.org.au>

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Wed, 28 Jun 2006 14:29:59 +1000

> On Wed, Jun 28, 2006 at 12:18:25AM -0400, Jeff Garzik wrote:
> > 
> > A PCI device that presents itself as a SCSI controller, but under the 
> > hood is really iSCSI-over-TCP smells like TOE.  Running a virtualized 
> > Linux guest on top of a proprietary stack [which provides networking 
> > services to guests] also smells like TOE.  :)
> 
> Agreed.  However, when they start adding hooks to the ARP table, the
> routing table, and PMTU management, it begs the question what more is
> there to add for TOE (well, user-space driven TOE at least)?

Socket state, and that is one thing I don't see them doing yet.

> Put it another way, I think the dividing line between TOE and iSCSI or
> virtualisation is exactly the interface between them and the Linux kernel.
> If the interface is an existing one such as SCSI or standard IP then it's
> OK.  However, when it starts poking in the guts of the Linux stack I'd say
> that it has crossed the line.

Yeah, it's starting to smell really bad.

But we have to realize they've already been given %95 of the
interfaces they need to speak IP using our routes and our neighbour
entries.

Right?

^ permalink raw reply

* Re: [PATCH]NET: Add ECN support for TSO
From: Michael Chan @ 2006-06-28  4:54 UTC (permalink / raw)
  To: Herbert Xu; +Cc: davem, netdev
In-Reply-To: <20060628044209.GA5673@gondor.apana.org.au>

On Wed, 2006-06-28 at 14:42 +1000, Herbert Xu wrote:
> On Tue, Jun 27, 2006 at 09:37:01PM -0700, Michael Chan wrote:

> > @@ -56,6 +55,9 @@ static inline void TCP_ECN_send(struct s
> >  			if (tp->ecn_flags&TCP_ECN_QUEUE_CWR) {
> >  				tp->ecn_flags &= ~TCP_ECN_QUEUE_CWR;
> >  				skb->h.th->cwr = 1;
> > +				if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4)
> > +					skb_shinfo(skb)->gso_type |=
> > +						SKB_GSO_TCPV4_ECN;
> 
> As a byte-pincher I must suggest that you turn this check into something
> like 
> 
> 				if (skb_shinfo(skb)->gso_type)
> 
> or even
> 
> 				if (skb_shinfo(skb)->gso_size)
> 
Assuming that we'll later have GSO_TCPV6, isn't it better to check for
TCPV4 explicitly now?  Or just change it later when necessary.


^ 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