Merge pull request #5281 from authmillenon/gnrc_ipv6/fix/ext-fix-with-nhc

gnrc_ipv6: fix asserts for NHC

sys/net/gnrc/network_layer/ipv6/ext/gnrc_ipv6_ext.c

@@ -89,6 +89,56 @@ static enum gnrc_ipv6_ext_demux_status _handle_rh(gnrc_pktsnip_t *current, gnrc_
 
 #endif
 
+/**
+ * @brief marks IPv6 extension header if needed.
+ *   updates pkt and returns next header.
+ * @param[in] current  The current header
+ * @param[in,out] pkt  The whole packet
+ * @return The next header
+ * @return NULL on error
+ */
+static gnrc_pktsnip_t *_mark_extension_header(gnrc_pktsnip_t *current,
+                                              gnrc_pktsnip_t **pkt)
+{
+    gnrc_pktsnip_t *ext_snip, *tmp, *next;
+    ipv6_ext_t *ext = (ipv6_ext_t *) current->data;
+    size_t offset = ((ext->len * IPV6_EXT_LEN_UNIT) + IPV6_EXT_LEN_UNIT);
+
+    if (current == *pkt) {
+        if ((tmp = gnrc_pktbuf_start_write(*pkt)) == NULL) {
+            DEBUG("ipv6: could not get a copy of pkt\n");
+            gnrc_pktbuf_release(*pkt);
+            return NULL;
+        }
+        *pkt = tmp;
+
+        ext_snip = gnrc_pktbuf_mark(*pkt, offset, GNRC_NETTYPE_IPV6_EXT);
+        next = *pkt;
+
+        if (ext_snip == NULL) {
+            gnrc_pktbuf_release(*pkt);
+            return NULL;
+        }
+    }
+    else {
+        /* the header is already marked */
+
+        next = NULL;
+
+        for (tmp = *pkt; tmp != NULL; tmp = tmp->next) {
+            if (tmp->next == current) {
+                next = tmp;
+                break;
+            }
+        }
+
+        assert(next != NULL);
+    }
+
+    return next;
+}
+
+
 /*
  *         current                 pkt
  *         |                       |
@@ -100,10 +150,9 @@ void gnrc_ipv6_ext_demux(kernel_pid_t iface,
                          gnrc_pktsnip_t *pkt,
                          uint8_t nh)
 {
-    gnrc_pktsnip_t *ext_snip, *tmp, *next;
     ipv6_ext_t *ext;
-    size_t offset = 0;
 
+    while (true) {
         ext = (ipv6_ext_t *) current->data;
 
         switch (nh) {
@@ -114,14 +163,21 @@ void gnrc_ipv6_ext_demux(kernel_pid_t iface,
                         /* We are the final destination. So proceeds like normal packet. */
                         nh = ext->nh;
                         DEBUG("ipv6_ext: next header = %" PRIu8 "\n", nh);
-                    offset = ((ext->len * IPV6_EXT_LEN_UNIT) + IPV6_EXT_LEN_UNIT);
+
-                    break;
+                        if ((current = _mark_extension_header(current, &pkt)) == NULL) {
+                            return;
+                        }
+
+                        gnrc_ipv6_demux(iface, current, pkt, nh); /* demultiplex next header */
+
+                        return;
 
                     case GNRC_IPV6_EXT_ERROR:
                         /* already released by _handle_rh, so no release here */
                         return;
 
                     case GNRC_IPV6_EXT_FORWARDED:
+                        /* the packet is forwarded and released. finish processing */
                         return;
                 }
 
