DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH 00/12] Fix compilation with gcc 12
@ 2022-05-18 10:16 David Marchand
  2022-05-18 10:16 ` [PATCH 01/12] common/cpt: fix build with GCC 12 David Marchand
                   ` (13 more replies)
  0 siblings, 14 replies; 73+ messages in thread
From: David Marchand @ 2022-05-18 10:16 UTC (permalink / raw)
  To: dev; +Cc: thomas, ferruh.yigit

Fedora 36 is out since early may and comes with gcc 12.
This series fixes compilation or waives some checks.

There might be something fishy with rte_memcpy on x86 but, for now,
the rte_memcpy related fixes are on the caller side.

Some "base" drivers have issues, I chose the simple solution of waiving
the checks for them.

Compilation is the only thing checked.
Please driver maintainers, check nothing got broken.

-- 
David Marchand

David Marchand (12):
  common/cpt: fix build with GCC 12
  crypto/cnxk: fix build with GCC 12
  crypto/ipsec_mb: fix build with GCC 12
  net/ena: fix build with GCC 12
  net/enetfec: fix build with GCC 12
  net/ice: fix build with GCC 12
  net/ice/base: fix build with GCC 12
  net/qede/base: fix build with GCC 12
  vdpa/ifc: fix build with GCC 12
  vhost/crypto: fix build with GCC 12
  app/flow-perf: fix build with GCC 12
  test/ipsec: fix build with GCC 12

 app/test-flow-perf/main.c            | 48 ++++++----------------------
 app/test/test_ipsec.c                | 48 +++++++++++++++++-----------
 drivers/common/cpt/cpt_ucode.h       |  8 +++++
 drivers/crypto/cnxk/cnxk_se.h        |  8 +++++
 drivers/crypto/ipsec_mb/pmd_snow3g.c |  7 ++--
 drivers/net/ena/ena_rss.c            |  7 ++--
 drivers/net/enetfec/enet_ethdev.c    |  9 ++++++
 drivers/net/ice/base/meson.build     |  5 +++
 drivers/net/ice/ice_ethdev.c         |  3 +-
 drivers/net/qede/base/meson.build    |  5 +++
 drivers/vdpa/ifc/ifcvf_vdpa.c        |  2 ++
 lib/vhost/vhost_crypto.c             |  8 ++---
 12 files changed, 88 insertions(+), 70 deletions(-)

-- 
2.36.1


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

* [PATCH 01/12] common/cpt: fix build with GCC 12
  2022-05-18 10:16 [PATCH 00/12] Fix compilation with gcc 12 David Marchand
@ 2022-05-18 10:16 ` David Marchand
  2022-05-20 20:23   ` Stephen Hemminger
  2022-06-10 13:11   ` David Marchand
  2022-05-18 10:16 ` [PATCH 02/12] crypto/cnxk: " David Marchand
                   ` (12 subsequent siblings)
  13 siblings, 2 replies; 73+ messages in thread
From: David Marchand @ 2022-05-18 10:16 UTC (permalink / raw)
  To: dev; +Cc: thomas, ferruh.yigit, stable, Anoob Joseph

GCC 12 raises the following warning:

In function ‘fill_sg_comp_from_iov’,
    inlined from ‘cpt_kasumi_enc_prep’ at
        ../drivers/common/cpt/cpt_ucode.h:2176:8,
    inlined from ‘cpt_fc_enc_hmac_prep’ at
        ../drivers/common/cpt/cpt_ucode.h:2475:3,
    inlined from ‘fill_digest_params’ at
        ../drivers/common/cpt/cpt_ucode.h:3548:14,
    inlined from ‘otx_cpt_enq_single_sym’ at
        ../drivers/crypto/octeontx/otx_cryptodev_ops.c:541:9,
    inlined from ‘otx_cpt_enq_single_sym_sessless’ at
        ../drivers/crypto/octeontx/otx_cryptodev_ops.c:584:8,
    inlined from ‘otx_cpt_enq_single’ at
        ../drivers/crypto/octeontx/otx_cryptodev_ops.c:611:11,
    inlined from ‘otx_cpt_pkt_enqueue’ at
        ../drivers/crypto/octeontx/otx_cryptodev_ops.c:643:9,
    inlined from ‘otx_cpt_enqueue_sym’ at
        ../drivers/crypto/octeontx/otx_cryptodev_ops.c:668:9:
../drivers/common/cpt/cpt_ucode.h:415:36: error: array subscript 0 is
        outside array bounds of ‘buf_ptr_t[0]’ {aka ‘struct buf_ptr[]’}
        [-Werror=array-bounds]
  415 |                         e_dma_addr = bufs[j].dma_addr;
      |                         ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
../drivers/common/cpt/cpt_ucode.h:416:48: error: array subscript 0 is
        outside array bounds of ‘buf_ptr_t[0]’ {aka ‘struct buf_ptr[]’}
        [-Werror=array-bounds]
  416 |                         e_len = (size > bufs[j].size) ?
      |                                         ~~~~~~~^~~~~

For now, waive this warning until we have a proper fix.

Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/common/cpt/cpt_ucode.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/common/cpt/cpt_ucode.h b/drivers/common/cpt/cpt_ucode.h
index e1f2f6005d..bdf72b400c 100644
--- a/drivers/common/cpt/cpt_ucode.h
+++ b/drivers/common/cpt/cpt_ucode.h
@@ -412,9 +412,17 @@ fill_sg_comp_from_iov(sg_comp_t *list,
 				(bufs[j].size - from_offset) : size;
 			from_offset = 0;
 		} else {
+/* FIXME */
+#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 120000)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Warray-bounds"
+#endif
 			e_dma_addr = bufs[j].dma_addr;
 			e_len = (size > bufs[j].size) ?
 				bufs[j].size : size;
+#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 120000)
+#pragma GCC diagnostic pop
+#endif
 		}
 
 		to->u.s.len[i % 4] = rte_cpu_to_be_16(e_len);
-- 
2.36.1


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

* [PATCH 02/12] crypto/cnxk: fix build with GCC 12
  2022-05-18 10:16 [PATCH 00/12] Fix compilation with gcc 12 David Marchand
  2022-05-18 10:16 ` [PATCH 01/12] common/cpt: fix build with GCC 12 David Marchand
@ 2022-05-18 10:16 ` David Marchand
  2022-05-20 20:24   ` Stephen Hemminger
  2022-05-18 10:16 ` [PATCH 03/12] crypto/ipsec_mb: " David Marchand
                   ` (11 subsequent siblings)
  13 siblings, 1 reply; 73+ messages in thread
From: David Marchand @ 2022-05-18 10:16 UTC (permalink / raw)
  To: dev
  Cc: thomas, ferruh.yigit, stable, Ankur Dwivedi, Anoob Joseph,
	Tejasree Kondoj

GCC 12 raises the following warning:

In file included from ../drivers/crypto/cnxk/cn10k_cryptodev_ops.c:17:
In function ‘fill_sg_comp_from_iov’,
    inlined from ‘cpt_kasumi_enc_prep’ at
        ../drivers/crypto/cnxk/cnxk_se.h:1413:8,
    inlined from ‘cpt_fc_enc_hmac_prep’ at
        ../drivers/crypto/cnxk/cnxk_se.h:1635:9,
    inlined from ‘fill_digest_params’ at
        ../drivers/crypto/cnxk/cnxk_se.h:2524:8,
    inlined from ‘cpt_sym_inst_fill’ at
        ../drivers/crypto/cnxk/cn10k_cryptodev_ops.c:92:9,
    inlined from ‘cn10k_cpt_fill_inst.constprop.isra’ at
        ../drivers/crypto/cnxk/cn10k_cryptodev_ops.c:146:10:
../drivers/crypto/cnxk/cnxk_se.h:208:52: error: array subscript 0 is
        outside array bounds of ‘struct roc_se_buf_ptr[0]’
        [-Werror=array-bounds]
  208 |             e_vaddr = (uint64_t)bufs[j].vaddr;
      |                                 ~~~~~~~^~~~~~
../drivers/crypto/cnxk/cnxk_se.h:209:48: error: array subscript 0 is
        outside array bounds of ‘struct roc_se_buf_ptr[0]’
        [-Werror=array-bounds]
  209 |             e_len = (size > bufs[j].size) ? bufs[j].size : size;
      |                             ~~~~~~~^~~~~

For now, waive this warning until we have a proper fix.

Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/crypto/cnxk/cnxk_se.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/crypto/cnxk/cnxk_se.h b/drivers/crypto/cnxk/cnxk_se.h
index ce7ca2eda9..c9d147601f 100644
--- a/drivers/crypto/cnxk/cnxk_se.h
+++ b/drivers/crypto/cnxk/cnxk_se.h
@@ -205,8 +205,16 @@ fill_sg_comp_from_iov(struct roc_se_sglist_comp *list, uint32_t i,
 					size;
 			from_offset = 0;
 		} else {
+/* FIXME */
+#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 120000)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Warray-bounds"
+#endif
 			e_vaddr = (uint64_t)bufs[j].vaddr;
 			e_len = (size > bufs[j].size) ? bufs[j].size : size;
+#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 120000)
+#pragma GCC diagnostic pop
+#endif
 		}
 
 		to->u.s.len[i % 4] = rte_cpu_to_be_16(e_len);
-- 
2.36.1


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

* [PATCH 03/12] crypto/ipsec_mb: fix build with GCC 12
  2022-05-18 10:16 [PATCH 00/12] Fix compilation with gcc 12 David Marchand
  2022-05-18 10:16 ` [PATCH 01/12] common/cpt: fix build with GCC 12 David Marchand
  2022-05-18 10:16 ` [PATCH 02/12] crypto/cnxk: " David Marchand
@ 2022-05-18 10:16 ` David Marchand
  2022-06-02  9:50   ` Bruce Richardson
  2022-06-11 15:34   ` Stephen Hemminger
  2022-05-18 10:16 ` [PATCH 04/12] net/ena: " David Marchand
                   ` (10 subsequent siblings)
  13 siblings, 2 replies; 73+ messages in thread
From: David Marchand @ 2022-05-18 10:16 UTC (permalink / raw)
  To: dev
  Cc: thomas, ferruh.yigit, stable, Fan Zhang, Pablo de Lara,
	Deepak Kumar Jain

GCC 12 raises the following warning:

In function ‘__rte_ring_enqueue_elems_64’,
    inlined from ‘__rte_ring_enqueue_elems’ at
        ../lib/ring/rte_ring_elem_pvt.h:130:3,
    inlined from ‘__rte_ring_do_hts_enqueue_elem’ at
        ../lib/ring/rte_ring_hts_elem_pvt.h:196:3,
    inlined from ‘rte_ring_mp_hts_enqueue_burst_elem’ at
        ../lib/ring/rte_ring_hts.h:110:9,
    inlined from ‘rte_ring_enqueue_burst_elem’ at
        ../lib/ring/rte_ring_elem.h:577:10,
    inlined from ‘rte_ring_enqueue_burst’ at
        ../lib/ring/rte_ring.h:738:9,
    inlined from ‘process_op_bit’ at
        ../drivers/crypto/ipsec_mb/pmd_snow3g.c:425:16,
    inlined from ‘snow3g_pmd_dequeue_burst’ at
        ../drivers/crypto/ipsec_mb/pmd_snow3g.c:484:20:
../lib/ring/rte_ring_elem_pvt.h:68:44: error: array subscript 1 is
        outside array bounds of ‘struct rte_crypto_op[0]’
        [-Werror=array-bounds]
   68 |                         ring[idx + 1] = obj[i + 1];
      |                                         ~~~^~~~~~~
../drivers/crypto/ipsec_mb/pmd_snow3g.c: In function
        ‘snow3g_pmd_dequeue_burst’:
../drivers/crypto/ipsec_mb/pmd_snow3g.c:434:1: note:
        at offset 8 into object ‘op’ of size 8
  434 | snow3g_pmd_dequeue_burst(void *queue_pair,
      | ^~~~~~~~~~~~~~~~~~~~~~~~

Validate that one (exactly) op has been processed or return early.

Fixes: b537abdbee74 ("crypto/snow3g: support bit-level operations")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/crypto/ipsec_mb/pmd_snow3g.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/ipsec_mb/pmd_snow3g.c b/drivers/crypto/ipsec_mb/pmd_snow3g.c
index ebc9a0b562..9a85f46721 100644
--- a/drivers/crypto/ipsec_mb/pmd_snow3g.c
+++ b/drivers/crypto/ipsec_mb/pmd_snow3g.c
@@ -422,12 +422,13 @@ process_op_bit(struct rte_crypto_op *op, struct snow3g_session *session,
 		op->sym->session = NULL;
 	}
 
-	enqueued_op = rte_ring_enqueue_burst(qp->ingress_queue,
-			(void **)&op, processed_op, NULL);
+	if (unlikely(processed_op != 1))
+		return 0;
+	enqueued_op = rte_ring_enqueue(qp->ingress_queue, op);
 	qp->stats.enqueued_count += enqueued_op;
 	*accumulated_enqueued_ops += enqueued_op;
 
-	return enqueued_op;
+	return 1;
 }
 
 static uint16_t
-- 
2.36.1


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

* [PATCH 04/12] net/ena: fix build with GCC 12
  2022-05-18 10:16 [PATCH 00/12] Fix compilation with gcc 12 David Marchand
                   ` (2 preceding siblings ...)
  2022-05-18 10:16 ` [PATCH 03/12] crypto/ipsec_mb: " David Marchand
@ 2022-05-18 10:16 ` David Marchand
  2022-05-20 20:28   ` Stephen Hemminger
  2022-06-11 15:34   ` Stephen Hemminger
  2022-05-18 10:16 ` [PATCH 05/12] net/enetfec: " David Marchand
                   ` (9 subsequent siblings)
  13 siblings, 2 replies; 73+ messages in thread
From: David Marchand @ 2022-05-18 10:16 UTC (permalink / raw)
  To: dev
  Cc: thomas, ferruh.yigit, stable, Marcin Wojtas, Michal Krawczyk,
	Shai Brandes, Evgeny Schemeilin, Igor Chauskin

GCC 12 raises the following warning:

In file included from ../lib/mempool/rte_mempool.h:46,
                 from ../lib/mbuf/rte_mbuf.h:38,
                 from ../lib/net/rte_ether.h:22,
                 from ../drivers/net/ena/ena_ethdev.h:10,
                 from ../drivers/net/ena/ena_rss.c:6:
../drivers/net/ena/ena_rss.c: In function ‘ena_rss_key_fill’:
../lib/eal/x86/include/rte_memcpy.h:370:9: warning: array subscript 64 is
        outside array bounds of ‘uint8_t[40]’ {aka ‘unsigned char[40]’}
        [-Warray-bounds]
  370 | rte_mov32((uint8_t *)dst + 2 * 32, (const uint8_t *)src + 2 * 32);
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/ena/ena_rss.c:51:24: note: while referencing ‘default_key’
   51 | static uint8_t default_key[ENA_HASH_KEY_SIZE];
      |                ^~~~~~~~~~~

This is a false positive because the copied size is checked against
ENA_HASH_KEY_SIZE in a (build) assert.
Silence this warning by calling memcpy with the minimal size.

Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/ena/ena_rss.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ena/ena_rss.c b/drivers/net/ena/ena_rss.c
index b6c4f76e38..b682d01c20 100644
--- a/drivers/net/ena/ena_rss.c
+++ b/drivers/net/ena/ena_rss.c
@@ -51,15 +51,14 @@ void ena_rss_key_fill(void *key, size_t size)
 	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)
+		for (i = 0; i < RTE_DIM(default_key); ++i)
 			default_key[i] = rte_rand() & 0xff;
 		key_generated = true;
 	}
 
