Message ID | 20200408082921.31000-5-mk@semihalf.com |
---|---|
State | Accepted, archived |
Delegated to: | Ferruh Yigit |
Headers | show |
Series |
|
Related | show |
Context | Check | Description |
---|---|---|
ci/Intel-compilation | success | Compilation OK |
ci/checkpatch | success | coding style OK |
On 4/8/2020 9:28 AM, Michal Krawczyk wrote: > 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]; | ^~~~~~~~~~~
>> 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
diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_com.c index 17b51b5a11..38a474b1bd 100644 --- a/drivers/net/ena/base/ena_com.c +++ b/drivers/net/ena/base/ena_com.c @@ -1,5 +1,5 @@ /* SPDX-License-Identifier: BSD-3-Clause - * Copyright (c) 2015-2019 Amazon.com, Inc. or its affiliates. + * Copyright (c) 2015-2020 Amazon.com, Inc. or its affiliates. * All rights reserved. */ @@ -1032,6 +1032,19 @@ static int ena_com_get_feature(struct ena_com_dev *ena_dev, feature_ver); } +static void ena_com_hash_key_fill_default_key(struct ena_com_dev *ena_dev) +{ + struct ena_admin_feature_rss_flow_hash_control *hash_key = + (ena_dev->rss).hash_key; + + ENA_RSS_FILL_KEY(&hash_key->key, sizeof(hash_key->key)); + /* The key is stored in the device in uint32_t array + * as well as the API requires the key to be passed in this + * format. Thus the size of our array should be divided by 4 + */ + hash_key->keys_num = sizeof(hash_key->key) / sizeof(uint32_t); +} + static int ena_com_hash_key_allocate(struct ena_com_dev *ena_dev) { struct ena_rss *rss = &ena_dev->rss; @@ -2405,15 +2418,16 @@ int ena_com_fill_hash_function(struct ena_com_dev *ena_dev, switch (func) { case ENA_ADMIN_TOEPLITZ: - if (key_len > sizeof(hash_key->key)) { - ena_trc_err("key len (%hu) is bigger than the max supported (%zu)\n", - key_len, sizeof(hash_key->key)); - return ENA_COM_INVAL; + if (key) { + if (key_len != sizeof(hash_key->key)) { + ena_trc_err("key len (%hu) doesn't equal the supported size (%zu)\n", + key_len, sizeof(hash_key->key)); + return ENA_COM_INVAL; + } + memcpy(hash_key->key, key, key_len); + rss->hash_init_val = init_val; + hash_key->keys_num = key_len / sizeof(u32); } - - memcpy(hash_key->key, key, key_len); - rss->hash_init_val = init_val; - hash_key->keys_num = key_len >> 2; break; case ENA_ADMIN_CRC32: rss->hash_init_val = init_val; @@ -2738,6 +2752,8 @@ int ena_com_rss_init(struct ena_com_dev *ena_dev, u16 indr_tbl_log_size) if (unlikely(rc)) goto err_hash_key; + ena_com_hash_key_fill_default_key(ena_dev); + rc = ena_com_hash_ctrl_init(ena_dev); if (unlikely(rc)) goto err_hash_ctrl; diff --git a/drivers/net/ena/base/ena_com.h b/drivers/net/ena/base/ena_com.h index f2ef26c91b..d58c802edf 100644 --- a/drivers/net/ena/base/ena_com.h +++ b/drivers/net/ena/base/ena_com.h @@ -1,5 +1,5 @@ /* SPDX-License-Identifier: BSD-3-Clause - * Copyright (c) 2015-2019 Amazon.com, Inc. or its affiliates. + * Copyright (c) 2015-2020 Amazon.com, Inc. or its affiliates. * All rights reserved. */ @@ -53,6 +53,7 @@ #define ENA_INTR_DELAY_NEW_VALUE_WEIGHT 4 #define ENA_INTR_MODER_LEVEL_STRIDE 1 #define ENA_INTR_BYTE_COUNT_NOT_SUPPORTED 0xFFFFFF +#define ENA_HASH_KEY_SIZE 40 #define ENA_HW_HINTS_NO_TIMEOUT 0xFFFF diff --git a/drivers/net/ena/base/ena_plat_dpdk.h b/drivers/net/ena/base/ena_plat_dpdk.h index 4b8fe017dd..e9b33bc36c 100644 --- a/drivers/net/ena/base/ena_plat_dpdk.h +++ b/drivers/net/ena/base/ena_plat_dpdk.h @@ -301,6 +301,10 @@ extern rte_atomic32_t ena_alloc_cnt; #define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d)) +void ena_rss_key_fill(void *key, size_t size); + +#define ENA_RSS_FILL_KEY(key, size) ena_rss_key_fill(key, size) + #include "ena_includes.h" #endif /* DPDK_ENA_COM_ENA_PLAT_DPDK_H_ */ diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c index e0ed28419c..f1202d99f2 100644 --- a/drivers/net/ena/ena_ethdev.c +++ b/drivers/net/ena/ena_ethdev.c @@ -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); +} + static inline void ena_rx_mbuf_prepare(struct rte_mbuf *mbuf, struct ena_com_rx_ctx *ena_rx_ctx) {