Netdev List
 help / color / mirror / Atom feed
* [net-next-2.6 PATCH 3/3] enic: Update MAINTAINERS
From: Vasanthy Kolluri @ 2010-09-30 23:36 UTC (permalink / raw)
  To: davem; +Cc: netdev, shemminger, roprabhu, dwang2
In-Reply-To: <20100930233412.16528.52523.stgit@savbu-pc100.cisco.com>

From: Vasanthy Kolluri <vkolluri@cisco.com>

Update MAINTAINERS list

Signed-off-by: Vasanthy Kolluri <vkolluri@cisco.com>
Signed-off-by: Roopa Prabhu <roprabhu@cisco.com>
Signed-off-by: David Wang <dwang2@cisco.com>
---
 MAINTAINERS |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


diff --git a/MAINTAINERS b/MAINTAINERS
index 3168d0c..9ddb5ac 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1567,9 +1567,9 @@ S:	Supported
 F:	scripts/checkpatch.pl
 
 CISCO VIC ETHERNET NIC DRIVER
-M:	Scott Feldman <scofeldm@cisco.com>
 M:	Vasanthy Kolluri <vkolluri@cisco.com>
 M:	Roopa Prabhu <roprabhu@cisco.com>
+M:	David Wang <dwang2@cisco.com>
 S:	Supported
 F:	drivers/net/enic/
 


^ permalink raw reply related

* [PATCH 9/9] isdn/gigaset: improve bas_gigaset USB error reporting
From: Tilman Schmidt @ 2010-09-30 23:35 UTC (permalink / raw)
  To: Karsten Keil, David Miller
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, netdev, linux-kernel
In-Reply-To: <20100930-patch-gigaset-00.tilman@imap.cc>

Rephrase some USB error messages to make them clearer and more consistent.
Downgrade some warning messages that may occur during normal operation to
debug messages.

Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
 drivers/isdn/gigaset/bas-gigaset.c |  112 +++++++++++++++++++----------------
 1 files changed, 61 insertions(+), 51 deletions(-)

diff --git a/drivers/isdn/gigaset/bas-gigaset.c b/drivers/isdn/gigaset/bas-gigaset.c
index 71e3fde..178942a 100644
--- a/drivers/isdn/gigaset/bas-gigaset.c
+++ b/drivers/isdn/gigaset/bas-gigaset.c
@@ -172,7 +172,7 @@ static char *get_usb_rcmsg(int rc)
 	case -EAGAIN:
 		return "start frame too early or too much scheduled";
 	case -EFBIG:
-		return "too many isochronous frames requested";
+		return "too many isoc frames requested";
 	case -EPIPE:
 		return "endpoint stalled";
 	case -EMSGSIZE:
@@ -203,13 +203,13 @@ static char *get_usb_statmsg(int status)
 	case -ENOENT:
 		return "unlinked (sync)";
 	case -EINPROGRESS:
-		return "pending";
+		return "URB still pending";
 	case -EPROTO:
-		return "bit stuffing error, timeout, or unknown USB error";
+		return "bitstuff error, timeout, or unknown USB error";
 	case -EILSEQ:
 		return "CRC mismatch, timeout, or unknown USB error";
 	case -ETIME:
-		return "timed out";
+		return "USB response timeout";
 	case -EPIPE:
 		return "endpoint stalled";
 	case -ECOMM:
@@ -217,15 +217,15 @@ static char *get_usb_statmsg(int status)
 	case -ENOSR:
 		return "OUT buffer underrun";
 	case -EOVERFLOW:
-		return "too much data";
+		return "endpoint babble";
 	case -EREMOTEIO:
-		return "short packet detected";
+		return "short packet";
 	case -ENODEV:
 		return "device removed";
 	case -EXDEV:
-		return "partial isochronous transfer";
+		return "partial isoc transfer";
 	case -EINVAL:
-		return "invalid argument";
+		return "ISO madness";
 	case -ECONNRESET:
 		return "unlinked (async)";
 	case -ESHUTDOWN:
@@ -872,6 +872,7 @@ static void read_iso_callback(struct urb *urb)
 		tasklet_hi_schedule(&ubc->rcvd_tasklet);
 	} else {
 		/* tasklet still busy, drop data and resubmit URB */
+		gig_dbg(DEBUG_ISO, "%s: overrun", __func__);
 		ubc->loststatus = status;
 		for (i = 0; i < BAS_NUMFRAMES; i++) {
 			ubc->isoinlost += urb->iso_frame_desc[i].actual_length;
@@ -887,13 +888,11 @@ static void read_iso_callback(struct urb *urb)
 			urb->dev = bcs->cs->hw.bas->udev;
 			urb->transfer_flags = URB_ISO_ASAP;
 			urb->number_of_packets = BAS_NUMFRAMES;
-			gig_dbg(DEBUG_ISO, "%s: isoc read overrun/resubmit",
-				__func__);
 			rc = usb_submit_urb(urb, GFP_ATOMIC);
 			if (unlikely(rc != 0 && rc != -ENODEV)) {
 				dev_err(bcs->cs->dev,
-					"could not resubmit isochronous read "
-					"URB: %s\n", get_usb_rcmsg(rc));
+				       "could not resubmit isoc read URB: %s\n",
+					get_usb_rcmsg(rc));
 				dump_urb(DEBUG_ISO, "isoc read", urb);
 				error_hangup(bcs);
 			}
@@ -1135,7 +1134,7 @@ static int submit_iso_write_urb(struct isow_urbctx_t *ucx)
 			gig_dbg(DEBUG_ISO, "%s: disconnected", __func__);
 		else
 			dev_err(ucx->bcs->cs->dev,
-				"could not submit isochronous write URB: %s\n",
+				"could not submit isoc write URB: %s\n",
 				get_usb_rcmsg(rc));
 		return rc;
 	}
@@ -1180,7 +1179,7 @@ static void write_iso_tasklet(unsigned long data)
 		ubc->isooutovfl = NULL;
 		spin_unlock_irqrestore(&ubc->isooutlock, flags);
 		if (ovfl) {
-			dev_err(cs->dev, "isochronous write buffer underrun\n");
+			dev_err(cs->dev, "isoc write underrun\n");
 			error_hangup(bcs);
 			break;
 		}
@@ -1205,7 +1204,7 @@ static void write_iso_tasklet(unsigned long data)
 				if (next) {
 					/* couldn't put it back */
 					dev_err(cs->dev,
-					      "losing isochronous write URB\n");
+						"losing isoc write URB\n");
 					error_hangup(bcs);
 				}
 			}
@@ -1232,10 +1231,10 @@ static void write_iso_tasklet(unsigned long data)
 				if (ifd->status ||
 				    ifd->actual_length != ifd->length) {
 					dev_warn(cs->dev,
-					     "isochronous write: frame %d: %s, "
-					     "only %d of %d bytes sent\n",
-					     i, get_usb_statmsg(ifd->status),
-					     ifd->actual_length, ifd->length);
+					    "isoc write: frame %d[%d/%d]: %s\n",
+						 i, ifd->actual_length,
+						 ifd->length,
+						 get_usb_statmsg(ifd->status));
 					offset = (ifd->offset +
 						  ifd->actual_length)
 						 % BAS_OUTBUFSIZE;
@@ -1244,11 +1243,11 @@ static void write_iso_tasklet(unsigned long data)
 			}
 			break;
 		case -EPIPE:			/* stall - probably underrun */
-			dev_err(cs->dev, "isochronous write stalled\n");
+			dev_err(cs->dev, "isoc write: stalled\n");
 			error_hangup(bcs);
 			break;
-		default:			/* severe trouble */
-			dev_warn(cs->dev, "isochronous write: %s\n",
+		default:			/* other errors */
+			dev_warn(cs->dev, "isoc write: %s\n",
 				 get_usb_statmsg(status));
 		}
 
@@ -1304,6 +1303,7 @@ static void read_iso_tasklet(unsigned long data)
 	struct cardstate *cs = bcs->cs;
 	struct urb *urb;
 	int status;
+	struct usb_iso_packet_descriptor *ifd;
 	char *rcvbuf;
 	unsigned long flags;
 	int totleft, numbytes, offset, frame, rc;
@@ -1321,8 +1321,7 @@ static void read_iso_tasklet(unsigned long data)
 		ubc->isoindone = NULL;
 		if (unlikely(ubc->loststatus != -EINPROGRESS)) {
 			dev_warn(cs->dev,
-				 "isochronous read overrun, "
-				 "dropped URB with status: %s, %d bytes lost\n",
+		"isoc read overrun, URB dropped (status: %s, %d bytes)\n",
 				 get_usb_statmsg(ubc->loststatus),
 				 ubc->isoinlost);
 			ubc->loststatus = -EINPROGRESS;
@@ -1352,11 +1351,11 @@ static void read_iso_tasklet(unsigned long data)
 				__func__, get_usb_statmsg(status));
 			continue;		/* -> skip */
 		case -EPIPE:
-			dev_err(cs->dev, "isochronous read stalled\n");
+			dev_err(cs->dev, "isoc read: stalled\n");
 			error_hangup(bcs);
 			continue;		/* -> skip */
-		default:			/* severe trouble */
-			dev_warn(cs->dev, "isochronous read: %s\n",
+		default:			/* other error */
+			dev_warn(cs->dev, "isoc read: %s\n",
 				 get_usb_statmsg(status));
 			goto error;
 		}
@@ -1364,40 +1363,52 @@ static void read_iso_tasklet(unsigned long data)
 		rcvbuf = urb->transfer_buffer;
 		totleft = urb->actual_length;
 		for (frame = 0; totleft > 0 && frame < BAS_NUMFRAMES; frame++) {
-			numbytes = urb->iso_frame_desc[frame].actual_length;
-			if (unlikely(urb->iso_frame_desc[frame].status))
+			ifd = &urb->iso_frame_desc[frame];
+			numbytes = ifd->actual_length;
+			switch (ifd->status) {
+			case 0:			/* success */
+				break;
+			case -EPROTO:		/* protocol error or unplug */
+			case -EILSEQ:
+			case -ETIME:
+				/* probably just disconnected, ignore */
+				gig_dbg(DEBUG_ISO,
+					"isoc read: frame %d[%d]: %s\n",
+					frame, numbytes,
+					get_usb_statmsg(ifd->status));
+				break;
+			default:		/* other error */
+				/* report, assume transferred bytes are ok */
 				dev_warn(cs->dev,
-					 "isochronous read: frame %d[%d]: %s\n",
+					 "isoc read: frame %d[%d]: %s\n",
 					 frame, numbytes,
-					 get_usb_statmsg(
-					    urb->iso_frame_desc[frame].status));
+					 get_usb_statmsg(ifd->status));
+			}
 			if (unlikely(numbytes > BAS_MAXFRAME))
 				dev_warn(cs->dev,
-					 "isochronous read: frame %d: "
-					 "numbytes (%d) > BAS_MAXFRAME\n",
-					 frame, numbytes);
+					 "isoc read: frame %d[%d]: %s\n",
+					 frame, numbytes,
+					 "exceeds max frame size");
 			if (unlikely(numbytes > totleft)) {
 				dev_warn(cs->dev,
-					 "isochronous read: frame %d: "
-					 "numbytes (%d) > totleft (%d)\n",
-					 frame, numbytes, totleft);
+					 "isoc read: frame %d[%d]: %s\n",
+					 frame, numbytes,
+					 "exceeds total transfer length");
 				numbytes = totleft;
 			}
-			offset = urb->iso_frame_desc[frame].offset;
+			offset = ifd->offset;
 			if (unlikely(offset + numbytes > BAS_INBUFSIZE)) {
 				dev_warn(cs->dev,
-					 "isochronous read: frame %d: "
-					 "offset (%d) + numbytes (%d) "
-					 "> BAS_INBUFSIZE\n",
-					 frame, offset, numbytes);
+					 "isoc read: frame %d[%d]: %s\n",
+					 frame, numbytes,
+					 "exceeds end of buffer");
 				numbytes = BAS_INBUFSIZE - offset;
 			}
 			gigaset_isoc_receive(rcvbuf + offset, numbytes, bcs);
 			totleft -= numbytes;
 		}
 		if (unlikely(totleft > 0))
-			dev_warn(cs->dev,
-				 "isochronous read: %d data bytes missing\n",
+			dev_warn(cs->dev, "isoc read: %d data bytes missing\n",
 				 totleft);
 
 error:
@@ -1413,9 +1424,9 @@ error:
 		rc = usb_submit_urb(urb, GFP_ATOMIC);
 		if (unlikely(rc != 0 && rc != -ENODEV)) {
 			dev_err(cs->dev,
-				"could not resubmit isochronous read URB: %s\n",
+				"could not resubmit isoc read URB: %s\n",
 				get_usb_rcmsg(rc));
-			dump_urb(DEBUG_ISO, "resubmit iso read", urb);
+			dump_urb(DEBUG_ISO, "resubmit isoc read", urb);
 			error_hangup(bcs);
 		}
 	}
@@ -1647,8 +1658,7 @@ static int gigaset_init_bchannel(struct bc_state *bcs)
 
 	if (cs->hw.bas->basstate & BS_SUSPEND) {
 		dev_notice(cs->dev,
-			   "not starting isochronous I/O, "
-			   "suspend in progress\n");
+			   "not starting isoc I/O, suspend in progress\n");
 		spin_unlock_irqrestore(&cs->lock, flags);
 		return -EHOSTUNREACH;
 	}
