Re: [PATCH] hash: fix pointer alignment

[Radu Nicolau <radu.nicolau@intel.com>]

On 26-Feb-26 7:47 PM, Bruce Richardson wrote:
> On Thu, Feb 26, 2026 at 04:43:26PM +0000, Radu Nicolau wrote:
>> On 26-Feb-26 4:22 PM, Marat Khalili wrote:
>>>> diff --git a/lib/hash/rte_hash_crc.h b/lib/hash/rte_hash_crc.h
>>>> index fa07c97685..66f11fafcd 100644
>>>> --- a/lib/hash/rte_hash_crc.h
>>>> +++ b/lib/hash/rte_hash_crc.h
>>>> @@ -127,6 +127,24 @@ rte_hash_crc(const void *data, uint32_t data_len, uint32_t init_val)
>>>>    	unsigned i;
>>>>    	uintptr_t pd = (uintptr_t) data;
>>>>
>>>> +	/* align input to 8 byte boundary if needed */
>>>> +	if ((pd & 0x7) && data_len >= 8) {
>>> Perhaps the case data_len < 8 should also be included, with each of the if's below checking and correcting data_len individually?
>> No need to include this case; if data_len < 8 it will skip the for loop and
>> fall through those 3 if's, and get processed there.
>>
>>
>>>> +		uintptr_t unaligned_bytes = 8 - (pd & 0x7);
>>>> +		data_len -= unaligned_bytes;
>>>> +		if (unaligned_bytes & 0x4) {
>>>> +			init_val = rte_hash_crc_4byte(*(const uint32_t *)pd, init_val);
>>>> +			pd += 4;
>>>> +		}
>>>> +		if (unaligned_bytes & 0x2) {
>>>> +			init_val = rte_hash_crc_2byte(*(const uint16_t *)pd, init_val);
>>>> +			pd += 2;
>>>> +		}
>>>> +		if (unaligned_bytes & 0x1) {
>>>> +			init_val = rte_hash_crc_1byte(*(const uint8_t *)pd, init_val);
>>>> +			pd += 1;
>>>> +		}
>>> Shouldn't the order be the opposite?
>> As long as we process the right number of bytes the order doesn't matter.
> But if we reverse the order we get more natural alignment. If the data
> pointer is off-by-one, e.g. 0x65, and unaligned_bytes == 7, if we do as
> here, we calculate the 4-byte version and 2-byte versions with 1-byte
> alignment of the data. By reversing the order, we would do the 2-byte
> calculation on 2-byte aligned data, and the 4-byte calc on 4-byte aligned
> data etc.
Yes, that is correct, I will send a v2.