Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] ppp_generic: fix multilink fragment sizes
From: David Miller @ 2010-06-02 15:31 UTC (permalink / raw)
  To: gabriele.paoloni; +Cc: ben, netdev, linux-kernel, alan, linux-ppp, paulus
In-Reply-To: <DF7BB929B28FCF479E888E3D9F8D9E88D3E16B5A@irsmsx502.ger.corp.intel.com>

From: "Paoloni, Gabriele" <gabriele.paoloni@intel.com>
Date: Wed, 2 Jun 2010 16:17:59 +0100

> The proposed patch looks wrong to me.

Can you please reply to postings properly?  Use the reply function
of your mail client and do not "top post."

Because of the way you just simply inlined the entirety of Ben's
patch posting, it ends up looking like a new patch being posted
on patchwork and this makes more work and analysis for me.

Thanks.

^ permalink raw reply

* [net-next PATCH] drivers/net/enic: Use (pr|netdev)_<level> macro helpers
From: Joe Perches @ 2010-06-02 15:30 UTC (permalink / raw)
  To: Scott Feldman; +Cc: netdev, LKML

Compile tested only

Add #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
Remove #define PFX
Use pr_<level>
Use netdev_<level>
Remove trailing periods from most formats

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/net/enic/enic.h      |    1 -
 drivers/net/enic/enic_main.c |  120 ++++++++++++++++++------------------------
 drivers/net/enic/enic_res.c  |   51 +++++++++---------
 drivers/net/enic/vnic_cq.c   |    2 +-
 drivers/net/enic/vnic_dev.c  |   37 ++++++-------
 drivers/net/enic/vnic_intr.c |    5 +-
 drivers/net/enic/vnic_rq.c   |    8 ++-
 drivers/net/enic/vnic_wq.c   |    8 ++-
 8 files changed, 108 insertions(+), 124 deletions(-)

diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index 85f2a2e..4277975 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -36,7 +36,6 @@
 #define DRV_DESCRIPTION		"Cisco VIC Ethernet NIC Driver"
 #define DRV_VERSION		"1.3.1.1-pp"
 #define DRV_COPYRIGHT		"Copyright 2008-2009 Cisco Systems, Inc"
-#define PFX			DRV_NAME ": "
 
 #define ENIC_LRO_MAX_DESC	8
 #define ENIC_LRO_MAX_AGGR	64
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 6586b5c..8b3f300 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -17,6 +17,8 @@
  *
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/string.h>
@@ -399,15 +401,15 @@ static void enic_log_q_error(struct enic *enic)
 	for (i = 0; i < enic->wq_count; i++) {
 		error_status = vnic_wq_error_status(&enic->wq[i]);
 		if (error_status)
-			printk(KERN_ERR PFX "%s: WQ[%d] error_status %d\n",
-				enic->netdev->name, i, error_status);
+			netdev_err(enic->netdev, "WQ[%d] error_status %d\n",
+				   i, error_status);
 	}
 
 	for (i = 0; i < enic->rq_count; i++) {
 		error_status = vnic_rq_error_status(&enic->rq[i]);
 		if (error_status)
-			printk(KERN_ERR PFX "%s: RQ[%d] error_status %d\n",
-				enic->netdev->name, i, error_status);
+			netdev_err(enic->netdev, "RQ[%d] error_status %d\n",
+				   i, error_status);
 	}
 }
 
@@ -417,10 +419,10 @@ static void enic_link_check(struct enic *enic)
 	int carrier_ok = netif_carrier_ok(enic->netdev);
 
 	if (link_status && !carrier_ok) {
-		printk(KERN_INFO PFX "%s: Link UP\n", enic->netdev->name);
+		netdev_info(enic->netdev, "Link UP\n");
 		netif_carrier_on(enic->netdev);
 	} else if (!link_status && carrier_ok) {
-		printk(KERN_INFO PFX "%s: Link DOWN\n", enic->netdev->name);
+		netdev_info(enic->netdev, "Link DOWN\n");
 		netif_carrier_off(enic->netdev);
 	}
 }
@@ -432,10 +434,10 @@ static void enic_mtu_check(struct enic *enic)
 	if (mtu && mtu != enic->port_mtu) {
 		enic->port_mtu = mtu;
 		if (mtu < enic->netdev->mtu)
-			printk(KERN_WARNING PFX
-				"%s: interface MTU (%d) set higher "
-				"than switch port MTU (%d)\n",
-				enic->netdev->name, enic->netdev->mtu, mtu);
+			netdev_warn(enic->netdev,
+				    "interface MTU (%d) set higher "
+				    "than switch port MTU (%d)\n",
+				    enic->netdev->mtu, mtu);
 	}
 }
 
@@ -444,8 +446,8 @@ static void enic_msglvl_check(struct enic *enic)
 	u32 msg_enable = vnic_dev_msg_lvl(enic->vdev);
 
 	if (msg_enable != enic->msg_enable) {
-		printk(KERN_INFO PFX "%s: msg lvl changed from 0x%x to 0x%x\n",
-			enic->netdev->name, enic->msg_enable, msg_enable);
+		netdev_info(enic->netdev, "msg lvl changed from 0x%x to 0x%x\n",
+			    enic->msg_enable, msg_enable);
 		enic->msg_enable = msg_enable;
 	}
 }
@@ -769,8 +771,7 @@ static netdev_tx_t enic_hard_start_xmit(struct sk_buff *skb,
 	    skb_shinfo(skb)->nr_frags + ENIC_DESC_MAX_SPLITS) {
 		netif_stop_queue(netdev);
 		/* This is a hard error, log it */
-		printk(KERN_ERR PFX "%s: BUG! Tx ring full when "
-			"queue awake!\n", netdev->name);
+		netdev_err(netdev, "BUG! Tx ring full when queue awake!\n");
 		spin_unlock_irqrestore(&enic->wq_lock[0], flags);
 		return NETDEV_TX_BUSY;
 	}
@@ -1703,16 +1704,13 @@ static int enic_open(struct net_device *netdev)
 
 	err = enic_request_intr(enic);
 	if (err) {
-		printk(KERN_ERR PFX "%s: Unable to request irq.\n",
-			netdev->name);
+		netdev_err(netdev, "Unable to request irq\n");
 		return err;
 	}
 
 	err = enic_notify_set(enic);
 	if (err) {
-		printk(KERN_ERR PFX
-			"%s: Failed to alloc notify buffer, aborting.\n",
-			netdev->name);
+		netdev_err(netdev, "Failed to alloc notify buffer, aborting\n");
 		goto err_out_free_intr;
 	}
 
@@ -1720,9 +1718,7 @@ static int enic_open(struct net_device *netdev)
 		vnic_rq_fill(&enic->rq[i], enic->rq_alloc_buf);
 		/* Need at least one buffer on ring to get going */
 		if (vnic_rq_desc_used(&enic->rq[i]) == 0) {
-			printk(KERN_ERR PFX
-				"%s: Unable to alloc receive buffers.\n",
-				netdev->name);
+			netdev_err(netdev, "Unable to alloc receive buffers\n");
 			err = -ENOMEM;
 			goto err_out_notify_unset;
 		}
@@ -1824,10 +1820,9 @@ static int enic_change_mtu(struct net_device *netdev, int new_mtu)
 	netdev->mtu = new_mtu;
 
 	if (netdev->mtu > enic->port_mtu)
-		printk(KERN_WARNING PFX
-			"%s: interface MTU (%d) set higher "
-			"than port MTU (%d)\n",
-			netdev->name, netdev->mtu, enic->port_mtu);
+		netdev_warn(netdev,
+			    "interface MTU (%d) set higher than port MTU (%d)\n",
+			    netdev->mtu, enic->port_mtu);
 
 	if (running)
 		enic_open(netdev);
@@ -1900,8 +1895,8 @@ static int enic_dev_open(struct enic *enic)
 	err = enic_dev_wait(enic->vdev, vnic_dev_open,
 		vnic_dev_open_done, 0);
 	if (err)
-		printk(KERN_ERR PFX
-			"vNIC device open failed, err %d.\n", err);
+		netdev_err(enic->netdev, "vNIC device open failed, err %d\n",
+			   err);
 
 	return err;
 }
@@ -1913,8 +1908,8 @@ static int enic_dev_soft_reset(struct enic *enic)
 	err = enic_dev_wait(enic->vdev, vnic_dev_soft_reset,
 		vnic_dev_soft_reset_done, 0);
 	if (err)