@@ -1657,7 +1667,7 @@ static int gigaset_init_bchannel(struct bc_state *bcs)
 	if (ret < 0) {
 		spin_unlock_irqrestore(&cs->lock, flags);
 		dev_err(cs->dev,
-			"could not start isochronous I/O for channel B%d: %s\n",
+			"could not start isoc I/O for channel B%d: %s\n",
 			bcs->channel + 1,
 			ret == -EFAULT ? "null URB" : get_usb_rcmsg(ret));
 		if (ret != -ENODEV)
@@ -2086,7 +2096,7 @@ static int gigaset_freebcshw(struct bc_state *bcs)
 
 	/* kill URBs and tasklets before freeing - better safe than sorry */
 	ubc->running = 0;
-	gig_dbg(DEBUG_INIT, "%s: killing iso URBs", __func__);
+	gig_dbg(DEBUG_INIT, "%s: killing isoc URBs", __func__);
 	for (i = 0; i < BAS_OUTURBS; ++i) {
 		usb_kill_urb(ubc->isoouturbs[i].urb);
 		usb_free_urb(ubc->isoouturbs[i].urb);
-- 
1.7.3.15.g442cb

^ permalink raw reply related

* [net-next-2.6 PATCH 2/3] enic: Make local functions static
From: Vasanthy Kolluri @ 2010-09-30 23:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, shemminger, roprabhu, dwang2
In-Reply-To: <20100930233412.16528.52523.stgit@savbu-pc100.cisco.com>

From: Vasanthy Kolluri <vkolluri@cisco.com>

Make functions used locally in a file as static

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: Vasanthy Kolluri <vkolluri@cisco.com>
Signed-off-by: Roopa Prabhu <roprabhu@cisco.com>
Signed-off-by: David Wang <dwang2@cisco.com>
---
 drivers/net/enic/enic_main.c |    6 +++---
 drivers/net/enic/vnic_dev.c  |   10 +++++-----
 drivers/net/enic/vnic_dev.h  |    7 -------
 drivers/net/enic/vnic_rq.c   |    2 +-
 drivers/net/enic/vnic_rq.h   |    4 ----
 drivers/net/enic/vnic_wq.c   |    2 +-
 drivers/net/enic/vnic_wq.h   |    4 ----
 7 files changed, 10 insertions(+), 25 deletions(-)


diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 711077a..a1f92f1 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -1972,7 +1972,7 @@ static int enic_dev_hang_notify(struct enic *enic)
 	return err;
 }
 
-int enic_dev_set_ig_vlan_rewrite_mode(struct enic *enic)
+static int enic_dev_set_ig_vlan_rewrite_mode(struct enic *enic)
 {
 	int err;
 
@@ -2147,14 +2147,14 @@ static const struct net_device_ops enic_netdev_ops = {
 #endif
 };
 
-void enic_dev_deinit(struct enic *enic)
+static void enic_dev_deinit(struct enic *enic)
 {
 	netif_napi_del(&enic->napi);
 	enic_free_vnic_resources(enic);
 	enic_clear_intr_mode(enic);
 }
 
-int enic_dev_init(struct enic *enic)
+static int enic_dev_init(struct enic *enic)
 {
 	struct device *dev = enic_get_dev(enic);
 	struct net_device *netdev = enic->netdev;
diff --git a/drivers/net/enic/vnic_dev.c b/drivers/net/enic/vnic_dev.c
index f99ddee..11dc8f7 100644
--- a/drivers/net/enic/vnic_dev.c
+++ b/drivers/net/enic/vnic_dev.c
@@ -186,7 +186,7 @@ void __iomem *vnic_dev_get_res(struct vnic_dev *vdev, enum vnic_res_type type,
 	}
 }
 
-unsigned int vnic_dev_desc_ring_size(struct vnic_dev_ring *ring,
+static unsigned int vnic_dev_desc_ring_size(struct vnic_dev_ring *ring,
 	unsigned int desc_count, unsigned int desc_size)
 {
 	/* The base address of the desc rings must be 512 byte aligned.
@@ -525,14 +525,14 @@ int vnic_dev_open_done(struct vnic_dev *vdev, int *done)
 	return 0;
 }
 
-int vnic_dev_soft_reset(struct vnic_dev *vdev, int arg)
+static int vnic_dev_soft_reset(struct vnic_dev *vdev, int arg)
 {
 	u64 a0 = (u32)arg, a1 = 0;
 	int wait = 1000;
 	return vnic_dev_cmd(vdev, CMD_SOFT_RESET, &a0, &a1, wait);
 }
 
-int vnic_dev_soft_reset_done(struct vnic_dev *vdev, int *done)
+static int vnic_dev_soft_reset_done(struct vnic_dev *vdev, int *done)
 {
 	u64 a0 = 0, a1 = 0;
 	int wait = 1000;
@@ -681,7 +681,7 @@ int vnic_dev_set_ig_vlan_rewrite_mode(struct vnic_dev *vdev,
 	return err;
 }
 
-int vnic_dev_notify_setcmd(struct vnic_dev *vdev,
+static int vnic_dev_notify_setcmd(struct vnic_dev *vdev,
 	void *notify_addr, dma_addr_t notify_pa, u16 intr)
 {
 	u64 a0, a1;
@@ -720,7 +720,7 @@ int vnic_dev_notify_set(struct vnic_dev *vdev, u16 intr)
 	return vnic_dev_notify_setcmd(vdev, notify_addr, notify_pa, intr);
 }
 
-int vnic_dev_notify_unsetcmd(struct vnic_dev *vdev)
+static int vnic_dev_notify_unsetcmd(struct vnic_dev *vdev)
 {
 	u64 a0, a1;
 	int wait = 1000;
diff --git a/drivers/net/enic/vnic_dev.h b/drivers/net/enic/vnic_dev.h
index 008304b..3f00143 100644
--- a/drivers/net/enic/vnic_dev.h
+++ b/drivers/net/enic/vnic_dev.h
@@ -84,8 +84,6 @@ unsigned int vnic_dev_get_res_count(struct vnic_dev *vdev,
 	enum vnic_res_type type);
 void __iomem *vnic_dev_get_res(struct vnic_dev *vdev, enum vnic_res_type type,
 	unsigned int index);
-unsigned int vnic_dev_desc_ring_size(struct vnic_dev_ring *ring,
-	unsigned int desc_count, unsigned int desc_size);
 void vnic_dev_clear_desc_ring(struct vnic_dev_ring *ring);
 int vnic_dev_alloc_desc_ring(struct vnic_dev *vdev, struct vnic_dev_ring *ring,
 	unsigned int desc_count, unsigned int desc_size);
@@ -106,10 +104,7 @@ int vnic_dev_packet_filter(struct vnic_dev *vdev, int directed, int multicast,
 int vnic_dev_add_addr(struct vnic_dev *vdev, u8 *addr);
 int vnic_dev_del_addr(struct vnic_dev *vdev, u8 *addr);
 int vnic_dev_mac_addr(struct vnic_dev *vdev, u8 *mac_addr);
-int vnic_dev_notify_setcmd(struct vnic_dev *vdev,
-	void *notify_addr, dma_addr_t notify_pa, u16 intr);
 int vnic_dev_notify_set(struct vnic_dev *vdev, u16 intr);
-int vnic_dev_notify_unsetcmd(struct vnic_dev *vdev);
 int vnic_dev_notify_unset(struct vnic_dev *vdev);
 int vnic_dev_link_status(struct vnic_dev *vdev);
 u32 vnic_dev_port_speed(struct vnic_dev *vdev);
@@ -124,8 +119,6 @@ int vnic_dev_init(struct vnic_dev *vdev, int arg);
 int vnic_dev_init_done(struct vnic_dev *vdev, int *done, int *err);
 int vnic_dev_init_prov(struct vnic_dev *vdev, u8 *buf, u32 len);
 int vnic_dev_deinit(struct vnic_dev *vdev);
-int vnic_dev_soft_reset(struct vnic_dev *vdev, int arg);
-int vnic_dev_soft_reset_done(struct vnic_dev *vdev, int *done);
 int vnic_dev_hang_reset(struct vnic_dev *vdev, int arg);
 int vnic_dev_hang_reset_done(struct vnic_dev *vdev, int *done);
 void vnic_dev_set_intr_mode(struct vnic_dev *vdev,
diff --git a/drivers/net/enic/vnic_rq.c b/drivers/net/enic/vnic_rq.c
index b236d7c..34105e0 100644
--- a/drivers/net/enic/vnic_rq.c
+++ b/drivers/net/enic/vnic_rq.c
@@ -115,7 +115,7 @@ int vnic_rq_alloc(struct vnic_dev *vdev, struct vnic_rq *rq, unsigned int index,
 	return 0;
 }
 
-void vnic_rq_init_start(struct vnic_rq *rq, unsigned int cq_index,
+static void vnic_rq_init_start(struct vnic_rq *rq, unsigned int cq_index,
 	unsigned int fetch_index, unsigned int posted_index,
 	unsigned int error_interrupt_enable,
 	unsigned int error_interrupt_offset)
diff --git a/drivers/net/enic/vnic_rq.h b/drivers/net/enic/vnic_rq.h
index 4b6f021..37f08de 100644
--- a/drivers/net/enic/vnic_rq.h
+++ b/drivers/net/enic/vnic_rq.h
@@ -202,10 +202,6 @@ static inline int vnic_rq_fill(struct vnic_rq *rq,
 void vnic_rq_free(struct vnic_rq *rq);
 int vnic_rq_alloc(struct vnic_dev *vdev, struct vnic_rq *rq, unsigned int index,
 	unsigned int desc_count, unsigned int desc_size);
-void vnic_rq_init_start(struct vnic_rq *rq, unsigned int cq_index,
-	unsigned int fetch_index, unsigned int posted_index,
-	unsigned int error_interrupt_enable,
-	unsigned int error_interrupt_offset);
 void vnic_rq_init(struct vnic_rq *rq, unsigned int cq_index,
 	unsigned int error_interrupt_enable,
 	unsigned int error_interrupt_offset);
diff --git a/drivers/net/enic/vnic_wq.c b/drivers/net/enic/vnic_wq.c
index 4b2a6c6..df61bd9 100644
--- a/drivers/net/enic/vnic_wq.c
+++ b/drivers/net/enic/vnic_wq.c
@@ -115,7 +115,7 @@ int vnic_wq_alloc(struct vnic_dev *vdev, struct vnic_wq *wq, unsigned int index,
 	return 0;
 }
 
-void vnic_wq_init_start(struct vnic_wq *wq, unsigned int cq_index,
+static void vnic_wq_init_start(struct vnic_wq *wq, unsigned int cq_index,
 	unsigned int fetch_index, unsigned int posted_index,
 	unsigned int error_interrupt_enable,
 	unsigned int error_interrupt_offset)
diff --git a/drivers/net/enic/vnic_wq.h b/drivers/net/enic/vnic_wq.h
index 94ac462..7dd937a 100644
--- a/drivers/net/enic/vnic_wq.h
+++ b/drivers/net/enic/vnic_wq.h
@@ -153,10 +153,6 @@ static inline void vnic_wq_service(struct vnic_wq *wq,
 void vnic_wq_free(struct vnic_wq *wq);
 int vnic_wq_alloc(struct vnic_dev *vdev, struct vnic_wq *wq, unsigned int index,
 	unsigned int desc_count, unsigned int desc_size);
-void vnic_wq_init_start(struct vnic_wq *wq, unsigned int cq_index,
-	unsigned int fetch_index, unsigned int posted_index,
-	unsigned int error_interrupt_enable,
-	unsigned int error_interrupt_offset);
 void vnic_wq_init(struct vnic_wq *wq, unsigned int cq_index,
 	unsigned int error_interrupt_enable,
 	unsigned int error_interrupt_offset);


^ permalink raw reply related

* [PATCH 8/9] isdn/gigaset: fix bas_gigaset interrupt read error handling
From: Tilman Schmidt @ 2010-09-30 23:35 UTC (permalink / raw)
  To: Karsten Keil, David Miller
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, netdev, linux-kernel
In-Reply-To: <20100930-patch-gigaset-00.tilman@imap.cc>

Rework the handling of USB errors in interrupt input reads
to clear halts correctly, delay URB resubmission after errors,
limit retries, and improve error recovery.

Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
 drivers/isdn/gigaset/bas-gigaset.c |  106 +++++++++++++++++++++++++++++++-----
 1 files changed, 93 insertions(+), 13 deletions(-)

diff --git a/drivers/isdn/gigaset/bas-gigaset.c b/drivers/isdn/gigaset/bas-gigaset.c
index 540f6d0..71e3fde 100644
--- a/drivers/isdn/gigaset/bas-gigaset.c
+++ b/drivers/isdn/gigaset/bas-gigaset.c
@@ -109,6 +109,9 @@ struct bas_cardstate {
 
 	struct urb		*urb_int_in;	/* URB for interrupt pipe */
 	unsigned char		*int_in_buf;
+	struct work_struct	int_in_wq;	/* for usb_clear_halt() */
+	struct timer_list	timer_int_in;	/* int read retry delay */
+	int			retry_int_in;
 
 	spinlock_t		lock;		/* locks all following */
 	int			basstate;	/* bitmap (BS_*) */
@@ -595,6 +598,62 @@ static int atread_submit(struct cardstate *cs, int timeout)
 	return 0;
 }
 
+/* int_in_work
+ * workqueue routine to clear halt on interrupt in endpoint
+ */
+
+static void int_in_work(struct work_struct *work)
+{
+	struct bas_cardstate *ucs =
+		container_of(work, struct bas_cardstate, int_in_wq);
+	struct urb *urb = ucs->urb_int_in;
+	struct cardstate *cs = urb->context;
+	int rc;
+
+	/* clear halt condition */
+	rc = usb_clear_halt(ucs->udev, urb->pipe);
+	gig_dbg(DEBUG_USBREQ, "clear_halt: %s", get_usb_rcmsg(rc));
+	if (rc == 0)
+		/* success, resubmit interrupt read URB */
+		rc = usb_submit_urb(urb, GFP_ATOMIC);
+	if (rc != 0 && rc != -ENODEV) {
+		dev_err(cs->dev, "clear halt failed: %s\n", get_usb_rcmsg(rc));
+		rc = usb_lock_device_for_reset(ucs->udev, ucs->interface);
+		if (rc == 0) {
+			rc = usb_reset_device(ucs->udev);
+			usb_unlock_device(ucs->udev);
+		}
+	}
+	ucs->retry_int_in = 0;
+}
+
+/* int_in_resubmit
+ * timer routine for interrupt read delayed resubmit
+ * argument:
+ *	controller state structure
+ */
+static void int_in_resubmit(unsigned long data)
+{
+	struct cardstate *cs = (struct cardstate *) data;
+	struct bas_cardstate *ucs = cs->hw.bas;
+	int rc;
+
+	if (ucs->retry_int_in++ >= BAS_RETRY) {
+		dev_err(cs->dev, "interrupt read: giving up after %d tries\n",
+			ucs->retry_int_in);
+		usb_queue_reset_device(ucs->interface);
+		return;
+	}
+
+	gig_dbg(DEBUG_USBREQ, "%s: retry %d", __func__, ucs->retry_int_in);
+	rc = usb_submit_urb(ucs->urb_int_in, GFP_ATOMIC);
+	if (rc != 0 && rc != -ENODEV) {
+		dev_err(cs->dev, "could not resubmit interrupt URB: %s\n",
+			get_usb_rcmsg(rc));
+		usb_queue_reset_device(ucs->interface);
+	}
+}
+
 /* read_int_callback
  * USB completion handler for interrupt pipe input
  * called by the USB subsystem in interrupt context
@@ -615,19 +674,29 @@ static void read_int_callback(struct urb *urb)
 
 	switch (status) {
 	case 0:			/* success */
+		ucs->retry_int_in = 0;
 		break;
+	case -EPIPE:			/* endpoint stalled */
+		schedule_work(&ucs->int_in_wq);
+		/* fall through */
 	case -ENOENT:			/* cancelled */
 	case -ECONNRESET:		/* cancelled (async) */
 	case -EINPROGRESS:		/* pending */
-		/* ignore silently */
+	case -ENODEV:			/* device removed */
+	case -ESHUTDOWN:		/* device shut down */
+		/* no further action necessary */
 		gig_dbg(DEBUG_USBREQ, "%s: %s",
 			__func__, get_usb_statmsg(status));
 		return;
-	case -ENODEV:			/* device removed */
-	case -ESHUTDOWN:		/* device shut down */
-		gig_dbg(DEBUG_USBREQ, "%s: device disconnected", __func__);
+	case -EPROTO:			/* protocol error or unplug */
+	case -EILSEQ:
+	case -ETIME:
+		/* resubmit after delay */
+		gig_dbg(DEBUG_USBREQ, "%s: %s",
+			__func__, get_usb_statmsg(status));
+		mod_timer(&ucs->timer_int_in, jiffies + HZ / 10);
 		return;
-	default:		/* severe trouble */
+	default:		/* other errors: just resubmit */
 		dev_warn(cs->dev, "interrupt read: %s\n",
 			 get_usb_statmsg(status));
 		goto resubmit;
@@ -705,6 +774,13 @@ static void read_int_callback(struct urb *urb)
 			break;
 		}
 		spin_lock_irqsave(&cs->lock, flags);
+		if (ucs->basstate & BS_ATRDPEND) {
+			spin_unlock_irqrestore(&cs->lock, flags);
+			dev_warn(cs->dev,
+	"HD_RECEIVEATDATA_ACK(%d) during HD_READ_ATMESSAGE(%d) ignored\n",
+				 l, ucs->rcvbuf_size);
+			break;
+		}
 		if (ucs->rcvbuf_size) {
 			/* throw away previous buffer - we have no queue */
 			dev_err(cs->dev,
@@ -717,7 +793,6 @@ static void read_int_callback(struct urb *urb)
 		if (ucs->rcvbuf == NULL) {
 			spin_unlock_irqrestore(&cs->lock, flags);
 			dev_err(cs->dev, "out of memory receiving AT data\n");
-			error_reset(cs);
 			break;
 		}
 		ucs->rcvbuf_size = l;
@@ -727,13 +802,10 @@ static void read_int_callback(struct urb *urb)
 			kfree(ucs->rcvbuf);
 			ucs->rcvbuf = NULL;
 			ucs->rcvbuf_size = 0;
-			if (rc != -ENODEV) {
-				spin_unlock_irqrestore(&cs->lock, flags);
-				error_reset(cs);
-				break;
-			}
 		}
 		spin_unlock_irqrestore(&cs->lock, flags);
+		if (rc < 0 && rc != -ENODEV)
+			error_reset(cs);
 		break;
 
 	case HD_RESET_INTERRUPT_PIPE_ACK:
@@ -2138,7 +2210,9 @@ static int gigaset_initcshw(struct cardstate *cs)
 	setup_timer(&ucs->timer_ctrl, req_timeout, (unsigned long) cs);
 	setup_timer(&ucs->timer_atrdy, atrdy_timeout, (unsigned long) cs);
 	setup_timer(&ucs->timer_cmd_in, cmd_in_timeout, (unsigned long) cs);
+	setup_timer(&ucs->timer_int_in, int_in_resubmit, (unsigned long) cs);
 	init_waitqueue_head(&ucs->waitqueue);
+	INIT_WORK(&ucs->int_in_wq, int_in_work);
 
 	return 1;
 }
@@ -2286,6 +2360,7 @@ static int gigaset_probe(struct usb_interface *interface,
 			get_usb_rcmsg(rc));
 		goto error;
 	}
+	ucs->retry_int_in = 0;
 
 	/* tell the device that the driver is ready */
 	rc = req_submit(cs->bcs, HD_DEVICE_INIT_ACK, 0, 0);
@@ -2338,10 +2413,12 @@ static void gigaset_disconnect(struct usb_interface *interface)
 	/* stop driver (common part) */
 	gigaset_stop(cs);
 
-	/* stop timers and URBs, free ressources */
+	/* stop delayed work and URBs, free ressources */
 	del_timer_sync(&ucs->timer_ctrl);
 	del_timer_sync(&ucs->timer_atrdy);
 	del_timer_sync(&ucs->timer_cmd_in);
+	del_timer_sync(&ucs->timer_int_in);
+	cancel_work_sync(&ucs->int_in_wq);
 	freeurbs(cs);
 	usb_set_intfdata(interface, NULL);
 	kfree(ucs->rcvbuf);
@@ -2404,12 +2481,14 @@ static int gigaset_suspend(struct usb_interface *intf, pm_message_t message)
 		/* in case of timeout, proceed anyway */
 	}
 
