net: l2: ieee802154: document L1/L2 sep. of concerns

The method ieee802154_radio_handle_ack() does not belong to the PHY/radio layer but to the L2 layer. It is a callback called from the radio layer into the L2 layer and to be implemented by all L2 stacks. This is the same pattern as is used for ieee802154_init(). The '_radio_' infix in this function is therefore confusing and conceptually wrong. This change fixes the naming inconsistency and extensively documents its rationale. It is assumed that the change can be made without prior deprecation of the existing method as in the rare cases where users have implemented custom radio drivers these will break in obvious ways and can easily be fixed. Nevertheless such a rename would not be justified on its own if it were not for an important conceptual reason: The renamed function represents a generic "inversion-of-control" pattern which will become important in the TSCH context: It allows for clean separation of concerns between the PHY/radio driver layer and the MAC/L2 layer even in situations where the radio driver needs to be involved for performance or deterministic timing reasons. This "inversion-of-control" pattern can be applied to negotiate timing sensitive reception and transmission windows, it let's the L2 layer deterministically timestamp information elements just-in-time with internal radio timer counter values, etc. Signed-off-by: Florian Grandel <fgrandel@code-for-humans.de>
Willmish · Jun 17, 2023 · 1ee4d3e · 1ee4d3e
1 parent ffcae5f
commit 1ee4d3e
Show file tree

Hide file tree

Showing 17 changed files with 95 additions and 43 deletions.
diff --git a/doc/connectivity/networking/api/net_l2.rst b/doc/connectivity/networking/api/net_l2.rst
@@ -113,8 +113,8 @@ IEEE 802.15.4 device driver
 ===========================
 
 Device drivers for IEEE 802.15.4 L2 work basically the same as for
-Ethernet.  What has been described above, especially for ``recv()``, applies
-here as well.  There are two specific differences however:
+Ethernet. What has been described above, especially for ``recv()``, applies
+here as well. There are two specific differences however:
 
 - It requires a dedicated device driver API: :c:struct:`ieee802154_radio_api`,
   which overloads :c:struct:`net_if_api`. This is because 802.15.4 L2 needs more from the device
@@ -123,24 +123,58 @@ here as well.  There are two specific differences however:
   IEEE 802.15.4 device driver must provide a valid pointer on such
   relevantly filled-in API structure.
 
-- Sending a packet is slightly different than in Ethernet. IEEE 802.15.4 sends
-  relatively small frames, 127 bytes all inclusive: frame header,
-  payload and frame checksum.  Buffers are meant to fit such
-  frame size limitation.  But a buffer containing an IPv6/UDP packet
-  might have more than one fragment. IEEE 802.15.4 drivers
-  handle only one buffer at a time.  This is why the
+- Sending a packet is slightly different than in Ethernet. Most IEEE 802.15.4
+  PHYs support relatively small frames only, 127 bytes all inclusive: frame
+  header, payload and frame checksum. Buffers to be sent over the radio will
+  often not fit this frame size limitation, e.g. a buffer containing an IPv6
+  packet will often have to be split into several fragments and IP6 packet headers
+  and fragments need to be compressed using a protocol like 6LoWPAN before being
+  passed on to the radio driver. Additionally the IEEE 802.15.4 standard defines
+  medium access (e.g. CSMA/CA), frame retransmission, encryption and other pre-
+  processing procedures (e.g. addition of information elements) that individual
+  radio drivers should not have to care about. This is why the
   :c:struct:`ieee802154_radio_api` requires a tx function pointer which differs
