[PATCH net-next 0/5] 802154: some cleanups and fixes

[PATCH net-next 0/5] 802154: some cleanups and fixes

[Phoebe Buckheister]

This series adds some definitions for 802.15.4 header fields that were missing,
changes 6lowpan packet size calculation to always be correct and fixes a bug in
802.15.4 dgram sockets where sockets would ignore arguments to sendmsg(). Also,
a few minor changes to mac_cb handling, and mark a single-use function static.

---

Phoebe Buckheister (5):
      ieee802154: add definitions for link-layer security and header functions
      6lowpan: simplify/fix payload length calculation
      ieee802154: change _cb handling slightly
      ieee802154: don't ignore "to" argument in unbound dgram sendmsg
      mac802154: make mac802154_wpan_open static


 include/net/ieee802154.h        |    9 +++++++
 include/net/ieee802154_netdev.h |   54 ++++++++++++++++++++++++++-------------
 net/ieee802154/6lowpan_rtnl.c   |   33 ++++++++++++++----------
 net/ieee802154/dgram.c          |   23 +++++++++++------
 net/ieee802154/header_ops.c     |   52 ++++++++++++++++++++++++++++++++-----
 net/mac802154/rx.c              |    2 --
 net/mac802154/wpan.c            |   28 ++++++++++----------
 7 files changed, 139 insertions(+), 62 deletions(-)

[PATCH net-next 1/5] ieee802154: add definitions for link-layer security and header functions

[Phoebe Buckheister]

When dealing with 802.15.4, one often has to know the maximum payload
size for a given packet. This depends on many factors, one of which is
whether or not a security header is present in the frame. These
definitions and functions provide an easy way for any upper layer to
calculate the maximum payload size for a packet. The first obvious user
for this is 6lowpan, which duplicates this calculation and gets it
partially wrong because it ignores security headers.

Signed-off-by: Phoebe Buckheister <phoebe.buckheister@itwm.fraunhofer.de>
---
 include/net/ieee802154.h        |    9 +++++++
 include/net/ieee802154_netdev.h |   29 ++++++++++++++++++++++
 net/ieee802154/header_ops.c     |   52 +++++++++++++++++++++++++++++++++------
 3 files changed, 83 insertions(+), 7 deletions(-)

diff --git a/include/net/ieee802154.h b/include/net/ieee802154.h
index c7ae0ac..0aa7122 100644
--- a/include/net/ieee802154.h
+++ b/include/net/ieee802154.h
@@ -79,6 +79,15 @@
 #define IEEE802154_SCF_KEY_SHORT_INDEX		2
 #define IEEE802154_SCF_KEY_HW_INDEX		3
 
+#define IEEE802154_SCF_SECLEVEL_NONE		0
+#define IEEE802154_SCF_SECLEVEL_MIC32		1
+#define IEEE802154_SCF_SECLEVEL_MIC64		2
+#define IEEE802154_SCF_SECLEVEL_MIC128		3
+#define IEEE802154_SCF_SECLEVEL_ENC		4
+#define IEEE802154_SCF_SECLEVEL_ENC_MIC32	5
+#define IEEE802154_SCF_SECLEVEL_ENC_MIC64	6
+#define IEEE802154_SCF_SECLEVEL_ENC_MIC128	7
+
 /* MAC footer size */
 #define IEEE802154_MFR_SIZE	2 /* 2 octets */
 
diff --git a/include/net/ieee802154_netdev.h b/include/net/ieee802154_netdev.h
index 5a719ca..6e4d3e1 100644
--- a/include/net/ieee802154_netdev.h
+++ b/include/net/ieee802154_netdev.h
@@ -27,6 +27,7 @@
 #ifndef IEEE802154_NETDEVICE_H
 #define IEEE802154_NETDEVICE_H
 
+#include <net/ieee802154.h>
 #include <net/af_ieee802154.h>
 #include <linux/netdevice.h>
 #include <linux/skbuff.h>
