batman-adv: move the ttl field to the third position of unicast_* packets

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

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