-  from the :c:struct:`net_if_api` send function pointer.
-  Instead, the IEEE 802.15.4 L2, provides a generic
-  :c:func:`ieee802154_radio_send` meant to be given as
-  :c:type:`net_if` send function. It turn, the implementation
-  of :c:func:`ieee802154_radio_send` will ensure the same behavior:
-  sending one buffer at a time through :c:type:`ieee802154_radio_api` tx
-  function, and unreferencing the network packet
-  only when all the transmission were successful.
-
-Each IEEE 802.15.4 device driver, in the end, will need to call
-``NET_DEVICE_INIT_INSTANCE()`` that way:
+  from the :c:struct:`net_if_api` send function pointer. Zephyr's native
+  IEEE 802.15.4 L2 implementation provides a generic :c:func:`ieee802154_send`
+  instead, meant to be given as :c:type:`net_if` send function. The implementation
+  of :c:func:`ieee802154_send` takes care of IEEE 802.15.4 standard packet
+  preparation procedures, splitting the packet into possibly compressed,
+  encrypted and otherwise pre-processed fragment buffers, sending one buffer
+  at a time through :c:type:`ieee802154_radio_api` tx function and unreferencing
+  the network packet only when the transmission as a whole was either successful
+  or failed.
+
+Interaction between IEEE 802.15.4 radio device drivers and L2 is bidirectional:
+
+- L2 -> L1: Methods as :c:func:`ieee802154_send` and several IEEE 802.15.4 net
+  management calls will call into the driver, e.g. to send a packet over the
+  radio link or re-configure the driver at runtime. These incoming calls will
+  all be handled by the methods in the :c:type:`ieee802154_radio_api`.
+
+- L1 -> L2: There are several situations in which the driver needs to initiate
+  calls into the L2/MAC layer. Zephyr's IEEE 802.15.4 L1 -> L2 adaptation API
+  employs an "inversion-of-control" pattern in such cases avoids duplication of
+  complex logic across independent driver implementations and ensures
+  implementation agnostic loose coupling and clean separation of concerns between
+  MAC (L2) and PHY (L1) whenever reverse information transfer or close co-operation
+  between hardware and L2 is required. During driver initialization, for example,
+  the driver calls :c:func:`ieee802154_init` to pass the interface's MAC address
+  as well as other hardware-related configuration to L2. Similarly, drivers may
+  indicate performance or timing critical radio events to L2 that require close
+  integration with the hardware (e.g. :c:func:`ieee802154_handle_ack`). Calls
+  from L1 into L2 are not implemented as methods in :c:type:`ieee802154_radio_api`
+  but are standalone functions declared and documented as such in
+  :zephyr_file:`include/zephyr/net/ieee802154_radio.h`. The API documentation will
+  clearly state which functions must be implemented by all L2 stacks as part
+  of the L1 -> L2 "inversion-of-control" adaptation API.
+
+Note: Standalone functions in :zephyr_file:`include/zephyr/net/ieee802154_radio.h`
+that are not explicitly documented as callbacks are considered to be helper functions
+within the PHY (L1) layer implemented independently of any specific L2 stack, see for
+example :c:func:`ieee802154_is_ar_flag_set`.
+
+As all net interfaces, IEEE 802.15.4 device driver implementations will have to call
+``NET_DEVICE_INIT_INSTANCE()`` in the end:
 
 .. code-block:: c
 

diff --git a/drivers/ieee802154/ieee802154_b91.c b/drivers/ieee802154/ieee802154_b91.c
@@ -227,7 +227,7 @@ static void b91_handle_ack(void)
 	net_pkt_cursor_init(ack_pkt);
 
 	/* handle ack */
-	if (ieee802154_radio_handle_ack(data.iface, ack_pkt) != NET_OK) {
+	if (ieee802154_handle_ack(data.iface, ack_pkt) != NET_OK) {
 		LOG_INF("ACK packet not handled - releasing.");
 	}
 

diff --git a/drivers/ieee802154/ieee802154_cc1200.c b/drivers/ieee802154/ieee802154_cc1200.c
@@ -486,7 +486,7 @@ static void cc1200_rx(void *arg)
 			goto out;
 		}
 
-		if (ieee802154_radio_handle_ack(cc1200->iface, pkt) == NET_OK) {
+		if (ieee802154_handle_ack(cc1200->iface, pkt) == NET_OK) {
 			LOG_DBG("ACK packet handled");
 			goto out;
 		}

diff --git a/drivers/ieee802154/ieee802154_cc2520.c b/drivers/ieee802154/ieee802154_cc2520.c
@@ -637,7 +637,7 @@ static void cc2520_rx(void *arg)
 			goto out;
 		}
 
