[B.A.T.M.A.N.] [PATCH] batman-adv: move smallest_signed_int(), seq_before() and seq_after() into main.h
smallest_signed_int(), seq_before() and seq_after() are very useful functions that help to handle comparisons between sequence numbers. However they were only defined in vis.c. With this patch every batman-adv function will be able to use them.
Signed-off-by: Antonio Quartulli ordex@autistici.org --- main.h | 16 ++++++++++++++++ vis.c | 16 ---------------- 2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/main.h b/main.h index 3ca3941..90db244 100644 --- a/main.h +++ b/main.h @@ -181,4 +181,20 @@ static inline int compare_eth(void *data1, void *data2)
#define atomic_dec_not_zero(v) atomic_add_unless((v), -1, 0)
+/* 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) _dummy = (x - y); \ + _dummy > smallest_signed_int(_dummy); }) +#define seq_after(x, y) seq_before(y, x) + #endif /* _NET_BATMAN_ADV_MAIN_H_ */ diff --git a/vis.c b/vis.c index c39f20c..e1ed552 100644 --- a/vis.c +++ b/vis.c @@ -30,22 +30,6 @@
#define MAX_VIS_PACKET_SIZE 1000
-/* 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) _dummy = (x - y); \ - _dummy > smallest_signed_int(_dummy); }) -#define seq_after(x, y) seq_before(y, x) - static void start_vis_timer(struct bat_priv *bat_priv);
/* free the info */
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(-)
diff --git a/include/linux/net.h b/include/linux/net.h index 94de83c..c7bc9bf 100644 --- a/include/linux/net.h +++ b/include/linux/net.h @@ -295,4 +295,21 @@ extern struct ratelimit_state net_ratelimit_state; #endif
#endif /* __KERNEL__ */ + +/* 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) _dummy = (x - y); \ + _dummy > smallest_signed_int(_dummy); }) +#define seq_after(x, y) seq_before(y, x) + #endif /* _LINUX_NET_H */
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
Antonio Quartulli wrote:
smallest_signed_int(), seq_before() and seq_after() are very useful functions that help to handle comparisons between sequence numbers. However they were only defined in vis.c. With this patch every batman-adv function will be able to use them.
Signed-off-by: Antonio Quartulli ordex@autistici.org
Acked-by: Sven Eckelmann sven@narfation.org
David S. Miller doesn't want to have the seq_before/seq_after in a global header: http://lkml.org/lkml/2011/5/19/88
I would still like to propose the type check on top of that patch
Kind regards, Sven
main.h | 16 ++++++++++++++++ vis.c | 16 ---------------- 2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/main.h b/main.h index 3ca3941..90db244 100644 --- a/main.h +++ b/main.h @@ -181,4 +181,20 @@ static inline int compare_eth(void *data1, void *data2)
#define atomic_dec_not_zero(v) atomic_add_unless((v), -1, 0)
+/* 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) _dummy = (x - y); \
_dummy > smallest_signed_int(_dummy); })+#define seq_after(x, y) seq_before(y, x)
#endif /* _NET_BATMAN_ADV_MAIN_H_ */ diff --git a/vis.c b/vis.c index c39f20c..e1ed552 100644 --- a/vis.c +++ b/vis.c @@ -30,22 +30,6 @@
#define MAX_VIS_PACKET_SIZE 1000
-/* 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) _dummy = (x - y); \
_dummy > smallest_signed_int(_dummy); })-#define seq_after(x, y) seq_before(y, x)
static void start_vis_timer(struct bat_priv *bat_priv);
/* free the info */
seq_before and seq_after depend on the fact that both sequence numbers have the same type and thus the same bitwidth. We can ensure that by compile time checking using a compare between the pointer to the temporary buffers which were created using the typeof of both parameters. For example gcc would create a warning like "warning: comparison of distinct pointer types lacks a cast".
Signed-off-by: Sven Eckelmann sven@narfation.org --- main.h | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/main.h b/main.h index db29444..3395bf9 100644 --- a/main.h +++ b/main.h @@ -196,8 +196,11 @@ static inline int compare_eth(const void *data1, const void *data2) * - 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_before(x, y) ({typeof(x) _d1 = (x); \ + typeof(y) _d2 = (y); \ + typeof(x) _dummy = (_d1 - _d2); \ + (void) (&_d1 == &_d2); \ + _dummy > smallest_signed_int(_dummy); }) #define seq_after(x, y) seq_before(y, x)
#endif /* _NET_BATMAN_ADV_MAIN_H_ */
On Thursday 19 May 2011 21:43:08 Sven Eckelmann wrote:
seq_before and seq_after depend on the fact that both sequence numbers have the same type and thus the same bitwidth. We can ensure that by compile time checking using a compare between the pointer to the temporary buffers which were created using the typeof of both parameters. For example gcc would create a warning like "warning: comparison of distinct pointer types lacks a cast".
Applied in revision 288421e.
Thanks, Marek
On Wednesday 18 May 2011 09:20:50 Antonio Quartulli wrote:
smallest_signed_int(), seq_before() and seq_after() are very useful functions that help to handle comparisons between sequence numbers. However they were only defined in vis.c. With this patch every batman-adv function will be able to use them.
Applied in revision 7962ae7.
Thanks, Marek
b.a.t.m.a.n@lists.open-mesh.org
-
Antonio Quartulli -
Marek Lindner -
Sven Eckelmann