According to RFC 1071, ICMP checksum is byte order independent. Just had to swap bytes of input data on EB because in_cksum() casts them to u_short.
Details
cd /usr/src/sbin/ping/tests
make clean
make
make install
kyua test -k /usr/tests/sbin/ping/Kyuafile
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Hello, thank you for the patch. I think I can see your point.
In my view, the tests in in_cksum_test.c should prove that the in_cksum() is byte order indepentent no matter for which endianness (big, little) they are compiled for and executed on. That means, that there should be both types of input data (big and little endian).
Maybe, there could be some issues with interpretation of the results of in_cksum() on different endianness architectures. I need to look into this a bit more.
Ok, now it makes sense. But then something is wrong with the test or with the in_cksum implementation, since it is failing on all tests on BE. For been independent of the endianess, I suppose the result checksum for {0x12, 0x34, 0x56, 0x78} and {0x34, 0x12, 0x78, 0x56} should be the same, right?
Original test wasn't working on BE. Just swaping the input data and testing on the same platform is not the same as testing the same input on a different platform. The best prove that in_cksum is enadiannes independent is that the same input outputs the same value on both platforms as long as you test the output byte-a-byte.
I understand that those are two different strategies testing different properties of the algorithm or its implementation. We have to wait for a FreeBSD developer to choose an approach that will be accepted.
I agree with Renato. There's no way to test in_cksum on both endiannesses without using something complicated like qemu. The right approach is to checksum the same input on different platforms, and assert that the result is identical on all. That's what Renato's change does. @renato.riolino_eldorado.org.br I don't think you have a commit bit, right? If not, I can commit the change for you. But I don't have any BE hardware. Can you first confirm that with your change the tests pass on BE hardware on a newish build?
This review was approved in March last year. It seems all issues have been solved. Could anyone commit this?