[olsr-dev] bug in mdi code

[Erik Tromp]

Bill,

I'm afraid that this fix is not enough.

Is updating 'neighbor->neighbor_main_addr' safe? It seems that the neighbor set is hashed through 'neighbor_main_addr'. If 'neighbor_main_addr' is changed, the neighbor entry must get another position in the hash table.

Also, a problem arises when a node is neighbor to another node via two separate network interfaces (i.e. via 2 links).

In that case there will be one neighbor entry with linkcount 2 (1 for each link).

When the main IP address of the neighbor node changes, the HELLO messages from that interface are sent with the new IP address as source ('remote'), and with the new IP address as originator address ('remote_main').

Because 'remote' does not match any existing link, the while(tmp_link_set) loop will not find any existing link, so a new link entry is made.

Also, since ''remote_main' does not match any existing neighbor entry, a new neighbor entry is made:
  if(NULL == (neighbor = olsr_lookup_neighbor_table(remote_main)))
    {

The linkcount will be 1:
  neighbor->linkcount++;

Next, a HELLO message comes in via the second network interface, with the new IP address as originator address ('remote_main'). The fix you propose will correctly update the (old) neighbor entry (the one with with 'linkcount' 2):
    COPY_IP(&tmp_link_set->neighbor->neighbor_main_addr, remote_main);

After a while, the link with the old remote IP address will time out. However, the old, but updated neigbor entry will not be removed, since its 'linkcount' > 1. Its 'linkcount' value will be decreased from 2 to 1.

The result: we have two neighbor entries, each with 'linkcount' 1, and each with the same 'neighbor_main_addr'.

Besides this, there is a small bug in the function chk_if_changed (src/unix/ifnet.c)

       memcpy(&main_addr, 
       &((struct sockaddr_in *)&ifp->int_addr)->sin_addr.s_addr, 
       ipsize);
     }

   ifp->int_addr = ifr.ifr_addr;

The memcpy is copying the existing IP address in main_addr, not the new IP address. Or: ifr.ifr_addr is copied into ifp->int_addr too late.

Last: the olsr_process_received_mid function only adds MID aliases, not remove. I have written a function that removes the MID aliases which are registered but no longer declared in the received MID message. Here is the code:

/**
 *Remove aliases from 'entry' which are not listed in 'declared_aliases'.
 *
 *@param entry the MID entry
 *@param declared_aliases the list of declared aliases for the MID entry
 *
 *@return nada
 */
void
olsr_prune_aliases(union olsr_ip_addr *m_addr, struct mid_alias *declared_aliases)
{
  struct mid_entry *entry;
  olsr_u32_t hash;
  struct mid_address *registered_aliases;
  struct mid_address *previous_alias;
  struct mid_alias *save_declared_aliases = declared_aliases;

  hash = olsr_hashing(m_addr);

  /* Check for registered entry */
  for(entry = mid_set[hash].next;
      entry != &mid_set[hash];
      entry = entry->next)
    {
      if(COMP_IP(&entry->main_addr, m_addr))
        break;
    }
  if(entry == &mid_set[hash])
    {
      /* MID entry not found, nothing to prune here */
      return;
    }

  registered_aliases = entry->aliases;
  previous_alias = NULL;

  while(registered_aliases != 0)
    {
      struct mid_address *current_alias = registered_aliases;
      registered_aliases = registered_aliases->next_alias;

      declared_aliases = save_declared_aliases;

      /* Go through the list of declared aliases to find the matching current alias */
      while(declared_aliases != 0 &&
            ! COMP_IP(&current_alias->alias, &declared_aliases->alias_addr))
        {
          declared_aliases = declared_aliases->next;
        }

      if (declared_aliases == 0)
        {
          /* Current alias not found in list of declared aliases: free current alias */
          OLSR_PRINTF(1, "MID remove: (%s, ", olsr_ip_to_string(&entry->main_addr))
          OLSR_PRINTF(1, "%s)\n", olsr_ip_to_string(&current_alias->alias))

          /* Update linked list as seen by 'entry' */
          if (previous_alias != NULL)
            {
              previous_alias->next_alias = current_alias->next_alias;
            }
          else
            {
              entry->aliases = current_alias->next_alias;
            }

          /* Remove from hash table */
          DEQUEUE_ELEM(current_alias);
 
          free(current_alias);

          /*
           *Recalculate topology
           */
          changes_neighborhood = OLSR_TRUE;
          changes_topology = OLSR_TRUE;
        }
      else
        {
          previous_alias = current_alias;
        }
    }
}




Regards,
Erik



----- Original Message ----- 
From: "Acinonyx" <(spam-protected)>
To: "OLSR development" <(spam-protected)>
Sent: Friday, November 10, 2006 11:16 PM
Subject: Re: [olsr-dev] bug in mdi code


> Ok, here is a patch i made. It adds neighbor main address update on
> every link entry update.
>
> I tested it and it works.
>
> Bill
>
> -------------------
> diff -Naur olsrd-0.4.10/src/link_set.c olsrd-0.4.10-patched/src/link_set.c
> --- olsrd-0.4.10/src/link_set.c 2005-11-17 06:25:44.000000000 +0200
> +++ olsrd-0.4.10-patched/src/link_set.c 2006-11-10 23:45:24.000000000 +0200
> @@ -403,7 +466,10 @@
>      {
>        if(COMP_IP(remote, &tmp_link_set->neighbor_iface_addr) &&
>          COMP_IP(local, &tmp_link_set->local_iface_addr))
> -       return tmp_link_set;
> +        {
> +          COPY_IP(&tmp_link_set->neighbor->neighbor_main_addr,
> remote_main);
> +         return tmp_link_set;
> +        }
>        tmp_link_set = tmp_link_set->next;
>      }
>
>
>
>
> _______________________________________________
> olsr-dev mailing list
> (spam-protected)
> https://www.olsr.org/mailman/listinfo/olsr-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.olsr.org/pipermail/olsr-dev/attachments/20061112/a5d3c879/attachment.html>