[PATCH 0/3] Add IMIX mode

From: Nick Richardson <redacted>

Adds internet mix (IMIX) mode to pktgen. Internet mix is
included in many user-space network perf testing tools. It allows
for the user to specify a distribution of discrete packet sizes to be
generated. This type of test is common among vendors when perf testing 
their devices.
[RFC link: https://datatracker.ietf.org/doc/html/rfc2544#section-9.1]

This allows users to get a
more complete picture of how their device will perform in the
real-world.

This feature adds a command that allows users to specify an imix
distribution in the following format:
  imix_weights size_1,weight_1 size_2,weight_2 ... size_n,weight_n

The distribution of packets with size_i will be 
(weight_i / total_weights) where
total_weights = weight_1 + weight_2 + ... + weight_n

For example:
  imix_weights 40,7 576,4 1500,1

The pkt_size "40" will account for 7 / (7 + 4 + 1) = ~58% of the total
packets sent.

This patch was tested with the following:
1. imix_weights = 40,7 576,4 1500,1
2. imix_weights = 0,7 576,4 1500,1
  - Packet size of 0 is resized to the minimum, 42
3. imix_weights = 40,7 576,4 1500,1 count = 0
  - Zero count.
  - Runs until user stops pktgen.
Invalid Configurations
1. clone_skb = 200 imix_weights = 40,7 576,4 1500,1
    - Returns error code -524 (-ENOTSUPP) when setting imix_weights
2. len(imix_weights) > MAX_IMIX_ENTRIES
    - Returns -7 (-E2BIG)

This patch is split into three parts, each provide different aspects of
required functionality:
  1. Parse internet mix input.
  2. Add IMIX Distribution representation.
  3. Process and output IMIX results.

Nick Richardson (3):
  pktgen: Parse internet mix (imix) input
  pktgen: Add imix distribution bins
  pktgen: Add output for imix results

 net/core/pktgen.c | 163 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 162 insertions(+), 1 deletion(-)

-- 
2.32.0.605.g8dce9f2422-goog

From: Nick Richardson <redacted>

Adds "imix_weights" command for specifying internet mix distribution.

The command is in this format:
"imix_weights size_1,weight_1 size_2,weight_2 ... size_n,weight_n"
where the probability that packet size_i is picked is:
weight_i / (weight_1 + weight_2 + .. + weight_n)

The user may provide up to 20 imix entries (size_i,weight_i) in this
command.

The user specified imix entries will be displayed in the "Params"
section of the interface output.

Values for clone_skb > 0 is not supported in IMIX mode.

Summary of changes:
Add flag for enabling internet mix mode.
Add command (imix_weights) for internet mix input.
Return -ENOTSUPP when clone_skb > 0 in IMIX mode.
Display imix_weights in Params.
Create data structures to store imix entries and distribution.

Signed-off-by: Nick Richardson <redacted>
---
 net/core/pktgen.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 95 insertions(+)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 7e258d255e90..83c83e1b5f28 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c

@@ -175,6 +175,8 @@
 #define IP_NAME_SZ 32
 #define MAX_MPLS_LABELS 16 /* This is the max label stack depth */
 #define MPLS_STACK_BOTTOM htonl(0x00000100)
+/* Max number of internet mix entries that can be specified in imix_weights. */
+#define MAX_IMIX_ENTRIES 20
 
 #define func_enter() pr_debug("entering %s\n", __func__);
 

@@ -242,6 +244,12 @@ static char *pkt_flag_names[] = {
 #define VLAN_TAG_SIZE(x) ((x)->vlan_id == 0xffff ? 0 : 4)
 #define SVLAN_TAG_SIZE(x) ((x)->svlan_id == 0xffff ? 0 : 4)
 
+struct imix_pkt {
+	__u64 size;
+	__u64 weight;
+	__u64 count_so_far;
+};
+
 struct flow_state {
 	__be32 cur_daddr;
 	int count;

@@ -343,6 +351,10 @@ struct pktgen_dev {
 	__u8 traffic_class;  /* ditto for the (former) Traffic Class in IPv6
 				(see RFC 3260, sec. 4) */
 
+	/* IMIX */
+	unsigned int n_imix_entries;
+	struct imix_pkt imix_entries[MAX_IMIX_ENTRIES];
+
 	/* MPLS */
 	unsigned int nr_labels;	/* Depth of stack, 0 = no MPLS */
 	__be32 labels[MAX_MPLS_LABELS];

@@ -552,6 +564,16 @@ static int pktgen_if_show(struct seq_file *seq, void *v)
 		   (unsigned long long)pkt_dev->count, pkt_dev->min_pkt_size,
 		   pkt_dev->max_pkt_size);
 
+	if (pkt_dev->n_imix_entries > 0) {
+		seq_printf(seq, "     imix_weights: ");
+		for (i = 0; i < pkt_dev->n_imix_entries; i++) {
+			seq_printf(seq, "%llu,%llu ",
+				   pkt_dev->imix_entries[i].size,
+				   pkt_dev->imix_entries[i].weight);
+		}
+		seq_printf(seq, "\n");
+	}
+
 	seq_printf(seq,
 		   "     frags: %d  delay: %llu  clone_skb: %d  ifname: %s\n",
 		   pkt_dev->nfrags, (unsigned long long) pkt_dev->delay,

@@ -792,6 +814,61 @@ static int strn_len(const char __user * user_buffer, unsigned int maxlen)
 	return i;
 }
 
+static ssize_t get_imix_entries(const char __user *buffer,
+				struct pktgen_dev *pkt_dev)
+{
+	/* Parses imix entries from user buffer.
+	 * The user buffer should consist of imix entries separated by spaces
+	 * where each entry consists of size and weight delimited by commas.
+	 * "size1,weight_1 size2,weight_2 ... size_n,weight_n" for example.
+	 */
+	long len;
+	char c;
+	int i = 0;
+	const int max_digits = 10;
+
+	pkt_dev->n_imix_entries = 0;
+
+	do {
+		unsigned long size;
+		unsigned long weight;
+
+		len = num_arg(&buffer[i], max_digits, &size);
+		if (len < 0)
+			return len;
+		i += len;
+		if (get_user(c, &buffer[i]))
+			return -EFAULT;
+		/* Check for comma between size_i and weight_i */
+		if (c != ',')
+			return -EINVAL;
+		i++;
+
+		if (size < 14 + 20 + 8)
+			size = 14 + 20 + 8;
+
+		len = num_arg(&buffer[i], max_digits, &weight);
+		if (len < 0)
+			return len;
+		if (weight <= 0)
+			return -EINVAL;
+
+		pkt_dev->imix_entries[pkt_dev->n_imix_entries].size = size;
+		pkt_dev->imix_entries[pkt_dev->n_imix_entries].weight = weight;
+
+		i += len;
+		if (get_user(c, &buffer[i]))
+			return -EFAULT;
+
+		i++;
+		pkt_dev->n_imix_entries++;
+
+		if (pkt_dev->n_imix_entries > MAX_IMIX_ENTRIES)
+			return -E2BIG;
+	} while (c == ' ');
+	return i;
+}
+
 static ssize_t get_labels(const char __user *buffer, struct pktgen_dev *pkt_dev)
 {
 	unsigned int n = 0;

@@ -960,6 +1037,18 @@ static ssize_t pktgen_if_write(struct file *file,
 		return count;
 	}
 
+	if (!strcmp(name, "imix_weights")) {
+		if (pkt_dev->clone_skb > 0)
+			return -ENOTSUPP;
+
+		len = get_imix_entries(&user_buffer[i], pkt_dev);
+		if (len < 0)
+			return len;
+
+		i += len;
+		return count;
+	}
+
 	if (!strcmp(name, "debug")) {
 		len = num_arg(&user_buffer[i], 10, &value);
 		if (len < 0)

@@ -1082,10 +1171,16 @@ static ssize_t pktgen_if_write(struct file *file,
 		len = num_arg(&user_buffer[i], 10, &value);
 		if (len < 0)
 			return len;
+		/* clone_skb is not supported for netif_receive xmit_mode and
+		 * IMIX mode.
+		 */
 		if ((value > 0) &&
 		    ((pkt_dev->xmit_mode == M_NETIF_RECEIVE) ||
 		     !(pkt_dev->odev->priv_flags & IFF_TX_SKB_SHARING)))
 			return -ENOTSUPP;
+		if (value > 0 && pkt_dev->n_imix_entries > 0)
+			return -ENOTSUPP;
+
 		i += len;
 		pkt_dev->clone_skb = value;
 

-- 
2.32.0.605.g8dce9f2422-goog

From: Nick Richardson <redacted>

In order to represent the distribution of imix packet sizes, a
pre-computed data structure is used. It features 100 (IMIX_PRECISION)
"bins". Contiguous ranges of these bins represent the respective
packet size of each imix entry. This is done to avoid the overhead of
selecting the correct imix packet size based on the corresponding weights.

Example:
imix_weights 40,7 576,4 1500,1
total_weight = 7 + 4 + 1 = 12

pkt_size 40 occurs 7/total_weight = 58% of the time
pkt_size 576 occurs 4/total_weight = 33% of the time
pkt_size 1500 occurs 1/total_weight = 9% of the time

We generate a random number between [0-99] and select the corresponding
packet size based on the specified weights.
Eg. random number = 358723895 % 100 = 65
Selects the packet size corresponding to index:65 in the pre-computed
imix_distribution array.
An example of the  pre-computed array is below:

The imix_distribution will look like the following:
0        ->  0 (index of imix_entry.size == 40)
1        ->  0 (index of imix_entry.size == 40)
2        ->  0 (index of imix_entry.size == 40)
[...]    ->  0 (index of imix_entry.size == 40)
57       ->  0 (index of imix_entry.size == 40)
58       ->  1 (index of imix_entry.size == 576)
[...]    ->  1 (index of imix_entry.size == 576)
90       ->  1 (index of imix_entry.size == 576)
91       ->  2 (index of imix_entry.size == 1500)
[...]    ->  2 (index of imix_entry.size == 1500)
99       ->  2 (index of imix_entry.size == 1500)

Create and use "bin" representation of the imix distribution.

Signed-off-by: Nick Richardson <redacted>
---
 net/core/pktgen.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 83c83e1b5f28..be46de1d6eec 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c

@@ -177,6 +177,7 @@
 #define MPLS_STACK_BOTTOM htonl(0x00000100)
 /* Max number of internet mix entries that can be specified in imix_weights. */
 #define MAX_IMIX_ENTRIES 20
+#define IMIX_PRECISION 100 /* Precision of IMIX distribution */
 
 #define func_enter() pr_debug("entering %s\n", __func__);
 

@@ -354,6 +355,8 @@ struct pktgen_dev {
 	/* IMIX */
 	unsigned int n_imix_entries;
 	struct imix_pkt imix_entries[MAX_IMIX_ENTRIES];
+	/* Maps 0-IMIX_PRECISION range to imix_entry based on probability*/
+	__u8 imix_distribution[IMIX_PRECISION];
 
 	/* MPLS */
 	unsigned int nr_labels;	/* Depth of stack, 0 = no MPLS */

@@ -483,6 +486,7 @@ static void pktgen_stop_all_threads(struct pktgen_net *pn);
 
 static void pktgen_stop(struct pktgen_thread *t);
 static void pktgen_clear_counters(struct pktgen_dev *pkt_dev);
+static void fill_imix_distribution(struct pktgen_dev *pkt_dev);
 
 /* Module parameters, defaults. */
 static int pg_count_d __read_mostly = 1000;

@@ -1045,6 +1049,8 @@ static ssize_t pktgen_if_write(struct file *file,
 		if (len < 0)
 			return len;
 
+		fill_imix_distribution(pkt_dev);
+
 		i += len;
 		return count;
 	}

@@ -2572,6 +2578,14 @@ static void mod_cur_headers(struct pktgen_dev *pkt_dev)
 				t = pkt_dev->min_pkt_size;
 		}
 		pkt_dev->cur_pkt_size = t;
+	} else if (pkt_dev->n_imix_entries > 0) {
+		struct imix_pkt *entry;
+		__u32 t = prandom_u32() % IMIX_PRECISION;
+		__u8 entry_index = pkt_dev->imix_distribution[t];
+
+		entry = &pkt_dev->imix_entries[entry_index];
+		entry->count_so_far++;
+		pkt_dev->cur_pkt_size = entry->size;
 	}
 
 	set_cur_queue_map(pkt_dev);

@@ -2640,6 +2654,33 @@ static void free_SAs(struct pktgen_dev *pkt_dev)
 	}
 }
 
+static void fill_imix_distribution(struct pktgen_dev *pkt_dev)
+{
+	int cumulative_probabilites[MAX_IMIX_ENTRIES];
+	int j = 0;
+	__u64 cumulative_prob = 0;
+	__u64 total_weight = 0;
+	int i = 0;
+
+	for (i = 0; i < pkt_dev->n_imix_entries; i++)
+		total_weight += pkt_dev->imix_entries[i].weight;
+
+	/* Fill cumulative_probabilites with sum of normalized probabilities */
+	for (i = 0; i < pkt_dev->n_imix_entries - 1; i++) {
+		cumulative_prob += div64_u64(pkt_dev->imix_entries[i].weight *
+						     IMIX_PRECISION,
+					     total_weight);
+		cumulative_probabilites[i] = cumulative_prob;
+	}
+	cumulative_probabilites[pkt_dev->n_imix_entries - 1] = 100;
+
+	for (i = 0; i < IMIX_PRECISION; i++) {
+		if (i == cumulative_probabilites[j])
+			j++;
+		pkt_dev->imix_distribution[i] = j;
+	}
+}
+
 static int process_ipsec(struct pktgen_dev *pkt_dev,
 			      struct sk_buff *skb, __be16 protocol)
 {

-- 
2.32.0.605.g8dce9f2422-goog

From: Nick Richardson <redacted>

The bps for imix mode is calculated by:
sum(imix_entry.size) / time_elapsed

The actual counts of each imix_entry are displayed under the
"Current:" section of the interface output in the following format:
imix_size_counts: size_1,count_1 size_2,count_2 ... size_n,count_n

Example (count = 200000):
imix_weights: 256,1 859,3 205,2
imix_counts: 256,32082 859,99796 205,68122
Result: OK: 17992362(c17964678+d27684) usec, 200000 (859byte,0frags)
  11115pps 47Mb/sec (47977140bps) errors: 0

Summary of changes:
Calculate bps based on imix counters when in IMIX mode.
Add output for IMIX counters.

Signed-off-by: Nick Richardson <redacted>
---
 net/core/pktgen.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index be46de1d6eec..b1705e62b10f 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c

@@ -695,6 +695,18 @@ static int pktgen_if_show(struct seq_file *seq, void *v)
 		   (unsigned long long)pkt_dev->sofar,
 		   (unsigned long long)pkt_dev->errors);
 
+	if (pkt_dev->n_imix_entries > 0) {
+		int i;
+
+		seq_printf(seq, "     imix_size_counts: ");
+		for (i = 0; i < pkt_dev->n_imix_entries; i++) {
+			seq_printf(seq, "%llu,%llu ",
+				   pkt_dev->imix_entries[i].size,
+				   pkt_dev->imix_entries[i].count_so_far);
+		}
+		seq_printf(seq, "\n");
+	}
+
 	seq_printf(seq,
 		   "     started: %lluus  stopped: %lluus idle: %lluus\n",
 		   (unsigned long long) ktime_to_us(pkt_dev->started_at),

@@ -3281,7 +3293,19 @@ static void show_results(struct pktgen_dev *pkt_dev, int nr_frags)
 	pps = div64_u64(pkt_dev->sofar * NSEC_PER_SEC,
 			ktime_to_ns(elapsed));
 
-	bps = pps * 8 * pkt_dev->cur_pkt_size;
+	if (pkt_dev->n_imix_entries > 0) {
+		int i;
+		struct imix_pkt *entry;
+
+		bps = 0;
+		for (i = 0; i < pkt_dev->n_imix_entries; i++) {
+			entry = &pkt_dev->imix_entries[i];
+			bps += entry->size * entry->count_so_far;
+		}
+		bps = div64_u64(bps * 8 * NSEC_PER_SEC, ktime_to_ns(elapsed));
+	} else {
+		bps = pps * 8 * pkt_dev->cur_pkt_size;
+	}
 
 	mbps = bps;
 	do_div(mbps, 1000000);

-- 
2.32.0.605.g8dce9f2422-goog

On Mon,  9 Aug 2021 17:22:02 +0000 Nicholas Richardson wrote:

no need for the __ prefix outside of uAPI.

seq_puts()

This comments belongs before the function.

+	long len;
+	char c;
+	int i = 0;
+	const int max_digits = 10;

Please order these lines longest to shortest (reverse xmas tree).

+	pkt_dev->n_imix_entries = 0;
+
+	do {
+		unsigned long size;
+		unsigned long weight;

same

+
+		len = num_arg(&buffer[i], max_digits, &size);
+		if (len < 0)
+			return len;
+		i += len;
+		if (get_user(c, &buffer[i]))
+			return -EFAULT;
+		/* Check for comma between size_i and weight_i */
+		if (c != ',')
+			return -EINVAL;
+		i++;
+
+		if (size < 14 + 20 + 8)
+			size = 14 + 20 + 8;

Why overwrite instead of rejecting?

+		len = num_arg(&buffer[i], max_digits, &weight);
+		if (len < 0)
+			return len;
+		if (weight <= 0)
+			return -EINVAL;
+
+		pkt_dev->imix_entries[pkt_dev->n_imix_entries].size = size;
+		pkt_dev->imix_entries[pkt_dev->n_imix_entries].weight = weight;
+
+		i += len;
+		if (get_user(c, &buffer[i]))
+			return -EFAULT;

What if this is the last entry?

+		i++;
+		pkt_dev->n_imix_entries++;
+
+		if (pkt_dev->n_imix_entries > MAX_IMIX_ENTRIES)
+			return -E2BIG;
+	} while (c == ' ');

empty line here

ENOTSUPP should not be returned to user space, please use a different
one.

ditto

 		i += len;
 		pkt_dev->clone_skb = value;
 

Hey Jakub, thanks for the quick response!

quoted

+
+             len = num_arg(&buffer[i], max_digits, &size);
+             if (len < 0)
+                     return len;
+             i += len;
+             if (get_user(c, &buffer[i]))
+                     return -EFAULT;
+             /* Check for comma between size_i and weight_i */
+             if (c != ',')
+                     return -EINVAL;
+             i++;
+
+             if (size < 14 + 20 + 8)
+                     size = 14 + 20 + 8;

Why overwrite instead of rejecting?

I overwrite here to keep behavior similar to when pkt_size is set directly.
When the pkt_size command is used the size value is overwritten to the
minimum packet size (14 + 8 + 20).
See the pkt_size section in pktgen_if_write().

quoted

+             len = num_arg(&buffer[i], max_digits, &weight);
+             if (len < 0)
+                     return len;
+             if (weight <= 0)
+                     return -EINVAL;
+
+             pkt_dev->imix_entries[pkt_dev->n_imix_entries].size = size;
+             pkt_dev->imix_entries[pkt_dev->n_imix_entries].weight = weight;
+
+             i += len;
+             if (get_user(c, &buffer[i]))
+                     return -EFAULT;

What if this is the last entry?

If this is the last entry then the line terminating character is read.
Similar code can be found in the get_labels() function in pktgen.c


On Mon, Aug 9, 2021 at 5:05 PM Jakub Kicinski [off-list ref] wrote:

On Mon,  9 Aug 2021 17:22:02 +0000 Nicholas Richardson wrote:

quoted

From: Nick Richardson <redacted>

Adds "imix_weights" command for specifying internet mix distribution.

The command is in this format:
"imix_weights size_1,weight_1 size_2,weight_2 ... size_n,weight_n"
where the probability that packet size_i is picked is:
weight_i / (weight_1 + weight_2 + .. + weight_n)

The user may provide up to 20 imix entries (size_i,weight_i) in this
command.

The user specified imix entries will be displayed in the "Params"
section of the interface output.

Values for clone_skb > 0 is not supported in IMIX mode.

Summary of changes:
Add flag for enabling internet mix mode.
Add command (imix_weights) for internet mix input.
Return -ENOTSUPP when clone_skb > 0 in IMIX mode.
Display imix_weights in Params.
Create data structures to store imix entries and distribution.

Signed-off-by: Nick Richardson <redacted>
---
 net/core/pktgen.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 95 insertions(+)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 7e258d255e90..83c83e1b5f28 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c

@@ -175,6 +175,8 @@
 #define IP_NAME_SZ 32
 #define MAX_MPLS_LABELS 16 /* This is the max label stack depth */
 #define MPLS_STACK_BOTTOM htonl(0x00000100)
+/* Max number of internet mix entries that can be specified in imix_weights. */
+#define MAX_IMIX_ENTRIES 20

 #define func_enter() pr_debug("entering %s\n", __func__);

@@ -242,6 +244,12 @@ static char *pkt_flag_names[] = {
 #define VLAN_TAG_SIZE(x) ((x)->vlan_id == 0xffff ? 0 : 4)
 #define SVLAN_TAG_SIZE(x) ((x)->svlan_id == 0xffff ? 0 : 4)

+struct imix_pkt {
+     __u64 size;
+     __u64 weight;
+     __u64 count_so_far;

no need for the __ prefix outside of uAPI.

quoted

+};
+
 struct flow_state {
      __be32 cur_daddr;
      int count;

@@ -343,6 +351,10 @@ struct pktgen_dev {
      __u8 traffic_class;  /* ditto for the (former) Traffic Class in IPv6
                              (see RFC 3260, sec. 4) */

+     /* IMIX */
+     unsigned int n_imix_entries;
+     struct imix_pkt imix_entries[MAX_IMIX_ENTRIES];
+
      /* MPLS */
      unsigned int nr_labels; /* Depth of stack, 0 = no MPLS */
      __be32 labels[MAX_MPLS_LABELS];

@@ -552,6 +564,16 @@ static int pktgen_if_show(struct seq_file *seq, void *v)
                 (unsigned long long)pkt_dev->count, pkt_dev->min_pkt_size,
                 pkt_dev->max_pkt_size);

+     if (pkt_dev->n_imix_entries > 0) {
+             seq_printf(seq, "     imix_weights: ");
+             for (i = 0; i < pkt_dev->n_imix_entries; i++) {
+                     seq_printf(seq, "%llu,%llu ",
+                                pkt_dev->imix_entries[i].size,
+                                pkt_dev->imix_entries[i].weight);
+             }
+             seq_printf(seq, "\n");

seq_puts()

quoted

+     }
+
      seq_printf(seq,
                 "     frags: %d  delay: %llu  clone_skb: %d  ifname: %s\n",
                 pkt_dev->nfrags, (unsigned long long) pkt_dev->delay,

@@ -792,6 +814,61 @@ static int strn_len(const char __user * user_buffer, unsigned int maxlen)
      return i;
 }

+static ssize_t get_imix_entries(const char __user *buffer,
+                             struct pktgen_dev *pkt_dev)
+{
+     /* Parses imix entries from user buffer.
+      * The user buffer should consist of imix entries separated by spaces
+      * where each entry consists of size and weight delimited by commas.
+      * "size1,weight_1 size2,weight_2 ... size_n,weight_n" for example.
+      */

This comments belongs before the function.

quoted

+     long len;
+     char c;
+     int i = 0;
+     const int max_digits = 10;

Please order these lines longest to shortest (reverse xmas tree).

quoted

+     pkt_dev->n_imix_entries = 0;
+
+     do {
+             unsigned long size;
+             unsigned long weight;

same

quoted

+
+             len = num_arg(&buffer[i], max_digits, &size);
+             if (len < 0)
+                     return len;
+             i += len;
+             if (get_user(c, &buffer[i]))
+                     return -EFAULT;
+             /* Check for comma between size_i and weight_i */
+             if (c != ',')
+                     return -EINVAL;
+             i++;
+
+             if (size < 14 + 20 + 8)
+                     size = 14 + 20 + 8;

Why overwrite instead of rejecting?

quoted

+             len = num_arg(&buffer[i], max_digits, &weight);
+             if (len < 0)
+                     return len;
+             if (weight <= 0)
+                     return -EINVAL;
+
+             pkt_dev->imix_entries[pkt_dev->n_imix_entries].size = size;
+             pkt_dev->imix_entries[pkt_dev->n_imix_entries].weight = weight;
+
+             i += len;
+             if (get_user(c, &buffer[i]))
+                     return -EFAULT;

What if this is the last entry?

quoted

+             i++;
+             pkt_dev->n_imix_entries++;
+
+             if (pkt_dev->n_imix_entries > MAX_IMIX_ENTRIES)
+                     return -E2BIG;
+     } while (c == ' ');

empty line here

quoted

+     return i;
+}
+
 static ssize_t get_labels(const char __user *buffer, struct pktgen_dev *pkt_dev)
 {
      unsigned int n = 0;

@@ -960,6 +1037,18 @@ static ssize_t pktgen_if_write(struct file *file,
              return count;
      }

+     if (!strcmp(name, "imix_weights")) {
+             if (pkt_dev->clone_skb > 0)
+                     return -ENOTSUPP;

ENOTSUPP should not be returned to user space, please use a different
one.

quoted

+             len = get_imix_entries(&user_buffer[i], pkt_dev);
+             if (len < 0)
+                     return len;
+
+             i += len;
+             return count;
+     }
+
      if (!strcmp(name, "debug")) {
              len = num_arg(&user_buffer[i], 10, &value);
              if (len < 0)

@@ -1082,10 +1171,16 @@ static ssize_t pktgen_if_write(struct file *file,
              len = num_arg(&user_buffer[i], 10, &value);
              if (len < 0)
                      return len;
+             /* clone_skb is not supported for netif_receive xmit_mode and
+              * IMIX mode.
+              */
              if ((value > 0) &&
                  ((pkt_dev->xmit_mode == M_NETIF_RECEIVE) ||
                   !(pkt_dev->odev->priv_flags & IFF_TX_SKB_SHARING)))
                      return -ENOTSUPP;
+             if (value > 0 && pkt_dev->n_imix_entries > 0)
+                     return -ENOTSUPP;

ditto

quoted

              i += len;
              pkt_dev->clone_skb = value;

-- 





Nick Richardson (he/him/his)

SWE Intern

1 (919) 410 3510

careers.google.com/students


|Learn more about our candidate privacy policy.|