Re: [dpdk-dev] [PATCH v3 04/30] net/ena/base: generate default, random RSS hash key

[David Christensen <drc@linux.vnet.ibm.com>]

>> Although the RSS key still cannot be set, it is now being generated
>> every time the driver is being initialized.
>>
>> Multiple devices can still have the same key if they're used by the same
>> driver.
>>
>> Signed-off-by: Michal Krawczyk <mk@semihalf.com>
>> Reviewed-by: Igor Chauskin <igorch@amazon.com>
>> Reviewed-by: Guy Tzalik <gtzalik@amazon.com>
> 
> <...>
> 
>> @@ -256,6 +256,23 @@ static const struct eth_dev_ops ena_dev_ops = {
>>   	.reta_query           = ena_rss_reta_query,
>>   };
>>   
>> +void ena_rss_key_fill(void *key, size_t size)
>> +{
>> +	static bool key_generated;
>> +	static uint8_t default_key[ENA_HASH_KEY_SIZE];
>> +	size_t i;
>> +
>> +	RTE_ASSERT(size <= ENA_HASH_KEY_SIZE);
>> +
>> +	if (!key_generated) {
>> +		for (i = 0; i < ENA_HASH_KEY_SIZE; ++i)
>> +			default_key[i] = rte_rand() & 0xff;
>> +		key_generated = true;
>> +	}
>> +
>> +	rte_memcpy(key, default_key, size);
>> +}
>> +
> 
> I have updated PPC cross compiler [1] and now getting following build error [2].
> cc'ed David from IBM too. Can you please check this?
> 
> [1]
> powerpc64le-linux-gcc.br_real (Buildroot 2020.02-00011-g7ea8a52) 9.3.0
> 
> 
> [2] https://pastebin.com/h70uFJmm
> 
> In file included from /.../dpdk/_ppc_64-power8-linuxapp-gcc/include/rte_ether.h:21,
>                   from /.../dpdk/drivers/net/ena/ena_ethdev.c:7:
> /.../dpdk/drivers/net/ena/ena_ethdev.c: In function ‘ena_rss_key_fill’:
> /.../dpdk/_ppc_64-power8-linuxapp-gcc/include/rte_memcpy.h:47:2: error: array
> subscript 3 is outside array bounds of ‘uint8_t[40]’ {aka ‘unsigned char[40]’}
> [-Werror=array-bounds]
>     47 |  vec_vsx_st(vec_vsx_ld(48, src), 48, dst);
>        |  ^~~~~~~~~~
> /.../dpdk/drivers/net/ena/ena_ethdev.c:277:17: note: while referencing ‘default_key’
>    277 |  static uint8_t default_key[ENA_HASH_KEY_SIZE];
>        |                 ^~~~~~~~~~~
> In file included from /.../dpdk/_ppc_64-power8-linuxapp-gcc/include/rte_ether.h:21,
>                   from /.../dpdk/drivers/net/ena/ena_ethdev.c:7:
> /.../dpdk/_ppc_64-power8-linuxapp-gcc/include/rte_memcpy.h:47:2: error: array
> subscript [3, 7] is outside array bounds of ‘uint8_t[40]’ {aka ‘unsigned
> char[40]’} [-Werror=array-bounds]
>     47 |  vec_vsx_st(vec_vsx_ld(48, src), 48, dst);
>        |  ^~~~~~~~~~
> /.../dpdk/drivers/net/ena/ena_ethdev.c:277:17: note: while referencing ‘default_key’
>    277 |  static uint8_t default_key[ENA_HASH_KEY_SIZE];
>        |                 ^~~~~~~~~~~
> 

Appears to be an open gcc bug 
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90387).  PPC optimized 
version of rte_memcpy() uses __builtin_constant_p() which triggers the 
error when -Werror=array-bounds is used, x86 DPDK does not use 
__builtin_constant_p() and does not encounter an error.  Can you try the 
following?  Worked for me with Ubuntu 20.04.

Dave

diff --git a/lib/librte_eal/ppc/include/rte_memcpy.h 
b/lib/librte_eal/ppc/include/rte_memcpy.h
index 25311ba1d..973e2bebe 100644
--- a/lib/librte_eal/ppc/include/rte_memcpy.h
+++ b/lib/librte_eal/ppc/include/rte_memcpy.h
@@ -17,6 +17,11 @@ extern "C" {

  #include "generic/rte_memcpy.h"

+#if (__GNUC__ == 9 && __GNUC_MINOR__ < 4)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Warray-bounds"
+#endif
+
  static inline void
  rte_mov16(uint8_t *dst, const uint8_t *src)
  {
@@ -108,6 +113,13 @@ rte_memcpy_func(void *dst, const void *src, size_t n)
                 return ret;
         }

+       if (n <= 48) {
+               rte_mov32((uint8_t *)dst, (const uint8_t *)src);
+               rte_mov16((uint8_t *)dst - 16 + n,
+                       (const uint8_t *)src - 16 + n);
+               return ret;
+       }
+
         if (n <= 64) {
                 rte_mov32((uint8_t *)dst, (const uint8_t *)src);
                 rte_mov32((uint8_t *)dst - 32 + n,
@@ -192,6 +204,11 @@ rte_memcpy_func(void *dst, const void *src, size_t n)
         return ret;
  }

+#if (__GNUC__ == 9 && __GNUC_MINOR__ < 4)
+#pragma GCC diagnostic pop
+#endif
+
+
  #ifdef __cplusplus
  }
  #endif