netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v1 0/5] Some pktgen fixes/improvments
@ 2025-01-17 14:16 Peter Seiderer
  2025-01-17 14:16 ` [PATCH net-next v1 1/5] net: pktgen: replace ENOTSUPP with EOPNOTSUPP Peter Seiderer
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Peter Seiderer @ 2025-01-17 14:16 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, linux-kselftest, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Shuah Khan,
	Toke Høiland-Jørgensen, Frederic Weisbecker,
	Artem Chernyshev, Nam Cao, Peter Seiderer

While taking a look at '[PATCH net] pktgen: Avoid out-of-range in
get_imix_entries' ([1]) and '[PATCH net v2] pktgen: Avoid out-of-bounds access
in get_imix_entries' ([2], [3]) and doing some tests and code review I
detected that the /proc/net/pktgen/... parsing logic does not honour the
user given buffer bounds (resulting in out-of-bounds access).

This can be observed e.g. by the following simple test (sometimes the
old/'longer' previous value is re-read from the buffer):

	$ echo add_device lo@0 > /proc/net/pktgen/kpktgend_0

	$ echo "min_pkt_size 12345" > /proc/net/pktgen/lo\@0 && grep min_pkt_size /proc/net/pktgen/lo\@0
Params: count 1000  min_pkt_size: 12345  max_pkt_size: 0
Result: OK: min_pkt_size=12345

	$ echo -n "min_pkt_size 123" > /proc/net/pktgen/lo\@0 && grep min_pkt_size /proc/net/pktgen/lo\@0
Params: count 1000  min_pkt_size: 12345  max_pkt_size: 0
Result: OK: min_pkt_size=12345

	$ echo "min_pkt_size 123" > /proc/net/pktgen/lo\@0 && grep min_pkt_size /proc/net/pktgen/lo\@0
Params: count 1000  min_pkt_size: 123  max_pkt_size: 0
Result: OK: min_pkt_size=123

So fix the out-of-bounds access (and two minor findings) and add a simple
proc_net_pktgen selftest...

Regards,
Peter

[1] https://lore.kernel.org/netdev/20241006221221.3744995-1-artem.chernyshev@red-soft.ru/
[2] https://lore.kernel.org/netdev/20250109083039.14004-1-pchelkin@ispras.ru/
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=76201b5979768500bca362871db66d77cb4c225e

Peter Seiderer (5):
  net: pktgen: replace ENOTSUPP with EOPNOTSUPP
  net: pktgen: enable 'param=value' parsing
  net: pktgen: fix access outside of user given buffer in
    pktgen_thread_write()
  net: pktgen: fix access outside of user given buffer in
    pktgen_if_write()
  selftest: net: add proc_net_pktgen

 net/core/pktgen.c                             | 210 ++++---
 tools/testing/selftests/net/Makefile          |   1 +
 tools/testing/selftests/net/proc_net_pktgen.c | 575 ++++++++++++++++++
 3 files changed, 712 insertions(+), 74 deletions(-)
 create mode 100644 tools/testing/selftests/net/proc_net_pktgen.c

-- 
2.48.0


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

* [PATCH net-next v1 1/5] net: pktgen: replace ENOTSUPP with EOPNOTSUPP
  2025-01-17 14:16 [PATCH net-next v1 0/5] Some pktgen fixes/improvments Peter Seiderer
@ 2025-01-17 14:16 ` Peter Seiderer
  2025-01-17 14:16 ` [PATCH net-next v1 2/5] net: pktgen: enable 'param=value' parsing Peter Seiderer
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Peter Seiderer @ 2025-01-17 14:16 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, linux-kselftest, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Shuah Khan,
	Toke Høiland-Jørgensen, Frederic Weisbecker,
	Artem Chernyshev, Nam Cao, Peter Seiderer

Replace ENOTSUPP with EOPNOTSUPP, fixes checkpatch hint

  WARNING: ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP

and e.g.

  $ echo "clone_skb 1" > /proc/net/pktgen/lo\@0
  -bash: echo: write error: Unknown error 524

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
---
 net/core/pktgen.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 82b6a2c3c141..496aa16773e7 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -1198,7 +1198,7 @@ static ssize_t pktgen_if_write(struct file *file,
 		if ((value > 0) &&
 		    ((pkt_dev->xmit_mode == M_NETIF_RECEIVE) ||
 		     !(pkt_dev->odev->priv_flags & IFF_TX_SKB_SHARING)))
-			return -ENOTSUPP;
+			return -EOPNOTSUPP;
 		if (value > 0 && (pkt_dev->n_imix_entries > 0 ||
 				  !(pkt_dev->flags & F_SHARED)))
 			return -EINVAL;
@@ -1258,7 +1258,7 @@ static ssize_t pktgen_if_write(struct file *file,
 		    ((pkt_dev->xmit_mode == M_QUEUE_XMIT) ||
 		     ((pkt_dev->xmit_mode == M_START_XMIT) &&
 		     (!(pkt_dev->odev->priv_flags & IFF_TX_SKB_SHARING)))))
-			return -ENOTSUPP;
+			return -EOPNOTSUPP;
 
 		if (value > 1 && !(pkt_dev->flags & F_SHARED))
 			return -EINVAL;
@@ -1303,7 +1303,7 @@ static ssize_t pktgen_if_write(struct file *file,
 		} else if (strcmp(f, "netif_receive") == 0) {
 			/* clone_skb set earlier, not supported in this mode */
 			if (pkt_dev->clone_skb > 0)
-				return -ENOTSUPP;
+				return -EOPNOTSUPP;
 
 			pkt_dev->xmit_mode = M_NETIF_RECEIVE;
 
-- 
2.48.0


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

* [PATCH net-next v1 2/5] net: pktgen: enable 'param=value' parsing
  2025-01-17 14:16 [PATCH net-next v1 0/5] Some pktgen fixes/improvments Peter Seiderer
  2025-01-17 14:16 ` [PATCH net-next v1 1/5] net: pktgen: replace ENOTSUPP with EOPNOTSUPP Peter Seiderer
@ 2025-01-17 14:16 ` Peter Seiderer
  2025-01-17 14:16 ` [PATCH net-next v1 3/5] net: pktgen: fix access outside of user given buffer in pktgen_thread_write() Peter Seiderer
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Peter Seiderer @ 2025-01-17 14:16 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, linux-kselftest, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Shuah Khan,
	Toke Høiland-Jørgensen, Frederic Weisbecker,
	Artem Chernyshev, Nam Cao, Peter Seiderer

Enable additional to 'parm value' the 'param=value' parsing (otherwise
skipping '=' in count_trail_chars() is useless).

