Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 2/8] bnx2x: store module parameters in driver main structure
From: Dmitry Kravkov @ 2010-07-27 22:32 UTC (permalink / raw)
  To: davem, netdev; +Cc: eilong

Store module parameters during initialization of main driver
structure. This will allow access to the parameters from different
files.

Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
 drivers/net/bnx2x/bnx2x.h      |    2 ++
 drivers/net/bnx2x/bnx2x_main.c |    7 ++++---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x.h b/drivers/net/bnx2x/bnx2x.h
index 3b51c5f..237609f 100644
--- a/drivers/net/bnx2x/bnx2x.h
+++ b/drivers/net/bnx2x/bnx2x.h
@@ -1006,6 +1006,8 @@ struct bnx2x {
 
 	int			multi_mode;
 	int			num_queues;
+	int			disable_tpa;
+	int			int_mode;
 
 	u32			rx_mode;
 #define BNX2X_RX_MODE_NONE		0
diff --git a/drivers/net/bnx2x/bnx2x_main.c b/drivers/net/bnx2x/bnx2x_main.c
index 51b7883..1e0ac8b 100644
--- a/drivers/net/bnx2x/bnx2x_main.c
+++ b/drivers/net/bnx2x/bnx2x_main.c
@@ -7878,7 +7878,7 @@ static int bnx2x_set_num_queues(struct bnx2x *bp)
 {
 	int rc = 0;
 
-	switch (int_mode) {
+	switch (bp->int_mode) {
 	case INT_MODE_INTx:
 	case INT_MODE_MSI:
 		bp->num_queues = 1;
@@ -9951,7 +9951,7 @@ static int __devinit bnx2x_init_bp(struct bnx2x *bp)
 		multi_mode = ETH_RSS_MODE_DISABLED;
 	}
 	bp->multi_mode = multi_mode;
-
+	bp->int_mode = int_mode;
 
 	bp->dev->features |= NETIF_F_GRO;
 
@@ -9963,6 +9963,7 @@ static int __devinit bnx2x_init_bp(struct bnx2x *bp)
 		bp->flags |= TPA_ENABLE_FLAG;
 		bp->dev->features |= NETIF_F_LRO;
 	}
+	bp->disable_tpa = disable_tpa;
 
 	if (CHIP_IS_E1(bp))
 		bp->dropless_fc = 0;
@@ -11006,7 +11007,7 @@ static int bnx2x_set_flags(struct net_device *dev, u32 data)
 
 	/* TPA requires Rx CSUM offloading */
 	if ((data & ETH_FLAG_LRO) && bp->rx_csum) {
-		if (!disable_tpa) {
+		if (!bp->disable_tpa) {
 			if (!(dev->features & NETIF_F_LRO)) {
 				dev->features |= NETIF_F_LRO;
 				bp->flags |= TPA_ENABLE_FLAG;
-- 
1.7.1





^ permalink raw reply related

* [PATCH net-next 1/8] bnx2x: Create separate folder for bnx2x driver
From: Dmitry Kravkov @ 2010-07-27 22:31 UTC (permalink / raw)
  To: davem, netdev; +Cc: eilong

This commit includes files movement to newly created folder
using git-mv command and fixes references in cnic and bnx2x code
to each other.

files moved using following:
#!/bin/bash
mkdir drivers/net/bnx2x/
list=$(cd drivers/net/ && ls bnx2x*.[ch])
for f in $list; do
        git mv -f drivers/net/$f drivers/net/bnx2x/$f
done

Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
 drivers/net/Makefile                        |    3 +--
 drivers/net/bnx2x/Makefile                  |    7 +++++++
 drivers/net/{ => bnx2x}/bnx2x.h             |    2 +-
 drivers/net/{ => bnx2x}/bnx2x_dump.h        |    0
 drivers/net/{ => bnx2x}/bnx2x_fw_defs.h     |    0
 drivers/net/{ => bnx2x}/bnx2x_fw_file_hdr.h |    0
 drivers/net/{ => bnx2x}/bnx2x_hsi.h         |    0
 drivers/net/{ => bnx2x}/bnx2x_init.h        |    0
 drivers/net/{ => bnx2x}/bnx2x_init_ops.h    |    0
 drivers/net/{ => bnx2x}/bnx2x_link.c        |    0
 drivers/net/{ => bnx2x}/bnx2x_link.h        |    0
 drivers/net/{ => bnx2x}/bnx2x_main.c        |    0
 drivers/net/{ => bnx2x}/bnx2x_reg.h         |    0
 drivers/net/cnic.c                          |    6 +++---
 14 files changed, 12 insertions(+), 6 deletions(-)
 create mode 100644 drivers/net/bnx2x/Makefile
 rename drivers/net/{ => bnx2x}/bnx2x.h (99%)
 rename drivers/net/{ => bnx2x}/bnx2x_dump.h (100%)
 rename drivers/net/{ => bnx2x}/bnx2x_fw_defs.h (100%)
 rename drivers/net/{ => bnx2x}/bnx2x_fw_file_hdr.h (100%)
 rename drivers/net/{ => bnx2x}/bnx2x_hsi.h (100%)
 rename drivers/net/{ => bnx2x}/bnx2x_init.h (100%)
 rename drivers/net/{ => bnx2x}/bnx2x_init_ops.h (100%)
 rename drivers/net/{ => bnx2x}/bnx2x_link.c (100%)
 rename drivers/net/{ => bnx2x}/bnx2x_link.h (100%)
 rename drivers/net/{ => bnx2x}/bnx2x_main.c (100%)
 rename drivers/net/{ => bnx2x}/bnx2x_reg.h (100%)

diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index ce55581..56e8c27 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -84,8 +84,7 @@ obj-$(CONFIG_FEALNX) += fealnx.o
 obj-$(CONFIG_TIGON3) += tg3.o
 obj-$(CONFIG_BNX2) += bnx2.o
 obj-$(CONFIG_CNIC) += cnic.o
-obj-$(CONFIG_BNX2X) += bnx2x.o
-bnx2x-objs := bnx2x_main.o bnx2x_link.o
+obj-$(CONFIG_BNX2X) += bnx2x/
 spidernet-y += spider_net.o spider_net_ethtool.o
 obj-$(CONFIG_SPIDER_NET) += spidernet.o sungem_phy.o
 obj-$(CONFIG_GELIC_NET) += ps3_gelic.o
diff --git a/drivers/net/bnx2x/Makefile b/drivers/net/bnx2x/Makefile
new file mode 100644
index 0000000..46c853b
--- /dev/null
+++ b/drivers/net/bnx2x/Makefile
@@ -0,0 +1,7 @@
+#
+# Makefile for Broadcom 10-Gigabit ethernet driver
+#
+
+obj-$(CONFIG_BNX2X) += bnx2x.o
+
+bnx2x-objs := bnx2x_main.o bnx2x_link.o
diff --git a/drivers/net/bnx2x.h b/drivers/net/bnx2x/bnx2x.h
similarity index 99%
rename from drivers/net/bnx2x.h
rename to drivers/net/bnx2x/bnx2x.h
index 8bd2368..3b51c5f 100644
--- a/drivers/net/bnx2x.h
+++ b/drivers/net/bnx2x/bnx2x.h
@@ -32,7 +32,7 @@
 
 #if defined(CONFIG_CNIC) || defined(CONFIG_CNIC_MODULE)
 #define BCM_CNIC 1
-#include "cnic_if.h"
+#include "../cnic_if.h"
 #endif
 
 
diff --git a/drivers/net/bnx2x_dump.h b/drivers/net/bnx2x/bnx2x_dump.h
similarity index 100%
rename from drivers/net/bnx2x_dump.h
rename to drivers/net/bnx2x/bnx2x_dump.h
diff --git a/drivers/net/bnx2x_fw_defs.h b/drivers/net/bnx2x/bnx2x_fw_defs.h
similarity index 100%
rename from drivers/net/bnx2x_fw_defs.h
rename to drivers/net/bnx2x/bnx2x_fw_defs.h
diff --git a/drivers/net/bnx2x_fw_file_hdr.h b/drivers/net/bnx2x/bnx2x_fw_file_hdr.h
similarity index 100%
rename from drivers/net/bnx2x_fw_file_hdr.h
rename to drivers/net/bnx2x/bnx2x_fw_file_hdr.h
diff --git a/drivers/net/bnx2x_hsi.h b/drivers/net/bnx2x/bnx2x_hsi.h
similarity index 100%
rename from drivers/net/bnx2x_hsi.h
rename to drivers/net/bnx2x/bnx2x_hsi.h
diff --git a/drivers/net/bnx2x_init.h b/drivers/net/bnx2x/bnx2x_init.h
similarity index 100%
rename from drivers/net/bnx2x_init.h
rename to drivers/net/bnx2x/bnx2x_init.h
diff --git a/drivers/net/bnx2x_init_ops.h b/drivers/net/bnx2x/bnx2x_init_ops.h
similarity index 100%
rename from drivers/net/bnx2x_init_ops.h
rename to drivers/net/bnx2x/bnx2x_init_ops.h
diff --git a/drivers/net/bnx2x_link.c b/drivers/net/bnx2x/bnx2x_link.c
similarity index 100%
rename from drivers/net/bnx2x_link.c
rename to drivers/net/bnx2x/bnx2x_link.c
diff --git a/drivers/net/bnx2x_link.h b/drivers/net/bnx2x/bnx2x_link.h
similarity index 100%
rename from drivers/net/bnx2x_link.h
rename to drivers/net/bnx2x/bnx2x_link.h
diff --git a/drivers/net/bnx2x_main.c b/drivers/net/bnx2x/bnx2x_main.c
similarity index 100%
rename from drivers/net/bnx2x_main.c
rename to drivers/net/bnx2x/bnx2x_main.c
diff --git a/drivers/net/bnx2x_reg.h b/drivers/net/bnx2x/bnx2x_reg.h
similarity index 100%
rename from drivers/net/bnx2x_reg.h
rename to drivers/net/bnx2x/bnx2x_reg.h
diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
index 5ecf0bc..0961032 100644
--- a/drivers/net/cnic.c
+++ b/drivers/net/cnic.c
@@ -40,9 +40,9 @@
 
 #include "cnic_if.h"
 #include "bnx2.h"
-#include "bnx2x_reg.h"
-#include "bnx2x_fw_defs.h"
-#include "bnx2x_hsi.h"
+#include "bnx2x/bnx2x_reg.h"
+#include "bnx2x/bnx2x_fw_defs.h"
+#include "bnx2x/bnx2x_hsi.h"
 #include "../scsi/bnx2i/57xx_iscsi_constants.h"
 #include "../scsi/bnx2i/57xx_iscsi_hsi.h"
 #include "cnic.h"
-- 
1.7.1





^ permalink raw reply related

* Re: [PATCH] Add missing memory barriers to clean_rx_irq functions in Intel Drivers
From: Jeff Kirsher @ 2010-07-27 22:41 UTC (permalink / raw)
  To: Sonny Rao; +Cc: netdev, e1000-devel
In-Reply-To: <20100727223452.GM17248@us.ibm.com>

On Tue, Jul 27, 2010 at 15:34, Sonny Rao <sonnyrao@us.ibm.com> wrote:
> This patch is similar to what was fixed in ixgbe in this patch:
>
> http://marc.info/?l=e1000-devel&m=126593062701537&w=3
>
> We should add read memory barriers to all the similar cases across the
> Intel ethernet driver family.  In the case of ixgbevf I've also added
> a missing barrier to the clean_tx_irq path because I missed it in my
> last patch.
>
> Without the barrier a processor can speculate a load ahead of the load
> which looks at the status bit and get stale information causing a
> number of different issues including invalid packet length, NULL
> pointers, or bad data since checksumming was assumed to be done
> in hardware.
>
> Signed-off-by: Milton Miller <miltonm@bga.com>
> Signed-off-by: Sonny Rao <sonnyrao@us.ibm.com>
> cc: stable <stable@kernel.org>
>

I already have a similar patch in my queue from you Sonny, although I
see that this patch has made a few more changes.  Is this version 2?

-- 
Cheers,
Jeff

^ permalink raw reply

* [PATCH] Add missing memory barriers to clean_rx_irq functions in Intel Drivers
From: Sonny Rao @ 2010-07-27 22:34 UTC (permalink / raw)
  To: netdev; +Cc: e1000-devel, Jeff Kirsher

This patch is similar to what was fixed in ixgbe in this patch:

http://marc.info/?l=e1000-devel&m=126593062701537&w=3

We should add read memory barriers to all the similar cases across the
Intel ethernet driver family.  In the case of ixgbevf I've also added 
a missing barrier to the clean_tx_irq path because I missed it in my
last patch.

Without the barrier a processor can speculate a load ahead of the load
which looks at the status bit and get stale information causing a
number of different issues including invalid packet length, NULL
pointers, or bad data since checksumming was assumed to be done
in hardware.

Signed-off-by: Milton Miller <miltonm@bga.com>
Signed-off-by: Sonny Rao <sonnyrao@us.ibm.com>
cc: stable <stable@kernel.org>

Index: linux-2.6.35-rc5/drivers/net/e1000/e1000_main.c
===================================================================
--- linux-2.6.35-rc5.orig/drivers/net/e1000/e1000_main.c	2010-07-27 16:15:18.000000000 -0500
+++ linux-2.6.35-rc5/drivers/net/e1000/e1000_main.c	2010-07-27 16:22:21.000000000 -0500
@@ -3638,6 +3638,7 @@ static bool e1000_clean_jumbo_rx_irq(str
 		if (*work_done >= work_to_do)
 			break;
 		(*work_done)++;
+		rmb(); /* read descriptor and rx_buffer_info after status DD */
 
 		status = rx_desc->status;
 		skb = buffer_info->skb;
@@ -3844,6 +3845,7 @@ static bool e1000_clean_rx_irq(struct e1
 		if (*work_done >= work_to_do)
 			break;
 		(*work_done)++;
+		rmb(); /* read descriptor and rx_buffer_info after status DD */
 
 		status = rx_desc->status;
 		skb = buffer_info->skb;
Index: linux-2.6.35-rc5/drivers/net/e1000e/netdev.c
===================================================================
--- linux-2.6.35-rc5.orig/drivers/net/e1000e/netdev.c	2010-07-27 16:22:38.000000000 -0500
+++ linux-2.6.35-rc5/drivers/net/e1000e/netdev.c	2010-07-27 16:25:23.000000000 -0500
@@ -774,6 +774,7 @@ static bool e1000_clean_rx_irq(struct e1
 		if (*work_done >= work_to_do)
 			break;
 		(*work_done)++;
+		rmb();	/* read descriptor and rx_buffer_info after status DD */
 
 		status = rx_desc->status;
 		skb = buffer_info->skb;
@@ -1081,6 +1082,7 @@ static bool e1000_clean_rx_irq_ps(struct
 			break;
 		(*work_done)++;
 		skb = buffer_info->skb;
+		rmb();	/* read descriptor and rx_buffer_info after status DD */
 
 		/* in the packet split case this is header only */
 		prefetch(skb->data - NET_IP_ALIGN);
@@ -1280,6 +1282,7 @@ static bool e1000_clean_jumbo_rx_irq(str
 		if (*work_done >= work_to_do)
 			break;
 		(*work_done)++;
+		rmb();	/* read descriptor and rx_buffer_info after status DD */
 
 		status = rx_desc->status;
 		skb = buffer_info->skb;
Index: linux-2.6.35-rc5/drivers/net/ixgb/ixgb_main.c
===================================================================
--- linux-2.6.35-rc5.orig/drivers/net/ixgb/ixgb_main.c	2010-07-27 16:27:23.000000000 -0500
+++ linux-2.6.35-rc5/drivers/net/ixgb/ixgb_main.c	2010-07-27 16:41:57.000000000 -0500
@@ -1977,6 +1977,7 @@ ixgb_clean_rx_irq(struct ixgb_adapter *a
 			break;
 
 		(*work_done)++;
+		rmb();	/* read descriptor and rx_buffer_info after status DD */
 		status = rx_desc->status;
 		skb = buffer_info->skb;
 		buffer_info->skb = NULL;
Index: linux-2.6.35-rc5/drivers/net/ixgbevf/ixgbevf_main.c
===================================================================
--- linux-2.6.35-rc5.orig/drivers/net/ixgbevf/ixgbevf_main.c	2010-07-27 16:30:51.000000000 -0500
+++ linux-2.6.35-rc5/drivers/net/ixgbevf/ixgbevf_main.c	2010-07-27 16:40:19.000000000 -0500
@@ -231,6 +231,7 @@ static bool ixgbevf_clean_tx_irq(struct 
 	while ((eop_desc->wb.status & cpu_to_le32(IXGBE_TXD_STAT_DD)) &&
 	       (count < tx_ring->work_limit)) {
 		bool cleaned = false;
+		rmb(); /* read buffer_info after eop_desc */
 		for ( ; !cleaned; count++) {
 			struct sk_buff *skb;
 			tx_desc = IXGBE_TX_DESC_ADV(*tx_ring, i);
@@ -518,6 +519,7 @@ static bool ixgbevf_clean_rx_irq(struct 
 			break;
 		(*work_done)++;
 
+		rmb(); /* read descriptor and rx_buffer_info after status DD */
 		if (adapter->flags & IXGBE_FLAG_RX_PS_ENABLED) {
 			hdr_info = le16_to_cpu(ixgbevf_get_hdr_info(rx_desc));
 			len = (hdr_info & IXGBE_RXDADV_HDRBUFLEN_MASK) >>

^ permalink raw reply

* [net-2.6 PATCH] e1000e: 82577/82578 PHY register access issues
From: Jeff Kirsher @ 2010-07-27 22:28 UTC (permalink / raw)
  To: davem; +Cc: netdev, gospo, bphilips, stable, Bruce Allan, Jeff Kirsher

From: Bruce Allan <bruce.w.allan@intel.com>

The MAC-PHY interconnect on 82577/82578 uses a power management feature
(called K1) which must be disabled when in 1Gbps due to a hardware issue on
these parts.  The #define bit setting used to enable/disable K1 is
incorrect and can cause PHY register accesses to stop working altogether
until the next device reset.  This patch sets the register correctly.

This issue is present in kernels since 2.6.32.

CC: stable@kernel.org
Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---

 drivers/net/e1000e/hw.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/e1000e/hw.h b/drivers/net/e1000e/hw.h
index 5d1220d..664ed58 100644
--- a/drivers/net/e1000e/hw.h
+++ b/drivers/net/e1000e/hw.h
@@ -308,7 +308,7 @@ enum e1e_registers {
 #define E1000_KMRNCTRLSTA_INBAND_PARAM	0x9    /* Kumeran InBand Parameters */
 #define E1000_KMRNCTRLSTA_DIAG_NELPBK	0x1000 /* Nearend Loopback mode */
 #define E1000_KMRNCTRLSTA_K1_CONFIG	0x7
-#define E1000_KMRNCTRLSTA_K1_ENABLE	0x140E
+#define E1000_KMRNCTRLSTA_K1_ENABLE	0x0002
 #define E1000_KMRNCTRLSTA_K1_DISABLE	0x1400
 
 #define IFE_PHY_EXTENDED_STATUS_CONTROL	0x10


^ permalink raw reply related

* Query on Linux Kernel Network - Transmission Path
From: Kumar Sanghvi @ 2010-07-27 22:15 UTC (permalink / raw)
  To: netdev

Hi All,

I have a query on the Transmission Path for the Linux Kernel Network
stack.
I understand that Qdisc/Traffic control can be used to prioritize
traffic on a per net_device basis.

However, I would like to know how to prioritize traffic between the
net_device(s) ?

Assume the below scenario:
-A Linux server has two net_device.
-One net_device -eth0, deals with IPv4 traffic over Ethernet
 This eth0 net_device is heavily loaded and there is a continuous
network activity going on.

-2nd net_device -ph0, deals with Phonet traffic.
 It occassionally transmits Voice traffic. It is required that traffic
over net_device ph0 should not suffer any delay.

Now as I understand, for the transmission path, the function
"__netif_schedule" will queue a net_device on the output_queue of
sofnet_data.
Also, the net_tx_action will pick up the net_device from output_queue
of softnet_data on last-in--first-out basis.

So, since eth0 has always IPv4 traffic to be sent, it will get queued
up continously in the output_queue of softnet_data. In this scenario,
what would happen to the ph0 device carrying the Phonet traffic? Will
the ph0 device get a chance to queue up on the output_queue of
softnet_data in the middle of continously coming IPv4 traffic from
eth0?
Or will the ph0 get starved in this case? Or the ph0 will suffer a
delay in getting scheduled for transmission?

In short, is there any way to prioritize the transmission among the
net_device eth0 and ph0 i.e. is it possible to queue up ph0 carrying
Phonet traffic on output_queue of softnet_data first in comparison to
eth0 carrying IPv4 data, so that ph0 traffic does not suffer any delay?

Pardon me if I have misunderstood anything or if my knowledge is
outdated in this regards.
I would really appreciate some pointers related to this priority among
net_device(s) for transmission.

Thanks & Regards,
Kumar.

^ permalink raw reply

* [PATCH net-next] drivers/net/vxge/vxge-main.c: Use pr_<level> and netdev_<level>
From: Joe Perches @ 2010-07-27 21:47 UTC (permalink / raw)
  To: Ramkrishna Vepa, Sreenivasa Honnur, Jon Mason
  Cc: David S. Miller, netdev, LKML

Use pr_fmt, pr_<level> and netdev_<level> where appropriate.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/net/vxge/vxge-main.c |   27 +++++++++++----------------
 1 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/drivers/net/vxge/vxge-main.c b/drivers/net/vxge/vxge-main.c
index 94d87e8..c7c5605 100644
--- a/drivers/net/vxge/vxge-main.c
+++ b/drivers/net/vxge/vxge-main.c
@@ -41,6 +41,8 @@
 *
 ******************************************************************************/
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/if_vlan.h>
 #include <linux/pci.h>
 #include <linux/slab.h>
@@ -144,7 +146,7 @@ vxge_callback_link_up(struct __vxge_hw_device *hldev)
 
 	vxge_debug_entryexit(VXGE_TRACE, "%s: %s:%d",
 		vdev->ndev->name, __func__, __LINE__);
-	printk(KERN_NOTICE "%s: Link Up\n", vdev->ndev->name);
+	netdev_notice(vdev->ndev, "Link Up\n");
 	vdev->stats.link_up++;
 
 	netif_carrier_on(vdev->ndev);
@@ -168,7 +170,7 @@ vxge_callback_link_down(struct __vxge_hw_device *hldev)
 
 	vxge_debug_entryexit(VXGE_TRACE,
 		"%s: %s:%d", vdev->ndev->name, __func__, __LINE__);
-	printk(KERN_NOTICE "%s: Link Down\n", vdev->ndev->name);
+	netdev_notice(vdev->ndev, "Link Down\n");
 
 	vdev->stats.link_down++;
 	netif_carrier_off(vdev->ndev);
@@ -2679,7 +2681,7 @@ vxge_open(struct net_device *dev)
 
 	if (vxge_hw_device_link_state_get(vdev->devh) == VXGE_HW_LINK_UP) {
 		netif_carrier_on(vdev->ndev);
-		printk(KERN_NOTICE "%s: Link Up\n", vdev->ndev->name);
+		netdev_notice(vdev->ndev, "Link Up\n");
 		vdev->stats.link_up++;
 	}
 
@@ -2817,7 +2819,7 @@ int do_vxge_close(struct net_device *dev, int do_io)
 	}
 
 	netif_carrier_off(vdev->ndev);
-	printk(KERN_NOTICE "%s: Link Down\n", vdev->ndev->name);
+	netdev_notice(vdev->ndev, "Link Down\n");
 	netif_tx_stop_all_queues(vdev->ndev);
 
 	/* Note that at this point xmit() is stopped by upper layer */
@@ -3844,9 +3846,7 @@ static pci_ers_result_t vxge_io_slot_reset(struct pci_dev *pdev)
 	struct vxgedev *vdev = netdev_priv(netdev);
 
 	if (pci_enable_device(pdev)) {
-		printk(KERN_ERR "%s: "
-			"Cannot re-enable device after reset\n",
-			VXGE_DRIVER_NAME);
+		netdev_err(netdev, "Cannot re-enable device after reset\n");
 		return PCI_ERS_RESULT_DISCONNECT;
 	}
 
@@ -3871,9 +3871,8 @@ static void vxge_io_resume(struct pci_dev *pdev)
 
 	if (netif_running(netdev)) {
 		if (vxge_open(netdev)) {
-			printk(KERN_ERR "%s: "
-				"Can't bring device back up after reset\n",
-				VXGE_DRIVER_NAME);
+			netdev_err(netdev,
+				   "Can't bring device back up after reset\n");
 			return;
 		}
 	}
@@ -4430,13 +4429,9 @@ static int __init
 vxge_starter(void)
 {
 	int ret = 0;
-	char version[32];
-	snprintf(version, 32, "%s", DRV_VERSION);
 
-	printk(KERN_INFO "%s: Copyright(c) 2002-2010 Exar Corp.\n",
-		VXGE_DRIVER_NAME);
-	printk(KERN_INFO "%s: Driver version: %s\n",
-			VXGE_DRIVER_NAME, version);
+	pr_info("Copyright(c) 2002-2010 Exar Corp.\n");
+	pr_info("Driver version: %s\n", DRV_VERSION);
 
 	verify_bandwidth();
 

^ permalink raw reply related

* Re: [PATCH net-next] drivers/net/vxge: Use pr_<level>, fix logging macros
From: Joe Perches @ 2010-07-27 21:43 UTC (permalink / raw)
  To: Jon Mason
  Cc: Ramkrishna Vepa, Sreenivasa Honnur, David S. Miller, netdev, LKML
In-Reply-To: <20100727213819.GC25556@exar.com>

On Tue, 2010-07-27 at 16:38 -0500, Jon Mason wrote:
> On Tue, Jul 27, 2010 at 01:43:08PM -0700, Joe Perches wrote:
> > Use direct printing of logging messages to save text.
> I have a similar patch queued internally (pending testing).

Great.

[]

> Why not make the entire vxge_debug part of vxge_debug_ll?  If
> disabled, that code can be removed completely as it is unnecessary.
> Also, why not call printk directly from vxge_debug?  This code had too
> many levels of indirection before, remove all of them (as that is way
> I did in the internal patch).

'cause I don't really want to figure out the nesting,
I just want it to be smaller and easier to understand.

> I did not have any of these changes in my patch.  If you want to push
> this seperately, I ack it.

I'll resend separately.

cheers, Joe


^ permalink raw reply

* Re: [PATCH net-next] drivers/net/vxge: Use pr_<level>, fix logging macros
From: Jon Mason @ 2010-07-27 21:38 UTC (permalink / raw)
  To: Joe Perches
  Cc: Ramkrishna Vepa, Sreenivasa Honnur, David S. Miller, netdev, LKML
In-Reply-To: <1280263388.24054.27.camel@Joe-Laptop.home>

On Tue, Jul 27, 2010 at 01:43:08PM -0700, Joe Perches wrote:
> Use pr_fmt, pr_<level> and netdev_<level> where appropriate.
> Use direct printing of logging messages to save text.
> 
> Reduces object size ~ 4k or 20k.
> 
> (x86 defconfig with vxge)
> $ size drivers/net/vxge/built-in.o*
>    text	   data	    bss	    dec	    hex	filename
>   68463	    784	      8	  69255	  10e87	drivers/net/vxge/built-in.o.new
>   72417	    784	      8	  73209	  11df9	drivers/net/vxge/built-in.o.old
> 
> (x86 allyesconfig)
> $ size drivers/net/vxge/built-in.o.*
>    text	   data	    bss	    dec	    hex	filename
>  125843	   1039	  27528	 154410	  25b2a	drivers/net/vxge/built-in.o.new
>  142589	   1039	  31024	 174652	  2aa3c	drivers/net/vxge/built-in.o.old
> 
> Signed-off-by: Joe Perches <joe@perches.com>

I have a similar patch queued internally (pending testing).

> ---
>  drivers/net/vxge/vxge-config.h |   38 ++++++++++++++++----------------------
>  drivers/net/vxge/vxge-main.c   |   27 +++++++++++----------------
>  2 files changed, 27 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/net/vxge/vxge-config.h b/drivers/net/vxge/vxge-config.h
> index 1a94343..dfe0e2d 100644
> --- a/drivers/net/vxge/vxge-config.h
> +++ b/drivers/net/vxge/vxge-config.h
> @@ -20,12 +20,8 @@
>  #define VXGE_CACHE_LINE_SIZE 128
>  #endif
>  
> -#define vxge_os_vaprintf(level, mask, fmt, ...) { \
> -	char buff[255]; \
> -		snprintf(buff, 255, fmt, __VA_ARGS__); \
> -		printk(buff); \
> -		printk("\n"); \
> -}
> +#define vxge_os_vaprintf(level, mask, fmt, ...) \
> +	printk(fmt "\n", ##__VA_ARGS__)
>  
>  #ifndef VXGE_ALIGN
>  #define VXGE_ALIGN(adrs, size) \
> @@ -2228,7 +2224,6 @@ vxge_hw_vpath_strip_fcs_check(struct __vxge_hw_device *hldev, u64 vpath_mask);
>   * vxge_debug
>   * @level: level of debug verbosity.
>   * @mask: mask for the debug
> - * @buf: Circular buffer for tracing
>   * @fmt: printf like format string
>   *
>   * Provides logging facilities. Can be customized on per-module
> @@ -2238,24 +2233,23 @@ vxge_hw_vpath_strip_fcs_check(struct __vxge_hw_device *hldev, u64 vpath_mask);
>   * See also: enum vxge_debug_level{}.
>   */
>  
> -#define vxge_trace_aux(level, mask, fmt, ...) \
> -{\
> -		vxge_os_vaprintf(level, mask, fmt, __VA_ARGS__);\
> -}
> +#define vxge_trace_aux(level, mask, fmt, ...)			\
> +	vxge_os_vaprintf(level, mask, fmt, ##__VA_ARGS__)
>  
> -#define vxge_debug(module, level, mask, fmt, ...) { \
> -if ((level >= VXGE_TRACE && ((module & VXGE_DEBUG_TRACE_MASK) == module)) || \
> -	(level >= VXGE_ERR && ((module & VXGE_DEBUG_ERR_MASK) == module))) {\
> -	if ((mask & VXGE_DEBUG_MASK) == mask)\
> -		vxge_trace_aux(level, mask, fmt, __VA_ARGS__); \
> -} \
> -}
> +#define vxge_debug(module, level, mask, fmt, ...)			\
> +do {									\
> +	if ((level >= VXGE_TRACE &&					\
> +	     ((module & VXGE_DEBUG_TRACE_MASK) == module)) ||		\
> +	    (level >= VXGE_ERR &&					\
> +	     ((module & VXGE_DEBUG_ERR_MASK) == module))) {		\
> +		if ((mask & VXGE_DEBUG_MASK) == mask)			\
> +			vxge_trace_aux(level, mask, fmt, ##__VA_ARGS__); \
> +	}								\
> +} while (0)
>  
>  #if (VXGE_COMPONENT_LL & VXGE_DEBUG_MODULE_MASK)
> -#define vxge_debug_ll(level, mask, fmt, ...) \
> -{\
> -	vxge_debug(VXGE_COMPONENT_LL, level, mask, fmt, __VA_ARGS__);\
> -}
> +#define vxge_debug_ll(level, mask, fmt, ...)				\
> +	vxge_debug(VXGE_COMPONENT_LL, level, mask, fmt, ##__VA_ARGS__)

Why not make the entire vxge_debug part of vxge_debug_ll?  If
disabled, that code can be removed completely as it is unnecessary.
Also, why not call printk directly from vxge_debug?  This code had too
many levels of indirection before, remove all of them (as that is way
I did in the internal patch).

>  #else
>  #define vxge_debug_ll(level, mask, fmt, ...)
> diff --git a/drivers/net/vxge/vxge-main.c b/drivers/net/vxge/vxge-main.c
> index 94d87e8..c7c5605 100644
> --- a/drivers/net/vxge/vxge-main.c
> +++ b/drivers/net/vxge/vxge-main.c
> @@ -41,6 +41,8 @@
>  *
>  ******************************************************************************/
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/if_vlan.h>
>  #include <linux/pci.h>
>  #include <linux/slab.h>
> @@ -144,7 +146,7 @@ vxge_callback_link_up(struct __vxge_hw_device *hldev)
>  
>  	vxge_debug_entryexit(VXGE_TRACE, "%s: %s:%d",
>  		vdev->ndev->name, __func__, __LINE__);
> -	printk(KERN_NOTICE "%s: Link Up\n", vdev->ndev->name);
> +	netdev_notice(vdev->ndev, "Link Up\n");
>  	vdev->stats.link_up++;
>  
>  	netif_carrier_on(vdev->ndev);
> @@ -168,7 +170,7 @@ vxge_callback_link_down(struct __vxge_hw_device *hldev)
>  
>  	vxge_debug_entryexit(VXGE_TRACE,
>  		"%s: %s:%d", vdev->ndev->name, __func__, __LINE__);
> -	printk(KERN_NOTICE "%s: Link Down\n", vdev->ndev->name);
> +	netdev_notice(vdev->ndev, "Link Down\n");
>  
>  	vdev->stats.link_down++;
>  	netif_carrier_off(vdev->ndev);
> @@ -2679,7 +2681,7 @@ vxge_open(struct net_device *dev)
>  
>  	if (vxge_hw_device_link_state_get(vdev->devh) == VXGE_HW_LINK_UP) {
>  		netif_carrier_on(vdev->ndev);
> -		printk(KERN_NOTICE "%s: Link Up\n", vdev->ndev->name);
> +		netdev_notice(vdev->ndev, "Link Up\n");
>  		vdev->stats.link_up++;
>  	}
>  
> @@ -2817,7 +2819,7 @@ int do_vxge_close(struct net_device *dev, int do_io)
>  	}
>  
>  	netif_carrier_off(vdev->ndev);
> -	printk(KERN_NOTICE "%s: Link Down\n", vdev->ndev->name);
> +	netdev_notice(vdev->ndev, "Link Down\n");
>  	netif_tx_stop_all_queues(vdev->ndev);
>  
>  	/* Note that at this point xmit() is stopped by upper layer */
> @@ -3844,9 +3846,7 @@ static pci_ers_result_t vxge_io_slot_reset(struct pci_dev *pdev)
>  	struct vxgedev *vdev = netdev_priv(netdev);
>  
>  	if (pci_enable_device(pdev)) {
> -		printk(KERN_ERR "%s: "
> -			"Cannot re-enable device after reset\n",
> -			VXGE_DRIVER_NAME);
> +		netdev_err(netdev, "Cannot re-enable device after reset\n");
>  		return PCI_ERS_RESULT_DISCONNECT;
>  	}
>  
> @@ -3871,9 +3871,8 @@ static void vxge_io_resume(struct pci_dev *pdev)
>  
>  	if (netif_running(netdev)) {
>  		if (vxge_open(netdev)) {
> -			printk(KERN_ERR "%s: "
> -				"Can't bring device back up after reset\n",
> -				VXGE_DRIVER_NAME);
> +			netdev_err(netdev,
> +				   "Can't bring device back up after reset\n");
>  			return;
>  		}
>  	}
> @@ -4430,13 +4429,9 @@ static int __init
>  vxge_starter(void)
>  {
>  	int ret = 0;
> -	char version[32];
> -	snprintf(version, 32, "%s", DRV_VERSION);
>  
> -	printk(KERN_INFO "%s: Copyright(c) 2002-2010 Exar Corp.\n",
> -		VXGE_DRIVER_NAME);
> -	printk(KERN_INFO "%s: Driver version: %s\n",
> -			VXGE_DRIVER_NAME, version);
> +	pr_info("Copyright(c) 2002-2010 Exar Corp.\n");
> +	pr_info("Driver version: %s\n", DRV_VERSION);
>  
>  	verify_bandwidth();

I did not have any of these changes in my patch.  If you want to push
this seperately, I ack it.

Thanks,
Jon

>  
> 
> 

^ permalink raw reply

* Re: [PATCH] Driver-core: Fix bluetooth network device rename  regression
From: Eric W. Biederman @ 2010-07-27 20:53 UTC (permalink / raw)
  To: Kay Sievers
  Cc: Greg KH, Greg KH, Johannes Berg, Andrew Morton, Rafael J. Wysocki,
	Maciej W. Rozycki, netdev
In-Reply-To: <AANLkTi=LB7qV3S1miOocpQCtCaxhqJsLS51UDcnGjK+u@mail.gmail.com>

Kay Sievers <kay.sievers@vrfy.org> writes:

> On Tue, Jul 27, 2010 at 20:44, Eric W. Biederman <ebiederm@xmission.com> wrote:
>> Greg KH <greg@kroah.com> writes:
>>> On Tue, Jul 27, 2010 at 08:24:31PM +0200, Kay Sievers wrote:
>>>> On Tue, Jul 27, 2010 at 20:17, Eric W. Biederman <ebiederm@xmission.com> wrote:
>>>> > Greg KH <greg@kroah.com> writes:
>>>>
>>>> > Let me say this again clearly.
>>>> >
>>>> > class -> bus BROKEN
>>>> >
>>>> > In the case of bluetooth it would require changing /sys/class/bluetooth
>>>> > to /sys/bus/bluetooth. ??Which is user visible in the worst possible
>>>> > way and quick google search confirmed it will break user space scripts.
>>>>
>>>> Clearly, we even have a compat API for that, and nothing would break.
>>>> If needed, a bus can create compat class links. Did you even check
>>>> with the bluetooth guys, last time I talked to them, they have been
>>>> totally fine with such a change.
>>
>> Hmm.  It appears I missed the compat API in my review of the bus code.
>> Could I get a pointer?
>
> It's:
>   class_compat_create_link()
>   http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=drivers/base/class.c;h=8e231d05b40058c6c3191b4a06a15ceff1714be3;hb=HEAD#l559
>
>
> I think i2c uses this when the class was converted to a bus.

> It should only be used if it's really needed for known used userspace
> interfaces. A few others that got converted already did not need it.

Interesting.  The symlink creation is slightly buggy in that it is
created after the uevent for device creation has been sent.  Which can
lead to some interesting races in userspace.

As for the rest the bus compat code is similar but not quite the same
as the class code, so I would be extremely reluctant to deploy it
except in extremely limited cases.  Backwards compatibility is
important, and we should strive our best to maintain backwards
compatibility it for the kernel<->userspace ABIs.

Eric

^ permalink raw reply

* Re: [PATCH net-next] drivers/net/bfin_mac.c: Use pr_fmt, netdev_<level>
From: Joe Perches @ 2010-07-27 20:52 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Michael Hennerich, uclinux-dist-devel, netdev, LKML
In-Reply-To: <20100727134417.62420ec1@nehalam>

On Tue, 2010-07-27 at 13:44 -0700, Stephen Hemminger wrote:
> On Tue, 27 Jul 2010 12:22:11 -0700
> Joe Perches <joe@perches.com> wrote:
> > +static struct sk_buff *bfin_alloc_skb(void)
> > +{
> > +	/* allocate a new skb */
> > +	struct sk_buff *new_skb = dev_alloc_skb(PKT_BUF_SIZE + NET_IP_ALIGN);
> > +
> > +	if (!new_skb)
> > +		return NULL;
> > +
> > +	skb_reserve(new_skb, NET_IP_ALIGN);
>
> Why not use netdev_alloc_skb_ip_align ?

I was consolidating duplicated code.

It could be changed, but right now, desc_list_init isn't
passed a struct netdevice.

Perhaps you'll submit that as a follow-on?

^ permalink raw reply

* Make vhost multi-threaded and associate each thread to its guest's cgroup
From: Michael S. Tsirkin @ 2010-07-27 20:42 UTC (permalink / raw)
  To: Sridhar Samudrala; +Cc: netdev, lkml, kvm@vger.kernel.org, Tejun Heo, Li Zefan

Sridhar,
I pushed a patchset with all known issues fixed,
on my vhost-net-next branch.

For now this ignores the cpu mask issue, addressing
only the cgroups issue.

Would appreciate testing and reports.

Thanks,
-- 
MST

^ permalink raw reply

* Re: [PATCH net-next] drivers/net/bfin_mac.c: Use pr_fmt, netdev_<level>
From: Stephen Hemminger @ 2010-07-27 20:44 UTC (permalink / raw)
  To: Joe Perches; +Cc: Michael Hennerich, uclinux-dist-devel, netdev, LKML
In-Reply-To: <1280258531.24054.10.camel@Joe-Laptop.home>

On Tue, 27 Jul 2010 12:22:11 -0700
Joe Perches <joe@perches.com> wrote:

>  
> +static struct sk_buff *bfin_alloc_skb(void)
> +{
> +	/* allocate a new skb */
> +	struct sk_buff *new_skb = dev_alloc_skb(PKT_BUF_SIZE + NET_IP_ALIGN);
> +
> +	if (!new_skb)
> +		return NULL;
> +
> +	skb_reserve(new_skb, NET_IP_ALIGN);

Why not use netdev_alloc_skb_ip_align ?

-- 

^ permalink raw reply

* [PATCH net-next] drivers/net/vxge: Use pr_<level>, fix logging macros
From: Joe Perches @ 2010-07-27 20:43 UTC (permalink / raw)
  To: Ramkrishna Vepa, Sreenivasa Honnur, Jon Mason
  Cc: David S. Miller, netdev, LKML

Use pr_fmt, pr_<level> and netdev_<level> where appropriate.
Use direct printing of logging messages to save text.

Reduces object size ~ 4k or 20k.

(x86 defconfig with vxge)
$ size drivers/net/vxge/built-in.o*
   text	   data	    bss	    dec	    hex	filename
  68463	    784	      8	  69255	  10e87	drivers/net/vxge/built-in.o.new
  72417	    784	      8	  73209	  11df9	drivers/net/vxge/built-in.o.old

(x86 allyesconfig)
$ size drivers/net/vxge/built-in.o.*
   text	   data	    bss	    dec	    hex	filename
 125843	   1039	  27528	 154410	  25b2a	drivers/net/vxge/built-in.o.new
 142589	   1039	  31024	 174652	  2aa3c	drivers/net/vxge/built-in.o.old

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/net/vxge/vxge-config.h |   38 ++++++++++++++++----------------------
 drivers/net/vxge/vxge-main.c   |   27 +++++++++++----------------
 2 files changed, 27 insertions(+), 38 deletions(-)

diff --git a/drivers/net/vxge/vxge-config.h b/drivers/net/vxge/vxge-config.h
index 1a94343..dfe0e2d 100644
--- a/drivers/net/vxge/vxge-config.h
+++ b/drivers/net/vxge/vxge-config.h
@@ -20,12 +20,8 @@
 #define VXGE_CACHE_LINE_SIZE 128
 #endif
 
-#define vxge_os_vaprintf(level, mask, fmt, ...) { \
-	char buff[255]; \
-		snprintf(buff, 255, fmt, __VA_ARGS__); \
-		printk(buff); \
-		printk("\n"); \
-}
+#define vxge_os_vaprintf(level, mask, fmt, ...) \
+	printk(fmt "\n", ##__VA_ARGS__)
 
 #ifndef VXGE_ALIGN
 #define VXGE_ALIGN(adrs, size) \
@@ -2228,7 +2224,6 @@ vxge_hw_vpath_strip_fcs_check(struct __vxge_hw_device *hldev, u64 vpath_mask);
  * vxge_debug
  * @level: level of debug verbosity.
  * @mask: mask for the debug
- * @buf: Circular buffer for tracing
  * @fmt: printf like format string
  *
  * Provides logging facilities. Can be customized on per-module
@@ -2238,24 +2233,23 @@ vxge_hw_vpath_strip_fcs_check(struct __vxge_hw_device *hldev, u64 vpath_mask);
  * See also: enum vxge_debug_level{}.
  */
 
-#define vxge_trace_aux(level, mask, fmt, ...) \
-{\
-		vxge_os_vaprintf(level, mask, fmt, __VA_ARGS__);\
-}
+#define vxge_trace_aux(level, mask, fmt, ...)			\
+	vxge_os_vaprintf(level, mask, fmt, ##__VA_ARGS__)
 
-#define vxge_debug(module, level, mask, fmt, ...) { \
-if ((level >= VXGE_TRACE && ((module & VXGE_DEBUG_TRACE_MASK) == module)) || \
-	(level >= VXGE_ERR && ((module & VXGE_DEBUG_ERR_MASK) == module))) {\
-	if ((mask & VXGE_DEBUG_MASK) == mask)\
-		vxge_trace_aux(level, mask, fmt, __VA_ARGS__); \
-} \
-}
+#define vxge_debug(module, level, mask, fmt, ...)			\
+do {									\
+	if ((level >= VXGE_TRACE &&					\
+	     ((module & VXGE_DEBUG_TRACE_MASK) == module)) ||		\
+	    (level >= VXGE_ERR &&					\
+	     ((module & VXGE_DEBUG_ERR_MASK) == module))) {		\
+		if ((mask & VXGE_DEBUG_MASK) == mask)			\
+			vxge_trace_aux(level, mask, fmt, ##__VA_ARGS__); \
+	}								\
+} while (0)
 
 #if (VXGE_COMPONENT_LL & VXGE_DEBUG_MODULE_MASK)
-#define vxge_debug_ll(level, mask, fmt, ...) \
-{\
-	vxge_debug(VXGE_COMPONENT_LL, level, mask, fmt, __VA_ARGS__);\
-}
+#define vxge_debug_ll(level, mask, fmt, ...)				\
+	vxge_debug(VXGE_COMPONENT_LL, level, mask, fmt, ##__VA_ARGS__)
 
 #else
 #define vxge_debug_ll(level, mask, fmt, ...)
diff --git a/drivers/net/vxge/vxge-main.c b/drivers/net/vxge/vxge-main.c
index 94d87e8..c7c5605 100644
--- a/drivers/net/vxge/vxge-main.c
+++ b/drivers/net/vxge/vxge-main.c
@@ -41,6 +41,8 @@
 *
 ******************************************************************************/
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/if_vlan.h>
 #include <linux/pci.h>
 #include <linux/slab.h>
@@ -144,7 +146,7 @@ vxge_callback_link_up(struct __vxge_hw_device *hldev)
 
 	vxge_debug_entryexit(VXGE_TRACE, "%s: %s:%d",
 		vdev->ndev->name, __func__, __LINE__);
-	printk(KERN_NOTICE "%s: Link Up\n", vdev->ndev->name);
+	netdev_notice(vdev->ndev, "Link Up\n");
 	vdev->stats.link_up++;
 
 	netif_carrier_on(vdev->ndev);
@@ -168,7 +170,7 @@ vxge_callback_link_down(struct __vxge_hw_device *hldev)
 
 	vxge_debug_entryexit(VXGE_TRACE,
 		"%s: %s:%d", vdev->ndev->name, __func__, __LINE__);
-	printk(KERN_NOTICE "%s: Link Down\n", vdev->ndev->name);
+	netdev_notice(vdev->ndev, "Link Down\n");
 
 	vdev->stats.link_down++;
 	netif_carrier_off(vdev->ndev);
@@ -2679,7 +2681,7 @@ vxge_open(struct net_device *dev)
 
 	if (vxge_hw_device_link_state_get(vdev->devh) == VXGE_HW_LINK_UP) {
 		netif_carrier_on(vdev->ndev);
-		printk(KERN_NOTICE "%s: Link Up\n", vdev->ndev->name);
+		netdev_notice(vdev->ndev, "Link Up\n");
 		vdev->stats.link_up++;
 	}
 
@@ -2817,7 +2819,7 @@ int do_vxge_close(struct net_device *dev, int do_io)
 	}
 
 	netif_carrier_off(vdev->ndev);
-	printk(KERN_NOTICE "%s: Link Down\n", vdev->ndev->name);
+	netdev_notice(vdev->ndev, "Link Down\n");
 	netif_tx_stop_all_queues(vdev->ndev);
 
 	/* Note that at this point xmit() is stopped by upper layer */
@@ -3844,9 +3846,7 @@ static pci_ers_result_t vxge_io_slot_reset(struct pci_dev *pdev)
 	struct vxgedev *vdev = netdev_priv(netdev);
 
 	if (pci_enable_device(pdev)) {
-		printk(KERN_ERR "%s: "
-			"Cannot re-enable device after reset\n",
-			VXGE_DRIVER_NAME);
+		netdev_err(netdev, "Cannot re-enable device after reset\n");
 		return PCI_ERS_RESULT_DISCONNECT;
 	}
 
@@ -3871,9 +3871,8 @@ static void vxge_io_resume(struct pci_dev *pdev)
 
 	if (netif_running(netdev)) {
 		if (vxge_open(netdev)) {
-			printk(KERN_ERR "%s: "
-				"Can't bring device back up after reset\n",
-				VXGE_DRIVER_NAME);
+			netdev_err(netdev,
+				   "Can't bring device back up after reset\n");
 			return;
 		}
 	}
@@ -4430,13 +4429,9 @@ static int __init
 vxge_starter(void)
 {
 	int ret = 0;
-	char version[32];
-	snprintf(version, 32, "%s", DRV_VERSION);
 
-	printk(KERN_INFO "%s: Copyright(c) 2002-2010 Exar Corp.\n",
-		VXGE_DRIVER_NAME);
-	printk(KERN_INFO "%s: Driver version: %s\n",
-			VXGE_DRIVER_NAME, version);
+	pr_info("Copyright(c) 2002-2010 Exar Corp.\n");
+	pr_info("Driver version: %s\n", DRV_VERSION);
 
 	verify_bandwidth();
 

^ permalink raw reply related

* Re: br_forward.c - rcu dereference warning
From: Stephen Hemminger @ 2010-07-27 20:42 UTC (permalink / raw)
  To: Johannes Berg; +Cc: paulmck, netdev
In-Reply-To: <1280261679.3755.6.camel@jlt3.sipsolutions.net>

On Tue, 27 Jul 2010 22:14:39 +0200
Johannes Berg <johannes@sipsolutions.net> wrote:

> On Tue, 2010-07-27 at 11:26 -0700, Stephen Hemminger wrote:
> 
> > -/* net device transmit always called with no BH (preempt_disabled) */
> > +/* net device transmit always called with no BH disabled */
> 
> "no BH disabled"?
> 
> > @@ -48,6 +48,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *
> >  	skb_reset_mac_header(skb);
> >  	skb_pull(skb, ETH_HLEN);
> >  
> > +	rcu_read_lock();
> >  	if (is_multicast_ether_addr(dest)) {
> >  		if (unlikely(netpoll_tx_running(dev))) {
> >  			br_flood_deliver(br, skb);
> > @@ -67,6 +68,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *
> >  		br_flood_deliver(br, skb);
> >  
> >  out:
> > +	rcu_read_unlock();
> >  	return NETDEV_TX_OK;
> >  }
> >  
> > --- a/net/bridge/br_fdb.c	2010-07-27 11:18:30.815320981 -0700
> > +++ b/net/bridge/br_fdb.c	2010-07-27 11:18:59.597710975 -0700
> > @@ -214,7 +214,7 @@ void br_fdb_delete_by_port(struct net_br
> >  	spin_unlock_bh(&br->hash_lock);
> >  }
> >  
> > -/* No locking or refcounting, assumes caller has no preempt (rcu_read_lock) */
> > +/* No locking or refcounting, assumes caller has rcu_read_lock */
> >  struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
> >  					  const unsigned char *addr)
> >  {
> > --- a/net/bridge/br_input.c	2010-07-27 11:19:05.266181492 -0700
> > +++ b/net/bridge/br_input.c	2010-07-27 11:19:29.148163149 -0700
> > @@ -39,7 +39,7 @@ static int br_pass_frame_up(struct sk_bu
> >  		       netif_receive_skb);
> >  }
> >  
> > -/* note: already called with rcu_read_lock (preempt_disabled) */
> > +/* note: already called with rcu_read_lock */
> >  int br_handle_frame_finish(struct sk_buff *skb)
> >  {
> >  	const unsigned char *dest = eth_hdr(skb)->h_dest;
> > @@ -110,7 +110,7 @@ drop:
> >  	goto out;
> >  }
> >  
> > -/* note: already called with rcu_read_lock (preempt_disabled) */
> > +/* note: already called with rcu_read_lock */
> >  static int br_handle_local_finish(struct sk_buff *skb)
> >  {
> >  	struct net_bridge_port *p = br_port_get_rcu(skb->dev);
> > @@ -133,8 +133,7 @@ static inline int is_link_local(const un
> >  
> >  /*
> >   * Return NULL if skb is handled
> > - * note: already called with rcu_read_lock (preempt_disabled) from
> > - * netif_receive_skb
> > + * note: already called with rcu_read_lock from netif_receive_skb
> >   */
> >  struct sk_buff *br_handle_frame(struct sk_buff *skb)
> >  {
> > --- a/net/bridge/br_stp_bpdu.c	2010-07-27 11:19:34.092573294 -0700
> > +++ b/net/bridge/br_stp_bpdu.c	2010-07-27 11:19:40.725123403 -0700
> > @@ -131,7 +131,7 @@ void br_send_tcn_bpdu(struct net_bridge_
> >  /*
> >   * Called from llc.
> >   *
> > - * NO locks, but rcu_read_lock (preempt_disabled)
> > + * NO locks, but rcu_read_lock
> >   */
> >  void br_stp_rcv(const struct stp_proto *proto, struct sk_buff *skb,
> >  		struct net_device *dev)
> > 
> 
> Did you want me to test the patch?

Yes please, I can make sure it works, but not that it gets rid
of your error

-- 

^ permalink raw reply

* Re: [net-next-2.6 PATCH] igbvf, ixgbevf: use dev_hw_addr_random
From: David Miller @ 2010-07-27 20:18 UTC (permalink / raw)
  To: sassmann; +Cc: netdev, e1000-devel, jeffrey.t.kirsher, gregory.v.rose
In-Reply-To: <4C4EA5E2.1050909@redhat.com>

From: Stefan Assmann <sassmann@redhat.com>
Date: Tue, 27 Jul 2010 11:24:50 +0200

> From: Stefan Assmann <sassmann@redhat.com>
> 
> Both igbvf and ixgbevf should set addr_assign_type to NET_ADDR_RANDOM
> so udev creates persistent net rules by matching the device path.
> Do this by using the dev_hw_addr_random helper function.
> 
> Signed-off-by: Stefan Assmann <sassmann@redhat.com>

Applied.

^ permalink raw reply

* Re: [net-next-2.6 PATCH] ixgbe: priority tagging FCoE frames without FCoE offload
From: David Miller @ 2010-07-27 20:18 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips, yi.zou, john.r.fastabend
In-Reply-To: <20100727064058.24599.57844.stgit@localhost.localdomain>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 26 Jul 2010 23:41:31 -0700

> From: John Fastabend <john.r.fastabend@intel.com>
> 
> The DCB user priority for FCoE is available regardless of whether
> FCoE offload is enabled (IXGBE_FLAG_FCOE_ENABLED bit is set).
> This allows proper DCB user priority tagging for FCoE
> traffic on both 82598 and 82599 devices.
> 
> Signed-off-by: Yi Zou <yi.zou@intel.com>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> Tested-by: Ross Brattain <ross.b.brattain@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next 0/7] bnx2x: move bnx2x to separate folder and divide to files
From: David Miller @ 2010-07-27 20:16 UTC (permalink / raw)
  To: dmitry; +Cc: netdev, eilong
In-Reply-To: <1280257298.8566.8.camel@lb-tlvb-dmitry>

From: "Dmitry Kravkov" <dmitry@broadcom.com>
Date: Tue, 27 Jul 2010 22:01:38 +0300

> Please consider applying following patch series to the net-next.
> The series creates separate folder for bxn2x files and divides
> bnx2x_main file into several logical parts:
> 	bnx2x_cmn.[ch] - common functionality for PF and upcommimg 
> 			 VF drivers
> 	bnx2x_stat.[ch] - statistics handling
> 	ethtool.c - ethtool related function and structs
> 
> After applying this series, integration of latest bnx2x patches
> from net-2.6 which include fixes for statistics handling can't
> be performed in a "automatic" way then i can suggest to re-submitting
> these two patches.

These patches generate the following warnings from GIT
when applying:

Applying: bnx2x: Create separate folder for bnx2x driver
/home/davem/src/GIT/net-next-2.6/.git/rebase-apply/patch:49: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
Applying: bnx2x: store module parameters in driver main structure
Applying: bnx2x: move global variable load_count to bnx2x.h
Applying: bnx2x: Create bnx2x_cmn.* files
/home/davem/src/GIT/net-next-2.6/.git/rebase-apply/patch:39: new blank line at EOF.
+
/home/davem/src/GIT/net-next-2.6/.git/rebase-apply/patch:2297: new blank line at EOF.
+
warning: 2 lines add whitespace errors.
Applying: bnx2x: Create separate file for ethtool routines
/home/davem/src/GIT/net-next-2.6/.git/rebase-apply/patch:58: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
Applying: bnx2x: Move statistics handling code to bnx2x_stats.*
Applying: bnx2x: update driver version to 1.52.53-3
Applying: bnx2x: Update MAINTAINERS according to new location

Fix them up and resubmit the series, thanks.

^ permalink raw reply

* Re: br_forward.c - rcu dereference warning
From: Johannes Berg @ 2010-07-27 20:14 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: paulmck, netdev
In-Reply-To: <20100727112630.5a5ce84b@nehalam>

On Tue, 2010-07-27 at 11:26 -0700, Stephen Hemminger wrote:

> -/* net device transmit always called with no BH (preempt_disabled) */
> +/* net device transmit always called with no BH disabled */

"no BH disabled"?

> @@ -48,6 +48,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *
>  	skb_reset_mac_header(skb);
>  	skb_pull(skb, ETH_HLEN);
>  
> +	rcu_read_lock();
>  	if (is_multicast_ether_addr(dest)) {
>  		if (unlikely(netpoll_tx_running(dev))) {
>  			br_flood_deliver(br, skb);
> @@ -67,6 +68,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *
>  		br_flood_deliver(br, skb);
>  
>  out:
> +	rcu_read_unlock();
>  	return NETDEV_TX_OK;
>  }
>  
> --- a/net/bridge/br_fdb.c	2010-07-27 11:18:30.815320981 -0700
> +++ b/net/bridge/br_fdb.c	2010-07-27 11:18:59.597710975 -0700
> @@ -214,7 +214,7 @@ void br_fdb_delete_by_port(struct net_br
>  	spin_unlock_bh(&br->hash_lock);
>  }
>  
> -/* No locking or refcounting, assumes caller has no preempt (rcu_read_lock) */
> +/* No locking or refcounting, assumes caller has rcu_read_lock */
>  struct net_bridge_fdb_entry *__br_fdb_get(struct net_bridge *br,
>  					  const unsigned char *addr)
>  {
> --- a/net/bridge/br_input.c	2010-07-27 11:19:05.266181492 -0700
> +++ b/net/bridge/br_input.c	2010-07-27 11:19:29.148163149 -0700
> @@ -39,7 +39,7 @@ static int br_pass_frame_up(struct sk_bu
>  		       netif_receive_skb);
>  }
>  
> -/* note: already called with rcu_read_lock (preempt_disabled) */
> +/* note: already called with rcu_read_lock */
>  int br_handle_frame_finish(struct sk_buff *skb)
>  {
>  	const unsigned char *dest = eth_hdr(skb)->h_dest;
> @@ -110,7 +110,7 @@ drop:
>  	goto out;
>  }
>  
> -/* note: already called with rcu_read_lock (preempt_disabled) */
> +/* note: already called with rcu_read_lock */
>  static int br_handle_local_finish(struct sk_buff *skb)
>  {
>  	struct net_bridge_port *p = br_port_get_rcu(skb->dev);
> @@ -133,8 +133,7 @@ static inline int is_link_local(const un
>  
>  /*
>   * Return NULL if skb is handled
> - * note: already called with rcu_read_lock (preempt_disabled) from
> - * netif_receive_skb
> + * note: already called with rcu_read_lock from netif_receive_skb
>   */
>  struct sk_buff *br_handle_frame(struct sk_buff *skb)
>  {
> --- a/net/bridge/br_stp_bpdu.c	2010-07-27 11:19:34.092573294 -0700
> +++ b/net/bridge/br_stp_bpdu.c	2010-07-27 11:19:40.725123403 -0700
> @@ -131,7 +131,7 @@ void br_send_tcn_bpdu(struct net_bridge_
>  /*
>   * Called from llc.
>   *
> - * NO locks, but rcu_read_lock (preempt_disabled)
> + * NO locks, but rcu_read_lock
>   */
>  void br_stp_rcv(const struct stp_proto *proto, struct sk_buff *skb,
>  		struct net_device *dev)
> 

Did you want me to test the patch?

johannes


^ permalink raw reply

* [PATCH net-next 8/7] bnx2x: Update MAINTAINERS according to new location
From: Dmitry Kravkov @ 2010-07-27 20:09 UTC (permalink / raw)
  To: davem, netdev; +Cc: joe, eilong

Thanks to Joe for noticing

Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
 MAINTAINERS |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index aa73c9a..1e4eb63 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1365,7 +1365,7 @@ BROADCOM BNX2X 10 GIGABIT ETHERNET DRIVER
 M:	Eilon Greenstein <eilong@broadcom.com>
 L:	netdev@vger.kernel.org
 S:	Supported
-F:	drivers/net/bnx2x*
+F:	drivers/net/bnx2x/
 
 BROADCOM TG3 GIGABIT ETHERNET DRIVER
 M:	Matt Carlson <mcarlson@broadcom.com>
-- 
1.7.1





^ permalink raw reply related

* Re: [PATCH net-next 0/7] bnx2x: move bnx2x to separate folder and divide to files
From: Dmitry Kravkov @ 2010-07-27 20:09 UTC (permalink / raw)
  To: davem@davemloft.net; +Cc: netdev@vger.kernel.org, Eilon Greenstein
In-Reply-To: <1280257298.8566.8.camel@lb-tlvb-dmitry>

PATCH 8/7 will include MAINTAINERS update for new location

On Tue, 2010-07-27 at 12:01 -0700, Dmitry Kravkov wrote:
> Hi,
> Please consider applying following patch series to the net-next.
> The series creates separate folder for bxn2x files and divides
> bnx2x_main file into several logical parts:
> 	bnx2x_cmn.[ch] - common functionality for PF and upcommimg 
> 			 VF drivers
> 	bnx2x_stat.[ch] - statistics handling
> 	ethtool.c - ethtool related function and structs
> 
> After applying this series, integration of latest bnx2x patches
> from net-2.6 which include fixes for statistics handling can't
> be performed in a "automatic" way then i can suggest to re-submitting
> these two patches.
> 
> Dmitry
> ---
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 




^ permalink raw reply

* Re: [PATCH][2.6.32.y] igb: change how we handle alternate mac addresses
From: Brandon Philips @ 2010-07-27 19:47 UTC (permalink / raw)
  To: stable, Alexander Duyck, Jeff Kirsher; +Cc: netdev
In-Reply-To: <20100727164825.GH16596@mmm.home.ifup.org>

On 09:48 Tue 27 Jul 2010, bphilips@suse.de wrote:
> Without this patch bonding with igb doesn't work correctly. Applies to
> 2.6.32.y.

Sorry, I missed the CCing netdev. -stable patch proposal for 2.6.32.y

Thanks, Brandon

> From 22896639af98ebc721a94ed71fc3acf2fb4a24dc Mon Sep 17 00:00:00 2001
> From: Alexander Duyck <alexander.h.duyck@intel.com>
> Date: Mon, 5 Oct 2009 06:34:25 +0000
> Subject: [PATCH] igb: change how we handle alternate mac addresses
> 
> This patch allows us to treat the alternate mac address as though it is the
> physical address on the adapter.  This is accomplished by letting the
> alt_mac_address function to only fail on an NVM error.  If no errors occur
> and the alternate mac address is not present then RAR0 is read as the
> default mac address.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
>  drivers/net/igb/e1000_82575.c |   13 +++++++++++--
>  drivers/net/igb/e1000_hw.h    |    2 ++
>  drivers/net/igb/e1000_mac.c   |   17 +++++++++--------
>  3 files changed, 22 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/igb/e1000_82575.c b/drivers/net/igb/e1000_82575.c
> index e07f66c..45063c2 100644
> --- a/drivers/net/igb/e1000_82575.c
> +++ b/drivers/net/igb/e1000_82575.c
> @@ -1152,9 +1152,18 @@ static s32 igb_read_mac_addr_82575(struct e1000_hw *hw)
>  {
>  	s32 ret_val = 0;
>  
> -	if (igb_check_alt_mac_addr(hw))
> -		ret_val = igb_read_mac_addr(hw);
> +	/*
> +	 * If there's an alternate MAC address place it in RAR0
> +	 * so that it will override the Si installed default perm
> +	 * address.
> +	 */
> +	ret_val = igb_check_alt_mac_addr(hw);
> +	if (ret_val)
> +		goto out;
> +
> +	ret_val = igb_read_mac_addr(hw);
>  
> +out:
>  	return ret_val;
>  }
>  
> diff --git a/drivers/net/igb/e1000_hw.h b/drivers/net/igb/e1000_hw.h
> index 4e7850d..fad7cf5 100644
> --- a/drivers/net/igb/e1000_hw.h
> +++ b/drivers/net/igb/e1000_hw.h
> @@ -54,6 +54,8 @@ struct e1000_hw;
>  #define E1000_FUNC_0     0
>  #define E1000_FUNC_1     1
>  
> +#define E1000_ALT_MAC_ADDRESS_OFFSET_LAN1   3
> +
>  enum e1000_mac_type {
>  	e1000_undefined = 0,
>  	e1000_82575,
> diff --git a/drivers/net/igb/e1000_mac.c b/drivers/net/igb/e1000_mac.c
> index 986aa90..4969a5b 100644
> --- a/drivers/net/igb/e1000_mac.c
> +++ b/drivers/net/igb/e1000_mac.c
> @@ -185,13 +185,12 @@ s32 igb_check_alt_mac_addr(struct e1000_hw *hw)
>  	}
>  
>  	if (nvm_alt_mac_addr_offset == 0xFFFF) {
> -		ret_val = -(E1000_NOT_IMPLEMENTED);
> +		/* There is no Alternate MAC Address */
>  		goto out;
>  	}
>  
>  	if (hw->bus.func == E1000_FUNC_1)
> -		nvm_alt_mac_addr_offset += ETH_ALEN/sizeof(u16);
> -
> +		nvm_alt_mac_addr_offset += E1000_ALT_MAC_ADDRESS_OFFSET_LAN1;
>  	for (i = 0; i < ETH_ALEN; i += 2) {
>  		offset = nvm_alt_mac_addr_offset + (i >> 1);
>  		ret_val = hw->nvm.ops.read(hw, offset, 1, &nvm_data);
> @@ -206,14 +205,16 @@ s32 igb_check_alt_mac_addr(struct e1000_hw *hw)
>  
>  	/* if multicast bit is set, the alternate address will not be used */
>  	if (alt_mac_addr[0] & 0x01) {
> -		ret_val = -(E1000_NOT_IMPLEMENTED);
> +		hw_dbg("Ignoring Alternate Mac Address with MC bit set\n");
>  		goto out;
>  	}
>  
> -	for (i = 0; i < ETH_ALEN; i++)
> -		hw->mac.addr[i] = hw->mac.perm_addr[i] = alt_mac_addr[i];
> -
> -	hw->mac.ops.rar_set(hw, hw->mac.perm_addr, 0);
> +	/*
> +	 * We have a valid alternate MAC address, and we want to treat it the
> +	 * same as the normal permanent MAC address stored by the HW into the
> +	 * RAR. Do this by mapping this address into RAR0.
> +	 */
> +	hw->mac.ops.rar_set(hw, alt_mac_addr, 0);
>  
>  out:
>  	return ret_val;
> -- 
> 1.7.1
> 

^ permalink raw reply

* Re: Tx queue selection
From: Ben Hutchings @ 2010-07-27 19:31 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: netdev
In-Reply-To: <1280227867.1970.208.camel@pasglop>

On Tue, 2010-07-27 at 20:51 +1000, Benjamin Herrenschmidt wrote:
> Hi folks !
> 
> I'm putting my newbie hat on ... :-)
> 
> While looking at our ehea driver (and in fact another upcoming driver
> I'm helping with), I noticed it's using the "old style" multiqueue. IE.
> It doesn't use the alloc_netdev_mq() variant, creates one queue on the
> linux side, an makes its own selection of HW queue in start_xmit.
> 
> This had many drawbacks, obviously, such as not getting per-queue locks
> etc...
> 
> Now, the mechanics of converting that to the new scheme are easy enough
> to figure out by reading the code. However, where my lack of networking
> background fails me is when it comes to the policy of choosing a Tx
> queue.
> 
> ehea uses its own hash of the header, different from the "default" queue
> selection in the net core. Looking at other drivers such as ixgbe, I see
> that it can chose to use smp_processor_id() when a flag is set for which
> I don't totally understand the meaning or default to the core algorithm.
>
> Now, while I can understand why it's a good idea to use the current
> processor, in order to limit cache ping pong etc... I'm not really
> confident I understand the pro/cons of using the hashing for tx. I
> understand that the net core can play interesting games with associating
> sockets with queues etc... but I'm a bit at a loss when it comes to
> deciding what's best for this driver. I suppose I could start by
> implementing my own queue selection based on what ehea does today but I
> have the nasty feeling that's going to be sub-optimal :-)
>
> So I would very much appreciate (and reward with free beer at the next
> conference) if somebody could give me a bit of a heads up on how things
> are expected to be done there, pro/cons, perf impact etc...

In the past Dave has recommended against implementing
ndo_select_queue().

When forwarding between multiqueue interfaces, we expect the input
device to spread traffic out between RX queues and we then use the
corresponding TX queue on output (assuming equal numbers of queues on
interfaces).  Thus we should easily avoid contention on TX queues.

For endpoints, the situation is more complex.  Ideally we would have one
IRQ, one RX queue and one TX queue per processor; we would let each
processor send on its own TX queue and NICs would automatically steer RX
packets to the RX queue for wherever the receiving thread will be
scheduled.  In practice the NIC doesn't know that and even if it does we
can easily introduce reordering.  This also depends on the driver being
able to control affinity of its IRQs.

ixgbe, in conjunction with the firmware 'Flow Director' feature attempts
to implement this, using the last TX queue for the flow as an indicator
of which RX queue to use, but so far as I can see it punts on the
reordering issue.  It sets affinity 'hints' and apparently requires that
irqbalance follows these.

Another approach is to assume that when a receiving thread is regularly
woken up by packet reception on a given CPU then it will tend to be
scheduled and to transmit on the same flow from that CPU.  On that basis
we should set the TX queue for a connected socket to match the RX queue
it last received on.  (See
<http://article.gmane.org/gmane.linux.network/158477>.)  It's not clear
whether this is really true.

Receive Flow Steering implements the steering entirely in software, but
AFAIK does nothing for the TX side; it seems mostly targetted at
single-queue NICs.

I will shortly be proposing some changes that I hope will allow at least
some multiqueue NIC drivers to move closer to that ideal.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* [PATCH net-next] drivers/net/bfin_mac.c: Use pr_fmt, netdev_<level>
From: Joe Perches @ 2010-07-27 19:22 UTC (permalink / raw)
  To: Michael Hennerich; +Cc: uclinux-dist-devel, netdev, LKML

And some assorted neatening for checkpatch:

	80 column reformatting (mostly comments)
	argument alignment
	couple of spelling/grammar typos corrected

Added bfin_alloc_skb to centralize allocation/dcache invalidation
Added get_mac_addr for symmetry

$ ./scripts/checkpatch.pl -f drivers/net/bfin_mac.c | grep "^total:"
total: 2 errors, 25 warnings, 1723 lines checked
$ ./scripts/checkpatch.pl -f drivers/net/bfin_mac.c | grep "^total:"
total: 0 errors, 0 warnings, 1743 lines checked

Uncompiled, untested.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/net/bfin_mac.c |  284 ++++++++++++++++++++++++++----------------------
 1 files changed, 152 insertions(+), 132 deletions(-)

diff --git a/drivers/net/bfin_mac.c b/drivers/net/bfin_mac.c
index 012613f..46a4576 100644
--- a/drivers/net/bfin_mac.c
+++ b/drivers/net/bfin_mac.c
@@ -8,6 +8,8 @@
  * Licensed under the GPL-2 or later.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
@@ -81,6 +83,24 @@ static u16 pin_req[] = P_RMII0;
 static u16 pin_req[] = P_MII0;
 #endif
 
+static struct sk_buff *bfin_alloc_skb(void)
+{
+	/* allocate a new skb */
+	struct sk_buff *new_skb = dev_alloc_skb(PKT_BUF_SIZE + NET_IP_ALIGN);
+
+	if (!new_skb)
+		return NULL;
+
+	skb_reserve(new_skb, NET_IP_ALIGN);
+	/* Invalidate the data cache of skb->data range when it is write back
+	 * cache.  It will prevent overwriting the new data from DMA
+	 */
+	blackfin_dcache_invalidate_range((unsigned long)new_skb->head,
+					 (unsigned long)new_skb->end);
+
+	return new_skb;
+}
+
 static void desc_list_free(void)
 {
 	struct net_dma_desc_rx *r;
@@ -132,14 +152,14 @@ static int desc_list_init(void)
 #endif
 
 	tx_desc = bfin_mac_alloc(&dma_handle,
-				sizeof(struct net_dma_desc_tx) *
-				CONFIG_BFIN_TX_DESC_NUM);
+				 sizeof(struct net_dma_desc_tx) *
+				 CONFIG_BFIN_TX_DESC_NUM);
 	if (tx_desc == NULL)
 		goto init_error;
 
 	rx_desc = bfin_mac_alloc(&dma_handle,
-				sizeof(struct net_dma_desc_rx) *
-				CONFIG_BFIN_RX_DESC_NUM);
+				 sizeof(struct net_dma_desc_rx) *
+				 CONFIG_BFIN_RX_DESC_NUM);
 	if (rx_desc == NULL)
 		goto init_error;
 
@@ -192,19 +212,11 @@ static int desc_list_init(void)
 		struct dma_descriptor *a = &(r->desc_a);
 		struct dma_descriptor *b = &(r->desc_b);
 
-		/* allocate a new skb for next time receive */
-		new_skb = dev_alloc_skb(PKT_BUF_SZ + NET_IP_ALIGN);
+		new_skb = bfin_alloc_skb();
 		if (!new_skb) {
-			printk(KERN_NOTICE DRV_NAME
-			       ": init: low on mem - packet dropped\n");
+			pr_notice("init: low on mem - packet dropped\n");
 			goto init_error;
 		}
-		skb_reserve(new_skb, NET_IP_ALIGN);
-		/* Invidate the data cache of skb->data range when it is write back
-		 * cache. It will prevent overwritting the new data from DMA
-		 */
-		blackfin_dcache_invalidate_range((unsigned long)new_skb->head,
-					 (unsigned long)new_skb->end);
 		r->skb = new_skb;
 
 		/*
@@ -229,8 +241,8 @@ static int desc_list_init(void)
 		 * 6 half words is desc size
 		 * large desc flow
 		 */
-		b->config = DMAEN | WNR | WDSIZE_32 | DI_EN |
-				NDSIZE_6 | DMAFLOW_LARGE;
+		b->config = (DMAEN | WNR | WDSIZE_32 | DI_EN |
+			     NDSIZE_6 | DMAFLOW_LARGE);
 		b->start_addr = (unsigned long)(&(r->status));
 		b->x_count = 0;
 
@@ -246,7 +258,7 @@ static int desc_list_init(void)
 
 init_error:
 	desc_list_free();
-	printk(KERN_ERR DRV_NAME ": kmalloc failed\n");
+	pr_err("kmalloc failed\n");
 	return -ENOMEM;
 }
 
@@ -263,12 +275,11 @@ static int bfin_mdio_poll(void)
 
 	/* poll the STABUSY bit */
 	while ((bfin_read_EMAC_STAADD()) & STABUSY) {
-		udelay(1);
 		if (timeout_cnt-- < 0) {
-			printk(KERN_ERR DRV_NAME
-			": wait MDC/MDIO transaction to complete timeout\n");
+			pr_err("wait MDC/MDIO transaction to complete timeout\n");
 			return -ETIMEDOUT;
 		}
+		udelay(1);
 	}
 
 	return 0;
@@ -284,15 +295,15 @@ static int bfin_mdiobus_read(struct mii_bus *bus, int phy_addr, int regnum)
 		return ret;
 
 	/* read mode */
-	bfin_write_EMAC_STAADD(SET_PHYAD((u16) phy_addr) |
+	bfin_write_EMAC_STAADD((SET_PHYAD((u16) phy_addr) |
 				SET_REGAD((u16) regnum) |
-				STABUSY);
+				STABUSY));
 
 	ret = bfin_mdio_poll();
 	if (ret)
 		return ret;
 
-	return (int) bfin_read_EMAC_STADAT();
+	return (int)bfin_read_EMAC_STADAT();
 }
 
 /* Write an off-chip register in a PHY through the MDC/MDIO port */
@@ -308,10 +319,10 @@ static int bfin_mdiobus_write(struct mii_bus *bus, int phy_addr, int regnum,
 	bfin_write_EMAC_STADAT((u32) value);
 
 	/* write mode */
-	bfin_write_EMAC_STAADD(SET_PHYAD((u16) phy_addr) |
+	bfin_write_EMAC_STAADD((SET_PHYAD((u16) phy_addr) |
 				SET_REGAD((u16) regnum) |
 				STAOP |
-				STABUSY);
+				STABUSY));
 
 	return bfin_mdio_poll();
 }
@@ -356,9 +367,9 @@ static void bfin_mac_adjust_link(struct net_device *dev)
 				opmode &= ~(RMII_10);
 				break;
 			default:
-				printk(KERN_WARNING
-					"%s: Ack!  Speed (%d) is not 10/100!\n",
-					DRV_NAME, phydev->speed);
+				netdev_warn(dev,
+					    "Ack!  Speed (%d) is not 10/100!\n",
+					    phydev->speed);
 				break;
 			}
 			bfin_write_EMAC_OPMODE(opmode);
@@ -382,7 +393,7 @@ static void bfin_mac_adjust_link(struct net_device *dev)
 	if (new_state) {
 		u32 opmode = bfin_read_EMAC_OPMODE();
 		phy_print_status(phydev);
-		pr_debug("EMAC_OPMODE = 0x%08x\n", opmode);
+		netdev_dbg(dev, "EMAC_OPMODE = 0x%08x\n", opmode);
 	}
 
 	spin_unlock_irqrestore(&lp->lock, flags);
@@ -421,35 +432,35 @@ static int mii_probe(struct net_device *dev)
 		break; /* found it */
 	}
 
-	/* now we are supposed to have a proper phydev, to attach to... */
+	/* now we are supposed to have a proper phydev to attach to... */
 	if (!phydev) {
-		printk(KERN_INFO "%s: Don't found any phy device at all\n",
-			dev->name);
+		netdev_info(dev, "No PHY device found\n");
 		return -ENODEV;
 	}
 
 #if defined(CONFIG_BFIN_MAC_RMII)
 	phydev = phy_connect(dev, dev_name(&phydev->dev), &bfin_mac_adjust_link,
-			0, PHY_INTERFACE_MODE_RMII);
+			     0, PHY_INTERFACE_MODE_RMII);
 #else
 	phydev = phy_connect(dev, dev_name(&phydev->dev), &bfin_mac_adjust_link,
-			0, PHY_INTERFACE_MODE_MII);
+			     0, PHY_INTERFACE_MODE_MII);
 #endif
 
 	if (IS_ERR(phydev)) {
-		printk(KERN_ERR "%s: Could not attach to PHY\n", dev->name);
+		netdev_err(dev, "Could not attach to PHY\n");
 		return PTR_ERR(phydev);
 	}
 
 	/* mask with MAC supported features */
-	phydev->supported &= (SUPPORTED_10baseT_Half
-			      | SUPPORTED_10baseT_Full
-			      | SUPPORTED_100baseT_Half
-			      | SUPPORTED_100baseT_Full
-			      | SUPPORTED_Autoneg
-			      | SUPPORTED_Pause | SUPPORTED_Asym_Pause
-			      | SUPPORTED_MII
-			      | SUPPORTED_TP);
+	phydev->supported &= (SUPPORTED_10baseT_Half |
+			      SUPPORTED_10baseT_Full |
+			      SUPPORTED_100baseT_Half |
+			      SUPPORTED_100baseT_Full |
+			      SUPPORTED_Autoneg |
+			      SUPPORTED_Pause |
+			      SUPPORTED_Asym_Pause |
+			      SUPPORTED_MII |
+			      SUPPORTED_TP);
 
 	phydev->advertising = phydev->supported;
 
@@ -458,11 +469,11 @@ static int mii_probe(struct net_device *dev)
 	lp->old_duplex = -1;
 	lp->phydev = phydev;
 
-	printk(KERN_INFO "%s: attached PHY driver [%s] "
-	       "(mii_bus:phy_addr=%s, irq=%d, mdc_clk=%dHz(mdc_div=%d)"
-	       "@sclk=%dMHz)\n",
-	       DRV_NAME, phydev->drv->name, dev_name(&phydev->dev), phydev->irq,
-	       MDC_CLK, mdc_div, sclk/1000000);
+	netdev_info(dev, "attached PHY driver [%s] "
+		    "(mii_bus:phy_addr=%s, irq=%d, mdc_clk=%dHz(mdc_div=%d)"
+		    "@sclk=%dMHz)\n",
+		    phydev->drv->name, dev_name(&phydev->dev), phydev->irq,
+		    MDC_CLK, mdc_div, sclk/1000000);
 
 	return 0;
 }
@@ -514,7 +525,7 @@ static void bfin_mac_ethtool_getdrvinfo(struct net_device *dev,
 }
 
 static void bfin_mac_ethtool_getwol(struct net_device *dev,
-	struct ethtool_wolinfo *wolinfo)
+				    struct ethtool_wolinfo *wolinfo)
 {
 	struct bfin_mac_local *lp = netdev_priv(dev);
 
@@ -523,7 +534,7 @@ static void bfin_mac_ethtool_getwol(struct net_device *dev,
 }
 
 static int bfin_mac_ethtool_setwol(struct net_device *dev,
-	struct ethtool_wolinfo *wolinfo)
+				   struct ethtool_wolinfo *wolinfo)
 {
 	struct bfin_mac_local *lp = netdev_priv(dev);
 	int rc;
@@ -599,10 +610,20 @@ void setup_system_regs(struct net_device *dev)
 	bfin_write_DMA1_Y_MODIFY(0);
 }
 
+/* Grab the MAC address in the MAC */
+static void get_mac_addr(u8 *mac_addr)
+{
+	__le32 addr_hi = cpu_to_le32(bfin_read_EMAC_ADDRLO());
+	__le16 addr_low = cpu_to_le16((u16)bfin_read_EMAC_ADDRHI());
+
+	memcpy(mac_addr, &addr_hi, 4);
+	memcpy(mac_addr + 4, &addr_low, 2);
+}
+
 static void setup_mac_addr(u8 *mac_addr)
 {
-	u32 addr_low = le32_to_cpu(*(__le32 *) & mac_addr[0]);
-	u16 addr_hi = le16_to_cpu(*(__le16 *) & mac_addr[4]);
+	u32 addr_low = le32_to_cpu(*(__le32 *)&mac_addr[0]);
+	u16 addr_hi = le16_to_cpu(*(__le16 *)&mac_addr[4]);
 
 	/* this depends on a little-endian machine */
 	bfin_write_EMAC_ADDRLO(addr_low);
@@ -612,6 +633,7 @@ static void setup_mac_addr(u8 *mac_addr)
 static int bfin_mac_set_mac_address(struct net_device *dev, void *p)
 {
 	struct sockaddr *addr = p;
+
 	if (netif_running(dev))
 		return -EBUSY;
 	memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
@@ -623,7 +645,7 @@ static int bfin_mac_set_mac_address(struct net_device *dev, void *p)
 #define bfin_mac_hwtstamp_is_none(cfg) ((cfg) == HWTSTAMP_FILTER_NONE)
 
 static int bfin_mac_hwtstamp_ioctl(struct net_device *netdev,
-		struct ifreq *ifr, int cmd)
+				   struct ifreq *ifr, int cmd)
 {
 	struct hwtstamp_config config;
 	struct bfin_mac_local *lp = netdev_priv(netdev);
@@ -633,15 +655,15 @@ static int bfin_mac_hwtstamp_ioctl(struct net_device *netdev,
 	if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
 		return -EFAULT;
 
-	pr_debug("%s config flag:0x%x, tx_type:0x%x, rx_filter:0x%x\n",
-			__func__, config.flags, config.tx_type, config.rx_filter);
+	netdev_dbg("%s config flag:0x%x, tx_type:0x%x, rx_filter:0x%x\n",
+		   __func__, config.flags, config.tx_type, config.rx_filter);
 
 	/* reserved for future extensions */
 	if (config.flags)
 		return -EINVAL;
 
 	if ((config.tx_type != HWTSTAMP_TX_OFF) &&
-			(config.tx_type != HWTSTAMP_TX_ON))
+	    (config.tx_type != HWTSTAMP_TX_ON))
 		return -ERANGE;
 
 	ptpctl = bfin_read_EMAC_PTP_CTL();
@@ -658,7 +680,8 @@ static int bfin_mac_hwtstamp_ioctl(struct net_device *netdev,
 	case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
 	case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
 		/*
-		 * Clear the five comparison mask bits (bits[12:8]) in EMAC_PTP_CTL)
+		 * Clear the five comparison mask bits
+		 * (bits[12:8] in EMAC_PTP_CTL)
 		 * to enable all the field matches.
 		 */
 		ptpctl &= ~0x1F00;
@@ -694,8 +717,8 @@ static int bfin_mac_hwtstamp_ioctl(struct net_device *netdev,
 		ptpctl &= ~0x1F00;
 		bfin_write_EMAC_PTP_CTL(ptpctl);
 		/*
-		 * Keep the default values of the EMAC_PTP_FOFF register, except set
-		 * the PTPCOF field to 0x2A.
+		 * Keep the default values of the EMAC_PTP_FOFF register,
+		 * except set the PTPCOF field to 0x2A.
 		 */
 		ptpfoff = 0x2A24170C;
 		bfin_write_EMAC_PTP_FOFF(ptpfoff);
@@ -720,20 +743,20 @@ static int bfin_mac_hwtstamp_ioctl(struct net_device *netdev,
 	case HWTSTAMP_FILTER_PTP_V2_L2_SYNC:
 	case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ:
 		/*
-		 * Clear bits 8 and 12 of the EMAC_PTP_CTL register to enable only the
-		 * EFTM and PTPCM field comparison.
+		 * Clear bits 8 and 12 of the EMAC_PTP_CTL register to enable
+		 * only the EFTM and PTPCM field comparison.
 		 */
 		ptpctl &= ~0x1100;
 		bfin_write_EMAC_PTP_CTL(ptpctl);
 		/*
-		 * Keep the default values of all the fields of the EMAC_PTP_FOFF
-		 * register, except set the PTPCOF field to 0x0E.
+		 * Keep the default values of all the fields of the
+		 * EMAC_PTP_FOFF register, except set the PTPCOF field to 0x0E.
 		 */
 		ptpfoff = 0x0E24170C;
 		bfin_write_EMAC_PTP_FOFF(ptpfoff);
 		/*
-		 * Program bits [15:0] of the EMAC_PTP_FV1 register to 0x88F7, which
-		 * corresponds to PTP messages on the MAC layer.
+		 * Program bits [15:0] of the EMAC_PTP_FV1 register to 0x88F7,
+		 * which corresponds to PTP messages on the MAC layer.
 		 */
 		ptpfv1 = 0x110488F7;
 		bfin_write_EMAC_PTP_FV1(ptpfv1);
@@ -791,13 +814,17 @@ static int bfin_mac_hwtstamp_ioctl(struct net_device *netdev,
 		-EFAULT : 0;
 }
 
-static void bfin_dump_hwtamp(char *s, ktime_t *hw, ktime_t *ts, struct timecompare *cmp)
+static void bfin_dump_hwtamp(char *s, ktime_t *hw, ktime_t *ts,
+			     struct timecompare *cmp)
 {
 	ktime_t sys = ktime_get_real();
 
 	pr_debug("%s %s hardware:%d,%d transform system:%d,%d system:%d,%d, cmp:%lld, %lld\n",
-			__func__, s, hw->tv.sec, hw->tv.nsec, ts->tv.sec, ts->tv.nsec, sys.tv.sec,
-			sys.tv.nsec, cmp->offset, cmp->skew);
+		 __func__, s,
+		 hw->tv.sec, hw->tv.nsec,
+		 ts->tv.sec, ts->tv.nsec,
+		 sys.tv.sec, sys.tv.nsec,
+		 cmp->offset, cmp->skew);
 }
 
 static void bfin_tx_hwtstamp(struct net_device *netdev, struct sk_buff *skb)
@@ -814,16 +841,17 @@ static void bfin_tx_hwtstamp(struct net_device *netdev, struct sk_buff *skb)
 		shtx->in_progress = 1;
 
 		/*
-		 * The timestamping is done at the EMAC module's MII/RMII interface
-		 * when the module sees the Start of Frame of an event message packet. This
-		 * interface is the closest possible place to the physical Ethernet transmission
+		 * The timestamping is done at the EMAC module's MII/RMII
+		 * interface when the module sees the Start of Frame of an
+		 * event message packet. This interface is the closest
+		 * possible place to the physical Ethernet transmission
 		 * medium, providing the best timing accuracy.
 		 */
-		while ((!(bfin_read_EMAC_PTP_ISTAT() & TXTL)) && (--timeout_cnt))
+		while ((!(bfin_read_EMAC_PTP_ISTAT() & TXTL)) &&
+		       (--timeout_cnt))
 			udelay(1);
 		if (timeout_cnt == 0)
-			printk(KERN_ERR DRV_NAME
-					": fails to timestamp the TX packet\n");
+			netdev_err(dev, "failed to timestamp the TX packet\n");
 		else {
 			struct skb_shared_hwtstamps shhwtstamps;
 			u64 ns;
@@ -832,15 +860,15 @@ static void bfin_tx_hwtstamp(struct net_device *netdev, struct sk_buff *skb)
 			regval = bfin_read_EMAC_PTP_TXSNAPLO();
 			regval |= (u64)bfin_read_EMAC_PTP_TXSNAPHI() << 32;
 			memset(&shhwtstamps, 0, sizeof(shhwtstamps));
-			ns = timecounter_cyc2time(&lp->clock,
-					regval);
+			ns = timecounter_cyc2time(&lp->clock, regval);
 			timecompare_update(&lp->compare, ns);
 			shhwtstamps.hwtstamp = ns_to_ktime(ns);
 			shhwtstamps.syststamp =
 				timecompare_transform(&lp->compare, ns);
 			skb_tstamp_tx(skb, &shhwtstamps);
 
-			bfin_dump_hwtamp("TX", &shhwtstamps.hwtstamp, &shhwtstamps.syststamp, &lp->compare);
+			bfin_dump_hwtamp("TX", &shhwtstamps.hwtstamp,
+					 &shhwtstamps.syststamp, &lp->compare);
 		}
 	}
 }
@@ -869,7 +897,8 @@ static void bfin_rx_hwtstamp(struct net_device *netdev, struct sk_buff *skb)
 	shhwtstamps->hwtstamp = ns_to_ktime(ns);
 	shhwtstamps->syststamp = timecompare_transform(&lp->compare, ns);
 
-	bfin_dump_hwtamp("RX", &shhwtstamps->hwtstamp, &shhwtstamps->syststamp, &lp->compare);
+	bfin_dump_hwtamp("RX", &shhwtstamps->hwtstamp,
+			 &shhwtstamps->syststamp, &lp->compare);
 }
 
 /*
@@ -879,8 +908,8 @@ static cycle_t bfin_read_clock(const struct cyclecounter *tc)
 {
 	u64 stamp;
 
-	stamp =  bfin_read_EMAC_PTP_TIMELO();
-	stamp |= (u64)bfin_read_EMAC_PTP_TIMEHI() << 32ULL;
+	stamp = bfin_read_EMAC_PTP_TIMELO();
+	stamp |= ((u64)bfin_read_EMAC_PTP_TIMEHI()) << 32;
 
 	return stamp;
 }
@@ -961,7 +990,7 @@ static void tx_reclaim_skb(struct bfin_mac_local *lp)
 	}
 
 	if (current_tx_ptr->next != tx_list_head &&
-		netif_queue_stopped(lp->ndev))
+	    netif_queue_stopped(lp->ndev))
 		netif_wake_queue(lp->ndev);
 
 	if (tx_list_head != current_tx_ptr) {
@@ -974,7 +1003,7 @@ static void tx_reclaim_skb(struct bfin_mac_local *lp)
 				jiffies + TX_RECLAIM_JIFFIES;
 
 		mod_timer(&lp->tx_reclaim_timer,
-			lp->tx_reclaim_timer.expires);
+			  lp->tx_reclaim_timer.expires);
 	}
 
 	return;
@@ -985,8 +1014,7 @@ static void tx_reclaim_skb_timeout(unsigned long lp)
 	tx_reclaim_skb((struct bfin_mac_local *)lp);
 }
 
-static int bfin_mac_hard_start_xmit(struct sk_buff *skb,
-				struct net_device *dev)
+static int bfin_mac_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct bfin_mac_local *lp = netdev_priv(dev);
 	u16 *data;
@@ -1000,10 +1028,11 @@ static int bfin_mac_hard_start_xmit(struct sk_buff *skb,
 		data = (u16 *)(skb->data) - 1;
 		*data = (u16)(skb->len);
 		/*
-		 * When transmitting an Ethernet packet, the PTP_TSYNC module requires
-		 * a DMA_Length_Word field associated with the packet. The lower 12 bits
-		 * of this field are the length of the packet payload in bytes and the higher
-		 * 4 bits are the timestamping enable field.
+		 * When transmitting an Ethernet packet, the PTP_TSYNC module
+		 * requires a DMA_Length_Word field associated with the packet.
+		 * The lower 12 bits of this field are the length of the packet
+		 * payload in bytes and the higher 4 bits are the timestamping
+		 * enable field.
 		 */
 		if (shtx->hardware)
 			*data |= 0x1000;
@@ -1011,7 +1040,7 @@ static int bfin_mac_hard_start_xmit(struct sk_buff *skb,
 		current_tx_ptr->desc_a.start_addr = (u32)data;
 		/* this is important! */
 		blackfin_dcache_flush_range((u32)data,
-				(u32)((u8 *)data + skb->len + 4));
+					    (u32)((u8 *)data + skb->len + 4));
 	} else {
 		*((u16 *)(current_tx_ptr->packet)) = (u16)(skb->len);
 		/* enable timestamping for the sent packet */
@@ -1063,7 +1092,8 @@ out:
 
 #define IP_HEADER_OFF  0
 #define RX_ERROR_MASK (RX_LONG | RX_ALIGN | RX_CRC | RX_LEN | \
-	RX_FRAG | RX_ADDR | RX_DMAO | RX_PHY | RX_LATE | RX_RANGE)
+		       RX_FRAG | RX_ADDR | RX_DMAO | RX_PHY | \
+		       RX_LATE | RX_RANGE)
 
 static void bfin_mac_rx(struct net_device *dev)
 {
@@ -1079,8 +1109,7 @@ static void bfin_mac_rx(struct net_device *dev)
 	 * we which case we simply drop the packet
 	 */
 	if (current_rx_ptr->status.status_word & RX_ERROR_MASK) {
-		printk(KERN_NOTICE DRV_NAME
-		       ": rx: receive error - packet dropped\n");
+		netdev_notice(dev, "rx: receive error - packet dropped\n");
 		dev->stats.rx_dropped++;
 		goto out;
 	}
@@ -1088,20 +1117,12 @@ static void bfin_mac_rx(struct net_device *dev)
 	/* allocate a new skb for next time receive */
 	skb = current_rx_ptr->skb;
 
-	new_skb = dev_alloc_skb(PKT_BUF_SZ + NET_IP_ALIGN);
+	new_skb = bfin_alloc_skb();
 	if (!new_skb) {
-		printk(KERN_NOTICE DRV_NAME
-		       ": rx: low on mem - packet dropped\n");
+		netdev_notice(dev, "rx: low on mem - packet dropped\n");
 		dev->stats.rx_dropped++;
 		goto out;
 	}
-	/* reserve 2 bytes for RXDWA padding */
-	skb_reserve(new_skb, NET_IP_ALIGN);
-	/* Invidate the data cache of skb->data range when it is write back
-	 * cache. It will prevent overwritting the new data from DMA
-	 */
-	blackfin_dcache_invalidate_range((unsigned long)new_skb->head,
-					 (unsigned long)new_skb->end);
 
 	current_rx_ptr->skb = new_skb;
 	current_rx_ptr->desc_a.start_addr = (unsigned long)new_skb->data - 2;
@@ -1116,25 +1137,28 @@ static void bfin_mac_rx(struct net_device *dev)
 	bfin_rx_hwtstamp(dev, skb);
 
 #if defined(BFIN_MAC_CSUM_OFFLOAD)
-	/* Checksum offloading only works for IPv4 packets with the standard IP header
-	 * length of 20 bytes, because the blackfin MAC checksum calculation is
-	 * based on that assumption. We must NOT use the calculated checksum if our
-	 * IP version or header break that assumption.
+	/* Checksum offloading only works for IPv4 packets with the standard
+	 * IP header length of 20 bytes, because the blackfin MAC checksum
+	 * calculation is based on that assumption. We must NOT use the
+	 * calculated checksum if our IP version or header break that
+	 * assumption.
 	 */
 	if (skb->data[IP_HEADER_OFF] == 0x45) {
 		skb->csum = current_rx_ptr->status.ip_payload_csum;
 		/*
-		 * Deduce Ethernet FCS from hardware generated IP payload checksum.
-		 * IP checksum is based on 16-bit one's complement algorithm.
-		 * To deduce a value from checksum is equal to add its inversion.
-		 * If the IP payload len is odd, the inversed FCS should also
-		 * begin from odd address and leave first byte zero.
+		 * Deduce Ethernet FCS from hardware generated IP payload
+		 * checksum.  IP checksum is based on 16-bit one's complement
+		 * algorithm.  To deduce a value from checksum is equal to
+		 * add its inversion.  If the IP payload len is odd, the
+		 * inversed FCS should also begin from odd address and leave
+		 * first byte zero.
 		 */
 		if (skb->len % 2) {
 			fcs[0] = 0;
 			for (i = 0; i < ETH_FCS_LEN; i++)
 				fcs[i + 1] = ~skb->data[skb->len + i];
-			skb->csum = csum_partial(fcs, ETH_FCS_LEN + 1, skb->csum);
+			skb->csum = csum_partial(fcs, ETH_FCS_LEN + 1,
+						 skb->csum);
 		} else {
 			for (i = 0; i < ETH_FCS_LEN; i++)
 				fcs[i] = ~skb->data[skb->len + i];
@@ -1209,7 +1233,7 @@ static int bfin_mac_enable(void)
 	int ret;
 	u32 opmode;
 
-	pr_debug("%s: %s\n", DRV_NAME, __func__);
+	pr_debug("%s\n", __func__);
 
 	/* Set RX DMA */
 	bfin_write_DMA1_NEXT_DESC_PTR(&(rx_list_head->desc_a));
@@ -1251,7 +1275,7 @@ static void bfin_mac_timeout(struct net_device *dev)
 {
 	struct bfin_mac_local *lp = netdev_priv(dev);
 
-	pr_debug("%s: %s\n", dev->name, __func__);
+	netdev_dbg(dev, "%s\n", __func__);
 
 	bfin_mac_disable();
 
@@ -1318,7 +1342,7 @@ static void bfin_mac_set_multicast_list(struct net_device *dev)
 	u32 sysctl;
 
 	if (dev->flags & IFF_PROMISC) {
-		printk(KERN_INFO "%s: set to promisc mode\n", dev->name);
+		netdev_info(dev, "set to promisc mode\n");
 		sysctl = bfin_read_EMAC_OPMODE();
 		sysctl |= PR;
 		bfin_write_EMAC_OPMODE(sysctl);
@@ -1372,7 +1396,7 @@ static int bfin_mac_open(struct net_device *dev)
 {
 	struct bfin_mac_local *lp = netdev_priv(dev);
 	int ret;
-	pr_debug("%s: %s\n", dev->name, __func__);
+	netdev_dbg(dev, "%s\n", __func__);
 
 	/*
 	 * Check that the address is valid.  If its not, refuse
@@ -1380,7 +1404,7 @@ static int bfin_mac_open(struct net_device *dev)
 	 * address using ifconfig eth0 hw ether xx:xx:xx:xx:xx:xx
 	 */
 	if (!is_valid_ether_addr(dev->dev_addr)) {
-		printk(KERN_WARNING DRV_NAME ": no valid ethernet hw addr\n");
+		netdev_warn(dev, "no valid ethernet hw addr\n");
 		return -EINVAL;
 	}
 
@@ -1398,7 +1422,7 @@ static int bfin_mac_open(struct net_device *dev)
 	ret = bfin_mac_enable();
 	if (ret)
 		return ret;
-	pr_debug("hardware init finished\n");
+	netdev_dbg(dev, "hardware init finished\n");
 
 	netif_start_queue(dev);
 	netif_carrier_on(dev);
@@ -1414,7 +1438,7 @@ static int bfin_mac_open(struct net_device *dev)
 static int bfin_mac_close(struct net_device *dev)
 {
 	struct bfin_mac_local *lp = netdev_priv(dev);
-	pr_debug("%s: %s\n", dev->name, __func__);
+	netdev_dbg(dev, "%s\n", __func__);
 
 	netif_stop_queue(dev);
 	netif_carrier_off(dev);
@@ -1464,9 +1488,7 @@ static int __devinit bfin_mac_probe(struct platform_device *pdev)
 	lp = netdev_priv(ndev);
 	lp->ndev = ndev;
 
-	/* Grab the MAC address in the MAC */
-	*(__le32 *) (&(ndev->dev_addr[0])) = cpu_to_le32(bfin_read_EMAC_ADDRLO());
-	*(__le16 *) (&(ndev->dev_addr[4])) = cpu_to_le16((u16) bfin_read_EMAC_ADDRHI());
+	get_mac_addr(ndev->dev_addr);
 
 	/* probe mac */
 	/*todo: how to proble? which is revision_register */
@@ -1526,8 +1548,8 @@ static int __devinit bfin_mac_probe(struct platform_device *pdev)
 
 	/* now, enable interrupts */
 	/* register irq handler */
-	rc = request_irq(IRQ_MAC_RX, bfin_mac_interrupt,
-			IRQF_DISABLED, "EMAC_RX", ndev);
+	rc = request_irq(IRQ_MAC_RX, bfin_mac_interrupt, IRQF_DISABLED,
+			 "EMAC_RX", ndev);
 	if (rc) {
 		dev_err(&pdev->dev, "Cannot request Blackfin MAC RX IRQ!\n");
 		rc = -EBUSY;
@@ -1647,7 +1669,7 @@ static int __devinit bfin_mii_bus_probe(struct platform_device *pdev)
 	miibus->parent = &pdev->dev;
 	miibus->name = "bfin_mii_bus";
 	snprintf(miibus->id, MII_BUS_ID_SIZE, "0");
-	miibus->irq = kmalloc(sizeof(int)*PHY_MAX_ADDR, GFP_KERNEL);
+	miibus->irq = kmalloc(sizeof(int) * PHY_MAX_ADDR, GFP_KERNEL);
 	if (miibus->irq == NULL)
 		goto out_err_alloc;
 	for (i = 0; i < PHY_MAX_ADDR; ++i)
@@ -1674,6 +1696,7 @@ out_err_alloc:
 static int __devexit bfin_mii_bus_remove(struct platform_device *pdev)
 {
 	struct mii_bus *miibus = platform_get_drvdata(pdev);
+
 	platform_set_drvdata(pdev, NULL);
 	mdiobus_unregister(miibus);
 	kfree(miibus->irq);
@@ -1698,17 +1721,15 @@ static struct platform_driver bfin_mac_driver = {
 	.suspend = bfin_mac_suspend,
 	.driver = {
 		.name = DRV_NAME,
-		.owner	= THIS_MODULE,
+		.owner = THIS_MODULE,
 	},
 };
 
 static int __init bfin_mac_init(void)
 {
-	int ret;
-	ret = platform_driver_register(&bfin_mii_bus_driver);
-	if (!ret)
-		return platform_driver_register(&bfin_mac_driver);
-	return -ENODEV;
+	if (platform_driver_register(&bfin_mii_bus_driver))
+		return -ENODEV;
+	return platform_driver_register(&bfin_mac_driver);
 }
 
 module_init(bfin_mac_init);
@@ -1720,4 +1741,3 @@ static void __exit bfin_mac_cleanup(void)
 }
 
 module_exit(bfin_mac_cleanup);
-



^ permalink raw reply related

* Re: [PATCH UPDATED 1/3] vhost: replace vhost_workqueue with per-vhost kthread
From: Michael S. Tsirkin @ 2010-07-27 19:19 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Oleg Nesterov, Sridhar Samudrala, netdev, lkml,
	kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev, Jiri Kosina,
	Thomas Gleixner, Ingo Molnar, Andi Kleen
In-Reply-To: <4C4DE2AE.40302@kernel.org>

On Mon, Jul 26, 2010 at 09:31:58PM +0200, Tejun Heo wrote:
> Hello,
> 
> On 07/26/2010 09:14 PM, Tejun Heo wrote:
> > On 07/26/2010 06:51 PM, Michael S. Tsirkin wrote:
> >> I noticed that with vhost, flush_work was getting the worker
> >> pointer as well. Can we live with this API change?
> > 
> > Yeah, the flushing mechanism wouldn't work reliably if the work is
> > queued to a different worker without flushing, so yeah passing in
> > @worker might actually be better.
> 
> Thinking a bit more about it, it kind of sucks that queueing to
> another worker from worker->func() breaks flush.  Maybe the right
> thing to do there is using atomic_t for done_seq?

I don't believe it will help: we might have:

worker1 runs work
work requeues itself queued index = 1
worker1 reads queued index = 1
worker2 runs work
work requeues itself queued index = 2
worker2 runs work
worker2 reads queued index = 2
worker2 writes done index = 2
worker1 writes done index = 1

As you see, done index got moved back.



>  It pays a bit more
> overhead but maybe that's justifiable to keep the API saner?  It would
> be great if it can be fixed somehow even if it means that the work has
> to be separately flushed for each worker it has been on before being
> destroyed.
> 
> Or, if flushing has to be associated with a specific worker anyway,
> maybe it would be better to move the sequence counter to
> kthread_worker and do it similarly with the original workqueue so that
> work can be destroyed once execution starts?  Then, it can at least
> remain semantically identical to the original workqueue.
> 
> Thanks.
> 
> -- 
> tejun

^ permalink raw reply


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