[PATCH net-next v3 0/3] 6lowpan fixes

STALE5199d

Revision v3 of 2 in this series.

7 messages, 2 authors, 2012-06-13 · open the first message on its own page

[PATCH net-next v3 0/3] 6lowpan fixes

From: Alexander Smirnov <hidden>
Date: 2012-06-09 15:18:42

Hi David,

this is the 3-rd version of the patches.
Changes since v2:
 - Removed WARN_ON() and BUG() from skb fetch methods. That wasn't a good idea
   to crash kernel by such an unsignificant issue.
 - Added new patch (I've decided to add it here just to keep all the 6lowpan
   related code together)

Could you please review this set?

With best regards,
Alexander Smirnov

Alexander Smirnov (3):
  6lowpan: read data from skb safely
  6lowpan: fix hop limit compression
  6lowpan: remove excessive argument in pr_debug

 net/ieee802154/6lowpan.c |  163 ++++++++++++++++++++++------------------------
 1 files changed, 78 insertions(+), 85 deletions(-)

--
1.7.2.3

[PATCH net-next v3 2/3] 6lowpan: fix hop limit compression

From: Alexander Smirnov <hidden>
Date: 2012-06-09 15:18:53

Add missing pointer shift for the 'default' case.

Signed-off-by: Alexander Smirnov <redacted>
Cc: Tony Cheneau <redacted>
---
 net/ieee802154/6lowpan.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index 5c7bcf9..b45e229 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -492,6 +492,7 @@ static int lowpan_header_create(struct sk_buff *skb,
 		break;
 	default:
 		*hc06_ptr = hdr->hop_limit;
+		hc06_ptr += 1;
 		break;
 	}
 
-- 
1.7.2.3

[PATCH net-next v3 1/3] 6lowpan: read data from skb safely

From: Alexander Smirnov <hidden>
Date: 2012-06-09 15:18:54

Check if skb buffer can pull requested amount of bytes and return
an error in opposite case.

Signed-off-by: Alexander Smirnov <redacted>
---
 net/ieee802154/6lowpan.c |   70 +++++++++++++++++++++++++---------------------
 1 files changed, 38 insertions(+), 32 deletions(-)
diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index 32eb417..5c7bcf9 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -291,25 +291,26 @@ lowpan_compress_udp_header(u8 **hc06_ptr, struct sk_buff *skb)
 	*hc06_ptr += 2;
 }
 
-static u8 lowpan_fetch_skb_u8(struct sk_buff *skb)
+static inline int lowpan_fetch_skb_u8(struct sk_buff *skb, u8 *val)
 {
-	u8 ret;
+	if (unlikely(!pskb_may_pull(skb, 1)))
+		return -EINVAL;
 
-	ret = skb->data[0];
+	*val = skb->data[0];
 	skb_pull(skb, 1);
 
-	return ret;
+	return 0;
 }
 
-static u16 lowpan_fetch_skb_u16(struct sk_buff *skb)
+static inline int lowpan_fetch_skb_u16(struct sk_buff *skb, u16 *val)
 {
-	u16 ret;
-
-	BUG_ON(!pskb_may_pull(skb, 2));
+	if (unlikely(!pskb_may_pull(skb, 2)))
+		return -EINVAL;
 
-	ret = skb->data[0] | (skb->data[1] << 8);
+	*val = skb->data[0] | (skb->data[1] << 8);
 	skb_pull(skb, 2);
-	return ret;
+
+	return 0;
 }
 
 static int
@@ -318,7 +319,8 @@ lowpan_uncompress_udp_header(struct sk_buff *skb)
 	struct udphdr *uh = udp_hdr(skb);
 	u8 tmp;
 
-	tmp = lowpan_fetch_skb_u8(skb);
+	if (lowpan_fetch_skb_u8(skb, &tmp))
+		goto err;
 
 	if ((tmp & LOWPAN_NHC_UDP_MASK) == LOWPAN_NHC_UDP_ID) {
 		pr_debug("(%s): UDP header uncompression\n", __func__);
@@ -710,7 +712,9 @@ lowpan_process_data(struct sk_buff *skb)
 	/* at least two bytes will be used for the encoding */
 	if (skb->len < 2)
 		goto drop;
-	iphc0 = lowpan_fetch_skb_u8(skb);
+
+	if (lowpan_fetch_skb_u8(skb, &iphc0))
+		goto drop;
 
 	/* fragments assembling */
 	switch (iphc0 & LOWPAN_DISPATCH_MASK) {
@@ -722,8 +726,9 @@ lowpan_process_data(struct sk_buff *skb)
 		u16 tag;
 		bool found = false;
 
-		len = lowpan_fetch_skb_u8(skb); /* frame length */
-		tag = lowpan_fetch_skb_u16(skb);
+		if (lowpan_fetch_skb_u8(skb, &len) || /* frame length */
+		    lowpan_fetch_skb_u16(skb, &tag))  /* fragment tag */
+			goto drop;
 
 		/*
 		 * check if frame assembling with the same tag is
@@ -747,7 +752,8 @@ lowpan_process_data(struct sk_buff *skb)
 		if ((iphc0 & LOWPAN_DISPATCH_MASK) == LOWPAN_DISPATCH_FRAG1)
 			goto unlock_and_drop;
 
-		offset = lowpan_fetch_skb_u8(skb); /* fetch offset */
+		if (lowpan_fetch_skb_u8(skb, &offset)) /* fetch offset */
+			goto unlock_and_drop;
 
 		/* if payload fits buffer, copy it */
 		if (likely((offset * 8 + skb->len) <= frame->length))
@@ -769,7 +775,10 @@ lowpan_process_data(struct sk_buff *skb)
 			dev_kfree_skb(skb);
 			skb = frame->skb;
 			kfree(frame);
-			iphc0 = lowpan_fetch_skb_u8(skb);
+
+			if (lowpan_fetch_skb_u8(skb, &iphc0))
+				goto unlock_and_drop;
+
 			break;
 		}
 		spin_unlock(&flist_lock);
@@ -780,7 +789,8 @@ lowpan_process_data(struct sk_buff *skb)
 		break;
 	}
 