-	rte_memcpy(key, default_key, size);
+	RTE_ASSERT(size <= sizeof(default_key));
+	rte_memcpy(key, default_key, RTE_MIN(size, sizeof(default_key)));
 }
 
 int ena_rss_reta_update(struct rte_eth_dev *dev,
-- 
2.36.1


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

* [PATCH 05/12] net/enetfec: fix build with GCC 12
  2022-05-18 10:16 [PATCH 00/12] Fix compilation with gcc 12 David Marchand
                   ` (3 preceding siblings ...)
  2022-05-18 10:16 ` [PATCH 04/12] net/ena: " David Marchand
@ 2022-05-18 10:16 ` David Marchand
  2022-06-10 13:08   ` David Marchand
  2022-06-11 15:35   ` Stephen Hemminger
  2022-05-18 10:16 ` [PATCH 06/12] net/ice: " David Marchand
                   ` (8 subsequent siblings)
  13 siblings, 2 replies; 73+ messages in thread
From: David Marchand @ 2022-05-18 10:16 UTC (permalink / raw)
  To: dev; +Cc: thomas, ferruh.yigit, stable, Apeksha Gupta, Sachin Saxena

GCC 12 raises the following warning:

../drivers/net/enetfec/enet_ethdev.c: In function
        ‘enetfec_rx_queue_setup’:
../drivers/net/enetfec/enet_ethdev.c:473:9: error: array
        subscript 1 is
    above array bounds of ‘uint32_t[1]’ {aka ‘unsigned int[1]’}
        [-Werror=array-bounds]
  473 | rte_write32(rte_cpu_to_le_32(fep->bd_addr_p_r[queue_idx]),
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  474 |     (uint8_t *)fep->hw_baseaddr_v + ENETFEC_RD_START(queue_idx));
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../drivers/net/enetfec/enet_ethdev.c:9:
../drivers/net/enetfec/enet_ethdev.h:113:33: note: while referencing
        ‘bd_addr_p_r’
  113 | uint32_t                bd_addr_p_r[ENETFEC_MAX_Q];
      |                                 ^~~~~~~~~~~

This driver properly announces that it only supports 1 rxq.
Silence this warning by adding an explicit check on the queue id.

Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/enetfec/enet_ethdev.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/enetfec/enet_ethdev.c b/drivers/net/enetfec/enet_ethdev.c
index 714f8ac7ec..c938e58204 100644
--- a/drivers/net/enetfec/enet_ethdev.c
+++ b/drivers/net/enetfec/enet_ethdev.c
@@ -2,9 +2,12 @@
  * Copyright 2020-2021 NXP
  */
 
+#include <inttypes.h>
+
 #include <ethdev_vdev.h>
 #include <ethdev_driver.h>
 #include <rte_io.h>
+
 #include "enet_pmd_logs.h"
 #include "enet_ethdev.h"
 #include "enet_regs.h"
@@ -454,6 +457,12 @@ enetfec_rx_queue_setup(struct rte_eth_dev *dev,
 		return -EINVAL;
 	}
 
+	if (queue_idx >= ENETFEC_MAX_Q) {
+		ENETFEC_PMD_ERR("Invalid queue id %" PRIu16 ", max %d\n",
+			queue_idx, ENETFEC_MAX_Q);
+		return -EINVAL;
+	}
+
 	/* allocate receive queue */
 	rxq = rte_zmalloc(NULL, sizeof(*rxq), RTE_CACHE_LINE_SIZE);
 	if (rxq == NULL) {
-- 
2.36.1


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

* [PATCH 06/12] net/ice: fix build with GCC 12
  2022-05-18 10:16 [PATCH 00/12] Fix compilation with gcc 12 David Marchand
                   ` (4 preceding siblings ...)
  2022-05-18 10:16 ` [PATCH 05/12] net/enetfec: " David Marchand
@ 2022-05-18 10:16 ` David Marchand
  2022-06-11 15:36   ` Stephen Hemminger
  2022-05-18 10:16 ` [PATCH 07/12] net/ice/base: " David Marchand
                   ` (7 subsequent siblings)
  13 siblings, 1 reply; 73+ messages in thread
From: David Marchand @ 2022-05-18 10:16 UTC (permalink / raw)
  To: dev; +Cc: thomas, ferruh.yigit, stable, Qiming Yang, Qi Zhang

GCC 12 raises the following warning:

In file included from ../lib/mempool/rte_mempool.h:46,
                 from ../lib/mbuf/rte_mbuf.h:38,
                 from ../lib/net/rte_ether.h:22,
                 from ../lib/ethdev/rte_ethdev.h:172,
                 from ../lib/ethdev/ethdev_driver.h:22,
                 from ../lib/ethdev/ethdev_pci.h:17,
                 from ../drivers/net/ice/ice_ethdev.c:6:
../drivers/net/ice/ice_ethdev.c: In function ‘ice_dev_configure’:
../lib/eal/x86/include/rte_memcpy.h:370:9: warning: array subscript 64 is
        outside array bounds of ‘struct ice_aqc_get_set_rss_keys[1]’
        [-Warray-bounds]
  370 | rte_mov32((uint8_t *)dst + 2 * 32, (const uint8_t *)src + 2 * 32);
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/ice/ice_ethdev.c:3202:41: note: while referencing ‘key’
 3202 |         struct ice_aqc_get_set_rss_keys key;
      |                                         ^~~

Restrict copy to minimum size.

Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/ice/ice_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 00ac2bb191..d69d480268 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -3263,7 +3263,8 @@ static int ice_init_rss(struct ice_pf *pf)
 			   RTE_MIN(rss_conf->rss_key_len,
 				   vsi->rss_key_size));
 
-	rte_memcpy(key.standard_rss_key, vsi->rss_key, vsi->rss_key_size);
+	rte_memcpy(key.standard_rss_key, vsi->rss_key,
+		RTE_MIN(sizeof(key.standard_rss_key), vsi->rss_key_size));
 	ret = ice_aq_set_rss_key(hw, vsi->idx, &key);
 	if (ret)
 		goto out;
-- 
2.36.1


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

* [PATCH 07/12] net/ice/base: fix build with GCC 12
  2022-05-18 10:16 [PATCH 00/12] Fix compilation with gcc 12 David Marchand
                   ` (5 preceding siblings ...)
  2022-05-18 10:16 ` [PATCH 06/12] net/ice: " David Marchand
@ 2022-05-18 10:16 ` David Marchand
  2022-05-18 10:16 ` [PATCH 08/12] net/qede/base: " David Marchand
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 73+ messages in thread
From: David Marchand @ 2022-05-18 10:16 UTC (permalink / raw)
  To: dev; +Cc: thomas, ferruh.yigit, stable, Qiming Yang, Qi Zhang

GCC 12 raises the following warning:

../drivers/net/ice/base/ice_switch.c: In function ‘ice_add_sw_recipe’:
../drivers/net/ice/base/ice_switch.c:7219:61: error: writing 1 byte
        into a region of size 0 [-Werror=stringop-overflow=]
 7219 |         buf[recps].content.lkup_indx[i + 1] = entry->fv_idx[i];
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
In file included from ../drivers/net/ice/base/ice_controlq.h:8,
                 from ../drivers/net/ice/base/ice_type.h:54,
                 from ../drivers/net/ice/base/ice_common.h:8,
                 from ../drivers/net/ice/base/ice_switch.h:8,
                 from ../drivers/net/ice/base/ice_switch.c:5:
../drivers/net/ice/base/ice_adminq_cmd.h:744:12: note: at offset 5
        into destination object ‘lkup_indx’ of size 5
  744 |         u8 lkup_indx[5];
      |            ^~~~~~~~~

Since this code is in the base driver, waive the check until the base
driver is fixed by the relevant people.

Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/ice/base/meson.build | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ice/base/meson.build b/drivers/net/ice/base/meson.build
index 3cf4ce05fa..89d8c5eba1 100644
--- a/drivers/net/ice/base/meson.build
+++ b/drivers/net/ice/base/meson.build
@@ -40,6 +40,11 @@ if (toolchain == 'gcc' and cc.version().version_compare('>=11.0.0'))
     error_cflags += ['-Wno-array-bounds']
 endif
 
+# FIXME
+if (toolchain == 'gcc' and cc.version().version_compare('>=12.0.0'))
+    error_cflags += ['-Wno-stringop-overflow']
+endif
+
 if is_windows and cc.get_id() != 'clang'
     cflags += ['-fno-asynchronous-unwind-tables']
 endif
-- 
2.36.1


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

* [PATCH 08/12] net/qede/base: fix build with GCC 12
  2022-05-18 10:16 [PATCH 00/12] Fix compilation with gcc 12 David Marchand
                   ` (6 preceding siblings ...)
  2022-05-18 10:16 ` [PATCH 07/12] net/ice/base: " David Marchand
@ 2022-05-18 10:16 ` David Marchand
  2022-05-20 20:29   ` Stephen Hemminger
  2022-06-21 23:17   ` Stephen Hemminger
  2022-05-18 10:16 ` [PATCH 09/12] vdpa/ifc: " David Marchand
                   ` (5 subsequent siblings)
  13 siblings, 2 replies; 73+ messages in thread
From: David Marchand @ 2022-05-18 10:16 UTC (permalink / raw)
  To: dev; +Cc: thomas, ferruh.yigit, stable, Rasesh Mody, Devendra Singh Rawat

GCC raises the following warning:

In function ‘_mm256_storeu_si256’,
    inlined from ‘rte_mov32’ at
        ../lib/eal/x86/include/rte_memcpy.h:320:2,
    inlined from ‘rte_mov128’ at
        ../lib/eal/x86/include/rte_memcpy.h:342:2,
    inlined from ‘rte_memcpy_generic’ at
        ../lib/eal/x86/include/rte_memcpy.h:438:4,
    inlined from ‘rte_memcpy’ at
        ../lib/eal/x86/include/rte_memcpy.h:882:10,
    inlined from ‘__ecore_mcp_cmd_and_union’ at
        ../drivers/net/qede/base/ecore_mcp.c:541:3,
    inlined from ‘_ecore_mcp_cmd_and_union’ at
        ../drivers/net/qede/base/ecore_mcp.c:638:2,
    inlined from ‘ecore_mcp_cmd_and_union’ at
        ../drivers/net/qede/base/ecore_mcp.c:742:9:
/usr/lib/gcc/x86_64-redhat-linux/12/include/avxintrin.h:935:8: error:
        array subscript 1 is outside array bounds of
        ‘union drv_union_data[1]’ [-Werror=array-bounds]
  935 |   *__P = __A;
      |   ~~~~~^~~~~
../drivers/net/qede/base/ecore_mcp.c: In function
        ‘ecore_mcp_cmd_and_union’:
../drivers/net/qede/base/ecore_mcp.c:533:30: note: at offset 32 into
        object ‘union_data’ of size 32
  533 |         union drv_union_data union_data;
      |                              ^~~~~~~~~~

Since this code is in the base driver, waive the check until the base
driver is fixed by the relevant people.

Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/qede/base/meson.build | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/qede/base/meson.build b/drivers/net/qede/base/meson.build
index 4ad177b478..c7b19be20a 100644
--- a/drivers/net/qede/base/meson.build
+++ b/drivers/net/qede/base/meson.build
@@ -44,6 +44,11 @@ error_cflags = [
         '-Wno-sometimes-uninitialized',
         '-Wno-pointer-bool-conversion',
 ]
+# FIXME
+if (toolchain == 'gcc' and cc.version().version_compare('>=12.0.0'))
+    error_cflags += ['-Wno-array-bounds']
+endif
+
 c_args = cflags
 foreach flag: error_cflags
         if cc.has_argument(flag)
-- 
2.36.1


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

* [PATCH 09/12] vdpa/ifc: fix build with GCC 12
  2022-05-18 10:16 [PATCH 00/12] Fix compilation with gcc 12 David Marchand
                   ` (7 preceding siblings ...)
  2022-05-18 10:16 ` [PATCH 08/12] net/qede/base: " David Marchand
@ 2022-05-18 10:16 ` David Marchand
  2022-05-18 11:48   ` Wang, Xiao W
  2022-06-11 15:36   ` Stephen Hemminger
  2022-05-18 10:16 ` [PATCH 10/12] vhost/crypto: " David Marchand
                   ` (4 subsequent siblings)
  13 siblings, 2 replies; 73+ messages in thread
From: David Marchand @ 2022-05-18 10:16 UTC (permalink / raw)
  To: dev; +Cc: thomas, ferruh.yigit, stable, Xiao Wang

GCC 12 raises the following warning:

../drivers/vdpa/ifc/ifcvf_vdpa.c: In function ‘vdpa_enable_vfio_intr’:
../drivers/vdpa/ifc/ifcvf_vdpa.c:383:62: error: writing 4 bytes into a
    region of size 0 [-Werror=stringop-overflow=]
  383 |                         fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = fd;
      |                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
../drivers/vdpa/ifc/ifcvf_vdpa.c:348:14: note: at offset 32 into
    destination object ‘irq_set_buf’ of size 32
  348 |         char irq_set_buf[MSIX_IRQ_SET_BUF_LEN];
      |              ^~~~~~~~~~~

Validate number of vrings to avoid out of bound access.

Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/vdpa/ifc/ifcvf_vdpa.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c b/drivers/vdpa/ifc/ifcvf_vdpa.c
index 9f05595b6b..6708849bd3 100644
--- a/drivers/vdpa/ifc/ifcvf_vdpa.c
+++ b/drivers/vdpa/ifc/ifcvf_vdpa.c
@@ -354,6 +354,8 @@ vdpa_enable_vfio_intr(struct ifcvf_internal *internal, bool m_rx)
 	vring.callfd = -1;
 
 	nr_vring = rte_vhost_get_vring_num(internal->vid);
+	if (nr_vring > IFCVF_MAX_QUEUES * 2)
+		return -1;
 
 	irq_set = (struct vfio_irq_set *)irq_set_buf;
 	irq_set->argsz = sizeof(irq_set_buf);
-- 
2.36.1


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

* [PATCH 10/12] vhost/crypto: fix build with GCC 12
  2022-05-18 10:16 [PATCH 00/12] Fix compilation with gcc 12 David Marchand
                   ` (8 preceding siblings ...)
  2022-05-18 10:16 ` [PATCH 09/12] vdpa/ifc: " David Marchand
@ 2022-05-18 10:16 ` David Marchand
  2022-06-02 10:08   ` Bruce Richardson
                     ` (2 more replies)
  2022-05-18 10:16 ` [PATCH 11/12] app/flow-perf: " David Marchand
                   ` (3 subsequent siblings)
  13 siblings, 3 replies; 73+ messages in thread
From: David Marchand @ 2022-05-18 10:16 UTC (permalink / raw)
  To: dev; +Cc: thomas, ferruh.yigit, stable, Maxime Coquelin, Chenbo Xia, Fan Zhang

GCC 12 raises the following warning:

In file included from ../lib/mempool/rte_mempool.h:46,
                 from ../lib/mbuf/rte_mbuf.h:38,
                 from ../lib/vhost/vhost_crypto.c:7:
../lib/vhost/vhost_crypto.c: In function ‘rte_vhost_crypto_fetch_requests’:
../lib/eal/x86/include/rte_memcpy.h:371:9: warning: array subscript 1 is
     outside array bounds of ‘struct virtio_crypto_op_data_req[1]’
     [-Warray-bounds]
  371 | rte_mov32((uint8_t *)dst + 3 * 32, (const uint8_t *)src + 3 * 32);
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../lib/vhost/vhost_crypto.c:1178:42: note: while referencing ‘req’
 1178 |         struct virtio_crypto_op_data_req req;
      |                                          ^~~

Check that copied length is within req boundaries.

Fixes: 3c79609fda7c ("vhost/crypto: handle virtually non-contiguous buffers")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/vhost/vhost_crypto.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/vhost/vhost_crypto.c b/lib/vhost/vhost_crypto.c
index b1c0eb6a0f..83325b7042 100644
--- a/lib/vhost/vhost_crypto.c
+++ b/lib/vhost/vhost_crypto.c
@@ -576,16 +576,16 @@ copy_data(void *dst_data, struct vhost_crypto_data_req *vc_req,
 	uint32_t to_copy;
 	uint8_t *data = dst_data;
 	uint8_t *src;
-	int left = size;
+	uint32_t left = size;
 
-	to_copy = RTE_MIN(desc->len, (uint32_t)left);
+	to_copy = RTE_MIN(desc->len, left);
 	dlen = to_copy;
 	src = IOVA_TO_VVA(uint8_t *, vc_req, desc->addr, &dlen,
 			VHOST_ACCESS_RO);
-	if (unlikely(!src || !dlen))
+	if (unlikely(!src || !dlen || dlen > left))
 		return -1;
 
-	rte_memcpy((uint8_t *)data, src, dlen);
+	rte_memcpy(data, src, dlen);
 	data += dlen;
 
 	if (unlikely(dlen < to_copy)) {
-- 
2.36.1


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

* [PATCH 11/12] app/flow-perf: fix build with GCC 12
  2022-05-18 10:16 [PATCH 00/12] Fix compilation with gcc 12 David Marchand
                   ` (9 preceding siblings ...)
  2022-05-18 10:16 ` [PATCH 10/12] vhost/crypto: " David Marchand
@ 2022-05-18 10:16 ` David Marchand
  2022-06-02 10:21   ` Bruce Richardson
                     ` (2 more replies)
  2022-05-18 10:16 ` [PATCH 12/12] test/ipsec: " David Marchand
                   ` (2 subsequent siblings)
  13 siblings, 3 replies; 73+ messages in thread
From: David Marchand @ 2022-05-18 10:16 UTC (permalink / raw)
  To: dev; +Cc: thomas, ferruh.yigit, stable, Wisam Jaddo

GCC 12 raises the following warning:

../app/test-flow-perf/main.c: In function ‘start_forwarding’:
../app/test-flow-perf/main.c:1737:28: error: ‘sprintf’ may write a
    terminating nul past the end of the destination
    [-Werror=format-overflow=]
 1737 |         sprintf(p[i++], "%d", (int)n);
      |                            ^
In function ‘pretty_number’,
    inlined from ‘packet_per_second_stats’ at
        ../app/test-flow-perf/main.c:1792:4,
    inlined from ‘start_forwarding’ at
        ../app/test-flow-perf/main.c:1831:3:
[...]

We can simplify this code and rely on libc integer formatting via
this system locales.

Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test-flow-perf/main.c | 48 ++++++++-------------------------------
 1 file changed, 9 insertions(+), 39 deletions(-)

diff --git a/app/test-flow-perf/main.c b/app/test-flow-perf/main.c
index 56d43734e3..3922e92ded 100644
--- a/app/test-flow-perf/main.c
+++ b/app/test-flow-perf/main.c
@@ -16,6 +16,7 @@
  * gives packet per second measurement.
  */
 
+#include <locale.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -1713,36 +1714,6 @@ do_tx(struct lcore_info *li, uint16_t cnt, uint16_t tx_port,
 		rte_pktmbuf_free(li->pkts[i]);
 }
 
-/*
- * Method to convert numbers into pretty numbers that easy
- * to read. The design here is to add comma after each three
- * digits and set all of this inside buffer.
- *
- * For example if n = 1799321, the output will be
- * 1,799,321 after this method which is easier to read.
- */
-static char *
-pretty_number(uint64_t n, char *buf)
-{
-	char p[6][4];
-	int i = 0;
-	int off = 0;
-
-	while (n > 1000) {
-		sprintf(p[i], "%03d", (int)(n % 1000));
-		n /= 1000;
-		i += 1;
-	}
-
-	sprintf(p[i++], "%d", (int)n);
-
-	while (i--)
-		off += sprintf(buf + off, "%s,", p[i]);
-	buf[strlen(buf) - 1] = '\0';
-
-	return buf;
-}
-
 static void
 packet_per_second_stats(void)
 {
@@ -1764,7 +1735,6 @@ packet_per_second_stats(void)
 		uint64_t total_rx_pkts = 0;
 		uint64_t total_tx_drops = 0;
 		uint64_t tx_delta, rx_delta, drops_delta;
-		char buf[3][32];
 		int nr_valid_core = 0;
 
 		sleep(1);
@@ -1789,10 +1759,8 @@ packet_per_second_stats(void)
 			tx_delta    = li->tx_pkts  - oli->tx_pkts;
 			rx_delta    = li->rx_pkts  - oli->rx_pkts;
 			drops_delta = li->tx_drops - oli->tx_drops;
-			printf("%6d %16s %16s %16s\n", i,
-				pretty_number(tx_delta,    buf[0]),
-				pretty_number(drops_delta, buf[1]),
-				pretty_number(rx_delta,    buf[2]));
+			printf("%6d %'16.3"PRId64" %'16.3"PRId64" %'16.3"PRId64"\n",
+				i, tx_delta, drops_delta, rx_delta);
 
 			total_tx_pkts  += tx_delta;
 			total_rx_pkts  += rx_delta;
@@ -1803,10 +1771,9 @@ packet_per_second_stats(void)
 		}
 
 		if (nr_valid_core > 1) {
-			printf("%6s %16s %16s %16s\n", "total",
-				pretty_number(total_tx_pkts,  buf[0]),
-				pretty_number(total_tx_drops, buf[1]),
-				pretty_number(total_rx_pkts,  buf[2]));
+			printf("%6s %'16.3"PRId64" %'16.3"PRId64" %'16.3"PRId64"\n",
+				"total", total_tx_pkts, total_tx_drops,
+				total_rx_pkts);
 			nr_lines += 1;
 		}
 
@@ -2139,6 +2106,9 @@ main(int argc, char **argv)
 	if (argc > 1)
 		args_parse(argc, argv);
 
+	/* For more fancy, localised integer formatting. */
+	setlocale(LC_NUMERIC, "");
+
 	init_port();
 
 	nb_lcores = rte_lcore_count();
-- 
2.36.1


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

* [PATCH 12/12] test/ipsec: fix build with GCC 12
  2022-05-18 10:16 [PATCH 00/12] Fix compilation with gcc 12 David Marchand
                   ` (10 preceding siblings ...)
  2022-05-18 10:16 ` [PATCH 11/12] app/flow-perf: " David Marchand
@ 2022-05-18 10:16 ` David Marchand
  2022-06-02 18:41   ` Medvedkin, Vladimir
                     ` (3 more replies)
  2022-05-20 20:13 ` [PATCH 00/12] Fix compilation with gcc 12 Stephen Hemminger
  2022-06-15  8:49 ` David Marchand
  13 siblings, 4 replies; 73+ messages in thread
From: David Marchand @ 2022-05-18 10:16 UTC (permalink / raw)
  To: dev
  Cc: thomas, ferruh.yigit, stable, Konstantin Ananyev,
	Bernard Iremonger, Vladimir Medvedkin

GCC 12 raises the following warning:

In function ‘_mm256_loadu_si256’,
    inlined from ‘rte_mov32’ at
        ../lib/eal/x86/include/rte_memcpy.h:319:9,
    inlined from ‘rte_mov128’ at
        ../lib/eal/x86/include/rte_memcpy.h:344:2,
    inlined from ‘rte_memcpy_generic’ at
        ../lib/eal/x86/include/rte_memcpy.h:438:4,
    inlined from ‘rte_memcpy’ at
        ../lib/eal/x86/include/rte_memcpy.h:882:10,
    inlined from ‘setup_test_string.constprop’ at
        ../app/test/test_ipsec.c:572:4:
/usr/lib/gcc/x86_64-redhat-linux/12/include/avxintrin.h:929:10: error:
    array subscript ‘__m256i_u[3]’ is partly outside array bounds of
    ‘const char[108]’ [-Werror=array-bounds]
  929 |   return *__P;
      |          ^~~~
../app/test/test_ipsec.c: In function ‘setup_test_string.constprop’:
../app/test/test_ipsec.c:539:12: note: at offset 96 into object
    ‘null_plain_data’ of size 108
  539 | const char null_plain_data[] =
      |            ^~~~~~~~~~~~~~~

Split copy request into copies of string lengths and remove unused
blocksize.

Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/test_ipsec.c | 48 ++++++++++++++++++++++++++-----------------
 1 file changed, 29 insertions(+), 19 deletions(-)

diff --git a/app/test/test_ipsec.c b/app/test/test_ipsec.c
index 8da025bf66..d7455fd021 100644
--- a/app/test/test_ipsec.c
+++ b/app/test/test_ipsec.c
@@ -554,24 +554,28 @@ struct rte_ipv4_hdr ipv4_outer  = {
 };
 
 static struct rte_mbuf *
-setup_test_string(struct rte_mempool *mpool,
-		const char *string, size_t len, uint8_t blocksize)
+setup_test_string(struct rte_mempool *mpool, const char *string,
+	size_t string_len, size_t len)
 {
 	struct rte_mbuf *m = rte_pktmbuf_alloc(mpool);
-	size_t t_len = len - (blocksize ? (len % blocksize) : 0);
 
 	if (m) {
 		memset(m->buf_addr, 0, m->buf_len);
-		char *dst = rte_pktmbuf_append(m, t_len);
+		char *dst = rte_pktmbuf_append(m, len);
 
 		if (!dst) {
 			rte_pktmbuf_free(m);
 			return NULL;
 		}
-		if (string != NULL)
-			rte_memcpy(dst, string, t_len);
-		else
-			memset(dst, 0, t_len);
+		if (string != NULL) {
+			size_t off;
+
+			for (off = 0; off + string_len < len; off += string_len)
+				rte_memcpy(&dst[off], string, string_len);
+			rte_memcpy(&dst[off], string, len % string_len);
+		} else {
+			memset(dst, 0, len);
+		}
 	}
 
 	return m;
@@ -1365,7 +1369,8 @@ test_ipsec_crypto_outb_burst_null_null(int i)
 	/* Generate input mbuf data */
 	for (j = 0; j < num_pkts && rc == 0; j++) {
 		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
-			null_plain_data, test_cfg[i].pkt_sz, 0);
+			null_plain_data, sizeof(null_plain_data),
+			test_cfg[i].pkt_sz);
 		if (ut_params->ibuf[j] == NULL)
 			rc = TEST_FAILED;
 		else {
@@ -1483,7 +1488,8 @@ test_ipsec_inline_crypto_inb_burst_null_null(int i)
 			/* Generate test mbuf data */
 			ut_params->obuf[j] = setup_test_string(
 				ts_params->mbuf_pool,
-				null_plain_data, test_cfg[i].pkt_sz, 0);
+				null_plain_data, sizeof(null_plain_data),
+				test_cfg[i].pkt_sz);
 			if (ut_params->obuf[j] == NULL)
 				rc = TEST_FAILED;
 		}
@@ -1551,16 +1557,17 @@ test_ipsec_inline_proto_inb_burst_null_null(int i)
 
 	/* Generate inbound mbuf data */
 	for (j = 0; j < num_pkts && rc == 0; j++) {
-		ut_params->ibuf[j] = setup_test_string(
-			ts_params->mbuf_pool,
-			null_plain_data, test_cfg[i].pkt_sz, 0);
+		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
+			null_plain_data, sizeof(null_plain_data),
+			test_cfg[i].pkt_sz);
 		if (ut_params->ibuf[j] == NULL)
 			rc = TEST_FAILED;
 		else {
 			/* Generate test mbuf data */
 			ut_params->obuf[j] = setup_test_string(
 				ts_params->mbuf_pool,
-				null_plain_data, test_cfg[i].pkt_sz, 0);
+				null_plain_data, sizeof(null_plain_data),
+				test_cfg[i].pkt_sz);
 			if (ut_params->obuf[j] == NULL)
 				rc = TEST_FAILED;
 		}
@@ -1660,7 +1667,8 @@ test_ipsec_inline_crypto_outb_burst_null_null(int i)
 	/* Generate test mbuf data */
 	for (j = 0; j < num_pkts && rc == 0; j++) {
 		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
-			null_plain_data, test_cfg[i].pkt_sz, 0);
+			null_plain_data, sizeof(null_plain_data),
+			test_cfg[i].pkt_sz);
 		if (ut_params->ibuf[0] == NULL)
 			rc = TEST_FAILED;
 
@@ -1738,15 +1746,16 @@ test_ipsec_inline_proto_outb_burst_null_null(int i)
 	/* Generate test mbuf data */
 	for (j = 0; j < num_pkts && rc == 0; j++) {
 		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
-			null_plain_data, test_cfg[i].pkt_sz, 0);
+			null_plain_data, sizeof(null_plain_data),
+			test_cfg[i].pkt_sz);
 		if (ut_params->ibuf[0] == NULL)
 			rc = TEST_FAILED;
 
 		if (rc == 0) {
 			/* Generate test tunneled mbuf data for comparison */
 			ut_params->obuf[j] = setup_test_string(
-					ts_params->mbuf_pool,
-					null_plain_data, test_cfg[i].pkt_sz, 0);
+				ts_params->mbuf_pool, null_plain_data,
+				sizeof(null_plain_data), test_cfg[i].pkt_sz);
 			if (ut_params->obuf[j] == NULL)
 				rc = TEST_FAILED;
 		}
@@ -1815,7 +1824,8 @@ test_ipsec_lksd_proto_inb_burst_null_null(int i)
 	for (j = 0; j < num_pkts && rc == 0; j++) {
 		/* packet with sequence number 0 is invalid */
 		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
-			null_encrypted_data, test_cfg[i].pkt_sz, 0);
+			null_encrypted_data, sizeof(null_encrypted_data),
+			 test_cfg[i].pkt_sz);
 		if (ut_params->ibuf[j] == NULL)
 			rc = TEST_FAILED;
 	}
-- 
2.36.1


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

* RE: [PATCH 09/12] vdpa/ifc: fix build with GCC 12
  2022-05-18 10:16 ` [PATCH 09/12] vdpa/ifc: " David Marchand
@ 2022-05-18 11:48   ` Wang, Xiao W
  2022-06-11 15:36   ` Stephen Hemminger
  1 sibling, 0 replies; 73+ messages in thread
From: Wang, Xiao W @ 2022-05-18 11:48 UTC (permalink / raw)
  To: David Marchand, dev; +Cc: thomas, ferruh.yigit, stable

Hi,

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Wednesday, May 18, 2022 6:17 PM
> To: dev@dpdk.org
> Cc: thomas@monjalon.net; ferruh.yigit@xilinx.com; stable@dpdk.org;
> Wang, Xiao W <xiao.w.wang@intel.com>
> Subject: [PATCH 09/12] vdpa/ifc: fix build with GCC 12
> 
> GCC 12 raises the following warning:
> 
> ../drivers/vdpa/ifc/ifcvf_vdpa.c: In function ‘vdpa_enable_vfio_intr’:
> ../drivers/vdpa/ifc/ifcvf_vdpa.c:383:62: error: writing 4 bytes into a
>     region of size 0 [-Werror=stringop-overflow=]
>   383 |                         fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = fd;
>       |                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
> ../drivers/vdpa/ifc/ifcvf_vdpa.c:348:14: note: at offset 32 into
>     destination object ‘irq_set_buf’ of size 32
>   348 |         char irq_set_buf[MSIX_IRQ_SET_BUF_LEN];
>       |              ^~~~~~~~~~~
> 
> Validate number of vrings to avoid out of bound access.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  drivers/vdpa/ifc/ifcvf_vdpa.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c b/drivers/vdpa/ifc/ifcvf_vdpa.c
> index 9f05595b6b..6708849bd3 100644
> --- a/drivers/vdpa/ifc/ifcvf_vdpa.c
> +++ b/drivers/vdpa/ifc/ifcvf_vdpa.c
> @@ -354,6 +354,8 @@ vdpa_enable_vfio_intr(struct ifcvf_internal *internal,
> bool m_rx)
>  	vring.callfd = -1;
> 
>  	nr_vring = rte_vhost_get_vring_num(internal->vid);
> +	if (nr_vring > IFCVF_MAX_QUEUES * 2)
> +		return -1;
> 
>  	irq_set = (struct vfio_irq_set *)irq_set_buf;
>  	irq_set->argsz = sizeof(irq_set_buf);
> --
> 2.36.1

Acked-by: Xiao Wang <xiao.w.wang@intel.com>

BRs,
Xiao

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

* Re: [PATCH 00/12] Fix compilation with gcc 12
  2022-05-18 10:16 [PATCH 00/12] Fix compilation with gcc 12 David Marchand
                   ` (11 preceding siblings ...)
  2022-05-18 10:16 ` [PATCH 12/12] test/ipsec: " David Marchand
@ 2022-05-20 20:13 ` Stephen Hemminger
  2022-05-21  9:39   ` Morten Brørup
  2022-06-15  8:49 ` David Marchand
  13 siblings, 1 reply; 73+ messages in thread
From: Stephen Hemminger @ 2022-05-20 20:13 UTC (permalink / raw)
  To: David Marchand; +Cc: dev, thomas, ferruh.yigit

On Wed, 18 May 2022 12:16:45 +0200
David Marchand <david.marchand@redhat.com> wrote:

> Fedora 36 is out since early may and comes with gcc 12.
> This series fixes compilation or waives some checks.
> 
> There might be something fishy with rte_memcpy on x86 but, for now,
> the rte_memcpy related fixes are on the caller side.
> 
> Some "base" drivers have issues, I chose the simple solution of waiving
> the checks for them.
> 
> Compilation is the only thing checked.
> Please driver maintainers, check nothing got broken.
> 


We need to purge all code still using array size of one
instead of proper flex array member.


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

* Re: [PATCH 01/12] common/cpt: fix build with GCC 12
  2022-05-18 10:16 ` [PATCH 01/12] common/cpt: fix build with GCC 12 David Marchand
@ 2022-05-20 20:23   ` Stephen Hemminger
  2022-06-10 13:11   ` David Marchand
  1 sibling, 0 replies; 73+ messages in thread
From: Stephen Hemminger @ 2022-05-20 20:23 UTC (permalink / raw)
  To: David Marchand; +Cc: dev, thomas, ferruh.yigit, stable, Anoob Joseph

On Wed, 18 May 2022 12:16:46 +0200
David Marchand <david.marchand@redhat.com> wrote:

> GCC 12 raises the following warning:
> 
> In function ‘fill_sg_comp_from_iov’,
>     inlined from ‘cpt_kasumi_enc_prep’ at
>         ../drivers/common/cpt/cpt_ucode.h:2176:8,
>     inlined from ‘cpt_fc_enc_hmac_prep’ at
>         ../drivers/common/cpt/cpt_ucode.h:2475:3,
>     inlined from ‘fill_digest_params’ at
>         ../drivers/common/cpt/cpt_ucode.h:3548:14,
>     inlined from ‘otx_cpt_enq_single_sym’ at
>         ../drivers/crypto/octeontx/otx_cryptodev_ops.c:541:9,
>     inlined from ‘otx_cpt_enq_single_sym_sessless’ at
>         ../drivers/crypto/octeontx/otx_cryptodev_ops.c:584:8,
>     inlined from ‘otx_cpt_enq_single’ at
>         ../drivers/crypto/octeontx/otx_cryptodev_ops.c:611:11,
>     inlined from ‘otx_cpt_pkt_enqueue’ at
>         ../drivers/crypto/octeontx/otx_cryptodev_ops.c:643:9,
>     inlined from ‘otx_cpt_enqueue_sym’ at
>         ../drivers/crypto/octeontx/otx_cryptodev_ops.c:668:9:
> ../drivers/common/cpt/cpt_ucode.h:415:36: error: array subscript 0 is
>         outside array bounds of ‘buf_ptr_t[0]’ {aka ‘struct buf_ptr[]’}
>         [-Werror=array-bounds]
>   415 |                         e_dma_addr = bufs[j].dma_addr;
>       |                         ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
> ../drivers/common/cpt/cpt_ucode.h:416:48: error: array subscript 0 is
>         outside array bounds of ‘buf_ptr_t[0]’ {aka ‘struct buf_ptr[]’}
>         [-Werror=array-bounds]
>   416 |                         e_len = (size > bufs[j].size) ?
>       |                                         ~~~~~~~^~~~~
> 
> For now, waive this warning until we have a proper fix.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
 
NAK
Please fix properly, with something like:

--- a/drivers/common/cpt/cpt_mcode_defines.h
+++ b/drivers/common/cpt/cpt_mcode_defines.h
@@ -387,7 +387,7 @@ typedef struct buf_ptr {
 /* IOV Pointer */
 typedef struct{
        int buf_cnt;
-       buf_ptr_t bufs[0];
+       buf_ptr_t bufs[];
 } iov_ptr_t;
 
 typedef struct fc_params {




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

* Re: [PATCH 02/12] crypto/cnxk: fix build with GCC 12
  2022-05-18 10:16 ` [PATCH 02/12] crypto/cnxk: " David Marchand
@ 2022-05-20 20:24   ` Stephen Hemminger
  0 siblings, 0 replies; 73+ messages in thread
From: Stephen Hemminger @ 2022-05-20 20:24 UTC (permalink / raw)
  To: David Marchand
  Cc: dev, thomas, ferruh.yigit, stable, Ankur Dwivedi, Anoob Joseph,
	Tejasree Kondoj

On Wed, 18 May 2022 12:16:47 +0200
David Marchand <david.marchand@redhat.com> wrote:

> GCC 12 raises the following warning:
> 
> In file included from ../drivers/crypto/cnxk/cn10k_cryptodev_ops.c:17:
> In function ‘fill_sg_comp_from_iov’,
>     inlined from ‘cpt_kasumi_enc_prep’ at
>         ../drivers/crypto/cnxk/cnxk_se.h:1413:8,
>     inlined from ‘cpt_fc_enc_hmac_prep’ at
>         ../drivers/crypto/cnxk/cnxk_se.h:1635:9,
>     inlined from ‘fill_digest_params’ at
>         ../drivers/crypto/cnxk/cnxk_se.h:2524:8,
>     inlined from ‘cpt_sym_inst_fill’ at
>         ../drivers/crypto/cnxk/cn10k_cryptodev_ops.c:92:9,
>     inlined from ‘cn10k_cpt_fill_inst.constprop.isra’ at
>         ../drivers/crypto/cnxk/cn10k_cryptodev_ops.c:146:10:
> ../drivers/crypto/cnxk/cnxk_se.h:208:52: error: array subscript 0 is
>         outside array bounds of ‘struct roc_se_buf_ptr[0]’
>         [-Werror=array-bounds]
>   208 |             e_vaddr = (uint64_t)bufs[j].vaddr;
>       |                                 ~~~~~~~^~~~~~
> ../drivers/crypto/cnxk/cnxk_se.h:209:48: error: array subscript 0 is
>         outside array bounds of ‘struct roc_se_buf_ptr[0]’
>         [-Werror=array-bounds]
>   209 |             e_len = (size > bufs[j].size) ? bufs[j].size : size;
>       |                             ~~~~~~~^~~~~
> 
> For now, waive this warning until we have a proper fix.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

If you fix iov_ptr_t to be flexible array this won't be needed.

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

* Re: [PATCH 04/12] net/ena: fix build with GCC 12
  2022-05-18 10:16 ` [PATCH 04/12] net/ena: " David Marchand
