Re: [PATCH net-next] pktgen: Fix fall-through annotation
From: "Gustavo A. R. Silva" Date: Thu, 13 Sep 2018 14:03:20 -0500 > Replace "fallthru" with a proper "fall through" annotation. > > This fix is part of the ongoing efforts to enabling > -Wimplicit-fallthrough > > Signed-off-by: Gustavo A. R. Silva Applied.
Re: [PATCH net-next] pktgen: convert safe uses of strncpy() to strcpy() to avoid string truncation warning
From: Jakub Kicinski Date: Tue, 17 Jul 2018 14:32:24 -0700 > GCC 8 complains: > > net/core/pktgen.c: In function ‘pktgen_if_write’: > net/core/pktgen.c:1419:4: warning: ‘strncpy’ output may be truncated copying > between 0 and 31 bytes from a string of length 127 [-Wstringop-truncation] > strncpy(pkt_dev->src_max, buf, len); > ^~~ > net/core/pktgen.c:1399:4: warning: ‘strncpy’ output may be truncated copying > between 0 and 31 bytes from a string of length 127 [-Wstringop-truncation] > strncpy(pkt_dev->src_min, buf, len); > ^~~ > net/core/pktgen.c:1290:4: warning: ‘strncpy’ output may be truncated copying > between 0 and 31 bytes from a string of length 127 [-Wstringop-truncation] > strncpy(pkt_dev->dst_max, buf, len); > ^~~ > net/core/pktgen.c:1268:4: warning: ‘strncpy’ output may be truncated copying > between 0 and 31 bytes from a string of length 127 [-Wstringop-truncation] > strncpy(pkt_dev->dst_min, buf, len); > ^~~ > > There is no bug here, but the code is not perfect either. It copies > sizeof(pkt_dev->/member/) - 1 from user space into buf, and then does > a strcmp(pkt_dev->/member/, buf) hence assuming buf will be null-terminated > and shorter than pkt_dev->/member/ (pkt_dev->/member/ is never > explicitly null-terminated, and strncpy() doesn't have to null-terminate > so the assumption must be on buf). The use of strncpy() without explicit > null-termination looks suspicious. Convert to use straight strcpy(). > > strncpy() would also null-pad the output, but that's clearly unnecessary > since the author calls memset(pkt_dev->/member/, 0, sizeof(..)); prior > to strncpy(), anyway. > > While at it format the code for "dst_min", "dst_max", "src_min" and > "src_max" in the same way by removing extra new lines in one case. > > Signed-off-by: Jakub Kicinski > Reviewed-by: Jiong Wang Applied, thanks Jakub.
[PATCH net-next] pktgen: convert safe uses of strncpy() to strcpy() to avoid string truncation warning
GCC 8 complains: net/core/pktgen.c: In function ‘pktgen_if_write’: net/core/pktgen.c:1419:4: warning: ‘strncpy’ output may be truncated copying between 0 and 31 bytes from a string of length 127 [-Wstringop-truncation] strncpy(pkt_dev->src_max, buf, len); ^~~ net/core/pktgen.c:1399:4: warning: ‘strncpy’ output may be truncated copying between 0 and 31 bytes from a string of length 127 [-Wstringop-truncation] strncpy(pkt_dev->src_min, buf, len); ^~~ net/core/pktgen.c:1290:4: warning: ‘strncpy’ output may be truncated copying between 0 and 31 bytes from a string of length 127 [-Wstringop-truncation] strncpy(pkt_dev->dst_max, buf, len); ^~~ net/core/pktgen.c:1268:4: warning: ‘strncpy’ output may be truncated copying between 0 and 31 bytes from a string of length 127 [-Wstringop-truncation] strncpy(pkt_dev->dst_min, buf, len); ^~~ There is no bug here, but the code is not perfect either. It copies sizeof(pkt_dev->/member/) - 1 from user space into buf, and then does a strcmp(pkt_dev->/member/, buf) hence assuming buf will be null-terminated and shorter than pkt_dev->/member/ (pkt_dev->/member/ is never explicitly null-terminated, and strncpy() doesn't have to null-terminate so the assumption must be on buf). The use of strncpy() without explicit null-termination looks suspicious. Convert to use straight strcpy(). strncpy() would also null-pad the output, but that's clearly unnecessary since the author calls memset(pkt_dev->/member/, 0, sizeof(..)); prior to strncpy(), anyway. While at it format the code for "dst_min", "dst_max", "src_min" and "src_max" in the same way by removing extra new lines in one case. Signed-off-by: Jakub Kicinski Reviewed-by: Jiong Wang --- net/core/pktgen.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/net/core/pktgen.c b/net/core/pktgen.c index 49368e21d228..308ed04984de 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -1265,7 +1265,7 @@ static ssize_t pktgen_if_write(struct file *file, buf[len] = 0; if (strcmp(buf, pkt_dev->dst_min) != 0) { memset(pkt_dev->dst_min, 0, sizeof(pkt_dev->dst_min)); - strncpy(pkt_dev->dst_min, buf, len); + strcpy(pkt_dev->dst_min, buf); pkt_dev->daddr_min = in_aton(pkt_dev->dst_min); pkt_dev->cur_daddr = pkt_dev->daddr_min; } @@ -1280,14 +1280,12 @@ static ssize_t pktgen_if_write(struct file *file, if (len < 0) return len; - if (copy_from_user(buf, _buffer[i], len)) return -EFAULT; - buf[len] = 0; if (strcmp(buf, pkt_dev->dst_max) != 0) { memset(pkt_dev->dst_max, 0, sizeof(pkt_dev->dst_max)); - strncpy(pkt_dev->dst_max, buf, len); + strcpy(pkt_dev->dst_max, buf); pkt_dev->daddr_max = in_aton(pkt_dev->dst_max); pkt_dev->cur_daddr = pkt_dev->daddr_max; } @@ -1396,7 +1394,7 @@ static ssize_t pktgen_if_write(struct file *file, buf[len] = 0; if (strcmp(buf, pkt_dev->src_min) != 0) { memset(pkt_dev->src_min, 0, sizeof(pkt_dev->src_min)); - strncpy(pkt_dev->src_min, buf, len); + strcpy(pkt_dev->src_min, buf); pkt_dev->saddr_min = in_aton(pkt_dev->src_min); pkt_dev->cur_saddr = pkt_dev->saddr_min; } @@ -1416,7 +1414,7 @@ static ssize_t pktgen_if_write(struct file *file, buf[len] = 0; if (strcmp(buf, pkt_dev->src_max) != 0) { memset(pkt_dev->src_max, 0, sizeof(pkt_dev->src_max)); - strncpy(pkt_dev->src_max, buf, len); + strcpy(pkt_dev->src_max, buf); pkt_dev->saddr_max = in_aton(pkt_dev->src_max); pkt_dev->cur_saddr = pkt_dev->saddr_max; } -- 2.17.1
Re: [PATCH] pktgen: Fix memory leak in pktgen_if_write
From: "Gustavo A. R. Silva" <gust...@embeddedor.com> Date: Wed, 14 Mar 2018 03:07:27 -0500 > _buf_ is an array and the one that must be freed is _tp_ instead. > > Fixes: a870a02cc963 ("pktgen: use dynamic allocation for debug print buffer") > Reported-by: Wang Jian <jianjian.wa...@gmail.com> > Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com> Applied, thanks.
Re: [PATCH] pktgen: Fix memory leak in pktgen_if_write
On Wed, Mar 14, 2018 at 9:07 AM, Gustavo A. R. Silva <gust...@embeddedor.com> wrote: > _buf_ is an array and the one that must be freed is _tp_ instead. > > Fixes: a870a02cc963 ("pktgen: use dynamic allocation for debug print buffer") > Reported-by: Wang Jian <jianjian.wa...@gmail.com> > Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com> Acked-by: Arnd Bergmann <a...@arndb.de> Thanks for fixing up my mistake so quickly, and thanks to Wang for the report I was about to send the same patch, but you got there first.
Re: [PATCH] pktgen: use dynamic allocation for debug print buffer
Arnd: Thanks for the fix. On 03/13/2018 10:02 PM, Wang Jian wrote: + kfree(buf); free tb? buf is an array. Wang: Thanks for the report. I already sent a patch to fix this: https://patchwork.kernel.org/patch/10281587/ -- Gustavo On Wed, Mar 14, 2018 at 8:25 AM, David Miller <da...@davemloft.net> wrote: From: Arnd Bergmann <a...@arndb.de> Date: Tue, 13 Mar 2018 21:58:39 +0100 After the removal of the VLA, we get a harmless warning about a large stack frame: net/core/pktgen.c: In function 'pktgen_if_write': net/core/pktgen.c:1710:1: error: the frame size of 1076 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] The function was previously shown to be safe despite hitting the 1024 bye warning level. To get rid of the annoyging warning, while keeping it readable, this changes it to use strndup_user(). Obviously this is not a fast path, so the kmalloc() overhead can be disregarded. Fixes: 35951393bbff ("pktgen: Remove VLA usage") Signed-off-by: Arnd Bergmann <a...@arndb.de> Applied, thanks.
[PATCH] pktgen: Fix memory leak in pktgen_if_write
_buf_ is an array and the one that must be freed is _tp_ instead. Fixes: a870a02cc963 ("pktgen: use dynamic allocation for debug print buffer") Reported-by: Wang Jian <jianjian.wa...@gmail.com> Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com> --- net/core/pktgen.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/core/pktgen.c b/net/core/pktgen.c index fd65761..545cf08 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -913,7 +913,7 @@ static ssize_t pktgen_if_write(struct file *file, return PTR_ERR(tp); pr_debug("%s,%zu buffer -:%s:-\n", name, count, tp); - kfree(buf); + kfree(tp); } if (!strcmp(name, "min_pkt_size")) { -- 2.7.4
Re: [PATCH] pktgen: use dynamic allocation for debug print buffer
>> + kfree(buf); free tb? buf is an array. On Wed, Mar 14, 2018 at 8:25 AM, David Miller <da...@davemloft.net> wrote: > From: Arnd Bergmann <a...@arndb.de> > Date: Tue, 13 Mar 2018 21:58:39 +0100 > >> After the removal of the VLA, we get a harmless warning about a large >> stack frame: >> >> net/core/pktgen.c: In function 'pktgen_if_write': >> net/core/pktgen.c:1710:1: error: the frame size of 1076 bytes is larger than >> 1024 bytes [-Werror=frame-larger-than=] >> >> The function was previously shown to be safe despite hitting >> the 1024 bye warning level. To get rid of the annoyging warning, >> while keeping it readable, this changes it to use strndup_user(). >> >> Obviously this is not a fast path, so the kmalloc() overhead >> can be disregarded. >> >> Fixes: 35951393bbff ("pktgen: Remove VLA usage") >> Signed-off-by: Arnd Bergmann <a...@arndb.de> > > Applied, thanks.
Re: [PATCH] pktgen: use dynamic allocation for debug print buffer
From: Arnd Bergmann <a...@arndb.de> Date: Tue, 13 Mar 2018 21:58:39 +0100 > After the removal of the VLA, we get a harmless warning about a large > stack frame: > > net/core/pktgen.c: In function 'pktgen_if_write': > net/core/pktgen.c:1710:1: error: the frame size of 1076 bytes is larger than > 1024 bytes [-Werror=frame-larger-than=] > > The function was previously shown to be safe despite hitting > the 1024 bye warning level. To get rid of the annoyging warning, > while keeping it readable, this changes it to use strndup_user(). > > Obviously this is not a fast path, so the kmalloc() overhead > can be disregarded. > > Fixes: 35951393bbff ("pktgen: Remove VLA usage") > Signed-off-by: Arnd Bergmann <a...@arndb.de> Applied, thanks.
[PATCH] pktgen: use dynamic allocation for debug print buffer
After the removal of the VLA, we get a harmless warning about a large stack frame: net/core/pktgen.c: In function 'pktgen_if_write': net/core/pktgen.c:1710:1: error: the frame size of 1076 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] The function was previously shown to be safe despite hitting the 1024 bye warning level. To get rid of the annoyging warning, while keeping it readable, this changes it to use strndup_user(). Obviously this is not a fast path, so the kmalloc() overhead can be disregarded. Fixes: 35951393bbff ("pktgen: Remove VLA usage") Signed-off-by: Arnd Bergmann <a...@arndb.de> --- net/core/pktgen.c | 15 --- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/net/core/pktgen.c b/net/core/pktgen.c index de17a9f3e1f6..9216cf99b5a0 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -906,13 +906,14 @@ static ssize_t pktgen_if_write(struct file *file, i += len; if (debug) { - size_t copy = min_t(size_t, count, 1023); - char tb[1024]; - if (copy_from_user(tb, user_buffer, copy)) - return -EFAULT; - tb[copy] = 0; - pr_debug("%s,%lu buffer -:%s:-\n", -name, (unsigned long)count, tb); + size_t copy = min_t(size_t, count + 1, 1024); + char *tp = strndup_user(user_buffer, copy); + + if (IS_ERR(tp)) + return PTR_ERR(tp); + + pr_debug("%s,%zu buffer -:%s:-\n", name, count, tp); + kfree(buf); } if (!strcmp(name, "min_pkt_size")) { -- 2.9.0
Re: [PATCH] pktgen: Remove VLA usage
On 03/09/2018 10:58 AM, David Miller wrote: From: "Gustavo A. R. Silva"Date: Thu, 8 Mar 2018 23:43:40 -0600 In preparation to enabling -Wvla, remove VLA usage and replace it with a fixed-length array instead. Signed-off-by: Gustavo A. R. Silva --- David, I'm not sure how often this function is being called and, depending on the frequency it may be worth to use dynamic memory allocation instead? It happens every time a config setting is made via the sysfs files when debug is enabled. This is not something that happens often. I got it. So your patch is fine, applied to net-next, thanks. Awesome. Thanks, David. -- Gustavo
Re: [PATCH] pktgen: Remove VLA usage
From: "Gustavo A. R. Silva"Date: Thu, 8 Mar 2018 23:43:40 -0600 > In preparation to enabling -Wvla, remove VLA usage and replace it > with a fixed-length array instead. > > Signed-off-by: Gustavo A. R. Silva > --- > David, > > I'm not sure how often this function is being called and, > depending on the frequency it may be worth to use > dynamic memory allocation instead? It happens every time a config setting is made via the sysfs files when debug is enabled. This is not something that happens often. So your patch is fine, applied to net-next, thanks.
[PATCH] pktgen: Remove VLA usage
In preparation to enabling -Wvla, remove VLA usage and replace it with a fixed-length array instead. Signed-off-by: Gustavo A. R. Silva--- David, I'm not sure how often this function is being called and, depending on the frequency it may be worth to use dynamic memory allocation instead? Thanks -- Gustavo net/core/pktgen.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/core/pktgen.c b/net/core/pktgen.c index d81bddd..e2d6ae3 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -907,7 +907,7 @@ static ssize_t pktgen_if_write(struct file *file, if (debug) { size_t copy = min_t(size_t, count, 1023); - char tb[copy + 1]; + char tb[1024]; if (copy_from_user(tb, user_buffer, copy)) return -EFAULT; tb[copy] = 0; -- 2.7.4
Re: [PATCHv2 0/5] pktgen: Behavior flags fixes
From: Dmitry SafonovDate: Thu, 18 Jan 2018 18:31:32 + > v2: > o fixed a nitpick from David Miller > > There are a bunch of fixes/cleanups/Documentations. > Diffstat says for itself, regardless added docs and missed flag > parameters. Series applied to net-next, thank you.
[PATCHv2 2/5] pktgen: Add missing !flag parameters
o FLOW_SEQ now can be disabled with pgset "flag !FLOW_SEQ" o FLOW_SEQ and FLOW_RND are antonyms, as it's shown by pktgen_if_show() o IPSEC now may be disabled Note, that IPV6 is enabled with dst6/src6 parameters, not with a flag parameter. Signed-off-by: Dmitry Safonov--- net/core/pktgen.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/net/core/pktgen.c b/net/core/pktgen.c index f95a15086225..ab63943ffd03 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -1285,9 +1285,12 @@ static ssize_t pktgen_if_write(struct file *file, else if (strcmp(f, "!SVID_RND") == 0) pkt_dev->flags &= ~F_SVID_RND; - else if (strcmp(f, "FLOW_SEQ") == 0) + else if (strcmp(f, "FLOW_SEQ") == 0 || strcmp(f, "!FLOW_RND") == 0) pkt_dev->flags |= F_FLOW_SEQ; + else if (strcmp(f, "FLOW_RND") == 0 || strcmp(f, "!FLOW_SEQ") == 0) + pkt_dev->flags &= ~F_FLOW_SEQ; + else if (strcmp(f, "QUEUE_MAP_RND") == 0) pkt_dev->flags |= F_QUEUE_MAP_RND; @@ -1302,6 +1305,9 @@ static ssize_t pktgen_if_write(struct file *file, #ifdef CONFIG_XFRM else if (strcmp(f, "IPSEC") == 0) pkt_dev->flags |= F_IPSEC_ON; + + else if (strcmp(f, "!IPSEC") == 0) + pkt_dev->flags &= ~F_IPSEC_ON; #endif else if (strcmp(f, "!IPV6") == 0) -- 2.13.6
[PATCHv2 0/5] pktgen: Behavior flags fixes
v2: o fixed a nitpick from David Miller There are a bunch of fixes/cleanups/Documentations. Diffstat says for itself, regardless added docs and missed flag parameters. Cc: Arnd Bergmann <a...@arndb.de> Cc: "David S. Miller" <da...@davemloft.net> Cc: David Windsor <dwind...@gmail.com> Cc: Eric Dumazet <eduma...@google.com> Cc: Ingo Molnar <mi...@kernel.org> Cc: Johannes Berg <johannes.b...@intel.com> Cc: Mark Rutland <mark.rutl...@arm.com> Cc: Radu Rendec <rren...@arista.com> Cc: "Reshetova, Elena" <elena.reshet...@intel.com> Cc: netdev@vger.kernel.org Dmitry Safonov (5): Documentation/pktgen: Clearify how-to use pktgen samples pktgen: Add missing !flag parameters pktgen: Add behaviour flags macro to generate flags/names pktgen: Remove brute-force printing of flags pktgen: Clean read user supplied flag mess Documentation/networking/pktgen.txt | 19 ++- net/core/pktgen.c | 266 2 files changed, 103 insertions(+), 182 deletions(-) -- 2.13.6
[PATCHv2 1/5] Documentation/pktgen: Clearify how-to use pktgen samples
o Change process name in ps output: looks like, these days the process is named kpktgend_, rather than pktgen/. o Use pg_ctrl for start/stop as it can work well with pgset without changes to $(PGDEV) variable. o Clarify a bit needed $(PGDEV) definition for sample scripts and that one needs to `source functions.sh`. o Document how-to unset a behaviour flag, note about history expansion. o Fix pgset spi parameter value. Cc: Jonathan Corbet <cor...@lwn.net> Cc: linux-...@vger.kernel.org Signed-off-by: Dmitry Safonov <d...@arista.com> --- Documentation/networking/pktgen.txt | 19 ++- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/Documentation/networking/pktgen.txt b/Documentation/networking/pktgen.txt index 2c4e3354e128..d2fd78f85aa4 100644 --- a/Documentation/networking/pktgen.txt +++ b/Documentation/networking/pktgen.txt @@ -12,8 +12,8 @@ suitable sample script and configure that. On a dual CPU: ps aux | grep pkt -root 129 0.3 0.0 00 ?SW2003 523:20 [pktgen/0] -root 130 0.3 0.0 00 ?SW2003 509:50 [pktgen/1] +root 129 0.3 0.0 00 ?SW2003 523:20 [kpktgend_0] +root 130 0.3 0.0 00 ?SW2003 509:50 [kpktgend_1] For monitoring and control pktgen creates: @@ -113,9 +113,16 @@ Configuring devices === This is done via the /proc interface, and most easily done via pgset as defined in the sample scripts. +You need to specify PGDEV environment variable to use functions from sample +scripts, i.e.: +export PGDEV=/proc/net/pktgen/eth4@0 +source samples/pktgen/functions.sh Examples: + pg_ctrl start starts injection. + pg_ctrl stopaborts injection. Also, ^C aborts generator. + pgset "clone_skb 1" sets the number of copies of the same packet pgset "clone_skb 0" use single SKB for all transmits pgset "burst 8" uses xmit_more API to queue 8 copies of the same @@ -165,8 +172,12 @@ Examples: IPSEC # IPsec encapsulation (needs CONFIG_XFRM) NODE_ALLOC # node specific memory allocation NO_TIMESTAMP # disable timestamping + pgset 'flag ![name]'Clear a flag to determine behaviour. + Note that you might need to use single quote in + interactive mode, so that your shell wouldn't expand + the specified flag as a history command. - pgset spi SPI_VALUE Set specific SA used to transform packet. + pgset "spi [SPI_VALUE]" Set specific SA used to transform packet. pgset "udp_src_min 9" set UDP source port min, If < udp_src_max, then cycle through the port range. @@ -207,8 +218,6 @@ Examples: pgset "tos XX" set former IPv4 TOS field (e.g. "tos 28" for AF11 no ECN, default 00) pgset "traffic_class XX" set former IPv6 TRAFFIC CLASS (e.g. "traffic_class B8" for EF no ECN, default 00) - pgset stop aborts injection. Also, ^C aborts generator. - pgset "rate 300M"set rate to 300 Mb/s pgset "ratep 100"set rate to 1Mpps -- 2.13.6
[PATCHv2 3/5] pktgen: Add behaviour flags macro to generate flags/names
PKT_FALGS macro will be used to add package behavior names definitions to simplify the code that prints/reads pkg flags. Sorted the array in order of printing the flags in pktgen_if_show() Note: Renamed IPSEC_ON => IPSEC for simplicity. No visible behavior change expected. Signed-off-by: Dmitry Safonov--- net/core/pktgen.c | 57 +-- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/net/core/pktgen.c b/net/core/pktgen.c index ab63943ffd03..596fadcd8bb8 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -184,25 +184,36 @@ #define func_enter() pr_debug("entering %s\n", __func__); +#define PKT_FLAGS \ + pf(IPV6)/* Interface in IPV6 Mode */\ + pf(IPSRC_RND) /* IP-Src Random */\ + pf(IPDST_RND) /* IP-Dst Random */\ + pf(TXSIZE_RND) /* Transmit size is random */ \ + pf(UDPSRC_RND) /* UDP-Src Random */\ + pf(UDPDST_RND) /* UDP-Dst Random */\ + pf(UDPCSUM) /* Include UDP checksum */ \ + pf(NO_TIMESTAMP)/* Don't timestamp packets (default TS) */ \ + pf(MPLS_RND)/* Random MPLS labels */\ + pf(QUEUE_MAP_RND) /* queue map Random */ \ + pf(QUEUE_MAP_CPU) /* queue map mirrors smp_processor_id() */ \ + pf(FLOW_SEQ)/* Sequential flows */ \ + pf(IPSEC) /* ipsec on for flows */\ + pf(MACSRC_RND) /* MAC-Src Random */\ + pf(MACDST_RND) /* MAC-Dst Random */\ + pf(VID_RND) /* Random VLAN ID */\ + pf(SVID_RND)/* Random SVLAN ID */ \ + pf(NODE)/* Node memory alloc*/ \ + +#define pf(flag) flag##_SHIFT, +enum pkt_flags { + PKT_FLAGS +}; +#undef pf + /* Device flag bits */ -#define F_IPSRC_RND (1<<0) /* IP-Src Random */ -#define F_IPDST_RND (1<<1) /* IP-Dst Random */ -#define F_UDPSRC_RND (1<<2) /* UDP-Src Random */ -#define F_UDPDST_RND (1<<3) /* UDP-Dst Random */ -#define F_MACSRC_RND (1<<4) /* MAC-Src Random */ -#define F_MACDST_RND (1<<5) /* MAC-Dst Random */ -#define F_TXSIZE_RND (1<<6) /* Transmit size is random */ -#define F_IPV6(1<<7) /* Interface in IPV6 Mode */ -#define F_MPLS_RND(1<<8) /* Random MPLS labels */ -#define F_VID_RND (1<<9) /* Random VLAN ID */ -#define F_SVID_RND(1<<10) /* Random SVLAN ID */ -#define F_FLOW_SEQ(1<<11) /* Sequential flows */ -#define F_IPSEC_ON(1<<12) /* ipsec on for flows */ -#define F_QUEUE_MAP_RND (1<<13)/* queue map Random */ -#define F_QUEUE_MAP_CPU (1<<14)/* queue map mirrors smp_processor_id() */ -#define F_NODE (1<<15)/* Node memory alloc*/ -#define F_UDPCSUM (1<<16)/* Include UDP checksum */ -#define F_NO_TIMESTAMP (1<<17)/* Don't timestamp packets (default TS) */ +#define pf(flag) static const __u32 F_##flag = (1< flags & F_IPSEC_ON) { + if (pkt_dev->flags & F_IPSEC) { seq_puts(seq, "IPSEC "); if (pkt_dev->spi) seq_printf(seq, "spi:%u", pkt_dev->spi); @@ -1304,10 +1315,10 @@ static ssize_t pktgen_if_write(struct file *file, pkt_dev->flags &= ~F_QUEUE_MAP_CPU; #ifdef CONFIG_XFRM else if (strcmp(f, "IPSEC") == 0) - pkt_dev->flags |= F_IPSEC_ON; + pkt_dev->flags |= F_IPSEC; else if (strcmp(f, "!IPSEC") == 0) - pkt_dev->flags &= ~F_IPSEC_ON; + pkt_dev->flags &= ~F_IPSEC; #endif else if (strcmp(f, "!IPV6") == 0) @@ -2550,7 +2561,7 @@ static void mod_cur_headers(struct pktgen_dev *pkt_dev) pkt_dev->flows[flow].cur_daddr = pkt_dev->cur_daddr; #ifdef CONFIG_XFRM - if (pkt_dev->flags & F_IPSEC_ON) + if (pkt_dev->flags & F_IPSEC) get_ipsec_sa(pkt_dev, flow); #endif pkt_dev->nflows++; @@ -2655,7 +2666,7 @@ static void free_SAs(struct pktgen_dev *pkt_dev) static int process_ipsec(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
[PATCHv2 4/5] pktgen: Remove brute-force printing of flags
Add macro generated pkt_flag_names array, with a little help of which the flags can be printed by using an index. Signed-off-by: Dmitry Safonov--- net/core/pktgen.c | 77 ++- 1 file changed, 19 insertions(+), 58 deletions(-) diff --git a/net/core/pktgen.c b/net/core/pktgen.c index 596fadcd8bb8..f9883139e311 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -215,6 +215,14 @@ enum pkt_flags { PKT_FLAGS #undef pf +#define pf(flag) __stringify(flag), +static char *pkt_flag_names[] = { + PKT_FLAGS +}; +#undef pf + +#define NR_PKT_FLAGS ARRAY_SIZE(pkt_flag_names) + /* Thread control flag bits */ #define T_STOP(1<<0) /* Stop run */ #define T_RUN (1<<1) /* Start run */ @@ -546,6 +554,7 @@ static int pktgen_if_show(struct seq_file *seq, void *v) { const struct pktgen_dev *pkt_dev = seq->private; ktime_t stopped; + unsigned int i; u64 idle; seq_printf(seq, @@ -607,7 +616,6 @@ static int pktgen_if_show(struct seq_file *seq, void *v) pkt_dev->src_mac_count, pkt_dev->dst_mac_count); if (pkt_dev->nr_labels) { - unsigned int i; seq_puts(seq, " mpls: "); for (i = 0; i < pkt_dev->nr_labels; i++) seq_printf(seq, "%08x%s", ntohl(pkt_dev->labels[i]), @@ -643,68 +651,21 @@ static int pktgen_if_show(struct seq_file *seq, void *v) seq_puts(seq, " Flags: "); - if (pkt_dev->flags & F_IPV6) - seq_puts(seq, "IPV6 "); - - if (pkt_dev->flags & F_IPSRC_RND) - seq_puts(seq, "IPSRC_RND "); - - if (pkt_dev->flags & F_IPDST_RND) - seq_puts(seq, "IPDST_RND "); - - if (pkt_dev->flags & F_TXSIZE_RND) - seq_puts(seq, "TXSIZE_RND "); - - if (pkt_dev->flags & F_UDPSRC_RND) - seq_puts(seq, "UDPSRC_RND "); - - if (pkt_dev->flags & F_UDPDST_RND) - seq_puts(seq, "UDPDST_RND "); - - if (pkt_dev->flags & F_UDPCSUM) - seq_puts(seq, "UDPCSUM "); - - if (pkt_dev->flags & F_NO_TIMESTAMP) - seq_puts(seq, "NO_TIMESTAMP "); - - if (pkt_dev->flags & F_MPLS_RND) - seq_puts(seq, "MPLS_RND "); - - if (pkt_dev->flags & F_QUEUE_MAP_RND) - seq_puts(seq, "QUEUE_MAP_RND "); - - if (pkt_dev->flags & F_QUEUE_MAP_CPU) - seq_puts(seq, "QUEUE_MAP_CPU "); + for (i = 0; i < NR_PKT_FLAGS; i++) { + if (i == F_FLOW_SEQ) + if (!pkt_dev->cflows) + continue; - if (pkt_dev->cflows) { - if (pkt_dev->flags & F_FLOW_SEQ) - seq_puts(seq, "FLOW_SEQ "); /*in sequence flows*/ - else - seq_puts(seq, "FLOW_RND "); - } + if (pkt_dev->flags & (1 << i)) + seq_printf(seq, "%s ", pkt_flag_names[i]); + else if (i == F_FLOW_SEQ) + seq_puts(seq, "FLOW_RND "); #ifdef CONFIG_XFRM - if (pkt_dev->flags & F_IPSEC) { - seq_puts(seq, "IPSEC "); - if (pkt_dev->spi) + if (i == F_IPSEC && pkt_dev->spi) seq_printf(seq, "spi:%u", pkt_dev->spi); - } #endif - - if (pkt_dev->flags & F_MACSRC_RND) - seq_puts(seq, "MACSRC_RND "); - - if (pkt_dev->flags & F_MACDST_RND) - seq_puts(seq, "MACDST_RND "); - - if (pkt_dev->flags & F_VID_RND) - seq_puts(seq, "VID_RND "); - - if (pkt_dev->flags & F_SVID_RND) - seq_puts(seq, "SVID_RND "); - - if (pkt_dev->flags & F_NODE) - seq_puts(seq, "NODE_ALLOC "); + } seq_puts(seq, "\n"); -- 2.13.6
[PATCHv2 5/5] pktgen: Clean read user supplied flag mess
Don't use error-prone-brute-force way. Signed-off-by: Dmitry Safonov--- net/core/pktgen.c | 144 +++--- 1 file changed, 39 insertions(+), 105 deletions(-) diff --git a/net/core/pktgen.c b/net/core/pktgen.c index f9883139e311..e335daa40211 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -831,6 +831,35 @@ static ssize_t get_labels(const char __user *buffer, struct pktgen_dev *pkt_dev) return i; } +static __u32 pktgen_read_flag(const char *f, bool *disable) +{ + __u32 i; + + if (f[0] == '!') { + *disable = true; + f++; + } + + for (i = 0; i < NR_PKT_FLAGS; i++) { + if (!IS_ENABLED(CONFIG_XFRM) && i == IPSEC_SHIFT) + continue; + + /* allow only disabling ipv6 flag */ + if (!*disable && i == IPV6_SHIFT) + continue; + + if (strcmp(f, pkt_flag_names[i]) == 0) + return 1 << i; + } + + if (strcmp(f, "FLOW_RND") == 0) { + *disable = !*disable; + return F_FLOW_SEQ; + } + + return 0; +} + static ssize_t pktgen_if_write(struct file *file, const char __user * user_buffer, size_t count, loff_t * offset) @@ -1188,7 +1217,10 @@ static ssize_t pktgen_if_write(struct file *file, return count; } if (!strcmp(name, "flag")) { + __u32 flag; char f[32]; + bool disable = false; + memset(f, 0, 32); len = strn_len(_buffer[i], sizeof(f) - 1); if (len < 0) @@ -1197,113 +1229,15 @@ static ssize_t pktgen_if_write(struct file *file, if (copy_from_user(f, _buffer[i], len)) return -EFAULT; i += len; - if (strcmp(f, "IPSRC_RND") == 0) - pkt_dev->flags |= F_IPSRC_RND; - - else if (strcmp(f, "!IPSRC_RND") == 0) - pkt_dev->flags &= ~F_IPSRC_RND; - - else if (strcmp(f, "TXSIZE_RND") == 0) - pkt_dev->flags |= F_TXSIZE_RND; - - else if (strcmp(f, "!TXSIZE_RND") == 0) - pkt_dev->flags &= ~F_TXSIZE_RND; - - else if (strcmp(f, "IPDST_RND") == 0) - pkt_dev->flags |= F_IPDST_RND; - - else if (strcmp(f, "!IPDST_RND") == 0) - pkt_dev->flags &= ~F_IPDST_RND; - - else if (strcmp(f, "UDPSRC_RND") == 0) - pkt_dev->flags |= F_UDPSRC_RND; - - else if (strcmp(f, "!UDPSRC_RND") == 0) - pkt_dev->flags &= ~F_UDPSRC_RND; - - else if (strcmp(f, "UDPDST_RND") == 0) - pkt_dev->flags |= F_UDPDST_RND; - - else if (strcmp(f, "!UDPDST_RND") == 0) - pkt_dev->flags &= ~F_UDPDST_RND; - - else if (strcmp(f, "MACSRC_RND") == 0) - pkt_dev->flags |= F_MACSRC_RND; - - else if (strcmp(f, "!MACSRC_RND") == 0) - pkt_dev->flags &= ~F_MACSRC_RND; - else if (strcmp(f, "MACDST_RND") == 0) - pkt_dev->flags |= F_MACDST_RND; + flag = pktgen_read_flag(f, ); - else if (strcmp(f, "!MACDST_RND") == 0) - pkt_dev->flags &= ~F_MACDST_RND; - - else if (strcmp(f, "MPLS_RND") == 0) - pkt_dev->flags |= F_MPLS_RND; - - else if (strcmp(f, "!MPLS_RND") == 0) - pkt_dev->flags &= ~F_MPLS_RND; - - else if (strcmp(f, "VID_RND") == 0) - pkt_dev->flags |= F_VID_RND; - - else if (strcmp(f, "!VID_RND") == 0) - pkt_dev->flags &= ~F_VID_RND; - - else if (strcmp(f, "SVID_RND") == 0) - pkt_dev->flags |= F_SVID_RND; - - else if (strcmp(f, "!SVID_RND") == 0) - pkt_dev->flags &= ~F_SVID_RND; - - else if (strcmp(f, "FLOW_SEQ") == 0 || strcmp(f, "!FLOW_RND") == 0) - pkt_dev->flags |= F_FLOW_SEQ; - - else if (strcmp(f, "FLOW_RND") == 0 || strcmp(f, "!FLOW_SEQ") == 0) - pkt_dev->flags &= ~F_FLOW_SEQ; - - else if (strcmp(f, "QUEUE_MAP_RND") == 0) - pkt_dev->flags |= F_QUEUE_MAP_RND; - - else if (strcmp(f, "!QUEUE_MAP_RND") == 0) - pkt_dev->flags &= ~F_QUEUE_MAP_RND; - - else if (strcmp(f, "QUEUE_MAP_CPU") == 0) - pkt_dev->flags |= F_QUEUE_MAP_CPU; - - else if (strcmp(f, "!QUEUE_MAP_CPU") == 0) - pkt_dev->flags &= ~F_QUEUE_MAP_CPU;
Re: [PATCH 3/5] pktgen: Add behavior flag names array - pkt_flag_names
On Mon, 2018-01-15 at 13:09 -0500, David Miller wrote: > From: Dmitry Safonov> Date: Tue, 9 Jan 2018 13:55:33 + > > > +#define pf(flag) __stringify(flag), > > +char *pkt_flag_names[] = { > > + PKT_FLAGS > > +}; > > +#undef pf > > This should be static, also you don't use this table in this patch. > You > should add the table in the patch that actually uses the table. Sure, will do. -- Thanks, Dmitry
Re: [PATCH 3/5] pktgen: Add behavior flag names array - pkt_flag_names
From: Dmitry SafonovDate: Tue, 9 Jan 2018 13:55:33 + > +#define pf(flag) __stringify(flag), > +char *pkt_flag_names[] = { > + PKT_FLAGS > +}; > +#undef pf This should be static, also you don't use this table in this patch. You should add the table in the patch that actually uses the table.
[PATCH 0/5] pktgen: Behavior flags fixes
There are a bunch of fixes/cleanups/Documentations. Diffstat says for itself, regardless added docs and missed flag parameters. Cc: Arnd Bergmann <a...@arndb.de> Cc: "David S. Miller" <da...@davemloft.net> Cc: David Windsor <dwind...@gmail.com> Cc: Eric Dumazet <eduma...@google.com> Cc: Ingo Molnar <mi...@kernel.org> Cc: Johannes Berg <johannes.b...@intel.com> Cc: Mark Rutland <mark.rutl...@arm.com> Cc: Radu Rendec <rren...@arista.com> Cc: "Reshetova, Elena" <elena.reshet...@intel.com> Cc: netdev@vger.kernel.org Dmitry Safonov (5): Documentation/pktgen: Clearify how-to use pktgen samples pktgen: Add missing !flag parameters pktgen: Add behavior flag names array - pkt_flag_names pktgen: Remove brute-force printing of flags pktgen: Clean read user supplied flag mess Documentation/networking/pktgen.txt | 19 ++- net/core/pktgen.c | 267 2 files changed, 104 insertions(+), 182 deletions(-) -- 2.13.6
[PATCH 1/5] Documentation/pktgen: Clearify how-to use pktgen samples
o Change process name in ps output: looks like, these days the process is named kpktgend_, rather than pktgen/. o Use pg_ctrl for start/stop as it can work well with pgset without changes to $(PGDEV) variable. o Clarify a bit needed $(PGDEV) definition for sample scripts and that one needs to `source functions.sh`. o Document how-to unset a behaviour flag, note about history expansion. o Fix pgset spi parameter value. Cc: Jonathan Corbet <cor...@lwn.net> Cc: linux-...@vger.kernel.org Signed-off-by: Dmitry Safonov <d...@arista.com> --- Documentation/networking/pktgen.txt | 19 ++- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/Documentation/networking/pktgen.txt b/Documentation/networking/pktgen.txt index 2c4e3354e128..d2fd78f85aa4 100644 --- a/Documentation/networking/pktgen.txt +++ b/Documentation/networking/pktgen.txt @@ -12,8 +12,8 @@ suitable sample script and configure that. On a dual CPU: ps aux | grep pkt -root 129 0.3 0.0 00 ?SW2003 523:20 [pktgen/0] -root 130 0.3 0.0 00 ?SW2003 509:50 [pktgen/1] +root 129 0.3 0.0 00 ?SW2003 523:20 [kpktgend_0] +root 130 0.3 0.0 00 ?SW2003 509:50 [kpktgend_1] For monitoring and control pktgen creates: @@ -113,9 +113,16 @@ Configuring devices === This is done via the /proc interface, and most easily done via pgset as defined in the sample scripts. +You need to specify PGDEV environment variable to use functions from sample +scripts, i.e.: +export PGDEV=/proc/net/pktgen/eth4@0 +source samples/pktgen/functions.sh Examples: + pg_ctrl start starts injection. + pg_ctrl stopaborts injection. Also, ^C aborts generator. + pgset "clone_skb 1" sets the number of copies of the same packet pgset "clone_skb 0" use single SKB for all transmits pgset "burst 8" uses xmit_more API to queue 8 copies of the same @@ -165,8 +172,12 @@ Examples: IPSEC # IPsec encapsulation (needs CONFIG_XFRM) NODE_ALLOC # node specific memory allocation NO_TIMESTAMP # disable timestamping + pgset 'flag ![name]'Clear a flag to determine behaviour. + Note that you might need to use single quote in + interactive mode, so that your shell wouldn't expand + the specified flag as a history command. - pgset spi SPI_VALUE Set specific SA used to transform packet. + pgset "spi [SPI_VALUE]" Set specific SA used to transform packet. pgset "udp_src_min 9" set UDP source port min, If < udp_src_max, then cycle through the port range. @@ -207,8 +218,6 @@ Examples: pgset "tos XX" set former IPv4 TOS field (e.g. "tos 28" for AF11 no ECN, default 00) pgset "traffic_class XX" set former IPv6 TRAFFIC CLASS (e.g. "traffic_class B8" for EF no ECN, default 00) - pgset stop aborts injection. Also, ^C aborts generator. - pgset "rate 300M"set rate to 300 Mb/s pgset "ratep 100"set rate to 1Mpps -- 2.13.6
[PATCH 3/5] pktgen: Add behavior flag names array - pkt_flag_names
The array will be used to simplify the code that prints/reads pkg flags. Sorted the array in order of printing the flags in pktgen_if_show() Note: Renamed IPSEC_ON => IPSEC for simplicity. No visible behavior change expected. Signed-off-by: Dmitry Safonov--- net/core/pktgen.c | 64 +++ 1 file changed, 41 insertions(+), 23 deletions(-) diff --git a/net/core/pktgen.c b/net/core/pktgen.c index ab63943ffd03..51f273319baf 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -184,25 +184,43 @@ #define func_enter() pr_debug("entering %s\n", __func__); +#define PKT_FLAGS \ + pf(IPV6)/* Interface in IPV6 Mode */\ + pf(IPSRC_RND) /* IP-Src Random */\ + pf(IPDST_RND) /* IP-Dst Random */\ + pf(TXSIZE_RND) /* Transmit size is random */ \ + pf(UDPSRC_RND) /* UDP-Src Random */\ + pf(UDPDST_RND) /* UDP-Dst Random */\ + pf(UDPCSUM) /* Include UDP checksum */ \ + pf(NO_TIMESTAMP)/* Don't timestamp packets (default TS) */ \ + pf(MPLS_RND)/* Random MPLS labels */\ + pf(QUEUE_MAP_RND) /* queue map Random */ \ + pf(QUEUE_MAP_CPU) /* queue map mirrors smp_processor_id() */ \ + pf(FLOW_SEQ)/* Sequential flows */ \ + pf(IPSEC) /* ipsec on for flows */\ + pf(MACSRC_RND) /* MAC-Src Random */\ + pf(MACDST_RND) /* MAC-Dst Random */\ + pf(VID_RND) /* Random VLAN ID */\ + pf(SVID_RND)/* Random SVLAN ID */ \ + pf(NODE)/* Node memory alloc*/ \ + +#define pf(flag) flag##_SHIFT, +enum pkt_flags { + PKT_FLAGS +}; +#undef pf + +#define pf(flag) static const __u32 F_##flag = (1< flags & F_IPSEC_ON) { + if (pkt_dev->flags & F_IPSEC) { seq_puts(seq, "IPSEC "); if (pkt_dev->spi) seq_printf(seq, "spi:%u", pkt_dev->spi); @@ -1304,10 +1322,10 @@ static ssize_t pktgen_if_write(struct file *file, pkt_dev->flags &= ~F_QUEUE_MAP_CPU; #ifdef CONFIG_XFRM else if (strcmp(f, "IPSEC") == 0) - pkt_dev->flags |= F_IPSEC_ON; + pkt_dev->flags |= F_IPSEC; else if (strcmp(f, "!IPSEC") == 0) - pkt_dev->flags &= ~F_IPSEC_ON; + pkt_dev->flags &= ~F_IPSEC; #endif else if (strcmp(f, "!IPV6") == 0) @@ -2550,7 +2568,7 @@ static void mod_cur_headers(struct pktgen_dev *pkt_dev) pkt_dev->flows[flow].cur_daddr = pkt_dev->cur_daddr; #ifdef CONFIG_XFRM - if (pkt_dev->flags & F_IPSEC_ON) + if (pkt_dev->flags & F_IPSEC) get_ipsec_sa(pkt_dev, flow); #endif pkt_dev->nflows++; @@ -2655,7 +2673,7 @@ static void free_SAs(struct pktgen_dev *pkt_dev) static int process_ipsec(struct
[PATCH 2/5] pktgen: Add missing !flag parameters
o FLOW_SEQ now can be disabled with pgset "flag !FLOW_SEQ" o FLOW_SEQ and FLOW_RND are antonyms, as it's shown by pktgen_if_show() o IPSEC now may be disabled Note, that IPV6 is enabled with dst6/src6 parameters, not with a flag parameter. Signed-off-by: Dmitry Safonov--- net/core/pktgen.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/net/core/pktgen.c b/net/core/pktgen.c index f95a15086225..ab63943ffd03 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -1285,9 +1285,12 @@ static ssize_t pktgen_if_write(struct file *file, else if (strcmp(f, "!SVID_RND") == 0) pkt_dev->flags &= ~F_SVID_RND; - else if (strcmp(f, "FLOW_SEQ") == 0) + else if (strcmp(f, "FLOW_SEQ") == 0 || strcmp(f, "!FLOW_RND") == 0) pkt_dev->flags |= F_FLOW_SEQ; + else if (strcmp(f, "FLOW_RND") == 0 || strcmp(f, "!FLOW_SEQ") == 0) + pkt_dev->flags &= ~F_FLOW_SEQ; + else if (strcmp(f, "QUEUE_MAP_RND") == 0) pkt_dev->flags |= F_QUEUE_MAP_RND; @@ -1302,6 +1305,9 @@ static ssize_t pktgen_if_write(struct file *file, #ifdef CONFIG_XFRM else if (strcmp(f, "IPSEC") == 0) pkt_dev->flags |= F_IPSEC_ON; + + else if (strcmp(f, "!IPSEC") == 0) + pkt_dev->flags &= ~F_IPSEC_ON; #endif else if (strcmp(f, "!IPV6") == 0) -- 2.13.6
[PATCH 4/5] pktgen: Remove brute-force printing of flags
Like, we can do it using index. Signed-off-by: Dmitry Safonov--- net/core/pktgen.c | 71 ++- 1 file changed, 13 insertions(+), 58 deletions(-) diff --git a/net/core/pktgen.c b/net/core/pktgen.c index 51f273319baf..e320f0cbfd62 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -220,6 +220,8 @@ char *pkt_flag_names[] = { }; #undef pf +#define NR_PKT_FLAGS ARRAY_SIZE(pkt_flag_names) + /* Device flag bits */ /* Thread control flag bits */ @@ -553,6 +555,7 @@ static int pktgen_if_show(struct seq_file *seq, void *v) { const struct pktgen_dev *pkt_dev = seq->private; ktime_t stopped; + unsigned int i; u64 idle; seq_printf(seq, @@ -614,7 +617,6 @@ static int pktgen_if_show(struct seq_file *seq, void *v) pkt_dev->src_mac_count, pkt_dev->dst_mac_count); if (pkt_dev->nr_labels) { - unsigned int i; seq_puts(seq, " mpls: "); for (i = 0; i < pkt_dev->nr_labels; i++) seq_printf(seq, "%08x%s", ntohl(pkt_dev->labels[i]), @@ -650,68 +652,21 @@ static int pktgen_if_show(struct seq_file *seq, void *v) seq_puts(seq, " Flags: "); - if (pkt_dev->flags & F_IPV6) - seq_puts(seq, "IPV6 "); - - if (pkt_dev->flags & F_IPSRC_RND) - seq_puts(seq, "IPSRC_RND "); - - if (pkt_dev->flags & F_IPDST_RND) - seq_puts(seq, "IPDST_RND "); - - if (pkt_dev->flags & F_TXSIZE_RND) - seq_puts(seq, "TXSIZE_RND "); - - if (pkt_dev->flags & F_UDPSRC_RND) - seq_puts(seq, "UDPSRC_RND "); - - if (pkt_dev->flags & F_UDPDST_RND) - seq_puts(seq, "UDPDST_RND "); - - if (pkt_dev->flags & F_UDPCSUM) - seq_puts(seq, "UDPCSUM "); - - if (pkt_dev->flags & F_NO_TIMESTAMP) - seq_puts(seq, "NO_TIMESTAMP "); - - if (pkt_dev->flags & F_MPLS_RND) - seq_puts(seq, "MPLS_RND "); - - if (pkt_dev->flags & F_QUEUE_MAP_RND) - seq_puts(seq, "QUEUE_MAP_RND "); - - if (pkt_dev->flags & F_QUEUE_MAP_CPU) - seq_puts(seq, "QUEUE_MAP_CPU "); + for (i = 0; i < NR_PKT_FLAGS; i++) { + if (i == F_FLOW_SEQ) + if (!pkt_dev->cflows) + continue; - if (pkt_dev->cflows) { - if (pkt_dev->flags & F_FLOW_SEQ) - seq_puts(seq, "FLOW_SEQ "); /*in sequence flows*/ - else - seq_puts(seq, "FLOW_RND "); - } + if (pkt_dev->flags & (1 << i)) + seq_printf(seq, "%s ", pkt_flag_names[i]); + else if (i == F_FLOW_SEQ) + seq_puts(seq, "FLOW_RND "); #ifdef CONFIG_XFRM - if (pkt_dev->flags & F_IPSEC) { - seq_puts(seq, "IPSEC "); - if (pkt_dev->spi) + if (i == F_IPSEC && pkt_dev->spi) seq_printf(seq, "spi:%u", pkt_dev->spi); - } #endif - - if (pkt_dev->flags & F_MACSRC_RND) - seq_puts(seq, "MACSRC_RND "); - - if (pkt_dev->flags & F_MACDST_RND) - seq_puts(seq, "MACDST_RND "); - - if (pkt_dev->flags & F_VID_RND) - seq_puts(seq, "VID_RND "); - - if (pkt_dev->flags & F_SVID_RND) - seq_puts(seq, "SVID_RND "); - - if (pkt_dev->flags & F_NODE) - seq_puts(seq, "NODE_ALLOC "); + } seq_puts(seq, "\n"); -- 2.13.6
[PATCH 5/5] pktgen: Clean read user supplied flag mess
Don't use error-prone-brute-force way. Signed-off-by: Dmitry Safonov--- net/core/pktgen.c | 144 +++--- 1 file changed, 39 insertions(+), 105 deletions(-) diff --git a/net/core/pktgen.c b/net/core/pktgen.c index e320f0cbfd62..a3862e500643 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -832,6 +832,35 @@ static ssize_t get_labels(const char __user *buffer, struct pktgen_dev *pkt_dev) return i; } +static __u32 pktgen_read_flag(const char *f, bool *disable) +{ + __u32 i; + + if (f[0] == '!') { + *disable = true; + f++; + } + + for (i = 0; i < NR_PKT_FLAGS; i++) { + if (!IS_ENABLED(CONFIG_XFRM) && i == IPSEC_SHIFT) + continue; + + /* allow only disabling ipv6 flag */ + if (!*disable && i == IPV6_SHIFT) + continue; + + if (strcmp(f, pkt_flag_names[i]) == 0) + return 1 << i; + } + + if (strcmp(f, "FLOW_RND") == 0) { + *disable = !*disable; + return F_FLOW_SEQ; + } + + return 0; +} + static ssize_t pktgen_if_write(struct file *file, const char __user * user_buffer, size_t count, loff_t * offset) @@ -1189,7 +1218,10 @@ static ssize_t pktgen_if_write(struct file *file, return count; } if (!strcmp(name, "flag")) { + __u32 flag; char f[32]; + bool disable = false; + memset(f, 0, 32); len = strn_len(_buffer[i], sizeof(f) - 1); if (len < 0) @@ -1198,113 +1230,15 @@ static ssize_t pktgen_if_write(struct file *file, if (copy_from_user(f, _buffer[i], len)) return -EFAULT; i += len; - if (strcmp(f, "IPSRC_RND") == 0) - pkt_dev->flags |= F_IPSRC_RND; - - else if (strcmp(f, "!IPSRC_RND") == 0) - pkt_dev->flags &= ~F_IPSRC_RND; - - else if (strcmp(f, "TXSIZE_RND") == 0) - pkt_dev->flags |= F_TXSIZE_RND; - - else if (strcmp(f, "!TXSIZE_RND") == 0) - pkt_dev->flags &= ~F_TXSIZE_RND; - - else if (strcmp(f, "IPDST_RND") == 0) - pkt_dev->flags |= F_IPDST_RND; - - else if (strcmp(f, "!IPDST_RND") == 0) - pkt_dev->flags &= ~F_IPDST_RND; - - else if (strcmp(f, "UDPSRC_RND") == 0) - pkt_dev->flags |= F_UDPSRC_RND; - - else if (strcmp(f, "!UDPSRC_RND") == 0) - pkt_dev->flags &= ~F_UDPSRC_RND; - - else if (strcmp(f, "UDPDST_RND") == 0) - pkt_dev->flags |= F_UDPDST_RND; - - else if (strcmp(f, "!UDPDST_RND") == 0) - pkt_dev->flags &= ~F_UDPDST_RND; - - else if (strcmp(f, "MACSRC_RND") == 0) - pkt_dev->flags |= F_MACSRC_RND; - - else if (strcmp(f, "!MACSRC_RND") == 0) - pkt_dev->flags &= ~F_MACSRC_RND; - else if (strcmp(f, "MACDST_RND") == 0) - pkt_dev->flags |= F_MACDST_RND; + flag = pktgen_read_flag(f, ); - else if (strcmp(f, "!MACDST_RND") == 0) - pkt_dev->flags &= ~F_MACDST_RND; - - else if (strcmp(f, "MPLS_RND") == 0) - pkt_dev->flags |= F_MPLS_RND; - - else if (strcmp(f, "!MPLS_RND") == 0) - pkt_dev->flags &= ~F_MPLS_RND; - - else if (strcmp(f, "VID_RND") == 0) - pkt_dev->flags |= F_VID_RND; - - else if (strcmp(f, "!VID_RND") == 0) - pkt_dev->flags &= ~F_VID_RND; - - else if (strcmp(f, "SVID_RND") == 0) - pkt_dev->flags |= F_SVID_RND; - - else if (strcmp(f, "!SVID_RND") == 0) - pkt_dev->flags &= ~F_SVID_RND; - - else if (strcmp(f, "FLOW_SEQ") == 0 || strcmp(f, "!FLOW_RND") == 0) - pkt_dev->flags |= F_FLOW_SEQ; - - else if (strcmp(f, "FLOW_RND") == 0 || strcmp(f, "!FLOW_SEQ") == 0) - pkt_dev->flags &= ~F_FLOW_SEQ; - - else if (strcmp(f, "QUEUE_MAP_RND") == 0) - pkt_dev->flags |= F_QUEUE_MAP_RND; - - else if (strcmp(f, "!QUEUE_MAP_RND") == 0) - pkt_dev->flags &= ~F_QUEUE_MAP_RND; - - else if (strcmp(f, "QUEUE_MAP_CPU") == 0) - pkt_dev->flags |= F_QUEUE_MAP_CPU; - - else if (strcmp(f, "!QUEUE_MAP_CPU") == 0) - pkt_dev->flags &= ~F_QUEUE_MAP_CPU;
Re: [PATCH] pktgen: document 32-bit timestamp overflow
From: Arnd Bergmann <a...@arndb.de> Date: Tue, 7 Nov 2017 11:38:32 +0100 > Timestamps in pktgen are currently retrieved using the deprecated > do_gettimeofday() function that wraps its signed 32-bit seconds in 2038 > (on 32-bit architectures) and requires a division operation to calculate > microseconds. > > The pktgen header is also defined with the same limitations, hardcoding > to a 32-bit seconds field that can be interpreted as unsigned to produce > times that only wrap in 2106. Whatever code reads the timestamps should > be aware of that problem in general, but probably doesn't care too > much as we are mostly interested in the time passing between packets, > and that is correctly represented. > > Using 64-bit nanoseconds would be cheaper and good for 584 years. Using > monotonic times would also make this unambiguous by avoiding the overflow, > but would make it harder to correlate to the times with those on remote > machines. Either approach would require adding a new runtime flag and > implementing the same thing on the remote side, which we probably don't > want to do unless someone sees it as a real problem. Also, this should > be coordinated with other pktgen implementations and might need a new > magic number. > > For the moment, I'm documenting the overflow in the source code, and > changing the implementation over to an open-coded ktime_get_real_ts64() > plus division, so we don't have to look at it again while scanning for > deprecated time interfaces. > > Signed-off-by: Arnd Bergmann <a...@arndb.de> Applied to net-next, thanks Arnd.
[PATCH] pktgen: document 32-bit timestamp overflow
Timestamps in pktgen are currently retrieved using the deprecated do_gettimeofday() function that wraps its signed 32-bit seconds in 2038 (on 32-bit architectures) and requires a division operation to calculate microseconds. The pktgen header is also defined with the same limitations, hardcoding to a 32-bit seconds field that can be interpreted as unsigned to produce times that only wrap in 2106. Whatever code reads the timestamps should be aware of that problem in general, but probably doesn't care too much as we are mostly interested in the time passing between packets, and that is correctly represented. Using 64-bit nanoseconds would be cheaper and good for 584 years. Using monotonic times would also make this unambiguous by avoiding the overflow, but would make it harder to correlate to the times with those on remote machines. Either approach would require adding a new runtime flag and implementing the same thing on the remote side, which we probably don't want to do unless someone sees it as a real problem. Also, this should be coordinated with other pktgen implementations and might need a new magic number. For the moment, I'm documenting the overflow in the source code, and changing the implementation over to an open-coded ktime_get_real_ts64() plus division, so we don't have to look at it again while scanning for deprecated time interfaces. Signed-off-by: Arnd Bergmann <a...@arndb.de> --- net/core/pktgen.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/net/core/pktgen.c b/net/core/pktgen.c index 6c5763b4411c..f95a15086225 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -2711,7 +2711,7 @@ static inline __be16 build_tci(unsigned int id, unsigned int cfi, static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb, int datalen) { - struct timeval timestamp; + struct timespec64 timestamp; struct pktgen_hdr *pgh; pgh = skb_put(skb, sizeof(*pgh)); @@ -2773,9 +2773,17 @@ static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb, pgh->tv_sec = 0; pgh->tv_usec = 0; } else { - do_gettimeofday(); + /* +* pgh->tv_sec wraps in y2106 when interpreted as unsigned +* as done by wireshark, or y2038 when interpreted as signed. +* This is probably harmless, but if anyone wants to improve +* it, we could introduce a variant that puts 64-bit nanoseconds +* into the respective header bytes. +* This would also be slightly faster to read. +*/ + ktime_get_real_ts64(); pgh->tv_sec = htonl(timestamp.tv_sec); - pgh->tv_usec = htonl(timestamp.tv_usec); + pgh->tv_usec = htonl(timestamp.tv_nsec / NSEC_PER_USEC); } } -- 2.9.0
Re: [PATCH net-next] pktgen: do not abuse IN6_ADDR_HSIZE
From: Eric Dumazet <eric.duma...@gmail.com> Date: Sat, 04 Nov 2017 08:27:14 -0700 > From: Eric Dumazet <eduma...@google.com> > > pktgen accidentally used IN6_ADDR_HSIZE, instead of using the size of an > IPv6 address. > > Since IN6_ADDR_HSIZE recently was increased from 16 to 256, this old > bug is hitting us. > > Fixes: 3f27fb23219e ("ipv6: addrconf: add per netns perturbation in > inet6_addr_hash()") > Signed-off-by: Eric Dumazet <eduma...@google.com> > Reported-by: Dan Carpenter <dan.carpen...@oracle.com> Applied.
[PATCH net-next] pktgen: do not abuse IN6_ADDR_HSIZE
From: Eric Dumazet <eduma...@google.com> pktgen accidentally used IN6_ADDR_HSIZE, instead of using the size of an IPv6 address. Since IN6_ADDR_HSIZE recently was increased from 16 to 256, this old bug is hitting us. Fixes: 3f27fb23219e ("ipv6: addrconf: add per netns perturbation in inet6_addr_hash()") Signed-off-by: Eric Dumazet <eduma...@google.com> Reported-by: Dan Carpenter <dan.carpen...@oracle.com> --- net/core/pktgen.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/core/pktgen.c b/net/core/pktgen.c index 6e1e10ff433a5f4097d1d4b33848ab13d4e005c6..e3fa53a07d34b3e5f6b438e08b440f520b3cd6d4 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -2165,7 +2165,7 @@ static void pktgen_setup_inject(struct pktgen_dev *pkt_dev) + pkt_dev->pkt_overhead; } - for (i = 0; i < IN6_ADDR_HSIZE; i++) + for (i = 0; i < sizeof(struct in6_addr); i++) if (pkt_dev->cur_in6_saddr.s6_addr[i]) { set = 1; break;
Re: [net-next PATCH 0/4] Updates for samples/pktgen
From: Jesper Dangaard Brouer <bro...@redhat.com> Date: Wed, 01 Nov 2017 11:41:04 +0100 > This patchset updates samples/pktgen and synchronize with changes > maintained in https://github.com/netoptimizer/network-testing/ > > Features wise Robert Hoo <robert...@intel.com> added support for > detecting and determining dev NUMA node IRQs, and added a new script > named pktgen_sample06_numa_awared_queue_irq_affinity.sh that use these > features. > > Cleanup remove last of the old sample files, as IPv6 is covered by > existing sample code. Series applied, thanks Jesper.
Re: [net-next PATCH 0/4] Updates for samples/pktgen
On Wed, Nov 01, 2017 at 11:41:04AM +0100, Jesper Dangaard Brouer wrote: > This patchset updates samples/pktgen and synchronize with changes > maintained in https://github.com/netoptimizer/network-testing/ > > Features wise Robert Hoo <robert...@intel.com> added support for > detecting and determining dev NUMA node IRQs, and added a new script > named pktgen_sample06_numa_awared_queue_irq_affinity.sh that use these > features. > > Cleanup remove last of the old sample files, as IPv6 is covered by > existing sample code. Looks nice. Thanks for doing this work. Acked-by: Alexei Starovoitov <a...@kernel.org>
[net-next PATCH 0/4] Updates for samples/pktgen
This patchset updates samples/pktgen and synchronize with changes maintained in https://github.com/netoptimizer/network-testing/ Features wise Robert Hoo <robert...@intel.com> added support for detecting and determining dev NUMA node IRQs, and added a new script named pktgen_sample06_numa_awared_queue_irq_affinity.sh that use these features. Cleanup remove last of the old sample files, as IPv6 is covered by existing sample code. --- Jesper Dangaard Brouer (2): samples/pktgen: update sample03, no need for clones when bursting samples/pktgen: remove remaining old pktgen sample scripts Robert Hoo (2): samples/pktgen: Add some helper functions samples/pktgen: add script pktgen_sample06_numa_awared_queue_irq_affinity.sh samples/pktgen/functions.sh| 43 + samples/pktgen/pktgen.conf-1-1-ip6 | 60 samples/pktgen/pktgen.conf-1-1-ip6-rdos| 63 - samples/pktgen/pktgen.conf-1-2 | 69 -- .../pktgen/pktgen_sample03_burst_single_flow.sh|2 ...tgen_sample06_numa_awared_queue_irq_affinity.sh | 97 6 files changed, 141 insertions(+), 193 deletions(-) delete mode 100755 samples/pktgen/pktgen.conf-1-1-ip6 delete mode 100755 samples/pktgen/pktgen.conf-1-1-ip6-rdos delete mode 100755 samples/pktgen/pktgen.conf-1-2 create mode 100755 samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh --
[net-next PATCH 4/4] samples/pktgen: remove remaining old pktgen sample scripts
Since commit 0f06a6787e05 ("samples: Add an IPv6 '-6' option to the pktgen scripts") the newer pktgen_sampleXX script does show howto use IPv6 with pktgen. Thus, there is no longer a reason to keep the older sample scripts around. Signed-off-by: Jesper Dangaard Brouer <bro...@redhat.com> --- samples/pktgen/pktgen.conf-1-1-ip6 | 60 --- samples/pktgen/pktgen.conf-1-1-ip6-rdos | 63 ---- samples/pktgen/pktgen.conf-1-2 | 69 --- 3 files changed, 192 deletions(-) delete mode 100755 samples/pktgen/pktgen.conf-1-1-ip6 delete mode 100755 samples/pktgen/pktgen.conf-1-1-ip6-rdos delete mode 100755 samples/pktgen/pktgen.conf-1-2 diff --git a/samples/pktgen/pktgen.conf-1-1-ip6 b/samples/pktgen/pktgen.conf-1-1-ip6 deleted file mode 100755 index 0b9ffd47fd41..0000 --- a/samples/pktgen/pktgen.conf-1-1-ip6 +++ /dev/null @@ -1,60 +0,0 @@ -#!/bin/bash - -#modprobe pktgen - - -function pgset() { -local result - -echo $1 > $PGDEV - -result=`cat $PGDEV | fgrep "Result: OK:"` -if [ "$result" = "" ]; then - cat $PGDEV | fgrep Result: -fi -} - -# Config Start Here --- - - -# thread config -# Each CPU has its own thread. One CPU example. We add eth1. -# IPv6. Note increase in minimal packet length - -PGDEV=/proc/net/pktgen/kpktgend_0 - echo "Removing all devices" - pgset "rem_device_all" - echo "Adding eth1" - pgset "add_device eth1" - - -# device config -# delay 0 - -CLONE_SKB="clone_skb 100" -# NIC adds 4 bytes CRC -PKT_SIZE="pkt_size 66" - -# COUNT 0 means forever -#COUNT="count 0" -COUNT="count 1000" -DELAY="delay 0" - -PGDEV=/proc/net/pktgen/eth1 - echo "Configuring $PGDEV" - pgset "$COUNT" - pgset "$CLONE_SKB" - pgset "$PKT_SIZE" - pgset "$DELAY" - pgset "dst6 fec0::1" - pgset "src6 fec0::2" - pgset "dst_mac 00:04:23:08:91:dc" - -# Time to run -PGDEV=/proc/net/pktgen/pgctrl - - echo "Running... ctrl^C to stop" - trap true INT - pgset "start" - echo "Done" - cat /proc/net/pktgen/eth1 diff --git a/samples/pktgen/pktgen.conf-1-1-ip6-rdos b/samples/pktgen/pktgen.conf-1-1-ip6-rdos deleted file mode 100755 index ad98e5f40776.. --- a/samples/pktgen/pktgen.conf-1-1-ip6-rdos +++ /dev/null @@ -1,63 +0,0 @@ -#!/bin/bash - -#modprobe pktgen - - -function pgset() { -local result - -echo $1 > $PGDEV - -result=`cat $PGDEV | fgrep "Result: OK:"` -if [ "$result" = "" ]; then - cat $PGDEV | fgrep Result: -fi -} - -# Config Start Here --- - - -# thread config -# Each CPU has its own thread. One CPU example. We add eth1. -# IPv6. Note increase in minimal packet length - -PGDEV=/proc/net/pktgen/kpktgend_0 - echo "Removing all devices" - pgset "rem_device_all" - echo "Adding eth1" - pgset "add_device eth1" - - -# device config -# delay 0 means maximum speed. - -# We need to do alloc for every skb since we cannot clone here. -CLONE_SKB="clone_skb 0" - -# NIC adds 4 bytes CRC -PKT_SIZE="pkt_size 66" - -# COUNT 0 means forever -#COUNT="count 0" -COUNT="count 1000" -DELAY="delay 0" - -PGDEV=/proc/net/pktgen/eth1 - echo "Configuring $PGDEV" - pgset "$COUNT" - pgset "$CLONE_SKB" - pgset "$PKT_SIZE" - pgset "$DELAY" - pgset "dst6_min fec0::1" - pgset "dst6_max fec0::FFFF:FFFF" - - pgset "dst_mac 00:04:23:08:91:dc" - -# Time to run -PGDEV=/proc/net/pktgen/pgctrl - - echo "Running... ctrl^C to stop" - trap true INT - pgset "start" - echo "Done" - cat /proc/net/pktgen/eth1 diff --git a/samples/pktgen/pktgen.conf-1-2 b/samples/pktgen/pktgen.conf-1-2 deleted file mode 100755 index ba4eb26e168d.. --- a/samples/pktgen/pktgen.conf-1-2 +++ /dev/null @@ -1,69 +0,0 @@ -#!/bin/bash - -#modprobe pktgen - - -function pgset() { -local result - -echo $1 > $PGDEV - -result=`cat $PGDEV | fgrep "Result: OK:"` -if [ "$result" = "" ]; then - cat $PGDEV | fgrep Result: -fi -} - -# Config Start Here --- - - -# thread config -# One CPU means one thread. One CPU example. We add eth1, eth2 respectivly. - -PGDEV=/proc/net/pktgen/kpktgend_0 - echo "Removing all devices" - pgset "rem_device_all" - echo "Adding eth1" - pgset "add_device eth1" - echo "Adding eth2" - pgset "add_device eth2" - - -# device config -#
[net-next PATCH 1/4] samples/pktgen: Add some helper functions
From: Robert Hoo <robert...@intel.com> 1. given a device, get its NUMA belongings 2. given a device, get its queues' irq numbers. 3. given a NUMA node, get its cpu id list. Signed-off-by: Robert Hoo <robert...@intel.com> Signed-off-by: Jesper Dangaard Brouer <bro...@redhat.com> --- samples/pktgen/functions.sh | 43 +++ 1 file changed, 43 insertions(+) diff --git a/samples/pktgen/functions.sh b/samples/pktgen/functions.sh index 205e4cde4601..f8bb3cd0f4ce 100644 --- a/samples/pktgen/functions.sh +++ b/samples/pktgen/functions.sh @@ -119,3 +119,46 @@ function root_check_run_with_sudo() { err 4 "cannot perform sudo run of $0" fi } + +# Exact input device's NUMA node info +function get_iface_node() +{ +local node=$(
[net-next PATCH 2/4] samples/pktgen: add script pktgen_sample06_numa_awared_queue_irq_affinity.sh
From: Robert Hoo <robert...@intel.com> This script simply does: * Detect $DEV's NUMA node belonging. * Bind each thread (processor of NUMA locality) with each $DEV queue's irq affinity, 1:1 mapping. * How many '-t' threads input determines how many queues will be utilized. If '-f' designates first cpu id, then offset in the NUMA node's cpu list. (Changes by Jesper: allow changing count from cmdline via '-n') Signed-off-by: Robert Hoo <robert...@intel.com> Signed-off-by: Jesper Dangaard Brouer <bro...@redhat.com> --- ...tgen_sample06_numa_awared_queue_irq_affinity.sh | 97 1 file changed, 97 insertions(+) create mode 100755 samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh diff --git a/samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh b/samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh new file mode 100755 index ..353adc17205e --- /dev/null +++ b/samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh @@ -0,0 +1,97 @@ +#!/bin/bash +# +# Multiqueue: Using pktgen threads for sending on multiple CPUs +# * adding devices to kernel threads which are in the same NUMA node +# * bound devices queue's irq affinity to the threads, 1:1 mapping +# * notice the naming scheme for keeping device names unique +# * nameing scheme: dev@thread_number +# * flow variation via random UDP source port +# +basedir=`dirname $0` +source ${basedir}/functions.sh +root_check_run_with_sudo "$@" +# +# Required param: -i dev in $DEV +source ${basedir}/parameters.sh + +# Base Config +DELAY="0"# Zero means max speed +[ -z "$COUNT" ] && COUNT="2000" # Zero means indefinitely +[ -z "$CLONE_SKB" ] && CLONE_SKB="0" + +# Flow variation random source port between min and max +UDP_MIN=9 +UDP_MAX=109 + +node=`get_iface_node $DEV` +irq_array=(`get_iface_irqs $DEV`) +cpu_array=(`get_node_cpus $node`) + +[ $THREADS -gt ${#irq_array[*]} -o $THREADS -gt ${#cpu_array[*]} ] && \ + err 1 "Thread number $THREADS exceeds: min (${#irq_array[*]},${#cpu_array[*]})" + +# (example of setting default params in your script) +if [ -z "$DEST_IP" ]; then +[ -z "$IP6" ] && DEST_IP="198.18.0.42" || DEST_IP="FD00::1" +fi +[ -z "$DST_MAC" ] && DST_MAC="90:e2:ba:ff:ff:ff" + +# General cleanup everything since last run +pg_ctrl "reset" + +# Threads are specified with parameter -t value in $THREADS +for ((i = 0; i < $THREADS; i++)); do +# The device name is extended with @name, using thread number to +# make then unique, but any name will do. +# Set the queue's irq affinity to this $thread (processor) +# if '-f' is designated, offset cpu id +thread=${cpu_array[$((i+F_THREAD))]} +dev=${DEV}@${thread} +echo $thread > /proc/irq/${irq_array[$i]}/smp_affinity_list +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" +pg_thread $thread "add_device" $dev + +# select queue and bind the queue and $dev in 1:1 relationship +queue_num=$i +info "queue number is $queue_num" +pg_set $dev "queue_map_min $queue_num" +pg_set $dev "queue_map_max $queue_num" + +# Notice config queue to map to cpu (mirrors smp_processor_id()) +# It is beneficial to map IRQ /proc/irq/*/smp_affinity 1:1 to CPU number +pg_set $dev "flag QUEUE_MAP_CPU" + +# Base config of dev +pg_set $dev "count $COUNT" +pg_set $dev "clone_skb $CLONE_SKB" +pg_set $dev "pkt_size $PKT_SIZE" +pg_set $dev "delay $DELAY" + +# Flag example disabling timestamping +pg_set $dev "flag NO_TIMESTAMP" + +# Destination +pg_set $dev "dst_mac $DST_MAC" +pg_set $dev "dst$IP6 $DEST_IP" + +# Setup random UDP port src range +pg_set $dev "flag UDPSRC_RND" +pg_set $dev "udp_src_min $UDP_MIN" +pg_set $dev "udp_src_max $UDP_MAX" +done + +# start_run +echo "Running... ctrl^C to stop" >&2 +pg_ctrl "start" +echo "Done" >&2 + +# Print results +for ((i = 0; i < $THREADS; i++)); do +thread=${cpu_array[$((i+F_THREAD))]} +dev=${DEV}@${thread} +echo "Device: $dev" +cat /proc/net/pktgen/$dev | grep -A2 "Result:" +done
[net-next PATCH 3/4] samples/pktgen: update sample03, no need for clones when bursting
Like sample05, don't use pktgen clone_skb feature when using 'burst' feature, it is not really needed. This brings the burst users in sync. Signed-off-by: Jesper Dangaard Brouer <bro...@redhat.com> --- .../pktgen/pktgen_sample03_burst_single_flow.sh|2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/pktgen/pktgen_sample03_burst_single_flow.sh b/samples/pktgen/pktgen_sample03_burst_single_flow.sh index 8d26e0ca683d..8a46daf27966 100755 --- a/samples/pktgen/pktgen_sample03_burst_single_flow.sh +++ b/samples/pktgen/pktgen_sample03_burst_single_flow.sh @@ -30,7 +30,7 @@ if [ -z "$DEST_IP" ]; then fi [ -z "$DST_MAC" ] && DST_MAC="90:e2:ba:ff:ff:ff" [ -z "$BURST" ] && BURST=32 -[ -z "$CLONE_SKB" ] && CLONE_SKB="10" +[ -z "$CLONE_SKB" ] && CLONE_SKB="0" # No need for clones when bursting [ -z "$COUNT" ] && COUNT="0" # Zero means indefinitely # Base Config
Re: [pktgen script v2 0/2] Add a pktgen sample script of NUMA awareness
On Mon, 18 Sep 2017 11:06:21 +0200 Jesper Dangaard Brouer <bro...@redhat.com> wrote: > On Sun, 17 Sep 2017 20:36:36 +0800 Robert Hoo <robert...@linux.intel.com> > wrote: > > > Change log > > v2: > > Rebased to > > https://github.com/netoptimizer/network-testing/tree/master/pktgen > > Hi Robert, > > Thank you for submitting this against my git tree[1]. I skimmed the > patches and they looked okay. I'll give them a test run, before I > accept them into my tree. > > Later I'll synchronize my pktgen scripts/git-tree with the kernel via > regular patches against DaveM's net-next tree[2] (and I'll try to > remember to give you author credit). > > [1] https://github.com/netoptimizer/network-testing/tree/master/pktgen > [2] > https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/tree/samples/pktgen FYI, I've applied and pushed these patches to my tree. https://github.com/netoptimizer/network-testing/commits?author=robert-hoo https://github.com/netoptimizer/network-testing/commit/1b9b4b797a4f112 https://github.com/netoptimizer/network-testing/commit/65efc2352f63dde https://github.com/netoptimizer/network-testing/commit/54eb5178aaf4031 I fixed the description a bit, and only made one simple change: https://github.com/netoptimizer/network-testing/commit/9ff58568b3f8c91 Thanks for working on improving the pktgen scripts :-) -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer
Re: [pktgen script v2 0/2] Add a pktgen sample script of NUMA awareness
On Sun, 17 Sep 2017 20:36:36 +0800 Robert Hoo <robert...@linux.intel.com> wrote: > Change log > v2: > Rebased to https://github.com/netoptimizer/network-testing/tree/master/pktgen Hi Robert, Thank you for submitting this against my git tree[1]. I skimmed the patches and they looked okay. I'll give them a test run, before I accept them into my tree. Later I'll synchronize my pktgen scripts/git-tree with the kernel via regular patches against DaveM's net-next tree[2] (and I'll try to remember to give you author credit). [1] https://github.com/netoptimizer/network-testing/tree/master/pktgen [2] https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/tree/samples/pktgen -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer
[pktgen script v2 1/2] Add some helper functions
From: Robert Hoo <robert...@intel.com> 1. given a device, get its NUMA belongings 2. given a device, get its queues' irq numbers. 3. given a NUMA node, get its cpu id list. Signed-off-by: Robert Hoo <robert...@intel.com> --- pktgen/functions.sh | 44 1 file changed, 44 insertions(+) diff --git a/pktgen/functions.sh b/pktgen/functions.sh index 205e4cd..09dfe7a 100644 --- a/pktgen/functions.sh +++ b/pktgen/functions.sh @@ -119,3 +119,47 @@ function root_check_run_with_sudo() { err 4 "cannot perform sudo run of $0" fi } + +# Exact input device's NUMA node info +function get_iface_node() +{ +local node=$(
[pktgen script v2 2/2] Add pktgen script: pktgen_sample06_numa_awared_queue_irq_affinity.sh
From: Robert Hoo <robert...@intel.com> This script simply does: Detect $DEV's NUMA node belonging. Bind each thread (processor of NUMA locality) with each $DEV queue's irq affinity, 1:1 mapping. How many '-t' threads input determines how many queues will be utilized. If '-f' designates first cpu id, then offset in the NUMA node's cpu list. Signed-off-by: Robert Hoo <robert...@intel.com> --- ...tgen_sample06_numa_awared_queue_irq_affinity.sh | 97 ++ 1 file changed, 97 insertions(+) create mode 100755 pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh diff --git a/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh b/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh new file mode 100755 index 000..52da0f4 --- /dev/null +++ b/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh @@ -0,0 +1,97 @@ +#!/bin/bash +# +# Multiqueue: Using pktgen threads for sending on multiple CPUs +# * adding devices to kernel threads which are in the same NUMA node +# * bound devices queue's irq affinity to the threads, 1:1 mapping +# * notice the naming scheme for keeping device names unique +# * nameing scheme: dev@thread_number +# * flow variation via random UDP source port +# +basedir=`dirname $0` +source ${basedir}/functions.sh +root_check_run_with_sudo "$@" +# +# Required param: -i dev in $DEV +source ${basedir}/parameters.sh + +# Base Config +DELAY="0"# Zero means max speed +COUNT="2000" # Zero means indefinitely +[ -z "$CLONE_SKB" ] && CLONE_SKB="0" + +# Flow variation random source port between min and max +UDP_MIN=9 +UDP_MAX=109 + +node=`get_iface_node $DEV` +irq_array=(`get_iface_irqs $DEV`) +cpu_array=(`get_node_cpus $node`) + +[ $THREADS -gt ${#irq_array[*]} -o $THREADS -gt ${#cpu_array[*]} ] && \ + err 1 "Thread number $THREADS exceeds: min (${#irq_array[*]},${#cpu_array[*]})" + +# (example of setting default params in your script) +if [ -z "$DEST_IP" ]; then +[ -z "$IP6" ] && DEST_IP="198.18.0.42" || DEST_IP="FD00::1" +fi +[ -z "$DST_MAC" ] && DST_MAC="90:e2:ba:ff:ff:ff" + +# General cleanup everything since last run +pg_ctrl "reset" + +# Threads are specified with parameter -t value in $THREADS +for ((i = 0; i < $THREADS; i++)); do +# The device name is extended with @name, using thread number to +# make then unique, but any name will do. +# Set the queue's irq affinity to this $thread (processor) +# if '-f' is designated, offset cpu id +thread=${cpu_array[$((i+F_THREAD))]} +dev=${DEV}@${thread} +echo $thread > /proc/irq/${irq_array[$i]}/smp_affinity_list +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" +pg_thread $thread "add_device" $dev + +# select queue and bind the queue and $dev in 1:1 relationship +queue_num=$i +info "queue number is $queue_num" +pg_set $dev "queue_map_min $queue_num" +pg_set $dev "queue_map_max $queue_num" + +# Notice config queue to map to cpu (mirrors smp_processor_id()) +# It is beneficial to map IRQ /proc/irq/*/smp_affinity 1:1 to CPU number +pg_set $dev "flag QUEUE_MAP_CPU" + +# Base config of dev +pg_set $dev "count $COUNT" +pg_set $dev "clone_skb $CLONE_SKB" +pg_set $dev "pkt_size $PKT_SIZE" +pg_set $dev "delay $DELAY" + +# Flag example disabling timestamping +pg_set $dev "flag NO_TIMESTAMP" + +# Destination +pg_set $dev "dst_mac $DST_MAC" +pg_set $dev "dst$IP6 $DEST_IP" + +# Setup random UDP port src range +pg_set $dev "flag UDPSRC_RND" +pg_set $dev "udp_src_min $UDP_MIN" +pg_set $dev "udp_src_max $UDP_MAX" +done + +# start_run +echo "Running... ctrl^C to stop" >&2 +pg_ctrl "start" +echo "Done" >&2 + +# Print results +for ((i = 0; i < $THREADS; i++)); do +thread=${cpu_array[$((i+F_THREAD))]} +dev=${DEV}@${thread} +echo "Device: $dev" +cat /proc/net/pktgen/$dev | grep -A2 "Result:" +done -- 1.8.3.1
[pktgen script v2 0/2] Add a pktgen sample script of NUMA awareness
From: Robert Hoo <robert...@intel.com> It's hard to benchmark 40G+ network bandwidth using ordinary tools like iperf, netperf (see reference 1). Pktgen, packet generator from Kernel sapce, shall be a candidate. I derived this NUMA awared irq affinity sample script from multi-queue sample02, successfully benchmarked 40G link. I think this can also be useful for 100G reference, though I haven't got device to test yet. This script simply does: Detect $DEV's NUMA node belonging. Bind each thread (processor of NUMA locality) with each $DEV queue's irq affinity, 1:1 mapping. How many '-t' threads input determines how many queues will be utilized. If '-f' designates first cpu id, then offset in the NUMA node's cpu list. Tested with Intel XL710 NIC with Cisco 3172 switch. Referrences: https://people.netfilter.org/hawk/presentations/LCA2015/net_stack_challenges_100G_LCA2015.pdf http://www.intel.cn/content/dam/www/public/us/en/documents/reference-guides/xl710-x710-performance-tuning-linux-guide.pdf Change log v2: Rebased to https://github.com/netoptimizer/network-testing/tree/master/pktgen Move helper functions to functions.sh More concise shell grammar usage Take '-f' parameter into consideration. If the first CPU is designaed, offset in the NUMA-aware CPU list. Use err(), info() helper functions for such outputs. Robert Hoo (2): Add some helper functions Add pktgen script: pktgen_sample06_numa_awared_queue_irq_affinity.sh pktgen/functions.sh| 44 ++ ...tgen_sample06_numa_awared_queue_irq_affinity.sh | 97 ++ 2 files changed, 141 insertions(+) create mode 100755 pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh -- 1.8.3.1
Re: [PATCH] pktgen: add a new sample script for 40G and above link testing
On Fri, 2017-08-25 at 14:24 +, Waskiewicz Jr, Peter wrote: > On 8/25/17 5:19 AM, Jesper Dangaard Brouer wrote: > >> > >> Tested with Intel XL710 NIC with Cisco 3172 switch. > >> > >> It would be even slightly better if the irqbalance service is turned > >> off outside. > > > > Yes, if you don't turn-off (kill) irqbalance it will move around the > > IRQs behind your back... > > Or you can use the --banirq option to irqbalance to ignore your device's > interrupts as targets for balancing. Oh, I wasn't aware of this parameter. I will be glad to have try later. Meanwhile, in my test above, the irqbalance service just affect result very little. > > Cheers, > -PJ
Re: [PATCH] pktgen: add a new sample script for 40G and above link testing
On Sun, 2017-08-27 at 11:25 +0300, Tariq Toukan wrote: > > On 25/08/2017 12:26 PM, Robert Hoo wrote: > > (Sorry for yesterday's wrong sending, I finally fixed my MTA and git > > send-email settings.) > > > > It's hard to benchmark 40G+ network bandwidth using ordinary > > tools like iperf, netperf (see reference 1). > > Pktgen, packet generator from Kernel sapce, shall be a candidate. > > I then tried with pktgen multiqueue sample scripts, but still > > cannot reach line rate. > > Try samples 03 and 04. Thanks Tariq for review. Sorry for late reply; I do this part time. Yes, I just tried sample 03 and 04. They can approximately reach 40G line rate; though still slightly less than my script :) (see my reply to Jesper). > > > I then derived this NUMA awared irq affinity sample script from > > multi-queue sample one, successfully benchmarked 40G link. I think this can > > also be useful for 100G reference, though I haven't got device to test yet. > > > > This script simply does: > > Detect $DEV's NUMA node belonging. > > Bind each thread (processor from that NUMA node) with each $DEV queue's > > irq affinity, 1:1 mapping. > > How many '-t' threads input determines how many queues will be > > utilized. > > I agree this is an essential capability. > This was the main reason I added support for the -f argument. > Using it, I could choose cores of local NUMA, especially for single > thread, or when cores of the NUMA are sequential. Indeed this argument is very helpful. Sorry I haven't taken it into consideration in v1. I should consider this, what if user designate '-f'. I can improve this in v2. > > > > > Tested with Intel XL710 NIC with Cisco 3172 switch. > > > > It would be even slightly better if the irqbalance service is turned > > off outside. > > > > Referrences: > > https://people.netfilter.org/hawk/presentations/LCA2015/net_stack_challenges_100G_LCA2015.pdf > > http://www.intel.cn/content/dam/www/public/us/en/documents/reference-guides/xl710-x710-performance-tuning-linux-guide.pdf > > > > Signed-off-by: Robert Hoo <robert...@linux.intel.com> > > --- > > Regards, > Tariq Toukan
Re: [PATCH] pktgen: add a new sample script for 40G and above link testing
On Fri, 2017-08-25 at 11:19 +0200, Jesper Dangaard Brouer wrote: > (please don't use BCC on the netdev list, replies might miss the list in cc) > > Comments inlined below: > > On Fri, 25 Aug 2017 10:24:30 +0800 Robert Hoo <robert...@intel.com> wrote: > > > From: Robert Ho <robert...@intel.com> > > > > It's hard to benchmark 40G+ network bandwidth using ordinary > > tools like iperf, netperf. I then tried with pktgen multiqueue sample > > scripts, but still cannot reach line rate. > > The pktgen_sample02_multiqueue.sh does not use burst or skb_cloning. > Thus, the performance will suffer. > > See the samples that use the burst feature: > pktgen_sample03_burst_single_flow.sh > pktgen_sample05_flow_per_thread.sh > > With the pktgen "burst" feature, I can easily generate 40G. Generating > 100G is also possible, but often you will hit some HW limits before the > pktgen limit. I experienced hitting both (1) PCIe Gen3 x8 limit, and (2) > memory bandwidth limit. Thanks Jesper for review. Sorry for late reply, I do this part time. I just tried 'pktgen_sample03_burst_single_flow.sh' and 'pktgen_sample05_flow_per_thread.sh' cmd: ./pktgen_sample05_flow_per_thread.sh -i ens801 -s 1500 -m 3c:fd:fe:9d:6f:f0 -t 2 -v -x -d 192.168.0.107 ./pktgen_sample03_burst_single_flow.sh -i ens801 -s 1500 -m 3c:fd:fe:9d:6f:f0 -t 2 -v -x -d 192.168.0.107 indeed, they can achieve nearly 40G. (though still slightly less than my script). pktgen_sample03 and pktgen_sample05 can approximately achieve 38xxxMb/sec ~ 39xxxMb/sec; my script can achieve 40xxxMb/sec ~ 41xxxMb/sec. (threads >= 2) So a general question: is it still necessary to continue my sample06_numa_awared_queue_irq_affinity work? as sample03 and sample05 already approximately achieved 40G line rate. > > > > I then derived this NUMA awared irq affinity sample script from > > multi-queue sample one, successfully benchmarked 40G link. I think this can > > also be useful for 100G reference, though I haven't got device to test. > > Okay, so your issue was really related to NUMA irq affinity. I do feel > that IRQ tuning lives outside the realm of the pktgen scripts, but > looking closer at your script, I it doesn't look like you change the > IRQ setting which is good. Sorry I don't quite understand above. I changed the irq affinities. See "echo $thread > /proc/irq/${irq_array[$i]}/smp_affinity_list". You would not like me to change it? I can restore them to original at the end of the script. > > You introduce some helper functions take makes it possible to extract > NUMA information in the shell script code, really cool. I would like > to see these functions being integrated into the function.sh file. Yes, it is doable, if you maintainer think so. > > > > This script simply does: > > Detect $DEV's NUMA node belonging. > > Bind each thread (processor from that NUMA node) with each $DEV queue's > > irq affinity, 1:1 mapping. > > How many '-t' threads input determines how many queues will be > > utilized. > > > > Tested with Intel XL710 NIC with Cisco 3172 switch. > > > > It would be even slightly better if the irqbalance service is turned > > off outside. > > Yes, if you don't turn-off (kill) irqbalance it will move around the > IRQs behind your back... Yes; while the experiment result turns out it affects just very little. > > > > Referrences: > > https://people.netfilter.org/hawk/presentations/LCA2015/net_stack_challenges_100G_LCA2015.pdf > > http://www.intel.cn/content/dam/www/public/us/en/documents/reference-guides/xl710-x710-performance-tuning-linux-guide.pdf > > > > Signed-off-by: Robert Hoo <robert...@intel.com> > > --- > > ...tgen_sample06_numa_awared_queue_irq_affinity.sh | 132 > > + > > 1 file changed, 132 insertions(+) > > create mode 100755 > > samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh > > > > diff --git > > a/samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh > > b/samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh > > new file mode 100755 > > index 000..f0ee25c > > --- /dev/null > > +++ b/samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh > > @@ -0,0 +1,132 @@ > > +#!/bin/bash > > +# > > +# Multiqueue: Using pktgen threads for sending on multiple CPUs > > +# * adding devices to kernel threads which are in the same NUMA node > > +# * bound devices queue's irq affinity to the threads, 1:1 mapping > > +# * notice the naming scheme for keeping device names unique > > +# * nameing scheme:
Re: [PATCH] pktgen: add a new sample script for 40G and above link testing
On 25/08/2017 12:26 PM, Robert Hoo wrote: (Sorry for yesterday's wrong sending, I finally fixed my MTA and git send-email settings.) It's hard to benchmark 40G+ network bandwidth using ordinary tools like iperf, netperf (see reference 1). Pktgen, packet generator from Kernel sapce, shall be a candidate. I then tried with pktgen multiqueue sample scripts, but still cannot reach line rate. Try samples 03 and 04. I then derived this NUMA awared irq affinity sample script from multi-queue sample one, successfully benchmarked 40G link. I think this can also be useful for 100G reference, though I haven't got device to test yet. This script simply does: Detect $DEV's NUMA node belonging. Bind each thread (processor from that NUMA node) with each $DEV queue's irq affinity, 1:1 mapping. How many '-t' threads input determines how many queues will be utilized. I agree this is an essential capability. This was the main reason I added support for the -f argument. Using it, I could choose cores of local NUMA, especially for single thread, or when cores of the NUMA are sequential. Tested with Intel XL710 NIC with Cisco 3172 switch. It would be even slightly better if the irqbalance service is turned off outside. Referrences: https://people.netfilter.org/hawk/presentations/LCA2015/net_stack_challenges_100G_LCA2015.pdf http://www.intel.cn/content/dam/www/public/us/en/documents/reference-guides/xl710-x710-performance-tuning-linux-guide.pdf Signed-off-by: Robert Hoo <robert...@linux.intel.com> --- Regards, Tariq Toukan
Re: [PATCH] pktgen: add a new sample script for 40G and above link testing
On 8/25/17 10:59 AM, Jesper Dangaard Brouer wrote: > On Fri, 25 Aug 2017 14:24:28 + > "Waskiewicz Jr, Peter"wrote: > >> On 8/25/17 5:19 AM, Jesper Dangaard Brouer wrote: Tested with Intel XL710 NIC with Cisco 3172 switch. It would be even slightly better if the irqbalance service is turned off outside. >>> >>> Yes, if you don't turn-off (kill) irqbalance it will move around the >>> IRQs behind your back... >> >> Or you can use the --banirq option to irqbalance to ignore your device's >> interrupts as targets for balancing. > > It might be worth mentioning that --banirq=X is specified for each IRQ > that you want to exclude, and --banirq is simply specified multiple > times on the command line. > > Is it possible to tell a running irqbalance that I want to excluded an > extra IRQ? (just before I do my manual adjustment). It isn't possible today, since we don't have a way to attach a foreground/oneshot irqbalance run to a currently-running daemon. That's an interesting feature enhancement...I can add it to our list as a feature request so I don't forget about it. That way I can also get Neil's thoughts on this. -PJ
Re: [PATCH] pktgen: add a new sample script for 40G and above link testing
On Fri, 25 Aug 2017 14:24:28 + "Waskiewicz Jr, Peter"wrote: > On 8/25/17 5:19 AM, Jesper Dangaard Brouer wrote: > >> > >> Tested with Intel XL710 NIC with Cisco 3172 switch. > >> > >> It would be even slightly better if the irqbalance service is turned > >> off outside. > > > > Yes, if you don't turn-off (kill) irqbalance it will move around the > > IRQs behind your back... > > Or you can use the --banirq option to irqbalance to ignore your device's > interrupts as targets for balancing. It might be worth mentioning that --banirq=X is specified for each IRQ that you want to exclude, and --banirq is simply specified multiple times on the command line. Is it possible to tell a running irqbalance that I want to excluded an extra IRQ? (just before I do my manual adjustment). -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer
Re: [PATCH] pktgen: add a new sample script for 40G and above link testing
On 8/25/17 5:19 AM, Jesper Dangaard Brouer wrote: >> >> Tested with Intel XL710 NIC with Cisco 3172 switch. >> >> It would be even slightly better if the irqbalance service is turned >> off outside. > > Yes, if you don't turn-off (kill) irqbalance it will move around the > IRQs behind your back... Or you can use the --banirq option to irqbalance to ignore your device's interrupts as targets for balancing. Cheers, -PJ
Re: [PATCH] pktgen: add a new sample script for 40G and above link testing
On Fri, 25 Aug 2017 17:26:36 +0800 Robert Hoowrote: > (Sorry for yesterday's wrong sending, I finally fixed my MTA and git > send-email settings.) Please see my reply in: http://lkml.kernel.org/r/20170825111921.06171...@redhat.com -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer
[PATCH] pktgen: add a new sample script for 40G and above link testing
(Sorry for yesterday's wrong sending, I finally fixed my MTA and git send-email settings.) It's hard to benchmark 40G+ network bandwidth using ordinary tools like iperf, netperf (see reference 1). Pktgen, packet generator from Kernel sapce, shall be a candidate. I then tried with pktgen multiqueue sample scripts, but still cannot reach line rate. I then derived this NUMA awared irq affinity sample script from multi-queue sample one, successfully benchmarked 40G link. I think this can also be useful for 100G reference, though I haven't got device to test yet. This script simply does: Detect $DEV's NUMA node belonging. Bind each thread (processor from that NUMA node) with each $DEV queue's irq affinity, 1:1 mapping. How many '-t' threads input determines how many queues will be utilized. Tested with Intel XL710 NIC with Cisco 3172 switch. It would be even slightly better if the irqbalance service is turned off outside. Referrences: https://people.netfilter.org/hawk/presentations/LCA2015/net_stack_challenges_100G_LCA2015.pdf http://www.intel.cn/content/dam/www/public/us/en/documents/reference-guides/xl710-x710-performance-tuning-linux-guide.pdf Signed-off-by: Robert Hoo <robert...@linux.intel.com> --- ...tgen_sample06_numa_awared_queue_irq_affinity.sh | 132 + 1 file changed, 132 insertions(+) create mode 100755 samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh diff --git a/samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh b/samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh new file mode 100755 index 000..f0ee25c --- /dev/null +++ b/samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh @@ -0,0 +1,132 @@ +#!/bin/bash +# +# Multiqueue: Using pktgen threads for sending on multiple CPUs +# * adding devices to kernel threads which are in the same NUMA node +# * bound devices queue's irq affinity to the threads, 1:1 mapping +# * notice the naming scheme for keeping device names unique +# * nameing scheme: dev@thread_number +# * flow variation via random UDP source port +# +basedir=`dirname $0` +source ${basedir}/functions.sh +root_check_run_with_sudo "$@" +# +# Required param: -i dev in $DEV +source ${basedir}/parameters.sh + +get_iface_node() +{ + echo `cat /sys/class/net/$1/device/numa_node` +} + +get_iface_irqs() +{ + local IFACE=$1 + local queues="${IFACE}-.*TxRx" + + irqs=$(grep "$queues" /proc/interrupts | cut -f1 -d:) + [ -z "$irqs" ] && irqs=$(grep $IFACE /proc/interrupts | cut -f1 -d:) + [ -z "$irqs" ] && irqs=$(for i in `ls -Ux /sys/class/net/$IFACE/device/msi_irqs` ;\ + do grep "$i:.*TxRx" /proc/interrupts | grep -v fdir | cut -f 1 -d : ;\ + done) + [ -z "$irqs" ] && echo "Error: Could not find interrupts for $IFACE" + + echo $irqs +} + +get_node_cpus() +{ + local node=$1 + local node_cpu_list + local node_cpu_range_list=`cut -f1- -d, --output-delimiter=" " \ + /sys/devices/system/node/node$node/cpulist` + + for cpu_range in $node_cpu_range_list + do + node_cpu_list="$node_cpu_list "`seq -s " " ${cpu_range//-/ }` + done + + echo $node_cpu_list +} + + +# Base Config +DELAY="0"# Zero means max speed +COUNT="2000" # Zero means indefinitely +[ -z "$CLONE_SKB" ] && CLONE_SKB="0" + +# Flow variation random source port between min and max +UDP_MIN=9 +UDP_MAX=109 + +node=`get_iface_node $DEV` +irq_array=(`get_iface_irqs $DEV`) +cpu_array=(`get_node_cpus $node`) + +[ $THREADS -gt ${#irq_array[*]} -o $THREADS -gt ${#cpu_array[*]} ] && \ + err 1 "Thread number $THREADS exceeds: min (${#irq_array[*]},${#cpu_array[*]})" + +# (example of setting default params in your script) +if [ -z "$DEST_IP" ]; then +[ -z "$IP6" ] && DEST_IP="198.18.0.42" || DEST_IP="FD00::1" +fi +[ -z "$DST_MAC" ] && DST_MAC="90:e2:ba:ff:ff:ff" + +# General cleanup everything since last run +pg_ctrl "reset" + +# Threads are specified with parameter -t value in $THREADS +for ((i = 0; i < $THREADS; i++)); do +# The device name is extended with @name, using thread number to +# make then unique, but any name will do. +# Set the queue's irq affinity to this $thread (processor) +thread=${cpu_array[$i]} +dev=${DEV}@${thread} +echo $thread > /proc/irq/${irq_array[$i]}/smp_affinity_list +echo "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" +pg_thread $thread "add_device" $dev + +
Re: [PATCH] pktgen: add a new sample script for 40G and above link testing
(please don't use BCC on the netdev list, replies might miss the list in cc) Comments inlined below: On Fri, 25 Aug 2017 10:24:30 +0800 Robert Hoo <robert...@intel.com> wrote: > From: Robert Ho <robert...@intel.com> > > It's hard to benchmark 40G+ network bandwidth using ordinary > tools like iperf, netperf. I then tried with pktgen multiqueue sample > scripts, but still cannot reach line rate. The pktgen_sample02_multiqueue.sh does not use burst or skb_cloning. Thus, the performance will suffer. See the samples that use the burst feature: pktgen_sample03_burst_single_flow.sh pktgen_sample05_flow_per_thread.sh With the pktgen "burst" feature, I can easily generate 40G. Generating 100G is also possible, but often you will hit some HW limits before the pktgen limit. I experienced hitting both (1) PCIe Gen3 x8 limit, and (2) memory bandwidth limit. > I then derived this NUMA awared irq affinity sample script from > multi-queue sample one, successfully benchmarked 40G link. I think this can > also be useful for 100G reference, though I haven't got device to test. Okay, so your issue was really related to NUMA irq affinity. I do feel that IRQ tuning lives outside the realm of the pktgen scripts, but looking closer at your script, I it doesn't look like you change the IRQ setting which is good. You introduce some helper functions take makes it possible to extract NUMA information in the shell script code, really cool. I would like to see these functions being integrated into the function.sh file. > This script simply does: > Detect $DEV's NUMA node belonging. > Bind each thread (processor from that NUMA node) with each $DEV queue's > irq affinity, 1:1 mapping. > How many '-t' threads input determines how many queues will be > utilized. > > Tested with Intel XL710 NIC with Cisco 3172 switch. > > It would be even slightly better if the irqbalance service is turned > off outside. Yes, if you don't turn-off (kill) irqbalance it will move around the IRQs behind your back... > Referrences: > https://people.netfilter.org/hawk/presentations/LCA2015/net_stack_challenges_100G_LCA2015.pdf > http://www.intel.cn/content/dam/www/public/us/en/documents/reference-guides/xl710-x710-performance-tuning-linux-guide.pdf > > Signed-off-by: Robert Hoo <robert...@intel.com> > --- > ...tgen_sample06_numa_awared_queue_irq_affinity.sh | 132 > + > 1 file changed, 132 insertions(+) > create mode 100755 > samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh > > diff --git a/samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh > b/samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh > new file mode 100755 > index 000..f0ee25c > --- /dev/null > +++ b/samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh > @@ -0,0 +1,132 @@ > +#!/bin/bash > +# > +# Multiqueue: Using pktgen threads for sending on multiple CPUs > +# * adding devices to kernel threads which are in the same NUMA node > +# * bound devices queue's irq affinity to the threads, 1:1 mapping > +# * notice the naming scheme for keeping device names unique > +# * nameing scheme: dev@thread_number > +# * flow variation via random UDP source port > +# > +basedir=`dirname $0` > +source ${basedir}/functions.sh > +root_check_run_with_sudo "$@" > +# > +# Required param: -i dev in $DEV > +source ${basedir}/parameters.sh > + > +get_iface_node() > +{ > + echo `cat /sys/class/net/$1/device/numa_node` Here you could use the following shell trick to avoid using "cat": echo $( +} > + > +get_iface_irqs() > +{ > + local IFACE=$1 > + local queues="${IFACE}-.*TxRx" > + > + irqs=$(grep "$queues" /proc/interrupts | cut -f1 -d:) > + [ -z "$irqs" ] && irqs=$(grep $IFACE /proc/interrupts | cut -f1 -d:) > + [ -z "$irqs" ] && irqs=$(for i in `ls -Ux > /sys/class/net/$IFACE/device/msi_irqs` ;\ > + do grep "$i:.*TxRx" /proc/interrupts | grep -v fdir | cut -f 1 > -d : ;\ > + done) Nice that you handle all these different methods. I personally look in /proc/irq/*/$IFACE*/../smp_affinity_list , like (copy-paste): echo " --- Align IRQs ---" # I've named my NICs ixgbe1 + ixgbe2 for F in /proc/irq/*/ixgbe*-TxRx-*/../smp_affinity_list; do # Extract irqname e.g. "ixgbe2-TxRx-2" irqname=$(basename $(dirname $(dirname $F))) ; # Substring pattern removal hwq_nr=${irqname#*-*-} echo $hwq_nr > $F #grep . -H $F; done grep -H . /proc/irq/*/ixgbe*/../smp_affinity_list Maybe I should switch to use: /sys/class/net/$IFACE/device/msi_irqs/* > + [ -z "$irqs" ] && echo "Error:
[PATCH] pktgen: add a new sample script for 40G and above link testing
From: Robert Ho <robert...@intel.com> It's hard to benchmark 40G+ network bandwidth using ordinary tools like iperf, netperf. I then tried with pktgen multiqueue sample scripts, but still cannot reach line rate. I then derived this NUMA awared irq affinity sample script from multi-queue sample one, successfully benchmarked 40G link. I think this can also be useful for 100G reference, though I haven't got device to test. This script simply does: Detect $DEV's NUMA node belonging. Bind each thread (processor from that NUMA node) with each $DEV queue's irq affinity, 1:1 mapping. How many '-t' threads input determines how many queues will be utilized. Tested with Intel XL710 NIC with Cisco 3172 switch. It would be even slightly better if the irqbalance service is turned off outside. Referrences: https://people.netfilter.org/hawk/presentations/LCA2015/net_stack_challenges_100G_LCA2015.pdf http://www.intel.cn/content/dam/www/public/us/en/documents/reference-guides/xl710-x710-performance-tuning-linux-guide.pdf Signed-off-by: Robert Hoo <robert...@intel.com> --- ...tgen_sample06_numa_awared_queue_irq_affinity.sh | 132 + 1 file changed, 132 insertions(+) create mode 100755 samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh diff --git a/samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh b/samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh new file mode 100755 index 000..f0ee25c --- /dev/null +++ b/samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh @@ -0,0 +1,132 @@ +#!/bin/bash +# +# Multiqueue: Using pktgen threads for sending on multiple CPUs +# * adding devices to kernel threads which are in the same NUMA node +# * bound devices queue's irq affinity to the threads, 1:1 mapping +# * notice the naming scheme for keeping device names unique +# * nameing scheme: dev@thread_number +# * flow variation via random UDP source port +# +basedir=`dirname $0` +source ${basedir}/functions.sh +root_check_run_with_sudo "$@" +# +# Required param: -i dev in $DEV +source ${basedir}/parameters.sh + +get_iface_node() +{ + echo `cat /sys/class/net/$1/device/numa_node` +} + +get_iface_irqs() +{ + local IFACE=$1 + local queues="${IFACE}-.*TxRx" + + irqs=$(grep "$queues" /proc/interrupts | cut -f1 -d:) + [ -z "$irqs" ] && irqs=$(grep $IFACE /proc/interrupts | cut -f1 -d:) + [ -z "$irqs" ] && irqs=$(for i in `ls -Ux /sys/class/net/$IFACE/device/msi_irqs` ;\ + do grep "$i:.*TxRx" /proc/interrupts | grep -v fdir | cut -f 1 -d : ;\ + done) + [ -z "$irqs" ] && echo "Error: Could not find interrupts for $IFACE" + + echo $irqs +} + +get_node_cpus() +{ + local node=$1 + local node_cpu_list + local node_cpu_range_list=`cut -f1- -d, --output-delimiter=" " \ + /sys/devices/system/node/node$node/cpulist` + + for cpu_range in $node_cpu_range_list + do + node_cpu_list="$node_cpu_list "`seq -s " " ${cpu_range//-/ }` + done + + echo $node_cpu_list +} + + +# Base Config +DELAY="0"# Zero means max speed +COUNT="2000" # Zero means indefinitely +[ -z "$CLONE_SKB" ] && CLONE_SKB="0" + +# Flow variation random source port between min and max +UDP_MIN=9 +UDP_MAX=109 + +node=`get_iface_node $DEV` +irq_array=(`get_iface_irqs $DEV`) +cpu_array=(`get_node_cpus $node`) + +[ $THREADS -gt ${#irq_array[*]} -o $THREADS -gt ${#cpu_array[*]} ] && \ + err 1 "Thread number $THREADS exceeds: min (${#irq_array[*]},${#cpu_array[*]})" + +# (example of setting default params in your script) +if [ -z "$DEST_IP" ]; then +[ -z "$IP6" ] && DEST_IP="198.18.0.42" || DEST_IP="FD00::1" +fi +[ -z "$DST_MAC" ] && DST_MAC="90:e2:ba:ff:ff:ff" + +# General cleanup everything since last run +pg_ctrl "reset" + +# Threads are specified with parameter -t value in $THREADS +for ((i = 0; i < $THREADS; i++)); do +# The device name is extended with @name, using thread number to +# make then unique, but any name will do. +# Set the queue's irq affinity to this $thread (processor) +thread=${cpu_array[$i]} +dev=${DEV}@${thread} +echo $thread > /proc/irq/${irq_array[$i]}/smp_affinity_list +echo "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" +pg_thread $thread "add_device" $dev + +# select queue and bind the queue and $dev in 1:1 relationship +queue_num=$i +echo "queue number is $queue_num" +pg_
Re: [PATCH net-next V2 0/2] pktgen new parameters
From: Tariq Toukan <tar...@mellanox.com> Date: Thu, 15 Jun 2017 19:07:20 +0300 > This patchset adds two parameters to the pktgen scripts. > * The first patch adds a parameter to control the number of > packets sent by every pktgen thread. > * The second patch adds a parameter to control the index of > first thread, instead of always starting from cpu 0. > > Series generated against net-next commit: > f7aec129a356 rxrpc: Cache the congestion window setting Series applied, thanks Tariq.
Re: [PATCH net-next V2 2/2] pktgen: Specify the index of first thread
On Thu, 15 Jun 2017 19:07:22 +0300 Tariq Toukanwrote: > Use "-f ", to specify the index of the first > sender thread. > In default first thread is #0. > > Signed-off-by: Tariq Toukan > Cc: Jesper Dangaard Brouer [...] > > +export L_THREAD=$(( THREADS + F_THREAD - 1 )) > + Acked-by: Jesper Dangaard Brouer -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer
Re: [PATCH net-next V2 1/2] pktgen: Specify num packets per thread
On Thu, 15 Jun 2017 19:07:21 +0300 Tariq Toukanwrote: > Use -n , to specify the number of packets every > thread sends. > Zero means indefinitely. > > Signed-off-by: Tariq Toukan > Cc: Jesper Dangaard Brouer Acked-by: Jesper Dangaard Brouer -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer
[PATCH net-next V2 0/2] pktgen new parameters
Hi Dave, This patchset adds two parameters to the pktgen scripts. * The first patch adds a parameter to control the number of packets sent by every pktgen thread. * The second patch adds a parameter to control the index of first thread, instead of always starting from cpu 0. Series generated against net-next commit: f7aec129a356 rxrpc: Cache the congestion window setting Thanks, Tariq. V2: * rebased to latest net-next. * per Jesper's comments on Patch 2, fixed $F_THREAD description and $L_THREAD evaluation. Tariq Toukan (2): pktgen: Specify num packets per thread pktgen: Specify the index of first thread samples/pktgen/README.rst | 2 ++ samples/pktgen/parameters.sh | 25 -- .../pktgen/pktgen_bench_xmit_mode_netif_receive.sh | 6 +++--- .../pktgen/pktgen_bench_xmit_mode_queue_xmit.sh| 6 +++--- samples/pktgen/pktgen_sample01_simple.sh | 2 +- samples/pktgen/pktgen_sample02_multiqueue.sh | 7 +++--- .../pktgen/pktgen_sample03_burst_single_flow.sh| 6 +++--- samples/pktgen/pktgen_sample04_many_flows.sh | 6 +++--- samples/pktgen/pktgen_sample05_flow_per_thread.sh | 6 +++--- 9 files changed, 41 insertions(+), 25 deletions(-) -- 1.8.3.1
[PATCH net-next V2 1/2] pktgen: Specify num packets per thread
Use -n , to specify the number of packets every thread sends. Zero means indefinitely. Signed-off-by: Tariq Toukan <tar...@mellanox.com> Cc: Jesper Dangaard Brouer <bro...@redhat.com> --- samples/pktgen/README.rst | 1 + samples/pktgen/parameters.sh | 7 ++- samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh | 2 +- samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh| 2 +- samples/pktgen/pktgen_sample01_simple.sh | 2 +- samples/pktgen/pktgen_sample02_multiqueue.sh | 3 ++- samples/pktgen/pktgen_sample03_burst_single_flow.sh| 2 +- samples/pktgen/pktgen_sample04_many_flows.sh | 2 +- samples/pktgen/pktgen_sample05_flow_per_thread.sh | 2 +- 9 files changed, 15 insertions(+), 8 deletions(-) diff --git a/samples/pktgen/README.rst b/samples/pktgen/README.rst index 8365c4e5c513..c018d67da1a1 100644 --- a/samples/pktgen/README.rst +++ b/samples/pktgen/README.rst @@ -22,6 +22,7 @@ across the sample scripts. Usage example is printed on errors:: -m : ($DST_MAC) destination MAC-addr -t : ($THREADS) threads to start -c : ($SKB_CLONE) SKB clones send before alloc new SKB + -n : ($COUNT) num messages to send per thread, 0 means indefinitely -b : ($BURST) HW level bursting of SKBs -v : ($VERBOSE) verbose -x : ($DEBUG) debug diff --git a/samples/pktgen/parameters.sh b/samples/pktgen/parameters.sh index f70ea7dd5660..036147594a20 100644 --- a/samples/pktgen/parameters.sh +++ b/samples/pktgen/parameters.sh @@ -11,6 +11,7 @@ function usage() { echo " -m : (\$DST_MAC) destination MAC-addr" echo " -t : (\$THREADS) threads to start" echo " -c : (\$SKB_CLONE) SKB clones send before alloc new SKB" +echo " -n : (\$COUNT) num messages to send per thread, 0 means indefinitely" echo " -b : (\$BURST) HW level bursting of SKBs" echo " -v : (\$VERBOSE) verbose" echo " -x : (\$DEBUG) debug" @@ -20,7 +21,7 @@ function usage() { ## --- Parse command line arguments / parameters --- ## echo "Commandline options:" -while getopts "s:i:d:m:t:c:b:vxh6" option; do +while getopts "s:i:d:m:t:c:n:b:vxh6" option; do case $option in i) # interface export DEV=$OPTARG @@ -48,6 +49,10 @@ while getopts "s:i:d:m:t:c:b:vxh6" option; do export CLONE_SKB=$OPTARG info "CLONE_SKB=$CLONE_SKB" ;; +n) + export COUNT=$OPTARG + info "COUNT=$COUNT" + ;; b) export BURST=$OPTARG info "SKB bursting: BURST=$BURST" diff --git a/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh b/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh index f3e1bedfd77f..d2694a12de61 100755 --- a/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh +++ b/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh @@ -39,10 +39,10 @@ if [ -z "$DEST_IP" ]; then fi [ -z "$DST_MAC" ] && DST_MAC="90:e2:ba:ff:ff:ff" [ -z "$BURST" ] && BURST=1024 +[ -z "$COUNT" ] && COUNT="1000" # Zero means indefinitely # Base Config DELAY="0"# Zero means max speed -COUNT="1000" # Zero means indefinitely # General cleanup everything since last run pg_ctrl "reset" diff --git a/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh b/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh index cc102e923241..43604c2db726 100755 --- a/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh +++ b/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh @@ -22,10 +22,10 @@ fi if [[ -n "$BURST" ]]; then err 1 "Bursting not supported for this mode" fi +[ -z "$COUNT" ] && COUNT="1000" # Zero means indefinitely # Base Config DELAY="0"# Zero means max speed -COUNT="1000" # Zero means indefinitely # General cleanup everything since last run pg_ctrl "reset" diff --git a/samples/pktgen/pktgen_sample01_simple.sh b/samples/pktgen/pktgen_sample01_simple.sh index 29ef4ba50796..35b7fe34bda2 100755 --- a/samples/pktgen/pktgen_sample01_simple.sh +++ b/samples/pktgen/pktgen_sample01_simple.sh @@ -20,10 +20,10 @@ fi [ -z "$CLONE_SKB" ] && CLONE_SKB="0" # Example enforce param "-m" for dst_mac [ -z "$DST_MAC" ] && usage && err 2 "Must specify -m dst_mac" +[ -z "$COUNT" ] && COUNT="10" # Zero means indefinitely # Base Config DELAY="0"# Zero means max speed -COUNT="10" # Zero means indefinitely # Flow variation random source port between min and max UDP_MIN=9 diff --git a
[PATCH net-next V2 2/2] pktgen: Specify the index of first thread
Use "-f ", to specify the index of the first sender thread. In default first thread is #0. Signed-off-by: Tariq Toukan <tar...@mellanox.com> Cc: Jesper Dangaard Brouer <bro...@redhat.com> --- samples/pktgen/README.rst | 1 + samples/pktgen/parameters.sh | 20 ++-- .../pktgen/pktgen_bench_xmit_mode_netif_receive.sh | 4 ++-- samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh | 4 ++-- samples/pktgen/pktgen_sample02_multiqueue.sh | 4 ++-- samples/pktgen/pktgen_sample03_burst_single_flow.sh | 4 ++-- samples/pktgen/pktgen_sample04_many_flows.sh | 4 ++-- samples/pktgen/pktgen_sample05_flow_per_thread.sh| 4 ++-- 8 files changed, 27 insertions(+), 18 deletions(-) diff --git a/samples/pktgen/README.rst b/samples/pktgen/README.rst index c018d67da1a1..ff8929da61c5 100644 --- a/samples/pktgen/README.rst +++ b/samples/pktgen/README.rst @@ -21,6 +21,7 @@ across the sample scripts. Usage example is printed on errors:: -d : ($DEST_IP) destination IP -m : ($DST_MAC) destination MAC-addr -t : ($THREADS) threads to start + -f : ($F_THREAD) index of first thread (zero indexed CPU number) -c : ($SKB_CLONE) SKB clones send before alloc new SKB -n : ($COUNT) num messages to send per thread, 0 means indefinitely -b : ($BURST) HW level bursting of SKBs diff --git a/samples/pktgen/parameters.sh b/samples/pktgen/parameters.sh index 036147594a20..3a6244d5f47a 100644 --- a/samples/pktgen/parameters.sh +++ b/samples/pktgen/parameters.sh @@ -10,6 +10,7 @@ function usage() { echo " -d : (\$DEST_IP) destination IP" echo " -m : (\$DST_MAC) destination MAC-addr" echo " -t : (\$THREADS) threads to start" +echo " -f : (\$F_THREAD) index of first thread (zero indexed CPU number)" echo " -c : (\$SKB_CLONE) SKB clones send before alloc new SKB" echo " -n : (\$COUNT) num messages to send per thread, 0 means indefinitely" echo " -b : (\$BURST) HW level bursting of SKBs" @@ -21,7 +22,7 @@ function usage() { ## --- Parse command line arguments / parameters --- ## echo "Commandline options:" -while getopts "s:i:d:m:t:c:n:b:vxh6" option; do +while getopts "s:i:d:m:f:t:c:n:b:vxh6" option; do case $option in i) # interface export DEV=$OPTARG @@ -39,11 +40,13 @@ while getopts "s:i:d:m:t:c:n:b:vxh6" option; do export DST_MAC=$OPTARG info "Destination MAC set to: DST_MAC=$DST_MAC" ;; +f) + export F_THREAD=$OPTARG + info "Index of first thread (zero indexed CPU number): $F_THREAD" + ;; t) export THREADS=$OPTARG - export CPU_THREADS=$OPTARG - let "CPU_THREADS -= 1" - info "Number of threads to start: $THREADS (0 to $CPU_THREADS)" + info "Number of threads to start: $THREADS" ;; c) export CLONE_SKB=$OPTARG @@ -82,12 +85,17 @@ if [ -z "$PKT_SIZE" ]; then info "Default packet size set to: set to: $PKT_SIZE bytes" fi +if [ -z "$F_THREAD" ]; then +# First thread (F_THREAD) reference the zero indexed CPU number +export F_THREAD=0 +fi + if [ -z "$THREADS" ]; then -# Zero CPU threads means one thread, because CPU numbers are zero indexed -export CPU_THREADS=0 export THREADS=1 fi +export L_THREAD=$(( THREADS + F_THREAD - 1 )) + if [ -z "$DEV" ]; then usage err 2 "Please specify output device" diff --git a/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh b/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh index d2694a12de61..e5bfe759a0fb 100755 --- a/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh +++ b/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh @@ -48,7 +48,7 @@ DELAY="0"# Zero means max speed pg_ctrl "reset" # Threads are specified with parameter -t value in $THREADS -for ((thread = 0; thread < $THREADS; thread++)); do +for ((thread = $F_THREAD; thread <= $L_THREAD; thread++)); do # The device name is extended with @name, using thread number to # make then unique, but any name will do. dev=${DEV}@${thread} @@ -81,7 +81,7 @@ pg_ctrl "start" echo "Done" >&2 # Print results -for ((thread = 0; thread < $THREADS; thread++)); do +for ((thread = $F_THREAD; thread <= $L_THREAD; thread++)); do dev=${DEV}@${thread} echo "Device: $dev" cat /proc/net/pktgen/$dev | grep -A2 "Result:" diff --git a/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh b/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh index 43604c2db726..1ad878e95539 100755 --- a/samples/pktgen/pktgen_benc
Re: [PATCH net-next 2/2] pktgen: Specify the index of first thread
On Wed, 14 Jun 2017 14:10:37 +0300 Tariq Toukan <tar...@mellanox.com> wrote: > >> +export L_THREAD="$THREADS + $F_THREAD - 1" > >> + > > > > This is sort of bad-shell coding. This will first get expanded at the > > usage point. The way you use it, it will work, because of the for loop > > uses an expansion like "((xxx))". > > > > If you echo the $L_THREAD variable you will see: "1 + 0 - 1" > > > > IMHO the right thing is to use: > > > >export L_THREAD=$(( THREADS + F_THREAD - 1 )) > > > > (I tested this also works for dash + ksh + zsh) > > > Thanks, I'll use the suggested command. > > > >> diff --git a/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh > >> b/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh > >> index d2694a12de61..e5bfe759a0fb 100755 > >> --- a/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh > >> +++ b/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh > >> @@ -48,7 +48,7 @@ DELAY="0"# Zero means max speed > >> pg_ctrl "reset" > >> > >> # Threads are specified with parameter -t value in $THREADS > >> -for ((thread = 0; thread < $THREADS; thread++)); do > >> +for ((thread = $F_THREAD; thread <= $L_THREAD; thread++)); do > >> # The device name is extended with @name, using thread number to > >> # make then unique, but any name will do. > >> dev=${DEV}@${thread} > > > > The expansion/use of $L_THREAD only works because "for-loop" expanded > > it by using ""(("" arithmetic evaluation. > > > After changing the one above, this one should still be OK, right? Yes, this part is still correct. -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer
Re: [PATCH net-next 2/2] pktgen: Specify the index of first thread
On 14/06/2017 10:09 AM, Jesper Dangaard Brouer wrote: On Tue, 13 Jun 2017 18:04:49 +0300 Tariq Toukan <tar...@mellanox.com> wrote: Use "-f ", to specify the index of the first sender thread. In default first thread is #0. Signed-off-by: Tariq Toukan <tar...@mellanox.com> Cc: Jesper Dangaard Brouer <bro...@redhat.com> --- samples/pktgen/README.rst | 1 + samples/pktgen/parameters.sh | 19 ++- .../pktgen/pktgen_bench_xmit_mode_netif_receive.sh | 4 ++-- samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh | 4 ++-- samples/pktgen/pktgen_sample02_multiqueue.sh | 4 ++-- samples/pktgen/pktgen_sample03_burst_single_flow.sh | 4 ++-- samples/pktgen/pktgen_sample04_many_flows.sh | 4 ++-- samples/pktgen/pktgen_sample05_flow_per_thread.sh | 4 ++-- 8 files changed, 27 insertions(+), 17 deletions(-) diff --git a/samples/pktgen/README.rst b/samples/pktgen/README.rst index c018d67da1a1..527056bd27b7 100644 --- a/samples/pktgen/README.rst +++ b/samples/pktgen/README.rst @@ -21,6 +21,7 @@ across the sample scripts. Usage example is printed on errors:: -d : ($DEST_IP) destination IP -m : ($DST_MAC) destination MAC-addr -t : ($THREADS) threads to start + -f : ($F_THREAD) index of first thread IMHO the help text should be: "index of first thread (zero indexed CPU number)" Yeah sounds better. I'll change this. -c : ($SKB_CLONE) SKB clones send before alloc new SKB -n : ($COUNT) num messages to send per thread, 0 means indefinitely -b : ($BURST) HW level bursting of SKBs diff --git a/samples/pktgen/parameters.sh b/samples/pktgen/parameters.sh index 036147594a20..d2dfc5cfa66b 100644 --- a/samples/pktgen/parameters.sh +++ b/samples/pktgen/parameters.sh @@ -10,6 +10,7 @@ function usage() { echo " -d : (\$DEST_IP) destination IP" echo " -m : (\$DST_MAC) destination MAC-addr" echo " -t : (\$THREADS) threads to start" +echo " -f : (\$F_THREAD) index of first thread" Same here: IMHO the help text should be: "index of first thread (zero indexed CPU number)" Yes. echo " -c : (\$SKB_CLONE) SKB clones send before alloc new SKB" echo " -n : (\$COUNT) num messages to send per thread, 0 means indefinitely" echo " -b : (\$BURST) HW level bursting of SKBs" @@ -21,7 +22,7 @@ function usage() { ## --- Parse command line arguments / parameters --- ## echo "Commandline options:" -while getopts "s:i:d:m:t:c:n:b:vxh6" option; do +while getopts "s:i:d:m:f:t:c:n:b:vxh6" option; do case $option in i) # interface export DEV=$OPTARG @@ -39,11 +40,13 @@ while getopts "s:i:d:m:t:c:n:b:vxh6" option; do export DST_MAC=$OPTARG info "Destination MAC set to: DST_MAC=$DST_MAC" ;; +f) + export F_THREAD=$OPTARG + info "Index of first thread: $F_THREAD" + ;; t) export THREADS=$OPTARG - export CPU_THREADS=$OPTARG - let "CPU_THREADS -= 1" - info "Number of threads to start: $THREADS (0 to $CPU_THREADS)" + info "Number of threads to start: $THREADS" ;; c) export CLONE_SKB=$OPTARG @@ -82,12 +85,18 @@ if [ -z "$PKT_SIZE" ]; then info "Default packet size set to: set to: $PKT_SIZE bytes" fi +if [ -z "$F_THREAD" ]; then +# Zero CPU threads means one thread, because CPU numbers are zero indexed Wrong comment, use: # First thread (F_THREAD) reference the zero indexes CPU number I'll fix. +export F_THREAD=0 +fi + if [ -z "$THREADS" ]; then # Zero CPU threads means one thread, because CPU numbers are zero indexed -export CPU_THREADS=0 Also remove comment, it is talking about CPU_THREADS I'll fix. export THREADS=1 fi +export L_THREAD="$THREADS + $F_THREAD - 1" + This is sort of bad-shell coding. This will first get expanded at the usage point. The way you use it, it will work, because of the for loop uses an expansion like "((xxx))". If you echo the $L_THREAD variable you will see: "1 + 0 - 1" IMHO the right thing is to use: export L_THREAD=$(( THREADS + F_THREAD - 1 )) (I tested this also works for dash + ksh + zsh) Thanks, I'll use the suggested command. diff --git a/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh b/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh index d2694a12de61..e5bfe759a0fb 100755 --- a/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh +++ b/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh @@ -48,7 +48,7 @@ DELAY="0"# Zero means max s
Re: [PATCH net-next 1/2] pktgen: Specify num packets per thread
On Tue, 13 Jun 2017 18:04:48 +0300 Tariq Toukanwrote: > Use -n , to specify the number of packets every > thread sends. > Zero means indefinitely. > > Signed-off-by: Tariq Toukan > Cc: Jesper Dangaard Brouer Patch 1/2 looks good to me (have some comments on patch 2/2) Acked-by: Jesper Dangaard Brouer -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer
Re: [PATCH net-next 2/2] pktgen: Specify the index of first thread
On Tue, 13 Jun 2017 18:04:49 +0300 Tariq Toukan <tar...@mellanox.com> wrote: > Use "-f ", to specify the index of the first > sender thread. > In default first thread is #0. > > Signed-off-by: Tariq Toukan <tar...@mellanox.com> > Cc: Jesper Dangaard Brouer <bro...@redhat.com> > --- > samples/pktgen/README.rst | 1 + > samples/pktgen/parameters.sh | 19 > ++- > .../pktgen/pktgen_bench_xmit_mode_netif_receive.sh| 4 ++-- > samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh | 4 ++-- > samples/pktgen/pktgen_sample02_multiqueue.sh | 4 ++-- > samples/pktgen/pktgen_sample03_burst_single_flow.sh | 4 ++-- > samples/pktgen/pktgen_sample04_many_flows.sh | 4 ++-- > samples/pktgen/pktgen_sample05_flow_per_thread.sh | 4 ++-- > 8 files changed, 27 insertions(+), 17 deletions(-) > > diff --git a/samples/pktgen/README.rst b/samples/pktgen/README.rst > index c018d67da1a1..527056bd27b7 100644 > --- a/samples/pktgen/README.rst > +++ b/samples/pktgen/README.rst > @@ -21,6 +21,7 @@ across the sample scripts. Usage example is printed on > errors:: >-d : ($DEST_IP) destination IP >-m : ($DST_MAC) destination MAC-addr >-t : ($THREADS) threads to start > + -f : ($F_THREAD) index of first thread IMHO the help text should be: "index of first thread (zero indexed CPU number)" >-c : ($SKB_CLONE) SKB clones send before alloc new SKB >-n : ($COUNT) num messages to send per thread, 0 means indefinitely >-b : ($BURST) HW level bursting of SKBs > diff --git a/samples/pktgen/parameters.sh b/samples/pktgen/parameters.sh > index 036147594a20..d2dfc5cfa66b 100644 > --- a/samples/pktgen/parameters.sh > +++ b/samples/pktgen/parameters.sh > @@ -10,6 +10,7 @@ function usage() { > echo " -d : (\$DEST_IP) destination IP" > echo " -m : (\$DST_MAC) destination MAC-addr" > echo " -t : (\$THREADS) threads to start" > +echo " -f : (\$F_THREAD) index of first thread" Same here: IMHO the help text should be: "index of first thread (zero indexed CPU number)" > echo " -c : (\$SKB_CLONE) SKB clones send before alloc new SKB" > echo " -n : (\$COUNT) num messages to send per thread, 0 means > indefinitely" > echo " -b : (\$BURST) HW level bursting of SKBs" > @@ -21,7 +22,7 @@ function usage() { > > ## --- Parse command line arguments / parameters --- > ## echo "Commandline options:" > -while getopts "s:i:d:m:t:c:n:b:vxh6" option; do > +while getopts "s:i:d:m:f:t:c:n:b:vxh6" option; do > case $option in > i) # interface >export DEV=$OPTARG > @@ -39,11 +40,13 @@ while getopts "s:i:d:m:t:c:n:b:vxh6" option; do >export DST_MAC=$OPTARG > info "Destination MAC set to: DST_MAC=$DST_MAC" >;; > +f) > + export F_THREAD=$OPTARG > + info "Index of first thread: $F_THREAD" > + ;; > t) > export THREADS=$OPTARG > - export CPU_THREADS=$OPTARG > - let "CPU_THREADS -= 1" > - info "Number of threads to start: $THREADS (0 to $CPU_THREADS)" > + info "Number of threads to start: $THREADS" >;; > c) > export CLONE_SKB=$OPTARG > @@ -82,12 +85,18 @@ if [ -z "$PKT_SIZE" ]; then > info "Default packet size set to: set to: $PKT_SIZE bytes" > fi > > +if [ -z "$F_THREAD" ]; then > +# Zero CPU threads means one thread, because CPU numbers are zero indexed Wrong comment, use: # First thread (F_THREAD) reference the zero indexes CPU number > +export F_THREAD=0 > +fi > + > if [ -z "$THREADS" ]; then > # Zero CPU threads means one thread, because CPU numbers are zero indexed > -export CPU_THREADS=0 Also remove comment, it is talking about CPU_THREADS > export THREADS=1 > fi > > +export L_THREAD="$THREADS + $F_THREAD - 1" > + This is sort of bad-shell coding. This will first get expanded at the usage point. The way you use it, it will work, because of the for loop uses an expansion like "((xxx))". If you echo the $L_THREAD variable you will see: "1 + 0 - 1" IMHO the right thing is to use: export L_THREAD=$(( THREADS + F_THREAD - 1 )) (I tested this also works for dash + ksh + zsh) > diff --git a/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh > b/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh > index d2694a12d
[PATCH net-next 0/2] pktgen new parameters
Hi Dave, This patchset adds two parameters to the pktgen scripts. * The first patch adds a parameter to control the number of packets sent by every pktgen thread. * The second patch adds a parameter to control the index of first thread, instead of always starting from cpu 0. Series generated against net-next commit: f1c9eed7f437 bpf, arm64: take advantage of stack_depth tracking Thanks, Tariq. Tariq Toukan (2): pktgen: Specify num packets per thread pktgen: Specify the index of first thread samples/pktgen/README.rst | 2 ++ samples/pktgen/parameters.sh | 24 +- .../pktgen/pktgen_bench_xmit_mode_netif_receive.sh | 6 +++--- .../pktgen/pktgen_bench_xmit_mode_queue_xmit.sh| 6 +++--- samples/pktgen/pktgen_sample01_simple.sh | 2 +- samples/pktgen/pktgen_sample02_multiqueue.sh | 7 --- .../pktgen/pktgen_sample03_burst_single_flow.sh| 6 +++--- samples/pktgen/pktgen_sample04_many_flows.sh | 6 +++--- samples/pktgen/pktgen_sample05_flow_per_thread.sh | 6 +++--- 9 files changed, 41 insertions(+), 24 deletions(-) -- 1.8.3.1
[PATCH net-next 1/2] pktgen: Specify num packets per thread
Use -n , to specify the number of packets every thread sends. Zero means indefinitely. Signed-off-by: Tariq Toukan <tar...@mellanox.com> Cc: Jesper Dangaard Brouer <bro...@redhat.com> --- samples/pktgen/README.rst | 1 + samples/pktgen/parameters.sh | 7 ++- samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh | 2 +- samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh| 2 +- samples/pktgen/pktgen_sample01_simple.sh | 2 +- samples/pktgen/pktgen_sample02_multiqueue.sh | 3 ++- samples/pktgen/pktgen_sample03_burst_single_flow.sh| 2 +- samples/pktgen/pktgen_sample04_many_flows.sh | 2 +- samples/pktgen/pktgen_sample05_flow_per_thread.sh | 2 +- 9 files changed, 15 insertions(+), 8 deletions(-) diff --git a/samples/pktgen/README.rst b/samples/pktgen/README.rst index 8365c4e5c513..c018d67da1a1 100644 --- a/samples/pktgen/README.rst +++ b/samples/pktgen/README.rst @@ -22,6 +22,7 @@ across the sample scripts. Usage example is printed on errors:: -m : ($DST_MAC) destination MAC-addr -t : ($THREADS) threads to start -c : ($SKB_CLONE) SKB clones send before alloc new SKB + -n : ($COUNT) num messages to send per thread, 0 means indefinitely -b : ($BURST) HW level bursting of SKBs -v : ($VERBOSE) verbose -x : ($DEBUG) debug diff --git a/samples/pktgen/parameters.sh b/samples/pktgen/parameters.sh index f70ea7dd5660..036147594a20 100644 --- a/samples/pktgen/parameters.sh +++ b/samples/pktgen/parameters.sh @@ -11,6 +11,7 @@ function usage() { echo " -m : (\$DST_MAC) destination MAC-addr" echo " -t : (\$THREADS) threads to start" echo " -c : (\$SKB_CLONE) SKB clones send before alloc new SKB" +echo " -n : (\$COUNT) num messages to send per thread, 0 means indefinitely" echo " -b : (\$BURST) HW level bursting of SKBs" echo " -v : (\$VERBOSE) verbose" echo " -x : (\$DEBUG) debug" @@ -20,7 +21,7 @@ function usage() { ## --- Parse command line arguments / parameters --- ## echo "Commandline options:" -while getopts "s:i:d:m:t:c:b:vxh6" option; do +while getopts "s:i:d:m:t:c:n:b:vxh6" option; do case $option in i) # interface export DEV=$OPTARG @@ -48,6 +49,10 @@ while getopts "s:i:d:m:t:c:b:vxh6" option; do export CLONE_SKB=$OPTARG info "CLONE_SKB=$CLONE_SKB" ;; +n) + export COUNT=$OPTARG + info "COUNT=$COUNT" + ;; b) export BURST=$OPTARG info "SKB bursting: BURST=$BURST" diff --git a/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh b/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh index f3e1bedfd77f..d2694a12de61 100755 --- a/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh +++ b/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh @@ -39,10 +39,10 @@ if [ -z "$DEST_IP" ]; then fi [ -z "$DST_MAC" ] && DST_MAC="90:e2:ba:ff:ff:ff" [ -z "$BURST" ] && BURST=1024 +[ -z "$COUNT" ] && COUNT="1000" # Zero means indefinitely # Base Config DELAY="0"# Zero means max speed -COUNT="1000" # Zero means indefinitely # General cleanup everything since last run pg_ctrl "reset" diff --git a/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh b/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh index cc102e923241..43604c2db726 100755 --- a/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh +++ b/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh @@ -22,10 +22,10 @@ fi if [[ -n "$BURST" ]]; then err 1 "Bursting not supported for this mode" fi +[ -z "$COUNT" ] && COUNT="1000" # Zero means indefinitely # Base Config DELAY="0"# Zero means max speed -COUNT="1000" # Zero means indefinitely # General cleanup everything since last run pg_ctrl "reset" diff --git a/samples/pktgen/pktgen_sample01_simple.sh b/samples/pktgen/pktgen_sample01_simple.sh index 29ef4ba50796..35b7fe34bda2 100755 --- a/samples/pktgen/pktgen_sample01_simple.sh +++ b/samples/pktgen/pktgen_sample01_simple.sh @@ -20,10 +20,10 @@ fi [ -z "$CLONE_SKB" ] && CLONE_SKB="0" # Example enforce param "-m" for dst_mac [ -z "$DST_MAC" ] && usage && err 2 "Must specify -m dst_mac" +[ -z "$COUNT" ] && COUNT="10" # Zero means indefinitely # Base Config DELAY="0"# Zero means max speed -COUNT="10" # Zero means indefinitely # Flow variation random source port between min and max UDP_MIN=9 diff --git a
[PATCH net-next 2/2] pktgen: Specify the index of first thread
Use "-f ", to specify the index of the first sender thread. In default first thread is #0. Signed-off-by: Tariq Toukan <tar...@mellanox.com> Cc: Jesper Dangaard Brouer <bro...@redhat.com> --- samples/pktgen/README.rst | 1 + samples/pktgen/parameters.sh | 19 ++++++- .../pktgen/pktgen_bench_xmit_mode_netif_receive.sh | 4 ++-- samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh | 4 ++-- samples/pktgen/pktgen_sample02_multiqueue.sh | 4 ++-- samples/pktgen/pktgen_sample03_burst_single_flow.sh | 4 ++-- samples/pktgen/pktgen_sample04_many_flows.sh | 4 ++-- samples/pktgen/pktgen_sample05_flow_per_thread.sh | 4 ++-- 8 files changed, 27 insertions(+), 17 deletions(-) diff --git a/samples/pktgen/README.rst b/samples/pktgen/README.rst index c018d67da1a1..527056bd27b7 100644 --- a/samples/pktgen/README.rst +++ b/samples/pktgen/README.rst @@ -21,6 +21,7 @@ across the sample scripts. Usage example is printed on errors:: -d : ($DEST_IP) destination IP -m : ($DST_MAC) destination MAC-addr -t : ($THREADS) threads to start + -f : ($F_THREAD) index of first thread -c : ($SKB_CLONE) SKB clones send before alloc new SKB -n : ($COUNT) num messages to send per thread, 0 means indefinitely -b : ($BURST) HW level bursting of SKBs diff --git a/samples/pktgen/parameters.sh b/samples/pktgen/parameters.sh index 036147594a20..d2dfc5cfa66b 100644 --- a/samples/pktgen/parameters.sh +++ b/samples/pktgen/parameters.sh @@ -10,6 +10,7 @@ function usage() { echo " -d : (\$DEST_IP) destination IP" echo " -m : (\$DST_MAC) destination MAC-addr" echo " -t : (\$THREADS) threads to start" +echo " -f : (\$F_THREAD) index of first thread" echo " -c : (\$SKB_CLONE) SKB clones send before alloc new SKB" echo " -n : (\$COUNT) num messages to send per thread, 0 means indefinitely" echo " -b : (\$BURST) HW level bursting of SKBs" @@ -21,7 +22,7 @@ function usage() { ## --- Parse command line arguments / parameters --- ## echo "Commandline options:" -while getopts "s:i:d:m:t:c:n:b:vxh6" option; do +while getopts "s:i:d:m:f:t:c:n:b:vxh6" option; do case $option in i) # interface export DEV=$OPTARG @@ -39,11 +40,13 @@ while getopts "s:i:d:m:t:c:n:b:vxh6" option; do export DST_MAC=$OPTARG info "Destination MAC set to: DST_MAC=$DST_MAC" ;; +f) + export F_THREAD=$OPTARG + info "Index of first thread: $F_THREAD" + ;; t) export THREADS=$OPTARG - export CPU_THREADS=$OPTARG - let "CPU_THREADS -= 1" - info "Number of threads to start: $THREADS (0 to $CPU_THREADS)" + info "Number of threads to start: $THREADS" ;; c) export CLONE_SKB=$OPTARG @@ -82,12 +85,18 @@ if [ -z "$PKT_SIZE" ]; then info "Default packet size set to: set to: $PKT_SIZE bytes" fi +if [ -z "$F_THREAD" ]; then +# Zero CPU threads means one thread, because CPU numbers are zero indexed +export F_THREAD=0 +fi + if [ -z "$THREADS" ]; then # Zero CPU threads means one thread, because CPU numbers are zero indexed -export CPU_THREADS=0 export THREADS=1 fi +export L_THREAD="$THREADS + $F_THREAD - 1" + if [ -z "$DEV" ]; then usage err 2 "Please specify output device" diff --git a/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh b/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh index d2694a12de61..e5bfe759a0fb 100755 --- a/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh +++ b/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh @@ -48,7 +48,7 @@ DELAY="0"# Zero means max speed pg_ctrl "reset" # Threads are specified with parameter -t value in $THREADS -for ((thread = 0; thread < $THREADS; thread++)); do +for ((thread = $F_THREAD; thread <= $L_THREAD; thread++)); do # The device name is extended with @name, using thread number to # make then unique, but any name will do. dev=${DEV}@${thread} @@ -81,7 +81,7 @@ pg_ctrl "start" echo "Done" >&2 # Print results -for ((thread = 0; thread < $THREADS; thread++)); do +for ((thread = $F_THREAD; thread <= $L_THREAD; thread++)); do dev=${DEV}@${thread} echo "Device: $dev" cat /proc/net/pktgen/$dev | grep -A2 "Result:" diff --git a/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh b/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh index 43604c2db726..1ad878e95539 100755 --- a/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh +++ b/samples/pktgen/pktge
[PATCH 3/3] net: pktgen: Adjust five checks for null pointers
From: Markus ElfringDate: Mon, 22 May 2017 10:44:16 +0200 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The script “checkpatch.pl” pointed information out like the following. Comparison to NULL could be written !… Thus fix the affected source code places. Signed-off-by: Markus Elfring --- net/core/pktgen.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/net/core/pktgen.c b/net/core/pktgen.c index 51008ddc7af6..a28350c9ac67 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -1986,7 +1986,7 @@ static void pktgen_mark_device(const struct pktgen_net *pn, const char *ifname) while (1) { pkt_dev = __pktgen_NN_threads(pn, ifname, REMOVE); - if (pkt_dev == NULL) + if (!pkt_dev) break; /* success */ mutex_unlock(_thread_lock); @@ -3274,7 +3274,7 @@ static struct pktgen_dev *next_to_run(struct pktgen_thread *t) list_for_each_entry_rcu(pkt_dev, >if_list, list) { if (!pkt_dev->running) continue; - if (best == NULL) + if (!best) best = pkt_dev; else if (ktime_compare(pkt_dev->next_tx, best->next_tx) < 0) best = pkt_dev; @@ -3402,7 +3402,7 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev) kfree_skb(pkt_dev->skb); pkt_dev->skb = fill_packet(odev, pkt_dev); - if (pkt_dev->skb == NULL) { + if (!pkt_dev->skb) { pr_err("ERROR: couldn't allocate skb in fill_packet\n"); schedule(); pkt_dev->clone_count--; /* back out increment, OOM */ @@ -3685,7 +3685,7 @@ static int pktgen_add_device(struct pktgen_thread *t, const char *ifname) strcpy(pkt_dev->odevname, ifname); pkt_dev->flows = vzalloc_node(MAX_CFLOWS * sizeof(struct flow_state), node); - if (pkt_dev->flows == NULL) { + if (!pkt_dev->flows) { kfree(pkt_dev); return -ENOMEM; } @@ -3868,7 +3868,7 @@ static int __net_init pg_net_init(struct net *net) return -ENODEV; } pe = proc_create(PGCTRL, 0600, pn->proc_dir, _fops); - if (pe == NULL) { + if (!pe) { pr_err("cannot create %s procfs entry\n", PGCTRL); ret = -EINVAL; goto remove; -- 2.13.0
[PATCH 2/3] net: pktgen: Delete an error message for a failed memory allocation in pktgen_create_thread()
From: Markus ElfringDate: Mon, 22 May 2017 10:38:46 +0200 Omit an extra message for a memory allocation failure in this function. This issue was detected by using the Coccinelle software. Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf Signed-off-by: Markus Elfring --- net/core/pktgen.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/net/core/pktgen.c b/net/core/pktgen.c index c89f4ad21187..51008ddc7af6 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -3760,7 +3760,5 @@ static int __net_init pktgen_create_thread(int cpu, struct pktgen_net *pn) - if (!t) { - pr_err("ERROR: out of memory, can't create new thread\n"); + if (!t) return -ENOMEM; - } mutex_init(>if_lock); t->cpu = cpu; -- 2.13.0
[PATCH 1/3] net: pktgen: Improve four size determinations
From: Markus ElfringDate: Mon, 22 May 2017 10:34:11 +0200 Replace the specification of four data structures by pointer dereferences as the parameter for the operator "sizeof" to make the corresponding size determination a bit safer according to the Linux coding style convention. Signed-off-by: Markus Elfring --- net/core/pktgen.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/net/core/pktgen.c b/net/core/pktgen.c index 96947f5d41e4..c89f4ad21187 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -2995,10 +2995,10 @@ static struct sk_buff *fill_packet_ipv6(struct net_device *odev, skb_reset_mac_header(skb); skb_set_network_header(skb, skb->len); - iph = (struct ipv6hdr *) skb_put(skb, sizeof(struct ipv6hdr)); + iph = (struct ipv6hdr *)skb_put(skb, sizeof(*iph)); skb_set_transport_header(skb, skb->len); - udph = (struct udphdr *) skb_put(skb, sizeof(struct udphdr)); + udph = (struct udphdr *)skb_put(skb, sizeof(*udph)); skb_set_queue_mapping(skb, queue_map); skb->priority = pkt_dev->skb_priority; @@ -3678,5 +3678,5 @@ static int pktgen_add_device(struct pktgen_thread *t, const char *ifname) return -EBUSY; } - pkt_dev = kzalloc_node(sizeof(struct pktgen_dev), GFP_KERNEL, node); + pkt_dev = kzalloc_node(sizeof(*pkt_dev), GFP_KERNEL, node); if (!pkt_dev) @@ -3756,6 +3756,5 @@ static int __net_init pktgen_create_thread(int cpu, struct pktgen_net *pn) struct proc_dir_entry *pe; struct task_struct *p; - t = kzalloc_node(sizeof(struct pktgen_thread), GFP_KERNEL, -cpu_to_node(cpu)); + t = kzalloc_node(sizeof(*t), GFP_KERNEL, cpu_to_node(cpu)); if (!t) { -- 2.13.0
[PATCH 0/3] net-pktgen: Adjustments for some function implementations
From: Markus ElfringDate: Mon, 22 May 2017 11:01:23 +0200 A few update suggestions were taken into account from static source code analysis. Markus Elfring (3): Improve four size determinations Delete an error message for a failed memory allocation in pktgen_create_thread() Adjust five checks for null pointers net/core/pktgen.c | 23 ++- 1 file changed, 10 insertions(+), 13 deletions(-) -- 2.13.0
Re: [PATCH net] net: pktgen: remove rcu locking in pktgen_change_name()
From: Eric Dumazet <eric.duma...@gmail.com> Date: Sat, 15 Oct 2016 17:50:49 +0200 > From: Eric Dumazet <eduma...@google.com> > > After Jesper commit back in linux-3.18, we trigger a lockdep > splat in proc_create_data() while allocating memory from > pktgen_change_name(). > > This patch converts t->if_lock to a mutex, since it is now only > used from control path, and adds proper locking to pktgen_change_name() > > 1) pktgen_thread_lock to protect the outer loop (iterating threads) > 2) t->if_lock to protect the inner loop (iterating devices) > > Note that before Jesper patch, pktgen_change_name() was lacking proper > protection, but lockdep was not able to detect the problem. > > Fixes: 8788370a1d4b ("pktgen: RCU-ify "if_list" to remove lock in > next_to_run()") > Reported-by: John Sperbeck <jsperb...@google.com> > Signed-off-by: Eric Dumazet <eduma...@google.com> Applied and queued up for -stable, thanks.
[PATCH net] net: pktgen: remove rcu locking in pktgen_change_name()
From: Eric Dumazet <eduma...@google.com> After Jesper commit back in linux-3.18, we trigger a lockdep splat in proc_create_data() while allocating memory from pktgen_change_name(). This patch converts t->if_lock to a mutex, since it is now only used from control path, and adds proper locking to pktgen_change_name() 1) pktgen_thread_lock to protect the outer loop (iterating threads) 2) t->if_lock to protect the inner loop (iterating devices) Note that before Jesper patch, pktgen_change_name() was lacking proper protection, but lockdep was not able to detect the problem. Fixes: 8788370a1d4b ("pktgen: RCU-ify "if_list" to remove lock in next_to_run()") Reported-by: John Sperbeck <jsperb...@google.com> Signed-off-by: Eric Dumazet <eduma...@google.com> Cc: Jesper Dangaard Brouer <bro...@redhat.com> --- net/core/pktgen.c | 17 ++--- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/net/core/pktgen.c b/net/core/pktgen.c index 5219a9e2127a..306b8f0e03c1 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -216,8 +216,8 @@ #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)); -#define if_unlock(t) spin_unlock(&(t->if_lock)); +#define if_lock(t) mutex_lock(&(t->if_lock)); +#define if_unlock(t) mutex_unlock(&(t->if_lock)); /* Used to help with determining the pkts on receive */ #define PKTGEN_MAGIC 0xbe9be955 @@ -423,7 +423,7 @@ struct pktgen_net { }; struct pktgen_thread { - spinlock_t if_lock; /* for list of devices */ + struct mutex if_lock; /* for list of devices */ struct list_head if_list; /* All device here */ struct list_head th_list; struct task_struct *tsk; @@ -2010,11 +2010,13 @@ static void pktgen_change_name(const struct pktgen_net *pn, struct net_device *d { struct pktgen_thread *t; + mutex_lock(_thread_lock); + list_for_each_entry(t, >pktgen_threads, th_list) { struct pktgen_dev *pkt_dev; - rcu_read_lock(); - list_for_each_entry_rcu(pkt_dev, >if_list, list) { + if_lock(t); + list_for_each_entry(pkt_dev, >if_list, list) { if (pkt_dev->odev != dev) continue; @@ -2029,8 +2031,9 @@ static void pktgen_change_name(const struct pktgen_net *pn, struct net_device *d dev->name); break; } - rcu_read_unlock(); + if_unlock(t); } + mutex_unlock(_thread_lock); } static int pktgen_device_event(struct notifier_block *unused, @@ -3762,7 +3765,7 @@ static int __net_init pktgen_create_thread(int cpu, struct pktgen_net *pn) return -ENOMEM; } - spin_lock_init(>if_lock); + mutex_init(>if_lock); t->cpu = cpu; INIT_LIST_HEAD(>if_list);
Re: [PATCH net v2] net: pktgen: fix pkt_size
On Sat, 2016-10-01 at 20:05 +0200, Paolo Abeni wrote: > On Fri, 2016-09-30 at 08:39 -0700, Greg wrote: > > On Fri, 2016-09-30 at 16:56 +0200, Paolo Abeni wrote: > > > The commit 879c7220e828 ("net: pktgen: Observe needed_headroom > > > of the device") increased the 'pkt_overhead' field value by > > > LL_RESERVED_SPACE. > > > As a side effect the generated packet size, computed as: > > > > > > /* Eth + IPh + UDPh + mpls */ > > > datalen = pkt_dev->cur_pkt_size - 14 - 20 - 8 - > > > pkt_dev->pkt_overhead; > > > > > > is decreased by the same value. > > > The above changed slightly the behavior of existing pktgen users, > > > and made the procfs interface somewhat inconsistent. > > > Fix it by restoring the previous pkt_overhead value and using > > > LL_RESERVED_SPACE as extralen in skb allocation. > > > Also, change pktgen_alloc_skb() to only partially reserve > > > the headroom to allow the caller to prefetch from ll header > > > start. > > > > > > v1 -> v2: > > > - fixed some typos in the comments > > > > > > Fixes: 879c7220e828 ("net: pktgen: Observe needed_headroom of the device") > > > Suggested-by: Ben Greear <gree...@candelatech.com> > > > Signed-off-by: Paolo Abeni <pab...@redhat.com> > > > --- > > > net/core/pktgen.c | 21 ++--- > > > 1 file changed, 10 insertions(+), 11 deletions(-) > > > > > > diff --git a/net/core/pktgen.c b/net/core/pktgen.c > > > index bbd118b..5219a9e 100644 > > > --- a/net/core/pktgen.c > > > +++ b/net/core/pktgen.c > > > @@ -2286,7 +2286,7 @@ out: > > > > > > static inline void set_pkt_overhead(struct pktgen_dev *pkt_dev) > > > { > > > - pkt_dev->pkt_overhead = LL_RESERVED_SPACE(pkt_dev->odev); > > > + pkt_dev->pkt_overhead = 0; > > > pkt_dev->pkt_overhead += pkt_dev->nr_labels*sizeof(u32); > > > pkt_dev->pkt_overhead += VLAN_TAG_SIZE(pkt_dev); > > > pkt_dev->pkt_overhead += SVLAN_TAG_SIZE(pkt_dev); > > > @@ -2777,13 +2777,13 @@ static void pktgen_finalize_skb(struct pktgen_dev > > > *pkt_dev, struct sk_buff *skb, > > > } > > > > > > static struct sk_buff *pktgen_alloc_skb(struct net_device *dev, > > > - struct pktgen_dev *pkt_dev, > > > - unsigned int extralen) > > > + struct pktgen_dev *pkt_dev) > > > { > > > + unsigned int extralen = LL_RESERVED_SPACE(dev); > > > struct sk_buff *skb = NULL; > > > - unsigned int size = pkt_dev->cur_pkt_size + 64 + extralen + > > > - pkt_dev->pkt_overhead; > > > + unsigned int size; > > > > > > + size = pkt_dev->cur_pkt_size + 64 + extralen + pkt_dev->pkt_overhead; > > > if (pkt_dev->flags & F_NODE) { > > > int node = pkt_dev->node >= 0 ? pkt_dev->node : numa_node_id(); > > > > > > @@ -2796,8 +2796,9 @@ static struct sk_buff *pktgen_alloc_skb(struct > > > net_device *dev, > > >skb = __netdev_alloc_skb(dev, size, GFP_NOWAIT); > > > } > > > > > > + /* the caller pre-fetches from skb->data and reserves for the mac hdr */ > > > if (likely(skb)) > > > - skb_reserve(skb, LL_RESERVED_SPACE(dev)); > > > + skb_reserve(skb, extralen - 16); > > > > Is the 16 here the same as HD_DATA_MOD? > > > > Magic numbers... > > This magic numbers comes from the current code (the extralen argument), > this patch just move it around. I can send a v3 using a define for it, > if really needed, but it seems more a separated clean-up. Sure, sounds good. Looks like Dave applied it. - Greg > > Thank you, > > Paolo >
Re: [PATCH net v2] net: pktgen: fix pkt_size
From: Paolo Abeni <pab...@redhat.com> Date: Fri, 30 Sep 2016 16:56:45 +0200 > The commit 879c7220e828 ("net: pktgen: Observe needed_headroom > of the device") increased the 'pkt_overhead' field value by > LL_RESERVED_SPACE. > As a side effect the generated packet size, computed as: > > /* Eth + IPh + UDPh + mpls */ > datalen = pkt_dev->cur_pkt_size - 14 - 20 - 8 - > pkt_dev->pkt_overhead; > > is decreased by the same value. > The above changed slightly the behavior of existing pktgen users, > and made the procfs interface somewhat inconsistent. > Fix it by restoring the previous pkt_overhead value and using > LL_RESERVED_SPACE as extralen in skb allocation. > Also, change pktgen_alloc_skb() to only partially reserve > the headroom to allow the caller to prefetch from ll header > start. > > v1 -> v2: > - fixed some typos in the comments > > Fixes: 879c7220e828 ("net: pktgen: Observe needed_headroom of the device") > Suggested-by: Ben Greear <gree...@candelatech.com> > Signed-off-by: Paolo Abeni <pab...@redhat.com> Applied and queued up for -stable.
Re: [PATCH net v2] net: pktgen: fix pkt_size
On Fri, 2016-09-30 at 08:39 -0700, Greg wrote: > On Fri, 2016-09-30 at 16:56 +0200, Paolo Abeni wrote: > > The commit 879c7220e828 ("net: pktgen: Observe needed_headroom > > of the device") increased the 'pkt_overhead' field value by > > LL_RESERVED_SPACE. > > As a side effect the generated packet size, computed as: > > > > /* Eth + IPh + UDPh + mpls */ > > datalen = pkt_dev->cur_pkt_size - 14 - 20 - 8 - > > pkt_dev->pkt_overhead; > > > > is decreased by the same value. > > The above changed slightly the behavior of existing pktgen users, > > and made the procfs interface somewhat inconsistent. > > Fix it by restoring the previous pkt_overhead value and using > > LL_RESERVED_SPACE as extralen in skb allocation. > > Also, change pktgen_alloc_skb() to only partially reserve > > the headroom to allow the caller to prefetch from ll header > > start. > > > > v1 -> v2: > > - fixed some typos in the comments > > > > Fixes: 879c7220e828 ("net: pktgen: Observe needed_headroom of the device") > > Suggested-by: Ben Greear <gree...@candelatech.com> > > Signed-off-by: Paolo Abeni <pab...@redhat.com> > > --- > > net/core/pktgen.c | 21 ++--- > > 1 file changed, 10 insertions(+), 11 deletions(-) > > > > diff --git a/net/core/pktgen.c b/net/core/pktgen.c > > index bbd118b..5219a9e 100644 > > --- a/net/core/pktgen.c > > +++ b/net/core/pktgen.c > > @@ -2286,7 +2286,7 @@ out: > > > > static inline void set_pkt_overhead(struct pktgen_dev *pkt_dev) > > { > > - pkt_dev->pkt_overhead = LL_RESERVED_SPACE(pkt_dev->odev); > > + pkt_dev->pkt_overhead = 0; > > pkt_dev->pkt_overhead += pkt_dev->nr_labels*sizeof(u32); > > pkt_dev->pkt_overhead += VLAN_TAG_SIZE(pkt_dev); > > pkt_dev->pkt_overhead += SVLAN_TAG_SIZE(pkt_dev); > > @@ -2777,13 +2777,13 @@ static void pktgen_finalize_skb(struct pktgen_dev > > *pkt_dev, struct sk_buff *skb, > > } > > > > static struct sk_buff *pktgen_alloc_skb(struct net_device *dev, > > - struct pktgen_dev *pkt_dev, > > - unsigned int extralen) > > + struct pktgen_dev *pkt_dev) > > { > > + unsigned int extralen = LL_RESERVED_SPACE(dev); > > struct sk_buff *skb = NULL; > > - unsigned int size = pkt_dev->cur_pkt_size + 64 + extralen + > > - pkt_dev->pkt_overhead; > > + unsigned int size; > > > > + size = pkt_dev->cur_pkt_size + 64 + extralen + pkt_dev->pkt_overhead; > > if (pkt_dev->flags & F_NODE) { > > int node = pkt_dev->node >= 0 ? pkt_dev->node : numa_node_id(); > > > > @@ -2796,8 +2796,9 @@ static struct sk_buff *pktgen_alloc_skb(struct > > net_device *dev, > > skb = __netdev_alloc_skb(dev, size, GFP_NOWAIT); > > } > > > > + /* the caller pre-fetches from skb->data and reserves for the mac hdr */ > > if (likely(skb)) > > - skb_reserve(skb, LL_RESERVED_SPACE(dev)); > > + skb_reserve(skb, extralen - 16); > > Is the 16 here the same as HD_DATA_MOD? > > Magic numbers... This magic numbers comes from the current code (the extralen argument), this patch just move it around. I can send a v3 using a define for it, if really needed, but it seems more a separated clean-up. Thank you, Paolo
Re: [PATCH net v2] net: pktgen: fix pkt_size
On Fri, 2016-09-30 at 16:56 +0200, Paolo Abeni wrote: > The commit 879c7220e828 ("net: pktgen: Observe needed_headroom > of the device") increased the 'pkt_overhead' field value by > LL_RESERVED_SPACE. > As a side effect the generated packet size, computed as: > > /* Eth + IPh + UDPh + mpls */ > datalen = pkt_dev->cur_pkt_size - 14 - 20 - 8 - > pkt_dev->pkt_overhead; > > is decreased by the same value. > The above changed slightly the behavior of existing pktgen users, > and made the procfs interface somewhat inconsistent. > Fix it by restoring the previous pkt_overhead value and using > LL_RESERVED_SPACE as extralen in skb allocation. > Also, change pktgen_alloc_skb() to only partially reserve > the headroom to allow the caller to prefetch from ll header > start. > > v1 -> v2: > - fixed some typos in the comments > > Fixes: 879c7220e828 ("net: pktgen: Observe needed_headroom of the device") > Suggested-by: Ben Greear <gree...@candelatech.com> > Signed-off-by: Paolo Abeni <pab...@redhat.com> > --- > net/core/pktgen.c | 21 ++--- > 1 file changed, 10 insertions(+), 11 deletions(-) > > diff --git a/net/core/pktgen.c b/net/core/pktgen.c > index bbd118b..5219a9e 100644 > --- a/net/core/pktgen.c > +++ b/net/core/pktgen.c > @@ -2286,7 +2286,7 @@ out: > > static inline void set_pkt_overhead(struct pktgen_dev *pkt_dev) > { > - pkt_dev->pkt_overhead = LL_RESERVED_SPACE(pkt_dev->odev); > + pkt_dev->pkt_overhead = 0; > pkt_dev->pkt_overhead += pkt_dev->nr_labels*sizeof(u32); > pkt_dev->pkt_overhead += VLAN_TAG_SIZE(pkt_dev); > pkt_dev->pkt_overhead += SVLAN_TAG_SIZE(pkt_dev); > @@ -2777,13 +2777,13 @@ static void pktgen_finalize_skb(struct pktgen_dev > *pkt_dev, struct sk_buff *skb, > } > > static struct sk_buff *pktgen_alloc_skb(struct net_device *dev, > - struct pktgen_dev *pkt_dev, > - unsigned int extralen) > + struct pktgen_dev *pkt_dev) > { > + unsigned int extralen = LL_RESERVED_SPACE(dev); > struct sk_buff *skb = NULL; > - unsigned int size = pkt_dev->cur_pkt_size + 64 + extralen + > - pkt_dev->pkt_overhead; > + unsigned int size; > > + size = pkt_dev->cur_pkt_size + 64 + extralen + pkt_dev->pkt_overhead; > if (pkt_dev->flags & F_NODE) { > int node = pkt_dev->node >= 0 ? pkt_dev->node : numa_node_id(); > > @@ -2796,8 +2796,9 @@ static struct sk_buff *pktgen_alloc_skb(struct > net_device *dev, >skb = __netdev_alloc_skb(dev, size, GFP_NOWAIT); > } > > + /* the caller pre-fetches from skb->data and reserves for the mac hdr */ > if (likely(skb)) > - skb_reserve(skb, LL_RESERVED_SPACE(dev)); > + skb_reserve(skb, extralen - 16); Is the 16 here the same as HD_DATA_MOD? Magic numbers... Thanks, - Greg > > return skb; > } > @@ -2830,16 +2831,14 @@ static struct sk_buff *fill_packet_ipv4(struct > net_device *odev, > mod_cur_headers(pkt_dev); > queue_map = pkt_dev->cur_queue_map; > > - datalen = (odev->hard_header_len + 16) & ~0xf; > - > - skb = pktgen_alloc_skb(odev, pkt_dev, datalen); > + skb = pktgen_alloc_skb(odev, pkt_dev); > if (!skb) { > sprintf(pkt_dev->result, "No memory"); > return NULL; > } > > prefetchw(skb->data); > - skb_reserve(skb, datalen); > + skb_reserve(skb, 16); > > /* Reserve for ethernet and IP header */ > eth = (__u8 *) skb_push(skb, 14); > @@ -2959,7 +2958,7 @@ static struct sk_buff *fill_packet_ipv6(struct > net_device *odev, > mod_cur_headers(pkt_dev); > queue_map = pkt_dev->cur_queue_map; > > - skb = pktgen_alloc_skb(odev, pkt_dev, 16); > + skb = pktgen_alloc_skb(odev, pkt_dev); > if (!skb) { > sprintf(pkt_dev->result, "No memory"); > return NULL;
[PATCH net v2] net: pktgen: fix pkt_size
The commit 879c7220e828 ("net: pktgen: Observe needed_headroom of the device") increased the 'pkt_overhead' field value by LL_RESERVED_SPACE. As a side effect the generated packet size, computed as: /* Eth + IPh + UDPh + mpls */ datalen = pkt_dev->cur_pkt_size - 14 - 20 - 8 - pkt_dev->pkt_overhead; is decreased by the same value. The above changed slightly the behavior of existing pktgen users, and made the procfs interface somewhat inconsistent. Fix it by restoring the previous pkt_overhead value and using LL_RESERVED_SPACE as extralen in skb allocation. Also, change pktgen_alloc_skb() to only partially reserve the headroom to allow the caller to prefetch from ll header start. v1 -> v2: - fixed some typos in the comments Fixes: 879c7220e828 ("net: pktgen: Observe needed_headroom of the device") Suggested-by: Ben Greear <gree...@candelatech.com> Signed-off-by: Paolo Abeni <pab...@redhat.com> --- net/core/pktgen.c | 21 ++--- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/net/core/pktgen.c b/net/core/pktgen.c index bbd118b..5219a9e 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -2286,7 +2286,7 @@ out: static inline void set_pkt_overhead(struct pktgen_dev *pkt_dev) { - pkt_dev->pkt_overhead = LL_RESERVED_SPACE(pkt_dev->odev); + pkt_dev->pkt_overhead = 0; pkt_dev->pkt_overhead += pkt_dev->nr_labels*sizeof(u32); pkt_dev->pkt_overhead += VLAN_TAG_SIZE(pkt_dev); pkt_dev->pkt_overhead += SVLAN_TAG_SIZE(pkt_dev); @@ -2777,13 +2777,13 @@ static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb, } static struct sk_buff *pktgen_alloc_skb(struct net_device *dev, - struct pktgen_dev *pkt_dev, - unsigned int extralen) + struct pktgen_dev *pkt_dev) { + unsigned int extralen = LL_RESERVED_SPACE(dev); struct sk_buff *skb = NULL; - unsigned int size = pkt_dev->cur_pkt_size + 64 + extralen + - pkt_dev->pkt_overhead; + unsigned int size; + size = pkt_dev->cur_pkt_size + 64 + extralen + pkt_dev->pkt_overhead; if (pkt_dev->flags & F_NODE) { int node = pkt_dev->node >= 0 ? pkt_dev->node : numa_node_id(); @@ -2796,8 +2796,9 @@ static struct sk_buff *pktgen_alloc_skb(struct net_device *dev, skb = __netdev_alloc_skb(dev, size, GFP_NOWAIT); } + /* the caller pre-fetches from skb->data and reserves for the mac hdr */ if (likely(skb)) - skb_reserve(skb, LL_RESERVED_SPACE(dev)); + skb_reserve(skb, extralen - 16); return skb; } @@ -2830,16 +2831,14 @@ static struct sk_buff *fill_packet_ipv4(struct net_device *odev, mod_cur_headers(pkt_dev); queue_map = pkt_dev->cur_queue_map; - datalen = (odev->hard_header_len + 16) & ~0xf; - - skb = pktgen_alloc_skb(odev, pkt_dev, datalen); + skb = pktgen_alloc_skb(odev, pkt_dev); if (!skb) { sprintf(pkt_dev->result, "No memory"); return NULL; } prefetchw(skb->data); - skb_reserve(skb, datalen); + skb_reserve(skb, 16); /* Reserve for ethernet and IP header */ eth = (__u8 *) skb_push(skb, 14); @@ -2959,7 +2958,7 @@ static struct sk_buff *fill_packet_ipv6(struct net_device *odev, mod_cur_headers(pkt_dev); queue_map = pkt_dev->cur_queue_map; - skb = pktgen_alloc_skb(odev, pkt_dev, 16); + skb = pktgen_alloc_skb(odev, pkt_dev); if (!skb) { sprintf(pkt_dev->result, "No memory"); return NULL; -- 1.8.3.1
Re: [PATCH net] net: pktgen: fix pkt_size
Hello. On 09/29/2016 05:04 PM, Paolo Abeni wrote: The commit 879c7220e828 ("net: pktgen: Observe needed_headroom of the device") increased the 'pkt_overhead' field value by LL_RESERVED_SPACE. As a side effect the generated packet size, computed as: /* Eth + IPh + UDPh + mpls */ datalen = pkt_dev->cur_pkt_size - 14 - 20 - 8 - pkt_dev->pkt_overhead; is decreased by the same value. The above changed slightly the behavior of existing pktgen users, and made the interface somewhat inconsistent. Fix it by restoring the previous pkt_overhead value and using LL_RESERVED_SPACE as extralen in skb allocation. Also, change pktgen_alloc_skb() to only partially reserve the headroom to allow the caller to prefetch from ll header start. Fixes: 879c7220e828 ("net: pktgen: Observe needed_headroom of the device") Suggested-by: Ben Greear <gree...@candelatech.com> Signed-off-by: Paolo Abeni <pab...@redhat.com> --- net/core/pktgen.c | 21 ++--- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/net/core/pktgen.c b/net/core/pktgen.c index bbd118b..5c9b397 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c [...] @@ -2796,8 +2796,9 @@ static struct sk_buff *pktgen_alloc_skb(struct net_device *dev, skb = __netdev_alloc_skb(dev, size, GFP_NOWAIT); } + /* the caller prefetch from skb->data and reserve for the mac hdr */ Pre-fetches and reserves? [...] MBR, Sergei
[PATCH net] net: pktgen: fix pkt_size
The commit 879c7220e828 ("net: pktgen: Observe needed_headroom of the device") increased the 'pkt_overhead' field value by LL_RESERVED_SPACE. As a side effect the generated packet size, computed as: /* Eth + IPh + UDPh + mpls */ datalen = pkt_dev->cur_pkt_size - 14 - 20 - 8 - pkt_dev->pkt_overhead; is decreased by the same value. The above changed slightly the behavior of existing pktgen users, and made the interface somewhat inconsistent. Fix it by restoring the previous pkt_overhead value and using LL_RESERVED_SPACE as extralen in skb allocation. Also, change pktgen_alloc_skb() to only partially reserve the headroom to allow the caller to prefetch from ll header start. Fixes: 879c7220e828 ("net: pktgen: Observe needed_headroom of the device") Suggested-by: Ben Greear <gree...@candelatech.com> Signed-off-by: Paolo Abeni <pab...@redhat.com> --- net/core/pktgen.c | 21 ++--- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/net/core/pktgen.c b/net/core/pktgen.c index bbd118b..5c9b397 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -2286,7 +2286,7 @@ out: static inline void set_pkt_overhead(struct pktgen_dev *pkt_dev) { - pkt_dev->pkt_overhead = LL_RESERVED_SPACE(pkt_dev->odev); + pkt_dev->pkt_overhead = 0; pkt_dev->pkt_overhead += pkt_dev->nr_labels*sizeof(u32); pkt_dev->pkt_overhead += VLAN_TAG_SIZE(pkt_dev); pkt_dev->pkt_overhead += SVLAN_TAG_SIZE(pkt_dev); @@ -2777,13 +2777,13 @@ static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb, } static struct sk_buff *pktgen_alloc_skb(struct net_device *dev, - struct pktgen_dev *pkt_dev, - unsigned int extralen) + struct pktgen_dev *pkt_dev) { + unsigned int extralen = LL_RESERVED_SPACE(dev); struct sk_buff *skb = NULL; - unsigned int size = pkt_dev->cur_pkt_size + 64 + extralen + - pkt_dev->pkt_overhead; + unsigned int size; + size = pkt_dev->cur_pkt_size + 64 + extralen + pkt_dev->pkt_overhead; if (pkt_dev->flags & F_NODE) { int node = pkt_dev->node >= 0 ? pkt_dev->node : numa_node_id(); @@ -2796,8 +2796,9 @@ static struct sk_buff *pktgen_alloc_skb(struct net_device *dev, skb = __netdev_alloc_skb(dev, size, GFP_NOWAIT); } + /* the caller prefetch from skb->data and reserve for the mac hdr */ if (likely(skb)) - skb_reserve(skb, LL_RESERVED_SPACE(dev)); + skb_reserve(skb, extralen - 16); return skb; } @@ -2830,16 +2831,14 @@ static struct sk_buff *fill_packet_ipv4(struct net_device *odev, mod_cur_headers(pkt_dev); queue_map = pkt_dev->cur_queue_map; - datalen = (odev->hard_header_len + 16) & ~0xf; - - skb = pktgen_alloc_skb(odev, pkt_dev, datalen); + skb = pktgen_alloc_skb(odev, pkt_dev); if (!skb) { sprintf(pkt_dev->result, "No memory"); return NULL; } prefetchw(skb->data); - skb_reserve(skb, datalen); + skb_reserve(skb, 16); /* Reserve for ethernet and IP header */ eth = (__u8 *) skb_push(skb, 14); @@ -2959,7 +2958,7 @@ static struct sk_buff *fill_packet_ipv6(struct net_device *odev, mod_cur_headers(pkt_dev); queue_map = pkt_dev->cur_queue_map; - skb = pktgen_alloc_skb(odev, pkt_dev, 16); + skb = pktgen_alloc_skb(odev, pkt_dev); if (!skb) { sprintf(pkt_dev->result, "No memory"); return NULL; -- 1.8.3.1
Re: [PATCH net-next] samples: Add an IPv6 '-6' option to the pktgen scripts
From: Martin KaFai Lau <ka...@fb.com> Date: Wed, 20 Jul 2016 15:48:43 -0700 > Add a '-6' option to the sample pktgen scripts for sending out > IPv6 packets. > > [root@kerneldev010.prn1 ~/pktgen]# ./pktgen_sample03_burst_single_flow.sh -i > eth0 -s 64 -d fe80::f652:14ff:fec2:a14c -m f4:52:14:c2:a1:4c -b 32 -6 > > [root@kerneldev011.prn1 ~]# tcpdump -i eth0 -nn -c3 port 9 > tcpdump: WARNING: eth0: no IPv4 address assigned > tcpdump: verbose output suppressed, use -v or -vv for full protocol decode > listening on eth0, link-type EN10MB (Ethernet), capture size 65535 bytes > 14:38:51.815297 IP6 fe80::f652:14ff:fec2:2ad2.9 > > fe80::f652:14ff:fec2:a14c.9: UDP, length 16 > 14:38:51.815311 IP6 fe80::f652:14ff:fec2:2ad2.9 > > fe80::f652:14ff:fec2:a14c.9: UDP, length 16 > 14:38:51.815313 IP6 fe80::f652:14ff:fec2:2ad2.9 > > fe80::f652:14ff:fec2:a14c.9: UDP, length 16 > > Signed-off-by: Martin KaFai Lau <ka...@fb.com> Applied.
Re: [PATCH net-next] samples: Add an IPv6 '-6' option to the pktgen scripts
On 7/20/16 3:48 PM, Martin KaFai Lau wrote: Add a '-6' option to the sample pktgen scripts for sending out IPv6 packets. [root@kerneldev010.prn1 ~/pktgen]# ./pktgen_sample03_burst_single_flow.sh -i eth0 -s 64 -d fe80::f652:14ff:fec2:a14c -m f4:52:14:c2:a1:4c -b 32 -6 [root@kerneldev011.prn1 ~]# tcpdump -i eth0 -nn -c3 port 9 tcpdump: WARNING: eth0: no IPv4 address assigned tcpdump: verbose output suppressed, use -v or -vv for full protocol decode listening on eth0, link-type EN10MB (Ethernet), capture size 65535 bytes 14:38:51.815297 IP6 fe80::f652:14ff:fec2:2ad2.9 > fe80::f652:14ff:fec2:a14c.9: UDP, length 16 14:38:51.815311 IP6 fe80::f652:14ff:fec2:2ad2.9 > fe80::f652:14ff:fec2:a14c.9: UDP, length 16 14:38:51.815313 IP6 fe80::f652:14ff:fec2:2ad2.9 > fe80::f652:14ff:fec2:a14c.9: UDP, length 16 Signed-off-by: Martin KaFai Lau <ka...@fb.com> Awesome. Thanks! Acked-by: Alexei Starovoitov <a...@kernel.org> kerneldev011.prn1 lol :)
[PATCH net-next] samples: Add an IPv6 '-6' option to the pktgen scripts
Add a '-6' option to the sample pktgen scripts for sending out IPv6 packets. [root@kerneldev010.prn1 ~/pktgen]# ./pktgen_sample03_burst_single_flow.sh -i eth0 -s 64 -d fe80::f652:14ff:fec2:a14c -m f4:52:14:c2:a1:4c -b 32 -6 [root@kerneldev011.prn1 ~]# tcpdump -i eth0 -nn -c3 port 9 tcpdump: WARNING: eth0: no IPv4 address assigned tcpdump: verbose output suppressed, use -v or -vv for full protocol decode listening on eth0, link-type EN10MB (Ethernet), capture size 65535 bytes 14:38:51.815297 IP6 fe80::f652:14ff:fec2:2ad2.9 > fe80::f652:14ff:fec2:a14c.9: UDP, length 16 14:38:51.815311 IP6 fe80::f652:14ff:fec2:2ad2.9 > fe80::f652:14ff:fec2:a14c.9: UDP, length 16 14:38:51.815313 IP6 fe80::f652:14ff:fec2:2ad2.9 > fe80::f652:14ff:fec2:a14c.9: UDP, length 16 Signed-off-by: Martin KaFai Lau <ka...@fb.com> --- samples/pktgen/parameters.sh | 7 ++- samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh | 6 ++++-- samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh| 6 ++++-- samples/pktgen/pktgen_sample01_simple.sh | 6 ++++-- samples/pktgen/pktgen_sample02_multiqueue.sh | 6 ++++-- samples/pktgen/pktgen_sample03_burst_single_flow.sh| 6 -- 6 files changed, 26 insertions(+), 11 deletions(-) diff --git a/samples/pktgen/parameters.sh b/samples/pktgen/parameters.sh index 33b70fd..f70ea7d 100644 --- a/samples/pktgen/parameters.sh +++ b/samples/pktgen/parameters.sh @@ -14,12 +14,13 @@ function usage() { echo " -b : (\$BURST) HW level bursting of SKBs" echo " -v : (\$VERBOSE) verbose" echo " -x : (\$DEBUG) debug" +echo " -6 : (\$IP6) IPv6" echo "" } ## --- Parse command line arguments / parameters --- ## echo "Commandline options:" -while getopts "s:i:d:m:t:c:b:vxh" option; do +while getopts "s:i:d:m:t:c:b:vxh6" option; do case $option in i) # interface export DEV=$OPTARG @@ -59,6 +60,10 @@ while getopts "s:i:d:m:t:c:b:vxh" option; do export DEBUG=yes info "Debug mode: DEBUG=$DEBUG" ;; + 6) + export IP6=6 + info "IP6: IP6=$IP6" + ;; h|?|*) usage; err 2 "[ERROR] Unknown parameters!!!" diff --git a/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh b/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh index cb15903..f3e1bed 100755 --- a/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh +++ b/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh @@ -34,7 +34,9 @@ root_check_run_with_sudo "$@" 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" +if [ -z "$DEST_IP" ]; then +[ -z "$IP6" ] && DEST_IP="198.18.0.42" || DEST_IP="FD00::1" +fi [ -z "$DST_MAC" ] && DST_MAC="90:e2:ba:ff:ff:ff" [ -z "$BURST" ] && BURST=1024 @@ -64,7 +66,7 @@ for ((thread = 0; thread < $THREADS; thread++)); do # Destination pg_set $dev "dst_mac $DST_MAC" -pg_set $dev "dst $DEST_IP" + pg_set $dev "dst$IP6 $DEST_IP" # Inject packet into RX path of stack pg_set $dev "xmit_mode netif_receive" diff --git a/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh b/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh index 4e4e92b..cc102e9 100755 --- a/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh +++ b/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh @@ -13,7 +13,9 @@ root_check_run_with_sudo "$@" # Parameter parsing via include source ${basedir}/parameters.sh -[ -z "$DEST_IP" ] && DEST_IP="198.18.0.42" +if [ -z "$DEST_IP" ]; then +[ -z "$IP6" ] && DEST_IP="198.18.0.42" || DEST_IP="FD00::1" +fi [ -z "$DST_MAC" ] && DST_MAC="90:e2:ba:ff:ff:ff" # Burst greater than 1 are invalid for queue_xmit mode @@ -47,7 +49,7 @@ for ((thread = 0; thread < $THREADS; thread++)); do # Destination pg_set $dev "dst_mac $DST_MAC" -pg_set $dev "dst $DEST_IP" +pg_set $dev "dst$IP6 $DEST_IP" # Inject packet into TX qdisc egress path of stack pg_set $dev "xmit_mode queue_xmit" diff --git a/samples/pktgen/pktgen_sample01_simple.sh b/samples/pktgen/pktgen_sample01_simple.sh index 8c9d318..29ef4ba 100755 --- a/samples/pktgen/pktgen_sample01_simple.sh +++ b/samples/pktgen/pktgen_sample01_simple.sh @@ -14,7 +14,9 @@ root_check_run_with_sudo "$@" source ${basedir}/parameters.sh # # Set some default params, if they didn't get set -[ -z "$DEST_IP&
Re: [net-next PATCH 0/3] pktgen samples: new scripts and removing older samples
From: Jesper Dangaard Brouer <bro...@redhat.com> Date: Wed, 13 Jul 2016 22:05:59 +0200 > This patchset is adding some pktgen sample scripts that I've been > using for a while[1], and they seams to relevant for more people. > > Patchset also remove some of the older style pktgen samples. > > [1] https://github.com/netoptimizer/network-testing/tree/master/pktgen Series applied, thanks Jesper.
Re: [net-next PATCH 2/3] pktgen: add sample script pktgen_sample05_flow_per_thread.sh
On Wed, Jul 13, 2016 at 10:06:10PM +0200, Jesper Dangaard Brouer wrote: > This pktgen sample script is useful for scalability testing a > receiver. The script will simply generate one flow per > thread (option -t N) using the thread number as part of the > source IP-address. > > The single flow sample (pktgen_sample03_burst_single_flow.sh) > have become quite popular, but it is important that developers > also make sure to benchmark scalability of multiple receive > queues. > > Signed-off-by: Jesper Dangaard Brouer <bro...@redhat.com> > --- > samples/pktgen/pktgen_sample05_flow_per_thread.sh | 81 > + > 1 file changed, 81 insertions(+) ... > +# Setup source IP-addresses based on thread number > +pg_set $dev "src_min 198.18.$((thread+1)).1" > +pg_set $dev "src_max 198.18.$((thread+1)).1" I have similar script that uses udp_src_min/max to change port number, since port is easier to match on the target host and we don't use ipv4 ;) but this script is also good improvement. Thanks! Acked-by: Alexei Starovoitov <a...@kernel.org>
[net-next PATCH 0/3] pktgen samples: new scripts and removing older samples
This patchset is adding some pktgen sample scripts that I've been using for a while[1], and they seams to relevant for more people. Patchset also remove some of the older style pktgen samples. [1] https://github.com/netoptimizer/network-testing/tree/master/pktgen --- Jesper Dangaard Brouer (3): pktgen: add sample script pktgen_sample04_many_flows.sh pktgen: add sample script pktgen_sample05_flow_per_thread.sh pktgen: remove sample script pktgen.conf-1-1-rdos samples/pktgen/pktgen.conf-1-1-flows | 67 --- samples/pktgen/pktgen.conf-1-1-rdos | 64 -- samples/pktgen/pktgen_sample04_many_flows.sh | 93 + samples/pktgen/pktgen_sample05_flow_per_thread.sh | 81 ++ 4 files changed, 174 insertions(+), 131 deletions(-) delete mode 100755 samples/pktgen/pktgen.conf-1-1-flows delete mode 100755 samples/pktgen/pktgen.conf-1-1-rdos create mode 100755 samples/pktgen/pktgen_sample04_many_flows.sh create mode 100755 samples/pktgen/pktgen_sample05_flow_per_thread.sh --
[net-next PATCH 2/3] pktgen: add sample script pktgen_sample05_flow_per_thread.sh
This pktgen sample script is useful for scalability testing a receiver. The script will simply generate one flow per thread (option -t N) using the thread number as part of the source IP-address. The single flow sample (pktgen_sample03_burst_single_flow.sh) have become quite popular, but it is important that developers also make sure to benchmark scalability of multiple receive queues. Signed-off-by: Jesper Dangaard Brouer <bro...@redhat.com> --- samples/pktgen/pktgen_sample05_flow_per_thread.sh | 81 + 1 file changed, 81 insertions(+) create mode 100755 samples/pktgen/pktgen_sample05_flow_per_thread.sh diff --git a/samples/pktgen/pktgen_sample05_flow_per_thread.sh b/samples/pktgen/pktgen_sample05_flow_per_thread.sh new file mode 100755 index ..32ad818e2829 --- /dev/null +++ b/samples/pktgen/pktgen_sample05_flow_per_thread.sh @@ -0,0 +1,81 @@ +#!/bin/bash +# +# Script will generate one flow per thread (-t N) +# - Same destination IP +# - Fake source IPs for each flow (fixed based on thread number) +# +# Useful for scale testing on receiver, to see whether silo'ing flows +# works and scales. For optimal scalability (on receiver) each +# separate-flow should not access shared variables/data. This script +# helps magnify any of these scaling issues by overloading the receiver. +# +basedir=`dirname $0` +source ${basedir}/functions.sh +root_check_run_with_sudo "$@" + +# Parameter parsing via include +source ${basedir}/parameters.sh +# Set some default params, if they didn't get set +[ -z "$DEST_IP" ] && DEST_IP="198.18.0.42" +[ -z "$DST_MAC" ] && DST_MAC="90:e2:ba:ff:ff:ff" +[ -z "$CLONE_SKB" ] && CLONE_SKB="0" +[ -z "$BURST" ] && BURST=32 + + +# Base Config +DELAY="0" # Zero means max speed +COUNT="0" # 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 +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 +pg_set $dev "flag QUEUE_MAP_CPU" +pg_set $dev "count $COUNT" +pg_set $dev "clone_skb $CLONE_SKB" +pg_set $dev "pkt_size $PKT_SIZE" +pg_set $dev "delay $DELAY" +pg_set $dev "flag NO_TIMESTAMP" + +# Single destination +pg_set $dev "dst_mac $DST_MAC" +pg_set $dev "dst $DEST_IP" + +# Setup source IP-addresses based on thread number +pg_set $dev "src_min 198.18.$((thread+1)).1" +pg_set $dev "src_max 198.18.$((thread+1)).1" + +# Setup burst, for easy testing -b 0 disable bursting +# (internally in pktgen default and minimum burst=1) +if [[ ${BURST} -ne 0 ]]; then + pg_set $dev "burst $BURST" + else + info "$dev: Not using burst" +fi + +done + +# Run if user hits control-c +function print_result() { +# Print results +for ((thread = 0; thread < $THREADS; thread++)); do + dev=${DEV}@${thread} + echo "Device: $dev" + cat /proc/net/pktgen/$dev | grep -A2 "Result:" +done +} +# trap keyboard interrupt (Ctrl-C) +trap true SIGINT + +echo "Running... ctrl^C to stop" >&2 +pg_ctrl "start" + +print_result
[net-next PATCH 3/3] pktgen: remove sample script pktgen.conf-1-1-rdos
Removing the pktgen sample script pktgen.conf-1-1-rdos, because it does not contain anything that is not covered by the other and newer style sample scripts. Signed-off-by: Jesper Dangaard Brouer <bro...@redhat.com> --- samples/pktgen/pktgen.conf-1-1-rdos | 64 --- 1 file changed, 64 deletions(-) delete mode 100755 samples/pktgen/pktgen.conf-1-1-rdos diff --git a/samples/pktgen/pktgen.conf-1-1-rdos b/samples/pktgen/pktgen.conf-1-1-rdos deleted file mode 100755 index c7553be49b80.. --- a/samples/pktgen/pktgen.conf-1-1-rdos +++ /dev/null @@ -1,64 +0,0 @@ -#!/bin/bash - -#modprobe pktgen - - -function pgset() { -local result - -echo $1 > $PGDEV - -result=`cat $PGDEV | fgrep "Result: OK:"` -if [ "$result" = "" ]; then - cat $PGDEV | fgrep Result: -fi -} - -# Config Start Here --- - - -# thread config -# Each CPU has its own thread. One CPU example. We add eth1. - -PGDEV=/proc/net/pktgen/kpktgend_0 - echo "Removing all devices" - pgset "rem_device_all" - echo "Adding eth1" - pgset "add_device eth1" - - -# device config -# delay 0 - -# We need to do alloc for every skb since we cannot clone here. - -CLONE_SKB="clone_skb 0" -# NIC adds 4 bytes CRC -PKT_SIZE="pkt_size 60" - -# COUNT 0 means forever -#COUNT="count 0" -COUNT="count 1000" -DELAY="delay 0" - -PGDEV=/proc/net/pktgen/eth1 - echo "Configuring $PGDEV" - pgset "$COUNT" - pgset "$CLONE_SKB" - pgset "$PKT_SIZE" - pgset "$DELAY" - # Random address with in the min-max range - pgset "flag IPDST_RND" - pgset "dst_min 10.0.0.0" - pgset "dst_max 10.255.255.255" - - pgset "dst_mac 00:04:23:08:91:dc" - -# Time to run -PGDEV=/proc/net/pktgen/pgctrl - - echo "Running... ctrl^C to stop" - trap true INT - pgset "start" - echo "Done" - cat /proc/net/pktgen/eth1
[net-next PATCH 1/3] pktgen: add sample script pktgen_sample04_many_flows.sh
Adding a pktgen sample script that demonstrates how to use pktgen for simulating flows. Script will generate a certain number of concurrent flows ($FLOWS) and each flow will contain $FLOWLEN packets, which will be send back-to-back, before switching to a new flow, due to flag FLOW_SEQ. This script obsoletes the old sample script 'pktgen.conf-1-1-flows', which is removed. Signed-off-by: Jesper Dangaard Brouer <bro...@redhat.com> --- samples/pktgen/pktgen.conf-1-1-flows | 67 --- samples/pktgen/pktgen_sample04_many_flows.sh | 93 ++ 2 files changed, 93 insertions(+), 67 deletions(-) delete mode 100755 samples/pktgen/pktgen.conf-1-1-flows create mode 100755 samples/pktgen/pktgen_sample04_many_flows.sh diff --git a/samples/pktgen/pktgen.conf-1-1-flows b/samples/pktgen/pktgen.conf-1-1-flows deleted file mode 100755 index 081749c9707d.. --- a/samples/pktgen/pktgen.conf-1-1-flows +++ /dev/null @@ -1,67 +0,0 @@ -#!/bin/bash - -#modprobe pktgen - - -function pgset() { -local result - -echo $1 > $PGDEV - -result=`cat $PGDEV | fgrep "Result: OK:"` -if [ "$result" = "" ]; then - cat $PGDEV | fgrep Result: -fi -} - -# Config Start Here --- - - -# thread config -# Each CPU has its own thread. One CPU example. We add eth1. - -PGDEV=/proc/net/pktgen/kpktgend_0 - echo "Removing all devices" - pgset "rem_device_all" - echo "Adding eth1" - pgset "add_device eth1" - - -# device config -# delay 0 -# We need to do alloc for every skb since we cannot clone here. - -CLONE_SKB="clone_skb 0" -# NIC adds 4 bytes CRC -PKT_SIZE="pkt_size 60" - -# COUNT 0 means forever -#COUNT="count 0" -COUNT="count 1000" -DELAY="delay 0" - -PGDEV=/proc/net/pktgen/eth1 - echo "Configuring $PGDEV" - pgset "$COUNT" - pgset "$CLONE_SKB" - pgset "$PKT_SIZE" - pgset "$DELAY" - # Random address with in the min-max range - pgset "flag IPDST_RND" - pgset "dst_min 10.0.0.0" - pgset "dst_max 10.255.255.255" - - # 8k Concurrent flows at 4 pkts - pgset "flows 8192" - pgset "flowlen 4" - - pgset "dst_mac 00:04:23:08:91:dc" - -# Time to run -PGDEV=/proc/net/pktgen/pgctrl - - echo "Running... ctrl^C to stop" - trap true INT - pgset "start" - echo "Done" - cat /proc/net/pktgen/eth1 diff --git a/samples/pktgen/pktgen_sample04_many_flows.sh b/samples/pktgen/pktgen_sample04_many_flows.sh new file mode 100755 index ..f60412e445bb --- /dev/null +++ b/samples/pktgen/pktgen_sample04_many_flows.sh @@ -0,0 +1,93 @@ +#!/bin/bash +# +# Script example for many flows testing +# +# Number of simultaneous flows limited by variable $FLOWS +# and number of packets per flow controlled by variable $FLOWLEN +# +basedir=`dirname $0` +source ${basedir}/functions.sh +root_check_run_with_sudo "$@" + +# Parameter parsing via include +source ${basedir}/parameters.sh +# Set some default params, if they didn't get set +[ -z "$DEST_IP" ] && DEST_IP="198.18.0.42" +[ -z "$DST_MAC" ] && DST_MAC="90:e2:ba:ff:ff:ff" +[ -z "$CLONE_SKB" ] && CLONE_SKB="0" + +# NOTICE: Script specific settings +# === +# Limiting the number of concurrent flows ($FLOWS) +# and also set how many packets each flow contains ($FLOWLEN) +# +[ -z "$FLOWS" ] && FLOWS="8000" +[ -z "$FLOWLEN" ] && FLOWLEN="10" + +# Base Config +DELAY="0" # Zero means max speed +COUNT="0" # Zero means indefinitely + +if [[ -n "$BURST" ]]; then +err 1 "Bursting not supported for this mode" +fi + +# 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 +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 +pg_set $dev "flag QUEUE_MAP_CPU" +pg_set $dev "count $COUNT" +pg_set $dev "clone_skb $CLONE_SKB" +pg_set $dev "pkt_size $PKT_SIZE" +pg_set $dev "delay $DELAY" +pg_set $dev "flag NO_TIMESTAMP" + +# Single destination +pg_set $dev "dst_mac $DST_MAC" +pg_set $dev "dst $DEST_IP" + +# Randomize source IP-addresses +pg_set $dev "flag IPSRC_RND" +pg_set $dev "src_min 198.18.0.0" +pg_set $dev "src_max 198.19.255.255" + +# Limit number of flows (max 65535)
Re: [net-next PATCH v2 2/2] net: samples: pktgen mode samples/tests for qdisc layer
From: John Fastabend <john.fastab...@gmail.com> Date: Sat, 02 Jul 2016 14:13:13 -0700 > 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.fastab...@intel.com> Applied.
Re: [net-next PATCH v2 1/2] net: pktgen: support injecting packets for qdisc testing
From: John Fastabend <john.fastab...@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.fastab...@intel.com> Applied.
[net-next PATCH v2 1/2] net: pktgen: support injecting packets for qdisc testing
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.fastab...@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_RECEIVE1 /* 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(_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);
[net-next PATCH v2 2/2] net: samples: pktgen mode samples/tests for qdisc layer
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.fastab...@intel.com> --- .../pktgen/pktgen_bench_xmit_mode_queue_xmit.sh| 66 1 file changed, 66 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 000..4e4e92b --- /dev/null +++ b/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh @@ -0,0 +1,66 @@ +#!/bin/bash +# +# Benchmark script: +# - developed for benchmarking egress qdisc path, derived (more +#like cut'n'pasted) 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 +[ -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 for queue_xmit mode +if [[ -n "$BURST" ]]; then +err 1 "Bursting not supported for this mode" +fi + +# Base Config +DELAY="0"# Zero means max speed +COUNT="1000" # 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 TX qdisc egress path of stack +pg_set $dev "xmit_mode queue_xmit" +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
Re: [net-next PATCH 1/2] net: pktgen: support injecting packets for qdisc testing
On 16-07-01 05:56 AM, Jamal Hadi Salim wrote: > On 16-06-30 12:53 PM, John Fastabend wrote: >> On 16-06-30 03:21 AM, Jamal Hadi Salim wrote: >>> On 16-06-29 03:47 PM, John Fastabend wrote: > >> >> Taking a look at the link couple differences exist. First the patch >> linked does a 'netif_xmit_frozen_or_drv_stopped(txq)' check but this >> really shouldn't be needed it is handled by the sch_direct_xmit() >> logic in ./net/sched >> >> Also in this patch I made it way more conservative on when to back >> off then my original patch and now its closer to the one linked except >> I also back off with return code NET_XMIT_CN. >> >> As for clones what is the concern exactly? We allow them through the >> ingress pktgen mode that can hit classifiers and I don't see any issues >> testing with them. >> > > I sent you that because we discussed back then and you thought > there was something different and useful. > I dont remember why it was important to avoid clones - maybe i had a > bug. I thought we discussed it (I will try to dig into the machine > used for testing). > Anyways, doesnt matter if you tested and it works fine. Ah I was originally ignoring more of the error cases and continuing to try and enqueue packets into the qdisc even though it was returning error codes. You suggested we should abort in the error case and I adopted your idea in this patch. .John > > cheers, > jamal >
Re: [net-next PATCH 1/2] net: pktgen: support injecting packets for qdisc testing
On 16-06-30 12:53 PM, John Fastabend wrote: On 16-06-30 03:21 AM, Jamal Hadi Salim wrote: On 16-06-29 03:47 PM, John Fastabend wrote: Taking a look at the link couple differences exist. First the patch linked does a 'netif_xmit_frozen_or_drv_stopped(txq)' check but this really shouldn't be needed it is handled by the sch_direct_xmit() logic in ./net/sched Also in this patch I made it way more conservative on when to back off then my original patch and now its closer to the one linked except I also back off with return code NET_XMIT_CN. As for clones what is the concern exactly? We allow them through the ingress pktgen mode that can hit classifiers and I don't see any issues testing with them. I sent you that because we discussed back then and you thought there was something different and useful. I dont remember why it was important to avoid clones - maybe i had a bug. I thought we discussed it (I will try to dig into the machine used for testing). Anyways, doesnt matter if you tested and it works fine. cheers, jamal
Re: [net-next PATCH v2 1/2] net: pktgen: support injecting packets for qdisc testing
On 16-06-30 01:37 AM, Jesper Dangaard Brouer wrote: > On Wed, 29 Jun 2016 13:03:06 -0700 > John Fastabend <john.fastab...@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.fastab...@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, _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
Re: [net-next PATCH 1/2] net: pktgen: support injecting packets for qdisc testing
On 16-06-30 03:21 AM, Jamal Hadi Salim wrote: > On 16-06-29 03:47 PM, John Fastabend 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.fastab...@intel.com> >> --- [...] > > Acked-by: Jamal Hadi Salim <j...@mojatatu.com> > > In travel mode, dont have much cycles right now - but can you review > again: > http://www.spinics.net/lists/netdev/msg359545.html > I think you should disallow clone for example and i wasnt sure if you > covered all error scenarios etc. > Taking a look at the link couple differences exist. First the patch linked does a 'netif_xmit_frozen_or_drv_stopped(txq)' check but this really shouldn't be needed it is handled by the sch_direct_xmit() logic in ./net/sched Also in this patch I made it way more conservative on when to back off then my original patch and now its closer to the one linked except I also back off with return code NET_XMIT_CN. As for clones what is the concern exactly? We allow them through the ingress pktgen mode that can hit classifiers and I don't see any issues testing with them. .John > cheers, > jamal
Re: [net-next PATCH v2 2/2] net: samples: pktgen mode samples/tests for qdisc layer
On 16-06-30 01:23 AM, Jesper Dangaard Brouer wrote: > On Wed, 29 Jun 2016 13:03:26 -0700 > John Fastabend <john.fastab...@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="1000" # 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.
Re: [net-next PATCH 1/2] net: pktgen: support injecting packets for qdisc testing
On 16-06-29 03:47 PM, John Fastabend 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.fastab...@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, _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); Acked-by: Jamal Hadi Salim <j...@mojatatu.com> In travel mode, dont have much cycles right now - but can you review again: http://www.spinics.net/lists/netdev/msg359545.html I think you should disallow clone for example and i wasnt sure if you covered all error scenarios etc. cheers, jamal