All of lore.kernel.org
 help / color / mirror / Atom feed
* [B.A.T.M.A.N.] [PATCH] batman-adv: move the ttl field to the third position of unicast_* packets
@ 2011-05-27 22:10 Antonio Quartulli
  2011-05-27 22:20 ` Sven Eckelmann
  0 siblings, 1 reply; 14+ messages in thread
From: Antonio Quartulli @ 2011-05-27 22:10 UTC (permalink / raw)
  To: B.A.T.M.A.N

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

diff --git a/packet.h b/packet.h
index eda9965..e464ceb 100644
--- a/packet.h
+++ b/packet.h
@@ -32,7 +32,7 @@
 #define BAT_UNICAST_FRAG 0x06
 
 /* this file is included by batctl which needs these defines */
-#define COMPAT_VERSION 12
+#define COMPAT_VERSION 14
 #define DIRECTLINK 0x40
 #define VIS_SERVER 0x20
 #define PRIMARIES_FIRST_HOP 0x10
@@ -99,16 +99,19 @@ struct icmp_packet_rr {
 struct unicast_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];
 } __packed;
 
 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;
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH] batman-adv: move the ttl field to the third position of unicast_* packets
  2011-05-27 22:10 [B.A.T.M.A.N.] [PATCH] batman-adv: move the ttl field to the third position of unicast_* packets Antonio Quartulli
@ 2011-05-27 22:20 ` Sven Eckelmann
  2011-05-27 22:35   ` Marek Lindner
  0 siblings, 1 reply; 14+ messages in thread
From: Sven Eckelmann @ 2011-05-27 22:20 UTC (permalink / raw)
  To: b.a.t.m.a.n

[-- Attachment #1: Type: Text/Plain, Size: 711 bytes --]

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

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH] batman-adv: move the ttl field to the third position of unicast_* packets
  2011-05-27 22:20 ` Sven Eckelmann
@ 2011-05-27 22:35   ` Marek Lindner
  2011-05-28  7:26     ` Sven Eckelmann
  0 siblings, 1 reply; 14+ messages in thread
From: Marek Lindner @ 2011-05-27 22:35 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH] batman-adv: move the ttl field to the third position of unicast_* packets
  2011-05-27 22:35   ` Marek Lindner
@ 2011-05-28  7:26     ` Sven Eckelmann
  2011-05-28  8:43       ` [B.A.T.M.A.N.] [PATCH-sven] batman-adv: Unify the first 4 bytes in each packet Sven Eckelmann
  2011-05-28 11:20       ` [B.A.T.M.A.N.] [PATCH] batman-adv: move the ttl field to the third position of unicast_* packets Marek Lindner
  0 siblings, 2 replies; 14+ messages in thread
From: Sven Eckelmann @ 2011-05-28  7:26 UTC (permalink / raw)
  To: b.a.t.m.a.n; +Cc: Marek Lindner

[-- Attachment #1: Type: Text/Plain, Size: 1742 bytes --]

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

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [B.A.T.M.A.N.] [PATCH-sven] batman-adv: Unify the first 4 bytes in each packet
  2011-05-28  7:26     ` Sven Eckelmann
@ 2011-05-28  8:43       ` Sven Eckelmann
  2011-05-28 11:45         ` [B.A.T.M.A.N.] [PATCH-marek] batman-adv: Unify the first 3 " Marek Lindner
                           ` (2 more replies)
  2011-05-28 11:20       ` [B.A.T.M.A.N.] [PATCH] batman-adv: move the ttl field to the third position of unicast_* packets Marek Lindner
  1 sibling, 3 replies; 14+ messages in thread
From: Sven Eckelmann @ 2011-05-28  8:43 UTC (permalink / raw)
  To: b.a.t.m.a.n

From: Antonio Quartulli <ordex@autistici.org>

The amount of duplicated code in the receive and routing code can be reduced
when all headers provide the packet type, version and ttl in the same first
bytes.

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
just my suggestion

 packet.h |   21 ++++++++++++++-------
 1 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/packet.h b/packet.h
index eda9965..c84f325 100644
--- a/packet.h
+++ b/packet.h
@@ -32,7 +32,7 @@
 #define BAT_UNICAST_FRAG 0x06
 
 /* this file is included by batctl which needs these defines */
-#define COMPAT_VERSION 12
+#define COMPAT_VERSION 14
 #define DIRECTLINK 0x40
 #define VIS_SERVER 0x20
 #define PRIMARIES_FIRST_HOP 0x10
@@ -55,12 +55,13 @@
 struct batman_packet {
 	uint8_t  packet_type;
 	uint8_t  version;  /* batman version field */
+	uint8_t  ttl;
+	uint8_t  reserved;
 	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_tt;
 	uint8_t  gw_flags;  /* flags related to gateway class */
 	uint8_t  align;
@@ -71,8 +72,9 @@ struct batman_packet {
 struct icmp_packet {
 	uint8_t  packet_type;
 	uint8_t  version;  /* batman version field */
-	uint8_t  msg_type; /* see ICMP message types above */
 	uint8_t  ttl;
+	uint8_t  reserved;
+	uint8_t  msg_type; /* see ICMP message types above */
 	uint8_t  dst[6];
 	uint8_t  orig[6];
 	uint16_t seqno;
@@ -86,8 +88,9 @@ struct icmp_packet {
 struct icmp_packet_rr {
 	uint8_t  packet_type;
 	uint8_t  version;  /* batman version field */
-	uint8_t  msg_type; /* see ICMP message types above */
 	uint8_t  ttl;
+	uint8_t  reserved;
+	uint8_t  msg_type; /* see ICMP message types above */
 	uint8_t  dst[6];
 	uint8_t  orig[6];
 	uint16_t seqno;
@@ -99,16 +102,19 @@ struct icmp_packet_rr {
 struct unicast_packet {
 	uint8_t  packet_type;
 	uint8_t  version;  /* batman version field */
-	uint8_t  dest[6];
 	uint8_t  ttl;
+	uint8_t  reserved;
+	uint8_t  dest[6];
 } __packed;
 
 struct unicast_frag_packet {
 	uint8_t  packet_type;
 	uint8_t  version;  /* batman version field */
-	uint8_t  dest[6];
 	uint8_t  ttl;
+	uint8_t  reserved;
+	uint8_t  dest[6];
 	uint8_t  flags;
+	uint8_t  align;
 	uint8_t  orig[6];
 	uint16_t seqno;
 } __packed;
@@ -116,8 +122,9 @@ struct unicast_frag_packet {
 struct bcast_packet {
 	uint8_t  packet_type;
 	uint8_t  version;  /* batman version field */
-	uint8_t  orig[6];
 	uint8_t  ttl;
+	uint8_t  reserved;
+	uint8_t  orig[6];
 	uint32_t seqno;
 } __packed;
 
-- 
1.7.5.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH] batman-adv: move the ttl field to the third position of unicast_* packets
  2011-05-28  7:26     ` Sven Eckelmann
  2011-05-28  8:43       ` [B.A.T.M.A.N.] [PATCH-sven] batman-adv: Unify the first 4 bytes in each packet Sven Eckelmann
@ 2011-05-28 11:20       ` Marek Lindner
  1 sibling, 0 replies; 14+ messages in thread
From: Marek Lindner @ 2011-05-28 11:20 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [B.A.T.M.A.N.] [PATCH-marek] batman-adv: Unify the first 3 bytes in each packet
  2011-05-28  8:43       ` [B.A.T.M.A.N.] [PATCH-sven] batman-adv: Unify the first 4 bytes in each packet Sven Eckelmann
@ 2011-05-28 11:45         ` Marek Lindner
  2011-05-28 12:07           ` Sven Eckelmann
  2011-05-28 11:46         ` [B.A.T.M.A.N.] [PATCH-sven] batman-adv: Unify the first 4 " Sven Eckelmann
  2011-05-28 11:46         ` [B.A.T.M.A.N.] [PATCHv2-sven] batman-adv: Unify the first 3 " Sven Eckelmann
  2 siblings, 1 reply; 14+ messages in thread
From: Marek Lindner @ 2011-05-28 11:45 UTC (permalink / raw)
  To: b.a.t.m.a.n

From: Antonio Quartulli <ordex@autistici.org>

The amount of duplicated code in the receive and routing code can be reduced
when all headers provide the packet type, version and ttl in the same first
bytes.

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
* Only the first 3 bytes have to be the same.
* vis_packet was not changed yet.

 packet.h |   30 ++++++++++++++++++------------
 1 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/packet.h b/packet.h
index eda9965..36a3aad 100644
--- a/packet.h
+++ b/packet.h
@@ -32,7 +32,7 @@
 #define BAT_UNICAST_FRAG 0x06
 
 /* this file is included by batctl which needs these defines */
-#define COMPAT_VERSION 12
+#define COMPAT_VERSION 14
 #define DIRECTLINK 0x40
 #define VIS_SERVER 0x20
 #define PRIMARIES_FIRST_HOP 0x10
@@ -55,15 +55,15 @@
 struct batman_packet {
 	uint8_t  packet_type;
 	uint8_t  version;  /* batman version field */
+	uint8_t  ttl;
 	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_tt;
 	uint8_t  gw_flags;  /* flags related to gateway class */
-	uint8_t  align;
+	uint8_t  tq;
+	uint8_t  num_tt;
+	uint8_t  reserved;
 } __packed;
 
 #define BAT_PACKET_LEN sizeof(struct batman_packet)
@@ -71,12 +71,13 @@ struct batman_packet {
 struct icmp_packet {
 	uint8_t  packet_type;
 	uint8_t  version;  /* batman version field */
-	uint8_t  msg_type; /* see ICMP message types above */
 	uint8_t  ttl;
+	uint8_t  msg_type; /* see ICMP message types above */
 	uint8_t  dst[6];
 	uint8_t  orig[6];
 	uint16_t seqno;
 	uint8_t  uid;
+	uint8_t  reserved;
 } __packed;
 
 #define BAT_RR_LEN 16
@@ -86,8 +87,8 @@ struct icmp_packet {
 struct icmp_packet_rr {
 	uint8_t  packet_type;
 	uint8_t  version;  /* batman version field */
-	uint8_t  msg_type; /* see ICMP message types above */
 	uint8_t  ttl;
+	uint8_t  msg_type; /* see ICMP message types above */
 	uint8_t  dst[6];
 	uint8_t  orig[6];
 	uint16_t seqno;
@@ -99,16 +100,19 @@ struct icmp_packet_rr {
 struct unicast_packet {
 	uint8_t  packet_type;
 	uint8_t  version;  /* batman version field */
-	uint8_t  dest[6];
 	uint8_t  ttl;
+	uint8_t  reserved;
+	uint8_t  dest[6];
 } __packed;
 
 struct unicast_frag_packet {
 	uint8_t  packet_type;
 	uint8_t  version;  /* batman version field */
-	uint8_t  dest[6];
 	uint8_t  ttl;
+	uint8_t  reserved;
+	uint8_t  dest[6];
 	uint8_t  flags;
+	uint8_t  align;
 	uint8_t  orig[6];
 	uint16_t seqno;
 } __packed;
@@ -116,18 +120,20 @@ struct unicast_frag_packet {
 struct bcast_packet {
 	uint8_t  packet_type;
 	uint8_t  version;  /* batman version field */
-	uint8_t  orig[6];
 	uint8_t  ttl;
+	uint8_t  reserved;
+	uint8_t  orig[6];
 	uint32_t seqno;
 } __packed;
 
 struct vis_packet {
 	uint8_t  packet_type;
 	uint8_t  version;        /* batman version field */
+	uint8_t  ttl;		 /* TTL */
 	uint8_t  vis_type;	 /* which type of vis-participant sent this? */
-	uint8_t  entries;	 /* number of entries behind this struct */
 	uint32_t seqno;		 /* sequence number */
-	uint8_t  ttl;		 /* TTL */
+	uint8_t  entries;	 /* number of entries behind this struct */
+	uint8_t  reserved;
 	uint8_t  vis_orig[6];	 /* originator that announces its neighbors */
 	uint8_t  target_orig[6]; /* who should receive this packet */
 	uint8_t  sender_orig[6]; /* who sent or rebroadcasted this packet */
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH-sven] batman-adv: Unify the first 4 bytes in each packet
  2011-05-28  8:43       ` [B.A.T.M.A.N.] [PATCH-sven] batman-adv: Unify the first 4 bytes in each packet Sven Eckelmann
  2011-05-28 11:45         ` [B.A.T.M.A.N.] [PATCH-marek] batman-adv: Unify the first 3 " Marek Lindner
@ 2011-05-28 11:46         ` Sven Eckelmann
  2011-05-28 11:46         ` [B.A.T.M.A.N.] [PATCHv2-sven] batman-adv: Unify the first 3 " Sven Eckelmann
  2 siblings, 0 replies; 14+ messages in thread
From: Sven Eckelmann @ 2011-05-28 11:46 UTC (permalink / raw)
  To: b.a.t.m.a.n

[-- Attachment #1: Type: Text/Plain, Size: 495 bytes --]

On Saturday 28 May 2011 10:43:03 Sven Eckelmann wrote:
> From: Antonio Quartulli <ordex@autistici.org>
> 
> The amount of duplicated code in the receive and routing code can be
> reduced when all headers provide the packet type, version and ttl in the
> same first bytes.
> 
> Signed-off-by: Antonio Quartulli <ordex@autistici.org>
> ---
> just my suggestion


Hm, forgot vis_packet and the 4 byte header is ugly. Maybe reordering 
everything is better....

Kind regards,
	Sven

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [B.A.T.M.A.N.] [PATCHv2-sven] batman-adv: Unify the first 3 bytes in each packet
  2011-05-28  8:43       ` [B.A.T.M.A.N.] [PATCH-sven] batman-adv: Unify the first 4 bytes in each packet Sven Eckelmann
  2011-05-28 11:45         ` [B.A.T.M.A.N.] [PATCH-marek] batman-adv: Unify the first 3 " Marek Lindner
  2011-05-28 11:46         ` [B.A.T.M.A.N.] [PATCH-sven] batman-adv: Unify the first 4 " Sven Eckelmann
@ 2011-05-28 11:46         ` Sven Eckelmann
  2 siblings, 0 replies; 14+ messages in thread
From: Sven Eckelmann @ 2011-05-28 11:46 UTC (permalink / raw)
  To: b.a.t.m.a.n

From: Antonio Quartulli <ordex@autistici.org>

The amount of duplicated code in the receive and routing code can be reduced
when all headers provide the packet type, version and ttl in the same first
bytes.

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
* reduce to 3 bytes, but still keep alignments
* corrected vis_packet

 packet.h |   28 ++++++++++++++++------------
 1 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/packet.h b/packet.h
index eda9965..ede441b 100644
--- a/packet.h
+++ b/packet.h
@@ -32,7 +32,7 @@
 #define BAT_UNICAST_FRAG 0x06
 
 /* this file is included by batctl which needs these defines */
-#define COMPAT_VERSION 12
+#define COMPAT_VERSION 14
 #define DIRECTLINK 0x40
 #define VIS_SERVER 0x20
 #define PRIMARIES_FIRST_HOP 0x10
@@ -55,15 +55,15 @@
 struct batman_packet {
 	uint8_t  packet_type;
 	uint8_t  version;  /* batman version field */
+	uint8_t  ttl;
 	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_tt;
 	uint8_t  gw_flags;  /* flags related to gateway class */
-	uint8_t  align;
+	uint8_t  tq;
+	uint8_t  num_tt;
+	uint8_t  reserved;
 } __packed;
 
 #define BAT_PACKET_LEN sizeof(struct batman_packet)
@@ -71,12 +71,13 @@ struct batman_packet {
 struct icmp_packet {
 	uint8_t  packet_type;
 	uint8_t  version;  /* batman version field */
-	uint8_t  msg_type; /* see ICMP message types above */
 	uint8_t  ttl;
+	uint8_t  msg_type; /* see ICMP message types above */
 	uint8_t  dst[6];
 	uint8_t  orig[6];
 	uint16_t seqno;
 	uint8_t  uid;
+	uint8_t  reserved;
 } __packed;
 
 #define BAT_RR_LEN 16
@@ -86,8 +87,8 @@ struct icmp_packet {
 struct icmp_packet_rr {
 	uint8_t  packet_type;
 	uint8_t  version;  /* batman version field */
-	uint8_t  msg_type; /* see ICMP message types above */
 	uint8_t  ttl;
+	uint8_t  msg_type; /* see ICMP message types above */
 	uint8_t  dst[6];
 	uint8_t  orig[6];
 	uint16_t seqno;
@@ -99,16 +100,17 @@ struct icmp_packet_rr {
 struct unicast_packet {
 	uint8_t  packet_type;
 	uint8_t  version;  /* batman version field */
-	uint8_t  dest[6];
 	uint8_t  ttl;
+	uint8_t  reserved;
+	uint8_t  dest[6];
 } __packed;
 
 struct unicast_frag_packet {
 	uint8_t  packet_type;
 	uint8_t  version;  /* batman version field */
-	uint8_t  dest[6];
 	uint8_t  ttl;
 	uint8_t  flags;
+	uint8_t  dest[6];
 	uint8_t  orig[6];
 	uint16_t seqno;
 } __packed;
@@ -116,18 +118,20 @@ struct unicast_frag_packet {
 struct bcast_packet {
 	uint8_t  packet_type;
 	uint8_t  version;  /* batman version field */
-	uint8_t  orig[6];
 	uint8_t  ttl;
+	uint8_t  reserved;
 	uint32_t seqno;
+	uint8_t  orig[6];
 } __packed;
 
 struct vis_packet {
 	uint8_t  packet_type;
 	uint8_t  version;        /* batman version field */
+	uint8_t  ttl;		 /* TTL */
 	uint8_t  vis_type;	 /* which type of vis-participant sent this? */
-	uint8_t  entries;	 /* number of entries behind this struct */
 	uint32_t seqno;		 /* sequence number */
-	uint8_t  ttl;		 /* TTL */
+	uint8_t  entries;	 /* number of entries behind this struct */
+	uint8_t  reserved;
 	uint8_t  vis_orig[6];	 /* originator that announces its neighbors */
 	uint8_t  target_orig[6]; /* who should receive this packet */
 	uint8_t  sender_orig[6]; /* who sent or rebroadcasted this packet */
-- 
1.7.5.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH-marek] batman-adv: Unify the first 3 bytes in each packet
  2011-05-28 11:45         ` [B.A.T.M.A.N.] [PATCH-marek] batman-adv: Unify the first 3 " Marek Lindner
@ 2011-05-28 12:07           ` Sven Eckelmann
  2011-05-28 12:17             ` Marek Lindner
  0 siblings, 1 reply; 14+ messages in thread
From: Sven Eckelmann @ 2011-05-28 12:07 UTC (permalink / raw)
  To: b.a.t.m.a.n; +Cc: Marek Lindner

[-- Attachment #1: Type: Text/Plain, Size: 1193 bytes --]

On Saturday 28 May 2011 13:45:29 Marek Lindner wrote:
> @@ -99,16 +100,19 @@ struct icmp_packet_rr {
>  struct unicast_packet {
>  	uint8_t  packet_type;
>  	uint8_t  version;  /* batman version field */
> -	uint8_t  dest[6];
>  	uint8_t  ttl;
> +	uint8_t  reserved;
> +	uint8_t  dest[6];
>  } __packed;
> 
>  struct unicast_frag_packet {
>  	uint8_t  packet_type;
>  	uint8_t  version;  /* batman version field */
> -	uint8_t  dest[6];
>  	uint8_t  ttl;
> +	uint8_t  reserved;
> +	uint8_t  dest[6];
>  	uint8_t  flags;
> +	uint8_t  align;
>  	uint8_t  orig[6];
>  	uint16_t seqno;
>  } __packed;

Why dont you use the reserverd part for the flags which are currently not in 
unicast? Then you could also remove that other align byte. It would still be 
possible to add flags to unicast packets when necessary.

> @@ -116,18 +120,20 @@ struct unicast_frag_packet {
>  struct bcast_packet {
>  	uint8_t  packet_type;
>  	uint8_t  version;  /* batman version field */
> -	uint8_t  orig[6];
>  	uint8_t  ttl;
> +	uint8_t  reserved;
> +	uint8_t  orig[6];
>  	uint32_t seqno;
>  } __packed;

Wouldn't that lead to an unaligned seqno?

Kind regards,
	Sven

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH-marek] batman-adv: Unify the first 3 bytes in each packet
  2011-05-28 12:07           ` Sven Eckelmann
@ 2011-05-28 12:17             ` Marek Lindner
  2011-05-28 12:43               ` Sven Eckelmann
  0 siblings, 1 reply; 14+ messages in thread
From: Marek Lindner @ 2011-05-28 12:17 UTC (permalink / raw)
  To: b.a.t.m.a.n

On Saturday 28 May 2011 14:07:10 Sven Eckelmann wrote:
> Why dont you use the reserverd part for the flags which are currently not
> in unicast? Then you could also remove that other align byte. It would
> still be possible to add flags to unicast packets when necessary.

The unicast_frag header assumes it looks like the unicast header plus some 
extra fields:

struct unicast_frag_packet {
	struct unicast_packet unicast_packet;
	uint8_t  flags;
	uint8_t  align;
	uint8_t  orig[6];
	uint16_t seqno;
};

We probably should add a little comment there to make that clear.

Both reserved fields (unicast + unicast frag) are going to be converted to 
ttvn with the tt patches.


> > +	uint8_t  reserved;
> > +	uint8_t  orig[6];
> > 
> >  	uint32_t seqno;
> >  
> >  } __packed;
> 
> Wouldn't that lead to an unaligned seqno?

Yes, I overlooked that the seqno is 32bit wide.

Regards,
Marek

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH-marek] batman-adv: Unify the first 3 bytes in each packet
  2011-05-28 12:17             ` Marek Lindner
@ 2011-05-28 12:43               ` Sven Eckelmann
  2011-05-28 12:51                 ` [B.A.T.M.A.N.] [PATCHv3-sven] " Sven Eckelmann
  0 siblings, 1 reply; 14+ messages in thread
From: Sven Eckelmann @ 2011-05-28 12:43 UTC (permalink / raw)
  To: Marek Lindner; +Cc: b.a.t.m.a.n

[-- Attachment #1: Type: Text/Plain, Size: 942 bytes --]

On Saturday 28 May 2011 14:17:14 Marek Lindner wrote:
> On Saturday 28 May 2011 14:07:10 Sven Eckelmann wrote:
> > Why dont you use the reserverd part for the flags which are currently not
> > in unicast? Then you could also remove that other align byte. It would
> > still be possible to add flags to unicast packets when necessary.
> 
> The unicast_frag header assumes it looks like the unicast header plus some
> extra fields:

Where does it assume that the frag is _not_ part of the unicast_packet header?

> 
> struct unicast_frag_packet {
> 	struct unicast_packet unicast_packet;
> 	uint8_t  flags;
> 	uint8_t  align;
> 	uint8_t  orig[6];
> 	uint16_t seqno;
> };
> 
> We probably should add a little comment there to make that clear.
> 
> Both reserved fields (unicast + unicast frag) are going to be converted to
> ttvn with the tt patches.

Ok, makes sense to use reserve fields then

Kind regards,
	Sven

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [B.A.T.M.A.N.] [PATCHv3-sven] batman-adv: Unify the first 3 bytes in each packet
  2011-05-28 12:43               ` Sven Eckelmann
@ 2011-05-28 12:51                 ` Sven Eckelmann
  2011-05-28 15:08                   ` Marek Lindner
  0 siblings, 1 reply; 14+ messages in thread
From: Sven Eckelmann @ 2011-05-28 12:51 UTC (permalink / raw)
  To: b.a.t.m.a.n

From: Antonio Quartulli <ordex@autistici.org>

The amount of duplicated code in the receive and routing code can be
reduced when all headers provide the packet type, version and ttl in the
same first bytes.

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
* Keep the reserved fields in unicast_frag_packet for ordex's changes

 packet.h |   30 ++++++++++++++++++------------
 1 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/packet.h b/packet.h
index eda9965..f4052f2 100644
--- a/packet.h
+++ b/packet.h
@@ -32,7 +32,7 @@
 #define BAT_UNICAST_FRAG 0x06
 
 /* this file is included by batctl which needs these defines */
-#define COMPAT_VERSION 12
+#define COMPAT_VERSION 14
 #define DIRECTLINK 0x40
 #define VIS_SERVER 0x20
 #define PRIMARIES_FIRST_HOP 0x10
@@ -55,15 +55,15 @@
 struct batman_packet {
 	uint8_t  packet_type;
 	uint8_t  version;  /* batman version field */
+	uint8_t  ttl;
 	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_tt;
 	uint8_t  gw_flags;  /* flags related to gateway class */
-	uint8_t  align;
+	uint8_t  tq;
+	uint8_t  num_tt;
+	uint8_t  reserved;
 } __packed;
 
 #define BAT_PACKET_LEN sizeof(struct batman_packet)
@@ -71,12 +71,13 @@ struct batman_packet {
 struct icmp_packet {
 	uint8_t  packet_type;
 	uint8_t  version;  /* batman version field */
-	uint8_t  msg_type; /* see ICMP message types above */
 	uint8_t  ttl;
+	uint8_t  msg_type; /* see ICMP message types above */
 	uint8_t  dst[6];
 	uint8_t  orig[6];
 	uint16_t seqno;
 	uint8_t  uid;
+	uint8_t  reserved;
 } __packed;
 
 #define BAT_RR_LEN 16
@@ -86,8 +87,8 @@ struct icmp_packet {
 struct icmp_packet_rr {
 	uint8_t  packet_type;
 	uint8_t  version;  /* batman version field */
-	uint8_t  msg_type; /* see ICMP message types above */
 	uint8_t  ttl;
+	uint8_t  msg_type; /* see ICMP message types above */
 	uint8_t  dst[6];
 	uint8_t  orig[6];
 	uint16_t seqno;
@@ -99,16 +100,19 @@ struct icmp_packet_rr {
 struct unicast_packet {
 	uint8_t  packet_type;
 	uint8_t  version;  /* batman version field */
-	uint8_t  dest[6];
 	uint8_t  ttl;
+	uint8_t  reserved;
+	uint8_t  dest[6];
 } __packed;
 
 struct unicast_frag_packet {
 	uint8_t  packet_type;
 	uint8_t  version;  /* batman version field */
-	uint8_t  dest[6];
 	uint8_t  ttl;
+	uint8_t  reserved;
+	uint8_t  dest[6];
 	uint8_t  flags;
+	uint8_t  align;
 	uint8_t  orig[6];
 	uint16_t seqno;
 } __packed;
@@ -116,18 +120,20 @@ struct unicast_frag_packet {
 struct bcast_packet {
 	uint8_t  packet_type;
 	uint8_t  version;  /* batman version field */
-	uint8_t  orig[6];
 	uint8_t  ttl;
+	uint8_t  reserved;
 	uint32_t seqno;
+	uint8_t  orig[6];
 } __packed;
 
 struct vis_packet {
 	uint8_t  packet_type;
 	uint8_t  version;        /* batman version field */
+	uint8_t  ttl;		 /* TTL */
 	uint8_t  vis_type;	 /* which type of vis-participant sent this? */
-	uint8_t  entries;	 /* number of entries behind this struct */
 	uint32_t seqno;		 /* sequence number */
-	uint8_t  ttl;		 /* TTL */
+	uint8_t  entries;	 /* number of entries behind this struct */
+	uint8_t  reserved;
 	uint8_t  vis_orig[6];	 /* originator that announces its neighbors */
 	uint8_t  target_orig[6]; /* who should receive this packet */
 	uint8_t  sender_orig[6]; /* who sent or rebroadcasted this packet */
-- 
1.7.5.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCHv3-sven] batman-adv: Unify the first 3 bytes in each packet
  2011-05-28 12:51                 ` [B.A.T.M.A.N.] [PATCHv3-sven] " Sven Eckelmann
@ 2011-05-28 15:08                   ` Marek Lindner
  0 siblings, 0 replies; 14+ messages in thread
From: Marek Lindner @ 2011-05-28 15:08 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

On Saturday 28 May 2011 14:51:06 Sven Eckelmann wrote:
> From: Antonio Quartulli <ordex@autistici.org>
> 
> The amount of duplicated code in the receive and routing code can be
> reduced when all headers provide the packet type, version and ttl in the
> same first bytes.

Applied in revision c7fb529.

Thanks,
Marek

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2011-05-28 15:08 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-27 22:10 [B.A.T.M.A.N.] [PATCH] batman-adv: move the ttl field to the third position of unicast_* packets Antonio Quartulli
2011-05-27 22:20 ` Sven Eckelmann
2011-05-27 22:35   ` Marek Lindner
2011-05-28  7:26     ` Sven Eckelmann
2011-05-28  8:43       ` [B.A.T.M.A.N.] [PATCH-sven] batman-adv: Unify the first 4 bytes in each packet Sven Eckelmann
2011-05-28 11:45         ` [B.A.T.M.A.N.] [PATCH-marek] batman-adv: Unify the first 3 " Marek Lindner
2011-05-28 12:07           ` Sven Eckelmann
2011-05-28 12:17             ` Marek Lindner
2011-05-28 12:43               ` Sven Eckelmann
2011-05-28 12:51                 ` [B.A.T.M.A.N.] [PATCHv3-sven] " Sven Eckelmann
2011-05-28 15:08                   ` Marek Lindner
2011-05-28 11:46         ` [B.A.T.M.A.N.] [PATCH-sven] batman-adv: Unify the first 4 " Sven Eckelmann
2011-05-28 11:46         ` [B.A.T.M.A.N.] [PATCHv2-sven] batman-adv: Unify the first 3 " Sven Eckelmann
2011-05-28 11:20       ` [B.A.T.M.A.N.] [PATCH] batman-adv: move the ttl field to the third position of unicast_* packets Marek Lindner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.