[olsr-dev] bug in mdi code
Acinonyx
(spam-protected)
Tue Nov 21 21:04:34 CET 2006
Hello Erik,
I tried it and it produces a segmentation fault in olsr_calculate_lq_mpr:
MPRS: Update 10.2.16.78
Program received signal SIGSEGV, Segmentation fault.
0x080527a5 in olsr_calculate_lq_mpr ()
(gdb) bt
#0 0x080527a5 in olsr_calculate_lq_mpr ()
#1 0x08053245 in olsr_output_lq_tc ()
#2 0x0805db89 in scheduler ()
#3 0x080546ac in main ()
(gdb)
I could look further into it with the debuger but I believe that it
doesn't make sense to have simultaneously two links with the same local
and remote ip. I am not suprised that olsrd can't handle it because it
is as if it had the same link with two different neighbors.
Regards,
Bill
>
> Actually, I think the problem is surprisingly easy to solve.
>
> It's like working for the secret service. If your ID changes, no-one
> is updated about this. Your old ID just kind of disappears over time
> (times out), and your new ID gets known gradually.
>
> I propose to leave the link entries with the old ID to time out,
> taking their neighbor entries with them. The new link entries with the
> new ID automatically take care of setting up a new neighbor entry.
>
> What do you think of the following changes in the add_new_entry()
> function (src/link_set.c, lines 402 a.o.):
>
> while(tmp_link_set)
> {
> if(COMP_IP(remote, &tmp_link_set->neighbor_iface_addr) &&
> COMP_IP(local, &tmp_link_set->local_iface_addr) &&
> COMP_IP(remote_main,
> &tmp_link_set->neighbor->neighbor_main_addr))
> return tmp_link_set;
> tmp_link_set = tmp_link_set->next;
> }
>
> And also, a couple of lines down (lines 502 a.o.):
>
> /* Copy the main address - make sure this is done every time
> * as neighbors might change main address */
> /* Erik Tromp - OOPS! Don't do this! Neighbor entries are hashed
> through their
> * neighbor_main_addr field, and when that field is changed, their
> position
> * in the hash table is no longer correct, so that the function
> * olsr_lookup_neighbor_table() can no longer find the neighbor
> * entry. */
> /*COPY_IP(&neighbor->neighbor_main_addr, remote_main);*/
>
> ------------------------------------------------------------------------
> From: /Acinonyx <(spam-protected)>/
> Reply-To: /OLSR development <(spam-protected)>/
> To: /OLSR development <(spam-protected)>/
> Subject: /Re: [olsr-dev] bug in mdi code/
> Date: /Sun, 12 Nov 2006 15:29:22 +0200/
>
> Yes you are right Erik,
>
> the previous fix doesn't update the neighbor set. So we end up
> with a "ghost" neighbor entry. :(
>
> Ok, so could we delete (or decrease linkcount if > 1) and
> recreate a neighbor entry with the new main address?
>
> Do you think it will work?
>
> 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 <mailto:*@param> entry the MID entry
> *@param <mailto:*@param> declared_aliases the list of
> declared aliases for the MID entry
> *
> *@return <mailto:*@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(¤t_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(¤t_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) <mailto:(spam-protected)>>
> To: "OLSR development" <(spam-protected)
> <mailto:(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) <mailto:(spam-protected)>
> > https://www.olsr.org/mailman/listinfo/olsr-dev
> >
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> olsr-dev mailing list
> (spam-protected)
> https://www.olsr.org/mailman/listinfo/olsr-dev
>
>
>
>
> >_______________________________________________
> >olsr-dev mailing list
> >(spam-protected)
> >https://www.olsr.org/mailman/listinfo/olsr-dev
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> 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/20061121/52da8316/attachment.html>
More information about the Olsr-dev
mailing list