| Message ID | 20210122150517.7650-3-irusskikh@marvell.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | pktgen: scripts improvements | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 1 maintainers not CCed: mchehab+huawei@kernel.org |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: adding a line without newline at end of file WARNING: line length of 88 exceeds 80 columns WARNING: line length of 99 exceeds 80 columns |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Fri, 22 Jan 2021 16:05:17 +0100 Igor Russkikh <irusskikh@marvell.com> wrote: > To configure various complex flows we for sure can create custom > pktgen init scripts, but sometimes thats not that easy. > > New "-a" (append) option in all the existing sample scripts allows > to append more "devices" into pktgen threads. > > The most straightforward usecases for that are: > - using multiple devices. We have to generate full linerate on > all physical functions (ports) of our multiport device. > - pushing multiple flows (with different packet options) The use-case makes sense. More comment inlined below. > Signed-off-by: Igor Russkikh <irusskikh@marvell.com> > --- > samples/pktgen/README.rst | 18 ++++++++++++++++++ > samples/pktgen/functions.sh | 2 +- > samples/pktgen/parameters.sh | 7 ++++++- > samples/pktgen/pktgen_sample01_simple.sh | 10 ++++++++-- > samples/pktgen/pktgen_sample02_multiqueue.sh | 10 ++++++++-- > .../pktgen_sample03_burst_single_flow.sh | 10 ++++++++-- > samples/pktgen/pktgen_sample04_many_flows.sh | 10 ++++++++-- > .../pktgen/pktgen_sample05_flow_per_thread.sh | 10 ++++++++-- > ..._sample06_numa_awared_queue_irq_affinity.sh | 10 ++++++++-- > 9 files changed, 73 insertions(+), 14 deletions(-) > > diff --git a/samples/pktgen/README.rst b/samples/pktgen/README.rst > index f9c53ca5cf93..f7d8dd76b0c4 100644 > --- a/samples/pktgen/README.rst > +++ b/samples/pktgen/README.rst > @@ -28,10 +28,28 @@ across the sample scripts. Usage example is printed on errors:: > -b : ($BURST) HW level bursting of SKBs > -v : ($VERBOSE) verbose > -x : ($DEBUG) debug > + -6 : ($IP6) IPv6 > + -w : ($DELAY) Tx Delay value (us) > + -a : ($APPENDCFG) Script will not reset generator's state, but will append its config You called it $APPENDCFG, but code use $APPEND. > The global variable being set is also listed. E.g. the required > interface/device parameter "-i" sets variable $DEV. > > +"-a" parameter may be used to create different flows simultaneously. > +In this mode script will keep the existing config, will append its settings. > +In this mode you'll have to manually run traffic with "pg_ctrl start". > + > +For example you may use: > + > + source ./samples/pktgen/functions.sh > + pg_ctrl reset > + # add first device > + ./pktgen_sample06_numa_awared_queue_irq_affinity.sh -a -i ens1f0 -m 34:80:0d:a3:fc:c9 -t 8 > + # add second device > + ./pktgen_sample06_numa_awared_queue_irq_affinity.sh -a -i ens1f1 -m 34:80:0d:a3:fc:c9 -t 8 > + # run joint traffic on two devs > + pg_ctrl start > + > Common functions > ---------------- > The functions.sh file provides; Three different shell functions for > diff --git a/samples/pktgen/functions.sh b/samples/pktgen/functions.sh > index dae06d5b38fa..8db945ee4f55 100644 > --- a/samples/pktgen/functions.sh > +++ b/samples/pktgen/functions.sh > @@ -108,7 +108,7 @@ function pgset() { > fi > } > > -[[ $EUID -eq 0 ]] && trap 'pg_ctrl "reset"' EXIT > +[ -z "$APPEND" ] && [ "$EUID" -eq 0 ] && trap '[ -z "$APPEND" ] && pg_ctrl "reset"' EXIT This looks confusing and wrong (I think). (e.g. is the second '[ -z "$APPEND" ] && ...' needed). In functions.sh we don't need to "compress" the lines that much. I prefer readability in this file. (Cc Daniel T. Lee as he added this line). Maybe we can make it more human readable: if [[ -z "$APPEND" ]]; then if [[ $EUID -eq 0 ]]; then # Cleanup pktgen setup on exit trap 'pg_ctrl "reset"' EXIT fi fi I'm a little confused how the "trap" got added into 'functions.sh', as my original intend was that function.sh should only provide helper functions and not have a side-effect. (But I can see I acked the change). > ## -- General shell tricks -- > > diff --git a/samples/pktgen/parameters.sh b/samples/pktgen/parameters.sh > index 70cc2878d479..3fd4d5e8107a 100644 > --- a/samples/pktgen/parameters.sh > +++ b/samples/pktgen/parameters.sh > @@ -20,12 +20,13 @@ function usage() { > echo " -x : (\$DEBUG) debug" > echo " -6 : (\$IP6) IPv6" > echo " -w : (\$DELAY) Tx Delay value (us)" > + echo " -a : (\$APPENDCFG) Script will not reset generator's state, but will append its config" You called it $APPENDCFG, but code use $APPEND. > echo "" > } > > ## --- Parse command line arguments / parameters --- > ## echo "Commandline options:" > -while getopts "s:i:d:m:p:f:t:c:n:b:w:vxh6" option; do > +while getopts "s:i:d:m:p:f:t:c:n:b:w:vxh6a" option; do > case $option in > i) # interface > export DEV=$OPTARG > @@ -83,6 +84,10 @@ while getopts "s:i:d:m:p:f:t:c:n:b:w:vxh6" option; do > export IP6=6 > info "IP6: IP6=$IP6" > ;; > + a) > + export APPEND=yes See variable name $APPEND is used here, but help says $APPENDCFG > + info "Append mode: APPEND=$APPEND" > + ;; > h|?|*) > usage; > err 2 "[ERROR] Unknown parameters!!!" > diff --git a/samples/pktgen/pktgen_sample01_simple.sh b/samples/pktgen/pktgen_sample01_simple.sh > index c2ad1fa32d3f..8ca7913eaf8a 100755 > --- a/samples/pktgen/pktgen_sample01_simple.sh > +++ b/samples/pktgen/pktgen_sample01_simple.sh > @@ -37,11 +37,11 @@ UDP_SRC_MAX=109 > > # General cleanup everything since last run > # (especially important if other threads were configured by other scripts) > -pg_ctrl "reset" > +[ -z "$APPEND" ] && pg_ctrl "reset" Makes sense. > # Add remove all other devices and add_device $DEV to thread 0 > thread=0 > -pg_thread $thread "rem_device_all" > +[ -z "$APPEND" ] && pg_thread $thread "rem_device_all" > pg_thread $thread "add_device" $DEV > > # How many packets to send (zero means indefinitely) > @@ -77,6 +77,8 @@ pg_set $DEV "flag UDPSRC_RND" > pg_set $DEV "udp_src_min $UDP_SRC_MIN" > pg_set $DEV "udp_src_max $UDP_SRC_MAX" > > +if [ -z "$APPEND" ]; then > + > # start_run > echo "Running... ctrl^C to stop" >&2 > pg_ctrl "start" > @@ -85,3 +87,7 @@ echo "Done" >&2 > # Print results > echo "Result device: $DEV" > cat /proc/net/pktgen/$DEV > + > +else > +echo "Append mode: config done. Do more or use 'pg_ctrl start' to run" > +fi Hmm, could we indent lines for readability? (Same in other files)
Hi Jesper, Thanks for reviewing this. >> DELAY may now be explicitly specified via common parameter -w > > What are you actually using this for? Basically, for the second patch. When running multidev pktgen (using that -a option) with large amount of clones and bursts (-c and -b) I saw that some of the devices got stuck - i.e. traffic was not distributed evenly I think the reason of that is `next_to_run` selection logic, which always takes first pkt_dev in a list. Adding even a small delay param makes it consider next_tx data and creates a uniform distribution between devices. May be it makes sense to reconsider `next_to_run` selection logic, but I was concentrating on scripts, so thats it. > Notice there is also an option called "ratep" which can be used for > setting the packet rate per sec. In the pktgen.c code, it will use the > "delay" variable. Yes, I think in current form it makes no much harm if user knows what he wants. >> + -w : ($DELAY) Tx Delay value (us) >> + -a : ($APPENDCFG) Script will not reset generator's state, but will > append its config > > You called it $APPENDCFG, but code use $APPEND. Thanks, will fix. >> >> -[[ $EUID -eq 0 ]] && trap 'pg_ctrl "reset"' EXIT >> +[ -z "$APPEND" ] && [ "$EUID" -eq 0 ] && trap '[ -z "$APPEND" ] && > pg_ctrl "reset"' EXIT > > This looks confusing and wrong (I think). > (e.g. is the second '[ -z "$APPEND" ] && ...' needed). > > In functions.sh we don't need to "compress" the lines that much. I > prefer readability in this file. (Cc Daniel T. Lee as he added this > line). Maybe we can make it more human readable: Agree on style, could be fixed. > if [[ -z "$APPEND" ]]; then > if [[ $EUID -eq 0 ]]; then > # Cleanup pktgen setup on exit > trap 'pg_ctrl "reset"' EXIT > fi > fi > > I'm a little confused how the "trap" got added into 'functions.sh', as > my original intend was that function.sh should only provide helper > functions and not have a side-effect. (But I can see I acked the > change). I also don't like much the fact trap is being placed in that file. Here I've placed one extra "-z $APPEND" exactly because of that. In append mode of usage we do `source functions.sh` directly from bash. That causes a side effect that trap is installed in root shell. I can't check if thats APPEND mode or not at this moment. Thats why I do check APPEND inside of the trap. An alternative would be moving trap (or a function installing the trap) into each of the scripts. That was the old behavior BTW. >> +if [ -z "$APPEND" ]; then >> + >> # start_run >> echo "Running... ctrl^C to stop" >&2 >> pg_ctrl "start" >> @@ -85,3 +87,7 @@ echo "Done" >&2 >> # Print results >> echo "Result device: $DEV" >> cat /proc/net/pktgen/$DEV >> + >> +else >> +echo "Append mode: config done. Do more or use 'pg_ctrl start' to run" >> +fi > > Hmm, could we indent lines for readability? > (Same in other files) Agreed, will fix. Thanks, Igor
diff --git a/samples/pktgen/README.rst b/samples/pktgen/README.rst index f9c53ca5cf93..f7d8dd76b0c4 100644 --- a/samples/pktgen/README.rst +++ b/samples/pktgen/README.rst @@ -28,10 +28,28 @@ across the sample scripts. Usage example is printed on errors:: -b : ($BURST) HW level bursting of SKBs -v : ($VERBOSE) verbose -x : ($DEBUG) debug + -6 : ($IP6) IPv6 + -w : ($DELAY) Tx Delay value (us) + -a : ($APPENDCFG) Script will not reset generator's state, but will append its config The global variable being set is also listed. E.g. the required interface/device parameter "-i" sets variable $DEV. +"-a" parameter may be used to create different flows simultaneously. +In this mode script will keep the existing config, will append its settings. +In this mode you'll have to manually run traffic with "pg_ctrl start". + +For example you may use: + + source ./samples/pktgen/functions.sh + pg_ctrl reset + # add first device + ./pktgen_sample06_numa_awared_queue_irq_affinity.sh -a -i ens1f0 -m 34:80:0d:a3:fc:c9 -t 8 + # add second device + ./pktgen_sample06_numa_awared_queue_irq_affinity.sh -a -i ens1f1 -m 34:80:0d:a3:fc:c9 -t 8 + # run joint traffic on two devs + pg_ctrl start + Common functions ---------------- The functions.sh file provides; Three different shell functions for diff --git a/samples/pktgen/functions.sh b/samples/pktgen/functions.sh index dae06d5b38fa..8db945ee4f55 100644 --- a/samples/pktgen/functions.sh +++ b/samples/pktgen/functions.sh @@ -108,7 +108,7 @@ function pgset() { fi } -[[ $EUID -eq 0 ]] && trap 'pg_ctrl "reset"' EXIT +[ -z "$APPEND" ] && [ "$EUID" -eq 0 ] && trap '[ -z "$APPEND" ] && pg_ctrl "reset"' EXIT ## -- General shell tricks -- diff --git a/samples/pktgen/parameters.sh b/samples/pktgen/parameters.sh index 70cc2878d479..3fd4d5e8107a 100644 --- a/samples/pktgen/parameters.sh +++ b/samples/pktgen/parameters.sh @@ -20,12 +20,13 @@ function usage() { echo " -x : (\$DEBUG) debug" echo " -6 : (\$IP6) IPv6" echo " -w : (\$DELAY) Tx Delay value (us)" + echo " -a : (\$APPENDCFG) Script will not reset generator's state, but will append its config" echo "" } ## --- Parse command line arguments / parameters --- ## echo "Commandline options:" -while getopts "s:i:d:m:p:f:t:c:n:b:w:vxh6" option; do +while getopts "s:i:d:m:p:f:t:c:n:b:w:vxh6a" option; do case $option in i) # interface export DEV=$OPTARG @@ -83,6 +84,10 @@ while getopts "s:i:d:m:p:f:t:c:n:b:w:vxh6" option; do export IP6=6 info "IP6: IP6=$IP6" ;; + a) + export APPEND=yes + info "Append mode: APPEND=$APPEND" + ;; h|?|*) usage; err 2 "[ERROR] Unknown parameters!!!" diff --git a/samples/pktgen/pktgen_sample01_simple.sh b/samples/pktgen/pktgen_sample01_simple.sh index c2ad1fa32d3f..8ca7913eaf8a 100755 --- a/samples/pktgen/pktgen_sample01_simple.sh +++ b/samples/pktgen/pktgen_sample01_simple.sh @@ -37,11 +37,11 @@ UDP_SRC_MAX=109 # General cleanup everything since last run # (especially important if other threads were configured by other scripts) -pg_ctrl "reset" +[ -z "$APPEND" ] && pg_ctrl "reset" # Add remove all other devices and add_device $DEV to thread 0 thread=0 -pg_thread $thread "rem_device_all" +[ -z "$APPEND" ] && pg_thread $thread "rem_device_all" pg_thread $thread "add_device" $DEV # How many packets to send (zero means indefinitely) @@ -77,6 +77,8 @@ pg_set $DEV "flag UDPSRC_RND" pg_set $DEV "udp_src_min $UDP_SRC_MIN" pg_set $DEV "udp_src_max $UDP_SRC_MAX" +if [ -z "$APPEND" ]; then + # start_run echo "Running... ctrl^C to stop" >&2 pg_ctrl "start" @@ -85,3 +87,7 @@ echo "Done" >&2 # Print results echo "Result device: $DEV" cat /proc/net/pktgen/$DEV + +else +echo "Append mode: config done. Do more or use 'pg_ctrl start' to run" +fi \ No newline at end of file diff --git a/samples/pktgen/pktgen_sample02_multiqueue.sh b/samples/pktgen/pktgen_sample02_multiqueue.sh index 49e1e81a2945..ecf35d4506bf 100755 --- a/samples/pktgen/pktgen_sample02_multiqueue.sh +++ b/samples/pktgen/pktgen_sample02_multiqueue.sh @@ -38,7 +38,7 @@ if [ -n "$DST_PORT" ]; then fi # General cleanup everything since last run -pg_ctrl "reset" +[ -z "$APPEND" ] && pg_ctrl "reset" # Threads are specified with parameter -t value in $THREADS for ((thread = $F_THREAD; thread <= $L_THREAD; thread++)); do @@ -47,7 +47,7 @@ for ((thread = $F_THREAD; thread <= $L_THREAD; thread++)); do dev=${DEV}@${thread} # Add remove all other devices and add_device $dev to thread - pg_thread $thread "rem_device_all" + [ -z "$APPEND" ] && pg_thread $thread "rem_device_all" pg_thread $thread "add_device" $dev # Notice config queue to map to cpu (mirrors smp_processor_id()) @@ -81,6 +81,8 @@ for ((thread = $F_THREAD; thread <= $L_THREAD; thread++)); do pg_set $dev "udp_src_max $UDP_SRC_MAX" done +if [ -z "$APPEND" ]; then + # start_run echo "Running... ctrl^C to stop" >&2 pg_ctrl "start" @@ -92,3 +94,7 @@ for ((thread = $F_THREAD; thread <= $L_THREAD; thread++)); do echo "Device: $dev" cat /proc/net/pktgen/$dev | grep -A2 "Result:" done + +else +echo "Append mode: config done. Do more or use 'pg_ctrl start' to run" +fi diff --git a/samples/pktgen/pktgen_sample03_burst_single_flow.sh b/samples/pktgen/pktgen_sample03_burst_single_flow.sh index f9b67affb567..132ee87e9227 100755 --- a/samples/pktgen/pktgen_sample03_burst_single_flow.sh +++ b/samples/pktgen/pktgen_sample03_burst_single_flow.sh @@ -43,14 +43,14 @@ if [ -n "$DST_PORT" ]; then fi # General cleanup everything since last run -pg_ctrl "reset" +[ -z "$APPEND" ] && pg_ctrl "reset" # Threads are specified with parameter -t value in $THREADS for ((thread = $F_THREAD; thread <= $L_THREAD; thread++)); do dev=${DEV}@${thread} # Add remove all other devices and add_device $dev to thread - pg_thread $thread "rem_device_all" + [ -z "$APPEND" ] && pg_thread $thread "rem_device_all" pg_thread $thread "add_device" $dev # Base config @@ -94,5 +94,11 @@ function control_c() { # trap keyboard interrupt (Ctrl-C) trap control_c SIGINT +if [ -z "$APPEND" ]; then + echo "Running... ctrl^C to stop" >&2 pg_ctrl "start" + +else +echo "Append mode: config done. Do more or use 'pg_ctrl start' to run" +fi diff --git a/samples/pktgen/pktgen_sample04_many_flows.sh b/samples/pktgen/pktgen_sample04_many_flows.sh index ac2d037a6160..b9ea9823446c 100755 --- a/samples/pktgen/pktgen_sample04_many_flows.sh +++ b/samples/pktgen/pktgen_sample04_many_flows.sh @@ -42,14 +42,14 @@ fi read -r SRC_MIN SRC_MAX <<< $(parse_addr 198.18.0.0/15) # General cleanup everything since last run -pg_ctrl "reset" +[ -z "$APPEND" ] && pg_ctrl "reset" # Threads are specified with parameter -t value in $THREADS for ((thread = $F_THREAD; thread <= $L_THREAD; thread++)); do dev=${DEV}@${thread} # Add remove all other devices and add_device $dev to thread - pg_thread $thread "rem_device_all" + [ -z "$APPEND" ] && pg_thread $thread "rem_device_all" pg_thread $thread "add_device" $dev # Base config @@ -104,7 +104,13 @@ function print_result() { # trap keyboard interrupt (Ctrl-C) trap true SIGINT +if [ -z "$APPEND" ]; then + echo "Running... ctrl^C to stop" >&2 pg_ctrl "start" print_result + +else +echo "Append mode: config done. Do more or use 'pg_ctrl start' to run" +fi diff --git a/samples/pktgen/pktgen_sample05_flow_per_thread.sh b/samples/pktgen/pktgen_sample05_flow_per_thread.sh index 85256484c86f..a73f2587e11f 100755 --- a/samples/pktgen/pktgen_sample05_flow_per_thread.sh +++ b/samples/pktgen/pktgen_sample05_flow_per_thread.sh @@ -32,14 +32,14 @@ if [ -n "$DST_PORT" ]; then fi # General cleanup everything since last run -pg_ctrl "reset" +[ -z "$APPEND" ] && pg_ctrl "reset" # Threads are specified with parameter -t value in $THREADS for ((thread = $F_THREAD; thread <= $L_THREAD; thread++)); do dev=${DEV}@${thread} # Add remove all other devices and add_device $dev to thread - pg_thread $thread "rem_device_all" + [ -z "$APPEND" ] && pg_thread $thread "rem_device_all" pg_thread $thread "add_device" $dev # Base config @@ -88,7 +88,13 @@ function print_result() { # trap keyboard interrupt (Ctrl-C) trap true SIGINT +if [ -z "$APPEND" ]; then + echo "Running... ctrl^C to stop" >&2 pg_ctrl "start" print_result + +else +echo "Append mode: config done. Do more or use 'pg_ctrl start' to run" +fi diff --git a/samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh b/samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh index 7c73ab8fbe3c..70fb15e67a1c 100755 --- a/samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh +++ b/samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh @@ -44,7 +44,7 @@ if [ -n "$DST_PORT" ]; then fi # General cleanup everything since last run -pg_ctrl "reset" +[ -z "$APPEND" ] && pg_ctrl "reset" # Threads are specified with parameter -t value in $THREADS for ((i = 0; i < $THREADS; i++)); do @@ -58,7 +58,7 @@ for ((i = 0; i < $THREADS; i++)); do info "irq ${irq_array[$i]} is set affinity to `cat /proc/irq/${irq_array[$i]}/smp_affinity_list`" # Add remove all other devices and add_device $dev to thread - pg_thread $thread "rem_device_all" + [ -z "$APPEND" ] && pg_thread $thread "rem_device_all" pg_thread $thread "add_device" $dev # select queue and bind the queue and $dev in 1:1 relationship @@ -99,6 +99,8 @@ for ((i = 0; i < $THREADS; i++)); do done # start_run +if [ -z "$APPEND" ]; then + echo "Running... ctrl^C to stop" >&2 pg_ctrl "start" echo "Done" >&2 @@ -110,3 +112,7 @@ for ((i = 0; i < $THREADS; i++)); do echo "Device: $dev" cat /proc/net/pktgen/$dev | grep -A2 "Result:" done + +else +echo "Append mode: config done. Do more or use 'pg_ctrl start' to run" +fi
To configure various complex flows we for sure can create custom pktgen init scripts, but sometimes thats not that easy. New "-a" (append) option in all the existing sample scripts allows to append more "devices" into pktgen threads. The most straightforward usecases for that are: - using multiple devices. We have to generate full linerate on all physical functions (ports) of our multiport device. - pushing multiple flows (with different packet options) Signed-off-by: Igor Russkikh <irusskikh@marvell.com> --- samples/pktgen/README.rst | 18 ++++++++++++++++++ samples/pktgen/functions.sh | 2 +- samples/pktgen/parameters.sh | 7 ++++++- samples/pktgen/pktgen_sample01_simple.sh | 10 ++++++++-- samples/pktgen/pktgen_sample02_multiqueue.sh | 10 ++++++++-- .../pktgen_sample03_burst_single_flow.sh | 10 ++++++++-- samples/pktgen/pktgen_sample04_many_flows.sh | 10 ++++++++-- .../pktgen/pktgen_sample05_flow_per_thread.sh | 10 ++++++++-- ..._sample06_numa_awared_queue_irq_affinity.sh | 10 ++++++++-- 9 files changed, 73 insertions(+), 14 deletions(-)