@ 2022-05-20 20:28   ` Stephen Hemminger
  2022-05-21  9:49     ` Morten Brørup
  2022-06-11 15:34   ` Stephen Hemminger
  1 sibling, 1 reply; 73+ messages in thread
From: Stephen Hemminger @ 2022-05-20 20:28 UTC (permalink / raw)
  To: David Marchand
  Cc: dev, thomas, ferruh.yigit, stable, Marcin Wojtas,
	Michal Krawczyk, Shai Brandes, Evgeny Schemeilin, Igor Chauskin

On Wed, 18 May 2022 12:16:49 +0200
David Marchand <david.marchand@redhat.com> wrote:

> +		for (i = 0; i < RTE_DIM(default_key); ++i)
>  			default_key[i] = rte_rand() & 0xff;

We should have rte_random_bytes() functionality if this gets
used often.

Also, worth considering dropping DPDK random number generator
in userspace for security reasons and just using more secure kernel code.

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

* Re: [PATCH 08/12] net/qede/base: fix build with GCC 12
  2022-05-18 10:16 ` [PATCH 08/12] net/qede/base: " David Marchand
@ 2022-05-20 20:29   ` Stephen Hemminger
  2022-06-21 23:17   ` Stephen Hemminger
  1 sibling, 0 replies; 73+ messages in thread
From: Stephen Hemminger @ 2022-05-20 20:29 UTC (permalink / raw)
  To: David Marchand
  Cc: dev, thomas, ferruh.yigit, stable, Rasesh Mody, Devendra Singh Rawat

On Wed, 18 May 2022 12:16:53 +0200
David Marchand <david.marchand@redhat.com> wrote:

> GCC raises the following warning:
> 
> In function ‘_mm256_storeu_si256’,
>     inlined from ‘rte_mov32’ at
>         ../lib/eal/x86/include/rte_memcpy.h:320:2,
>     inlined from ‘rte_mov128’ at
>         ../lib/eal/x86/include/rte_memcpy.h:342:2,
>     inlined from ‘rte_memcpy_generic’ at
>         ../lib/eal/x86/include/rte_memcpy.h:438:4,
>     inlined from ‘rte_memcpy’ at
>         ../lib/eal/x86/include/rte_memcpy.h:882:10,
>     inlined from ‘__ecore_mcp_cmd_and_union’ at
>         ../drivers/net/qede/base/ecore_mcp.c:541:3,
>     inlined from ‘_ecore_mcp_cmd_and_union’ at
>         ../drivers/net/qede/base/ecore_mcp.c:638:2,
>     inlined from ‘ecore_mcp_cmd_and_union’ at
>         ../drivers/net/qede/base/ecore_mcp.c:742:9:
> /usr/lib/gcc/x86_64-redhat-linux/12/include/avxintrin.h:935:8: error:
>         array subscript 1 is outside array bounds of
>         ‘union drv_union_data[1]’ [-Werror=array-bounds]
>   935 |   *__P = __A;
>       |   ~~~~~^~~~~
> ../drivers/net/qede/base/ecore_mcp.c: In function
>         ‘ecore_mcp_cmd_and_union’:
> ../drivers/net/qede/base/ecore_mcp.c:533:30: note: at offset 32 into
>         object ‘union_data’ of size 32
>   533 |         union drv_union_data union_data;
>       |                              ^~~~~~~~~~
> 
> Since this code is in the base driver, waive the check until the base
> driver is fixed by the relevant people.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Or mark driver broken with gcc-12 and get the maintainer to fix?


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

* RE: [PATCH 00/12] Fix compilation with gcc 12
  2022-05-20 20:13 ` [PATCH 00/12] Fix compilation with gcc 12 Stephen Hemminger
@ 2022-05-21  9:39   ` Morten Brørup
  0 siblings, 0 replies; 73+ messages in thread
From: Morten Brørup @ 2022-05-21  9:39 UTC (permalink / raw)
  To: Stephen Hemminger, David Marchand; +Cc: dev, thomas, ferruh.yigit

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Friday, 20 May 2022 22.14
> 
> On Wed, 18 May 2022 12:16:45 +0200
> David Marchand <david.marchand@redhat.com> wrote:
> 
> > Fedora 36 is out since early may and comes with gcc 12.
> > This series fixes compilation or waives some checks.
> >
> > There might be something fishy with rte_memcpy on x86 but, for now,
> > the rte_memcpy related fixes are on the caller side.
> >
> > Some "base" drivers have issues, I chose the simple solution of
> waiving
> > the checks for them.
> >
> > Compilation is the only thing checked.
> > Please driver maintainers, check nothing got broken.
> >
> 
> 
> We need to purge all code still using array size of one
> instead of proper flex array member.

+1 to that!


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

* RE: [PATCH 04/12] net/ena: fix build with GCC 12
  2022-05-20 20:28   ` Stephen Hemminger
@ 2022-05-21  9:49     ` Morten Brørup
  2022-05-21 16:23       ` Stephen Hemminger
  0 siblings, 1 reply; 73+ messages in thread
From: Morten Brørup @ 2022-05-21  9:49 UTC (permalink / raw)
  To: Stephen Hemminger, David Marchand
  Cc: dev, thomas, ferruh.yigit, stable, Marcin Wojtas,
	Michal Krawczyk, Shai Brandes, Evgeny Schemeilin, Igor Chauskin

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Friday, 20 May 2022 22.28
> 
> On Wed, 18 May 2022 12:16:49 +0200
> David Marchand <david.marchand@redhat.com> wrote:
> 
> > +		for (i = 0; i < RTE_DIM(default_key); ++i)
> >  			default_key[i] = rte_rand() & 0xff;
> 
> We should have rte_random_bytes() functionality if this gets
> used often.

Since the other pseudorandom functions are called rand, such a function should be named rte_rand_bytes().

> 
> Also, worth considering dropping DPDK random number generator
> in userspace for security reasons and just using more secure kernel
> code.

Absolutely not! We need a fast pseudorandom number generator in DPDK.

If anything, we could consider renaming the functions and header file to reflect that they are pseudorandom number generators, and not (cryptographically) random generators. That would cause an API/ABI breakage, so it's probably not going to happen. ;-)



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

* Re: [PATCH 04/12] net/ena: fix build with GCC 12
  2022-05-21  9:49     ` Morten Brørup
@ 2022-05-21 16:23       ` Stephen Hemminger
  2022-05-22 20:30         ` Morten Brørup
  0 siblings, 1 reply; 73+ messages in thread
From: Stephen Hemminger @ 2022-05-21 16:23 UTC (permalink / raw)
  To: Morten Brørup
  Cc: David Marchand, dev, thomas, ferruh.yigit, stable, Marcin Wojtas,
	Michal Krawczyk, Shai Brandes, Evgeny Schemeilin, Igor Chauskin

On Sat, 21 May 2022 11:49:47 +0200
Morten Brørup <mb@smartsharesystems.com> wrote:

> > 
> > Also, worth considering dropping DPDK random number generator
> > in userspace for security reasons and just using more secure kernel
> > code.  
> 
> Absolutely not! We need a fast pseudorandom number generator in DPDK.
> 
> If anything, we could consider renaming the functions and header file to reflect that they are pseudorandom number generators, and not (cryptographically) random generators. That would cause an API/ABI breakage, so it's probably not going to happen. ;-)


The Linux kernel has received an way more attention on random numbers than
DPDK. If you follow the history, what happens is that a simple dumb LCG
or similar random number generator gets invented, and then gets used for
lots of things that people don't think need a strong generator.

Followed by DoS and other attacks where the weak random number generator
is broken when used for doing things like creating sequence numbers of
TCP port assignment.  This is then followed by even more work on the
kernel random number generator to make the default random number generator
stronger.

I bring up this history, so that DPDK won't have to repeat it.

Right now the DPDK random number generator is insecure because it uses
long but weak PRNG and never reseeds itself.

See:
https://lwn.net/Articles/884875/

There is also FIPS to consider.
https://lwn.net/Articles/877607/

Since random number generators are hard, prefer that someone else do it :-)

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

* RE: [PATCH 04/12] net/ena: fix build with GCC 12
  2022-05-21 16:23       ` Stephen Hemminger
@ 2022-05-22 20:30         ` Morten Brørup
  0 siblings, 0 replies; 73+ messages in thread
From: Morten Brørup @ 2022-05-22 20:30 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: David Marchand, dev, thomas, ferruh.yigit, stable, Marcin Wojtas,
	Michal Krawczyk, Shai Brandes, Evgeny Schemeilin, Igor Chauskin

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Saturday, 21 May 2022 18.24
> 
> On Sat, 21 May 2022 11:49:47 +0200
> Morten Brørup <mb@smartsharesystems.com> wrote:
> 
> > >
> > > Also, worth considering dropping DPDK random number generator
> > > in userspace for security reasons and just using more secure kernel
> > > code.
> >
> > Absolutely not! We need a fast pseudorandom number generator in DPDK.
> >
> > If anything, we could consider renaming the functions and header file
> to reflect that they are pseudorandom number generators, and not
> (cryptographically) random generators. That would cause an API/ABI
> breakage, so it's probably not going to happen. ;-)
> 
> 
> The Linux kernel has received an way more attention on random numbers
> than
> DPDK. If you follow the history, what happens is that a simple dumb LCG
> or similar random number generator gets invented, and then gets used
> for
> lots of things that people don't think need a strong generator.
> 
> Followed by DoS and other attacks where the weak random number
> generator
> is broken when used for doing things like creating sequence numbers of
> TCP port assignment.  This is then followed by even more work on the
> kernel random number generator to make the default random number
> generator
> stronger.
> 
> I bring up this history, so that DPDK won't have to repeat it.
> 
> Right now the DPDK random number generator is insecure because it uses
> long but weak PRNG and never reseeds itself.
> 
> See:
> https://lwn.net/Articles/884875/
> 
> There is also FIPS to consider.
> https://lwn.net/Articles/877607/
> 
> Since random number generators are hard, prefer that someone else do it
> :-)

First of all, I would like to thank you for the history lesson and references, Stephen, it made my Saturday evening much more nerdy and interesting than expected! Not being a native English speaker, please understand that I mean this sincerely. I really enjoyed reading about this corner of the Linux kernel history.

Overall, I think that RNGs generally fall into two categories: Unsafe (regardless how advanced) and safe for crypto use.

It should be OK for DPDK to provide something blazing fast, but unsafe. The DPDK documentation clearly states that the provided random functions are not safe for crypto, so I would expect the developers to use them accordingly.

Having thought about it, I came to this conclusion: Regardless if we provide unsafe RNG functions in DPDK or not, it is ultimately up to the application developers to choose which RNG category to use for different purposes. If we don't provide something fast, developers will just use the standard rand48() functions or similar. And a blazing fast (but unsafe) RNG is useful for simple things like pseudo-random packet sampling in the data plane.

Who would have thought that using a simple RNG for TCP port assignment could end up being a security problem... The developers will always have a choice between secure and fast, and the risk of a developer making the wrong decision is not affected by DPDK providing some unsafe RNG or not.

At a higher level, I come to think of the RFCs, which all have a Security Considerations chapter. Ideally, all patches had such a chapter, and all reviews considered the security aspects, so someone would catch the use of an unsafe RNG where a safe RNG should be used. Removing the rand() functions from DPDK will not have the desired effect, only raising security awareness will.

And just to leave off where you left off: I 100 % agree that we should not try to invent our own crypto safe RNG!

PS: I assume that safe RNGs cannot generate numbers at the same rate as unsafe RNGs. If this was not generally true, there should be no need to use unsafe RNGs (except for test purposes, where reproducibility is a requirement).

In cases where the safe RNG can generate numbers at a sufficiently high rate, why not use it? This, however, requires that the application developer knows both the required rate and the rate of the safe RNG, which I guess very few developers do.


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

* Re: [PATCH 03/12] crypto/ipsec_mb: fix build with GCC 12
  2022-05-18 10:16 ` [PATCH 03/12] crypto/ipsec_mb: " David Marchand
@ 2022-06-02  9:50   ` Bruce Richardson
  2022-06-10 13:07     ` David Marchand
  2022-06-11 15:34   ` Stephen Hemminger
  1 sibling, 1 reply; 73+ messages in thread
From: Bruce Richardson @ 2022-06-02  9:50 UTC (permalink / raw)
  To: David Marchand
  Cc: dev, thomas, ferruh.yigit, stable, Fan Zhang, Pablo de Lara,
	Deepak Kumar Jain

