[dpdk-dev] [PATCH v4] eal/service: improve error checking of coremasks

[Harry van Haaren]

This commit improves the error checking performed on the
core masks (or lists) of the service cores, in particular
with respect to the data-plane (RTE) cores of DPDK.

With this commit, invalid configurations are detected at
runtime, and warning messages are printed to inform the user.

For example specifying the coremask as 0xf, and the service
coremask as 0xff00 is invalid as not all service-cores are
contained within the coremask. A warning is now printed to
inform the user.

Reported-by: Vipin Varghese <vipin.varghese at intel.com>
Signed-off-by: Harry van Haaren <harry.van.haaren at intel.com>
Acked-by: Vipin Varghese <vipin.varghese at intel.com>

---

v4:
- Re-wrap lines breaking at sentence ends (Thomas)
- Consistency message endings with rest of file (Thomas)
- Including Vipin's Ack from v3

v3:
- Use WARNING instead of ERR log level (Vipin)
- Put strings on one line for grep-ability (Harry)

v2, thanks for review:
- Consistency in message endings - vs . (Thomas)
- Wrap lines as they're very long otherwise (Thomas)

Cc: thomas at monjalon.net
Cc: vipin.varghese at intel.com
---
 lib/librte_eal/common/eal_common_options.c | 48 ++++++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 80f11d0..8fcb6ef 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -321,6 +321,7 @@ eal_parse_service_coremask(const char *coremask)
 	unsigned int count = 0;
 	char c;
 	int val;
+	uint32_t taken_lcore_count = 0;
 
 	if (coremask == NULL)
 		return -1;
@@ -354,7 +355,7 @@ eal_parse_service_coremask(const char *coremask)
 				if (master_lcore_parsed &&
 						cfg->master_lcore == lcore) {
 					RTE_LOG(ERR, EAL,
-						"Error: lcore %u is master lcore, cannot use as service core\n",
+						"lcore %u is master lcore, cannot use as service core\n",
 						idx);
 					return -1;
 				}
@@ -364,6 +365,10 @@ eal_parse_service_coremask(const char *coremask)
 						"lcore %u unavailable\n", idx);
 					return -1;
 				}
+
+				if (cfg->lcore_role[idx] == ROLE_RTE)
+					taken_lcore_count++;
+
 				lcore_config[idx].core_role = ROLE_SERVICE;
 				count++;
 			}
@@ -380,11 +385,28 @@ eal_parse_service_coremask(const char *coremask)
 	if (count == 0)
 		return -1;
 
+	if (core_parsed && taken_lcore_count != count) {
+		RTE_LOG(WARNING, EAL,
+			"Not all service cores are in the coremask."
+			"Please ensure -c or -l includes service cores\n");
+	}
+
 	cfg->service_lcore_count = count;
 	return 0;
 }
 
 static int
+eal_service_cores_parsed(void)
+{
+	int idx;
+	for (idx = 0; idx < RTE_MAX_LCORE; idx++) {
+		if (lcore_config[idx].core_role == ROLE_SERVICE)
+			return 1;
+	}
+	return 0;
+}
+
+static int
 eal_parse_coremask(const char *coremask)
 {
 	struct rte_config *cfg = rte_eal_get_configuration();
@@ -393,6 +415,11 @@ eal_parse_coremask(const char *coremask)
 	char c;
 	int val;
 
+	if (eal_service_cores_parsed())
+		RTE_LOG(WARNING, EAL,
+			"Service cores parsed before dataplane cores."
+			"Please ensure -c is before -s or -S\n");
+
 	if (coremask == NULL)
 		return -1;
 	/* Remove all blank characters ahead and after .
@@ -424,6 +451,7 @@ eal_parse_coremask(const char *coremask)
 					        "unavailable\n", idx);
 					return -1;
 				}
+
 				cfg->lcore_role[idx] = ROLE_RTE;
 				lcore_config[idx].core_index = count;
 				count++;
@@ -455,6 +483,7 @@ eal_parse_service_corelist(const char *corelist)
 	unsigned count = 0;
 	char *end = NULL;
 	int min, max;
+	uint32_t taken_lcore_count = 0;
 
 	if (corelist == NULL)
 		return -1;
@@ -496,6 +525,9 @@ eal_parse_service_corelist(const char *corelist)
 							idx);
 						return -1;
 					}
+					if (cfg->lcore_role[idx] == ROLE_RTE)
+						taken_lcore_count++;
+
 					lcore_config[idx].core_role =
 							ROLE_SERVICE;
 					count++;
@@ -510,6 +542,12 @@ eal_parse_service_corelist(const char *corelist)
 	if (count == 0)
 		return -1;
 
+	if (core_parsed && taken_lcore_count != count) {
+		RTE_LOG(WARNING, EAL,
+			"Not all service cores were in the coremask."
+			"Please ensure -c or -l includes service cores\n");
+	}
+
 	return 0;
 }
 
@@ -522,6 +560,11 @@ eal_parse_corelist(const char *corelist)
 	char *end = NULL;
 	int min, max;
 
+	if (eal_service_cores_parsed())
+		RTE_LOG(WARNING, EAL,
+			"Service cores parsed before dataplane cores."
+			"Please ensure -l is before -s or -S\n");
+
 	if (corelist == NULL)
 		return -1;
 
@@ -596,7 +639,8 @@ eal_parse_master_lcore(const char *arg)
 
 	/* ensure master core is not used as service core */
 	if (lcore_config[cfg->master_lcore].core_role == ROLE_SERVICE) {
-		RTE_LOG(ERR, EAL, "Error: Master lcore is used as a service core.\n");
+		RTE_LOG(ERR, EAL,
+			"Error: Master lcore is used as a service core\n");
 		return -1;
 	}
 
-- 
2.7.4