Tested with:

	$ echo "min_pkt_size 999" > /proc/net/pktgen/lo\@0
	$ echo "min_pkt_size=999" > /proc/net/pktgen/lo\@0
	$ echo "min_pkt_size =999" > /proc/net/pktgen/lo\@0
	$ echo "min_pkt_size= 999" > /proc/net/pktgen/lo\@0
	$ echo "min_pkt_size = 999" > /proc/net/pktgen/lo\@0

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
---
 net/core/pktgen.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 496aa16773e7..4f8ec6c9bed4 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -823,6 +823,7 @@ static int strn_len(const char __user * user_buffer, unsigned int maxlen)
 		case '\r':
 		case '\t':
 		case ' ':
+		case '=':
 			goto done_str;
 		default:
 			break;
-- 
2.48.0


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

* [PATCH net-next v1 3/5] net: pktgen: fix access outside of user given buffer in pktgen_thread_write()
  2025-01-17 14:16 [PATCH net-next v1 0/5] Some pktgen fixes/improvments Peter Seiderer
  2025-01-17 14:16 ` [PATCH net-next v1 1/5] net: pktgen: replace ENOTSUPP with EOPNOTSUPP Peter Seiderer
  2025-01-17 14:16 ` [PATCH net-next v1 2/5] net: pktgen: enable 'param=value' parsing Peter Seiderer
