gnrc_netif: only use prefix matching as tie-breaker in source selection

sys/net/gnrc/netif/gnrc_netif.c

@@ -474,7 +474,6 @@ void gnrc_netif_release(gnrc_netif_t *netif)
 }
 
 #ifdef MODULE_GNRC_IPV6
-static inline bool _addr_anycast(const gnrc_netif_t *netif, unsigned idx);
 static int _addr_idx(const gnrc_netif_t *netif, const ipv6_addr_t *addr);
 static int _group_idx(const gnrc_netif_t *netif, const ipv6_addr_t *addr);
 
@@ -501,18 +500,14 @@ static unsigned _match_to_len(const gnrc_netif_t *netif,
  *
  * @param[in] netif     the network interface
  * @param[in] addr      the address to match
- * @param[in] filter    a bitfield with the bits at the position equal to the
- *                      indexes of the addresses you want to include in the
- *                      search set to one. NULL for all addresses
  *
  * @return  index of the best match for @p addr
  * @return  -1 if no match was found
  *
  * @pre `netif != NULL` and `addr != NULL`
  */
 static int _match_to_idx(const gnrc_netif_t *netif,
-                        const ipv6_addr_t *addr,
-                        const uint8_t *filter);
+                        const ipv6_addr_t *addr);
 /**
  * @brief Determines the scope of the given address.
  *
@@ -700,7 +695,7 @@ int gnrc_netif_ipv6_addr_match(gnrc_netif_t *netif,
 {
     assert((netif != NULL) && (addr != NULL));
     gnrc_netif_acquire(netif);
-    int idx = _match_to_idx(netif, addr, NULL);
+    int idx = _match_to_idx(netif, addr);
     gnrc_netif_release(netif);
     return idx;
 }
@@ -818,11 +813,6 @@ int gnrc_netif_ipv6_group_idx(gnrc_netif_t *netif, const ipv6_addr_t *addr)
     return idx;
 }
 
-static inline bool _addr_anycast(const gnrc_netif_t *netif, unsigned idx)
-{
-    return (netif->ipv6.addrs_flags[idx] & GNRC_NETIF_IPV6_ADDRS_FLAGS_ANYCAST);
-}
-
 static int _idx(const gnrc_netif_t *netif, const ipv6_addr_t *addr, bool mcast)
 {
     if (!ipv6_addr_is_unspecified(addr)) {
@@ -852,13 +842,12 @@ static unsigned _match_to_len(const gnrc_netif_t *netif,
 {
     assert((netif != NULL) && (addr != NULL));
 
-    int n = _match_to_idx(netif, addr, NULL);
+    int n = _match_to_idx(netif, addr);
     return (n >= 0) ? ipv6_addr_match_prefix(&(netif->ipv6.addrs[n]), addr) : 0;
 }
 
 static int _match_to_idx(const gnrc_netif_t *netif,
-                         const ipv6_addr_t *addr,
-                         const uint8_t *filter)
+                         const ipv6_addr_t *addr)
 {
     assert((netif != NULL) && (addr != NULL));
 
@@ -867,15 +856,11 @@ static int _match_to_idx(const gnrc_netif_t *netif,
     for (int i = 0; i < GNRC_NETIF_IPV6_ADDRS_NUMOF; i++) {
         unsigned match;
 
-        if ((netif->ipv6.addrs_flags[i] == 0) ||
-            ((filter != NULL) && _addr_anycast(netif, i)) ||
-            /* discard const intentionally */
-            ((filter != NULL) && !(bf_isset((uint8_t *)filter, i)))) {
+        if (netif->ipv6.addrs_flags[i] == 0) {
             continue;
         }
         match = ipv6_addr_match_prefix(&(netif->ipv6.addrs[i]), addr);
-        if (((match > 64U) || !ipv6_addr_is_link_local(&(netif->ipv6.addrs[i]))) &&
-            (match >= best_match)) {
+        if (match > best_match) {
             idx = i;
             best_match = match;
         }
@@ -885,17 +870,15 @@ static int _match_to_idx(const gnrc_netif_t *netif,
               ipv6_addr_to_str(addr_str, &netif->ipv6.addrs[idx],
                                sizeof(addr_str)),
               netif->pid);
-        DEBUG("%s by %u bits (used as source address = %s)\n",
+        DEBUG("%s by %u bits\n",
               ipv6_addr_to_str(addr_str, addr, sizeof(addr_str)),
-              best_match,
-              (filter != NULL) ? "true" : "false");
+              best_match);
     }
     else {
         DEBUG("gnrc_netif: Did not found any address on interface %" PRIkernel_pid
-              " matching %s (used as source address = %s)\n",
+              " matching %s\n",
               netif->pid,
-              ipv6_addr_to_str(addr_str, addr, sizeof(addr_str)),
-              (filter != NULL) ? "true" : "false");
+              ipv6_addr_to_str(addr_str, addr, sizeof(addr_str)));
     }
     return idx;
 }