@@ -114,6 +115,34 @@ int ieee802154_hdr_pull(struct sk_buff *skb, struct ieee802154_hdr *hdr);
 int ieee802154_hdr_peek_addrs(const struct sk_buff *skb,
 			      struct ieee802154_hdr *hdr);
 
+/* parses the full 802.15.4 header a given skb and stores them into hdr,
+ * performing pan id decompression and length checks to be suitable for use in
+ * header_ops.parse
+ */
+int ieee802154_hdr_peek(const struct sk_buff *skb, struct ieee802154_hdr *hdr);
+
+int ieee802154_max_payload(const struct ieee802154_hdr *hdr);
+
+static inline int
+ieee802154_sechdr_authtag_len(const struct ieee802154_sechdr *sec)
+{
+	switch (sec->level) {
+	case IEEE802154_SCF_SECLEVEL_MIC32:
+	case IEEE802154_SCF_SECLEVEL_ENC_MIC32:
+		return 4;
+	case IEEE802154_SCF_SECLEVEL_MIC64:
+	case IEEE802154_SCF_SECLEVEL_ENC_MIC64:
+		return 8;
+	case IEEE802154_SCF_SECLEVEL_MIC128:
+	case IEEE802154_SCF_SECLEVEL_ENC_MIC128:
+		return 16;
+	case IEEE802154_SCF_SECLEVEL_NONE:
+	case IEEE802154_SCF_SECLEVEL_ENC:
+	default:
+		return 0;
+	}
+}
+
 static inline int ieee802154_hdr_length(struct sk_buff *skb)
 {
 	struct ieee802154_hdr hdr;
diff --git a/net/ieee802154/header_ops.c b/net/ieee802154/header_ops.c
index bed42a4..c09294e 100644
--- a/net/ieee802154/header_ops.c
+++ b/net/ieee802154/header_ops.c
@@ -195,15 +195,16 @@ ieee802154_hdr_get_sechdr(const u8 *buf, struct ieee802154_sechdr *hdr)
 	return pos;
 }
 
+static int ieee802154_sechdr_lengths[4] = {
+	[IEEE802154_SCF_KEY_IMPLICIT] = 5,
+	[IEEE802154_SCF_KEY_INDEX] = 6,
+	[IEEE802154_SCF_KEY_SHORT_INDEX] = 10,
+	[IEEE802154_SCF_KEY_HW_INDEX] = 14,
+};
+
 static int ieee802154_hdr_sechdr_len(u8 sc)
 {
-	switch (IEEE802154_SCF_KEY_ID_MODE(sc)) {
-	case IEEE802154_SCF_KEY_IMPLICIT: return 5;
-	case IEEE802154_SCF_KEY_INDEX: return 6;
-	case IEEE802154_SCF_KEY_SHORT_INDEX: return 10;
-	case IEEE802154_SCF_KEY_HW_INDEX: return 14;
-	default: return -EINVAL;
-	}
+	return ieee802154_sechdr_lengths[IEEE802154_SCF_KEY_ID_MODE(sc)];
 }
 
 static int ieee802154_hdr_minlen(const struct ieee802154_hdr *hdr)
@@ -285,3 +286,40 @@ ieee802154_hdr_peek_addrs(const struct sk_buff *skb, struct ieee802154_hdr *hdr)
 	return pos;
 }
 EXPORT_SYMBOL_GPL(ieee802154_hdr_peek_addrs);
+
+int
+ieee802154_hdr_peek(const struct sk_buff *skb, struct ieee802154_hdr *hdr)
+{
+	const u8 *buf = skb_mac_header(skb);
+	int pos;
+
+	pos = ieee802154_hdr_peek_addrs(skb, hdr);
+	if (pos < 0)
+		return -EINVAL;
+
+	if (hdr->fc.security_enabled) {
+		u8 key_id_mode = IEEE802154_SCF_KEY_ID_MODE(*(buf + pos));
+		int want = pos + ieee802154_sechdr_lengths[key_id_mode];
+
+		if (buf + want > skb_tail_pointer(skb))
+			return -EINVAL;
+
+		pos += ieee802154_hdr_get_sechdr(buf + pos, &hdr->sec);
+	}
+
+	return pos;
+}
+EXPORT_SYMBOL_GPL(ieee802154_hdr_peek);
+
+int ieee802154_max_payload(const struct ieee802154_hdr *hdr)
+{
+	int hlen = ieee802154_hdr_minlen(hdr);
+
+	if (hdr->fc.security_enabled) {
+		hlen += ieee802154_sechdr_lengths[hdr->sec.key_id_mode] - 1;
+		hlen += ieee802154_sechdr_authtag_len(&hdr->sec);
+	}
+
+	return IEEE802154_MTU - hlen - IEEE802154_MFR_SIZE;
+}
+EXPORT_SYMBOL_GPL(ieee802154_max_payload);
-- 
1.7.9.5

[PATCH net-next 2/5] 6lowpan: simplify/fix payload length calculation

[Phoebe Buckheister]

Instead of accounting for all fields of a packet we can think about,
simply peek the header of a packet we're building and ask ieee802154 how
many bytes of payload space we have. This also accounts for security
headers, which were ignored previously.

Signed-off-by: Phoebe Buckheister <phoebe.buckheister@itwm.fraunhofer.de>
---
 net/ieee802154/6lowpan_rtnl.c |   24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/net/ieee802154/6lowpan_rtnl.c b/net/ieee802154/6lowpan_rtnl.c
index 0f5a69e..3e11b46 100644
--- a/net/ieee802154/6lowpan_rtnl.c
+++ b/net/ieee802154/6lowpan_rtnl.c
@@ -256,6 +256,19 @@ lowpan_fragment_xmit(struct sk_buff *skb, u8 *head,
 	return dev_queue_xmit(frag);
 }
 
