From fe8850cb3b7c1051784e5587b9f79beeaf87a804 Mon Sep 17 00:00:00 2001
From: bullvolkan <volkan.atli@b-ulltech.com>
Date: Fri, 20 Oct 2023 21:48:30 +0300
Subject: [PATCH] distributor: enhance error handling for consistency
This commit improves error handling in the distributor component to ensure
consistency and reliability. It addresses invalid 'num' values in 'lcore_worker',
corrects 'retptr64' initialization in 'rte_distributor_request_pkt', and checks
the validity of 'num' in 'rte_distributor_return_pkt'. These changes enhance
error handling and maintain code consistency.
Signed-off-by: Volkan Atli <volkan.atli@b-ulltech.com>
---
.mailmap | 2 +-
examples/distributor/main.c | 9 +++++++--
lib/distributor/rte_distributor.c | 24 ++++++++++++++----------
lib/distributor/rte_distributor.h | 11 ++++++++++-
4 files changed, 32 insertions(+), 14 deletions(-)
diff --git a/.mailmap b/.mailmap
index 3f5bab26a8..d0fe75fa8b 100644
--- a/.mailmap
+++ b/.mailmap
@@ -63,7 +63,7 @@ Alfredo Cardigliano <cardigliano@ntop.org>
Ali Alnubani <alialnu@nvidia.com> <alialnu@mellanox.com>
Alice Michael <alice.michael@intel.com>
Alin Rauta <alin.rauta@intel.com>
-Ali Volkan Atli <volkan.atli@argela.com.tr>
+Ali Volkan Atli <volkan.atli@b-ulltech.com>
Allain Legacy <allain.legacy@windriver.com>
Allen Hubbe <allen.hubbe@amd.com>
Alok Makhariya <alok.makhariya@nxp.com>
diff --git a/examples/distributor/main.c b/examples/distributor/main.c
index 21304d6618..58369c3faa 100644
--- a/examples/distributor/main.c
+++ b/examples/distributor/main.c
@@ -634,8 +634,8 @@ lcore_worker(struct lcore_params *p)
{
struct rte_distributor *d = p->d;
const unsigned id = p->worker_id;
- unsigned int num = 0;
- unsigned int i;
+ int num = 0;
+ int i;
/*
* for single port, xor_val will be zero so we won't modify the output
@@ -652,6 +652,11 @@ lcore_worker(struct lcore_params *p)
printf("\nCore %u acting as worker core.\n", rte_lcore_id());
while (!quit_signal_work) {
num = rte_distributor_get_pkt(d, id, buf, buf, num);
+
+ if (unlikely(num < 0 || num > 8)) {
+ rte_exit(EXIT_FAILURE, "strange retcount %u\n", num);
+ }
+
/* Do a little bit of work for each packet */
for (i = 0; i < num; i++) {
uint64_t t = rte_rdtsc()+100;
diff --git a/lib/distributor/rte_distributor.c b/lib/distributor/rte_distributor.c
index 5ca80dd44f..92653dd9ca 100644
--- a/lib/distributor/rte_distributor.c
+++ b/lib/distributor/rte_distributor.c
@@ -64,15 +64,16 @@ rte_distributor_request_pkt(struct rte_distributor *d,
* handshake bits. Populate the retptrs with returning packets.
*/
- for (i = count; i < RTE_DIST_BURST_SIZE; i++)
- buf->retptr64[i] = 0;
-
/* Set VALID_BUF bit for each packet returned */
- for (i = count; i-- > 0; )
+ size_t arr_len = sizeof(buf->retptr64) / sizeof(buf->retptr64[0]);
+ for (i = 0; i < RTE_MIN(count, arr_len); ++i)
buf->retptr64[i] =
(((int64_t)(uintptr_t)(oldpkt[i])) <<
RTE_DISTRIB_FLAG_BITS) | RTE_DISTRIB_VALID_BUF;
+ for (i = count; i < RTE_DIST_BURST_SIZE; i++)
+ buf->retptr64[i] = 0;
+
/*
* Finally, set the GET_BUF to signal to distributor that cache
* line is ready for processing
@@ -161,7 +162,7 @@ rte_distributor_return_pkt(struct rte_distributor *d,
unsigned int worker_id, struct rte_mbuf **oldpkt, int num)
{
struct rte_distributor_buffer *buf = &d->bufs[worker_id];
- unsigned int i;
+ int i;
if (unlikely(d->alg_type == RTE_DIST_ALG_SINGLE)) {
if (num == 1)
@@ -186,16 +187,19 @@ rte_distributor_return_pkt(struct rte_distributor *d,
rte_pause();
}
+ if (unlikely(num > RTE_DIST_BURST_SIZE || num < 0)) {
+ return -EINVAL;
+ }
+
/* Sync with distributor to acquire retptrs */
__atomic_thread_fence(__ATOMIC_ACQUIRE);
- for (i = 0; i < RTE_DIST_BURST_SIZE; i++)
- /* Switch off the return bit first */
- buf->retptr64[i] = 0;
-
- for (i = num; i-- > 0; )
+ for (i = 0; i < num; i++)
buf->retptr64[i] = (((int64_t)(uintptr_t)oldpkt[i]) <<
RTE_DISTRIB_FLAG_BITS) | RTE_DISTRIB_VALID_BUF;
+ for (i = num; i < RTE_DIST_BURST_SIZE; i++)
+ buf->retptr64[i] = 0;
+
/* Use RETURN_BUF on bufptr64 to notify distributor that
* we won't read any mbufs from there even if GET_BUF is set.
* This allows distributor to retrieve in-flight already sent packets.
diff --git a/lib/distributor/rte_distributor.h b/lib/distributor/rte_distributor.h
index a073e64612..337ff8e0fe 100644
--- a/lib/distributor/rte_distributor.h
+++ b/lib/distributor/rte_distributor.h
@@ -160,7 +160,11 @@ rte_distributor_clear_returns(struct rte_distributor *d);
* The number of packets being returned
*
* @return
- * The number of packets in the pkts array
+ * The number of packets in the pkts array or
+ * -EINVAL if num is greater than 1 (RTE_DIST_ALG_SINGLE)
+ * -EINVAL if num is greater than RTE_DIST_BURST_SIZE or
+ * less than zero (RTE_DIST_ALG_BURST)
+
*/
int
rte_distributor_get_pkt(struct rte_distributor *d,
@@ -180,6 +184,11 @@ rte_distributor_get_pkt(struct rte_distributor *d,
* The previous packets being processed by the worker
* @param num
* The number of packets in the oldpkt array
+ *
+ * @return
+ * The number of packets in the pkts array or
+ * -EINVAL if retcount is greater than 1 (RTE_DIST_ALG_SINGLE)
+ * -EINVAL if retcount is greater than RTE_DIST_BURST_SIZE (RTE_DIST_ALG_BURST)
*/
int
rte_distributor_return_pkt(struct rte_distributor *d,
--
2.41.0