Merge pull request #16090 from maribu/gnrc_tx_sync_frag_sfr

sys/net/gnrc: add gnrc_tx_sync for gnrc_sixlowpan_frag_sfr

sys/net/gnrc/Makefile.dep

@@ -229,10 +229,6 @@ ifneq (,$(filter gnrc_sixlowpan_frag_sfr,$(USEMODULE)))
   USEMODULE += gnrc_sixlowpan_frag_rb
   USEMODULE += evtimer
   USEMODULE += xtimer
-  ifneq (,$(filter gnrc_tx_sync,$(USEMODULE)))
-    # TODO: Implement gnrc_tx_sync for gnrc_sixlowpand_frag_sfr
-    $(error module gnrc_tx_sync conflicts with gnrc_sixlowpand_frag_sfr)
-  endif
 endif
 
 ifneq (,$(filter gnrc_sixlowpan_frag_sfr_stats,$(USEMODULE)))

sys/net/gnrc/network_layer/sixlowpan/frag/sfr/gnrc_sixlowpan_frag_sfr.c

@@ -31,6 +31,7 @@
 #include "net/gnrc/sixlowpan/frag/fb.h"
 #include "net/gnrc/sixlowpan/frag/rb.h"
 #include "net/gnrc/sixlowpan/frag/vrb.h"
+#include "net/gnrc/tx_sync.h"
 #include "net/sixlowpan/sfr.h"
 #include "thread.h"
 #include "unaligned.h"
@@ -143,12 +144,15 @@ static void _clean_up_fbuf(gnrc_sixlowpan_frag_fb_t *fbuf, int error);
  * @param[in] netif         Network interface to send fragment over
  * @param[in] fbuf          Fragmentation buffer for the datagram to fragment
  * @param[in] page          Current 6Lo dispatch parsing page.
+ * @param[in,out] tx_sync   Packet snip used to synchronize with transmission, if gnrc_tx_sync is
+ *                          used
  *
  * @return  Size of the fragment
  */
 static uint16_t _send_1st_fragment(gnrc_netif_t *netif,
                                    gnrc_sixlowpan_frag_fb_t *fbuf,
-                                   unsigned page);
+                                   unsigned page,
+                                   gnrc_pktsnip_t **tx_sync);
 
 /**
  * @brief   Send subsequent fragment.
@@ -156,12 +160,15 @@ static uint16_t _send_1st_fragment(gnrc_netif_t *netif,
  * @param[in] netif         Network interface to send fragment over
  * @param[in] fbuf          Fragmentation buffer for the datagram to fragment
  * @param[in] page          Current 6Lo dispatch parsing page.
+ * @param[in,out] tx_sync   Packet snip used to synchronize with transmission, if gnrc_tx_sync is
+ *                          used
  *
  * @return  Size of the fragment
  */
 static uint16_t _send_nth_fragment(gnrc_netif_t *netif,
                                    gnrc_sixlowpan_frag_fb_t *fbuf,
-                                   unsigned page);
+                                   unsigned page,
+                                   gnrc_pktsnip_t **tx_sync);
 
 /**
  * @brief   Send a abort pseudo fragment for datagram identified by @p tag
@@ -305,6 +312,7 @@ void gnrc_sixlowpan_frag_sfr_send(gnrc_pktsnip_t *pkt, void *ctx,
     gnrc_sixlowpan_frag_fb_t *fbuf = ctx;
     gnrc_netif_t *netif;
     int error_no = GNRC_NETERR_SUCCESS;
+    gnrc_pktsnip_t *tx_sync = NULL;
     uint16_t res;
 
     assert((fbuf != NULL) && ((fbuf->pkt == pkt) || (pkt == NULL)));
@@ -314,9 +322,13 @@ void gnrc_sixlowpan_frag_sfr_send(gnrc_pktsnip_t *pkt, void *ctx,
     netif = gnrc_netif_hdr_get_netif(pkt->data);
     assert(netif != NULL);
 
+    if (IS_USED(MODULE_GNRC_TX_SYNC)) {
+        tx_sync = gnrc_tx_sync_split(pkt);
+    }
+
     if (fbuf->offset == 0) {
         DEBUG("6lo sfr: sending first fragment\n");
-        res = _send_1st_fragment(netif, fbuf, page);
+        res = _send_1st_fragment(netif, fbuf, page, &tx_sync);
         if (res == 0) {
             DEBUG("6lo sfr: error sending first fragment\n");
             /* _send_1st_fragment only returns 0 if there is a memory problem */
