From e1c24084243f5759fb47d3b33b8b929852f99b6a Mon Sep 17 00:00:00 2001 From: Florian Grandel Date: Sun, 18 Sep 2022 13:47:05 +0200 Subject: [PATCH 01/12] net: ip: fix minor formatting glitch This changes just fixes an erroneous indentation from a previous patch. Signed-off-by: Florian Grandel --- subsys/net/ip/connection.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/subsys/net/ip/connection.c b/subsys/net/ip/connection.c index 9cd4eafd872da..454543a96905f 100644 --- a/subsys/net/ip/connection.c +++ b/subsys/net/ip/connection.c @@ -760,8 +760,7 @@ enum net_verdict net_conn_input(struct net_pkt *pkt, } else if (IS_ENABLED(CONFIG_NET_SOCKETS_CAN) && conn_family == AF_CAN) { best_match = conn; } - /* loop end */ - } + } /* loop end */ if (IS_ENABLED(CONFIG_NET_SOCKETS_PACKET) && pkt_family == AF_PACKET) { if (raw_pkt_continue) { From 60910702b9b2f6bf5604fc6b41c318ae61219292 Mon Sep 17 00:00:00 2001 From: Florian Grandel Date: Sat, 1 Oct 2022 10:34:27 +0200 Subject: [PATCH 02/12] drivers: ieee802154: document endianness The IEEE 802.15.4 radio driver encodes attributes in: * little endian for everything that is close to the protocol as IEEE 802.15.4 frames are little endian encoded. * mixed big and little endian in its configuration where extended addresses are being represented. These inconsistencies are unfortunate but cannot be easily fixed in a backwards compatible way so will be left untouched in this change. Endianness was almost nowhere documented which explains these inconsistencies and led to several bugs where assignments of different byte order are not converted (or sometimes converted, sometimes not). This change documents intended endianness within the realm of the IEEE 802.15.4 radio driver code. Conversion bugs are fixed in a separate commit. Signed-off-by: Florian Grandel --- drivers/ieee802154/ieee802154_nrf5.c | 5 ++-- include/zephyr/net/ieee802154_radio.h | 34 ++++++++++++++++----------- 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/drivers/ieee802154/ieee802154_nrf5.c b/drivers/ieee802154/ieee802154_nrf5.c index ae07aa292077c..580ed7d518d98 100644 --- a/drivers/ieee802154/ieee802154_nrf5.c +++ b/drivers/ieee802154/ieee802154_nrf5.c @@ -940,9 +940,10 @@ static int nrf5_configure(const struct device *dev, sys_put_le16(config->ack_ie.short_addr, short_addr_le); /** * The extended address field passed to this function starts - * with the leftmost octet and ends with the rightmost octet. + * with the most significant octet and ends with the least + * significant octet (i.e. big endian byte order). * The IEEE 802.15.4 transmission order mandates this order to be - * reversed in a transmitted frame. + * reversed (i.e. little endian byte order) in a transmitted frame. * * The nrf_802154_ack_data_set expects extended address in transmission * order. diff --git a/include/zephyr/net/ieee802154_radio.h b/include/zephyr/net/ieee802154_radio.h index 925ae488d0751..ea5f7cd8bc723 100644 --- a/include/zephyr/net/ieee802154_radio.h +++ b/include/zephyr/net/ieee802154_radio.h @@ -90,9 +90,9 @@ typedef void (*ieee802154_event_cb_t)(const struct device *dev, struct ieee802154_filter { /** @cond ignore */ union { - uint8_t *ieee_addr; - uint16_t short_addr; - uint16_t pan_id; + uint8_t *ieee_addr; /* in little endian */ + uint16_t short_addr; /* in CPU byte order */ + uint16_t pan_id; /* in CPU byte order */ }; /* @endcond */ }; @@ -142,7 +142,7 @@ enum ieee802154_config_type { * determine whether to set the bit or not based on the information * provided with ``IEEE802154_CONFIG_ACK_FPB`` config and FPB address * matching mode specified. Otherwise, Frame Pending bit should be set - * to ``1``(see IEEE Std 802.15.4-2006, 7.2.2.3.1). + * to ``1`` (see IEEE Std 802.15.4-2006, 7.2.2.3.1). */ IEEE802154_CONFIG_AUTO_ACK_FPB, @@ -233,7 +233,7 @@ struct ieee802154_config { /** ``IEEE802154_CONFIG_ACK_FPB`` */ struct { - uint8_t *addr; + uint8_t *addr; /* in little endian for both, short and extended address */ bool extended; bool enabled; } ack_fpb; @@ -271,24 +271,30 @@ struct ieee802154_config { } rx_slot; /** ``IEEE802154_CONFIG_CSL_PERIOD`` */ - uint32_t csl_period; + uint32_t csl_period; /* in CPU byte order */ /** ``IEEE802154_CONFIG_CSL_RX_TIME`` */ - uint32_t csl_rx_time; + uint32_t csl_rx_time; /* in microseconds, + * based on ieee802154_radio_api.get_time() + */ /** ``IEEE802154_CONFIG_ENH_ACK_HEADER_IE`` */ struct { - const uint8_t *data; + const uint8_t *data; /* header IEs to be added to the Enh-Ack frame in + * little endian byte order + */ uint16_t data_len; - uint16_t short_addr; + uint16_t short_addr; /* in CPU byte order */ /** * The extended address is expected to be passed starting - * with the leftmost octet and ending with the rightmost octet. + * with the most significant octet and ending with the + * least significant octet. * A device with an extended address 01:23:45:67:89:ab:cd:ef - * should provide a pointer to array containing values in the - * same exact order. + * as written in the usual big-endian hex notation should + * provide a pointer to an array containing values in the + * exact same order. */ - const uint8_t *ext_addr; + const uint8_t *ext_addr; /* in big endian */ } ack_ie; }; }; @@ -312,7 +318,7 @@ struct ieee802154_radio_api { /** Clear Channel Assessment - Check channel's activity */ int (*cca)(const struct device *dev); - /** Set current channel */ + /** Set current channel, channel is in CPU byte order. */ int (*set_channel)(const struct device *dev, uint16_t channel); /** Set/Unset filters (for IEEE802154_HW_FILTER ) */ From df984228857b443be528fcf3bf6a976743e8f110 Mon Sep 17 00:00:00 2001 From: Florian Grandel Date: Sat, 1 Oct 2022 10:35:50 +0200 Subject: [PATCH 03/12] net: ip: document endianness To maintain POSIX compatibility, link-layer addresses are encoded in big endian in the IP stack and socket API. The intended endianness was however not documented everywhere which led to bugs and inconsistencies. The IEEE 802.15.4 L2 stack, for example, sometimes stores addresses in little endian in structures that intend them to be stored in POSIX-compliant big endian byte order. This change documents intended endianness within the realm of the IP and sockets stack. Conversion bugs are fixed in a separate commit. Signed-off-by: Florian Grandel --- include/zephyr/net/net_ip.h | 30 +++++++++++++++--------------- include/zephyr/net/net_linkaddr.h | 4 ++-- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/include/zephyr/net/net_ip.h b/include/zephyr/net/net_ip.h index efa7137c067f4..d2e97ae312788 100644 --- a/include/zephyr/net/net_ip.h +++ b/include/zephyr/net/net_ip.h @@ -201,23 +201,23 @@ struct sockaddr_in_ptr { /** Socket address struct for packet socket. */ struct sockaddr_ll { - sa_family_t sll_family; /* Always AF_PACKET */ - uint16_t sll_protocol; /* Physical-layer protocol */ - int sll_ifindex; /* Interface number */ - uint16_t sll_hatype; /* ARP hardware type */ - uint8_t sll_pkttype; /* Packet type */ - uint8_t sll_halen; /* Length of address */ - uint8_t sll_addr[8]; /* Physical-layer address */ + sa_family_t sll_family; /* Always AF_PACKET */ + uint16_t sll_protocol; /* Physical-layer protocol */ + int sll_ifindex; /* Interface number */ + uint16_t sll_hatype; /* ARP hardware type */ + uint8_t sll_pkttype; /* Packet type */ + uint8_t sll_halen; /* Length of address */ + uint8_t sll_addr[8]; /* Physical-layer address, big endian */ }; struct sockaddr_ll_ptr { - sa_family_t sll_family; /* Always AF_PACKET */ - uint16_t sll_protocol; /* Physical-layer protocol */ - int sll_ifindex; /* Interface number */ - uint16_t sll_hatype; /* ARP hardware type */ - uint8_t sll_pkttype; /* Packet type */ - uint8_t sll_halen; /* Length of address */ - uint8_t *sll_addr; /* Physical-layer address */ + sa_family_t sll_family; /* Always AF_PACKET */ + uint16_t sll_protocol; /* Physical-layer protocol */ + int sll_ifindex; /* Interface number */ + uint16_t sll_hatype; /* ARP hardware type */ + uint8_t sll_pkttype; /* Packet type */ + uint8_t sll_halen; /* Length of address */ + uint8_t *sll_addr; /* Physical-layer address, big endian */ }; struct sockaddr_can_ptr { @@ -233,7 +233,7 @@ struct iovec { #endif struct msghdr { - void *msg_name; /* optional socket address */ + void *msg_name; /* optional socket address, big endian */ socklen_t msg_namelen; /* size of socket address */ struct iovec *msg_iov; /* scatter/gather array */ size_t msg_iovlen; /* number of elements in msg_iov */ diff --git a/include/zephyr/net/net_linkaddr.h b/include/zephyr/net/net_linkaddr.h index ffa605b1939b8..9e21e7dc9f809 100644 --- a/include/zephyr/net/net_linkaddr.h +++ b/include/zephyr/net/net_linkaddr.h @@ -66,7 +66,7 @@ enum net_link_type { */ struct net_linkaddr { /** The array of byte representing the address */ - uint8_t *addr; + uint8_t *addr; /* in big endian */ /** Length of that address array */ uint8_t len; @@ -93,7 +93,7 @@ struct net_linkaddr_storage { uint8_t len; /** The array of bytes representing the address */ - uint8_t addr[NET_LINK_ADDR_MAX_LENGTH]; + uint8_t addr[NET_LINK_ADDR_MAX_LENGTH]; /* in big endian */ }; /** From c739c837d3727ee36cac7caf079a1114bacd0da9 Mon Sep 17 00:00:00 2001 From: Florian Grandel Date: Sat, 1 Oct 2022 10:39:12 +0200 Subject: [PATCH 04/12] net: l2: ieee802154: document endianness The IEEE 802.15.4 L2 code stores representation of attributes like PAN id, short address and extended address in different encodings: * big endian for extended address and CPU byte order for everything else whenever such attributes enter user space (except for IP/socket link layer addresses which are always big endian - even in case of short addresses - to maintain POSIX compatibility). * little endian for everything that is close to the radio driver as IEEE 802.15.4 frames are little endian encoded. Endianness was almost nowhere documented which led to several bugs and inconsistencies where assignments of different byte order were not converted (or sometimes converted, sometimes not). This change documents endianness wherever possible within the realm of the IEEE 802.15.4 L2 code. Conversion bugs and inconsistencies that were revealed by the improved documentation will be fixed in a separate commit. Signed-off-by: Florian Grandel --- include/zephyr/net/ieee802154.h | 10 ++++---- include/zephyr/net/ieee802154_mgmt.h | 24 +++++++++---------- subsys/net/l2/ieee802154/ieee802154.c | 4 ++-- subsys/net/l2/ieee802154/ieee802154_frame.h | 16 ++++++++----- .../net/l2/ieee802154/ieee802154_security.h | 23 ++++++++++++++++++ subsys/net/l2/ieee802154/ieee802154_shell.c | 15 ++++++++---- subsys/net/l2/ieee802154/ieee802154_utils.h | 9 +++++-- 7 files changed, 70 insertions(+), 31 deletions(-) diff --git a/include/zephyr/net/ieee802154.h b/include/zephyr/net/ieee802154.h index fca43dbe8210c..89b1f98c3afaa 100644 --- a/include/zephyr/net/ieee802154.h +++ b/include/zephyr/net/ieee802154.h @@ -58,11 +58,11 @@ struct ieee802154_security_ctx { /* This not meant to be used by any code but 802.15.4 L2 stack */ struct ieee802154_context { enum net_l2_flags flags; - uint16_t pan_id; + uint16_t pan_id; /* in CPU byte order */ uint16_t channel; struct k_sem ack_lock; - uint16_t short_addr; - uint8_t ext_addr[IEEE802154_MAX_ADDR_LENGTH]; + uint16_t short_addr; /* in CPU byte order */ + uint8_t ext_addr[IEEE802154_MAX_ADDR_LENGTH]; /* in little endian */ #ifdef CONFIG_NET_L2_IEEE802154_MGMT struct ieee802154_req_params *scan_ctx; union { @@ -70,8 +70,8 @@ struct ieee802154_context { struct k_sem req_lock; }; union { - uint8_t ext_addr[IEEE802154_MAX_ADDR_LENGTH]; - uint16_t short_addr; + uint8_t ext_addr[IEEE802154_MAX_ADDR_LENGTH]; /* in little endian */ + uint16_t short_addr; /* in CPU byte order */ } coord; uint8_t coord_addr_len; #endif diff --git a/include/zephyr/net/ieee802154_mgmt.h b/include/zephyr/net/ieee802154_mgmt.h index 04abb7776770b..6d49dcb5f4635 100644 --- a/include/zephyr/net/ieee802154_mgmt.h +++ b/include/zephyr/net/ieee802154_mgmt.h @@ -42,14 +42,14 @@ enum net_request_ieee802154_cmd { NET_REQUEST_IEEE802154_CMD_CANCEL_SCAN, NET_REQUEST_IEEE802154_CMD_ASSOCIATE, NET_REQUEST_IEEE802154_CMD_DISASSOCIATE, - NET_REQUEST_IEEE802154_CMD_SET_CHANNEL, - NET_REQUEST_IEEE802154_CMD_GET_CHANNEL, - NET_REQUEST_IEEE802154_CMD_SET_PAN_ID, - NET_REQUEST_IEEE802154_CMD_GET_PAN_ID, - NET_REQUEST_IEEE802154_CMD_SET_EXT_ADDR, - NET_REQUEST_IEEE802154_CMD_GET_EXT_ADDR, - NET_REQUEST_IEEE802154_CMD_SET_SHORT_ADDR, - NET_REQUEST_IEEE802154_CMD_GET_SHORT_ADDR, + NET_REQUEST_IEEE802154_CMD_SET_CHANNEL, /* in CPU byte order */ + NET_REQUEST_IEEE802154_CMD_GET_CHANNEL, /* in CPU byte order */ + NET_REQUEST_IEEE802154_CMD_SET_PAN_ID, /* in CPU byte order */ + NET_REQUEST_IEEE802154_CMD_GET_PAN_ID, /* in CPU byte order */ + NET_REQUEST_IEEE802154_CMD_SET_EXT_ADDR, /* in big endian byte order */ + NET_REQUEST_IEEE802154_CMD_GET_EXT_ADDR, /* in big endian byte order */ + NET_REQUEST_IEEE802154_CMD_SET_SHORT_ADDR, /* in CPU byte order */ + NET_REQUEST_IEEE802154_CMD_GET_SHORT_ADDR, /* in CPU byte order */ NET_REQUEST_IEEE802154_CMD_GET_TX_POWER, NET_REQUEST_IEEE802154_CMD_SET_TX_POWER, NET_REQUEST_IEEE802154_CMD_SET_SECURITY_SETTINGS, @@ -191,14 +191,14 @@ struct ieee802154_req_params { uint32_t duration; /** Current channel in use as a result */ - uint16_t channel; + uint16_t channel; /* in CPU byte order */ /** Current pan_id in use as a result */ - uint16_t pan_id; + uint16_t pan_id; /* in CPU byte order */ /** Result address */ union { - uint8_t addr[IEEE802154_MAX_ADDR_LENGTH]; - uint16_t short_addr; + uint8_t addr[IEEE802154_MAX_ADDR_LENGTH]; /* in big endian */ + uint16_t short_addr; /* in CPU byte order */ }; /** length of address */ diff --git a/subsys/net/l2/ieee802154/ieee802154.c b/subsys/net/l2/ieee802154/ieee802154.c index 65392b1ca746d..68f6448bf0851 100644 --- a/subsys/net/l2/ieee802154/ieee802154.c +++ b/subsys/net/l2/ieee802154/ieee802154.c @@ -384,9 +384,9 @@ NET_L2_INIT(IEEE802154_L2, ieee802154_recv, ieee802154_send, ieee802154_enable, void ieee802154_init(struct net_if *iface) { struct ieee802154_context *ctx = net_if_l2_data(iface); - const uint8_t *mac = net_if_get_link_addr(iface)->addr; + const uint8_t *mac = net_if_get_link_addr(iface)->addr; /* in big endian */ int16_t tx_power = CONFIG_NET_L2_IEEE802154_RADIO_DFLT_TX_POWER; - uint8_t long_addr[8]; + uint8_t long_addr[8]; /* in little endian */ NET_DBG("Initializing IEEE 802.15.4 stack on iface %p", iface); diff --git a/subsys/net/l2/ieee802154/ieee802154_frame.h b/subsys/net/l2/ieee802154/ieee802154_frame.h index 20eabe50dcf88..dc37f47990fe0 100644 --- a/subsys/net/l2/ieee802154/ieee802154_frame.h +++ b/subsys/net/l2/ieee802154/ieee802154_frame.h @@ -21,6 +21,10 @@ /* All specification references in this file refer to IEEE 802.15.4-2006 * unless otherwise noted. + * + * Note: All structs and attributes (e.g. PAN id, ext address and short + * address) in this file that directly represent IEEE 802.15.4 frames + * are in LITTLE ENDIAN, see section 4, especially section 4.3. */ #define IEEE802154_MIN_LENGTH 3 @@ -383,7 +387,7 @@ struct ieee802154_cmd_coord_realign { uint16_t coordinator_short_addr; uint8_t channel; uint16_t short_addr; - uint8_t channel_page; /* Optional */ + uint8_t channel_page; /* optional */ } __packed; #define IEEE802154_CMD_COORD_REALIGN_LENGTH 3 @@ -453,16 +457,16 @@ struct ieee802154_mpdu { struct ieee802154_frame_params { struct { union { - uint8_t *ext_addr; - uint16_t short_addr; + uint8_t *ext_addr; /* in big endian */ + uint16_t short_addr; /* in CPU byte order */ }; uint16_t len; - uint16_t pan_id; + uint16_t pan_id; /* in CPU byte order */ } dst; - uint16_t short_addr; - uint16_t pan_id; + uint16_t short_addr; /* in CPU byte order */ + uint16_t pan_id; /* in CPU byte order */ } __packed; #ifdef CONFIG_NET_L2_IEEE802154_SECURITY diff --git a/subsys/net/l2/ieee802154/ieee802154_security.h b/subsys/net/l2/ieee802154/ieee802154_security.h index 61a81ef8fec4c..dfcc5fce8ae36 100644 --- a/subsys/net/l2/ieee802154/ieee802154_security.h +++ b/subsys/net/l2/ieee802154/ieee802154_security.h @@ -18,10 +18,33 @@ int ieee802154_security_setup_session(struct ieee802154_security_ctx *sec_ctx, uint8_t level, uint8_t key_mode, uint8_t *key, uint8_t key_len); +/** + * @brief Decrypt an authenticated payload. + * + * @param sec_ctx Pointer to an IEEE 802.15.4 security context. + * @param frame Pointer to the frame data in original (little endian) byte order. + * @param hdr_len Length of the MHR. + * @param payload_len Length of the MAC payload. + * @param tag_size Length of the authentication tag. + * @param src_ext_addr Pointer to the extended source address of the frame (in little endian byte + * order). + * @param frame_counter Frame counter in CPU byte order. + */ bool ieee802154_decrypt_auth(struct ieee802154_security_ctx *sec_ctx, uint8_t *frame, uint8_t hdr_len, uint8_t payload_len, uint8_t tag_size, uint8_t *src_ext_addr, uint32_t frame_counter); +/** + * @brief Encrypt an authenticated payload. + * + * @param sec_ctx Pointer to an IEEE 802.15.4 security context. + * @param frame Pointer to the frame data in original (little endian) byte order. + * @param hdr_len Length of the MHR. + * @param payload_len Length of the MAC payload. + * @param tag_size Length of the authentication tag. + * @param src_ext_addr Pointer to the extended source address of the frame (in little endian byte + * order). + */ bool ieee802154_encrypt_auth(struct ieee802154_security_ctx *sec_ctx, uint8_t *frame, uint8_t hdr_len, uint8_t payload_len, uint8_t tag_size, uint8_t *src_ext_addr); diff --git a/subsys/net/l2/ieee802154/ieee802154_shell.c b/subsys/net/l2/ieee802154/ieee802154_shell.c index d15652ab26e0f..77648a0a0bf40 100644 --- a/subsys/net/l2/ieee802154/ieee802154_shell.c +++ b/subsys/net/l2/ieee802154/ieee802154_shell.c @@ -60,6 +60,13 @@ static int cmd_ieee802154_ack(const struct shell *shell, return 0; } +/** + * Parse a string representing an extended address in ASCII HEX + * format into a big endian binary representation of the address. + * + * @param addr Extended address as a string. + * @param ext_addr Extended address in big endian byte ordering. + */ static inline void parse_extended_address(char *addr, uint8_t *ext_addr) { net_bytes_from_str(ext_addr, IEEE802154_EXT_ADDR_LENGTH, addr); @@ -401,7 +408,7 @@ static int cmd_ieee802154_set_ext_addr(const struct shell *shell, size_t argc, char *argv[]) { struct net_if *iface = net_if_get_ieee802154(); - uint8_t addr[IEEE802154_EXT_ADDR_LENGTH]; + uint8_t addr[IEEE802154_EXT_ADDR_LENGTH]; /* in big endian */ if (argc < 2) { shell_help(shell); @@ -440,7 +447,7 @@ static int cmd_ieee802154_get_ext_addr(const struct shell *shell, size_t argc, char *argv[]) { struct net_if *iface = net_if_get_ieee802154(); - uint8_t addr[IEEE802154_EXT_ADDR_LENGTH]; + uint8_t addr[IEEE802154_EXT_ADDR_LENGTH]; /* in big endian */ if (!iface) { shell_fprintf(shell, SHELL_INFO, @@ -476,7 +483,7 @@ static int cmd_ieee802154_set_short_addr(const struct shell *shell, size_t argc, char *argv[]) { struct net_if *iface = net_if_get_ieee802154(); - uint16_t short_addr; + uint16_t short_addr; /* in CPU byte order */ if (argc < 2) { shell_help(shell); @@ -509,7 +516,7 @@ static int cmd_ieee802154_get_short_addr(const struct shell *shell, size_t argc, char *argv[]) { struct net_if *iface = net_if_get_ieee802154(); - uint16_t short_addr; + uint16_t short_addr; /* in CPU byte order */ if (!iface) { shell_fprintf(shell, SHELL_INFO, diff --git a/subsys/net/l2/ieee802154/ieee802154_utils.h b/subsys/net/l2/ieee802154/ieee802154_utils.h index de4ccd08f48a2..09450dde7786a 100644 --- a/subsys/net/l2/ieee802154/ieee802154_utils.h +++ b/subsys/net/l2/ieee802154/ieee802154_utils.h @@ -102,8 +102,13 @@ static inline int ieee802154_stop(struct net_if *iface) return radio->stop(net_if_get_device(iface)); } -static inline void ieee802154_filter_ieee_addr(struct net_if *iface, - uint8_t *ieee_addr) +/** + * Sets the radio drivers extended address filter. + * + * @param iface Pointer to the IEEE 802.15.4 interface + * @param ieee_addr Pointer to an extended address in little endian byte order + */ +static inline void ieee802154_filter_ieee_addr(struct net_if *iface, uint8_t *ieee_addr) { const struct ieee802154_radio_api *radio = net_if_get_device(iface)->api; From 0b4a6ad70b4ef6b80dcb82a22f949c2dbc7d9e52 Mon Sep 17 00:00:00 2001 From: Florian Grandel Date: Sat, 1 Oct 2022 10:39:51 +0200 Subject: [PATCH 05/12] samples: net: wpanusb: document endianness The WPAN-USB sample did not document endianness of some user space variables. As the IEEE 802.15.4 stack uses attributes in several different encodings, the endianness should be documented. Signed-off-by: Florian Grandel --- samples/net/wpan_serial/src/main.c | 2 +- samples/net/wpanusb/src/wpanusb.h | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/samples/net/wpan_serial/src/main.c b/samples/net/wpan_serial/src/main.c index 973e94214ac1d..73fc5d9a70dc2 100644 --- a/samples/net/wpan_serial/src/main.c +++ b/samples/net/wpan_serial/src/main.c @@ -58,7 +58,7 @@ static uint8_t slip_buf[1 + 2 * CONFIG_NET_BUF_DATA_SIZE]; static struct ieee802154_radio_api *radio_api; static const struct device *const ieee802154_dev = DEVICE_DT_GET(DT_CHOSEN(zephyr_ieee802154)); -uint8_t mac_addr[8]; +uint8_t mac_addr[8]; /* in little endian */ /* UART device */ static const struct device *const uart_dev = diff --git a/samples/net/wpanusb/src/wpanusb.h b/samples/net/wpanusb/src/wpanusb.h index 0945a8a2fed72..d9ffa7a67c610 100644 --- a/samples/net/wpanusb/src/wpanusb.h +++ b/samples/net/wpanusb/src/wpanusb.h @@ -28,13 +28,13 @@ struct set_channel { } __packed; struct set_short_addr { - uint16_t short_addr; + uint16_t short_addr; /* in CPU byte order */ } __packed; struct set_pan_id { - uint16_t pan_id; + uint16_t pan_id; /* in CPU byte order */ } __packed; struct set_ieee_addr { - uint64_t ieee_addr; + uint64_t ieee_addr; /* big endian */ } __packed; From 74d9891583edb445e66036a6baf11fded7bb313a Mon Sep 17 00:00:00 2001 From: Florian Grandel Date: Sun, 18 Sep 2022 16:43:10 +0200 Subject: [PATCH 06/12] net: l2: ieee802154: add short address to ll address This is a preparatory change that fixes one aspect of short address handling before fixing endianness so that the endianness fix can be applied consistently in this method. Signed-off-by: Florian Grandel --- subsys/net/l2/ieee802154/ieee802154.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/subsys/net/l2/ieee802154/ieee802154.c b/subsys/net/l2/ieee802154/ieee802154.c index 68f6448bf0851..7a695809b9e0d 100644 --- a/subsys/net/l2/ieee802154/ieee802154.c +++ b/subsys/net/l2/ieee802154/ieee802154.c @@ -96,11 +96,10 @@ static inline void set_pkt_ll_addr(struct net_linkaddr *addr, bool comp, enum ieee802154_addressing_mode mode, struct ieee802154_address_field *ll) { - if (mode == IEEE802154_ADDR_MODE_NONE) { - return; - } + addr->type = NET_LINK_IEEE802154; - if (mode == IEEE802154_ADDR_MODE_EXTENDED) { + switch (mode) { + case IEEE802154_ADDR_MODE_EXTENDED: addr->len = IEEE802154_EXT_ADDR_LENGTH; if (comp) { @@ -108,13 +107,23 @@ static inline void set_pkt_ll_addr(struct net_linkaddr *addr, bool comp, } else { addr->addr = ll->plain.addr.ext_addr; } - } else { - /* TODO: Handle short address (lookup known nbr, ...) */ + break; + + case IEEE802154_ADDR_MODE_SHORT: + addr->len = IEEE802154_SHORT_ADDR_LENGTH; + + if (comp) { + addr->addr = (uint8_t *)&ll->comp.addr.short_addr; + } else { + addr->addr = (uint8_t *)&ll->plain.addr.short_addr; + } + break; + + case IEEE802154_ADDR_MODE_NONE: + default: addr->len = 0U; addr->addr = NULL; } - - addr->type = NET_LINK_IEEE802154; } /** From 791e4abc437989daa5ca7e9cce96d562b0e1e5d6 Mon Sep 17 00:00:00 2001 From: Florian Grandel Date: Sat, 1 Oct 2022 10:41:38 +0200 Subject: [PATCH 07/12] net: l2: ieee802154: fix short/ext address endianness This changes fixes several bugs and inconsistencies in the IEEE 802.15.4 L2 implementation. These bugs were revealed while documenting intended endianness of driver, IP, socket and L2 attributes (see previous changes). Signed-off-by: Florian Grandel --- subsys/net/l2/ieee802154/ieee802154.c | 11 +++++++- subsys/net/l2/ieee802154/ieee802154_frame.c | 25 ++++++++++++++--- subsys/net/l2/ieee802154/ieee802154_mgmt.c | 15 ++++++---- subsys/net/l2/ieee802154/ieee802154_shell.c | 28 +++++++++---------- tests/net/ieee802154/l2/src/ieee802154_test.c | 5 +++- 5 files changed, 59 insertions(+), 25 deletions(-) diff --git a/subsys/net/l2/ieee802154/ieee802154.c b/subsys/net/l2/ieee802154/ieee802154.c index 7a695809b9e0d..96b2fc94f3a1e 100644 --- a/subsys/net/l2/ieee802154/ieee802154.c +++ b/subsys/net/l2/ieee802154/ieee802154.c @@ -124,6 +124,15 @@ static inline void set_pkt_ll_addr(struct net_linkaddr *addr, bool comp, addr->len = 0U; addr->addr = NULL; } + + /* Swap address byte order in place from little to big endian. + * This is ok as the ll address field comes from the header + * part of the packet buffer which will not be directly accessible + * once the packet reaches the upper layers. + */ + if (addr->len > 0) { + sys_mem_swap(addr->addr, addr->len); + } } /** @@ -163,7 +172,7 @@ static bool ieeee802154_check_dst_addr(struct net_if *iface, struct ieee802154_m * address. */ if (!(dst_plain->addr.short_addr == IEEE802154_BROADCAST_ADDRESS || - dst_plain->addr.short_addr == ctx->short_addr)) { + dst_plain->addr.short_addr == sys_cpu_to_le16(ctx->short_addr))) { LOG_DBG("Frame dst address (short) does not match!"); return false; } diff --git a/subsys/net/l2/ieee802154/ieee802154_frame.c b/subsys/net/l2/ieee802154/ieee802154_frame.c index 68d4168368fc0..ae2641e4722bb 100644 --- a/subsys/net/l2/ieee802154/ieee802154_frame.c +++ b/subsys/net/l2/ieee802154/ieee802154_frame.c @@ -249,6 +249,7 @@ static inline bool validate_mac_command_cfi_to_mhr(struct ieee802154_mhr *mhr, u /* This should be set only when comp == 0 */ if (dst_brdcst_chk) { + /* broadcast address is symmetric so no need to swap byte order */ if (mhr->dst_addr->plain.addr.short_addr != IEEE802154_BROADCAST_ADDRESS) { return false; } @@ -535,7 +536,7 @@ static inline enum ieee802154_addressing_mode get_dst_addr_mode(struct net_linka if (dst->len == IEEE802154_SHORT_ADDR_LENGTH) { uint16_t short_addr = ntohs(*(uint16_t *)(dst->addr)); - *broadcast = (short_addr == 0xffff); + *broadcast = (short_addr == IEEE802154_BROADCAST_ADDRESS); return IEEE802154_ADDR_MODE_SHORT; } else { *broadcast = false; @@ -674,6 +675,20 @@ bool ieee802154_create_data_frame(struct ieee802154_context *ctx, struct net_lin params.pan_id = ctx->pan_id; if (src->addr && src->len == IEEE802154_SHORT_ADDR_LENGTH) { params.short_addr = ntohs(*(uint16_t *)(src->addr)); + if (ctx->short_addr != params.short_addr) { + return false; + } + } else { + if (src->len != IEEE802154_EXT_ADDR_LENGTH) { + return false; + } + + uint8_t ext_addr_le[IEEE802154_EXT_ADDR_LENGTH]; + + sys_memcpy_swap(ext_addr_le, src->addr, IEEE802154_EXT_ADDR_LENGTH); + if (memcmp(ctx->ext_addr, ext_addr_le, src->len)) { + return false; + } } broadcast = data_addr_to_fs_settings(dst, fs, ¶ms); @@ -767,7 +782,7 @@ static inline bool cfi_to_fs_settings(enum ieee802154_cfi cfi, struct ieee802154 break; case IEEE802154_CFI_DATA_REQUEST: fs->fc.ar = 1U; - /* TODO: src/dst addr mode: see 5.3.4 */ + /* TODO: src/dst addr mode: see section 7.3.4 */ break; case IEEE802154_CFI_ORPHAN_NOTIFICATION: @@ -782,7 +797,7 @@ static inline bool cfi_to_fs_settings(enum ieee802154_cfi cfi, struct ieee802154 break; case IEEE802154_CFI_COORDINATOR_REALIGNEMENT: fs->fc.src_addr_mode = IEEE802154_ADDR_MODE_EXTENDED; - /* Todo: ar and dst addr mode: see 5.3.8 */ + /* TODO: ar and dst addr mode: see section 7.3.8 */ break; case IEEE802154_CFI_GTS_REQUEST: @@ -927,9 +942,11 @@ bool ieee802154_decipher_data_frame(struct net_if *iface, struct net_pkt *pkt, uint8_t tag_size = level_2_tag_size[level]; uint8_t hdr_len = (uint8_t *)mpdu->payload - net_pkt_data(pkt); uint8_t payload_len = net_pkt_get_len(pkt) - hdr_len - tag_size; + uint8_t ext_addr_le[IEEE802154_EXT_ADDR_LENGTH]; + sys_memcpy_swap(ext_addr_le, net_pkt_lladdr_src(pkt)->addr, IEEE802154_EXT_ADDR_LENGTH); if (!ieee802154_decrypt_auth(&ctx->sec_ctx, net_pkt_data(pkt), hdr_len, payload_len, - tag_size, net_pkt_lladdr_src(pkt)->addr, + tag_size, ext_addr_le, sys_le32_to_cpu(mpdu->mhr.aux_sec->frame_counter))) { NET_ERR("Could not decipher the frame"); return false; diff --git a/subsys/net/l2/ieee802154/ieee802154_mgmt.c b/subsys/net/l2/ieee802154/ieee802154_mgmt.c index b57265749a637..bab1b86281958 100644 --- a/subsys/net/l2/ieee802154/ieee802154_mgmt.c +++ b/subsys/net/l2/ieee802154/ieee802154_mgmt.c @@ -46,7 +46,7 @@ enum net_verdict ieee802154_handle_beacon(struct net_if *iface, if (mpdu->mhr.fs->fc.src_addr_mode == IEEE802154_ADDR_MODE_SHORT) { ctx->scan_ctx->len = IEEE802154_SHORT_ADDR_LENGTH; ctx->scan_ctx->short_addr = - mpdu->mhr.src_addr->plain.addr.short_addr; + sys_le16_to_cpu(mpdu->mhr.src_addr->plain.addr.short_addr); } else { ctx->scan_ctx->len = IEEE802154_EXT_ADDR_LENGTH; sys_memcpy_swap(ctx->scan_ctx->addr, @@ -320,7 +320,9 @@ static int ieee802154_disassociate(uint32_t mgmt_request, struct net_if *iface, if (params.dst.len == IEEE802154_SHORT_ADDR_LENGTH) { params.dst.short_addr = ctx->coord.short_addr; } else { - params.dst.ext_addr = ctx->coord.ext_addr; + params.dst.len = IEEE802154_EXT_ADDR_LENGTH; + sys_memcpy_swap(params.dst.ext_addr, ctx->coord_ext_addr, + IEEE802154_EXT_ADDR_LENGTH); } params.pan_id = ctx->pan_id; @@ -413,8 +415,11 @@ static int ieee802154_set_parameters(uint32_t mgmt_request, return -EINVAL; } - if (memcmp(ctx->ext_addr, data, IEEE802154_EXT_ADDR_LENGTH)) { - memcpy(ctx->ext_addr, data, IEEE802154_EXT_ADDR_LENGTH); + uint8_t ext_addr_le[IEEE802154_EXT_ADDR_LENGTH]; + + sys_memcpy_swap(ext_addr_le, data, IEEE802154_EXT_ADDR_LENGTH); + if (memcmp(ctx->ext_addr, ext_addr_le, IEEE802154_EXT_ADDR_LENGTH)) { + memcpy(ctx->ext_addr, ext_addr_le, IEEE802154_EXT_ADDR_LENGTH); ieee802154_filter_ieee_addr(iface, ctx->ext_addr); } } else if (mgmt_request == NET_REQUEST_IEEE802154_SET_SHORT_ADDR) { @@ -472,7 +477,7 @@ static int ieee802154_get_parameters(uint32_t mgmt_request, return -EINVAL; } - memcpy(data, ctx->ext_addr, IEEE802154_EXT_ADDR_LENGTH); + sys_memcpy_swap(data, ctx->ext_addr, IEEE802154_EXT_ADDR_LENGTH); } else if (mgmt_request == NET_REQUEST_IEEE802154_GET_SHORT_ADDR) { *value = ctx->short_addr; } else if (mgmt_request == NET_REQUEST_IEEE802154_GET_TX_POWER) { diff --git a/subsys/net/l2/ieee802154/ieee802154_shell.c b/subsys/net/l2/ieee802154/ieee802154_shell.c index 77648a0a0bf40..b9b1791b428d6 100644 --- a/subsys/net/l2/ieee802154/ieee802154_shell.c +++ b/subsys/net/l2/ieee802154/ieee802154_shell.c @@ -22,7 +22,7 @@ LOG_MODULE_REGISTER(net_ieee802154_shell, CONFIG_NET_L2_IEEE802154_LOG_LEVEL); #include "ieee802154_frame.h" -#define MAX_EXT_ADDR_STR_LEN sizeof("xx:xx:xx:xx:xx:xx:xx:xx") +#define EXT_ADDR_STR_LEN sizeof("xx:xx:xx:xx:xx:xx:xx:xx") struct ieee802154_req_params params; static struct net_mgmt_event_callback scan_cb; @@ -76,7 +76,7 @@ static int cmd_ieee802154_associate(const struct shell *shell, size_t argc, char *argv[]) { struct net_if *iface = net_if_get_ieee802154(); - char ext_addr[MAX_EXT_ADDR_STR_LEN]; + char ext_addr[EXT_ADDR_STR_LEN]; if (argc < 3) { shell_help(shell); @@ -90,9 +90,9 @@ static int cmd_ieee802154_associate(const struct shell *shell, } params.pan_id = atoi(argv[1]); - strncpy(ext_addr, argv[2], MAX_EXT_ADDR_STR_LEN - 1); + strncpy(ext_addr, argv[2], EXT_ADDR_STR_LEN - 1); - if (strlen(ext_addr) == MAX_EXT_ADDR_STR_LEN) { + if (strlen(ext_addr) == EXT_ADDR_STR_LEN) { parse_extended_address(ext_addr, params.addr); params.len = IEEE802154_EXT_ADDR_LENGTH; } else { @@ -182,7 +182,7 @@ static inline char *print_coordinator_address(char *buf, int buf_len) pos += snprintk(buf + pos, buf_len - pos, "(extended) "); - for (i = IEEE802154_EXT_ADDR_LENGTH - 1; i > -1; i--) { + for (i = 0; i < IEEE802154_EXT_ADDR_LENGTH; i++) { pos += snprintk(buf + pos, buf_len - pos, "%02X:", params.addr[i]); } @@ -421,9 +421,9 @@ static int cmd_ieee802154_set_ext_addr(const struct shell *shell, return -ENOEXEC; } - if (strlen(argv[1]) != MAX_EXT_ADDR_STR_LEN) { + if (strlen(argv[1]) != EXT_ADDR_STR_LEN) { shell_fprintf(shell, SHELL_INFO, - "%zd characters needed\n", MAX_EXT_ADDR_STR_LEN); + "%zd characters needed\n", EXT_ADDR_STR_LEN); return -ENOEXEC; } @@ -461,16 +461,16 @@ static int cmd_ieee802154_get_ext_addr(const struct shell *shell, "Could not get extended address\n"); return -ENOEXEC; } else { - static char ext_addr[MAX_EXT_ADDR_STR_LEN]; - int i, j; - - for (j = 0, i = IEEE802154_EXT_ADDR_LENGTH - 1; i > -1; i--) { - snprintf(&ext_addr[j], 4, "%02X:", addr[i]); + static char ext_addr[EXT_ADDR_STR_LEN]; + int i, pos = 0; - j += 3; + for (i = 0; i < IEEE802154_EXT_ADDR_LENGTH; i++) { + pos += snprintk(ext_addr + pos, + IEEE802154_EXT_ADDR_LENGTH - pos, + "%02X:", addr[i]); } - ext_addr[MAX_EXT_ADDR_STR_LEN - 1] = '\0'; + ext_addr[EXT_ADDR_STR_LEN - 1] = '\0'; shell_fprintf(shell, SHELL_NORMAL, "Extended address: %s\n", ext_addr); diff --git a/tests/net/ieee802154/l2/src/ieee802154_test.c b/tests/net/ieee802154/l2/src/ieee802154_test.c index 7c030a138d07b..cf027015b2ff2 100644 --- a/tests/net/ieee802154/l2/src/ieee802154_test.c +++ b/tests/net/ieee802154/l2/src/ieee802154_test.c @@ -264,7 +264,10 @@ static bool test_dgram_packet_sending(struct sockaddr_ll *pkt_sll, uint32_t secu goto release_frag; } - net_pkt_lladdr_src(current_pkt)->addr = ctx->ext_addr; + uint8_t lladdr_be[IEEE802154_EXT_ADDR_LENGTH]; + + sys_memcpy_swap(lladdr_be, ctx->ext_addr, IEEE802154_EXT_ADDR_LENGTH); + net_pkt_lladdr_src(current_pkt)->addr = lladdr_be; net_pkt_lladdr_src(current_pkt)->len = IEEE802154_MAX_ADDR_LENGTH; if (!ieee802154_decipher_data_frame(iface, current_pkt, &mpdu)) { NET_ERR("*** Cannot decipher/authenticate packet\n"); From 9a6ea913a5611f82c86fcd54aa88e622cea90441 Mon Sep 17 00:00:00 2001 From: Florian Grandel Date: Sat, 1 Oct 2022 10:30:26 +0200 Subject: [PATCH 08/12] net: ip: test and clear net_if flag This change introduces an additional function into the API that allows to test and clear a net_if flag atomically. A similar function already exists to test and set a flag. So this change actually improves symmetry of the API. The change is required in later commits of this PR. Signed-off-by: Florian Grandel --- include/zephyr/net/net_if.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/include/zephyr/net/net_if.h b/include/zephyr/net/net_if.h index 94fe5331962df..af1ae7967fcbb 100644 --- a/include/zephyr/net/net_if.h +++ b/include/zephyr/net/net_if.h @@ -542,6 +542,22 @@ static inline void net_if_flag_clear(struct net_if *iface, atomic_clear_bit(iface->if_dev->flags, value); } +/** + * @brief Test and clear a value in network interface flags + * + * @param iface Pointer to network interface + * @param value Flag value + * + * @return true if the bit was set, false if it wasn't. + */ +static inline bool net_if_flag_test_and_clear(struct net_if *iface, + enum net_if_flag value) +{ + NET_ASSERT(iface); + + return atomic_test_and_clear_bit(iface->if_dev->flags, value); +} + /** * @brief Check if a value in network interface flags is set * From bd59a0a85f1ba55d7d454c91fe0ca8e2d45e4fe8 Mon Sep 17 00:00:00 2001 From: Florian Grandel Date: Tue, 20 Sep 2022 04:27:46 +0200 Subject: [PATCH 09/12] net: l2: ieee802154: improve short address support IEEE 802.15.4 short address support is incomplete in several places. This change improves short address support without claiming to fix it everywhere. Future iterations will have to continue where this change leaves off. The purpose of this change was to: * use the short address returned by association responses, * automatically bind IEEE 802.15.4 datagram sockets to the short address if available, * use the short address in outgoing packages where applicable, * improve validation of association/disassociation frames, * model association more closely to the spec by tying it to the existence of a short address in the MAC PIB thereby removing redundancy in the PIB (which makes race conditions less probable), * keep both, the short and extended addresses, of the coordinator. Signed-off-by: Florian Grandel --- include/zephyr/net/ieee802154.h | 15 +- subsys/net/ip/net_context.c | 21 +-- subsys/net/l2/ieee802154/ieee802154.c | 20 +- subsys/net/l2/ieee802154/ieee802154_frame.c | 17 +- subsys/net/l2/ieee802154/ieee802154_mgmt.c | 171 +++++++++++++++--- tests/net/ieee802154/l2/src/ieee802154_test.c | 80 +++++++- 6 files changed, 256 insertions(+), 68 deletions(-) diff --git a/include/zephyr/net/ieee802154.h b/include/zephyr/net/ieee802154.h index 89b1f98c3afaa..6eb57bfa029e9 100644 --- a/include/zephyr/net/ieee802154.h +++ b/include/zephyr/net/ieee802154.h @@ -42,6 +42,8 @@ extern "C" { /* See IEEE 802.15.4-2006, section 7.2.1.4 */ #define IEEE802154_BROADCAST_ADDRESS 0xFFFF +#define IEEE802154_NO_SHORT_ADDRESS_ASSIGNED 0xFFFE +#define IEEE802154_SHORT_ADDRESS_NOT_ASSOCIATED 0x0000 #define IEEE802154_BROADCAST_PAN_ID 0xFFFF struct ieee802154_security_ctx { @@ -61,19 +63,22 @@ struct ieee802154_context { uint16_t pan_id; /* in CPU byte order */ uint16_t channel; struct k_sem ack_lock; + /* short address: + * 0 == not associated, + * 0xfffe == associated but no short address assigned + * see section 7.4.2 + */ uint16_t short_addr; /* in CPU byte order */ uint8_t ext_addr[IEEE802154_MAX_ADDR_LENGTH]; /* in little endian */ + struct net_linkaddr_storage linkaddr; /* in big endian */ #ifdef CONFIG_NET_L2_IEEE802154_MGMT struct ieee802154_req_params *scan_ctx; union { struct k_sem res_lock; struct k_sem req_lock; }; - union { - uint8_t ext_addr[IEEE802154_MAX_ADDR_LENGTH]; /* in little endian */ - uint16_t short_addr; /* in CPU byte order */ - } coord; - uint8_t coord_addr_len; + uint8_t coord_ext_addr[IEEE802154_MAX_ADDR_LENGTH]; /* in little endian */ + uint16_t coord_short_addr; /* in CPU byte order */ #endif #ifdef CONFIG_NET_L2_IEEE802154_SECURITY struct ieee802154_security_ctx sec_ctx; diff --git a/subsys/net/ip/net_context.c b/subsys/net/ip/net_context.c index a952a93293534..b9e424f31bed5 100644 --- a/subsys/net/ip/net_context.c +++ b/subsys/net/ip/net_context.c @@ -757,23 +757,10 @@ int net_context_bind(struct net_context *context, const struct sockaddr *addr, ll_addr->sll_ifindex; net_sll_ptr(&context->local)->sll_protocol = ll_addr->sll_protocol; - if (IS_ENABLED(CONFIG_NET_L2_IEEE802154) && - ll_addr->sll_protocol == ETH_P_IEEE802154 && - ll_addr->sll_halen > 0) { - if (ll_addr->sll_halen != - IEEE802154_SHORT_ADDR_LENGTH) { - return -EINVAL; - } - net_sll_ptr(&context->local)->sll_addr = - ll_addr->sll_addr; - net_sll_ptr(&context->local)->sll_halen = - ll_addr->sll_halen; - } else { - net_sll_ptr(&context->local)->sll_addr = - net_if_get_link_addr(iface)->addr; - net_sll_ptr(&context->local)->sll_halen = - net_if_get_link_addr(iface)->len; - } + net_sll_ptr(&context->local)->sll_addr = + net_if_get_link_addr(iface)->addr; + net_sll_ptr(&context->local)->sll_halen = + net_if_get_link_addr(iface)->len; NET_DBG("Context %p bind to type 0x%04x iface[%d] %p addr %s", context, htons(net_context_get_proto(context)), diff --git a/subsys/net/l2/ieee802154/ieee802154.c b/subsys/net/l2/ieee802154/ieee802154.c index 96b2fc94f3a1e..3bdd82118a233 100644 --- a/subsys/net/l2/ieee802154/ieee802154.c +++ b/subsys/net/l2/ieee802154/ieee802154.c @@ -18,6 +18,7 @@ LOG_MODULE_REGISTER(net_ieee802154, CONFIG_NET_L2_IEEE802154_LOG_LEVEL); #include #include #include +#include #ifdef CONFIG_NET_6LO #include "ieee802154_6lo.h" @@ -402,15 +403,27 @@ NET_L2_INIT(IEEE802154_L2, ieee802154_recv, ieee802154_send, ieee802154_enable, void ieee802154_init(struct net_if *iface) { struct ieee802154_context *ctx = net_if_l2_data(iface); - const uint8_t *mac = net_if_get_link_addr(iface)->addr; /* in big endian */ + const uint8_t *eui64_be = net_if_get_link_addr(iface)->addr; int16_t tx_power = CONFIG_NET_L2_IEEE802154_RADIO_DFLT_TX_POWER; - uint8_t long_addr[8]; /* in little endian */ NET_DBG("Initializing IEEE 802.15.4 stack on iface %p", iface); ctx->channel = IEEE802154_NO_CHANNEL; ctx->flags = NET_L2_MULTICAST; + ctx->short_addr = IEEE802154_SHORT_ADDRESS_NOT_ASSOCIATED; + sys_memcpy_swap(ctx->ext_addr, eui64_be, IEEE802154_EXT_ADDR_LENGTH); + + /* We switch to a link address store that we + * own so that we can write user defined short + * or extended addresses w/o mutating internal + * driver storage. + */ + ctx->linkaddr.type = NET_LINK_IEEE802154; + ctx->linkaddr.len = IEEE802154_EXT_ADDR_LENGTH; + memcpy(ctx->linkaddr.addr, eui64_be, IEEE802154_EXT_ADDR_LENGTH); + net_if_set_link_addr(iface, ctx->linkaddr.addr, ctx->linkaddr.len, ctx->linkaddr.type); + if (IS_ENABLED(CONFIG_IEEE802154_NET_IF_NO_AUTO_START)) { LOG_DBG("Interface auto start disabled."); net_if_flag_set(iface, NET_IF_NO_AUTO_START); @@ -424,8 +437,7 @@ void ieee802154_init(struct net_if *iface) } #endif - sys_memcpy_swap(long_addr, mac, 8); - memcpy(ctx->ext_addr, long_addr, 8); + sys_memcpy_swap(ctx->ext_addr, eui64_be, IEEE802154_EXT_ADDR_LENGTH); ieee802154_filter_ieee_addr(iface, ctx->ext_addr); if (!ieee802154_set_tx_power(iface, tx_power)) { diff --git a/subsys/net/l2/ieee802154/ieee802154_frame.c b/subsys/net/l2/ieee802154/ieee802154_frame.c index ae2641e4722bb..2d1846dae7a55 100644 --- a/subsys/net/l2/ieee802154/ieee802154_frame.c +++ b/subsys/net/l2/ieee802154/ieee802154_frame.c @@ -450,7 +450,7 @@ uint8_t ieee802154_compute_header_and_authtag_size(struct net_if *iface, struct hdr_len += broadcast ? IEEE802154_SHORT_ADDR_LENGTH : dst->len; /* Source Address - see data_addr_to_fs_settings() */ - hdr_len += broadcast ? IEEE802154_EXT_ADDR_LENGTH : (src->addr ? src->len : dst->len); + hdr_len += src->addr ? src->len : dst->len; #ifdef CONFIG_NET_L2_IEEE802154_SECURITY if (broadcast) { @@ -931,10 +931,6 @@ bool ieee802154_decipher_data_frame(struct net_if *iface, struct net_pkt *pkt, return false; } - /* TODO: handle src short address - * This will require to look up in nbr cache with short addr - * in order to get the extended address related to it - */ if (level >= IEEE802154_SECURITY_LEVEL_ENC) { level -= 4U; } @@ -944,7 +940,16 @@ bool ieee802154_decipher_data_frame(struct net_if *iface, struct net_pkt *pkt, uint8_t payload_len = net_pkt_get_len(pkt) - hdr_len - tag_size; uint8_t ext_addr_le[IEEE802154_EXT_ADDR_LENGTH]; - sys_memcpy_swap(ext_addr_le, net_pkt_lladdr_src(pkt)->addr, IEEE802154_EXT_ADDR_LENGTH); + /* TODO: Handle src short address. + * This will require to look up in nbr cache with short addr + * in order to get the extended address related to it. + */ + if (net_pkt_lladdr_src(pkt)->len != IEEE802154_EXT_ADDR_LENGTH) { + NET_ERR("Decrypting packages with short source addresses is not supported."); + goto out; + } + + sys_memcpy_swap(ext_addr_le, net_pkt_lladdr_src(pkt)->addr, net_pkt_lladdr_src(pkt)->len); if (!ieee802154_decrypt_auth(&ctx->sec_ctx, net_pkt_data(pkt), hdr_len, payload_len, tag_size, ext_addr_le, sys_le32_to_cpu(mpdu->mhr.aux_sec->frame_counter))) { diff --git a/subsys/net/l2/ieee802154/ieee802154_mgmt.c b/subsys/net/l2/ieee802154/ieee802154_mgmt.c index bab1b86281958..9631f65fe9604 100644 --- a/subsys/net/l2/ieee802154/ieee802154_mgmt.c +++ b/subsys/net/l2/ieee802154/ieee802154_mgmt.c @@ -130,7 +130,8 @@ static int ieee802154_scan(uint32_t mgmt_request, struct net_if *iface, } /* TODO: For now, we assume we are on 2.4Ghz - * (device will have to export capabilities) */ + * (device will have to export current channel page) + */ for (channel = 11U; channel <= 26U; channel++) { if (IEEE802154_IS_CHAN_UNSCANNED(scan->channel_set, channel)) { continue; @@ -184,6 +185,56 @@ NET_MGMT_REGISTER_REQUEST_HANDLER(NET_REQUEST_IEEE802154_PASSIVE_SCAN, NET_MGMT_REGISTER_REQUEST_HANDLER(NET_REQUEST_IEEE802154_ACTIVE_SCAN, ieee802154_scan); +static inline void update_net_if_link_addr(struct net_if *iface, struct ieee802154_context *ctx) +{ + bool was_if_up; + + was_if_up = net_if_flag_test_and_clear(iface, NET_IF_UP); + net_if_set_link_addr(iface, ctx->linkaddr.addr, ctx->linkaddr.len, ctx->linkaddr.type); + + if (was_if_up) { + net_if_flag_set(iface, NET_IF_UP); + } +} + +static inline void set_linkaddr_to_ext_addr(struct net_if *iface, struct ieee802154_context *ctx) +{ + ctx->linkaddr.len = IEEE802154_EXT_ADDR_LENGTH; + sys_memcpy_swap(ctx->linkaddr.addr, ctx->ext_addr, IEEE802154_EXT_ADDR_LENGTH); + + update_net_if_link_addr(iface, ctx); +} + +static inline void set_association(struct net_if *iface, struct ieee802154_context *ctx, + uint16_t short_addr) +{ + __ASSERT_NO_MSG(short_addr != IEEE802154_SHORT_ADDRESS_NOT_ASSOCIATED); + + uint16_t short_addr_be; + + ctx->linkaddr.len = IEEE802154_SHORT_ADDR_LENGTH; + ctx->short_addr = short_addr; + short_addr_be = htons(short_addr); + memcpy(ctx->linkaddr.addr, &short_addr_be, IEEE802154_SHORT_ADDR_LENGTH); + + update_net_if_link_addr(iface, ctx); + ieee802154_filter_short_addr(iface, ctx->short_addr); +} + +static inline void remove_association(struct net_if *iface, struct ieee802154_context *ctx) +{ + ctx->short_addr = IEEE802154_SHORT_ADDRESS_NOT_ASSOCIATED; + memset(ctx->coord_ext_addr, 0, IEEE802154_EXT_ADDR_LENGTH); + ctx->coord_short_addr = 0U; + set_linkaddr_to_ext_addr(iface, ctx); + ieee802154_filter_short_addr(iface, IEEE802154_SHORT_ADDRESS_NOT_ASSOCIATED); +} + +static inline bool is_associated(struct ieee802154_context *ctx) +{ + return ctx->short_addr != IEEE802154_SHORT_ADDRESS_NOT_ASSOCIATED; +} + enum net_verdict ieee802154_handle_mac_command(struct net_if *iface, struct ieee802154_mpdu *mpdu) { @@ -195,7 +246,23 @@ enum net_verdict ieee802154_handle_mac_command(struct net_if *iface, return NET_DROP; } - ctx->associated = true; + /* validation of the association response, see section 7.3.3.1 */ + + if (mpdu->mhr.fs->fc.src_addr_mode != + IEEE802154_ADDR_MODE_EXTENDED || + mpdu->mhr.fs->fc.dst_addr_mode != + IEEE802154_ADDR_MODE_EXTENDED || + mpdu->mhr.fs->fc.ar != 1 || + mpdu->mhr.fs->fc.pan_id_comp != 1) { + return NET_DROP; + } + + set_association(iface, ctx, sys_le16_to_cpu(mpdu->command->assoc_res.short_addr)); + + memcpy(ctx->coord_ext_addr, + mpdu->mhr.src_addr->comp.addr.ext_addr, + IEEE802154_EXT_ADDR_LENGTH); + k_sem_give(&ctx->req_lock); return NET_OK; @@ -207,14 +274,26 @@ enum net_verdict ieee802154_handle_mac_command(struct net_if *iface, return NET_DROP; } - if (ctx->associated) { - /* TODO: check src address vs coord ones and reject - * if they don't match - */ - ctx->associated = false; + if (!is_associated(ctx)) { + return NET_DROP; + } + + /* validation of the disassociation request, see section 7.3.3.1 */ - return NET_OK; + if (mpdu->mhr.fs->fc.src_addr_mode != + IEEE802154_ADDR_MODE_EXTENDED || + mpdu->mhr.fs->fc.pan_id_comp != 1) { + return NET_DROP; + } + + if (memcmp(ctx->coord_ext_addr, + mpdu->mhr.src_addr->comp.addr.ext_addr, + IEEE802154_EXT_ADDR_LENGTH)) { + return NET_DROP; } + + remove_association(iface, ctx); + return NET_OK; } NET_DBG("Drop MAC command, unsupported CFI: 0x%x", @@ -266,7 +345,7 @@ static int ieee802154_associate(uint32_t mgmt_request, struct net_if *iface, cmd->assoc_req.ci.sec_capability = 0U; /* TODO: security support */ cmd->assoc_req.ci.alloc_addr = 0U; /* TODO: handle short addr */ - ctx->associated = false; + remove_association(iface, ctx); ieee802154_mac_cmd_finalize(pkt, IEEE802154_CFI_ASSOCIATION_REQUEST); @@ -276,20 +355,38 @@ static int ieee802154_associate(uint32_t mgmt_request, struct net_if *iface, goto out; } - /* TODO: current timeout is arbitrary */ + /* TODO: current timeout is arbitrary, see IEEE 802.15.4-2015, 8.4.2, macResponseWaitTime */ k_sem_take(&ctx->req_lock, K_SECONDS(1)); - if (ctx->associated) { - ctx->channel = req->channel; - ctx->pan_id = req->pan_id; + if (is_associated(ctx)) { + bool validated = false; - ctx->coord_addr_len = req->len; - if (ctx->coord_addr_len == IEEE802154_SHORT_ADDR_LENGTH) { - ctx->coord.short_addr = req->short_addr; + if (req->len == IEEE802154_SHORT_ADDR_LENGTH && + ctx->coord_short_addr == req->short_addr) { + validated = true; } else { - memcpy(ctx->coord.ext_addr, - req->addr, IEEE802154_EXT_ADDR_LENGTH); + if (req->len == IEEE802154_EXT_ADDR_LENGTH) { + uint8_t ext_addr_le[IEEE802154_EXT_ADDR_LENGTH]; + + sys_memcpy_swap(ext_addr_le, req->addr, + IEEE802154_EXT_ADDR_LENGTH); + if (!memcmp(ctx->coord_ext_addr, ext_addr_le, + IEEE802154_EXT_ADDR_LENGTH)) { + validated = true; + } + } + } + + if (!validated) { + remove_association(iface, ctx); + ret = -EFAULT; + goto out; } + + ctx->channel = req->channel; + ctx->pan_id = req->pan_id; + goto out; + } else { ret = -EACCES; } @@ -311,14 +408,18 @@ static int ieee802154_disassociate(uint32_t mgmt_request, struct net_if *iface, struct ieee802154_command *cmd; struct net_pkt *pkt; - if (!ctx->associated) { + if (!is_associated(ctx)) { return -EALREADY; } + /* See section 7.3.3 */ + params.dst.pan_id = ctx->pan_id; - params.dst.len = ctx->coord_addr_len; - if (params.dst.len == IEEE802154_SHORT_ADDR_LENGTH) { - params.dst.short_addr = ctx->coord.short_addr; + + if (ctx->coord_short_addr != 0 && + ctx->coord_short_addr != IEEE802154_NO_SHORT_ADDRESS_ASSIGNED) { + params.dst.len = IEEE802154_SHORT_ADDR_LENGTH; + params.dst.short_addr = ctx->coord_short_addr; } else { params.dst.len = IEEE802154_EXT_ADDR_LENGTH; sys_memcpy_swap(params.dst.ext_addr, ctx->coord_ext_addr, @@ -344,7 +445,7 @@ static int ieee802154_disassociate(uint32_t mgmt_request, struct net_if *iface, return -EIO; } - ctx->associated = false; + remove_association(iface, ctx); return 0; } @@ -383,10 +484,6 @@ static int ieee802154_set_parameters(uint32_t mgmt_request, uint16_t value; int ret = 0; - if (ctx->associated) { - return -EBUSY; - } - if (mgmt_request != NET_REQUEST_IEEE802154_SET_EXT_ADDR && (len != sizeof(uint16_t) || !data)) { return -EINVAL; @@ -394,6 +491,11 @@ static int ieee802154_set_parameters(uint32_t mgmt_request, value = *((uint16_t *) data); + if (is_associated(ctx) && !(mgmt_request == NET_REQUEST_IEEE802154_SET_SHORT_ADDR && + value == IEEE802154_SHORT_ADDRESS_NOT_ASSOCIATED)) { + return -EBUSY; + } + if (mgmt_request == NET_REQUEST_IEEE802154_SET_CHANNEL) { if (ctx->channel != value) { if (!ieee802154_verify_channel(iface, value)) { @@ -418,14 +520,23 @@ static int ieee802154_set_parameters(uint32_t mgmt_request, uint8_t ext_addr_le[IEEE802154_EXT_ADDR_LENGTH]; sys_memcpy_swap(ext_addr_le, data, IEEE802154_EXT_ADDR_LENGTH); + if (memcmp(ctx->ext_addr, ext_addr_le, IEEE802154_EXT_ADDR_LENGTH)) { memcpy(ctx->ext_addr, ext_addr_le, IEEE802154_EXT_ADDR_LENGTH); + + if (net_if_get_link_addr(iface)->len == IEEE802154_EXT_ADDR_LENGTH) { + set_linkaddr_to_ext_addr(iface, ctx); + } + ieee802154_filter_ieee_addr(iface, ctx->ext_addr); } } else if (mgmt_request == NET_REQUEST_IEEE802154_SET_SHORT_ADDR) { if (ctx->short_addr != value) { - ctx->short_addr = value; - ieee802154_filter_short_addr(iface, ctx->short_addr); + if (value == IEEE802154_SHORT_ADDRESS_NOT_ASSOCIATED) { + remove_association(iface, ctx); + } else { + set_association(iface, ctx, value); + } } } else if (mgmt_request == NET_REQUEST_IEEE802154_SET_TX_POWER) { if (ctx->tx_power != (int16_t)value) { @@ -513,7 +624,7 @@ static int ieee802154_set_security_settings(uint32_t mgmt_request, struct ieee802154_context *ctx = net_if_l2_data(iface); struct ieee802154_security_params *params; - if (ctx->associated) { + if (is_associated(ctx)) { return -EBUSY; } diff --git a/tests/net/ieee802154/l2/src/ieee802154_test.c b/tests/net/ieee802154/l2/src/ieee802154_test.c index cf027015b2ff2..3526e14df8e9c 100644 --- a/tests/net/ieee802154/l2/src/ieee802154_test.c +++ b/tests/net/ieee802154/l2/src/ieee802154_test.c @@ -144,6 +144,38 @@ static void ieee_addr_hexdump(uint8_t *addr, uint8_t length) printk("%02x\n", *addr); } +static int set_up_short_addr(struct net_if *iface, struct ieee802154_context *ctx) +{ + uint16_t short_addr = 0x5678; + + int ret = net_mgmt(NET_REQUEST_IEEE802154_SET_SHORT_ADDR, iface, &short_addr, + sizeof(short_addr)); + if (ret) { + NET_ERR("*** Failed to set short address\n"); + } + + return ret; +} + +static int tear_down_short_addr(struct net_if *iface, struct ieee802154_context *ctx) +{ + uint16_t short_addr; + + if (ctx->linkaddr.len != IEEE802154_SHORT_ADDR_LENGTH) { + /* nothing to do */ + return 0; + } + + short_addr = IEEE802154_SHORT_ADDRESS_NOT_ASSOCIATED; + int ret = net_mgmt(NET_REQUEST_IEEE802154_SET_SHORT_ADDR, iface, &short_addr, + sizeof(short_addr)); + if (ret) { + NET_ERR("*** Failed to unset short address\n"); + } + + return ret; +} + static bool test_packet_parsing(struct ieee802154_pkt_test *t) { struct ieee802154_mpdu mpdu; @@ -170,17 +202,27 @@ static bool test_packet_parsing(struct ieee802154_pkt_test *t) return true; } -static bool test_ns_sending(struct ieee802154_pkt_test *t) +static bool test_ns_sending(struct ieee802154_pkt_test *t, bool with_short_addr) { + struct ieee802154_context *ctx = net_if_l2_data(iface); struct ieee802154_mpdu mpdu; NET_INFO("- Sending NS packet\n"); + if (with_short_addr) { + if (set_up_short_addr(iface, ctx)) { + return false; + } + } + if (net_ipv6_send_ns(iface, NULL, &t->src, &t->dst, &t->dst, false)) { NET_ERR("*** Could not create IPv6 NS packet\n"); + tear_down_short_addr(iface, ctx); return false; } + tear_down_short_addr(iface, ctx); + k_yield(); k_sem_take(&driver_lock, K_SECONDS(1)); @@ -234,9 +276,23 @@ static bool test_dgram_packet_sending(struct sockaddr_ll *pkt_sll, uint32_t secu goto release_sec_ctx; } + /* In case we have a short destination address + * we simulate an associated device. + */ + /* TODO: support short addresses with encryption (requires neighbour cache) */ + bool bind_short_address = pkt_sll->sll_halen == IEEE802154_SHORT_ADDR_LENGTH && + security_level == IEEE802154_SECURITY_LEVEL_NONE; + + if (bind_short_address) { + if (set_up_short_addr(iface, ctx)) { + goto release_fd; + } + } + socket_sll.sll_ifindex = net_if_get_by_iface(iface); socket_sll.sll_family = AF_PACKET; socket_sll.sll_protocol = ETH_P_IEEE802154; + if (bind(fd, (const struct sockaddr *)&socket_sll, sizeof(struct sockaddr_ll))) { NET_ERR("*** Failed to bind packet socket : %d\n", errno); goto release_fd; @@ -264,11 +320,9 @@ static bool test_dgram_packet_sending(struct sockaddr_ll *pkt_sll, uint32_t secu goto release_frag; } - uint8_t lladdr_be[IEEE802154_EXT_ADDR_LENGTH]; + net_pkt_lladdr_src(current_pkt)->addr = net_if_get_link_addr(iface)->addr; + net_pkt_lladdr_src(current_pkt)->len = net_if_get_link_addr(iface)->len; - sys_memcpy_swap(lladdr_be, ctx->ext_addr, IEEE802154_EXT_ADDR_LENGTH); - net_pkt_lladdr_src(current_pkt)->addr = lladdr_be; - net_pkt_lladdr_src(current_pkt)->len = IEEE802154_MAX_ADDR_LENGTH; if (!ieee802154_decipher_data_frame(iface, current_pkt, &mpdu)) { NET_ERR("*** Cannot decipher/authenticate packet\n"); goto release_frag; @@ -285,6 +339,7 @@ static bool test_dgram_packet_sending(struct sockaddr_ll *pkt_sll, uint32_t secu net_pkt_frag_unref(current_pkt->frags); current_pkt->frags = NULL; release_fd: + tear_down_short_addr(iface, ctx); close(fd); release_sec_ctx: cipher_free_session(ctx->sec_ctx.enc.device, &ctx->sec_ctx.enc); @@ -315,6 +370,7 @@ static bool test_raw_packet_sending(void) socket_sll.sll_ifindex = net_if_get_by_iface(iface); socket_sll.sll_family = AF_PACKET; socket_sll.sll_protocol = ETH_P_IEEE802154; + if (bind(fd, (const struct sockaddr *)&socket_sll, sizeof(struct sockaddr_ll))) { NET_ERR("*** Failed to bind packet socket : %d\n", errno); goto release_fd; @@ -503,7 +559,16 @@ ZTEST(ieee802154_l2, test_sending_ns_pkt) { bool ret; - ret = test_ns_sending(&test_ns_pkt); + ret = test_ns_sending(&test_ns_pkt, false); + + zassert_true(ret, "NS sent"); +} + +ZTEST(ieee802154_l2, test_sending_ns_pkt_with_short_addr) +{ + bool ret; + + ret = test_ns_sending(&test_ns_pkt, true); zassert_true(ret, "NS sent"); } @@ -552,6 +617,7 @@ ZTEST(ieee802154_l2, test_sending_broadcast_dgram_pkt) .sll_halen = sizeof(dst_short_addr), .sll_protocol = htons(ETH_P_IEEE802154), }; + memcpy(pkt_sll.sll_addr, &dst_short_addr, sizeof(dst_short_addr)); ret = test_dgram_packet_sending(&pkt_sll, IEEE802154_SECURITY_LEVEL_NONE); @@ -567,6 +633,7 @@ ZTEST(ieee802154_l2, test_sending_authenticated_dgram_pkt) .sll_halen = sizeof(dst_short_addr), .sll_protocol = htons(ETH_P_IEEE802154), }; + memcpy(pkt_sll.sll_addr, &dst_short_addr, sizeof(dst_short_addr)); ret = test_dgram_packet_sending(&pkt_sll, IEEE802154_SECURITY_LEVEL_MIC_128); @@ -582,6 +649,7 @@ ZTEST(ieee802154_l2, test_sending_encrypted_and_authenticated_dgram_pkt) .sll_halen = sizeof(dst_ext_addr), .sll_protocol = htons(ETH_P_IEEE802154), }; + memcpy(pkt_sll.sll_addr, dst_ext_addr, sizeof(dst_ext_addr)); ret = test_dgram_packet_sending(&pkt_sll, IEEE802154_SECURITY_LEVEL_ENC_MIC_128); From afb56078504ac9ebf1823cfbc53b492a490c56d5 Mon Sep 17 00:00:00 2001 From: Florian Grandel Date: Tue, 20 Sep 2022 04:33:12 +0200 Subject: [PATCH 10/12] net: l2: ieee802154: improved context thread safety Several attributes in the ieee802154_context struct may potentially be accessed from different threads and/or ISR context. Only some of these attributes were properly guarded against race conditions. This may not have been to problematic in the past but as other changes in this PR introduce additional attributes and mutate several attributes in a single atomic transaction, leaving such changes unprotected seems dangerous. This change therefore introduces systematic locking of the ieee802154_context structure. Signed-off-by: Florian Grandel --- include/zephyr/net/ieee802154.h | 24 +-- subsys/net/l2/ieee802154/ieee802154.c | 31 +++- subsys/net/l2/ieee802154/ieee802154_frame.c | 63 ++++++-- subsys/net/l2/ieee802154/ieee802154_mgmt.c | 147 +++++++++++++----- .../net/l2/ieee802154/ieee802154_mgmt_priv.h | 2 +- tests/net/ieee802154/l2/src/ieee802154_test.c | 2 + 6 files changed, 197 insertions(+), 72 deletions(-) diff --git a/include/zephyr/net/ieee802154.h b/include/zephyr/net/ieee802154.h index 6eb57bfa029e9..3a9db59569417 100644 --- a/include/zephyr/net/ieee802154.h +++ b/include/zephyr/net/ieee802154.h @@ -62,7 +62,6 @@ struct ieee802154_context { enum net_l2_flags flags; uint16_t pan_id; /* in CPU byte order */ uint16_t channel; - struct k_sem ack_lock; /* short address: * 0 == not associated, * 0xfffe == associated but no short address assigned @@ -72,11 +71,9 @@ struct ieee802154_context { uint8_t ext_addr[IEEE802154_MAX_ADDR_LENGTH]; /* in little endian */ struct net_linkaddr_storage linkaddr; /* in big endian */ #ifdef CONFIG_NET_L2_IEEE802154_MGMT - struct ieee802154_req_params *scan_ctx; - union { - struct k_sem res_lock; - struct k_sem req_lock; - }; + struct ieee802154_req_params *scan_ctx; /* guarded by scan_ctx_lock */ + struct k_sem scan_ctx_lock; + uint8_t coord_ext_addr[IEEE802154_MAX_ADDR_LENGTH]; /* in little endian */ uint16_t coord_short_addr; /* in CPU byte order */ #endif @@ -85,11 +82,16 @@ struct ieee802154_context { #endif int16_t tx_power; uint8_t sequence; - uint8_t ack_seq; - uint8_t ack_received : 1; - uint8_t ack_requested : 1; - uint8_t associated : 1; - uint8_t _unused : 5; + + uint8_t ack_seq; /* guarded by ack_lock */ + uint8_t ack_received : 1; /* guarded by ack_lock */ + uint8_t ack_requested : 1; /* guarded by ack_lock */ + uint8_t _unused : 6; + struct k_sem ack_lock; + + struct k_sem ctx_lock; /* guards all mutable context attributes unless + * otherwise mentioned on attribute level + */ }; #define IEEE802154_L2_CTX_TYPE struct ieee802154_context diff --git a/subsys/net/l2/ieee802154/ieee802154.c b/subsys/net/l2/ieee802154/ieee802154.c index 3bdd82118a233..9e12e705925eb 100644 --- a/subsys/net/l2/ieee802154/ieee802154.c +++ b/subsys/net/l2/ieee802154/ieee802154.c @@ -142,8 +142,9 @@ static inline void set_pkt_ll_addr(struct net_linkaddr *addr, bool comp, */ static bool ieeee802154_check_dst_addr(struct net_if *iface, struct ieee802154_mhr *mhr) { - struct ieee802154_context *ctx = net_if_l2_data(iface); struct ieee802154_address_field_plain *dst_plain = &mhr->dst_addr->plain; + struct ieee802154_context *ctx = net_if_l2_data(iface); + bool ret = false; /* * Apply filtering requirements from chapter 6.7.2 of the IEEE @@ -156,6 +157,8 @@ static bool ieeee802154_check_dst_addr(struct net_if *iface, struct ieee802154_m return false; } + k_sem_take(&ctx->ctx_lock, K_FOREVER); + /* * c. If a destination PAN ID is included in the frame, it shall match * macPanId or shall be the broadcastPAN ID @@ -175,7 +178,7 @@ static bool ieeee802154_check_dst_addr(struct net_if *iface, struct ieee802154_m if (!(dst_plain->addr.short_addr == IEEE802154_BROADCAST_ADDRESS || dst_plain->addr.short_addr == sys_cpu_to_le16(ctx->short_addr))) { LOG_DBG("Frame dst address (short) does not match!"); - return false; + goto out; } } else if (mhr->fs->fc.dst_addr_mode == IEEE802154_ADDR_MODE_EXTENDED) { @@ -188,11 +191,14 @@ static bool ieeee802154_check_dst_addr(struct net_if *iface, struct ieee802154_m if (memcmp(dst_plain->addr.ext_addr, ctx->ext_addr, IEEE802154_EXT_ADDR_LENGTH) != 0) { LOG_DBG("Frame dst address (ext) does not match!"); - return false; + goto out; } } + ret = true; - return true; +out: + k_sem_give(&ctx->ctx_lock); + return ret; } static enum net_verdict ieee802154_recv(struct net_if *iface, struct net_pkt *pkt) @@ -279,10 +285,12 @@ static int ieee802154_send(struct net_if *iface, struct net_pkt *pkt) if (!context) { return -EINVAL; } + switch (net_context_get_type(context)) { case SOCK_RAW: send_raw = true; break; + #if defined(CONFIG_NET_SOCKETS_PACKET_DGRAM) case SOCK_DGRAM: { struct sockaddr_ll *dst_addr = (struct sockaddr_ll *)&context->remote; @@ -380,10 +388,15 @@ static int ieee802154_enable(struct net_if *iface, bool state) NET_DBG("iface %p %s", iface, state ? "up" : "down"); + k_sem_take(&ctx->ctx_lock, K_FOREVER); + if (ctx->channel == IEEE802154_NO_CHANNEL) { + k_sem_give(&ctx->ctx_lock); return -ENETDOWN; } + k_sem_give(&ctx->ctx_lock); + if (state) { return ieee802154_start(iface); } @@ -391,10 +404,13 @@ static int ieee802154_enable(struct net_if *iface, bool state) return ieee802154_stop(iface); } -enum net_l2_flags ieee802154_flags(struct net_if *iface) +static enum net_l2_flags ieee802154_flags(struct net_if *iface) { struct ieee802154_context *ctx = net_if_l2_data(iface); + /* No need for locking as these flags are set once + * during L2 initialization and then never changed. + */ return ctx->flags; } @@ -408,6 +424,11 @@ void ieee802154_init(struct net_if *iface) NET_DBG("Initializing IEEE 802.15.4 stack on iface %p", iface); + k_sem_init(&ctx->ctx_lock, 1, 1); + + /* no need to lock the context here as it has + * not been published yet. + */ ctx->channel = IEEE802154_NO_CHANNEL; ctx->flags = NET_L2_MULTICAST; diff --git a/subsys/net/l2/ieee802154/ieee802154_frame.c b/subsys/net/l2/ieee802154/ieee802154_frame.c index 2d1846dae7a55..4de9d95d8971f 100644 --- a/subsys/net/l2/ieee802154/ieee802154_frame.c +++ b/subsys/net/l2/ieee802154/ieee802154_frame.c @@ -458,10 +458,14 @@ uint8_t ieee802154_compute_header_and_authtag_size(struct net_if *iface, struct goto done; } + struct ieee802154_context *ctx = (struct ieee802154_context *)net_if_l2_data(iface); + + k_sem_take(&ctx->ctx_lock, K_FOREVER); + struct ieee802154_security_ctx *sec_ctx = - &((struct ieee802154_context *)net_if_l2_data(iface))->sec_ctx; + &ctx->sec_ctx; if (sec_ctx->level == IEEE802154_SECURITY_LEVEL_NONE) { - goto done; + goto release; } /* Compute aux-sec hdr size and add it to hdr_len */ @@ -496,6 +500,9 @@ uint8_t ieee802154_compute_header_and_authtag_size(struct net_if *iface, struct } else { hdr_len += level_2_tag_size[sec_ctx->level - 4U]; } + +release: + k_sem_give(&ctx->ctx_lock); done: #endif /* CONFIG_NET_L2_IEEE802154_SECURITY */ @@ -664,8 +671,11 @@ bool ieee802154_create_data_frame(struct ieee802154_context *ctx, struct net_lin struct ieee802154_fcf_seq *fs; uint8_t *p_buf = buf->data; uint8_t *buf_start = p_buf; + bool ret = false; bool broadcast; + k_sem_take(&ctx->ctx_lock, K_FOREVER); + fs = generate_fcf_grounds(&p_buf, ctx->ack_requested); fs->fc.frame_type = IEEE802154_FRAME_TYPE_DATA; @@ -676,18 +686,18 @@ bool ieee802154_create_data_frame(struct ieee802154_context *ctx, struct net_lin if (src->addr && src->len == IEEE802154_SHORT_ADDR_LENGTH) { params.short_addr = ntohs(*(uint16_t *)(src->addr)); if (ctx->short_addr != params.short_addr) { - return false; + goto out; } } else { if (src->len != IEEE802154_EXT_ADDR_LENGTH) { - return false; + goto out; } uint8_t ext_addr_le[IEEE802154_EXT_ADDR_LENGTH]; sys_memcpy_swap(ext_addr_le, src->addr, IEEE802154_EXT_ADDR_LENGTH); if (memcmp(ctx->ext_addr, ext_addr_le, src->len)) { - return false; + goto out; } } @@ -712,7 +722,7 @@ bool ieee802154_create_data_frame(struct ieee802154_context *ctx, struct net_lin p_buf = generate_aux_security_hdr(&ctx->sec_ctx, p_buf); if (!p_buf) { NET_ERR("Unsupported key mode."); - return false; + goto out; } uint8_t payload_len = buf->len - hdr_len; @@ -735,7 +745,7 @@ bool ieee802154_create_data_frame(struct ieee802154_context *ctx, struct net_lin /* Let's encrypt/auth only in the end, if needed */ if (!ieee802154_encrypt_auth(broadcast ? NULL : &ctx->sec_ctx, buf_start, hdr_len, payload_len, tag_size, ctx->ext_addr)) { - return false; + goto out; }; no_security_hdr: @@ -743,12 +753,16 @@ bool ieee802154_create_data_frame(struct ieee802154_context *ctx, struct net_lin if ((p_buf - buf_start) != hdr_len) { /* hdr_len was too small? We probably overwrote payload bytes */ NET_ERR("Could not generate data frame %zu vs %u", (p_buf - buf_start), hdr_len); - return false; + goto out; } dbg_print_fs(fs); - return true; + ret = true; + +out: + k_sem_give(&ctx->ctx_lock); + return ret; } #ifdef CONFIG_NET_L2_IEEE802154_RFD @@ -841,15 +855,17 @@ struct net_pkt *ieee802154_create_mac_cmd_frame(struct net_if *iface, enum ieee8 { struct ieee802154_context *ctx = net_if_l2_data(iface); struct ieee802154_fcf_seq *fs; - struct net_pkt *pkt; + struct net_pkt *pkt = NULL; uint8_t *p_buf, *p_start; + k_sem_take(&ctx->ctx_lock, K_FOREVER); + /* It would be costly to compute the size when actual frame are never * bigger than 125 bytes, so let's allocate that size as buffer. */ pkt = net_pkt_alloc_with_buffer(iface, IEEE802154_MTU, AF_UNSPEC, 0, BUF_TIMEOUT); if (!pkt) { - return NULL; + goto out; } p_buf = net_pkt_data(pkt); @@ -874,11 +890,15 @@ struct net_pkt *ieee802154_create_mac_cmd_frame(struct net_if *iface, enum ieee8 dbg_print_fs(fs); - return pkt; + goto out; + error: net_pkt_unref(pkt); + pkt = NULL; - return NULL; +out: + k_sem_give(&ctx->ctx_lock); + return pkt; } void ieee802154_mac_cmd_finalize(struct net_pkt *pkt, enum ieee802154_cfi type) @@ -917,10 +937,15 @@ bool ieee802154_decipher_data_frame(struct net_if *iface, struct net_pkt *pkt, struct ieee802154_mpdu *mpdu) { struct ieee802154_context *ctx = net_if_l2_data(iface); + bool ret = false; + + k_sem_take(&ctx->ctx_lock, K_FOREVER); + uint8_t level = ctx->sec_ctx.level; if (!mpdu->mhr.fs->fc.security_enabled) { - return true; + ret = true; + goto out; } /* Section 7.2.3 (i) talks about "security level policy" conformance @@ -928,7 +953,7 @@ bool ieee802154_decipher_data_frame(struct net_if *iface, struct net_pkt *pkt, * ends should have same security level. */ if (mpdu->mhr.aux_sec->control.security_level != level) { - return false; + goto out; } if (level >= IEEE802154_SECURITY_LEVEL_ENC) { @@ -954,12 +979,16 @@ bool ieee802154_decipher_data_frame(struct net_if *iface, struct net_pkt *pkt, tag_size, ext_addr_le, sys_le32_to_cpu(mpdu->mhr.aux_sec->frame_counter))) { NET_ERR("Could not decipher the frame"); - return false; + goto out; } /* We remove tag size from buf's length, it is now useless. */ pkt->buffer->len -= tag_size; - return true; + ret = true; + +out: + k_sem_give(&ctx->ctx_lock); + return ret; } #endif /* CONFIG_NET_L2_IEEE802154_SECURITY */ diff --git a/subsys/net/l2/ieee802154/ieee802154_mgmt.c b/subsys/net/l2/ieee802154/ieee802154_mgmt.c index 9631f65fe9604..723010a1d1e2d 100644 --- a/subsys/net/l2/ieee802154/ieee802154_mgmt.c +++ b/subsys/net/l2/ieee802154/ieee802154_mgmt.c @@ -38,7 +38,7 @@ enum net_verdict ieee802154_handle_beacon(struct net_if *iface, return NET_DROP; } - k_sem_take(&ctx->res_lock, K_FOREVER); + k_sem_take(&ctx->scan_ctx_lock, K_FOREVER); ctx->scan_ctx->pan_id = mpdu->mhr.src_addr->plain.pan_id; ctx->scan_ctx->lqi = lqi; @@ -56,7 +56,7 @@ enum net_verdict ieee802154_handle_beacon(struct net_if *iface, net_mgmt_event_notify(NET_EVENT_IEEE802154_SCAN_RESULT, iface); - k_sem_give(&ctx->res_lock); + k_sem_give(&ctx->scan_ctx_lock); return NET_OK; } @@ -71,7 +71,9 @@ static int ieee802154_cancel_scan(uint32_t mgmt_request, struct net_if *iface, NET_DBG("Cancelling scan request"); + k_sem_take(&ctx->scan_ctx_lock, K_FOREVER); ctx->scan_ctx = NULL; + k_sem_give(&ctx->scan_ctx_lock); return 0; } @@ -93,14 +95,17 @@ static int ieee802154_scan(uint32_t mgmt_request, struct net_if *iface, mgmt_request == NET_REQUEST_IEEE802154_ACTIVE_SCAN ? "Active" : "Passive"); - if (ctx->scan_ctx) { - return -EALREADY; - } - if (scan == NULL) { return -EINVAL; } + k_sem_take(&ctx->scan_ctx_lock, K_FOREVER); + + if (ctx->scan_ctx) { + ret = -EALREADY; + goto out; + } + if (mgmt_request == NET_REQUEST_IEEE802154_ACTIVE_SCAN) { struct ieee802154_frame_params params; @@ -111,13 +116,16 @@ static int ieee802154_scan(uint32_t mgmt_request, struct net_if *iface, iface, IEEE802154_CFI_BEACON_REQUEST, ¶ms); if (!pkt) { NET_DBG("Could not create Beacon Request"); - return -ENOBUFS; + ret = -ENOBUFS; + goto out; } ieee802154_mac_cmd_finalize(pkt, IEEE802154_CFI_BEACON_REQUEST); } ctx->scan_ctx = scan; + k_sem_give(&ctx->scan_ctx_lock); + ret = 0; ieee802154_filter_pan_id(iface, IEEE802154_BROADCAST_PAN_ID); @@ -151,27 +159,32 @@ static int ieee802154_scan(uint32_t mgmt_request, struct net_if *iface, NET_DBG("Could not send Beacon Request (%d)", ret); net_pkt_unref(pkt); - - break; + k_sem_take(&ctx->scan_ctx_lock, K_FOREVER); + goto out; } } /* Context aware sleep */ k_sleep(K_MSEC(scan->duration)); + k_sem_take(&ctx->scan_ctx_lock, K_FOREVER); + if (!ctx->scan_ctx) { NET_DBG("Scan request cancelled"); ret = -ECANCELED; - - break; + goto out; } + + k_sem_give(&ctx->scan_ctx_lock); } /* Let's come back to context's settings */ ieee802154_filter_pan_id(iface, ctx->pan_id); ieee802154_set_channel(iface, ctx->channel); + out: ctx->scan_ctx = NULL; + k_sem_give(&ctx->scan_ctx_lock); if (pkt) { net_pkt_unref(pkt); @@ -185,6 +198,7 @@ NET_MGMT_REGISTER_REQUEST_HANDLER(NET_REQUEST_IEEE802154_PASSIVE_SCAN, NET_MGMT_REGISTER_REQUEST_HANDLER(NET_REQUEST_IEEE802154_ACTIVE_SCAN, ieee802154_scan); +/* Requires the context lock to be held. */ static inline void update_net_if_link_addr(struct net_if *iface, struct ieee802154_context *ctx) { bool was_if_up; @@ -197,6 +211,7 @@ static inline void update_net_if_link_addr(struct net_if *iface, struct ieee8021 } } +/* Requires the context lock to be held. */ static inline void set_linkaddr_to_ext_addr(struct net_if *iface, struct ieee802154_context *ctx) { ctx->linkaddr.len = IEEE802154_EXT_ADDR_LENGTH; @@ -205,6 +220,7 @@ static inline void set_linkaddr_to_ext_addr(struct net_if *iface, struct ieee802 update_net_if_link_addr(iface, ctx); } +/* Requires the context lock to be held. */ static inline void set_association(struct net_if *iface, struct ieee802154_context *ctx, uint16_t short_addr) { @@ -221,6 +237,7 @@ static inline void set_association(struct net_if *iface, struct ieee802154_conte ieee802154_filter_short_addr(iface, ctx->short_addr); } +/* Requires the context lock to be held. */ static inline void remove_association(struct net_if *iface, struct ieee802154_context *ctx) { ctx->short_addr = IEEE802154_SHORT_ADDRESS_NOT_ASSOCIATED; @@ -230,6 +247,7 @@ static inline void remove_association(struct net_if *iface, struct ieee802154_co ieee802154_filter_short_addr(iface, IEEE802154_SHORT_ADDRESS_NOT_ASSOCIATED); } +/* Requires the context lock to be held. */ static inline bool is_associated(struct ieee802154_context *ctx) { return ctx->short_addr != IEEE802154_SHORT_ADDRESS_NOT_ASSOCIATED; @@ -257,25 +275,33 @@ enum net_verdict ieee802154_handle_mac_command(struct net_if *iface, return NET_DROP; } + k_sem_take(&ctx->ctx_lock, K_FOREVER); + set_association(iface, ctx, sys_le16_to_cpu(mpdu->command->assoc_res.short_addr)); memcpy(ctx->coord_ext_addr, mpdu->mhr.src_addr->comp.addr.ext_addr, IEEE802154_EXT_ADDR_LENGTH); - k_sem_give(&ctx->req_lock); + k_sem_give(&ctx->ctx_lock); + + k_sem_give(&ctx->scan_ctx_lock); return NET_OK; } if (mpdu->command->cfi == IEEE802154_CFI_DISASSOCIATION_NOTIFICATION) { + enum net_verdict ret = NET_DROP; + if (mpdu->command->disassoc_note.reason != IEEE802154_DRF_COORDINATOR_WISH) { return NET_DROP; } + k_sem_take(&ctx->ctx_lock, K_FOREVER); + if (!is_associated(ctx)) { - return NET_DROP; + goto out; } /* validation of the disassociation request, see section 7.3.3.1 */ @@ -283,17 +309,21 @@ enum net_verdict ieee802154_handle_mac_command(struct net_if *iface, if (mpdu->mhr.fs->fc.src_addr_mode != IEEE802154_ADDR_MODE_EXTENDED || mpdu->mhr.fs->fc.pan_id_comp != 1) { - return NET_DROP; + goto out; } if (memcmp(ctx->coord_ext_addr, mpdu->mhr.src_addr->comp.addr.ext_addr, IEEE802154_EXT_ADDR_LENGTH)) { - return NET_DROP; + goto out; } remove_association(iface, ctx); - return NET_OK; + ret = NET_OK; + +out: + k_sem_give(&ctx->ctx_lock); + return ret; } NET_DBG("Drop MAC command, unsupported CFI: 0x%x", @@ -313,8 +343,6 @@ static int ieee802154_associate(uint32_t mgmt_request, struct net_if *iface, struct net_pkt *pkt; int ret = 0; - k_sem_take(&ctx->req_lock, K_FOREVER); - params.dst.len = req->len; if (params.dst.len == IEEE802154_SHORT_ADDR_LENGTH) { params.dst.short_addr = req->short_addr; @@ -345,7 +373,9 @@ static int ieee802154_associate(uint32_t mgmt_request, struct net_if *iface, cmd->assoc_req.ci.sec_capability = 0U; /* TODO: security support */ cmd->assoc_req.ci.alloc_addr = 0U; /* TODO: handle short addr */ + k_sem_take(&ctx->ctx_lock, K_FOREVER); remove_association(iface, ctx); + k_sem_give(&ctx->ctx_lock); ieee802154_mac_cmd_finalize(pkt, IEEE802154_CFI_ASSOCIATION_REQUEST); @@ -355,8 +385,16 @@ static int ieee802154_associate(uint32_t mgmt_request, struct net_if *iface, goto out; } + /* acquire the lock so that the next k_sem_take() blocks */ + k_sem_take(&ctx->scan_ctx_lock, K_FOREVER); + /* TODO: current timeout is arbitrary, see IEEE 802.15.4-2015, 8.4.2, macResponseWaitTime */ - k_sem_take(&ctx->req_lock, K_SECONDS(1)); + k_sem_take(&ctx->scan_ctx_lock, K_SECONDS(1)); + + /* release the lock */ + k_sem_give(&ctx->scan_ctx_lock); + + k_sem_take(&ctx->ctx_lock, K_FOREVER); if (is_associated(ctx)) { bool validated = false; @@ -392,8 +430,7 @@ static int ieee802154_associate(uint32_t mgmt_request, struct net_if *iface, } out: - k_sem_give(&ctx->req_lock); - + k_sem_give(&ctx->ctx_lock); return ret; } @@ -407,9 +444,13 @@ static int ieee802154_disassociate(uint32_t mgmt_request, struct net_if *iface, struct ieee802154_frame_params params; struct ieee802154_command *cmd; struct net_pkt *pkt; + int ret = 0; + + k_sem_take(&ctx->ctx_lock, K_FOREVER); if (!is_associated(ctx)) { - return -EALREADY; + ret = -EALREADY; + goto out; } /* See section 7.3.3 */ @@ -431,7 +472,8 @@ static int ieee802154_disassociate(uint32_t mgmt_request, struct net_if *iface, pkt = ieee802154_create_mac_cmd_frame( iface, IEEE802154_CFI_DISASSOCIATION_NOTIFICATION, ¶ms); if (!pkt) { - return -ENOBUFS; + ret = -ENOBUFS; + goto out; } cmd = ieee802154_get_mac_command(pkt); @@ -442,12 +484,15 @@ static int ieee802154_disassociate(uint32_t mgmt_request, struct net_if *iface, if (net_if_send_data(iface, pkt)) { net_pkt_unref(pkt); - return -EIO; + ret = -EIO; + goto out; } remove_association(iface, ctx); - return 0; +out: + k_sem_give(&ctx->ctx_lock); + return ret; } NET_MGMT_REGISTER_REQUEST_HANDLER(NET_REQUEST_IEEE802154_DISASSOCIATE, @@ -461,12 +506,16 @@ static int ieee802154_set_ack(uint32_t mgmt_request, struct net_if *iface, ARG_UNUSED(data); ARG_UNUSED(len); + k_sem_take(&ctx->ctx_lock, K_FOREVER); + if (mgmt_request == NET_REQUEST_IEEE802154_SET_ACK) { ctx->ack_requested = true; } else if (mgmt_request == NET_REQUEST_IEEE802154_UNSET_ACK) { ctx->ack_requested = false; } + k_sem_give(&ctx->ctx_lock); + return 0; } @@ -491,15 +540,19 @@ static int ieee802154_set_parameters(uint32_t mgmt_request, value = *((uint16_t *) data); + k_sem_take(&ctx->ctx_lock, K_FOREVER); + if (is_associated(ctx) && !(mgmt_request == NET_REQUEST_IEEE802154_SET_SHORT_ADDR && value == IEEE802154_SHORT_ADDRESS_NOT_ASSOCIATED)) { - return -EBUSY; + ret = -EBUSY; + goto out; } if (mgmt_request == NET_REQUEST_IEEE802154_SET_CHANNEL) { if (ctx->channel != value) { if (!ieee802154_verify_channel(iface, value)) { - return -EINVAL; + ret = -EINVAL; + goto out; } ret = ieee802154_set_channel(iface, value); @@ -514,7 +567,8 @@ static int ieee802154_set_parameters(uint32_t mgmt_request, } } else if (mgmt_request == NET_REQUEST_IEEE802154_SET_EXT_ADDR) { if (len != IEEE802154_EXT_ADDR_LENGTH) { - return -EINVAL; + ret = -EINVAL; + goto out; } uint8_t ext_addr_le[IEEE802154_EXT_ADDR_LENGTH]; @@ -547,6 +601,8 @@ static int ieee802154_set_parameters(uint32_t mgmt_request, } } +out: + k_sem_give(&ctx->ctx_lock); return ret; } @@ -571,6 +627,7 @@ static int ieee802154_get_parameters(uint32_t mgmt_request, { struct ieee802154_context *ctx = net_if_l2_data(iface); uint16_t *value; + int ret = 0; if (mgmt_request != NET_REQUEST_IEEE802154_GET_EXT_ADDR && (len != sizeof(uint16_t) || !data)) { @@ -579,13 +636,16 @@ static int ieee802154_get_parameters(uint32_t mgmt_request, value = (uint16_t *)data; + k_sem_take(&ctx->ctx_lock, K_FOREVER); + if (mgmt_request == NET_REQUEST_IEEE802154_GET_CHANNEL) { *value = ctx->channel; } else if (mgmt_request == NET_REQUEST_IEEE802154_GET_PAN_ID) { *value = ctx->pan_id; } else if (mgmt_request == NET_REQUEST_IEEE802154_GET_EXT_ADDR) { if (len != IEEE802154_EXT_ADDR_LENGTH) { - return -EINVAL; + ret = -EINVAL; + goto out; } sys_memcpy_swap(data, ctx->ext_addr, IEEE802154_EXT_ADDR_LENGTH); @@ -597,7 +657,9 @@ static int ieee802154_get_parameters(uint32_t mgmt_request, *s_value = ctx->tx_power; } - return 0; +out: + k_sem_give(&ctx->ctx_lock); + return ret; } NET_MGMT_REGISTER_REQUEST_HANDLER(NET_REQUEST_IEEE802154_GET_CHANNEL, @@ -623,14 +685,18 @@ static int ieee802154_set_security_settings(uint32_t mgmt_request, { struct ieee802154_context *ctx = net_if_l2_data(iface); struct ieee802154_security_params *params; + int ret = 0; + + k_sem_take(&ctx->ctx_lock, K_FOREVER); if (is_associated(ctx)) { - return -EBUSY; + ret = -EBUSY; + goto out; } - if (len != sizeof(struct ieee802154_security_params) || - !data) { - return -EINVAL; + if (len != sizeof(struct ieee802154_security_params) || !data) { + ret = -EINVAL; + goto out; } params = (struct ieee802154_security_params *)data; @@ -639,10 +705,12 @@ static int ieee802154_set_security_settings(uint32_t mgmt_request, params->key_mode, params->key, params->key_len)) { NET_ERR("Could not set the security parameters"); - return -EINVAL; + ret = -EINVAL; } - return 0; +out: + k_sem_give(&ctx->ctx_lock); + return ret; } NET_MGMT_REGISTER_REQUEST_HANDLER(NET_REQUEST_IEEE802154_SET_SECURITY_SETTINGS, @@ -655,18 +723,21 @@ static int ieee802154_get_security_settings(uint32_t mgmt_request, struct ieee802154_context *ctx = net_if_l2_data(iface); struct ieee802154_security_params *params; - if (len != sizeof(struct ieee802154_security_params) || - !data) { + if (len != sizeof(struct ieee802154_security_params) || !data) { return -EINVAL; } params = (struct ieee802154_security_params *)data; + k_sem_take(&ctx->ctx_lock, K_FOREVER); + memcpy(params->key, ctx->sec_ctx.key, ctx->sec_ctx.key_len); params->key_len = ctx->sec_ctx.key_len; params->key_mode = ctx->sec_ctx.key_mode; params->level = ctx->sec_ctx.level; + k_sem_give(&ctx->ctx_lock); + return 0; } diff --git a/subsys/net/l2/ieee802154/ieee802154_mgmt_priv.h b/subsys/net/l2/ieee802154/ieee802154_mgmt_priv.h index e0818afc262ee..94f74800e873a 100644 --- a/subsys/net/l2/ieee802154/ieee802154_mgmt_priv.h +++ b/subsys/net/l2/ieee802154/ieee802154_mgmt_priv.h @@ -27,7 +27,7 @@ static inline void ieee802154_mgmt_init(struct net_if *iface) { struct ieee802154_context *ctx = net_if_l2_data(iface); - k_sem_init(&ctx->res_lock, 1, 1); + k_sem_init(&ctx->scan_ctx_lock, 1, 1); } enum net_verdict ieee802154_handle_beacon(struct net_if *iface, diff --git a/tests/net/ieee802154/l2/src/ieee802154_test.c b/tests/net/ieee802154/l2/src/ieee802154_test.c index 3526e14df8e9c..7ccbab8d08b65 100644 --- a/tests/net/ieee802154/l2/src/ieee802154_test.c +++ b/tests/net/ieee802154/l2/src/ieee802154_test.c @@ -249,6 +249,7 @@ static bool test_ns_sending(struct ieee802154_pkt_test *t, bool with_short_addr) static bool test_dgram_packet_sending(struct sockaddr_ll *pkt_sll, uint32_t security_level) { + /* tests should be run sequentially, so no need for context locking */ struct ieee802154_context *ctx = net_if_l2_data(iface); int fd; struct sockaddr_ll socket_sll = {0}; @@ -351,6 +352,7 @@ static bool test_dgram_packet_sending(struct sockaddr_ll *pkt_sll, uint32_t secu static bool test_raw_packet_sending(void) { + /* tests should be run sequentially, so no need for context locking */ struct ieee802154_context *ctx = net_if_l2_data(iface); int fd; struct sockaddr_ll socket_sll = {0}; From b6aacfae934726fffb9812b93cf86faca06d84d7 Mon Sep 17 00:00:00 2001 From: Florian Grandel Date: Sat, 1 Oct 2022 14:12:09 +0200 Subject: [PATCH 11/12] net: l2: ieee802154: remove struct padding in context This change re-orders attributes in struct ieee802154_context to completely optimize struct padding away. This is done in a way that does not impact readability. Signed-off-by: Florian Grandel --- include/zephyr/net/ieee802154.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/zephyr/net/ieee802154.h b/include/zephyr/net/ieee802154.h index 3a9db59569417..536dbfba70478 100644 --- a/include/zephyr/net/ieee802154.h +++ b/include/zephyr/net/ieee802154.h @@ -59,7 +59,6 @@ struct ieee802154_security_ctx { /* This not meant to be used by any code but 802.15.4 L2 stack */ struct ieee802154_context { - enum net_l2_flags flags; uint16_t pan_id; /* in CPU byte order */ uint16_t channel; /* short address: @@ -70,17 +69,18 @@ struct ieee802154_context { uint16_t short_addr; /* in CPU byte order */ uint8_t ext_addr[IEEE802154_MAX_ADDR_LENGTH]; /* in little endian */ struct net_linkaddr_storage linkaddr; /* in big endian */ +#ifdef CONFIG_NET_L2_IEEE802154_SECURITY + struct ieee802154_security_ctx sec_ctx; +#endif #ifdef CONFIG_NET_L2_IEEE802154_MGMT struct ieee802154_req_params *scan_ctx; /* guarded by scan_ctx_lock */ struct k_sem scan_ctx_lock; uint8_t coord_ext_addr[IEEE802154_MAX_ADDR_LENGTH]; /* in little endian */ uint16_t coord_short_addr; /* in CPU byte order */ -#endif -#ifdef CONFIG_NET_L2_IEEE802154_SECURITY - struct ieee802154_security_ctx sec_ctx; #endif int16_t tx_power; + enum net_l2_flags flags; uint8_t sequence; uint8_t ack_seq; /* guarded by ack_lock */ From 166f4ab193029b219c1779ee85ebe4364adc4624 Mon Sep 17 00:00:00 2001 From: Florian Grandel Date: Sat, 24 Sep 2022 10:19:01 +0200 Subject: [PATCH 12/12] net: l2: ieee802154: fix frame type field check The LLDN frame has been obsoleted in IEEE 802.15.4-2015f. This change removes it from the code, introduces frame types from current spec levels and updates the frame validation rules in accordance with the spec. Signed-off-by: Florian Grandel --- subsys/net/l2/ieee802154/ieee802154_frame.c | 20 +++++++++++++------- subsys/net/l2/ieee802154/ieee802154_frame.h | 7 ++++--- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/subsys/net/l2/ieee802154/ieee802154_frame.c b/subsys/net/l2/ieee802154/ieee802154_frame.c index 4de9d95d8971f..b89cfb4a334df 100644 --- a/subsys/net/l2/ieee802154/ieee802154_frame.c +++ b/subsys/net/l2/ieee802154/ieee802154_frame.c @@ -43,17 +43,23 @@ struct ieee802154_fcf_seq *ieee802154_validate_fc_seq(uint8_t *buf, uint8_t **p_ dbg_print_fs(fs); /** Basic FC checks */ - if (fs->fc.frame_type >= IEEE802154_FRAME_TYPE_RESERVED || + if (fs->fc.frame_type == IEEE802154_FRAME_TYPE_RESERVED || fs->fc.frame_version >= IEEE802154_VERSION_RESERVED) { return NULL; } - /** Only for versions 2003/2006 */ - if (fs->fc.frame_version < IEEE802154_VERSION_802154 && - (fs->fc.dst_addr_mode == IEEE802154_ADDR_MODE_RESERVED || - fs->fc.src_addr_mode == IEEE802154_ADDR_MODE_RESERVED || - fs->fc.frame_type >= IEEE802154_FRAME_TYPE_LLDN)) { - return NULL; + if (fs->fc.frame_type == IEEE802154_FRAME_TYPE_MULTIPURPOSE) { + if (fs->fc.frame_version != 0) { + return NULL; + } + } else { + /** Only for versions 2003/2006 */ + if (fs->fc.frame_version < IEEE802154_VERSION_802154 && + (fs->fc.dst_addr_mode == IEEE802154_ADDR_MODE_RESERVED || + fs->fc.src_addr_mode == IEEE802154_ADDR_MODE_RESERVED || + fs->fc.frame_type >= IEEE802154_FRAME_TYPE_RESERVED)) { + return NULL; + } } if (fs->fc.frame_type == IEEE802154_FRAME_TYPE_BEACON && diff --git a/subsys/net/l2/ieee802154/ieee802154_frame.h b/subsys/net/l2/ieee802154/ieee802154_frame.h index dc37f47990fe0..04b4a0d63a1b4 100644 --- a/subsys/net/l2/ieee802154/ieee802154_frame.h +++ b/subsys/net/l2/ieee802154/ieee802154_frame.h @@ -46,15 +46,16 @@ #define IEEE802154_BEACON_GTS_RX 1 #define IEEE802154_BEACON_GTS_TX 0 -/* See section 7.2.1.1.1 */ +/* See section 7.2.1.1.1 and IEEE 802.15.4-2020, section 7.2.2.2 */ enum ieee802154_frame_type { IEEE802154_FRAME_TYPE_BEACON = 0x0, IEEE802154_FRAME_TYPE_DATA = 0x1, IEEE802154_FRAME_TYPE_ACK = 0x2, IEEE802154_FRAME_TYPE_MAC_COMMAND = 0x3, - IEEE802154_FRAME_TYPE_LLDN = 0x4, + IEEE802154_FRAME_TYPE_RESERVED = 0x4, IEEE802154_FRAME_TYPE_MULTIPURPOSE = 0x5, - IEEE802154_FRAME_TYPE_RESERVED = 0x6, + IEEE802154_FRAME_TYPE_FRAK = 0x6, + IEEE802154_FRAME_TYPE_EXTENDED = 0x7, }; /* See section 7.2.1.1.6 */