@ 2025-01-17 14:16 ` Peter Seiderer
  2025-01-17 14:16 ` [PATCH net-next v1 4/5] net: pktgen: fix access outside of user given buffer in pktgen_if_write() Peter Seiderer
  2025-01-17 14:16 ` [PATCH net-next v1 5/5] selftest: net: add proc_net_pktgen Peter Seiderer
  4 siblings, 0 replies; 8+ messages in thread
From: Peter Seiderer @ 2025-01-17 14:16 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, linux-kselftest, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Shuah Khan,
	Toke Høiland-Jørgensen, Frederic Weisbecker,
	Artem Chernyshev, Nam Cao, Peter Seiderer

Honour the user given buffer size for the strn_len() calls (otherwise
strn_len() will access memory outside of the user given buffer).

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
---
 net/core/pktgen.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 4f8ec6c9bed4..9536f9c4d9ef 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -1897,8 +1897,8 @@ static ssize_t pktgen_thread_write(struct file *file,
 	i = len;
 
 	/* Read variable name */
-
-	len = strn_len(&user_buffer[i], sizeof(name) - 1);
+	max = min(sizeof(name) - 1, count - i);
+	len = strn_len(&user_buffer[i], max);
 	if (len < 0)
 		return len;
 
@@ -1928,7 +1928,8 @@ static ssize_t pktgen_thread_write(struct file *file,
 	if (!strcmp(name, "add_device")) {
 		char f[32];
 		memset(f, 0, 32);
-		len = strn_len(&user_buffer[i], sizeof(f) - 1);
+		max = min(sizeof(f) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0) {
 			ret = len;
 			goto out;
-- 
2.48.0


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

* [PATCH net-next v1 4/5] net: pktgen: fix access outside of user given buffer in pktgen_if_write()
  2025-01-17 14:16 [PATCH net-next v1 0/5] Some pktgen fixes/improvments Peter Seiderer
                   ` (2 preceding siblings ...)
  2025-01-17 14:16 ` [PATCH net-next v1 3/5] net: pktgen: fix access outside of user given buffer in pktgen_thread_write() Peter Seiderer
@ 2025-01-17 14:16 ` Peter Seiderer
  2025-01-17 14:16 ` [PATCH net-next v1 5/5] selftest: net: add proc_net_pktgen Peter Seiderer
  4 siblings, 0 replies; 8+ messages in thread
From: Peter Seiderer @ 2025-01-17 14:16 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, linux-kselftest, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Shuah Khan,
	Toke Høiland-Jørgensen, Frederic Weisbecker,
	Artem Chernyshev, Nam Cao, Peter Seiderer

Honour the user given buffer size for the hex32_arg(), num_arg() and
strn_len() calls (otherwise they will access memory outside of the user
given buffer).

In all three functions error out in case no characters a available
(maxlen = 0), in num_arg() additional error out in case no valid
character is parsed.

Additional remove some superfluous variable initializing and align some
variable declarations to the most common pattern.

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
---
 net/core/pktgen.c | 196 ++++++++++++++++++++++++++++++----------------
 1 file changed, 128 insertions(+), 68 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 9536f9c4d9ef..5f54a056fa7c 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -750,6 +750,9 @@ static int hex32_arg(const char __user *user_buffer, unsigned long maxlen,
 	int i = 0;
 	*num = 0;
 
+	if (!maxlen)
+		return -EINVAL;
+
 	for (; i < maxlen; i++) {
 		int value;
 		char c;
@@ -796,6 +799,9 @@ static long num_arg(const char __user *user_buffer, unsigned long maxlen,
 	int i;
 	*num = 0;
 
+	if (!maxlen)
+		return -EINVAL;
+
 	for (i = 0; i < maxlen; i++) {
 		char c;
 		if (get_user(c, &user_buffer[i]))
@@ -803,6 +809,9 @@ static long num_arg(const char __user *user_buffer, unsigned long maxlen,
 		if ((c >= '0') && (c <= '9')) {
 			*num *= 10;
 			*num += c - '0';
+		} else if (i == 0) {
+			// no valid character parsed, error out
+			return -EINVAL;
 		} else
 			break;
 	}
@@ -813,6 +822,9 @@ static int strn_len(const char __user * user_buffer, unsigned int maxlen)
 {
 	int i;
 
+	if (!maxlen)
+		return -EINVAL;
+
 	for (i = 0; i < maxlen; i++) {
 		char c;
 		if (get_user(c, &user_buffer[i]))
@@ -839,11 +851,10 @@ static int strn_len(const char __user * user_buffer, unsigned int maxlen)
  * "size1,weight_1 size2,weight_2 ... size_n,weight_n" for example.
  */
 static ssize_t get_imix_entries(const char __user *buffer,
+				unsigned int maxlen,
 				struct pktgen_dev *pkt_dev)
 {
-	const int max_digits = 10;
-	int i = 0;
-	long len;
+	int i = 0, max, len;
 	char c;
 
 	pkt_dev->n_imix_entries = 0;
@@ -855,7 +866,8 @@ static ssize_t get_imix_entries(const char __user *buffer,
 		if (pkt_dev->n_imix_entries >= MAX_IMIX_ENTRIES)
 			return -E2BIG;
 
-		len = num_arg(&buffer[i], max_digits, &size);
+		max = min(10, maxlen - i);
+		len = num_arg(&buffer[i], max, &size);
 		if (len < 0)
 			return len;
 		i += len;
@@ -869,7 +881,8 @@ static ssize_t get_imix_entries(const char __user *buffer,
 		if (size < 14 + 20 + 8)
 			size = 14 + 20 + 8;
 
-		len = num_arg(&buffer[i], max_digits, &weight);
+		max = min(10, maxlen - i);
+		len = num_arg(&buffer[i], max, &weight);
 		if (len < 0)
 			return len;
 		if (weight <= 0)
@@ -889,18 +902,19 @@ static ssize_t get_imix_entries(const char __user *buffer,
 	return i;
 }
 
-static ssize_t get_labels(const char __user *buffer, struct pktgen_dev *pkt_dev)
+static ssize_t get_labels(const char __user *buffer, int maxlen, struct pktgen_dev *pkt_dev)
 {
 	unsigned int n = 0;
 	char c;
-	ssize_t i = 0;
-	int len;
+	int i = 0, max, len;
 
 	pkt_dev->nr_labels = 0;
 	do {
 		__u32 tmp;
-		len = hex32_arg(&buffer[i], 8, &tmp);
-		if (len <= 0)
+
+		max = min(8, maxlen - i);
+		len = hex32_arg(&buffer[i], max, &tmp);
+		if (len < 0)
 			return len;
 		pkt_dev->labels[n] = htonl(tmp);
 		if (pkt_dev->labels[n] & MPLS_STACK_BOTTOM)
@@ -957,7 +971,6 @@ static ssize_t pktgen_if_write(struct file *file,
 	char name[16], valstr[32];
 	unsigned long value = 0;
 	char *pg_result = NULL;
-	int tmp = 0;
 	char buf[128];
 
 	pg_result = &(pkt_dev->result[0]);
@@ -967,17 +980,16 @@ static ssize_t pktgen_if_write(struct file *file,
 		return -EINVAL;
 	}
 
-	max = count;
-	tmp = count_trail_chars(user_buffer, max);
-	if (tmp < 0) {
+	len = count_trail_chars(user_buffer, count);
+	if (len < 0) {
 		pr_warn("illegal format\n");
-		return tmp;
+		return len;
 	}
-	i = tmp;
+	i = len;
 
 	/* Read variable name */
-
-	len = strn_len(&user_buffer[i], sizeof(name) - 1);
+	max = min(sizeof(name) - 1, count - i);
+	len = strn_len(&user_buffer[i], max);
 	if (len < 0)
 		return len;
 
@@ -1005,7 +1017,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "min_pkt_size")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		max = min(10, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1022,7 +1035,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "max_pkt_size")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		max = min(10, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1041,7 +1055,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	/* Shortcut for min = max */
 
 	if (!strcmp(name, "pkt_size")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		max = min(10, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1061,7 +1076,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		if (pkt_dev->clone_skb > 0)
 			return -EINVAL;
 
-		len = get_imix_entries(&user_buffer[i], pkt_dev);
+		max = count - i;
+		len = get_imix_entries(&user_buffer[i], max, pkt_dev);
 		if (len < 0)
 			return len;
 
@@ -1072,7 +1088,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "debug")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		max = min(10, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1083,7 +1100,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "frags")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		max = min(10, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1093,7 +1111,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "delay")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		max = min(10, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1108,7 +1127,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "rate")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		max = min(10, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1123,7 +1143,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "ratep")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		max = min(10, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1138,7 +1159,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "udp_src_min")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		max = min(10, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1151,7 +1173,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "udp_dst_min")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		max = min(10, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1164,7 +1187,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "udp_src_max")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		max = min(10, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1177,7 +1201,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "udp_dst_max")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		max = min(10, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1190,7 +1215,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "clone_skb")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		max = min(10, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 		/* clone_skb is not supported for netif_receive xmit_mode and
@@ -1211,7 +1237,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "count")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		max = min(10, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1222,7 +1249,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "src_mac_count")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		max = min(10, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1236,7 +1264,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "dst_mac_count")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		max = min(10, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1250,7 +1279,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "burst")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		max = min(10, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1269,7 +1299,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "node")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		max = min(10, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1290,11 +1321,12 @@ static ssize_t pktgen_if_write(struct file *file,
 	if (!strcmp(name, "xmit_mode")) {
 		char f[32];
 
-		memset(f, 0, 32);
-		len = strn_len(&user_buffer[i], sizeof(f) - 1);
+		max = min(sizeof(f) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0)
 			return len;
 
+		memset(f, 0, sizeof(f));
 		if (copy_from_user(f, &user_buffer[i], len))
 			return -EFAULT;
 		i += len;
@@ -1330,11 +1362,12 @@ static ssize_t pktgen_if_write(struct file *file,
 		char f[32];
 		char *end;
 
-		memset(f, 0, 32);
-		len = strn_len(&user_buffer[i], sizeof(f) - 1);
+		max = min(sizeof(f) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0)
 			return len;
 
+		memset(f, 0, 32);
 		if (copy_from_user(f, &user_buffer[i], len))
 			return -EFAULT;
 		i += len;
@@ -1379,7 +1412,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "dst_min") || !strcmp(name, "dst")) {
-		len = strn_len(&user_buffer[i], sizeof(pkt_dev->dst_min) - 1);
+		max = min(sizeof(pkt_dev->dst_min) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0)
 			return len;
 
@@ -1399,7 +1433,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "dst_max")) {
-		len = strn_len(&user_buffer[i], sizeof(pkt_dev->dst_max) - 1);
+		max = min(sizeof(pkt_dev->dst_max) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0)
 			return len;
 
@@ -1419,7 +1454,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "dst6")) {
-		len = strn_len(&user_buffer[i], sizeof(buf) - 1);
+		max = min(sizeof(buf) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0)
 			return len;
 
@@ -1442,7 +1478,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "dst6_min")) {
-		len = strn_len(&user_buffer[i], sizeof(buf) - 1);
+		max = min(sizeof(buf) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0)
 			return len;
 
@@ -1464,7 +1501,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "dst6_max")) {
-		len = strn_len(&user_buffer[i], sizeof(buf) - 1);
+		max = min(sizeof(buf) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0)
 			return len;
 
@@ -1485,7 +1523,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "src6")) {
-		len = strn_len(&user_buffer[i], sizeof(buf) - 1);
+		max = min(sizeof(buf) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0)
 			return len;
 
@@ -1508,7 +1547,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "src_min")) {
-		len = strn_len(&user_buffer[i], sizeof(pkt_dev->src_min) - 1);
+		max = min(sizeof(pkt_dev->src_min) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0)
 			return len;
 
@@ -1528,7 +1568,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "src_max")) {
-		len = strn_len(&user_buffer[i], sizeof(pkt_dev->src_max) - 1);
+		max = min(sizeof(pkt_dev->src_max) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0)
 			return len;
 
@@ -1548,7 +1589,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "dst_mac")) {
-		len = strn_len(&user_buffer[i], sizeof(valstr) - 1);
+		max = min(sizeof(valstr) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0)
 			return len;
 
@@ -1565,7 +1607,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 	if (!strcmp(name, "src_mac")) {
-		len = strn_len(&user_buffer[i], sizeof(valstr) - 1);
+		max = min(sizeof(valstr) - 1, count - i);
+		len = strn_len(&user_buffer[i], max);
 		if (len < 0)
 			return len;
 
@@ -1589,7 +1632,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "flows")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		max = min(10, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1603,7 +1647,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 #ifdef CONFIG_XFRM
 	if (!strcmp(name, "spi")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		max = min(10, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1614,7 +1659,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 #endif
 	if (!strcmp(name, "flowlen")) {
-		len = num_arg(&user_buffer[i], 10, &value);
+		max = min(10, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1625,7 +1671,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "queue_map_min")) {
-		len = num_arg(&user_buffer[i], 5, &value);
+		max = min(5, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1636,7 +1683,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "queue_map_max")) {
-		len = num_arg(&user_buffer[i], 5, &value);
+		max = min(5, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1649,7 +1697,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	if (!strcmp(name, "mpls")) {
 		unsigned int n, cnt;
 
-		len = get_labels(&user_buffer[i], pkt_dev);
+		max = count - i;
+		len = get_labels(&user_buffer[i], max, pkt_dev);
 		if (len < 0)
 			return len;
 		i += len;
@@ -1670,7 +1719,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "vlan_id")) {
-		len = num_arg(&user_buffer[i], 4, &value);
+		max = min(4, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1697,7 +1747,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "vlan_p")) {
-		len = num_arg(&user_buffer[i], 1, &value);
+		max = min(1, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1712,7 +1763,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "vlan_cfi")) {
-		len = num_arg(&user_buffer[i], 1, &value);
+		max = min(1, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1727,7 +1779,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "svlan_id")) {
-		len = num_arg(&user_buffer[i], 4, &value);
+		max = min(4, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1754,7 +1807,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "svlan_p")) {
-		len = num_arg(&user_buffer[i], 1, &value);
+		max = min(1, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1769,7 +1823,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "svlan_cfi")) {
-		len = num_arg(&user_buffer[i], 1, &value);
+		max = min(1, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
@@ -1784,8 +1839,10 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "tos")) {
-		__u32 tmp_value = 0;
-		len = hex32_arg(&user_buffer[i], 2, &tmp_value);
+		__u32 tmp_value;
+
+		max = min(2, count - i);
+		len = hex32_arg(&user_buffer[i], max, &tmp_value);
 		if (len < 0)
 			return len;
 
@@ -1800,8 +1857,10 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "traffic_class")) {
-		__u32 tmp_value = 0;
-		len = hex32_arg(&user_buffer[i], 2, &tmp_value);
+		__u32 tmp_value;
+
+		max = min(2, count - i);
+		len = hex32_arg(&user_buffer[i], max, &tmp_value);
 		if (len < 0)
 			return len;
 
@@ -1816,7 +1875,8 @@ static ssize_t pktgen_if_write(struct file *file,
 	}
 
 	if (!strcmp(name, "skb_priority")) {
-		len = num_arg(&user_buffer[i], 9, &value);
+		max = min(9, count - i);
+		len = num_arg(&user_buffer[i], max, &value);
 		if (len < 0)
 			return len;
 
-- 
2.48.0


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

* [PATCH net-next v1 5/5] selftest: net: add proc_net_pktgen
  2025-01-17 14:16 [PATCH net-next v1 0/5] Some pktgen fixes/improvments Peter Seiderer
                   ` (3 preceding siblings ...)
  2025-01-17 14:16 ` [PATCH net-next v1 4/5] net: pktgen: fix access outside of user given buffer in pktgen_if_write() Peter Seiderer
@ 2025-01-17 14:16 ` Peter Seiderer
  2025-01-17 21:11   ` Jakub Kicinski
  4 siblings, 1 reply; 8+ messages in thread
From: Peter Seiderer @ 2025-01-17 14:16 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, linux-kselftest, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Shuah Khan,
	Toke Høiland-Jørgensen, Frederic Weisbecker,
	Artem Chernyshev, Nam Cao, Peter Seiderer

Add some test for /proc/net/pktgen/... interface.

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
---
 tools/testing/selftests/net/Makefile          |   1 +
 tools/testing/selftests/net/proc_net_pktgen.c | 575 ++++++++++++++++++
 2 files changed, 576 insertions(+)
 create mode 100644 tools/testing/selftests/net/proc_net_pktgen.c

diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 73ee88d6b043..095708cd8345 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -100,6 +100,7 @@ TEST_PROGS += vlan_bridge_binding.sh
 TEST_PROGS += bpf_offload.py
 TEST_PROGS += ipv6_route_update_soft_lockup.sh
 TEST_PROGS += busy_poll_test.sh
+TEST_GEN_PROGS += proc_net_pktgen
 
 # YNL files, must be before "include ..lib.mk"
 YNL_GEN_FILES := busy_poller netlink-dumps
diff --git a/tools/testing/selftests/net/proc_net_pktgen.c b/tools/testing/selftests/net/proc_net_pktgen.c
new file mode 100644
index 000000000000..1d01fa2a96e9
--- /dev/null
+++ b/tools/testing/selftests/net/proc_net_pktgen.c
@@ -0,0 +1,575 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * proc_net_pktgen: kselftest for /proc/net/pktgen interface
+ *
+ * Copyright (c) 2025 Peter Seiderer <ps.report@gmx.net>
+ *
+ */
+#include <errno.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+#include "../kselftest_harness.h"
+
+static const char add_loopback_0[] = "add_device lo@0";
+static const char rm_loopback_0[] = "rem_device_all";
+
+static const char wrong_ctrl_command[] = "forsureawrongcommand";
+static const char legacy_ctrl_command[] = "max_before_softirq";
+
+static const char wrong_device_command[] = "forsurewrongcommand";
+static const char device_command_min_pkt_size_0[] = "min_pkt_size";
+static const char device_command_min_pkt_size_1[] = "min_pkt_size ";
+static const char device_command_min_pkt_size_2[] = "min_pkt_size 0";
+static const char device_command_min_pkt_size_3[] = "min_pkt_size 1";
+static const char device_command_min_pkt_size_4[] = "min_pkt_size 100";
+static const char device_command_min_pkt_size_5[] = "min_pkt_size=1001";
+static const char device_command_min_pkt_size_6[] = "min_pkt_size =2002";
+static const char device_command_min_pkt_size_7[] = "min_pkt_size= 3003";
+static const char device_command_min_pkt_size_8[] = "min_pkt_size = 4004";
+static const char device_command_max_pkt_size_0[] = "max_pkt_size 200";
+static const char device_command_pkt_size_0[] = "pkt_size 300";
+static const char device_command_imix_weights_0[] = "imix_weights 0,7 576,4 1500,1";
+static const char device_command_imix_weights_1[] = "imix_weights 101,1 102,2 103,3 104,4 105,5 106,6 107,7 108,8 109,9 110,10 111,11 112,12 113,13 114,14 115,15 116,16 117,17 118,18 119,19 120,20";
+static const char device_command_imix_weights_2[] = "imix_weights 100,1 102,2 103,3 104,4 105,5 106,6 107,7 108,8 109,9 110,10 111,11 112,12 113,13 114,14 115,15 116,16 117,17 118,18 119,19 120,20 121,21";
+static const char device_command_debug_0[] = "debug 1";
+static const char device_command_debug_1[] = "debug 0";
+static const char device_command_frags_0[] = "frags 100";
+static const char device_command_delay_0[] = "delay 100";
+static const char device_command_delay_1[] = "delay 2147483647";
+static const char device_command_rate_0[] = "rate 100";
+static const char device_command_ratep_0[] = "ratep 200";
+static const char device_command_udp_src_min_0[] = "udp_src_min 1";
+static const char device_command_udp_dst_min_0[] = "udp_dst_min 2";
+static const char device_command_udp_src_max_0[] = "udp_src_max 3";
+static const char device_command_udp_dst_max_0[] = "udp_dst_max 4";
+static const char device_command_clone_skb_0[] = "clone_skb 1";
+static const char device_command_clone_skb_1[] = "clone_skb 0";
+static const char device_command_count_0[] = "count 100";
+static const char device_command_src_mac_count_0[] = "src_mac_count 100";
+static const char device_command_dst_mac_count_0[] = "dst_mac_count 100";
+static const char device_command_burst_0[] = "burst 0";
+static const char device_command_node_0[] = "node 100";
+static const char device_command_xmit_mode_0[] = "xmit_mode start_xmit";
+static const char device_command_xmit_mode_1[] = "xmit_mode netif_receive";
+static const char device_command_xmit_mode_2[] = "xmit_mode queue_xmit";
+static const char device_command_xmit_mode_3[] = "xmit_mode nonsense";
+static const char device_command_flag_0[] = "flag UDPCSUM";
+static const char device_command_flag_1[] = "flag !UDPCSUM";
+static const char device_command_flag_2[] = "flag nonsense";
+static const char device_command_dst_min_0[] = "dst_min 101.102.103.104";
+static const char device_command_dst_0[] = "dst 101.102.103.104";
+static const char device_command_dst_max_0[] = "dst_max 201.202.203.204";
+static const char device_command_dst6_0[] = "dst6 2001:db38:1234:0000:0000:0000:0000:0000";
+static const char device_command_dst6_min_0[] = "dst6_min 2001:db8:1234:0000:0000:0000:0000:0000";
+static const char device_command_dst6_max_0[] = "dst6_max 2001:db8:1234:0000:0000:0000:0000:0000";
+static const char device_command_src6_0[] = "src6 2001:db38:1234:0000:0000:0000:0000:0000";
+static const char device_command_src_min_0[] = "src_min 101.102.103.104";
+static const char device_command_src_max_0[] = "src_max 201.202.203.204";
+static const char device_command_dst_mac_0[] = "dst_mac 01:02:03:04:05:06";
+static const char device_command_src_mac_0[] = "src_mac 11:12:13:14:15:16";
+static const char device_command_clear_counters_0[] = "clear_counters";
+static const char device_command_flows_0[] = "flows 100";
+#if 0 // needs CONFIG_XFRM
+static const char device_command_spi_0[] = "spi 100";
+#endif
+static const char device_command_flowlen_0[] = "flowlen 100";
+static const char device_command_queue_map_min_0[] = "queue_map_min 1";
+static const char device_command_queue_map_max_0[] = "queue_map_max 2";
+static const char device_command_mpls_0[] = "mpls 00000001,000000f2,00000ff3,0000fff4,000ffff5,00fffff6,0ffffff7,fffffff8";
+static const char device_command_vlan_id_0[] = "vlan_id 1";
+static const char device_command_vlan_p_0[] = "vlan_p 1";
+static const char device_command_vlan_cfi_0[] = "vlan_cfi 1";
+static const char device_command_vlan_id_1[] = "vlan_id 4096";
+static const char device_command_svlan_id_0[] = "svlan_id 1";
+static const char device_command_svlan_p_0[] = "svlan_p 1";
+static const char device_command_svlan_cfi_0[] = "svlan_cfi 1";
+static const char device_command_svlan_id_1[] = "svlan_id 4096";
+static const char device_command_tos_0[] = "tos 0";
+static const char device_command_tos_1[] = "tos 0f";
+static const char device_command_tos_2[] = "tos 0ff";
+static const char device_command_traffic_class_0[] = "traffic_class f0";
+static const char device_command_skb_priority_0[] = "skb_priority 999";
+
+FIXTURE(proc_net_pktgen) {
+	int ctrl_fd;
+	int device_fd;
+};
+
+FIXTURE_SETUP(proc_net_pktgen) {
+	ssize_t len;
+
+	self->ctrl_fd = open("/proc/net/pktgen/kpktgend_0", O_RDWR);
+	ASSERT_GE(self->ctrl_fd, 0) TH_LOG("CONFIG_NET_PKTGEN not enabled, module pktgen nod loaded?");
+
+	len = write(self->ctrl_fd, add_loopback_0, sizeof(add_loopback_0));
+	ASSERT_EQ(len, sizeof(add_loopback_0)) TH_LOG("device lo@0 already registered?");
+
+	self->device_fd = open("/proc/net/pktgen/lo@0", O_RDWR);
+	ASSERT_GE(self->device_fd, 0) TH_LOG("device entry for lo@0 missing?");
+}
+
+FIXTURE_TEARDOWN(proc_net_pktgen) {
+	int ret;
+	ssize_t len;
+
+	ret = close(self->device_fd);
+	EXPECT_EQ(ret, 0);
+
+	len = write(self->ctrl_fd, rm_loopback_0, sizeof(rm_loopback_0));
+	EXPECT_EQ(len, sizeof(rm_loopback_0));
+
+	ret = close(self->ctrl_fd);
+	EXPECT_EQ(ret, 0);
+}
+
+TEST_F(proc_net_pktgen, wrong_ctrl_command) {
+	for (int i = 0; i <= sizeof(wrong_ctrl_command); i++) {
+		ssize_t len = write(self->ctrl_fd, wrong_ctrl_command, i);
+		EXPECT_EQ(len, -1);
+		EXPECT_EQ(errno, EINVAL);
+	}
+}
+
+TEST_F(proc_net_pktgen, legacy_ctrl_command) {
+	for (int i = 0; i <= sizeof(legacy_ctrl_command); i++) {
+		ssize_t len = write(self->ctrl_fd, legacy_ctrl_command, i);
+		if (i < (sizeof(legacy_ctrl_command) - 1)) {
+			// incomplete command string
+			EXPECT_EQ(len, -1);
+			EXPECT_EQ(errno, EINVAL);
+		} else {
+			// complete command string without/with trailing '\0'
+			 EXPECT_EQ(len, i);
+		}
+	}
+}
+
+TEST_F(proc_net_pktgen, wrong_device_command) {
+	for (int i = 0; i <= sizeof(wrong_device_command); i++) {
+		ssize_t len = write(self->device_fd, wrong_device_command, i);
+		EXPECT_EQ(len, -1);
+		EXPECT_EQ(errno, EINVAL);
+	}
+}
+
+TEST_F(proc_net_pktgen, device_command_min_pkt_size) {
+	ssize_t len;
+
+	// with trailing '\0'
+	len = write(self->device_fd, device_command_min_pkt_size_0, sizeof(device_command_min_pkt_size_0));
+	EXPECT_EQ(len, -1);
+	EXPECT_EQ(errno, EINVAL);
+
+	// without trailing '\0'
+	len = write(self->device_fd, device_command_min_pkt_size_0, sizeof(device_command_min_pkt_size_0) - 1);
+	EXPECT_EQ(len, -1);
+	EXPECT_EQ(errno, EINVAL);
+
+	// with trailing '\0'
+	len = write(self->device_fd, device_command_min_pkt_size_1, sizeof(device_command_min_pkt_size_1));
+	EXPECT_EQ(len, -1);
+	EXPECT_EQ(errno, EINVAL);
+
+	// without trailing '\0'
+	len = write(self->device_fd, device_command_min_pkt_size_1, sizeof(device_command_min_pkt_size_1) - 1);
+	EXPECT_EQ(len, -1);
+	EXPECT_EQ(errno, EINVAL);
+
+	// with trailing '\0'
+	len = write(self->device_fd, device_command_min_pkt_size_2, sizeof(device_command_min_pkt_size_2));
+	EXPECT_EQ(len, sizeof(device_command_min_pkt_size_2));
+
+	// without trailing '\0'
+	len = write(self->device_fd, device_command_min_pkt_size_2, sizeof(device_command_min_pkt_size_2) - 1);
+	EXPECT_EQ(len, sizeof(device_command_min_pkt_size_2) - 1);
+
+	len = write(self->device_fd, device_command_min_pkt_size_3, sizeof(device_command_min_pkt_size_3));
+	EXPECT_EQ(len, sizeof(device_command_min_pkt_size_3));
+
+	len = write(self->device_fd, device_command_min_pkt_size_4, sizeof(device_command_min_pkt_size_4));
+	EXPECT_EQ(len, sizeof(device_command_min_pkt_size_4));
+
+	len = write(self->device_fd, device_command_min_pkt_size_5, sizeof(device_command_min_pkt_size_5));
+	EXPECT_EQ(len, sizeof(device_command_min_pkt_size_5));
+
+	len = write(self->device_fd, device_command_min_pkt_size_6, sizeof(device_command_min_pkt_size_6));
+	EXPECT_EQ(len, sizeof(device_command_min_pkt_size_6));
+
+	len = write(self->device_fd, device_command_min_pkt_size_7, sizeof(device_command_min_pkt_size_7));
+	EXPECT_EQ(len, sizeof(device_command_min_pkt_size_7));
+
+	len = write(self->device_fd, device_command_min_pkt_size_8, sizeof(device_command_min_pkt_size_8));
+	EXPECT_EQ(len, sizeof(device_command_min_pkt_size_8));
+}
+
+TEST_F(proc_net_pktgen, device_command_max_pkt_size) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_max_pkt_size_0, sizeof(device_command_max_pkt_size_0));
+	EXPECT_EQ(len, sizeof(device_command_max_pkt_size_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_pkt_size) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_pkt_size_0, sizeof(device_command_pkt_size_0));
+	EXPECT_EQ(len, sizeof(device_command_pkt_size_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_imix_weights) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_imix_weights_0, sizeof(device_command_imix_weights_0));
+	EXPECT_EQ(len, sizeof(device_command_imix_weights_0));
+
+	len = write(self->device_fd, device_command_imix_weights_1, sizeof(device_command_imix_weights_1));
+	EXPECT_EQ(len, sizeof(device_command_imix_weights_1));
+
+	len = write(self->device_fd, device_command_imix_weights_2, sizeof(device_command_imix_weights_2));
+	EXPECT_EQ(len, -1);
+	EXPECT_EQ(errno, E2BIG);
+}
+
+TEST_F(proc_net_pktgen, device_command_debug) {
+	ssize_t len;
+
+	// debug on
+	len = write(self->device_fd, device_command_debug_0, sizeof(device_command_debug_0));
+	EXPECT_EQ(len, sizeof(device_command_debug_0));
+
+	// debug off
+	len = write(self->device_fd, device_command_debug_1, sizeof(device_command_debug_1));
+	EXPECT_EQ(len, sizeof(device_command_debug_1));
+}
+
+TEST_F(proc_net_pktgen, device_command_frags) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_frags_0, sizeof(device_command_frags_0));
+	EXPECT_EQ(len, sizeof(device_command_frags_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_delay) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_delay_0, sizeof(device_command_delay_0));
+	EXPECT_EQ(len, sizeof(device_command_delay_0));
+
+	len = write(self->device_fd, device_command_delay_1, sizeof(device_command_delay_1));
+	EXPECT_EQ(len, sizeof(device_command_delay_1));
+}
+
+TEST_F(proc_net_pktgen, device_command_rate) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_rate_0, sizeof(device_command_rate_0));
+	EXPECT_EQ(len, sizeof(device_command_rate_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_ratep) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_ratep_0, sizeof(device_command_ratep_0));
+	EXPECT_EQ(len, sizeof(device_command_ratep_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_udp_src_min) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_udp_src_min_0, sizeof(device_command_udp_src_min_0));
+	EXPECT_EQ(len, sizeof(device_command_udp_src_min_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_udp_dst_min) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_udp_dst_min_0, sizeof(device_command_udp_dst_min_0));
+	EXPECT_EQ(len, sizeof(device_command_udp_dst_min_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_udp_src_max) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_udp_src_max_0, sizeof(device_command_udp_src_max_0));
+	EXPECT_EQ(len, sizeof(device_command_udp_src_max_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_udp_dst_max) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_udp_dst_max_0, sizeof(device_command_udp_dst_max_0));
+	EXPECT_EQ(len, sizeof(device_command_udp_dst_max_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_clone_skb) {
+	ssize_t len;
+
+	// clone_skb on (gives EOPNOTSUPP on lo device)
+	len = write(self->device_fd, device_command_clone_skb_0, sizeof(device_command_clone_skb_0));
+	EXPECT_EQ(len, -1);
+	EXPECT_EQ(errno, EOPNOTSUPP);
+
+	// clone_skb off
+	len = write(self->device_fd, device_command_clone_skb_1, sizeof(device_command_clone_skb_1));
+	EXPECT_EQ(len, sizeof(device_command_clone_skb_1));
+}
+
+TEST_F(proc_net_pktgen, device_command_count) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_count_0, sizeof(device_command_count_0));
+	EXPECT_EQ(len, sizeof(device_command_count_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_src_mac_count) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_src_mac_count_0, sizeof(device_command_src_mac_count_0));
+	EXPECT_EQ(len, sizeof(device_command_src_mac_count_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_dst_mac_count) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_dst_mac_count_0, sizeof(device_command_dst_mac_count_0));
+	EXPECT_EQ(len, sizeof(device_command_dst_mac_count_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_burst) {
+	ssize_t len;
+
+	// burst off
+	len = write(self->device_fd, device_command_burst_0, sizeof(device_command_burst_0));
+	EXPECT_EQ(len, sizeof(device_command_burst_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_node) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_node_0, sizeof(device_command_node_0));
+	EXPECT_EQ(len, sizeof(device_command_node_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_xmit_mode) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_xmit_mode_0, sizeof(device_command_xmit_mode_0));
+	EXPECT_EQ(len, sizeof(device_command_xmit_mode_0));
+
+	len = write(self->device_fd, device_command_xmit_mode_1, sizeof(device_command_xmit_mode_1));
+	EXPECT_EQ(len, sizeof(device_command_xmit_mode_1));
+
+	len = write(self->device_fd, device_command_xmit_mode_2, sizeof(device_command_xmit_mode_2));
+	EXPECT_EQ(len, sizeof(device_command_xmit_mode_2));
+
+	len = write(self->device_fd, device_command_xmit_mode_3, sizeof(device_command_xmit_mode_3));
+	EXPECT_EQ(len, sizeof(device_command_xmit_mode_3));
+}
+
+TEST_F(proc_net_pktgen, device_command_flag) {
+	ssize_t len;
+
+	// flag UDPCSUM on
+	len = write(self->device_fd, device_command_flag_0, sizeof(device_command_flag_0));
+	EXPECT_EQ(len, sizeof(device_command_flag_0));
+
+	// flag UDPCSUM off
+	len = write(self->device_fd, device_command_flag_1, sizeof(device_command_flag_1));
+	EXPECT_EQ(len, sizeof(device_command_flag_1));
+
+	// flag invalid
+	len = write(self->device_fd, device_command_flag_2, sizeof(device_command_flag_2));
+	EXPECT_EQ(len, sizeof(device_command_flag_2));
+}
+
+TEST_F(proc_net_pktgen, device_command_dst_min) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_dst_min_0, sizeof(device_command_dst_min_0));
+	EXPECT_EQ(len, sizeof(device_command_dst_min_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_dst) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_dst_0, sizeof(device_command_dst_0));
+	EXPECT_EQ(len, sizeof(device_command_dst_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_dst_max) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_dst_max_0, sizeof(device_command_dst_max_0));
+	EXPECT_EQ(len, sizeof(device_command_dst_max_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_dst6) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_dst6_0, sizeof(device_command_dst6_0));
+	EXPECT_EQ(len, sizeof(device_command_dst6_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_dst6_min) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_dst6_min_0, sizeof(device_command_dst6_min_0));
+	EXPECT_EQ(len, sizeof(device_command_dst6_min_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_dst6_max) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_dst6_max_0, sizeof(device_command_dst6_max_0));
+	EXPECT_EQ(len, sizeof(device_command_dst6_max_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_src6) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_src6_0, sizeof(device_command_src6_0));
+	EXPECT_EQ(len, sizeof(device_command_src6_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_src_min) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_src_min_0, sizeof(device_command_src_min_0));
+	EXPECT_EQ(len, sizeof(device_command_src_min_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_src_max) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_src_max_0, sizeof(device_command_src_max_0));
+	EXPECT_EQ(len, sizeof(device_command_src_max_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_dst_mac) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_dst_mac_0, sizeof(device_command_dst_mac_0));
+	EXPECT_EQ(len, sizeof(device_command_dst_mac_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_src_mac) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_src_mac_0, sizeof(device_command_src_mac_0));
+	EXPECT_EQ(len, sizeof(device_command_src_mac_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_clear_counters) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_clear_counters_0, sizeof(device_command_clear_counters_0));
+	EXPECT_EQ(len, sizeof(device_command_clear_counters_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_flows) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_flows_0, sizeof(device_command_flows_0));
+	EXPECT_EQ(len, sizeof(device_command_flows_0));
+}
+
+#if 0 // needs CONFIG_XFRM
+TEST_F(proc_net_pktgen, device_command_spi) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_spi_0, sizeof(device_command_spi_0));
+	EXPECT_EQ(len, sizeof(device_command_spi_0));
+}
+#endif
+
+TEST_F(proc_net_pktgen, device_command_flowlen) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_flowlen_0, sizeof(device_command_flowlen_0));
+	EXPECT_EQ(len, sizeof(device_command_flowlen_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_queue_map_min) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_queue_map_min_0, sizeof(device_command_queue_map_min_0));
+	EXPECT_EQ(len, sizeof(device_command_queue_map_min_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_queue_map_max) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_queue_map_max_0, sizeof(device_command_queue_map_max_0));
+	EXPECT_EQ(len, sizeof(device_command_queue_map_max_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_mpls) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_mpls_0, sizeof(device_command_mpls_0));
+	EXPECT_EQ(len, sizeof(device_command_mpls_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_vlan_id) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_vlan_id_0, sizeof(device_command_vlan_id_0));
+	EXPECT_EQ(len, sizeof(device_command_vlan_id_0));
+
+	len = write(self->device_fd, device_command_vlan_p_0, sizeof(device_command_vlan_p_0));
+	EXPECT_EQ(len, sizeof(device_command_vlan_p_0));
+
+	len = write(self->device_fd, device_command_vlan_cfi_0, sizeof(device_command_vlan_cfi_0));
+	EXPECT_EQ(len, sizeof(device_command_vlan_cfi_0));
+
+	len = write(self->device_fd, device_command_vlan_id_1, sizeof(device_command_vlan_id_1));
+	EXPECT_EQ(len, sizeof(device_command_vlan_id_1));
+}
+
+TEST_F(proc_net_pktgen, device_command_svlan_id) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_svlan_id_0, sizeof(device_command_svlan_id_0));
+	EXPECT_EQ(len, sizeof(device_command_svlan_id_0));
+
+	len = write(self->device_fd, device_command_svlan_p_0, sizeof(device_command_svlan_p_0));
+	EXPECT_EQ(len, sizeof(device_command_svlan_p_0));
+
+	len = write(self->device_fd, device_command_svlan_cfi_0, sizeof(device_command_svlan_cfi_0));
+	EXPECT_EQ(len, sizeof(device_command_svlan_cfi_0));
+
+	len = write(self->device_fd, device_command_svlan_id_1, sizeof(device_command_svlan_id_1));
+	EXPECT_EQ(len, sizeof(device_command_svlan_id_1));
+}
+
+
+TEST_F(proc_net_pktgen, device_command_tos) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_tos_0, sizeof(device_command_tos_0));
+	EXPECT_EQ(len, sizeof(device_command_tos_0));
+
+	len = write(self->device_fd, device_command_tos_1, sizeof(device_command_tos_1));
+	EXPECT_EQ(len, sizeof(device_command_tos_1));
+
+	len = write(self->device_fd, device_command_tos_2, sizeof(device_command_tos_2));
+	EXPECT_EQ(len, sizeof(device_command_tos_2));
+}
+
+
+TEST_F(proc_net_pktgen, device_command_traffic_class) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_traffic_class_0, sizeof(device_command_traffic_class_0));
+	EXPECT_EQ(len, sizeof(device_command_traffic_class_0));
+}
+
+TEST_F(proc_net_pktgen, device_command_skb_priority) {
+	ssize_t len;
+
+	len = write(self->device_fd, device_command_skb_priority_0, sizeof(device_command_skb_priority_0));
+	EXPECT_EQ(len, sizeof(device_command_skb_priority_0));
+}
+
+TEST_HARNESS_MAIN
-- 
2.48.0


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

* Re: [PATCH net-next v1 5/5] selftest: net: add proc_net_pktgen
  2025-01-17 14:16 ` [PATCH net-next v1 5/5] selftest: net: add proc_net_pktgen Peter Seiderer
