All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/9] IrDA 2.6.28 bug fix
@ 2008-12-15  1:57 Samuel Ortiz
  2008-12-15  1:57 ` [RFC PATCH 1/9] irda: Introduce irda_alloc_skb Samuel Ortiz
                   ` (9 more replies)
  0 siblings, 10 replies; 13+ messages in thread
From: Samuel Ortiz @ 2008-12-15  1:57 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, irda-users

Hi Dave,

This is a 9 patches series for IrDA, against your net-2.6 tree.
This is an attempt to fix kernel.bugzilla.org bug #11795, where we noticed
skb->cb could be altered once submitted to dev_queue_xmit. Since IrDA is using
this callback to pass per-skb information, we are now stuffing it in front of
skb->data, after allocating the right headroom.

Another solution would be to play with the IrDA physical header and skb_pull
our skbs whenever we are physically transmitting the data.

Cheers,
Samuel.

^ permalink raw reply	[flat|nested] 13+ messages in thread
* [RFC PATCH 1/9] irda: Introduce irda_alloc_skb
  2008-12-15  1:57 [RFC PATCH 0/9] IrDA 2.6.28 bug fix Samuel Ortiz
@ 2008-12-15  1:57 ` Samuel Ortiz
  2008-12-15  1:57 ` [RFC PATCH 2/9] irda: stack should call irda_get_skb_cb() Samuel Ortiz
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Samuel Ortiz @ 2008-12-15  1:57 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, irda-users