@@ -137,49 +193,25 @@ void gnrc_ipv6_ext_demux(kernel_pid_t iface,
                 /* TODO: add handling of types */
                 nh = ext->nh;
                 DEBUG("ipv6_ext: next header = %" PRIu8 "\n", nh);
-            offset = ((ext->len * IPV6_EXT_LEN_UNIT) + IPV6_EXT_LEN_UNIT);
+
+                if ((current = _mark_extension_header(current, &pkt)) == NULL) {
+                    return;
+                }
+
+                gnrc_pktbuf_hold(pkt, 1);   /* don't release on next dispatch */
+                if (gnrc_netapi_dispatch_receive(GNRC_NETTYPE_IPV6, nh, pkt) == 0) {
+                    gnrc_pktbuf_release(pkt);
+                }
+
                 break;
 
             default:
-            DEBUG("ipv6_ext: unknown next header: %" PRIu8 "\n", nh);
+                gnrc_ipv6_demux(iface, current, pkt, nh); /* demultiplex next header */
-            gnrc_pktbuf_release(pkt);
-            return;
-    }
-
-    if (current == pkt) {
-        if ((tmp = gnrc_pktbuf_start_write(pkt)) == NULL) {
-            DEBUG("ipv6: could not get a copy of pkt\n");
-            gnrc_pktbuf_release(pkt);
-            return;
-        }
-        pkt = tmp;
-
-        ext_snip = gnrc_pktbuf_mark(pkt, offset, GNRC_NETTYPE_IPV6_EXT);
-        next = pkt;
-
-        if (ext_snip == NULL) {
-            gnrc_pktbuf_release(pkt);
                 return;
         }
     }
-    else {
-        /* the header is already marked */
 
-        next = NULL;
+    assert(false); /* never reaches here */
-
-        for (tmp = pkt; tmp != NULL; tmp = tmp->next) {
-            if (tmp->next == current) {
-                next = tmp;
-                break;
-            }
-        }
-
-        assert(next != NULL);
-    }
-
-    gnrc_ipv6_demux(iface, next, pkt, nh);    /* demultiplex next header */
-
-    return;
 }
 
 gnrc_pktsnip_t *gnrc_ipv6_ext_build(gnrc_pktsnip_t *ipv6, gnrc_pktsnip_t *next,

sys/net/gnrc/network_layer/ipv6/gnrc_ipv6.c

@@ -69,9 +69,6 @@ kernel_pid_t gnrc_ipv6_pid = KERNEL_PID_UNDEF;
 
 /* handles GNRC_NETAPI_MSG_TYPE_RCV commands */
 static void _receive(gnrc_pktsnip_t *pkt);
-/* dispatches received IPv6 packet for upper layer */
-static void _dispatch_rcv_pkt(gnrc_nettype_t type, uint32_t demux_ctx,
-                              gnrc_pktsnip_t *pkt);
 /* Sends packet over the appropriate interface(s).
  * prep_hdr: prepare header for sending (call to _fill_ipv6_hdr()), otherwise
  * assume it is already prepared */
@@ -100,6 +97,9 @@ kernel_pid_t gnrc_ipv6_init(void)
     return gnrc_ipv6_pid;
 }
 