@ 2025-01-17 21:11   ` Jakub Kicinski
  2025-01-22 14:40     ` Peter Seiderer
  0 siblings, 1 reply; 8+ messages in thread
From: Jakub Kicinski @ 2025-01-17 21:11 UTC (permalink / raw)
  To: Peter Seiderer
  Cc: netdev, linux-kernel, linux-kselftest, David S . Miller,
	Eric Dumazet, Paolo Abeni, Simon Horman, Shuah Khan,
	Toke Høiland-Jørgensen, Frederic Weisbecker,
	Artem Chernyshev, Nam Cao

On Fri, 17 Jan 2025 15:16:13 +0100 Peter Seiderer wrote:
> +FIXTURE_SETUP(proc_net_pktgen) {
> +	ssize_t len;
> +
> +	self->ctrl_fd = open("/proc/net/pktgen/kpktgend_0", O_RDWR);
> +	ASSERT_GE(self->ctrl_fd, 0) TH_LOG("CONFIG_NET_PKTGEN not enabled, module pktgen nod loaded?");

nod -> not?

Please take a look at the instructions here:
https://github.com/linux-netdev/nipa/wiki/How-to-run-netdev-selftests-CI-style
the test currently fails in our CI, you need to add it to
tools/testing/selftests/net/config, and perhaps try to call
modprobe in the test?

> +	len = write(self->ctrl_fd, add_loopback_0, sizeof(add_loopback_0));
> +	ASSERT_EQ(len, sizeof(add_loopback_0)) TH_LOG("device lo@0 already registered?");

FWIW we prefer to stick to 80 char line width in networking, 
but it's not a big deal for a test, up to you.

> +			// complete command string without/with trailing '\0'
> +			 EXPECT_EQ(len, i);

Run this patch thru checkpatch, please. This looks misaligned.

> +		}
> +	}
> +}

