* [rfc 4/4] igb: expose 82576 bandiwidth allocation
From: Simon Horman @ 2009-11-05 0:58 UTC (permalink / raw)
To: e1000-devel, netdev; +Cc: Arnd Bergmann, Jeff Kirsher
In-Reply-To: <20091105005847.941190065@vergenet.net>
[-- Attachment #1: igb-ba.patch --]
[-- Type: text/plain, Size: 12335 bytes --]
The 82576 has support for bandwidth allocation to VFs.
Contrary to the documentation in the 82576 datasheet v2.41 this
appears to work as follows:
* The ratio supplied is always proportional to 1Gbit/s,
regardless of if the link speed.
* The ratio supplied is an upper-bound on bandwidth available
to the VF, not a minimun guarantee
This patch exposes bandwidth control to userspace through a simple
per-device (PF) sysfs file, bandwidth_allocation.
* The file contains a whitespace delimited list of values, one per VF.
* The first value corresponds to the first VF and so on.
* Valid values are integers from 0 to 1000
* A value of 0 indicates that bandwidth_allocation is disabled.
* Other values indicate the allocated bandwidth, in 1/1000ths of a gigabit/s
e.g. The following for a PF with 4 VFs allocates ~20Mbits/ to VF 1,
~100Mbit/s to VF 2, and leave the other 2 VFs with no allocation.
echo "20 100 0 0" > /sys/class/net/eth3/device/bandwidth_allocation
This interface is intended to allow testing of the hardware feature.
There are ongoing discussions about how to expose this feature
to user-space in a more generic way.
Signed-off-by: Simon Horman <horms@verge.net.au>
Index: net-next-2.6/drivers/net/igb/igb_main.c
===================================================================
--- net-next-2.6.orig/drivers/net/igb/igb_main.c 2009-11-05 04:55:06.000000000 +0900
+++ net-next-2.6/drivers/net/igb/igb_main.c 2009-11-05 05:12:54.000000000 +0900
@@ -47,6 +47,9 @@
#ifdef CONFIG_IGB_DCA
#include <linux/dca.h>
#endif
+#ifdef CONFIG_PCI_IOV
+#include <linux/ctype.h>
+#endif
#include "igb.h"
#define DRV_VERSION "1.3.16-k2"
@@ -152,6 +155,15 @@ static unsigned int max_vfs = 0;
module_param(max_vfs, uint, 0);
MODULE_PARM_DESC(max_vfs, "Maximum number of virtual functions to allocate "
"per physical function");
+
+static ssize_t igb_set_bandwidth_allocation(struct device *,
+ struct device_attribute *,
+ const char *, size_t);
+static ssize_t igb_show_bandwidth_allocation(struct device *,
+ struct device_attribute *,
+ char *);
+DEVICE_ATTR(bandwidth_allocation, S_IRUGO | S_IWUSR,
+ igb_show_bandwidth_allocation, igb_set_bandwidth_allocation);
#endif /* CONFIG_PCI_IOV */
static pci_ers_result_t igb_io_error_detected(struct pci_dev *,
@@ -1754,7 +1766,18 @@ static void __devinit igb_init_vf(struct
}
if (pci_enable_sriov(pdev, vfn))
- goto err;
+ goto err_kfree;
+
+ if (device_create_file(&pdev->dev, &dev_attr_bandwidth_allocation))
+ goto err_sriov;
+
+ adapter->bandwidth_allocation = kcalloc(vfn, sizeof(unsigned int),
+ GFP_KERNEL);
+ if (!adapter->bandwidth_allocation)
+ goto err_file;
+ memset(adapter->bandwidth_allocation, vfn * sizeof(unsigned int), 0);
+
+ spin_lock_init(&adapter->bandwidth_allocation_lock);
dev_info(&pdev->dev, "%d vfs allocated\n", vfn);
for (i = 0; i < vfn; i++) {
@@ -1765,7 +1788,11 @@ static void __devinit igb_init_vf(struct
adapter->vfs_allocated_count = vfn;
return;
-err:
+err_file:
+ device_remove_file(&pdev->dev, &dev_attr_bandwidth_allocation);
+err_sriov:
+ pci_disable_sriov(pdev);
+err_kfree:
kfree(adapter->vf_data);
adapter->vf_data = NULL;
#endif /* CONFIG_PCI_IOV */
@@ -1781,6 +1808,7 @@ err:
static void igb_cleanup_vf(struct igb_adapter * adapter)
{
#ifdef CONFIG_PCI_IOV
+ struct pci_dev *pdev = adapter->pdev;
struct e1000_hw *hw = &adapter->hw;
if (!adapter->vf_data)
@@ -1797,6 +1825,9 @@ static void igb_cleanup_vf(struct igb_ad
wr32(E1000_IOVCTL, E1000_IOVCTL_REUSE_VFQ);
msleep(100);
dev_info(&adapter->pdev->dev, "IOV Disabled\n");
+
+ device_remove_file(&pdev->dev, &dev_attr_bandwidth_allocation);
+ kfree(adapter->bandwidth_allocation);
#endif
}
@@ -2088,6 +2119,123 @@ void igb_configure_tx_ring(struct igb_ad
wr32(E1000_TXDCTL(reg_idx), txdctl);
}
+#ifdef CONFIG_PCI_IOV
+static void igb_disable_bandwidth_allocation_vf(struct e1000_hw *hw, int vf)
+{
+ wr32(E1000_VMBASEL, vf);
+ wr32(E1000_VMBAC, 0);
+}
+
+static void igb_disable_bandwidth_allocation(struct igb_adapter *adapter)
+{
+ struct e1000_hw *hw = &adapter->hw;
+ int i;
+
+ for (i = 0; i < adapter->vfs_allocated_count; i++)
+ igb_disable_bandwidth_allocation_vf(hw, i);
+}
+
+static void igb_enable_bandwidth_allocation_vf(struct e1000_hw *hw, int vf,
+ unsigned int allocation)
+{
+ u32 rq;
+
+ /* Allocation is expressed as 1000ths of link speed [+]
+ *
+ * rq is calcualted as 1 / (allocation / 1000) = 1000 / allocation
+ *
+ * E1000_VMBAC_RF_INT_SHIFT and E1000_VMBAC_RF_MASK are used
+ * to marshal the result into the desired format: 23 bits of
+ * which 14 are to the right of the decimal point.
+ *
+ * [+] According to the the 82576 v2.41 datasheet rq should
+ * be a ratio of the link speed, however, empirically
+ * it appears to always be a ration of to 1Gbit/s,
+ * even when the link is 100Mbit/s.
+ */
+ rq = ((1000 << E1000_VMBAC_RF_INT_SHIFT) / allocation) &
+ E1000_VMBAC_RF_MASK;
+
+ wr32(E1000_VMBASEL, vf);
+ wr32(E1000_VMBAC, rq|E1000_VMBAC_RC_ENA);
+}
+
+static void igb_enable_bandwidth_allocation(struct igb_adapter *adapter)
+{
+ u32 i, reg;
+ struct e1000_hw *hw = &adapter->hw;
+
+ /* Only enable bandwidth_allocation if it has been set
+ * and the link speed is 100Mbit/s or 1Gbit/s */
+ if (!adapter->bandwidth_allocation ||
+ (adapter->link_speed != SPEED_100 &&
+ adapter->link_speed != SPEED_1000)) {
+ igb_disable_bandwidth_allocation(adapter);
+ return;
+ }
+
+ for (i = 0; i < adapter->vfs_allocated_count; i++) {
+ wr32(E1000_VMBASEL, i);
+ if (adapter->bandwidth_allocation[i])
+ igb_enable_bandwidth_allocation_vf(hw, i,
+ adapter->bandwidth_allocation[i]);
+ else
+ igb_disable_bandwidth_allocation_vf(hw, i);
+
+ /* XXX:
+ *
+ * The 82576 datasheet, section 4.5.11.1.5.1 "Configuring Tx
+ * Bandwidth to VMs" states that the desired setting is:
+ * VMBAMMW.MMW_SIZE = 16 * MSS
+ *
+ * But isn't MSS a property of skbs that are using tso
+ * rather than adapters?
+ *
+ * If so, should we use the maximum value here? */
+ /* XXX: Should this go inside or outside the for loop ? */
+ reg = 64 * 16;
+ wr32(E1000_VMBAMMW, reg);
+ }
+}
+#endif
+
+static void igb_check_bandwidth_allocation(struct igb_adapter *adapter)
+{
+#ifdef CONFIG_PCI_IOV
+ u32 vmbacs;
+ struct e1000_hw *hw = &adapter->hw;
+
+ if (!adapter->vf_data)
+ return;
+
+ /* The 82576 datasheet, section 4.5.11.1.5.2 "Link Speed Change
+ * Procedure" describes the sequence below. However the
+ * SPEED_CHG never seems to be set.
+ */
+ vmbacs = rd32(E1000_VMBACS);
+ if (vmbacs & E1000_VMBACS_SPEED_CHG) {
+ /* XXX: Never seem to get here */
+ int err = 0;
+
+ if (vmbacs & E1000_VMBACS_VMBA_SET) {
+ igb_disable_bandwidth_allocation(adapter);
+ err = 1;
+ }
+
+ vmbacs &= ~E1000_VMBACS_SPEED_CHG;
+ wr32(E1000_VMBACS, vmbacs);
+
+ if (err)
+ return;
+ }
+
+ spin_lock(&adapter->bandwidth_allocation_lock);
+ igb_enable_bandwidth_allocation(adapter);
+ spin_unlock(&adapter->bandwidth_allocation_lock);
+#endif
+ return;
+}
+
/**
* igb_configure_tx - Configure transmit Unit after Reset
* @adapter: board private structure
@@ -2969,6 +3117,8 @@ static void igb_watchdog_task(struct wor
break;
}
+ igb_check_bandwidth_allocation(adapter);
+
netif_carrier_on(netdev);
igb_ping_all_vfs(adapter);
@@ -5854,4 +6004,101 @@ static void igb_vmm_control(struct igb_a
}
}
+#ifdef CONFIG_PCI_IOV
+static ssize_t igb_show_bandwidth_allocation(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct net_device *netdev = dev_get_drvdata(dev);
+ struct igb_adapter *adapter = netdev_priv(netdev);
+ int i;
+
+ if (!adapter->vf_data)
+ return -ENOENT;
+
+ *buf = '\0';
+ for (i = 0; i < adapter->vfs_allocated_count; i++) {
+ if (i > 0)
+ strcat(buf, " ");
+ sprintf(buf + strlen(buf), "%i",
+ adapter->bandwidth_allocation[i]);
+ }
+ strcat(buf, "\n");
+
+ return strlen(buf);
+}
+
+static unsigned long igb_strtoul(const char *cp, char **endp, unsigned int base)
+{
+ const char *orig = cp;
+ unsigned long x;
+
+ while (isspace(*cp))
+ cp++;
+
+ x = simple_strtoul(cp, endp, base);
+ if (cp == *endp)
+ *endp = (char *)orig;
+
+ return x;
+}
+
+static ssize_t igb_set_bandwidth_allocation(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct net_device *netdev = dev_get_drvdata(dev);
+ struct igb_adapter *adapter = netdev_priv(netdev);
+ int i;
+ size_t len;
+ ssize_t status = -ENOENT;
+ unsigned int *new, total;
+ unsigned long x;
+ const char *p;
+ char *next_p;
+
+ if (!adapter->vf_data)
+ return -ENOENT;
+
+ len = adapter->vfs_allocated_count * sizeof(unsigned int);
+
+ new = kmalloc(len, GFP_KERNEL);
+ if (!new)
+ return -ENOMEM;
+
+ p = buf;
+ total = 0;
+ for (i = 0; i < adapter->vfs_allocated_count; i++) {
+ x = igb_strtoul(p, &next_p, 10);
+ if (p == next_p) {
+ dev_err(dev, "not enough values\n");
+ goto err;
+ }
+ if (x > 1000) {
+ dev_err(dev, "value is too large\n");
+ goto err;
+ }
+ new[i] = x;
+ total += x;
+ p = next_p;
+ }
+
+ /* Check for trailing rubbish */
+ igb_strtoul(p, &next_p, 10);
+ if (p != next_p) {
+ dev_err(dev, "trailing rubbish\n");
+ goto err;
+ }
+
+ spin_lock(&adapter->bandwidth_allocation_lock);
+ memcpy(adapter->bandwidth_allocation, new, len);
+ igb_enable_bandwidth_allocation(adapter);
+ spin_unlock(&adapter->bandwidth_allocation_lock);
+
+ status = count;
+err:
+ kfree(new);
+ return status;
+}
+#endif /* CONFIG_PCI_IOV */
/* igb_main.c */
Index: net-next-2.6/drivers/net/igb/e1000_regs.h
===================================================================
--- net-next-2.6.orig/drivers/net/igb/e1000_regs.h 2009-11-05 03:07:08.000000000 +0900
+++ net-next-2.6/drivers/net/igb/e1000_regs.h 2009-11-05 05:01:35.000000000 +0900
@@ -308,6 +308,16 @@
#define E1000_VLVF(_n) (0x05D00 + (4 * (_n))) /* VLAN Virtual Machine
* Filter - RW */
+/* Tx Bandwidth Allocation to VM Registers */
+#define E1000_VMBACS 0x03600 /* VM Bandwidth Allocation
+ * Control & Status - RW */
+#define E1000_VMBAMMW 0x03670 /* VM Bandwidth Allocation
+ * Max Memory Window - RW */
+#define E1000_VMBASEL 0x03604 /* VM Bandwidth Allocation
+ * Select - RW */
+#define E1000_VMBAC 0x03608 /* VM Bandwidth Allocation
+ * Config - RW */
+
#define wr32(reg, value) (writel(value, hw->hw_addr + reg))
#define rd32(reg) (readl(hw->hw_addr + reg))
#define wrfl() ((void)rd32(E1000_STATUS))
Index: net-next-2.6/drivers/net/igb/e1000_defines.h
===================================================================
--- net-next-2.6.orig/drivers/net/igb/e1000_defines.h 2009-11-05 03:07:08.000000000 +0900
+++ net-next-2.6/drivers/net/igb/e1000_defines.h 2009-11-05 05:01:35.000000000 +0900
@@ -711,4 +711,13 @@
#define E1000_VFTA_ENTRY_MASK 0x7F
#define E1000_VFTA_ENTRY_BIT_SHIFT_MASK 0x1F
+/* VM Bandwidth Allocation Control & Status */
+#define E1000_VMBACS_VMBA_SET 0x00001000
+#define E1000_VMBACS_SPEED_CHG 0x80000000
+
+/* VM Bandwidth Allocation Config */
+#define E1000_VMBAC_RF_INT_SHIFT 14
+#define E1000_VMBAC_RF_MASK ((1<<23)-1) /* RF_DEC and RF_INT */
+#define E1000_VMBAC_RC_ENA 0x80000000
+
#endif
Index: net-next-2.6/drivers/net/igb/igb.h
===================================================================
--- net-next-2.6.orig/drivers/net/igb/igb.h 2009-11-05 03:07:08.000000000 +0900
+++ net-next-2.6/drivers/net/igb/igb.h 2009-11-05 05:01:35.000000000 +0900
@@ -315,6 +315,10 @@ struct igb_adapter {
u16 rx_ring_count;
unsigned int vfs_allocated_count;
struct vf_data_storage *vf_data;
+#ifdef CONFIG_PCI_IOV
+ unsigned int *bandwidth_allocation;
+ spinlock_t bandwidth_allocation_lock;
+#endif
};
#define IGB_FLAG_HAS_MSI (1 << 0)
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
^ permalink raw reply
* [PATCH] usbnet: Set link down initially for drivers that update link state
From: Ben Hutchings @ 2009-11-05 1:29 UTC (permalink / raw)
To: David Miller, David Brownell; +Cc: Greg Kroah-Hartman, Peter Korsgaard, netdev
Some usbnet drivers update link state while others do not due to
hardware limitations. Add a flag to distinguish those that do, and
set the link down initially for their devices.
This is intended to fix this bug: http://bugs.debian.org/444043
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
All drivers that update the link status are supposed to set it down
initially. These drivers will need to be tested to verify that if the
physical link is already up then they update the link status correctly
when the interface is brought up.
Ben.
drivers/net/usb/asix.c | 12 ++++++------
drivers/net/usb/cdc_ether.c | 2 +-
drivers/net/usb/dm9601.c | 2 +-
drivers/net/usb/usbnet.c | 4 +++-
include/linux/usb/usbnet.h | 1 +
5 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/net/usb/asix.c b/drivers/net/usb/asix.c
index 6ce7f77..1bef39a 100644
--- a/drivers/net/usb/asix.c
+++ b/drivers/net/usb/asix.c
@@ -1327,7 +1327,7 @@ static const struct driver_info ax8817x_info = {
.status = asix_status,
.link_reset = ax88172_link_reset,
.reset = ax88172_link_reset,
- .flags = FLAG_ETHER,
+ .flags = FLAG_ETHER | FLAG_LINK_INTR,
.data = 0x00130103,
};
@@ -1337,7 +1337,7 @@ static const struct driver_info dlink_dub_e100_info = {
.status = asix_status,
.link_reset = ax88172_link_reset,
.reset = ax88172_link_reset,
- .flags = FLAG_ETHER,
+ .flags = FLAG_ETHER | FLAG_LINK_INTR,
.data = 0x009f9d9f,
};
@@ -1347,7 +1347,7 @@ static const struct driver_info netgear_fa120_info = {
.status = asix_status,
.link_reset = ax88172_link_reset,
.reset = ax88172_link_reset,
- .flags = FLAG_ETHER,
+ .flags = FLAG_ETHER | FLAG_LINK_INTR,
.data = 0x00130103,
};
@@ -1357,7 +1357,7 @@ static const struct driver_info hawking_uf200_info = {
.status = asix_status,
.link_reset = ax88172_link_reset,
.reset = ax88172_link_reset,
- .flags = FLAG_ETHER,
+ .flags = FLAG_ETHER | FLAG_LINK_INTR,
.data = 0x001f1d1f,
};
@@ -1367,7 +1367,7 @@ static const struct driver_info ax88772_info = {
.status = asix_status,
.link_reset = ax88772_link_reset,
.reset = ax88772_link_reset,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR,
.rx_fixup = asix_rx_fixup,
.tx_fixup = asix_tx_fixup,
};
@@ -1378,7 +1378,7 @@ static const struct driver_info ax88178_info = {
.status = asix_status,
.link_reset = ax88178_link_reset,
.reset = ax88178_link_reset,
- .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR,
.rx_fixup = asix_rx_fixup,
.tx_fixup = asix_tx_fixup,
};
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 71e65fc..dcf6dbd 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -413,7 +413,7 @@ static int cdc_bind(struct usbnet *dev, struct usb_interface *intf)
static const struct driver_info cdc_info = {
.description = "CDC Ethernet Device",
- .flags = FLAG_ETHER,
+ .flags = FLAG_ETHER | FLAG_LINK_INTR,
// .check_connect = cdc_check_connect,
.bind = cdc_bind,
.unbind = usbnet_cdc_unbind,
diff --git a/drivers/net/usb/dm9601.c b/drivers/net/usb/dm9601.c
index a2b30a1..3d406f9 100644
--- a/drivers/net/usb/dm9601.c
+++ b/drivers/net/usb/dm9601.c
@@ -611,7 +611,7 @@ static int dm9601_link_reset(struct usbnet *dev)
static const struct driver_info dm9601_info = {
.description = "Davicom DM9601 USB Ethernet",
- .flags = FLAG_ETHER,
+ .flags = FLAG_ETHER | FLAG_LINK_INTR,
.bind = dm9601_bind,
.rx_fixup = dm9601_rx_fixup,
.tx_fixup = dm9601_tx_fixup,
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 378da8c..04f3f28 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1352,9 +1352,11 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
// ok, it's ready to go.
usb_set_intfdata (udev, dev);
- // start as if the link is up
netif_device_attach (net);
+ if (dev->driver_info->flags & FLAG_LINK_INTR)
+ netif_carrier_off(net);
+
return 0;
out3:
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 86c31b7..8c84881 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -92,6 +92,7 @@ struct driver_info {
#define FLAG_SEND_ZLP 0x0200 /* hw requires ZLPs are sent */
#define FLAG_WWAN 0x0400 /* use "wwan%d" names */
+#define FLAG_LINK_INTR 0x0800 /* updates link (carrier) status */
/* init device ... can sleep, or cause probe() failure */
int (*bind)(struct usbnet *, struct usb_interface *);
--
1.6.5.2
^ permalink raw reply related
* Re: [rfc 0/4] igb: bandwidth allocation
From: Jeff Kirsher @ 2009-11-05 1:46 UTC (permalink / raw)
To: Simon Horman; +Cc: e1000-devel, netdev, Arnd Bergmann
In-Reply-To: <20091105005847.941190065@vergenet.net>
On Wed, Nov 4, 2009 at 16:58, Simon Horman <horms@verge.net.au> wrote:
> Hi,
>
> this series of patches exposes the bandwidth allocation
> hardware support of the Intel 82576. It does so through
> a rather hackish sysfs entry. That interface is just intended
> for testing so that the exposed hardware feature can
> be exercised. I would like to find a generic way to expose
> this feature to user-space.
>
Thanks Simon. I have add the 4 patch series to my tree for testing.
--
Cheers,
Jeff
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
^ permalink raw reply
* Re: [rfc 0/4] igb: bandwidth allocation
From: Simon Horman @ 2009-11-05 2:21 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: e1000-devel, netdev, Alexander Duyck, Arnd Bergmann
In-Reply-To: <9929d2390911041746g2b5f51cdia489bd87d87e41ef@mail.gmail.com>
On Wed, Nov 04, 2009 at 05:46:50PM -0800, Jeff Kirsher wrote:
> On Wed, Nov 4, 2009 at 16:58, Simon Horman <horms@verge.net.au> wrote:
> > Hi,
> >
> > this series of patches exposes the bandwidth allocation
> > hardware support of the Intel 82576. It does so through
> > a rather hackish sysfs entry. That interface is just intended
> > for testing so that the exposed hardware feature can
> > be exercised. I would like to find a generic way to expose
> > this feature to user-space.
> >
>
> Thanks Simon. I have add the 4 patch series to my tree for testing.
Thanks. I wanted to get the code out rather than sitting on it
for lack of a better user-space interface. Although there
is a lot of fluff the actual register twiddling for
bandwidth allocation turned out to be quite simple.
^ permalink raw reply
* Small pktgen bug.
From: Ben Greear @ 2009-11-05 2:22 UTC (permalink / raw)
To: NetDev
There is a subtle bug in pktgen tx_bytes accounting.
If one is using clone_skb, then the cur_pkt_size may be modified
without a new skb actually being created (quite yet) by setting
min_pkt_size through the proc fs.
I think if you just saved pkt_dev->skb->len before transmitting and used
that to increment pkt_dev->tx_bytes that would fix the counter
problem.
if (unlikely(netif_tx_queue_stopped(txq) || netif_tx_queue_frozen(txq)))
ret = NETDEV_TX_BUSY;
else
ret = (*xmit)(pkt_dev->skb, odev);
switch (ret) {
case NETDEV_TX_OK:
txq_trans_update(txq);
pkt_dev->last_ok = 1;
pkt_dev->sofar++;
pkt_dev->seq_num++;
pkt_dev->tx_bytes += pkt_dev->cur_pkt_size;
break;
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: 2.6.32-rc5-mmotm1101 - kernel BUG at net/ipv4/tcp_input.c:3707!
From: Valdis.Kletnieks @ 2009-11-05 2:33 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: Ilpo Järvinen, Eric Dumazet, Andrew Morton, linux-kernel,
netdev, Ori Finkelman
In-Reply-To: <4AF1AF28.2030106@codefidence.com>
[-- Attachment #1: Type: text/plain, Size: 617 bytes --]
On Wed, 04 Nov 2009 18:43:20 +0200, Gilad Ben-Yossef said:
> OK, I thing I've got it.
>
> Kindly try the next patch. It goes on top (in addition to) the previous
> one. This should fix the crash.
OK. much better. Have been up for about 25 minutes now, and fetchmail has
pulled down e-mail several times, and no proboems seen.
> There is still some small cruft in the handling of the per route TCP
> options for IPv6 left, which means that the per route options might get
> ignored for
> incoming IPv6 connections right now. I will fix this if this works.
Yell if you want something tested. ;)
[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]
^ permalink raw reply
* Re: [PATCH 2.6.33/5 00/12] WiMAX pull request
From: Inaky Perez-Gonzalez @ 2009-11-05 4:37 UTC (permalink / raw)
To: netdev; +Cc: wimax
In-Reply-To: <cover.1257370738.git.inaky@linux.intel.com>
On Wed, 2009-11-04 at 13:40 -0800, Inaky Perez-Gonzalez wrote:
> The following changes since commit 4a78fd9a736db4c871bc8b583d66b61c38abd299:
> Inaky Perez-Gonzalez (1):
> wimax/i2400m: fix oops caused by race condition when exiting USB kthreads
Hmm, this is supposed to be batch #5 of the patch series, but somehow my
scripts to break'em up took a life of their own and overwrote the
subject. Sorry for that!
^ permalink raw reply
* Re: virtio-net: new section mismatch warning. revert patch?
From: Rusty Russell @ 2009-11-05 4:40 UTC (permalink / raw)
To: Michael S. Tsirkin, Greg KH, Sam Ravnborg
Cc: Uwe Kleine-König, Sam Ravnborg, David S. Miller,
Alex Williamson, Mark McLoughlin, netdev, linux-kernel
In-Reply-To: <20091104141729.GA27288@redhat.com>
On Thu, 5 Nov 2009 12:47:30 am Michael S. Tsirkin wrote:
> With v2.6.32-rcX I started getting section mismatch warnings for
> virtio_net.
> make with CONFIG_DEBUG_SECTION_MISMATCH=y shows:
>
> WARNING: drivers/net/virtio_net.o(.data+0x90): Section mismatch in
> reference from the variable virtio_net to the function
> .devexit.text:virtnet_remove()
> The variable virtio_net references
> the function __devexit virtnet_remove()
> If the reference is valid then annotate the
> variable with __exit* (see linux/init.h) or name the variable:
> *driver, *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console,
>
> virtnet_remove was converted to devexit by this commit:
>
> commit 3d1285beff2e8467b8c3884d83b7a91a99aa9fcd
> Author: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Date: Wed Sep 30 22:28:34 2009 +0000
>
> move virtnet_remove to .devexit.text
>
> We didn't have these warnings in v2.6.31, so this is a regression.
> revert?
No, just rename "virtio_net" to "virtio_net_driver". Meanwhile, ignore it.
It's worked well for me so far.
Uwe: I apologize for accepting your patches. I will be more careful in
future.
Cheers,
Rusty.
^ permalink raw reply
* [PATCH v2] TI DaVinci EMAC: Add suspend/resume capability
From: Ranjith Lohithakshan @ 2009-11-05 4:41 UTC (permalink / raw)
To: netdev
Cc: davem, davinci-linux-open-source, Ranjith Lohithakshan,
Chaithrika U S
Add suspend/resume capability to TI DaVinci EMAC driver.
Signed-off-by: Ranjith Lohithakshan <ranjithl@ti.com>
Signed-off-by: Chaithrika U S <chaithrika@ti.com>
---
Applies to net-next-2.6
drivers/net/davinci_emac.c | 30 +++++++++++++++++++++++++++---
1 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c
index a876dce..f1b09c0 100644
--- a/drivers/net/davinci_emac.c
+++ b/drivers/net/davinci_emac.c
@@ -2806,11 +2806,33 @@ static int __devexit davinci_emac_remove(struct platform_device *pdev)
return 0;
}
+static
+int davinci_emac_suspend(struct platform_device *pdev, pm_message_t state)
+{
+ struct net_device *dev = platform_get_drvdata(pdev);
+
+ if (netif_running(dev))
+ emac_dev_stop(dev);
+
+ clk_disable(emac_clk);
+
+ return 0;
+}
+
+static int davinci_emac_resume(struct platform_device *pdev)
+{
+ struct net_device *dev = platform_get_drvdata(pdev);
+
+ clk_enable(emac_clk);
+
+ if (netif_running(dev))
+ emac_dev_open(dev);
+
+ return 0;
+}
+
/**
* davinci_emac_driver: EMAC platform driver structure
- *
- * We implement only probe and remove functions - suspend/resume and
- * others not supported by this module
*/
static struct platform_driver davinci_emac_driver = {
.driver = {
@@ -2819,6 +2841,8 @@ static struct platform_driver davinci_emac_driver = {
},
.probe = davinci_emac_probe,
.remove = __devexit_p(davinci_emac_remove),
+ .suspend = davinci_emac_suspend,
+ .resume = davinci_emac_resume,
};
/**
--
1.5.6
^ permalink raw reply related
* RE: [PATCH 1/3] fsl_pq_mdio: Fix compiler/sparse warnings (part 1)
From: Kumar Gopalpet-B05799 @ 2009-11-05 5:01 UTC (permalink / raw)
To: Anton Vorontsov; +Cc: Fleming Andy-AFLEMING, netdev, linuxppc-dev, David Miller
In-Reply-To: <20091104225256.GA29537@oksana.dev.rtsoft.ru>
>-----Original Message-----
>From: Anton Vorontsov [mailto:avorontsov@ru.mvista.com]
>Sent: Thursday, November 05, 2009 4:23 AM
>To: David Miller
>Cc: Fleming Andy-AFLEMING; Kumar Gopalpet-B05799;
>netdev@vger.kernel.org; linuxppc-dev@ozlabs.org
>Subject: [PATCH 1/3] fsl_pq_mdio: Fix compiler/sparse warnings (part 1)
>
>commit 1d2397d742b7a2b39b2f09dd9da3b9d1463f55e9 ("fsl_pq_mdio:
>Add Suport for etsec2.0 devices") introduced the following warnings:
>
> CHECK fsl_pq_mdio.c
>fsl_pq_mdio.c:287:22: warning: incorrect type in initializer
>(different base types)
>fsl_pq_mdio.c:287:22: expected unknown type 11 const *__mptr
>fsl_pq_mdio.c:287:22: got unsigned long long [unsigned]
>[assigned] [usertype] addr
>fsl_pq_mdio.c:287:19: warning: incorrect type in assignment
>(different base types)
>fsl_pq_mdio.c:287:19: expected unsigned long long
>[unsigned] [usertype] ioremap_miimcfg
>fsl_pq_mdio.c:287:19: got struct fsl_pq_mdio *<noident>
> CC fsl_pq_mdio.o
>fsl_pq_mdio.c: In function 'fsl_pq_mdio_probe':
>fsl_pq_mdio.c:287: warning: initialization makes pointer from
>integer without a cast
>fsl_pq_mdio.c:287: warning: assignment makes integer from
>pointer without a cast
>
>These warnings are not easy to fix without ugly __force casts.
>So, instead of introducing the casts, rework the code to
>substitute an offset from an already mapped area. This makes
>the code a lot simpler and less duplicated.
>
>Plus, from now on we don't actually map reserved registers on
>non-etsec2.0 devices, so we have more chances to catch
>programming errors.
>
>Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
>---
> drivers/net/fsl_pq_mdio.c | 31 ++++++++++++-------------------
> 1 files changed, 12 insertions(+), 19 deletions(-)
>
>diff --git a/drivers/net/fsl_pq_mdio.c
>b/drivers/net/fsl_pq_mdio.c index 4065b7c..fb8c8d9 100644
>--- a/drivers/net/fsl_pq_mdio.c
>+++ b/drivers/net/fsl_pq_mdio.c
>@@ -262,10 +262,11 @@ static int fsl_pq_mdio_probe(struct
>of_device *ofdev,
> struct device_node *np = ofdev->node;
> struct device_node *tbi;
> struct fsl_pq_mdio __iomem *regs = NULL;
>+ void __iomem *map;
> u32 __iomem *tbipa;
> struct mii_bus *new_bus;
> int tbiaddr = -1;
>- u64 addr = 0, size = 0, ioremap_miimcfg = 0;
>+ u64 addr = 0, size = 0;
> int err = 0;
>
> new_bus = mdiobus_alloc();
>@@ -279,28 +280,20 @@ static int fsl_pq_mdio_probe(struct
>of_device *ofdev,
> fsl_pq_mdio_bus_name(new_bus->id, np);
>
> /* Set the PHY base address */
>- if (of_device_is_compatible(np,"fsl,gianfar-mdio") ||
>- of_device_is_compatible(np, "fsl,gianfar-tbi") ||
>- of_device_is_compatible(np, "fsl,ucc-mdio") ||
>- of_device_is_compatible(np,"ucc_geth_phy" )) {
>- addr = of_translate_address(np,
>of_get_address(np, 0, &size, NULL));
>- ioremap_miimcfg = container_of(addr, struct
>fsl_pq_mdio, miimcfg);
>- regs = ioremap(ioremap_miimcfg, size +
>- offsetof(struct fsl_pq_mdio, miimcfg));
>- } else if (of_device_is_compatible(np,"fsl,etsec2-mdio") ||
>- of_device_is_compatible(np, "fsl,etsec2-tbi")) {
>- addr = of_translate_address(np,
>of_get_address(np, 0, &size, NULL));
>- regs = ioremap(addr, size);
>- } else {
>- err = -EINVAL;
>- goto err_free_bus;
>- }
>-
>- if (NULL == regs) {
>+ addr = of_translate_address(np, of_get_address(np, 0,
>&size, NULL));
>+ map = ioremap(addr, size);
>+ if (!map) {
> err = -ENOMEM;
> goto err_free_bus;
> }
>
>+ if (of_device_is_compatible(np, "fsl,gianfar-mdio") ||
>+ of_device_is_compatible(np,
>"fsl,gianfar-tbi") ||
>+ of_device_is_compatible(np, "fsl,ucc-mdio") ||
>+ of_device_is_compatible(np, "ucc_geth_phy"))
>+ map -= offsetof(struct fsl_pq_mdio, miimcfg);
>+ regs = map;
>+
> new_bus->priv = (void __force *)regs;
>
> new_bus->irq = kcalloc(PHY_MAX_ADDR, sizeof(int), GFP_KERNEL);
>--
HI Anton, thanks for the changes. I have only one concern, has this code
been tried for ucc_geth ? I remember I had some issues with getting the
ucc_geth mdio also working. I will take in these changes and try at my
end for ucc_geth.
-Thanks
Sandeep
^ permalink raw reply
* [PATCH net-next-2.6] pktgen: tx_bytes might be slightly wrong
From: Eric Dumazet @ 2009-11-05 5:20 UTC (permalink / raw)
To: Ben Greear; +Cc: NetDev
In-Reply-To: <4AF236D1.7020006@candelatech.com>
From: Ben Greear <greearb@candelatech.com>
Ben Greear a écrit :
> There is a subtle bug in pktgen tx_bytes accounting.
>
> If one is using clone_skb, then the cur_pkt_size may be modified
> without a new skb actually being created (quite yet) by setting
> min_pkt_size through the proc fs.
>
> I think if you just saved pkt_dev->skb->len before transmitting and used
> that to increment pkt_dev->tx_bytes that would fix the counter
> problem.
>
> if (unlikely(netif_tx_queue_stopped(txq) ||
> netif_tx_queue_frozen(txq)))
> ret = NETDEV_TX_BUSY;
> else
> ret = (*xmit)(pkt_dev->skb, odev);
>
> switch (ret) {
> case NETDEV_TX_OK:
> txq_trans_update(txq);
> pkt_dev->last_ok = 1;
> pkt_dev->sofar++;
> pkt_dev->seq_num++;
> pkt_dev->tx_bytes += pkt_dev->cur_pkt_size;
> break;
>
>
Hi Ben
Nice catch :)
Note that clone_skb>0 makes the race window bigger, but its also racy for clone_skb=0
Instead of storing pkt_dev->skb->len in a temporary variable,
we could store it in pkt_dev->last_pkt_size, it'll be a bit faster.
[PATCH net-next-2.6] pktgen: tx_bytes might be slightly wrong
cur_pkt_size can be changed in proc fs while pktgen is running,
we better use a private field to get precise tx-bytes counter.
Signed-off-by: Ben Greear <greearb@candelatech.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 5ce017b..d38470a 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -340,6 +340,7 @@ struct pktgen_dev {
__u16 cur_udp_src;
__u16 cur_queue_map;
__u32 cur_pkt_size;
+ __u32 last_pkt_size;
__u8 hh[14];
/* = {
@@ -3434,7 +3435,7 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev)
pkt_dev->clone_count--; /* back out increment, OOM */
return;
}
-
+ pkt_dev->last_pkt_size = pkt_dev->skb->len;
pkt_dev->allocated_skbs++;
pkt_dev->clone_count = 0; /* reset counter */
}
@@ -3461,7 +3462,7 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev)
pkt_dev->last_ok = 1;
pkt_dev->sofar++;
pkt_dev->seq_num++;
- pkt_dev->tx_bytes += pkt_dev->cur_pkt_size;
+ pkt_dev->tx_bytes += pkt_dev->last_pkt_size;
break;
default: /* Drivers are not supposed to return other values! */
if (net_ratelimit())
^ permalink raw reply related
* RE: [PATCH 1/3] fsl_pq_mdio: Fix compiler/sparse warnings (part 1)
From: Kumar Gopalpet-B05799 @ 2009-11-05 5:41 UTC (permalink / raw)
To: Kumar Gopalpet-B05799, Anton Vorontsov
Cc: Fleming Andy-AFLEMING, netdev, linuxppc-dev, David Miller
In-Reply-To: <20091104225256.GA29537@oksana.dev.rtsoft.ru>
>-----Original Message-----
>From: Kumar Gopalpet-B05799
>Sent: Thursday, November 05, 2009 10:31 AM
>To: 'Anton Vorontsov'
>Cc: Fleming Andy-AFLEMING; netdev@vger.kernel.org;
>linuxppc-dev@ozlabs.org; David Miller
>Subject: RE: [PATCH 1/3] fsl_pq_mdio: Fix compiler/sparse
>warnings (part 1)
>
>
>
>>-----Original Message-----
>>From: Anton Vorontsov [mailto:avorontsov@ru.mvista.com]
>>Sent: Thursday, November 05, 2009 4:23 AM
>>To: David Miller
>>Cc: Fleming Andy-AFLEMING; Kumar Gopalpet-B05799;
>>netdev@vger.kernel.org; linuxppc-dev@ozlabs.org
>>Subject: [PATCH 1/3] fsl_pq_mdio: Fix compiler/sparse
>warnings (part 1)
>>
>>commit 1d2397d742b7a2b39b2f09dd9da3b9d1463f55e9 ("fsl_pq_mdio:
>>Add Suport for etsec2.0 devices") introduced the following warnings:
>>
>> CHECK fsl_pq_mdio.c
>>fsl_pq_mdio.c:287:22: warning: incorrect type in initializer
>(different
>>base types)
>>fsl_pq_mdio.c:287:22: expected unknown type 11 const *__mptr
>>fsl_pq_mdio.c:287:22: got unsigned long long [unsigned]
>>[assigned] [usertype] addr
>>fsl_pq_mdio.c:287:19: warning: incorrect type in assignment
>(different
>>base types)
>>fsl_pq_mdio.c:287:19: expected unsigned long long
>>[unsigned] [usertype] ioremap_miimcfg
>>fsl_pq_mdio.c:287:19: got struct fsl_pq_mdio *<noident>
>> CC fsl_pq_mdio.o
>>fsl_pq_mdio.c: In function 'fsl_pq_mdio_probe':
>>fsl_pq_mdio.c:287: warning: initialization makes pointer from integer
>>without a cast
>>fsl_pq_mdio.c:287: warning: assignment makes integer from pointer
>>without a cast
>>
>>These warnings are not easy to fix without ugly __force casts.
>>So, instead of introducing the casts, rework the code to
>substitute an
>>offset from an already mapped area. This makes the code a lot simpler
>>and less duplicated.
>>
>>Plus, from now on we don't actually map reserved registers on
>>non-etsec2.0 devices, so we have more chances to catch programming
>>errors.
>>
>>Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
>>---
>> drivers/net/fsl_pq_mdio.c | 31 ++++++++++++-------------------
>> 1 files changed, 12 insertions(+), 19 deletions(-)
>>
>>diff --git a/drivers/net/fsl_pq_mdio.c b/drivers/net/fsl_pq_mdio.c
>>index 4065b7c..fb8c8d9 100644
>>--- a/drivers/net/fsl_pq_mdio.c
>>+++ b/drivers/net/fsl_pq_mdio.c
>>@@ -262,10 +262,11 @@ static int fsl_pq_mdio_probe(struct of_device
>>*ofdev,
>> struct device_node *np = ofdev->node;
>> struct device_node *tbi;
>> struct fsl_pq_mdio __iomem *regs = NULL;
>>+ void __iomem *map;
>> u32 __iomem *tbipa;
>> struct mii_bus *new_bus;
>> int tbiaddr = -1;
>>- u64 addr = 0, size = 0, ioremap_miimcfg = 0;
>>+ u64 addr = 0, size = 0;
>> int err = 0;
>>
>> new_bus = mdiobus_alloc();
>>@@ -279,28 +280,20 @@ static int fsl_pq_mdio_probe(struct of_device
>>*ofdev,
>> fsl_pq_mdio_bus_name(new_bus->id, np);
>>
>> /* Set the PHY base address */
>>- if (of_device_is_compatible(np,"fsl,gianfar-mdio") ||
>>- of_device_is_compatible(np, "fsl,gianfar-tbi") ||
>>- of_device_is_compatible(np, "fsl,ucc-mdio") ||
>>- of_device_is_compatible(np,"ucc_geth_phy" )) {
>>- addr = of_translate_address(np,
>>of_get_address(np, 0, &size, NULL));
>>- ioremap_miimcfg = container_of(addr, struct
>>fsl_pq_mdio, miimcfg);
>>- regs = ioremap(ioremap_miimcfg, size +
>>- offsetof(struct fsl_pq_mdio, miimcfg));
>>- } else if (of_device_is_compatible(np,"fsl,etsec2-mdio") ||
>>- of_device_is_compatible(np, "fsl,etsec2-tbi")) {
>>- addr = of_translate_address(np,
>>of_get_address(np, 0, &size, NULL));
>>- regs = ioremap(addr, size);
>>- } else {
>>- err = -EINVAL;
>>- goto err_free_bus;
>>- }
>>-
>>- if (NULL == regs) {
>>+ addr = of_translate_address(np, of_get_address(np, 0,
>>&size, NULL));
>>+ map = ioremap(addr, size);
>>+ if (!map) {
>> err = -ENOMEM;
>> goto err_free_bus;
>> }
>>
>>+ if (of_device_is_compatible(np, "fsl,gianfar-mdio") ||
>>+ of_device_is_compatible(np,
>>"fsl,gianfar-tbi") ||
>>+ of_device_is_compatible(np, "fsl,ucc-mdio") ||
>>+ of_device_is_compatible(np, "ucc_geth_phy"))
>>+ map -= offsetof(struct fsl_pq_mdio, miimcfg);
>>+ regs = map;
>>+
>> new_bus->priv = (void __force *)regs;
>>
>> new_bus->irq = kcalloc(PHY_MAX_ADDR, sizeof(int), GFP_KERNEL);
>>--
>
>HI Anton, thanks for the changes. I have only one concern, has
>this code been tried for ucc_geth ? I remember I had some
>issues with getting the ucc_geth mdio also working. I will
>take in these changes and try at my end for ucc_geth.
Sorry, if my earlier statement was confusing. What I meant was with
similar changes as Anton's changes
I had some issues with ucc_geth mdio ( may be Anton's changes don't have
that issue).
--
Thanks
Sandeep
^ permalink raw reply
* Re: [PATCH] sch_htb.c consume the classes's tokens bellow the HTB_CAN_SEND level
From: Changli Gao @ 2009-11-05 5:44 UTC (permalink / raw)
To: Martin Devera; +Cc: Jarek Poplawski, Jamal Hadi Salim, netdev
In-Reply-To: <4AF17066.1050504@cdi.cz>
On Wed, Nov 4, 2009 at 8:15 PM, Martin Devera <martin.devera@cdi.cz> wrote:
> Changli Gao wrote:
>
> Ok, then the reason is in "logic". When you have parent P and children
> A,B each with half of rate and only A is sending, than we want A to use
> its own rate and also B's rate. B's rate is available as excess in P.
> Here I see correct to charge part to A and part to P, while you want
> to charge one P and twice A. Only mbuffer will limit the duplicity.
> And it is not good idea because tokens can go very negative and then
> ittroduce A's stall for some time.
> I see no principial benefit...
>
I get it. After reviewing the code, I find mbuffer is 1 minute. It
means that bandwidth will be unfair between A and B(A/B = 1/3) for at
least 2 minute long. It is unacceptable. I'm wrong, and need to think
it again!
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH v2] TI DaVinci EMAC: Add suspend/resume capability
From: David Miller @ 2009-11-05 6:06 UTC (permalink / raw)
To: ranjithl; +Cc: netdev, davinci-linux-open-source, chaithrika
In-Reply-To: <1257396077-26882-1-git-send-email-ranjithl@ti.com>
From: Ranjith Lohithakshan <ranjithl@ti.com>
Date: Thu, 5 Nov 2009 10:11:17 +0530
> Add suspend/resume capability to TI DaVinci EMAC driver.
>
> Signed-off-by: Ranjith Lohithakshan <ranjithl@ti.com>
> Signed-off-by: Chaithrika U S <chaithrika@ti.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH 2.6.33/1 00/12] WiMAX patches for 2.6.33 (batch #1)
From: David Miller @ 2009-11-05 6:24 UTC (permalink / raw)
To: inaky; +Cc: netdev, wimax
In-Reply-To: <cover.1257370735.git.inaky@linux.intel.com>
From: Inaky Perez-Gonzalez <inaky@linux.intel.com>
Date: Wed, 4 Nov 2009 13:39:36 -0800
> Please pull from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/inaky/wimax.git
Well you screwed up the patch posting and you screwed up the
GIT tree too :-/
This git tree is not based upon a clone of net-next-2.6, instead
it's a mish-mash of net-next-2.6 and some by-hand net-2.6
pulls you've done yourself.
Do NOT do this.
I even saw a random kbuild: change in there as well, which
also shouldn't have been there.
Fix up your tree and make it pull cleanly before submitting this
work again.
Also, when you give ma GIT URL you have to give me the branch to pull
from at the end of the URL, if I just give that URL without the branch
specifier to GIT for the pull it's going to fail.
^ permalink raw reply
* Re: HTB accuracy on 10GbE
From: Ryousei Takano @ 2009-11-05 7:08 UTC (permalink / raw)
To: Eric Dumazet
Cc: Stephen Hemminger, Patrick McHardy, Linux Netdev List,
takano-ryousei
In-Reply-To: <4AF1B3CB.1050008@gmail.com>
Hi Eric,
On Thu, Nov 5, 2009 at 2:03 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Ryousei Takano a écrit :
>> Hi Eric,
>>
>> Thanks for your suggestion.
>>
>> On Wed, Nov 4, 2009 at 8:31 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>> Ryousei Takano a écrit :
>>>
>>>> I tried iperf with 60 seconds samples. I got the almost same result.
>>>>
>>>> Here is the result:
>>>> sender receiver
>>>> 1.000 1.00 1.00
>>>> 2.000 2.01 2.01
>>>> 3.000 3.03 3.02
>>>> 4.000 4.07 4.07
>>>> 5.000 5.05 5.05
>>>> 6.000 6.16 6.16
>>>> 7.000 7.22 7.22
>>>> 8.000 8.15 8.15
>>>> 9.000 9.23 9.23
>>>> 9.900 9.69 9.69
>>>>
>>> One thing to consider is the estimation error in qdisc_l2t(), rate table has only 256 slots
>>>
>>> static inline u32 qdisc_l2t(struct qdisc_rate_table* rtab, unsigned int pktlen)
>>> {
>>> int slot = pktlen + rtab->rate.cell_align + rtab->rate.overhead;
>>> if (slot < 0)
>>> slot = 0;
>>> slot >>= rtab->rate.cell_log;
>>> if (slot > 255)
>>> return (rtab->data[255]*(slot >> 8) + rtab->data[slot & 0xFF]);
>>> return rtab->data[slot];
>>> }
>>>
>>>
>>> Maybe you can try changing class mtu to 40000 instead of 9000, and quantum to 60000 too
>>>
>>> tc class add dev $DEV parent 1: classid 1:1 htb rate ${rate}mbit mtu 40000 quantum 60000
>>>
>>> (because your tcp stack sends large buffers ( ~ 60000 bytes) as your NIC can offload tcp segmentation)
>>>
>>>
>> You are right!
>> I am using TSO. The myri10ge driver is passing 64KB packets to the NIC.
>> I changed the class mtu parameter to 64000 instead of 9000.
>>
>> Here is the result:
>> 1.000 1.00
>> 2.000 2.01
>> 3.000 2.99
>> 4.000 4.01
>> 5.000 5.01
>> 6.000 6.04
>> 7.000 7.06
>> 8.000 8.09
>> 9.000 9.11
>> 9.900 9.64
>>
>> It's not so bad!
>> For more information, I updated the results on my page.
>>
>
>
> In fact, I gave you 40000 because rtab will contain 256 elements from 0 to 65280
>
> If you use 64000, you lose some precision (for small packets for example)
>
I see.
In my experiment, it is not very big problem. I do not send short packets.
I got the almost same result in the both cases "mtu 64000" and "mtu
40000 quantum 60000".
Anyway, setting larger mtu size than the physical mtu does not quiet make sense.
Best regards,
Ryousei
^ permalink raw reply
* Re: HTB accuracy on 10GbE
From: Eric Dumazet @ 2009-11-05 7:10 UTC (permalink / raw)
To: Ryousei Takano
Cc: Stephen Hemminger, Patrick McHardy, Linux Netdev List,
takano-ryousei
In-Reply-To: <b30d1c3b0911042308n616bc360v7b96b6543029f232@mail.gmail.com>
Ryousei Takano a écrit :
> In my experiment, it is not very big problem. I do not send short packets.
> I got the almost same result in the both cases "mtu 64000" and "mtu
> 40000 quantum 60000".
>
> Anyway, setting larger mtu size than the physical mtu does not quiet make sense.
>
tc class mtu is a hint given to stack, about average packet size, ie not
related to physical MTU (because of TSO)
You could use same mtu, but disable tso on device
^ permalink raw reply
* Re: [PATCH testing] Do not call IPv4 specific func in tcp_check_req
From: David Miller @ 2009-11-05 7:22 UTC (permalink / raw)
To: gilad; +Cc: Valdis.Kletnieks, netdev, ori, linux-kernel, akpm
In-Reply-To: <1257352854-19490-1-git-send-email-gilad@codefidence.com>
From: Gilad Ben-Yossef <gilad@codefidence.com>
Date: Wed, 4 Nov 2009 18:40:54 +0200
> Calling IPv4 specific inet_csk_route_req in tcp_check_req
> is a bad idea and crashes machine on IPv6 connections, as reported
> by Valdis Kletnieks
>
> Also, all we are really interested in is the timestamp
> option in the header, so calling tcp_parse_options()
> with the "estab" set to false flag is an overkill as
> it tries to parse half a dozen other TCP options.
>
> We know whether timestamp should be enabled or not
> using data from request_sock.
>
> Signed-off-by: Gilad Ben-Yossef <gilad@codefidence.com>
Applied, thanks.
I assume that your other patch which attempted to fix this isn't
necessary.
^ permalink raw reply
* Re: 2.6.32-rc5-mmotm1101 - kernel BUG at net/ipv4/tcp_input.c:3707!
From: David Miller @ 2009-11-05 7:22 UTC (permalink / raw)
To: gilad
Cc: Valdis.Kletnieks, ilpo.jarvinen, eric.dumazet, akpm, linux-kernel,
netdev, ori
In-Reply-To: <4AF1AF28.2030106@codefidence.com>
From: Gilad Ben-Yossef <gilad@codefidence.com>
Date: Wed, 04 Nov 2009 18:43:20 +0200
> Kindly try the next patch. It goes on top (in addition to) the
> previous one. This should fix the crash.
Ok, now I know that both patches need to go in, I'll apply
the other one too.
Forget about my confusion in my other reply.
^ permalink raw reply
* Re: 2.6.32-rc5-mmotm1101 - kernel BUG at net/ipv4/tcp_input.c:3707!
From: David Miller @ 2009-11-05 7:25 UTC (permalink / raw)
To: Valdis.Kletnieks
Cc: gilad, ilpo.jarvinen, eric.dumazet, akpm, linux-kernel, netdev,
ori
In-Reply-To: <6025.1257388392@turing-police.cc.vt.edu>
From: Valdis.Kletnieks@vt.edu
Date: Wed, 04 Nov 2009 21:33:12 -0500
> On Wed, 04 Nov 2009 18:43:20 +0200, Gilad Ben-Yossef said:
>
>> OK, I thing I've got it.
>>
>> Kindly try the next patch. It goes on top (in addition to) the previous
>> one. This should fix the crash.
>
> OK. much better. Have been up for about 25 minutes now, and fetchmail has
> pulled down e-mail several times, and no proboems seen.
Thank you for testing.
^ permalink raw reply
* [PATCH] virtio_net: rename driver struct to please modpost
From: Uwe Kleine-König @ 2009-11-05 8:17 UTC (permalink / raw)
To: mst
Cc: rusty, Sam Ravnborg, David S. Miller, Alex Williamson,
Mark McLoughlin, netdev
In-Reply-To: <200911051510.54163.rusty@rustcorp.com.au>
Commit
3d1285b (move virtnet_remove to .devexit.text)
introduced the first reference to __devexit in struct virtio_driver
virtio_net which upset modpost ("Section mismatch in reference from the
variable virtio_net to the function .devexit.text:virtnet_remove()").
Fix this by renaming virtio_net to virtio_net_driver.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Reported-by: Michael S. Tsirkin <mst@redhat.com>
Blame-taken-by: Rusty Russell <rusty@rustcorp.com.au>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: David S. Miller <davem@davemloft.net>
Cc: Alex Williamson <alex.williamson@hp.com>
Cc: Mark McLoughlin <markmc@redhat.com>
Cc: netdev@vger.kernel.org
---
Hello,
@Rusty: IMHO there's no need to apologize. The driver was enhanced by
3d1285b and I consider it OK not to notice the (false) section warning.
(But if you think it was you who took my first patch, then you forgot to
add you S-o-b which is a worse trespass :-)
Best regards
Uwe
drivers/net/virtio_net.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 05630f2..b9e002f 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -998,7 +998,7 @@ static unsigned int features[] = {
VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN,
};
-static struct virtio_driver virtio_net = {
+static struct virtio_driver virtio_net_driver = {
.feature_table = features,
.feature_table_size = ARRAY_SIZE(features),
.driver.name = KBUILD_MODNAME,
@@ -1011,12 +1011,12 @@ static struct virtio_driver virtio_net = {
static int __init init(void)
{
- return register_virtio_driver(&virtio_net);
+ return register_virtio_driver(&virtio_net_driver);
}
static void __exit fini(void)
{
- unregister_virtio_driver(&virtio_net);
+ unregister_virtio_driver(&virtio_net_driver);
}
module_init(init);
module_exit(fini);
--
1.6.5.2
^ permalink raw reply related
* Re: test
From: Julian Anastasov @ 2009-11-05 9:26 UTC (permalink / raw)
To: Simon Kirby; +Cc: netdev, Wensong Zhang
In-Reply-To: <20091104202716.GE14821@hostway.ca>
Hello,
On Wed, 4 Nov 2009, Simon Kirby wrote:
> Hello!
>
> I was noticing a significant amount of what seems/seemed to be
> destination lists with multiple entries with the lblcr LVS algorithm.
> While tracking it down, I think I stumbled over a mistake. In
> ip_vs_lblcr_full_check(), it appears the time check logic is reversed:
>
> for (i=0, j=tbl->rover; i<IP_VS_LBLCR_TAB_SIZE; i++) {
> j = (j + 1) & IP_VS_LBLCR_TAB_MASK;
>
> write_lock(&svc->sched_lock);
> list_for_each_entry_safe(en, nxt, &tbl->bucket[j], list) {
If 'time to expire' is after current time then continue,
i.e. current time didn't reached the limit, seems correct,
no need to patch. For better reading and to match
ip_vs_lblcr_check_expire() it can be converted to:
if (time_before(now, en->lastuse+sysctl_ip_vs_lblcr_expiration))
continue;
> if (time_after(en->lastuse+sysctl_ip_vs_lblcr_expiration,
> now))
> continue;
>
> ip_vs_lblcr_free(en);
> atomic_dec(&tbl->entries);
> }
> write_unlock(&svc->sched_lock);
> }
>
> Shouldn't this be "time_before"? It seems that it currently nukes all
> recently-used entries every time this function is called, which seems to
> be every 30 minutes, rather than removing the not-recently-used ones.
>
> If my reading is correct, this patch should fix it. Am I missing
> something?
include/linux/jiffies.h:
time_after(a,b) returns true if the time a is after time b.
#define time_before(a,b) time_after(b,a)
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [PATCH] virtio_net: rename driver struct to please modpost
From: David Miller @ 2009-11-05 9:32 UTC (permalink / raw)
To: u.kleine-koenig; +Cc: mst, rusty, sam, alex.williamson, markmc, netdev
In-Reply-To: <1257409037-29916-1-git-send-email-u.kleine-koenig@pengutronix.de>
From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Date: Thu, 5 Nov 2009 09:17:17 +0100
> Commit
>
> 3d1285b (move virtnet_remove to .devexit.text)
>
> introduced the first reference to __devexit in struct virtio_driver
> virtio_net which upset modpost ("Section mismatch in reference from the
> variable virtio_net to the function .devexit.text:virtnet_remove()").
>
> Fix this by renaming virtio_net to virtio_net_driver.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Reported-by: Michael S. Tsirkin <mst@redhat.com>
> Blame-taken-by: Rusty Russell <rusty@rustcorp.com.au>
Applied, thanks.
^ permalink raw reply
* [PATCH net-next-2.6] mac80211: Speedup ieee80211_remove_interfaces()
From: Eric Dumazet @ 2009-11-05 10:06 UTC (permalink / raw)
To: David S. Miller; +Cc: Linux Netdev List, John W. Linville
Speedup ieee80211_remove_interfaces() by factorizing synchronize_rcu() calls
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/mac80211/iface.c | 14 +++++---------
1 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 14f10eb..a445f50 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -852,22 +852,18 @@ void ieee80211_if_remove(struct ieee80211_sub_if_data *sdata)
void ieee80211_remove_interfaces(struct ieee80211_local *local)
{
struct ieee80211_sub_if_data *sdata, *tmp;
+ LIST_HEAD(unreg_list);
ASSERT_RTNL();
+ mutex_lock(&local->iflist_mtx);
list_for_each_entry_safe(sdata, tmp, &local->interfaces, list) {
- /*
- * we cannot hold the iflist_mtx across unregister_netdevice,
- * but we only need to hold it for list modifications to lock
- * out readers since we're under the RTNL here as all other
- * writers.
- */
- mutex_lock(&local->iflist_mtx);
list_del(&sdata->list);
- mutex_unlock(&local->iflist_mtx);
- unregister_netdevice(sdata->dev);
+ unregister_netdevice_queue(sdata->dev, &unreg_list);
}
+ mutex_unlock(&local->iflist_mtx);
+ unregister_netdevice_many(&unreg_list);
}
static u32 ieee80211_idle_off(struct ieee80211_local *local,
^ permalink raw reply related
* Re: test
From: Simon Kirby @ 2009-11-05 10:11 UTC (permalink / raw)
To: Julian Anastasov; +Cc: netdev, lvs-devel
In-Reply-To: <Pine.LNX.4.58.0911051119190.2761@u.domain.uli>
On Thu, Nov 05, 2009 at 11:26:27AM +0200, Julian Anastasov wrote:
> If 'time to expire' is after current time then continue,
> i.e. current time didn't reached the limit, seems correct,
> no need to patch. For better reading and to match
> ip_vs_lblcr_check_expire() it can be converted to:
>
> if (time_before(now, en->lastuse+sysctl_ip_vs_lblcr_expiration))
> continue;
D'oh. I noticed the use of time_before() further down in
ip_vs_lblcr_check_expire(), but not the reversed arguments, hence my
confusion.
I still suspect there may be something not quite right, or which could
perhaps do with some tuning. It's difficult to see exactly how it's
working internally, since there's currently nothing to get a summary of
the dest_sets to userspace. I'll follow up if I find anything.
Simon-
^ 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