[next,3/4] batman-adv: fix size of batadv_icmp_header
Commit Message
struct batadv_icmp_header currently has a size of 17, which will be
padded to 20 on some architectures. Fix this by moving more common
fields into the icmp header.
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
packet.h | 13 +++++--------
routing.c | 6 +++---
2 files changed, 8 insertions(+), 11 deletions(-)
Comments
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 02/12/13 20:38, Simon Wunderlich wrote:
> struct batadv_icmp_header { uint8_t packet_type; @@ -208,18
> +211,16 @@ struct batadv_icmp_header { uint8_t dst[ETH_ALEN];
> uint8_t orig[ETH_ALEN]; uint8_t uid; + uint8_t rr_cur; + __be16
> seqno; };
>
seqno was not in the header on purpose because new features may need a
larger seqno space (e.g. the bw meter uses 32bits long seqnos).
I think we have to re-work this structure differently..
rr_cur was not in the header because it is only used by the PING and
not by all the others ICMP packets. But having it in the header is
harmless I think.
Cheers,
- --
Antonio Quartulli
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
iEYEARECAAYFAlKc6pAACgkQpGgxIkP9cwdUkgCeNYm2xfFIIKO8OL2TJ9au7rQe
4ncAnieIgls5N4Ea+939rMD2taypkJP5
=nWqI
-----END PGP SIGNATURE-----
Hey Antonio,
> On 02/12/13 20:38, Simon Wunderlich wrote:
> > struct batadv_icmp_header { uint8_t packet_type; @@ -208,18
> > +211,16 @@ struct batadv_icmp_header { uint8_t dst[ETH_ALEN];
> > uint8_t orig[ETH_ALEN]; uint8_t uid; + uint8_t rr_cur; + __be16
> > seqno; };
>
> seqno was not in the header on purpose because new features may need a
> larger seqno space (e.g. the bw meter uses 32bits long seqnos).
> I think we have to re-work this structure differently..
ah ok, didn't know that. Well I also thought to move that uid into the other
structures to have the icmp header at 16 bytes, but then we need to access the
icmp_packet instead of icmp_header in functions like batadv_socket_write() - I
changed that just recently. ;)
What do you suggest? These new types can't work with 16bit seqnos? :)
>
> rr_cur was not in the header because it is only used by the PING and
> not by all the others ICMP packets. But having it in the header is
> harmless I think.
Yeah, agreed.
Cheers,
Simon
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
On 03/12/13 11:54, Simon Wunderlich wrote:
> Hey Antonio,
>
>> On 02/12/13 20:38, Simon Wunderlich wrote:
>>> struct batadv_icmp_header { uint8_t packet_type; @@ -208,18
>>> +211,16 @@ struct batadv_icmp_header { uint8_t dst[ETH_ALEN];
>>> uint8_t orig[ETH_ALEN]; uint8_t uid; + uint8_t rr_cur; +
>>> __be16 seqno; };
>>
>> seqno was not in the header on purpose because new features may
>> need a larger seqno space (e.g. the bw meter uses 32bits long
>> seqnos). I think we have to re-work this structure differently..
>
> ah ok, didn't know that. Well I also thought to move that uid into
> the other structures to have the icmp header at 16 bytes, but then
> we need to access the icmp_packet instead of icmp_header in
> functions like batadv_socket_write() - I changed that just
> recently. ;)
>
> What do you suggest? These new types can't work with 16bit seqnos?
> :)
>>
No, we can;t use 16bit seqnos for that...sorry :)
Right now I don't have a clear solution in mind. I'll try to look at
that header closely tomorrow.
Cheers,
- --
Antonio Quartulli
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
iQIcBAEBCAAGBQJSn9AQAAoJEEKTMo6mOh1VyCUQAJhyn+YJIUznEHq/U3GkIvy7
q2rXosmUKmZX5wxdXQDOPkVzUGRwBf2azNq77F7oIjnzJ/oGCJKOKtvPZ6gFzH/l
MeSEgk3LZ6RjEHPO46lVFM5YeaF5V/xZzuFVUaEfU4mOlotFYqMlJeeDWmCA8lzV
eglll0q7xwHAJkmD0Dg8qh/Yqep+lmjF4RNYz4QTP4VShyWP5swX9EedoocnZ1Rp
COFgCV0ZwpV95PIrX/cnrE7qX7VNJfqJdyL7S8Nabfd1pdhxE1j45I0CdETPszSs
hdN6tgQjcghQUwVAauXyy4oU51B0ISjjcV3Oi9CyLIxS7wF1SwnlHHL5R0Lt372/
WPm62gAhM7qHjOTQGPZ+Z7Ap/qSxXO6nmUMEJUPdXJYmVlg2ZkryPPEFD4SzDfPl
8fnWjtmO06yo3WMIozTqav+ZCNt/qinyt/JDINV+Pos7oxW9JZFF1vWyo1NZyljW
R88GmwOnPoMvPL0M6O4d+I3wrEMHI0MFNLkP6GAiRzC/HhiuYxgoCZj47kLtrb+U
Dtc0Ix7cuvvUt3GC4xBAXJ/12fGljldXRrcVVQkoluboVpgls8RScbCztET/dpTr
1QQhUWFgiR7BSP1lZyZDu3hxFiPBOBCqzcmEXQ+Po1HYyzOB1Ju4X1JCNIrxBcvO
SZ/IhMR6OWuI61qhv7GN
=3QQX
-----END PGP SIGNATURE-----
@@ -199,6 +199,9 @@ struct batadv_ogm_packet {
* @dst: address of the destination node
* @orig: address of the source node
* @uid: local ICMP socket identifier
+ * @rr_cur: for record route packets: number of entries the rr array,
+ * reserved for other packet types.
+ * @seqno: ICMP sequence number
*/
struct batadv_icmp_header {
uint8_t packet_type;
@@ -208,18 +211,16 @@ struct batadv_icmp_header {
uint8_t dst[ETH_ALEN];
uint8_t orig[ETH_ALEN];
uint8_t uid;
+ uint8_t rr_cur;
+ __be16 seqno;
};
/**
* batadv_icmp_packet - ICMP packet
* @icmph: common ICMP header
- * @reserved: not used - useful for alignment
- * @seqno: ICMP sequence number
*/
struct batadv_icmp_packet {
struct batadv_icmp_header icmph;
- uint8_t reserved;
- __be16 seqno;
};
#define BATADV_RR_LEN 16
@@ -227,14 +228,10 @@ struct batadv_icmp_packet {
/**
* batadv_icmp_packet_rr - ICMP RouteRecord packet
* @icmph: common ICMP header
- * @rr_cur: number of entries the rr array
- * @seqno: ICMP sequence number
* @rr: route record array
*/
struct batadv_icmp_packet_rr {
struct batadv_icmp_header icmph;
- uint8_t rr_cur;
- __be16 seqno;
uint8_t rr[BATADV_RR_LEN][ETH_ALEN];
};
@@ -421,12 +421,12 @@ int batadv_recv_icmp_packet(struct sk_buff *skb,
icmph = (struct batadv_icmp_header *)skb->data;
icmp_packet_rr = (struct batadv_icmp_packet_rr *)icmph;
- if (icmp_packet_rr->rr_cur >= BATADV_RR_LEN)
+ if (icmp_packet_rr->icmph.rr_cur >= BATADV_RR_LEN)
goto out;
- memcpy(&(icmp_packet_rr->rr[icmp_packet_rr->rr_cur]),
+ memcpy(&(icmp_packet_rr->rr[icmp_packet_rr->icmph.rr_cur]),
ethhdr->h_dest, ETH_ALEN);
- icmp_packet_rr->rr_cur++;
+ icmp_packet_rr->icmph.rr_cur++;
}
/* packet for me */