From a8e1620f3c9f2248e90449ecb6dd65d512ff713d Mon Sep 17 00:00:00 2001 From: Anthony Merlino Date: Sat, 10 Aug 2019 18:25:13 +0000 Subject: [PATCH] Merged in antmerlino/nuttx/mac802154_fix_eaddr (pull request #985) mac802154: Fixes issues with extended address. 1) Extended address should be read-only. 2) Extended address should be placed in frame in "reverse-canonical" order. The extended address is a read-only attribute and thus an attempt to write the extended address should be denied. Instead, the extended address should really be either set by the PHY/radio itself, or provided at board bring-up time to the radio layer. The MAC layer now pulls in the extended address from the radio any time the MAC is reset. The extended address is also supposed to be sent in the frame in reverse-canonical order. This is very confusing in the standard and it wasn't until I realized it was backwards in Wireshark that I researched this further. Searching online I find documents from the committee for suggestions/feedback on the future standard. It isn't in the 2015 standard but a newer version of the standard will presumably clarify this. It says that the extended address should be written in reverse-canonical form, meaning the OUI comes last, not first inside the frame. Approved-by: Gregory Nutt --- wireless/ieee802154/Kconfig | 6 ----- wireless/ieee802154/mac802154.c | 11 --------- wireless/ieee802154/mac802154_data.c | 22 +++++++++++++---- wireless/ieee802154/mac802154_getset.c | 4 +++- wireless/ieee802154/mac802154_internal.h | 30 ++++++++++++++---------- wireless/ieee802154/mac802154_reset.c | 9 +++---- 6 files changed, 44 insertions(+), 38 deletions(-) diff --git a/wireless/ieee802154/Kconfig b/wireless/ieee802154/Kconfig index 2b0da5a8161..a18adb17b4e 100644 --- a/wireless/ieee802154/Kconfig +++ b/wireless/ieee802154/Kconfig @@ -49,12 +49,6 @@ config IEEE802154_PRIMITIVE_IRQRESERVE because there are no interrupt level allocations performed by the current IEEE 802.15.4 MAC code. -config IEEE802154_DEFAULT_EADDR - hex "IEEE 802.15.4 Default Extended Address" - default 0x00fade00deadbeef - ---help--- - Set the default extended address to be used by MAC networks on init - config IEEE802154_MAC bool "Software MAC layer" default n diff --git a/wireless/ieee802154/mac802154.c b/wireless/ieee802154/mac802154.c index e781a22f005..a04fd4ccf54 100644 --- a/wireless/ieee802154/mac802154.c +++ b/wireless/ieee802154/mac802154.c @@ -2132,8 +2132,6 @@ MACHANDLE mac802154_create(FAR struct ieee802154_radio_s *radiodev) { FAR struct ieee802154_privmac_s *mac; FAR struct ieee802154_radiocb_s *radiocb; - uint8_t eaddr[IEEE802154_EADDRSIZE]; - int i; /* Allocate object */ @@ -2180,14 +2178,5 @@ MACHANDLE mac802154_create(FAR struct ieee802154_radio_s *radiodev) mac802154_req_reset((MACHANDLE)mac, true); - /* Set the default extended address */ - - for (i = 0; i < IEEE802154_EADDRSIZE; i++) - { - eaddr[i] = (CONFIG_IEEE802154_DEFAULT_EADDR >> (8 * i)) & 0xFF; - } - - mac802154_seteaddr(mac, eaddr); - return (MACHANDLE)mac; } diff --git a/wireless/ieee802154/mac802154_data.c b/wireless/ieee802154/mac802154_data.c index ec40509e205..33cf0152b58 100644 --- a/wireless/ieee802154/mac802154_data.c +++ b/wireless/ieee802154/mac802154_data.c @@ -136,8 +136,15 @@ int mac802154_req_data(MACHANDLE mac, } else if (meta->destaddr.mode == IEEE802154_ADDRMODE_EXTENDED) { - IEEE802154_EADDRCOPY(&frame->io_data[mhr_len], meta->destaddr.eaddr); - mhr_len += IEEE802154_EADDRSIZE; + /* The IEEE 802.15.4 Standard is confusing with regards to byte-order + * for * extended address. More research discovers that the extended + * address should be sent in reverse-canonical form. + */ + + for (int index = IEEE802154_EADDRSIZE - 1; index >= 0; index--) + { + frame->io_data[mhr_len++] = meta->destaddr.eaddr[index]; + } } } @@ -195,8 +202,15 @@ int mac802154_req_data(MACHANDLE mac, } else if (meta->srcmode == IEEE802154_ADDRMODE_EXTENDED) { - IEEE802154_EADDRCOPY(&frame->io_data[mhr_len], priv->addr.eaddr); - mhr_len += IEEE802154_EADDRSIZE; + /* The IEEE 802.15.4 Standard is confusing with regards to byte-order + * for * extended address. More research discovers that the extended + * address should be sent in reverse-canonical form. + */ + + for (int index = IEEE802154_EADDRSIZE - 1; index >= 0; index--) + { + frame->io_data[mhr_len++] = priv->addr.eaddr[index]; + } } } else diff --git a/wireless/ieee802154/mac802154_getset.c b/wireless/ieee802154/mac802154_getset.c index a8d4874c401..6222ddbf589 100644 --- a/wireless/ieee802154/mac802154_getset.c +++ b/wireless/ieee802154/mac802154_getset.c @@ -193,7 +193,9 @@ int mac802154_req_set(MACHANDLE mac, enum ieee802154_attr_e attr, break; case IEEE802154_ATTR_MAC_EADDR: { - mac802154_seteaddr(priv, attrval->mac.eaddr); + /* macExtendedAddress is a read-only attribute */ + + ret = IEEE802154_STATUS_DENIED; } break; case IEEE802154_ATTR_MAC_COORD_SADDR: diff --git a/wireless/ieee802154/mac802154_internal.h b/wireless/ieee802154/mac802154_internal.h index dcdb62776ce..d2b2ecf7068 100644 --- a/wireless/ieee802154/mac802154_internal.h +++ b/wireless/ieee802154/mac802154_internal.h @@ -365,11 +365,18 @@ void mac802154_notify(FAR struct ieee802154_privmac_s *priv, } \ while(0) +/* The IEEE 802.15.4 Standard is confusing with regards to byte-order for + * extended address. More research discovers that the extended address should + * be sent in reverse-canonical form. + */ + #define mac802154_puteaddr(iob, eaddr) \ do \ { \ - IEEE802154_EADDRCOPY(&iob->io_data[iob->io_len], eaddr); \ - iob->io_len += IEEE802154_EADDRSIZE; \ + for (int index = IEEE802154_EADDRSIZE - 1; index >= 0; index--) \ + { \ + iob->io_data[iob->io_len++] = eaddr[index]; \ + } \ } \ while(0) @@ -389,11 +396,18 @@ void mac802154_notify(FAR struct ieee802154_privmac_s *priv, } \ while(0) +/* The IEEE 802.15.4 Standard is confusing with regards to byte-order for + * extended address. More research discovers that the extended address should + * be sent in reverse-canonical form. + */ + #define mac802154_takeeaddr(iob, eaddr) \ do \ { \ - IEEE802154_EADDRCOPY(eaddr, &iob->io_data[iob->io_offset]); \ - iob->io_offset += IEEE802154_EADDRSIZE; \ + for (int index = IEEE802154_EADDRSIZE - 1; index >= 0; index--) \ + { \ + eaddr[index] = iob->io_data[iob->io_offset++]; \ + } \ } \ while(0) @@ -735,14 +749,6 @@ static inline void mac802154_setsaddr(FAR struct ieee802154_privmac_s *priv, (FAR const union ieee802154_attr_u *)saddr); } -static inline void mac802154_seteaddr(FAR struct ieee802154_privmac_s *priv, - const uint8_t *eaddr) -{ - IEEE802154_EADDRCOPY(priv->addr.eaddr, eaddr); - priv->radio->setattr(priv->radio, IEEE802154_ATTR_MAC_EADDR, - (FAR const union ieee802154_attr_u *)eaddr); -} - static inline void mac802154_setcoordsaddr(FAR struct ieee802154_privmac_s *priv, const uint8_t *saddr) { diff --git a/wireless/ieee802154/mac802154_reset.c b/wireless/ieee802154/mac802154_reset.c index f48c6bfe986..7e049b2d9c8 100644 --- a/wireless/ieee802154/mac802154_reset.c +++ b/wireless/ieee802154/mac802154_reset.c @@ -116,13 +116,11 @@ int mac802154_req_reset(MACHANDLE mac, bool resetattr) priv->trans_persisttime = 0x01F4; /* Reset the short address and PAN ID. The extended address does not - * get reset but must be set at the radio layer again. The only time the - * MAC layer sets the extended address internally is immediately after - * this function is called in mac802154_create() + * get reset. It is a read-only attribute and the radio driver should + * be in charge of managing it. We pull a local copy for us to use below. */ priv->addr.mode = IEEE802154_ADDRMODE_EXTENDED; - mac802154_seteaddr(priv, priv->addr.eaddr); mac802154_setsaddr(priv, IEEE802154_SADDR_UNSPEC); mac802154_setpanid(priv, IEEE802154_PANID_UNSPEC); @@ -137,6 +135,9 @@ int mac802154_req_reset(MACHANDLE mac, bool resetattr) * reset. */ + priv->radio->getattr(priv->radio, IEEE802154_ATTR_MAC_EADDR, &attr); + IEEE802154_EADDRCOPY(priv->addr.eaddr, attr.mac.eaddr); + priv->radio->getattr(priv->radio, IEEE802154_ATTR_MAC_MAX_FRAME_WAITTIME, &attr); priv->max_frame_waittime = attr.mac.max_frame_waittime;