Re: [B.A.T.M.A.N.] [PATCH 01/10] batman-adv: Creating new neighbor discovery packet

[Marek Lindner <lindner_marek@yahoo.de>]

On Tuesday 14 December 2010 10:58:07 Linus Lüssing wrote:
> This patch creates the new neighbor discovery packet type (which is not
> actually being used with this patch). It will later be responsible to
> detect neighbors and calculate the link quality to them.

In fact, most of the patch renames batman_packet to batman_packet_ogm. Why not 
making this a rename patch only ? The next patch still renames some more stuff 
which could go into this patch as well (e.g. schedule_own_packet).

How about renaming batman_packet to batman_ogm and batman_packet_ndp to 
batman_ndp ? Or ogm_packet / ndp_packet ? The 80 chars per line limitation 
does not make these long names very handy ...


> -#define BAT_PACKET       0x01
> -#define BAT_ICMP         0x02
> -#define BAT_UNICAST      0x03
> -#define BAT_BCAST        0x04
> -#define BAT_VIS          0x05
> -#define BAT_UNICAST_FRAG 0x06
> +#define BAT_PACKET_NDP   0x01
> +#define BAT_PACKET_OGM   0x02
> +#define BAT_ICMP         0x03
> +#define BAT_UNICAST      0x04
> +#define BAT_BCAST        0x05
> +#define BAT_VIS          0x06
> +#define BAT_UNICAST_FRAG 0x07

Although I like this reordering (I would like to see BAT_UNICAST followed by 
BAT_UNICAST_FRAG) I think we can't easily touch this without annoying the 
wireshark folks. Maybe we don't care because they hate us already ?


> -struct batman_packet {
> +/* Neighbor discovery packet */
> +struct batman_packet_ndp {
> +	uint8_t  packet_type;
> +	uint8_t  version;  /* batman version field */
> +	uint8_t  orig[6];
> +	uint32_t seqno;
> +	uint8_t  num_neighbors;
> +	uint8_t  align[3];
> +} __attribute__((packed));

This should go into one of the later patches where it is used.

Cheers,
Marek