-		if (ieee802154_radio_handle_ack(cc2520->iface, pkt) == NET_OK) {
+		if (ieee802154_handle_ack(cc2520->iface, pkt) == NET_OK) {
 			LOG_DBG("ACK packet handled");
 			goto out;
 		}

diff --git a/drivers/ieee802154/ieee802154_dw1000.c b/drivers/ieee802154/ieee802154_dw1000.c
@@ -491,7 +491,7 @@ static inline void dwt_irq_handle_rx(const struct device *dev, uint32_t sys_stat
 		flags_to_clear |= DWT_SYS_STATUS_AAT;
 	}
 
-	if (ieee802154_radio_handle_ack(ctx->iface, pkt) == NET_OK) {
+	if (ieee802154_handle_ack(ctx->iface, pkt) == NET_OK) {
 		LOG_INF("ACK packet handled");
 		goto rx_out_unref_pkt;
 	}

diff --git a/drivers/ieee802154/ieee802154_kw41z.c b/drivers/ieee802154/ieee802154_kw41z.c
@@ -606,7 +606,7 @@ static void handle_ack(struct kw41z_context *kw41z, uint8_t seq_number)
 
 	net_pkt_cursor_init(ack_pkt);
 
-	if (ieee802154_radio_handle_ack(kw41z->iface, ack_pkt) != NET_OK) {
+	if (ieee802154_handle_ack(kw41z->iface, ack_pkt) != NET_OK) {
 		LOG_INF("ACK packet not handled - releasing.");
 	}
 

diff --git a/drivers/ieee802154/ieee802154_mcr20a.c b/drivers/ieee802154/ieee802154_mcr20a.c
@@ -581,7 +581,7 @@ static inline void mcr20a_rx(const struct device *dev, uint8_t len)
 		goto out;
 	}
 
-	if (ieee802154_radio_handle_ack(mcr20a->iface, pkt) == NET_OK) {
+	if (ieee802154_handle_ack(mcr20a->iface, pkt) == NET_OK) {
 		LOG_DBG("ACK packet handled");
 		goto out;
 	}

diff --git a/drivers/ieee802154/ieee802154_nrf5.c b/drivers/ieee802154/ieee802154_nrf5.c
@@ -412,7 +412,7 @@ static int handle_ack(struct nrf5_802154_data *nrf5_radio)
 
 	net_pkt_cursor_init(ack_pkt);
 
-	if (ieee802154_radio_handle_ack(nrf5_radio->iface, ack_pkt) != NET_OK) {
+	if (ieee802154_handle_ack(nrf5_radio->iface, ack_pkt) != NET_OK) {
 		LOG_INF("ACK packet not handled - releasing.");
 	}
 

diff --git a/drivers/ieee802154/ieee802154_rf2xx.c b/drivers/ieee802154/ieee802154_rf2xx.c
@@ -629,7 +629,7 @@ static void rf2xx_handle_ack(struct rf2xx_context *ctx, struct net_buf *frag)
 
 	net_pkt_cursor_init(&rf2xx_ack_pkt);
 
-	if (ieee802154_radio_handle_ack(ctx->iface, &rf2xx_ack_pkt) != NET_OK) {
+	if (ieee802154_handle_ack(ctx->iface, &rf2xx_ack_pkt) != NET_OK) {
 		LOG_INF("ACK packet not handled.");
 	}
 }

diff --git a/drivers/ieee802154/ieee802154_uart_pipe.c b/drivers/ieee802154/ieee802154_uart_pipe.c
@@ -154,7 +154,7 @@ static uint8_t *upipe_rx(uint8_t *buf, size_t *off)
 		}
 #endif
 
-		if (ieee802154_radio_handle_ack(upipe->iface, pkt) == NET_OK) {
+		if (ieee802154_handle_ack(upipe->iface, pkt) == NET_OK) {
 			LOG_DBG("ACK packet handled");
 			goto out;
 		}

diff --git a/include/zephyr/net/ieee802154_radio.h b/include/zephyr/net/ieee802154_radio.h
@@ -455,26 +455,45 @@ static inline bool ieee802154_is_ar_flag_set(struct net_buf *frag)
 }
 
 /**
- * @brief Radio driver ACK handling function that hw drivers should use
+ * @brief Radio driver ACK handling callback into L2 that radio
+ *        drivers must call when receiving an ACK package.
  *
- * @details ACK handling requires fast handling and thus such function
- *          helps to hook directly the hw drivers to the radio driver.
+ * @details The IEEE 802.15.4 standard prescribes generic procedures for ACK
+ *          handling on L2 (MAC) level. L2 stacks therefore have to provides a
+ *          fast and re-usable generic implementation of this callback for
+ *          radio drivers to call when receiving an ACK packet.
+ *
+ *          Note: This function is part of Zephyr's 802.15.4 stack L1 -> L2
+ *          "inversion-of-control" adaptation API and must be implemented by
+ *          all IEEE 802.15.4 L2 stacks.
  *
  * @param iface A valid pointer on a network interface that received the packet
  * @param pkt A valid pointer on a packet to check
  *
- * @return NET_OK if it was handled, NET_CONTINUE otherwise
+ * @return NET_OK if L2 handles the ACK package, NET_CONTINUE or NET_DROP otherwise.
+ *
+ *         Note: Deviating from other functions in the net stack returning net_verdict,
+ *               this function will not unref the package even if it returns NET_OK.
+ *
+ *         TODO: Fix this deviating behavior.
  */
-extern enum net_verdict ieee802154_radio_handle_ack(struct net_if *iface,
-						    struct net_pkt *pkt);
+extern enum net_verdict ieee802154_handle_ack(struct net_if *iface, struct net_pkt *pkt);
 
 /**
- * @brief Initialize L2 stack for a given interface
+ * @brief Radio driver initialization callback into L2 called by radio drivers
+ *        to initialize the active L2 stack for a given interface.
+ *
+ * @details Radio drivers must call this function as part of their own
+ *          initialization routine.
+ *
+ *          Note: This function is part of Zephyr's 802.15.4 stack L1 -> L2
+ *          "inversion-of-control" adaptation API and must be implemented by
+ *          all IEEE 802.15.4 L2 stacks.
  *
  * @param iface A valid pointer on a network interface
  */
 #ifndef CONFIG_IEEE802154_RAW_MODE
-void ieee802154_init(struct net_if *iface);
+extern void ieee802154_init(struct net_if *iface);
 #else
 #define ieee802154_init(_iface_)
 #endif /* CONFIG_IEEE802154_RAW_MODE */

diff --git a/modules/openthread/platform/radio.c b/modules/openthread/platform/radio.c
@@ -154,8 +154,7 @@ void energy_detected(const struct device *dev, int16_t max_ed)
 	}
 }
 
-enum net_verdict ieee802154_radio_handle_ack(struct net_if *iface,
-					     struct net_pkt *pkt)
+enum net_verdict ieee802154_handle_ack(struct net_if *iface, struct net_pkt *pkt)
 {
 	ARG_UNUSED(iface);
 

diff --git a/samples/net/wpan_serial/src/main.c b/samples/net/wpan_serial/src/main.c
@@ -521,7 +521,7 @@ int net_recv_data(struct net_if *iface, struct net_pkt *pkt)
 	return 0;
 }
 
-enum net_verdict ieee802154_radio_handle_ack(struct net_if *iface, struct net_pkt *pkt)
+enum net_verdict ieee802154_handle_ack(struct net_if *iface, struct net_pkt *pkt)
 {
 	return NET_CONTINUE;
 }

diff --git a/samples/net/wpanusb/src/wpanusb.c b/samples/net/wpanusb/src/wpanusb.c
@@ -414,7 +414,7 @@ int net_recv_data(struct net_if *iface, struct net_pkt *pkt)
 	return ret;
 }
 
-enum net_verdict ieee802154_radio_handle_ack(struct net_if *iface, struct net_pkt *pkt)
+enum net_verdict ieee802154_handle_ack(struct net_if *iface, struct net_pkt *pkt)
 {
 	return NET_CONTINUE;
 }

diff --git a/subsys/net/l2/ieee802154/ieee802154.c b/subsys/net/l2/ieee802154/ieee802154.c
@@ -113,7 +113,7 @@ inline bool ieee802154_prepare_for_ack(struct ieee802154_context *ctx, struct ne
 	return false;
 }
 
-enum net_verdict ieee802154_radio_handle_ack(struct net_if *iface, struct net_pkt *pkt)
+enum net_verdict ieee802154_handle_ack(struct net_if *iface, struct net_pkt *pkt)
 {
 	struct ieee802154_context *ctx = net_if_l2_data(iface);
 

diff --git a/tests/net/ieee802154/l2/src/ieee802154_test.c b/tests/net/ieee802154/l2/src/ieee802154_test.c
@@ -482,7 +482,7 @@ static bool test_wait_for_ack(struct ieee802154_pkt_test *t)
 
 	pkt_hexdump(net_pkt_data(ack_pkt), net_pkt_get_len(ack_pkt));
 
-	if (ieee802154_radio_handle_ack(iface, ack_pkt) != NET_OK) {
+	if (ieee802154_handle_ack(iface, ack_pkt) != NET_OK) {
 		NET_ERR("*** Ack frame was not handled.\n");
 		goto release_ack_pkt;
 	}

diff --git a/tests/subsys/openthread/radio_test.c b/tests/subsys/openthread/radio_test.c
@@ -245,7 +245,7 @@ static void create_ack_frame(void)
 
 	net_pkt_set_ieee802154_rssi_dbm(packet, rssi);
 	net_pkt_set_ieee802154_lqi(packet, lqi);
-	zassert_equal(ieee802154_radio_handle_ack(NULL, packet), NET_OK, "Handling ack failed.");
+	zassert_equal(ieee802154_handle_ack(NULL, packet), NET_OK, "Handling ack failed.");
 	net_pkt_unref(packet);
 }