| Message ID | 1306534222-11710-1-git-send-email-ordex@autistici.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Saturday 28 May 2011 00:10:22 Antonio Quartulli wrote: > Move the ttl field to the third position of unicast_packet and > unicast_frag_packet. In this way it possible to give them a better shape > for later usage > > Signed-off-by: Antonio Quartulli <ordex@autistici.org> > --- [..] > struct unicast_frag_packet { > uint8_t packet_type; > uint8_t version; /* batman version field */ > - uint8_t dest[6]; > uint8_t ttl; > + uint8_t align; > + uint8_t dest[6]; > uint8_t flags; > + uint8_t align; > uint8_t orig[6]; > uint16_t seqno; > } __packed; Wait a second - are there two "align" in a single struct. Sry, have to Nack that one :) Kind regards, Sven
On Saturday 28 May 2011 00:20:21 Sven Eckelmann wrote: > > struct unicast_frag_packet { > > uint8_t packet_type; > > uint8_t version; /* batman version field */ > > > > - uint8_t dest[6]; > > > > uint8_t ttl; > > > > + uint8_t align; > > + uint8_t dest[6]; > > > > uint8_t flags; > > > > + uint8_t align; > > > > uint8_t orig[6]; > > uint16_t seqno; > > } __packed; > > Wait a second - are there two "align" in a single struct. Sry, have to > Nack that one :) Yes, I suggested that because the tt patches will replace the first align with the ttvn field. Cheers, Marek
Marek Lindner wrote: > On Saturday 28 May 2011 00:20:21 Sven Eckelmann wrote: > > > struct unicast_frag_packet { > > > > > > uint8_t packet_type; > > > uint8_t version; /* batman version field */ > > > > > > - uint8_t dest[6]; > > > > > > uint8_t ttl; > > > > > > + uint8_t align; > > > + uint8_t dest[6]; > > > > > > uint8_t flags; > > > > > > + uint8_t align; > > > > > > uint8_t orig[6]; > > > uint16_t seqno; > > > > > > } __packed; > > > > Wait a second - are there two "align" in a single struct. Sry, have to > > Nack that one :) > > Yes, I suggested that because the tt patches will replace the first align > with the ttvn field. But it doesn't compile. I cannot forward stuff which is too broken. And personally I don't care who suggested it when my builtin c-parser explodes. Ok, now I did the "work" to apply and and to compile that stuff to prove my point. Here are the slightly shortened results (gcc-4.6 from sid): CC [M] aggregation.o In file included from types.h:27:0, from main.h:121, from aggregation.c:22: packet.h:114:11: error: duplicate member ‘align’ make[2]: *** [aggregation.o] Error 1 make[1]: *** [_module_/batman-adv] Error 2 make[1]: Leaving directory `linux-2.6.39' make: *** [all] Error 2 My suggestion: Rename the first align in every packet structure to 'reserved'. Then to the next part which I don't like: the patch forgot to touch batman_packet, icmp_packet, icmp_packet_rr, bcast_packet and vis_packet. It doesn't make sense to patch half of the on-wire types to reduce the number of COMPAT_VERSIONs. Kind regards, Sven
On Saturday 28 May 2011 09:26:30 Sven Eckelmann wrote: > > Yes, I suggested that because the tt patches will replace the first align > > with the ttvn field. > > But it doesn't compile. I cannot forward stuff which is too broken. And > personally I don't care who suggested it when my builtin c-parser explodes. You got a point here. :-) > My suggestion: Rename the first align in every packet structure to > 'reserved'. Ok. > Then to the next part which I don't like: the patch forgot to touch > batman_packet, icmp_packet, icmp_packet_rr, bcast_packet and vis_packet. It > doesn't make sense to patch half of the on-wire types to reduce the number > of COMPAT_VERSIONs. If we touch every packet definition we also have to ensure that the code touching these packets does not implicitly assume certain positions (like route_unicast_packet() does). Regards, Marek
diff --git a/packet.h b/packet.h index eda9965..e464ceb 100644 --- a/packet.h +++ b/packet.h @@ -32,7 +32,7 @@ #define BAT_UNICAST_FRAG 0x06 /* this file is included by batctl which needs these defines */ -#define COMPAT_VERSION 12 +#define COMPAT_VERSION 14 #define DIRECTLINK 0x40 #define VIS_SERVER 0x20 #define PRIMARIES_FIRST_HOP 0x10 @@ -99,16 +99,19 @@ struct icmp_packet_rr { struct unicast_packet { uint8_t packet_type; uint8_t version; /* batman version field */ - uint8_t dest[6]; uint8_t ttl; + uint8_t align; + uint8_t dest[6]; } __packed; struct unicast_frag_packet { uint8_t packet_type; uint8_t version; /* batman version field */ - uint8_t dest[6]; uint8_t ttl; + uint8_t align; + uint8_t dest[6]; uint8_t flags; + uint8_t align; uint8_t orig[6]; uint16_t seqno; } __packed;
Move the ttl field to the third position of unicast_packet and unicast_frag_packet. In this way it possible to give them a better shape for later usage Signed-off-by: Antonio Quartulli <ordex@autistici.org> --- packet.h | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-)