-	/* kill all URBs and timers that might still be pending */
+	/* kill all URBs and delayed work that might still be pending */
 	usb_kill_urb(ucs->urb_ctrl);
 	usb_kill_urb(ucs->urb_int_in);
 	del_timer_sync(&ucs->timer_ctrl);
 	del_timer_sync(&ucs->timer_atrdy);
 	del_timer_sync(&ucs->timer_cmd_in);
+	del_timer_sync(&ucs->timer_int_in);
+	cancel_work_sync(&ucs->int_in_wq);
 
 	gig_dbg(DEBUG_SUSPEND, "suspend complete");
 	return 0;
@@ -2431,6 +2510,7 @@ static int gigaset_resume(struct usb_interface *intf)
 			get_usb_rcmsg(rc));
 		return rc;
 	}
+	ucs->retry_int_in = 0;
 
 	/* clear suspend flag to reallow activity */
 	update_basstate(ucs, 0, BS_SUSPEND);
-- 
1.7.3.15.g442cb

^ permalink raw reply related

* [net-next-2.6 PATCH 1/3] enic: Remove dead code
From: Vasanthy Kolluri @ 2010-09-30 23:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, shemminger, roprabhu, dwang2
In-Reply-To: <20100930233412.16528.52523.stgit@savbu-pc100.cisco.com>

From: Vasanthy Kolluri <vkolluri@cisco.com>

Removed code that is unused

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: Vasanthy Kolluri <vkolluri@cisco.com>
Signed-off-by: Roopa Prabhu <roprabhu@cisco.com>
Signed-off-by: David Wang <dwang2@cisco.com>
---
 drivers/net/enic/enic.h      |    3 -
 drivers/net/enic/enic_res.c  |   17 -------
 drivers/net/enic/enic_res.h  |    2 -
 drivers/net/enic/vnic_dev.c  |  104 ------------------------------------------
 drivers/net/enic/vnic_dev.h  |   12 -----
 drivers/net/enic/vnic_intr.c |    5 --
 drivers/net/enic/vnic_rss.h  |   45 ------------------
 7 files changed, 1 insertions(+), 187 deletions(-)
 delete mode 100644 drivers/net/enic/vnic_rss.h


diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index 75869ed..ae62320 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -28,11 +28,10 @@
 #include "vnic_intr.h"
 #include "vnic_stats.h"
 #include "vnic_nic.h"
-#include "vnic_rss.h"
 
 #define DRV_NAME		"enic"
 #define DRV_DESCRIPTION		"Cisco VIC Ethernet NIC Driver"
-#define DRV_VERSION		"1.4.1.2"
+#define DRV_VERSION		"1.4.1.2a"
 #define DRV_COPYRIGHT		"Copyright 2008-2010 Cisco Systems, Inc"
 
 #define ENIC_BARS_MAX		6
diff --git a/drivers/net/enic/enic_res.c b/drivers/net/enic/enic_res.c
index 29ede8a..19a276c 100644
--- a/drivers/net/enic/enic_res.c
+++ b/drivers/net/enic/enic_res.c
@@ -35,7 +35,6 @@
 #include "vnic_intr.h"
 #include "vnic_stats.h"
 #include "vnic_nic.h"
-#include "vnic_rss.h"
 #include "enic_res.h"
 #include "enic.h"
 
@@ -149,22 +148,6 @@ int enic_set_nic_cfg(struct enic *enic, u8 rss_default_cpu, u8 rss_hash_type,
 	return vnic_dev_cmd(enic->vdev, CMD_NIC_CFG, &a0, &a1, wait);
 }
 
-int enic_set_rss_key(struct enic *enic, dma_addr_t key_pa, u64 len)
-{
-	u64 a0 = (u64)key_pa, a1 = len;
-	int wait = 1000;
-
-	return vnic_dev_cmd(enic->vdev, CMD_RSS_KEY, &a0, &a1, wait);
-}
-
-int enic_set_rss_cpu(struct enic *enic, dma_addr_t cpu_pa, u64 len)
-{
-	u64 a0 = (u64)cpu_pa, a1 = len;
-	int wait = 1000;
-
-	return vnic_dev_cmd(enic->vdev, CMD_RSS_CPU, &a0, &a1, wait);
-}
-
 void enic_free_vnic_resources(struct enic *enic)
 {
 	unsigned int i;
diff --git a/drivers/net/enic/enic_res.h b/drivers/net/enic/enic_res.h
index 83bd172..3c59f54 100644
--- a/drivers/net/enic/enic_res.h
+++ b/drivers/net/enic/enic_res.h
@@ -137,8 +137,6 @@ int enic_del_vlan(struct enic *enic, u16 vlanid);
 int enic_set_nic_cfg(struct enic *enic, u8 rss_default_cpu, u8 rss_hash_type,
 	u8 rss_hash_bits, u8 rss_base_cpu, u8 rss_enable, u8 tso_ipid_split_en,
 	u8 ig_vlan_strip_en);
-int enic_set_rss_key(struct enic *enic, dma_addr_t key_pa, u64 len);
-int enic_set_rss_cpu(struct enic *enic, dma_addr_t cpu_pa, u64 len);
 void enic_get_res_counts(struct enic *enic);
 void enic_init_vnic_resources(struct enic *enic);
 int enic_alloc_vnic_resources(struct enic *);
diff --git a/drivers/net/enic/vnic_dev.c b/drivers/net/enic/vnic_dev.c
index 08d5d42..f99ddee 100644
--- a/drivers/net/enic/vnic_dev.c
+++ b/drivers/net/enic/vnic_dev.c
@@ -186,21 +186,6 @@ void __iomem *vnic_dev_get_res(struct vnic_dev *vdev, enum vnic_res_type type,
 	}
 }
 