@@ -955,8 +938,19 @@ static int _create_candidate_set(const gnrc_netif_t *netif,
               ipv6_addr_to_str(addr_str, tmp, sizeof(addr_str)));
         /* "In any case, multicast addresses and the unspecified address MUST NOT
          *  be included in a candidate set."
+         *
+         * flags are set if not unspecfied and multicast addresses are in
+         * `netif->ipv6.groups` so not considered here.
          */
         if ((netif->ipv6.addrs_flags[i] == 0) ||
+            /* https://tools.ietf.org/html/rfc4862#section-2:
+             *  A tentative address is not considered assigned to an interface
+             *  in the usual sense.  An interface discards received packets
+             *  addressed to a tentative address, but accepts Neighbor Discovery
+             *  packets related to Duplicate Address Detection for the tentative
+             *  address.
+             *  (so don't consider tentative addresses for source address
+             *  selection) */
             gnrc_netif_ipv6_addr_dad_trans(netif, i)) {
             continue;
         }
@@ -993,10 +987,46 @@ static int _create_candidate_set(const gnrc_netif_t *netif,
 /* number of "points" assigned to an source address candidate in preferred state */
 #define RULE_3_PTS          (1)
 
+/**
+ * @brief   Caps the match at a source addresses prefix length
+ *
+ * @see https://tools.ietf.org/html/rfc6724#section-2.2
+ *
+ * @param[in] netif The network interface @p src was selected from
+ * @param[in] src   A potential source address
+ * @param[in] match The number of bits matching of @p src with another address
+ *
+ * @return  @p match or a number lesser than @p match, if @p src has a shorter
+ *          prefix.
+ */
+static unsigned _cap_match(const gnrc_netif_t *netif, const ipv6_addr_t *src,
+                           unsigned match)
+{
+    unsigned best_prefix = 0;
+
+    if (ipv6_addr_is_link_local(src)) {
+        best_prefix = 64U;  /* Link-local prefix is always of length 64 */
+    }
+#ifdef MODULE_GNRC_IPV6_NIB
+    else {
+        void *state = NULL;
+        gnrc_ipv6_nib_pl_t ple;
+
+        while (gnrc_ipv6_nib_pl_iter(netif->pid, &state, &ple)) {
+            if ((ipv6_addr_match_prefix(&ple.pfx, src) > best_prefix)) {
+                best_prefix = ple.pfx_len;
+            }
+        }
+    }
+#endif /* MODULE_GNRC_IPV6_NIB */
+    return ((best_prefix > 0) && (best_prefix < match)) ? best_prefix : match;
+}
+
 static ipv6_addr_t *_src_addr_selection(gnrc_netif_t *netif,
                                         const ipv6_addr_t *dst,
                                         uint8_t *candidate_set)
 {
+    int idx = -1;
     /* create temporary set for assigning "points" to candidates winning in the
      * corresponding rules.
      */
@@ -1031,24 +1061,16 @@ static ipv6_addr_t *_src_addr_selection(gnrc_netif_t *netif,
         if (candidate_scope == dst_scope) {
             DEBUG("winner for rule 2 (same scope) found\n");
             winner_set[i] += RULE_2A_PTS;
-            if (winner_set[i] > max_pts) {
-                max_pts = RULE_2A_PTS;
-            }
         }
         else if (candidate_scope < dst_scope) {
             DEBUG("winner for rule 2 (smaller scope) found\n");
             winner_set[i] += RULE_2B_PTS;
-            if (winner_set[i] > max_pts) {
-                max_pts = winner_set[i];
-            }
         }
+
         /* Rule 3: Avoid deprecated addresses. */
         if (_get_state(netif, i) != GNRC_NETIF_IPV6_ADDRS_FLAGS_STATE_DEPRECATED) {
             DEBUG("winner for rule 3 found\n");
             winner_set[i] += RULE_3_PTS;
-            if (winner_set[i] > max_pts) {
-                max_pts = winner_set[i];
-            }
         }
 
         /* Rule 4: Prefer home addresses.
@@ -1077,19 +1099,39 @@ static ipv6_addr_t *_src_addr_selection(gnrc_netif_t *netif,
          * Temporary addresses are currently not supported by gnrc.
          * TODO: update as soon as gnrc supports temporary addresses
          */
+
+        if (winner_set[i] > max_pts) {
+            idx = i;
+            max_pts = winner_set[i];
+        }
     }
-    /* reset candidate set to mark winners */
-    memset(candidate_set, 0, (GNRC_NETIF_IPV6_ADDRS_NUMOF + 7) / 8);
-    /* check if we have a clear winner */
+    /* check if we have a clear winner, otherwise
+     * rule 8: Use longest matching prefix.*/
+    uint8_t best_match = 0;
     /* collect candidates with maximum points */
     for (int i = 0; i < GNRC_NETIF_IPV6_ADDRS_NUMOF; i++) {
         if (winner_set[i] == max_pts) {
-            bf_set(candidate_set, i);
+            const ipv6_addr_t *addr = &netif->ipv6.addrs[i];
+            unsigned match = ipv6_addr_match_prefix(addr, dst);
+
+            match = _cap_match(netif, addr, match);
+            /* if match == 0 for all case, it takes above selected idx */
+            if (match > best_match) {
+                idx = i;
+                best_match = match;
+            }
         }
     }
-    /* otherwise apply rule 8: Use longest matching prefix. */
-    int idx = _match_to_idx(netif, dst, candidate_set);
-    return (idx < 0) ? NULL : &netif->ipv6.addrs[idx];
+    if (idx < 0) {
+        DEBUG("No winner found\n");
+        return NULL;
+    }
+    else {
+        DEBUG("Winner is: %s\n", ipv6_addr_to_str(addr_str,
+                                                  &netif->ipv6.addrs[idx],
+                                                  sizeof(addr_str)));
+        return &netif->ipv6.addrs[idx];
+    }
 }
 #endif  /* MODULE_GNRC_IPV6 */
 

tests/gnrc_netif/main.c

@@ -472,6 +472,60 @@ static void test_ipv6_addr_best_src__ula_src_dst(void)
     TEST_ASSERT(ipv6_addr_equal(&ula_src, out));
 }
 