-		printk(KERN_ERR PFX
-			"vNIC soft reset failed, err %d.\n", err);
+		netdev_err(enic->netdev, "vNIC soft reset failed, err %d\n",
+			   err);
 
 	return err;
 }
@@ -2122,8 +2117,7 @@ int enic_dev_init(struct enic *enic)
 
 	err = enic_get_vnic_config(enic);
 	if (err) {
-		printk(KERN_ERR PFX
-			"Get vNIC configuration failed, aborting.\n");
+		netdev_err(netdev, "Get vNIC configuration failed, aborting\n");
 		return err;
 	}
 
@@ -2138,9 +2132,9 @@ int enic_dev_init(struct enic *enic)
 
 	err = enic_set_intr_mode(enic);
 	if (err) {
-		printk(KERN_ERR PFX
-			"Failed to set intr mode based on resource "
-			"counts and system capabilities, aborting.\n");
+		netdev_err(netdev,
+			   "Failed to set intr mode based on resource "
+			   "counts and system capabilities, aborting\n");
 		return err;
 	}
 
@@ -2149,8 +2143,7 @@ int enic_dev_init(struct enic *enic)
 
 	err = enic_alloc_vnic_resources(enic);
 	if (err) {
-		printk(KERN_ERR PFX
-			"Failed to alloc vNIC resources, aborting.\n");
+		netdev_err(netdev, "Failed to alloc vNIC resources, aborting\n");
 		goto err_out_free_vnic_resources;
 	}
 
@@ -2158,15 +2151,13 @@ int enic_dev_init(struct enic *enic)
 
 	err = enic_set_rq_alloc_buf(enic);
 	if (err) {
-		printk(KERN_ERR PFX
-			"Failed to set RQ buffer allocator, aborting.\n");
+		netdev_err(netdev, "Failed to set RQ buffer allocator, aborting\n");
 		goto err_out_free_vnic_resources;
 	}
 
 	err = enic_set_niccfg(enic);
 	if (err) {
-		printk(KERN_ERR PFX
-			"Failed to config nic, aborting.\n");
+		netdev_err(netdev, "Failed to config nic, aborting\n");
 		goto err_out_free_vnic_resources;
 	}
 
