batman-adv: Fixing wrap-around bug in vis

When the seqno for a vis packet had a wrap around from i.e. 255 to 0,
add_packet() would falsely claim the older packet with the seqno 255 as
newer as the one with the seqno of 0 and would therefore ignore the new
packet. This happens with all following vis packets until the old vis
packet expires after 180 seconds timeout. This patch fixes this issue
and gets rid of these highly undesired 3min. breaks for the vis-server.

Signed-off-by: Linus Lüssing <linus.luessing@web.de>
---
 batman-adv-kernelland/vis.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

Sorry, typo there, has to be ">=" and not "<=" of coures. My
fault.

On Thu, Mar 11, 2010 at 10:19:06PM +0100, Linus Lüssing wrote:
> When the seqno for a vis packet had a wrap around from i.e. 255 to 0,
> add_packet() would falsely claim the older packet with the seqno 255 as
> newer as the one with the seqno of 0 and would therefore ignore the new
> packet. This happens with all following vis packets until the old vis
> packet expires after 180 seconds timeout. This patch fixes this issue
> and gets rid of these highly undesired 3min. breaks for the vis-server.
> 
> Signed-off-by: Linus Lüssing <linus.luessing@web.de>
> ---
>  batman-adv-kernelland/vis.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/batman-adv-kernelland/vis.c b/batman-adv-kernelland/vis.c
> index fa8a487..5735a6a 100644
> --- a/batman-adv-kernelland/vis.c
> +++ b/batman-adv-kernelland/vis.c
> @@ -213,7 +213,8 @@ static struct vis_info *add_packet(struct vis_packet *vis_packet,
>  	old_info = hash_find(vis_hash, &search_elem);
>  
>  	if (old_info != NULL) {
> -		if (vis_packet->seqno - old_info->packet.seqno <= 0) {
> +		if (vis_packet->seqno - old_info->packet.seqno
> +		    <= 1 << 7 * sizeof(vis_packet->seqno)) {
>  			if (old_info->packet.seqno == vis_packet->seqno) {
>  				recv_list_add(&old_info->recv_list,
>  					      vis_packet->sender_orig);
> -- 
> 1.7.0
>

Linus Lüssing wrote:
> -               if (vis_packet->seqno - old_info->packet.seqno <= 0) {
> +               if (vis_packet->seqno - old_info->packet.seqno
> +                   <= 1 << 7 * sizeof(vis_packet->seqno)) {

Shouldn't that be 
 1 << 7 + 8 * (sizeof(vis_packet->seqno) - 1))?

Otherwise you would have left the the sizeof(vis_packet->seqno) most 
significant bits 0 and the rest one. And maybe a seq_before/seq_after static 
inline function could be created to make this thing a lot more readable. You 
could also do that in a macro if you don't want to assume the type of 
vis_packet->seqno... which is probably what you tried by using sizeof

#define seq_before(x,y) ((x - y) >= 1 << 7 + 8 * (sizeof(x) - 1))
#define seq_after(x,y) ((y - x) >= 1 << 7 + 8 * (sizeof(x) - 1))
....

And maybe you see here that the equal part which is needed bellow that line 
isn't possible. So it must be changed to 

if (!seq_after(vis_packet->seqno, old_info->packet.seqno))

I hope that this is right - please check twice.


Best regards,
	Sven