> +#if 0 // needs CONFIG_XFRM

Add it to the config, too, then?

> +TEST_F(proc_net_pktgen, device_command_spi) {
> +	ssize_t len;
> +
> +	len = write(self->device_fd, device_command_spi_0, sizeof(device_command_spi_0));
> +	EXPECT_EQ(len, sizeof(device_command_spi_0));
> +}
> +#endif

Thanks for working on a test!

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

* Re: [PATCH net-next v1 5/5] selftest: net: add proc_net_pktgen
  2025-01-17 21:11   ` Jakub Kicinski
@ 2025-01-22 14:40     ` Peter Seiderer
  0 siblings, 0 replies; 8+ messages in thread
From: Peter Seiderer @ 2025-01-22 14:40 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, linux-kernel, linux-kselftest, David S . Miller,
	Eric Dumazet, Paolo Abeni, Simon Horman, Shuah Khan,
	Toke Høiland-Jørgensen, Frederic Weisbecker,
	Artem Chernyshev, Nam Cao

Hello Jakub,

On Fri, 17 Jan 2025 13:11:54 -0800, Jakub Kicinski <kuba@kernel.org> wrote:

> On Fri, 17 Jan 2025 15:16:13 +0100 Peter Seiderer wrote:
> > +FIXTURE_SETUP(proc_net_pktgen) {
> > +	ssize_t len;
> > +
> > +	self->ctrl_fd = open("/proc/net/pktgen/kpktgend_0", O_RDWR);
> > +	ASSERT_GE(self->ctrl_fd, 0) TH_LOG("CONFIG_NET_PKTGEN not enabled, module pktgen nod loaded?");
>
> nod -> not?

Fixed...

>
> Please take a look at the instructions here:
> https://github.com/linux-netdev/nipa/wiki/How-to-run-netdev-selftests-CI-style
> the test currently fails in our CI, you need to add it to
> tools/testing/selftests/net/config, and perhaps try to call
> modprobe in the test?

Thanks for the hint, fixed (modprobe and CONFIG_NET_PKTGEN enabeled)...

>
> > +	len = write(self->ctrl_fd, add_loopback_0, sizeof(add_loopback_0));
> > +	ASSERT_EQ(len, sizeof(add_loopback_0)) TH_LOG("device lo@0 already registered?");
>
> FWIW we prefer to stick to 80 char line width in networking,
> but it's not a big deal for a test, up to you.
>
> > +			// complete command string without/with trailing '\0'
> > +			 EXPECT_EQ(len, i);

Fixed...

>
> Run this patch thru checkpatch, please. This looks misaligned.

O.k.

>
> > +		}
> > +	}
> > +}
>
> > +#if 0 // needs CONFIG_XFRM
>
> Add it to the config, too, then?
>
> > +TEST_F(proc_net_pktgen, device_command_spi) {
> > +	ssize_t len;
> > +
> > +	len = write(self->device_fd, device_command_spi_0, sizeof(device_command_spi_0));
> > +	EXPECT_EQ(len, sizeof(device_command_spi_0));
> > +}
> > +#endif

