Should this be a static inline function so we are always passing the proper type?

Why are we shifting this value on Big Endian systems? May be worth a comment.

Can you structure the ifdef as 

ifdef _LITTLE
#elif _BIG
#else
#error
#endif

This is compiler specific. It should be properly wrapped in sys/ccompile.h so we transparently handle the case where it's not supported.

Can we scope this to the main while loop as it's not used outside of it? I found 'w' not very descriptive relative to the more important role it has.

Also, can 'w' be const?

Can we scope these to the main while loop as they're  not used outside of it?

The fact that this results in 32-bit byte swaps kind of confuses me. Based on the comment, I interpreted this as at first trying to say that we if we end in an odd value or start at a particular point, we need to do something like tread the aforementioned 'cc' as either '00 cc' or 'cc 00' depending on the situation.

Can you explain a bit more about why we're swapping the entire bit and how that really offsets when we start processing the next mblk_t?

Why was this pulled in?

Maybe I'm being a stick in the mud, but I feel like this is something we should do as a separate ticket showing that it actually helped. Mostly because it adds noise to the code and I'm not sure it's actually better with the depth of modern branch predictors?
Also, when we refactor a very important function like this I feel like it's best to do separate things in separate stages, so they can be thoroughly documented and tested in isolation. In this case, the point of the ticket is to clean up this function, not optimize it.

I feel like we should use a fixed-length type for initial_sum.

While I think is a very clever optimization, I have the same reservations about it as I do UNLIKELY. I'd prefer to see it as it's own follow-up ticket, separate from the cleanup.

I feel like we should use fixed size types here.

If offset must always be positive I wonder why it's defined as an int? Do we have code that could ever use a negative offset for some reason?

Since this is a comment, can we please spell out "checksum".

What does "less-private" mean? Are there actually levels (I don't know)? If not, perhaps just say "public".

Or really, split between multiple mblks. I understand why you comment would simplify to just two mblks, but I think it's worth calling out that we consider the case of two or more mblks.

It took me quite a while to understand what is going on here. It wasn't until I read RFC 1071 and thought through severral cases did things start to make senese. It might help to change the comment in two ways:


Add a refernece to RFC 1071, specifically point out the final example in section 3 which covers the case of needing to byte swap because of summation occuring across multiple groups where one group starts on an odd boundary.


Elbaorate on your example and use the style that RFC 1071 uses to make it more clear what's happening. E.g., here's me taking a stab at that (feel free to steal this or come up with your own).



Given the byte sequence [AA BB CC DD EE FF], you would calculate the

16-bit sum as follows:
Bytes           "Normal" Order          Little-Endian

                                            Order
1/2                 0xAABB                  0xBBAA

3/4                 0xCCDD                  0xDDCC

5/6                 0xEEFF                  0xFFEE

                   -------                 -------

                   0x26697                 0x29964
Carry (FOLD):       0x6699                  0x9966
However, if the byte sequence is split across buffers as so, [AA BB

CC] [DD EE FF], then the intermediate ushort_t values change.

Bytes           "Normal" Order          Little-Endian

                                            Order
1/2                 0xAABB                  0xBBAA

3/-                 0xCC00                  0x00CC

                   -------                 -------

                   0x176BB                  0xBC76

4/5                 0xDDEE                  0xEEDD

6/-                 0xFF00                  0x00FF

                   -------                 -------

No Swap            0x1DCEE                  0xEFDC

Swap               0xEEDC0100               0xDCEF0000
               0x176BB                  0xBC76
               0x1DCEE                  0xEFDC
              --------                 -------

Sum w/o SWAP:      0x353A9                 0x1AC52

Carry (FOLD):       0x53AC                  0xAC53
               0x176BB                  0xBC76

Sum w/ SWAP:    0xEEDC0100              0xDCEF0000

               -----------             -----------

Sum w/ SWAP:    0xEEDD77BB              0xDCEFBC76

Carry (FOLD):      0x16698                 0x19965

Carry (FOLD):       0x6699                  0x9966
Notice that the new sums, without using the swap, do not match

the old. However, with the swap, they do. This happens because now DD

gets moved from the low-byte to the high-byte, and all bytes that

follow get swapped out of the byte they were supposed to be added to.

If we change the byte sequence to [AA BB AA BB AA BB] this becomes

even more evident. All the AAs should be added in the high-byte, all

the BBs in the low, but if we string the sequence acorss two mblks,

the first with an odd length, we end up with swapped bytes.
One mblk            Two mblks

                (frist odd length)
AA BB               AA BB

AA BB               AA 00

AA BB               BB AA

                    BB AA