1. Topics
  2. ICMP Checksum Errors

ICMP Checksum Errors

Nashif, Anas
 

Hi Erwin,
Can you submit the patch as a pull request and file a bug?

Thanks,
Anas

-----Original Message-----
From: zephyr-devel-bounces@... [mailto:zephyr-devel-bounces@...] On Behalf Of Erwin Rol
Sent: Wednesday, June 28, 2017 11:54 AM
To: zephyr-devel <zephyr-devel@...>
Subject: Re: [Zephyr-devel] ICMP Checksum Errors

Hey all,

I looked into it a bit more and the ICMP checksum calculation is broken.
I made a small patch to try and fix it, and it does seem to fix the problem, but I don't think the patch is good enough for a pull-request, comments are welcome.

- Erwin

On 28-6-2017 10:50, Erwin Rol wrote:
Hello,

while testing my STM32 Ethernet driver I noticed that Wireshark
complained about incorrect ICMP checksums. After a bit more testing I
found out that it started complaining when the total packet size was
128 bytes.
The ICMP checksum is calculated with

u16_t net_calc_chksum(struct net_pkt *pkt, u8_t proto)

in zephyr/subsys/net/ip/utils.c

And that has;
....
if (proto == IPPROTO_ICMP) {
return htons(calc_chksum(0, net_pkt_ip_data(pkt) +
net_pkt_ip_hdr_len(pkt),
upper_layer_len)); } ....

but unlike the

calc_chksum_pkt(sum, pkt, upper_layer_len);

function calc_chksum does not take into account that data could be
divided over multiple (128byte) net_pkt fragments.

Also are the other uses of calc_chksum in net_calc_chksum correct ?
Because 128 bytes isn't that much and especially with IPv6 things
might need more than 1 net_pkt fragment.

- Erwin
_______________________________________________
Zephyr-devel mailing list
Zephyr-devel@...
https://lists.zephyrproject.org/mailman/listinfo/zephyr-devel
Erwin Rol
 

Hey all,

I looked into it a bit more and the ICMP checksum calculation is broken.
I made a small patch to try and fix it, and it does seem to fix the
problem, but I don't think the patch is good enough for a pull-request,
comments are welcome.

- Erwin

On 28-6-2017 10:50, Erwin Rol wrote:
Hello,

while testing my STM32 Ethernet driver I noticed that Wireshark
complained about incorrect ICMP checksums. After a bit more testing I
found out that it started complaining when the total packet size was
128 bytes.
The ICMP checksum is calculated with

u16_t net_calc_chksum(struct net_pkt *pkt, u8_t proto)

in zephyr/subsys/net/ip/utils.c

And that has;
....
if (proto == IPPROTO_ICMP) {
return htons(calc_chksum(0, net_pkt_ip_data(pkt) +
net_pkt_ip_hdr_len(pkt),
upper_layer_len));
}
....

but unlike the

calc_chksum_pkt(sum, pkt, upper_layer_len);

function calc_chksum does not take into account that data could be
divided over multiple (128byte) net_pkt fragments.

Also are the other uses of calc_chksum in net_calc_chksum correct ?
Because 128 bytes isn't that much and especially with IPv6 things might
need more than 1 net_pkt fragment.

- Erwin
_______________________________________________
Zephyr-devel mailing list
Zephyr-devel@...
https://lists.zephyrproject.org/mailman/listinfo/zephyr-devel

?
icmp_chsum.diff

Erwin Rol
 

Hello,

while testing my STM32 Ethernet driver I noticed that Wireshark
complained about incorrect ICMP checksums. After a bit more testing I
found out that it started complaining when the total packet size was
128 bytes.
The ICMP checksum is calculated with

u16_t net_calc_chksum(struct net_pkt *pkt, u8_t proto)

in zephyr/subsys/net/ip/utils.c

And that has;
....
if (proto == IPPROTO_ICMP) {
return htons(calc_chksum(0, net_pkt_ip_data(pkt) +
net_pkt_ip_hdr_len(pkt),
upper_layer_len));
}
....

but unlike the

calc_chksum_pkt(sum, pkt, upper_layer_len);

function calc_chksum does not take into account that data could be
divided over multiple (128byte) net_pkt fragments.

Also are the other uses of calc_chksum in net_calc_chksum correct ?
Because 128 bytes isn't that much and especially with IPv6 things might
need more than 1 net_pkt fragment.

- Erwin
1 - 3 of 3