Fix for multicast ping forwarding before adding MPL header(ARMmbed#1883)

Author: deepakvenugopal

source/Common_Protocols/ipv6.c

@@ -583,6 +583,24 @@ buffer_t *ipv6_forwarding_down(buffer_t *buf)
         return icmpv6_error(buf, NULL, ICMPV6_TYPE_ERROR_DESTINATION_UNREACH, ICMPV6_CODE_DST_UNREACH_NO_ROUTE, 0);
     }
 
+    /* Consider multicast forwarding /before/ calling routing code to modify
+     * extension headers - if that actually decides to tunnel it will
+     * overwrite the buffer's src_sa and dst_sa, when we want to consider
+     * forwarding the inner packet. This ordering works out for our only
+     * header-modifying multicast case of MPL:
+     * 1) We never want to forward packets with MPL headers, which means the
+     *    outer packet in a tunnel gets ignored anyway.
+     * 2) This also means we don't have to worry that we're forwarding packets
+     *    with the extension header not filled in yet.
+     * If we ever do have a multicast system where we are working with
+     * extension headers and forwarding those across interfaces, ipv6_get_exthdrs
+     * system will need a rework - probably split the "try MODIFY" call from the
+     * subsequent "give me tunnel parameters" part.
+     */
+    if (!buf->ip_routed_up && addr_is_ipv6_multicast(buf->dst_sa.address)) {
+        buf = ipv6_consider_forwarding_multicast_packet(buf, buf->interface, true);
+    }
+
     /* Allow routing code to update extension headers */
     int16_t exthdr_result;
     buf = ipv6_get_exthdrs(buf, IPV6_EXTHDR_MODIFY, &exthdr_result);
@@ -593,10 +611,6 @@ buffer_t *ipv6_forwarding_down(buffer_t *buf)
         goto drop;
     }
 
-    if (!buf->ip_routed_up && addr_is_ipv6_multicast(buf->dst_sa.address)) {
-        buf = ipv6_consider_forwarding_multicast_packet(buf, buf->interface, true);
-    }
-
     /* Routing code may say it needs to tunnel to add headers - loop back to IP layer if requested */
     if (exthdr_result == IPV6_EXTHDR_MODIFY_TUNNEL) {
         /* Avoid an infinite loop in the event of routing code failure - never

source/MPL/mpl.c

@@ -1011,17 +1011,41 @@ void mpl_clear_realm_scope_seeds(protocol_interface_info_entry_t *cur)
 static buffer_t *mpl_exthdr_provider(buffer_t *buf, ipv6_exthdr_stage_t stage, int16_t *result)
 {
     mpl_domain_t *domain = mpl_domain_lookup_with_realm_check(buf->interface, buf->dst_sa.address);
-    if (!domain) {
-        // We need to tunnel
 
-        if (stage != IPV6_EXTHDR_MODIFY) {
-            *result = 0;
+    /* Deal with simpler modify-already-created-header case first. Note that no error returns. */
+    if (stage == IPV6_EXTHDR_MODIFY) {
+        if (!domain) {
+            *result = IPV6_EXTHDR_MODIFY_TUNNEL;
+            memcpy(buf->dst_sa.address, ADDR_ALL_MPL_FORWARDERS, 16);
+            buf->src_sa.addr_type = ADDR_NONE; // force auto-selection
             return buf;
         }
 
-        *result = IPV6_EXTHDR_MODIFY_TUNNEL;
-        memcpy(buf->dst_sa.address, ADDR_ALL_MPL_FORWARDERS, 16);
-        buf->src_sa.addr_type = ADDR_NONE; // force auto-selection
+        if (buf->options.ip_extflags & IPEXT_HBH_MPL_UNFILLED) {
+            /* We assume we created this, therefore our option is in place
+             * in the expected place. Sequence is set now, AFTER
+             * fragmentation.
+             */
+            uint8_t *iphdr = buffer_data_pointer(buf);
+            uint8_t *ext = iphdr + IPV6_HDRLEN;
+            if (iphdr[IPV6_HDROFF_NH] != IPV6_NH_HOP_BY_HOP || ext[2] != IPV6_OPTION_MPL) {
+                tr_err("modify");
+                return buffer_free(buf);
+            }
+            /* We don't bother setting the M flag on these initial packets. Setting to 0 is always acceptable. */
+            ext[5] = domain->sequence++;
+            buf->options.ip_extflags &=~ IPEXT_HBH_MPL_UNFILLED;
+            buf->mpl_option_data_offset = IPV6_HDRLEN + 4;
+            mpl_forwarder_process_message(buf, domain, true);
+        }
+        *result = 0;
+        return buf;
+    }
+
+    /* Rest of code deals with header insertion */
+    if (!domain) {
+        // We will need to tunnel - do nothing on the inner packet
+        *result = 0;
         return buf;
     }
 
@@ -1112,26 +1136,6 @@ static buffer_t *mpl_exthdr_provider(buffer_t *buf, ipv6_exthdr_stage_t stage, i
             buf->options.ip_extflags |= IPEXT_HBH_MPL | IPEXT_HBH_MPL_UNFILLED;
             return buf;
         }
-        case IPV6_EXTHDR_MODIFY:
-            if (buf->options.ip_extflags & IPEXT_HBH_MPL_UNFILLED) {
-                /* We assume we created this, therefore our option is in place
-                 * in the expected place. Sequence is set now, AFTER
-                 * fragmentation.
-                 */
-                uint8_t *iphdr = buffer_data_pointer(buf);
-                uint8_t *ext = iphdr + IPV6_HDRLEN;
-                if (iphdr[IPV6_HDROFF_NH] != IPV6_NH_HOP_BY_HOP || ext[2] != IPV6_OPTION_MPL) {
-                    tr_err("modify");
-                    return buffer_free(buf);
-                }
-                /* We don't bother setting the M flag on these initial packets. Setting to 0 is always acceptable. */
-                ext[5] = domain->sequence++;
-                buf->options.ip_extflags &=~ IPEXT_HBH_MPL_UNFILLED;
-                buf->mpl_option_data_offset = IPV6_HDRLEN + 4;
-                mpl_forwarder_process_message(buf, domain, true);
-            }
-            *result = 0;
-            return buf;
         default:
             return buffer_free(buf);
     }