Merge pull request #10234 from miri64/gnrc_ipv6/enh/iterate-out-of-ext

gnrc_ipv6: move ipv6_ext iteration out of ext_demux()

sys/include/net/gnrc/ipv6.h

@@ -127,32 +127,6 @@ extern fib_table_t gnrc_ipv6_fib_table;
  */
 kernel_pid_t gnrc_ipv6_init(void);
 
-/**
- * @brief   Demultiplexes a packet according to @p nh.
- *
- * @internal
- *
- * **Do not use outside this module or its submodules!!!**
- * Public access needed for Extension Headers.
- *
- * About `current` and `pkt`:
- *
- *                     current     pkt
- *                     |           |
- *                     v           v
- * IPv6 <- IPv6_EXT <- IPv6_EXT <- UNDEF
- *
- * This situation may happen when the packet has a source routing extension
- * header (RFC 6554), and the packet is forwarded from an interface to another.
- *
- * @param[in] netif     The receiving interface.
- * @param[in] current   A snip to process.
- * @param[in] pkt       A packet.
- * @param[in] nh        A protocol number (see @ref net_protnum) of the current snip.
- */
-void gnrc_ipv6_demux(gnrc_netif_t *netif, gnrc_pktsnip_t *current,
-                     gnrc_pktsnip_t *pkt, uint8_t nh);
-
 /**
  * @brief   Get the IPv6 header from a given list of @ref gnrc_pktsnip_t
  *

sys/include/net/gnrc/ipv6/ext.h

@@ -27,7 +27,6 @@
 #include <stdint.h>
 #include <stdlib.h>
 
-#include "net/gnrc/netif.h"
 #include "net/gnrc/pkt.h"
 #include "net/ipv6/ext.h"
 
@@ -40,29 +39,17 @@ extern "C" {
 #endif
 
 /**
- * @brief   Demultiplex extension headers according to @p nh.
+ * @brief   Demultiplex an extension header according to @p nh.
  *
- * About `current` and `pkt`:
+ * @param[in] pkt       A packet with the first snip pointing to the extension
+ *                      header to process.
+ * @param[in] nh        Protocol number of @p pkt.
  *
- *                     current     pkt
+ * @return  The packet for further processing.
- *                     |           |
+ * @return  NULL, on error or if packet was consumed (by e.g. forwarding via
- *                     v           v
+ *          a routing header). @p pkt is released in case of error.
- * IPv6 <- IPv6_EXT <- IPv6_EXT <- UNDEF
- *
- * This situation may happen when the packet has a source routing extension
- * header (RFC 6554), and the packet is forwarded from an interface to another.
- *
- * @internal
- *
- * @param[in] netif     The receiving interface.
- * @param[in] current   A snip to process.
- * @param[in] pkt       A packet.
- * @param[in] nh        A protocol number (see @ref net_protnum) of the current snip.
  */
-void gnrc_ipv6_ext_demux(gnrc_netif_t *netif,
+gnrc_pktsnip_t *gnrc_ipv6_ext_demux(gnrc_pktsnip_t *pkt, unsigned nh);
-                         gnrc_pktsnip_t *current,
-                         gnrc_pktsnip_t *pkt,
-                         uint8_t nh);
 
 /**
  * @brief   Builds an extension header for sending.

sys/include/net/gnrc/pktbuf.h

@@ -286,8 +286,8 @@ gnrc_pktsnip_t *gnrc_pktbuf_reverse_snips(gnrc_pktsnip_t *pkt);
  *        The original snip is keeped as is except `users` decremented.
  *
  * @deprecated  This function breaks the abstraction of `gnrc_pktbuf` and its
- *              only user within the RIOT code base `gnrc_ipv6_ext` is going to
+ *              only user within the RIOT code base `gnrc_ipv6_ext` was reworked
- *              be reworked so it isn't needed anymore.
+ *              so it isn't needed anymore.
  *              It will be removed after the 2019.04 release.
  *
  * @param[in,out] pkt   The snip to duplicate.

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

@@ -1,5 +1,6 @@
 /*
- * Copyright (C) 2015 Martine Lenders <mlenders@inf.fu-berlin.de>
+ * Copyright (C) 2015 Cenk Gündoğan <cnkgndgn@gmail.com>
+ * Copyright (C) 2018 Freie Universität Berlin
  *
  * This file is subject to the terms and conditions of the GNU Lesser
  * General Public License v2.1. See the file LICENSE in the top level
@@ -10,6 +11,8 @@
  * @{
  *
  * @file
+ * @author Cenk Gündoğan <cnkgndgn@gmail.com>
+ * @author Martine Lenders <m.lenders@fu-berlin.de>
  */
 
 #include <errno.h>