-dma_addr_t vnic_dev_get_res_bus_addr(struct vnic_dev *vdev,
-	enum vnic_res_type type, unsigned int index)
-{
-	switch (type) {
-	case RES_TYPE_WQ:
-	case RES_TYPE_RQ:
-	case RES_TYPE_CQ:
-	case RES_TYPE_INTR_CTRL:
-		return vdev->res[type].bus_addr +
-			index * VNIC_RES_STRIDE;
-	default:
-		return vdev->res[type].bus_addr;
-	}
-}
-
 unsigned int vnic_dev_desc_ring_size(struct vnic_dev_ring *ring,
 	unsigned int desc_count, unsigned int desc_size)
 {
@@ -384,18 +369,6 @@ static int vnic_dev_cmd_no_proxy(struct vnic_dev *vdev,
 	return err;
 }
 
-void vnic_dev_cmd_proxy_by_bdf_start(struct vnic_dev *vdev, u16 bdf)
-{
-	vdev->proxy = PROXY_BY_BDF;
-	vdev->proxy_index = bdf;
-}
-
-void vnic_dev_cmd_proxy_end(struct vnic_dev *vdev)
-{
-	vdev->proxy = PROXY_NONE;
-	vdev->proxy_index = 0;
-}
-
 int vnic_dev_cmd(struct vnic_dev *vdev, enum vnic_devcmd_cmd cmd,
 	u64 *a0, u64 *a1, int wait)
 {
@@ -488,13 +461,6 @@ int vnic_dev_spec(struct vnic_dev *vdev, unsigned int offset, unsigned int size,
 	return err;
 }
 
-int vnic_dev_stats_clear(struct vnic_dev *vdev)
-{
-	u64 a0 = 0, a1 = 0;
-	int wait = 1000;
-	return vnic_dev_cmd(vdev, CMD_STATS_CLEAR, &a0, &a1, wait);
-}
-
 int vnic_dev_stats_dump(struct vnic_dev *vdev, struct vnic_stats **stats)
 {
 	u64 a0, a1;
@@ -528,19 +494,6 @@ int vnic_dev_enable(struct vnic_dev *vdev)
 	return vnic_dev_cmd(vdev, CMD_ENABLE, &a0, &a1, wait);
 }
 
-int vnic_dev_enable_wait(struct vnic_dev *vdev)
-{
-	u64 a0 = 0, a1 = 0;
-	int wait = 1000;
-	int err;
-
-	err = vnic_dev_cmd(vdev, CMD_ENABLE_WAIT, &a0, &a1, wait);
-	if (err == ERR_ECMDUNKNOWN)
-		return vnic_dev_cmd(vdev, CMD_ENABLE, &a0, &a1, wait);
-
-	return err;
-}
-
 int vnic_dev_disable(struct vnic_dev *vdev)
 {
 	u64 a0 = 0, a1 = 0;
@@ -680,26 +633,6 @@ int vnic_dev_packet_filter(struct vnic_dev *vdev, int directed, int multicast,
 	return err;
 }
 
-int vnic_dev_packet_filter_all(struct vnic_dev *vdev, int directed,
-	int multicast, int broadcast, int promisc, int allmulti)
-{
-	u64 a0, a1 = 0;
-	int wait = 1000;
-	int err;
-
-	a0 = (directed ? CMD_PFILTER_DIRECTED : 0) |
-	     (multicast ? CMD_PFILTER_MULTICAST : 0) |
-	     (broadcast ? CMD_PFILTER_BROADCAST : 0) |
-	     (promisc ? CMD_PFILTER_PROMISCUOUS : 0) |
-	     (allmulti ? CMD_PFILTER_ALL_MULTICAST : 0);
-
-	err = vnic_dev_cmd(vdev, CMD_PACKET_FILTER_ALL, &a0, &a1, wait);
-	if (err)
-		pr_err("Can't set packet filter\n");
-
-	return err;
-}
-
 int vnic_dev_add_addr(struct vnic_dev *vdev, u8 *addr)
 {
 	u64 a0 = 0, a1 = 0;
@@ -748,19 +681,6 @@ int vnic_dev_set_ig_vlan_rewrite_mode(struct vnic_dev *vdev,
 	return err;
 }
 
-int vnic_dev_raise_intr(struct vnic_dev *vdev, u16 intr)
-{
-	u64 a0 = intr, a1 = 0;
-	int wait = 1000;
-	int err;
-
-	err = vnic_dev_cmd(vdev, CMD_IAR, &a0, &a1, wait);
-	if (err)
-		pr_err("Failed to raise INTR[%d], err %d\n", intr, err);
-
-	return err;
-}
-
 int vnic_dev_notify_setcmd(struct vnic_dev *vdev,
 	void *notify_addr, dma_addr_t notify_pa, u16 intr)
 {
@@ -954,30 +874,6 @@ u32 vnic_dev_mtu(struct vnic_dev *vdev)
 	return vdev->notify_copy.mtu;
 }
 
-u32 vnic_dev_link_down_cnt(struct vnic_dev *vdev)
-{
-	if (!vnic_dev_notify_ready(vdev))
-		return 0;
-
-	return vdev->notify_copy.link_down_cnt;
-}
-
-u32 vnic_dev_notify_status(struct vnic_dev *vdev)
-{
-	if (!vnic_dev_notify_ready(vdev))
-		return 0;
-
-	return vdev->notify_copy.status;
-}
-
-u32 vnic_dev_uif(struct vnic_dev *vdev)
-{
-	if (!vnic_dev_notify_ready(vdev))
-		return 0;
-
-	return vdev->notify_copy.uif;
-}
-
 void vnic_dev_set_intr_mode(struct vnic_dev *vdev,
 	enum vnic_dev_intr_mode intr_mode)
 {
diff --git a/drivers/net/enic/vnic_dev.h b/drivers/net/enic/vnic_dev.h
index 3a61873..008304b 100644
--- a/drivers/net/enic/vnic_dev.h
+++ b/drivers/net/enic/vnic_dev.h
@@ -84,8 +84,6 @@ unsigned int vnic_dev_get_res_count(struct vnic_dev *vdev,
 	enum vnic_res_type type);
 void __iomem *vnic_dev_get_res(struct vnic_dev *vdev, enum vnic_res_type type,
 	unsigned int index);
-dma_addr_t vnic_dev_get_res_bus_addr(struct vnic_dev *vdev,
-	enum vnic_res_type type, unsigned int index);
 unsigned int vnic_dev_desc_ring_size(struct vnic_dev_ring *ring,
 	unsigned int desc_count, unsigned int desc_size);
 void vnic_dev_clear_desc_ring(struct vnic_dev_ring *ring);
@@ -95,25 +93,19 @@ void vnic_dev_free_desc_ring(struct vnic_dev *vdev,
 	struct vnic_dev_ring *ring);
 int vnic_dev_cmd(struct vnic_dev *vdev, enum vnic_devcmd_cmd cmd,
 	u64 *a0, u64 *a1, int wait);
-void vnic_dev_cmd_proxy_by_bdf_start(struct vnic_dev *vdev, u16 bdf);
-void vnic_dev_cmd_proxy_end(struct vnic_dev *vdev);
 int vnic_dev_fw_info(struct vnic_dev *vdev,
 	struct vnic_devcmd_fw_info **fw_info);
 int vnic_dev_hw_version(struct vnic_dev *vdev,
 	enum vnic_dev_hw_version *hw_ver);
 int vnic_dev_spec(struct vnic_dev *vdev, unsigned int offset, unsigned int size,
 	void *value);
-int vnic_dev_stats_clear(struct vnic_dev *vdev);
 int vnic_dev_stats_dump(struct vnic_dev *vdev, struct vnic_stats **stats);
 int vnic_dev_hang_notify(struct vnic_dev *vdev);
 int vnic_dev_packet_filter(struct vnic_dev *vdev, int directed, int multicast,
 	int broadcast, int promisc, int allmulti);
-int vnic_dev_packet_filter_all(struct vnic_dev *vdev, int directed,
-	int multicast, int broadcast, int promisc, int allmulti);
 int vnic_dev_add_addr(struct vnic_dev *vdev, u8 *addr);
 int vnic_dev_del_addr(struct vnic_dev *vdev, u8 *addr);
 int vnic_dev_mac_addr(struct vnic_dev *vdev, u8 *mac_addr);
-int vnic_dev_raise_intr(struct vnic_dev *vdev, u16 intr);
 int vnic_dev_notify_setcmd(struct vnic_dev *vdev,
 	void *notify_addr, dma_addr_t notify_pa, u16 intr);
 int vnic_dev_notify_set(struct vnic_dev *vdev, u16 intr);
@@ -123,12 +115,8 @@ int vnic_dev_link_status(struct vnic_dev *vdev);
 u32 vnic_dev_port_speed(struct vnic_dev *vdev);
 u32 vnic_dev_msg_lvl(struct vnic_dev *vdev);
 u32 vnic_dev_mtu(struct vnic_dev *vdev);
-u32 vnic_dev_link_down_cnt(struct vnic_dev *vdev);
-u32 vnic_dev_notify_status(struct vnic_dev *vdev);
-u32 vnic_dev_uif(struct vnic_dev *vdev);
 int vnic_dev_close(struct vnic_dev *vdev);
 int vnic_dev_enable(struct vnic_dev *vdev);
-int vnic_dev_enable_wait(struct vnic_dev *vdev);
 int vnic_dev_disable(struct vnic_dev *vdev);
 int vnic_dev_open(struct vnic_dev *vdev, int arg);
 int vnic_dev_open_done(struct vnic_dev *vdev, int *done);
diff --git a/drivers/net/enic/vnic_intr.c b/drivers/net/enic/vnic_intr.c
index 52ab61a..3873771 100644
--- a/drivers/net/enic/vnic_intr.c
+++ b/drivers/net/enic/vnic_intr.c
@@ -65,8 +65,3 @@ void vnic_intr_clean(struct vnic_intr *intr)
 {
 	iowrite32(0, &intr->ctrl->int_credits);
 }
-
-void vnic_intr_raise(struct vnic_intr *intr)
-{
-	vnic_dev_raise_intr(intr->vdev, (u16)intr->index);
-}
diff --git a/drivers/net/enic/vnic_rss.h b/drivers/net/enic/vnic_rss.h
deleted file mode 100644
index f62d187..0000000
--- a/drivers/net/enic/vnic_rss.h
+++ /dev/null
@@ -1,45 +0,0 @@
-/*
- * Copyright 2008-2010 Cisco Systems, Inc.  All rights reserved.
- * Copyright 2007 Nuova Systems, Inc.  All rights reserved.
- *
- * This program is free software; you may redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; version 2 of the License.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
- * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
- * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
- * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
- * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
- * SOFTWARE.
- */
-
-#ifndef _VNIC_RSS_H_
-#define _VNIC_RSS_H_
-
-/* RSS key array */
-union vnic_rss_key {
-	struct {
-		u8 b[10];
-		u8 b_pad[6];
-	} key[4];
-	u64 raw[8];
-};
-
-/* RSS cpu array */
-union vnic_rss_cpu {
-	struct {
-		u8 b[4] ;
-		u8 b_pad[4];
-	} cpu[32];
-	u64 raw[32];
-};
-
-void vnic_set_rss_key(union vnic_rss_key *rss_key, u8 *key);
-void vnic_set_rss_cpu(union vnic_rss_cpu *rss_cpu, u8 *cpu);
-void vnic_get_rss_key(union vnic_rss_key *rss_key, u8 *key);
-void vnic_get_rss_cpu(union vnic_rss_cpu *rss_cpu, u8 *cpu);
-
-#endif /* _VNIC_RSS_H_ */


^ permalink raw reply related

* [PATCH 6/9] isdn/gigaset: try USB reset for bas_gigaset error recovery
From: Tilman Schmidt @ 2010-09-30 23:35 UTC (permalink / raw)
  To: Karsten Keil, David Miller
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, netdev, linux-kernel
In-Reply-To: <20100930-patch-gigaset-00.tilman@imap.cc>

In error_reset(), if sending HD_RESET_INTERRUPT_PIPE to the device
fails, try performing an USB reset.
Also correct an error in the leading comment.

Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
 drivers/isdn/gigaset/bas-gigaset.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/isdn/gigaset/bas-gigaset.c b/drivers/isdn/gigaset/bas-gigaset.c
index e865c5d..7520bc6 100644
--- a/drivers/isdn/gigaset/bas-gigaset.c
+++ b/drivers/isdn/gigaset/bas-gigaset.c
@@ -350,7 +350,7 @@ static inline void error_hangup(struct bc_state *bcs)
  * reset Gigaset device because of an unrecoverable error
  * This function may be called from any context, and takes care of
  * scheduling the necessary actions for execution outside of interrupt context.
- * cs->lock must not be held.
+ * cs->hw.bas->lock must not be held.
  * argument:
  *	controller state structure
  */
@@ -358,7 +358,9 @@ static inline void error_reset(struct cardstate *cs)
 {
 	/* reset interrupt pipe to recover (ignore errors) */
 	update_basstate(cs->hw.bas, BS_RESETTING, 0);
-	req_submit(cs->bcs, HD_RESET_INTERRUPT_PIPE, 0, BAS_TIMEOUT);
+	if (req_submit(cs->bcs, HD_RESET_INTERRUPT_PIPE, 0, BAS_TIMEOUT))
+		/* submission failed, escalate to USB port reset */
+		usb_queue_reset_device(cs->hw.bas->interface);
 }
 
 /* check_pending
-- 
1.7.3.15.g442cb

^ permalink raw reply related

* [PATCH 5/9] isdn/gigaset: bas_gigaset timer cleanup
From: Tilman Schmidt @ 2010-09-30 23:35 UTC (permalink / raw)
  To: Karsten Keil, David Miller
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, netdev, linux-kernel
In-Reply-To: <20100930-patch-gigaset-00.tilman@imap.cc>

Use setup_timer() and mod_timer() instead of direct assignment to
timer structure members, simplify the argument of one timer routine,
and make extra sure all timers are stopped during suspend.

Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
 drivers/isdn/gigaset/bas-gigaset.c |   65 +++++++++++++++++------------------
 1 files changed, 32 insertions(+), 33 deletions(-)

diff --git a/drivers/isdn/gigaset/bas-gigaset.c b/drivers/isdn/gigaset/bas-gigaset.c
index 131976d..e865c5d 100644
--- a/drivers/isdn/gigaset/bas-gigaset.c
+++ b/drivers/isdn/gigaset/bas-gigaset.c
@@ -588,10 +588,7 @@ static int atread_submit(struct cardstate *cs, int timeout)
 
 	if (timeout > 0) {
 		gig_dbg(DEBUG_USBREQ, "setting timeout of %d/10 secs", timeout);
-		ucs->timer_cmd_in.expires = jiffies + timeout * HZ / 10;
-		ucs->timer_cmd_in.data = (unsigned long) cs;
-		ucs->timer_cmd_in.function = cmd_in_timeout;
-		add_timer(&ucs->timer_cmd_in);
+		mod_timer(&ucs->timer_cmd_in, jiffies + timeout * HZ / 10);
 	}
 	return 0;
 }
@@ -1356,12 +1353,12 @@ error:
 /* req_timeout
  * timeout routine for control output request
  * argument:
- *	B channel control structure
+ *	controller state structure
  */
 static void req_timeout(unsigned long data)
 {
-	struct bc_state *bcs = (struct bc_state *) data;
-	struct bas_cardstate *ucs = bcs->cs->hw.bas;
+	struct cardstate *cs = (struct cardstate *) data;
+	struct bas_cardstate *ucs = cs->hw.bas;
 	int pending;
 	unsigned long flags;
 
@@ -1378,38 +1375,44 @@ static void req_timeout(unsigned long data)
 		break;
 
 	case HD_OPEN_ATCHANNEL:
-		dev_err(bcs->cs->dev, "timeout opening AT channel\n");
-		error_reset(bcs->cs);
+		dev_err(cs->dev, "timeout opening AT channel\n");
+		error_reset(cs);
 		break;
 
-	case HD_OPEN_B2CHANNEL:
 	case HD_OPEN_B1CHANNEL:
-		dev_err(bcs->cs->dev, "timeout opening channel %d\n",
-			bcs->channel + 1);
-		error_hangup(bcs);
+		dev_err(cs->dev, "timeout opening channel 1\n");
+		error_hangup(&cs->bcs[0]);
+		break;
+
+	case HD_OPEN_B2CHANNEL:
+		dev_err(cs->dev, "timeout opening channel 2\n");
+		error_hangup(&cs->bcs[1]);
 		break;
 
 	case HD_CLOSE_ATCHANNEL:
-		dev_err(bcs->cs->dev, "timeout closing AT channel\n");
-		error_reset(bcs->cs);
+		dev_err(cs->dev, "timeout closing AT channel\n");
+		error_reset(cs);
 		break;
 
-	case HD_CLOSE_B2CHANNEL:
 	case HD_CLOSE_B1CHANNEL:
-		dev_err(bcs->cs->dev, "timeout closing channel %d\n",
-			bcs->channel + 1);
-		error_reset(bcs->cs);
+		dev_err(cs->dev, "timeout closing channel 1\n");
+		error_reset(cs);
+		break;
+
+	case HD_CLOSE_B2CHANNEL:
+		dev_err(cs->dev, "timeout closing channel 2\n");
+		error_reset(cs);
 		break;
 
 	case HD_RESET_INTERRUPT_PIPE:
 		/* error recovery escalation */
-		dev_err(bcs->cs->dev,
+		dev_err(cs->dev,
 			"reset interrupt pipe timeout, attempting USB reset\n");
-		usb_queue_reset_device(bcs->cs->hw.bas->interface);
+		usb_queue_reset_device(ucs->interface);
 		break;
 
 	default:
-		dev_warn(bcs->cs->dev, "request 0x%02x timed out, clearing\n",
+		dev_warn(cs->dev, "request 0x%02x timed out, clearing\n",
 			 pending);
 	}
 
@@ -1540,10 +1543,7 @@ static int req_submit(struct bc_state *bcs, int req, int val, int timeout)
 
 	if (timeout > 0) {
 		gig_dbg(DEBUG_USBREQ, "setting timeout of %d/10 secs", timeout);
-		ucs->timer_ctrl.expires = jiffies + timeout * HZ / 10;
-		ucs->timer_ctrl.data = (unsigned long) bcs;
-		ucs->timer_ctrl.function = req_timeout;
-		add_timer(&ucs->timer_ctrl);
+		mod_timer(&ucs->timer_ctrl, jiffies + timeout * HZ / 10);
 	}
 
 	spin_unlock_irqrestore(&ucs->lock, flags);
@@ -1809,10 +1809,7 @@ static int atwrite_submit(struct cardstate *cs, unsigned char *buf, int len)
 	if (!(update_basstate(ucs, BS_ATTIMER, BS_ATREADY) & BS_ATTIMER)) {
 		gig_dbg(DEBUG_OUTPUT, "setting ATREADY timeout of %d/10 secs",
 			ATRDY_TIMEOUT);
-		ucs->timer_atrdy.expires = jiffies + ATRDY_TIMEOUT * HZ / 10;
-		ucs->timer_atrdy.data = (unsigned long) cs;
-		ucs->timer_atrdy.function = atrdy_timeout;
-		add_timer(&ucs->timer_atrdy);
+		mod_timer(&ucs->timer_atrdy, jiffies + ATRDY_TIMEOUT * HZ / 10);
 	}
 	return 0;
 }
@@ -2114,9 +2111,9 @@ static int gigaset_initcshw(struct cardstate *cs)
 	ucs->pending = 0;
 
 	ucs->basstate = 0;
-	init_timer(&ucs->timer_ctrl);
-	init_timer(&ucs->timer_atrdy);
-	init_timer(&ucs->timer_cmd_in);
+	setup_timer(&ucs->timer_ctrl, req_timeout, (unsigned long) cs);
+	setup_timer(&ucs->timer_atrdy, atrdy_timeout, (unsigned long) cs);
+	setup_timer(&ucs->timer_cmd_in, cmd_in_timeout, (unsigned long) cs);
 	init_waitqueue_head(&ucs->waitqueue);
 
 	return 1;
@@ -2387,6 +2384,8 @@ static int gigaset_suspend(struct usb_interface *intf, pm_message_t message)
 	usb_kill_urb(ucs->urb_ctrl);
 	usb_kill_urb(ucs->urb_int_in);
 	del_timer_sync(&ucs->timer_ctrl);
+	del_timer_sync(&ucs->timer_atrdy);
+	del_timer_sync(&ucs->timer_cmd_in);
 
 	gig_dbg(DEBUG_SUSPEND, "suspend complete");
 	return 0;
-- 
1.7.3.15.g442cb

^ permalink raw reply related

* [PATCH 4/9] isdn/gigaset: drop obsolete debug option
From: Tilman Schmidt @ 2010-09-30 23:35 UTC (permalink / raw)
  To: Karsten Keil, David Miller
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, netdev, linux-kernel
In-Reply-To: <20100930-patch-gigaset-00.tilman@imap.cc>

Remove the debug flag DEBUG_DRIVER and associated code.
It doesn't serve any useful purpose anymore.

Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
 drivers/isdn/gigaset/common.c  |   26 --------------------------
 drivers/isdn/gigaset/gigaset.h |    3 +--
 drivers/isdn/gigaset/i4l.c     |    2 --
 3 files changed, 1 insertions(+), 30 deletions(-)

diff --git a/drivers/isdn/gigaset/common.c b/drivers/isdn/gigaset/common.c
index 3ca561e..db621db 100644
--- a/drivers/isdn/gigaset/common.c
+++ b/drivers/isdn/gigaset/common.c
@@ -1026,32 +1026,6 @@ struct cardstate *gigaset_get_cs_by_id(int id)
 	return ret;
 }
 
-void gigaset_debugdrivers(void)
-{
-	unsigned long flags;
-	static struct cardstate *cs;
-	struct gigaset_driver *drv;
-	unsigned i;
-
-	spin_lock_irqsave(&driver_lock, flags);
-	list_for_each_entry(drv, &drivers, list) {
-		gig_dbg(DEBUG_DRIVER, "driver %p", drv);
-		spin_lock(&drv->lock);
-		for (i = 0; i < drv->minors; ++i) {
-			gig_dbg(DEBUG_DRIVER, "  index %u", i);
-			cs = drv->cs + i;
-			gig_dbg(DEBUG_DRIVER, "    cardstate %p", cs);
-			gig_dbg(DEBUG_DRIVER, "    flags 0x%02x", cs->flags);
-			gig_dbg(DEBUG_DRIVER, "    minor_index %u",
-				cs->minor_index);
-			gig_dbg(DEBUG_DRIVER, "    driver %p", cs->driver);
-			gig_dbg(DEBUG_DRIVER, "    i4l id %d", cs->myid);
-		}
-		spin_unlock(&drv->lock);
-	}
-	spin_unlock_irqrestore(&driver_lock, flags);
-}
-
 static struct cardstate *gigaset_get_cs_by_minor(unsigned minor)
 {
 	unsigned long flags;
diff --git a/drivers/isdn/gigaset/gigaset.h b/drivers/isdn/gigaset/gigaset.h
index a69512f..6dd3607 100644
--- a/drivers/isdn/gigaset/gigaset.h
+++ b/drivers/isdn/gigaset/gigaset.h
@@ -70,7 +70,6 @@ enum debuglevel {
 	DEBUG_STREAM_DUMP = 0x00080, /* application data stream content */
 	DEBUG_LLDATA	  = 0x00100, /* sent/received LL data */
 	DEBUG_EVENT	  = 0x00200, /* event processing */
-	DEBUG_DRIVER	  = 0x00400, /* driver structure */
 	DEBUG_HDLC	  = 0x00800, /* M10x HDLC processing */
 	DEBUG_CHANNEL	  = 0x01000, /* channel allocation/deallocation */
 	DEBUG_TRANSCMD	  = 0x02000, /* AT-COMMANDS+RESPONSES */
@@ -727,7 +726,7 @@ struct gigaset_driver *gigaset_initdriver(unsigned minor, unsigned minors,
 
 /* Deallocate driver structure. */
 void gigaset_freedriver(struct gigaset_driver *drv);
-void gigaset_debugdrivers(void);
+
 struct cardstate *gigaset_get_cs_by_tty(struct tty_struct *tty);
 struct cardstate *gigaset_get_cs_by_id(int id);
 void gigaset_blockdriver(struct gigaset_driver *drv);
diff --git a/drivers/isdn/gigaset/i4l.c b/drivers/isdn/gigaset/i4l.c
index 34bca37..9bec8b9 100644
--- a/drivers/isdn/gigaset/i4l.c
+++ b/drivers/isdn/gigaset/i4l.c
@@ -201,8 +201,6 @@ static int command_from_LL(isdn_ctrl *cntrl)
 	int i;
 	size_t l;
 
-	gigaset_debugdrivers();
-
 	gig_dbg(DEBUG_CMD, "driver: %d, command: %d, arg: 0x%lx",
 		cntrl->driver, cntrl->command, cntrl->arg);
 
-- 
1.7.3.15.g442cb

^ permalink raw reply related

* [net-next-2.6 PATCH 0/3] enic: updates to version 1.4.1.2a
From: Vasanthy Kolluri @ 2010-09-30 23:34 UTC (permalink / raw)
  To: davem; +Cc: netdev, shemminger, roprabhu, dwang2

The following patch series implements enic driver updates:

1/3 - Remove dead code
2/3 - Make local functions static
3/3 - Update MAINTAINERS

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: Vasanthy Kolluri <vkolluri@cisco.com>
Signed-off-by: Roopa Prabhu <roprabhu@cisco.com>
Signed-off-by: David Wang <dwang2@cisco.com>

^ permalink raw reply

* [PATCH 3/9] isdn/gigaset: correct bas_gigaset rx buffer handling
From: Tilman Schmidt @ 2010-09-30 23:34 UTC (permalink / raw)
  To: Karsten Keil, David Miller
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, netdev, linux-kernel
In-Reply-To: <20100930-patch-gigaset-00.tilman@imap.cc>

In transparent data reception, avoid a NULL pointer dereference
in case an skbuff cannot be allocated, remove an inappropriate
call to the HDLC flush routine, and correct the accounting of
received bytes for continued buffers.

Signed-off-by: Tilman Schmidt <tilman@imap.cc>
CC: stable <stable@kernel.org>
---
 drivers/isdn/gigaset/isocdata.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/isdn/gigaset/isocdata.c b/drivers/isdn/gigaset/isocdata.c
index 2dfd346..f39ccdf 100644
--- a/drivers/isdn/gigaset/isocdata.c
+++ b/drivers/isdn/gigaset/isocdata.c
@@ -842,13 +842,14 @@ static inline void trans_receive(unsigned char *src, unsigned count,
 
 	if (unlikely(bcs->ignore)) {
 		bcs->ignore--;
-		hdlc_flush(bcs);
 		return;
 	}
 	skb = bcs->rx_skb;
-	if (skb == NULL)
+	if (skb == NULL) {
 		skb = gigaset_new_rx_skb(bcs);
-	bcs->hw.bas->goodbytes += skb->len;
+		if (skb == NULL)
+			return;
+	}
 	dobytes = bcs->rx_bufsize - skb->len;
 	while (count > 0) {
 		dst = skb_put(skb, count < dobytes ? count : dobytes);
@@ -860,6 +861,7 @@ static inline void trans_receive(unsigned char *src, unsigned count,
 		if (dobytes == 0) {
 			dump_bytes(DEBUG_STREAM_DUMP,
 				   "rcv data", skb->data, skb->len);
+			bcs->hw.bas->goodbytes += skb->len;
 			gigaset_skb_rcvd(bcs, skb);
 			skb = gigaset_new_rx_skb(bcs);
 			if (skb == NULL)
-- 
1.7.3.15.g442cb

^ permalink raw reply related

* [PATCH 1/9] isdn/gigaset: bas_gigaset locking fix
From: Tilman Schmidt @ 2010-09-30 23:34 UTC (permalink / raw)
  To: Karsten Keil, David Miller
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, netdev, linux-kernel
In-Reply-To: <20100930-patch-gigaset-00.tilman@imap.cc>

Unlock cs->lock before calling error_hangup() which is marked
"cs->lock must not be held".

Signed-off-by: Tilman Schmidt <tilman@imap.cc>
CC: stable <stable@kernel.org>
---
 drivers/isdn/gigaset/bas-gigaset.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/isdn/gigaset/bas-gigaset.c b/drivers/isdn/gigaset/bas-gigaset.c
index 707d9c9..e143050 100644
--- a/drivers/isdn/gigaset/bas-gigaset.c
+++ b/drivers/isdn/gigaset/bas-gigaset.c
@@ -1598,13 +1598,13 @@ static int gigaset_init_bchannel(struct bc_state *bcs)
 
 	ret = starturbs(bcs);
 	if (ret < 0) {
+		spin_unlock_irqrestore(&cs->lock, flags);
 		dev_err(cs->dev,
 			"could not start isochronous I/O for channel B%d: %s\n",
 			bcs->channel + 1,
 			ret == -EFAULT ? "null URB" : get_usb_rcmsg(ret));
 		if (ret != -ENODEV)
 			error_hangup(bcs);
-		spin_unlock_irqrestore(&cs->lock, flags);
 		return ret;
 	}
 
@@ -1614,11 +1614,11 @@ static int gigaset_init_bchannel(struct bc_state *bcs)
 		dev_err(cs->dev, "could not open channel B%d\n",
 			bcs->channel + 1);
 		stopurbs(bcs->hw.bas);
-		if (ret != -ENODEV)
-			error_hangup(bcs);
 	}
 
 	spin_unlock_irqrestore(&cs->lock, flags);
+	if (ret < 0 && ret != -ENODEV)
+		error_hangup(bcs);
 	return ret;
 }
 
-- 
1.7.3.15.g442cb

^ permalink raw reply related

* [PATCH 0/9] Gigaset patches for net-next
From: Tilman Schmidt @ 2010-09-30 23:34 UTC (permalink / raw)
  To: Karsten Keil, David Miller
  Cc: Hansjoerg Lipp, Karsten Keil, i4ldeveloper, netdev, linux-kernel

Karsten, David,

here's a series of nine patches to the Gigaset driver.
Please consider for application to net-next-2.6.
The first three should also go into -stable and are
tagged "CC: stable" accordingly.

Thanks,
Tilman

Tilman Schmidt (9):
      isdn/gigaset: bas_gigaset locking fix
      isdn/gigaset: fix bas_gigaset AT read error handling
      isdn/gigaset: correct oom handling in data receive path
      isdn/gigaset: drop obsolete debug option
      isdn/gigaset: bas_gigaset timer cleanup
      isdn/gigaset: try USB reset for bas_gigaset error recovery
      isdn/gigaset: unclog bas_gigaset AT response pipe
      isdn/gigaset: fix bas_gigaset interrupt read error handling
      isdn/gigaset: improve USB error reporting

 drivers/isdn/gigaset/bas-gigaset.c |  400 ++++++++++++++++++++++--------------
 drivers/isdn/gigaset/common.c      |   26 ---
 drivers/isdn/gigaset/gigaset.h     |    3 +-
 drivers/isdn/gigaset/i4l.c         |    2 -
 drivers/isdn/gigaset/isocdata.c    |    8 +-
 5 files changed, 254 insertions(+), 185 deletions(-)

^ permalink raw reply

* Re: [RFC 1/2] genetlink: introduce pre_doit/post_doit hooks
From: Julian Calaby @ 2010-09-30 23:14 UTC (permalink / raw)
  To: Johannes Berg
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA, tgraf-G/eBtMaohhA
In-Reply-To: <1285887352.5137.34.camel-8upI4CBIZJIJvtFkdXX2HixXY32XiHfO@public.gmane.org>

On Fri, Oct 1, 2010 at 08:55, Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org> wrote:
> On Fri, 2010-10-01 at 08:51 +1000, Julian Calaby wrote:
>
>> > Come to think of it -- I could get away with a single pointer, since, if
>> > both are assigned,
>> >
>> > user_ptr[0] == wiphy_to_rdev(((netdev *)user_ptr[1])->ieee80211_ptr->wiphy)
>> >
>> > but that's a lot of pointy things, and some functions only have the rdev
>> > so it gets more complex. I think allowing two private pointers is a
>> > decent compromise.
>>
>> Come to think of it -- if someone wanted to have a massive structure
>> with 10 pointers and a set of random data structures, then they could
>> easily create their priv struct and assign it to user_ptr[0], hence
>> rendering my argument null and void.
>
> Oh, well, I thought your argument was that it was arbitrary and not
> really necessary :-)

My argument was more that someone's likely to come up with a scheme
that requires more than 2 pointers, so why not accommodate them from
start with an element for a priv struct - but that requires a new
struct, allocating and freeing it, as well as heap space for it, and a
pointer on the stack to it. (though I'm sure there'll be some sensible
way to make them persistent, but that's another issue)

> Also, this rather cheap, it just needs a bit more stack space in a place
> that isn't typically deeply nested. So if some protocol came around and
> needed three pointers, I'd probably advocate just bumping it to three.
> At some point I might draw a line (10 is probably too much).

I was considering pointing out that a compromise might need to be
made, but I figured you'd already thought of that =)

> But you're right, of course, they can just use the first one and put
> something dynamically allocated into that, if really needed.

Exactly.

Thanks,

-- 

Julian Calaby

Email: julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [net-next-2.6 PATCH 2/3] e1000e: reset PHY after errors detected on 82574 Si
From: Jeff Kirsher @ 2010-09-30 23:00 UTC (permalink / raw)
  To: davem
  Cc: netdev, gospo, bphilips, Jesse Brandeburg, Carolyn Wyborny,
	Jeff Kirsher
In-Reply-To: <20100930073909.13378.34776.stgit@localhost.localdomain>

On Thu, Sep 30, 2010 at 00:39, Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
> From: Carolyn Wyborny <carolyn.wyborny@intel.com>
>
> Some errors can be induced in the PHY via environmental testing
> (specifically extreme temperature changes and electro static
> discharge testing), and in the case of the PHY hanging due to
> this input, this detects the problem and resets to continue.
> This issue only applies to 82574 silicon.
>
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Signed-off-by: Carolyn Wyborny <carolyn.wyborny@intel.com>
> Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>
>  drivers/net/e1000e/82571.c  |   40 +++++++++++++++++++++++++++++++++++++++-
>  drivers/net/e1000e/e1000.h  |    3 +++
>  drivers/net/e1000e/netdev.c |   25 +++++++++++++++++++++++++
>  3 files changed, 67 insertions(+), 1 deletions(-)
>

Dave - please ignore this patch.  An issue was found with this patch.

-- 
Cheers,
Jeff

^ permalink raw reply

* [PATCH net-next2.6] net: dynamic ingress_queue allocation
From: Eric Dumazet @ 2010-09-30 22:58 UTC (permalink / raw)
  To: hadi; +Cc: Jarek Poplawski, David Miller, netdev
In-Reply-To: <1285757817.3561.2.camel@bigi>

Hi Jamal

Here is the dynamic allocation I promised. I lightly tested it, could
you review it please ?

Thanks !

[PATCH net-next2.6] net: dynamic ingress_queue allocation

ingress being not used very much, and net_device->ingress_queue being
quite a big object (128 or 256 bytes), use a dynamic allocation if
needed (tc qdisc add dev eth0 ingress ...)

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/linux/netdevice.h |   17 +++++++++++-
 net/core/dev.c            |   48 +++++++++++++++++++++++++++---------
 net/sched/sch_api.c       |   40 ++++++++++++++++++++----------
 net/sched/sch_generic.c   |   36 +++++++++++++++------------
 4 files changed, 98 insertions(+), 43 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index ceed347..7a21a91 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -986,8 +986,9 @@ struct net_device {
 	rx_handler_func_t	*rx_handler;
 	void			*rx_handler_data;
 
-	struct netdev_queue	ingress_queue; /* use two cache lines */
-
+#ifdef CONFIG_NET_CLS_ACT
+	struct netdev_queue	*ingress_queue;
+#endif
 /*
  * Cache lines mostly used on transmit path
  */
@@ -1115,6 +1116,18 @@ static inline void netdev_for_each_tx_queue(struct net_device *dev,
 		f(dev, &dev->_tx[i], arg);
 }
 
+
+static inline struct netdev_queue *dev_ingress_queue(struct net_device *dev)
+{
+#ifdef CONFIG_NET_CLS_ACT
+	return dev->ingress_queue;
+#else
+	return NULL;
+#endif
+}
+
+extern struct netdev_queue *dev_ingress_queue_create(struct net_device *dev);
+
 /*
  * Net namespace inlines
  */
diff --git a/net/core/dev.c b/net/core/dev.c
index a313bab..1e68259 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2702,11 +2702,10 @@ EXPORT_SYMBOL_GPL(br_fdb_test_addr_hook);
  * the ingress scheduler, you just cant add policies on ingress.
  *
  */
-static int ing_filter(struct sk_buff *skb)
+static int ing_filter(struct sk_buff *skb, struct netdev_queue *rxq)
 {
 	struct net_device *dev = skb->dev;
 	u32 ttl = G_TC_RTTL(skb->tc_verd);
-	struct netdev_queue *rxq;
 	int result = TC_ACT_OK;
 	struct Qdisc *q;
 
@@ -2720,8 +2719,6 @@ static int ing_filter(struct sk_buff *skb)
 	skb->tc_verd = SET_TC_RTTL(skb->tc_verd, ttl);
 	skb->tc_verd = SET_TC_AT(skb->tc_verd, AT_INGRESS);
 
-	rxq = &dev->ingress_queue;
-
 	q = rxq->qdisc;
 	if (q != &noop_qdisc) {
 		spin_lock(qdisc_lock(q));
@@ -2737,7 +2734,9 @@ static inline struct sk_buff *handle_ing(struct sk_buff *skb,
 					 struct packet_type **pt_prev,
 					 int *ret, struct net_device *orig_dev)
 {
-	if (skb->dev->ingress_queue.qdisc == &noop_qdisc)
+	struct netdev_queue *rxq = dev_ingress_queue(skb->dev);
+
+	if (!rxq || rxq->qdisc == &noop_qdisc)
 		goto out;
 
 	if (*pt_prev) {
@@ -2745,7 +2744,7 @@ static inline struct sk_buff *handle_ing(struct sk_buff *skb,
 		*pt_prev = NULL;
 	}
 
-	switch (ing_filter(skb)) {
+	switch (ing_filter(skb, rxq)) {
 	case TC_ACT_SHOT:
 	case TC_ACT_STOLEN:
 		kfree_skb(skb);
@@ -4932,15 +4931,17 @@ static void __netdev_init_queue_locks_one(struct net_device *dev,
 					  struct netdev_queue *dev_queue,
 					  void *_unused)
 {
-	spin_lock_init(&dev_queue->_xmit_lock);
-	netdev_set_xmit_lockdep_class(&dev_queue->_xmit_lock, dev->type);
-	dev_queue->xmit_lock_owner = -1;
+	if (dev_queue) {
+		spin_lock_init(&dev_queue->_xmit_lock);
+		netdev_set_xmit_lockdep_class(&dev_queue->_xmit_lock, dev->type);
+		dev_queue->xmit_lock_owner = -1;
+	}
 }
 
 static void netdev_init_queue_locks(struct net_device *dev)
 {
 	netdev_for_each_tx_queue(dev, __netdev_init_queue_locks_one, NULL);
-	__netdev_init_queue_locks_one(dev, &dev->ingress_queue, NULL);
+	__netdev_init_queue_locks_one(dev, dev_ingress_queue(dev), NULL);
 }
 
 unsigned long netdev_fix_features(unsigned long features, const char *name)
@@ -5447,16 +5448,37 @@ static void netdev_init_one_queue(struct net_device *dev,
 				  struct netdev_queue *queue,
 				  void *_unused)
 {
-	queue->dev = dev;
+	if (queue)
+		queue->dev = dev;
 }
 
 static void netdev_init_queues(struct net_device *dev)
 {
-	netdev_init_one_queue(dev, &dev->ingress_queue, NULL);
+	netdev_init_one_queue(dev, dev_ingress_queue(dev), NULL);
 	netdev_for_each_tx_queue(dev, netdev_init_one_queue, NULL);
 	spin_lock_init(&dev->tx_global_lock);
 }
 
+struct netdev_queue *dev_ingress_queue_create(struct net_device *dev)
+{
+	struct netdev_queue *queue = NULL;
+
+#ifdef CONFIG_NET_CLS_ACT
+	if (dev->ingress_queue)
+		return dev->ingress_queue;
+	queue = kzalloc(sizeof(*queue), GFP_KERNEL);
+	if (!queue)
+		return NULL;
+	netdev_init_one_queue(dev, queue, NULL);
+	__netdev_init_queue_locks_one(dev, queue, NULL);
+	queue->qdisc = &noop_qdisc;
+	queue->qdisc_sleeping = &noop_qdisc;
+	smp_wmb();
+	dev->ingress_queue = queue;
+#endif
+	return queue;
+}
+
 /**
  *	alloc_netdev_mq - allocate network device
  *	@sizeof_priv:	size of private data to allocate space for
@@ -5559,6 +5581,8 @@ void free_netdev(struct net_device *dev)
 
 	kfree(dev->_tx);
 
+	kfree(dev_ingress_queue(dev));
+
 	/* Flush device addresses */
 	dev_addr_flush(dev);
 
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index b802078..8635110 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -240,7 +240,10 @@ struct Qdisc *qdisc_lookup(struct net_device *dev, u32 handle)
 	if (q)
 		goto out;
 
-	q = qdisc_match_from_root(dev->ingress_queue.qdisc_sleeping, handle);
+	if (!dev_ingress_queue(dev))
+		goto out;
+	q = qdisc_match_from_root(dev_ingress_queue(dev)->qdisc_sleeping,
+				  handle);
 out:
 	return q;
 }
@@ -690,6 +693,8 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent,
 		    (new && new->flags & TCQ_F_INGRESS)) {
 			num_q = 1;
 			ingress = 1;
+			if (!dev_ingress_queue(dev))
+				return -ENOENT;
 		}
 
 		if (dev->flags & IFF_UP)
@@ -701,7 +706,7 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent,
 		}
 
 		for (i = 0; i < num_q; i++) {
-			struct netdev_queue *dev_queue = &dev->ingress_queue;
+			struct netdev_queue *dev_queue = dev_ingress_queue(dev);
 
 			if (!ingress)
 				dev_queue = netdev_get_tx_queue(dev, i);
@@ -979,7 +984,8 @@ static int tc_get_qdisc(struct sk_buff *skb, struct nlmsghdr *n, void *arg)
 					return -ENOENT;
 				q = qdisc_leaf(p, clid);
 			} else { /* ingress */
-				q = dev->ingress_queue.qdisc_sleeping;
+				if (dev_ingress_queue(dev))
+					q = dev_ingress_queue(dev)->qdisc_sleeping;
 			}
 		} else {
 			q = dev->qdisc;
@@ -1044,7 +1050,8 @@ replay:
 					return -ENOENT;
 				q = qdisc_leaf(p, clid);
 			} else { /*ingress */
-				q = dev->ingress_queue.qdisc_sleeping;
+				if (dev_ingress_queue_create(dev))
+					q = dev_ingress_queue(dev)->qdisc_sleeping;
 			}
 		} else {
 			q = dev->qdisc;
@@ -1123,11 +1130,14 @@ replay:
 create_n_graft:
 	if (!(n->nlmsg_flags&NLM_F_CREATE))
 		return -ENOENT;
-	if (clid == TC_H_INGRESS)
-		q = qdisc_create(dev, &dev->ingress_queue, p,
-				 tcm->tcm_parent, tcm->tcm_parent,
-				 tca, &err);
-	else {
+	if (clid == TC_H_INGRESS) {
+		if (dev_ingress_queue(dev))
+			q = qdisc_create(dev, dev_ingress_queue(dev), p,
+					 tcm->tcm_parent, tcm->tcm_parent,
+					 tca, &err);
+		else
+			err = -ENOENT;
+	} else {
 		struct netdev_queue *dev_queue;
 
 		if (p && p->ops->cl_ops && p->ops->cl_ops->select_queue)
@@ -1304,8 +1314,10 @@ static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
 		if (tc_dump_qdisc_root(dev->qdisc, skb, cb, &q_idx, s_q_idx) < 0)
 			goto done;
 
-		dev_queue = &dev->ingress_queue;
-		if (tc_dump_qdisc_root(dev_queue->qdisc_sleeping, skb, cb, &q_idx, s_q_idx) < 0)
+		dev_queue = dev_ingress_queue(dev);
+		if (dev_queue &&
+		    tc_dump_qdisc_root(dev_queue->qdisc_sleeping, skb, cb,
+				       &q_idx, s_q_idx) < 0)
 			goto done;
 
 cont:
@@ -1595,8 +1607,10 @@ static int tc_dump_tclass(struct sk_buff *skb, struct netlink_callback *cb)
 	if (tc_dump_tclass_root(dev->qdisc, skb, tcm, cb, &t, s_t) < 0)
 		goto done;
 
-	dev_queue = &dev->ingress_queue;
-	if (tc_dump_tclass_root(dev_queue->qdisc_sleeping, skb, tcm, cb, &t, s_t) < 0)
+	dev_queue = dev_ingress_queue(dev);
+	if (dev_queue &&
+	    tc_dump_tclass_root(dev_queue->qdisc_sleeping, skb, tcm, cb,
+				&t, s_t) < 0)
 		goto done;
 
 done:
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 545278a..c42dec5 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -721,16 +721,18 @@ static void transition_one_qdisc(struct net_device *dev,
 				 struct netdev_queue *dev_queue,
 				 void *_need_watchdog)
 {
-	struct Qdisc *new_qdisc = dev_queue->qdisc_sleeping;
-	int *need_watchdog_p = _need_watchdog;
+	if (dev_queue) {
+		struct Qdisc *new_qdisc = dev_queue->qdisc_sleeping;
+		int *need_watchdog_p = _need_watchdog;
 
-	if (!(new_qdisc->flags & TCQ_F_BUILTIN))
-		clear_bit(__QDISC_STATE_DEACTIVATED, &new_qdisc->state);
+		if (!(new_qdisc->flags & TCQ_F_BUILTIN))
+			clear_bit(__QDISC_STATE_DEACTIVATED, &new_qdisc->state);
 
-	rcu_assign_pointer(dev_queue->qdisc, new_qdisc);
-	if (need_watchdog_p && new_qdisc != &noqueue_qdisc) {
-		dev_queue->trans_start = 0;
-		*need_watchdog_p = 1;
+		rcu_assign_pointer(dev_queue->qdisc, new_qdisc);
+		if (need_watchdog_p && new_qdisc != &noqueue_qdisc) {
+			dev_queue->trans_start = 0;
+			*need_watchdog_p = 1;
+		}
 	}
 }
 
@@ -753,7 +755,7 @@ void dev_activate(struct net_device *dev)
 
 	need_watchdog = 0;
 	netdev_for_each_tx_queue(dev, transition_one_qdisc, &need_watchdog);
-	transition_one_qdisc(dev, &dev->ingress_queue, NULL);
+	transition_one_qdisc(dev, dev_ingress_queue(dev), NULL);
 
 	if (need_watchdog) {
 		dev->trans_start = jiffies;
@@ -768,7 +770,7 @@ static void dev_deactivate_queue(struct net_device *dev,
 	struct Qdisc *qdisc_default = _qdisc_default;
 	struct Qdisc *qdisc;
 
-	qdisc = dev_queue->qdisc;
+	qdisc = dev_queue ? dev_queue->qdisc : NULL;
 	if (qdisc) {
 		spin_lock_bh(qdisc_lock(qdisc));
 
@@ -812,7 +814,7 @@ static bool some_qdisc_is_busy(struct net_device *dev)
 void dev_deactivate(struct net_device *dev)
 {
 	netdev_for_each_tx_queue(dev, dev_deactivate_queue, &noop_qdisc);
-	dev_deactivate_queue(dev, &dev->ingress_queue, &noop_qdisc);
+	dev_deactivate_queue(dev, dev_ingress_queue(dev), &noop_qdisc);
 
 	dev_watchdog_down(dev);
 
@@ -830,15 +832,17 @@ static void dev_init_scheduler_queue(struct net_device *dev,
 {
 	struct Qdisc *qdisc = _qdisc;
 
-	dev_queue->qdisc = qdisc;
-	dev_queue->qdisc_sleeping = qdisc;
+	if (dev_queue) {
+		dev_queue->qdisc = qdisc;
+		dev_queue->qdisc_sleeping = qdisc;
+	}
 }
 
 void dev_init_scheduler(struct net_device *dev)
 {
 	dev->qdisc = &noop_qdisc;
 	netdev_for_each_tx_queue(dev, dev_init_scheduler_queue, &noop_qdisc);
-	dev_init_scheduler_queue(dev, &dev->ingress_queue, &noop_qdisc);
+	dev_init_scheduler_queue(dev, dev_ingress_queue(dev), &noop_qdisc);
 
 	setup_timer(&dev->watchdog_timer, dev_watchdog, (unsigned long)dev);
 }
@@ -847,7 +851,7 @@ static void shutdown_scheduler_queue(struct net_device *dev,
 				     struct netdev_queue *dev_queue,
 				     void *_qdisc_default)
 {
-	struct Qdisc *qdisc = dev_queue->qdisc_sleeping;
+	struct Qdisc *qdisc = dev_queue ? dev_queue->qdisc_sleeping : NULL;
 	struct Qdisc *qdisc_default = _qdisc_default;
 
 	if (qdisc) {
@@ -861,7 +865,7 @@ static void shutdown_scheduler_queue(struct net_device *dev,
 void dev_shutdown(struct net_device *dev)
 {
 	netdev_for_each_tx_queue(dev, shutdown_scheduler_queue, &noop_qdisc);
-	shutdown_scheduler_queue(dev, &dev->ingress_queue, &noop_qdisc);
+	shutdown_scheduler_queue(dev, dev_ingress_queue(dev), &noop_qdisc);
 	qdisc_destroy(dev->qdisc);
 	dev->qdisc = &noop_qdisc;
 



^ permalink raw reply related

* Re: [RFC 1/2] genetlink: introduce pre_doit/post_doit hooks
From: Johannes Berg @ 2010-09-30 22:55 UTC (permalink / raw)
  To: Julian Calaby
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA, tgraf-G/eBtMaohhA
In-Reply-To: <AANLkTimVMGHNkK=o650qzZfvUp3mWRAG_=60etVbEBxh-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, 2010-10-01 at 08:51 +1000, Julian Calaby wrote:

> > Come to think of it -- I could get away with a single pointer, since, if
> > both are assigned,
> >
> > user_ptr[0] == wiphy_to_rdev(((netdev *)user_ptr[1])->ieee80211_ptr->wiphy)
> >
> > but that's a lot of pointy things, and some functions only have the rdev
> > so it gets more complex. I think allowing two private pointers is a
> > decent compromise.
> 
> Come to think of it -- if someone wanted to have a massive structure
> with 10 pointers and a set of random data structures, then they could
> easily create their priv struct and assign it to user_ptr[0], hence
> rendering my argument null and void.

Oh, well, I thought your argument was that it was arbitrary and not
really necessary :-)

Also, this rather cheap, it just needs a bit more stack space in a place
that isn't typically deeply nested. So if some protocol came around and
needed three pointers, I'd probably advocate just bumping it to three.
At some point I might draw a line (10 is probably too much).

But you're right, of course, they can just use the first one and put
something dynamically allocated into that, if really needed.

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC 1/2] genetlink: introduce pre_doit/post_doit hooks
From: Julian Calaby @ 2010-09-30 22:51 UTC (permalink / raw)
  To: Johannes Berg; +Cc: netdev, linux-wireless, tgraf
In-Reply-To: <1285886955.5137.31.camel@jlt3.sipsolutions.net>

On Fri, Oct 1, 2010 at 08:49, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Fri, 2010-10-01 at 00:47 +0200, Johannes Berg wrote:
>> On Fri, 2010-10-01 at 00:44 +0200, Johannes Berg wrote:
>> > On Fri, 2010-10-01 at 08:41 +1000, Julian Calaby wrote:
>> >
>> > > >  d) include two private pointers in each info
>> > > >    struct passed between all these operations
>> > > >    including doit(). (It's two because I'll
>> > > >    need two in nl80211 -- can be extended.)
>> > >
>> > > Stupid question:
>> > >
>> > > Why not have a priv struct rather than an arbitrary array of two pointers?
>> >
>> > It'd have to be dynamically allocated, and the "arbitrary" array of two
>> > pointers can be on the stack.
>>
>> Maybe I should elaborate -- a priv struct basically means just a single
>> pointer, and then I'd have to allocate something to hold two pointers in
>> nl80211 and assign it to that single pointer.
>
> Come to think of it -- I could get away with a single pointer, since, if
> both are assigned,
>
> user_ptr[0] == wiphy_to_rdev(((netdev *)user_ptr[1])->ieee80211_ptr->wiphy)
>
> but that's a lot of pointy things, and some functions only have the rdev
> so it gets more complex. I think allowing two private pointers is a
> decent compromise.

Come to think of it -- if someone wanted to have a massive structure
with 10 pointers and a set of random data structures, then they could
easily create their priv struct and assign it to user_ptr[0], hence
rendering my argument null and void.

Thanks,

-- 

Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/

^ permalink raw reply

* Re: [RFC 1/2] genetlink: introduce pre_doit/post_doit hooks
From: Johannes Berg @ 2010-09-30 22:49 UTC (permalink / raw)
  To: Julian Calaby; +Cc: netdev, linux-wireless, tgraf
In-Reply-To: <1285886821.5137.29.camel@jlt3.sipsolutions.net>

On Fri, 2010-10-01 at 00:47 +0200, Johannes Berg wrote:
> On Fri, 2010-10-01 at 00:44 +0200, Johannes Berg wrote:
> > On Fri, 2010-10-01 at 08:41 +1000, Julian Calaby wrote:
> > 
> > > >  d) include two private pointers in each info
> > > >    struct passed between all these operations
> > > >    including doit(). (It's two because I'll
> > > >    need two in nl80211 -- can be extended.)
> > > 
> > > Stupid question:
> > > 
> > > Why not have a priv struct rather than an arbitrary array of two pointers?
> > 
> > It'd have to be dynamically allocated, and the "arbitrary" array of two
> > pointers can be on the stack.
> 
> Maybe I should elaborate -- a priv struct basically means just a single
> pointer, and then I'd have to allocate something to hold two pointers in
> nl80211 and assign it to that single pointer.

Come to think of it -- I could get away with a single pointer, since, if
both are assigned,

user_ptr[0] == wiphy_to_rdev(((netdev *)user_ptr[1])->ieee80211_ptr->wiphy)

but that's a lot of pointy things, and some functions only have the rdev
so it gets more complex. I think allowing two private pointers is a
decent compromise.

johannes


^ permalink raw reply

* Re: [RFC 1/2] genetlink: introduce pre_doit/post_doit hooks
From: Johannes Berg @ 2010-09-30 22:47 UTC (permalink / raw)
  To: Julian Calaby; +Cc: netdev, linux-wireless, tgraf
In-Reply-To: <1285886690.5137.28.camel@jlt3.sipsolutions.net>

On Fri, 2010-10-01 at 00:44 +0200, Johannes Berg wrote:
> On Fri, 2010-10-01 at 08:41 +1000, Julian Calaby wrote:
> 
> > >  d) include two private pointers in each info
> > >    struct passed between all these operations
> > >    including doit(). (It's two because I'll
> > >    need two in nl80211 -- can be extended.)
> > 
> > Stupid question:
> > 
> > Why not have a priv struct rather than an arbitrary array of two pointers?
> 
> It'd have to be dynamically allocated, and the "arbitrary" array of two
> pointers can be on the stack.

Maybe I should elaborate -- a priv struct basically means just a single
pointer, and then I'd have to allocate something to hold two pointers in
nl80211 and assign it to that single pointer.

johannes


^ permalink raw reply

* Re: [RFC 1/2] genetlink: introduce pre_doit/post_doit hooks
From: Johannes Berg @ 2010-09-30 22:44 UTC (permalink / raw)
  To: Julian Calaby; +Cc: netdev, linux-wireless, tgraf
In-Reply-To: <AANLkTik9+zA4PPPx=q6CRM-sgOjjECJdtJTgkCG9pGAY@mail.gmail.com>

On Fri, 2010-10-01 at 08:41 +1000, Julian Calaby wrote:

> >  d) include two private pointers in each info
> >    struct passed between all these operations
> >    including doit(). (It's two because I'll
> >    need two in nl80211 -- can be extended.)
> 
> Stupid question:
> 
> Why not have a priv struct rather than an arbitrary array of two pointers?

It'd have to be dynamically allocated, and the "arbitrary" array of two
pointers can be on the stack.

johannes


^ permalink raw reply

* Re: [RFC 1/2] genetlink: introduce pre_doit/post_doit hooks
From: Julian Calaby @ 2010-09-30 22:41 UTC (permalink / raw)
  To: Johannes Berg
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA, tgraf-G/eBtMaohhA
In-Reply-To: <20100930211131.021309930-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>

On Fri, Oct 1, 2010 at 07:10, Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org> wrote:
> From: Johannes Berg <johannes.berg-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>
> Each family may have some amount of boilerplate
> locking code that applies to most, or even all,
> commands.
>
> This allows a family to handle such things in
> a more generic way, by allowing it to
>  a) include private flags in each operation
>  b) specify a pre_doit hook that is called,
>    before an operation's doit() callback and
>    may return an error directly,
>  c) specify a post_doit hook that can undo
>    locking or similar things done by pre_doit,
>    and finally
>  d) include two private pointers in each info
>    struct passed between all these operations
>    including doit(). (It's two because I'll
>    need two in nl80211 -- can be extended.)

Stupid question:

Why not have a priv struct rather than an arbitrary array of two pointers?

Thanks,

-- 

Julian Calaby

Email: julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC 0/2] generic netlink doit generalisation
From: Johannes Berg @ 2010-09-30 22:39 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA, tgraf-G/eBtMaohhA
In-Reply-To: <20100930211013.472957770-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>

On Thu, 2010-09-30 at 23:10 +0200, Johannes Berg wrote:
> nl80211 does a lot of boilerplate work
> for each generic netlink doit() operation,
> this is an attempt to reduce it.

Here are the completed versions of the nl80211 cleanups:

http://johannes.sipsolutions.net/patches/kernel/all/LATEST/NNN-nl80211-use-pre-post.patch
http://johannes.sipsolutions.net/patches/kernel/all/LATEST/NNN-nl80211-generic-running.patch

The combined diffstat of these two is

 net/wireless/nl80211.c | 1756 +++++++++++++++----------------------------------
 1 file changed, 541 insertions(+), 1215 deletions(-)

and they reduce code size by >4k on my x86_64 compile.

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: VLAN packets silently dropped in promiscuous mode
From: Eric Dumazet @ 2010-09-30 22:04 UTC (permalink / raw)
  To: Jesse Gross; +Cc: Roger Luethi, netdev, Patrick McHardy
In-Reply-To: <AANLkTikrfx1T3ay0AGWkS6Ab28J4O_er2TsbRinBSGen@mail.gmail.com>

Le jeudi 30 septembre 2010 à 14:21 -0700, Jesse Gross a écrit :
> On Thu, Sep 30, 2010 at 1:07 AM, Roger Luethi <rl@hellgate.ch> wrote:
> > On Wed, 29 Sep 2010 10:44:26 -0700, Jesse Gross wrote:
> >> On Wed, Sep 29, 2010 at 4:37 AM, Roger Luethi <rl@hellgate.ch> wrote:
> >> > I noticed packets for unknown VLANs getting silently dropped even in
> >> > promiscuous mode (this is true only for the hardware accelerated path).
> >> > netif_nit_deliver was introduced specifically to prevent that, but the
> >> > function gets called only _after_ packets from unknown VLANs have been
> >> > dropped.
> >>
> >> Some drivers are fixing this on a case by case basis by disabling
> >> hardware accelerated VLAN stripping when in promiscuous mode, i.e.:
> >> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=5f6c01819979afbfec7e0b15fe52371b8eed87e8
> >>
> >> However, at this point it is more or less random which drivers do
> >> this.  It would obviously be much better if it were consistent.
> >
> > My understanding is this. Hardware VLAN tagging and stripping can always be
> > enabled. The kernel passes 802.1Q information along with the stripped
> > header to libpcap which reassembles the original header where necessary.
> > Works for me.
> 
> Sorry, I misread your original post as saying that the VLAN header
> gets dropped, rather than the entire packet.  I agree that this is how
> it should work but not necessarily how it does work (again, depending
> on the driver).  Here's the problem that I was talking about:
> 
> Most drivers have a snippet of code that looks something like this
> (taken from ixgbe):
> 
> if (adapter->vlgrp && is_vlan && (tag & VLAN_VID_MASK))
> 	vlan_gro_receive(napi, adapter->vlgrp, tag, skb);
> else
> 	napi_gro_receive(napi, skb);
> 
> At this point the VLAN has already been stripped in hardware.  If
> there is no VLAN group configured on the device then we hit the second
> case.  The VLAN header was removed from the SKB and the tag variable
> is unused.  It is no longer possible for libpcap to reconstruct the
> header because the information was thrown away (even the fact that
> there was a VLAN tag at all).
> 
> There are a couple ways to fix this:
> 
> * Turn off VLAN stripping when in promiscuous mode (as done by the ixgbe driver)
> * Reconstruct the VLAN header when there is no VLAN group (as done by
> the tg3 driver)
> 
> A bunch of drivers do neither (bnx2x, for example) and exhibit this
> problem.  It's getting better but it seems like a common issue.

tg3 is not perfect, because it does the reconstruction of VLAN header
even if device is not in promiscuous mode.

It could drop the frame instead.

I wonder which SNMP counter is incremented in this case.

Apparently, none :(




^ permalink raw reply

* IGMP when multiple interfaces share a subnet (2.6.32-2.6.35+)
From: Mr. Berkley Shands @ 2010-09-30 21:24 UTC (permalink / raw)
  To: Net Dev, Dave Lloyd, Ed Spitznagel

When there are multiple interfaces on the same subnet
(I.E. 10.19.20.21 on eth1, 10.19.20.22 on eth2, ...)
and the subnet mask is 255.255.255.0
The Cicso Layer 3 sends out IGMP queries, both eth1, eth2 see them.
If a multicast group is subscribed on eth1, all is fine.
But if another multicast group is subscribed off eth2 at the same time,
then the IGMP reports for eth2 go back out eth1, or don't go out at all.
Worse, if you ifdown eth1, eth2 and then ifup them (service network 
restart)
then neither interface will respond to IGMP reports/requests.

We tried forcing all the IGMP traffic to V2, no luck.
the kernel's responses go out the lowest interface index, or not at all
once the second interface does a multicast join.
Hacking the kernel to continuously send IGMP reports works
until the window of the report times overlaps the routers pings.
then the router assumes a drop.

This did not happen under 2.6.22 (centos-5.2 through centos 5.5 on X86_64)
with Intel igb based nics.

We do not control the IP addresses.
We see this with VLANs as well. tcpdump sees the request come in
every 60 seconds or so, but nothing responds back.

Would using a 32-bit netmask on those ports force the kernel to
respond on the correct interface?

Why does bouncing an interface completely disable IGMP reports?
We currently have 2.5.35.2 running. Did the setsockopt() code
change from "struct ip_mreq" to "struct ip_mreqn" to throw in an 
interface index?

Any suggestions as to what to poke?

Berkley Shands

^ permalink raw reply

* Re: VLAN packets silently dropped in promiscuous mode
From: Jesse Gross @ 2010-09-30 21:21 UTC (permalink / raw)
  To: Roger Luethi; +Cc: netdev, Patrick McHardy
In-Reply-To: <20100930080703.GA10827@core.hellgate.ch>

On Thu, Sep 30, 2010 at 1:07 AM, Roger Luethi <rl@hellgate.ch> wrote:
> On Wed, 29 Sep 2010 10:44:26 -0700, Jesse Gross wrote:
>> On Wed, Sep 29, 2010 at 4:37 AM, Roger Luethi <rl@hellgate.ch> wrote:
>> > I noticed packets for unknown VLANs getting silently dropped even in
>> > promiscuous mode (this is true only for the hardware accelerated path).
>> > netif_nit_deliver was introduced specifically to prevent that, but the
>> > function gets called only _after_ packets from unknown VLANs have been
>> > dropped.
>>
>> Some drivers are fixing this on a case by case basis by disabling
>> hardware accelerated VLAN stripping when in promiscuous mode, i.e.:
>> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=5f6c01819979afbfec7e0b15fe52371b8eed87e8
>>
>> However, at this point it is more or less random which drivers do
>> this.  It would obviously be much better if it were consistent.
>
> My understanding is this. Hardware VLAN tagging and stripping can always be
> enabled. The kernel passes 802.1Q information along with the stripped
> header to libpcap which reassembles the original header where necessary.
> Works for me.

Sorry, I misread your original post as saying that the VLAN header
gets dropped, rather than the entire packet.  I agree that this is how
it should work but not necessarily how it does work (again, depending
on the driver).  Here's the problem that I was talking about:

Most drivers have a snippet of code that looks something like this
(taken from ixgbe):

if (adapter->vlgrp && is_vlan && (tag & VLAN_VID_MASK))
	vlan_gro_receive(napi, adapter->vlgrp, tag, skb);
else
	napi_gro_receive(napi, skb);

At this point the VLAN has already been stripped in hardware.  If
there is no VLAN group configured on the device then we hit the second
case.  The VLAN header was removed from the SKB and the tag variable
is unused.  It is no longer possible for libpcap to reconstruct the
header because the information was thrown away (even the fact that
there was a VLAN tag at all).

There are a couple ways to fix this:

* Turn off VLAN stripping when in promiscuous mode (as done by the ixgbe driver)
* Reconstruct the VLAN header when there is no VLAN group (as done by
the tg3 driver)

A bunch of drivers do neither (bnx2x, for example) and exhibit this
problem.  It's getting better but it seems like a common issue.

^ 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