-	iphc1 = lowpan_fetch_skb_u8(skb);
+	if (lowpan_fetch_skb_u8(skb, &iphc1))
+		goto drop;
 
 	_saddr = mac_cb(skb)->sa.hwaddr;
 	_daddr = mac_cb(skb)->da.hwaddr;
@@ -791,9 +801,8 @@ lowpan_process_data(struct sk_buff *skb)
 	if (iphc1 & LOWPAN_IPHC_CID) {
 		pr_debug("(%s): CID flag is set, increase header with one\n",
 								__func__);
-		if (!skb->len)
+		if (lowpan_fetch_skb_u8(skb, &num_context))
 			goto drop;
-		num_context = lowpan_fetch_skb_u8(skb);
 	}
 
 	hdr.version = 6;
@@ -805,9 +814,9 @@ lowpan_process_data(struct sk_buff *skb)
 	 * ECN + DSCP + 4-bit Pad + Flow Label (4 bytes)
 	 */
 	case 0: /* 00b */
-		if (!skb->len)
+		if (lowpan_fetch_skb_u8(skb, &tmp))
 			goto drop;
-		tmp = lowpan_fetch_skb_u8(skb);
+
 		memcpy(&hdr.flow_lbl, &skb->data[0], 3);
 		skb_pull(skb, 3);
 		hdr.priority = ((tmp >> 2) & 0x0f);
@@ -819,9 +828,9 @@ lowpan_process_data(struct sk_buff *skb)
 	 * ECN + DSCP (1 byte), Flow Label is elided
 	 */
 	case 1: /* 10b */
-		if (!skb->len)
+		if (lowpan_fetch_skb_u8(skb, &tmp))
 			goto drop;
-		tmp = lowpan_fetch_skb_u8(skb);
+
 		hdr.priority = ((tmp >> 2) & 0x0f);
 		hdr.flow_lbl[0] = ((tmp << 6) & 0xC0) | ((tmp >> 2) & 0x30);
 		hdr.flow_lbl[1] = 0;
@@ -832,9 +841,9 @@ lowpan_process_data(struct sk_buff *skb)
 	 * ECN + 2-bit Pad + Flow Label (3 bytes), DSCP is elided
 	 */
 	case 2: /* 01b */
-		if (!skb->len)
+		if (lowpan_fetch_skb_u8(skb, &tmp))
 			goto drop;
-		tmp = lowpan_fetch_skb_u8(skb);
+
 		hdr.flow_lbl[0] = (skb->data[0] & 0x0F) | ((tmp >> 2) & 0x30);
 		memcpy(&hdr.flow_lbl[1], &skb->data[0], 2);
 		skb_pull(skb, 2);
@@ -853,9 +862,9 @@ lowpan_process_data(struct sk_buff *skb)
 	/* Next Header */
 	if ((iphc0 & LOWPAN_IPHC_NH_C) == 0) {
 		/* Next header is carried inline */
-		if (!skb->len)
+		if (lowpan_fetch_skb_u8(skb, &(hdr.nexthdr)))
 			goto drop;
-		hdr.nexthdr = lowpan_fetch_skb_u8(skb);
+
 		pr_debug("(%s): NH flag is set, next header is carried "
 			 "inline: %02x\n", __func__, hdr.nexthdr);
 	}
@@ -864,9 +873,8 @@ lowpan_process_data(struct sk_buff *skb)
 	if ((iphc0 & 0x03) != LOWPAN_IPHC_TTL_I)
 		hdr.hop_limit = lowpan_ttl_values[iphc0 & 0x03];
 	else {
-		if (!skb->len)
+		if (lowpan_fetch_skb_u8(skb, &(hdr.hop_limit)))
 			goto drop;
-		hdr.hop_limit = lowpan_fetch_skb_u8(skb);
 	}
 
 	/* Extract SAM to the tmp variable */