@@ -2212,7 +2203,7 @@ static int __devinit enic_probe(struct pci_dev *pdev,
 
 	netdev = alloc_etherdev(sizeof(struct enic));
 	if (!netdev) {
-		printk(KERN_ERR PFX "Etherdev alloc failed, aborting.\n");
+		pr_err("Etherdev alloc failed, aborting\n");
 		return -ENOMEM;
 	}
 
@@ -2229,15 +2220,13 @@ static int __devinit enic_probe(struct pci_dev *pdev,
 
 	err = pci_enable_device(pdev);
 	if (err) {
-		printk(KERN_ERR PFX
-			"Cannot enable PCI device, aborting.\n");
+		netdev_err(netdev, "Cannot enable PCI device, aborting\n");
 		goto err_out_free_netdev;
 	}
 
 	err = pci_request_regions(pdev, DRV_NAME);
 	if (err) {
-		printk(KERN_ERR PFX
-			"Cannot request PCI regions, aborting.\n");
+		netdev_err(netdev, "Cannot request PCI regions, aborting\n");
 		goto err_out_disable_device;
 	}
 
@@ -2252,23 +2241,23 @@ static int __devinit enic_probe(struct pci_dev *pdev,
 	if (err) {
 		err = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
 		if (err) {
-			printk(KERN_ERR PFX
-				"No usable DMA configuration, aborting.\n");
+			netdev_err(netdev,
+				   "No usable DMA configuration, aborting\n");
 			goto err_out_release_regions;
 		}
 		err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
 		if (err) {
-			printk(KERN_ERR PFX
-				"Unable to obtain 32-bit DMA "
-				"for consistent allocations, aborting.\n");
+			netdev_err(netdev,
+				   "Unable to obtain %u-bit DMA for consistent allocations, aborting\n",
+				   32);
 			goto err_out_release_regions;
 		}
 	} else {
 		err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(40));
 		if (err) {
-			printk(KERN_ERR PFX
-				"Unable to obtain 40-bit DMA "
-				"for consistent allocations, aborting.\n");
+			netdev_err(netdev,
+				   "Unable to obtain %u-bit DMA for consistent allocations, aborting\n",
+				   40);
 			goto err_out_release_regions;
 		}
 		using_dac = 1;
@@ -2283,8 +2272,7 @@ static int __devinit enic_probe(struct pci_dev *pdev,
 		enic->bar[i].len = pci_resource_len(pdev, i);
 		enic->bar[i].vaddr = pci_iomap(pdev, i, enic->bar[i].len);
 		if (!enic->bar[i].vaddr) {
-			printk(KERN_ERR PFX
-				"Cannot memory-map BAR %d, aborting.\n", i);
+			netdev_err(netdev, "Cannot memory-map BAR %d, aborting\n", i);
 			err = -ENODEV;
 			goto err_out_iounmap;
 		}
@@ -2297,8 +2285,7 @@ static int __devinit enic_probe(struct pci_dev *pdev,
 	enic->vdev = vnic_dev_register(NULL, enic, pdev, enic->bar,
 		ARRAY_SIZE(enic->bar));
 	if (!enic->vdev) {
-		printk(KERN_ERR PFX
-			"vNIC registration failed, aborting.\n");
+		netdev_err(netdev, "vNIC registration failed, aborting\n");
 		err = -ENODEV;
 		goto err_out_iounmap;
 	}
@@ -2308,8 +2295,7 @@ static int __devinit enic_probe(struct pci_dev *pdev,
 
 	err = enic_dev_open(enic);
 	if (err) {
-		printk(KERN_ERR PFX
-			"vNIC dev open failed, aborting.\n");
+		netdev_err(netdev, "vNIC dev open failed, aborting\n");
 		goto err_out_vnic_unregister;
 	}
 
@@ -2326,16 +2312,14 @@ static int __devinit enic_probe(struct pci_dev *pdev,
 	if (!enic_is_dynamic(enic)) {
 		err = vnic_dev_init(enic->vdev, 0);
 		if (err) {
-			printk(KERN_ERR PFX
-				"vNIC dev init failed, aborting.\n");
+			netdev_err(netdev, "vNIC dev init failed, aborting\n");
 			goto err_out_dev_close;
 		}
 	}
 
 	err = enic_dev_init(enic);
 	if (err) {
-		printk(KERN_ERR PFX
-			"Device initialization failed, aborting.\n");
+		netdev_err(netdev, "Device initialization failed, aborting\n");
 		goto err_out_dev_close;
 	}
 
@@ -2361,8 +2345,7 @@ static int __devinit enic_probe(struct pci_dev *pdev,
 
 	err = enic_set_mac_addr(netdev, enic->mac_addr);
 	if (err) {
-		printk(KERN_ERR PFX
-			"Invalid MAC address, aborting.\n");
+		netdev_err(netdev, "Invalid MAC address, aborting\n");
 		goto err_out_dev_deinit;
 	}
 
@@ -2401,8 +2384,7 @@ static int __devinit enic_probe(struct pci_dev *pdev,
 
 	err = register_netdev(netdev);
 	if (err) {
-		printk(KERN_ERR PFX
-			"Cannot register net device, aborting.\n");
+		netdev_err(netdev, "Cannot register net device, aborting\n");
 		goto err_out_dev_deinit;
 	}
 
@@ -2456,7 +2438,7 @@ static struct pci_driver enic_driver = {
 
 static int __init enic_init_module(void)
 {
-	printk(KERN_INFO PFX "%s, ver %s\n", DRV_DESCRIPTION, DRV_VERSION);
+	pr_info("%s, ver %s\n", DRV_DESCRIPTION, DRV_VERSION);
 
 	return pci_register_driver(&enic_driver);
 }
diff --git a/drivers/net/enic/enic_res.c b/drivers/net/enic/enic_res.c
index 9b18840..b69a016 100644
--- a/drivers/net/enic/enic_res.c
+++ b/drivers/net/enic/enic_res.c
@@ -46,7 +46,7 @@ int enic_get_vnic_config(struct enic *enic)
 
 	err = vnic_dev_mac_addr(enic->vdev, enic->mac_addr);
 	if (err) {
-		printk(KERN_ERR PFX "Error getting MAC addr, %d\n", err);
+		netdev_err(enic->netdev, "Error getting MAC addr, %d\n", err);
 		return err;
 	}
 
@@ -56,8 +56,8 @@ int enic_get_vnic_config(struct enic *enic)
 			offsetof(struct vnic_enet_config, m), \
 			sizeof(c->m), &c->m); \
 		if (err) { \
-			printk(KERN_ERR PFX \
-				"Error getting %s, %d\n", #m, err); \
+			netdev_err(enic->netdev, \
+				   "Error getting %s, %d\n", #m, err);	\
 			return err; \
 		} \
 	} while (0)
@@ -92,13 +92,13 @@ int enic_get_vnic_config(struct enic *enic)
 		INTR_COALESCE_HW_TO_USEC(VNIC_INTR_TIMER_MAX),
 		c->intr_timer_usec);
 
-	printk(KERN_INFO PFX "vNIC MAC addr %pM wq/rq %d/%d\n",
-		enic->mac_addr, c->wq_desc_count, c->rq_desc_count);
-	printk(KERN_INFO PFX "vNIC mtu %d csum tx/rx %d/%d tso/lro %d/%d "
-		"intr timer %d usec\n",
-		c->mtu, ENIC_SETTING(enic, TXCSUM),
-		ENIC_SETTING(enic, RXCSUM), ENIC_SETTING(enic, TSO),
-		ENIC_SETTING(enic, LRO), c->intr_timer_usec);
+	netdev_info(enic->netdev, "vNIC MAC addr %pM wq/rq %d/%d\n",
+		    enic->mac_addr, c->wq_desc_count, c->rq_desc_count);
+	netdev_info(enic->netdev, "vNIC mtu %d csum tx/rx %d/%d tso/lro %d/%d "
+		    "intr timer %d usec\n",
+		    c->mtu, ENIC_SETTING(enic, TXCSUM),
+		    ENIC_SETTING(enic, RXCSUM), ENIC_SETTING(enic, TSO),
+		    ENIC_SETTING(enic, LRO), c->intr_timer_usec);
 
 	return 0;
 }
@@ -121,7 +121,7 @@ void enic_add_vlan(struct enic *enic, u16 vlanid)
 
 	err = vnic_dev_cmd(enic->vdev, CMD_VLAN_ADD, &a0, &a1, wait);
 	if (err)
-		printk(KERN_ERR PFX "Can't add vlan id, %d\n", err);
+		netdev_err(enic->netdev, "Can't add vlan id, %d\n", err);
 }
 
 void enic_del_vlan(struct enic *enic, u16 vlanid)
@@ -132,7 +132,7 @@ void enic_del_vlan(struct enic *enic, u16 vlanid)
 
 	err = vnic_dev_cmd(enic->vdev, CMD_VLAN_DEL, &a0, &a1, wait);
 	if (err)
-		printk(KERN_ERR PFX "Can't delete vlan id, %d\n", err);
+		netdev_err(enic->netdev, "Can't delete vlan id, %d\n", err);
 }
 
 int enic_set_nic_cfg(struct enic *enic, u8 rss_default_cpu, u8 rss_hash_type,
@@ -198,10 +198,10 @@ void enic_get_res_counts(struct enic *enic)
 		vnic_dev_get_res_count(enic->vdev, RES_TYPE_INTR_CTRL),
 		ENIC_INTR_MAX);
 
-	printk(KERN_INFO PFX "vNIC resources avail: "
-		"wq %d rq %d cq %d intr %d\n",
-		enic->wq_count, enic->rq_count,
-		enic->cq_count, enic->intr_count);
+	netdev_info(enic->netdev, "vNIC resources avail: "
+		    "wq %d rq %d cq %d intr %d\n",
+		    enic->wq_count, enic->rq_count,
+		    enic->cq_count, enic->intr_count);
 }
 
 void enic_init_vnic_resources(struct enic *enic)
@@ -319,15 +319,14 @@ int enic_alloc_vnic_resources(struct enic *enic)
 
 	intr_mode = vnic_dev_get_intr_mode(enic->vdev);
 
-	printk(KERN_INFO PFX "vNIC resources used:  "
-		"wq %d rq %d cq %d intr %d intr mode %s\n",
-		enic->wq_count, enic->rq_count,
-		enic->cq_count, enic->intr_count,
-		intr_mode == VNIC_DEV_INTR_MODE_INTX ? "legacy PCI INTx" :
-		intr_mode == VNIC_DEV_INTR_MODE_MSI ? "MSI" :
-		intr_mode == VNIC_DEV_INTR_MODE_MSIX ? "MSI-X" :
-		"unknown"
-		);
+	netdev_info(enic->netdev, "vNIC resources used:  "
+		    "wq %d rq %d cq %d intr %d intr mode %s\n",
+		    enic->wq_count, enic->rq_count,
+		    enic->cq_count, enic->intr_count,
+		    intr_mode == VNIC_DEV_INTR_MODE_INTX ? "legacy PCI INTx" :
+		    intr_mode == VNIC_DEV_INTR_MODE_MSI ? "MSI" :
+		    intr_mode == VNIC_DEV_INTR_MODE_MSIX ? "MSI-X" :
+		    "unknown");
 
 	/* Allocate queue resources
 	 */
@@ -373,7 +372,7 @@ int enic_alloc_vnic_resources(struct enic *enic)
 	enic->legacy_pba = vnic_dev_get_res(enic->vdev,
 		RES_TYPE_INTR_PBA_LEGACY, 0);
 	if (!enic->legacy_pba && intr_mode == VNIC_DEV_INTR_MODE_INTX) {
-		printk(KERN_ERR PFX "Failed to hook legacy pba resource\n");
+		netdev_err(enic->netdev, "Failed to hook legacy pba resource\n");
 		err = -ENODEV;
 		goto err_out_cleanup;
 	}
diff --git a/drivers/net/enic/vnic_cq.c b/drivers/net/enic/vnic_cq.c
index 020ae6c..326ea40 100644
--- a/drivers/net/enic/vnic_cq.c
+++ b/drivers/net/enic/vnic_cq.c
@@ -42,7 +42,7 @@ int vnic_cq_alloc(struct vnic_dev *vdev, struct vnic_cq *cq, unsigned int index,
 
 	cq->ctrl = vnic_dev_get_res(vdev, RES_TYPE_CQ, index);
 	if (!cq->ctrl) {
-		printk(KERN_ERR "Failed to hook CQ[%d] resource\n", index);
+		pr_err("Failed to hook CQ[%d] resource\n", index);
 		return -EINVAL;
 	}
 
diff --git a/drivers/net/enic/vnic_dev.c b/drivers/net/enic/vnic_dev.c
index 2b3e16d..6a7fb71 100644
--- a/drivers/net/enic/vnic_dev.c
+++ b/drivers/net/enic/vnic_dev.c
@@ -17,6 +17,8 @@
  *
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/kernel.h>
 #include <linux/errno.h>
 #include <linux/types.h>
@@ -78,22 +80,22 @@ static int vnic_dev_discover_res(struct vnic_dev *vdev,
 		return -EINVAL;
 
 	if (bar->len < VNIC_MAX_RES_HDR_SIZE) {
-		printk(KERN_ERR "vNIC BAR0 res hdr length error\n");
+		pr_err("vNIC BAR0 res hdr length error\n");
 		return -EINVAL;
 	}
 
 	rh = bar->vaddr;
 	if (!rh) {
-		printk(KERN_ERR "vNIC BAR0 res hdr not mem-mapped\n");
+		pr_err("vNIC BAR0 res hdr not mem-mapped\n");
 		return -EINVAL;
 	}
 
 	if (ioread32(&rh->magic) != VNIC_RES_MAGIC ||
 	    ioread32(&rh->version) != VNIC_RES_VERSION) {
-		printk(KERN_ERR "vNIC BAR0 res magic/version error "
-			"exp (%lx/%lx) curr (%x/%x)\n",
-			VNIC_RES_MAGIC, VNIC_RES_VERSION,
-			ioread32(&rh->magic), ioread32(&rh->version));
+		pr_err("vNIC BAR0 res magic/version error "
+		       "exp (%lx/%lx) curr (%x/%x)\n",
+		       VNIC_RES_MAGIC, VNIC_RES_VERSION,
+		       ioread32(&rh->magic), ioread32(&rh->version));
 		return -EINVAL;
 	}
 
@@ -122,7 +124,7 @@ static int vnic_dev_discover_res(struct vnic_dev *vdev,
 			/* each count is stride bytes long */
 			len = count * VNIC_RES_STRIDE;
 			if (len + bar_offset > bar[bar_num].len) {
-				printk(KERN_ERR "vNIC BAR0 resource %d "
+				pr_err("vNIC BAR0 resource %d "
 					"out-of-bounds, offset 0x%x + "
 					"size 0x%x > bar len 0x%lx\n",
 					type, bar_offset,
@@ -229,8 +231,7 @@ int vnic_dev_alloc_desc_ring(struct vnic_dev *vdev, struct vnic_dev_ring *ring,
 		&ring->base_addr_unaligned);
 
 	if (!ring->descs_unaligned) {
-		printk(KERN_ERR
-		  "Failed to allocate ring (size=%d), aborting\n",
+		pr_err("Failed to allocate ring (size=%d), aborting\n",
 			(int)ring->size);
 		return -ENOMEM;
 	}
@@ -268,7 +269,7 @@ int vnic_dev_cmd(struct vnic_dev *vdev, enum vnic_devcmd_cmd cmd,
 
 	status = ioread32(&devcmd->status);
 	if (status & STAT_BUSY) {
-		printk(KERN_ERR "Busy devcmd %d\n", _CMD_N(cmd));
+		pr_err("Busy devcmd %d\n", _CMD_N(cmd));
 		return -EBUSY;
 	}
 
@@ -294,7 +295,7 @@ int vnic_dev_cmd(struct vnic_dev *vdev, enum vnic_devcmd_cmd cmd,
 				err = (int)readq(&devcmd->args[0]);
 				if (err != ERR_ECMDUNKNOWN ||
 				    cmd != CMD_CAPABILITY)
-					printk(KERN_ERR "Error %d devcmd %d\n",
+					pr_err("Error %d devcmd %d\n",
 						err, _CMD_N(cmd));
 				return err;
 			}
@@ -309,7 +310,7 @@ int vnic_dev_cmd(struct vnic_dev *vdev, enum vnic_devcmd_cmd cmd,
 		}
 	}
 
-	printk(KERN_ERR "Timedout devcmd %d\n", _CMD_N(cmd));
+	pr_err("Timedout devcmd %d\n", _CMD_N(cmd));
 	return -ETIMEDOUT;
 }
 
@@ -527,7 +528,7 @@ void vnic_dev_packet_filter(struct vnic_dev *vdev, int directed, int multicast,
 
 	err = vnic_dev_cmd(vdev, CMD_PACKET_FILTER, &a0, &a1, wait);
 	if (err)
-		printk(KERN_ERR "Can't set packet filter\n");
+		pr_err("Can't set packet filter\n");
 }
 
 int vnic_dev_add_addr(struct vnic_dev *vdev, u8 *addr)
@@ -542,7 +543,7 @@ int vnic_dev_add_addr(struct vnic_dev *vdev, u8 *addr)
 
 	err = vnic_dev_cmd(vdev, CMD_ADDR_ADD, &a0, &a1, wait);
 	if (err)
-		printk(KERN_ERR "Can't add addr [%pM], %d\n", addr, err);
+		pr_err("Can't add addr [%pM], %d\n", addr, err);
 
 	return err;
 }
@@ -559,7 +560,7 @@ int vnic_dev_del_addr(struct vnic_dev *vdev, u8 *addr)
 
 	err = vnic_dev_cmd(vdev, CMD_ADDR_DEL, &a0, &a1, wait);
 	if (err)
-		printk(KERN_ERR "Can't del addr [%pM], %d\n", addr, err);
+		pr_err("Can't del addr [%pM], %d\n", addr, err);
 
 	return err;
 }
@@ -572,8 +573,7 @@ int vnic_dev_raise_intr(struct vnic_dev *vdev, u16 intr)
 
 	err = vnic_dev_cmd(vdev, CMD_IAR, &a0, &a1, wait);
 	if (err)
-		printk(KERN_ERR "Failed to raise INTR[%d], err %d\n",
-			intr, err);
+		pr_err("Failed to raise INTR[%d], err %d\n", intr, err);
 
 	return err;
 }
@@ -604,8 +604,7 @@ int vnic_dev_notify_set(struct vnic_dev *vdev, u16 intr)
 	dma_addr_t notify_pa;
 
 	if (vdev->notify || vdev->notify_pa) {
-		printk(KERN_ERR "notify block %p still allocated",
-			vdev->notify);
+		pr_err("notify block %p still allocated", vdev->notify);
 		return -EINVAL;
 	}
 
diff --git a/drivers/net/enic/vnic_intr.c b/drivers/net/enic/vnic_intr.c
index 3934309..617e744 100644
--- a/drivers/net/enic/vnic_intr.c
+++ b/drivers/net/enic/vnic_intr.c
@@ -17,6 +17,8 @@
  *
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/kernel.h>
 #include <linux/errno.h>
 #include <linux/types.h>
@@ -39,8 +41,7 @@ int vnic_intr_alloc(struct vnic_dev *vdev, struct vnic_intr *intr,
 
 	intr->ctrl = vnic_dev_get_res(vdev, RES_TYPE_INTR_CTRL, index);
 	if (!intr->ctrl) {
-		printk(KERN_ERR "Failed to hook INTR[%d].ctrl resource\n",
-			index);
+		pr_err("Failed to hook INTR[%d].ctrl resource\n", index);
 		return -EINVAL;
 	}
 
diff --git a/drivers/net/enic/vnic_rq.c b/drivers/net/enic/vnic_rq.c
index cc580cf..0e456a2 100644
--- a/drivers/net/enic/vnic_rq.c
+++ b/drivers/net/enic/vnic_rq.c
@@ -17,6 +17,8 @@
  *
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/kernel.h>
 #include <linux/errno.h>
 #include <linux/types.h>
@@ -39,7 +41,7 @@ static int vnic_rq_alloc_bufs(struct vnic_rq *rq)
 	for (i = 0; i < blks; i++) {
 		rq->bufs[i] = kzalloc(VNIC_RQ_BUF_BLK_SZ, GFP_ATOMIC);
 		if (!rq->bufs[i]) {
-			printk(KERN_ERR "Failed to alloc rq_bufs\n");
+			pr_err("Failed to alloc rq_bufs\n");
 			return -ENOMEM;
 		}
 	}
@@ -94,7 +96,7 @@ int vnic_rq_alloc(struct vnic_dev *vdev, struct vnic_rq *rq, unsigned int index,
 
 	rq->ctrl = vnic_dev_get_res(vdev, RES_TYPE_RQ, index);
 	if (!rq->ctrl) {
-		printk(KERN_ERR "Failed to hook RQ[%d] resource\n", index);
+		pr_err("Failed to hook RQ[%d] resource\n", index);
 		return -EINVAL;
 	}
 
@@ -174,7 +176,7 @@ int vnic_rq_disable(struct vnic_rq *rq)
 		udelay(10);
 	}
 
-	printk(KERN_ERR "Failed to disable RQ[%d]\n", rq->index);
+	pr_err("Failed to disable RQ[%d]\n", rq->index);
 
 	return -ETIMEDOUT;
 }
diff --git a/drivers/net/enic/vnic_wq.c b/drivers/net/enic/vnic_wq.c
index 1378afb..abf4c27 100644
--- a/drivers/net/enic/vnic_wq.c
+++ b/drivers/net/enic/vnic_wq.c
@@ -17,6 +17,8 @@
  *
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/kernel.h>
 #include <linux/errno.h>
 #include <linux/types.h>
@@ -39,7 +41,7 @@ static int vnic_wq_alloc_bufs(struct vnic_wq *wq)
 	for (i = 0; i < blks; i++) {
 		wq->bufs[i] = kzalloc(VNIC_WQ_BUF_BLK_SZ, GFP_ATOMIC);
 		if (!wq->bufs[i]) {
-			printk(KERN_ERR "Failed to alloc wq_bufs\n");
+			pr_err("Failed to alloc wq_bufs\n");
 			return -ENOMEM;
 		}
 	}
@@ -94,7 +96,7 @@ int vnic_wq_alloc(struct vnic_dev *vdev, struct vnic_wq *wq, unsigned int index,
 
 	wq->ctrl = vnic_dev_get_res(vdev, RES_TYPE_WQ, index);
 	if (!wq->ctrl) {
-		printk(KERN_ERR "Failed to hook WQ[%d] resource\n", index);
+		pr_err("Failed to hook WQ[%d] resource\n", index);
 		return -EINVAL;
 	}
 
@@ -167,7 +169,7 @@ int vnic_wq_disable(struct vnic_wq *wq)
 		udelay(10);
 	}
 
-	printk(KERN_ERR "Failed to disable WQ[%d]\n", wq->index);
+	pr_err("Failed to disable WQ[%d]\n", wq->index);
 
 	return -ETIMEDOUT;
 }

^ permalink raw reply related

* Re: [PATCH] IP: Increment INADDRERRORS if routing for a packet is not successful
From: David Miller @ 2010-06-02 15:29 UTC (permalink / raw)
  To: cl; +Cc: eric.dumazet, netdev, shemminger
In-Reply-To: <alpine.DEB.2.00.1006021025200.30182@router.home>

From: Christoph Lameter <cl@linux-foundation.org>
Date: Wed, 2 Jun 2010 10:27:13 -0500 (CDT)

> LINUX_MIB_INROUTEERRORS? Does it mean I can create a series of new
> counters that allow us to diagnose and distinguish all the different
> causes of packet loss? We would love to have that.

Within reason.

If you're going to spam the tree with something like 10 or 20 new
stat counters getting bumped all over the place, that's not what
we're trying to suggest here.

Consolidate as much as possible, add new things when absolutely
nothing existing fits the bill.

^ permalink raw reply

* Re: [PATCH] IP: Increment INADDRERRORS if routing for a packet is not successful
From: Christoph Lameter @ 2010-06-02 15:27 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, Stephen Hemminger, David Miller
In-Reply-To: <1275430054.2638.115.camel@edumazet-laptop>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1665 bytes --]

On Wed, 2 Jun 2010, Eric Dumazet wrote:

> Le mardi 01 juin 2010 à 16:13 -0500, Christoph Lameter a écrit :
> > Something like this would have been very helpful during recent debugging
> > of multicast issues. Silent discards are bad.
> >
>
> >
> > If the kernel perceives that something is wrong with an incoming packet then the
> > IP stack currently silently discards packets. This makes it difficult to diagnose
> > problems with the network configurations (such as a misbehaving kernel
> > subsystem discarding multicast packets because the reverse path filter
> > does not like multicast subscriptions on the second NIC with rp_filter=1).
> >
> > It is also necessary to know how many inbound packets are discarded to
> > assess networking issues in general with a NIC.
> >
> > Signed-off-by: Christoph Lameter <cl@linux-foundation.org>
> > Acked-by: Stephen Hemminger <shemminger@vyatta.com>
> >
>
> I disagree with this patch.
>
> IPSTATS_MIB_INADDRERRORS has a strong meaning, part of RFCS.
>
> In this path, we simulate the routing of a virtual packet, not its
> delivery.
>
> This should not affect IPSTATS SNMP entries.
>
> You should use another MIB entry, say LINUX_MIB_INROUTEERRORS ?
>
> Dont inet_rtm_getroute() caller gets an error status anyway ?

Yes but they are not increment any counter. If packets are dropped because
of the rp_filter setting interfering f.e. then the packets vanish without
any accounting.

LINUX_MIB_INROUTEERRORS? Does it mean I can create a series of new
counters that allow us to diagnose and distinguish all the different
causes of packet loss? We would love to have that.


^ permalink raw reply

* Re: cls_u32: check unaligned data access
From: David Miller @ 2010-06-02 15:18 UTC (permalink / raw)
  To: xiaosuo; +Cc: hadi, netdev
In-Reply-To: <1275491747-29888-1-git-send-email-xiaosuo@gmail.com>

From: Changli Gao <xiaosuo@gmail.com>
Date: Wed,  2 Jun 2010 23:15:47 +0800

> check unaligned data access
> 
> before accessing data, check if the corresponding address is aligned, and if
> not, return -1.
> 
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>

The user will find out when he gets warnings in his kernel log
messages on platforms where this matters.

And, if anything, silently just skipping over things is not
acceptable.  And imposing a 4-byte alignment could break
existing setups that actually work on x86 and powerpc which
are platforms that don't have alignment issues.

I basically tried to explain to you earlier that I wasn't going to
accept patches that try to deal with alignment in any way here in this
code, we explicitly and intentionally blindly dereference the data.


^ permalink raw reply

* RE: [PATCH] ppp_generic: fix multilink fragment sizes
From: Paoloni, Gabriele @ 2010-06-02 15:17 UTC (permalink / raw)
  To: Ben McKeegan, davem@davemloft.net
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	alan@lxorguk.ukuu.org.uk, linux-ppp@vger.kernel.org,
	paulus@samba.org
In-Reply-To: <1275491094-9526-1-git-send-email-ben@netservers.co.uk>

The proposed patch looks wrong to me.

nbigger is already doing the job; I didn't use DIV_ROUND_UP because in general we don't have always to roundup, otherwise we would exceed the total bandwidth.

Regards

Gabriele Paoloni

-----Original Message-----
From: Ben McKeegan [mailto:ben@netservers.co.uk] 
Sent: 02 June 2010 16:05
To: davem@davemloft.net
Cc: ben@netservers.co.uk; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Paoloni, Gabriele; alan@lxorguk.ukuu.org.uk; linux-ppp@vger.kernel.org; paulus@samba.org
Subject: [PATCH] ppp_generic: fix multilink fragment sizes

Fix bug in multilink fragment size calculation introduced by
commit 9c705260feea6ae329bc6b6d5f6d2ef0227eda0a
"ppp: ppp_mp_explode() redesign"

Signed-off-by: Ben McKeegan <ben@netservers.co.uk>
---
 drivers/net/ppp_generic.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ppp_generic.c b/drivers/net/ppp_generic.c
index 0db3894..8b36bfe 100644
--- a/drivers/net/ppp_generic.c
+++ b/drivers/net/ppp_generic.c
@@ -1416,7 +1416,8 @@ static int ppp_mp_explode(struct ppp *ppp, struct sk_buff *skb)
 		flen = len;
 		if (nfree > 0) {
 			if (pch->speed == 0) {
-				flen = totlen/nfree;
+				if (nfree > 1)
+					flen = DIV_ROUND_UP(len, nfree);
 				if (nbigger > 0) {
 					flen++;
 					nbigger--;
-- 
1.5.6.5

--------------------------------------------------------------
Intel Shannon Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263
Business address: Dromore House, East Park, Shannon, Co. Clare

This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.



^ permalink raw reply related

* cls_u32: check unaligned data access
From: Changli Gao @ 2010-06-02 15:15 UTC (permalink / raw)
  To: Jamal Hadi Salim; +Cc: David S. Miller, netdev, Changli Gao

check unaligned data access

before accessing data, check if the corresponding address is aligned, and if
not, return -1.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
 net/sched/cls_u32.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 4f52214..309d275 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -102,7 +102,8 @@ static int u32_classify(struct sk_buff *skb, struct tcf_proto *tp, struct tcf_re
 	} stack[TC_U32_MAXDEPTH];
 
 	struct tc_u_hnode *ht = (struct tc_u_hnode*)tp->root;
-	unsigned int off = skb_network_offset(skb);
+	unsigned int noff = skb_network_offset(skb);
+	unsigned int off = noff;
 	struct tc_u_knode *n;
 	int sdepth = 0;
 	int off2 = 0;
@@ -138,6 +139,8 @@ next_knode:
 			__be32 *data, _data;
 
 			toff = off + key->off + (off2 & key->offmask);
+			if ((toff - noff) % 4)
+				goto out;
 			data = skb_header_pointer(skb, toff, 4, &_data);
 			if (!data)
 				goto out;
@@ -188,6 +191,8 @@ check_terminal:
 		if (ht->divisor) {
 			__be32 *data, _data;
 
+			if ((off + n->sel.hoff - noff) % 4)
+				goto out;
 			data = skb_header_pointer(skb, off + n->sel.hoff, 4,
 						  &_data);
 			if (!data)
@@ -203,6 +208,8 @@ check_terminal:
 			if (n->sel.flags & TC_U32_VAROFFSET) {
 				__be16 *data, _data;
 
+				if ((off + n->sel.offoff - noff) % 2)
+					goto out;
 				data = skb_header_pointer(skb,
 							  off + n->sel.offoff,
 							  2, &_data);

^ permalink raw reply related

* Re: [PATCH v2] netfilter: Xtables: idletimer target implementation
From: Jan Engelhardt @ 2010-06-02 15:16 UTC (permalink / raw)
  To: Luciano Coelho; +Cc: netdev, netfilter-devel, kaber, Timo Teras
In-Reply-To: <1275486062-23753-1-git-send-email-luciano.coelho@nokia.com>


On Wednesday 2010-06-02 15:41, Luciano Coelho wrote:

>+static int __init idletimer_tg_init(void)
>+{
>+	int ret;
>+
>+	idletimer_tg_kobj = kobject_create_and_add("idletimer",
>+						   &THIS_MODULE->mkobj.kobj);

Isn't this going to oops when you compile this module as =y?


^ permalink raw reply

* Re: [PATCH net-next-2.6] net: replace hooks in __netif_receive_skb V5
From: Eric Dumazet @ 2010-06-02 15:15 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Jiri Pirko, netdev, davem, kaber, Paul E. McKenney
In-Reply-To: <20100602080730.53abea6d@nehalam>

Le mercredi 02 juin 2010 à 08:07 -0700, Stephen Hemminger a écrit :
> On Wed, 2 Jun 2010 09:52:08 +0200
> Jiri Pirko <jpirko@redhat.com> wrote:
> 
> > +
> > +	err = netdev_rx_handler_register(dev, macvlan_handle_frame);
> > +	if (err) {
> > +		rcu_assign_pointer(dev->macvlan_port, NULL);
> > +		kfree(port);
> > +	}
> > +
> > +	return err;
> >  }
> 
> Rcu assign is not necessary here for because the hook didn't
> get registered so there is no way for other CPU to see it.
> 

Thats a valid point, but we should use it, and not care of this litle
detail. Compiler generates same code anyway, since NULL value is tested
by rcu_assign_pointer().

If we dont use rcu_assign_pointer() ourself, Paul or Arnd will put it
one day or another :)

http://lkml.org/lkml/2010/6/1/290




^ permalink raw reply

* Re: [PATCH] phylib: Add support for the LXT973 phy.
From: David Miller @ 2010-06-02 15:15 UTC (permalink / raw)
  To: richardcochran; +Cc: afleming, netdev
In-Reply-To: <20100602150851.GA22591@riccoc20.at.omicron.at>

From: Richard Cochran <richardcochran@gmail.com>
Date: Wed, 2 Jun 2010 17:08:51 +0200

> Well, is it okay to just use the first patch?

Wasn't there another change you were asked to make that got included
in the v2 patch?

^ permalink raw reply

* Re: [PATCH] phylib: Add support for the LXT973 phy.
From: Richard Cochran @ 2010-06-02 15:08 UTC (permalink / raw)
  To: David Miller; +Cc: afleming, netdev
In-Reply-To: <20100602.065017.139108801.davem@davemloft.net>

On Wed, Jun 02, 2010 at 06:50:17AM -0700, David Miller wrote:
> phy_device->priv "could one day get used for a different purpose"?
> What in the world are you smoking Andy?

I think he meant that the 'priv' pointer may one day be used to point
to some dynamically allocated data structure.

> It's clearly a private state pointer for the PHY driver to use,
> full stop.  There is absolutely no ambiguity of what this value
> is and what it is used for and who owns it.  The comments in the
> layout of struct phy_device state this clearly as well.
...
>
> Richard, please respin your patch so that you're using the ->priv
> field like in your original patch.

Well, is it okay to just use the first patch?

The fix needs just one bit of data, and I thought that (ab)using the
'priv' pointer would be okay, if a bit hacky. If, over time, the
driver needs more private data, it should be clear enought that the
existing bit "fiber selected" will also appear in the data structure.

Otherwise, the driver would have to allocate a structure with one
field, just to remember one bit.

Richard

^ permalink raw reply

* Re: [PATCH net-next-2.6] net: replace hooks in __netif_receive_skb V5
From: Stephen Hemminger @ 2010-06-02 15:07 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, kaber, eric.dumazet
In-Reply-To: <20100602075207.GD2603@psychotron.redhat.com>

On Wed, 2 Jun 2010 09:52:08 +0200
Jiri Pirko <jpirko@redhat.com> wrote:

> +
> +	err = netdev_rx_handler_register(dev, macvlan_handle_frame);
> +	if (err) {
> +		rcu_assign_pointer(dev->macvlan_port, NULL);
> +		kfree(port);
> +	}
> +
> +	return err;
>  }

Rcu assign is not necessary here for because the hook didn't
get registered so there is no way for other CPU to see it.

-- 

^ permalink raw reply

* [PATCH] ppp_generic: fix multilink fragment sizes
From: Ben McKeegan @ 2010-06-02 15:04 UTC (permalink / raw)
  To: davem; +Cc: ben, netdev, linux-kernel, gabriele.paoloni, alan, linux-ppp,
	paulus
In-Reply-To: <4C0670E2.1090708@netservers.co.uk>

Fix bug in multilink fragment size calculation introduced by
commit 9c705260feea6ae329bc6b6d5f6d2ef0227eda0a
"ppp: ppp_mp_explode() redesign"

Signed-off-by: Ben McKeegan <ben@netservers.co.uk>
---
 drivers/net/ppp_generic.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ppp_generic.c b/drivers/net/ppp_generic.c
index 0db3894..8b36bfe 100644
--- a/drivers/net/ppp_generic.c
+++ b/drivers/net/ppp_generic.c
@@ -1416,7 +1416,8 @@ static int ppp_mp_explode(struct ppp *ppp, struct sk_buff *skb)
 		flen = len;
 		if (nfree > 0) {
 			if (pch->speed == 0) {
-				flen = totlen/nfree;
+				if (nfree > 1)
+					flen = DIV_ROUND_UP(len, nfree);
 				if (nbigger > 0) {
 					flen++;
 					nbigger--;
-- 
1.5.6.5

^ permalink raw reply related

* Re: [PATCH 00/12] sfc changes for 2.6.36
From: Ben Hutchings @ 2010-06-02 15:04 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <20100602.065711.191161830.davem@davemloft.net>

On Wed, 2010-06-02 at 06:57 -0700, David Miller wrote:
> From: Ben Hutchings <bhutchings@solarflare.com>
> Date: Wed, 02 Jun 2010 14:52:26 +0100
> 
> > Did you apply patch 12 "sfc: Get port number from CS_PORT_NUM, not PCI
> > function number" to net-2.6?
> 
> When you mix bug fixes and cleanups, I'm going to apply it all to
> net-next-2.6 You didn't even specify an intended destination in your
> subject lines, so you leave it entirely open for interpretation and
> my choice.
> 
> So, if you want a bug fix added to net-2.6, tossing it into a series
> which is not wholly appropriate for net-next-2.6 is not that way to
> accomplish that.

Well, I included a comment in the message and left it to your judgement
as to whether it was more appropriate for net-next-2.6 or net-2.6.

> The way to do it is:
> 
> 1) Submit the bug fix, explicitly state "[PATCH net-2.6 xxx] "
>    in your subject line.
> 
> 2) Prepare your net-next-2.6 changes on top of that, and do one
>    of two things:
>    a) Explicitly state in your patch series in the "[PATCH net-next-2.6 0/xxx] "
>       posting "this depends upon the bug fixes posted in the series
>       XXX posted earlier.
>    b) Wait for the bug fix to reach net-next-2.6 when I do a merge,
>       then post your series.

There are no dependencies in this case.

> Otherwise you make life way to miserable and complicated for me,
> the one who has to review and integrate all of your work.

Sorry.

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

* Re: [Patch] fix packet loss and massive ping spikes with PPP multi-link
From: Ben McKeegan @ 2010-06-02 14:55 UTC (permalink / raw)
  To: Paul Mackerras
  Cc: netdev, linux-ppp, Alan Cox, Alexander E. Patrakov,
	Richard Hartmann, linux-kernel, gabriele.paoloni
In-Reply-To: <4C03E1CD.4000809@netservers.co.uk>

Ben McKeegan wrote:
> Paul Mackerras wrote:
> 
>>> I needed to do something similar a while back and I took a very
>>> different approach, which I think is more flexible.   Rather than
>>> implement a new round-robin scheduler I simply introduced a target
>>> minimum fragment size into the fragment size calculation, as a per
>>> bundle parameter that can be configured via a new ioctl.  This
>>> modifies the algorithm so that it tries to limit the number of
>>> fragments such that each fragment is at least the minimum size.  If
>>> the minimum size is greater than the packet size it will not be
>>> fragmented all but will instead just get sent down the next
>>> available channel.
>>>
>>> A pppd plugin generates the ioctl call allowing this to be tweaked
>>> per connection.  It is more flexible in that you can still have the
>>> larger packets fragmented if you wish.
>>
>> I like this a lot better than the other proposed patch.  It adds less
>> code because it uses the fact that ppp_mp_explode() already has a
>> round-robin capability using the ppp->nxchan field, plus it provides a
>> way to control it per bundle via pppd.
>>
>> If you fix up the indentation issues (2-space indent in some of the
>> added code -- if you're using emacs, set c-basic-offset to 8), I'll
>> ack it and hopefully DaveM will pick it up.
> 
> Okay, I'll fix it up when I'm back at work Tuesday and submit (today is 
> a UK public holiday) and also dig out and fix up the userspace code to 
> go with it.

Still working on this, updating the patch wasn't as trivial as I thought 
as it clashed with Gabriele Paoloni's ppp_mp_explode redesign.  However, 
while looking at this code I believe I have found a bug which might have 
been contributing to the poor performance the OP was experiencing.   For 
the case where channel speeds are unknown and there are more than 2 
channels it would miscalculate the fragment sizes so they are not 
balanced on the channels.

Patch for the bug to follow immediately.

Regards,
Ben.

^ permalink raw reply

* [PATCH] act_pedit: access skb->data safely
From: Changli Gao @ 2010-06-02 14:55 UTC (permalink / raw)
  To: Jamal Hadi Salim; +Cc: David S. Miller, netdev, Changli Gao

access skb->data safely

we should use skb_header_pointer() and skb_store_bits() to access skb->data to
handle small or non-linear skbs.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
 net/sched/act_pedit.c |   24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index fdbd0b7..50e3d94 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -125,7 +125,7 @@ static int tcf_pedit(struct sk_buff *skb, struct tc_action *a,
 {
 	struct tcf_pedit *p = a->priv;
 	int i, munged = 0;
-	u8 *pptr;
+	unsigned int off;
 
 	if (!(skb->tc_verd & TC_OK2MUNGE)) {
 		/* should we set skb->cloned? */
@@ -134,7 +134,7 @@ static int tcf_pedit(struct sk_buff *skb, struct tc_action *a,
 		}
 	}
 
-	pptr = skb_network_header(skb);
+	off = skb_network_offset(skb);
 
 	spin_lock(&p->tcf_lock);
 
@@ -144,17 +144,17 @@ static int tcf_pedit(struct sk_buff *skb, struct tc_action *a,
 		struct tc_pedit_key *tkey = p->tcfp_keys;
 
 		for (i = p->tcfp_nkeys; i > 0; i--, tkey++) {
-			u32 *ptr;
+			u32 *ptr, _data;
 			int offset = tkey->off;
 
 			if (tkey->offmask) {
-				if (skb->len > tkey->at) {
-					 char *j = pptr + tkey->at;
-					 offset += ((*j & tkey->offmask) >>
-						   tkey->shift);
-				} else {
+				char *d, _d;
+
+				d = skb_header_pointer(skb, off + tkey->at, 1,
+						       &_d);
+				if (!d)
 					goto bad;
-				}
+				offset += (*d & tkey->offmask) >> tkey->shift;
 			}
 
 			if (offset % 4) {
@@ -169,9 +169,13 @@ static int tcf_pedit(struct sk_buff *skb, struct tc_action *a,
 				goto bad;
 			}
 
-			ptr = (u32 *)(pptr+offset);
+			ptr = skb_header_pointer(skb, off + offset, 4, &_data);
+			if (!ptr)
+				goto bad;
 			/* just do it, baby */
 			*ptr = ((*ptr & tkey->mask) ^ tkey->val);
+			if (ptr == &_data)
+				skb_store_bits(skb, off + offset, ptr, 4);
 			munged++;
 		}
 

^ permalink raw reply related

* Re: [PATCH] net: add additional lock to qdisc to increase throughput
From: Eric Dumazet @ 2010-06-02 14:52 UTC (permalink / raw)
  To: David Miller; +Cc: alexander.h.duyck, netdev
In-Reply-To: <20100602.051019.180420492.davem@davemloft.net>

Le mercredi 02 juin 2010 à 05:10 -0700, David Miller a écrit :
> From: "Duyck, Alexander H" <alexander.h.duyck@intel.com>
> Date: Fri, 21 May 2010 13:04:20 -0700
> 
> > Eric Dumazet wrote:
> >> Tests with following script gave a boost from ~50.000 pps to ~600.000
> >> pps on a dual quad core machine (E5450 @3.00GHz), tg3 driver.
> >> (A single netperf flow can reach ~800.000 pps on this platform)
> >> 
> >> for j in `seq 0 3`; do
> >>   for i in `seq 0 7`; do
> >>     netperf -H 192.168.0.1 -t UDP_STREAM -l 60 -N -T $i -- -m 6 &
> >>   done
> >> done
> > 
> > Running the same script with your patch my results went from 200Kpps to 1.2Mpps on a dual Xeon 5570.
> > 
> > Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> Applied, thanks guys.

Thanks David, I realize you did all the necessary changes after
qdisc_run_begin/qdisc_is_running/ integration ;)



^ permalink raw reply

* Re: [PATCH v3] cls_u32: use skb_header_pointer() to dereference data safely
From: David Miller @ 2010-06-02 14:33 UTC (permalink / raw)
  To: xiaosuo; +Cc: hadi, netdev
In-Reply-To: <1275488718-19588-1-git-send-email-xiaosuo@gmail.com>

From: Changli Gao <xiaosuo@gmail.com>
Date: Wed,  2 Jun 2010 22:25:18 +0800

> use skb_header_pointer() to dereference data safely
> 
> the original skb->data dereference isn't safe, as there isn't any skb->len or
> skb_is_nonlinear() check. skb_header_pointer() is used instead in this patch.
> And when the skb isn't long enough, we terminate the function u32_classify()
> immediately with -1.
> 
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>

Looks good, applied.

^ permalink raw reply

* [PATCH v3] cls_u32: use skb_header_pointer() to dereference data safely
From: Changli Gao @ 2010-06-02 14:25 UTC (permalink / raw)
  To: Jamal Hadi Salim; +Cc: David S. Miller, netdev, Changli Gao

use skb_header_pointer() to dereference data safely

the original skb->data dereference isn't safe, as there isn't any skb->len or
skb_is_nonlinear() check. skb_header_pointer() is used instead in this patch.
And when the skb isn't long enough, we terminate the function u32_classify()
immediately with -1.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
 net/sched/cls_u32.c |   49 ++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 13 deletions(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 9627542..4f52214 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -98,11 +98,11 @@ static int u32_classify(struct sk_buff *skb, struct tcf_proto *tp, struct tcf_re
 {
 	struct {
 		struct tc_u_knode *knode;
-		u8		  *ptr;
+		unsigned int	  off;
 	} stack[TC_U32_MAXDEPTH];
 
 	struct tc_u_hnode *ht = (struct tc_u_hnode*)tp->root;
-	u8 *ptr = skb_network_header(skb);
+	unsigned int off = skb_network_offset(skb);
 	struct tc_u_knode *n;
 	int sdepth = 0;
 	int off2 = 0;
@@ -134,8 +134,14 @@ next_knode:
 #endif
 
 		for (i = n->sel.nkeys; i>0; i--, key++) {
-
-			if ((*(__be32*)(ptr+key->off+(off2&key->offmask))^key->val)&key->mask) {
+			unsigned int toff;
+			__be32 *data, _data;
+
+			toff = off + key->off + (off2 & key->offmask);
+			data = skb_header_pointer(skb, toff, 4, &_data);
+			if (!data)
+				goto out;
+			if ((*data ^ key->val) & key->mask) {
 				n = n->next;
 				goto next_knode;
 			}
@@ -174,29 +180,45 @@ check_terminal:
 		if (sdepth >= TC_U32_MAXDEPTH)
 			goto deadloop;
 		stack[sdepth].knode = n;
-		stack[sdepth].ptr = ptr;
+		stack[sdepth].off = off;
 		sdepth++;
 
 		ht = n->ht_down;
 		sel = 0;
-		if (ht->divisor)
-			sel = ht->divisor&u32_hash_fold(*(__be32*)(ptr+n->sel.hoff), &n->sel,n->fshift);
-
+		if (ht->divisor) {
+			__be32 *data, _data;
+
+			data = skb_header_pointer(skb, off + n->sel.hoff, 4,
+						  &_data);
+			if (!data)
+				goto out;
+			sel = ht->divisor & u32_hash_fold(*data, &n->sel,
+							  n->fshift);
+		}
 		if (!(n->sel.flags&(TC_U32_VAROFFSET|TC_U32_OFFSET|TC_U32_EAT)))
 			goto next_ht;
 
 		if (n->sel.flags&(TC_U32_OFFSET|TC_U32_VAROFFSET)) {
 			off2 = n->sel.off + 3;
-			if (n->sel.flags&TC_U32_VAROFFSET)
-				off2 += ntohs(n->sel.offmask & *(__be16*)(ptr+n->sel.offoff)) >>n->sel.offshift;
+			if (n->sel.flags & TC_U32_VAROFFSET) {
+				__be16 *data, _data;
+
+				data = skb_header_pointer(skb,
+							  off + n->sel.offoff,
+							  2, &_data);
+				if (!data)
+					goto out;
+				off2 += ntohs(n->sel.offmask & *data) >>
+					n->sel.offshift;
+			}
 			off2 &= ~3;
 		}
 		if (n->sel.flags&TC_U32_EAT) {
-			ptr += off2;
+			off += off2;
 			off2 = 0;
 		}
 
-		if (ptr < skb_tail_pointer(skb))
+		if (off < skb->len)
 			goto next_ht;
 	}
 
@@ -204,9 +226,10 @@ check_terminal:
 	if (sdepth--) {
 		n = stack[sdepth].knode;
 		ht = n->ht_up;
-		ptr = stack[sdepth].ptr;
+		off = stack[sdepth].off;
 		goto check_terminal;
 	}
+out:
 	return -1;
 
 deadloop:

^ permalink raw reply related

* Re: [Regression] [2.6.35-rc1] ssb_is_sprom_available
From: Maciej Rutecki @ 2010-06-02 14:20 UTC (permalink / raw)
  To: John W. Linville
  Cc: Matthew Wilcox, linux-kernel@vger.kernel.org, linux-pci,
	linux-usb, Rafael J. Wysocki, netdev, Michael Buesch
In-Reply-To: <20100601133402.GB26701@tuxdriver.com>

On wtorek, 1 czerwca 2010 o 15:34:02 John W. Linville wrote:
> On Mon, May 31, 2010 at 02:53:00PM -0600, Matthew Wilcox wrote:
> > On Mon, May 31, 2010 at 09:55:20PM +0200, Maciej Rutecki wrote:
> > > Last known good: 2.6.34
> > > Failing kernel: 2.6.35-rc1
> > >
> > > subsystem: PCI, USB(?)
> > >
> > > Kernel dies during booting on message "ssb_is_sprom_available", see
> > > picture:
> > > http://www.unixy.pl/maciek/download/kernel/2.6.35-rc1/gumis/DSC_0011.JP
> > >G
> >
> > Um, looks like it's something to do with the Sonics Silicon Backplane,
> > not PCI, nor USB.
> 
> Fix is on its way...
> 
> commit da1fdb02d9200ff28b6f3a380d21930335fe5429
> Author: Christoph Fritz <chf.fritz@googlemail.com>
> Date:   Fri May 28 10:45:59 2010 +0200
> 
>     ssb: fix NULL ptr deref when pcihost_wrapper is used
> 
>     Ethernet driver b44 does register ssb by it's pcihost_wrapper
>     and doesn't set ssb_chipcommon. A check on this value
>     introduced with commit d53cdbb94a52a920d5420ed64d986c3523a56743
>     and ea2db495f92ad2cf3301623e60cb95b4062bc484 triggers:
> 
>     BUG: unable to handle kernel NULL pointer dereference at 00000010
>     IP: [<c1266c36>] ssb_is_sprom_available+0x16/0x30
> 
>     Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com>
>     Signed-off-by: John W. Linville <linville@tuxdriver.com>
> 

Yes this commit solves problem. Thanks

Tested-by: Maciej Rutecki <maciej.rutecki@gmail.com>

-- 
Maciej Rutecki
http://www.maciek.unixy.pl

^ permalink raw reply

* Re: [PATCH] TCP: tcp_hybla: Fix integer overflow in slow start increment
From: David Miller @ 2010-06-02 14:16 UTC (permalink / raw)
  To: root; +Cc: netdev, kuznet, linux-kernel
In-Reply-To: <1275480124-24383-1-git-send-email-root@danielinux.net>

From: Daniele Lacamera <root@danielinux.net>
Date: Wed,  2 Jun 2010 14:02:04 +0200

> From: root <root@kitchen.(none)>

Please do not include incomplete authorship tags in your patch postings
like this, I used the more complete one from your email headers.

> For large values of rtt, 2^rho operation may overflow u32. Clamp down the increment to 2^16.
> Signed-off-by: Daniele Lacamera <root@danielinux.net>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH v2] cls_u32: use skb_header_pointer() to dereference data safely
From: David Miller @ 2010-06-02 14:15 UTC (permalink / raw)
  To: xiaosuo; +Cc: hadi, netdev
In-Reply-To: <1275487236-14452-1-git-send-email-xiaosuo@gmail.com>

From: Changli Gao <xiaosuo@gmail.com>
Date: Wed,  2 Jun 2010 22:00:36 +0800

> use skb_header_pointer() to dereference data safely
> 
> the original skb->data dereference isn't safe, as there isn't any skb->len or
> skb_is_nonlinear() check. skb_header_pointer() is used instead in this patch.
> And when the skb isn't long enough, we terminate the function u32_classify()
> immediately with -1. Unaligned access is also fixed.
> 
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>

Please don't do so many things at once Changli.

Everything is fine except the get_unaligned() addition.  It's
intentionally not there, and by adding the get_unaligned() you're
going to kill performance as this expands to 4 byte loads per u32 key
check on RISC systems.

If it's not aligned, that's a path or configuration that will need to
be fixed up such that the accesses are aligned.

Please respin this patch with the get_unaligned() part removed.

Thanks.

^ permalink raw reply

* Re: [PATCH net-next-2.6] net: replace hooks in __netif_receive_skb V5
From: David Miller @ 2010-06-02 14:11 UTC (permalink / raw)
  To: jpirko; +Cc: netdev, kaber, eric.dumazet, shemminger
In-Reply-To: <20100602075207.GD2603@psychotron.redhat.com>

From: Jiri Pirko <jpirko@redhat.com>
Date: Wed, 2 Jun 2010 09:52:08 +0200

> What this patch does is it removes two receive frame hooks (for bridge and for
> macvlan) from __netif_receive_skb. These are replaced them with a single
> hook for both. It only supports one hook per device because it makes no
> sense to do bridging and macvlan on the same device.
> 
> Then a network driver (of virtual netdev like macvlan or bridge) can register
> an rx_handler for needed net device.
> 
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Ok, this looks good to me.  Applied, thanks everyone.

^ permalink raw reply

* Re: [PATCHv3] Refactor update of IPv6 flowi destination address for srcrt (RH) option
From: David Miller @ 2010-06-02 14:08 UTC (permalink / raw)
  To: arno; +Cc: joe, yoshfuji, netdev
In-Reply-To: <87sk55q44a.fsf_-_@small.ssi.corp>

From: arno@natisbad.org (Arnaud Ebalard)
Date: Wed, 02 Jun 2010 09:35:01 +0200

> Here is a v3 based on the comments I received from Joe. The helper is
> now in exthdrs.c (no more inline) and has a proper name.
> 
> Applies on net-2.6. Tested on 2.6.34.

Looks good, applied to net-next-2.6, thanks.

^ permalink raw reply

* Re: [PATCH 2/2] mac8390: raise error logging priority
From: David Miller @ 2010-06-02 14:06 UTC (permalink / raw)
  To: fthain; +Cc: geert, joe, p_gortmaker, netdev, linux-kernel, linux-m68k
In-Reply-To: <alpine.OSX.2.00.1006011739160.299@localhost>

From: Finn Thain <fthain@telegraphics.com.au>
Date: Tue, 1 Jun 2010 22:18:56 +1000 (EST)

> Log error conditions using KERN_ERR priority.
> 
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

Applied, thank you.

^ 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