+static void _dispatch_next_header(gnrc_pktsnip_t *current, gnrc_pktsnip_t *pkt,
+                                  uint8_t nh, bool interested);
+
 /*
  *         current                 pkt
  *         |                       |
@@ -108,10 +108,9 @@ kernel_pid_t gnrc_ipv6_init(void)
  */
 void gnrc_ipv6_demux(kernel_pid_t iface, gnrc_pktsnip_t *current, gnrc_pktsnip_t *pkt, uint8_t nh)
 {
-    int receiver_num;
     bool interested = false;
 
-    pkt->type = gnrc_nettype_from_protnum(nh);
+    current->type = gnrc_nettype_from_protnum(nh);
 
     switch (nh) {
 #ifdef MODULE_GNRC_ICMPV6
@@ -139,48 +138,38 @@ void gnrc_ipv6_demux(kernel_pid_t iface, gnrc_pktsnip_t *current, gnrc_pktsnip_t
             break;
         default:
             (void)iface;
+#ifdef MODULE_GNRC_SIXLOWPAN_IPHC_NHC
+            /* second statement is true for small 6LoWPAN NHC decompressed frames
+             * since in this case it looks like
+             *
+             * * GNRC_NETTYPE_UNDEF <- pkt
+             * v
+             * * GNRC_NETTYPE_UDP <- current
+             * v
+             * * GNRC_NETTYPE_EXT
+             * v
+             * * GNRC_NETTYPE_IPV6
+             */
+            assert((current == pkt) || (current == pkt->next));
+#else
+            assert(current == pkt);
+#endif
             break;
     }
 
-    DEBUG("ipv6: forward nh = %u to other threads\n", nh);
+    _dispatch_next_header(current, pkt, nh, interested);
-    receiver_num = gnrc_netreg_num(pkt->type, GNRC_NETREG_DEMUX_CTX_ALL) +
-                   gnrc_netreg_num(GNRC_NETTYPE_IPV6, nh);
-
-    if (receiver_num == 0) {
-        DEBUG("ipv6: unable to forward packet as no one is interested in it\n");
-
-        if (!interested) {
-            assert(current == pkt);
-            gnrc_pktbuf_release(pkt);
-            return;
-        }
-    }
-    else {
-        if (!interested) {
-            assert(current == pkt);
-            /* IPv6 is not interested anymore so `- 1` */
-            receiver_num--;
-        }
-
-        gnrc_pktbuf_hold(current, receiver_num);
-
-        /* XXX can't use gnrc_netapi_dispatch_receive() twice here since a call to that function
-         *     implicitly hands all rights to the packet to one of the receiving threads. As a
-         *     result, the second call to gnrc_netapi_dispatch_receive() would be invalid */
-        _dispatch_rcv_pkt(current->type, GNRC_NETREG_DEMUX_CTX_ALL, current);
-        _dispatch_rcv_pkt(GNRC_NETTYPE_IPV6, nh, current);
 
     if (!interested) {
         return;
     }
-    }
 
     switch (nh) {
 #ifdef MODULE_GNRC_ICMPV6
         case PROTNUM_ICMPV6:
             DEBUG("ipv6: handle ICMPv6 packet (nh = %u)\n", nh);
             gnrc_icmpv6_demux(iface, pkt);
-        break;
+            gnrc_pktbuf_release(pkt);
+            return;
 #endif
 #ifdef MODULE_GNRC_IPV6_EXT
         case PROTNUM_IPV6_EXT_HOPOPT:
@@ -201,14 +190,54 @@ void gnrc_ipv6_demux(kernel_pid_t iface, gnrc_pktsnip_t *current, gnrc_pktsnip_t
             _decapsulate(pkt);
             return;
         default:
+            assert(false);
             break;
     }
 
-    assert(current == pkt);
+    assert(false);
-    gnrc_pktbuf_release(pkt);
 }
 
 /* internal functions */
+static void _dispatch_next_header(gnrc_pktsnip_t *current, gnrc_pktsnip_t *pkt,
+                                  uint8_t nh, bool interested)
+{
+#ifdef MODULE_GNRC_IPV6_EXT
+    const bool should_dispatch_current_type = ((current->type != GNRC_NETTYPE_IPV6_EXT) ||
+                                               (current->next->type == GNRC_NETTYPE_IPV6));
+#else
+    const bool should_dispatch_current_type = (current->next->type == GNRC_NETTYPE_IPV6);
+#endif
+
+    DEBUG("ipv6: forward nh = %u to other threads\n", nh);
+
+    /* dispatch IPv6 extension header only once */
+    if (should_dispatch_current_type) {
+        bool should_release = (gnrc_netreg_num(GNRC_NETTYPE_IPV6, nh) == 0) &&
+                              (!interested);
+
+        if (!should_release) {
+            gnrc_pktbuf_hold(pkt, 1);   /* don't remove from packet buffer in
+                                         * next dispatch */
+        }
+        if (gnrc_netapi_dispatch_receive(current->type,
+                                         GNRC_NETREG_DEMUX_CTX_ALL,
+                                         pkt) == 0) {
+            gnrc_pktbuf_release(pkt);
+        }
+
+        if (should_release) {
+            return;
+        }
+    }
+    if (interested) {
+        gnrc_pktbuf_hold(pkt, 1);   /* don't remove from packet buffer in
+                                     * next dispatch */
+    }
+    if (gnrc_netapi_dispatch_receive(GNRC_NETTYPE_IPV6, nh, pkt) == 0) {
+        gnrc_pktbuf_release(pkt);
+    }
+}
+
 static void *_event_loop(void *args)
 {
     msg_t msg, reply, msg_q[GNRC_IPV6_MSG_QUEUE_SIZE];
@@ -747,22 +776,6 @@ static inline bool _pkt_not_for_me(kernel_pid_t *iface, ipv6_hdr_t *hdr)
     }
 }
 
-static void _dispatch_rcv_pkt(gnrc_nettype_t type, uint32_t demux_ctx,
-                              gnrc_pktsnip_t *pkt)
-{
-    gnrc_netreg_entry_t *entry = gnrc_netreg_lookup(type, demux_ctx);
-
-    while (entry) {
-        DEBUG("ipv6: Send receive command for %p to %" PRIu16 "\n", (void *)pkt,
-              entry->pid);
-        if (gnrc_netapi_receive(entry->pid, pkt) < 1) {
-            DEBUG("ipv6: unable to deliver packet\n");
-            gnrc_pktbuf_release(pkt);
-        }
-        entry = gnrc_netreg_getnext(entry);
-    }
-}
-
 static void _receive(gnrc_pktsnip_t *pkt)
 {
     kernel_pid_t iface = KERNEL_PID_UNDEF;
@@ -873,7 +886,7 @@ static void _receive(gnrc_pktsnip_t *pkt)
          * links."
          */
         if ((ipv6_addr_is_link_local(&(hdr->src))) || (ipv6_addr_is_link_local(&(hdr->dst)))) {
-            DEBUG("ipv6: do not forward packets with link-local source or"\
+            DEBUG("ipv6: do not forward packets with link-local source or"
                   " destination address\n");
             gnrc_pktbuf_release(pkt);
             return;

tests/gnrc_ipv6_ext/Makefile

@@ -21,9 +21,10 @@ USEMODULE += gnrc_ipv6_router_default
 # IPv6 extension headers
 USEMODULE += gnrc_ipv6_ext
 USEMODULE += gnrc_rpl_srh
+USEMODULE += gnrc_sixlowpan_iphc_nhc
+# UDP
+USEMODULE += gnrc_udp
 # Add also the shell, some shell commands
-USEMODULE += shell
-USEMODULE += shell_commands
 USEMODULE += ps
 
 # Comment this out to disable code in RIOT that does safety checking

tests/gnrc_ipv6_ext/main.c

@@ -30,11 +30,10 @@
 #include "net/gnrc/netreg.h"
 #include "net/gnrc/netapi.h"
 #include "net/gnrc/netif.h"
+#include "net/gnrc/netif/hdr.h"
 
-#define MAIN_QUEUE_SIZE     (8)
+static void _init_interface(void)
-static msg_t _main_msg_queue[MAIN_QUEUE_SIZE];
+{
-
-static void _send_packet(void) {
     kernel_pid_t ifs[GNRC_NETIF_NUMOF];
     ipv6_addr_t addr = IPV6_ADDR_UNSPECIFIED;
 
@@ -49,11 +48,24 @@ static void _send_packet(void) {
     addr.u8[15] = 0x03;
     /* fd01::03 */
     gnrc_ipv6_netif_add_addr(ifs[0], &addr, 64, GNRC_IPV6_NETIF_ADDR_FLAGS_UNICAST);
+}
+
+static void _send_packet_raw(void)
+{
+    kernel_pid_t ifs[GNRC_NETIF_NUMOF];
+
+    gnrc_netif_get(ifs);
+
+    gnrc_netif_hdr_t netif_hdr;
+
+    gnrc_netif_hdr_init(&netif_hdr, 8, 8);
+
+    netif_hdr.if_pid = ifs[0];
 
     uint8_t data[] = {
         /* IPv6 Header */
         0x60, 0x00, 0x00, 0x00, /* version, traffic class, flow label */
-        0x00, 0x28, /* payload length: 40 */
+        0x00, 0x2a,             /* payload length: 42 */
         0x00,                   /* next header: Hop-by-Hop Option */
         0x10,                   /* hop limit: 16 */
         /* source address: fd01::1 */
@@ -94,11 +106,19 @@ static void _send_packet(void) {
         /* UDP (ignored) */
         0x1f, 0x90, /* source port: 8080 */
         0x1f, 0x90, /* destination port: 8080 */
-        0x00, 0x08, /* length: 8 */
+        0x00, 0x0a, /* length: 10 */
         0xff, 0xff, /* checksum */
+        0x00, 0x00, /* payload */
     };
 
-    gnrc_pktsnip_t *pkt = gnrc_pktbuf_add(NULL, data, sizeof(data), GNRC_NETTYPE_UNDEF);
+    gnrc_pktsnip_t *netif = gnrc_pktbuf_add(NULL,
+                                            &netif_hdr,
+                                            sizeof(netif_hdr),
+                                            GNRC_NETTYPE_NETIF);
+    gnrc_pktsnip_t *pkt = gnrc_pktbuf_add(netif,
+                                          data,
+                                          sizeof(data),
+                                          GNRC_NETTYPE_UNDEF);
 
     gnrc_netapi_dispatch_receive(GNRC_NETTYPE_IPV6, GNRC_NETREG_DEMUX_CTX_ALL, pkt);
 
@@ -106,19 +126,121 @@ static void _send_packet(void) {
     assert(pkt->users == 0);
 }
 
+static void _send_packet_parsed(void)
+{
+    kernel_pid_t ifs[GNRC_NETIF_NUMOF];
+
+    gnrc_netif_get(ifs);
+
+    gnrc_netif_hdr_t netif_hdr;
+
+    gnrc_netif_hdr_init(&netif_hdr, 8, 8);
+
+    netif_hdr.if_pid = ifs[0];
+
+    uint8_t ipv6_data[] = {
+        /* IPv6 Header */
+        0x60, 0x00, 0x00, 0x00, /* version, traffic class, flow label */
+        0x00, 0x2a,             /* payload length: 42 */
+        0x00,                   /* next header: Hop-by-Hop Option */
+        0x10,                   /* hop limit: 16 */
+        /* source address: fd01::1 */
+        0xfd, 0x01, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x01,
+        /* destination address: fd01::2 */
+        0xfd, 0x01, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x02,
+    };
+
+    uint8_t hop_by_hop_options_data[] = {
+        /* Hop-by-Hop Options Header */
+        /* https://tools.ietf.org/html/rfc6553 */
+        0x2b,       /* next header: IPv6-Route */
+        0x00,       /* hdr ext len: 0 * 8 + 8 = 8 */
+        0x63,       /* option type: RPL Option */
+        0x04,       /* opt data len: 4 */
+        0x80,       /* flags, Down: 1, Rank-Error: 0, Forwarding-Error: 0 */
+        0x00,       /* RPLInstanceID */
+        0x80, 0x00, /* SenderRank */
+    };
+
+    uint8_t rpl_routing_data[] = {
+        /* RPL Routing Header */
+        /* https://tools.ietf.org/html/rfc6554 */
+        0x11,               /* next header: UDP */
+        0x02,               /* hdr ext len: 2 * 8 + 8 = 24 */
+        0x03,               /* routing type: SRH */
+        0x02,               /* segments left: 2 */
+        0xef,               /* ComprI: 14, ComprE: 15 */
+        0xd0, 0x00, 0x00,   /* pad and reserved */
+        /* address: fd01::3, fd01::2 */
+        0x00, 0x03, 0x02, 0x00,
+        0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00,
+    };
+
+    uint8_t udp_data[] = {
+        /* UDP (ignored) */
+        0x1f, 0x90, /* source port: 8080 */
+        0x1f, 0x90, /* destination port: 8080 */
+        0x00, 0x0a, /* length: 10 */
+        0xff, 0xff, /* checksum */
+    };
+
+    uint8_t udp_payload[] = {
+        0x00, 0x00,
+    };
+
+    gnrc_pktsnip_t *netif =
+        gnrc_pktbuf_add(NULL,
+                        &netif_hdr,
+                        sizeof(netif_hdr),
+                        GNRC_NETTYPE_NETIF);
+    gnrc_pktsnip_t *ipv6 =
+        gnrc_pktbuf_add(netif,
+                        &ipv6_data,
+                        sizeof(ipv6_data),
+                        GNRC_NETTYPE_IPV6);
+    gnrc_pktsnip_t *hop_by_hop_options =
+        gnrc_pktbuf_add(ipv6,
+                        &hop_by_hop_options_data,
+                        sizeof(hop_by_hop_options_data),
+                        GNRC_NETTYPE_IPV6_EXT);
+    gnrc_pktsnip_t *rpl_routing =
+        gnrc_pktbuf_add(hop_by_hop_options,
+                        &rpl_routing_data,
+                        sizeof(rpl_routing_data),
+                        GNRC_NETTYPE_IPV6_EXT);
+    gnrc_pktsnip_t *udp =
+        gnrc_pktbuf_add(rpl_routing,
+                        udp_data,
+                        sizeof(udp_data),
+                        GNRC_NETTYPE_UDP);
+    gnrc_pktsnip_t *pkt =
+        gnrc_pktbuf_add(udp,
+                        &udp_payload,
+                        sizeof(udp_payload),
+                        GNRC_NETTYPE_UNDEF);
+
+    gnrc_netapi_dispatch_receive(GNRC_NETTYPE_IPV6, GNRC_NETREG_DEMUX_CTX_ALL, pkt);
+
+    printf("pkt->users: %d\n", pkt->users);
+    assert(pkt->users == 0);
+}
+
+
 int main(void)
 {
-    /* we need a message queue for the thread running the shell in order to
-     * receive potentially fast incoming networking packets */
-    msg_init_queue(_main_msg_queue, MAIN_QUEUE_SIZE);
     puts("RIOT network stack example application");
 
-    _send_packet();
+    _init_interface();
-
+    _send_packet_raw();
-    /* start shell */
+    _send_packet_parsed();
-    puts("All up, running the shell now");
-    char line_buf[SHELL_DEFAULT_BUFSIZE];
-    shell_run(NULL, line_buf, SHELL_DEFAULT_BUFSIZE);
 
     /* should be never reached */
     return 0;

tests/gnrc_ipv6_ext/tests/01-run.py

@@ -21,16 +21,21 @@ def testfunc(child):
 
     if index == 1:
         # debug is disabled
+        child.expect_exact("pkt->users: 0")
         return
 
     child.expect_exact("ipv6: handle extension header (nh = 0)")
-    child.expect_exact("ipv6: handle extension header (nh = 43)")
     child.expect_exact("ipv6: Received (src = fd01::1, dst = fd01::3, next header = 0, length = 40)")
     child.expect_exact("ipv6: handle extension header (nh = 0)")
-    child.expect_exact("ipv6: handle extension header (nh = 43)")
     child.expect_exact("ipv6: Received (src = fd01::1, dst = fd01::2, next header = 0, length = 40)")
     child.expect_exact("ipv6: handle extension header (nh = 0)")
-    child.expect_exact("ipv6: handle extension header (nh = 43)")
+    child.expect_exact("ipv6: forward nh = 17 to other threads")
+    child.expect_exact("pkt->users: 0")
+    child.expect_exact("ipv6: handle extension header (nh = 0)")
+    child.expect_exact("ipv6: Received (src = fd01::1, dst = fd01::3, next header = 0, length = 40)")
+    child.expect_exact("ipv6: handle extension header (nh = 0)")
+    child.expect_exact("ipv6: Received (src = fd01::1, dst = fd01::2, next header = 0, length = 40)")
+    child.expect_exact("ipv6: handle extension header (nh = 0)")
     child.expect_exact("ipv6: forward nh = 17 to other threads")
     child.expect_exact("pkt->users: 0")