Andrew Lunn <[email protected]> schrieb am 16.02.2011 07:08:11: > [Bild entfernt] > > Re: [B.A.T.M.A.N.] [PATCH 02/13] batman-adv: Add explicit batman > header structure > > Andrew Lunn > > an: > > The list for a Better Approach To Mobile Ad-hoc Networking > > 16.02.2011 07:08 > > Kopie: > > Linus L??ssing > > On Tue, Feb 15, 2011 at 06:12:17PM +0100, Linus L??ssing wrote: > > Just a minor style adjustment, to give people a hint which fields should > > not be reordered and need to be at the beginning of each packet with > > batman-adv's frame type. > > Hi Linus > > > +struct batman_header { > > + uint8_t packet_type; > > + uint8_t version; /* batman version field */ > > + uint8_t ttl; > > + uint8_t align; > > +} __packed; > > + > > struct batman_packet { > > - uint8_t packet_type; > > - uint8_t version; /* batman version field */ > > + struct batman_header header; > > + uint32_t seqno; > > uint8_t flags; /* 0x40: DIRECTLINK flag, 0x20 VIS_SERVER flag... */ > > uint8_t tq; > > - uint32_t seqno; > > uint8_t orig[6]; > > uint8_t prev_sender[6]; > > - uint8_t ttl; > > uint8_t num_hna; > > uint8_t gw_flags; /* flags related to gateway class */ > > - uint8_t align; > > } __packed; > > Two different ideas, both triggered by the align byte in header. This > byte is a waste of space, it is not used, and it is probably not easy > to find something which all packet types are going to need in the > future.
Sure, anything that not all packet types have in common can't go inside of the batman_header. But are those three variables something we could agree on, being something that all packet types will need today and in the future? version + packet_type is something we'll always need I think, the TTL might be something to argue about, as packets that do not get routed would not necessarily need them (though it could be a "safety belt" in case of bugs, to really make sure packets only intended for one hop will not be routed). > > 1) Did you try not having it. So long header is __packed, and all the > structures it is used in are __packed, i think the compiler will do > the right thing. The downside is that alignment is not obvious. Most > people will assume header is 4 bytes, not 3, and think the alignment > of the packets is wrong. > > 2) Did you consider using a union? It is a more invasive patch, but > the alignment is then clear. The downside is sizeof() no longer gives > you the per packet type size, rather it gives you the size of the > biggest member of the union. So this makes the change even more > invasive and bug prone. Hmm, no, actually haven't thought about that. So we'd actually put the batman packet types as a union inside of the batman-header structure, instead of putting the header structure at the front of each packet type. Don't know if I'd like that either, due to the reason you've mentioned. > > I think i prefer 1), with appropriate comments to explain the > alignment issue. So I think I'd prefer the first option, that should work too :). Just didn't think that an extra Byte would matter that much, and had the readability / making the alignment a little more obvious in mind. (And who knows when we might need to increase the version number size already ;) ) > > Andrew Cheers, Linus
