public inbox for dev@dpdk.org
 help / color / mirror / Atom feed
* [PATCH] hash: fix pointer alignment
@ 2026-02-26 14:22 Radu Nicolau
  2026-02-26 16:22 ` Marat Khalili
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Radu Nicolau @ 2026-02-26 14:22 UTC (permalink / raw)
  To: dev
  Cc: Radu Nicolau, stable, stephen, Yipeng Wang, Sameh Gobriel,
	Bruce Richardson, Vladimir Medvedkin, Pablo de Lara,
	Yerden Zhumabekov

rte_hash_crc assumes input pointer address is 8 byte aligned
which may not be always the case.
This fix aligns the input pointer before proceeding to process it
in 8 byte chunks.

Fixes: 504a29af13a7 ("hash: fix strict-aliasing for CRC")
Cc: stable@dpdk.org
Cc: stephen@networkplumber.org

Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
---
 lib/hash/rte_hash_crc.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

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) {
+		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;
+		}
+	}
+
 	for (i = 0; i < data_len / 8; i++) {
 		init_val = rte_hash_crc_8byte(*(const uint64_t *)pd, init_val);
 		pd += 8;
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* RE: [PATCH] hash: fix pointer alignment
  2026-02-26 14:22 [PATCH] hash: fix pointer alignment Radu Nicolau
@ 2026-02-26 16:22 ` Marat Khalili
  2026-02-26 16:43   ` Radu Nicolau
  2026-02-27  9:48 ` [PATCH v2] " Radu Nicolau
  2026-02-27 13:59 ` [PATCH v3] " Radu Nicolau
  2 siblings, 1 reply; 11+ messages in thread
From: Marat Khalili @ 2026-02-26 16:22 UTC (permalink / raw)
  To: Radu Nicolau
  Cc: stable@dpdk.org, stephen@networkplumber.org, Yipeng Wang,
	Sameh Gobriel, Bruce Richardson, Vladimir Medvedkin,
	Pablo de Lara, Yerden Zhumabekov, dev@dpdk.org

> 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?

> +		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?

> +	}
> +
>  	for (i = 0; i < data_len / 8; i++) {
>  		init_val = rte_hash_crc_8byte(*(const uint64_t *)pd, init_val);
>  		pd += 8;
> --
> 2.52.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] hash: fix pointer alignment
  2026-02-26 16:22 ` Marat Khalili
@ 2026-02-26 16:43   ` Radu Nicolau
  2026-02-26 19:47     ` Bruce Richardson
  0 siblings, 1 reply; 11+ messages in thread
From: Radu Nicolau @ 2026-02-26 16:43 UTC (permalink / raw)
  To: Marat Khalili
  Cc: stable@dpdk.org, stephen@networkplumber.org, Yipeng Wang,
	Sameh Gobriel, Bruce Richardson, Vladimir Medvedkin,
	Pablo de Lara, Yerden Zhumabekov, dev@dpdk.org


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.
>
>> +	}
>> +
>>   	for (i = 0; i < data_len / 8; i++) {
>>   		init_val = rte_hash_crc_8byte(*(const uint64_t *)pd, init_val);
>>   		pd += 8;
>> --
>> 2.52.0

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] hash: fix pointer alignment
  2026-02-26 16:43   ` Radu Nicolau
@ 2026-02-26 19:47     ` Bruce Richardson
  2026-02-27  9:44       ` Radu Nicolau
  0 siblings, 1 reply; 11+ messages in thread
From: Bruce Richardson @ 2026-02-26 19:47 UTC (permalink / raw)
  To: Radu Nicolau
  Cc: Marat Khalili, stable@dpdk.org, stephen@networkplumber.org,
	Yipeng Wang, Sameh Gobriel, Vladimir Medvedkin, Pablo de Lara,
	Yerden Zhumabekov, dev@dpdk.org

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] hash: fix pointer alignment
  2026-02-26 19:47     ` Bruce Richardson
@ 2026-02-27  9:44       ` Radu Nicolau
  0 siblings, 0 replies; 11+ messages in thread
From: Radu Nicolau @ 2026-02-27  9:44 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: Marat Khalili, stable@dpdk.org, stephen@networkplumber.org,
	Yipeng Wang, Sameh Gobriel, Vladimir Medvedkin, Pablo de Lara,
	Yerden Zhumabekov, dev@dpdk.org


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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2] hash: fix pointer alignment
  2026-02-26 14:22 [PATCH] hash: fix pointer alignment Radu Nicolau
  2026-02-26 16:22 ` Marat Khalili
