Yes, all seems to be correct, except that the user_data
not being included in the CRC computation is surprising. Why do you want it to be this way? If this is indeed to be so, then shouldn’t it be swapped places with the CRC field?
fixed
@maksim.drachov seems like we’ve reached a tentative consensus here. Someone else might still raise objections, but generally, I would say it is now safe to proceed with the implementation of the new header layout and the refined multicast address structure. The new header layout is defined in the previous post by Scott: [Cyphal/UDP] Architectural issues caused by the dependency between the node's IP address and its identity - #60 by scottdixon
The new IP address format is defined here: [Cyphal/UDP] Architectural issues caused by the dependency between the node's IP address and its identity - #46 by scottdixon
The transfer-CRC is now to be present not only in multi-frame transfers but also in single-frame transfers, always.
I think these are all changes. Scott, please correct me if I missed anything.
I updated #60 to be a single post with the entire result of this thread’s negotiations.
The header CRC function is CRC-16-CCITT-FALSE. Some details are available in the chat room here:
@scottdixon @schoberm @maksim.drachov is the header CRC supposed to be serialized in the little endian or big endian byte order? Considering that DSDL prescribes little-endian, the answer seems obvious, but the native byte ordering for CRC16-CCITT is big-endian, and the native byte ordering is used in the Cyphal/CAN multi-frame transfer CRC.
>>> from pycyphal.transport.commons.crc import CRC16CCITT
>>> CRC16CCITT.new(b'123456789').value
10673
>>> CRC16CCITT.new(b'123456789').value_as_bytes.hex() # Big endian
'29b1'
>>> hex(10673)
'0x29b1'
>>> CRC16CCITT.new(b'123456789\x29\xb1').value # Correct residue
0
>>> CRC16CCITT.new(b'123456789\xb1\x29').value # Incorrect residue
37875
The byte ordering currently implemented in PyCyphal UDP transport by @maksim.drachov is little endian.
The byte ordering currently implemented in @samcrow’s Canadensis is little endian.
The byte ordering currently implemented in libudpard is, uhm, platform-dependent.
P.S.: The native byte ordering of CRC32C is little-endian, so there is no ambiguity there.
Currently the next MR should move Cyphal/Serial to little-endianness , so that only leaves Cyphal/CAN to be ported eventually. (Speaking about pycyphal
here)
Also: all other values in the header are encoded as little-endian, so I see no reason to make an exception for CRC16-CCITT.
Cyphal/CAN will not be ported because that would break wire compatibility. Either we move the header CRC field in Cyphal/UDP and Cyphal/serial to big-endian now, or we retain the inconsistency forever.
Well…
In that case big-endian
Can you point me to the governing standard here?
It follows from the definition of this algorithm; more info here Catalogue of parametrised CRC algorithms. You can see in the example I provided that the correct residue is obtained only if the bytes are fed in the big endian order.
Right, but that’s the endianness of that algorithm. We’re only concerned with the representation of the 2-byte result in our UDP header. Does the standard actually control the endianness of that value on the wire or should the user simply assume network byte order?
No. It’s our value and we can do whatever we want with it. The problem here is that the process of verification becomes a bit (a few cycles) more complex because you need to compute the CRC for the entire header sans the last two bytes and then compare the result against the value contained in the header. If the value was in the big-endian byte order, you would simply run the CRC algorithm over the entire header and then ensure that the result is zero. This is not a significant shortcoming by any margin but still.
Admittedly I haven’t had the time to do more then some Googling and rushed reading on my iPhone but the ITU standard appears to avoid any discussion of bytes at all. The sourceforge page (heh, remember sourceforge! That was before we all switched over to Google code.) seems to suggest that even popular implementations of CCITT are confused on the subject of endinaness. The ISO11784/85 standard appears to transmit their data LSB first followed by the CRC but the Atmel datasheet cited in the sourceforge page just says it uses “the CRC-CCIT algorithm” and doesn’t provide more details.
All this to say, unless we can find some authoritative specification we should be following for this particular issue, I think we’re in good company if we choose to optimize this per our needs.
Moving to big-endian:
We need to define our needs. I think consistency with Cyphal/CAN is not a strict requirement but is desirable, plus it matches the default expectation of someone who is familiar with this CRC function. If we were to switch to the big-endian format, the DSDL header definition introduced in post #60 would become:
uint4 version # <- 1
void4
@assert _offset_ == {8}
uint3 priority # Duplicates QoS for ease of access; 0 -- highest, 7 -- lowest.
void5
@assert _offset_ == {16}
uint16 source_node_id
uint16 destination_node_id
uint16 data_specifier # Like in Cyphal/serial: subject-ID | (service-ID + request/response discriminator).
@assert _offset_ == {64}
uint64 transfer_id
@assert _offset_ == {128}
uint31 frame_index
bool end_of_transfer
uint16 user_data
# Opaque application-specific data with user-defined semantics. Generic implementations should ignore
@assert _offset_ % 16 == {0}
-uint16 header_crc
+uint8[2] header_crc_be # Most significant byte first.
@assert _offset_ / 8 == {24} # Fixed-size 24-byte header with natural alignment for each field ensured.
@sealed
Plus one for meeting default expectations and no one dare to touch libcanard (otherwise pitchfork ).
We’ve clarified the ad-hoc specification (post 60 above) to indicate the Cyphal header checksum in Cyphal/UDP is a two-byte, MSB-first value in the header.
IPv6 eliminates the need for a DHCP server (ie. one less component and point of failure). The concept is being applied in the marine industry with NMEA OneNet