'#if' removed as as CONFIG_XFRM is already enabled via tools/testing/selftests/net/config
CONFIG_XFRM_INTERFACE/CONFIG_XFRM_USER...

Thanks for review!

New patch iteration is on the way...

Regards,
Peter

>
> Thanks for working on a test!


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

end of thread, other threads:[~2025-01-22 14:40 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-17 14:16 [PATCH net-next v1 0/5] Some pktgen fixes/improvments Peter Seiderer
2025-01-17 14:16 ` [PATCH net-next v1 1/5] net: pktgen: replace ENOTSUPP with EOPNOTSUPP Peter Seiderer
2025-01-17 14:16 ` [PATCH net-next v1 2/5] net: pktgen: enable 'param=value' parsing Peter Seiderer
2025-01-17 14:16 ` [PATCH net-next v1 3/5] net: pktgen: fix access outside of user given buffer in pktgen_thread_write() Peter Seiderer
2025-01-17 14:16 ` [PATCH net-next v1 4/5] net: pktgen: fix access outside of user given buffer in pktgen_if_write() Peter Seiderer
2025-01-17 14:16 ` [PATCH net-next v1 5/5] selftest: net: add proc_net_pktgen Peter Seiderer
2025-01-17 21:11   ` Jakub Kicinski
2025-01-22 14:40     ` Peter Seiderer

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