@ 2026-02-27  9:48 ` Radu Nicolau
  2026-02-27 12:37   ` Marat Khalili
  2026-02-27 13:59 ` [PATCH v3] " Radu Nicolau
  2 siblings, 1 reply; 11+ messages in thread
From: Radu Nicolau @ 2026-02-27  9:48 UTC (permalink / raw)
  To: dev
  Cc: marat.khalili, Radu Nicolau, stable, stephen, Yipeng Wang,
	Sameh Gobriel, Bruce Richardson, Vladimir Medvedkin,
	Pablo de Lara, Yerden Zhumabekov

rte_hash_crc assumes input pointer address is 8 byte aligned
which may not be always the case.
This fix aligns the input pointer before proceeding to process it
in 8 byte chunks.

Fixes: 504a29af13a7 ("hash: fix strict-aliasing for CRC")
Cc: stable@dpdk.org
Cc: stephen@networkplumber.org

Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
---
v2: reverse the order of alignment adjustment calls

 lib/hash/rte_hash_crc.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/lib/hash/rte_hash_crc.h b/lib/hash/rte_hash_crc.h
index fa07c97685..d61420868a 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) {
+		uintptr_t unaligned_bytes = 8 - (pd & 0x7);
+		data_len -= unaligned_bytes;
+		if (unaligned_bytes & 0x1) {
+			init_val = rte_hash_crc_1byte(*(const uint8_t *)pd, init_val);
+			pd += 1;
+		}
+		if (unaligned_bytes & 0x2) {
+			init_val = rte_hash_crc_2byte(*(const uint16_t *)pd, init_val);
+			pd += 2;
+		}
+		if (unaligned_bytes & 0x4) {
+			init_val = rte_hash_crc_4byte(*(const uint32_t *)pd, init_val);
+			pd += 4;
+		}
+	}
+
 	for (i = 0; i < data_len / 8; i++) {
 		init_val = rte_hash_crc_8byte(*(const uint64_t *)pd, init_val);
 		pd += 8;
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* RE: [PATCH v2] hash: fix pointer alignment
  2026-02-27  9:48 ` [PATCH v2] " Radu Nicolau
@ 2026-02-27 12:37   ` Marat Khalili
  2026-02-27 13:00     ` Radu Nicolau
  0 siblings, 1 reply; 11+ messages in thread
From: Marat Khalili @ 2026-02-27 12:37 UTC (permalink / raw)
  To: Radu Nicolau, dev@dpdk.org
  Cc: stable@dpdk.org, stephen@networkplumber.org, Yipeng Wang,
	Sameh Gobriel, Bruce Richardson, Vladimir Medvedkin,
	Pablo de Lara, Yerden Zhumabekov