@@ -330,7 +342,7 @@ void gnrc_sixlowpan_frag_sfr_send(gnrc_pktsnip_t *pkt, void *ctx,
     }
     else if (fbuf->offset < fbuf->datagram_size) {
         DEBUG("6lo sfr: sending subsequent fragment\n");
-        res = _send_nth_fragment(netif, fbuf, page);
+        res = _send_nth_fragment(netif, fbuf, page, &tx_sync);
         if (res == 0) {
             DEBUG("6lo sfr: error sending subsequent fragment (offset = %u)\n",
                   fbuf->offset);
@@ -370,6 +382,12 @@ void gnrc_sixlowpan_frag_sfr_send(gnrc_pktsnip_t *pkt, void *ctx,
         }
         _sched_arq_timeout(fbuf, fbuf->sfr.arq_timeout);
     }
+
+    if (IS_USED(MODULE_GNRC_TX_SYNC) && tx_sync) {
+        /* re-attach tx_sync to allow releasing it at end
+         * of transmission, or transmission failure */
+        gnrc_pkt_append(pkt, tx_sync);
+    }
     thread_yield();
     return;
 error:
@@ -383,6 +401,9 @@ error:
     else {
         _clean_up_fbuf(fbuf, error_no);
     }
+    if (IS_USED(MODULE_GNRC_TX_SYNC) && tx_sync) {
+        gnrc_pktbuf_release(tx_sync);
+    }
 }
 
 void gnrc_sixlowpan_frag_sfr_recv(gnrc_pktsnip_t *pkt, void *ctx,
@@ -1249,7 +1270,7 @@ static void _clean_up_fbuf(gnrc_sixlowpan_frag_fb_t *fbuf, int error)
 
 static uint16_t _send_1st_fragment(gnrc_netif_t *netif,
                                    gnrc_sixlowpan_frag_fb_t *fbuf,
-                                   unsigned page)
+                                   unsigned page, gnrc_pktsnip_t **tx_sync)
 {
     gnrc_pktsnip_t *frag, *pkt = fbuf->pkt;
     sixlowpan_sfr_rfrag_t *hdr;
@@ -1291,6 +1312,10 @@ static uint16_t _send_1st_fragment(gnrc_netif_t *netif,
     sixlowpan_sfr_rfrag_set_offset(hdr, fbuf->datagram_size);
     /* don't copy netif header of pkt => pkt->next */
     frag_size = _copy_pkt_to_frag(data, pkt->next, frag_size, 0);
+    if (IS_USED(MODULE_GNRC_TX_SYNC) && *tx_sync && (frag_size >= fbuf->datagram_size)) {
+        gnrc_pkt_append(frag, *tx_sync);
+        *tx_sync = NULL;
+    }
 
     DEBUG("6lo sfr: send first fragment (tag: %u, X: %i, seq: %u, "
           "frag_size: %u, datagram_size: %u)\n",
@@ -1306,7 +1331,8 @@ static uint16_t _send_1st_fragment(gnrc_netif_t *netif,
 
 static uint16_t _send_nth_fragment(gnrc_netif_t *netif,
                                    gnrc_sixlowpan_frag_fb_t *fbuf,
-                                   unsigned page)
+                                   unsigned page,
+                                   gnrc_pktsnip_t **tx_sync)
 {
     gnrc_pktsnip_t *frag, *pkt = fbuf->pkt;
     sixlowpan_sfr_rfrag_t *hdr;
@@ -1336,6 +1362,10 @@ static uint16_t _send_nth_fragment(gnrc_netif_t *netif,
                                               fbuf->offset);
     /* copy remaining packet snips */
     local_offset = _copy_pkt_to_frag(data, pkt, frag_size, local_offset);
+    if (IS_USED(MODULE_GNRC_TX_SYNC) && *tx_sync && (local_offset >= fbuf->datagram_size)) {
+        gnrc_pkt_append(frag, *tx_sync);
+        *tx_sync = NULL;
+    }
     DEBUG("6lo sfr: send subsequent fragment (tag: %u, X: %i, seq: %u, "
           "frag_size: %u, offset: %u)\n",
           hdr->base.tag, sixlowpan_sfr_rfrag_ack_req(hdr),

tests/gnrc_tx_sync/Makefile

@@ -1,10 +1,14 @@
 include ../Makefile.tests_common
 
 TEST_6LO ?= 1
+TEST_FRAG_SFR ?= 1
 
 ifeq (1,$(TEST_6LO))
   USEMODULE += gnrc_sixlowpan_default
   USEMODULE += netdev_ieee802154
+  ifeq (1,$(TEST_FRAG_SFR))
+    USEMODULE += gnrc_sixlowpan_frag_sfr
+  endif
 else
   USEMODULE += gnrc_ipv6_default
   USEMODULE += netdev_eth

tests/gnrc_tx_sync/Makefile.ci

@@ -5,21 +5,33 @@ BOARD_INSUFFICIENT_MEMORY := \
     arduino-nano \
     arduino-uno \
     atmega1281 \
+    atmega1284p \
     atmega328p \
     atmega328p-xplained-mini \
+    atxmega-a3bu-xplained \
     bluepill-stm32f030c8 \
+    derfmega128 \
+    hifive1 \
+    hifive1b \
     i-nucleo-lrwan1 \
+    im880b \
+    mega-xplained \
+    microduino-corerf \
     msb-430 \
     msb-430h \
     nucleo-f030r8 \
     nucleo-f031k6 \
     nucleo-f042k6 \
+    nucleo-f070rb \
+    nucleo-f072rb \
     nucleo-f303k8 \
     nucleo-f334r8 \
     nucleo-l011k4 \
     nucleo-l031k6 \
     nucleo-l053r8 \
     samd10-xmini \
+    saml10-xpro \
+    saml11-xpro \
     slstk3400a \
     stk3200 \
     stm32f030f4-demo \
@@ -29,4 +41,5 @@ BOARD_INSUFFICIENT_MEMORY := \
     telosb \
     waspmote-pro \
     z1 \
+    zigduino \
     #

tests/gnrc_tx_sync/main.c

@@ -42,33 +42,48 @@ static msg_t main_msg_queue[MAIN_QUEUE_SIZE];
 static atomic_int sends_completed = ATOMIC_VAR_INIT(0);
 static gnrc_netif_t netif;
 static netdev_test_t netdev_test;
-static netdev_t *netdev =
+static netdev_t *netdev = &netdev_test.netdev.netdev;
-#if IS_USED(MODULE_NETDEV_IEEE802154)
-    &netdev_test.netdev.netdev;
-#else
-    &netdev_test.netdev;
-#endif
 
-static bool carries_test_message(const iolist_t *iolist)
+/* With 6LoWPAN, This test message needs exactly two fragments to be transmitted
+ * due to the maximum L2 PDU of 96 bytes */
+static const char test_msg[] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTU"
+                               "VWXYZ0123456789.,:;!?@#$%^&*()[]{}-_=+/<>`~\'\""
+                               "\\";
+static unsigned pld_pos = 0;
+
+static bool carries_test_message(const iolist_t *iol)
 {
-    /* This is dark magic, don't do this at home.
+    /* Dark magic: We just assume that the test message will be placed at the
-     * We rely on two things here:
+     * end of one single iolist chunk and that fragments (if applicable) are
-     * 1.)  As we're testing gnrc_tx_sync, we can be sure that the used network stack is GNRC.
+     * send in order. We also assume that no more than four bytes in a row will
-     *      Hence, iolist actually is of type gnrc_pktsnip_t * and we can just cast it back
+     * never match the test message payload by chance.
-     * 2.)  There are only two types of messages being send in this test: ICMPv6 messages and
-     *      our UDP test message (or fragments of it).
      *
-     * Detecting (unfragmented) ICMPv6 messages is trivial, so we just do this. All other frames
+     * Note that this intentionally ignores retransmissions, which can occur in
-     * must be carrying our test message.
+     * the SFR case.
      */
-    for (const gnrc_pktsnip_t *iter = (const gnrc_pktsnip_t *)iolist; iter; iter = iter->next) {
-        if (iter->type == GNRC_NETTYPE_ICMPV6) {
-            return false;
-        }
-    }
 
+    while (iol) {
+        if ((iol->iol_base != NULL) && (iol->iol_len > 0)) {
+            const char *pos = iol->iol_base;
+            const char *end = pos + iol->iol_len;
+            while (pos < end) {
+                if (*pos == test_msg[pld_pos]) {
+                    size_t len = (size_t)end - (size_t)pos;
+                    if ((len > 4) &&
+                            (pld_pos + len <= sizeof(test_msg)) &&
+                            !memcmp(pos, &test_msg[pld_pos], len)) {
+                        /* data matches next chunk of test message */
+                        pld_pos += len;
                         return true;
                     }
+                }
+                pos++;
+            }
+        }
+        iol = iol->iol_next;
+    }
+    return false;
+}
 
 static int netdev_send(netdev_t *dev, const iolist_t *iolist)
 {
@@ -100,8 +115,8 @@ static int netdev_get_max_pdu_size(netdev_t *dev, void *value, size_t max_len)
 {
     (void)dev;
     const uint16_t pdu_size_ethernet = 1500;
-    const uint16_t pdu_size_6lo = 32;
+    const uint16_t pdu_size_6lo = 96;
-    expect(max_len == sizeof(uint16_t));
+    assert(max_len == sizeof(uint16_t));
     if (IS_USED(MODULE_NETDEV_IEEE802154)) {
         memcpy(value, &pdu_size_6lo, sizeof(pdu_size_6lo));
     }
@@ -180,7 +195,9 @@ int main(void)
     );
 
     if (IS_USED(MODULE_NETDEV_IEEE802154)) {
-        puts("IEEE 802.15.4 mode (TEST_6LO=1), sending 2 6LoWPAN fragments");
+        printf("IEEE 802.15.4 mode (TEST_6LO=1, TEST_FRAG_SFR=%u), "
+               "sending 2 6LoWPAN fragments",
+               (unsigned)IS_USED(MODULE_GNRC_SIXLOWPAN_FRAG_SFR));
     }
     else {
         puts("Ethernet mode (TEST_6LO=0), sending 1 IPv6 packet");
@@ -204,9 +221,6 @@ int main(void)
     ipv6_addr_set_all_nodes_multicast((ipv6_addr_t *)&remote.addr.ipv6,
                                       IPV6_ADDR_MCAST_SCP_LINK_LOCAL);
     expect(sock_udp_create(&sock, &local, NULL, 0) == 0);
-    /* With 6LoWPAN, This test message needs exactly two fragments to be transmitted due
-     * to the maximum L2 PDU of 32 bytes */
-    static const char test_msg[33] = { 'T', 'e', 's', 't' };
     /* with gnrc_tx_sync, we expect sock_udp_send() to block until transmission is done */
     expect(sock_udp_send(&sock, test_msg, sizeof(test_msg), &remote) > 0);
     /* the virtual netdev device increments sends_completed for each frame carrying the test
@@ -214,8 +228,11 @@ int main(void)
      * the test has failed. */
     int sends = atomic_load(&sends_completed);
     int sends_expected = (IS_USED(MODULE_NETDEV_IEEE802154)) ? 2 : 1;
-    printf("transmissions expected = %d, transmissions completed = %d\n", sends_expected, sends);
+    printf("transmissions expected = %d, transmissions completed = %d\n",
+           sends_expected, sends);
+    printf("sent %u out of %u bytes\n", pld_pos, (unsigned)sizeof(test_msg));
     expect(sends == sends_expected);
+    expect(pld_pos == sizeof(test_msg));
 
     puts("TEST PASSED");