[Olsr-dev] Bug in function olsr_tc_update_edge ?

[Erik Tromp]

Hi all,

I think there may be a bug in the 0.5.5 (and also 0.5.4) version of OLSRd.

The function 'olsr_tc_update_edge' in the file 'tc_set.c' calls the function
'olsr_etx_significant_change' to determine whether 'the etx change is
meaningful enough in order to trigger a SPF calculation'. The function
'olsr_etx_significant_change' returns true if the change is 10% or more.

To do this, it compares the current 'tc_edge->link_quality' with the value
of 'neigh_link_quality' which is a value as received in the most recent TC
message. If the difference is large enough, the boolean 'edge_change' is set
to 1 to trigger the SPF calculation.

However, after that, the 'neigh_link_quality' is always copied into
'tc_edge->link_quality'.

What happens if the changes are very small, say 5% every time? None of the
changes will ever trigger the SPF calculation, because all the changes are <
10%.

In the pre-0.5.4 era [the good old times ;-) ], the relevant value was only
saved (in a 'saved_...' field) when the calculation was actually done. In
this way, incremental changes were detected with respect to the values as
used for the last calculation.

In the 0.5.3 version of tc_set.h:

struct topo_dst
{
  union olsr_ip_addr T_dest_addr;
  clock_t            T_time;
  olsr_u16_t         T_seq;
  struct topo_dst   *next;
  struct topo_dst   *prev;
  double             link_quality;
  double             inverse_link_quality;
  double             saved_link_quality;
  double             saved_inverse_link_quality;
}; 

The 'saved_...' fields have gone in the 0.5.4 version of tc_set.h:

struct tc_edge_entry
{
  struct avl_node    edge_node; /* edge_tree node in tc_entry */
  union olsr_ip_addr T_dest_addr; /* edge_node key */
  struct tc_edge_entry *edge_inv; /* shortcut, used during SPF calculation
*/
  struct tc_entry    *tc; /* backpointer to owning tc entry */
  clock_t            T_time; /* expiration timer, timer_node key */
  olsr_u16_t         T_seq; /* sequence number */
  olsr_u16_t         flags; /* misc flags */
  float              etx; /* metric used for SPF calculation */
  float              link_quality;
  float              inverse_link_quality;
};

Am i missing something here?

Another point of interest: the function 'pkt_get_u32' has an error. It is
left as an exercise to the reader to find it :-) . (It is not a problem,
since that function is never used, unless you are writing a link-cost
version of OLSR...)

Erik