[Olsr-dev] [PATCH] check_neighbor_link is wrong

[Giovanni Di Stasi]

Check_neighbor_link function searches a link based only on the receiver interface address. This is not correct as two links may exist toward the same neighbor interface. In that case the function considers only the first link, neglecting the others (which may be better). The patch makes sure that the right link is considered by comparing both the local_address and the remote_address.
 
Consider as an example this scenario: two nodes, each equipped with two wireless interfaces; first node has its interfaces on channels 1 and 2, while the second on channels 1 and 5. After a while, node 1 switches the first interface from channel 1 to 5. At that point two links towards node 2 exist (in the link set) towards the same neighbor interface (the old on channel 1 and the new on channel 5). At that point, check_neighbor_link only considers, as previously steted, the old link and the neighbor is soon considered not reachable (because that link becomes lost). Eventually, the old link is deleted from the link set and the new link is considered, making the neighbor reachable again after a long interruption. The downtime with the patch is reduced from about 35 seconds to 6-7 seconds (with hysteresis and default configuration). 

The patch also fixes get_best_link_to_neighbor (in case of RFC behaviour). Indeed, as in the previous case, code was just picking the first link without considering the others. So, if the first link is asymmetric and the second is symmetric (and with the same metric), the first one is taken as backup and the good link is NULL.

Signed-off-by: Giovanni Di Stasi <(spam-protected)>
---
 src/hna_set.c  |    4 ++--
 src/link_set.c |    9 ++++-----
 src/link_set.h |    2 +-
 src/mid_set.c  |    4 ++--
 src/tc_set.c   |    4 ++--
 5 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/src/hna_set.c b/src/hna_set.c
index 13d0dc3..469348b 100644
--- a/src/hna_set.c
+++ b/src/hna_set.c
@@ -362,7 +362,7 @@ olsr_print_hna_set(void)
  */
 
 bool