On Wed, May 18, 2022 at 12:16:48PM +0200, David Marchand wrote:
> GCC 12 raises the following warning:
> 
> In function ‘__rte_ring_enqueue_elems_64’,
>     inlined from ‘__rte_ring_enqueue_elems’ at
>         ../lib/ring/rte_ring_elem_pvt.h:130:3,
>     inlined from ‘__rte_ring_do_hts_enqueue_elem’ at
>         ../lib/ring/rte_ring_hts_elem_pvt.h:196:3,
>     inlined from ‘rte_ring_mp_hts_enqueue_burst_elem’ at
>         ../lib/ring/rte_ring_hts.h:110:9,
>     inlined from ‘rte_ring_enqueue_burst_elem’ at
>         ../lib/ring/rte_ring_elem.h:577:10,
>     inlined from ‘rte_ring_enqueue_burst’ at
>         ../lib/ring/rte_ring.h:738:9,
>     inlined from ‘process_op_bit’ at
>         ../drivers/crypto/ipsec_mb/pmd_snow3g.c:425:16,
>     inlined from ‘snow3g_pmd_dequeue_burst’ at
>         ../drivers/crypto/ipsec_mb/pmd_snow3g.c:484:20:
> ../lib/ring/rte_ring_elem_pvt.h:68:44: error: array subscript 1 is
>         outside array bounds of ‘struct rte_crypto_op[0]’
>         [-Werror=array-bounds]
>    68 |                         ring[idx + 1] = obj[i + 1];
>       |                                         ~~~^~~~~~~
> ../drivers/crypto/ipsec_mb/pmd_snow3g.c: In function
>         ‘snow3g_pmd_dequeue_burst’:
> ../drivers/crypto/ipsec_mb/pmd_snow3g.c:434:1: note:
>         at offset 8 into object ‘op’ of size 8
>   434 | snow3g_pmd_dequeue_burst(void *queue_pair,
>       | ^~~~~~~~~~~~~~~~~~~~~~~~
> 
> Validate that one (exactly) op has been processed or return early.
> 
> Fixes: b537abdbee74 ("crypto/snow3g: support bit-level operations")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  drivers/crypto/ipsec_mb/pmd_snow3g.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/crypto/ipsec_mb/pmd_snow3g.c b/drivers/crypto/ipsec_mb/pmd_snow3g.c
> index ebc9a0b562..9a85f46721 100644
> --- a/drivers/crypto/ipsec_mb/pmd_snow3g.c
> +++ b/drivers/crypto/ipsec_mb/pmd_snow3g.c
> @@ -422,12 +422,13 @@ process_op_bit(struct rte_crypto_op *op, struct snow3g_session *session,
>  		op->sym->session = NULL;
>  	}
>  
> -	enqueued_op = rte_ring_enqueue_burst(qp->ingress_queue,
> -			(void **)&op, processed_op, NULL);
> +	if (unlikely(processed_op != 1))
> +		return 0;
> +	enqueued_op = rte_ring_enqueue(qp->ingress_queue, op);

As a fix for the compiler warning this looks ok, but question for
maintainer would be - should this check for processed_op != 1 not go
earlier in the function, immediately after the switch statement?

Fan, Pablo, can you please comment?

/Bruce

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

* Re: [PATCH 10/12] vhost/crypto: fix build with GCC 12
  2022-05-18 10:16 ` [PATCH 10/12] vhost/crypto: " David Marchand
@ 2022-06-02 10:08   ` Bruce Richardson
  2022-06-14  9:22     ` David Marchand
  2022-06-11 15:36   ` Stephen Hemminger
  2022-06-16  9:32   ` [PATCH v2] " David Marchand
  2 siblings, 1 reply; 73+ messages in thread
From: Bruce Richardson @ 2022-06-02 10:08 UTC (permalink / raw)
  To: David Marchand
  Cc: dev, thomas, ferruh.yigit, stable, Maxime Coquelin, Chenbo Xia,
	Fan Zhang

On Wed, May 18, 2022 at 12:16:55PM +0200, David Marchand wrote:
> GCC 12 raises the following warning:
> 
> In file included from ../lib/mempool/rte_mempool.h:46,
>                  from ../lib/mbuf/rte_mbuf.h:38,
>                  from ../lib/vhost/vhost_crypto.c:7:
> ../lib/vhost/vhost_crypto.c: In function ‘rte_vhost_crypto_fetch_requests’:
> ../lib/eal/x86/include/rte_memcpy.h:371:9: warning: array subscript 1 is
>      outside array bounds of ‘struct virtio_crypto_op_data_req[1]’
>      [-Warray-bounds]
>   371 | rte_mov32((uint8_t *)dst + 3 * 32, (const uint8_t *)src + 3 * 32);
>       | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../lib/vhost/vhost_crypto.c:1178:42: note: while referencing ‘req’
>  1178 |         struct virtio_crypto_op_data_req req;
>       |                                          ^~~
> 
> Check that copied length is within req boundaries.
> 
> Fixes: 3c79609fda7c ("vhost/crypto: handle virtually non-contiguous buffers")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  lib/vhost/vhost_crypto.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/vhost/vhost_crypto.c b/lib/vhost/vhost_crypto.c
> index b1c0eb6a0f..83325b7042 100644
> --- a/lib/vhost/vhost_crypto.c
> +++ b/lib/vhost/vhost_crypto.c
> @@ -576,16 +576,16 @@ copy_data(void *dst_data, struct vhost_crypto_data_req *vc_req,
>  	uint32_t to_copy;
>  	uint8_t *data = dst_data;
>  	uint8_t *src;
> -	int left = size;
> +	uint32_t left = size;
>  
> -	to_copy = RTE_MIN(desc->len, (uint32_t)left);
> +	to_copy = RTE_MIN(desc->len, left);
>  	dlen = to_copy;
>  	src = IOVA_TO_VVA(uint8_t *, vc_req, desc->addr, &dlen,
>  			VHOST_ACCESS_RO);

Tracking the functions which end up being called by this macro, the dlen
parameter ends up being of type "uint64_t *", passing a value of int * or
uint32_t * seems wrong to me. If we are changing the type from int to
uint32_t, I think it should be promoted all the way to uint64_t.

> -	if (unlikely(!src || !dlen))
> +	if (unlikely(!src || !dlen || dlen > left))
>  		return -1;
>  

If this change is omitted, does the compiler still give warnings. Looking
through the called code, the dlen parameter can only ever be reduced, not
incremented (function rte_vhost_va_from_guest_pa() in rte_vhost.h).

> -	rte_memcpy((uint8_t *)data, src, dlen);
> +	rte_memcpy(data, src, dlen);
>  	data += dlen;
>  
>  	if (unlikely(dlen < to_copy)) {
> -- 
> 2.36.1
> 

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

* Re: [PATCH 11/12] app/flow-perf: fix build with GCC 12
  2022-05-18 10:16 ` [PATCH 11/12] app/flow-perf: " David Marchand
@ 2022-06-02 10:21   ` Bruce Richardson
  2022-06-08  9:03   ` Wisam Monther
  2022-06-11 15:37   ` Stephen Hemminger
  2 siblings, 0 replies; 73+ messages in thread
From: Bruce Richardson @ 2022-06-02 10:21 UTC (permalink / raw)
  To: David Marchand; +Cc: dev, thomas, ferruh.yigit, stable, Wisam Jaddo

On Wed, May 18, 2022 at 12:16:56PM +0200, David Marchand wrote:
> GCC 12 raises the following warning:
> 
> ../app/test-flow-perf/main.c: In function ‘start_forwarding’:
> ../app/test-flow-perf/main.c:1737:28: error: ‘sprintf’ may write a
>     terminating nul past the end of the destination
>     [-Werror=format-overflow=]
>  1737 |         sprintf(p[i++], "%d", (int)n);
>       |                            ^
> In function ‘pretty_number’,
>     inlined from ‘packet_per_second_stats’ at
>         ../app/test-flow-perf/main.c:1792:4,
>     inlined from ‘start_forwarding’ at
>         ../app/test-flow-perf/main.c:1831:3:
> [...]
> 
> We can simplify this code and rely on libc integer formatting via
> this system locales.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Good idea.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

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

* Re: [PATCH 12/12] test/ipsec: fix build with GCC 12
  2022-05-18 10:16 ` [PATCH 12/12] test/ipsec: " David Marchand
@ 2022-06-02 18:41   ` Medvedkin, Vladimir
  2022-06-03  7:45     ` David Marchand
  2022-06-11 15:38   ` Stephen Hemminger
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 73+ messages in thread
From: Medvedkin, Vladimir @ 2022-06-02 18:41 UTC (permalink / raw)
  To: David Marchand, dev
  Cc: thomas, ferruh.yigit, stable, Konstantin Ananyev, Bernard Iremonger

Hi David,

On 18/05/2022 11:16, David Marchand wrote:
> GCC 12 raises the following warning:
> 
> In function ‘_mm256_loadu_si256’,
>      inlined from ‘rte_mov32’ at
>          ../lib/eal/x86/include/rte_memcpy.h:319:9,
>      inlined from ‘rte_mov128’ at
>          ../lib/eal/x86/include/rte_memcpy.h:344:2,
>      inlined from ‘rte_memcpy_generic’ at
>          ../lib/eal/x86/include/rte_memcpy.h:438:4,
>      inlined from ‘rte_memcpy’ at
>          ../lib/eal/x86/include/rte_memcpy.h:882:10,
>      inlined from ‘setup_test_string.constprop’ at
>          ../app/test/test_ipsec.c:572:4:
> /usr/lib/gcc/x86_64-redhat-linux/12/include/avxintrin.h:929:10: error:
>      array subscript ‘__m256i_u[3]’ is partly outside array bounds of
>      ‘const char[108]’ [-Werror=array-bounds]
>    929 |   return *__P;
>        |          ^~~~
> ../app/test/test_ipsec.c: In function ‘setup_test_string.constprop’:
> ../app/test/test_ipsec.c:539:12: note: at offset 96 into object
>      ‘null_plain_data’ of size 108
>    539 | const char null_plain_data[] =
>        |            ^~~~~~~~~~~~~~~
> 
> Split copy request into copies of string lengths and remove unused
> blocksize.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>   app/test/test_ipsec.c | 48 ++++++++++++++++++++++++++-----------------
>   1 file changed, 29 insertions(+), 19 deletions(-)
> 
> diff --git a/app/test/test_ipsec.c b/app/test/test_ipsec.c
> index 8da025bf66..d7455fd021 100644
> --- a/app/test/test_ipsec.c
> +++ b/app/test/test_ipsec.c
> @@ -554,24 +554,28 @@ struct rte_ipv4_hdr ipv4_outer  = {
>   };
>   
>   static struct rte_mbuf *
> -setup_test_string(struct rte_mempool *mpool,
> -		const char *string, size_t len, uint8_t blocksize)
> +setup_test_string(struct rte_mempool *mpool, const char *string,
> +	size_t string_len, size_t len)
>   {
>   	struct rte_mbuf *m = rte_pktmbuf_alloc(mpool);
> -	size_t t_len = len - (blocksize ? (len % blocksize) : 0);
>   
>   	if (m) {
>   		memset(m->buf_addr, 0, m->buf_len);
> -		char *dst = rte_pktmbuf_append(m, t_len);
> +		char *dst = rte_pktmbuf_append(m, len);
>   
>   		if (!dst) {
>   			rte_pktmbuf_free(m);
>   			return NULL;
>   		}
> -		if (string != NULL)
> -			rte_memcpy(dst, string, t_len);
> -		else
> -			memset(dst, 0, t_len);
> +		if (string != NULL) {
> +			size_t off;
> +
> +			for (off = 0; off + string_len < len; off += string_len)

I think it should be off + string_len <= len here, because otherwise, if 
len is a multiple of string_len, the last ret_memcpy (after this loop) 
will copy 0 bytes.

> +				rte_memcpy(&dst[off], string, string_len);
> +			rte_memcpy(&dst[off], string, len % string_len);
> +		} else {
> +			memset(dst, 0, len);
> +		}
>   	}
>   
>   	return m;
> @@ -1365,7 +1369,8 @@ test_ipsec_crypto_outb_burst_null_null(int i)
>   	/* Generate input mbuf data */
>   	for (j = 0; j < num_pkts && rc == 0; j++) {
>   		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
> -			null_plain_data, test_cfg[i].pkt_sz, 0);
> +			null_plain_data, sizeof(null_plain_data),
> +			test_cfg[i].pkt_sz);
>   		if (ut_params->ibuf[j] == NULL)
>   			rc = TEST_FAILED;
>   		else {
> @@ -1483,7 +1488,8 @@ test_ipsec_inline_crypto_inb_burst_null_null(int i)
>   			/* Generate test mbuf data */
>   			ut_params->obuf[j] = setup_test_string(
>   				ts_params->mbuf_pool,
> -				null_plain_data, test_cfg[i].pkt_sz, 0);
> +				null_plain_data, sizeof(null_plain_data),
> +				test_cfg[i].pkt_sz);
>   			if (ut_params->obuf[j] == NULL)
>   				rc = TEST_FAILED;
>   		}
> @@ -1551,16 +1557,17 @@ test_ipsec_inline_proto_inb_burst_null_null(int i)
>   
>   	/* Generate inbound mbuf data */
>   	for (j = 0; j < num_pkts && rc == 0; j++) {
> -		ut_params->ibuf[j] = setup_test_string(
> -			ts_params->mbuf_pool,
> -			null_plain_data, test_cfg[i].pkt_sz, 0);
> +		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
> +			null_plain_data, sizeof(null_plain_data),
> +			test_cfg[i].pkt_sz);
>   		if (ut_params->ibuf[j] == NULL)
>   			rc = TEST_FAILED;
>   		else {
>   			/* Generate test mbuf data */
>   			ut_params->obuf[j] = setup_test_string(
>   				ts_params->mbuf_pool,
> -				null_plain_data, test_cfg[i].pkt_sz, 0);
> +				null_plain_data, sizeof(null_plain_data),
> +				test_cfg[i].pkt_sz);
>   			if (ut_params->obuf[j] == NULL)
>   				rc = TEST_FAILED;
>   		}
> @@ -1660,7 +1667,8 @@ test_ipsec_inline_crypto_outb_burst_null_null(int i)
>   	/* Generate test mbuf data */
>   	for (j = 0; j < num_pkts && rc == 0; j++) {
>   		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
> -			null_plain_data, test_cfg[i].pkt_sz, 0);
> +			null_plain_data, sizeof(null_plain_data),
> +			test_cfg[i].pkt_sz);
>   		if (ut_params->ibuf[0] == NULL)
>   			rc = TEST_FAILED;
>   
> @@ -1738,15 +1746,16 @@ test_ipsec_inline_proto_outb_burst_null_null(int i)
>   	/* Generate test mbuf data */
>   	for (j = 0; j < num_pkts && rc == 0; j++) {
>   		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
> -			null_plain_data, test_cfg[i].pkt_sz, 0);
> +			null_plain_data, sizeof(null_plain_data),
> +			test_cfg[i].pkt_sz);
>   		if (ut_params->ibuf[0] == NULL)
>   			rc = TEST_FAILED;
>   
>   		if (rc == 0) {
>   			/* Generate test tunneled mbuf data for comparison */
>   			ut_params->obuf[j] = setup_test_string(
> -					ts_params->mbuf_pool,
> -					null_plain_data, test_cfg[i].pkt_sz, 0);
> +				ts_params->mbuf_pool, null_plain_data,
> +				sizeof(null_plain_data), test_cfg[i].pkt_sz);
>   			if (ut_params->obuf[j] == NULL)
>   				rc = TEST_FAILED;
>   		}
> @@ -1815,7 +1824,8 @@ test_ipsec_lksd_proto_inb_burst_null_null(int i)
>   	for (j = 0; j < num_pkts && rc == 0; j++) {
>   		/* packet with sequence number 0 is invalid */
>   		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
> -			null_encrypted_data, test_cfg[i].pkt_sz, 0);
> +			null_encrypted_data, sizeof(null_encrypted_data),
> +			 test_cfg[i].pkt_sz);
>   		if (ut_params->ibuf[j] == NULL)
>   			rc = TEST_FAILED;
>   	}

-- 
Regards,
Vladimir

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

* Re: [PATCH 12/12] test/ipsec: fix build with GCC 12
  2022-06-02 18:41   ` Medvedkin, Vladimir
@ 2022-06-03  7:45     ` David Marchand
  2022-06-03  7:56       ` Bruce Richardson
  0 siblings, 1 reply; 73+ messages in thread
From: David Marchand @ 2022-06-03  7:45 UTC (permalink / raw)
  To: Medvedkin, Vladimir
  Cc: dev, Thomas Monjalon, Ferruh Yigit, dpdk stable,
	Konstantin Ananyev, Bernard Iremonger

Hello Vladimir,

On Thu, Jun 2, 2022 at 8:42 PM Medvedkin, Vladimir
<vladimir.medvedkin@intel.com> wrote:
> >               if (!dst) {
> >                       rte_pktmbuf_free(m);
> >                       return NULL;
> >               }
> > -             if (string != NULL)
> > -                     rte_memcpy(dst, string, t_len);
> > -             else
> > -                     memset(dst, 0, t_len);
> > +             if (string != NULL) {
> > +                     size_t off;
> > +
> > +                     for (off = 0; off + string_len < len; off += string_len)
>
> I think it should be off + string_len <= len here, because otherwise, if
> len is a multiple of string_len, the last ret_memcpy (after this loop)
> will copy 0 bytes.

Changing to off + string_len <= len would trigger an oob access to dst
(by one extra byte)?
Otoh, I don't think it is an issue to have a 0-length call to rte_memcpy.

>
> > +                             rte_memcpy(&dst[off], string, string_len);
> > +                     rte_memcpy(&dst[off], string, len % string_len);


-- 
David Marchand


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

* Re: [PATCH 12/12] test/ipsec: fix build with GCC 12
  2022-06-03  7:45     ` David Marchand
@ 2022-06-03  7:56       ` Bruce Richardson
  2022-06-03  9:41         ` David Marchand
  0 siblings, 1 reply; 73+ messages in thread
From: Bruce Richardson @ 2022-06-03  7:56 UTC (permalink / raw)
  To: David Marchand
  Cc: Medvedkin, Vladimir, dev, Thomas Monjalon, Ferruh Yigit,
	dpdk stable, Konstantin Ananyev, Bernard Iremonger

On Fri, Jun 03, 2022 at 09:45:45AM +0200, David Marchand wrote:
> Hello Vladimir,
> 
> On Thu, Jun 2, 2022 at 8:42 PM Medvedkin, Vladimir
> <vladimir.medvedkin@intel.com> wrote:
> > >               if (!dst) {
> > >                       rte_pktmbuf_free(m);
> > >                       return NULL;
> > >               }
> > > -             if (string != NULL)
> > > -                     rte_memcpy(dst, string, t_len);
> > > -             else
> > > -                     memset(dst, 0, t_len);
> > > +             if (string != NULL) {
> > > +                     size_t off;
> > > +
> > > +                     for (off = 0; off + string_len < len; off += string_len)
> >
> > I think it should be off + string_len <= len here, because otherwise, if
> > len is a multiple of string_len, the last ret_memcpy (after this loop)
> > will copy 0 bytes.
> 
> Changing to off + string_len <= len would trigger an oob access to dst
> (by one extra byte)?
> Otoh, I don't think it is an issue to have a 0-length call to rte_memcpy.
> 
Given this is test code, do we need rte_memcpy for performance over regular
libc memcpy? Does fixing the warning become any easier or clearer if libc
memcpy is used?

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

* Re: [PATCH 12/12] test/ipsec: fix build with GCC 12
  2022-06-03  7:56       ` Bruce Richardson
@ 2022-06-03  9:41         ` David Marchand
  2022-06-03 15:57           ` Medvedkin, Vladimir
  0 siblings, 1 reply; 73+ messages in thread
From: David Marchand @ 2022-06-03  9:41 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: Medvedkin, Vladimir, dev, Thomas Monjalon, Ferruh Yigit,
	dpdk stable, Konstantin Ananyev, Bernard Iremonger

On Fri, Jun 3, 2022 at 9:56 AM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Fri, Jun 03, 2022 at 09:45:45AM +0200, David Marchand wrote:
> > Hello Vladimir,
> >
> > On Thu, Jun 2, 2022 at 8:42 PM Medvedkin, Vladimir
> > <vladimir.medvedkin@intel.com> wrote:
> > > >               if (!dst) {
> > > >                       rte_pktmbuf_free(m);
> > > >                       return NULL;
> > > >               }
> > > > -             if (string != NULL)
> > > > -                     rte_memcpy(dst, string, t_len);
> > > > -             else
> > > > -                     memset(dst, 0, t_len);
> > > > +             if (string != NULL) {
> > > > +                     size_t off;
> > > > +
> > > > +                     for (off = 0; off + string_len < len; off += string_len)
> > >
> > > I think it should be off + string_len <= len here, because otherwise, if
> > > len is a multiple of string_len, the last ret_memcpy (after this loop)
> > > will copy 0 bytes.
> >
> > Changing to off + string_len <= len would trigger an oob access to dst
> > (by one extra byte)?
> > Otoh, I don't think it is an issue to have a 0-length call to rte_memcpy.
> >
> Given this is test code, do we need rte_memcpy for performance over regular
> libc memcpy? Does fixing the warning become any easier or clearer if libc
> memcpy is used?

There was a similar proposal in vhost/crypto code.
I am not a fan to switching to libc memcpy.
We would be waiving a potential issue in rte_memcpy itself (which
could also be a problem in how gcc understands this inlined code) or
in the rte_memcpy caller code.

Here, gcc is probably too picky.
No path currently leads to oob access on the src string.

Adding a simple hint (see simplified hunk below) seems to help gcc enough:

@@ -554,12 +554,14 @@ struct rte_ipv4_hdr ipv4_outer  = {
 };

 static struct rte_mbuf *
-setup_test_string(struct rte_mempool *mpool,
-        const char *string, size_t len, uint8_t blocksize)
+setup_test_string(struct rte_mempool *mpool, const char *string,
+    size_t string_len, size_t len, uint8_t blocksize)
 {
     struct rte_mbuf *m = rte_pktmbuf_alloc(mpool);
     size_t t_len = len - (blocksize ? (len % blocksize) : 0);

+    RTE_VERIFY(len <= string_len);
+

     if (m) {
         memset(m->buf_addr, 0, m->buf_len);


-- 
David Marchand


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

* Re: [PATCH 12/12] test/ipsec: fix build with GCC 12
  2022-06-03  9:41         ` David Marchand
@ 2022-06-03 15:57           ` Medvedkin, Vladimir
  0 siblings, 0 replies; 73+ messages in thread
From: Medvedkin, Vladimir @ 2022-06-03 15:57 UTC (permalink / raw)
  To: David Marchand, Bruce Richardson
  Cc: dev, Thomas Monjalon, Ferruh Yigit, dpdk stable,
	Konstantin Ananyev, Bernard Iremonger

Hi David,

On 03/06/2022 10:41, David Marchand wrote:
> On Fri, Jun 3, 2022 at 9:56 AM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
>>
>> On Fri, Jun 03, 2022 at 09:45:45AM +0200, David Marchand wrote:
>>> Hello Vladimir,
>>>
>>> On Thu, Jun 2, 2022 at 8:42 PM Medvedkin, Vladimir
>>> <vladimir.medvedkin@intel.com> wrote:
>>>>>                if (!dst) {
>>>>>                        rte_pktmbuf_free(m);
>>>>>                        return NULL;
>>>>>                }
>>>>> -             if (string != NULL)
>>>>> -                     rte_memcpy(dst, string, t_len);
>>>>> -             else
>>>>> -                     memset(dst, 0, t_len);
>>>>> +             if (string != NULL) {
>>>>> +                     size_t off;
>>>>> +
>>>>> +                     for (off = 0; off + string_len < len; off += string_len)
>>>>
>>>> I think it should be off + string_len <= len here, because otherwise, if
>>>> len is a multiple of string_len, the last ret_memcpy (after this loop)
>>>> will copy 0 bytes.
>>>
>>> Changing to off + string_len <= len would trigger an oob access to dst
>>> (by one extra byte)?
>>> Otoh, I don't think it is an issue to have a 0-length call to rte_memcpy.
>>>

The problem here is that if, for example, string_len is 8 bytes and len 
is 16, then it will write only 8 bytes.

>> Given this is test code, do we need rte_memcpy for performance over regular
>> libc memcpy? Does fixing the warning become any easier or clearer if libc
>> memcpy is used?
> 
> There was a similar proposal in vhost/crypto code.
> I am not a fan to switching to libc memcpy.
> We would be waiving a potential issue in rte_memcpy itself (which
> could also be a problem in how gcc understands this inlined code) or
> in the rte_memcpy caller code.
> 
> Here, gcc is probably too picky.
> No path currently leads to oob access on the src string.
> 
> Adding a simple hint (see simplified hunk below) seems to help gcc enough:
> 
> @@ -554,12 +554,14 @@ struct rte_ipv4_hdr ipv4_outer  = {
>   };
> 
>   static struct rte_mbuf *
> -setup_test_string(struct rte_mempool *mpool,
> -        const char *string, size_t len, uint8_t blocksize)
> +setup_test_string(struct rte_mempool *mpool, const char *string,
> +    size_t string_len, size_t len, uint8_t blocksize)
>   {
>       struct rte_mbuf *m = rte_pktmbuf_alloc(mpool);
>       size_t t_len = len - (blocksize ? (len % blocksize) : 0);
> 
> +    RTE_VERIFY(len <= string_len);
> +

RTE_VERIFY looks better here to make picky GCC happy.

> 
>       if (m) {
>           memset(m->buf_addr, 0, m->buf_len);
> 
> 

-- 
Regards,
Vladimir

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

* RE: [PATCH 11/12] app/flow-perf: fix build with GCC 12
  2022-05-18 10:16 ` [PATCH 11/12] app/flow-perf: " David Marchand
  2022-06-02 10:21   ` Bruce Richardson
@ 2022-06-08  9:03   ` Wisam Monther
  2022-06-08 12:20     ` David Marchand
  2022-06-11 15:37   ` Stephen Hemminger
  2 siblings, 1 reply; 73+ messages in thread
From: Wisam Monther @ 2022-06-08  9:03 UTC (permalink / raw)
  To: David Marchand, dev
  Cc: NBU-Contact-Thomas Monjalon (EXTERNAL), ferruh.yigit, stable

Hi David,

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Wednesday, May 18, 2022 1:17 PM
> To: dev@dpdk.org
> Cc: NBU-Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>;
> ferruh.yigit@xilinx.com; stable@dpdk.org; Wisam Monther
> <wisamm@nvidia.com>
> Subject: [PATCH 11/12] app/flow-perf: fix build with GCC 12
> 
> GCC 12 raises the following warning:
> 
> ../app/test-flow-perf/main.c: In function ‘start_forwarding’:
> ../app/test-flow-perf/main.c:1737:28: error: ‘sprintf’ may write a
>     terminating nul past the end of the destination
>     [-Werror=format-overflow=]
>  1737 |         sprintf(p[i++], "%d", (int)n);
>       |                            ^
> In function ‘pretty_number’,
>     inlined from ‘packet_per_second_stats’ at
>         ../app/test-flow-perf/main.c:1792:4,
>     inlined from ‘start_forwarding’ at
>         ../app/test-flow-perf/main.c:1831:3:
> [...]
> 
> We can simplify this code and rely on libc integer formatting via this system
> locales.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

I've tested the patch and reviewed it, it's working fine, so thank you for that.
One comment
The initial value of 0 is 000

Example:
CMD: ./dpdk-test-flow-perf -n 4 -a <PCI> -- ingress --group=1 --ether --queue --rules-count=200000 --enable-fwd
  core               tx         tx drops               rx
------ ---------------- ---------------- ----------------
     1              000              000              000

Can you handle this to be single 0 instead of not needed leading zeros? 

BRs,
Wisam Jaddo

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

* Re: [PATCH 11/12] app/flow-perf: fix build with GCC 12
  2022-06-08  9:03   ` Wisam Monther
@ 2022-06-08 12:20     ` David Marchand
  2022-06-13  7:49       ` Wisam Monther
  0 siblings, 1 reply; 73+ messages in thread
From: David Marchand @ 2022-06-08 12:20 UTC (permalink / raw)
  To: Wisam Monther
  Cc: dev, NBU-Contact-Thomas Monjalon (EXTERNAL), ferruh.yigit, stable

On Wed, Jun 8, 2022 at 11:03 AM Wisam Monther <wisamm@nvidia.com> wrote:
>
> Hi David,
>
> > -----Original Message-----
> > From: David Marchand <david.marchand@redhat.com>
> > Sent: Wednesday, May 18, 2022 1:17 PM
> > To: dev@dpdk.org
> > Cc: NBU-Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>;
> > ferruh.yigit@xilinx.com; stable@dpdk.org; Wisam Monther
> > <wisamm@nvidia.com>
> > Subject: [PATCH 11/12] app/flow-perf: fix build with GCC 12
> >
> > GCC 12 raises the following warning:
> >
> > ../app/test-flow-perf/main.c: In function ‘start_forwarding’:
> > ../app/test-flow-perf/main.c:1737:28: error: ‘sprintf’ may write a
> >     terminating nul past the end of the destination
> >     [-Werror=format-overflow=]
> >  1737 |         sprintf(p[i++], "%d", (int)n);
> >       |                            ^
> > In function ‘pretty_number’,
> >     inlined from ‘packet_per_second_stats’ at
> >         ../app/test-flow-perf/main.c:1792:4,
> >     inlined from ‘start_forwarding’ at
> >         ../app/test-flow-perf/main.c:1831:3:
> > [...]
> >
> > We can simplify this code and rely on libc integer formatting via this system
> > locales.
> >
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
>
> I've tested the patch and reviewed it, it's working fine, so thank you for that.
> One comment
> The initial value of 0 is 000
>
> Example:
> CMD: ./dpdk-test-flow-perf -n 4 -a <PCI> -- ingress --group=1 --ether --queue --rules-count=200000 --enable-fwd
>   core               tx         tx drops               rx
> ------ ---------------- ---------------- ----------------
>      1              000              000              000
>
> Can you handle this to be single 0 instead of not needed leading zeros?

Hum, I don't remember why I added this precision...
This should be just a matter of changing the format from %'16.3s to
%'16s, can you confirm?

-- 
David Marchand


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

* Re: [PATCH 03/12] crypto/ipsec_mb: fix build with GCC 12
  2022-06-02  9:50   ` Bruce Richardson
@ 2022-06-10 13:07     ` David Marchand
  0 siblings, 0 replies; 73+ messages in thread
From: David Marchand @ 2022-06-10 13:07 UTC (permalink / raw)
  To: Fan Zhang, Pablo de Lara
  Cc: dev, Thomas Monjalon, Ferruh Yigit, dpdk stable,
	Deepak Kumar Jain, Bruce Richardson

On Thu, Jun 2, 2022 at 11:50 AM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Wed, May 18, 2022 at 12:16:48PM +0200, David Marchand wrote:
> > GCC 12 raises the following warning:
> >
> > In function ‘__rte_ring_enqueue_elems_64’,
> >     inlined from ‘__rte_ring_enqueue_elems’ at
> >         ../lib/ring/rte_ring_elem_pvt.h:130:3,
> >     inlined from ‘__rte_ring_do_hts_enqueue_elem’ at
> >         ../lib/ring/rte_ring_hts_elem_pvt.h:196:3,
> >     inlined from ‘rte_ring_mp_hts_enqueue_burst_elem’ at
> >         ../lib/ring/rte_ring_hts.h:110:9,
> >     inlined from ‘rte_ring_enqueue_burst_elem’ at
> >         ../lib/ring/rte_ring_elem.h:577:10,
> >     inlined from ‘rte_ring_enqueue_burst’ at
> >         ../lib/ring/rte_ring.h:738:9,
> >     inlined from ‘process_op_bit’ at
> >         ../drivers/crypto/ipsec_mb/pmd_snow3g.c:425:16,
> >     inlined from ‘snow3g_pmd_dequeue_burst’ at
> >         ../drivers/crypto/ipsec_mb/pmd_snow3g.c:484:20:
> > ../lib/ring/rte_ring_elem_pvt.h:68:44: error: array subscript 1 is
> >         outside array bounds of ‘struct rte_crypto_op[0]’
> >         [-Werror=array-bounds]
> >    68 |                         ring[idx + 1] = obj[i + 1];
> >       |                                         ~~~^~~~~~~
> > ../drivers/crypto/ipsec_mb/pmd_snow3g.c: In function
> >         ‘snow3g_pmd_dequeue_burst’:
> > ../drivers/crypto/ipsec_mb/pmd_snow3g.c:434:1: note:
> >         at offset 8 into object ‘op’ of size 8
> >   434 | snow3g_pmd_dequeue_burst(void *queue_pair,
> >       | ^~~~~~~~~~~~~~~~~~~~~~~~
> >
> > Validate that one (exactly) op has been processed or return early.
> >
> > Fixes: b537abdbee74 ("crypto/snow3g: support bit-level operations")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> >  drivers/crypto/ipsec_mb/pmd_snow3g.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/crypto/ipsec_mb/pmd_snow3g.c b/drivers/crypto/ipsec_mb/pmd_snow3g.c
> > index ebc9a0b562..9a85f46721 100644
> > --- a/drivers/crypto/ipsec_mb/pmd_snow3g.c
> > +++ b/drivers/crypto/ipsec_mb/pmd_snow3g.c
> > @@ -422,12 +422,13 @@ process_op_bit(struct rte_crypto_op *op, struct snow3g_session *session,
> >               op->sym->session = NULL;
> >       }
> >
> > -     enqueued_op = rte_ring_enqueue_burst(qp->ingress_queue,
> > -                     (void **)&op, processed_op, NULL);
> > +     if (unlikely(processed_op != 1))
> > +             return 0;
> > +     enqueued_op = rte_ring_enqueue(qp->ingress_queue, op);
>
> As a fix for the compiler warning this looks ok, but question for
> maintainer would be - should this check for processed_op != 1 not go
> earlier in the function, immediately after the switch statement?
>
> Fan, Pablo, can you please comment?

Fan? Pablo?


-- 
David Marchand


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

* Re: [PATCH 05/12] net/enetfec: fix build with GCC 12
  2022-05-18 10:16 ` [PATCH 05/12] net/enetfec: " David Marchand
@ 2022-06-10 13:08   ` David Marchand
  2022-06-13  5:22     ` Sachin Saxena (OSS)
  2022-06-14  8:16     ` Sachin Saxena (OSS)
  2022-06-11 15:35   ` Stephen Hemminger
  1 sibling, 2 replies; 73+ messages in thread
From: David Marchand @ 2022-06-10 13:08 UTC (permalink / raw)
  To: Apeksha Gupta, Sachin Saxena
  Cc: Thomas Monjalon, Ferruh Yigit, dpdk stable, dev

On Wed, May 18, 2022 at 12:17 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> GCC 12 raises the following warning:
>
> ../drivers/net/enetfec/enet_ethdev.c: In function
>         ‘enetfec_rx_queue_setup’:
> ../drivers/net/enetfec/enet_ethdev.c:473:9: error: array
>         subscript 1 is
>     above array bounds of ‘uint32_t[1]’ {aka ‘unsigned int[1]’}
>         [-Werror=array-bounds]
>   473 | rte_write32(rte_cpu_to_le_32(fep->bd_addr_p_r[queue_idx]),
>       | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   474 |     (uint8_t *)fep->hw_baseaddr_v + ENETFEC_RD_START(queue_idx));
>       |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from ../drivers/net/enetfec/enet_ethdev.c:9:
> ../drivers/net/enetfec/enet_ethdev.h:113:33: note: while referencing
>         ‘bd_addr_p_r’
>   113 | uint32_t                bd_addr_p_r[ENETFEC_MAX_Q];
>       |                                 ^~~~~~~~~~~
>
> This driver properly announces that it only supports 1 rxq.
> Silence this warning by adding an explicit check on the queue id.
>
> Cc: stable@dpdk.org
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Any comment from driver maintainers?
Thanks.


-- 
David Marchand


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

* Re: [PATCH 01/12] common/cpt: fix build with GCC 12
  2022-05-18 10:16 ` [PATCH 01/12] common/cpt: fix build with GCC 12 David Marchand
  2022-05-20 20:23   ` Stephen Hemminger
@ 2022-06-10 13:11   ` David Marchand
  2022-06-13 11:40     ` [EXT] " Ankur Dwivedi
  1 sibling, 1 reply; 73+ messages in thread
From: David Marchand @ 2022-06-10 13:11 UTC (permalink / raw)
  To: Anoob Joseph, Ankur Dwivedi
  Cc: Thomas Monjalon, Ferruh Yigit, dpdk stable, dev, Akhil Goyal,
	Jerin Jacob Kollanukkaran

Hello maintainers,

On Wed, May 18, 2022 at 12:17 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> GCC 12 raises the following warning:
>
> In function ‘fill_sg_comp_from_iov’,
>     inlined from ‘cpt_kasumi_enc_prep’ at
>         ../drivers/common/cpt/cpt_ucode.h:2176:8,
>     inlined from ‘cpt_fc_enc_hmac_prep’ at
>         ../drivers/common/cpt/cpt_ucode.h:2475:3,
>     inlined from ‘fill_digest_params’ at
>         ../drivers/common/cpt/cpt_ucode.h:3548:14,
>     inlined from ‘otx_cpt_enq_single_sym’ at
>         ../drivers/crypto/octeontx/otx_cryptodev_ops.c:541:9,
>     inlined from ‘otx_cpt_enq_single_sym_sessless’ at
>         ../drivers/crypto/octeontx/otx_cryptodev_ops.c:584:8,
>     inlined from ‘otx_cpt_enq_single’ at
>         ../drivers/crypto/octeontx/otx_cryptodev_ops.c:611:11,
>     inlined from ‘otx_cpt_pkt_enqueue’ at
>         ../drivers/crypto/octeontx/otx_cryptodev_ops.c:643:9,
>     inlined from ‘otx_cpt_enqueue_sym’ at
>         ../drivers/crypto/octeontx/otx_cryptodev_ops.c:668:9:
> ../drivers/common/cpt/cpt_ucode.h:415:36: error: array subscript 0 is
>         outside array bounds of ‘buf_ptr_t[0]’ {aka ‘struct buf_ptr[]’}
>         [-Werror=array-bounds]
>   415 |                         e_dma_addr = bufs[j].dma_addr;
>       |                         ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
> ../drivers/common/cpt/cpt_ucode.h:416:48: error: array subscript 0 is
>         outside array bounds of ‘buf_ptr_t[0]’ {aka ‘struct buf_ptr[]’}
>         [-Werror=array-bounds]
>   416 |                         e_len = (size > bufs[j].size) ?
>       |                                         ~~~~~~~^~~~~
>
> For now, waive this warning until we have a proper fix.

Both common/cpt and crypto/cnxk have the same code that triggers this warning.
Can you look into this please?

Thanks.

-- 
David Marchand


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

* Re: [PATCH 03/12] crypto/ipsec_mb: fix build with GCC 12
  2022-05-18 10:16 ` [PATCH 03/12] crypto/ipsec_mb: " David Marchand
  2022-06-02  9:50   ` Bruce Richardson
@ 2022-06-11 15:34   ` Stephen Hemminger
  1 sibling, 0 replies; 73+ messages in thread
From: Stephen Hemminger @ 2022-06-11 15:34 UTC (permalink / raw)
  To: David Marchand
  Cc: dev, thomas, ferruh.yigit, stable, Fan Zhang, Pablo de Lara,
	Deepak Kumar Jain

On Wed, 18 May 2022 12:16:48 +0200
David Marchand <david.marchand@redhat.com> wrote:

> GCC 12 raises the following warning:
> 
> In function ‘__rte_ring_enqueue_elems_64’,
>     inlined from ‘__rte_ring_enqueue_elems’ at
>         ../lib/ring/rte_ring_elem_pvt.h:130:3,
>     inlined from ‘__rte_ring_do_hts_enqueue_elem’ at
>         ../lib/ring/rte_ring_hts_elem_pvt.h:196:3,
>     inlined from ‘rte_ring_mp_hts_enqueue_burst_elem’ at
>         ../lib/ring/rte_ring_hts.h:110:9,
>     inlined from ‘rte_ring_enqueue_burst_elem’ at
>         ../lib/ring/rte_ring_elem.h:577:10,
>     inlined from ‘rte_ring_enqueue_burst’ at
>         ../lib/ring/rte_ring.h:738:9,
>     inlined from ‘process_op_bit’ at
>         ../drivers/crypto/ipsec_mb/pmd_snow3g.c:425:16,
>     inlined from ‘snow3g_pmd_dequeue_burst’ at
>         ../drivers/crypto/ipsec_mb/pmd_snow3g.c:484:20:
> ../lib/ring/rte_ring_elem_pvt.h:68:44: error: array subscript 1 is
>         outside array bounds of ‘struct rte_crypto_op[0]’
>         [-Werror=array-bounds]
>    68 |                         ring[idx + 1] = obj[i + 1];
>       |                                         ~~~^~~~~~~
> ../drivers/crypto/ipsec_mb/pmd_snow3g.c: In function
>         ‘snow3g_pmd_dequeue_burst’:
> ../drivers/crypto/ipsec_mb/pmd_snow3g.c:434:1: note:
>         at offset 8 into object ‘op’ of size 8
>   434 | snow3g_pmd_dequeue_burst(void *queue_pair,
>       | ^~~~~~~~~~~~~~~~~~~~~~~~
> 
> Validate that one (exactly) op has been processed or return early.
> 
> Fixes: b537abdbee74 ("crypto/snow3g: support bit-level operations")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Acked-by: Stephen Hemminger <stephen@networkplumber.org>


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

* Re: [PATCH 04/12] net/ena: fix build with GCC 12
  2022-05-18 10:16 ` [PATCH 04/12] net/ena: " David Marchand
  2022-05-20 20:28   ` Stephen Hemminger
@ 2022-06-11 15:34   ` Stephen Hemminger
  1 sibling, 0 replies; 73+ messages in thread
From: Stephen Hemminger @ 2022-06-11 15:34 UTC (permalink / raw)
  To: David Marchand
  Cc: dev, thomas, ferruh.yigit, stable, Marcin Wojtas,
	Michal Krawczyk, Shai Brandes, Evgeny Schemeilin, Igor Chauskin

On Wed, 18 May 2022 12:16:49 +0200
David Marchand <david.marchand@redhat.com> wrote:

> GCC 12 raises the following warning:
> 
> In file included from ../lib/mempool/rte_mempool.h:46,
>                  from ../lib/mbuf/rte_mbuf.h:38,
>                  from ../lib/net/rte_ether.h:22,
>                  from ../drivers/net/ena/ena_ethdev.h:10,
>                  from ../drivers/net/ena/ena_rss.c:6:
> ../drivers/net/ena/ena_rss.c: In function ‘ena_rss_key_fill’:
> ../lib/eal/x86/include/rte_memcpy.h:370:9: warning: array subscript 64 is
>         outside array bounds of ‘uint8_t[40]’ {aka ‘unsigned char[40]’}
>         [-Warray-bounds]
>   370 | rte_mov32((uint8_t *)dst + 2 * 32, (const uint8_t *)src + 2 * 32);
>       | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../drivers/net/ena/ena_rss.c:51:24: note: while referencing ‘default_key’
>    51 | static uint8_t default_key[ENA_HASH_KEY_SIZE];
>       |                ^~~~~~~~~~~
> 
> This is a false positive because the copied size is checked against
> ENA_HASH_KEY_SIZE in a (build) assert.
> Silence this warning by calling memcpy with the minimal size.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---


Acked-by: Stephen Hemminger <stephen@networkplumber.org>

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

* Re: [PATCH 05/12] net/enetfec: fix build with GCC 12
  2022-05-18 10:16 ` [PATCH 05/12] net/enetfec: " David Marchand
  2022-06-10 13:08   ` David Marchand
@ 2022-06-11 15:35   ` Stephen Hemminger
  1 sibling, 0 replies; 73+ messages in thread
From: Stephen Hemminger @ 2022-06-11 15:35 UTC (permalink / raw)
  To: David Marchand
  Cc: dev, thomas, ferruh.yigit, stable, Apeksha Gupta, Sachin Saxena

On Wed, 18 May 2022 12:16:50 +0200
David Marchand <david.marchand@redhat.com> wrote:

> GCC 12 raises the following warning:
> 
> ../drivers/net/enetfec/enet_ethdev.c: In function
>         ‘enetfec_rx_queue_setup’:
> ../drivers/net/enetfec/enet_ethdev.c:473:9: error: array
>         subscript 1 is
>     above array bounds of ‘uint32_t[1]’ {aka ‘unsigned int[1]’}
>         [-Werror=array-bounds]
>   473 | rte_write32(rte_cpu_to_le_32(fep->bd_addr_p_r[queue_idx]),
>       | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   474 |     (uint8_t *)fep->hw_baseaddr_v + ENETFEC_RD_START(queue_idx));
>       |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from ../drivers/net/enetfec/enet_ethdev.c:9:
> ../drivers/net/enetfec/enet_ethdev.h:113:33: note: while referencing
>         ‘bd_addr_p_r’
>   113 | uint32_t                bd_addr_p_r[ENETFEC_MAX_Q];
>       |                                 ^~~~~~~~~~~
> 
> This driver properly announces that it only supports 1 rxq.
> Silence this warning by adding an explicit check on the queue id.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>


Acked-by: Stephen Hemminger <stephen@networkplumber.org>

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

* Re: [PATCH 06/12] net/ice: fix build with GCC 12
  2022-05-18 10:16 ` [PATCH 06/12] net/ice: " David Marchand
@ 2022-06-11 15:36   ` Stephen Hemminger
  0 siblings, 0 replies; 73+ messages in thread
From: Stephen Hemminger @ 2022-06-11 15:36 UTC (permalink / raw)
  To: David Marchand; +Cc: dev, thomas, ferruh.yigit, stable, Qiming Yang, Qi Zhang

On Wed, 18 May 2022 12:16:51 +0200
David Marchand <david.marchand@redhat.com> wrote:

> GCC 12 raises the following warning:
> 
> In file included from ../lib/mempool/rte_mempool.h:46,
>                  from ../lib/mbuf/rte_mbuf.h:38,
>                  from ../lib/net/rte_ether.h:22,
>                  from ../lib/ethdev/rte_ethdev.h:172,
>                  from ../lib/ethdev/ethdev_driver.h:22,
>                  from ../lib/ethdev/ethdev_pci.h:17,
>                  from ../drivers/net/ice/ice_ethdev.c:6:
> ../drivers/net/ice/ice_ethdev.c: In function ‘ice_dev_configure’:
> ../lib/eal/x86/include/rte_memcpy.h:370:9: warning: array subscript 64 is
>         outside array bounds of ‘struct ice_aqc_get_set_rss_keys[1]’
>         [-Warray-bounds]
>   370 | rte_mov32((uint8_t *)dst + 2 * 32, (const uint8_t *)src + 2 * 32);
>       | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../drivers/net/ice/ice_ethdev.c:3202:41: note: while referencing ‘key’
>  3202 |         struct ice_aqc_get_set_rss_keys key;
>       |                                         ^~~
> 
> Restrict copy to minimum size.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

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

* Re: [PATCH 09/12] vdpa/ifc: fix build with GCC 12
  2022-05-18 10:16 ` [PATCH 09/12] vdpa/ifc: " David Marchand
  2022-05-18 11:48   ` Wang, Xiao W
@ 2022-06-11 15:36   ` Stephen Hemminger
  1 sibling, 0 replies; 73+ messages in thread
From: Stephen Hemminger @ 2022-06-11 15:36 UTC (permalink / raw)
  To: David Marchand; +Cc: dev, thomas, ferruh.yigit, stable, Xiao Wang

On Wed, 18 May 2022 12:16:54 +0200
David Marchand <david.marchand@redhat.com> wrote:

> GCC 12 raises the following warning:
> 
> ../drivers/vdpa/ifc/ifcvf_vdpa.c: In function ‘vdpa_enable_vfio_intr’:
> ../drivers/vdpa/ifc/ifcvf_vdpa.c:383:62: error: writing 4 bytes into a
>     region of size 0 [-Werror=stringop-overflow=]
>   383 |                         fd_ptr[RTE_INTR_VEC_RXTX_OFFSET + i] = fd;
>       |                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
> ../drivers/vdpa/ifc/ifcvf_vdpa.c:348:14: note: at offset 32 into
>     destination object ‘irq_set_buf’ of size 32
>   348 |         char irq_set_buf[MSIX_IRQ_SET_BUF_LEN];
>       |              ^~~~~~~~~~~
> 
> Validate number of vrings to avoid out of bound access.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>


Acked-by: Stephen Hemminger <stephen@networkplumber.org>

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

* Re: [PATCH 10/12] vhost/crypto: fix build with GCC 12
  2022-05-18 10:16 ` [PATCH 10/12] vhost/crypto: " David Marchand
  2022-06-02 10:08   ` Bruce Richardson
@ 2022-06-11 15:36   ` Stephen Hemminger
  2022-06-16  9:32   ` [PATCH v2] " David Marchand
  2 siblings, 0 replies; 73+ messages in thread
From: Stephen Hemminger @ 2022-06-11 15:36 UTC (permalink / raw)
  To: David Marchand
  Cc: dev, thomas, ferruh.yigit, stable, Maxime Coquelin, Chenbo Xia,
	Fan Zhang

On Wed, 18 May 2022 12:16:55 +0200
David Marchand <david.marchand@redhat.com> wrote:

> GCC 12 raises the following warning:
> 
> In file included from ../lib/mempool/rte_mempool.h:46,
>                  from ../lib/mbuf/rte_mbuf.h:38,
>                  from ../lib/vhost/vhost_crypto.c:7:
> ../lib/vhost/vhost_crypto.c: In function ‘rte_vhost_crypto_fetch_requests’:
> ../lib/eal/x86/include/rte_memcpy.h:371:9: warning: array subscript 1 is
>      outside array bounds of ‘struct virtio_crypto_op_data_req[1]’
>      [-Warray-bounds]
>   371 | rte_mov32((uint8_t *)dst + 3 * 32, (const uint8_t *)src + 3 * 32);
>       | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../lib/vhost/vhost_crypto.c:1178:42: note: while referencing ‘req’
>  1178 |         struct virtio_crypto_op_data_req req;
>       |                                          ^~~
> 
> Check that copied length is within req boundaries.
> 
> Fixes: 3c79609fda7c ("vhost/crypto: handle virtually non-contiguous buffers")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>


Acked-by: Stephen Hemminger <stephen@networkplumber.org>

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

* Re: [PATCH 11/12] app/flow-perf: fix build with GCC 12
  2022-05-18 10:16 ` [PATCH 11/12] app/flow-perf: " David Marchand
  2022-06-02 10:21   ` Bruce Richardson
  2022-06-08  9:03   ` Wisam Monther
@ 2022-06-11 15:37   ` Stephen Hemminger
  2 siblings, 0 replies; 73+ messages in thread
From: Stephen Hemminger @ 2022-06-11 15:37 UTC (permalink / raw)
  To: David Marchand; +Cc: dev, thomas, ferruh.yigit, stable, Wisam Jaddo

On Wed, 18 May 2022 12:16:56 +0200
David Marchand <david.marchand@redhat.com> wrote:

> GCC 12 raises the following warning:
> 
> ../app/test-flow-perf/main.c: In function ‘start_forwarding’:
> ../app/test-flow-perf/main.c:1737:28: error: ‘sprintf’ may write a
>     terminating nul past the end of the destination
>     [-Werror=format-overflow=]
>  1737 |         sprintf(p[i++], "%d", (int)n);
>       |                            ^
> In function ‘pretty_number’,
>     inlined from ‘packet_per_second_stats’ at
>         ../app/test-flow-perf/main.c:1792:4,
>     inlined from ‘start_forwarding’ at
>         ../app/test-flow-perf/main.c:1831:3:
> [...]
> 
> We can simplify this code and rely on libc integer formatting via
> this system locales.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Fixes and ends up cleaner.

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

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

* Re: [PATCH 12/12] test/ipsec: fix build with GCC 12
  2022-05-18 10:16 ` [PATCH 12/12] test/ipsec: " David Marchand
  2022-06-02 18:41   ` Medvedkin, Vladimir
@ 2022-06-11 15:38   ` Stephen Hemminger
  2022-06-16  9:33   ` [PATCH v2] " David Marchand
  2022-06-16 14:46   ` [PATCH v3] vhost/crypto: " David Marchand
  3 siblings, 0 replies; 73+ messages in thread
From: Stephen Hemminger @ 2022-06-11 15:38 UTC (permalink / raw)
  To: David Marchand
  Cc: dev, thomas, ferruh.yigit, stable, Konstantin Ananyev,
	Bernard Iremonger, Vladimir Medvedkin

On Wed, 18 May 2022 12:16:57 +0200
David Marchand <david.marchand@redhat.com> wrote:

> GCC 12 raises the following warning:
> 
> In function ‘_mm256_loadu_si256’,
>     inlined from ‘rte_mov32’ at
>         ../lib/eal/x86/include/rte_memcpy.h:319:9,
>     inlined from ‘rte_mov128’ at
>         ../lib/eal/x86/include/rte_memcpy.h:344:2,
>     inlined from ‘rte_memcpy_generic’ at
>         ../lib/eal/x86/include/rte_memcpy.h:438:4,
>     inlined from ‘rte_memcpy’ at
>         ../lib/eal/x86/include/rte_memcpy.h:882:10,
>     inlined from ‘setup_test_string.constprop’ at
>         ../app/test/test_ipsec.c:572:4:
> /usr/lib/gcc/x86_64-redhat-linux/12/include/avxintrin.h:929:10: error:
>     array subscript ‘__m256i_u[3]’ is partly outside array bounds of
>     ‘const char[108]’ [-Werror=array-bounds]
>   929 |   return *__P;
>       |          ^~~~
> ../app/test/test_ipsec.c: In function ‘setup_test_string.constprop’:
> ../app/test/test_ipsec.c:539:12: note: at offset 96 into object
>     ‘null_plain_data’ of size 108
>   539 | const char null_plain_data[] =
>       |            ^~~~~~~~~~~~~~~
> 
> Split copy request into copies of string lengths and remove unused
> blocksize.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Why is test code for ipsec bother with using rte_memcpy at all.
Instead global replace rte_memcpy() with memcpy() for the whole test.

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

* Re: [PATCH 05/12] net/enetfec: fix build with GCC 12
  2022-06-10 13:08   ` David Marchand
@ 2022-06-13  5:22     ` Sachin Saxena (OSS)
  2022-06-14  8:16     ` Sachin Saxena (OSS)
  1 sibling, 0 replies; 73+ messages in thread
From: Sachin Saxena (OSS) @ 2022-06-13  5:22 UTC (permalink / raw)
  To: David Marchand; +Cc: thomas, Ferruh Yigit, dev, stable

Hello David,

I understood and agree with your suggestion. We are using GCC 11.3 where 
we were not seeing this warning.
We will fix this on priority and submit the patch asap.

regards,
Sachin Saxena

On 6/10/2022 6:38 PM, David Marchand wrote:
> On Wed, May 18, 2022 at 12:17 PM David Marchand
> <david.marchand@redhat.com> wrote:
>> GCC 12 raises the following warning:
>>
>> ../drivers/net/enetfec/enet_ethdev.c: In function
>>          ‘enetfec_rx_queue_setup’:
>> ../drivers/net/enetfec/enet_ethdev.c:473:9: error: array
>>          subscript 1 is
>>      above array bounds of ‘uint32_t[1]’ {aka ‘unsigned int[1]’}
>>          [-Werror=array-bounds]
>>    473 | rte_write32(rte_cpu_to_le_32(fep->bd_addr_p_r[queue_idx]),
>>        | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>    474 |     (uint8_t *)fep->hw_baseaddr_v + ENETFEC_RD_START(queue_idx));
>>        |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> In file included from ../drivers/net/enetfec/enet_ethdev.c:9:
>> ../drivers/net/enetfec/enet_ethdev.h:113:33: note: while referencing
>>          ‘bd_addr_p_r’
>>    113 | uint32_t                bd_addr_p_r[ENETFEC_MAX_Q];
>>        |                                 ^~~~~~~~~~~
>>
>> This driver properly announces that it only supports 1 rxq.
>> Silence this warning by adding an explicit check on the queue id.
>>
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Any comment from driver maintainers?
> Thanks.
>
>


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

* RE: [PATCH 11/12] app/flow-perf: fix build with GCC 12
  2022-06-08 12:20     ` David Marchand
@ 2022-06-13  7:49       ` Wisam Monther
  0 siblings, 0 replies; 73+ messages in thread
From: Wisam Monther @ 2022-06-13  7:49 UTC (permalink / raw)
  To: David Marchand
  Cc: dev, NBU-Contact-Thomas Monjalon (EXTERNAL), ferruh.yigit, stable

Hi,

> > > -----Original Message-----
> > > From: David Marchand <david.marchand@redhat.com>
> > > Sent: Wednesday, May 18, 2022 1:17 PM
> > > To: dev@dpdk.org
> > > Cc: NBU-Contact-Thomas Monjalon (EXTERNAL)
> <thomas@monjalon.net>;
> > > ferruh.yigit@xilinx.com; stable@dpdk.org; Wisam Monther
> > > <wisamm@nvidia.com>
> > > Subject: [PATCH 11/12] app/flow-perf: fix build with GCC 12
> > >
> > > GCC 12 raises the following warning:
> > >
> > > ../app/test-flow-perf/main.c: In function ‘start_forwarding’:
> > > ../app/test-flow-perf/main.c:1737:28: error: ‘sprintf’ may write a
> > >     terminating nul past the end of the destination
> > >     [-Werror=format-overflow=]
> > >  1737 |         sprintf(p[i++], "%d", (int)n);
> > >       |                            ^
> > > In function ‘pretty_number’,
> > >     inlined from ‘packet_per_second_stats’ at
> > >         ../app/test-flow-perf/main.c:1792:4,
> > >     inlined from ‘start_forwarding’ at
> > >         ../app/test-flow-perf/main.c:1831:3:
> > > [...]
> > >
> > > We can simplify this code and rely on libc integer formatting via
> > > this system locales.
> > >
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > ---
> >
> > I've tested the patch and reviewed it, it's working fine, so thank you for
> that.
> > One comment
> > The initial value of 0 is 000
> >
> > Example:
> > CMD: ./dpdk-test-flow-perf -n 4 -a <PCI> -- ingress --group=1 --ether --
> queue --rules-count=200000 --enable-fwd
> >   core               tx         tx drops               rx
> > ------ ---------------- ---------------- ----------------
> >      1              000              000              000
> >
> > Can you handle this to be single 0 instead of not needed leading zeros?
> 
> Hum, I don't remember why I added this precision...
> This should be just a matter of changing the format from %'16.3s to %'16s,
> can you confirm?

Confirmed, you can go with it. Thanks in advance.

BRs,
Wisam Jaddo

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

* RE: [EXT] Re: [PATCH 01/12] common/cpt: fix build with GCC 12
  2022-06-10 13:11   ` David Marchand
@ 2022-06-13 11:40     ` Ankur Dwivedi
  2022-06-16  9:30       ` David Marchand
  0 siblings, 1 reply; 73+ messages in thread
From: Ankur Dwivedi @ 2022-06-13 11:40 UTC (permalink / raw)
  To: David Marchand, Anoob Joseph
  Cc: Thomas Monjalon, Ferruh Yigit, dpdk stable, dev, Akhil Goyal,
	Jerin Jacob Kollanukkaran

Hi David,

>-----Original Message-----
>From: David Marchand <david.marchand@redhat.com>
>Sent: Friday, June 10, 2022 6:42 PM
>To: Anoob Joseph <anoobj@marvell.com>; Ankur Dwivedi
><adwivedi@marvell.com>
>Cc: Thomas Monjalon <thomas@monjalon.net>; Ferruh Yigit
><ferruh.yigit@xilinx.com>; dpdk stable <stable@dpdk.org>; dev
><dev@dpdk.org>; Akhil Goyal <gakhil@marvell.com>; Jerin Jacob
>Kollanukkaran <jerinj@marvell.com>
>Subject: [EXT] Re: [PATCH 01/12] common/cpt: fix build with GCC 12
>
>External Email
>
>----------------------------------------------------------------------
>Hello maintainers,
>
>On Wed, May 18, 2022 at 12:17 PM David Marchand
><david.marchand@redhat.com> wrote:
>>
>> GCC 12 raises the following warning:
>>
>> In function ‘fill_sg_comp_from_iov’,
>>     inlined from ‘cpt_kasumi_enc_prep’ at
>>         ../drivers/common/cpt/cpt_ucode.h:2176:8,
>>     inlined from ‘cpt_fc_enc_hmac_prep’ at
>>         ../drivers/common/cpt/cpt_ucode.h:2475:3,
>>     inlined from ‘fill_digest_params’ at
>>         ../drivers/common/cpt/cpt_ucode.h:3548:14,
>>     inlined from ‘otx_cpt_enq_single_sym’ at
>>         ../drivers/crypto/octeontx/otx_cryptodev_ops.c:541:9,
>>     inlined from ‘otx_cpt_enq_single_sym_sessless’ at
>>         ../drivers/crypto/octeontx/otx_cryptodev_ops.c:584:8,
>>     inlined from ‘otx_cpt_enq_single’ at
>>         ../drivers/crypto/octeontx/otx_cryptodev_ops.c:611:11,
>>     inlined from ‘otx_cpt_pkt_enqueue’ at
>>         ../drivers/crypto/octeontx/otx_cryptodev_ops.c:643:9,
>>     inlined from ‘otx_cpt_enqueue_sym’ at
>>         ../drivers/crypto/octeontx/otx_cryptodev_ops.c:668:9:
>> ../drivers/common/cpt/cpt_ucode.h:415:36: error: array subscript 0 is
>>         outside array bounds of ‘buf_ptr_t[0]’ {aka ‘struct buf_ptr[]’}
>>         [-Werror=array-bounds]
>>   415 |                         e_dma_addr = bufs[j].dma_addr;
>>       |                         ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
>> ../drivers/common/cpt/cpt_ucode.h:416:48: error: array subscript 0 is
>>         outside array bounds of ‘buf_ptr_t[0]’ {aka ‘struct buf_ptr[]’}
>>         [-Werror=array-bounds]
>>   416 |                         e_len = (size > bufs[j].size) ?
>>       |                                         ~~~~~~~^~~~~
>>
>> For now, waive this warning until we have a proper fix.
>
>Both common/cpt and crypto/cnxk have the same code that triggers this
>warning.
>Can you look into this please?

We will look into the issues in common/cpt and crypto/cnxk.
>
>Thanks.
>
>--
>David Marchand

Regards,
Ankur


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

* Re: [PATCH 05/12] net/enetfec: fix build with GCC 12
  2022-06-10 13:08   ` David Marchand
  2022-06-13  5:22     ` Sachin Saxena (OSS)
@ 2022-06-14  8:16     ` Sachin Saxena (OSS)
  1 sibling, 0 replies; 73+ messages in thread
From: Sachin Saxena (OSS) @ 2022-06-14  8:16 UTC (permalink / raw)
  To: David Marchand, Apeksha Gupta, Sachin Saxena
  Cc: Thomas Monjalon, Ferruh Yigit, dpdk stable, dev

[-- Attachment #1: Type: text/plain, Size: 1379 bytes --]

Acked-by: Sachin Saxena <sachin.saxena@nxp.com>

On 6/10/2022 6:38 PM, David Marchand wrote:
> On Wed, May 18, 2022 at 12:17 PM David Marchand
> <david.marchand@redhat.com> wrote:
>> GCC 12 raises the following warning:
>>
>> ../drivers/net/enetfec/enet_ethdev.c: In function
>>          ‘enetfec_rx_queue_setup’:
>> ../drivers/net/enetfec/enet_ethdev.c:473:9: error: array
>>          subscript 1 is
>>      above array bounds of ‘uint32_t[1]’ {aka ‘unsigned int[1]’}
>>          [-Werror=array-bounds]
>>    473 | rte_write32(rte_cpu_to_le_32(fep->bd_addr_p_r[queue_idx]),
>>        | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>    474 |     (uint8_t *)fep->hw_baseaddr_v + ENETFEC_RD_START(queue_idx));
>>        |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> In file included from ../drivers/net/enetfec/enet_ethdev.c:9:
>> ../drivers/net/enetfec/enet_ethdev.h:113:33: note: while referencing
>>          ‘bd_addr_p_r’
>>    113 | uint32_t                bd_addr_p_r[ENETFEC_MAX_Q];
>>        |                                 ^~~~~~~~~~~
>>
>> This driver properly announces that it only supports 1 rxq.
>> Silence this warning by adding an explicit check on the queue id.
>>
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Any comment from driver maintainers?
> Thanks.
>
>


[-- Attachment #2: Type: text/html, Size: 2127 bytes --]

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

* Re: [PATCH 10/12] vhost/crypto: fix build with GCC 12
  2022-06-02 10:08   ` Bruce Richardson
@ 2022-06-14  9:22     ` David Marchand
  2022-06-14  9:25       ` Bruce Richardson
  0 siblings, 1 reply; 73+ messages in thread
From: David Marchand @ 2022-06-14  9:22 UTC (permalink / raw)
  To: Bruce Richardson, Fan Zhang, Maxime Coquelin, Chenbo Xia
  Cc: dev, Thomas Monjalon, Ferruh Yigit, dpdk stable

On Thu, Jun 2, 2022 at 12:09 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Wed, May 18, 2022 at 12:16:55PM +0200, David Marchand wrote:
> > GCC 12 raises the following warning:
> >
> > In file included from ../lib/mempool/rte_mempool.h:46,
> >                  from ../lib/mbuf/rte_mbuf.h:38,
> >                  from ../lib/vhost/vhost_crypto.c:7:
> > ../lib/vhost/vhost_crypto.c: In function ‘rte_vhost_crypto_fetch_requests’:
> > ../lib/eal/x86/include/rte_memcpy.h:371:9: warning: array subscript 1 is
> >      outside array bounds of ‘struct virtio_crypto_op_data_req[1]’
> >      [-Warray-bounds]
> >   371 | rte_mov32((uint8_t *)dst + 3 * 32, (const uint8_t *)src + 3 * 32);
> >       | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > ../lib/vhost/vhost_crypto.c:1178:42: note: while referencing ‘req’
> >  1178 |         struct virtio_crypto_op_data_req req;
> >       |                                          ^~~
> >
> > Check that copied length is within req boundaries.
> >
> > Fixes: 3c79609fda7c ("vhost/crypto: handle virtually non-contiguous buffers")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> >  lib/vhost/vhost_crypto.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/vhost/vhost_crypto.c b/lib/vhost/vhost_crypto.c
> > index b1c0eb6a0f..83325b7042 100644
> > --- a/lib/vhost/vhost_crypto.c
> > +++ b/lib/vhost/vhost_crypto.c
> > @@ -576,16 +576,16 @@ copy_data(void *dst_data, struct vhost_crypto_data_req *vc_req,
> >       uint32_t to_copy;
> >       uint8_t *data = dst_data;
> >       uint8_t *src;
> > -     int left = size;
> > +     uint32_t left = size;
> >
> > -     to_copy = RTE_MIN(desc->len, (uint32_t)left);
> > +     to_copy = RTE_MIN(desc->len, left);
> >       dlen = to_copy;
> >       src = IOVA_TO_VVA(uint8_t *, vc_req, desc->addr, &dlen,
> >                       VHOST_ACCESS_RO);
>
> Tracking the functions which end up being called by this macro, the dlen
> parameter ends up being of type "uint64_t *", passing a value of int * or
> uint32_t * seems wrong to me. If we are changing the type from int to
> uint32_t, I think it should be promoted all the way to uint64_t.

Indeed.
I'll update in v2.

We already had some CVE on this part of the code, a careful review is needed.


>
> > -     if (unlikely(!src || !dlen))
> > +     if (unlikely(!src || !dlen || dlen > left))
> >               return -1;
> >
>
> If this change is omitted, does the compiler still give warnings. Looking
> through the called code, the dlen parameter can only ever be reduced, not
> incremented (function rte_vhost_va_from_guest_pa() in rte_vhost.h).

If I promote to_copy and left variables as uint64_t, gcc is still
unhappy, for the same reason.
The check on dlen > left seems necessary.


>
> > -     rte_memcpy((uint8_t *)data, src, dlen);
> > +     rte_memcpy(data, src, dlen);
> >       data += dlen;
> >
> >       if (unlikely(dlen < to_copy)) {
> > --
> > 2.36.1
> >
>

-- 
David Marchand


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

* Re: [PATCH 10/12] vhost/crypto: fix build with GCC 12
  2022-06-14  9:22     ` David Marchand
@ 2022-06-14  9:25       ` Bruce Richardson
  2022-06-16  9:27         ` David Marchand
  0 siblings, 1 reply; 73+ messages in thread
From: Bruce Richardson @ 2022-06-14  9:25 UTC (permalink / raw)
  To: David Marchand
  Cc: Fan Zhang, Maxime Coquelin, Chenbo Xia, dev, Thomas Monjalon,
	Ferruh Yigit, dpdk stable

On Tue, Jun 14, 2022 at 11:22:24AM +0200, David Marchand wrote:
> On Thu, Jun 2, 2022 at 12:09 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > On Wed, May 18, 2022 at 12:16:55PM +0200, David Marchand wrote:
> > > GCC 12 raises the following warning:
> > >
> > > In file included from ../lib/mempool/rte_mempool.h:46,
> > >                  from ../lib/mbuf/rte_mbuf.h:38,
> > >                  from ../lib/vhost/vhost_crypto.c:7:
> > > ../lib/vhost/vhost_crypto.c: In function ‘rte_vhost_crypto_fetch_requests’:
> > > ../lib/eal/x86/include/rte_memcpy.h:371:9: warning: array subscript 1 is
> > >      outside array bounds of ‘struct virtio_crypto_op_data_req[1]’
> > >      [-Warray-bounds]
> > >   371 | rte_mov32((uint8_t *)dst + 3 * 32, (const uint8_t *)src + 3 * 32);
> > >       | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > ../lib/vhost/vhost_crypto.c:1178:42: note: while referencing ‘req’
> > >  1178 |         struct virtio_crypto_op_data_req req;
> > >       |                                          ^~~
> > >
> > > Check that copied length is within req boundaries.
> > >
> > > Fixes: 3c79609fda7c ("vhost/crypto: handle virtually non-contiguous buffers")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > ---
> > >  lib/vhost/vhost_crypto.c | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/lib/vhost/vhost_crypto.c b/lib/vhost/vhost_crypto.c
> > > index b1c0eb6a0f..83325b7042 100644
> > > --- a/lib/vhost/vhost_crypto.c
> > > +++ b/lib/vhost/vhost_crypto.c
> > > @@ -576,16 +576,16 @@ copy_data(void *dst_data, struct vhost_crypto_data_req *vc_req,
> > >       uint32_t to_copy;
> > >       uint8_t *data = dst_data;
> > >       uint8_t *src;
> > > -     int left = size;
> > > +     uint32_t left = size;
> > >
> > > -     to_copy = RTE_MIN(desc->len, (uint32_t)left);
> > > +     to_copy = RTE_MIN(desc->len, left);
> > >       dlen = to_copy;
> > >       src = IOVA_TO_VVA(uint8_t *, vc_req, desc->addr, &dlen,
> > >                       VHOST_ACCESS_RO);
> >
> > Tracking the functions which end up being called by this macro, the dlen
> > parameter ends up being of type "uint64_t *", passing a value of int * or
> > uint32_t * seems wrong to me. If we are changing the type from int to
> > uint32_t, I think it should be promoted all the way to uint64_t.
> 
> Indeed.
> I'll update in v2.
> 
> We already had some CVE on this part of the code, a careful review is needed.
> 
> 
> >
> > > -     if (unlikely(!src || !dlen))
> > > +     if (unlikely(!src || !dlen || dlen > left))
> > >               return -1;
> > >
> >
> > If this change is omitted, does the compiler still give warnings. Looking
> > through the called code, the dlen parameter can only ever be reduced, not
> > incremented (function rte_vhost_va_from_guest_pa() in rte_vhost.h).
> 
> If I promote to_copy and left variables as uint64_t, gcc is still
> unhappy, for the same reason.
> The check on dlen > left seems necessary.
> 
> 
Ok, just thought I'd ask anyway. I wonder if we need to check for
wrap-around in the reduction case, since we are dealing with unsigned
values. This additional check should catch that anyway if it does occur.

/Bruce

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

* Re: [PATCH 00/12] Fix compilation with gcc 12
  2022-05-18 10:16 [PATCH 00/12] Fix compilation with gcc 12 David Marchand
                   ` (12 preceding siblings ...)
  2022-05-20 20:13 ` [PATCH 00/12] Fix compilation with gcc 12 Stephen Hemminger
@ 2022-06-15  8:49 ` David Marchand
  2022-06-15 14:45   ` Stephen Hemminger
  13 siblings, 1 reply; 73+ messages in thread
From: David Marchand @ 2022-06-15  8:49 UTC (permalink / raw)
  To: dev, Thomas Monjalon, Stephen Hemminger
  Cc: Ferruh Yigit, Bruce Richardson, Morten Brørup, Konstantin Ananyev

On Wed, May 18, 2022 at 12:17 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> Fedora 36 is out since early may and comes with gcc 12.
> This series fixes compilation or waives some checks.
>
> There might be something fishy with rte_memcpy on x86 but, for now,
> the rte_memcpy related fixes are on the caller side.
>
> Some "base" drivers have issues, I chose the simple solution of waiving
> the checks for them.
>
> Compilation is the only thing checked.
> Please driver maintainers, check nothing got broken.

I applied the patches that got acked and that had no objection or
comment from maintainers (i.e. patch 3, 4, 5, 6, 9, 11).
I also cleaned the mess in bugzilla where we had multiple reports of
the same issues, or stale bugs that I can't reproduce with released
gcc 12.

I'll respin separately the patches for which I have clear comments,
and drop my patches waiving the compiler checks.

We still need to agree on the best approach to handle the new checks.
We have two rfc series from Stephen, how do we move forward?


-- 
David Marchand


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

* Re: [PATCH 00/12] Fix compilation with gcc 12
  2022-06-15  8:49 ` David Marchand
@ 2022-06-15 14:45   ` Stephen Hemminger
  2022-06-15 14:59     ` Thomas Monjalon
  0 siblings, 1 reply; 73+ messages in thread
From: Stephen Hemminger @ 2022-06-15 14:45 UTC (permalink / raw)
  To: David Marchand
  Cc: dev, Thomas Monjalon, Ferruh Yigit, Bruce Richardson,
	Morten Brørup, Konstantin Ananyev

On Wed, 15 Jun 2022 10:49:17 +0200
David Marchand <david.marchand@redhat.com> wrote:

> On Wed, May 18, 2022 at 12:17 PM David Marchand
> <david.marchand@redhat.com> wrote:
> >
> > Fedora 36 is out since early may and comes with gcc 12.
> > This series fixes compilation or waives some checks.
> >
> > There might be something fishy with rte_memcpy on x86 but, for now,
> > the rte_memcpy related fixes are on the caller side.
> >
> > Some "base" drivers have issues, I chose the simple solution of waiving
> > the checks for them.
> >
> > Compilation is the only thing checked.
> > Please driver maintainers, check nothing got broken.  
> 
> I applied the patches that got acked and that had no objection or
> comment from maintainers (i.e. patch 3, 4, 5, 6, 9, 11).
> I also cleaned the mess in bugzilla where we had multiple reports of
> the same issues, or stale bugs that I can't reproduce with released
> gcc 12.
> 
> I'll respin separately the patches for which I have clear comments,
> and drop my patches waiving the compiler checks.
> 
> We still need to agree on the best approach to handle the new checks.
> We have two rfc series from Stephen, how do we move forward?

Lets fix all the bugs and remove any workarounds using pragma's.

Some of them may mean removing rte_memcpy where it is not needed.

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

* Re: [PATCH 00/12] Fix compilation with gcc 12
  2022-06-15 14:45   ` Stephen Hemminger
@ 2022-06-15 14:59     ` Thomas Monjalon
  2022-06-15 15:15       ` Stephen Hemminger
  0 siblings, 1 reply; 73+ messages in thread
From: Thomas Monjalon @ 2022-06-15 14:59 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: David Marchand, dev, Ferruh Yigit, Bruce Richardson,
	Morten Brørup, Konstantin Ananyev

15/06/2022 16:45, Stephen Hemminger:
> On Wed, 15 Jun 2022 10:49:17 +0200
> David Marchand <david.marchand@redhat.com> wrote:
> 
> > On Wed, May 18, 2022 at 12:17 PM David Marchand
> > <david.marchand@redhat.com> wrote:
> > >
> > > Fedora 36 is out since early may and comes with gcc 12.
> > > This series fixes compilation or waives some checks.
> > >
> > > There might be something fishy with rte_memcpy on x86 but, for now,
> > > the rte_memcpy related fixes are on the caller side.
> > >
> > > Some "base" drivers have issues, I chose the simple solution of waiving
> > > the checks for them.
> > >
> > > Compilation is the only thing checked.
> > > Please driver maintainers, check nothing got broken.  
> > 
> > I applied the patches that got acked and that had no objection or
> > comment from maintainers (i.e. patch 3, 4, 5, 6, 9, 11).
> > I also cleaned the mess in bugzilla where we had multiple reports of
> > the same issues, or stale bugs that I can't reproduce with released
> > gcc 12.
> > 
> > I'll respin separately the patches for which I have clear comments,
> > and drop my patches waiving the compiler checks.
> > 
> > We still need to agree on the best approach to handle the new checks.
> > We have two rfc series from Stephen, how do we move forward?
> 
> Lets fix all the bugs and remove any workarounds using pragma's.
> 
> Some of them may mean removing rte_memcpy where it is not needed.

What about your series Stephen?
Please would you like to respin?



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

* Re: [PATCH 00/12] Fix compilation with gcc 12
  2022-06-15 14:59     ` Thomas Monjalon
@ 2022-06-15 15:15       ` Stephen Hemminger
  0 siblings, 0 replies; 73+ messages in thread
From: Stephen Hemminger @ 2022-06-15 15:15 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: David Marchand, dev, Ferruh Yigit, Bruce Richardson,
	Morten Brørup, Konstantin Ananyev

On Wed, 15 Jun 2022 16:59:51 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:

> 15/06/2022 16:45, Stephen Hemminger:
> > On Wed, 15 Jun 2022 10:49:17 +0200
> > David Marchand <david.marchand@redhat.com> wrote:
> >   
> > > On Wed, May 18, 2022 at 12:17 PM David Marchand
> > > <david.marchand@redhat.com> wrote:  
> > > >
> > > > Fedora 36 is out since early may and comes with gcc 12.
> > > > This series fixes compilation or waives some checks.
> > > >
> > > > There might be something fishy with rte_memcpy on x86 but, for now,
> > > > the rte_memcpy related fixes are on the caller side.
> > > >
> > > > Some "base" drivers have issues, I chose the simple solution of waiving
> > > > the checks for them.
> > > >
> > > > Compilation is the only thing checked.
> > > > Please driver maintainers, check nothing got broken.    
> > > 
> > > I applied the patches that got acked and that had no objection or
> > > comment from maintainers (i.e. patch 3, 4, 5, 6, 9, 11).
> > > I also cleaned the mess in bugzilla where we had multiple reports of
> > > the same issues, or stale bugs that I can't reproduce with released
> > > gcc 12.
> > > 
> > > I'll respin separately the patches for which I have clear comments,
> > > and drop my patches waiving the compiler checks.
> > > 
> > > We still need to agree on the best approach to handle the new checks.
> > > We have two rfc series from Stephen, how do we move forward?  
> > 
> > Lets fix all the bugs and remove any workarounds using pragma's.
> > 
> > Some of them may mean removing rte_memcpy where it is not needed.  
> 
> What about your series Stephen?
> Please would you like to respin?
> 
> 

Yes will recollate based on current main branch.

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

* Re: [PATCH 10/12] vhost/crypto: fix build with GCC 12
  2022-06-14  9:25       ` Bruce Richardson
@ 2022-06-16  9:27         ` David Marchand
  0 siblings, 0 replies; 73+ messages in thread
From: David Marchand @ 2022-06-16  9:27 UTC (permalink / raw)
  To: Bruce Richardson, Fan Zhang
  Cc: Maxime Coquelin, Chenbo Xia, dev, Thomas Monjalon, Ferruh Yigit,
	dpdk stable

On Tue, Jun 14, 2022 at 11:25 AM Bruce Richardson
<bruce.richardson@intel.com> wrote:
> > > > -     if (unlikely(!src || !dlen))
> > > > +     if (unlikely(!src || !dlen || dlen > left))
> > > >               return -1;
> > > >
> > >
> > > If this change is omitted, does the compiler still give warnings. Looking
> > > through the called code, the dlen parameter can only ever be reduced, not
> > > incremented (function rte_vhost_va_from_guest_pa() in rte_vhost.h).
> >
> > If I promote to_copy and left variables as uint64_t, gcc is still
> > unhappy, for the same reason.
> > The check on dlen > left seems necessary.
> >
> >
> Ok, just thought I'd ask anyway. I wonder if we need to check for
> wrap-around in the reduction case, since we are dealing with unsigned
> values. This additional check should catch that anyway if it does occur.

I had a fresh look at this code and went with some splitting / simplification.
This makes the code clearer, and there is no added check.

I'll send a v2.


-- 
David Marchand


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

* Re: [EXT] Re: [PATCH 01/12] common/cpt: fix build with GCC 12
  2022-06-13 11:40     ` [EXT] " Ankur Dwivedi
@ 2022-06-16  9:30       ` David Marchand
  2022-06-16 11:59         ` Ankur Dwivedi
  0 siblings, 1 reply; 73+ messages in thread
From: David Marchand @ 2022-06-16  9:30 UTC (permalink / raw)
  To: Ankur Dwivedi, Jerin Jacob Kollanukkaran
  Cc: Anoob Joseph, Thomas Monjalon, Ferruh Yigit, dpdk stable, dev,
	Akhil Goyal

On Mon, Jun 13, 2022 at 1:40 PM Ankur Dwivedi <adwivedi@marvell.com> wrote:
> >> For now, waive this warning until we have a proper fix.
> >
> >Both common/cpt and crypto/cnxk have the same code that triggers this
> >warning.
> >Can you look into this please?
>
> We will look into the issues in common/cpt and crypto/cnxk.

Any update?
Thanks.


-- 
David Marchand


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

* [PATCH v2] vhost/crypto: fix build with GCC 12
  2022-05-18 10:16 ` [PATCH 10/12] vhost/crypto: " David Marchand
  2022-06-02 10:08   ` Bruce Richardson
  2022-06-11 15:36   ` Stephen Hemminger
@ 2022-06-16  9:32   ` David Marchand
  2022-06-16 14:53     ` David Marchand
  2 siblings, 1 reply; 73+ messages in thread
From: David Marchand @ 2022-06-16  9:32 UTC (permalink / raw)
  To: dev; +Cc: bruce.richardson, stable, Maxime Coquelin, Chenbo Xia, Fan Zhang

GCC 12 raises the following warning:

In file included from ../lib/mempool/rte_mempool.h:46,
                 from ../lib/mbuf/rte_mbuf.h:38,
                 from ../lib/vhost/vhost_crypto.c:7:
../lib/vhost/vhost_crypto.c: In function ‘rte_vhost_crypto_fetch_requests’:
../lib/eal/x86/include/rte_memcpy.h:371:9: warning: array subscript 1 is
     outside array bounds of ‘struct virtio_crypto_op_data_req[1]’
     [-Warray-bounds]
  371 | rte_mov32((uint8_t *)dst + 3 * 32, (const uint8_t *)src + 3 * 32);
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../lib/vhost/vhost_crypto.c:1178:42: note: while referencing ‘req’
 1178 |         struct virtio_crypto_op_data_req req;
      |                                          ^~~

Split this function and separate the per descriptor copy.
This makes the code clearer, and the compiler happier.

Note: logs for errors have been moved to callers to avoid duplicates.

Fixes: 3c79609fda7c ("vhost/crypto: handle virtually non-contiguous buffers")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changes since v1:
- refactored copy function,

---
 lib/vhost/vhost_crypto.c | 122 +++++++++++++++------------------------
 1 file changed, 45 insertions(+), 77 deletions(-)

diff --git a/lib/vhost/vhost_crypto.c b/lib/vhost/vhost_crypto.c
index b1c0eb6a0f..1bc42896ea 100644
--- a/lib/vhost/vhost_crypto.c
+++ b/lib/vhost/vhost_crypto.c
@@ -565,94 +565,57 @@ get_data_ptr(struct vhost_crypto_data_req *vc_req,
 	return data;
 }
 
-static __rte_always_inline int
-copy_data(void *dst_data, struct vhost_crypto_data_req *vc_req,
-		struct vhost_crypto_desc *head,
-		struct vhost_crypto_desc **cur_desc,
-		uint32_t size, uint32_t max_n_descs)
+static __rte_always_inline uint32_t
+copy_data_from_desc(void *dst, struct vhost_crypto_data_req *vc_req,
+	struct vhost_crypto_desc *desc, uint32_t size)
 {
-	struct vhost_crypto_desc *desc = *cur_desc;
-	uint64_t remain, addr, dlen, len;
-	uint32_t to_copy;
-	uint8_t *data = dst_data;
-	uint8_t *src;
-	int left = size;
-
-	to_copy = RTE_MIN(desc->len, (uint32_t)left);
-	dlen = to_copy;
-	src = IOVA_TO_VVA(uint8_t *, vc_req, desc->addr, &dlen,
-			VHOST_ACCESS_RO);
-	if (unlikely(!src || !dlen))
-		return -1;
+	uint64_t remain;
+	uint64_t addr;
+
+	remain = RTE_MIN(desc->len, size);
+	addr = desc->addr;
+	do {
+		uint64_t len;
+		void *src;
+
+		len = remain;
+		src = IOVA_TO_VVA(void *, vc_req, addr, &len, VHOST_ACCESS_RO);
+		if (unlikely(src == NULL || len == 0))
+			return 0;
 
-	rte_memcpy((uint8_t *)data, src, dlen);
-	data += dlen;
+		rte_memcpy(dst, src, len);
+		remain -= len;
+		dst = RTE_PTR_ADD(dst, len);
+		addr += len;
+	} while (unlikely(remain != 0));
 
-	if (unlikely(dlen < to_copy)) {
-		remain = to_copy - dlen;
-		addr = desc->addr + dlen;
+	return RTE_MIN(desc->len, size);
+}
 
-		while (remain) {
-			len = remain;
-			src = IOVA_TO_VVA(uint8_t *, vc_req, addr, &len,
-					VHOST_ACCESS_RO);
-			if (unlikely(!src || !len)) {
-				VC_LOG_ERR("Failed to map descriptor");
-				return -1;
-			}
 
-			rte_memcpy(data, src, len);
-			addr += len;
-			remain -= len;
-			data += len;
-		}
-	}
+static __rte_always_inline int
+copy_data(void *data, struct vhost_crypto_data_req *vc_req,
+	struct vhost_crypto_desc *head, struct vhost_crypto_desc **cur_desc,
+	uint32_t size, uint32_t max_n_descs)
+{
+	struct vhost_crypto_desc *desc = *cur_desc;
+	uint32_t left = size;
 
-	left -= to_copy;
+	do {
+		uint32_t copied;
 
-	while (desc >= head && desc - head < (int)max_n_descs && left) {
-		desc++;
-		to_copy = RTE_MIN(desc->len, (uint32_t)left);
-		dlen = to_copy;
-		src = IOVA_TO_VVA(uint8_t *, vc_req, desc->addr, &dlen,
-				VHOST_ACCESS_RO);
-		if (unlikely(!src || !dlen)) {
-			VC_LOG_ERR("Failed to map descriptor");
+		copied = copy_data_from_desc(data, vc_req, desc, left);
+		if (copied == 0)
 			return -1;
-		}
-
-		rte_memcpy(data, src, dlen);
-		data += dlen;
-
-		if (unlikely(dlen < to_copy)) {
-			remain = to_copy - dlen;
-			addr = desc->addr + dlen;
-
-			while (remain) {
-				len = remain;
-				src = IOVA_TO_VVA(uint8_t *, vc_req, addr, &len,
-						VHOST_ACCESS_RO);
-				if (unlikely(!src || !len)) {
-					VC_LOG_ERR("Failed to map descriptor");
-					return -1;
-				}
-
-				rte_memcpy(data, src, len);
-				addr += len;
-				remain -= len;
-				data += len;
-			}
-		}
-
-		left -= to_copy;
-	}
+		left -= copied;
+		data = RTE_PTR_ADD(data, copied);
+		desc++;
+	} while (desc < head + max_n_descs && left != 0);
 
-	if (unlikely(left > 0)) {
-		VC_LOG_ERR("Incorrect virtio descriptor");
+	if (unlikely(left != 0))
 		return -1;
-	}
 
-	if (unlikely(desc - head == (int)max_n_descs))
+	if (unlikely(desc == head + max_n_descs))
 		*cur_desc = NULL;
 	else
 		*cur_desc = desc + 1;
@@ -852,6 +815,7 @@ prepare_sym_cipher_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 	/* iv */
 	if (unlikely(copy_data(iv_data, vc_req, head, &desc,
 			cipher->para.iv_len, max_n_descs))) {
+		VC_LOG_ERR("Incorrect virtio descriptor");
 		ret = VIRTIO_CRYPTO_BADMSG;
 		goto error_exit;
 	}
@@ -883,6 +847,7 @@ prepare_sym_cipher_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 		if (unlikely(copy_data(rte_pktmbuf_mtod(m_src, uint8_t *),
 				vc_req, head, &desc, cipher->para.src_data_len,
 				max_n_descs) < 0)) {
+			VC_LOG_ERR("Incorrect virtio descriptor");
 			ret = VIRTIO_CRYPTO_BADMSG;
 			goto error_exit;
 		}
@@ -1006,6 +971,7 @@ prepare_sym_chain_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 	/* iv */
 	if (unlikely(copy_data(iv_data, vc_req, head, &desc,
 			chain->para.iv_len, max_n_descs) < 0)) {
+		VC_LOG_ERR("Incorrect virtio descriptor");
 		ret = VIRTIO_CRYPTO_BADMSG;
 		goto error_exit;
 	}
@@ -1037,6 +1003,7 @@ prepare_sym_chain_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 		if (unlikely(copy_data(rte_pktmbuf_mtod(m_src, uint8_t *),
 				vc_req, head, &desc, chain->para.src_data_len,
 				max_n_descs) < 0)) {
+			VC_LOG_ERR("Incorrect virtio descriptor");
 			ret = VIRTIO_CRYPTO_BADMSG;
 			goto error_exit;
 		}
@@ -1121,6 +1088,7 @@ prepare_sym_chain_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 		if (unlikely(copy_data(digest_addr, vc_req, head, &digest_desc,
 				chain->para.hash_result_len,
 				max_n_descs) < 0)) {
+			VC_LOG_ERR("Incorrect virtio descriptor");
 			ret = VIRTIO_CRYPTO_BADMSG;
 			goto error_exit;
 		}
-- 
2.36.1


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

* [PATCH v2] test/ipsec: fix build with GCC 12
  2022-05-18 10:16 ` [PATCH 12/12] test/ipsec: " David Marchand
  2022-06-02 18:41   ` Medvedkin, Vladimir
  2022-06-11 15:38   ` Stephen Hemminger
@ 2022-06-16  9:33   ` David Marchand
  2022-06-17 12:06     ` David Marchand
  2022-06-16 14:46   ` [PATCH v3] vhost/crypto: " David Marchand
  3 siblings, 1 reply; 73+ messages in thread
From: David Marchand @ 2022-06-16  9:33 UTC (permalink / raw)
  To: dev
  Cc: bruce.richardson, stable, Konstantin Ananyev, Bernard Iremonger,
	Vladimir Medvedkin

GCC 12 raises the following warning:

In function ‘_mm256_loadu_si256’,
    inlined from ‘rte_mov32’ at
        ../lib/eal/x86/include/rte_memcpy.h:319:9,
    inlined from ‘rte_mov128’ at
        ../lib/eal/x86/include/rte_memcpy.h:344:2,
    inlined from ‘rte_memcpy_generic’ at
        ../lib/eal/x86/include/rte_memcpy.h:438:4,
    inlined from ‘rte_memcpy’ at
        ../lib/eal/x86/include/rte_memcpy.h:882:10,
    inlined from ‘setup_test_string.constprop’ at
        ../app/test/test_ipsec.c:572:4:
/usr/lib/gcc/x86_64-redhat-linux/12/include/avxintrin.h:929:10: error:
    array subscript ‘__m256i_u[3]’ is partly outside array bounds of
    ‘const char[108]’ [-Werror=array-bounds]
  929 |   return *__P;
      |          ^~~~
../app/test/test_ipsec.c: In function ‘setup_test_string.constprop’:
../app/test/test_ipsec.c:539:12: note: at offset 96 into object
    ‘null_plain_data’ of size 108
  539 | const char null_plain_data[] =
      |            ^~~~~~~~~~~~~~~

Add a hint so that the compiler understands the copied data is within
the passed string boundaries.

Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changes since v1:
- let the code as is, simply added a RTE_VERIFY hint,

---
 app/test/test_ipsec.c | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/app/test/test_ipsec.c b/app/test/test_ipsec.c
index 8da025bf66..7047e17960 100644
--- a/app/test/test_ipsec.c
+++ b/app/test/test_ipsec.c
@@ -554,12 +554,14 @@ struct rte_ipv4_hdr ipv4_outer  = {
 };
 
 static struct rte_mbuf *
-setup_test_string(struct rte_mempool *mpool,
-		const char *string, size_t len, uint8_t blocksize)
+setup_test_string(struct rte_mempool *mpool, const char *string,
+	size_t string_len, size_t len, uint8_t blocksize)
 {
 	struct rte_mbuf *m = rte_pktmbuf_alloc(mpool);
 	size_t t_len = len - (blocksize ? (len % blocksize) : 0);
 
+	RTE_VERIFY(len <= string_len);
+
 	if (m) {
 		memset(m->buf_addr, 0, m->buf_len);
 		char *dst = rte_pktmbuf_append(m, t_len);
@@ -1365,7 +1367,8 @@ test_ipsec_crypto_outb_burst_null_null(int i)
 	/* Generate input mbuf data */
 	for (j = 0; j < num_pkts && rc == 0; j++) {
 		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
-			null_plain_data, test_cfg[i].pkt_sz, 0);
+			null_plain_data, sizeof(null_plain_data),
+			test_cfg[i].pkt_sz, 0);
 		if (ut_params->ibuf[j] == NULL)
 			rc = TEST_FAILED;
 		else {
@@ -1483,7 +1486,8 @@ test_ipsec_inline_crypto_inb_burst_null_null(int i)
 			/* Generate test mbuf data */
 			ut_params->obuf[j] = setup_test_string(
 				ts_params->mbuf_pool,
-				null_plain_data, test_cfg[i].pkt_sz, 0);
+				null_plain_data, sizeof(null_plain_data),
+				test_cfg[i].pkt_sz, 0);
 			if (ut_params->obuf[j] == NULL)
 				rc = TEST_FAILED;
 		}
@@ -1551,16 +1555,17 @@ test_ipsec_inline_proto_inb_burst_null_null(int i)
 
 	/* Generate inbound mbuf data */
 	for (j = 0; j < num_pkts && rc == 0; j++) {
-		ut_params->ibuf[j] = setup_test_string(
-			ts_params->mbuf_pool,
-			null_plain_data, test_cfg[i].pkt_sz, 0);
+		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
+			null_plain_data, sizeof(null_plain_data),
+			test_cfg[i].pkt_sz, 0);
 		if (ut_params->ibuf[j] == NULL)
 			rc = TEST_FAILED;
 		else {
 			/* Generate test mbuf data */
 			ut_params->obuf[j] = setup_test_string(
 				ts_params->mbuf_pool,
-				null_plain_data, test_cfg[i].pkt_sz, 0);
+				null_plain_data, sizeof(null_plain_data),
+				test_cfg[i].pkt_sz, 0);
 			if (ut_params->obuf[j] == NULL)
 				rc = TEST_FAILED;
 		}
@@ -1660,7 +1665,8 @@ test_ipsec_inline_crypto_outb_burst_null_null(int i)
 	/* Generate test mbuf data */
 	for (j = 0; j < num_pkts && rc == 0; j++) {
 		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
-			null_plain_data, test_cfg[i].pkt_sz, 0);
+			null_plain_data, sizeof(null_plain_data),
+			test_cfg[i].pkt_sz, 0);
 		if (ut_params->ibuf[0] == NULL)
 			rc = TEST_FAILED;
 
@@ -1738,15 +1744,17 @@ test_ipsec_inline_proto_outb_burst_null_null(int i)
 	/* Generate test mbuf data */
 	for (j = 0; j < num_pkts && rc == 0; j++) {
 		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
-			null_plain_data, test_cfg[i].pkt_sz, 0);
+			null_plain_data, sizeof(null_plain_data),
+			test_cfg[i].pkt_sz, 0);
 		if (ut_params->ibuf[0] == NULL)
 			rc = TEST_FAILED;
 
 		if (rc == 0) {
 			/* Generate test tunneled mbuf data for comparison */
 			ut_params->obuf[j] = setup_test_string(
-					ts_params->mbuf_pool,
-					null_plain_data, test_cfg[i].pkt_sz, 0);
+				ts_params->mbuf_pool, null_plain_data,
+				sizeof(null_plain_data), test_cfg[i].pkt_sz,
+				0);
 			if (ut_params->obuf[j] == NULL)
 				rc = TEST_FAILED;
 		}
@@ -1815,7 +1823,8 @@ test_ipsec_lksd_proto_inb_burst_null_null(int i)
 	for (j = 0; j < num_pkts && rc == 0; j++) {
 		/* packet with sequence number 0 is invalid */
 		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
-			null_encrypted_data, test_cfg[i].pkt_sz, 0);
+			null_encrypted_data, sizeof(null_encrypted_data),
+			test_cfg[i].pkt_sz, 0);
 		if (ut_params->ibuf[j] == NULL)
 			rc = TEST_FAILED;
 	}
-- 
2.36.1


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

* RE: [EXT] Re: [PATCH 01/12] common/cpt: fix build with GCC 12
  2022-06-16  9:30       ` David Marchand
@ 2022-06-16 11:59         ` Ankur Dwivedi
  0 siblings, 0 replies; 73+ messages in thread
From: Ankur Dwivedi @ 2022-06-16 11:59 UTC (permalink / raw)
  To: David Marchand, Jerin Jacob Kollanukkaran
  Cc: Anoob Joseph, Thomas Monjalon, Ferruh Yigit, dpdk stable, dev,
	Akhil Goyal

Hi David,

>-----Original Message-----
>From: David Marchand <david.marchand@redhat.com>
>Sent: Thursday, June 16, 2022 3:00 PM
>To: Ankur Dwivedi <adwivedi@marvell.com>; Jerin Jacob Kollanukkaran
><jerinj@marvell.com>
>Cc: Anoob Joseph <anoobj@marvell.com>; Thomas Monjalon
><thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@xilinx.com>; dpdk stable
><stable@dpdk.org>; dev <dev@dpdk.org>; Akhil Goyal <gakhil@marvell.com>
>Subject: Re: [EXT] Re: [PATCH 01/12] common/cpt: fix build with GCC 12
>
>On Mon, Jun 13, 2022 at 1:40 PM Ankur Dwivedi <adwivedi@marvell.com>
>wrote:
>> >> For now, waive this warning until we have a proper fix.
>> >
>> >Both common/cpt and crypto/cnxk have the same code that triggers this
>> >warning.
>> >Can you look into this please?
>>
>> We will look into the issues in common/cpt and crypto/cnxk.
>
>Any update?

We are working on the changes. Will send the patch once it is complete.

>Thanks.
>
>
>--
>David Marchand

Regards,
Ankur

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

* [PATCH v3] vhost/crypto: fix build with GCC 12
  2022-05-18 10:16 ` [PATCH 12/12] test/ipsec: " David Marchand
                     ` (2 preceding siblings ...)
  2022-06-16  9:33   ` [PATCH v2] " David Marchand
@ 2022-06-16 14:46   ` David Marchand
  2022-06-17 13:59     ` Maxime Coquelin
  2022-06-21  9:31     ` Maxime Coquelin
  3 siblings, 2 replies; 73+ messages in thread
From: David Marchand @ 2022-06-16 14:46 UTC (permalink / raw)
  To: dev; +Cc: stable, Maxime Coquelin, Chenbo Xia, Fan Zhang

GCC 12 raises the following warning:

In file included from ../lib/mempool/rte_mempool.h:46,
                 from ../lib/mbuf/rte_mbuf.h:38,
                 from ../lib/vhost/vhost_crypto.c:7:
../lib/vhost/vhost_crypto.c: In function ‘rte_vhost_crypto_fetch_requests’:
../lib/eal/x86/include/rte_memcpy.h:371:9: warning: array subscript 1 is
     outside array bounds of ‘struct virtio_crypto_op_data_req[1]’
     [-Warray-bounds]
  371 | rte_mov32((uint8_t *)dst + 3 * 32, (const uint8_t *)src + 3 * 32);
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../lib/vhost/vhost_crypto.c:1178:42: note: while referencing ‘req’
 1178 |         struct virtio_crypto_op_data_req req;
      |                                          ^~~

Split this function and separate the per descriptor copy.
This makes the code clearer, and the compiler happier.

Note: logs for errors have been moved to callers to avoid duplicates.

Fixes: 3c79609fda7c ("vhost/crypto: handle virtually non-contiguous buffers")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changes since v2:
- fixed 32-bits build,

Changes since v1:
- refactored copy function,

---
 lib/vhost/vhost_crypto.c | 123 +++++++++++++++------------------------
 1 file changed, 46 insertions(+), 77 deletions(-)

diff --git a/lib/vhost/vhost_crypto.c b/lib/vhost/vhost_crypto.c
index b1c0eb6a0f..96ffb82a5d 100644
--- a/lib/vhost/vhost_crypto.c
+++ b/lib/vhost/vhost_crypto.c
@@ -565,94 +565,58 @@ get_data_ptr(struct vhost_crypto_data_req *vc_req,
 	return data;
 }
 
-static __rte_always_inline int
-copy_data(void *dst_data, struct vhost_crypto_data_req *vc_req,
-		struct vhost_crypto_desc *head,
-		struct vhost_crypto_desc **cur_desc,
-		uint32_t size, uint32_t max_n_descs)
+static __rte_always_inline uint32_t
+copy_data_from_desc(void *dst, struct vhost_crypto_data_req *vc_req,
+	struct vhost_crypto_desc *desc, uint32_t size)
 {
-	struct vhost_crypto_desc *desc = *cur_desc;
-	uint64_t remain, addr, dlen, len;
-	uint32_t to_copy;
-	uint8_t *data = dst_data;
-	uint8_t *src;
-	int left = size;
-
-	to_copy = RTE_MIN(desc->len, (uint32_t)left);
-	dlen = to_copy;
-	src = IOVA_TO_VVA(uint8_t *, vc_req, desc->addr, &dlen,
-			VHOST_ACCESS_RO);
-	if (unlikely(!src || !dlen))
-		return -1;
+	uint64_t remain;
+	uint64_t addr;
+
+	remain = RTE_MIN(desc->len, size);
+	addr = desc->addr;
+	do {
+		uint64_t len;
+		void *src;
+
+		len = remain;
+		src = IOVA_TO_VVA(void *, vc_req, addr, &len, VHOST_ACCESS_RO);
+		if (unlikely(src == NULL || len == 0))
+			return 0;
 
-	rte_memcpy((uint8_t *)data, src, dlen);
-	data += dlen;
+		rte_memcpy(dst, src, len);
+		remain -= len;
+		/* cast is needed for 32-bit architecture */
+		dst = RTE_PTR_ADD(dst, (size_t)len);
+		addr += len;
+	} while (unlikely(remain != 0));
 
-	if (unlikely(dlen < to_copy)) {
-		remain = to_copy - dlen;
-		addr = desc->addr + dlen;
+	return RTE_MIN(desc->len, size);
+}
 
-		while (remain) {
-			len = remain;
-			src = IOVA_TO_VVA(uint8_t *, vc_req, addr, &len,
-					VHOST_ACCESS_RO);
-			if (unlikely(!src || !len)) {
-				VC_LOG_ERR("Failed to map descriptor");
-				return -1;
-			}
 
-			rte_memcpy(data, src, len);
-			addr += len;
-			remain -= len;
-			data += len;
-		}
-	}
+static __rte_always_inline int
+copy_data(void *data, struct vhost_crypto_data_req *vc_req,
+	struct vhost_crypto_desc *head, struct vhost_crypto_desc **cur_desc,
+	uint32_t size, uint32_t max_n_descs)
+{
+	struct vhost_crypto_desc *desc = *cur_desc;
+	uint32_t left = size;
 
-	left -= to_copy;
+	do {
+		uint32_t copied;
 
-	while (desc >= head && desc - head < (int)max_n_descs && left) {
-		desc++;
-		to_copy = RTE_MIN(desc->len, (uint32_t)left);
-		dlen = to_copy;
-		src = IOVA_TO_VVA(uint8_t *, vc_req, desc->addr, &dlen,
-				VHOST_ACCESS_RO);
-		if (unlikely(!src || !dlen)) {
-			VC_LOG_ERR("Failed to map descriptor");
+		copied = copy_data_from_desc(data, vc_req, desc, left);
+		if (copied == 0)
 			return -1;
-		}
-
-		rte_memcpy(data, src, dlen);
-		data += dlen;
-
-		if (unlikely(dlen < to_copy)) {
-			remain = to_copy - dlen;
-			addr = desc->addr + dlen;
-
-			while (remain) {
-				len = remain;
-				src = IOVA_TO_VVA(uint8_t *, vc_req, addr, &len,
-						VHOST_ACCESS_RO);
-				if (unlikely(!src || !len)) {
-					VC_LOG_ERR("Failed to map descriptor");
-					return -1;
-				}
-
-				rte_memcpy(data, src, len);
-				addr += len;
-				remain -= len;
-				data += len;
-			}
-		}
-
-		left -= to_copy;
-	}
+		left -= copied;
+		data = RTE_PTR_ADD(data, copied);
+		desc++;
+	} while (desc < head + max_n_descs && left != 0);
 
-	if (unlikely(left > 0)) {
-		VC_LOG_ERR("Incorrect virtio descriptor");
+	if (unlikely(left != 0))
 		return -1;
-	}
 
-	if (unlikely(desc - head == (int)max_n_descs))
+	if (unlikely(desc == head + max_n_descs))
 		*cur_desc = NULL;
 	else
 		*cur_desc = desc + 1;
@@ -852,6 +816,7 @@ prepare_sym_cipher_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 	/* iv */
 	if (unlikely(copy_data(iv_data, vc_req, head, &desc,
 			cipher->para.iv_len, max_n_descs))) {
+		VC_LOG_ERR("Incorrect virtio descriptor");
 		ret = VIRTIO_CRYPTO_BADMSG;
 		goto error_exit;
 	}
@@ -883,6 +848,7 @@ prepare_sym_cipher_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 		if (unlikely(copy_data(rte_pktmbuf_mtod(m_src, uint8_t *),
 				vc_req, head, &desc, cipher->para.src_data_len,
 				max_n_descs) < 0)) {
+			VC_LOG_ERR("Incorrect virtio descriptor");
 			ret = VIRTIO_CRYPTO_BADMSG;
 			goto error_exit;
 		}
@@ -1006,6 +972,7 @@ prepare_sym_chain_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 	/* iv */
 	if (unlikely(copy_data(iv_data, vc_req, head, &desc,
 			chain->para.iv_len, max_n_descs) < 0)) {
+		VC_LOG_ERR("Incorrect virtio descriptor");
 		ret = VIRTIO_CRYPTO_BADMSG;
 		goto error_exit;
 	}
@@ -1037,6 +1004,7 @@ prepare_sym_chain_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 		if (unlikely(copy_data(rte_pktmbuf_mtod(m_src, uint8_t *),
 				vc_req, head, &desc, chain->para.src_data_len,
 				max_n_descs) < 0)) {
+			VC_LOG_ERR("Incorrect virtio descriptor");
 			ret = VIRTIO_CRYPTO_BADMSG;
 			goto error_exit;
 		}
@@ -1121,6 +1089,7 @@ prepare_sym_chain_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op,
 		if (unlikely(copy_data(digest_addr, vc_req, head, &digest_desc,
 				chain->para.hash_result_len,
 				max_n_descs) < 0)) {
+			VC_LOG_ERR("Incorrect virtio descriptor");
 			ret = VIRTIO_CRYPTO_BADMSG;
 			goto error_exit;
 		}
-- 
2.36.1


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

* Re: [PATCH v2] vhost/crypto: fix build with GCC 12
  2022-06-16  9:32   ` [PATCH v2] " David Marchand
@ 2022-06-16 14:53     ` David Marchand
  0 siblings, 0 replies; 73+ messages in thread
From: David Marchand @ 2022-06-16 14:53 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, dpdk stable, Maxime Coquelin, Chenbo Xia, Fan Zhang

On Thu, Jun 16, 2022 at 11:32 AM David Marchand
<david.marchand@redhat.com> wrote:
>
> GCC 12 raises the following warning:
>
> In file included from ../lib/mempool/rte_mempool.h:46,
>                  from ../lib/mbuf/rte_mbuf.h:38,
>                  from ../lib/vhost/vhost_crypto.c:7:
> ../lib/vhost/vhost_crypto.c: In function ‘rte_vhost_crypto_fetch_requests’:
> ../lib/eal/x86/include/rte_memcpy.h:371:9: warning: array subscript 1 is
>      outside array bounds of ‘struct virtio_crypto_op_data_req[1]’
>      [-Warray-bounds]
>   371 | rte_mov32((uint8_t *)dst + 3 * 32, (const uint8_t *)src + 3 * 32);
>       | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../lib/vhost/vhost_crypto.c:1178:42: note: while referencing ‘req’
>  1178 |         struct virtio_crypto_op_data_req req;
>       |                                          ^~~
>
> Split this function and separate the per descriptor copy.
> This makes the code clearer, and the compiler happier.
>
> Note: logs for errors have been moved to callers to avoid duplicates.
>
> Fixes: 3c79609fda7c ("vhost/crypto: handle virtually non-contiguous buffers")
> Cc: stable@dpdk.org
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Build fails on 32-bit.
I had seen the issue while testing but sent a non amended patch... v3 incoming.

-- 
David Marchand


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

* Re: [PATCH v2] test/ipsec: fix build with GCC 12
  2022-06-16  9:33   ` [PATCH v2] " David Marchand
@ 2022-06-17 12:06     ` David Marchand
  2022-06-20  9:07       ` [EXT] " Akhil Goyal
  2022-06-20 15:06       ` Medvedkin, Vladimir
  0 siblings, 2 replies; 73+ messages in thread
From: David Marchand @ 2022-06-17 12:06 UTC (permalink / raw)
  To: dev
  Cc: Bruce Richardson, dpdk stable, Konstantin Ananyev,
	Bernard Iremonger, Vladimir Medvedkin

On Thu, Jun 16, 2022 at 11:33 AM David Marchand
<david.marchand@redhat.com> wrote:
>
> GCC 12 raises the following warning:
>
> In function ‘_mm256_loadu_si256’,
>     inlined from ‘rte_mov32’ at
>         ../lib/eal/x86/include/rte_memcpy.h:319:9,
>     inlined from ‘rte_mov128’ at
>         ../lib/eal/x86/include/rte_memcpy.h:344:2,
>     inlined from ‘rte_memcpy_generic’ at
>         ../lib/eal/x86/include/rte_memcpy.h:438:4,
>     inlined from ‘rte_memcpy’ at
>         ../lib/eal/x86/include/rte_memcpy.h:882:10,
>     inlined from ‘setup_test_string.constprop’ at
>         ../app/test/test_ipsec.c:572:4:
> /usr/lib/gcc/x86_64-redhat-linux/12/include/avxintrin.h:929:10: error:
>     array subscript ‘__m256i_u[3]’ is partly outside array bounds of
>     ‘const char[108]’ [-Werror=array-bounds]
>   929 |   return *__P;
>       |          ^~~~
> ../app/test/test_ipsec.c: In function ‘setup_test_string.constprop’:
> ../app/test/test_ipsec.c:539:12: note: at offset 96 into object
>     ‘null_plain_data’ of size 108
>   539 | const char null_plain_data[] =
>       |            ^~~~~~~~~~~~~~~
>
> Add a hint so that the compiler understands the copied data is within
> the passed string boundaries.
>

Ferruh had opened a bz.

Bugzilla ID: 848
> Cc: stable@dpdk.org
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>


-- 
David Marchand


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

* Re: [PATCH v3] vhost/crypto: fix build with GCC 12
  2022-06-16 14:46   ` [PATCH v3] vhost/crypto: " David Marchand
@ 2022-06-17 13:59     ` Maxime Coquelin
  2022-06-21  9:31     ` Maxime Coquelin
  1 sibling, 0 replies; 73+ messages in thread
From: Maxime Coquelin @ 2022-06-17 13:59 UTC (permalink / raw)
  To: David Marchand, dev; +Cc: stable, Chenbo Xia, Fan Zhang



On 6/16/22 16:46, David Marchand wrote:
> GCC 12 raises the following warning:
> 
> In file included from ../lib/mempool/rte_mempool.h:46,
>                   from ../lib/mbuf/rte_mbuf.h:38,
>                   from ../lib/vhost/vhost_crypto.c:7:
> ../lib/vhost/vhost_crypto.c: In function ‘rte_vhost_crypto_fetch_requests’:
> ../lib/eal/x86/include/rte_memcpy.h:371:9: warning: array subscript 1 is
>       outside array bounds of ‘struct virtio_crypto_op_data_req[1]’
>       [-Warray-bounds]
>    371 | rte_mov32((uint8_t *)dst + 3 * 32, (const uint8_t *)src + 3 * 32);
>        | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../lib/vhost/vhost_crypto.c:1178:42: note: while referencing ‘req’
>   1178 |         struct virtio_crypto_op_data_req req;
>        |                                          ^~~
> 
> Split this function and separate the per descriptor copy.
> This makes the code clearer, and the compiler happier.
> 
> Note: logs for errors have been moved to callers to avoid duplicates.
> 
> Fixes: 3c79609fda7c ("vhost/crypto: handle virtually non-contiguous buffers")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> Changes since v2:
> - fixed 32-bits build,
> 
> Changes since v1:
> - refactored copy function,
> 
> ---
>   lib/vhost/vhost_crypto.c | 123 +++++++++++++++------------------------
>   1 file changed, 46 insertions(+), 77 deletions(-)
> 

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

I'll wait until tuesday to apply it to my tree to let some time for testing

Thanks,
Maxime


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

* RE: [EXT] Re: [PATCH v2] test/ipsec: fix build with GCC 12
  2022-06-17 12:06     ` David Marchand
@ 2022-06-20  9:07       ` Akhil Goyal
  2022-06-20 15:06       ` Medvedkin, Vladimir
  1 sibling, 0 replies; 73+ messages in thread
From: Akhil Goyal @ 2022-06-20  9:07 UTC (permalink / raw)
  To: David Marchand, dev
  Cc: Bruce Richardson, dpdk stable, Konstantin Ananyev,
	Bernard Iremonger, Vladimir Medvedkin

> On Thu, Jun 16, 2022 at 11:33 AM David Marchand
> <david.marchand@redhat.com> wrote:
> >
> > GCC 12 raises the following warning:
> >
> > In function ‘_mm256_loadu_si256’,
> >     inlined from ‘rte_mov32’ at
> >         ../lib/eal/x86/include/rte_memcpy.h:319:9,
> >     inlined from ‘rte_mov128’ at
> >         ../lib/eal/x86/include/rte_memcpy.h:344:2,
> >     inlined from ‘rte_memcpy_generic’ at
> >         ../lib/eal/x86/include/rte_memcpy.h:438:4,
> >     inlined from ‘rte_memcpy’ at
> >         ../lib/eal/x86/include/rte_memcpy.h:882:10,
> >     inlined from ‘setup_test_string.constprop’ at
> >         ../app/test/test_ipsec.c:572:4:
> > /usr/lib/gcc/x86_64-redhat-linux/12/include/avxintrin.h:929:10: error:
> >     array subscript ‘__m256i_u[3]’ is partly outside array bounds of
> >     ‘const char[108]’ [-Werror=array-bounds]
> >   929 |   return *__P;
> >       |          ^~~~
> > ../app/test/test_ipsec.c: In function ‘setup_test_string.constprop’:
> > ../app/test/test_ipsec.c:539:12: note: at offset 96 into object
> >     ‘null_plain_data’ of size 108
> >   539 | const char null_plain_data[] =
> >       |            ^~~~~~~~~~~~~~~
> >
> > Add a hint so that the compiler understands the copied data is within
> > the passed string boundaries.
> >
> 
> Ferruh had opened a bz.
> 
> Bugzilla ID: 848
Added Fixes tag also.

Applied to dpdk-next-crypto

Thanks.

> > Cc: stable@dpdk.org
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>


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

* Re: [PATCH v2] test/ipsec: fix build with GCC 12
  2022-06-17 12:06     ` David Marchand
  2022-06-20  9:07       ` [EXT] " Akhil Goyal
@ 2022-06-20 15:06       ` Medvedkin, Vladimir
  1 sibling, 0 replies; 73+ messages in thread
From: Medvedkin, Vladimir @ 2022-06-20 15:06 UTC (permalink / raw)
  To: David Marchand, dev
  Cc: Bruce Richardson, dpdk stable, Konstantin Ananyev, Bernard Iremonger


On 17/06/2022 13:06, David Marchand wrote:
> On Thu, Jun 16, 2022 at 11:33 AM David Marchand
> <david.marchand@redhat.com> wrote:
>> GCC 12 raises the following warning:
>>
>> In function ‘_mm256_loadu_si256’,
>>      inlined from ‘rte_mov32’ at
>>          ../lib/eal/x86/include/rte_memcpy.h:319:9,
>>      inlined from ‘rte_mov128’ at
>>          ../lib/eal/x86/include/rte_memcpy.h:344:2,
>>      inlined from ‘rte_memcpy_generic’ at
>>          ../lib/eal/x86/include/rte_memcpy.h:438:4,
>>      inlined from ‘rte_memcpy’ at
>>          ../lib/eal/x86/include/rte_memcpy.h:882:10,
>>      inlined from ‘setup_test_string.constprop’ at
>>          ../app/test/test_ipsec.c:572:4:
>> /usr/lib/gcc/x86_64-redhat-linux/12/include/avxintrin.h:929:10: error:
>>      array subscript ‘__m256i_u[3]’ is partly outside array bounds of
>>      ‘const char[108]’ [-Werror=array-bounds]
>>    929 |   return *__P;
>>        |          ^~~~
>> ../app/test/test_ipsec.c: In function ‘setup_test_string.constprop’:
>> ../app/test/test_ipsec.c:539:12: note: at offset 96 into object
>>      ‘null_plain_data’ of size 108
>>    539 | const char null_plain_data[] =
>>        |            ^~~~~~~~~~~~~~~
>>
>> Add a hint so that the compiler understands the copied data is within
>> the passed string boundaries.
>>
> Ferruh had opened a bz.
>
> Bugzilla ID: 848
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: David Marchand <david.marchand@redhat.com>


Acked-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>

>
-- 
Regards,
Vladimir


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

* Re: [PATCH v3] vhost/crypto: fix build with GCC 12
  2022-06-16 14:46   ` [PATCH v3] vhost/crypto: " David Marchand
  2022-06-17 13:59     ` Maxime Coquelin
@ 2022-06-21  9:31     ` Maxime Coquelin
  2022-06-22  9:01       ` Poczatek, Jakub
  1 sibling, 1 reply; 73+ messages in thread
From: Maxime Coquelin @ 2022-06-21  9:31 UTC (permalink / raw)
  To: David Marchand, dev; +Cc: stable, Chenbo Xia, Fan Zhang



On 6/16/22 16:46, David Marchand wrote:
> GCC 12 raises the following warning:
> 
> In file included from ../lib/mempool/rte_mempool.h:46,
>                   from ../lib/mbuf/rte_mbuf.h:38,
>                   from ../lib/vhost/vhost_crypto.c:7:
> ../lib/vhost/vhost_crypto.c: In function ‘rte_vhost_crypto_fetch_requests’:
> ../lib/eal/x86/include/rte_memcpy.h:371:9: warning: array subscript 1 is
>       outside array bounds of ‘struct virtio_crypto_op_data_req[1]’
>       [-Warray-bounds]
>    371 | rte_mov32((uint8_t *)dst + 3 * 32, (const uint8_t *)src + 3 * 32);
>        | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../lib/vhost/vhost_crypto.c:1178:42: note: while referencing ‘req’
>   1178 |         struct virtio_crypto_op_data_req req;
>        |                                          ^~~
> 
> Split this function and separate the per descriptor copy.
> This makes the code clearer, and the compiler happier.
> 
> Note: logs for errors have been moved to callers to avoid duplicates.
> 
> Fixes: 3c79609fda7c ("vhost/crypto: handle virtually non-contiguous buffers")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> Changes since v2:
> - fixed 32-bits build,
> 
> Changes since v1:
> - refactored copy function,
> 
> ---
>   lib/vhost/vhost_crypto.c | 123 +++++++++++++++------------------------
>   1 file changed, 46 insertions(+), 77 deletions(-)
> 

Applied to dpdk-next-virtio/main.

Thanks,
Maxime


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

* Re: [PATCH 08/12] net/qede/base: fix build with GCC 12
  2022-05-18 10:16 ` [PATCH 08/12] net/qede/base: " David Marchand
  2022-05-20 20:29   ` Stephen Hemminger
@ 2022-06-21 23:17   ` Stephen Hemminger
  2022-06-22 15:42     ` David Marchand
  1 sibling, 1 reply; 73+ messages in thread
From: Stephen Hemminger @ 2022-06-21 23:17 UTC (permalink / raw)
  To: David Marchand
  Cc: dev, thomas, ferruh.yigit, stable, Rasesh Mody, Devendra Singh Rawat

On Wed, 18 May 2022 12:16:53 +0200
David Marchand <david.marchand@redhat.com> wrote:

> GCC raises the following warning:
> 
> In function ‘_mm256_storeu_si256’,
>     inlined from ‘rte_mov32’ at
>         ../lib/eal/x86/include/rte_memcpy.h:320:2,
>     inlined from ‘rte_mov128’ at
>         ../lib/eal/x86/include/rte_memcpy.h:342:2,
>     inlined from ‘rte_memcpy_generic’ at
>         ../lib/eal/x86/include/rte_memcpy.h:438:4,
>     inlined from ‘rte_memcpy’ at
>         ../lib/eal/x86/include/rte_memcpy.h:882:10,
>     inlined from ‘__ecore_mcp_cmd_and_union’ at
>         ../drivers/net/qede/base/ecore_mcp.c:541:3,
>     inlined from ‘_ecore_mcp_cmd_and_union’ at
>         ../drivers/net/qede/base/ecore_mcp.c:638:2,
>     inlined from ‘ecore_mcp_cmd_and_union’ at
>         ../drivers/net/qede/base/ecore_mcp.c:742:9:
> /usr/lib/gcc/x86_64-redhat-linux/12/include/avxintrin.h:935:8: error:
>         array subscript 1 is outside array bounds of
>         ‘union drv_union_data[1]’ [-Werror=array-bounds]
>   935 |   *__P = __A;
>       |   ~~~~~^~~~~
> ../drivers/net/qede/base/ecore_mcp.c: In function
>         ‘ecore_mcp_cmd_and_union’:
> ../drivers/net/qede/base/ecore_mcp.c:533:30: note: at offset 32 into
>         object ‘union_data’ of size 32
>   533 |         union drv_union_data union_data;
>       |                              ^~~~~~~~~~
> 
> Since this code is in the base driver, waive the check until the base
> driver is fixed by the relevant people.

Even there are two maintainers, haven't heard a response from them.
It could be a real bug.



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

* RE: [PATCH v3] vhost/crypto: fix build with GCC 12
  2022-06-21  9:31     ` Maxime Coquelin
@ 2022-06-22  9:01       ` Poczatek, Jakub
  2022-06-22  9:26         ` Zhang, Roy Fan
  2022-06-22 14:07         ` David Marchand
  0 siblings, 2 replies; 73+ messages in thread
From: Poczatek, Jakub @ 2022-06-22  9:01 UTC (permalink / raw)
  To: Maxime Coquelin, David Marchand, dev; +Cc: stable, Xia, Chenbo, Zhang, Roy Fan

Hey everyone, 

When running a Virtio performance test on a VM using VHost with this patch applied,
VHost gives the following error message: 

> VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_FEATURES
> VHOST_CONFIG: (/tmp/vhost) negotiated Virtio features: 0x0
> VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_MEM_TABLE
> VHOST_CONFIG: (/tmp/vhost) guest memory region size: 0x80000000
> VHOST_CONFIG: (/tmp/vhost)       guest physical addr: 0x0
> VHOST_CONFIG: (/tmp/vhost)       guest virtual  addr: 0x7f17c0000000
> VHOST_CONFIG: (/tmp/vhost)       host  virtual  addr: 0x7f94c0000000
> VHOST_CONFIG: (/tmp/vhost)       mmap addr : 0x7f94c0000000
> VHOST_CONFIG: (/tmp/vhost)       mmap size : 0x80000000
> VHOST_CONFIG: (/tmp/vhost)       mmap align: 0x40000000
> VHOST_CONFIG: (/tmp/vhost)       mmap off  : 0x0
> VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_VRING_NUM
> VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_VRING_BASE
> VHOST_CONFIG: (/tmp/vhost) vring base idx:0 last_used_idx:0 last_avail_idx:0.
> VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_VRING_ADDR
> VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_VRING_KICK
> VHOST_CONFIG: (/tmp/vhost) vring kick idx:0 file:37
> VHOST_CONFIG: (/tmp/vhost) reallocated virtqueue on node 1
> VHOST_CONFIG: (/tmp/vhost) reallocated device on node 1
> VHOST_CONFIG: (/tmp/vhost) virtio is now ready for processing.
> USER1: New Vhost-crypto Device /tmp/vhost, Device ID 0
> VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_VRING_CALL
> VHOST_CONFIG: (/tmp/vhost) vring call idx:0 file:38
> USER1: [VHOST-Crypto]: Session 1 created for vdev 0.
> USER1: [VHOST-Crypto]: Incorrect virtio descriptor
> USER1: [VHOST-Crypto]: Failed to process sym request
> USER1: [VHOST-Crypto]: Incorrect virtio descriptor
> USER1: [VHOST-Crypto]: Failed to process sym request

Due to this, performance test hangs and never finishes. 

Kind Regards, 
Jakub Poczatek

-----Original Message-----
From: Maxime Coquelin <maxime.coquelin@redhat.com> 
Sent: Tuesday 21 June 2022 10:31
To: David Marchand <david.marchand@redhat.com>; dev@dpdk.org
Cc: stable@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>
Subject: Re: [PATCH v3] vhost/crypto: fix build with GCC 12



On 6/16/22 16:46, David Marchand wrote:
> GCC 12 raises the following warning:
> 
> In file included from ../lib/mempool/rte_mempool.h:46,
>                   from ../lib/mbuf/rte_mbuf.h:38,
>                   from ../lib/vhost/vhost_crypto.c:7:
> ../lib/vhost/vhost_crypto.c: In function ‘rte_vhost_crypto_fetch_requests’:
> ../lib/eal/x86/include/rte_memcpy.h:371:9: warning: array subscript 1 is
>       outside array bounds of ‘struct virtio_crypto_op_data_req[1]’
>       [-Warray-bounds]
>    371 | rte_mov32((uint8_t *)dst + 3 * 32, (const uint8_t *)src + 3 * 32);
>        | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../lib/vhost/vhost_crypto.c:1178:42: note: while referencing ‘req’
>   1178 |         struct virtio_crypto_op_data_req req;
>        |                                          ^~~
> 
> Split this function and separate the per descriptor copy.
> This makes the code clearer, and the compiler happier.
> 
> Note: logs for errors have been moved to callers to avoid duplicates.
> 
> Fixes: 3c79609fda7c ("vhost/crypto: handle virtually non-contiguous buffers")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> Changes since v2:
> - fixed 32-bits build,
> 
> Changes since v1:
> - refactored copy function,
> 
> ---
>   lib/vhost/vhost_crypto.c | 123 +++++++++++++++------------------------
>   1 file changed, 46 insertions(+), 77 deletions(-)
> 

Applied to dpdk-next-virtio/main.

Thanks,
Maxime


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

* RE: [PATCH v3] vhost/crypto: fix build with GCC 12
  2022-06-22  9:01       ` Poczatek, Jakub
@ 2022-06-22  9:26         ` Zhang, Roy Fan
  2022-06-22 14:07         ` David Marchand
  1 sibling, 0 replies; 73+ messages in thread
From: Zhang, Roy Fan @ 2022-06-22  9:26 UTC (permalink / raw)
  To: Poczatek, Jakub, Maxime Coquelin, David Marchand, dev; +Cc: stable, Xia, Chenbo

Hi Maxime,

I know it is over Tuesday so we understand you merged the patch already.
But any suggestions? Should we raise a Bugzilla for this problem?
BTW we reverted the patch and the test finished no problem.

Regards,
Fan
> -----Original Message-----
> From: Poczatek, Jakub <jakub.poczatek@intel.com>
> Sent: Wednesday, June 22, 2022 10:02 AM
> To: Maxime Coquelin <maxime.coquelin@redhat.com>; David Marchand
> <david.marchand@redhat.com>; dev@dpdk.org
> Cc: stable@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>
> Subject: RE: [PATCH v3] vhost/crypto: fix build with GCC 12
> 
> Hey everyone,
> 
> When running a Virtio performance test on a VM using VHost with this patch
> applied,
> VHost gives the following error message:
> 
> > VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_FEATURES
> > VHOST_CONFIG: (/tmp/vhost) negotiated Virtio features: 0x0
> > VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_MEM_TABLE
> > VHOST_CONFIG: (/tmp/vhost) guest memory region size: 0x80000000
> > VHOST_CONFIG: (/tmp/vhost)       guest physical addr: 0x0
> > VHOST_CONFIG: (/tmp/vhost)       guest virtual  addr: 0x7f17c0000000
> > VHOST_CONFIG: (/tmp/vhost)       host  virtual  addr: 0x7f94c0000000
> > VHOST_CONFIG: (/tmp/vhost)       mmap addr : 0x7f94c0000000
> > VHOST_CONFIG: (/tmp/vhost)       mmap size : 0x80000000
> > VHOST_CONFIG: (/tmp/vhost)       mmap align: 0x40000000
> > VHOST_CONFIG: (/tmp/vhost)       mmap off  : 0x0
> > VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_VRING_NUM
> > VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_VRING_BASE
> > VHOST_CONFIG: (/tmp/vhost) vring base idx:0 last_used_idx:0
> last_avail_idx:0.
> > VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_VRING_ADDR
> > VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_VRING_KICK
> > VHOST_CONFIG: (/tmp/vhost) vring kick idx:0 file:37
> > VHOST_CONFIG: (/tmp/vhost) reallocated virtqueue on node 1
> > VHOST_CONFIG: (/tmp/vhost) reallocated device on node 1
> > VHOST_CONFIG: (/tmp/vhost) virtio is now ready for processing.
> > USER1: New Vhost-crypto Device /tmp/vhost, Device ID 0
> > VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_VRING_CALL
> > VHOST_CONFIG: (/tmp/vhost) vring call idx:0 file:38
> > USER1: [VHOST-Crypto]: Session 1 created for vdev 0.
> > USER1: [VHOST-Crypto]: Incorrect virtio descriptor
> > USER1: [VHOST-Crypto]: Failed to process sym request
> > USER1: [VHOST-Crypto]: Incorrect virtio descriptor
> > USER1: [VHOST-Crypto]: Failed to process sym request
> 
> Due to this, performance test hangs and never finishes.
> 
> Kind Regards,
> Jakub Poczatek
> 
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Tuesday 21 June 2022 10:31
> To: David Marchand <david.marchand@redhat.com>; dev@dpdk.org
> Cc: stable@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>
> Subject: Re: [PATCH v3] vhost/crypto: fix build with GCC 12
> 
> 
> 
> On 6/16/22 16:46, David Marchand wrote:
> > GCC 12 raises the following warning:
> >
> > In file included from ../lib/mempool/rte_mempool.h:46,
> >                   from ../lib/mbuf/rte_mbuf.h:38,
> >                   from ../lib/vhost/vhost_crypto.c:7:
> > ../lib/vhost/vhost_crypto.c: In function ‘rte_vhost_crypto_fetch_requests’:
> > ../lib/eal/x86/include/rte_memcpy.h:371:9: warning: array subscript 1 is
> >       outside array bounds of ‘struct virtio_crypto_op_data_req[1]’
> >       [-Warray-bounds]
> >    371 | rte_mov32((uint8_t *)dst + 3 * 32, (const uint8_t *)src + 3 * 32);
> >        |
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > ../lib/vhost/vhost_crypto.c:1178:42: note: while referencing ‘req’
> >   1178 |         struct virtio_crypto_op_data_req req;
> >        |                                          ^~~
> >
> > Split this function and separate the per descriptor copy.
> > This makes the code clearer, and the compiler happier.
> >
> > Note: logs for errors have been moved to callers to avoid duplicates.
> >
> > Fixes: 3c79609fda7c ("vhost/crypto: handle virtually non-contiguous buffers")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> > Changes since v2:
> > - fixed 32-bits build,
> >
> > Changes since v1:
> > - refactored copy function,
> >
> > ---
> >   lib/vhost/vhost_crypto.c | 123 +++++++++++++++------------------------
> >   1 file changed, 46 insertions(+), 77 deletions(-)
> >
> 
> Applied to dpdk-next-virtio/main.
> 
> Thanks,
> Maxime


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

* Re: [PATCH v3] vhost/crypto: fix build with GCC 12
  2022-06-22  9:01       ` Poczatek, Jakub
  2022-06-22  9:26         ` Zhang, Roy Fan
@ 2022-06-22 14:07         ` David Marchand
  2022-06-22 15:21           ` Poczatek, Jakub
  1 sibling, 1 reply; 73+ messages in thread
From: David Marchand @ 2022-06-22 14:07 UTC (permalink / raw)
  To: Poczatek, Jakub, Zhang, Roy Fan; +Cc: Maxime Coquelin, dev, stable, Xia, Chenbo

Hello Jakub, Roy,

On Wed, Jun 22, 2022 at 11:01 AM Poczatek, Jakub
<jakub.poczatek@intel.com> wrote:
> When running a Virtio performance test on a VM using VHost with this patch applied,
> VHost gives the following error message:
>
> > VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_FEATURES
> > VHOST_CONFIG: (/tmp/vhost) negotiated Virtio features: 0x0
> > VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_MEM_TABLE
> > VHOST_CONFIG: (/tmp/vhost) guest memory region size: 0x80000000
> > VHOST_CONFIG: (/tmp/vhost)       guest physical addr: 0x0
> > VHOST_CONFIG: (/tmp/vhost)       guest virtual  addr: 0x7f17c0000000
> > VHOST_CONFIG: (/tmp/vhost)       host  virtual  addr: 0x7f94c0000000
> > VHOST_CONFIG: (/tmp/vhost)       mmap addr : 0x7f94c0000000
> > VHOST_CONFIG: (/tmp/vhost)       mmap size : 0x80000000
> > VHOST_CONFIG: (/tmp/vhost)       mmap align: 0x40000000
> > VHOST_CONFIG: (/tmp/vhost)       mmap off  : 0x0
> > VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_VRING_NUM
> > VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_VRING_BASE
> > VHOST_CONFIG: (/tmp/vhost) vring base idx:0 last_used_idx:0 last_avail_idx:0.
> > VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_VRING_ADDR
> > VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_VRING_KICK
> > VHOST_CONFIG: (/tmp/vhost) vring kick idx:0 file:37
> > VHOST_CONFIG: (/tmp/vhost) reallocated virtqueue on node 1
> > VHOST_CONFIG: (/tmp/vhost) reallocated device on node 1
> > VHOST_CONFIG: (/tmp/vhost) virtio is now ready for processing.
> > USER1: New Vhost-crypto Device /tmp/vhost, Device ID 0
> > VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_VRING_CALL
> > VHOST_CONFIG: (/tmp/vhost) vring call idx:0 file:38
> > USER1: [VHOST-Crypto]: Session 1 created for vdev 0.
> > USER1: [VHOST-Crypto]: Incorrect virtio descriptor
> > USER1: [VHOST-Crypto]: Failed to process sym request
> > USER1: [VHOST-Crypto]: Incorrect virtio descriptor
> > USER1: [VHOST-Crypto]: Failed to process sym request

Could you test with the following snippet:

diff --git a/lib/vhost/vhost_crypto.c b/lib/vhost/vhost_crypto.c
index 96ffb82a5d..54946f46d9 100644
--- a/lib/vhost/vhost_crypto.c
+++ b/lib/vhost/vhost_crypto.c
@@ -610,8 +610,7 @@ copy_data(void *data, struct vhost_crypto_data_req *vc_req,
                        return -1;
                left -= copied;
                data = RTE_PTR_ADD(data, copied);
-               desc++;
-       } while (desc < head + max_n_descs && left != 0);
+       } while (left != 0 && ++desc < head + max_n_descs);

        if (unlikely(left != 0))
                return -1;


-- 
David Marchand


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

* RE: [PATCH v3] vhost/crypto: fix build with GCC 12
  2022-06-22 14:07         ` David Marchand
@ 2022-06-22 15:21           ` Poczatek, Jakub
  2022-06-22 15:31             ` David Marchand
  0 siblings, 1 reply; 73+ messages in thread
From: Poczatek, Jakub @ 2022-06-22 15:21 UTC (permalink / raw)
  To: David Marchand, Zhang, Roy Fan; +Cc: Maxime Coquelin, dev, stable, Xia, Chenbo

Hey David, 

The code change fixes the errors and the performance test completes. 

Kind Regards, 
Jakub Poczatek

-----Original Message-----
From: David Marchand <david.marchand@redhat.com> 
Sent: Wednesday 22 June 2022 15:08
To: Poczatek, Jakub <jakub.poczatek@intel.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>
Cc: Maxime Coquelin <maxime.coquelin@redhat.com>; dev@dpdk.org; stable@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>
Subject: Re: [PATCH v3] vhost/crypto: fix build with GCC 12

Hello Jakub, Roy,

On Wed, Jun 22, 2022 at 11:01 AM Poczatek, Jakub <jakub.poczatek@intel.com> wrote:
> When running a Virtio performance test on a VM using VHost with this 
> patch applied, VHost gives the following error message:
>
> > VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_FEATURES
> > VHOST_CONFIG: (/tmp/vhost) negotiated Virtio features: 0x0
> > VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_MEM_TABLE
> > VHOST_CONFIG: (/tmp/vhost) guest memory region size: 0x80000000
> > VHOST_CONFIG: (/tmp/vhost)       guest physical addr: 0x0
> > VHOST_CONFIG: (/tmp/vhost)       guest virtual  addr: 0x7f17c0000000
> > VHOST_CONFIG: (/tmp/vhost)       host  virtual  addr: 0x7f94c0000000
> > VHOST_CONFIG: (/tmp/vhost)       mmap addr : 0x7f94c0000000
> > VHOST_CONFIG: (/tmp/vhost)       mmap size : 0x80000000
> > VHOST_CONFIG: (/tmp/vhost)       mmap align: 0x40000000
> > VHOST_CONFIG: (/tmp/vhost)       mmap off  : 0x0
> > VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_VRING_NUM
> > VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_VRING_BASE
> > VHOST_CONFIG: (/tmp/vhost) vring base idx:0 last_used_idx:0 last_avail_idx:0.
> > VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_VRING_ADDR
> > VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_VRING_KICK
> > VHOST_CONFIG: (/tmp/vhost) vring kick idx:0 file:37
> > VHOST_CONFIG: (/tmp/vhost) reallocated virtqueue on node 1
> > VHOST_CONFIG: (/tmp/vhost) reallocated device on node 1
> > VHOST_CONFIG: (/tmp/vhost) virtio is now ready for processing.
> > USER1: New Vhost-crypto Device /tmp/vhost, Device ID 0
> > VHOST_CONFIG: (/tmp/vhost) read message VHOST_USER_SET_VRING_CALL
> > VHOST_CONFIG: (/tmp/vhost) vring call idx:0 file:38
> > USER1: [VHOST-Crypto]: Session 1 created for vdev 0.
> > USER1: [VHOST-Crypto]: Incorrect virtio descriptor
> > USER1: [VHOST-Crypto]: Failed to process sym request
> > USER1: [VHOST-Crypto]: Incorrect virtio descriptor
> > USER1: [VHOST-Crypto]: Failed to process sym request

Could you test with the following snippet:

diff --git a/lib/vhost/vhost_crypto.c b/lib/vhost/vhost_crypto.c index 96ffb82a5d..54946f46d9 100644
--- a/lib/vhost/vhost_crypto.c
+++ b/lib/vhost/vhost_crypto.c
@@ -610,8 +610,7 @@ copy_data(void *data, struct vhost_crypto_data_req *vc_req,
                        return -1;
                left -= copied;
                data = RTE_PTR_ADD(data, copied);
-               desc++;
-       } while (desc < head + max_n_descs && left != 0);
+       } while (left != 0 && ++desc < head + max_n_descs);

        if (unlikely(left != 0))
                return -1;


--
David Marchand


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

* Re: [PATCH v3] vhost/crypto: fix build with GCC 12
  2022-06-22 15:21           ` Poczatek, Jakub
@ 2022-06-22 15:31             ` David Marchand
  0 siblings, 0 replies; 73+ messages in thread
From: David Marchand @ 2022-06-22 15:31 UTC (permalink / raw)
  To: Poczatek, Jakub, Maxime Coquelin
  Cc: Zhang, Roy Fan, dev, stable, Xia, Chenbo, Thomas Monjalon

On Wed, Jun 22, 2022 at 5:22 PM Poczatek, Jakub
<jakub.poczatek@intel.com> wrote:
>
> Hey David,
>
> The code change fixes the errors and the performance test completes.

I posted this fix against (not yet pulled into main) next-virtio repo.

-- 
David Marchand


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

* Re: [PATCH 08/12] net/qede/base: fix build with GCC 12
  2022-06-21 23:17   ` Stephen Hemminger
@ 2022-06-22 15:42     ` David Marchand
  0 siblings, 0 replies; 73+ messages in thread
From: David Marchand @ 2022-06-22 15:42 UTC (permalink / raw)
  To: Stephen Hemminger, Rasesh Mody, Devendra Singh Rawat
  Cc: dev, Thomas Monjalon, Ferruh Yigit, dpdk stable,
	Jerin Jacob Kollanukkaran

On Wed, Jun 22, 2022 at 1:17 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
> > GCC raises the following warning:
> >
> > In function ‘_mm256_storeu_si256’,
> >     inlined from ‘rte_mov32’ at
> >         ../lib/eal/x86/include/rte_memcpy.h:320:2,
> >     inlined from ‘rte_mov128’ at
> >         ../lib/eal/x86/include/rte_memcpy.h:342:2,
> >     inlined from ‘rte_memcpy_generic’ at
> >         ../lib/eal/x86/include/rte_memcpy.h:438:4,
> >     inlined from ‘rte_memcpy’ at
> >         ../lib/eal/x86/include/rte_memcpy.h:882:10,
> >     inlined from ‘__ecore_mcp_cmd_and_union’ at
> >         ../drivers/net/qede/base/ecore_mcp.c:541:3,
> >     inlined from ‘_ecore_mcp_cmd_and_union’ at
> >         ../drivers/net/qede/base/ecore_mcp.c:638:2,
> >     inlined from ‘ecore_mcp_cmd_and_union’ at
> >         ../drivers/net/qede/base/ecore_mcp.c:742:9:
> > /usr/lib/gcc/x86_64-redhat-linux/12/include/avxintrin.h:935:8: error:
> >         array subscript 1 is outside array bounds of
> >         ‘union drv_union_data[1]’ [-Werror=array-bounds]
> >   935 |   *__P = __A;
> >       |   ~~~~~^~~~~
> > ../drivers/net/qede/base/ecore_mcp.c: In function
> >         ‘ecore_mcp_cmd_and_union’:
> > ../drivers/net/qede/base/ecore_mcp.c:533:30: note: at offset 32 into
> >         object ‘union_data’ of size 32
> >   533 |         union drv_union_data union_data;
> >       |                              ^~~~~~~~~~
> >
> > Since this code is in the base driver, waive the check until the base
> > driver is fixed by the relevant people.
>
> Even there are two maintainers, haven't heard a response from them.
> It could be a real bug.

Maintainers were pinged privately but I see no progress.
If I don't get a reply from them by tomorrow morning (GMT+2), I will
merge your RFC patch as it seems the best fix atm.

https://patchwork.dpdk.org/project/dpdk/patch/20220607171746.461772-3-stephen@networkplumber.org/


-- 
David Marchand


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

end of thread, other threads:[~2022-06-22 15:42 UTC | newest]

Thread overview: 73+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-18 10:16 [PATCH 00/12] Fix compilation with gcc 12 David Marchand
2022-05-18 10:16 ` [PATCH 01/12] common/cpt: fix build with GCC 12 David Marchand
2022-05-20 20:23   ` Stephen Hemminger
2022-06-10 13:11   ` David Marchand
2022-06-13 11:40     ` [EXT] " Ankur Dwivedi
2022-06-16  9:30       ` David Marchand
2022-06-16 11:59         ` Ankur Dwivedi
2022-05-18 10:16 ` [PATCH 02/12] crypto/cnxk: " David Marchand
2022-05-20 20:24   ` Stephen Hemminger
2022-05-18 10:16 ` [PATCH 03/12] crypto/ipsec_mb: " David Marchand
2022-06-02  9:50   ` Bruce Richardson
2022-06-10 13:07     ` David Marchand
2022-06-11 15:34   ` Stephen Hemminger
2022-05-18 10:16 ` [PATCH 04/12] net/ena: " David Marchand
2022-05-20 20:28   ` Stephen Hemminger
2022-05-21  9:49     ` Morten Brørup
2022-05-21 16:23       ` Stephen Hemminger
2022-05-22 20:30         ` Morten Brørup
2022-06-11 15:34   ` Stephen Hemminger
2022-05-18 10:16 ` [PATCH 05/12] net/enetfec: " David Marchand
2022-06-10 13:08   ` David Marchand
2022-06-13  5:22     ` Sachin Saxena (OSS)
2022-06-14  8:16     ` Sachin Saxena (OSS)
2022-06-11 15:35   ` Stephen Hemminger
2022-05-18 10:16 ` [PATCH 06/12] net/ice: " David Marchand
2022-06-11 15:36   ` Stephen Hemminger
2022-05-18 10:16 ` [PATCH 07/12] net/ice/base: " David Marchand
2022-05-18 10:16 ` [PATCH 08/12] net/qede/base: " David Marchand
2022-05-20 20:29   ` Stephen Hemminger
2022-06-21 23:17   ` Stephen Hemminger
2022-06-22 15:42     ` David Marchand
2022-05-18 10:16 ` [PATCH 09/12] vdpa/ifc: " David Marchand
2022-05-18 11:48   ` Wang, Xiao W
2022-06-11 15:36   ` Stephen Hemminger
2022-05-18 10:16 ` [PATCH 10/12] vhost/crypto: " David Marchand
2022-06-02 10:08   ` Bruce Richardson
2022-06-14  9:22     ` David Marchand
2022-06-14  9:25       ` Bruce Richardson
2022-06-16  9:27         ` David Marchand
2022-06-11 15:36   ` Stephen Hemminger
2022-06-16  9:32   ` [PATCH v2] " David Marchand
2022-06-16 14:53     ` David Marchand
2022-05-18 10:16 ` [PATCH 11/12] app/flow-perf: " David Marchand
2022-06-02 10:21   ` Bruce Richardson
2022-06-08  9:03   ` Wisam Monther
2022-06-08 12:20     ` David Marchand
2022-06-13  7:49       ` Wisam Monther
2022-06-11 15:37   ` Stephen Hemminger
2022-05-18 10:16 ` [PATCH 12/12] test/ipsec: " David Marchand
2022-06-02 18:41   ` Medvedkin, Vladimir
2022-06-03  7:45     ` David Marchand
2022-06-03  7:56       ` Bruce Richardson
2022-06-03  9:41         ` David Marchand
2022-06-03 15:57           ` Medvedkin, Vladimir
2022-06-11 15:38   ` Stephen Hemminger
2022-06-16  9:33   ` [PATCH v2] " David Marchand
2022-06-17 12:06     ` David Marchand
2022-06-20  9:07       ` [EXT] " Akhil Goyal
2022-06-20 15:06       ` Medvedkin, Vladimir
2022-06-16 14:46   ` [PATCH v3] vhost/crypto: " David Marchand
2022-06-17 13:59     ` Maxime Coquelin
2022-06-21  9:31     ` Maxime Coquelin
2022-06-22  9:01       ` Poczatek, Jakub
2022-06-22  9:26         ` Zhang, Roy Fan
2022-06-22 14:07         ` David Marchand
2022-06-22 15:21           ` Poczatek, Jakub
2022-06-22 15:31             ` David Marchand
2022-05-20 20:13 ` [PATCH 00/12] Fix compilation with gcc 12 Stephen Hemminger
2022-05-21  9:39   ` Morten Brørup
2022-06-15  8:49 ` David Marchand
2022-06-15 14:45   ` Stephen Hemminger
2022-06-15 14:59     ` Thomas Monjalon
2022-06-15 15:15       ` Stephen Hemminger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).