diff mbox

[net-next,2/8] ieee802154: add address struct with proper endiannes and some operations

Message ID 1394810613-5657-3-git-send-email-phoebe.buckheister@itwm.fraunhofer.de
State Superseded, archived
Delegated to: David Miller
Headers show

Commit Message

Phoebe Buckheister March 14, 2014, 3:23 p.m. UTC
Add a replacement ieee802154_addr struct with proper endianness on
fields. Short address fields are stored as __le16 as on the network,
extended (EUI64) addresses are __le64 as opposed to the u8[8] format
used previously. This disconnect with the netdev address, which is
stored as big-endian u8[8], is intentional.

Signed-off-by: Phoebe Buckheister <phoebe.buckheister@itwm.fraunhofer.de>
---
 include/net/ieee802154_netdev.h |   72 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

Comments

David Laight March 14, 2014, 4 p.m. UTC | #1
From: Phoebe Buckheister
> Add a replacement ieee802154_addr struct with proper endianness on
> fields. Short address fields are stored as __le16 as on the network,
> extended (EUI64) addresses are __le64 as opposed to the u8[8] format
> used previously. This disconnect with the netdev address, which is
> stored as big-endian u8[8], is intentional.
...
> +struct ieee802154_addr {
> +	u8 mode;
> +	__le16 pan_id;
> +	union {
> +		__le16 short_addr;
> +		__le64 extended_addr;
> +	};
> +};

There is a lot of padding in there - especially on 64bit systems.
You didn't make it clear where the above is used, but if it is
passed to userspace I'd add explicit padding fields to ensure
that the alignment is the same on all (sensible) architectures.

	David



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Phoebe Buckheister March 14, 2014, 4:30 p.m. UTC | #2
On Fri, 14 Mar 2014 16:00:39 +0000
David Laight <David.Laight@ACULAB.COM> wrote:

> From: Phoebe Buckheister
> > Add a replacement ieee802154_addr struct with proper endianness on
> > fields. Short address fields are stored as __le16 as on the network,
> > extended (EUI64) addresses are __le64 as opposed to the u8[8] format
> > used previously. This disconnect with the netdev address, which is
> > stored as big-endian u8[8], is intentional.
> ...
> > +struct ieee802154_addr {
> > +	u8 mode;
> > +	__le16 pan_id;
> > +	union {
> > +		__le16 short_addr;
> > +		__le64 extended_addr;
> > +	};
> > +};
> 
> There is a lot of padding in there - especially on 64bit systems.
> You didn't make it clear where the above is used, but if it is
> passed to userspace I'd add explicit padding fields to ensure
> that the alignment is the same on all (sensible) architectures

Yes, there is. The intention for this struct was to be used only within
the stack, not to be exported to userspace - the original
ieee802154_addr (now *_sa) should be used for that for consistency. If
there was a way to fix endianness exported through visible kernel
interfaces, this struct should indeed not be used. Since I'm not sure
whether that's possible, I haven't added padding fields yet.

As explained in the cover letter, I do think we could change that, and
if we ever wanted to, now would be the time to do it. If we did, I'd
lay out the struct as u8 mode, u8 __pad1, __le16 pan_id, u32 __pad2,
and then the union.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/net/ieee802154_netdev.h b/include/net/ieee802154_netdev.h
index 53937cd..86d5d50 100644
--- a/include/net/ieee802154_netdev.h
+++ b/include/net/ieee802154_netdev.h
@@ -29,6 +29,78 @@ 
 
 #include <net/af_ieee802154.h>
 
+struct ieee802154_addr {
+	u8 mode;
+	__le16 pan_id;
+	union {
+		__le16 short_addr;
+		__le64 extended_addr;
+	};
+};
+
+static inline bool ieee802154_addr_equal(const struct ieee802154_addr *a1,
+					 const struct ieee802154_addr *a2)
+{
+	if (a1->pan_id != a2->pan_id || a1->mode != a2->mode)
+		return false;
+
+	if ((a1->mode == IEEE802154_ADDR_LONG &&
+	     a1->extended_addr != a2->extended_addr) ||
+	    (a1->mode == IEEE802154_ADDR_SHORT &&
+	     a1->short_addr != a2->short_addr))
+		return false;
+
+	return true;
+}
+
+static inline __le64 ieee802154_devaddr_from_raw(const void *raw)
+{
+	u64 temp;
+
+	memcpy(&temp, raw, IEEE802154_ADDR_LEN);
+	return (__force __le64)swab64(temp);
+}
+
+static inline void ieee802154_devaddr_to_raw(void *raw, __le64 addr)
+{
+	u64 temp = swab64((__force u64)addr);
+
+	memcpy(raw, &temp, IEEE802154_ADDR_LEN);
+}
+
+static inline void ieee802154_addr_from_sa(struct ieee802154_addr *a,
+					   const struct ieee802154_addr_sa *sa)
+{
+	a->mode = sa->addr_type;
+	a->pan_id = cpu_to_le16(sa->pan_id);
+
+	switch (a->mode) {
+	case IEEE802154_ADDR_SHORT:
+		a->short_addr = cpu_to_le16(sa->short_addr);
+		break;
+	case IEEE802154_ADDR_LONG:
+		a->extended_addr = ieee802154_devaddr_from_raw(sa->hwaddr);
+		break;
+	}
+}
+
+static inline void ieee802154_addr_to_sa(struct ieee802154_addr_sa *sa,
+					 const struct ieee802154_addr *a)
+{
+	sa->addr_type = a->mode;
+	sa->pan_id = le16_to_cpu(a->pan_id);
+
+	switch (a->mode) {
+	case IEEE802154_ADDR_SHORT:
+		sa->short_addr = le16_to_cpu(a->short_addr);
+		break;
+	case IEEE802154_ADDR_LONG:
+		ieee802154_devaddr_to_raw(sa->hwaddr, a->extended_addr);
+		break;
+	}
+}
+
+
 struct ieee802154_frag_info {
 	__be16 d_tag;
 	u16 d_size;