search

SIDN / spin

SPIN Core Software
https://spin.sidnlabs.nl
GNU General Public License v2.0
77 stars 9 forks source link

Endianness fails #75

Closed ElmerLastdrager closed 4 years ago

ElmerLastdrager commented 4 years ago

Situation: vbox image is compiled on linux. Running this on a mac, results in unexpected endianness behaviour.

https://github.com/SIDN/spin/blob/34949920e0c10c1ac9885c17e0e1fbde8dfc7cf7/src/spind/core2conntrack.c

Anything >=8 bits goes wrong (e.g. nfct_get_attr_u16).

ElmerLastdrager commented 4 years ago

Test: edit in network output, here: https://github.com/SIDN/spin/blob/34949920e0c10c1ac9885c17e0e1fbde8dfc7cf7/src/lib/pkt_info.c

Todo: support 64-bit int as well?

ElmerLastdrager commented 4 years ago

https://github.com/SIDN/spin/commit/8db6e20bb626a3555fc81bfc6b7829f8289f895e#diff-6d3a136251f724f9d087d5133d6c88baL66

This was the wrong way around (network to host). It should be host to network.

ElmerLastdrager commented 4 years ago

Reference: https://github.com/VUTBR/natnf/blob/382db319ffcc8cfb1de6ad723c9a2b4df7eed687/main.c#L120

Ports seem to be in network byte order (big endian), so we need a conversion.

Please test proposed patch [EDIT: patch failed on LE debian]

cschutijser commented 4 years ago

Took a quick look at it, seems correct. Just to be sure, I wanted to test the patch but I ran into some unrelated troubles so I'll do that later.

It does not matter that much because be16toh() and ntohs() do the same thing but it seems to me that ntohs() is usually used for this type of conversion. Any reason you did not go with ntohs()?

Also, it would be best to keep the includes sorted alphabetically.

ElmerLastdrager commented 4 years ago

Should be fixed in be6170adc5e2a2269682c8ccf1bc6ce94f6f7dbf