On Thu, 29 Aug 2002, Mark Burton wrote:
> Hello,
>
> The attached patch reverses the byte ordering of the result of the
> CRC32C calculation. Apparently, it was incorrect. Thanks to Dave
> Wysochanski <davidw@xxxxxxxxxx> for pointing this out to me.
Sorry for being behind in responding to this.
While this patch will work, it's not really correct. The real problem is
the line:
guint32 sent = tvb_get_ntohl(tvb, offset + headerLen)
since the CRC is not in network byte order. What is really needed is
for the "sent" line to be tvb_get_letohl().
The place where the difference comes in is if someone tries to compare the
crcs printed if there's an error. As-coded, they won't match anything
other programs generate (unless they too suffer from the same
misunderstanding).
The problem is that when we do a "reversed" CRC, as the iSCSI one is,
x^31, the most significant CRC bit, ends up being the same bit as 2^0 in
the local register. i.e.
2^31 ... 2^24 2^23 ... 2^16 2^15 ... 2^8 2^7 ... 2^0
x^0 ... x^7 x^8 ... x^15 x^16 ... x^23 x^24 ... x^31
Since the CRC is sent in Network bit order, x^31 gets sent first, which
means the least-significant byte goes first. Thus tvb_get_letohl().
While I am uncertain what the best way to print the CRC is, I don't think
what the code does now is right. Right now it will print, in hex,
x^24 ... x^31 x^16 ... x^23 x^8 ... x^15 x^0 ... x^7
which seems quite, uhm, confusing.
What it should probably do is 1) get rid of the swaps that were added, and
2) bit-reflect the crc before printing.
Take care,
Bill
