public inbox for b.a.t.m.a.n@lists.open-mesh.org
 help / color / mirror / Atom feed
* [B.A.T.M.A.N.] Weird TTL for ICMP packets
@ 2011-10-30 13:31 Sven Eckelmann
  2011-10-30 15:36 ` Simon Wunderlich
  2011-10-30 15:40 ` [B.A.T.M.A.N.] [PATCH] batman-adv: explain the weird " Simon Wunderlich
  0 siblings, 2 replies; 4+ messages in thread
From: Sven Eckelmann @ 2011-10-30 13:31 UTC (permalink / raw)
  To: b.a.t.m.a.n

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

Hi,

just looked through some ICMP code in icmp_socket.c and found a little bit 
weird TTL:

218         if (icmp_packet->version != COMPAT_VERSION) {                                                                                      
219                 icmp_packet->msg_type = PARAMETER_PROBLEM;                                                                                 
220                 icmp_packet->ttl = COMPAT_VERSION;                                                                                         
221                 bat_socket_add_packet(socket_client, icmp_packet, 
packet_len);
222                 goto free_skb;                                                                                                             
223         }

It was introduced by dba95eb270ff9674326865a20d25921c69c04d6c. My guess is 
that TTL should be used.

Kind regards,
	Sven

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

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

* Re: [B.A.T.M.A.N.] Weird TTL for ICMP packets
  2011-10-30 13:31 [B.A.T.M.A.N.] Weird TTL for ICMP packets Sven Eckelmann
@ 2011-10-30 15:36 ` Simon Wunderlich
  2011-10-30 15:40 ` [B.A.T.M.A.N.] [PATCH] batman-adv: explain the weird " Simon Wunderlich
  1 sibling, 0 replies; 4+ messages in thread
From: Simon Wunderlich @ 2011-10-30 15:36 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

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

Hi Sven,

although looking a little bit confusing, this seems to be correct. The TTL
field is used to transport the kernel modules compat version when it differs
from the one used in batctl.

Compare the batctl code (ping.c:284):
               switch (icmp_packet_in.msg_type) {
[...]
               case PARAMETER_PROBLEM:
                        printf("Error - the batman adv kernel module version (%d) differs from ours (%d)\n",
                                        icmp_packet_in.ttl, COMPAT_VERSION);
                        printf("Please make sure to compatible versions!\n");
                        goto out;
[...]

However, I guess an explaining comment in the kernel code would not be wrong ...

Cheers
	Simon


On Sun, Oct 30, 2011 at 02:31:55PM +0100, Sven Eckelmann wrote:
> Hi,
> 
> just looked through some ICMP code in icmp_socket.c and found a little bit 
> weird TTL:
> 
> 218         if (icmp_packet->version != COMPAT_VERSION) {                                                                                      
> 219                 icmp_packet->msg_type = PARAMETER_PROBLEM;                                                                                 
> 220                 icmp_packet->ttl = COMPAT_VERSION;                                                                                         
> 221                 bat_socket_add_packet(socket_client, icmp_packet, 
> packet_len);
> 222                 goto free_skb;                                                                                                             
> 223         }
> 
> It was introduced by dba95eb270ff9674326865a20d25921c69c04d6c. My guess is 
> that TTL should be used.
> 
> Kind regards,
> 	Sven



[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

* [B.A.T.M.A.N.] [PATCH] batman-adv: explain the weird TTL for ICMP packets
  2011-10-30 13:31 [B.A.T.M.A.N.] Weird TTL for ICMP packets Sven Eckelmann
  2011-10-30 15:36 ` Simon Wunderlich
@ 2011-10-30 15:40 ` Simon Wunderlich
  2011-10-30 15:57   ` Sven Eckelmann
  1 sibling, 1 reply; 4+ messages in thread
From: Simon Wunderlich @ 2011-10-30 15:40 UTC (permalink / raw)
  To: b.a.t.m.a.n; +Cc: Simon Wunderlich

A comment may clear this up ...

Reported-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
---
 icmp_socket.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/icmp_socket.c b/icmp_socket.c
index ac3520e..7b832d2 100644
--- a/icmp_socket.c
+++ b/icmp_socket.c
@@ -216,6 +216,8 @@ static ssize_t bat_socket_write(struct file *file, const char __user *buff,
 	icmp_packet->uid = socket_client->index;
 
 	if (icmp_packet->version != COMPAT_VERSION) {
+		/* report the kernel modules COMPAT_VERSION through the ttl
+		 * field if it differs from the version in the copied packet. */
 		icmp_packet->msg_type = PARAMETER_PROBLEM;
 		icmp_packet->ttl = COMPAT_VERSION;
 		bat_socket_add_packet(socket_client, icmp_packet, packet_len);
-- 
1.7.7.1


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

* Re: [B.A.T.M.A.N.] [PATCH] batman-adv: explain the weird TTL for ICMP packets
  2011-10-30 15:40 ` [B.A.T.M.A.N.] [PATCH] batman-adv: explain the weird " Simon Wunderlich
@ 2011-10-30 15:57   ` Sven Eckelmann
  0 siblings, 0 replies; 4+ messages in thread
From: Sven Eckelmann @ 2011-10-30 15:57 UTC (permalink / raw)
  To: b.a.t.m.a.n; +Cc: Simon Wunderlich

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

On Sunday 30 October 2011 16:40:46 Simon Wunderlich wrote:
> A comment may clear this up ...
> 
> Reported-by: Sven Eckelmann <sven@narfation.org>
> Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>

NAcked-by: Sven Eckelmann <sven@narfation.org>

No, the problem is that there is a ttl field and a version field. There is no 
reason to use the ttl field for the version or the version field for the ttl. 
When we query the information from the kernel, why don't we use the correct 
stuff in the common header structure. And the ttl position wasn't stable... so 
it would create a random number anyway when mixing particular versions of 
batman-adv/batctl.

But yes, you are right that the TTL mentioned in my first post doesn't make 
sense either.

Kind regards,
	Sven

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

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

end of thread, other threads:[~2011-10-30 15:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-30 13:31 [B.A.T.M.A.N.] Weird TTL for ICMP packets Sven Eckelmann
2011-10-30 15:36 ` Simon Wunderlich
2011-10-30 15:40 ` [B.A.T.M.A.N.] [PATCH] batman-adv: explain the weird " Simon Wunderlich
2011-10-30 15:57   ` Sven Eckelmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox