• if ((vis_packet->seqno - old_info->packet.seqno) > 127) {
  

If anyone might have a more generic solution instead of using the fixed of 127 without having to utilise math.h, that'd be great.

Ok, and no some more words to the tests and discussions with Marek yesterday and their results.

What I also just noticed is, that when I refresh the vis-output every second only the TQ values of _2_ entries at a time get updated, the other ones are static for quite a while. However, looking at wireshark tells me, that I'm still getting a vis-packet of all other 4 nodes every second. So it looks like the vis-server might not be always updating its vis_hash to me, but not sure about that.

The issue seemed to be the thing I just sent a patch for. Pfeh, then I didn't see any ghost bug which was sometimes there and sometimes not. I started to doubt about the setup a lot first :).

I'd expect all 5 nodes to have a direct connection to all other 4 nodes. However, for instance 06:22:b0:98:87:f9 is just having three. A couple of seconds later, the missing entry is there again, but other ones can be missing.

The line which is causing this behavior is in generate_vis_packet(): ------ 398 if (orig_node->router != NULL 399 && compare_orig(orig_node->router->addr, 400 orig_node->orig) ------ It turned out that this is not a bug and that it was intended to leave out those vis entries. Looks like I had slightly different expectations at the vis-server. I wanted to see all possible links and alternative next-hops in the vis-graph while Marek enlightened me, that this would increase the throughput usage a lot as for instance a far distant node, from which we'd receive a batman-ogm up to every 120 seconds would result into a vis-entry then. So including alternative/non-used next-hops in the vis-output might need a new discussion.

Cheers, Linus

Sven Eckelmann wrote:

#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)) ....

That must be #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))

Or otherwise it would be wrong balanced between equal, after and before. In other words: For every x in N0 and y in N0 is only one true of (x == y) or seq_before(x, y) or seq_after(x, y) after the change of >= to >

Best regards, Sven

Sorry still does not seem to work :). Despite testing it on a setup in batman itself I also tested it with a small c program (which does not work as expected either): ----- #include <stdio.h> #include <stdint.h>

#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))

int main() { uint8_t old = 0; uint8_t new = 1; if (!seq_after(new,old)) printf("foobar\n");

return 0; } ----- $ ./test foobar $

On Fri, Mar 12, 2010 at 12:04:47AM +0100, Sven Eckelmann wrote:

Sven Eckelmann wrote:

...

That must be #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))

Nearly correct but accidentally deleted a =

#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))

Best regards, Sven

Linus Lüssing wrote:

Sorry still does not seem to work :). Despite testing it on a setup in batman itself I also tested it with a small c program (which does not work as expected either):

But your version works? I would really doubt that when we do the same. The macro will only replace the thing and we would have the same formular in that situation.

---

#include <stdio.h> #include <stdint.h>

#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))

int main() { uint8_t old = 0; uint8_t new = 1; if (!seq_after(new,old)) printf("foobar\n");

    return 0;

}

$ ./test foobar $

Now it gets real cruel, but who cares... C++ wasn't accept in the kernel, so we have to deal with it using some GNUish stuff.

#define seq_before(x,y) ({typeof(x) _dummy = (x - y); \ _dummy > 1u << (7u + 8u * (sizeof(_dummy) - 1u));}) #define seq_after(x,y) ({typeof(x) _dummy = (y - x); \ _dummy >= 1u << (7u + 8u * (sizeof(_dummy) - 1u));})

Thanks for testing (I never did it :) ) and please test that one.

Best regards, Sven

Sven Eckelmann wrote:

Sven Eckelmann wrote:

...

Now it gets real cruel, but who cares... C++ wasn't accept in the kernel, so we have to deal with it using some GNUish stuff.

#define seq_before(x,y) ({typeof(x) _dummy = (x - y); \ _dummy > 1u << (7u + 8u * (sizeof(_dummy) - 1u));}) #define seq_after(x,y) ({typeof(x) _dummy = (y - x); \ _dummy >= 1u << (7u + 8u * (sizeof(_dummy) - 1u));})

After a small discussion with Marek I would like to suggest a easier readable and symmetric definition

/* Returns the smallest signed integer in two's complement with the sizeof x */ #define smallest_signed_int(x) (1u << (7u + 8u * (sizeof(x) - 1u)))

/* Checks if a sequence number x is a predecessor/successor of y. they handle overflows/underflows and can correctly check for a predecessor/successor unless the variable sequence number has grown by more then 2**(bitwidth(x)-1)-1. This means that for an uint8_t with the maximum value 255, it would think: * when adding nothing - it is neither a predecessor nor a successor

• before adding more than 127 to the starting value - it is a predecessor,
• when adding 128 - it is neither a predecessor nor a successor, * after

adding more than 127 to the starting value - it is a successor */ #define seq_before(x,y) ({typeof(x) _dummy = (x - y); \ _dummy > smallest_signed_int(_dummy); }) #define seq_after(x,y) seq_before(y,x)

@Linus: Here a small testcase - just as prove that I tested it this time :D

Best regards, Sven

On Sunday 14 March 2010 23:46:28 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.

Applied in rev 1597.

Thanks, Marek

On Fri, Mar 12, 2010 at 09:57:40PM +0100, Simon Wunderlich wrote:

#define seq_before(x,y) ((int8_t) (x - y) < 0) #define seq_after(x,y) seq_before(y,x)

Not so much bitshifting and may a little bit easier to understand, but also not general and does not pass Svens regression test ... :(

Wrong seq_before(0, 128) == true Wrong seq_after(0, 128) == true Wrong seq_before(1, 129) == true Wrong seq_after(1, 129) == true Wrong seq_before(2, 130) == true Wrong seq_after(2, 130) == true Wrong seq_before(3, 131) == true Wrong seq_after(3, 131) == true Wrong seq_before(4, 132) == true Wrong seq_after(4, 132) == true Wrong seq_before(5, 133) == true Wrong seq_after(5, 133) == true

What do you think? ;)

This is the version which also was mentioned by andrew - which has the advantage that it doesn't have "complicated" code, but isn't general purpose for unsigned ints. The only reason I started with the more complicated stuff was that Linus tried to make it easier to change the packet definition without ruining the code at different places.

That it doesn't pass the test is only the due to my symmetric definition of the operation. I am not really intrested in changing that code, but to support Linus.

Best regards, Sven