[-- Attachment #1: 0001-irda-Introduce-irda_alloc_skb.patch --]
[-- Type: text/plain, Size: 18716 bytes --]

We want to have extra space for the irda_skb_cb structure in front of
the actual IrDA data.
It's currently sitting in skb->cb, but that pointer can be altered after
hitting dev_queue_xmit. So, an additional skb header is a safer place.

Signed-off-by: Samuel Ortiz <samuel@sortiz.org>
---
 include/net/irda/irda.h         |    1 +
 include/net/irda/irda_device.h  |    6 +++
 net/irda/af_irda.c              |    4 +-
 net/irda/ircomm/ircomm_lmp.c    |    4 +-
 net/irda/ircomm/ircomm_param.c  |    2 +-
 net/irda/ircomm/ircomm_tty.c    |    6 ++--
 net/irda/irda_device.c          |   12 ++++++
 net/irda/iriap.c                |   10 +++---
 net/irda/iriap_event.c          |    2 +-
 net/irda/irlan/irlan_common.c   |   77 +++++++++++++++++++++------------------
 net/irda/irlan/irlan_provider.c |   18 ++++++----
 net/irda/irlap.c                |    2 +-
 net/irda/irlap_frame.c          |   28 ++++++++-------
 net/irda/irlmp.c                |    2 +-
 net/irda/irttp.c                |   14 ++++----
 15 files changed, 110 insertions(+), 78 deletions(-)

diff --git a/include/net/irda/irda.h b/include/net/irda/irda.h
index 7e58206..ce510aa 100644
--- a/include/net/irda/irda.h
+++ b/include/net/irda/irda.h
@@ -132,4 +132,5 @@ extern int irlap_driver_rcv(struct sk_buff *skb, struct net_device *dev,
 			    struct packet_type *ptype,
 			    struct net_device *orig_dev);
 
+extern struct sk_buff *irda_alloc_skb(unsigned int size, gfp_t priority);
 #endif /* NET_IRDA_H */
diff --git a/include/net/irda/irda_device.h b/include/net/irda/irda_device.h
index 3025ae1..305eb05 100644
--- a/include/net/irda/irda_device.h
+++ b/include/net/irda/irda_device.h
@@ -230,6 +230,12 @@ struct net_device *alloc_irdadev(int sizeof_priv);
 
 void irda_setup_dma(int channel, dma_addr_t buffer, int count, int mode);
 
+
+static inline struct irda_skb_cb *irda_get_skb_cb(struct sk_buff *skb)
+{
+	return (struct irda_skb_cb *)(skb->data - sizeof(struct irda_skb_cb));
+}
+
 /*
  * Function irda_get_mtt (skb)
  *
diff --git a/net/irda/af_irda.c b/net/irda/af_irda.c
index 3eb5bcc..61def98 100644
--- a/net/irda/af_irda.c
+++ b/net/irda/af_irda.c
@@ -304,8 +304,8 @@ static void irda_connect_response(struct irda_sock *self)
 
 	IRDA_DEBUG(2, "%s()\n", __func__);
 
-	skb = alloc_skb(TTP_MAX_HEADER + TTP_SAR_HEADER,
-			GFP_ATOMIC);
+	skb = irda_alloc_skb(TTP_MAX_HEADER + TTP_SAR_HEADER,
+			     GFP_ATOMIC);
 	if (skb == NULL) {
 		IRDA_DEBUG(0, "%s() Unable to allocate sk_buff!\n",
 			   __func__);
diff --git a/net/irda/ircomm/ircomm_lmp.c b/net/irda/ircomm/ircomm_lmp.c
index 67c99d2..407bad3 100644
--- a/net/irda/ircomm/ircomm_lmp.c
+++ b/net/irda/ircomm/ircomm_lmp.c
@@ -80,7 +80,7 @@ static int ircomm_lmp_connect_response(struct ircomm_cb *self,
 
 	/* Any userdata supplied? */
 	if (userdata == NULL) {
-		tx_skb = alloc_skb(LMP_MAX_HEADER, GFP_ATOMIC);
+		tx_skb = irda_alloc_skb(LMP_MAX_HEADER, GFP_ATOMIC);
 		if (!tx_skb)
 			return -ENOMEM;
 
@@ -114,7 +114,7 @@ static int ircomm_lmp_disconnect_request(struct ircomm_cb *self,
 	IRDA_DEBUG(0, "%s()\n", __func__ );
 
 	if (!userdata) {
-		tx_skb = alloc_skb(LMP_MAX_HEADER, GFP_ATOMIC);
+		tx_skb = irda_alloc_skb(LMP_MAX_HEADER, GFP_ATOMIC);
 		if (!tx_skb)
 			return -ENOMEM;
 
diff --git a/net/irda/ircomm/ircomm_param.c b/net/irda/ircomm/ircomm_param.c
index d57aefd..e62979b 100644
--- a/net/irda/ircomm/ircomm_param.c
+++ b/net/irda/ircomm/ircomm_param.c
@@ -120,7 +120,7 @@ int ircomm_param_request(struct ircomm_tty_cb *self, __u8 pi, int flush)
 
 	skb = self->ctrl_skb;
 	if (!skb) {
-		skb = alloc_skb(256, GFP_ATOMIC);
+		skb = irda_alloc_skb(256, GFP_ATOMIC);
 		if (!skb) {
 			spin_unlock_irqrestore(&self->spinlock, flags);
 			return -ENOMEM;
diff --git a/net/irda/ircomm/ircomm_tty.c b/net/irda/ircomm/ircomm_tty.c
index e4e2cae..fa62490 100644
--- a/net/irda/ircomm/ircomm_tty.c
+++ b/net/irda/ircomm/ircomm_tty.c
@@ -752,9 +752,9 @@ static int ircomm_tty_write(struct tty_struct *tty,
 			}
 		} else {
 			/* Prepare a full sized frame */
-			skb = alloc_skb(self->max_data_size+
-					self->max_header_size,
-					GFP_ATOMIC);
+			skb = irda_alloc_skb(self->max_data_size+
+					     self->max_header_size,
+					     GFP_ATOMIC);
 			if (!skb) {
 				spin_unlock_irqrestore(&self->spinlock, flags);
 				return -ENOBUFS;
diff --git a/net/irda/irda_device.c b/net/irda/irda_device.c
index ea319e3..5271015 100644
--- a/net/irda/irda_device.c
+++ b/net/irda/irda_device.c
@@ -296,6 +296,18 @@ struct net_device *alloc_irdadev(int sizeof_priv)
 }
 EXPORT_SYMBOL(alloc_irdadev);
 
+struct sk_buff *irda_alloc_skb(unsigned int size, gfp_t priority)
+{
+	struct sk_buff *skb;
+
+	skb = alloc_skb(size + sizeof(struct irda_skb_cb), priority);
+	if (likely(skb))
+		skb_reserve(skb, sizeof(struct irda_skb_cb));
+
+	return skb;
+}
+EXPORT_SYMBOL(irda_alloc_skb);
+
 #ifdef CONFIG_ISA_DMA_API
 /*
  * Function setup_dma (idev, buffer, count, mode)
diff --git a/net/irda/iriap.c b/net/irda/iriap.c
index 4a105dc..dbd91bf 100644
--- a/net/irda/iriap.c
+++ b/net/irda/iriap.c
@@ -345,7 +345,7 @@ static void iriap_disconnect_request(struct iriap_cb *self)
 	IRDA_ASSERT(self != NULL, return;);
 	IRDA_ASSERT(self->magic == IAS_MAGIC, return;);
 
-	tx_skb = alloc_skb(LMP_MAX_HEADER, GFP_ATOMIC);
+	tx_skb = irda_alloc_skb(LMP_MAX_HEADER, GFP_ATOMIC);
 	if (tx_skb == NULL) {
 		IRDA_DEBUG(0,
 			   "%s(), Could not allocate an sk_buff of length %d\n",
@@ -397,7 +397,7 @@ int iriap_getvaluebyclass_request(struct iriap_cb *self,
 	attr_len = strlen(attr);	/* Up to IAS_MAX_ATTRIBNAME = 60 */
 
 	skb_len = self->max_header_size+2+name_len+1+attr_len+4;
-	tx_skb = alloc_skb(skb_len, GFP_ATOMIC);
+	tx_skb = irda_alloc_skb(skb_len, GFP_ATOMIC);
 	if (!tx_skb)
 		return -ENOMEM;
 
@@ -565,8 +565,8 @@ static void iriap_getvaluebyclass_response(struct iriap_cb *self,
 	 *  value. We add 32 bytes because of the 6 bytes for the frame and
 	 *  max 5 bytes for the value coding.
 	 */
-	tx_skb = alloc_skb(value->len + self->max_header_size + 32,
-			   GFP_ATOMIC);
+	tx_skb = irda_alloc_skb(value->len + self->max_header_size + 32,
+				GFP_ATOMIC);
 	if (!tx_skb)
 		return;
 
@@ -704,7 +704,7 @@ void iriap_send_ack(struct iriap_cb *self)
 	IRDA_ASSERT(self != NULL, return;);
 	IRDA_ASSERT(self->magic == IAS_MAGIC, return;);
 
-	tx_skb = alloc_skb(LMP_MAX_HEADER + 1, GFP_ATOMIC);
+	tx_skb = irda_alloc_skb(LMP_MAX_HEADER + 1, GFP_ATOMIC);
 	if (!tx_skb)
 		return;
 
diff --git a/net/irda/iriap_event.c b/net/irda/iriap_event.c
index a301cbd..87cd337 100644
--- a/net/irda/iriap_event.c
+++ b/net/irda/iriap_event.c
@@ -365,7 +365,7 @@ static void state_r_disconnect(struct iriap_cb *self, IRIAP_EVENT event,
 
 	switch (event) {
 	case IAP_LM_CONNECT_INDICATION:
-		tx_skb = alloc_skb(LMP_MAX_HEADER, GFP_ATOMIC);
+		tx_skb = irda_alloc_skb(LMP_MAX_HEADER, GFP_ATOMIC);
 		if (tx_skb == NULL) {
 			IRDA_WARNING("%s: unable to malloc!\n", __func__);
 			return;
diff --git a/net/irda/irlan/irlan_common.c b/net/irda/irlan/irlan_common.c
index 9a1cd87..bcafd88 100644
--- a/net/irda/irlan/irlan_common.c
+++ b/net/irda/irlan/irlan_common.c
@@ -641,8 +641,8 @@ void irlan_get_provider_info(struct irlan_cb *self)
 	IRDA_ASSERT(self != NULL, return;);
 	IRDA_ASSERT(self->magic == IRLAN_MAGIC, return;);
 
-	skb = alloc_skb(IRLAN_MAX_HEADER + IRLAN_CMD_HEADER,
-			GFP_ATOMIC);
+	skb = irda_alloc_skb(IRLAN_MAX_HEADER + IRLAN_CMD_HEADER,
+			     GFP_ATOMIC);
 	if (!skb)
 		return;
 
@@ -674,10 +674,11 @@ void irlan_open_data_channel(struct irlan_cb *self)
 	IRDA_ASSERT(self != NULL, return;);
 	IRDA_ASSERT(self->magic == IRLAN_MAGIC, return;);
 
-	skb = alloc_skb(IRLAN_MAX_HEADER + IRLAN_CMD_HEADER +
-			IRLAN_STRING_PARAMETER_LEN("MEDIA", "802.3") +
-			IRLAN_STRING_PARAMETER_LEN("ACCESS_TYPE", "DIRECT"),
-			GFP_ATOMIC);
+	skb = irda_alloc_skb(IRLAN_MAX_HEADER + IRLAN_CMD_HEADER +
+			     IRLAN_STRING_PARAMETER_LEN("MEDIA", "802.3") +
+			     IRLAN_STRING_PARAMETER_LEN("ACCESS_TYPE",
+							"DIRECT"),
+			     GFP_ATOMIC);
 	if (!skb)
 		return;
 
@@ -713,9 +714,9 @@ void irlan_close_data_channel(struct irlan_cb *self)
 	if (self->client.tsap_ctrl == NULL)
 		return;
 
-	skb = alloc_skb(IRLAN_MAX_HEADER + IRLAN_CMD_HEADER +
-			IRLAN_BYTE_PARAMETER_LEN("DATA_CHAN"),
-			GFP_ATOMIC);
+	skb = irda_alloc_skb(IRLAN_MAX_HEADER + IRLAN_CMD_HEADER +
+			     IRLAN_BYTE_PARAMETER_LEN("DATA_CHAN"),
+			     GFP_ATOMIC);
 	if (!skb)
 		return;
 
@@ -750,11 +751,13 @@ static void irlan_open_unicast_addr(struct irlan_cb *self)
 	IRDA_ASSERT(self != NULL, return;);
 	IRDA_ASSERT(self->magic == IRLAN_MAGIC, return;);
 
-	skb = alloc_skb(IRLAN_MAX_HEADER + IRLAN_CMD_HEADER +
-			IRLAN_BYTE_PARAMETER_LEN("DATA_CHAN") +
-			IRLAN_STRING_PARAMETER_LEN("FILTER_TYPE", "DIRECTED") +
-			IRLAN_STRING_PARAMETER_LEN("FILTER_MODE", "FILTER"),
-			GFP_ATOMIC);
+	skb = irda_alloc_skb(IRLAN_MAX_HEADER + IRLAN_CMD_HEADER +
+			     IRLAN_BYTE_PARAMETER_LEN("DATA_CHAN") +
+			     IRLAN_STRING_PARAMETER_LEN("FILTER_TYPE",
+							"DIRECTED") +
+			     IRLAN_STRING_PARAMETER_LEN("FILTER_MODE",
+							"FILTER"),
+			     GFP_ATOMIC);
 	if (!skb)
 		return;
 
@@ -792,12 +795,14 @@ void irlan_set_broadcast_filter(struct irlan_cb *self, int status)
 	IRDA_ASSERT(self != NULL, return;);
 	IRDA_ASSERT(self->magic == IRLAN_MAGIC, return;);
 
-	skb = alloc_skb(IRLAN_MAX_HEADER + IRLAN_CMD_HEADER +
-			IRLAN_BYTE_PARAMETER_LEN("DATA_CHAN") +
-			IRLAN_STRING_PARAMETER_LEN("FILTER_TYPE", "BROADCAST") +
-			/* We may waste one byte here...*/
-			IRLAN_STRING_PARAMETER_LEN("FILTER_MODE", "FILTER"),
-			GFP_ATOMIC);
+	skb = irda_alloc_skb(IRLAN_MAX_HEADER + IRLAN_CMD_HEADER +
+			     IRLAN_BYTE_PARAMETER_LEN("DATA_CHAN") +
+			     IRLAN_STRING_PARAMETER_LEN("FILTER_TYPE",
+							"BROADCAST") +
+			     /* We may waste one byte here...*/
+			     IRLAN_STRING_PARAMETER_LEN("FILTER_MODE",
+							"FILTER"),
+			     GFP_ATOMIC);
 	if (!skb)
 		return;
 
@@ -836,12 +841,13 @@ void irlan_set_multicast_filter(struct irlan_cb *self, int status)
 	IRDA_ASSERT(self != NULL, return;);
 	IRDA_ASSERT(self->magic == IRLAN_MAGIC, return;);
 
-	skb = alloc_skb(IRLAN_MAX_HEADER + IRLAN_CMD_HEADER +
-			IRLAN_BYTE_PARAMETER_LEN("DATA_CHAN") +
-			IRLAN_STRING_PARAMETER_LEN("FILTER_TYPE", "MULTICAST") +
-			/* We may waste one byte here...*/
-			IRLAN_STRING_PARAMETER_LEN("FILTER_MODE", "NONE"),
-			GFP_ATOMIC);
+	skb = irda_alloc_skb(IRLAN_MAX_HEADER + IRLAN_CMD_HEADER +
+			     IRLAN_BYTE_PARAMETER_LEN("DATA_CHAN") +
+			     IRLAN_STRING_PARAMETER_LEN("FILTER_TYPE",
+							"MULTICAST") +
+			     /* We may waste one byte here...*/
+			     IRLAN_STRING_PARAMETER_LEN("FILTER_MODE", "NONE"),
+			     GFP_ATOMIC);
 	if (!skb)
 		return;
 
@@ -881,12 +887,13 @@ static void irlan_get_unicast_addr(struct irlan_cb *self)
 	IRDA_ASSERT(self != NULL, return;);
 	IRDA_ASSERT(self->magic == IRLAN_MAGIC, return;);
 
-	skb = alloc_skb(IRLAN_MAX_HEADER + IRLAN_CMD_HEADER +
-			IRLAN_BYTE_PARAMETER_LEN("DATA_CHAN") +
-			IRLAN_STRING_PARAMETER_LEN("FILTER_TYPE", "DIRECTED") +
-			IRLAN_STRING_PARAMETER_LEN("FILTER_OPERATION",
-						   "DYNAMIC"),
-			GFP_ATOMIC);
+	skb = irda_alloc_skb(IRLAN_MAX_HEADER + IRLAN_CMD_HEADER +
+			     IRLAN_BYTE_PARAMETER_LEN("DATA_CHAN") +
+			     IRLAN_STRING_PARAMETER_LEN("FILTER_TYPE",
+							"DIRECTED") +
+			     IRLAN_STRING_PARAMETER_LEN("FILTER_OPERATION",
+							"DYNAMIC"),
+			     GFP_ATOMIC);
 	if (!skb)
 		return;
 
@@ -921,9 +928,9 @@ void irlan_get_media_char(struct irlan_cb *self)
 	IRDA_ASSERT(self != NULL, return;);
 	IRDA_ASSERT(self->magic == IRLAN_MAGIC, return;);
 
-	skb = alloc_skb(IRLAN_MAX_HEADER + IRLAN_CMD_HEADER +
-			IRLAN_STRING_PARAMETER_LEN("MEDIA", "802.3"),
-			GFP_ATOMIC);
+	skb = irda_alloc_skb(IRLAN_MAX_HEADER + IRLAN_CMD_HEADER +
+			     IRLAN_STRING_PARAMETER_LEN("MEDIA", "802.3"),
+			     GFP_ATOMIC);
 
 	if (!skb)
 		return;
diff --git a/net/irda/irlan/irlan_provider.c b/net/irda/irlan/irlan_provider.c
index 3f81f81..4d8ce7f 100644
--- a/net/irda/irlan/irlan_provider.c
+++ b/net/irda/irlan/irlan_provider.c
@@ -296,13 +296,17 @@ void irlan_provider_send_reply(struct irlan_cb *self, int command,
 	IRDA_ASSERT(self != NULL, return;);
 	IRDA_ASSERT(self->magic == IRLAN_MAGIC, return;);
 
-	skb = alloc_skb(IRLAN_MAX_HEADER + IRLAN_CMD_HEADER +
-			/* Bigger param length comes from CMD_GET_MEDIA_CHAR */
-			IRLAN_STRING_PARAMETER_LEN("FILTER_TYPE", "DIRECTED") +
-			IRLAN_STRING_PARAMETER_LEN("FILTER_TYPE", "BORADCAST") +
-			IRLAN_STRING_PARAMETER_LEN("FILTER_TYPE", "MULTICAST") +
-			IRLAN_STRING_PARAMETER_LEN("ACCESS_TYPE", "HOSTED"),
-			GFP_ATOMIC);
+	skb = irda_alloc_skb(IRLAN_MAX_HEADER + IRLAN_CMD_HEADER +
+			     /* Bigger param length from CMD_GET_MEDIA_CHAR */
+			     IRLAN_STRING_PARAMETER_LEN("FILTER_TYPE",
+							"DIRECTED") +
+			     IRLAN_STRING_PARAMETER_LEN("FILTER_TYPE",
+							"BORADCAST") +
+			     IRLAN_STRING_PARAMETER_LEN("FILTER_TYPE",
+							"MULTICAST") +
+			     IRLAN_STRING_PARAMETER_LEN("ACCESS_TYPE",
+							"HOSTED"),
+			     GFP_ATOMIC);
 
 	if (!skb)
 		return;
diff --git a/net/irda/irlap.c b/net/irda/irlap.c
index e4965b7..4139e7a 100644
--- a/net/irda/irlap.c
+++ b/net/irda/irlap.c
@@ -881,7 +881,7 @@ static void irlap_change_speed(struct irlap_cb *self, __u32 speed, int now)
 	/* Change speed now, or just piggyback speed on frames */
 	if (now) {
 		/* Send down empty frame to trigger speed change */
-		skb = alloc_skb(0, GFP_ATOMIC);
+		skb = irda_alloc_skb(0, GFP_ATOMIC);
 		if (skb)
 			irlap_queue_xmit(self, skb);
 	}
diff --git a/net/irda/irlap_frame.c b/net/irda/irlap_frame.c
index f17b65a..bf41ebf 100644
--- a/net/irda/irlap_frame.c
+++ b/net/irda/irlap_frame.c
@@ -126,9 +126,9 @@ void irlap_send_snrm_frame(struct irlap_cb *self, struct qos_info *qos)
 	IRDA_ASSERT(self->magic == LAP_MAGIC, return;);
 
 	/* Allocate frame */
-	tx_skb = alloc_skb(sizeof(struct snrm_frame) +
-			   IRLAP_NEGOCIATION_PARAMS_LEN,
-			   GFP_ATOMIC);
+	tx_skb = irda_alloc_skb(sizeof(struct snrm_frame) +
+				IRLAP_NEGOCIATION_PARAMS_LEN,
+				GFP_ATOMIC);
 	if (!tx_skb)
 		return;
 
@@ -221,9 +221,9 @@ void irlap_send_ua_response_frame(struct irlap_cb *self, struct qos_info *qos)
 	IRDA_ASSERT(self->magic == LAP_MAGIC, return;);
 
 	/* Allocate frame */
-	tx_skb = alloc_skb(sizeof(struct ua_frame) +
-			   IRLAP_NEGOCIATION_PARAMS_LEN,
-			   GFP_ATOMIC);
+	tx_skb = irda_alloc_skb(sizeof(struct ua_frame) +
+				IRLAP_NEGOCIATION_PARAMS_LEN,
+				GFP_ATOMIC);
 	if (!tx_skb)
 		return;
 
@@ -263,7 +263,7 @@ void irlap_send_dm_frame( struct irlap_cb *self)
 	IRDA_ASSERT(self != NULL, return;);
 	IRDA_ASSERT(self->magic == LAP_MAGIC, return;);
 
-	tx_skb = alloc_skb(sizeof(struct dm_frame), GFP_ATOMIC);
+	tx_skb = irda_alloc_skb(sizeof(struct dm_frame), GFP_ATOMIC);
 	if (!tx_skb)
 		return;
 
@@ -295,7 +295,7 @@ void irlap_send_disc_frame(struct irlap_cb *self)
 	IRDA_ASSERT(self != NULL, return;);
 	IRDA_ASSERT(self->magic == LAP_MAGIC, return;);
 
-	tx_skb = alloc_skb(sizeof(struct disc_frame), GFP_ATOMIC);
+	tx_skb = irda_alloc_skb(sizeof(struct disc_frame), GFP_ATOMIC);
 	if (!tx_skb)
 		return;
 
@@ -328,8 +328,9 @@ void irlap_send_discovery_xid_frame(struct irlap_cb *self, int S, __u8 s,
 	IRDA_ASSERT(self->magic == LAP_MAGIC, return;);
 	IRDA_ASSERT(discovery != NULL, return;);
 
-	tx_skb = alloc_skb(sizeof(struct xid_frame) + IRLAP_DISCOVERY_INFO_LEN,
-			   GFP_ATOMIC);
+	tx_skb = irda_alloc_skb(sizeof(struct xid_frame) +
+				IRLAP_DISCOVERY_INFO_LEN,
+				GFP_ATOMIC);
 	if (!tx_skb)
 		return;
 
@@ -589,7 +590,7 @@ void irlap_send_rr_frame(struct irlap_cb *self, int command)
 	struct sk_buff *tx_skb;
 	struct rr_frame *frame;
 
-	tx_skb = alloc_skb(sizeof(struct rr_frame), GFP_ATOMIC);
+	tx_skb = irda_alloc_skb(sizeof(struct rr_frame), GFP_ATOMIC);
 	if (!tx_skb)
 		return;
 
@@ -614,7 +615,7 @@ void irlap_send_rd_frame(struct irlap_cb *self)
 	struct sk_buff *tx_skb;
 	struct rd_frame *frame;
 
-	tx_skb = alloc_skb(sizeof(struct rd_frame), GFP_ATOMIC);
+	tx_skb = irda_alloc_skb(sizeof(struct rd_frame), GFP_ATOMIC);
 	if (!tx_skb)
 		return;
 
@@ -1231,7 +1232,8 @@ void irlap_send_test_frame(struct irlap_cb *self, __u8 caddr, __u32 daddr,
 	struct test_frame *frame;
 	__u8 *info;
 
-	tx_skb = alloc_skb(cmd->len + sizeof(struct test_frame), GFP_ATOMIC);
+	tx_skb = irda_alloc_skb(cmd->len + sizeof(struct test_frame),
+				GFP_ATOMIC);
 	if (!tx_skb)
 		return;
 
diff --git a/net/irda/irlmp.c b/net/irda/irlmp.c
index 7bf5b91..18188f2 100644
--- a/net/irda/irlmp.c
+++ b/net/irda/irlmp.c
@@ -396,7 +396,7 @@ int irlmp_connect_request(struct lsap_cb *self, __u8 dlsap_sel,
 
 	/* Any userdata? */
 	if (tx_skb == NULL) {
-		tx_skb = alloc_skb(LMP_MAX_HEADER, GFP_ATOMIC);
+		tx_skb = irda_alloc_skb(LMP_MAX_HEADER, GFP_ATOMIC);
 		if (!tx_skb)
 			return -ENOMEM;
 
diff --git a/net/irda/irttp.c b/net/irda/irttp.c
index 74e439e..4b913de 100644
--- a/net/irda/irttp.c
+++ b/net/irda/irttp.c
@@ -306,8 +306,8 @@ static inline void irttp_fragment_skb(struct tsap_cb *self,
 		IRDA_DEBUG(2, "%s(), fragmenting ...\n", __func__);
 
 		/* Make new segment */
-		frag = alloc_skb(self->max_seg_size+self->max_header_size,
-				 GFP_ATOMIC);
+		frag = irda_alloc_skb(self->max_seg_size+self->max_header_size,
+				      GFP_ATOMIC);
 		if (!frag)
 			return;
 
@@ -817,7 +817,7 @@ static inline void irttp_give_credit(struct tsap_cb *self)
 		   self->send_credit, self->avail_credit, self->remote_credit);
 
 	/* Give credit to peer */
-	tx_skb = alloc_skb(TTP_MAX_HEADER, GFP_ATOMIC);
+	tx_skb = irda_alloc_skb(TTP_MAX_HEADER, GFP_ATOMIC);
 	if (!tx_skb)
 		return;
 
@@ -1106,8 +1106,8 @@ int irttp_connect_request(struct tsap_cb *self, __u8 dtsap_sel,
 
 	/* Any userdata supplied? */
 	if (userdata == NULL) {
-		tx_skb = alloc_skb(TTP_MAX_HEADER + TTP_SAR_HEADER,
-				   GFP_ATOMIC);
+		tx_skb = irda_alloc_skb(TTP_MAX_HEADER + TTP_SAR_HEADER,
+					GFP_ATOMIC);
 		if (!tx_skb)
 			return -ENOMEM;
 
@@ -1355,7 +1355,7 @@ int irttp_connect_response(struct tsap_cb *self, __u32 max_sdu_size,
 
 	/* Any userdata supplied? */
 	if (userdata == NULL) {
-		tx_skb = alloc_skb(TTP_MAX_HEADER + TTP_SAR_HEADER,
+		tx_skb = irda_alloc_skb(TTP_MAX_HEADER + TTP_SAR_HEADER,
 				   GFP_ATOMIC);
 		if (!tx_skb)
 			return -ENOMEM;
@@ -1553,7 +1553,7 @@ int irttp_disconnect_request(struct tsap_cb *self, struct sk_buff *userdata,
 
 	if (!userdata) {
 		struct sk_buff *tx_skb;
-		tx_skb = alloc_skb(LMP_MAX_HEADER, GFP_ATOMIC);
+		tx_skb = irda_alloc_skb(LMP_MAX_HEADER, GFP_ATOMIC);
 		if (!tx_skb)
 			return -ENOMEM;
 
-- 
1.6.0.4.766.g6fc4a.dirty

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [RFC PATCH 2/9] irda: stack should call irda_get_skb_cb()
  2008-12-15  1:57 [RFC PATCH 0/9] IrDA 2.6.28 bug fix Samuel Ortiz
  2008-12-15  1:57 ` [RFC PATCH 1/9] irda: Introduce irda_alloc_skb Samuel Ortiz
@ 2008-12-15  1:57 ` Samuel Ortiz
  2008-12-15  1:57 ` [RFC PATCH 3/9] irda: IrDA drivers " Samuel Ortiz
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Samuel Ortiz @ 2008-12-15  1:57 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, irda-users

[-- Attachment #1: 0002-irda-stack-should-call-irda_get_skb_cb.patch --]
[-- Type: text/plain, Size: 3889 bytes --]

Instead of accessing the irda_skb_cb directly, the stack callers should
use the irda_get_skb_cb() routine.

Signed-off-by: Samuel Ortiz <samuel@sortiz.org>
---
 include/net/irda/irda_device.h |    8 ++++----
 net/irda/ircomm/ircomm_lmp.c   |    4 ++--
 net/irda/irlap_frame.c         |    4 +++-
 net/irda/wrapper.c             |    4 +++-
 4 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/include/net/irda/irda_device.h b/include/net/irda/irda_device.h
index 305eb05..3bbf695 100644
--- a/include/net/irda/irda_device.h
+++ b/include/net/irda/irda_device.h
@@ -244,7 +244,7 @@ static inline struct irda_skb_cb *irda_get_skb_cb(struct sk_buff *skb)
  */
 static inline __u16 irda_get_mtt(const struct sk_buff *skb)
 {
-	const struct irda_skb_cb *cb = (const struct irda_skb_cb *) skb->cb;
+	const struct irda_skb_cb *cb = (const struct irda_skb_cb *) skb->head;
 	return (cb->magic == LAP_MAGIC) ? cb->mtt : 10000;
 }
 
@@ -257,7 +257,7 @@ static inline __u16 irda_get_mtt(const struct sk_buff *skb)
  */
 static inline __u32 irda_get_next_speed(const struct sk_buff *skb)
 {
-	const struct irda_skb_cb *cb = (const struct irda_skb_cb *) skb->cb;
+	const struct irda_skb_cb *cb = (const struct irda_skb_cb *) skb->head;
 	return (cb->magic == LAP_MAGIC) ? cb->next_speed : -1;
 }
 
@@ -270,7 +270,7 @@ static inline __u32 irda_get_next_speed(const struct sk_buff *skb)
  */
 static inline __u16 irda_get_xbofs(const struct sk_buff *skb)
 {
-	const struct irda_skb_cb *cb = (const struct irda_skb_cb *) skb->cb;
+	const struct irda_skb_cb *cb = (const struct irda_skb_cb *) skb->head;
 	return (cb->magic == LAP_MAGIC) ? cb->xbofs : 10;
 }
 
@@ -283,7 +283,7 @@ static inline __u16 irda_get_xbofs(const struct sk_buff *skb)
  */
 static inline __u16 irda_get_next_xbofs(const struct sk_buff *skb)
 {
-	const struct irda_skb_cb *cb = (const struct irda_skb_cb *) skb->cb;
+	const struct irda_skb_cb *cb = (const struct irda_skb_cb *) skb->head;
 	return (cb->magic == LAP_MAGIC) ? cb->next_xbofs : -1;
 }
 #endif /* IRDA_DEVICE_H */
diff --git a/net/irda/ircomm/ircomm_lmp.c b/net/irda/ircomm/ircomm_lmp.c
index 407bad3..c463bec 100644
--- a/net/irda/ircomm/ircomm_lmp.c
+++ b/net/irda/ircomm/ircomm_lmp.c
@@ -146,7 +146,7 @@ static void ircomm_lmp_flow_control(struct sk_buff *skb)
 
 	IRDA_ASSERT(skb != NULL, return;);
 
-	cb = (struct irda_skb_cb *) skb->cb;
+	cb = irda_get_skb_cb(skb);
 
 	IRDA_DEBUG(2, "%s()\n", __func__ );
 
@@ -187,7 +187,7 @@ static int ircomm_lmp_data_request(struct ircomm_cb *self,
 
 	IRDA_ASSERT(skb != NULL, return -1;);
 
-	cb = (struct irda_skb_cb *) skb->cb;
+	cb = irda_get_skb_cb(skb);
 
 	cb->line = self->line;
 
diff --git a/net/irda/irlap_frame.c b/net/irda/irlap_frame.c
index bf41ebf..8760af6 100644
--- a/net/irda/irlap_frame.c
+++ b/net/irda/irlap_frame.c
@@ -56,7 +56,9 @@ static void irlap_send_i_frame(struct irlap_cb *self, struct sk_buff *skb,
 static inline void irlap_insert_info(struct irlap_cb *self,
 				     struct sk_buff *skb)
 {
-	struct irda_skb_cb *cb = (struct irda_skb_cb *) skb->cb;
+	struct irda_skb_cb *cb;
+
+	cb = irda_get_skb_cb(skb);
 
 	/*
 	 * Insert MTT (min. turn time) and speed into skb, so that the
diff --git a/net/irda/wrapper.c b/net/irda/wrapper.c
index fd0995b..b13cb3b 100644
--- a/net/irda/wrapper.c
+++ b/net/irda/wrapper.c
@@ -82,7 +82,7 @@ static inline int stuff_byte(__u8 byte, __u8 *buf)
  */
 int async_wrap_skb(struct sk_buff *skb, __u8 *tx_buff, int buffsize)
 {
-	struct irda_skb_cb *cb = (struct irda_skb_cb *) skb->cb;
+	struct irda_skb_cb *cb;
 	int xbofs;
 	int i;
 	int n;
@@ -91,6 +91,8 @@ int async_wrap_skb(struct sk_buff *skb, __u8 *tx_buff, int buffsize)
 		__u8 bytes[2];
 	} fcs;
 
+	cb = irda_get_skb_cb(skb);
+
 	/* Initialize variables */
 	fcs.value = INIT_FCS;
 	n = 0;
-- 
1.6.0.4.766.g6fc4a.dirty

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [RFC PATCH 3/9] irda: IrDA drivers should call irda_get_skb_cb()
  2008-12-15  1:57 [RFC PATCH 0/9] IrDA 2.6.28 bug fix Samuel Ortiz
  2008-12-15  1:57 ` [RFC PATCH 1/9] irda: Introduce irda_alloc_skb Samuel Ortiz
  2008-12-15  1:57 ` [RFC PATCH 2/9] irda: stack should call irda_get_skb_cb() Samuel Ortiz
@ 2008-12-15  1:57 ` Samuel Ortiz
  2008-12-15  1:57 ` [RFC PATCH 4/9] irda: reserve irda_skb on sock_alloc_send_skb() skbs Samuel Ortiz
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Samuel Ortiz @ 2008-12-15  1:57 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, irda-users

[-- Attachment #1: 0003-irda-IrDA-drivers-should-call-irda_get_skb_cb.patch --]
[-- Type: text/plain, Size: 3556 bytes --]

Instead of accessing irda_skb_cb directly, irda drivers should call
irda_get_skb_cb().

Signed-off-by: Samuel Ortiz <samuel@sortiz.org>
---
 drivers/net/irda/donauboe.c |    9 +--------
 drivers/net/irda/irda-usb.c |   19 ++++++++++++++-----
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/net/irda/donauboe.c b/drivers/net/irda/donauboe.c
index 69d16b3..1f0f9ee 100644
--- a/drivers/net/irda/donauboe.c
+++ b/drivers/net/irda/donauboe.c
@@ -977,20 +977,13 @@ toshoboe_hard_xmit (struct sk_buff *skb, struct net_device *dev)
   __s32 speed;
   int mtt, len, ctl;
   unsigned long flags;
-  struct irda_skb_cb *cb = (struct irda_skb_cb *) skb->cb;
 
   self = (struct toshoboe_cb *) dev->priv;
 
   IRDA_ASSERT (self != NULL, return 0; );
 
   IRDA_DEBUG (1, "%s.tx:%x(%x)%x\n", __func__
-      ,skb->len,self->txpending,INB (OBOE_ENABLEH));
-  if (!cb->magic) {
-      IRDA_DEBUG (2, "%s.Not IrLAP:%x\n", __func__, cb->magic);
-#ifdef DUMP_PACKETS
-      _dumpbufs(skb->data,skb->len,'>');
-#endif
-    }
+	      , skb->len, self->txpending, INB(OBOE_ENABLEH));
 
   /* change speed pending, wait for its execution */
   if (self->new_speed)
diff --git a/drivers/net/irda/irda-usb.c b/drivers/net/irda/irda-usb.c
index b5d6b9a..f5c13af 100644
--- a/drivers/net/irda/irda-usb.c
+++ b/drivers/net/irda/irda-usb.c
@@ -385,6 +385,7 @@ static void speed_bulk_callback(struct urb *urb)
 static int irda_usb_hard_xmit(struct sk_buff *skb, struct net_device *netdev)
 {
 	struct irda_usb_cb *self = netdev->priv;
+	struct irda_skb_cb *cb;
 	struct urb *urb = self->tx_urb;
 	unsigned long flags;
 	s32 speed;
@@ -464,7 +465,8 @@ static int irda_usb_hard_xmit(struct sk_buff *skb, struct net_device *netdev)
 	}
 
 	/* FIXME: Make macro out of this one */
-	((struct irda_skb_cb *)skb->cb)->context = self;
+	cb = irda_get_skb_cb(skb);
+	cb->context = self;
 
 	usb_fill_bulk_urb(urb, self->usbdev,
 		      usb_sndbulkpipe(self->usbdev, self->bulk_out_ep),
@@ -554,10 +556,16 @@ static void write_bulk_callback(struct urb *urb)
 {
 	unsigned long flags;
 	struct sk_buff *skb = urb->context;
-	struct irda_usb_cb *self = ((struct irda_skb_cb *) skb->cb)->context;
+	struct irda_skb_cb *cb;
+	struct irda_usb_cb *self;
 	
 	IRDA_DEBUG(2, "%s()\n", __func__);
 
+	cb = irda_get_skb_cb(skb);
+	IRDA_ASSERT(cb != NULL, return;);
+
+	self = cb->context;
+
 	/* We should always have a context */
 	IRDA_ASSERT(self != NULL, return;);
 	/* We should always be called for the speed URB */
@@ -770,7 +778,7 @@ static void irda_usb_submit(struct irda_usb_cb *self, struct sk_buff *skb, struc
 	IRDA_ASSERT(urb != NULL, return;);
 
 	/* Save ourselves in the skb */
-	cb = (struct irda_skb_cb *) skb->cb;
+	cb = irda_get_skb_cb(skb);
 	cb->context = self;
 
 	/* Reinitialize URB */
@@ -810,7 +818,7 @@ static void irda_usb_receive(struct urb *urb)
 	IRDA_DEBUG(2, "%s(), len=%d\n", __func__, urb->actual_length);
 	
 	/* Find ourselves */
-	cb = (struct irda_skb_cb *) skb->cb;
+	cb = irda_get_skb_cb(skb);
 	IRDA_ASSERT(cb != NULL, return;);
 	self = (struct irda_usb_cb *) cb->context;
 	IRDA_ASSERT(self != NULL, return;);
@@ -970,7 +978,8 @@ static void irda_usb_rx_defer_expired(unsigned long data)
 	IRDA_DEBUG(2, "%s()\n", __func__);
 
 	/* Find ourselves */
-	cb = (struct irda_skb_cb *) skb->cb;
+	cb = irda_get_skb_cb(skb);
+
 	IRDA_ASSERT(cb != NULL, return;);
 	self = (struct irda_usb_cb *) cb->context;
 	IRDA_ASSERT(self != NULL, return;);
-- 
1.6.0.4.766.g6fc4a.dirty

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [RFC PATCH 4/9] irda: reserve irda_skb on sock_alloc_send_skb() skbs
  2008-12-15  1:57 [RFC PATCH 0/9] IrDA 2.6.28 bug fix Samuel Ortiz
                   ` (2 preceding siblings ...)
  2008-12-15  1:57 ` [RFC PATCH 3/9] irda: IrDA drivers " Samuel Ortiz
@ 2008-12-15  1:57 ` Samuel Ortiz
  2008-12-15  1:57 ` [RFC PATCH 5/9] irda: Introduce irda_dev_alloc_skb Samuel Ortiz
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Samuel Ortiz @ 2008-12-15  1:57 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, irda-users

[-- Attachment #1: 0004-irda-reserve-irda_skb-on-sock_alloc_send_skb-skbs.patch --]
[-- Type: text/plain, Size: 2445 bytes --]

Those skbs are on the TX path, and need to have the irda_skb_cb space
allocated as well.

Signed-off-by: Samuel Ortiz <samuel@sortiz.org>
---
 net/irda/af_irda.c |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/net/irda/af_irda.c b/net/irda/af_irda.c
index 61def98..8b0ac27 100644
--- a/net/irda/af_irda.c
+++ b/net/irda/af_irda.c
@@ -59,6 +59,7 @@
 #include <net/tcp_states.h>
 
 #include <net/irda/af_irda.h>
+#include <net/irda/irda_device.h>
 
 static int irda_create(struct net *net, struct socket *sock, int protocol);
 
@@ -1288,12 +1289,14 @@ static int irda_sendmsg(struct kiocb *iocb, struct socket *sock,
 		len = self->max_data_size;
 	}
 
-	skb = sock_alloc_send_skb(sk, len + self->max_header_size + 16,
+	skb = sock_alloc_send_skb(sk, len + self->max_header_size + 16
+				  + sizeof(struct irda_skb_cb),
 				  msg->msg_flags & MSG_DONTWAIT, &err);
 	if (!skb)
 		goto out_err;
 
-	skb_reserve(skb, self->max_header_size + 16);
+	skb_reserve(skb, self->max_header_size + 16
+		    + sizeof(struct irda_skb_cb));
 	skb_reset_transport_header(skb);
 	skb_put(skb, len);
 	err = memcpy_fromiovec(skb_transport_header(skb), msg->msg_iov, len);
@@ -1534,12 +1537,13 @@ static int irda_sendmsg_dgram(struct kiocb *iocb, struct socket *sock,
 		len = self->max_data_size;
 	}
 
-	skb = sock_alloc_send_skb(sk, len + self->max_header_size,
+	skb = sock_alloc_send_skb(sk, len + self->max_header_size
+				  + sizeof(struct irda_skb_cb),
 				  msg->msg_flags & MSG_DONTWAIT, &err);
 	if (!skb)
 		return -ENOBUFS;
 
-	skb_reserve(skb, self->max_header_size);
+	skb_reserve(skb, self->max_header_size + sizeof(struct irda_skb_cb));
 	skb_reset_transport_header(skb);
 
 	IRDA_DEBUG(4, "%s(), appending user data\n", __func__);
@@ -1629,12 +1633,13 @@ static int irda_sendmsg_ultra(struct kiocb *iocb, struct socket *sock,
 		len = self->max_data_size;
 	}
 
-	skb = sock_alloc_send_skb(sk, len + self->max_header_size,
+	skb = sock_alloc_send_skb(sk, len + self->max_header_size
+				  + sizeof(struct irda_skb_cb),
 				  msg->msg_flags & MSG_DONTWAIT, &err);
 	if (!skb)
 		return -ENOBUFS;
 
-	skb_reserve(skb, self->max_header_size);
+	skb_reserve(skb, self->max_header_size + sizeof(struct irda_skb_cb));
 	skb_reset_transport_header(skb);
 
 	IRDA_DEBUG(4, "%s(), appending user data\n", __func__);
-- 
1.6.0.4.766.g6fc4a.dirty

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [RFC PATCH 5/9] irda: Introduce irda_dev_alloc_skb
  2008-12-15  1:57 [RFC PATCH 0/9] IrDA 2.6.28 bug fix Samuel Ortiz
                   ` (3 preceding siblings ...)
  2008-12-15  1:57 ` [RFC PATCH 4/9] irda: reserve irda_skb on sock_alloc_send_skb() skbs Samuel Ortiz
@ 2008-12-15  1:57 ` Samuel Ortiz
  2008-12-15  1:57 ` [RFC PATCH 6/9] irda: Drivers should use irda_dev_alloc_skb() on the RX path Samuel Ortiz
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Samuel Ortiz @ 2008-12-15  1:57 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, irda-users

[-- Attachment #1: 0005-irda-Introduce-irda_dev_alloc_skb.patch --]
[-- Type: text/plain, Size: 1512 bytes --]

Some RX skbs are cloned and then sent back to the transmitter.
In order to avoid reallocation on that code path, we get more space from
the beginning on the RX path.

Signed-off-by: Samuel Ortiz <samuel@sortiz.org>
---
 include/net/irda/irda.h |    1 +
 net/irda/irda_device.c  |   12 ++++++++++++
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/include/net/irda/irda.h b/include/net/irda/irda.h
index ce510aa..213d4c0 100644
--- a/include/net/irda/irda.h
+++ b/include/net/irda/irda.h
@@ -133,4 +133,5 @@ extern int irlap_driver_rcv(struct sk_buff *skb, struct net_device *dev,
 			    struct net_device *orig_dev);
 
 extern struct sk_buff *irda_alloc_skb(unsigned int size, gfp_t priority);
+extern struct sk_buff *irda_dev_alloc_skb(unsigned int size);
 #endif /* NET_IRDA_H */
diff --git a/net/irda/irda_device.c b/net/irda/irda_device.c
index 5271015..62b991c 100644
--- a/net/irda/irda_device.c
+++ b/net/irda/irda_device.c
@@ -308,6 +308,18 @@ struct sk_buff *irda_alloc_skb(unsigned int size, gfp_t priority)
 }
 EXPORT_SYMBOL(irda_alloc_skb);
 
+struct sk_buff *irda_dev_alloc_skb(unsigned int size)
+{
+	struct sk_buff *skb;
+
+	skb = dev_alloc_skb(size + sizeof(struct irda_skb_cb));
+	if (likely(skb))
+		skb_reserve(skb, sizeof(struct irda_skb_cb));
+
+	return skb;
+}
+EXPORT_SYMBOL(irda_dev_alloc_skb);
+
 #ifdef CONFIG_ISA_DMA_API
 /*
  * Function setup_dma (idev, buffer, count, mode)
-- 
1.6.0.4.766.g6fc4a.dirty

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [RFC PATCH 6/9] irda: Drivers should use irda_dev_alloc_skb() on the RX path
  2008-12-15  1:57 [RFC PATCH 0/9] IrDA 2.6.28 bug fix Samuel Ortiz
                   ` (4 preceding siblings ...)
  2008-12-15  1:57 ` [RFC PATCH 5/9] irda: Introduce irda_dev_alloc_skb Samuel Ortiz
@ 2008-12-15  1:57 ` Samuel Ortiz
  2008-12-15  1:57 ` [RFC PATCH 7/9] irda: Stack RX path callers should use irda_dev_alloc_skb Samuel Ortiz
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Samuel Ortiz @ 2008-12-15  1:57 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, irda-users

[-- Attachment #1: 0006-irda-Drivers-should-use-irda_dev_alloc_skb-on-the.patch --]
[-- Type: text/plain, Size: 10639 bytes --]

Signed-off-by: Samuel Ortiz <samuel@sortiz.org>
---
 drivers/net/irda/ali-ircc.c     |    2 +-
 drivers/net/irda/donauboe.c     |    2 +-
 drivers/net/irda/irda-usb.c     |   12 ++++++------
 drivers/net/irda/kingsun-sir.c  |    2 +-
 drivers/net/irda/ks959-sir.c    |    2 +-
 drivers/net/irda/ksdazzle-sir.c |    2 +-
 drivers/net/irda/mcs7780.c      |    6 +++---
 drivers/net/irda/nsc-ircc.c     |    2 +-
 drivers/net/irda/sir_dev.c      |    4 +++-
 drivers/net/irda/smsc-ircc2.c   |    2 +-
 drivers/net/irda/stir4200.c     |    6 +++---
 drivers/net/irda/via-ircc.c     |    8 ++++----
 drivers/net/irda/vlsi_ir.c      |    2 +-
 drivers/net/irda/w83977af_ir.c  |    2 +-
 14 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/drivers/net/irda/ali-ircc.c b/drivers/net/irda/ali-ircc.c
index 2ff1818..517e6b2 100644
--- a/drivers/net/irda/ali-ircc.c
+++ b/drivers/net/irda/ali-ircc.c
@@ -1904,7 +1904,7 @@ static int  ali_ircc_dma_receive_complete(struct ali_ircc_cb *self)
 			 */
 			do_gettimeofday(&self->stamp);
 
-			skb = dev_alloc_skb(len+1);
+			skb = irda_dev_alloc_skb(len+1);
 			if (skb == NULL)  
 			{
 				IRDA_WARNING("%s(), memory squeeze, "
diff --git a/drivers/net/irda/donauboe.c b/drivers/net/irda/donauboe.c
index 1f0f9ee..b4f954e 100644
--- a/drivers/net/irda/donauboe.c
+++ b/drivers/net/irda/donauboe.c
@@ -1265,7 +1265,7 @@ dumpbufs(self->rx_bufs[self->rxs],len,'<');
 
               if (len)
                 {
-                  skb = dev_alloc_skb (len + 1);
+		  skb = irda_dev_alloc_skb(len + 1);
                   if (skb)
                     {
                       skb_reserve (skb, 1);
diff --git a/drivers/net/irda/irda-usb.c b/drivers/net/irda/irda-usb.c
index f5c13af..0bfdcdf 100644
--- a/drivers/net/irda/irda-usb.c
+++ b/drivers/net/irda/irda-usb.c
@@ -890,12 +890,12 @@ static void irda_usb_receive(struct urb *urb)
 
 	/* Allocate a new skb */
 	if (self->capability & IUC_STIR421X)
-		newskb = dev_alloc_skb(docopy ? urb->actual_length :
-				       IRDA_SKB_MAX_MTU +
-				       USB_IRDA_STIR421X_HEADER);
+		newskb = irda_dev_alloc_skb(docopy ? urb->actual_length :
+					    IRDA_SKB_MAX_MTU +
+					    USB_IRDA_STIR421X_HEADER);
 	else
-		newskb = dev_alloc_skb(docopy ? urb->actual_length :
-				       IRDA_SKB_MAX_MTU);
+		newskb = irda_dev_alloc_skb(docopy ? urb->actual_length :
+					    IRDA_SKB_MAX_MTU);
 
 	if (!newskb)  {
 		self->stats.rx_dropped++;
@@ -1235,7 +1235,7 @@ static int irda_usb_net_open(struct net_device *netdev)
 	/* Now that we can pass data to IrLAP, allow the USB layer
 	 * to send us some data... */
 	for (i = 0; i < IU_MAX_ACTIVE_RX_URBS; i++) {
-		struct sk_buff *skb = dev_alloc_skb(IRDA_SKB_MAX_MTU);
+		struct sk_buff *skb = irda_dev_alloc_skb(IRDA_SKB_MAX_MTU);
 		if (!skb) {
 			/* If this ever happen, we are in deep s***.
 			 * Basically, we can't start the Rx path... */
diff --git a/drivers/net/irda/kingsun-sir.c b/drivers/net/irda/kingsun-sir.c
index e1429fc..47ba0a9 100644
--- a/drivers/net/irda/kingsun-sir.c
+++ b/drivers/net/irda/kingsun-sir.c
@@ -267,7 +267,7 @@ static int kingsun_net_open(struct net_device *netdev)
 	kingsun->rx_buff.in_frame = FALSE;
 	kingsun->rx_buff.state = OUTSIDE_FRAME;
 	kingsun->rx_buff.truesize = IRDA_SKB_MAX_MTU;
-	kingsun->rx_buff.skb = dev_alloc_skb(IRDA_SKB_MAX_MTU);
+	kingsun->rx_buff.skb = irda_dev_alloc_skb(IRDA_SKB_MAX_MTU);
 	if (!kingsun->rx_buff.skb)
 		goto free_mem;
 
diff --git a/drivers/net/irda/ks959-sir.c b/drivers/net/irda/ks959-sir.c
index 2e67ae0..97f49ed 100644
--- a/drivers/net/irda/ks959-sir.c
+++ b/drivers/net/irda/ks959-sir.c
@@ -507,7 +507,7 @@ static int ks959_net_open(struct net_device *netdev)
 	kingsun->rx_unwrap_buff.in_frame = FALSE;
 	kingsun->rx_unwrap_buff.state = OUTSIDE_FRAME;
 	kingsun->rx_unwrap_buff.truesize = IRDA_SKB_MAX_MTU;
-	kingsun->rx_unwrap_buff.skb = dev_alloc_skb(IRDA_SKB_MAX_MTU);
+	kingsun->rx_unwrap_buff.skb = irda_dev_alloc_skb(IRDA_SKB_MAX_MTU);
 	if (!kingsun->rx_unwrap_buff.skb)
 		goto free_mem;
 
diff --git a/drivers/net/irda/ksdazzle-sir.c b/drivers/net/irda/ksdazzle-sir.c
index 3843b5f..e747c59 100644
--- a/drivers/net/irda/ksdazzle-sir.c
+++ b/drivers/net/irda/ksdazzle-sir.c
@@ -401,7 +401,7 @@ static int ksdazzle_net_open(struct net_device *netdev)
 	kingsun->rx_unwrap_buff.in_frame = FALSE;
 	kingsun->rx_unwrap_buff.state = OUTSIDE_FRAME;
 	kingsun->rx_unwrap_buff.truesize = IRDA_SKB_MAX_MTU;
-	kingsun->rx_unwrap_buff.skb = dev_alloc_skb(IRDA_SKB_MAX_MTU);
+	kingsun->rx_unwrap_buff.skb = irda_dev_alloc_skb(IRDA_SKB_MAX_MTU);
 	if (!kingsun->rx_unwrap_buff.skb)
 		goto free_mem;
 
diff --git a/drivers/net/irda/mcs7780.c b/drivers/net/irda/mcs7780.c
index ad92d3f..9776f41 100644
--- a/drivers/net/irda/mcs7780.c
+++ b/drivers/net/irda/mcs7780.c
@@ -418,7 +418,7 @@ static void mcs_unwrap_mir(struct mcs_cb *mcs, __u8 *buf, int len)
 		return;
 	}
 
-	skb = dev_alloc_skb(new_len + 1);
+	skb = irda_dev_alloc_skb(new_len + 1);
 	if(unlikely(!skb)) {
 		++mcs->stats.rx_dropped;
 		return;
@@ -471,7 +471,7 @@ static void mcs_unwrap_fir(struct mcs_cb *mcs, __u8 *buf, int len)
 		return;
 	}
 
-	skb = dev_alloc_skb(new_len + 1);
+	skb = irda_dev_alloc_skb(new_len + 1);
 	if(unlikely(!skb)) {
 		++mcs->stats.rx_dropped;
 		return;
@@ -718,7 +718,7 @@ static int mcs_net_open(struct net_device *netdev)
 	/* Initialize for SIR/FIR to copy data directly into skb.  */
 	mcs->receiving = 0;
 	mcs->rx_buff.truesize = IRDA_SKB_MAX_MTU;
-	mcs->rx_buff.skb = dev_alloc_skb(IRDA_SKB_MAX_MTU);
+	mcs->rx_buff.skb = irda_dev_alloc_skb(IRDA_SKB_MAX_MTU);
 	if (!mcs->rx_buff.skb)
 		goto error1;
 
diff --git a/drivers/net/irda/nsc-ircc.c b/drivers/net/irda/nsc-ircc.c
index 8583d95..05f53ca 100644
--- a/drivers/net/irda/nsc-ircc.c
+++ b/drivers/net/irda/nsc-ircc.c
@@ -1858,7 +1858,7 @@ static int nsc_ircc_dma_receive_complete(struct nsc_ircc_cb *self, int iobase)
 			 */
 			do_gettimeofday(&self->stamp);
 
-			skb = dev_alloc_skb(len+1);
+			skb = irda_dev_alloc_skb(len+1);
 			if (skb == NULL)  {
 				IRDA_WARNING("%s(), memory squeeze, "
 					     "dropping frame.\n",
diff --git a/drivers/net/irda/sir_dev.c b/drivers/net/irda/sir_dev.c
index 3f32909..3ca64e8 100644
--- a/drivers/net/irda/sir_dev.c
+++ b/drivers/net/irda/sir_dev.c
@@ -760,7 +760,9 @@ static int sirdev_alloc_buffers(struct sir_dev *dev)
 	dev->rx_buff.truesize = IRDA_SKB_MAX_MTU; 
 
 	/* Bootstrap ZeroCopy Rx */
-	dev->rx_buff.skb = __dev_alloc_skb(dev->rx_buff.truesize, GFP_KERNEL);
+	dev->rx_buff.skb = __dev_alloc_skb(dev->rx_buff.truesize +
+					   sizeof(struct irda_skb_cb),
+					   GFP_KERNEL);
 	if (dev->rx_buff.skb == NULL)
 		return -ENOMEM;
 	skb_reserve(dev->rx_buff.skb, 1);
diff --git a/drivers/net/irda/smsc-ircc2.c b/drivers/net/irda/smsc-ircc2.c
index b5360fe..f13c79e 100644
--- a/drivers/net/irda/smsc-ircc2.c
+++ b/drivers/net/irda/smsc-ircc2.c
@@ -1450,7 +1450,7 @@ static void smsc_ircc_dma_receive_complete(struct smsc_ircc_cb *self)
 	}
 	IRDA_DEBUG(2, "%s: msgcnt = %d, len=%d\n", __func__, msgcnt, len);
 
-	skb = dev_alloc_skb(len + 1);
+	skb = irda_dev_alloc_skb(len + 1);
 	if (!skb) {
 		IRDA_WARNING("%s(), memory squeeze, dropping frame.\n",
 			     __func__);
diff --git a/drivers/net/irda/stir4200.c b/drivers/net/irda/stir4200.c
index 3575804..f0edc00 100644
--- a/drivers/net/irda/stir4200.c
+++ b/drivers/net/irda/stir4200.c
@@ -338,7 +338,7 @@ static void fir_eof(struct stir_cb *stir)
 
 	/* if frame is short then just copy it */
 	if (len < IRDA_RX_COPY_THRESHOLD) {
-		nskb = dev_alloc_skb(len + 1);
+		nskb = irda_dev_alloc_skb(len + 1);
 		if (unlikely(!nskb)) {
 			++stir->stats.rx_dropped;
 			return;
@@ -347,7 +347,7 @@ static void fir_eof(struct stir_cb *stir)
 		skb = nskb;
 		skb_copy_to_linear_data(nskb, rx_buff->data, len);
 	} else {
-		nskb = dev_alloc_skb(rx_buff->truesize);
+		nskb = irda_dev_alloc_skb(rx_buff->truesize);
 		if (unlikely(!nskb)) {
 			++stir->stats.rx_dropped;
 			return;
@@ -871,7 +871,7 @@ static int stir_net_open(struct net_device *netdev)
 	/* Initialize for SIR/FIR to copy data directly into skb.  */
 	stir->receiving = 0;
 	stir->rx_buff.truesize = IRDA_SKB_MAX_MTU;
-	stir->rx_buff.skb = dev_alloc_skb(IRDA_SKB_MAX_MTU);
+	stir->rx_buff.skb = irda_dev_alloc_skb(IRDA_SKB_MAX_MTU);
 	if (!stir->rx_buff.skb) 
 		goto err_out1;
 
diff --git a/drivers/net/irda/via-ircc.c b/drivers/net/irda/via-ircc.c
index 84e609e..8784215 100644
--- a/drivers/net/irda/via-ircc.c
+++ b/drivers/net/irda/via-ircc.c
@@ -1101,7 +1101,7 @@ static int via_ircc_dma_receive_complete(struct via_ircc_cb *self,
 
 	if (self->io.speed < 4000000) {	//Speed below FIR
 		len = GetRecvByte(iobase, self);
-		skb = dev_alloc_skb(len + 1);
+		skb = irda_dev_alloc_skb(len + 1);
 		if (skb == NULL)
 			return FALSE;
 		// Make sure IP header gets aligned 
@@ -1173,7 +1173,7 @@ F01_E */
 		st_fifo->head++;
 		st_fifo->len--;
 
-		skb = dev_alloc_skb(len + 1 - 4);
+		skb = irda_dev_alloc_skb(len + 1 - 4);
 		/*
 		 * if frame size,data ptr,or skb ptr are wrong ,the get next
 		 * entry.
@@ -1224,7 +1224,7 @@ static int upload_rxdata(struct via_ircc_cb *self, int iobase)
 		return FALSE;
 	}
 
-	skb = dev_alloc_skb(len + 1);
+	skb = irda_dev_alloc_skb(len + 1);
 	if (skb == NULL) {
 		self->stats.rx_dropped++;
 		return FALSE;
@@ -1288,7 +1288,7 @@ static int RxTimerHandler(struct via_ircc_cb *self, int iobase)
 			st_fifo->head++;
 			st_fifo->len--;
 
-			skb = dev_alloc_skb(len + 1 - 4);
+			skb = irda_dev_alloc_skb(len + 1 - 4);
 			/*
 			 * if frame size, data ptr, or skb ptr are wrong,
 			 * then get next entry.
diff --git a/drivers/net/irda/vlsi_ir.c b/drivers/net/irda/vlsi_ir.c
index 9c926d2..162872e 100644
--- a/drivers/net/irda/vlsi_ir.c
+++ b/drivers/net/irda/vlsi_ir.c
@@ -622,7 +622,7 @@ static void vlsi_fill_rx(struct vlsi_ring *r)
 			break;
 		}
 		if (!rd->skb) {
-			rd->skb = dev_alloc_skb(IRLAP_SKB_ALLOCSIZE);
+			rd->skb = irda_dev_alloc_skb(IRLAP_SKB_ALLOCSIZE);
 			if (rd->skb) {
 				skb_reserve(rd->skb,1);
 				rd->skb->protocol = htons(ETH_P_IRDA);
diff --git a/drivers/net/irda/w83977af_ir.c b/drivers/net/irda/w83977af_ir.c
index 002a6d7..a3a44c7 100644
--- a/drivers/net/irda/w83977af_ir.c
+++ b/drivers/net/irda/w83977af_ir.c
@@ -889,7 +889,7 @@ int w83977af_dma_receive_complete(struct w83977af_ir *self)
 #endif
 			}
 						
-			skb = dev_alloc_skb(len+1);
+			skb = irda_dev_alloc_skb(len+1);
 			if (skb == NULL)  {
 				printk(KERN_INFO
 				       "%s(), memory squeeze, dropping frame.\n", __func__);
-- 
1.6.0.4.766.g6fc4a.dirty

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [RFC PATCH 7/9] irda: Stack RX path callers should use irda_dev_alloc_skb
  2008-12-15  1:57 [RFC PATCH 0/9] IrDA 2.6.28 bug fix Samuel Ortiz
                   ` (5 preceding siblings ...)
  2008-12-15  1:57 ` [RFC PATCH 6/9] irda: Drivers should use irda_dev_alloc_skb() on the RX path Samuel Ortiz
@ 2008-12-15  1:57 ` Samuel Ortiz
  2008-12-15  1:57 ` [RFC PATCH 8/9] irda: Add a WARN_ON when our head room is too small Samuel Ortiz
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Samuel Ortiz @ 2008-12-15  1:57 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, irda-users

[-- Attachment #1: 0007-irda-Stack-RX-path-callers-should-use-irda_dev_allo.patch --]
[-- Type: text/plain, Size: 1225 bytes --]

Signed-off-by: Samuel Ortiz <samuel@sortiz.org>
---
 net/irda/irttp.c   |    2 +-
 net/irda/wrapper.c |    3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/irda/irttp.c b/net/irda/irttp.c
index 4b913de..6f28186 100644
--- a/net/irda/irttp.c
+++ b/net/irda/irttp.c
@@ -241,7 +241,7 @@ static struct sk_buff *irttp_reassemble_skb(struct tsap_cb *self)
 	IRDA_DEBUG(2, "%s(), self->rx_sdu_size=%d\n", __func__,
 		   self->rx_sdu_size);
 
-	skb = dev_alloc_skb(TTP_HEADER + self->rx_sdu_size);
+	skb = irda_dev_alloc_skb(TTP_HEADER + self->rx_sdu_size);
 	if (!skb)
 		return NULL;
 
diff --git a/net/irda/wrapper.c b/net/irda/wrapper.c
index b13cb3b..e1ab6d1 100644
--- a/net/irda/wrapper.c
+++ b/net/irda/wrapper.c
@@ -225,7 +225,8 @@ async_bump(struct net_device *dev,
 		  (rx_buff->len < IRDA_RX_COPY_THRESHOLD));
 
 	/* Allocate a new skb */
-	newskb = dev_alloc_skb(docopy ? rx_buff->len + 1 : rx_buff->truesize);
+	newskb = irda_dev_alloc_skb(docopy ?
+				    rx_buff->len + 1 : rx_buff->truesize);
 	if (!newskb)  {
 		stats->rx_dropped++;
 		/* We could deliver the current skb if doing ZeroCopy Rx,
-- 
1.6.0.4.766.g6fc4a.dirty

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [RFC PATCH 8/9] irda: Add a WARN_ON when our head room is too small
  2008-12-15  1:57 [RFC PATCH 0/9] IrDA 2.6.28 bug fix Samuel Ortiz
                   ` (6 preceding siblings ...)
  2008-12-15  1:57 ` [RFC PATCH 7/9] irda: Stack RX path callers should use irda_dev_alloc_skb Samuel Ortiz
@ 2008-12-15  1:57 ` Samuel Ortiz
  2008-12-15  1:57 ` [RFC PATCH 9/9] irda: Fix irda_skb_cb size Samuel Ortiz
       [not found] ` <20081215015729.587697008-jcdQHdrhKHMdnm+yROfE0A@public.gmane.org>
  9 siblings, 0 replies; 13+ messages in thread
From: Samuel Ortiz @ 2008-12-15  1:57 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, irda-users

[-- Attachment #1: 0008-irda-Add-a-WARN_ON-when-our-head-room-is-too-small.patch --]
[-- Type: text/plain, Size: 725 bytes --]

Signed-off-by: Samuel Ortiz <samuel@sortiz.org>
---
 include/net/irda/irda_device.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/net/irda/irda_device.h b/include/net/irda/irda_device.h
index 3bbf695..28c690c 100644
--- a/include/net/irda/irda_device.h
+++ b/include/net/irda/irda_device.h
@@ -233,6 +233,7 @@ void irda_setup_dma(int channel, dma_addr_t buffer, int count, int mode);
 
 static inline struct irda_skb_cb *irda_get_skb_cb(struct sk_buff *skb)
 {
+	WARN_ON(skb_headroom(skb) < sizeof(struct irda_skb_cb));
 	return (struct irda_skb_cb *)(skb->data - sizeof(struct irda_skb_cb));
 }
 
-- 
1.6.0.4.766.g6fc4a.dirty

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [RFC PATCH 9/9] irda: Fix irda_skb_cb size
  2008-12-15  1:57 [RFC PATCH 0/9] IrDA 2.6.28 bug fix Samuel Ortiz
                   ` (7 preceding siblings ...)
  2008-12-15  1:57 ` [RFC PATCH 8/9] irda: Add a WARN_ON when our head room is too small Samuel Ortiz
@ 2008-12-15  1:57 ` Samuel Ortiz
       [not found] ` <20081215015729.587697008-jcdQHdrhKHMdnm+yROfE0A@public.gmane.org>
  9 siblings, 0 replies; 13+ messages in thread
From: Samuel Ortiz @ 2008-12-15  1:57 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, irda-users

[-- Attachment #1: 0009-irda-Fix-irda_skb_cb-size.patch --]
[-- Type: text/plain, Size: 948 bytes --]

Since we're stuffing an irda_skb_cb structure in front of the skb->data,
having sizeof(irda_skb_cb) as a multiple of 4 would be a lot nicer.

Signed-off-by: Samuel Ortiz <samuel@sortiz.org>
---
 include/net/irda/irda_device.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/net/irda/irda_device.h b/include/net/irda/irda_device.h
index 28c690c..71c07ce 100644
--- a/include/net/irda/irda_device.h
+++ b/include/net/irda/irda_device.h
@@ -146,7 +146,7 @@ struct irda_skb_cb {
 	void    *context;    /* May be used by drivers */
 	void    (*destructor)(struct sk_buff *skb); /* Used for flow control */
 	__u16   xbofs_delay; /* Number of xbofs used for generating the mtt */
-	__u8    line;        /* Used by IrCOMM in IrLPT mode */
+	__u32   line;        /* Used by IrCOMM in IrLPT mode */
 };
 
 /* Chip specific info */
-- 
1.6.0.4.766.g6fc4a.dirty

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* Re: [RFC PATCH 0/9] IrDA 2.6.28 bug fix
       [not found] ` <20081215015729.587697008-jcdQHdrhKHMdnm+yROfE0A@public.gmane.org>
@ 2008-12-15  7:08   ` David Miller
  2008-12-15 12:31     ` [irda-users] " Samuel Ortiz
  0 siblings, 1 reply; 13+ messages in thread
From: David Miller @ 2008-12-15  7:08 UTC (permalink / raw)
  To: samuel-jcdQHdrhKHMdnm+yROfE0A
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	irda-users-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

From: Samuel Ortiz <samuel-jcdQHdrhKHMdnm+yROfE0A@public.gmane.org>
Date: Mon, 15 Dec 2008 02:57:29 +0100

> This is a 9 patches series for IrDA, against your net-2.6 tree.
> This is an attempt to fix kernel.bugzilla.org bug #11795, where we noticed
> skb->cb could be altered once submitted to dev_queue_xmit. Since IrDA is using
> this callback to pass per-skb information, we are now stuffing it in front of
> skb->data, after allocating the right headroom.
> 
> Another solution would be to play with the IrDA physical header and skb_pull
> our skbs whenever we are physically transmitting the data.

Hi Sam.

I appreciate you working on this.

But I there are two issues here:

1) There is no way I can put a series of 9 invasive patches like these
   so late into 2.6.28

   If we need to fix it in 2.6.28 it'd need to be a 5 to 10 line
   change at most, even if hackish, before I could seriously consider
   it.

2) Just like you can't claim ownership to skb->cb after dev_queue_xmit(),
   you just as equally can't claim ownership to areas in front of
   skb->data either.

I'm pretty sure we discussed how #2 wouldn't work for this problem.

I understand you need a way to pass information, but you're trying to
do it using things the IRDA (nor any other) stack does not own across
a dev_queue_xmit() invocation.

Furthermore, device layer schemes that try to use some shared
part of the skb for their communication is frail, and a good
way to see how frail it is is to consider how encapsulation of
such device types within themselves might be made to work.

You can't do it with skb->cb[] private storage, and you doubly can't
do it by sneaking things in front of skb->data because that's where
the encapsulated protocol headers would go.

------------------------------------------------------------------------------
SF.Net email is Sponsored by MIX09, March 18-20, 2009 in Las Vegas, Nevada.
The future of the web can't happen without you.  Join us at MIX09 to help
pave the way to the Next Web now. Learn more and register at
http://ad.doubleclick.net/clk;208669438;13503038;i?http://2009.visitmix.com/

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [irda-users] [RFC PATCH 0/9] IrDA 2.6.28 bug fix
  2008-12-15  7:08   ` [RFC PATCH 0/9] IrDA 2.6.28 bug fix David Miller
@ 2008-12-15 12:31     ` Samuel Ortiz
  2008-12-17  7:59       ` David Miller
  0 siblings, 1 reply; 13+ messages in thread
From: Samuel Ortiz @ 2008-12-15 12:31 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, irda-users

Hi Dave,

On Sun, Dec 14, 2008 at 11:08:37PM -0800, David Miller wrote:
> From: Samuel Ortiz <samuel@sortiz.org>
> Date: Mon, 15 Dec 2008 02:57:29 +0100
> 
> > This is a 9 patches series for IrDA, against your net-2.6 tree.
> > This is an attempt to fix kernel.bugzilla.org bug #11795, where we noticed
> > skb->cb could be altered once submitted to dev_queue_xmit. Since IrDA is using
> > this callback to pass per-skb information, we are now stuffing it in front of
> > skb->data, after allocating the right headroom.
> > 
> > Another solution would be to play with the IrDA physical header and skb_pull
> > our skbs whenever we are physically transmitting the data.
> 
> Hi Sam.
> 
> I appreciate you working on this.
> 
> But I there are two issues here:
> 
> 1) There is no way I can put a series of 9 invasive patches like these
>    so late into 2.6.28
> 
>    If we need to fix it in 2.6.28 it'd need to be a 5 to 10 line
>    change at most, even if hackish, before I could seriously consider
>    it.
Ok, I understand. Sorry for not being faster at tackling that bug.
Regarding hackish fixes, would something like commit
f7cd168645dda3e9067f24fabbfa787f9a237488 (see
http://git.kernel.org/?p=linux/kernel/git/davem/net-2.6.git;a=commit;h=f7cd168645dda3e9067f24fabbfa787f9a237488 )
be acceptable to you as a short term solution ?

 
> 2) Just like you can't claim ownership to skb->cb after dev_queue_xmit(),
>    you just as equally can't claim ownership to areas in front of
>    skb->data either.
> 
> I'm pretty sure we discussed how #2 wouldn't work for this problem.
> 
> I understand you need a way to pass information, but you're trying to
> do it using things the IRDA (nor any other) stack does not own across
> a dev_queue_xmit() invocation.
> 
> Furthermore, device layer schemes that try to use some shared
> part of the skb for their communication is frail, and a good
> way to see how frail it is is to consider how encapsulation of
> such device types within themselves might be made to work.
>
> You can't do it with skb->cb[] private storage, and you doubly can't
> do it by sneaking things in front of skb->data because that's where
> the encapsulated protocol headers would go.
Ok, so I guess I really have to work on passing that meta information along
with the actual data. I tried to avoid this as a lot of code on the IrDA stack
is accessing the IrLAP payload directly. I guess this is the right opportunity
to add proper IrDA accessors, and then stuff the IrDA cb in the actual data
header.
That's obviously -next material though...

Thanks a lot for the feedback.

Cheers,
Samuel.


^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [irda-users] [RFC PATCH 0/9] IrDA 2.6.28 bug fix
  2008-12-15 12:31     ` [irda-users] " Samuel Ortiz
@ 2008-12-17  7:59       ` David Miller
  0 siblings, 0 replies; 13+ messages in thread
From: David Miller @ 2008-12-17  7:59 UTC (permalink / raw)
  To: samuel; +Cc: netdev, irda-users

From: Samuel Ortiz <samuel@sortiz.org>
Date: Mon, 15 Dec 2008 13:31:44 +0100

> Regarding hackish fixes, would something like commit
> f7cd168645dda3e9067f24fabbfa787f9a237488 (see
> http://git.kernel.org/?p=linux/kernel/git/davem/net-2.6.git;a=commit;h=f7cd168645dda3e9067f24fabbfa787f9a237488 )
> be acceptable to you as a short term solution ?

Most definitely, yes.

^ permalink raw reply	[flat|nested] 13+ messages in thread
end of thread, other threads:[~2008-12-17  7:59 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-12-15  1:57 [RFC PATCH 0/9] IrDA 2.6.28 bug fix Samuel Ortiz
2008-12-15  1:57 ` [RFC PATCH 1/9] irda: Introduce irda_alloc_skb Samuel Ortiz
2008-12-15  1:57 ` [RFC PATCH 2/9] irda: stack should call irda_get_skb_cb() Samuel Ortiz
2008-12-15  1:57 ` [RFC PATCH 3/9] irda: IrDA drivers " Samuel Ortiz
2008-12-15  1:57 ` [RFC PATCH 4/9] irda: reserve irda_skb on sock_alloc_send_skb() skbs Samuel Ortiz
2008-12-15  1:57 ` [RFC PATCH 5/9] irda: Introduce irda_dev_alloc_skb Samuel Ortiz
2008-12-15  1:57 ` [RFC PATCH 6/9] irda: Drivers should use irda_dev_alloc_skb() on the RX path Samuel Ortiz
2008-12-15  1:57 ` [RFC PATCH 7/9] irda: Stack RX path callers should use irda_dev_alloc_skb Samuel Ortiz
2008-12-15  1:57 ` [RFC PATCH 8/9] irda: Add a WARN_ON when our head room is too small Samuel Ortiz
2008-12-15  1:57 ` [RFC PATCH 9/9] irda: Fix irda_skb_cb size Samuel Ortiz
     [not found] ` <20081215015729.587697008-jcdQHdrhKHMdnm+yROfE0A@public.gmane.org>
2008-12-15  7:08   ` [RFC PATCH 0/9] IrDA 2.6.28 bug fix David Miller
2008-12-15 12:31     ` [irda-users] " Samuel Ortiz
2008-12-17  7:59       ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.