+static void test_ipv6_addr_best_src__global_src_ula_dst(void)
+{
+    static const ipv6_addr_t src = { .u8 = NETIF0_IPV6_G };
+    static const ipv6_addr_t ula_dst = { .u8 = { ULA1, ULA2, ULA3, ULA4,
+                                                 ULA5, ULA6, ULA7, ULA8,
+                                                 0, 0, 0, 0, 0, 0, 0, 1 } };
+    ipv6_addr_t *out = NULL;
+    int idx;
+
+    test_ipv6_addr_add__success();  /* adds link-local address */
+    TEST_ASSERT(0 <= (idx = gnrc_netif_ipv6_addr_add_internal(netifs[0], &src, 64U,
+                                                     GNRC_NETIF_IPV6_ADDRS_FLAGS_STATE_VALID)));
+    TEST_ASSERT_EQUAL_INT(GNRC_NETIF_IPV6_ADDRS_FLAGS_STATE_VALID,
+                          netifs[0]->ipv6.addrs_flags[idx]);
+    TEST_ASSERT(ipv6_addr_equal(&src, &netifs[0]->ipv6.addrs[idx]));
+
+    TEST_ASSERT_NOT_NULL((out = gnrc_netif_ipv6_addr_best_src(netifs[0],
+                                                              &ula_dst,
+                                                              false)));
+    TEST_ASSERT(ipv6_addr_equal(&src, out));
+}
+
+static void test_ipv6_addr_best_src__deprecated_addr(void)
+{
+    static const ipv6_addr_t src = { .u8 = { LP1, LP2, LP3, LP4,
+                                             LP5, LP6, LP7, LP8,
+                                             0, 0, 0, 0, 0, 0, 0, 2 } };
+    static const ipv6_addr_t dst = { .u8 = { LP1, LP2, LP3, LP4,
+                                             LP5, LP6, LP7, LP8,
+                                             0, 0, 0, 0, 0, 0, 0, 1 } };
+    ipv6_addr_t *out = NULL;
+    int idx;
+    const unsigned exp_match = ipv6_addr_match_prefix(&src, &dst);
+
+    test_ipv6_addr_add__success();  /* adds EUI-64 based link-local address */
+    /* ensure that current addresses have smaller matches */
+    for (unsigned i = 0; i < GNRC_NETIF_IPV6_ADDRS_NUMOF; i++) {
+        ipv6_addr_t *addr = &netifs[0]->ipv6.addrs[i];
+        TEST_ASSERT(exp_match > ipv6_addr_match_prefix(addr, &dst));
+    }
+    /* add another link-local address but deprecated  */
+    TEST_ASSERT(0 <= (idx = gnrc_netif_ipv6_addr_add_internal(netifs[0], &src, 64U,
+                                                    GNRC_NETIF_IPV6_ADDRS_FLAGS_STATE_DEPRECATED)));
+    TEST_ASSERT_EQUAL_INT(GNRC_NETIF_IPV6_ADDRS_FLAGS_STATE_DEPRECATED,
+                          netifs[0]->ipv6.addrs_flags[idx]);
+    TEST_ASSERT(ipv6_addr_equal(&src, &netifs[0]->ipv6.addrs[idx]));
+
+    TEST_ASSERT_NOT_NULL((out = gnrc_netif_ipv6_addr_best_src(netifs[0],
+                                                              &dst,
+                                                              false)));
+    /* should be not `src` as it is deprecated */
+    TEST_ASSERT(!ipv6_addr_equal(&src, out));
+}
+
 static void test_get_by_ipv6_addr__empty(void)
 {
     static const ipv6_addr_t addr = { .u8 = NETIF0_IPV6_LL };
@@ -1494,6 +1548,8 @@ static Test *embunit_tests_gnrc_netif(void)
         new_TestFixture(test_ipv6_addr_best_src__unspecified_addr),
         new_TestFixture(test_ipv6_addr_best_src__other_subnet),
         new_TestFixture(test_ipv6_addr_best_src__ula_src_dst),
+        new_TestFixture(test_ipv6_addr_best_src__global_src_ula_dst),
+        new_TestFixture(test_ipv6_addr_best_src__deprecated_addr),
         new_TestFixture(test_get_by_ipv6_addr__empty),
         new_TestFixture(test_get_by_ipv6_addr__unspecified_addr),
         new_TestFixture(test_get_by_ipv6_addr__success),