* [net-next 5/8] ixgbe: add debugfs support
From: Jeff Kirsher @ 2012-09-17 4:15 UTC (permalink / raw)
To: davem; +Cc: Catherine Sullivan, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1347855342-6610-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Catherine Sullivan <catherine.sullivan@intel.com>
This patch adds debugfs support to the ixgbe driver to give
users the ability to access kernel information and to
simulate kernel events.
The filesystem is set up in the following driver/PCI-instance
hierarchy:
<debugfs>
|-- ixgbe
|-- PCI instance
| |-- attribute files
Signed-off-by: Catherine Sullivan <catherine.sullivan@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/Makefile | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 10 ++-
drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c | 79 ++++++++++++++++++++++++
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 17 +++++
4 files changed, 106 insertions(+), 2 deletions(-)
create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
diff --git a/drivers/net/ethernet/intel/ixgbe/Makefile b/drivers/net/ethernet/intel/ixgbe/Makefile
index 5fd5d04..89f40e5 100644
--- a/drivers/net/ethernet/intel/ixgbe/Makefile
+++ b/drivers/net/ethernet/intel/ixgbe/Makefile
@@ -32,7 +32,7 @@
obj-$(CONFIG_IXGBE) += ixgbe.o
-ixgbe-objs := ixgbe_main.o ixgbe_common.o ixgbe_ethtool.o \
+ixgbe-objs := ixgbe_main.o ixgbe_common.o ixgbe_ethtool.o ixgbe_debugfs.o\
ixgbe_82599.o ixgbe_82598.o ixgbe_phy.o ixgbe_sriov.o \
ixgbe_mbx.o ixgbe_x540.o ixgbe_lib.o
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index bffcf1f..5bd2676 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -597,6 +597,9 @@ struct ixgbe_adapter {
#ifdef CONFIG_IXGBE_HWMON
struct hwmon_buff ixgbe_hwmon_buff;
#endif /* CONFIG_IXGBE_HWMON */
+#ifdef CONFIG_DEBUG_FS
+ struct dentry *ixgbe_dbg_adapter;
+#endif /*CONFIG_DEBUG_FS*/
};
struct ixgbe_fdir_filter {
@@ -725,7 +728,12 @@ extern int ixgbe_fcoe_get_hbainfo(struct net_device *netdev,
struct netdev_fcoe_hbainfo *info);
extern u8 ixgbe_fcoe_get_tc(struct ixgbe_adapter *adapter);
#endif /* IXGBE_FCOE */
-
+#ifdef CONFIG_DEBUG_FS
+extern void ixgbe_dbg_adapter_init(struct ixgbe_adapter *adapter);
+extern void ixgbe_dbg_adapter_exit(struct ixgbe_adapter *adapter);
+extern void ixgbe_dbg_init(void);
+extern void ixgbe_dbg_exit(void);
+#endif /* CONFIG_DEBUG_FS */
static inline struct netdev_queue *txring_txq(const struct ixgbe_ring *ring)
{
return netdev_get_tx_queue(ring->netdev, ring->queue_index);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
new file mode 100644
index 0000000..0b08b6c
--- /dev/null
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
@@ -0,0 +1,79 @@
+/*******************************************************************************
+
+ Intel 10 Gigabit PCI Express Linux driver
+ Copyright(c) 1999 - 2012 Intel Corporation.
+
+ This program is free software; you can redistribute it and/or modify it
+ under the terms and conditions of the GNU General Public License,
+ version 2, as published by the Free Software Foundation.
+
+ This program is distributed in the hope it will be useful, but WITHOUT
+ ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ more details.
+
+ You should have received a copy of the GNU General Public License along with
+ this program; if not, write to the Free Software Foundation, Inc.,
+ 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+
+ The full GNU General Public License is included in this distribution in
+ the file called "COPYING".
+
+ Contact Information:
+ e1000-devel Mailing List <e1000-devel@lists.sourceforge.net>
+ Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
+
+*******************************************************************************/
+
+#ifdef CONFIG_DEBUG_FS
+
+#include <linux/debugfs.h>
+#include <linux/module.h>
+
+#include "ixgbe.h"
+
+static struct dentry *ixgbe_dbg_root;
+
+/**
+ * ixgbe_dbg_adapter_init - setup the debugfs directory for the adapter
+ * @adapter: the adapter that is starting up
+ **/
+void ixgbe_dbg_adapter_init(struct ixgbe_adapter *adapter)
+{
+ const char *name = pci_name(adapter->pdev);
+
+ adapter->ixgbe_dbg_adapter = debugfs_create_dir(name, ixgbe_dbg_root);
+ if (!adapter->ixgbe_dbg_adapter)
+ e_dev_err("debugfs entry for %s failed\n", name);
+}
+
+/**
+ * ixgbe_dbg_adapter_exit - clear out the adapter's debugfs entries
+ * @pf: the pf that is stopping
+ **/
+void ixgbe_dbg_adapter_exit(struct ixgbe_adapter *adapter)
+{
+ if (adapter->ixgbe_dbg_adapter)
+ debugfs_remove_recursive(adapter->ixgbe_dbg_adapter);
+ adapter->ixgbe_dbg_adapter = NULL;
+}
+
+/**
+ * ixgbe_dbg_init - start up debugfs for the driver
+ **/
+void ixgbe_dbg_init(void)
+{
+ ixgbe_dbg_root = debugfs_create_dir(ixgbe_driver_name, NULL);
+ if (ixgbe_dbg_root == NULL)
+ pr_err("init of debugfs failed\n");
+}
+
+/**
+ * ixgbe_dbg_exit - clean out the driver's debugfs entries
+ **/
+void ixgbe_dbg_exit(void)
+{
+ debugfs_remove_recursive(ixgbe_dbg_root);
+}
+
+#endif /* CONFIG_DEBUG_FS */
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index e641f14..b3b846b 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7448,6 +7448,10 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
e_err(probe, "failed to allocate sysfs resources\n");
#endif /* CONFIG_IXGBE_HWMON */
+#ifdef CONFIG_DEBUG_FS
+ ixgbe_dbg_adapter_init(adapter);
+#endif /* CONFIG_DEBUG_FS */
+
return 0;
err_register:
@@ -7482,6 +7486,10 @@ static void __devexit ixgbe_remove(struct pci_dev *pdev)
struct ixgbe_adapter *adapter = pci_get_drvdata(pdev);
struct net_device *netdev = adapter->netdev;
+#ifdef CONFIG_DEBUG_FS
+ ixgbe_dbg_adapter_exit(adapter);
+#endif /*CONFIG_DEBUG_FS */
+
set_bit(__IXGBE_DOWN, &adapter->state);
cancel_work_sync(&adapter->service_task);
@@ -7737,6 +7745,10 @@ static int __init ixgbe_init_module(void)
pr_info("%s - version %s\n", ixgbe_driver_string, ixgbe_driver_version);
pr_info("%s\n", ixgbe_copyright);
+#ifdef CONFIG_DEBUG_FS
+ ixgbe_dbg_init();
+#endif /* CONFIG_DEBUG_FS */
+
#ifdef CONFIG_IXGBE_DCA
dca_register_notify(&dca_notifier);
#endif
@@ -7759,6 +7771,11 @@ static void __exit ixgbe_exit_module(void)
dca_unregister_notify(&dca_notifier);
#endif
pci_unregister_driver(&ixgbe_driver);
+
+#ifdef CONFIG_DEBUG_FS
+ ixgbe_dbg_exit();
+#endif /* CONFIG_DEBUG_FS */
+
rcu_barrier(); /* Wait for completion of call_rcu()'s */
}
--
1.7.11.4
^ permalink raw reply related
* [net-next 7/8] ixgbe: added reg_ops file to debugfs
From: Jeff Kirsher @ 2012-09-17 4:15 UTC (permalink / raw)
To: davem; +Cc: Catherine Sullivan, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1347855342-6610-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Catherine Sullivan <catherine.sullivan@intel.com>
Added the reg_ops file to debugfs with commands to read and write
a register to give users the ability to read and write individual
registers on the fly.
Signed-off-by: Catherine Sullivan <catherine.sullivan@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c | 118 +++++++++++++++++++++++
1 file changed, 118 insertions(+)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
index 2dd169e..8d3a218 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
@@ -34,6 +34,119 @@
static struct dentry *ixgbe_dbg_root;
+static char ixgbe_dbg_reg_ops_buf[256] = "";
+
+/**
+ * ixgbe_dbg_reg_ops_open - prep the debugfs pokee data item when opened
+ * @inode: inode that was opened
+ * @filp: file info
+ *
+ * Stash the adapter pointer hiding in the inode into the file pointer where
+ * we can find it later in the read and write calls
+ **/
+static int ixgbe_dbg_reg_ops_open(struct inode *inode, struct file *filp)
+{
+ filp->private_data = inode->i_private;
+ return 0;
+}
+
+/**
+ * ixgbe_dbg_reg_ops_read - read for reg_ops datum
+ * @filp: the opened file
+ * @buffer: where to write the data for the user to read
+ * @count: the size of the user's buffer
+ * @ppos: file position offset
+ **/
+static ssize_t ixgbe_dbg_reg_ops_read(struct file *filp, char __user *buffer,
+ size_t count, loff_t *ppos)
+{
+ struct ixgbe_adapter *adapter = filp->private_data;
+ char buf[256];
+ int bytes_not_copied;
+ int len;
+
+ /* don't allow partial reads */
+ if (*ppos != 0)
+ return 0;
+
+ len = snprintf(buf, sizeof(buf), "%s: %s\n",
+ adapter->netdev->name, ixgbe_dbg_reg_ops_buf);
+ if (count < len)
+ return -ENOSPC;
+ bytes_not_copied = copy_to_user(buffer, buf, len);
+ if (bytes_not_copied < 0)
+ return bytes_not_copied;
+
+ *ppos = len;
+ return len;
+}
+
+/**
+ * ixgbe_dbg_reg_ops_write - write into reg_ops datum
+ * @filp: the opened file
+ * @buffer: where to find the user's data
+ * @count: the length of the user's data
+ * @ppos: file position offset
+ **/
+static ssize_t ixgbe_dbg_reg_ops_write(struct file *filp,
+ const char __user *buffer,
+ size_t count, loff_t *ppos)
+{
+ struct ixgbe_adapter *adapter = filp->private_data;
+ int bytes_not_copied;
+
+ /* don't allow partial writes */
+ if (*ppos != 0)
+ return 0;
+ if (count >= sizeof(ixgbe_dbg_reg_ops_buf))
+ return -ENOSPC;
+
+ bytes_not_copied = copy_from_user(ixgbe_dbg_reg_ops_buf, buffer, count);
+ if (bytes_not_copied < 0)
+ return bytes_not_copied;
+ else if (bytes_not_copied < count)
+ count -= bytes_not_copied;
+ else
+ return -ENOSPC;
+ ixgbe_dbg_reg_ops_buf[count] = '\0';
+
+ if (strncmp(ixgbe_dbg_reg_ops_buf, "write", 5) == 0) {
+ u32 reg, value;
+ int cnt;
+ cnt = sscanf(&ixgbe_dbg_reg_ops_buf[5], "%x %x", ®, &value);
+ if (cnt == 2) {
+ IXGBE_WRITE_REG(&adapter->hw, reg, value);
+ value = IXGBE_READ_REG(&adapter->hw, reg);
+ e_dev_info("write: 0x%08x = 0x%08x\n", reg, value);
+ } else {
+ e_dev_info("write <reg> <value>\n");
+ }
+ } else if (strncmp(ixgbe_dbg_reg_ops_buf, "read", 4) == 0) {
+ u32 reg, value;
+ int cnt;
+ cnt = sscanf(&ixgbe_dbg_reg_ops_buf[4], "%x", ®);
+ if (cnt == 1) {
+ value = IXGBE_READ_REG(&adapter->hw, reg);
+ e_dev_info("read 0x%08x = 0x%08x\n", reg, value);
+ } else {
+ e_dev_info("read <reg>\n");
+ }
+ } else {
+ e_dev_info("Unknown command %s\n", ixgbe_dbg_reg_ops_buf);
+ e_dev_info("Available commands:\n");
+ e_dev_info(" read <reg>\n");
+ e_dev_info(" write <reg> <value>\n");
+ }
+ return count;
+}
+
+static const struct file_operations ixgbe_dbg_reg_ops_fops = {
+ .owner = THIS_MODULE,
+ .open = ixgbe_dbg_reg_ops_open,
+ .read = ixgbe_dbg_reg_ops_read,
+ .write = ixgbe_dbg_reg_ops_write,
+};
+
static char ixgbe_dbg_netdev_ops_buf[256] = "";
/**
@@ -140,6 +253,11 @@ void ixgbe_dbg_adapter_init(struct ixgbe_adapter *adapter)
struct dentry *pfile;
adapter->ixgbe_dbg_adapter = debugfs_create_dir(name, ixgbe_dbg_root);
if (adapter->ixgbe_dbg_adapter) {
+ pfile = debugfs_create_file("reg_ops", 0600,
+ adapter->ixgbe_dbg_adapter, adapter,
+ &ixgbe_dbg_reg_ops_fops);
+ if (!pfile)
+ e_dev_err("debugfs reg_ops for %s failed\n", name);
pfile = debugfs_create_file("netdev_ops", 0600,
adapter->ixgbe_dbg_adapter, adapter,
&ixgbe_dbg_netdev_ops_fops);
--
1.7.11.4
^ permalink raw reply related
* [net-next 6/8] ixgbe: added netdev_ops file to debugfs
From: Jeff Kirsher @ 2012-09-17 4:15 UTC (permalink / raw)
To: davem; +Cc: Catherine Sullivan, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1347855342-6610-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Catherine Sullivan <catherine.sullivan@intel.com>
Added the netdev_ops file to debugfs with a command to call the
ndo_tx_timeout function to give users the ability to simulate a
tx_timeout call made by the kernel.
Signed-off-by: Catherine Sullivan <catherine.sullivan@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c | 107 ++++++++++++++++++++++-
1 file changed, 105 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
index 0b08b6c..2dd169e 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
@@ -34,6 +34,102 @@
static struct dentry *ixgbe_dbg_root;
+static char ixgbe_dbg_netdev_ops_buf[256] = "";
+
+/**
+ * ixgbe_dbg_netdev_ops_open - prep the debugfs netdev_ops data item
+ * @inode: inode that was opened
+ * @filp: file info
+ *
+ * Stash the adapter pointer hiding in the inode into the file pointer
+ * where we can find it later in the read and write calls
+ **/
+static int ixgbe_dbg_netdev_ops_open(struct inode *inode, struct file *filp)
+{
+ filp->private_data = inode->i_private;
+ return 0;
+}
+
+/**
+ * ixgbe_dbg_netdev_ops_read - read for netdev_ops datum
+ * @filp: the opened file
+ * @buffer: where to write the data for the user to read
+ * @count: the size of the user's buffer
+ * @ppos: file position offset
+ **/
+static ssize_t ixgbe_dbg_netdev_ops_read(struct file *filp,
+ char __user *buffer,
+ size_t count, loff_t *ppos)
+{
+ struct ixgbe_adapter *adapter = filp->private_data;
+ char buf[256];
+ int bytes_not_copied;
+ int len;
+
+ /* don't allow partial reads */
+ if (*ppos != 0)
+ return 0;
+
+ len = snprintf(buf, sizeof(buf), "%s: %s\n",
+ adapter->netdev->name, ixgbe_dbg_netdev_ops_buf);
+ if (count < len)
+ return -ENOSPC;
+ bytes_not_copied = copy_to_user(buffer, buf, len);
+ if (bytes_not_copied < 0)
+ return bytes_not_copied;
+
+ *ppos = len;
+ return len;
+}
+
+/**
+ * ixgbe_dbg_netdev_ops_write - write into netdev_ops datum
+ * @filp: the opened file
+ * @buffer: where to find the user's data
+ * @count: the length of the user's data
+ * @ppos: file position offset
+ **/
+static ssize_t ixgbe_dbg_netdev_ops_write(struct file *filp,
+ const char __user *buffer,
+ size_t count, loff_t *ppos)
+{
+ struct ixgbe_adapter *adapter = filp->private_data;
+ int bytes_not_copied;
+
+ /* don't allow partial writes */
+ if (*ppos != 0)
+ return 0;
+ if (count >= sizeof(ixgbe_dbg_netdev_ops_buf))
+ return -ENOSPC;
+
+ bytes_not_copied = copy_from_user(ixgbe_dbg_netdev_ops_buf,
+ buffer, count);
+ if (bytes_not_copied < 0)
+ return bytes_not_copied;
+ else if (bytes_not_copied < count)
+ count -= bytes_not_copied;
+ else
+ return -ENOSPC;
+ ixgbe_dbg_netdev_ops_buf[count] = '\0';
+
+ if (strncmp(ixgbe_dbg_netdev_ops_buf, "tx_timeout", 10) == 0) {
+ adapter->netdev->netdev_ops->ndo_tx_timeout(adapter->netdev);
+ e_dev_info("tx_timeout called\n");
+ } else {
+ e_dev_info("Unknown command: %s\n", ixgbe_dbg_netdev_ops_buf);
+ e_dev_info("Available commands:\n");
+ e_dev_info(" tx_timeout\n");
+ }
+ return count;
+}
+
+static const struct file_operations ixgbe_dbg_netdev_ops_fops = {
+ .owner = THIS_MODULE,
+ .open = ixgbe_dbg_netdev_ops_open,
+ .read = ixgbe_dbg_netdev_ops_read,
+ .write = ixgbe_dbg_netdev_ops_write,
+};
+
/**
* ixgbe_dbg_adapter_init - setup the debugfs directory for the adapter
* @adapter: the adapter that is starting up
@@ -41,10 +137,17 @@ static struct dentry *ixgbe_dbg_root;
void ixgbe_dbg_adapter_init(struct ixgbe_adapter *adapter)
{
const char *name = pci_name(adapter->pdev);
-
+ struct dentry *pfile;
adapter->ixgbe_dbg_adapter = debugfs_create_dir(name, ixgbe_dbg_root);
- if (!adapter->ixgbe_dbg_adapter)
+ if (adapter->ixgbe_dbg_adapter) {
+ pfile = debugfs_create_file("netdev_ops", 0600,
+ adapter->ixgbe_dbg_adapter, adapter,
+ &ixgbe_dbg_netdev_ops_fops);
+ if (!pfile)
+ e_dev_err("debugfs netdev_ops for %s failed\n", name);
+ } else {
e_dev_err("debugfs entry for %s failed\n", name);
+ }
}
/**
--
1.7.11.4
^ permalink raw reply related
* [net-next 8/8] ixgbe: Improve statistics accuracy for DDP traffic
From: Jeff Kirsher @ 2012-09-17 4:15 UTC (permalink / raw)
To: davem; +Cc: Mark Rustad, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1347855342-6610-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Mark Rustad <mark.d.rustad@intel.com>
Noticed that the byte and packet count statistics are under-
counting traffic handled by the DDP offload when there is more
than one DDP completion processed in a single call to
ixgbe_clean_rx_irq. This patch fixes that.
I tried to optimize the setting of the rss value so that it
only would have to be computed once, and only when there is
a DDP completion present.
Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 32 +++++++++++++--------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index b3b846b..2dc9d91 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1785,7 +1785,8 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
unsigned int total_rx_bytes = 0, total_rx_packets = 0;
#ifdef IXGBE_FCOE
struct ixgbe_adapter *adapter = q_vector->adapter;
- int ddp_bytes = 0;
+ int ddp_bytes;
+ unsigned int mss = 0;
#endif /* IXGBE_FCOE */
u16 cleaned_count = ixgbe_desc_unused(rx_ring);
@@ -1839,6 +1840,20 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
/* if ddp, not passing to ULD unless for FCP_RSP or error */
if (ixgbe_rx_is_fcoe(rx_ring, rx_desc)) {
ddp_bytes = ixgbe_fcoe_ddp(adapter, rx_desc, skb);
+ /* include DDPed FCoE data */
+ if (ddp_bytes > 0) {
+ if (!mss) {
+ mss = rx_ring->netdev->mtu -
+ sizeof(struct fcoe_hdr) -
+ sizeof(struct fc_frame_header) -
+ sizeof(struct fcoe_crc_eof);
+ if (mss > 512)
+ mss &= ~511;
+ }
+ total_rx_bytes += ddp_bytes;
+ total_rx_packets += DIV_ROUND_UP(ddp_bytes,
+ mss);
+ }
if (!ddp_bytes) {
dev_kfree_skb_any(skb);
continue;
@@ -1852,21 +1867,6 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
budget--;
} while (likely(budget));
-#ifdef IXGBE_FCOE
- /* include DDPed FCoE data */
- if (ddp_bytes > 0) {
- unsigned int mss;
-
- mss = rx_ring->netdev->mtu - sizeof(struct fcoe_hdr) -
- sizeof(struct fc_frame_header) -
- sizeof(struct fcoe_crc_eof);
- if (mss > 512)
- mss &= ~511;
- total_rx_bytes += ddp_bytes;
- total_rx_packets += DIV_ROUND_UP(ddp_bytes, mss);
- }
-
-#endif /* IXGBE_FCOE */
u64_stats_update_begin(&rx_ring->syncp);
rx_ring->stats.packets += total_rx_packets;
rx_ring->stats.bytes += total_rx_bytes;
--
1.7.11.4
^ permalink raw reply related
* Re: [net-next 0/6][pull request] Intel Wired LAN Driver Updates
From: David Miller @ 2012-09-17 4:53 UTC (permalink / raw)
To: jeffrey.t.kirsher
Cc: netdev, gospo, sassmann, matthew.vick, bhutchings, jacob.e.keller,
richardcochran
In-Reply-To: <1347570359.2219.96.camel@jtkirshe-mobl>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Thu, 13 Sep 2012 14:05:59 -0700
> On Wed, 2012-09-05 at 16:35 -0700, Kirsher, Jeffrey T wrote:
>> This series contains updates to igb (specifically PTP code).
>>
>> The following are changes since commit f6fe569fe056388166575af1cfaed0bcbc688305:
>> Revert "usbnet: drop unneeded check for NULL"
>> and are available in the git repository at:
>> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master
>>
>> Matthew Vick (6):
>> igb: Tidy up wrapping for CONFIG_IGB_PTP.
>> igb: Update PTP function names/variables and locations.
>> igb: Correct PTP support query from ethtool.
>> igb: Store the MAC address in the name in the PTP struct.
>> igb: Prevent dropped Tx timestamps via work items and interrupts.
>> igb: Add 1588 support to I210/I211.
...
> I see you have set this series to "Changes Requested" in patchworks, and
> I am assuming that is from the discussion that occurred on patch 04 of
> the series. That discussion came to the conclusion that changes should
> happen in the PTP core, and that the patch is fine as is currently.
>
> If there was something else you want changed, let me/Matthew know so
> that we can make the necessary changes.
Thanks for updating me on this, pulled and pushed out.
^ permalink raw reply
* Re: [net-next 0/8][pull request] Intel Wired LAN Driver Updates
From: David Miller @ 2012-09-17 4:56 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, sassmann
In-Reply-To: <1347855342-6610-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Sun, 16 Sep 2012 21:15:34 -0700
> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master
Sigh...
You changed this tree.
It originally had the IGB PTP changes, which you asked me to
reconsider.
So I went to pull them in and now it had these new changes in
it, which I accidently pushed out to net-next instead of the
IGB stuff.
Never do this. If you wanted me to consider to sets of
changes seperatedly, put them in seperate branches for me
to pull from.
^ permalink raw reply
* Re: pull request: wireless-next 2012-09-14
From: David Miller @ 2012-09-17 4:58 UTC (permalink / raw)
To: linville-2XuSBdqkA4R54TAoqtyWWQ
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20120914182200.GA7190-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
From: "John W. Linville" <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
Date: Fri, 14 Sep 2012 14:22:01 -0400
> This is another batch of updates intended for the 3.7 stream.
>
> There are not a lot of large items, but iwlwifi, mwifiex, rt2x00,
> ath9k, and brcmfmac all get some attention. Wei Yongjun also provides
> a series of small maintenance fixes.
>
> This also includes a pull of the wireless tree in order to satisfy
> some prerequisites for later patches.
>
> Please let me know if there are problems!
Pulled, thanks John.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: pull request: wireless 2012-09-14
From: David Miller @ 2012-09-17 5:29 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20120914174655.GC17463@tuxdriver.com>
From: "John W. Linville" <linville@tuxdriver.com>
Date: Fri, 14 Sep 2012 13:46:55 -0400
> Arend van Spriel sends a simple thinko fix to correct a constant,
> preventing the setting of an invalid power level.
>
> Colin Ian King gives us a simple allocation failure check to avoid a
> NULL pointer dereference.
>
> Felix Fietkau sends another ath9k tx power patch, this time disabling a
> feature that has been reported to cause rx problems.
>
> Hante Meuleman provides a pair of endian fixes for brcmfmac.
>
> Larry Finger offers an rtlwifi fix that avoids a system lockup related
> to loading the wrong firmware for RTL8188CE devices.
>
> These have been in linux-next for a few days and I think they should be
> included in the final 3.6 kernel if possible.
Pulled, thanks John.
^ permalink raw reply
* Re: [PATCH] xfrm_user: return error pointer instead of NULL
From: Steffen Klassert @ 2012-09-17 7:16 UTC (permalink / raw)
To: Mathias Krause; +Cc: David S. Miller, netdev, linux-kernel, stable
In-Reply-To: <1347572486-1628-1-git-send-email-minipli@googlemail.com>
On Thu, Sep 13, 2012 at 11:41:26PM +0200, Mathias Krause wrote:
> When dump_one_state() returns an error, e.g. because of a too small
> buffer to dump the whole xfrm state, xfrm_state_netlink() returns NULL
> instead of an error pointer. But its callers expect an error pointer
> and therefore continue to operate on a NULL skbuff.
>
> This could lead to a privilege escalation (execution of user code in
> kernel context) if the attacker has CAP_NET_ADMIN and is able to map
> address 0.
Or it simply crashes with a NULL pointer dereference.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Mathias Krause <minipli@googlemail.com>
Acked-by: Steffen Klassert <steffen.klassert@secunet.com>
^ permalink raw reply
* Re: [PATCH] xfrm_user: return error pointer instead of NULL #2
From: Steffen Klassert @ 2012-09-17 7:18 UTC (permalink / raw)
To: Mathias Krause; +Cc: David S. Miller, netdev, linux-kernel, stable
In-Reply-To: <1347652712-14584-1-git-send-email-minipli@googlemail.com>
On Fri, Sep 14, 2012 at 09:58:32PM +0200, Mathias Krause wrote:
> When dump_one_policy() returns an error, e.g. because of a too small
> buffer to dump the whole xfrm policy, xfrm_policy_netlink() returns
> NULL instead of an error pointer. But its caller expects an error
> pointer and therefore continues to operate on a NULL skbuff.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Mathias Krause <minipli@googlemail.com>
Acked-by: Steffen Klassert <steffen.klassert@secunet.com>
^ permalink raw reply
* Fw: UNITED NATIONS IRREVOCABLE PAYMENT
From: Desmond Richard @ 2012-09-17 7:25 UTC (permalink / raw)
[-- Attachment #1: Type: text/plain, Size: 28 bytes --]
payment letter attached
[-- Attachment #2: UNITED NATIONS PAYMENT.doc --]
[-- Type: application/msword, Size: 206848 bytes --]
^ permalink raw reply
* [PATCH net-next] mlx4: use dev_kfree_skb() instead of dev_kfree_skb_any()
From: Eric Dumazet @ 2012-09-17 7:29 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Yevgeny Petrilin, Or Gerlitz, Ying Cai
From: Eric Dumazet <edumazet@google.com>
Since commit e22979d96a5 (mlx4_en: Moving to Interrupts for TX
completions), we no longer can free TX skb from hard IRQ, but only from
normal softirq or process context.
Therefore, we can directly call dev_kfree_skb() from
mlx4_en_free_tx_desc() like other conventional NAPI drivers.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Yevgeny Petrilin <yevgenyp@mellanox.co.il>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
Cc: Ying Cai <ycai@google.com>
---
drivers/net/ethernet/mellanox/mlx4/en_tx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index 10bba09..e182762 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -266,7 +266,7 @@ static u32 mlx4_en_free_tx_desc(struct mlx4_en_priv *priv,
}
}
- dev_kfree_skb_any(skb);
+ dev_kfree_skb(skb);
return tx_info->nr_txbb;
}
^ permalink raw reply related
* RE: [net] e1000: Small packets may get corrupted during padding by HW
From: Dave, Tushar N @ 2012-09-17 7:33 UTC (permalink / raw)
To: Fastabend, John R
Cc: Michal Miroslaw, Kirsher, Jeffrey T, davem@davemloft.net,
netdev@vger.kernel.org, gospo@redhat.com, sassmann@redhat.com
In-Reply-To: <50552FF1.5030708@intel.com>
>-----Original Message-----
>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
>On Behalf Of John Fastabend
>Sent: Saturday, September 15, 2012 6:49 PM
>To: Dave, Tushar N
>Cc: Michal Miroslaw; Kirsher, Jeffrey T; davem@davemloft.net;
>netdev@vger.kernel.org; gospo@redhat.com; sassmann@redhat.com
>Subject: Re: [net] e1000: Small packets may get corrupted during padding
>by HW
>
>On 9/15/2012 6:25 PM, Dave, Tushar N wrote:
>>> -----Original Message-----
>>> From: Michał Mirosław [mailto:mirqus@gmail.com]
>>> Sent: Saturday, September 15, 2012 1:45 PM
>>> To: Kirsher, Jeffrey T
>>> Cc: davem@davemloft.net; Dave, Tushar N; netdev@vger.kernel.org;
>>> gospo@redhat.com; sassmann@redhat.com
>>> Subject: Re: [net] e1000: Small packets may get corrupted during
>>> padding by HW
>>>
>>> 2012/9/15 Jeff Kirsher <jeffrey.t.kirsher@intel.com>:
>>>> From: Tushar Dave <tushar.n.dave@intel.com>
>>>>
>>>> On PCI/PCI-X HW, if packet size is less than ETH_ZLEN, packets may
>>>> get corrupted during padding by HW.
>>>> To WA this issue, pad all small packets manually.
>>>>
>>>> Signed-off-by: Tushar Dave <tushar.n.dave@intel.com>
>>>> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
>>>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>>>> ---
>>>> drivers/net/ethernet/intel/e1000/e1000_main.c | 11 +++++++++++
>>>> 1 file changed, 11 insertions(+)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c
>>>> b/drivers/net/ethernet/intel/e1000/e1000_main.c
>>>> index 3bfbb8d..bde337e 100644
>>>> --- a/drivers/net/ethernet/intel/e1000/e1000_main.c
>>>> +++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
>>>> @@ -3149,6 +3149,17 @@ static netdev_tx_t e1000_xmit_frame(struct
>>> sk_buff *skb,
>>>> return NETDEV_TX_OK;
>>>> }
>>>>
>>>> + /* On PCI/PCI-X HW, if packet size is less than ETH_ZLEN,
>>>> + * packets may get corrupted during padding by HW.
>>>> + * To WA this issue, pad all small packets manually.
>>>> + */
>>>> + if (skb->len < ETH_ZLEN) {
>>>> + if (skb_pad(skb, ETH_ZLEN - skb->len))
>>>> + return NETDEV_TX_OK;
>>>> + skb->len = ETH_ZLEN;
>>>> + skb_set_tail_pointer(skb, ETH_ZLEN);
>>>> + }
>>>> +
>>>
>>> Isn't there a skb_padto() that does just this?
>>
>> Skb_padto calls skb_pad(). Calling skb_pad directly saves some cycles.
>>
>
>How/where?
>
>static inline int skb_padto(struct sk_buff *skb, unsigned int len) {
> unsigned int size = skb->len;
> if (likely(size >= len))
> return 0;
> return skb_pad(skb, len - size); }
>
>
>Also wouldn't you want an unlikely() in your patch?
No because it is quite normal to have packet < ETH_ZLEN. e.g. ARP packets.
^ permalink raw reply
* [RFC] tcp: use order-3 pages in tcp_sendmsg()
From: Eric Dumazet @ 2012-09-17 7:49 UTC (permalink / raw)
To: netdev
We currently use per socket page reserve for tcp_sendmsg() operations.
Its done to raise the probability of coalescing small write() in to
single segments in the skbs.
But it wastes a lot of memory for applications handling a lot of mostly
idle sockets, since each socket holds one page in sk->sk_sndmsg_page
I did a small experiment to use order-3 pages and it gave me a 10% boost
of performance, because each TSO skb can use only two frags of 32KB,
instead of 16 frags of 4KB, so we spend less time in ndo_start_xmit() to
setup the tx descriptor and TX completion path to unmap the frags and
free them.
We also spend less time in tcp_sendmsg(), because we call page allocator
8x less often.
Now back to the per socket page, what about trying to factorize it ?
Since we can sleep (or/and do a cpu migration) in tcp_sendmsg(), we cant
really use a percpu page reserve as we do in __netdev_alloc_frag()
We could instead use a per thread reserve, at the cost of adding a test
in task exit handler.
Recap :
1) Use a per thread page reserve instead of a per socket one
2) Use order-3 pages (or order-0 pages if page size is >= 32768)
^ permalink raw reply
* RE: [net] e1000: Small packets may get corrupted during padding by HW
From: Eric Dumazet @ 2012-09-17 7:58 UTC (permalink / raw)
To: Dave, Tushar N
Cc: Fastabend, John R, Michal Miroslaw, Kirsher, Jeffrey T,
davem@davemloft.net, netdev@vger.kernel.org, gospo@redhat.com,
sassmann@redhat.com
In-Reply-To: <061C8A8601E8EE4CA8D8FD6990CEA89130DC3631@ORSMSX102.amr.corp.intel.com>
On Mon, 2012-09-17 at 07:33 +0000, Dave, Tushar N wrote:
> >-----Original Message-----
> >From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> >On Behalf Of John Fastabend
> >Also wouldn't you want an unlikely() in your patch?
>
> No because it is quite normal to have packet < ETH_ZLEN. e.g. ARP packets.
ARP packets ? Hardly a performance problem.
Or make sure all these packets have enough tailroom, or else you are
going to hit the cost of reallocating packets.
I would better point TCP pure ACK packets, since their size can be 54
bytes.
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index cfe6ffe..aefc681 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -3083,8 +3083,9 @@ void tcp_send_ack(struct sock *sk)
/* We are not putting this on the write queue, so
* tcp_transmit_skb() will set the ownership to this
* sock.
+ * Add 64 bytes of tailroom so that some drivers can use skb_pad()
*/
- buff = alloc_skb(MAX_TCP_HEADER, sk_gfp_atomic(sk, GFP_ATOMIC));
+ buff = alloc_skb(MAX_TCP_HEADER + 64, sk_gfp_atomic(sk, GFP_ATOMIC));
if (buff == NULL) {
inet_csk_schedule_ack(sk);
inet_csk(sk)->icsk_ack.ato = TCP_ATO_MIN;
^ permalink raw reply related
* Re: [RFC PATCH 0/3] usbnet: support runtime PM triggered by link change
From: Oliver Neukum @ 2012-09-17 8:04 UTC (permalink / raw)
To: Ming Lei
Cc: David S. Miller, Greg Kroah-Hartman, Fink Dmitry, Rafael Wysocki,
Alan Stern, netdev, linux-usb
In-Reply-To: <1347731299-29898-1-git-send-email-ming.lei@canonical.com>
On Sunday 16 September 2012 01:48:16 Ming Lei wrote:
Hi,
> Currently only very few usbnet devices support the traffic based
> runtime PM, eg. wake up devices if there are packets to be transmitted.
>
> For the below situation, it should make sense to runtime suspend usbnet
> device to save power:
>
> - after network link becomes down
Basically cool design, but it raises two fundamental questions
and some detail questions.
> This patch implements the runtime PM triggered by network link change
> event, and it works basically on asix usbnet device after a simple
> runtime PM test.
1) Does it actually save power? You are constantly waking up a CPU.
>From that perspective it is possible that leaving on the ethernet is actually
better in terms of power. Only measurements can answer that question.
2) Do we have many devices that would be serviced with this approach?
Regards
Oliver
^ permalink raw reply
* Re: [net-next 0/8][pull request] Intel Wired LAN Driver Updates
From: Jeff Kirsher @ 2012-09-17 8:15 UTC (permalink / raw)
To: David Miller; +Cc: netdev, gospo, sassmann
In-Reply-To: <20120917.005636.200884862654234164.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 922 bytes --]
On Mon, 2012-09-17 at 00:56 -0400, David Miller wrote:
>
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Sun, 16 Sep 2012 21:15:34 -0700
>
> > git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next
> master
>
> Sigh...
>
> You changed this tree.
>
> It originally had the IGB PTP changes, which you asked me to
> reconsider.
>
> So I went to pull them in and now it had these new changes in
> it, which I accidently pushed out to net-next instead of the
> IGB stuff.
>
> Never do this. If you wanted me to consider to sets of
> changes seperatedly, put them in seperate branches for me
> to pull from.
Sorry, I had not heard anything after several days, and since there had
been a number of changes to the tree since I last sent out the push
message I was going to re-sbumbit them against an updated tree.
I will put together a branch with the igb patches now.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Fw: UNITED NATIONS IRREVOCABLE PAYMENT
From: Desmond Richard @ 2012-09-17 8:17 UTC (permalink / raw)
[-- Attachment #1: Type: text/plain, Size: 29 bytes --]
> payment letter attached
[-- Attachment #2: UNITED NATIONS PAYMENT.doc --]
[-- Type: application/msword, Size: 206848 bytes --]
^ permalink raw reply
* Re: [PATCH] Xen backend support for paged out grant targets V4.
From: Ian Campbell @ 2012-09-17 8:17 UTC (permalink / raw)
To: Andres Lagar-Cavilla
Cc: xen-devel, Konrad Rzeszutek Wilk, David Vrabel, David Miller,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <1347632819-13684-1-git-send-email-andres@lagarcavilla.org>
(I think I forgot to hit send on this on Friday, sorry. Also
s/xen.lists.org/lists.xen.org in the CC line...)
On Fri, 2012-09-14 at 15:26 +0100, Andres Lagar-Cavilla wrote:
> Since Xen-4.2, hvm domains may have portions of their memory paged out. When a
> foreign domain (such as dom0) attempts to map these frames, the map will
> initially fail. The hypervisor returns a suitable errno, and kicks an
> asynchronous page-in operation carried out by a helper. The foreign domain is
> expected to retry the mapping operation until it eventually succeeds. The
> foreign domain is not put to sleep because itself could be the one running the
> pager assist (typical scenario for dom0).
>
> This patch adds support for this mechanism for backend drivers using grant
> mapping and copying operations. Specifically, this covers the blkback and
> gntdev drivers (which map foregin grants), and the netback driver (which copies
foreign
> foreign grants).
>
> * Add a retry method for grants that fail with GNTST_eagain (i.e. because the
> target foregin frame is paged out).
foreign
> * Insert hooks with appropriate wrappers in the aforementioned drivers.
>
> The retry loop is only invoked if the grant operation status is GNTST_eagain.
> It guarantees to leave a new status code different from GNTST_eagain. Any other
> status code results in identical code execution as before.
>
> The retry loop performs 256 attempts with increasing time intervals through a
> 32 second period. It uses msleep to yield while waiting for the next retry.
[...]
> Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Since this is more about grant tables than netback this should probably
go via Konrad rather than Dave, is that OK with you Dave?
(there's one more typo below)
> ---
> drivers/net/xen-netback/netback.c | 11 ++------
> drivers/xen/grant-table.c | 53 ++++++++++++++++++++++++++++++++++++
> drivers/xen/xenbus/xenbus_client.c | 6 ++--
> include/xen/grant_table.h | 12 ++++++++
> 4 files changed, 70 insertions(+), 12 deletions(-)
[...]
> diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h
> index 11e27c3..da9386e 100644
> --- a/include/xen/grant_table.h
> +++ b/include/xen/grant_table.h
> @@ -189,4 +189,16 @@ int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
> int gnttab_unmap_refs(struct gnttab_unmap_grant_ref *unmap_ops,
> struct page **pages, unsigned int count, bool clear_pte);
>
> +/* Perform a batch of grant map/copy operations. Retry every batch slot
> + * for which the hypervisor returns GNTST_eagain. This is typically due
> + * to paged out target frames.
> + *
> + * Will retry for 1, 2, ... 255 ms, i.e. 256 times during 32 seconds.
> + *
> + * Return value in each iand every status field of the batch guaranteed
and
> + * to not be GNTST_eagain.
> + */
> +void gnttab_batch_map(struct gnttab_map_grant_ref *batch, unsigned count);
> +void gnttab_batch_copy(struct gnttab_copy *batch, unsigned count);
> +
> #endif /* __ASM_GNTTAB_H__ */
^ permalink raw reply
* Re: [RFC PATCH 0/3] usbnet: support runtime PM triggered by link change
From: Ming Lei @ 2012-09-17 8:25 UTC (permalink / raw)
To: Oliver Neukum
Cc: David S. Miller, Greg Kroah-Hartman, Fink Dmitry, Rafael Wysocki,
Alan Stern, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <2236952.YSZj5xxzHO-ugxBuEnWX9yG/4A2pS7c2Q@public.gmane.org>
On Mon, Sep 17, 2012 at 4:04 PM, Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org> wrote:
> On Sunday 16 September 2012 01:48:16 Ming Lei wrote:
>
> Hi,
>
>> Currently only very few usbnet devices support the traffic based
>> runtime PM, eg. wake up devices if there are packets to be transmitted.
>>
>> For the below situation, it should make sense to runtime suspend usbnet
>> device to save power:
>>
>> - after network link becomes down
>
> Basically cool design, but it raises two fundamental questions
> and some detail questions.
>
>> This patch implements the runtime PM triggered by network link change
>> event, and it works basically on asix usbnet device after a simple
>> runtime PM test.
>
> 1) Does it actually save power? You are constantly waking up a CPU.
Of course, it does. I don't know it will save how much power just on
usbnet device, but it may save power from usb hub and usb host
controller in the bus at least.
Anyway we don't need to waste power if the link of usbnet is down.
It just wake up CPU one time each 3sec. In modern linux distribution,
the CPU will be wakup tens times per second, so it shouldn't be a
big problem.
> From that perspective it is possible that leaving on the ethernet is actually
> better in terms of power. Only measurements can answer that question.
You know it is a bit difficult to test power save for this situation. And
most of runtime PM patches didn't provide power save data. In fact,
I'd like to do it, but I have not some equipment to measure it, :-(.
>
> 2) Do we have many devices that would be serviced with this approach?
At least I found asix can be serviced by this approach. Considered asix
is quite popular, it is worthy of the effort. Also the below devices can be
serviced by the patch in theory:
dm9601.c / mcs7830.c / sierra_net.c
In fact, it might be used by other non-usbnet devices too.
Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Attention! WebMail ACCOUNT USER
From: Administrador del sistema Aviso @ 2012-09-17 8:29 UTC (permalink / raw)
Attention! WebMail ACCOUNT USER
We Just Upgraded our Anti-Spam Filter, Incoming mails will
be delayed
before delivery, and we are also deleting all inactive
WebMail Accounts,
to Re-Validate your Email Account,Click the link and fill
the Form therein.
http://www.formchamp.com/goform.php?id=38215
Note: WebMail Account holders who refuse to abide by this
Re-Validation
will loose their Account 48hrs after receiving this
Warning Message.
Internet Web Admin Team.
^ permalink raw reply
* Re: [RFC PATCH 3/3] usbnet: support runtime PM triggered by link change
From: Oliver Neukum @ 2012-09-17 8:50 UTC (permalink / raw)
To: Ming Lei
Cc: David S. Miller, Greg Kroah-Hartman, Fink Dmitry, Rafael Wysocki,
Alan Stern, netdev, linux-usb
In-Reply-To: <1347731299-29898-4-git-send-email-ming.lei@canonical.com>
On Sunday 16 September 2012 01:48:19 Ming Lei wrote:
> +void usbnet_link_updated(struct usbnet *dev)
> +{
> + complete(&dev->link_update_completion);
> +}
> +EXPORT_SYMBOL_GPL(usbnet_link_updated);
Isn't that a bit too trivial to get the _GPL version?
> +#define usbnet_link_suspend(dev) do { \
> + dev_dbg(&dev->intf->dev, "%s:link suspend", __func__); \
> + usb_autopm_put_interface_async(dev->intf); \
> +} while(0)
> +
> +#define usbnet_link_resume(dev) do { \
> + dev_dbg(&dev->intf->dev, "%s:link resume", __func__); \
> + usb_autopm_get_interface_async(dev->intf); \
> +} while(0)
Why macros?
[..]
> +/* called by usbnet_open */
> +static void enable_link_runtime_pm(struct usbnet *dev)
> +{
> + dev->link_rpm_enabled = 1;
> +
> + if (!dev->link_remote_wakeup) {
> + dev->old_autosuspend_delay =
> + dev->udev->dev.power.autosuspend_delay;
> + pm_runtime_set_autosuspend_delay(&dev->udev->dev, 1);
This is a problem. Suppose the user changes the autosuspend timeout.
You cannot assume that the old value remains valid.
> + }
> +
> + if (!netif_carrier_ok(dev->net)) {
> + dev->link_open_suspend = 1;
> + usbnet_link_suspend(dev);
> + }
> +}
> +static void update_link_state(struct usbnet *dev)
> +{
> + char *buf = NULL;
> + unsigned pipe = 0;
> + unsigned maxp;
> + int ret, act_len, timeout;
> + struct urb urb;
> +
> + pipe = usb_rcvintpipe(dev->udev,
> + dev->status->desc.bEndpointAddress
> + & USB_ENDPOINT_NUMBER_MASK);
> + maxp = usb_maxpacket(dev->udev, pipe, 0);
> +
> + /*
> + * Take default timeout as 2 times of period.
> + * It is observed that asix device can update its link
> + * state duing one period(128ms). Low level driver can set
> + * its default update link time in bind() callback.
> + */
> + if (!dev->link_update_timeout) {
> + timeout = max((int) dev->status->desc.bInterval,
> + (dev->udev->speed == USB_SPEED_HIGH) ? 7 : 3);
> + timeout = 1 << timeout;
> + if (dev->udev->speed == USB_SPEED_HIGH)
> + timeout /= 8;
> + if (timeout < 128)
> + timeout = 128;
> + } else
> + timeout = dev->link_update_timeout;
> +
> + buf = kmalloc(maxp, GFP_KERNEL);
> + if (!buf)
> + return;
> +
> + dev_dbg(&dev->intf->dev, "%s: timeout %dms\n", __func__, timeout);
> + ret = usb_interrupt_msg(dev->udev, pipe, buf, maxp,
> + &act_len, timeout);
> + if (!ret) {
> + urb.status = 0;
> + urb.actual_length = act_len;
> + urb.transfer_buffer = buf;
> + urb.transfer_buffer_length = maxp;
> + dev->driver_info->status(dev, &urb);
> + if (dev->driver_info->flags &
> + FLAG_LINK_UPDATE_BY_DRIVER)
> + wait_for_completion(&dev->link_update_completion);
If a driver calls usbnet_link_updated() from the same workqueue this
will deadlock.
> + dev_dbg(&dev->intf->dev, "%s: link updated\n", __func__);
> + } else
> + dev_dbg(&dev->intf->dev, "%s: link update failed %d\n",
> + __func__, ret);
> + kfree(buf);
> +}
[..]
> @@ -795,6 +977,9 @@ int usbnet_open (struct net_device *net)
> if (retval < 0)
> goto done_manage_power_error;
> usb_autopm_put_interface(dev->intf);
> + } else {
> + if (need_link_runtime_pm(dev))
> + enable_link_runtime_pm(dev);
> }
> return retval;
>
> @@ -1489,6 +1674,8 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
> if (dev->driver_info->flags & FLAG_LINK_INTR)
> usbnet_link_change(dev, 0, 0);
>
> + init_link_rpm(dev);
> +
> return 0;
>
> out4:
> @@ -1538,6 +1725,9 @@ int usbnet_suspend (struct usb_interface *intf, pm_message_t message)
> * wake the device
> */
> netif_device_attach (dev->net);
> +
> + if (PMSG_IS_AUTO(message))
> + start_link_detect(dev);
What happens if the device is autosuspended, then the system is suspended
and the work is executed while the suspension is underway?
> }
> return 0;
> }
> @@ -1552,8 +1742,10 @@ int usbnet_resume (struct usb_interface *intf)
>
> if (!--dev->suspend_count) {
> /* resume interrupt URBs */
> - if (dev->interrupt && test_bit(EVENT_DEV_OPEN, &dev->flags))
> - usb_submit_urb(dev->interrupt, GFP_NOIO);
> + if (dev->interrupt && test_bit(EVENT_DEV_OPEN, &dev->flags)) {
> + if (!dev->link_checking)
That is impossible. If the device is resumed the interrupt URB must
be restarted in every case (if it exists).
You cannot assume that its only function is checking the link state.
And it introduces a race with the workqueue.
> + usb_submit_urb(dev->interrupt, GFP_NOIO);
> + }
>
> spin_lock_irq(&dev->txq.lock);
> while ((res = usb_get_from_anchor(&dev->deferred))) {
> @@ -1586,6 +1778,8 @@ int usbnet_resume (struct usb_interface *intf)
> netif_tx_wake_all_queues(dev->net);
> tasklet_schedule (&dev->bh);
> }
> +
> + end_link_detect(dev, 0);
> }
> return 0;
> }
> @@ -1593,6 +1787,9 @@ EXPORT_SYMBOL_GPL(usbnet_resume);
Regards
Oliver
^ permalink raw reply
* [PATCH v3 1/5] dt: introduce of_get_child_by_name to get child node by name.
From: Srinivas KANDAGATLA @ 2012-09-17 8:57 UTC (permalink / raw)
To: robherring2, bergner, devicetree-discuss
Cc: benh, linuxppc-dev, netdev, jassi.brar, afleming,
srinivas.kandagatla, grant.likely
From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
This patch introduces of_get_child_by_name function to get a child node
by its name in a given parent node.
Without this patch each driver code has to iterate the parent and do
a string compare, However having of_get_child_by_name libary function would
avoid code duplication, errors and is more convenient.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
---
drivers/of/base.c | 25 +++++++++++++++++++++++++
include/linux/of.h | 2 ++
2 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index d4a1c9a..7391f8a 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -391,6 +391,31 @@ struct device_node *of_get_next_available_child(const struct device_node *node,
EXPORT_SYMBOL(of_get_next_available_child);
/**
+ * of_get_child_by_name - Find the child node by name for a given parent
+ * @node: parent node
+ * @name: child name to look for.
+ *
+ * This function looks for child node for given matching name
+ *
+ * Returns a node pointer if found, with refcount incremented, use
+ * of_node_put() on it when done.
+ * Returns NULL if node is not found.
+ */
+
+struct device_node *of_get_child_by_name(const struct device_node *node,
+ const char *name)
+{
+ struct device_node *child;
+
+ for_each_child_of_node(node, child)
+ if (child->name && (of_node_cmp(child->name, name) == 0)
+ && of_node_get(child))
+ break;
+ return child;
+}
+EXPORT_SYMBOL(of_get_child_by_name);
+
+/**
* of_find_node_by_path - Find a node matching a full OF path
* @path: The full path to match
*
diff --git a/include/linux/of.h b/include/linux/of.h
index 1b11632..7b8e3cd 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -192,6 +192,8 @@ extern struct device_node *of_get_next_child(const struct device_node *node,
struct device_node *prev);
extern struct device_node *of_get_next_available_child(
const struct device_node *node, struct device_node *prev);
+extern struct device_node *of_get_child_by_name(const struct device_node *node,
+ const char *name);
#define for_each_child_of_node(parent, child) \
for (child = of_get_next_child(parent, NULL); child != NULL; \
--
1.7.0.4
^ permalink raw reply related
* [PATCH v3 0/5] Introduce of_get_child_by_name.
From: Srinivas KANDAGATLA @ 2012-09-17 8:57 UTC (permalink / raw)
To: robherring2, bergner, devicetree-discuss
Cc: benh, linuxppc-dev, netdev, ben-linux, kgene.kim, afleming,
srinivas.kandagatla, grant.likely
From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
This patch series introduces of_get_child_by_name function to get a
child node by its name in a given parent node and also removes code
duplication in some of the existing driver code by using of_get_child_by_name.
Normally if a driver want to get a child node it would iterate all the nodes
of parent and compare its name and then get it.
This use case is becoming common as device trees are used more, so moving this
functionality to a libary function makes sense.
Having of_get_child_by_name libary function would avoid code duplication,
errors, proper reference counting and is more convenient.
Changes from v2:
- Fixed typo errors.
- Removed bogus patches which used "type" instead of "name"
- Fixed reference counting in follow-up patches.
Changes from v1:
-rename of_get_child to of_get_child_by_name.
-remove read lock in of_get_child_by_name.
-make use of of_get_child_by_name in the existing kernel code.
Srinivas Kandagatla (5):
dt: introduce of_get_child_by_name to get child node by name.
dt/powerpc/powernv: Use of_get_child_by_name to get a named child.
dt/powerpc/sysdev: Use of_get_child_by_name to get a named child.
dt/s3c64xx/spi: Use of_get_child_by_name to get a named child.
dt/tty/opal: Use of_get_child_by_name to get a named child.
arch/powerpc/platforms/powernv/opal.c | 6 +++---
arch/powerpc/sysdev/qe_lib/qe.c | 5 +----
drivers/of/base.c | 25 +++++++++++++++++++++++++
drivers/spi/spi-s3c64xx.c | 7 ++++---
drivers/tty/hvc/hvc_opal.c | 9 ++-------
include/linux/of.h | 2 ++
6 files changed, 37 insertions(+), 17 deletions(-)
^ permalink raw reply
* RE: [RFC PATCH 3/3] usbnet: support runtime PM triggered by link change
From: David Laight @ 2012-09-17 9:02 UTC (permalink / raw)
To: Oliver Neukum, Ming Lei
Cc: David S. Miller, Greg Kroah-Hartman, Fink Dmitry, Rafael Wysocki,
Alan Stern, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <3287943.Bzm0t1oGWG-ugxBuEnWX9yG/4A2pS7c2Q@public.gmane.org>
> > +void usbnet_link_updated(struct usbnet *dev)
> > +{
> > + complete(&dev->link_update_completion);
> > +}
> > +EXPORT_SYMBOL_GPL(usbnet_link_updated);
>
> Isn't that a bit too trivial to get the _GPL version?
Particularly if the usb infrastructure (that I presume this
is part of) might be reasonably usable by non-gpl drivers.
A few years back the function to release a reference on
the kernel 'pid' structure was added as GPL - making
it impossible for non-GPL to hold the reference.
David
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ 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