All of lore.kernel.org
 help / color / mirror / Atom feed
* [net-next PATCH v2 1/2] net: pktgen: support injecting packets for qdisc testing
@ 2016-06-29 20:03 John Fastabend
  2016-06-29 20:03 ` [net-next PATCH v2 2/2] net: samples: pktgen mode samples/tests for qdisc layer John Fastabend
  2016-06-30  8:37 ` [net-next PATCH v2 1/2] net: pktgen: support injecting packets for qdisc testing Jesper Dangaard Brouer
  0 siblings, 2 replies; 8+ messages in thread
From: John Fastabend @ 2016-06-29 20:03 UTC (permalink / raw)
  To: jhs, brouer; +Cc: netdev

Add another xmit_mode to pktgen to allow testing xmit functionality
of qdiscs. The new mode "queue_xmit" injects packets at
__dev_queue_xmit() so that qdisc is called.

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---
 net/core/pktgen.c |   42 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index f74ab9c..4b3d467 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -213,6 +213,7 @@
 /* Xmit modes */
 #define M_START_XMIT		0	/* Default normal TX */
 #define M_NETIF_RECEIVE 	1	/* Inject packets into stack */
+#define M_QUEUE_XMIT		2	/* Inject packet into qdisc */
 
 /* If lock -- protects updating of if_list */
 #define   if_lock(t)           spin_lock(&(t->if_lock));
@@ -626,6 +627,8 @@ static int pktgen_if_show(struct seq_file *seq, void *v)
 
 	if (pkt_dev->xmit_mode == M_NETIF_RECEIVE)
 		seq_puts(seq, "     xmit_mode: netif_receive\n");
+	else if (pkt_dev->xmit_mode == M_QUEUE_XMIT)
+		seq_puts(seq, "     xmit_mode: xmit_queue\n");
 
 	seq_puts(seq, "     Flags: ");
 