> -----Original Message-----
> From: Radu Nicolau <radu.nicolau@intel.com>
> Sent: Friday 27 February 2026 09:48
> To: dev@dpdk.org
> Cc: Marat Khalili <marat.khalili@huawei.com>; Radu Nicolau <radu.nicolau@intel.com>; stable@dpdk.org;
> stephen@networkplumber.org; Yipeng Wang <yipeng1.wang@intel.com>; Sameh Gobriel
> <sameh.gobriel@intel.com>; Bruce Richardson <bruce.richardson@intel.com>; Vladimir Medvedkin
> <vladimir.medvedkin@intel.com>; Pablo de Lara <pablo.de.lara.guarch@intel.com>; Yerden Zhumabekov
> <e_zhumabekov@sts.kz>
> Subject: [PATCH v2] hash: fix pointer alignment
> 
> rte_hash_crc assumes input pointer address is 8 byte aligned
> which may not be always the case.
> This fix aligns the input pointer before proceeding to process it
> in 8 byte chunks.
> 
> Fixes: 504a29af13a7 ("hash: fix strict-aliasing for CRC")
> Cc: stable@dpdk.org
> Cc: stephen@networkplumber.org
> 
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> ---
> v2: reverse the order of alignment adjustment calls
> 
>  lib/hash/rte_hash_crc.h | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/lib/hash/rte_hash_crc.h b/lib/hash/rte_hash_crc.h
> index fa07c97685..d61420868a 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) {
> +		uintptr_t unaligned_bytes = 8 - (pd & 0x7);
> +		data_len -= unaligned_bytes;
> +		if (unaligned_bytes & 0x1) {
> +			init_val = rte_hash_crc_1byte(*(const uint8_t *)pd, init_val);
> +			pd += 1;
> +		}
> +		if (unaligned_bytes & 0x2) {
> +			init_val = rte_hash_crc_2byte(*(const uint16_t *)pd, init_val);
> +			pd += 2;
> +		}
> +		if (unaligned_bytes & 0x4) {
> +			init_val = rte_hash_crc_4byte(*(const uint32_t *)pd, init_val);
> +			pd += 4;
> +		}
> +	}
> +
>  	for (i = 0; i < data_len / 8; i++) {
>  		init_val = rte_hash_crc_8byte(*(const uint64_t *)pd, init_val);
>  		pd += 8;
> --
> 2.52.0
> 

Surprisingly, we do not seem to have any tests calling rte_hash_crc with misaligned data. I tried to tweak existing ones in test_hash.c and test_hash_functions.c, and found out that rte_hash_crc still fails (with ubsan) for sizes less than 8 in one of the last 3 if's, and also that jhash has the same problems. Still this commit is a step in the right direction IMO, so:

Acked-by: Marat Khalili <marat.khalili@huawei.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] hash: fix pointer alignment
  2026-02-27 12:37   ` Marat Khalili
@ 2026-02-27 13:00     ` Radu Nicolau
  0 siblings, 0 replies; 11+ messages in thread
From: Radu Nicolau @ 2026-02-27 13:00 UTC (permalink / raw)
  To: Marat Khalili, dev@dpdk.org
  Cc: stable@dpdk.org, stephen@networkplumber.org, Yipeng Wang,
	Sameh Gobriel, Bruce Richardson, Vladimir Medvedkin,
	Pablo de Lara, Yerden Zhumabekov


On 27-Feb-26 12:37 PM, Marat Khalili wrote:
>> -----Original Message-----
>> From: Radu Nicolau <radu.nicolau@intel.com>
>> Sent: Friday 27 February 2026 09:48
>> To: dev@dpdk.org
>> Cc: Marat Khalili <marat.khalili@huawei.com>; Radu Nicolau <radu.nicolau@intel.com>; stable@dpdk.org;
>> stephen@networkplumber.org; Yipeng Wang <yipeng1.wang@intel.com>; Sameh Gobriel
>> <sameh.gobriel@intel.com>; Bruce Richardson <bruce.richardson@intel.com>; Vladimir Medvedkin
>> <vladimir.medvedkin@intel.com>; Pablo de Lara <pablo.de.lara.guarch@intel.com>; Yerden Zhumabekov
>> <e_zhumabekov@sts.kz>
>> Subject: [PATCH v2] hash: fix pointer alignment
>>
>> rte_hash_crc assumes input pointer address is 8 byte aligned
>> which may not be always the case.
>> This fix aligns the input pointer before proceeding to process it
>> in 8 byte chunks.
>>
>> Fixes: 504a29af13a7 ("hash: fix strict-aliasing for CRC")
>> Cc: stable@dpdk.org
>> Cc: stephen@networkplumber.org
>>
>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
>> ---
>> v2: reverse the order of alignment adjustment calls
>>
>>   lib/hash/rte_hash_crc.h | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/lib/hash/rte_hash_crc.h b/lib/hash/rte_hash_crc.h
>> index fa07c97685..d61420868a 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) {
>> +		uintptr_t unaligned_bytes = 8 - (pd & 0x7);
>> +		data_len -= unaligned_bytes;
>> +		if (unaligned_bytes & 0x1) {
>> +			init_val = rte_hash_crc_1byte(*(const uint8_t *)pd, init_val);
>> +			pd += 1;
>> +		}
>> +		if (unaligned_bytes & 0x2) {
>> +			init_val = rte_hash_crc_2byte(*(const uint16_t *)pd, init_val);
>> +			pd += 2;
>> +		}
>> +		if (unaligned_bytes & 0x4) {
>> +			init_val = rte_hash_crc_4byte(*(const uint32_t *)pd, init_val);
>> +			pd += 4;
>> +		}
>> +	}
>> +
>>   	for (i = 0; i < data_len / 8; i++) {
>>   		init_val = rte_hash_crc_8byte(*(const uint64_t *)pd, init_val);
>>   		pd += 8;
>> --
>> 2.52.0
>>
> Surprisingly, we do not seem to have any tests calling rte_hash_crc with misaligned data. I tried to tweak existing ones in test_hash.c and test_hash_functions.c, and found out that rte_hash_crc still fails (with ubsan) for sizes less than 8 in one of the last 3 if's, and also that jhash has the same problems. Still this commit is a step in the right direction IMO, so:
>
> Acked-by: Marat Khalili <marat.khalili@huawei.com>

Thanks for the ack!

The issue still persist with less that 8 bytes unaligned input because 
the 3 ifs after the loop assume the input is aligned, which is the case 
only if the loop was taken. I will fix the patch to address this case in 
the added section - you were right with the first comment.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3] hash: fix pointer alignment
  2026-02-26 14:22 [PATCH] hash: fix pointer alignment Radu Nicolau
  2026-02-26 16:22 ` Marat Khalili
  2026-02-27  9:48 ` [PATCH v2] " Radu Nicolau