+static int lowpan_fragment_plen(struct sk_buff *skb, int extra)
+{
+	struct ieee802154_hdr hdr;
+
+	ieee802154_hdr_peek(skb, &hdr);
+
+	/* calc the nearest payload length(divided to 8) for a fragment with
+	 * a given 802.15.4 hdr and additional lowpan headers which fits into
+	 * IEEE802154_MTU
+	 */
+	return round_down(ieee802154_max_payload(&hdr) - extra, 8);
+}
+
 static int
 lowpan_skb_fragmentation(struct sk_buff *skb, struct net_device *dev)
 {
@@ -277,12 +290,8 @@ lowpan_skb_fragmentation(struct sk_buff *skb, struct net_device *dev)
 	head[1] = dgram_size & 0xff;
 	memcpy(head + 2, &tag, sizeof(tag));
 
-	/* calc the nearest payload length(divided to 8) for first fragment
-	 * which fits into a IEEE802154_MTU
-	 */
-	frag_plen = round_down(IEEE802154_MTU - header_length -
-			       LOWPAN_FRAG1_HEAD_SIZE - lowpan_size -
-			       IEEE802154_MFR_SIZE, 8);
+	frag_plen = lowpan_fragment_plen(skb, LOWPAN_FRAG1_HEAD_SIZE +
+					 lowpan_size);
 
 	err = lowpan_fragment_xmit(skb, head, header_length,
 				   frag_plen + lowpan_size, 0,
@@ -300,8 +309,7 @@ lowpan_skb_fragmentation(struct sk_buff *skb, struct net_device *dev)
 	head[0] &= ~LOWPAN_DISPATCH_FRAG1;
 	head[0] |= LOWPAN_DISPATCH_FRAGN;
 
-	frag_plen = round_down(IEEE802154_MTU - header_length -
-			       LOWPAN_FRAGN_HEAD_SIZE - IEEE802154_MFR_SIZE, 8);
+	frag_plen = lowpan_fragment_plen(skb, LOWPAN_FRAGN_HEAD_SIZE);
 
 	while (payload_length - offset > 0) {
 		int len = frag_plen;
-- 
1.7.9.5

Re: [PATCH net-next 2/5] 6lowpan: simplify/fix payload length calculation

[Alexander Aring]

Hi Phoebe,

sorry I can't test your patches at the moment.

On Tue, May 13, 2014 at 07:24:03PM +0200, Phoebe Buckheister wrote:
> Instead of accounting for all fields of a packet we can think about,
> simply peek the header of a packet we're building and ask ieee802154 how
> many bytes of payload space we have. This also accounts for security
> headers, which were ignored previously.
> 
> Signed-off-by: Phoebe Buckheister <phoebe.buckheister-mPn0NPGs4xGatNDF+KUbs4QuADTiUCJX@public.gmane.org>
> ---
>  net/ieee802154/6lowpan_rtnl.c |   24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/net/ieee802154/6lowpan_rtnl.c b/net/ieee802154/6lowpan_rtnl.c
> index 0f5a69e..3e11b46 100644
> --- a/net/ieee802154/6lowpan_rtnl.c
> +++ b/net/ieee802154/6lowpan_rtnl.c
> @@ -256,6 +256,19 @@ lowpan_fragment_xmit(struct sk_buff *skb, u8 *head,
>  	return dev_queue_xmit(frag);
>  }
>  
> +static int lowpan_fragment_plen(struct sk_buff *skb, int extra)
> +{
> +	struct ieee802154_hdr hdr;
> +
> +	ieee802154_hdr_peek(skb, &hdr);
> +
> +	/* calc the nearest payload length(divided to 8) for a fragment with
> +	 * a given 802.15.4 hdr and additional lowpan headers which fits into
> +	 * IEEE802154_MTU
> +	 */
> +	return round_down(ieee802154_max_payload(&hdr) - extra, 8);
> +}

Is there a change to use the skb_mac_header_length function here instead
of parsing the mac header? I mean we should already known the mac header
size from the wpan header_create function.

I am not sure when you have the information for the security headers...
maybe we don't have this available at the wpan header_create function.
But when this is true we should reset the skb_mac_header then.
This sounds also bad if the mac header is changed in lowpan device. I
don't know how the security stuff workds right now, sorry.


And maybe we should replace the skb->mac_len in the lowpan_skb_fragmentation
function. :-) This we could put on the todo list. For the global 6LoWPAN
cleanup code patch series.

- Alex


------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs

[PATCH net-next 3/5] ieee802154: change _cb handling slightly

[Phoebe Buckheister]

The current mac_cb handling of ieee802154 is rather awkward and limited.
Decompose the single flags fields into multiple fields with the meanings
of each subfield of the flags field to make future extensions (for
example, link-layer security) easier. Also don't set the frame sequence
number in upper layers, since that's a thing the MAC is supposed to set
on frame transmit.

Signed-off-by: Phoebe Buckheister <phoebe.buckheister-mPn0NPGs4xGatNDF+KUbs4QuADTiUCJX@public.gmane.org>
---
 include/net/ieee802154_netdev.h |   25 +++++++------------------
 net/ieee802154/6lowpan_rtnl.c   |    9 ++++-----
 net/ieee802154/dgram.c          |   10 ++++------
 net/mac802154/rx.c              |    2 --
 net/mac802154/wpan.c            |   26 +++++++++++++-------------
 5 files changed, 28 insertions(+), 44 deletions(-)

diff --git a/include/net/ieee802154_netdev.h b/include/net/ieee802154_netdev.h
index 6e4d3e1..ee5e9a1 100644
--- a/include/net/ieee802154_netdev.h
+++ b/include/net/ieee802154_netdev.h
@@ -222,8 +222,9 @@ static inline void ieee802154_addr_to_sa(struct ieee802154_addr_sa *sa,
  */
 struct ieee802154_mac_cb {
 	u8 lqi;
-	u8 flags;
-	u8 seq;
+	u8 type;
+	bool ackreq;
+	bool secen;
 	struct ieee802154_addr source;
 	struct ieee802154_addr dest;
 };
@@ -233,24 +234,12 @@ static inline struct ieee802154_mac_cb *mac_cb(struct sk_buff *skb)
 	return (struct ieee802154_mac_cb *)skb->cb;
 }
 
-#define MAC_CB_FLAG_TYPEMASK		((1 << 3) - 1)
-
-#define MAC_CB_FLAG_ACKREQ		(1 << 3)
-#define MAC_CB_FLAG_SECEN		(1 << 4)
-
-static inline bool mac_cb_is_ackreq(struct sk_buff *skb)
-{
-	return mac_cb(skb)->flags & MAC_CB_FLAG_ACKREQ;
-}
-
-static inline bool mac_cb_is_secen(struct sk_buff *skb)
+static inline struct ieee802154_mac_cb *mac_cb_alloc(struct sk_buff *skb)
 {
-	return mac_cb(skb)->flags & MAC_CB_FLAG_SECEN;
-}
+	BUILD_BUG_ON(sizeof(struct ieee802154_mac_cb) > sizeof(skb->cb));
 
-static inline int mac_cb_type(struct sk_buff *skb)
-{
-	return mac_cb(skb)->flags & MAC_CB_FLAG_TYPEMASK;
+	memset(skb->cb, 0, sizeof(struct ieee802154_mac_cb));
+	return mac_cb(skb);
 }
 
 #define IEEE802154_MAC_SCAN_ED		0
diff --git a/net/ieee802154/6lowpan_rtnl.c b/net/ieee802154/6lowpan_rtnl.c
index 3e11b46..a830b2e 100644
--- a/net/ieee802154/6lowpan_rtnl.c
+++ b/net/ieee802154/6lowpan_rtnl.c
@@ -92,6 +92,7 @@ static int lowpan_header_create(struct sk_buff *skb,
 	const u8 *saddr = _saddr;
 	const u8 *daddr = _daddr;
 	struct ieee802154_addr sa, da;
+	struct ieee802154_mac_cb *cb = mac_cb_alloc(skb);
 
 	/* TODO:
 	 * if this package isn't ipv6 one, where should it be routed?
@@ -115,8 +116,7 @@ static int lowpan_header_create(struct sk_buff *skb,
 	 * from MAC subif of the 'dev' and 'real_dev' network devices, but
 	 * this isn't implemented in mainline yet, so currently we assign 0xff
 	 */
-	mac_cb(skb)->flags = IEEE802154_FC_TYPE_DATA;
-	mac_cb(skb)->seq = ieee802154_mlme_ops(dev)->get_dsn(dev);
+	cb->type = IEEE802154_FC_TYPE_DATA;
 
 	/* prepare wpan address data */
 	sa.mode = IEEE802154_ADDR_LONG;
@@ -135,11 +135,10 @@ static int lowpan_header_create(struct sk_buff *skb,
 	} else {
 		da.mode = IEEE802154_ADDR_LONG;
 		da.extended_addr = ieee802154_devaddr_from_raw(daddr);
-
-		/* request acknowledgment */
-		mac_cb(skb)->flags |= MAC_CB_FLAG_ACKREQ;
 	}
 
+	cb->ackreq = !lowpan_is_addr_broadcast(daddr);
+
 	return dev_hard_header(skb, lowpan_dev_info(dev)->real_dev,
 			type, (void *)&da, (void *)&sa, 0);
 }
diff --git a/net/ieee802154/dgram.c b/net/ieee802154/dgram.c
index 786437b..2fefe12 100644
--- a/net/ieee802154/dgram.c
+++ b/net/ieee802154/dgram.c
@@ -209,6 +209,7 @@ static int dgram_sendmsg(struct kiocb *iocb, struct sock *sk,
 	struct net_device *dev;
 	unsigned int mtu;
 	struct sk_buff *skb;
+	struct ieee802154_mac_cb *cb;
 	struct dgram_sock *ro = dgram_sk(sk);
 	int hlen, tlen;
 	int err;
@@ -249,18 +250,15 @@ static int dgram_sendmsg(struct kiocb *iocb, struct sock *sk,
 
 	skb_reset_network_header(skb);
 
-	mac_cb(skb)->flags = IEEE802154_FC_TYPE_DATA;
-	if (ro->want_ack)
-		mac_cb(skb)->flags |= MAC_CB_FLAG_ACKREQ;
+	cb = mac_cb_alloc(skb);
+	cb->type = IEEE802154_FC_TYPE_DATA;
+	cb->ackreq = ro->want_ack;
 
-	mac_cb(skb)->seq = ieee802154_mlme_ops(dev)->get_dsn(dev);
 	err = dev_hard_header(skb, dev, ETH_P_IEEE802154, &ro->dst_addr,
 			ro->bound ? &ro->src_addr : NULL, size);
 	if (err < 0)
 		goto out_skb;
 
-	skb_reset_mac_header(skb);
-
 	err = memcpy_fromiovec(skb_put(skb, size), msg->msg_iov, size);
 	if (err < 0)
 		goto out_skb;
diff --git a/net/mac802154/rx.c b/net/mac802154/rx.c
index 03855b0..0597b96 100644
--- a/net/mac802154/rx.c
+++ b/net/mac802154/rx.c
@@ -59,8 +59,6 @@ mac802154_subif_rx(struct ieee802154_dev *hw, struct sk_buff *skb, u8 lqi)
 	skb->protocol = htons(ETH_P_IEEE802154);
 	skb_reset_mac_header(skb);
 
-	BUILD_BUG_ON(sizeof(struct ieee802154_mac_cb) > sizeof(skb->cb));
-
 	if (!(priv->hw.flags & IEEE802154_HW_OMIT_CKSUM)) {
 		u16 crc;
 
diff --git a/net/mac802154/wpan.c b/net/mac802154/wpan.c
index 1df7a6a..1477ee9 100644
--- a/net/mac802154/wpan.c
+++ b/net/mac802154/wpan.c
@@ -192,15 +192,17 @@ static int mac802154_header_create(struct sk_buff *skb,
 {
 	struct ieee802154_hdr hdr;
 	struct mac802154_sub_if_data *priv = netdev_priv(dev);
+	struct ieee802154_mac_cb *cb = mac_cb(skb);
 	int hlen;
 
 	if (!daddr)
 		return -EINVAL;
 
 	memset(&hdr.fc, 0, sizeof(hdr.fc));
-	hdr.fc.type = mac_cb_type(skb);
-	hdr.fc.security_enabled = mac_cb_is_secen(skb);
-	hdr.fc.ack_request = mac_cb_is_ackreq(skb);
+	hdr.fc.type = cb->type;
+	hdr.fc.security_enabled = cb->secen;
+	hdr.fc.ack_request = cb->ackreq;
+	hdr.seq = ieee802154_mlme_ops(dev)->get_dsn(dev);
 
 	if (!saddr) {
 		spin_lock_bh(&priv->mib_lock);
@@ -391,12 +393,12 @@ mac802154_subif_frame(struct mac802154_sub_if_data *sdata, struct sk_buff *skb)
 	sdata->dev->stats.rx_packets++;
 	sdata->dev->stats.rx_bytes += skb->len;
 
-	switch (mac_cb_type(skb)) {
+	switch (mac_cb(skb)->type) {
 	case IEEE802154_FC_TYPE_DATA:
 		return mac802154_process_data(sdata->dev, skb);
 	default:
 		pr_warn("ieee802154: bad frame received (type = %d)\n",
-			mac_cb_type(skb));
+			mac_cb(skb)->type);
 		kfree_skb(skb);
 		return NET_RX_DROP;
 	}
@@ -423,6 +425,7 @@ static int mac802154_parse_frame_start(struct sk_buff *skb)
 {
 	int hlen;
 	struct ieee802154_hdr hdr;
+	struct ieee802154_mac_cb *cb = mac_cb_alloc(skb);
 
 	hlen = ieee802154_hdr_pull(skb, &hdr);
 	if (hlen < 0)
@@ -433,18 +436,15 @@ static int mac802154_parse_frame_start(struct sk_buff *skb)
 	pr_debug("fc: %04x dsn: %02x\n", le16_to_cpup((__le16 *)&hdr.fc),
 		 hdr.seq);
 
-	mac_cb(skb)->flags = hdr.fc.type;
-
-	if (hdr.fc.ack_request)
-		mac_cb(skb)->flags |= MAC_CB_FLAG_ACKREQ;
-	if (hdr.fc.security_enabled)
-		mac_cb(skb)->flags |= MAC_CB_FLAG_SECEN;
+	cb->type = hdr.fc.type;
+	cb->ackreq = hdr.fc.ack_request;
+	cb->secen = hdr.fc.security_enabled;
 
 	mac802154_print_addr("destination", &hdr.dest);
 	mac802154_print_addr("source", &hdr.source);
 
-	mac_cb(skb)->source = hdr.source;
-	mac_cb(skb)->dest = hdr.dest;
+	cb->source = hdr.source;
+	cb->dest = hdr.dest;
 
 	if (hdr.fc.security_enabled) {
 		u64 key;
-- 
1.7.9.5


------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs

Re: [Linux-zigbee-devel] [PATCH net-next 3/5] ieee802154: change _cb handling slightly

[Werner Almesberger]

Phoebe Buckheister wrote:
> +static inline struct ieee802154_mac_cb *mac_cb_alloc(struct sk_buff *skb)

That's a rather misleading name for a mere initialization.
How about mac_cb_init ?

The rest of this patch looks nice.

- Werner

[PATCH net-next 4/5] ieee802154: don't ignore "to" argument in unbound dgram sendmsg

[Phoebe Buckheister]

For some reason, unconnected 802.15.4 dgram sockets ignore the destination
argument of sendmsg(), while bound sockets use it. Instead, send packets
to the destination given by the user, and default to the connected
destination only if no explicit target is given.

Signed-off-by: Phoebe Buckheister <phoebe.buckheister-mPn0NPGs4xGatNDF+KUbs4QuADTiUCJX@public.gmane.org>
---
 net/ieee802154/dgram.c |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/net/ieee802154/dgram.c b/net/ieee802154/dgram.c
index 2fefe12..525f18a 100644
--- a/net/ieee802154/dgram.c
+++ b/net/ieee802154/dgram.c
@@ -211,6 +211,7 @@ static int dgram_sendmsg(struct kiocb *iocb, struct sock *sk,
 	struct sk_buff *skb;
 	struct ieee802154_mac_cb *cb;
 	struct dgram_sock *ro = dgram_sk(sk);
+	struct ieee802154_addr dst_addr;
 	int hlen, tlen;
 	int err;
 
@@ -254,8 +255,16 @@ static int dgram_sendmsg(struct kiocb *iocb, struct sock *sk,
 	cb->type = IEEE802154_FC_TYPE_DATA;
 	cb->ackreq = ro->want_ack;
 
-	err = dev_hard_header(skb, dev, ETH_P_IEEE802154, &ro->dst_addr,
-			ro->bound ? &ro->src_addr : NULL, size);
+	if (msg->msg_name) {
+		DECLARE_SOCKADDR(struct sockaddr_ieee802154*, daddr, msg->msg_name);
+
+		ieee802154_addr_from_sa(&dst_addr, &daddr->addr);
+	} else {
+		dst_addr = ro->dst_addr;
+	}
+
+	err = dev_hard_header(skb, dev, ETH_P_IEEE802154, &dst_addr,
+			      ro->bound ? &ro->src_addr : NULL, size);
 	if (err < 0)
 		goto out_skb;
 
-- 
1.7.9.5


------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs

Re: [PATCH net-next 4/5] ieee802154: don't ignore "to" argument in unbound dgram sendmsg

[Werner Almesberger]

Phoebe Buckheister wrote:
> For some reason, unconnected 802.15.4 dgram sockets ignore the destination
> argument of sendmsg(), while bound sockets use it. Instead, send packets
> to the destination given by the user, and default to the connected
> destination only if no explicit target is given.

We discussed this a bit on IRC. Here's what I think happened and what
happens with your patch. Before:

conn'ed	msg_nam	action
-------	-------	------------------------------------------
no	no	send to initialization value (= broadcast)
no	yes	idem
yes	no	send to connected address
yes	yes	idem

After your patch:

conn'ed	msg_nam	action
-------	-------	------------------------------------------
no	no	send to initialization value (= broadcast)
no	yes	send to msg_name
yes	no	send to connected address
yes	yes	send to msg_name

This is certainly more correct. Blurting out a broadcast in the
no/no case isn't nice, though. It would be better to return either
ENOTCONN (POSIX [1]) or (even better, according to GNU libc [2])
EDESTADDRREQ.

POSIX [1] says we may get EISCONN in the yes/yes case and the Linux
man page [3] says we either get EISCONN or the msg_name argument is
ignored. So that also doesn't look quite right yet.

- Werner

[1] http://pubs.opengroup.org/onlinepubs/007904975/
[2] http://www.gnu.org/software/libc/manual/html_node/Error-Codes.html
[3] man sendmsg

------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs

Re: [Linux-zigbee-devel] [PATCH net-next 4/5] ieee802154: don't ignore "to" argument in unbound dgram sendmsg

[Phoebe Buckheister]

On Tue, May 13, 2014 10:13 pm, Werner Almesberger wrote:
> Phoebe Buckheister wrote:
>> For some reason, unconnected 802.15.4 dgram sockets ignore the
>> destination
>> argument of sendmsg(), while bound sockets use it. Instead, send packets
>> to the destination given by the user, and default to the connected
>> destination only if no explicit target is given.
>
> We discussed this a bit on IRC. Here's what I think happened and what
> happens with your patch. Before:
>
> conn'ed	msg_nam	action
> -------	-------	------------------------------------------
> no	no	send to initialization value (= broadcast)
> no	yes	idem
> yes	no	send to connected address
> yes	yes	idem
>
> After your patch:
>
> conn'ed	msg_nam	action
> -------	-------	------------------------------------------
> no	no	send to initialization value (= broadcast)
> no	yes	send to msg_name
> yes	no	send to connected address
> yes	yes	send to msg_name
>
> This is certainly more correct. Blurting out a broadcast in the
> no/no case isn't nice, though. It would be better to return either
> ENOTCONN (POSIX [1]) or (even better, according to GNU libc [2])
> EDESTADDRREQ.
>
> POSIX [1] says we may get EISCONN in the yes/yes case and the Linux
> man page [3] says we either get EISCONN or the msg_name argument is
> ignored. So that also doesn't look quite right yet.

You're completely right. I left the ENOTCONN case out on purpose and
forgot EISCONN, but as you listed, sendmsg() really never worked except in
one special case, and there's a bit of other nastiness going on in those
sockets. I'll revise this patch and add another to fix the rest.

>
> - Werner
>
> [1] http://pubs.opengroup.org/onlinepubs/007904975/
> [2] http://www.gnu.org/software/libc/manual/html_node/Error-Codes.html
> [3] man sendmsg
>

[PATCH net-next 5/5] mac802154: make mac802154_wpan_open static

[Phoebe Buckheister]

This function is only used within the same translation unit, so mark it
static.

Signed-off-by: Phoebe Buckheister <phoebe.buckheister@itwm.fraunhofer.de>
---
 net/mac802154/wpan.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mac802154/wpan.c b/net/mac802154/wpan.c
index 1477ee9..31b60b6 100644
--- a/net/mac802154/wpan.c
+++ b/net/mac802154/wpan.c
@@ -124,7 +124,7 @@ void mac802154_get_mac_params(struct net_device *dev,
 	mutex_unlock(&priv->hw->slaves_mtx);
 }
 
-int mac802154_wpan_open(struct net_device *dev)
+static int mac802154_wpan_open(struct net_device *dev)
 {
 	int rc;
 	struct mac802154_sub_if_data *priv = netdev_priv(dev);
-- 
1.7.9.5