DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH v1] app/mldev: improve checks for invalid options
@ 2023-06-12 17:58 Srikanth Yalavarthi
  2023-07-07  8:07 ` Thomas Monjalon
  0 siblings, 1 reply; 2+ messages in thread
From: Srikanth Yalavarthi @ 2023-06-12 17:58 UTC (permalink / raw)
  To: Srikanth Yalavarthi; +Cc: dev, sshankarnara, aprabhu, ptakkar

Improve checks for ML application options. Check for negative,
non-integral and hexadecimal values. Check for number of entries
in filelist option. Improve error reporting for invalid options.

Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
---
 app/test-mldev/ml_options.c            | 55 +++++++++++++++++++++-----
 app/test-mldev/parser.c                |  8 ++--
 app/test-mldev/test_common.c           |  6 +--
 app/test-mldev/test_inference_common.c | 10 ++---
 4 files changed, 58 insertions(+), 21 deletions(-)

diff --git a/app/test-mldev/ml_options.c b/app/test-mldev/ml_options.c
index a63506cf7d..bed06d1bf7 100644
--- a/app/test-mldev/ml_options.c
+++ b/app/test-mldev/ml_options.c
@@ -43,6 +43,7 @@ static int
 ml_parse_test_name(struct ml_options *opt, const char *arg)
 {
 	strlcpy(opt->test_name, arg, ML_TEST_NAME_MAX_LEN);
+
 	return 0;
 }
 
@@ -52,9 +53,8 @@ ml_parse_dev_id(struct ml_options *opt, const char *arg)
 	int ret;
 
 	ret = parser_read_int16(&opt->dev_id, arg);
-
 	if (ret < 0)
-		return -EINVAL;
+		ml_err("Invalid option: dev_id = %s\n", arg);
 
 	return ret;
 }
@@ -62,9 +62,13 @@ ml_parse_dev_id(struct ml_options *opt, const char *arg)
 static int
 ml_parse_socket_id(struct ml_options *opt, const char *arg)
 {
-	opt->socket_id = atoi(arg);
+	int ret;
 
-	return 0;
+	ret = parser_read_int32(&opt->socket_id, arg);
+	if (ret < 0)
+		ml_err("Invalid option: socket_id = %s\n", arg);
+
+	return ret;
 }
 
 static int
@@ -143,10 +147,17 @@ ml_parse_filelist(struct ml_options *opt, const char *arg)
 	else
 		memset(opt->filelist[opt->nb_filelist].reference, 0, PATH_MAX);
 
+	/* check for extra tokens */
+	token = strtok(NULL, delim);
+	if (token != NULL) {
+		ml_err("Invalid filelist. Entries > 4\n.");
+		return -EINVAL;
+	}
+
 	opt->nb_filelist++;
 
 	if (opt->nb_filelist == 0) {
-		ml_err("Empty filelist. Need at least one filelist entry for the test.");
+		ml_err("Empty filelist. Need at least one filelist entry for the test.\n");
 		return -EINVAL;
 	}
 
@@ -156,13 +167,25 @@ ml_parse_filelist(struct ml_options *opt, const char *arg)
 static int
 ml_parse_repetitions(struct ml_options *opt, const char *arg)
 {
-	return parser_read_uint64(&opt->repetitions, arg);
+	int ret;
+
+	ret = parser_read_uint64(&opt->repetitions, arg);
+	if (ret != 0)
+		ml_err("Invalid option, repetitions = %s\n", arg);
+
+	return ret;
 }
 
 static int
 ml_parse_burst_size(struct ml_options *opt, const char *arg)
 {
-	return parser_read_uint16(&opt->burst_size, arg);
+	int ret;
+
+	ret = parser_read_uint16(&opt->burst_size, arg);
+	if (ret != 0)
+		ml_err("Invalid option, burst_size = %s\n", arg);
+
+	return ret;
 }
 
 static int
@@ -171,6 +194,8 @@ ml_parse_queue_pairs(struct ml_options *opt, const char *arg)
 	int ret;
 
 	ret = parser_read_uint16(&opt->queue_pairs, arg);
+	if (ret != 0)
+		ml_err("Invalid option, queue_pairs = %s\n", arg);
 
 	return ret;
 }
@@ -178,13 +203,25 @@ ml_parse_queue_pairs(struct ml_options *opt, const char *arg)
 static int
 ml_parse_queue_size(struct ml_options *opt, const char *arg)
 {
-	return parser_read_uint16(&opt->queue_size, arg);
+	int ret;
+
+	ret = parser_read_uint16(&opt->queue_size, arg);
+	if (ret != 0)
+		ml_err("Invalid option, queue_size = %s\n", arg);
+
+	return ret;
 }
 
 static int
 ml_parse_batches(struct ml_options *opt, const char *arg)
 {
-	return parser_read_uint16(&opt->batches, arg);
+	int ret;
+
+	ret = parser_read_uint16(&opt->batches, arg);
+	if (ret != 0)
+		ml_err("Invalid option, batches = %s\n", arg);
+
+	return ret;
 }
 
 static int
diff --git a/app/test-mldev/parser.c b/app/test-mldev/parser.c
index 0b7fb63fe5..277dcaedb2 100644
--- a/app/test-mldev/parser.c
+++ b/app/test-mldev/parser.c
@@ -132,11 +132,11 @@ parser_read_int32(int32_t *value, const char *p)
 	int32_t val;
 
 	p = skip_white_spaces(p);
-	if (!isdigit(*p))
+	if ((*p != '-') && (!isdigit(*p)))
 		return -EINVAL;
 
 	val = strtol(p, &next, 10);
-	if (p == next)
+	if ((*next != '\0') || (p == next))
 		return -EINVAL;
 
 	*value = val;
@@ -150,11 +150,11 @@ parser_read_int16(int16_t *value, const char *p)
 	int16_t val;
 
 	p = skip_white_spaces(p);
-	if (!isdigit(*p))
+	if ((*p != '-') && (!isdigit(*p)))
 		return -EINVAL;
 
 	val = strtol(p, &next, 10);
-	if (p == next)
+	if ((*next != '\0') || (p == next))
 		return -EINVAL;
 
 	*value = val;
diff --git a/app/test-mldev/test_common.c b/app/test-mldev/test_common.c
index 891e8d0ca8..3cf1362285 100644
--- a/app/test-mldev/test_common.c
+++ b/app/test-mldev/test_common.c
@@ -95,13 +95,13 @@ ml_test_opt_check(struct ml_options *opt)
 		return -ENODEV;
 	}
 
-	if (opt->dev_id >= dev_count) {
-		ml_err("Invalid option dev_id = %d", opt->dev_id);
+	if ((opt->dev_id >= dev_count) || (opt->dev_id < 0)) {
+		ml_err("Invalid option, dev_id = %d", opt->dev_id);
 		return -EINVAL;
 	}
 
 	socket_id = rte_ml_dev_socket_id(opt->dev_id);
-	if (!((opt->socket_id != SOCKET_ID_ANY) || (opt->socket_id != socket_id))) {
+	if ((opt->socket_id != SOCKET_ID_ANY) && (opt->socket_id != socket_id)) {
 		ml_err("Invalid option, socket_id = %d\n", opt->socket_id);
 		return -EINVAL;
 	}
diff --git a/app/test-mldev/test_inference_common.c b/app/test-mldev/test_inference_common.c
index 231ef2defa..7c62726761 100644
--- a/app/test-mldev/test_inference_common.c
+++ b/app/test-mldev/test_inference_common.c
@@ -302,19 +302,19 @@ test_inference_cap_check(struct ml_options *opt)
 	rte_ml_dev_info_get(opt->dev_id, &dev_info);
 
 	if (opt->queue_pairs > dev_info.max_queue_pairs) {
-		ml_err("Insufficient capabilities: queue_pairs = %u, max_queue_pairs = %u",
+		ml_err("Insufficient capabilities: queue_pairs = %u > (max_queue_pairs = %u)",
 		       opt->queue_pairs, dev_info.max_queue_pairs);
 		return false;
 	}
 
 	if (opt->queue_size > dev_info.max_desc) {
-		ml_err("Insufficient capabilities: queue_size = %u, max_desc = %u", opt->queue_size,
-		       dev_info.max_desc);
+		ml_err("Insufficient capabilities: queue_size = %u > (max_desc = %u)",
+		       opt->queue_size, dev_info.max_desc);
 		return false;
 	}
 
 	if (opt->nb_filelist > dev_info.max_models) {
-		ml_err("Insufficient capabilities:  Filelist count exceeded device limit, count = %u (max limit = %u)",
+		ml_err("Insufficient capabilities:  Filelist count exceeded device limit, count = %u > (max limit = %u)",
 		       opt->nb_filelist, dev_info.max_models);
 		return false;
 	}
@@ -408,7 +408,7 @@ test_inference_opt_dump(struct ml_options *opt)
 	ml_dump("stats", "%s", (opt->stats ? "true" : "false"));
 
 	if (opt->batches == 0)
-		ml_dump("batches", "%u (default)", opt->batches);
+		ml_dump("batches", "%u (default batch size)", opt->batches);
 	else
 		ml_dump("batches", "%u", opt->batches);
 
-- 
2.17.1


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

* Re: [PATCH v1] app/mldev: improve checks for invalid options
  2023-06-12 17:58 [PATCH v1] app/mldev: improve checks for invalid options Srikanth Yalavarthi
@ 2023-07-07  8:07 ` Thomas Monjalon
  0 siblings, 0 replies; 2+ messages in thread
From: Thomas Monjalon @ 2023-07-07  8:07 UTC (permalink / raw)
  To: Srikanth Yalavarthi; +Cc: dev, sshankarnara, aprabhu, ptakkar

12/06/2023 19:58, Srikanth Yalavarthi:
> Improve checks for ML application options. Check for negative,
> non-integral and hexadecimal values. Check for number of entries
> in filelist option. Improve error reporting for invalid options.
> 
> Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>

Applied, thanks.




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

end of thread, other threads:[~2023-07-07  8:07 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-12 17:58 [PATCH v1] app/mldev: improve checks for invalid options Srikanth Yalavarthi
2023-07-07  8:07 ` Thomas Monjalon

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).