-olsr_input_hna(union olsr_message *m, struct interface *in_if __attribute__ ((unused)), union olsr_ip_addr *from_addr)
+olsr_input_hna(union olsr_message *m, struct interface *in_if, union olsr_ip_addr *from_addr)
 {
 
   uint8_t olsr_msgtype;
@@ -425,7 +425,7 @@ olsr_input_hna(union olsr_message *m, struct interface *in_if __attribute__ ((un
    *      is not in the symmetric 1-hop neighborhood of this node, the
    *      message MUST be discarded.
    */
-  if (check_neighbor_link(from_addr) != SYM_LINK) {
+  if (check_neighbor_link(&in_if->ip_addr, from_addr) != SYM_LINK) {
     OLSR_PRINTF(2, "Received HNA from NON SYM neighbor %s\n", olsr_ip_to_string(&buf, from_addr));
     return false;
   }
diff --git a/src/link_set.c b/src/link_set.c
index 399a5ee..cf3a76d 100644
--- a/src/link_set.c
+++ b/src/link_set.c
@@ -184,7 +184,7 @@ get_neighbor_status(const union olsr_ip_addr *address)
   if (!(main_addr = mid_lookup_main_addr(address)))
     main_addr = address;
 
-  /* Loop trough local interfaces to check all possebilities */
+  /* Loop trough local interfaces to check all possibilities */
   for (ifs = ifnet; ifs != NULL; ifs = ifs->int_next) {
     struct mid_address *aliases;
     struct link_entry *lnk = lookup_link_entry(main_addr, NULL, ifs);
@@ -257,9 +257,8 @@ get_best_link_to_neighbor(const union olsr_ip_addr *remote)
 
       /*
        * is this interface better than anything we had before ?
-       * use the requested remote interface address as a tie-breaker
        */
-      if ((tmp_if->int_metric < curr_metric) || ((tmp_if->int_metric == curr_metric) && ipequal(&walker->local_iface_addr, remote))) {
+      if ((tmp_if->int_metric < curr_metric) || ((tmp_if->int_metric == curr_metric) && ! good_link)) {
 
         /* memorize the interface's metric */
         curr_metric = tmp_if->int_metric;
@@ -627,12 +626,12 @@ add_link_entry(const union olsr_ip_addr *local, const union olsr_ip_addr *remote
  * @return 1 of the link is symmertic 0 if not
  */
 int
-check_neighbor_link(const union olsr_ip_addr *int_addr)
+check_neighbor_link(const union olsr_ip_addr * local, const union olsr_ip_addr *remote)
 {
   struct link_entry *link;
 
   OLSR_FOR_ALL_LINK_ENTRIES(link) {
-    if (ipequal(int_addr, &link->neighbor_iface_addr)) {
+    if (ipequal(remote, &link->neighbor_iface_addr)  && ipequal(local, &link->local_iface_addr)) {
       return lookup_link_status(link);
     }
   }
diff --git a/src/link_set.h b/src/link_set.h
index 9ae27f5..f7645e2 100644
--- a/src/link_set.h
+++ b/src/link_set.h
@@ -133,7 +133,7 @@ struct link_entry *lookup_link_entry(const union olsr_ip_addr *, const union ols
 struct link_entry *update_link_entry(const union olsr_ip_addr *, const union olsr_ip_addr *, const struct hello_message *,
                                      const struct interface *);
 
-int check_neighbor_link(const union olsr_ip_addr *);
+int check_neighbor_link(const union olsr_ip_addr *, const union olsr_ip_addr *);
 int replace_neighbor_link_set(const struct neighbor_entry *, struct neighbor_entry *);
 int lookup_link_status(const struct link_entry *);
 void olsr_update_packet_loss_hello_int(struct link_entry *, olsr_reltime);
diff --git a/src/mid_set.c b/src/mid_set.c
index 2e05106..4cfa7f2 100644
--- a/src/mid_set.c
+++ b/src/mid_set.c
@@ -560,7 +560,7 @@ olsr_print_mid_set(void)
  */
 
 bool
-olsr_input_mid(union olsr_message *m, struct interface *in_if __attribute__ ((unused)), union olsr_ip_addr *from_addr)
+olsr_input_mid(union olsr_message *m, struct interface *in_if, union olsr_ip_addr *from_addr)
 {
   struct ipaddr_str buf;
   struct mid_alias *tmp_adr;
@@ -583,7 +583,7 @@ olsr_input_mid(union olsr_message *m, struct interface *in_if __attribute__ ((un
    *      message MUST be discarded.
    */
 
-  if (check_neighbor_link(from_addr) != SYM_LINK) {
+  if (check_neighbor_link(&in_if->ip_addr, from_addr) != SYM_LINK) {
     OLSR_PRINTF(2, "Received MID from NON SYM neighbor %s\n", olsr_ip_to_string(&buf, from_addr));
     olsr_free_mid_packet(&message);
     return false;
diff --git a/src/tc_set.c b/src/tc_set.c
index 9c44323..768b3b2 100644
--- a/src/tc_set.c
+++ b/src/tc_set.c
@@ -764,7 +764,7 @@ olsr_calculate_tc_border(uint8_t lower_border, union olsr_ip_addr *lower_border_
  * hence the spot we are looking at.
  */
 bool
-olsr_input_tc(union olsr_message * msg, struct interface * input_if __attribute__ ((unused)), union olsr_ip_addr * from_addr)
+olsr_input_tc(union olsr_message * msg, struct interface * input_if, union olsr_ip_addr * from_addr)
 {
   struct ipaddr_str buf;
   uint16_t size, msg_seq, ansn;
@@ -794,7 +794,7 @@ olsr_input_tc(union olsr_message * msg, struct interface * input_if __attribute_
    * is not in the symmetric 1-hop neighborhood of this node, the
    * message MUST be discarded.
    */
-  if (check_neighbor_link(from_addr) != SYM_LINK) {
+  if (check_neighbor_link(&input_if->ip_addr, from_addr) != SYM_LINK) {
     OLSR_PRINTF(2, "Received TC from NON SYM neighbor %s\n", olsr_ip_to_string(&buf, from_addr));
     return false;
   }
-- 
1.7.10.4