@ 2026-02-27 13:59 ` Radu Nicolau
  2026-02-27 15:55   ` Marat Khalili
  2026-03-05 12:18   ` David Marchand
  2 siblings, 2 replies; 11+ messages in thread
From: Radu Nicolau @ 2026-02-27 13:59 UTC (permalink / raw)
  To: dev
  Cc: marat.khalili, Radu Nicolau, stable, stephen, Yipeng Wang,
	Sameh Gobriel, Bruce Richardson, Vladimir Medvedkin,
	Yerden Zhumabekov, Pablo de Lara

rte_hash_crc assumes input pointer address is 8 byte aligned
which may not be always the case.
This fix aligns the input pointer before proceeding to process it
in 8 byte chunks.

Bugzilla ID: 1892
Fixes: 504a29af13a7 ("hash: fix strict-aliasing for CRC")
Cc: stable@dpdk.org
Cc: stephen@networkplumber.org

Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
Acked-by: Marat Khalili <marat.khalili@huawei.com>
---
v3: revert alignment code to simple loop, it was getting too complex for a corner case


 lib/hash/rte_hash_crc.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lib/hash/rte_hash_crc.h b/lib/hash/rte_hash_crc.h
index fa07c97685..f60f4598d8 100644
--- a/lib/hash/rte_hash_crc.h
+++ b/lib/hash/rte_hash_crc.h
@@ -127,6 +127,16 @@ 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) {
+		unsigned int unaligned_bytes = RTE_MIN(8 - (pd & 0x7), data_len);
+		for (i = 0; i < unaligned_bytes; i++) {
+			init_val = rte_hash_crc_1byte(*(const uint8_t *)pd, init_val);
+			pd++;
+			data_len--;
+		}
+	}
+
 	for (i = 0; i < data_len / 8; i++) {
 		init_val = rte_hash_crc_8byte(*(const uint64_t *)pd, init_val);
 		pd += 8;
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* RE: [PATCH v3] hash: fix pointer alignment
  2026-02-27 13:59 ` [PATCH v3] " Radu Nicolau
@ 2026-02-27 15:55   ` Marat Khalili
  2026-03-05 12:18   ` David Marchand
  1 sibling, 0 replies; 11+ messages in thread
From: Marat Khalili @ 2026-02-27 15:55 UTC (permalink / raw)
  To: Radu Nicolau, dev@dpdk.org
  Cc: stable@dpdk.org, stephen@networkplumber.org, Yipeng Wang,
	Sameh Gobriel, Bruce Richardson, Vladimir Medvedkin,
	Yerden Zhumabekov, Pablo de Lara

