ipbus / ipbus-firmware

Firmware that implements a reliable high-performance control link for particle physics electronics, based on the IPbus protocol
https://ipbus.web.cern.ch
Other
41 stars 31 forks source link

Update IP checksum calculation to be compliant with RFC 1624 #210

Closed dpcsankey closed 1 year ago

dpcsankey commented 2 years ago

The IP (and UDP) checksums are defined in terms of 1's complement arithmetic. In this there are two values of zero, +0 = 0x0000 and -0 = 0xffff. An excellent description of the quirks arising from this are described in RFC 1624 sections 3, 4 and 6.

The original IP checksum calculation in process udp_send_data in udp_tx_mux.vhd forces the checksum when zero to be -0, by analogy with the UDP checksum definition in RFC 768. This was compatible with RFCs 791, 1071 and 1141, but, it transpires, not RFC 1624.

This has not been an issue until CERN started deploying Juniper QFX5120 routers, which claim compatibility with RFC 791 but really appear to be assuming RFC 1624.

The fix is to reverse the logic in process udp_send_data in udp_tx_mux.vhd:

send_special_int := '1';
special_int := ip_len(7 downto 0);
when 28 =>
- if ip_cksum(15 downto 8) = x"00" then
+ if ip_cksum(15 downto 8) = x"FF" then
flip_cksum := '1';
else
flip_cksum := '0';
end if;
when 29 =>
- if ip_cksum(7 downto 0) /= x"00" then
+ if ip_cksum(7 downto 0) /= x"FF" then
flip_cksum := '0';
end if;
when 30 =>
send_special_int := '1';
if flip_cksum = '1' then
- special_int := (Others => '1');
+ special_int := (Others => '0');
else
special_int := ip_cksum(15 downto 8);
end if;
when 31 =>
send_special_int := '1';
if flip_cksum = '1' then
- special_int := (Others => '1');
+ special_int := (Others => '0');
else
special_int := ip_cksum(7 downto 0);
end if;

I have included this in the feature/dhcp branch as this is the firmware that we are actually using now.

dpcsankey commented 2 years ago

I hadn't included the fix but have now!

https://github.com/ipbus/ipbus-firmware/commit/8b2985d5985ce7c4fc9dafc0dea06e1f4d7c11b6

tswilliams commented 1 year ago

Resolved in #212