[RFC/PATCH] net: add seq_before/seq_after functions

  Introduce two operations to handle comparison between packet sequence
numbers taking into account overflow/wraparound. Batman-adv uses
these functions already to check for successor packet even in case of
overflow.
---

I added this two functions in net.h because I didn't really know where
best placement is. I saw several modules that redefine their own functions
for the same purpose.

 include/linux/net.h |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)
  

On Wednesday 18 May 2011 14:22:13 Antonio Quartulli wrote:
> Introduce two operations to handle comparison between packet sequence
> numbers taking into account overflow/wraparound. Batman-adv uses
> these functions already to check for successor packet even in case of
> overflow.
> ---
> 
> I added this two functions in net.h because I didn't really know where
> best placement is. I saw several modules that redefine their own functions
> for the same purpose.

Wouldn't it be better to ask linux-kernel what Linus/David/... think
about the idea to provide the functionality of seq_after and seq_before
seen at net/batman-adv/vis.c. They will tell you that this stuff
has to be placed in include/linux/kernel.h and let you know that it is a
complete stupid implementation because it never checks that the type is
the same. Then you will start to reimplement it the following way:


/* 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 a 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) _d1 = (x); \
			{typeof(y) _d2 = (y); \
			(void) (&_d1 == &_d2); \
			(_d1 - _d2) > smallest_signed_int(_d1); })
#define seq_after(x, y) seq_before(y, x)


Or you could just sent this version as inline code and ask if it should
be moved from net/batman-adv/vis.c to a kernel header like
include/linux/kernel.h to provide a somewhat general solution for the
seq_before/seq_after. And maybe suggest the use in other protocols.
Hopefully they will say something like "no" or "yes, place it there".
Independent from the answer, we have a reference which clearly protects
our actions (your patch(es)). But somebody will still scream :)

Just look at Documentation/development-process/3.Early-stage
"3.3: WHO DO YOU TALK TO?" to find more about that "process".

Btw. I never tested the changes which does the type comparison. And
propably the ppp guys will scream because you conflict with there
definition of seq_before and seq_after - so you would have to remove
that definition from both places, get acks from both parties by proving
that everything still works the same way and get it merged by someone.
But providing a patch is propably the second step after getting the the
ok by a main kernel maintainer.

... and I will now write my own daily soap about people on mailinglist.

Kind regards,
	Sven