Hi Konstantin,

Please see inline.

Thanks,
Anoob

-----Original Message-----
From: Ananyev, Konstantin <redacted>
Sent: Tuesday, December 24, 2019 12:18 AM
To: Anoob Joseph <redacted>; Akhil Goyal <redacted>;
Nicolau, Radu [off-list ref]; Thomas Monjalon
[off-list ref]
Cc: Jerin Jacob Kollanukkaran <redacted>; Narayana Prasad Raju
Athreya [off-list ref]; Ankur Dwivedi [off-list ref];
Archana Muniganti [off-list ref]; Tejasree Kondoj
[off-list ref]; Vamsi Krishna Attunuru [off-list ref];
Lukas Bartosik [off-list ref]; dev@dpdk.org
Subject: [EXT] RE: [PATCH 04/14] examples/ipsec-secgw: add Rx adapter
support

External Email

----------------------------------------------------------------------

quoted

Add Rx adapter support. The event helper init routine will initialize
the Rx adapter according to the configuration. If Rx adapter config is
not present it will generate a default config. It will check the
available eth ports and event queues and map them 1:1. So one eth port
will be connected to one event queue. This way event queue ID could be
used to figure out the port on which a packet came in.

Signed-off-by: Anoob Joseph <redacted>
Signed-off-by: Lukasz Bartosik <redacted>
---
 examples/ipsec-secgw/event_helper.c | 289
+++++++++++++++++++++++++++++++++++-
 examples/ipsec-secgw/event_helper.h |  29 ++++
 2 files changed, 317 insertions(+), 1 deletion(-)

diff --git a/examples/ipsec-secgw/event_helper.c

b/examples/ipsec-secgw/event_helper.c
index d0157f4..f0eca01 100644

--- a/examples/ipsec-secgw/event_helper.c
+++ b/examples/ipsec-secgw/event_helper.c

@@ -4,10 +4,60 @@
 #include <rte_bitmap.h>
 #include <rte_ethdev.h>
 #include <rte_eventdev.h>
+#include <rte_event_eth_rx_adapter.h>
 #include <rte_malloc.h>

 #include "event_helper.h"

+static int
+eh_get_enabled_cores(struct rte_bitmap *eth_core_mask) {
+	int i;
+	int count = 0;
+
+	RTE_LCORE_FOREACH(i) {
+		/* Check if this core is enabled in core mask*/
+		if (rte_bitmap_get(eth_core_mask, i)) {
+			/* We have found enabled core */
+			count++;
+		}
+	}
+	return count;
+}
+
+static inline unsigned int
+eh_get_next_eth_core(struct eventmode_conf *em_conf) {
+	static unsigned int prev_core = -1;
+	unsigned int next_core;
+
+	/*
+	 * Make sure we have at least one eth core running, else the following
+	 * logic would lead to an infinite loop.
+	 */
+	if (eh_get_enabled_cores(em_conf->eth_core_mask) == 0) {
+		EH_LOG_ERR("No enabled eth core found");
+		return RTE_MAX_LCORE;
+	}
+
+get_next_core:
+	/* Get the next core */
+	next_core = rte_get_next_lcore(prev_core, 0, 1);
+
+	/* Check if we have reached max lcores */
+	if (next_core == RTE_MAX_LCORE)
+		return next_core;
+
+	/* Update prev_core */
+	prev_core = next_core;
+
+	/* Only some cores are marked as eth cores. Skip others */
+	if (!(rte_bitmap_get(em_conf->eth_core_mask, next_core)))
+		goto get_next_core;

Are loops statements forbidden in C now? 😉
As a generic comment - too many (unnecessary) gotos in this patch series.
It is not uncommon to see 2-3 labels inside the function and bunch gotos to
them.
Would be good to rework the code a bit to get rid of them.

[Anoob] Sure. Will rework the code and see if the gotos can be minimized. In this case, it seemed more straightforward to have goto instead of the loop. Will recheck anyway.
 

quoted

+
+	return next_core;
+}
+
 static inline unsigned int
 eh_get_next_active_core(struct eventmode_conf *em_conf, unsigned int
prev_core)  { @@ -154,6 +204,87 @@ eh_set_default_conf_link(struct
eventmode_conf *em_conf)  }