Re: [PATCH v4 3/5] batman-adv: mcast: implement multicast packet reception and forwarding

[Simon Wunderlich <sw@simonwunderlich.de>]

[-- Attachment #1: Type: text/plain, Size: 1261 bytes --]

On Tuesday, December 27, 2022 8:34:07 PM CET Linus Lüssing wrote:
> +/**
> + * struct batadv_tvlv_mcast_tracker - payload of a multicast tracker tvlv
> + * @num_dests: number of subsequent destination originator MAC addresses
> + * @align: (optional) alignment bytes to make the tracker TVLV 4 bytes
> aligned, + * present if num_dests are even, not present if odd
> + */
> +struct batadv_tvlv_mcast_tracker {
> +       __be16  num_dests;
> +       __u8    align[2];
> +};
> +
The one thing which I really don't like is to have the alignment in the 
beginning, and depending on the number of entries. Normally, such alignments 
should be at the end of the structure so it is straight forward for a parser 
to omit it.

My understanding is that the alignment is due to technical reasons (mac 
address list is assembled by pushing the data to the front), perhaps to save 
another memove/memcpy. However, the data is collected by traversing various 
lists, and if performance would be a concern, then this data should be cached 
and this "technicality" wouldn't be needed either.

So please, skip the alignment in the front and have it in the back.

The rest of the packet format looks good from what I've seen.

Cheers,
      Simon


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]