> -----Original Message-----
> From: Radu Nicolau <radu.nicolau@intel.com>
> Sent: Friday 27 February 2026 14:00
> To: dev@dpdk.org
> Cc: Marat Khalili <marat.khalili@huawei.com>; Radu Nicolau <radu.nicolau@intel.com>; stable@dpdk.org;
> stephen@networkplumber.org; Yipeng Wang <yipeng1.wang@intel.com>; Sameh Gobriel
> <sameh.gobriel@intel.com>; Bruce Richardson <bruce.richardson@intel.com>; Vladimir Medvedkin
> <vladimir.medvedkin@intel.com>; Yerden Zhumabekov <e_zhumabekov@sts.kz>; Pablo de Lara
> <pablo.de.lara.guarch@intel.com>
> Subject: [PATCH v3] hash: fix pointer alignment
> 
> rte_hash_crc assumes input pointer address is 8 byte aligned
> which may not be always the case.
> This fix aligns the input pointer before proceeding to process it
> in 8 byte chunks.
> 
> Bugzilla ID: 1892
> Fixes: 504a29af13a7 ("hash: fix strict-aliasing for CRC")
> Cc: stable@dpdk.org
> Cc: stephen@networkplumber.org
> 
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> Acked-by: Marat Khalili <marat.khalili@huawei.com>
> ---
> v3: revert alignment code to simple loop, it was getting too complex for a corner case
> 
> 
>  lib/hash/rte_hash_crc.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/lib/hash/rte_hash_crc.h b/lib/hash/rte_hash_crc.h
> index fa07c97685..f60f4598d8 100644
> --- a/lib/hash/rte_hash_crc.h
> +++ b/lib/hash/rte_hash_crc.h
> @@ -127,6 +127,16 @@ 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) {
> +		unsigned int unaligned_bytes = RTE_MIN(8 - (pd & 0x7), data_len);
> +		for (i = 0; i < unaligned_bytes; i++) {
> +			init_val = rte_hash_crc_1byte(*(const uint8_t *)pd, init_val);
> +			pd++;
> +			data_len--;
> +		}
> +	}
> +
>  	for (i = 0; i < data_len / 8; i++) {
>  		init_val = rte_hash_crc_8byte(*(const uint64_t *)pd, init_val);
>  		pd += 8;
> --
> 2.52.0
> 

My custom checks now pass even for small inputs.

(I also support the idea to simplify it to a single loop, until we have actual
benchmarks showing that some alternative is better. Same for many possible ways
to simplify if and/or for condition.)

Acked-by: Marat Khalili <marat.khalili@huawei.com>
Tested-by: Marat Khalili <marat.khalili@huawei.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3] hash: fix pointer alignment
  2026-02-27 13:59 ` [PATCH v3] " Radu Nicolau
  2026-02-27 15:55   ` Marat Khalili
@ 2026-03-05 12:18   ` David Marchand
  1 sibling, 0 replies; 11+ messages in thread
From: David Marchand @ 2026-03-05 12:18 UTC (permalink / raw)
  To: Radu Nicolau
  Cc: dev, marat.khalili, stable, stephen, Yipeng Wang, Sameh Gobriel,
	Bruce Richardson, Vladimir Medvedkin, Yerden Zhumabekov,
	Pablo de Lara

On Fri, 27 Feb 2026 at 15:01, Radu Nicolau <radu.nicolau@intel.com> wrote:
>
> rte_hash_crc assumes input pointer address is 8 byte aligned
> which may not be always the case.
> This fix aligns the input pointer before proceeding to process it
> in 8 byte chunks.
>
> Bugzilla ID: 1892
> Fixes: 504a29af13a7 ("hash: fix strict-aliasing for CRC")

This code has assumed integer alignment since day 0.
This was a 32-bit requirement until commit 614289298daf, and it has
been 64-bit since then.

So I updated the Fixes: reference to:
Fixes: af75078fece3 ("first public release")

> Cc: stable@dpdk.org
>
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> Acked-by: Marat Khalili <marat.khalili@huawei.com>

I am surprised we did not catch the bug with the ipsec_sad_autotest
(as reported in bugzilla) in the GHA UBSan job...
But I can reproduce it locally.

We could probably extend the hash unit test coverage.

In any case, the fix lgtm.
Applied, thanks.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-03-05 12:19 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-26 14:22 [PATCH] hash: fix pointer alignment Radu Nicolau
2026-02-26 16:22 ` Marat Khalili
2026-02-26 16:43   ` Radu Nicolau
2026-02-26 19:47     ` Bruce Richardson
2026-02-27  9:44       ` Radu Nicolau
2026-02-27  9:48 ` [PATCH v2] " Radu Nicolau
2026-02-27 12:37   ` Marat Khalili
2026-02-27 13:00     ` Radu Nicolau
2026-02-27 13:59 ` [PATCH v3] " Radu Nicolau
2026-02-27 15:55   ` Marat Khalili
2026-03-05 12:18   ` David Marchand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox