IP cheksum and libnet_pblock_coalesce

[Kenichi MORI]

Hi jarppe

Hello everybody,
Should libnet_pblock_coalesce() set the IP checksum correctly? I'm using libnet 1.1.2.1, compiled with GCC 3.2.2 20030222 on Linux.
When I create packet using libnet_build_[udp|tcp]() + libnet_build_ip(), then get the packet using libnet_pblock_coalesce() and then call libnet_ip_check() over the returned buffer, I get non-zero reply, indicating wrong IP checksum.
Interestingly enough, if I create the packet with libnet_build_icmpv4_echo() + libnet_build_ip(), then libnet_ip_check() returns zero (checksum ok).
Am I doing something wrong, or is there a bug?
-jarppe

I had something similar situation. I'm also using libnet 1.1.2.1 with
GCC (Redhat9).

I also found some wrong checksum packets created by libnet.
So, I replace the function libnet_in_checksum() (libnet_checksum.c) with
the same name function of libnet-1.0.2a, there is no wrong packet.

-Kenichi

[Frédéric Raynal]

Hi,

Post by Kenichi MORI
Hi jarppe

Hello everybody,
Should libnet_pblock_coalesce() set the IP checksum correctly? I'm
using libnet 1.1.2.1, compiled with GCC 3.2.2 20030222 on Linux.
When I create packet using libnet_build_[udp|tcp]() +
libnet_build_ip(), then get the packet using
libnet_pblock_coalesce() and then call libnet_ip_check() over the
returned buffer, I get non-zero reply, indicating wrong IP
checksum.
Interestingly enough, if I create the packet with
libnet_build_icmpv4_echo() + libnet_build_ip(), then
libnet_ip_check() returns zero (checksum ok).
Am I doing something wrong, or is there a bug?
-jarppe

I had something similar situation. I'm also using libnet 1.1.2.1
with GCC (Redhat9).
I also found some wrong checksum packets created by libnet. So, I
replace the function libnet_in_checksum() (libnet_checksum.c) with
the same name function of libnet-1.0.2a, there is no wrong packet.
-Kenichi

I dont understand why you call libnet_ip_check() :
u_int16_t
libnet_ip_check(u_int16_t *addr, int len)
{
int sum;

sum = libnet_in_cksum(addr, len);
return (LIBNET_CKSUM_CARRY(sum));
}

When you call libnet_pblock_coalesce() checksums are computed, so you
dont have to force any re-computation.

Fred

[Kenichi MORI]

Hello,

Post by Frédéric Raynal
u_int16_t
libnet_ip_check(u_int16_t *addr, int len)
{
int sum;
sum = libnet_in_cksum(addr, len);
return (LIBNET_CKSUM_CARRY(sum));
}
When you call libnet_pblock_coalesce() checksums are computed, so you
dont have to force any re-computation.
Fred

My situation was little different, I only use libnet_build_ipv4() &
libnet_build_tcp() functions.

But, I'm sorry I missed the messages #1513.
I think if I applied the pache, I was able to resolve the problem.
(Now that I changed my code and there is no error).

Thanks.

-Kenichi

[Jari Lansio]

Hello,

I think my problem _was_ just a misunderstanding from my part. The
pressure is on word "was", because now I think I have found a genuine
bug in checksum code.

This is function libnet_in_cksum() from libnet_checksum.c:

41 int
42 libnet_in_cksum(u_int16_t *addr, int len)
43 {
44 int sum;
45
46 sum = 0;
47
48 while (len > 1)
49 {
50 sum += *addr++;
51 len -= 2;
52 }
53 if (len == 1)
54 {
55 sum += *(u_int16_t *)addr;
56 }
57
58 return (sum);
59 }

Since the type of argument 'addr' is pointer to u_int16_t, it would seem
like this is function is supposed to work only with memory blocks that
have even length.

However, the code at line 53 checks if buffer has odd length. This is
important, because this function is used to calculate checksum for UDP
and TCP too, and they can have odd lengths.

I think that there is bug in on line 55, where 'addr' is casted to
u_int16_t*, the type it already is!!! The problem is that this code
reads 2 bytes from address 'addr', while it should read only one byte.

I changed to line 55 to read like this:

55 sum += *(u_int8_t *)addr;

Now only one byte is read (as the 'len' indicates) and everything seems
to work just fine.

How this bug has not caused any problems before? The only explanation is
that the compilers add 'padding' bytes right after buffers with odd
length, and that padding is always '0', and thus including this padding
to checksum calculation do not change the checksum. However, I used this
code on Symbian platform and guess what, there was no padding and it
failed to get correct checksums for packets that had odd length.

The rfc-1071 has example C code for checksum calculation, and it looks
very similar. It's not perfect example, since it has a bug that prevents
it to compile (missing '*' on line 381), but the relevant lines are:

385 /* Add left-over byte, if any */
386 if( count > 0 )
387 sum += * (unsigned char *) addr;

Notice the cast to u_int8_t.

Am I right? Have I found a real bug, or did I just made myself look
stupid :) I'm on holidays the next week and I won't read my mails until
4th of October.

-jarppe

Post by Kenichi MORI
Hi jarppe

Hello everybody,
Should libnet_pblock_coalesce() set the IP checksum correctly? I'm

using libnet 1.1.2.1, compiled with GCC 3.2.2 20030222 on Linux.

Post by Kenichi MORI

When I create packet using libnet_build_[udp|tcp]() +

libnet_build_ip(), then get the packet using libnet_pblock_coalesce()
and then call libnet_ip_check() over the returned buffer, I get non-zero
reply, indicating wrong IP checksum.

Post by Kenichi MORI

Interestingly enough, if I create the packet with

libnet_build_icmpv4_echo() + libnet_build_ip(), then libnet_ip_check()
returns zero (checksum ok).

Post by Kenichi MORI

Am I doing something wrong, or is there a bug?
-jarppe

I had something similar situation. I'm also using libnet 1.1.2.1 with
GCC (Redhat9).
I also found some wrong checksum packets created by libnet.
So, I replace the function libnet_in_checksum() (libnet_checksum.c) with
the same name function of libnet-1.0.2a, there is no wrong packet.
-Kenichi

[Frédéric Raynal]

Hi,

That is a known (but tricky) bug reported some times ago.
David Baroso proposed something like :

sum += ((*(u_int8_t *)addr & 0xFF)<<8);

The difficulty is to find something working on both big and little
endian systems.

Fred Raynal

Post by Jari Lansio
Hello,
I think my problem _was_ just a misunderstanding from my part. The
pressure is on word "was", because now I think I have found a genuine
bug in checksum code.
41 int
42 libnet_in_cksum(u_int16_t *addr, int len)
43 {
44 int sum;
45
46 sum = 0;
47
48 while (len > 1)
49 {
50 sum += *addr++;
51 len -= 2;
52 }
53 if (len == 1)
54 {
55 sum += *(u_int16_t *)addr;
56 }
57
58 return (sum);
59 }
Since the type of argument 'addr' is pointer to u_int16_t, it would seem
like this is function is supposed to work only with memory blocks that
have even length.
However, the code at line 53 checks if buffer has odd length. This is
important, because this function is used to calculate checksum for UDP
and TCP too, and they can have odd lengths.
I think that there is bug in on line 55, where 'addr' is casted to
u_int16_t*, the type it already is!!! The problem is that this code
reads 2 bytes from address 'addr', while it should read only one byte.
55 sum += *(u_int8_t *)addr;
Now only one byte is read (as the 'len' indicates) and everything seems
to work just fine.
How this bug has not caused any problems before? The only explanation is
that the compilers add 'padding' bytes right after buffers with odd
length, and that padding is always '0', and thus including this padding
to checksum calculation do not change the checksum. However, I used this
code on Symbian platform and guess what, there was no padding and it
failed to get correct checksums for packets that had odd length.
The rfc-1071 has example C code for checksum calculation, and it looks
very similar. It's not perfect example, since it has a bug that prevents
385 /* Add left-over byte, if any */
386 if( count > 0 )
387 sum += * (unsigned char *) addr;
Notice the cast to u_int8_t.
Am I right? Have I found a real bug, or did I just made myself look
stupid :) I'm on holidays the next week and I won't read my mails until
4th of October.
-jarppe

Post by Kenichi MORI
Hi jarppe

Hello everybody,
Should libnet_pblock_coalesce() set the IP checksum correctly? I'm

using libnet 1.1.2.1, compiled with GCC 3.2.2 20030222 on Linux.

Post by Kenichi MORI

When I create packet using libnet_build_[udp|tcp]() +

libnet_build_ip(), then get the packet using libnet_pblock_coalesce()
and then call libnet_ip_check() over the returned buffer, I get non-zero
reply, indicating wrong IP checksum.

Post by Kenichi MORI

Interestingly enough, if I create the packet with

libnet_build_icmpv4_echo() + libnet_build_ip(), then libnet_ip_check()
returns zero (checksum ok).

Post by Kenichi MORI

Am I doing something wrong, or is there a bug?
-jarppe

I had something similar situation. I'm also using libnet 1.1.2.1 with
GCC (Redhat9).
I also found some wrong checksum packets created by libnet.
So, I replace the function libnet_in_checksum() (libnet_checksum.c) with
the same name function of libnet-1.0.2a, there is no wrong packet.
-Kenichi

[sandr8]

Post by Frédéric Raynal
The difficulty is to find something working on both big and little
endian systems.

would it be that big shame to split the code within an #ifdef block, then?
cheers
sandr8)

[Frédéric Raynal]

Post by sandr8

Post by Frédéric Raynal
The difficulty is to find something working on both big and little
endian systems.

would it be that big shame to split the code within an #ifdef block, then?

Yes !
I am currently in Burkina Faso for the"Free Software African Meeting",
that is great :-) thus but I'm not sure to have time to work that much on
libnet before the 11th of October.


BW, I have started to port libnetng to Solaris. It requires some
changes in several macros and function due to the strict alignment on
that system, but RAW and PCAP interface should work quickly. For DLPI,
that is another story.


Fred Raynal

[David Barroso]

Post by Frédéric Raynal
Hi,
That is a known (but tricky) bug reported some times ago.
sum += ((*(u_int8_t *)addr & 0xFF)<<8);
The difficulty is to find something working on both big and little
endian systems.

Hello,
I tested that the code above in both sparc and x86 and it worked like a
charm. Perhaps it could be the best solution :-?

[Wu Yongwei]

Hi,
That is a known (but tricky) bug reported some times ago. David
sum += ((*(u_int8_t *)addr & 0xFF)<<8);
The difficulty is to find something working on both big and little
endian systems.

The expression above seems to work only on big-endian machines and
proves wrong in my test on x86 (contrary to Mr David Barroso's
statement). The third edition of UNIX Network Programming has something
like this (adapted):

u_int16_t last_byte = 0;
...
if (nleft == 1) {
*(u_int8_t*)&last_byte = *(u_int8_t*)addr;
sum += last_byte;
}

I feel this is good.

Best regards,

Yongwei

[Frédéric Raynal]

Post by Wu Yongwei

Hi,
That is a known (but tricky) bug reported some times ago. David
sum += ((*(u_int8_t *)addr & 0xFF)<<8);
The difficulty is to find something working on both big and little
endian systems.

The expression above seems to work only on big-endian machines and
proves wrong in my test on x86 (contrary to Mr David Barroso's
statement). The third edition of UNIX Network Programming has something
u_int16_t last_byte = 0;
...
if (nleft == 1) {
*(u_int8_t*)&last_byte = *(u_int8_t*)addr;
sum += last_byte;
}
I feel this is good.

Hi,

I like it ... but I feel there is a problem. Thus, I coded this small
example:

main()
{
char i[] = "\x01\x02\x03";
unsigned short last = 0, sum = 0x4455, *s = (unsigned
short*)i;


*(unsigned char*)&last = *(unsigned char*)s;

printf("last=0x%04x\n", last);
sum += last;
printf("sum=0x%04x\n", sum);

sum = 0x4455;
sum += (*(unsigned char*)s & 0xff)<<8;
printf("sum=0x%04x\n", sum);
}


And here are the results:


On Mac OS X (big endian) :
last=0x0100
sum=0x4555
sum=0x4555

On x86 (little endian):
last=0x0001
sum=0x4456
sum=0x4555

And checksm must be big endian (network ordered) ...

I am not in position to perform tests here, so if some people could do
it, that will be very helpful.

Fred Raynal

[Wu Yongwei]

Unless the checksumming is not intended upon the raw packet content, you
have the wrong test case. I think it should be like:

main()
{
char i[] = "\x01\x02\x03";
unsigned short last = 0, sum, *s = (unsigned short*)i;
unsigned char *sum_p = (unsigned char*)&sum;

*(unsigned char*)&last = *(unsigned char*)s;

printf("last=0x%04x\n", last);

sum_p[0] = 0x44;
sum_p[1] = 0x55;
sum += last;
printf("sum=0x%.2x%.2x\n", sum_p[0], sum_p[1]);

sum_p[0] = 0x44;
sum_p[1] = 0x55;
sum += (*(unsigned char*)s & 0xff)<<8;
printf("sum=0x%.2x%.2x\n", sum_p[0], sum_p[1]);
}

This will not change the result on big-endian machines, but will give

last=0x0001
sum=0x4555
sum=0x4456

on little-endian machines.

I did my test on a real packet. I chose a captured odd-sized UDP packet
and calculated its UDP checksum, and verified that the result should be
zero. My method passed the test but David's one failed it.

Best regards,

Yongwei

--- Original Message from Frédéric Raynal ---

Post by Wu Yongwei

Hi,
That is a known (but tricky) bug reported some times ago. David
sum += ((*(u_int8_t *)addr & 0xFF)<<8);
The difficulty is to find something working on both big and little
endian systems.

The expression above seems to work only on big-endian machines and
proves wrong in my test on x86 (contrary to Mr David Barroso's
statement). The third edition of UNIX Network Programming has
u_int16_t last_byte = 0;
...
if (nleft == 1) {
*(u_int8_t*)&last_byte = *(u_int8_t*)addr;
sum += last_byte;
}
I feel this is good.

Hi,

I like it ... but I feel there is a problem. Thus, I coded this small
example:

main()
{
char i[] = "\x01\x02\x03";
unsigned short last = 0, sum = 0x4455, *s = (unsigned
short*)i;


*(unsigned char*)&last = *(unsigned char*)s;

printf("last=0x%04x\n", last);
sum += last;
printf("sum=0x%04x\n", sum);

sum = 0x4455;
sum += (*(unsigned char*)s & 0xff)<<8;
printf("sum=0x%04x\n", sum);
}


And here are the results:


On Mac OS X (big endian) :
last=0x0100
sum=0x4555
sum=0x4555

On x86 (little endian):
last=0x0001
sum=0x4456
sum=0x4555

And checksm must be big endian (network ordered) ...

I am not in position to perform tests here, so if some people could do
it, that will be very helpful.

Fred Raynal