@@ -59,40 +62,21 @@ static void _forward_pkt(gnrc_pktsnip_t *pkt, ipv6_hdr_t *hdr)
     }
 }
 
-static int _handle_rh(gnrc_pktsnip_t *current, gnrc_pktsnip_t *pkt)
+/* handle routing header at pkt->data */
+static int _handle_rh(gnrc_pktsnip_t *pkt)
 {
     gnrc_pktsnip_t *ipv6;
-    ipv6_ext_t *ext = (ipv6_ext_t *) current->data;
+    ipv6_ext_t *ext = (ipv6_ext_t *)pkt->data;
-    size_t current_offset;
     ipv6_hdr_t *hdr;
     int res;
 
-    /* check seg_left early to avoid duplicating the packet */
+    /* check seg_left early to to exit quickly */
     if (((ipv6_ext_rh_t *)ext)->seg_left == 0) {
         return GNRC_IPV6_EXT_RH_AT_DST;
     }
-
-    /* We cannot use `gnrc_pktbuf_start_write` since it duplicates only
-       the head. `ipv6_ext_rh_process` modifies the IPv6 header as well as
-       the extension header */
-
-    current_offset = gnrc_pkt_len_upto(current->next, GNRC_NETTYPE_IPV6);
-
-    if (pkt->users != 1) {
-        if ((ipv6 = gnrc_pktbuf_duplicate_upto(pkt, GNRC_NETTYPE_IPV6)) == NULL) {
-            DEBUG("ipv6: could not get a copy of pkt\n");
-            gnrc_pktbuf_release(pkt);
-            return GNRC_IPV6_EXT_RH_ERROR;
-        }
-        pkt = ipv6;
-        hdr = ipv6->data;
-        ext = (ipv6_ext_t *)(((uint8_t *)ipv6->data) + current_offset);
-    }
-    else {
     ipv6 = gnrc_pktsnip_search_type(pkt, GNRC_NETTYPE_IPV6);
+    assert(ipv6 != NULL);
     hdr = ipv6->data;
-    }
-
     switch ((res = gnrc_ipv6_ext_rh_process(hdr, (ipv6_ext_rh_t *)ext))) {
         case GNRC_IPV6_EXT_RH_ERROR:
             /* TODO: send ICMPv6 error codes */
@@ -114,53 +98,29 @@ static int _handle_rh(gnrc_pktsnip_t *current, gnrc_pktsnip_t *pkt)
 #endif  /* MODULE_GNRC_IPV6_EXT_RH */
 
 /**
- * @brief marks IPv6 extension header if needed.
+ * @brief       Marks an IPv6 extension header according to the length field
- *   updates pkt and returns next header.
+ *              provided by the extension header itself.
- * @param[in] current  The current header
+ *
- * @param[in,out] pkt  The whole packet
+ * @param[in] pkt   The packet, with the extension header that is to be marked
- * @return The next header
+ *                  as first snip
- * @return NULL on error
+ *
+ * @return  The marked snip
+ * @return  NULL when no space in packet buffer left or the length field of
+ *          the extension header results in a number of bytes greater than
+ *          gnrc_pktsnip_t::size of @p pkt. In both error cases, @p pkt is
+ *          released.
  */
-static gnrc_pktsnip_t *_mark_extension_header(gnrc_pktsnip_t *current,
+static gnrc_pktsnip_t *_mark_extension_header(gnrc_pktsnip_t *pkt)
-                                              gnrc_pktsnip_t **pkt)
 {
-    gnrc_pktsnip_t *tmp, *next;
+    ipv6_ext_t *ext = (ipv6_ext_t *)pkt->data;
-    ipv6_ext_t *ext = (ipv6_ext_t *) current->data;
+    size_t hdr_size = ((ext->len * IPV6_EXT_LEN_UNIT) + IPV6_EXT_LEN_UNIT);
-    size_t offset = ((ext->len * IPV6_EXT_LEN_UNIT) + IPV6_EXT_LEN_UNIT);
+    gnrc_pktsnip_t *ext_snip = gnrc_pktbuf_mark(pkt, hdr_size,
-
+                                                GNRC_NETTYPE_IPV6_EXT);
-    if (current == *pkt) {
-        gnrc_pktsnip_t *ext_snip;
-        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);
+        gnrc_pktbuf_release(pkt);
-            return NULL;
     }
-    }
+    return ext_snip;
-    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;
 }
 
 static inline bool _has_valid_size(gnrc_pktsnip_t *pkt, uint8_t nh)
