From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <harry.van.haaren@intel.com>
Received: from mga17.intel.com (mga17.intel.com [192.55.52.151])
 by dpdk.org (Postfix) with ESMTP id 589201B84C
 for <dev@dpdk.org>; Tue, 15 May 2018 16:52:51 +0200 (CEST)
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from orsmga007.jf.intel.com ([10.7.209.58])
 by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 15 May 2018 07:52:50 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.49,403,1520924400"; d="scan'208";a="41121336"
Received: from silpixa00399779.ir.intel.com ([10.237.223.187])
 by orsmga007.jf.intel.com with ESMTP; 15 May 2018 07:52:48 -0700
From: Harry van Haaren <harry.van.haaren@intel.com>
To: dev@dpdk.org
Cc: Harry van Haaren <harry.van.haaren@intel.com>, thomas@monjalon.net,
 vipin.varghese@intel.com
Date: Tue, 15 May 2018 15:52:45 +0100
Message-Id: <1526395965-133647-1-git-send-email-harry.van.haaren@intel.com>
X-Mailer: git-send-email 2.7.4
Subject: [dpdk-dev] [PATCH] eal/service: improve error checking of coremasks
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 15 May 2018 14:52:51 -0000

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@intel.com>
Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

---

Cc: thomas@monjalon.net
Cc: vipin.varghese@intel.com

@Thomas, please consider this patch for RC4, it adds checks
and prints warnings, better usability, no functional changes.

---
 lib/librte_eal/common/eal_common_options.c | 39 ++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index ecebb29..b6b569f 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -315,6 +315,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;
@@ -358,6 +359,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++;
 			}
@@ -374,11 +379,27 @@ eal_parse_service_coremask(const char *coremask)
 	if (count == 0)
 		return -1;
 
+	if (core_parsed && taken_lcore_count != count) {
+		RTE_LOG(ERR, EAL,
+			"Warning: not all service cores were in the coremask - 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();
@@ -387,6 +408,10 @@ eal_parse_coremask(const char *coremask)
 	char c;
 	int val;
 
+	if (eal_service_cores_parsed())
+		RTE_LOG(ERR, EAL,
+			"Warning: Service cores parsed before dataplane cores. Ensure -c is before -s or -S.\n");
+
 	if (coremask == NULL)
 		return -1;
 	/* Remove all blank characters ahead and after .
@@ -418,6 +443,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++;
@@ -449,6 +475,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;
@@ -490,6 +517,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++;
@@ -504,6 +534,11 @@ eal_parse_service_corelist(const char *corelist)
 	if (count == 0)
 		return -1;
 
+	if (core_parsed && taken_lcore_count != count) {
+		RTE_LOG(ERR, EAL,
+			"Warning: not all service cores were in the coremask - ensure -c or -l includes service cores\n");
+	}
+
 	return 0;
 }
 
@@ -516,6 +551,10 @@ eal_parse_corelist(const char *corelist)
 	char *end = NULL;
 	int min, max;
 
+	if (eal_service_cores_parsed())
+		RTE_LOG(ERR, EAL,
+			"Warning: Service cores parsed before dataplane cores. Ensure -l is before -s or -S.\n");
+
 	if (corelist == NULL)
 		return -1;
 
-- 
2.7.4