@@ -894,10 +902,8 @@ lowpan_process_data(struct sk_buff *skb)
 			pr_debug("(%s): destination address non-context-based"
 				 " multicast compression\n", __func__);
 			if (0 < tmp && tmp < 3) {
-				if (!skb->len)
+				if (lowpan_fetch_skb_u8(skb, &prefix[1]))
 					goto drop;
-				else
-					prefix[1] = lowpan_fetch_skb_u8(skb);
 			}
 
 			err = lowpan_uncompress_addr(skb, &hdr.daddr, prefix,
-- 
1.7.2.3

[PATCH net-next v3 3/3] 6lowpan: remove excessive argument in pr_debug

From: Alexander Smirnov <hidden>
Date: 2012-06-09 15:18:54

Remove excessive __func__ argument in pr_debug function.

Signed-off-by: Alexander Smirnov <redacted>
---
 net/ieee802154/6lowpan.c |   92 +++++++++++++++++++--------------------------
 1 files changed, 39 insertions(+), 53 deletions(-)
diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index b45e229..068279e 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -240,7 +240,7 @@ lowpan_uncompress_addr(struct sk_buff *skb, struct in6_addr *ipaddr,
 		lowpan_uip_ds6_set_addr_iid(ipaddr, lladdr);
 	}
 
-	pr_debug("(%s): uncompressing %d + %d => ", __func__, prefcount,
+	pr_debug("uncompressing %d + %d => ", prefcount,
 								postcount);
 	lowpan_raw_dump_inline(NULL, NULL, ipaddr->s6_addr, 16);
 
@@ -252,13 +252,13 @@ lowpan_compress_udp_header(u8 **hc06_ptr, struct sk_buff *skb)
 {
 	struct udphdr *uh = udp_hdr(skb);
 
-	pr_debug("(%s): UDP header compression\n", __func__);
+	pr_debug("UDP header compression\n");
 
 	if (((uh->source & LOWPAN_NHC_UDP_4BIT_MASK) ==
 				LOWPAN_NHC_UDP_4BIT_PORT) &&
 	    ((uh->dest & LOWPAN_NHC_UDP_4BIT_MASK) ==
 				LOWPAN_NHC_UDP_4BIT_PORT)) {
-		pr_debug("(%s): both ports compression to 4 bits\n", __func__);
+		pr_debug("both ports compression to 4 bits\n");
 		**hc06_ptr = LOWPAN_NHC_UDP_CS_P_11;
 		**(hc06_ptr + 1) = /* subtraction is faster */
 		   (u8)((uh->dest - LOWPAN_NHC_UDP_4BIT_PORT) +
@@ -266,20 +266,20 @@ lowpan_compress_udp_header(u8 **hc06_ptr, struct sk_buff *skb)
 		*hc06_ptr += 2;
 	} else if ((uh->dest & LOWPAN_NHC_UDP_8BIT_MASK) ==
 			LOWPAN_NHC_UDP_8BIT_PORT) {
-		pr_debug("(%s): remove 8 bits of dest\n", __func__);
+		pr_debug("remove 8 bits of dest\n");
 		**hc06_ptr = LOWPAN_NHC_UDP_CS_P_01;
 		memcpy(*hc06_ptr + 1, &uh->source, 2);
 		**(hc06_ptr + 3) = (u8)(uh->dest - LOWPAN_NHC_UDP_8BIT_PORT);
 		*hc06_ptr += 4;
 	} else if ((uh->source & LOWPAN_NHC_UDP_8BIT_MASK) ==
 			LOWPAN_NHC_UDP_8BIT_PORT) {
-		pr_debug("(%s): remove 8 bits of source\n", __func__);
+		pr_debug("remove 8 bits of source\n");
 		**hc06_ptr = LOWPAN_NHC_UDP_CS_P_10;
 		memcpy(*hc06_ptr + 1, &uh->dest, 2);
 		**(hc06_ptr + 3) = (u8)(uh->source - LOWPAN_NHC_UDP_8BIT_PORT);
 		*hc06_ptr += 4;
 	} else {
-		pr_debug("(%s): can't compress header\n", __func__);
+		pr_debug("can't compress header\n");
 		**hc06_ptr = LOWPAN_NHC_UDP_CS_P_00;
 		memcpy(*hc06_ptr + 1, &uh->source, 2);
 		memcpy(*hc06_ptr + 3, &uh->dest, 2);
@@ -323,7 +323,7 @@ lowpan_uncompress_udp_header(struct sk_buff *skb)
 		goto err;
 
 	if ((tmp & LOWPAN_NHC_UDP_MASK) == LOWPAN_NHC_UDP_ID) {
-		pr_debug("(%s): UDP header uncompression\n", __func__);
+		pr_debug("UDP header uncompression\n");
 		switch (tmp & LOWPAN_NHC_UDP_CS_P_11) {
 		case LOWPAN_NHC_UDP_CS_P_00:
 			memcpy(&uh->source, &skb->data[0], 2);
@@ -349,19 +349,19 @@ lowpan_uncompress_udp_header(struct sk_buff *skb)
 			skb_pull(skb, 1);
 			break;
 		default:
-			pr_debug("(%s) ERROR: unknown UDP format\n", __func__);
+			pr_debug("ERROR: unknown UDP format\n");
 			goto err;
 			break;
 		}
 
-		pr_debug("(%s): uncompressed UDP ports: src = %d, dst = %d\n",
-					__func__, uh->source, uh->dest);
+		pr_debug("uncompressed UDP ports: src = %d, dst = %d\n",
+			 uh->source, uh->dest);
 
 		/* copy checksum */
 		memcpy(&uh->check, &skb->data[0], 2);
 		skb_pull(skb, 2);
 	} else {
-		pr_debug("(%s): ERROR: unsupported NH format\n", __func__);
+		pr_debug("ERROR: unsupported NH format\n");
 		goto err;
 	}
 
@@ -394,10 +394,9 @@ static int lowpan_header_create(struct sk_buff *skb,
 	hdr = ipv6_hdr(skb);
 	hc06_ptr = head + 2;
 
-	pr_debug("(%s): IPv6 header dump:\n\tversion = %d\n\tlength  = %d\n"
-		 "\tnexthdr = 0x%02x\n\thop_lim = %d\n", __func__,
-		hdr->version, ntohs(hdr->payload_len), hdr->nexthdr,
-		hdr->hop_limit);
+	pr_debug("IPv6 header dump:\n\tversion = %d\n\tlength  = %d\n"
+		 "\tnexthdr = 0x%02x\n\thop_lim = %d\n", hdr->version,
+		 ntohs(hdr->payload_len), hdr->nexthdr, hdr->hop_limit);
 
 	lowpan_raw_dump_table(__func__, "raw skb network header dump",
 		skb_network_header(skb), sizeof(struct ipv6hdr));
@@ -498,23 +497,22 @@ static int lowpan_header_create(struct sk_buff *skb,
 
 	/* source address compression */
 	if (is_addr_unspecified(&hdr->saddr)) {
-		pr_debug("(%s): source address is unspecified, setting SAC\n",
-								__func__);
+		pr_debug("source address is unspecified, setting SAC\n");
 		iphc1 |= LOWPAN_IPHC_SAC;
 	/* TODO: context lookup */
 	} else if (is_addr_link_local(&hdr->saddr)) {
-		pr_debug("(%s): source address is link-local\n", __func__);
+		pr_debug("source address is link-local\n");
 		iphc1 |= lowpan_compress_addr_64(&hc06_ptr,
 				LOWPAN_IPHC_SAM_BIT, &hdr->saddr, saddr);
 	} else {
-		pr_debug("(%s): send the full source address\n", __func__);
+		pr_debug("send the full source address\n");
 		memcpy(hc06_ptr, &hdr->saddr.s6_addr16[0], 16);
 		hc06_ptr += 16;
 	}
 
 	/* destination address compression */
 	if (is_addr_mcast(&hdr->daddr)) {
-		pr_debug("(%s): destination address is multicast", __func__);
+		pr_debug("destination address is multicast: ");
 		iphc1 |= LOWPAN_IPHC_M;
 		if (lowpan_is_mcast_addr_compressable8(&hdr->daddr)) {
 			pr_debug("compressed to 1 octet\n");
@@ -543,7 +541,7 @@ static int lowpan_header_create(struct sk_buff *skb,
 			hc06_ptr += 16;
 		}
 	} else {
-		pr_debug("(%s): destination address is unicast: ", __func__);
+		pr_debug("destination address is unicast: ");
 		/* TODO: context lookup */
 		if (is_addr_link_local(&hdr->daddr)) {
 			pr_debug("destination address is link-local\n");
@@ -642,8 +640,7 @@ static void lowpan_fragment_timer_expired(unsigned long entry_addr)
 {
 	struct lowpan_fragment *entry = (struct lowpan_fragment *)entry_addr;
 
-	pr_debug("%s: timer expired for frame with tag %d\n", __func__,
-								entry->tag);
+	pr_debug("timer expired for frame with tag %d\n", entry->tag);
 
 	spin_lock(&flist_lock);
 	list_del(&entry->list);
@@ -796,12 +793,11 @@ lowpan_process_data(struct sk_buff *skb)
 	_saddr = mac_cb(skb)->sa.hwaddr;
 	_daddr = mac_cb(skb)->da.hwaddr;
 
-	pr_debug("(%s): iphc0 = %02x, iphc1 = %02x\n", __func__, iphc0, iphc1);
+	pr_debug("iphc0 = %02x, iphc1 = %02x\n", iphc0, iphc1);
 
 	/* another if the CID flag is set */
 	if (iphc1 & LOWPAN_IPHC_CID) {
-		pr_debug("(%s): CID flag is set, increase header with one\n",
-								__func__);
+		pr_debug("CID flag is set, increase header with one\n");
 		if (lowpan_fetch_skb_u8(skb, &num_context))
 			goto drop;
 	}
@@ -866,8 +862,8 @@ lowpan_process_data(struct sk_buff *skb)
 		if (lowpan_fetch_skb_u8(skb, &(hdr.nexthdr)))
 			goto drop;
 
-		pr_debug("(%s): NH flag is set, next header is carried "
-			 "inline: %02x\n", __func__, hdr.nexthdr);
+		pr_debug("NH flag is set, next header carried inline: %02x\n",
+			 hdr.nexthdr);
 	}
 
 	/* Hop Limit */
@@ -882,7 +878,7 @@ lowpan_process_data(struct sk_buff *skb)
 	tmp = ((iphc1 & LOWPAN_IPHC_SAM) >> LOWPAN_IPHC_SAM_BIT) & 0x03;
 
 	/* Source address uncompression */
-	pr_debug("(%s): source address stateless compression\n", __func__);
+	pr_debug("source address stateless compression\n");
 	err = lowpan_uncompress_addr(skb, &hdr.saddr, lowpan_llprefix,
 				lowpan_unc_llconf[tmp], skb->data);
 	if (err)
@@ -894,14 +890,14 @@ lowpan_process_data(struct sk_buff *skb)
 	/* check for Multicast Compression */
 	if (iphc1 & LOWPAN_IPHC_M) {
 		if (iphc1 & LOWPAN_IPHC_DAC) {
-			pr_debug("(%s): destination address context-based "
-				 "multicast compression\n", __func__);
+			pr_debug("destination address context-based "
+				 "multicast compression\n");
 			/* TODO: implement this */
 		} else {
 			u8 prefix[] = {0xff, 0x02};
 
-			pr_debug("(%s): destination address non-context-based"
-				 " multicast compression\n", __func__);
+			pr_debug("destination address non-context-based"
+				 " multicast compression\n");
 			if (0 < tmp && tmp < 3) {
 				if (lowpan_fetch_skb_u8(skb, &prefix[1]))
 					goto drop;
@@ -913,8 +909,7 @@ lowpan_process_data(struct sk_buff *skb)
 				goto drop;
 		}
 	} else {
-		pr_debug("(%s): destination address stateless compression\n",
-								__func__);
+		pr_debug("destination address stateless compression\n");
 		err = lowpan_uncompress_addr(skb, &hdr.daddr, lowpan_llprefix,
 				lowpan_unc_llconf[tmp], skb->data);
 		if (err)
@@ -929,11 +924,11 @@ lowpan_process_data(struct sk_buff *skb)
 	/* Not fragmented package */
 	hdr.payload_len = htons(skb->len);
 
-	pr_debug("(%s): skb headroom size = %d, data length = %d\n", __func__,
-						skb_headroom(skb), skb->len);
+	pr_debug("skb headroom size = %d, data length = %d\n",
+		 skb_headroom(skb), skb->len);
 
-	pr_debug("(%s): IPv6 header dump:\n\tversion = %d\n\tlength  = %d\n\t"
-		 "nexthdr = 0x%02x\n\thop_lim = %d\n", __func__, hdr.version,
+	pr_debug("IPv6 header dump:\n\tversion = %d\n\tlength  = %d\n\t"
+		 "nexthdr = 0x%02x\n\thop_lim = %d\n", hdr.version,
 		 ntohs(hdr.payload_len), hdr.nexthdr, hdr.hop_limit);
 
 	lowpan_raw_dump_table(__func__, "raw header dump", (u8 *)&hdr,
@@ -1035,11 +1030,11 @@ static netdev_tx_t lowpan_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	int err = -1;
 
-	pr_debug("(%s): package xmit\n", __func__);
+	pr_debug("package xmit\n");
 
 	skb->dev = lowpan_dev_info(dev)->real_dev;
 	if (skb->dev == NULL) {
-		pr_debug("(%s) ERROR: no real wpan device found\n", __func__);
+		pr_debug("ERROR: no real wpan device found\n");
 		goto error;
 	}
 
@@ -1048,14 +1043,13 @@ static netdev_tx_t lowpan_xmit(struct sk_buff *skb, struct net_device *dev)
 		goto out;
 	}
 
-	pr_debug("(%s): frame is too big, fragmentation is needed\n",
-								__func__);
+	pr_debug("frame is too big, fragmentation is needed\n");
 	err = lowpan_skb_fragmentation(skb);
 error:
 	dev_kfree_skb(skb);
 out:
 	if (err < 0)
-		pr_debug("(%s): ERROR: xmit failed\n", __func__);
+		pr_debug("ERROR: xmit failed\n");
 
 	return (err < 0 ? NETDEV_TX_BUSY : NETDEV_TX_OK);
 }
@@ -1101,8 +1095,6 @@ static struct ieee802154_mlme_ops lowpan_mlme = {
 
 static void lowpan_setup(struct net_device *dev)
 {
-	pr_debug("(%s)\n", __func__);
-
 	dev->addr_len		= IEEE802154_ADDR_LEN;
 	memset(dev->broadcast, 0xff, IEEE802154_ADDR_LEN);
 	dev->type		= ARPHRD_IEEE802154;
@@ -1122,8 +1114,6 @@ static void lowpan_setup(struct net_device *dev)
 
 static int lowpan_validate(struct nlattr *tb[], struct nlattr *data[])
 {
-	pr_debug("(%s)\n", __func__);
-
 	if (tb[IFLA_ADDRESS]) {
 		if (nla_len(tb[IFLA_ADDRESS]) != IEEE802154_ADDR_LEN)
 			return -EINVAL;
@@ -1164,7 +1154,7 @@ static int lowpan_newlink(struct net *src_net, struct net_device *dev,
 	struct net_device *real_dev;
 	struct lowpan_dev_record *entry;
 
-	pr_debug("(%s)\n", __func__);
+	pr_debug("adding new link\n");
 
 	if (!tb[IFLA_LINK])
 		return -EINVAL;
@@ -1259,8 +1249,6 @@ static int __init lowpan_init_module(void)
 {
 	int err = 0;
 
-	pr_debug("(%s)\n", __func__);
-
 	err = lowpan_netlink_init();
 	if (err < 0)
 		goto out;
@@ -1272,8 +1260,6 @@ out:
 
 static void __exit lowpan_cleanup_module(void)
 {
-	pr_debug("(%s)\n", __func__);
-
 	lowpan_netlink_fini();
 
 	dev_remove_pack(&lowpan_packet_type);
-- 
1.7.2.3

Re: [PATCH net-next v3 3/3] 6lowpan: remove excessive argument in pr_debug

From: Joe Perches <joe@perches.com>
Date: 2012-06-09 19:37:43

On Sat, 2012-06-09 at 19:13 +0400, Alexander Smirnov wrote:
Remove excessive __func__ argument in pr_debug function.
Hello Alexander.

trivial comments below:
quoted hunk
diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
quoted hunk
@@ -240,7 +240,7 @@ lowpan_uncompress_addr(struct sk_buff *skb, struct in6_addr *ipaddr,
 		lowpan_uip_ds6_set_addr_iid(ipaddr, lladdr);
 	}
 
-	pr_debug("(%s): uncompressing %d + %d => ", __func__, prefcount,
+	pr_debug("uncompressing %d + %d => ", prefcount,
 								postcount);
It'd be nicer to move the second argument as well.

	pr_debug("uncompressing %d + %d => ", prefcount, postcount);
[]
quoted hunk
@@ -543,7 +541,7 @@ static int lowpan_header_create(struct sk_buff *skb,
 			hc06_ptr += 16;
 		}
 	} else {
-		pr_debug("(%s): destination address is unicast: ", __func__);
+		pr_debug("destination address is unicast: ");
 		/* TODO: context lookup */
 		if (is_addr_link_local(&hdr->daddr)) {
 			pr_debug("destination address is link-local\n");
This doesn't really look correct.  Should this be newline terminated?

[]
quoted hunk
@@ -894,14 +890,14 @@ lowpan_process_data(struct sk_buff *skb)
 	/* check for Multicast Compression */
 	if (iphc1 & LOWPAN_IPHC_M) {
 		if (iphc1 & LOWPAN_IPHC_DAC) {
-			pr_debug("(%s): destination address context-based "
-				 "multicast compression\n", __func__);
+			pr_debug("destination address context-based "
+				 "multicast compression\n");
Please coalesce format strings.

			pr_debug("destination address context-based multicast compression\n");
[]
+			pr_debug("destination address non-context-based"
+				 " multicast compression\n");
etc...

Re: [PATCH net-next v3 3/3] 6lowpan: remove excessive argument in pr_debug

From: Joe Perches <joe@perches.com>
Date: 2012-06-09 19:53:14

On Sat, 2012-06-09 at 12:37 -0700, Joe Perches wrote:
On Sat, 2012-06-09 at 19:13 +0400, Alexander Smirnov wrote:
quoted
Remove excessive __func__ argument in pr_debug function.
[]
quoted
diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
[]
quoted
@@ -543,7 +541,7 @@ static int lowpan_header_create(struct sk_buff *skb,
 			hc06_ptr += 16;
 		}
 	} else {
-		pr_debug("(%s): destination address is unicast: ", __func__);
+		pr_debug("destination address is unicast: ");
 		/* TODO: context lookup */
 		if (is_addr_link_local(&hdr->daddr)) {
 			pr_debug("destination address is link-local\n");
This doesn't really look correct.  Should this be newline terminated?
Instead of:
		pr_debug("destination address is unicast: ");
		/* TODO: context lookup */
		if (is_addr_link_local(&hdr->daddr)) {
			pr_debug("destination address is link-local\n");
			iphc1 |= lowpan_compress_addr_64(&hc06_ptr,
				LOWPAN_IPHC_DAM_BIT, &hdr->daddr, daddr);
		} else {
			pr_debug("using full address\n");
			memcpy(hc06_ptr, &hdr->daddr.s6_addr16[0], 16);
			hc06_ptr += 16;
		}

Perhaps something like the below is better:

		/* TODO: context lookup */
		if (is_addr_link_local(&hdr->daddr)) {
			pr_debug("destination address is unicast and link-local\n");
			iphc1 |= lowpan_compress_addr_64(&hc06_ptr,
				LOWPAN_IPHC_DAM_BIT, &hdr->daddr, daddr);
		} else {
			pr_debug("destination address is unicast: using full address\n");
			memcpy(hc06_ptr, &hdr->daddr.s6_addr16[0], 16);
			hc06_ptr += 16;
		}

[PATCH v2 net-next v3 3/3] 6lowpan: remove excessive argument in pr_debug

From: Alexander Smirnov <hidden>
Date: 2012-06-13 08:40:51

Remove excessive __func__ argument in pr_debug function and some
excessive debug messages.

Signed-off-by: Alexander Smirnov <redacted>
---
 net/ieee802154/6lowpan.c |   94 ++++++++++++++++++----------------------------
 1 files changed, 37 insertions(+), 57 deletions(-)
diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
index b45e229..ad0c226 100644
--- a/net/ieee802154/6lowpan.c
+++ b/net/ieee802154/6lowpan.c
@@ -240,8 +240,7 @@ lowpan_uncompress_addr(struct sk_buff *skb, struct in6_addr *ipaddr,
 		lowpan_uip_ds6_set_addr_iid(ipaddr, lladdr);
 	}
 
-	pr_debug("(%s): uncompressing %d + %d => ", __func__, prefcount,
-								postcount);
+	pr_debug("uncompressing %d + %d => ", prefcount, postcount);
 	lowpan_raw_dump_inline(NULL, NULL, ipaddr->s6_addr, 16);
 
 	return 0;
@@ -252,13 +251,11 @@ lowpan_compress_udp_header(u8 **hc06_ptr, struct sk_buff *skb)
 {
 	struct udphdr *uh = udp_hdr(skb);
 
-	pr_debug("(%s): UDP header compression\n", __func__);
-
 	if (((uh->source & LOWPAN_NHC_UDP_4BIT_MASK) ==
 				LOWPAN_NHC_UDP_4BIT_PORT) &&
 	    ((uh->dest & LOWPAN_NHC_UDP_4BIT_MASK) ==
 				LOWPAN_NHC_UDP_4BIT_PORT)) {
-		pr_debug("(%s): both ports compression to 4 bits\n", __func__);
+		pr_debug("UDP header: both ports compression to 4 bits\n");
 		**hc06_ptr = LOWPAN_NHC_UDP_CS_P_11;
 		**(hc06_ptr + 1) = /* subtraction is faster */
 		   (u8)((uh->dest - LOWPAN_NHC_UDP_4BIT_PORT) +
@@ -266,20 +263,20 @@ lowpan_compress_udp_header(u8 **hc06_ptr, struct sk_buff *skb)
 		*hc06_ptr += 2;
 	} else if ((uh->dest & LOWPAN_NHC_UDP_8BIT_MASK) ==
 			LOWPAN_NHC_UDP_8BIT_PORT) {
-		pr_debug("(%s): remove 8 bits of dest\n", __func__);
+		pr_debug("UDP header: remove 8 bits of dest\n");
 		**hc06_ptr = LOWPAN_NHC_UDP_CS_P_01;
 		memcpy(*hc06_ptr + 1, &uh->source, 2);
 		**(hc06_ptr + 3) = (u8)(uh->dest - LOWPAN_NHC_UDP_8BIT_PORT);
 		*hc06_ptr += 4;
 	} else if ((uh->source & LOWPAN_NHC_UDP_8BIT_MASK) ==
 			LOWPAN_NHC_UDP_8BIT_PORT) {
-		pr_debug("(%s): remove 8 bits of source\n", __func__);
+		pr_debug("UDP header: remove 8 bits of source\n");
 		**hc06_ptr = LOWPAN_NHC_UDP_CS_P_10;
 		memcpy(*hc06_ptr + 1, &uh->dest, 2);
 		**(hc06_ptr + 3) = (u8)(uh->source - LOWPAN_NHC_UDP_8BIT_PORT);
 		*hc06_ptr += 4;
 	} else {
-		pr_debug("(%s): can't compress header\n", __func__);
+		pr_debug("UDP header: can't compress\n");
 		**hc06_ptr = LOWPAN_NHC_UDP_CS_P_00;
 		memcpy(*hc06_ptr + 1, &uh->source, 2);
 		memcpy(*hc06_ptr + 3, &uh->dest, 2);
@@ -323,7 +320,7 @@ lowpan_uncompress_udp_header(struct sk_buff *skb)
 		goto err;
 
 	if ((tmp & LOWPAN_NHC_UDP_MASK) == LOWPAN_NHC_UDP_ID) {
-		pr_debug("(%s): UDP header uncompression\n", __func__);
+		pr_debug("UDP header uncompression\n");
 		switch (tmp & LOWPAN_NHC_UDP_CS_P_11) {
 		case LOWPAN_NHC_UDP_CS_P_00:
 			memcpy(&uh->source, &skb->data[0], 2);
@@ -349,19 +346,19 @@ lowpan_uncompress_udp_header(struct sk_buff *skb)
 			skb_pull(skb, 1);
 			break;
 		default:
-			pr_debug("(%s) ERROR: unknown UDP format\n", __func__);
+			pr_debug("ERROR: unknown UDP format\n");
 			goto err;
 			break;
 		}
 
-		pr_debug("(%s): uncompressed UDP ports: src = %d, dst = %d\n",
-					__func__, uh->source, uh->dest);
+		pr_debug("uncompressed UDP ports: src = %d, dst = %d\n",
+			 uh->source, uh->dest);
 
 		/* copy checksum */
 		memcpy(&uh->check, &skb->data[0], 2);
 		skb_pull(skb, 2);
 	} else {
-		pr_debug("(%s): ERROR: unsupported NH format\n", __func__);
+		pr_debug("ERROR: unsupported NH format\n");
 		goto err;
 	}
 
@@ -394,10 +391,9 @@ static int lowpan_header_create(struct sk_buff *skb,
 	hdr = ipv6_hdr(skb);
 	hc06_ptr = head + 2;
 
-	pr_debug("(%s): IPv6 header dump:\n\tversion = %d\n\tlength  = %d\n"
-		 "\tnexthdr = 0x%02x\n\thop_lim = %d\n", __func__,
-		hdr->version, ntohs(hdr->payload_len), hdr->nexthdr,
-		hdr->hop_limit);
+	pr_debug("IPv6 header dump:\n\tversion = %d\n\tlength  = %d\n"
+		 "\tnexthdr = 0x%02x\n\thop_lim = %d\n", hdr->version,
+		 ntohs(hdr->payload_len), hdr->nexthdr, hdr->hop_limit);
 
 	lowpan_raw_dump_table(__func__, "raw skb network header dump",
 		skb_network_header(skb), sizeof(struct ipv6hdr));
@@ -498,23 +494,22 @@ static int lowpan_header_create(struct sk_buff *skb,
 
 	/* source address compression */
 	if (is_addr_unspecified(&hdr->saddr)) {
-		pr_debug("(%s): source address is unspecified, setting SAC\n",
-								__func__);
+		pr_debug("source address is unspecified, setting SAC\n");
 		iphc1 |= LOWPAN_IPHC_SAC;
 	/* TODO: context lookup */
 	} else if (is_addr_link_local(&hdr->saddr)) {
-		pr_debug("(%s): source address is link-local\n", __func__);
+		pr_debug("source address is link-local\n");
 		iphc1 |= lowpan_compress_addr_64(&hc06_ptr,
 				LOWPAN_IPHC_SAM_BIT, &hdr->saddr, saddr);
 	} else {
-		pr_debug("(%s): send the full source address\n", __func__);
+		pr_debug("send the full source address\n");
 		memcpy(hc06_ptr, &hdr->saddr.s6_addr16[0], 16);
 		hc06_ptr += 16;
 	}
 
 	/* destination address compression */
 	if (is_addr_mcast(&hdr->daddr)) {
-		pr_debug("(%s): destination address is multicast", __func__);
+		pr_debug("destination address is multicast: ");
 		iphc1 |= LOWPAN_IPHC_M;
 		if (lowpan_is_mcast_addr_compressable8(&hdr->daddr)) {
 			pr_debug("compressed to 1 octet\n");
@@ -543,14 +538,13 @@ static int lowpan_header_create(struct sk_buff *skb,
 			hc06_ptr += 16;
 		}
 	} else {
-		pr_debug("(%s): destination address is unicast: ", __func__);
 		/* TODO: context lookup */
 		if (is_addr_link_local(&hdr->daddr)) {
-			pr_debug("destination address is link-local\n");
+			pr_debug("dest address is unicast and link-local\n");
 			iphc1 |= lowpan_compress_addr_64(&hc06_ptr,
 				LOWPAN_IPHC_DAM_BIT, &hdr->daddr, daddr);
 		} else {
-			pr_debug("using full address\n");
+			pr_debug("dest address is unicast: using full one\n");
 			memcpy(hc06_ptr, &hdr->daddr.s6_addr16[0], 16);
 			hc06_ptr += 16;
 		}
@@ -642,8 +636,7 @@ static void lowpan_fragment_timer_expired(unsigned long entry_addr)
 {
 	struct lowpan_fragment *entry = (struct lowpan_fragment *)entry_addr;
 
-	pr_debug("%s: timer expired for frame with tag %d\n", __func__,
-								entry->tag);
+	pr_debug("timer expired for frame with tag %d\n", entry->tag);
 
 	spin_lock(&flist_lock);
 	list_del(&entry->list);
@@ -796,12 +789,11 @@ lowpan_process_data(struct sk_buff *skb)
 	_saddr = mac_cb(skb)->sa.hwaddr;
 	_daddr = mac_cb(skb)->da.hwaddr;
 
-	pr_debug("(%s): iphc0 = %02x, iphc1 = %02x\n", __func__, iphc0, iphc1);
+	pr_debug("iphc0 = %02x, iphc1 = %02x\n", iphc0, iphc1);
 
 	/* another if the CID flag is set */
 	if (iphc1 & LOWPAN_IPHC_CID) {
-		pr_debug("(%s): CID flag is set, increase header with one\n",
-								__func__);
+		pr_debug("CID flag is set, increase header with one\n");
 		if (lowpan_fetch_skb_u8(skb, &num_context))
 			goto drop;
 	}
@@ -866,8 +858,8 @@ lowpan_process_data(struct sk_buff *skb)
 		if (lowpan_fetch_skb_u8(skb, &(hdr.nexthdr)))
 			goto drop;
 
-		pr_debug("(%s): NH flag is set, next header is carried "
-			 "inline: %02x\n", __func__, hdr.nexthdr);
+		pr_debug("NH flag is set, next header carried inline: %02x\n",
+			 hdr.nexthdr);
 	}
 
 	/* Hop Limit */
@@ -882,7 +874,7 @@ lowpan_process_data(struct sk_buff *skb)
 	tmp = ((iphc1 & LOWPAN_IPHC_SAM) >> LOWPAN_IPHC_SAM_BIT) & 0x03;
 
 	/* Source address uncompression */
-	pr_debug("(%s): source address stateless compression\n", __func__);
+	pr_debug("source address stateless compression\n");
 	err = lowpan_uncompress_addr(skb, &hdr.saddr, lowpan_llprefix,
 				lowpan_unc_llconf[tmp], skb->data);
 	if (err)
@@ -894,14 +886,12 @@ lowpan_process_data(struct sk_buff *skb)
 	/* check for Multicast Compression */
 	if (iphc1 & LOWPAN_IPHC_M) {
 		if (iphc1 & LOWPAN_IPHC_DAC) {
-			pr_debug("(%s): destination address context-based "
-				 "multicast compression\n", __func__);
+			pr_debug("dest: context-based mcast compression\n");
 			/* TODO: implement this */
 		} else {
 			u8 prefix[] = {0xff, 0x02};
 
-			pr_debug("(%s): destination address non-context-based"
-				 " multicast compression\n", __func__);
+			pr_debug("dest: non context-based mcast compression\n");
 			if (0 < tmp && tmp < 3) {
 				if (lowpan_fetch_skb_u8(skb, &prefix[1]))
 					goto drop;
@@ -913,8 +903,7 @@ lowpan_process_data(struct sk_buff *skb)
 				goto drop;
 		}
 	} else {
-		pr_debug("(%s): destination address stateless compression\n",
-								__func__);
+		pr_debug("dest: stateless compression\n");
 		err = lowpan_uncompress_addr(skb, &hdr.daddr, lowpan_llprefix,
 				lowpan_unc_llconf[tmp], skb->data);
 		if (err)
@@ -929,11 +918,11 @@ lowpan_process_data(struct sk_buff *skb)
 	/* Not fragmented package */
 	hdr.payload_len = htons(skb->len);
 
-	pr_debug("(%s): skb headroom size = %d, data length = %d\n", __func__,
-						skb_headroom(skb), skb->len);
+	pr_debug("skb headroom size = %d, data length = %d\n",
+		 skb_headroom(skb), skb->len);
 
-	pr_debug("(%s): IPv6 header dump:\n\tversion = %d\n\tlength  = %d\n\t"
-		 "nexthdr = 0x%02x\n\thop_lim = %d\n", __func__, hdr.version,
+	pr_debug("IPv6 header dump:\n\tversion = %d\n\tlength  = %d\n\t"
+		 "nexthdr = 0x%02x\n\thop_lim = %d\n", hdr.version,
 		 ntohs(hdr.payload_len), hdr.nexthdr, hdr.hop_limit);
 
 	lowpan_raw_dump_table(__func__, "raw header dump", (u8 *)&hdr,
@@ -1035,11 +1024,11 @@ static netdev_tx_t lowpan_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	int err = -1;
 
-	pr_debug("(%s): package xmit\n", __func__);
+	pr_debug("package xmit\n");
 
 	skb->dev = lowpan_dev_info(dev)->real_dev;
 	if (skb->dev == NULL) {
-		pr_debug("(%s) ERROR: no real wpan device found\n", __func__);
+		pr_debug("ERROR: no real wpan device found\n");
 		goto error;
 	}
 
@@ -1048,14 +1037,13 @@ static netdev_tx_t lowpan_xmit(struct sk_buff *skb, struct net_device *dev)
 		goto out;
 	}
 
-	pr_debug("(%s): frame is too big, fragmentation is needed\n",
-								__func__);
+	pr_debug("frame is too big, fragmentation is needed\n");
 	err = lowpan_skb_fragmentation(skb);
 error:
 	dev_kfree_skb(skb);
 out:
 	if (err < 0)
-		pr_debug("(%s): ERROR: xmit failed\n", __func__);
+		pr_debug("ERROR: xmit failed\n");
 
 	return (err < 0 ? NETDEV_TX_BUSY : NETDEV_TX_OK);
 }
@@ -1101,8 +1089,6 @@ static struct ieee802154_mlme_ops lowpan_mlme = {
 
 static void lowpan_setup(struct net_device *dev)
 {
-	pr_debug("(%s)\n", __func__);
-
 	dev->addr_len		= IEEE802154_ADDR_LEN;
 	memset(dev->broadcast, 0xff, IEEE802154_ADDR_LEN);
 	dev->type		= ARPHRD_IEEE802154;
@@ -1122,8 +1108,6 @@ static void lowpan_setup(struct net_device *dev)
 
 static int lowpan_validate(struct nlattr *tb[], struct nlattr *data[])
 {
-	pr_debug("(%s)\n", __func__);
-
 	if (tb[IFLA_ADDRESS]) {
 		if (nla_len(tb[IFLA_ADDRESS]) != IEEE802154_ADDR_LEN)
 			return -EINVAL;
@@ -1164,7 +1148,7 @@ static int lowpan_newlink(struct net *src_net, struct net_device *dev,
 	struct net_device *real_dev;
 	struct lowpan_dev_record *entry;
 
-	pr_debug("(%s)\n", __func__);
+	pr_debug("adding new link\n");
 
 	if (!tb[IFLA_LINK])
 		return -EINVAL;
@@ -1259,8 +1243,6 @@ static int __init lowpan_init_module(void)
 {
 	int err = 0;
 
-	pr_debug("(%s)\n", __func__);
-
 	err = lowpan_netlink_init();
 	if (err < 0)
 		goto out;
@@ -1272,8 +1254,6 @@ out:
 
 static void __exit lowpan_cleanup_module(void)
 {
-	pr_debug("(%s)\n", __func__);
-
 	lowpan_netlink_fini();
 
 	dev_remove_pack(&lowpan_packet_type);
-- 
1.7.2.3
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help