Child neighbor entry updates (#1550)

Cleanup any link we lose to something that looks like a child address.
Refactored thread_neighbor_remove logic to thread_reset_neighbour_info,
which is now called when MLE link to neighbour is lost or neighbour
is removed due to transmission failure.

Author: deepakvenugopal

source/6LoWPAN/Thread/thread_bootstrap.c

@@ -115,21 +115,6 @@ static void thread_bootsrap_network_join_start(struct protocol_interface_info_en
 static int8_t thread_child_keep_alive(int8_t interface_id, const uint8_t *mac64);
 
 
-int thread_bootstrap_reset_child_info(protocol_interface_info_entry_t *cur, mle_neigh_table_entry_t *child)
-{
-    thread_dynamic_storage_child_info_clear(cur->id, child);
-
-    // If Child's RLOC16 appears in the Network Data send the RLOC16 to the Leader
-    if (thread_network_data_services_registered(&cur->thread_info->networkDataStorage, child->short_adr)) {
-        tr_debug("Remove references to Child's RLOC16 from the Network Data");
-        thread_management_client_network_data_unregister(cur->id, child->short_adr);
-    }
-
-    // Clear all (sleepy) child registrations to multicast groups
-    thread_child_mcast_entries_remove(cur, child->mac64);
-
-    return 0;
-}
 
 static bool thread_interface_is_active(int8_t interface_id) {
     protocol_interface_info_entry_t *cur = protocol_stack_interface_info_get_by_id(interface_id);
@@ -146,57 +131,7 @@ static void thread_neighbor_remove(int8_t interface_id, mle_neigh_table_entry_t
     if (!cur_interface) {
         return;
     }
-    if (thread_info(cur_interface)->thread_device_mode == THREAD_DEVICE_MODE_END_DEVICE || thread_info(cur_interface)->thread_device_mode == THREAD_DEVICE_MODE_SLEEPY_END_DEVICE) {
-        thread_parent_info_t *thread_endnode_parent = thread_info(cur_interface)->thread_endnode_parent;
-        //Compare Parent
-        if (thread_endnode_parent) {
-            if (thread_endnode_parent->shortAddress == cur->short_adr) {
-                tr_warn("End device lost Parent!\n");
-                thread_bootstrap_connection_error(cur_interface->id, CON_PARENT_CONNECT_DOWN, NULL);
-            }
-        }
-    }
-    else {
-        if (thread_info(cur_interface)->thread_attached_state == THREAD_STATE_CONNECTED)
-        {
-            thread_parent_info_t *thread_endnode_parent = thread_info(cur_interface)->thread_endnode_parent;
-            if (thread_endnode_parent->shortAddress == cur->short_adr) {
-                tr_warn("REED has lost Parent!\n");
-                thread_routing_remove_link(cur_interface, cur->short_adr);
-                if(cur_interface->nwk_bootstrap_state != ER_CHILD_ID_REQ) {
-                    thread_bootstrap_connection_error(cur_interface->id, CON_PARENT_CONNECT_DOWN, NULL);
-                }
-            }
-            else{
-                tr_debug("Delete REED Neighbor");
-                if (thread_is_router_addr(cur->short_adr)) {
-                    tr_debug("Router Free");
-                    thread_routing_remove_link(cur_interface, cur->short_adr);
-                }
-            }
-        }
-        else if (thread_info(cur_interface)->thread_attached_state == THREAD_STATE_CONNECTED_ROUTER)
-        {
-            tr_debug("Delete Router Neighbor %x", cur->short_adr);
-            if (thread_is_router_addr(cur->short_adr)) {
-                tr_debug("Router free");
-                thread_routing_remove_link(cur_interface, cur->short_adr);
-            } else if (thread_addr_is_child(mac_helper_mac16_address_get(cur_interface), cur->short_adr)) {
-                tr_debug("Child free");
-                /* 16-bit neighbour cache entries are mesh addresses, so remain potentially valid even if an
-                 * MLE link fails. This is the only exception - if it was the link from us as a router to a
-                 * child. That means that device must be off the mesh (at that 16-bit address, at least).
-                 * This will actually clear either a GC cache entry for a FTD or a registered entry
-                 * for a MTD.
-                 */
-                protocol_6lowpan_release_short_link_address_from_neighcache(cur_interface, cur->short_adr);
-                thread_bootstrap_reset_child_info(cur_interface, cur);
-            }
-        }
-    }
-
-    protocol_6lowpan_release_long_link_address_from_neighcache(cur_interface, cur->mac64);
-    mac_helper_devicetable_remove(cur_interface->mac_api, cur->attribute_index);
+    thread_reset_neighbour_info(cur_interface, cur);
 }
 
 

source/6LoWPAN/Thread/thread_common.c

@@ -1837,36 +1837,27 @@ static void thread_tx_failure_handler(int8_t nwk_id, uint8_t accumulated_failure
         return;
     }
 
-    if (thread_i_am_router(cur)) {
-       if (thread_addr_is_child(mac_helper_mac16_address_get(cur), neighbor->short_adr)) {
-           if (accumulated_failures < THREAD_MAC_TRANSMISSIONS*THREAD_FAILED_CHILD_TRANSMISSIONS) {
-               return;
-           }
-
-           tr_debug("Free the Child node, mac16=%d", neighbor->short_adr);
-           thread_bootstrap_reset_child_info(cur, neighbor);
-
-           protocol_6lowpan_release_short_link_address_from_neighcache(cur, neighbor->short_adr);
-           protocol_6lowpan_release_long_link_address_from_neighcache(cur, neighbor->mac64);
-           mac_helper_devicetable_remove(cur->mac_api, neighbor->attribute_index);
-       } else if (thread_is_router_addr(neighbor->short_adr)) {
-           if (accumulated_failures < THREAD_MAC_TRANSMISSIONS*THREAD_FAILED_ROUTER_TRANSMISSIONS) {
-               return;
-           }
-
-           tr_debug("Set link quality to neighbor router to zero...");
-           thread_routing_force_link_margin(cur, neighbor->short_adr, 0);
-       }
-   } else { // We are a Child
-       if (thread_check_is_this_my_parent(cur, neighbor)) {
-           if (accumulated_failures < THREAD_MAC_TRANSMISSIONS*THREAD_FAILED_CHILD_TRANSMISSIONS) {
-               return;
-           }
-
-           tr_debug("Consider the parent gone...");
-           thread_bootstrap_connection_error(cur->id, CON_PARENT_CONNECT_DOWN, NULL);
-       }
-   }
+    if (accumulated_failures >= THREAD_MAC_TRANSMISSIONS*THREAD_FAILED_CHILD_TRANSMISSIONS) {
+        thread_reset_neighbour_info(cur, neighbor);
+    }
+}
+
+/* Called when MLE link to neighbour lost, or ETX callback says link is bad */
+void thread_reset_neighbour_info(protocol_interface_info_entry_t *cur, mle_neigh_table_entry_t *neighbour)
+{
+    thread_parent_info_t *thread_endnode_parent = thread_info(cur)->thread_endnode_parent;
+
+    if (!thread_i_am_router(cur) && thread_endnode_parent && thread_endnode_parent->shortAddress == neighbour->short_adr) {
+        tr_warn("End device lost Parent!\n");
+        if(cur->nwk_bootstrap_state != ER_CHILD_ID_REQ) {
+            thread_bootstrap_connection_error(cur->id, CON_PARENT_CONNECT_DOWN, NULL);
+        }
+    }
+
+    thread_routing_remove_link(cur, neighbour->short_adr);
+    thread_router_bootstrap_reset_child_info(cur, neighbour);
+    protocol_6lowpan_release_long_link_address_from_neighcache(cur, neighbour->mac64);
+    mac_helper_devicetable_remove(cur->mac_api, neighbour->attribute_index);
 }
 
 uint8_t thread_get_router_count_from_route_tlv(mle_tlv_info_t *routeTlv)

source/6LoWPAN/Thread/thread_common.h

@@ -359,6 +359,7 @@ uint16_t thread_network_data_generate_stable_set(protocol_interface_info_entry_t
 
 void thread_set_active_router(protocol_interface_info_entry_t *cur, if_address_entry_t *address_entry, uint8_t *routerId);
 uint8_t thread_get_router_count_from_route_tlv(mle_tlv_info_t *routeTlv);
+void thread_reset_neighbour_info(protocol_interface_info_entry_t *cur, mle_neigh_table_entry_t *neighbour);
 
 void thread_child_id_request_entry_clean(protocol_interface_info_entry_t *cur);
 thread_pending_child_id_req_t *thread_child_id_request_entry_get(protocol_interface_info_entry_t *cur, uint8_t *euid64);

source/6LoWPAN/Thread/thread_router_bootstrap.c

@@ -947,26 +947,45 @@ static int thread_attach_parent_response_build(protocol_interface_info_entry_t *
     return 0;
 }
 
-void thread_router_bootstrap_child_information_clear(protocol_interface_info_entry_t *cur)
+int thread_router_bootstrap_reset_child_info(protocol_interface_info_entry_t *cur, mle_neigh_table_entry_t *child)
 {
+    /* Cleanup occurs for /any/ link we lose to something that looks like a child address,
+     * not just links that are /now/ our children.
+     * Due to REED/partition transitions the address may not look like a current child address;
+     * we could be holding a child entry for future repromotion to router with same ID.
+     */
+    if (thread_is_router_addr(child->short_adr) || child->short_adr >= 0xfffe) {
+        return -1;
+    }
+    tr_debug("Child free %x", child->short_adr);
+    thread_dynamic_storage_child_info_clear(cur->id, child);
 
-    /*the default routerShortAddress if nothing is requested is 0xfffe so eliminate this case
-        Also make sure that the child info (from previous partition if any)
-        is cleared if no router address is got from leader */
+    /* As we are losing a link to a child address, we can assume that if we have an IP neighbour cache
+     * mapping to that address, it is no longer valid. We must have been their parent, and they must be
+     * finding a new parent, and hence a new 16-bit address. (Losing a link to a router address would not
+     * invalidate our IP->16-bit mapping.)
+     */
+    protocol_6lowpan_release_short_link_address_from_neighcache(cur, child->short_adr);
 
-    if (!cur->thread_info) {
-        return;
+    // If Child's RLOC16 appears in the Network Data send the RLOC16 to the Leader
+    if (thread_network_data_services_registered(&cur->thread_info->networkDataStorage, child->short_adr)) {
+        tr_debug("Remove references to Child's RLOC16 from the Network Data");
+        thread_management_client_network_data_unregister(cur->id, child->short_adr);
     }
 
-    if (cur->thread_info->routerShortAddress == 0xfffe) {
-        return;
-    }
+    // Clear all (sleepy) child registrations to multicast groups
+    thread_child_mcast_entries_remove(cur, child->mac64);
 
-    // Remove registered entries in IP neighbour cache
-    ns_list_foreach_safe(ipv6_neighbour_t, neighbour, &cur->ipv6_neighbour_cache.list) {
-        if (neighbour->type == IP_NEIGHBOUR_REGISTERED) {
-            ipv6_neighbour_entry_remove(&cur->ipv6_neighbour_cache, neighbour);
-        }
+    return 0;
+}
+
+void thread_router_bootstrap_child_information_clear(protocol_interface_info_entry_t *cur)
+{
+    /* make sure that the child info (from previous partition if any)
+        is cleared if no router address is got from leader */
+
+    if (!cur->thread_info) {
+        return;
     }
 
     // Remove mle neighbour entries for children in previous partition
@@ -975,11 +994,10 @@ void thread_router_bootstrap_child_information_clear(protocol_interface_info_ent
         return;
     }
     ns_list_foreach_safe(mle_neigh_table_entry_t, table_entry, entry_list) {
-        if (!thread_is_router_addr(table_entry->short_adr) && thread_router_addr_from_addr(table_entry->short_adr) == cur->thread_info->routerShortAddress) {
+        if (table_entry->short_adr < 0xfffe && !thread_is_router_addr(table_entry->short_adr)) {
             mle_class_remove_entry(cur->id, table_entry);
         }
     }
-
 }
 static void thread_router_bootstrap_invalid_child_information_clear(protocol_interface_info_entry_t *cur, uint16_t router_rloc)
 {
@@ -996,7 +1014,6 @@ static void thread_router_bootstrap_invalid_child_information_clear(protocol_int
         if (table_entry->short_adr < 0xfffe &&
                 !thread_is_router_addr(table_entry->short_adr) &&
                 thread_router_addr_from_addr(table_entry->short_adr) != router_rloc) {
-            ipv6_neighbour_delete_registered_by_eui64(&cur->ipv6_neighbour_cache, table_entry->mac64);
             mle_class_remove_entry(cur->id, table_entry);
         }
     }
@@ -2329,6 +2346,7 @@ static void thread_reed_advertisements_cb(void* arg)
     if (cur->thread_info->thread_attached_state == THREAD_STATE_CONNECTED &&
             cur->thread_info->thread_device_mode == THREAD_DEVICE_MODE_ROUTER){
         thread_reed_advertise(cur);
+        thread_router_bootstrap_child_information_clear(cur);
         cur->thread_info->routerSelectParameters.reedAdvertisementTimeout = eventOS_timeout_ms(thread_reed_advertisements_cb, thread_reed_timeout_calculate(&cur->thread_info->routerSelectParameters) * 1000, cur);
     }
 }

source/6LoWPAN/Thread/thread_router_bootstrap.h

@@ -48,6 +48,7 @@ void thread_router_bootstrap_reed_advertisements_start(protocol_interface_info_e
 int thread_router_bootstrap_mle_advertise(struct protocol_interface_info_entry *cur);
 
 void thread_router_bootstrap_child_information_clear(protocol_interface_info_entry_t *cur);
+int thread_router_bootstrap_reset_child_info(protocol_interface_info_entry_t *cur, mle_neigh_table_entry_t *child);
 uint16_t thread_router_bootstrap_child_count_get(protocol_interface_info_entry_t *cur);
 void thread_router_bootstrap_child_id_handler(struct protocol_interface_info_entry *cur);
 void thread_router_bootstrap_child_id_reject(struct protocol_interface_info_entry *cur);
@@ -93,6 +94,7 @@ void thread_router_bootstrap_address_change_notify_send(protocol_interface_info_
 #define thread_router_bootstrap_timer(cur, ticks)
 #define thread_router_bootstrap_random_upgrade_jitter() 0;
 #define thread_router_bootstrap_advertiment_analyze(cur, src_address, entry_temp, shortAddress)
+#define thread_router_bootstrap_reset_child_info(cur, child) 0
 
 #define thread_router_bootstrap_multicast_forwarder_enable(cur, buf) NULL
 #define thread_router_bootstrap_anycast_address_register(cur)

test/nanostack/unittest/stub/thread_common_stub.c

@@ -393,3 +393,6 @@ int thread_address_registration_primary_bbr_get(struct protocol_interface_info_e
 {
     return 0;
 }
+void thread_reset_neighbour_info(protocol_interface_info_entry_t *cur, mle_neigh_table_entry_t *neighbour)
+{
+}

test/nanostack/unittest/stub/thread_router_bootstrap_stub.c

@@ -154,4 +154,7 @@ void thread_router_bootstrap_network_data_distribute(protocol_interface_info_ent
 void thread_router_bootstrap_address_change_notify_send(protocol_interface_info_entry_t *cur)
 {
 }
-
+int thread_router_bootstrap_reset_child_info(protocol_interface_info_entry_t *cur, mle_neigh_table_entry_t *child)
+{
+    return 0;
+}