@@ -1142,8 +1145,10 @@ static ssize_t pktgen_if_write(struct file *file,
 			return len;
 
 		i += len;
-		if ((value > 1) && (pkt_dev->xmit_mode == M_START_XMIT) &&
-		    (!(pkt_dev->odev->priv_flags & IFF_TX_SKB_SHARING)))
+		if ((value > 1) &&
+		    ((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;
 		pkt_dev->burst = value < 1 ? 1 : value;
 		sprintf(pg_result, "OK: burst=%d", pkt_dev->burst);
@@ -1198,6 +1203,9 @@ static ssize_t pktgen_if_write(struct file *file,
 			 * at module loading time
 			 */
 			pkt_dev->clone_skb = 0;
+		} else if (strcmp(f, "queue_xmit") == 0) {
+			pkt_dev->xmit_mode = M_QUEUE_XMIT;
+			pkt_dev->last_ok = 1;
 		} else {
 			sprintf(pg_result,
 				"xmit_mode -:%s:- unknown\nAvailable modes: %s",
@@ -3434,6 +3442,36 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev)
 #endif
 		} while (--burst > 0);
 		goto out; /* Skips xmit_mode M_START_XMIT */
+	} else if (pkt_dev->xmit_mode == M_QUEUE_XMIT) {
+		local_bh_disable();
+		atomic_add(burst, &pkt_dev->skb->users);
+
+		ret = dev_queue_xmit(pkt_dev->skb);
+		switch (ret) {
+		case NET_XMIT_SUCCESS:
+			pkt_dev->sofar++;
+			pkt_dev->seq_num++;
+			pkt_dev->tx_bytes += pkt_dev->last_pkt_size;
+			break;
+		case NET_XMIT_DROP:
+		case NET_XMIT_CN:
+		/* These are all valid return codes for a qdisc but
+		 * indicate packets are being dropped or will likely
+		 * be dropped soon.
+		 */
+		case NETDEV_TX_BUSY:
+		/* qdisc may call dev_hard_start_xmit directly in cases
+		 * where no queues exist e.g. loopback device, virtual
+		 * devices, etc. In this case we need to handle
+		 * NETDEV_TX_ codes.
+		 */
+		default:
+			pkt_dev->errors++;
+			net_info_ratelimited("%s xmit error: %d\n",
+					     pkt_dev->odevname, ret);
+			break;
+		}
+		goto out;
 	}
 
 	txq = skb_get_tx_queue(odev, pkt_dev->skb);

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

* [net-next PATCH v2 2/2] net: samples: pktgen mode samples/tests for qdisc layer
  2016-06-29 20:03 [net-next PATCH v2 1/2] net: pktgen: support injecting packets for qdisc testing John Fastabend
@ 2016-06-29 20:03 ` John Fastabend
  2016-06-30  8:23   ` Jesper Dangaard Brouer
  2016-06-30  8:37 ` [net-next PATCH v2 1/2] net: pktgen: support injecting packets for qdisc testing Jesper Dangaard Brouer
  1 sibling, 1 reply; 8+ messages in thread
From: John Fastabend @ 2016-06-29 20:03 UTC (permalink / raw)
  To: jhs, brouer; +Cc: netdev

This adds samples for pktgen to use with new mode to inject pkts into
the qdisc layer. This also doubles as nice test cases to test any
patches against qdisc layer.

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---
 .../pktgen/pktgen_bench_xmit_mode_queue_xmit.sh    |   70 ++++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100755 samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh

diff --git a/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh b/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh
new file mode 100755
index 0000000..eee06cc
--- /dev/null
+++ b/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh
@@ -0,0 +1,70 @@
+#!/bin/bash
+#
+# Benchmark script:
+#  - developed for benchmarking egress qdisc path, derived from
+#    ingress benchmark script.
+#
+# Script for injecting packets into egress qdisc path of the stack
+# with pktgen "xmit_mode queue_xmit".
+#
+basedir=`dirname $0`
+source ${basedir}/functions.sh
+root_check_run_with_sudo "$@"
+
+# Parameter parsing via include
+source ${basedir}/parameters.sh
+# Using invalid DST_MAC will cause the packets to get dropped in
+# ip_rcv() which is part of the test
+[ -z "$DEST_IP" ] && DEST_IP="198.18.0.42"
+[ -z "$DST_MAC" ] && DST_MAC="90:e2:ba:ff:ff:ff"
+
+# Burst greater than 1 are invalid but allow users to specify it and
+# get an error instead of silently ignoring it.
+[ -z "$BURST" ] && BURST=1
+
+# Base Config
+DELAY="0"        # Zero means max speed
+COUNT="10000000" # Zero means indefinitely
+
+# General cleanup everything since last run
+pg_ctrl "reset"
+
+# Threads are specified with parameter -t value in $THREADS
+for ((thread = 0; thread < $THREADS; thread++)); do
+    # The device name is extended with @name, using thread number to
+    # make then unique, but any name will do.
+    dev=${DEV}@${thread}
+
+    # Add remove all other devices and add_device $dev to thread
+    pg_thread $thread "rem_device_all"
+    pg_thread $thread "add_device" $dev
+
+    # Base config of dev
+    pg_set $dev "flag QUEUE_MAP_CPU"
+    pg_set $dev "count $COUNT"
+    pg_set $dev "pkt_size $PKT_SIZE"
+    pg_set $dev "delay $DELAY"
+    pg_set $dev "flag NO_TIMESTAMP"
+
+    # Destination
+    pg_set $dev "dst_mac $DST_MAC"
+    pg_set $dev "dst $DEST_IP"
+
+    # Inject packet into RX path of stack
+    pg_set $dev "xmit_mode queue_xmit"
+
+    # Burst allow us to avoid measuring SKB alloc/free overhead
+    pg_set $dev "burst $BURST"
+done
+
+# start_run
+echo "Running... ctrl^C to stop" >&2
+pg_ctrl "start"
+echo "Done" >&2
+
+# Print results
+for ((thread = 0; thread < $THREADS; thread++)); do
+    dev=${DEV}@${thread}
+    echo "Device: $dev"
+    cat /proc/net/pktgen/$dev | grep -A2 "Result:"
+done

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

* Re: [net-next PATCH v2 2/2] net: samples: pktgen mode samples/tests for qdisc layer
  2016-06-29 20:03 ` [net-next PATCH v2 2/2] net: samples: pktgen mode samples/tests for qdisc layer John Fastabend
@ 2016-06-30  8:23   ` Jesper Dangaard Brouer
  2016-06-30 16:39     ` John Fastabend
  0 siblings, 1 reply; 8+ messages in thread
From: Jesper Dangaard Brouer @ 2016-06-30  8:23 UTC (permalink / raw)
  To: John Fastabend; +Cc: jhs, netdev, brouer

On Wed, 29 Jun 2016 13:03:26 -0700
John Fastabend <john.fastabend@gmail.com> wrote:

> This adds samples for pktgen to use with new mode to inject pkts into
> the qdisc layer. This also doubles as nice test cases to test any
> patches against qdisc layer.
> 
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> ---
>  .../pktgen/pktgen_bench_xmit_mode_queue_xmit.sh    |   70 ++++++++++++++++++++
>  1 file changed, 70 insertions(+)
>  create mode 100755 samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh
> 
> diff --git a/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh b/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh
> new file mode 100755
> index 0000000..eee06cc
> --- /dev/null
> +++ b/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh
> @@ -0,0 +1,70 @@
> +#!/bin/bash
> +#
> +# Benchmark script:
> +#  - developed for benchmarking egress qdisc path, derived from
> +#    ingress benchmark script.
> +#
> +# Script for injecting packets into egress qdisc path of the stack
> +# with pktgen "xmit_mode queue_xmit".
> +#
> +basedir=`dirname $0`
> +source ${basedir}/functions.sh
> +root_check_run_with_sudo "$@"
> +
> +# Parameter parsing via include
> +source ${basedir}/parameters.sh
> +# Using invalid DST_MAC will cause the packets to get dropped in
> +# ip_rcv() which is part of the test
> +[ -z "$DEST_IP" ] && DEST_IP="198.18.0.42"
> +[ -z "$DST_MAC" ] && DST_MAC="90:e2:ba:ff:ff:ff"
> +
> +# Burst greater than 1 are invalid but allow users to specify it and
> +# get an error instead of silently ignoring it.
> +[ -z "$BURST" ] && BURST=1

In other scripts I've rejected this at this step, instead of depending
on failure when sending the burst option to pktgen. Like:

https://github.com/netoptimizer/network-testing/blob/master/pktgen/pktgen_sample04_many_flows.sh#L31-L33

> +
> +# Base Config
> +DELAY="0"        # Zero means max speed
> +COUNT="10000000" # Zero means indefinitely
> +
> +# General cleanup everything since last run
> +pg_ctrl "reset"
> +
> +# Threads are specified with parameter -t value in $THREADS
> +for ((thread = 0; thread < $THREADS; thread++)); do
> +    # The device name is extended with @name, using thread number to
> +    # make then unique, but any name will do.
> +    dev=${DEV}@${thread}
> +
> +    # Add remove all other devices and add_device $dev to thread
> +    pg_thread $thread "rem_device_all"
> +    pg_thread $thread "add_device" $dev
> +
> +    # Base config of dev
> +    pg_set $dev "flag QUEUE_MAP_CPU"
> +    pg_set $dev "count $COUNT"
> +    pg_set $dev "pkt_size $PKT_SIZE"
> +    pg_set $dev "delay $DELAY"
> +    pg_set $dev "flag NO_TIMESTAMP"
> +
> +    # Destination
> +    pg_set $dev "dst_mac $DST_MAC"
> +    pg_set $dev "dst $DEST_IP"
> +
> +    # Inject packet into RX path of stack

Hmmm, maybe above comment need to be adjusted...

> +    pg_set $dev "xmit_mode queue_xmit"
> +
> +    # Burst allow us to avoid measuring SKB alloc/free overhead

This comment is confusing, maybe just remove. Didn't think burst is a
valid use-case.

> +    pg_set $dev "burst $BURST"
> +done
> +
> +# start_run
> +echo "Running... ctrl^C to stop" >&2
> +pg_ctrl "start"
> +echo "Done" >&2
> +
> +# Print results
> +for ((thread = 0; thread < $THREADS; thread++)); do
> +    dev=${DEV}@${thread}
> +    echo "Device: $dev"
> +    cat /proc/net/pktgen/$dev | grep -A2 "Result:"
> +done
> 



-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

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

* Re: [net-next PATCH v2 1/2] net: pktgen: support injecting packets for qdisc testing
  2016-06-29 20:03 [net-next PATCH v2 1/2] net: pktgen: support injecting packets for qdisc testing John Fastabend
  2016-06-29 20:03 ` [net-next PATCH v2 2/2] net: samples: pktgen mode samples/tests for qdisc layer John Fastabend
@ 2016-06-30  8:37 ` Jesper Dangaard Brouer
  2016-06-30 16:42   ` John Fastabend
  1 sibling, 1 reply; 8+ messages in thread
From: Jesper Dangaard Brouer @ 2016-06-30  8:37 UTC (permalink / raw)
  To: John Fastabend; +Cc: jhs, netdev, brouer

On Wed, 29 Jun 2016 13:03:06 -0700
John Fastabend <john.fastabend@gmail.com> wrote:

> Add another xmit_mode to pktgen to allow testing xmit functionality
> of qdiscs. The new mode "queue_xmit" injects packets at
> __dev_queue_xmit() so that qdisc is called.
> 
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>

I generally like this.

>  net/core/pktgen.c |   42 ++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> index f74ab9c..4b3d467 100644
> --- a/net/core/pktgen.c
> +++ b/net/core/pktgen.c
> @@ -213,6 +213,7 @@
>  /* Xmit modes */
>  #define M_START_XMIT		0	/* Default normal TX */
>  #define M_NETIF_RECEIVE 	1	/* Inject packets into stack */
> +#define M_QUEUE_XMIT		2	/* Inject packet into qdisc */
>  
>  /* If lock -- protects updating of if_list */
>  #define   if_lock(t)           spin_lock(&(t->if_lock));
> @@ -626,6 +627,8 @@ static int pktgen_if_show(struct seq_file *seq, void *v)
>  
>  	if (pkt_dev->xmit_mode == M_NETIF_RECEIVE)
>  		seq_puts(seq, "     xmit_mode: netif_receive\n");
> +	else if (pkt_dev->xmit_mode == M_QUEUE_XMIT)
> +		seq_puts(seq, "     xmit_mode: xmit_queue\n");
>  
>  	seq_puts(seq, "     Flags: ");
>  
> @@ -1142,8 +1145,10 @@ static ssize_t pktgen_if_write(struct file *file,
>  			return len;
>  
>  		i += len;
> -		if ((value > 1) && (pkt_dev->xmit_mode == M_START_XMIT) &&
> -		    (!(pkt_dev->odev->priv_flags & IFF_TX_SKB_SHARING)))
> +		if ((value > 1) &&
> +		    ((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;
>  		pkt_dev->burst = value < 1 ? 1 : value;
>  		sprintf(pg_result, "OK: burst=%d", pkt_dev->burst);
> @@ -1198,6 +1203,9 @@ static ssize_t pktgen_if_write(struct file *file,
>  			 * at module loading time
>  			 */
>  			pkt_dev->clone_skb = 0;
> +		} else if (strcmp(f, "queue_xmit") == 0) {
> +			pkt_dev->xmit_mode = M_QUEUE_XMIT;
> +			pkt_dev->last_ok = 1;
>  		} else {
>  			sprintf(pg_result,
>  				"xmit_mode -:%s:- unknown\nAvailable modes: %s",
> @@ -3434,6 +3442,36 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev)
>  #endif
>  		} while (--burst > 0);
>  		goto out; /* Skips xmit_mode M_START_XMIT */
> +	} else if (pkt_dev->xmit_mode == M_QUEUE_XMIT) {
> +		local_bh_disable();
> +		atomic_add(burst, &pkt_dev->skb->users);

Reading the code, people might think that "burst" is allowed for this
mode, which it is not. (You do handle this earlier in this patch when
configuring this mode).

> +		ret = dev_queue_xmit(pkt_dev->skb);
> +		switch (ret) {
> +		case NET_XMIT_SUCCESS:
> +			pkt_dev->sofar++;
> +			pkt_dev->seq_num++;
> +			pkt_dev->tx_bytes += pkt_dev->last_pkt_size;
> +			break;
> +		case NET_XMIT_DROP:
> +		case NET_XMIT_CN:
> +		/* These are all valid return codes for a qdisc but
> +		 * indicate packets are being dropped or will likely
> +		 * be dropped soon.
> +		 */
> +		case NETDEV_TX_BUSY:
> +		/* qdisc may call dev_hard_start_xmit directly in cases
> +		 * where no queues exist e.g. loopback device, virtual
> +		 * devices, etc. In this case we need to handle
> +		 * NETDEV_TX_ codes.
> +		 */
> +		default:
> +			pkt_dev->errors++;
> +			net_info_ratelimited("%s xmit error: %d\n",
> +					     pkt_dev->odevname, ret);
> +			break;
> +		}
> +		goto out;
>  	}
>  
>  	txq = skb_get_tx_queue(odev, pkt_dev->skb);
> 



-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

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

* Re: [net-next PATCH v2 2/2] net: samples: pktgen mode samples/tests for qdisc layer
  2016-06-30  8:23   ` Jesper Dangaard Brouer
@ 2016-06-30 16:39     ` John Fastabend
  0 siblings, 0 replies; 8+ messages in thread
From: John Fastabend @ 2016-06-30 16:39 UTC (permalink / raw)
  To: Jesper Dangaard Brouer; +Cc: jhs, netdev

On 16-06-30 01:23 AM, Jesper Dangaard Brouer wrote:
> On Wed, 29 Jun 2016 13:03:26 -0700
> John Fastabend <john.fastabend@gmail.com> wrote:
> 
>> This adds samples for pktgen to use with new mode to inject pkts into
>> the qdisc layer. This also doubles as nice test cases to test any
>> patches against qdisc layer.

[...]

>> +#
>> +# Benchmark script:
>> +#  - developed for benchmarking egress qdisc path, derived from
>> +#    ingress benchmark script.
>> +#

As you probably gathered 'derived' is giving me too much credit here
its more like cut'n'pasted from ingress benchmark scrip :)

>> +# Script for injecting packets into egress qdisc path of the stack
>> +# with pktgen "xmit_mode queue_xmit".
>> +#
>> +basedir=`dirname $0`
>> +source ${basedir}/functions.sh
>> +root_check_run_with_sudo "$@"
>> +
>> +# Parameter parsing via include
>> +source ${basedir}/parameters.sh
>> +# Using invalid DST_MAC will cause the packets to get dropped in
>> +# ip_rcv() which is part of the test
>> +[ -z "$DEST_IP" ] && DEST_IP="198.18.0.42"
>> +[ -z "$DST_MAC" ] && DST_MAC="90:e2:ba:ff:ff:ff"
>> +
>> +# Burst greater than 1 are invalid but allow users to specify it and
>> +# get an error instead of silently ignoring it.
>> +[ -z "$BURST" ] && BURST=1
> 
> In other scripts I've rejected this at this step, instead of depending
> on failure when sending the burst option to pktgen. Like:
> 
> https://github.com/netoptimizer/network-testing/blob/master/pktgen/pktgen_sample04_many_flows.sh#L31-L33
> 

Agreed that is nicer. I had originally left it to make sure I was
catching the burst > 1 case in pktgen but will remove.

>> +
>> +# Base Config
>> +DELAY="0"        # Zero means max speed
>> +COUNT="10000000" # Zero means indefinitely
>> +
>> +# General cleanup everything since last run
>> +pg_ctrl "reset"
>> +
>> +# Threads are specified with parameter -t value in $THREADS
>> +for ((thread = 0; thread < $THREADS; thread++)); do
>> +    # The device name is extended with @name, using thread number to
>> +    # make then unique, but any name will do.
>> +    dev=${DEV}@${thread}
>> +
>> +    # Add remove all other devices and add_device $dev to thread
>> +    pg_thread $thread "rem_device_all"
>> +    pg_thread $thread "add_device" $dev
>> +
>> +    # Base config of dev
>> +    pg_set $dev "flag QUEUE_MAP_CPU"
>> +    pg_set $dev "count $COUNT"
>> +    pg_set $dev "pkt_size $PKT_SIZE"
>> +    pg_set $dev "delay $DELAY"
>> +    pg_set $dev "flag NO_TIMESTAMP"
>> +
>> +    # Destination
>> +    pg_set $dev "dst_mac $DST_MAC"
>> +    pg_set $dev "dst $DEST_IP"
>> +
>> +    # Inject packet into RX path of stack
> 
> Hmmm, maybe above comment need to be adjusted...

Yep.

> 
>> +    pg_set $dev "xmit_mode queue_xmit"
>> +
>> +    # Burst allow us to avoid measuring SKB alloc/free overhead
> 
> This comment is confusing, maybe just remove. Didn't think burst is a
> valid use-case.

Yep.

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

* Re: [net-next PATCH v2 1/2] net: pktgen: support injecting packets for qdisc testing
  2016-06-30  8:37 ` [net-next PATCH v2 1/2] net: pktgen: support injecting packets for qdisc testing Jesper Dangaard Brouer
@ 2016-06-30 16:42   ` John Fastabend
  0 siblings, 0 replies; 8+ messages in thread
From: John Fastabend @ 2016-06-30 16:42 UTC (permalink / raw)
  To: Jesper Dangaard Brouer; +Cc: jhs, netdev

On 16-06-30 01:37 AM, Jesper Dangaard Brouer wrote:
> On Wed, 29 Jun 2016 13:03:06 -0700
> John Fastabend <john.fastabend@gmail.com> wrote:
> 
>> Add another xmit_mode to pktgen to allow testing xmit functionality
>> of qdiscs. The new mode "queue_xmit" injects packets at
>> __dev_queue_xmit() so that qdisc is called.
>>
>> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> 
> I generally like this.
> 

[...]

>> @@ -3434,6 +3442,36 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev)
>>  #endif
>>  		} while (--burst > 0);
>>  		goto out; /* Skips xmit_mode M_START_XMIT */
>> +	} else if (pkt_dev->xmit_mode == M_QUEUE_XMIT) {
>> +		local_bh_disable();
>> +		atomic_add(burst, &pkt_dev->skb->users);
> 
> Reading the code, people might think that "burst" is allowed for this
> mode, which it is not. (You do handle this earlier in this patch when
> configuring this mode).

Right we never get here without burst == 1 but sure it does read
a bit strange I'll use atomic_inc().


Thanks,
John

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

* Re: [net-next PATCH v2 1/2] net: pktgen: support injecting packets for qdisc testing
  2016-07-02 21:12 John Fastabend
@ 2016-07-04 23:07 ` David Miller
  0 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2016-07-04 23:07 UTC (permalink / raw)
  To: john.fastabend; +Cc: jhs, brouer, netdev

From: John Fastabend <john.fastabend@gmail.com>
Date: Sat, 02 Jul 2016 14:12:54 -0700

> Add another xmit_mode to pktgen to allow testing xmit functionality
> of qdiscs. The new mode "queue_xmit" injects packets at
> __dev_queue_xmit() so that qdisc is called.
> 
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>

Applied.

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

* [net-next PATCH v2 1/2] net: pktgen: support injecting packets for qdisc testing
@ 2016-07-02 21:12 John Fastabend
  2016-07-04 23:07 ` David Miller
  0 siblings, 1 reply; 8+ messages in thread
From: John Fastabend @ 2016-07-02 21:12 UTC (permalink / raw)
  To: jhs, brouer; +Cc: netdev

Add another xmit_mode to pktgen to allow testing xmit functionality
of qdiscs. The new mode "queue_xmit" injects packets at
__dev_queue_xmit() so that qdisc is called.

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---
 net/core/pktgen.c |   42 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index f74ab9c..bbd118b 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -213,6 +213,7 @@
 /* Xmit modes */
 #define M_START_XMIT		0	/* Default normal TX */
 #define M_NETIF_RECEIVE 	1	/* Inject packets into stack */
+#define M_QUEUE_XMIT		2	/* Inject packet into qdisc */
 
 /* If lock -- protects updating of if_list */
 #define   if_lock(t)           spin_lock(&(t->if_lock));
@@ -626,6 +627,8 @@ static int pktgen_if_show(struct seq_file *seq, void *v)
 
 	if (pkt_dev->xmit_mode == M_NETIF_RECEIVE)
 		seq_puts(seq, "     xmit_mode: netif_receive\n");
+	else if (pkt_dev->xmit_mode == M_QUEUE_XMIT)
+		seq_puts(seq, "     xmit_mode: xmit_queue\n");
 
 	seq_puts(seq, "     Flags: ");
 
@@ -1142,8 +1145,10 @@ static ssize_t pktgen_if_write(struct file *file,
 			return len;
 
 		i += len;
-		if ((value > 1) && (pkt_dev->xmit_mode == M_START_XMIT) &&
-		    (!(pkt_dev->odev->priv_flags & IFF_TX_SKB_SHARING)))
+		if ((value > 1) &&
+		    ((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;
 		pkt_dev->burst = value < 1 ? 1 : value;
 		sprintf(pg_result, "OK: burst=%d", pkt_dev->burst);
@@ -1198,6 +1203,9 @@ static ssize_t pktgen_if_write(struct file *file,
 			 * at module loading time
 			 */
 			pkt_dev->clone_skb = 0;
+		} else if (strcmp(f, "queue_xmit") == 0) {
+			pkt_dev->xmit_mode = M_QUEUE_XMIT;
+			pkt_dev->last_ok = 1;
 		} else {
 			sprintf(pg_result,
 				"xmit_mode -:%s:- unknown\nAvailable modes: %s",
@@ -3434,6 +3442,36 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev)
 #endif
 		} while (--burst > 0);
 		goto out; /* Skips xmit_mode M_START_XMIT */
+	} else if (pkt_dev->xmit_mode == M_QUEUE_XMIT) {
+		local_bh_disable();
+		atomic_inc(&pkt_dev->skb->users);
+
+		ret = dev_queue_xmit(pkt_dev->skb);
+		switch (ret) {
+		case NET_XMIT_SUCCESS:
+			pkt_dev->sofar++;
+			pkt_dev->seq_num++;
+			pkt_dev->tx_bytes += pkt_dev->last_pkt_size;
+			break;
+		case NET_XMIT_DROP:
+		case NET_XMIT_CN:
+		/* These are all valid return codes for a qdisc but
+		 * indicate packets are being dropped or will likely
+		 * be dropped soon.
+		 */
+		case NETDEV_TX_BUSY:
+		/* qdisc may call dev_hard_start_xmit directly in cases
+		 * where no queues exist e.g. loopback device, virtual
+		 * devices, etc. In this case we need to handle
+		 * NETDEV_TX_ codes.
+		 */
+		default:
+			pkt_dev->errors++;
+			net_info_ratelimited("%s xmit error: %d\n",
+					     pkt_dev->odevname, ret);
+			break;
+		}
+		goto out;
 	}
 
 	txq = skb_get_tx_queue(odev, pkt_dev->skb);

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

end of thread, other threads:[~2016-07-04 23:07 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-29 20:03 [net-next PATCH v2 1/2] net: pktgen: support injecting packets for qdisc testing John Fastabend
2016-06-29 20:03 ` [net-next PATCH v2 2/2] net: samples: pktgen mode samples/tests for qdisc layer John Fastabend
2016-06-30  8:23   ` Jesper Dangaard Brouer
2016-06-30 16:39     ` John Fastabend
2016-06-30  8:37 ` [net-next PATCH v2 1/2] net: pktgen: support injecting packets for qdisc testing Jesper Dangaard Brouer
2016-06-30 16:42   ` John Fastabend
2016-07-02 21:12 John Fastabend
2016-07-04 23:07 ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.