[Olsr-dev] Bug in LQ-Calculation (with patch)

[Andreas Jacobs]

Hello,

oje, I see I've started a longer discussion. At least, this is more
reaction than I've got on the Sourceforge bugtracker. I'm glad about that.

I like your solution, although there is a small problem (see below):
> BTW If I reformulate the subexpression in question
> ----  snip  ----
> 4 * ((entry->loss_window_size / 4 - 1) * entry->total_packets +
entry->loss_window_size) / entry->loss_window_size
> ----  snip  ----
> I come to 
> ----  snip  ----
> (1 - 4 / entry->loss_window_size) * entry->total_packets + 4
> ----  snip  ----
>which seems equivalent to the old one.

Well, it were, if we had floating point arithmetic here. The problem is,
that you divide 4 by entry->loss_window_size _before_ you multiply it with
entry->total_packets. Probably do it the other way round?
----  snip ----
entry->total_packets - 4 * entry->total_packets / entry->loss_window_size +
4
----  or ----
entry->total_packets + 4 * (1 - entry->total_packets /
entry->loss_window_size)
----  snip ----
Anyway, I like these solutions more than the current formula, because they
don't require loss_window_size to be a multiple of 4, but still don't
introduce unnecessary floating point operations. I think, the second one of
my two formulation even makes the intention of that formular more clearly
visible: The first summand is what we would usually expect, and the second
summand adds a penalty if entry->total_packets is smaller than
entry->loss_window_size.
By the way, is the clause "entry->loss_window_size < (2 * 4) ?
entry->loss_window_size:" still necessary then?

> After looking more into it: Why do we have the "loss_window_size" in
> "struct link_entry" in the first place? It is always initialized with
> "olsr_cnf->lq_wsize" (which constant through run-time AFAIK) and
> "loss_window_size" is never changed AFAICS?
> Just killing like in the attached patch?

> Since entry->loss_window_size is constant, we can precalculate it once
> (floating point) and simply use it.

Andreas Jacobs