@@ -188,57 +148,41 @@ static inline bool _has_valid_size(gnrc_pktsnip_t *pkt, uint8_t nh)
     }
 }
 
-/*
+gnrc_pktsnip_t *gnrc_ipv6_ext_demux(gnrc_pktsnip_t *pkt, unsigned nh)
- *         current                 pkt
- *         |                       |
- *         v                       v
- * IPv6 <- IPv6_EXT <- IPv6_EXT <- UNDEF
- */
-void gnrc_ipv6_ext_demux(gnrc_netif_t *netif,
-                         gnrc_pktsnip_t *current,
-                         gnrc_pktsnip_t *pkt,
-                         uint8_t nh)
 {
-    ipv6_ext_t *ext;
+    DEBUG("ipv6_ext: next header = %u", nh);
-
+    if (!_has_valid_size(pkt, nh)) {
-    while (true) {
-        ext = (ipv6_ext_t *) current->data;
-
-        switch (nh) {
-            case PROTNUM_IPV6_EXT_RH:
-#ifdef MODULE_GNRC_IPV6_EXT_RH
-                /* if current != pkt, size is already checked */
-                if (current == pkt && !_has_valid_size(pkt, nh)) {
         DEBUG("ipv6_ext: invalid size\n");
         gnrc_pktbuf_release(pkt);
-                    return;
+        return NULL;
     }
-
+    switch (nh) {
-                switch (_handle_rh(current, pkt)) {
+        case PROTNUM_IPV6_EXT_RH:
+#ifdef MODULE_GNRC_RPL_SRH
+            switch (_handle_rh(pkt)) {
                 case GNRC_IPV6_EXT_RH_AT_DST:
-                        /* We are the final destination. So proceeds like normal packet. */
+                    /* We are the final destination of the route laid out in
-                        nh = ext->nh;
+                     * the routing header. So proceeds like normal packet. */
-                        DEBUG("ipv6_ext: next header = %" PRIu8 "\n", nh);
+                    if (_mark_extension_header(pkt) == NULL) {
-
+                        /* we couldn't mark the routing header though, return
-                        if ((current = _mark_extension_header(current, &pkt)) == NULL) {
+                         * an error */
-                            return;
+                        return NULL;
                     }
+                    break;
 
-                        gnrc_ipv6_demux(netif, current, pkt, nh); /* demultiplex next header */
+                default:
-
+                    /* unexpected return value => error */
-                        return;
+                    gnrc_pktbuf_release(pkt);
-
+                    /* Intentionally falls through */
                 case GNRC_IPV6_EXT_RH_ERROR:
                     /* already released by _handle_rh, so no release here */
-                        return;
-
                 case GNRC_IPV6_EXT_RH_FORWARDED:
                     /* the packet is forwarded and released. finish processing */
-                        return;
+                    return NULL;
             }
 
             break;
-#endif  /* MODULE_GNRC_IPV6_EXT_RH */
+#endif
 
         case PROTNUM_IPV6_EXT_HOPOPT:
         case PROTNUM_IPV6_EXT_DST:
@@ -246,36 +190,18 @@ void gnrc_ipv6_ext_demux(gnrc_netif_t *netif,
         case PROTNUM_IPV6_EXT_AH:
         case PROTNUM_IPV6_EXT_ESP:
         case PROTNUM_IPV6_EXT_MOB:
-                /* TODO: add handling of types */
+            DEBUG("ipv6_ext: skipping over unsupported extension header\n");
-
+            if (_mark_extension_header(pkt) == NULL) {
-                /* if current != pkt, size is already checked */
+                /* unable mark it though to get it out of the way of the
-                if (current == pkt && !_has_valid_size(pkt, nh)) {
+                 * payload, so return an error */
-                    DEBUG("ipv6_ext: invalid size\n");
+                return NULL;
-                    gnrc_pktbuf_release(pkt);
-                    return;
             }
-
-                nh = ext->nh;
-                DEBUG("ipv6_ext: next header = %" PRIu8 "\n", nh);
-
-                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:
-                gnrc_ipv6_demux(netif, current, pkt, nh); /* demultiplex next header */
+            break;
-                return;
     }
-    }
+    return pkt;
-
-    assert(false); /* never reaches here */
 }
 
 gnrc_pktsnip_t *gnrc_ipv6_ext_build(gnrc_pktsnip_t *ipv6, gnrc_pktsnip_t *next,

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

@@ -90,95 +90,61 @@ kernel_pid_t gnrc_ipv6_init(void)
     return gnrc_ipv6_pid;
 }
 
-static void _dispatch_next_header(gnrc_pktsnip_t *current, gnrc_pktsnip_t *pkt,
+static void _dispatch_next_header(gnrc_pktsnip_t *pkt, unsigned nh,
-                                  uint8_t nh, bool interested);
+                                  bool interested);
 
-/*
+static void _demux(gnrc_netif_t *netif, gnrc_pktsnip_t *pkt, unsigned nh)
- *         current                 pkt
- *         |                       |
- *         v                       v
- * IPv6 <- IPv6_EXT <- IPv6_EXT <- UNDEF
- */
-void gnrc_ipv6_demux(gnrc_netif_t *netif, gnrc_pktsnip_t *current,
-                     gnrc_pktsnip_t *pkt, uint8_t nh)
 {
-    bool interested = false;
+    bool interested;
 
-    current->type = gnrc_nettype_from_protnum(nh);
-
-    switch (nh) {
-#ifdef MODULE_GNRC_ICMPV6
-        case PROTNUM_ICMPV6:
-            assert(current == pkt);
-            interested = true;
-            break;
-#endif
 #ifdef MODULE_GNRC_IPV6_EXT
+    bool is_ext = true;
+
+    while (is_ext) {
+        switch (nh) {
             case PROTNUM_IPV6_EXT_HOPOPT:
             case PROTNUM_IPV6_EXT_DST:
             case PROTNUM_IPV6_EXT_RH:
             case PROTNUM_IPV6_EXT_FRAG:
             case PROTNUM_IPV6_EXT_AH:
             case PROTNUM_IPV6_EXT_ESP:
-        case PROTNUM_IPV6_EXT_MOB:
+            case PROTNUM_IPV6_EXT_MOB: {
-            interested = true;
+                ipv6_ext_t *ext_hdr;
 
-            break;
+                DEBUG("ipv6: handle extension header (nh = %u)\n", nh);
-#endif
+                ext_hdr = pkt->data;
-        default:
+                if ((pkt = gnrc_ipv6_ext_demux(pkt, nh)) == NULL) {
-            (void)netif;
+                    DEBUG("ipv6: packet was consumed by extension header "
-#ifdef MODULE_GNRC_SIXLOWPAN_IPHC_NHC
+                          "handling\n");
-            /* 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;
-    }
-
-    _dispatch_next_header(current, pkt, nh, interested);
-
-    if (!interested) {
                     return;
                 }
+                nh = ext_hdr->nh;
+                break;
+            }
+            default:
+                is_ext = false;
+                break;
+        }
+    }
+#endif /* MODULE_GNRC_IPV6_EXT */
 
+#ifdef MODULE_GNRC_ICMPV6
+    interested = (nh == PROTNUM_ICMPV6);
+#else  /* MODULE_GNRC_ICMPV6 */
+    interested = false;
+#endif /* MODULE_GNRC_ICMPV6 */
+    pkt->type = gnrc_nettype_from_protnum(nh);
+    _dispatch_next_header(pkt, nh, interested);
     switch (nh) {
 #ifdef MODULE_GNRC_ICMPV6
         case PROTNUM_ICMPV6:
             DEBUG("ipv6: handle ICMPv6 packet (nh = %u)\n", nh);
             gnrc_icmpv6_demux(netif, pkt);
-            return;
+            break;
-#endif
+#endif /* MODULE_GNRC_ICMPV6 */
-#ifdef MODULE_GNRC_IPV6_EXT
-        case PROTNUM_IPV6_EXT_HOPOPT:
-        case PROTNUM_IPV6_EXT_DST:
-        case PROTNUM_IPV6_EXT_RH:
-        case PROTNUM_IPV6_EXT_FRAG:
-        case PROTNUM_IPV6_EXT_AH:
-        case PROTNUM_IPV6_EXT_ESP:
-        case PROTNUM_IPV6_EXT_MOB:
-            DEBUG("ipv6: handle extension header (nh = %u)\n", nh);
-
-            gnrc_ipv6_ext_demux(netif, current, pkt, nh);
-
-            return;
-#endif
         default:
-            assert(false);
             break;
     }
-
-    assert(false);
 }
 
 ipv6_hdr_t *gnrc_ipv6_get_header(gnrc_pktsnip_t *pkt)
@@ -196,37 +162,27 @@ ipv6_hdr_t *gnrc_ipv6_get_header(gnrc_pktsnip_t *pkt)
 }
 
 /* internal functions */
-static void _dispatch_next_header(gnrc_pktsnip_t *current, gnrc_pktsnip_t *pkt,
+static void _dispatch_next_header(gnrc_pktsnip_t *pkt, unsigned nh,
-                                  uint8_t nh, bool interested)
+                                  bool interested)
 {
-#ifdef MODULE_GNRC_IPV6_EXT
+    const bool has_nh_subs = (gnrc_netreg_num(GNRC_NETTYPE_IPV6, nh) > 0) ||
-    const bool should_dispatch_current_type = ((current->type != GNRC_NETTYPE_IPV6_EXT) ||
+                             interested;
-                                               (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 (has_nh_subs) {
-    if (should_dispatch_current_type) {
-        bool should_release = (!gnrc_netreg_lookup(GNRC_NETTYPE_IPV6, nh)) &&
-                              (!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,
+    if (gnrc_netapi_dispatch_receive(pkt->type,
                                      GNRC_NETREG_DEMUX_CTX_ALL,
                                      pkt) == 0) {
         gnrc_pktbuf_release(pkt);
     }
-
+    if (!has_nh_subs) {
-        if (should_release) {
+        /* we should exit early. pkt was already released above */
         return;
     }
-    }
     if (interested) {
         gnrc_pktbuf_hold(pkt, 1);   /* don't remove from packet buffer in
                                      * next dispatch */
@@ -872,8 +828,7 @@ static void _receive(gnrc_pktsnip_t *pkt)
 #endif /* MODULE_GNRC_IPV6_ROUTER */
     }
 
-    /* IPv6 internal demuxing (ICMPv6, Extension headers etc.) */
+    _demux(netif, pkt, hdr->nh);
-    gnrc_ipv6_demux(netif, pkt, pkt, hdr->nh);
 }
 
 /** @} */