[v2,4/9] examples/ipsec-secgw: add stats interval argument
Checks
Commit Message
Add -t for stats screen update interval, disabled by default.
Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
---
doc/guides/sample_app_ug/ipsec_secgw.rst | 5 ++++
examples/ipsec-secgw/ipsec-secgw.c | 29 ++++++++++++++++--------
examples/ipsec-secgw/ipsec-secgw.h | 15 ------------
3 files changed, 25 insertions(+), 24 deletions(-)
Comments
On 9/15/2021 7:15 PM, Radu Nicolau wrote:
> Add -t for stats screen update interval, disabled by default.
>
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> ---
> doc/guides/sample_app_ug/ipsec_secgw.rst | 5 ++++
> examples/ipsec-secgw/ipsec-secgw.c | 29 ++++++++++++++++--------
> examples/ipsec-secgw/ipsec-secgw.h | 15 ------------
> 3 files changed, 25 insertions(+), 24 deletions(-)
>
> diff --git a/doc/guides/sample_app_ug/ipsec_secgw.rst b/doc/guides/sample_app_ug/ipsec_secgw.rst
> index 20bc1e6bc4..0d55e74022 100644
> --- a/doc/guides/sample_app_ug/ipsec_secgw.rst
> +++ b/doc/guides/sample_app_ug/ipsec_secgw.rst
> @@ -127,6 +127,7 @@ The application has a number of command line options::
> -p PORTMASK -P -u PORTMASK -j FRAMESIZE
> -l -w REPLAY_WINDOW_SIZE -e -a
> -c SAD_CACHE_SIZE
> + -t STATISTICS_INTERVAL
> -s NUMBER_OF_MBUFS_IN_PACKET_POOL
> -f CONFIG_FILE_PATH
> --config (port,queue,lcore)[,(port,queue,lcore)]
> @@ -176,6 +177,10 @@ Where:
> Zero value disables cache.
> Default value: 128.
>
> +* ``-t``: specifies the statistics screen update interval. If set to zero or
> + omitted statistics screen is disabled.
> + Default value: 0.
> +
> * ``-s``: sets number of mbufs in packet pool, if not provided number of mbufs
> will be calculated based on number of cores, eth ports and crypto queues.
>
> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
> index 265fff4bef..60b25be872 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.c
> +++ b/examples/ipsec-secgw/ipsec-secgw.c
> @@ -181,6 +181,7 @@ static uint32_t frag_tbl_sz;
> static uint32_t frame_buf_size = RTE_MBUF_DEFAULT_BUF_SIZE;
> static uint32_t mtu_size = RTE_ETHER_MTU;
> static uint64_t frag_ttl_ns = MAX_FRAG_TTL_NS;
> +static uint32_t stats_interval;
>
> /* application wide librte_ipsec/SA parameters */
> struct app_sa_prm app_sa_prm = {
> @@ -292,7 +293,6 @@ adjust_ipv6_pktlen(struct rte_mbuf *m, const struct rte_ipv6_hdr *iph,
> }
> }
>
> -#if (STATS_INTERVAL > 0)
>
> /* Print out statistics on packet distribution */
> static void
> @@ -352,9 +352,8 @@ print_stats_cb(__rte_unused void *param)
> total_packets_dropped);
> printf("\n====================================================\n");
>
> - rte_eal_alarm_set(STATS_INTERVAL * US_PER_S, print_stats_cb, NULL);
> + rte_eal_alarm_set(stats_interval * US_PER_S, print_stats_cb, NULL);
> }
> -#endif /* STATS_INTERVAL */
>
> static inline void
> prepare_one_packet(struct rte_mbuf *pkt, struct ipsec_traffic *t)
> @@ -1435,6 +1434,7 @@ print_usage(const char *prgname)
> " [-e]"
> " [-a]"
> " [-c]"
> + " [-t STATS_INTERVAL]"
> " [-s NUMBER_OF_MBUFS_IN_PKT_POOL]"
> " -f CONFIG_FILE"
> " --config (port,queue,lcore)[,(port,queue,lcore)]"
> @@ -1459,6 +1459,8 @@ print_usage(const char *prgname)
> " -a enables SA SQN atomic behaviour\n"
> " -c specifies inbound SAD cache size,\n"
> " zero value disables the cache (default value: 128)\n"
> + " -t specifies statistics screen update interval,\n"
> + " zero disables statistics screen (default value: 0)\n"
> " -s number of mbufs in packet pool, if not specified number\n"
> " of mbufs will be calculated based on number of cores,\n"
> " ports and crypto queues\n"
> @@ -1666,7 +1668,7 @@ parse_args(int32_t argc, char **argv, struct eh_conf *eh_conf)
>
> argvopt = argv;
>
> - while ((opt = getopt_long(argc, argvopt, "aelp:Pu:f:j:w:c:s:",
> + while ((opt = getopt_long(argc, argvopt, "aelp:Pu:f:j:w:c:t:s:",
> lgopts, &option_index)) != EOF) {
>
> switch (opt) {
> @@ -1747,6 +1749,15 @@ parse_args(int32_t argc, char **argv, struct eh_conf *eh_conf)
> }
> app_sa_prm.cache_sz = ret;
> break;
> + case 't':
> + ret = parse_decimal(optarg);
> + if (ret < 0) {
> + printf("Invalid interval value: %s\n", optarg);
> + print_usage(prgname);
> + return -1;
> + }
> + stats_interval = ret;
> + break;
> case CMD_LINE_OPT_CONFIG_NUM:
> ret = parse_config(optarg);
> if (ret) {
> @@ -3350,11 +3361,11 @@ main(int32_t argc, char **argv)
>
> check_all_ports_link_status(enabled_port_mask);
>
> -#if (STATS_INTERVAL > 0)
> - rte_eal_alarm_set(STATS_INTERVAL * US_PER_S, print_stats_cb, NULL);
> -#else
> - RTE_LOG(INFO, IPSEC, "Stats display disabled\n");
> -#endif /* STATS_INTERVAL */
> + if (stats_interval > 0)
> + rte_eal_alarm_set(stats_interval * US_PER_S,
> + print_stats_cb, NULL);
> + else
> + RTE_LOG(INFO, IPSEC, "Stats display disabled\n");
>
> /* launch per-lcore init on every lcore */
> rte_eal_mp_remote_launch(ipsec_launch_one_lcore, eh_conf, CALL_MAIN);
> diff --git a/examples/ipsec-secgw/ipsec-secgw.h b/examples/ipsec-secgw/ipsec-secgw.h
> index f3082a1037..de9f382742 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.h
> +++ b/examples/ipsec-secgw/ipsec-secgw.h
> @@ -6,9 +6,6 @@
>
> #include <stdbool.h>
>
> -#ifndef STATS_INTERVAL
> -#define STATS_INTERVAL 0
> -#endif
>
> #define NB_SOCKETS 4
>
> @@ -144,38 +141,26 @@ is_unprotected_port(uint16_t port_id)
> static inline void
> core_stats_update_rx(int n)
> {
> -#if (STATS_INTERVAL > 0)
> int lcore_id = rte_lcore_id();
> core_statistics[lcore_id].rx += n;
> core_statistics[lcore_id].rx_call++;
> if (n == MAX_PKT_BURST)
> core_statistics[lcore_id].burst_rx += n;
> -#else
> - RTE_SET_USED(n);
> -#endif /* STATS_INTERVAL */
> }
>
> static inline void
> core_stats_update_tx(int n)
> {
> -#if (STATS_INTERVAL > 0)
> int lcore_id = rte_lcore_id();
> core_statistics[lcore_id].tx += n;
> core_statistics[lcore_id].tx_call++;
> -#else
> - RTE_SET_USED(n);
> -#endif /* STATS_INTERVAL */
> }
>
> static inline void
> core_stats_update_drop(int n)
> {
> -#if (STATS_INTERVAL > 0)
> int lcore_id = rte_lcore_id();
> core_statistics[lcore_id].dropped += n;
> -#else
> - RTE_SET_USED(n);
> -#endif /* STATS_INTERVAL */
> }
>
> /* helper routine to free bulk of packets */
Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
Hi Radu,
Making stats dynamic would have a perf cost. That was the reason it was introduced as a compile time option. Why do we need it changed?
Also, it looks like this patch is only disabling stats printing when timeout is 0. I don't see stats collection being conditional. Stats collection would also have perf impact, right?
Thanks,
Anoob
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Radu Nicolau
> Sent: Wednesday, September 15, 2021 7:15 PM
> To: Radu Nicolau <radu.nicolau@intel.com>; Akhil Goyal
> <gakhil@marvell.com>
> Cc: dev@dpdk.org; declan.doherty@intel.com;
> hemant.agrawal@oss.nxp.com
> Subject: [EXT] [dpdk-dev] [PATCH v2 4/9] examples/ipsec-secgw: add stats
> interval argument
>
> External Email
>
> ----------------------------------------------------------------------
> Add -t for stats screen update interval, disabled by default.
>
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> ---
> doc/guides/sample_app_ug/ipsec_secgw.rst | 5 ++++
> examples/ipsec-secgw/ipsec-secgw.c | 29 ++++++++++++++++--------
> examples/ipsec-secgw/ipsec-secgw.h | 15 ------------
> 3 files changed, 25 insertions(+), 24 deletions(-)
>
> diff --git a/doc/guides/sample_app_ug/ipsec_secgw.rst
> b/doc/guides/sample_app_ug/ipsec_secgw.rst
> index 20bc1e6bc4..0d55e74022 100644
> --- a/doc/guides/sample_app_ug/ipsec_secgw.rst
> +++ b/doc/guides/sample_app_ug/ipsec_secgw.rst
> @@ -127,6 +127,7 @@ The application has a number of command line
> options::
> -p PORTMASK -P -u PORTMASK -j FRAMESIZE
> -l -w REPLAY_WINDOW_SIZE -e -a
> -c SAD_CACHE_SIZE
> + -t STATISTICS_INTERVAL
> -s NUMBER_OF_MBUFS_IN_PACKET_POOL
> -f CONFIG_FILE_PATH
> --config (port,queue,lcore)[,(port,queue,lcore)]
> @@ -176,6 +177,10 @@ Where:
> Zero value disables cache.
> Default value: 128.
>
> +* ``-t``: specifies the statistics screen update interval. If set to zero or
> + omitted statistics screen is disabled.
> + Default value: 0.
> +
> * ``-s``: sets number of mbufs in packet pool, if not provided number of
> mbufs
> will be calculated based on number of cores, eth ports and crypto queues.
>
> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-
> secgw/ipsec-secgw.c
> index 265fff4bef..60b25be872 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.c
> +++ b/examples/ipsec-secgw/ipsec-secgw.c
> @@ -181,6 +181,7 @@ static uint32_t frag_tbl_sz; static uint32_t
> frame_buf_size = RTE_MBUF_DEFAULT_BUF_SIZE; static uint32_t mtu_size
> = RTE_ETHER_MTU; static uint64_t frag_ttl_ns = MAX_FRAG_TTL_NS;
> +static uint32_t stats_interval;
>
> /* application wide librte_ipsec/SA parameters */ struct app_sa_prm
> app_sa_prm = { @@ -292,7 +293,6 @@ adjust_ipv6_pktlen(struct rte_mbuf
> *m, const struct rte_ipv6_hdr *iph,
> }
> }
>
> -#if (STATS_INTERVAL > 0)
>
> /* Print out statistics on packet distribution */ static void @@ -352,9 +352,8
> @@ print_stats_cb(__rte_unused void *param)
> total_packets_dropped);
>
> printf("\n=============================================
> =======\n");
>
> - rte_eal_alarm_set(STATS_INTERVAL * US_PER_S, print_stats_cb,
> NULL);
> + rte_eal_alarm_set(stats_interval * US_PER_S, print_stats_cb, NULL);
> }
> -#endif /* STATS_INTERVAL */
>
> static inline void
> prepare_one_packet(struct rte_mbuf *pkt, struct ipsec_traffic *t) @@ -
> 1435,6 +1434,7 @@ print_usage(const char *prgname)
> " [-e]"
> " [-a]"
> " [-c]"
> + " [-t STATS_INTERVAL]"
> " [-s NUMBER_OF_MBUFS_IN_PKT_POOL]"
> " -f CONFIG_FILE"
> " --config (port,queue,lcore)[,(port,queue,lcore)]"
> @@ -1459,6 +1459,8 @@ print_usage(const char *prgname)
> " -a enables SA SQN atomic behaviour\n"
> " -c specifies inbound SAD cache size,\n"
> " zero value disables the cache (default value: 128)\n"
> + " -t specifies statistics screen update interval,\n"
> + " zero disables statistics screen (default value: 0)\n"
> " -s number of mbufs in packet pool, if not specified
> number\n"
> " of mbufs will be calculated based on number of cores,\n"
> " ports and crypto queues\n"
> @@ -1666,7 +1668,7 @@ parse_args(int32_t argc, char **argv, struct
> eh_conf *eh_conf)
>
> argvopt = argv;
>
> - while ((opt = getopt_long(argc, argvopt, "aelp:Pu:f:j:w:c:s:",
> + while ((opt = getopt_long(argc, argvopt, "aelp:Pu:f:j:w:c:t:s:",
> lgopts, &option_index)) != EOF) {
>
> switch (opt) {
> @@ -1747,6 +1749,15 @@ parse_args(int32_t argc, char **argv, struct
> eh_conf *eh_conf)
> }
> app_sa_prm.cache_sz = ret;
> break;
> + case 't':
> + ret = parse_decimal(optarg);
> + if (ret < 0) {
> + printf("Invalid interval value: %s\n", optarg);
> + print_usage(prgname);
> + return -1;
> + }
> + stats_interval = ret;
> + break;
> case CMD_LINE_OPT_CONFIG_NUM:
> ret = parse_config(optarg);
> if (ret) {
> @@ -3350,11 +3361,11 @@ main(int32_t argc, char **argv)
>
> check_all_ports_link_status(enabled_port_mask);
>
> -#if (STATS_INTERVAL > 0)
> - rte_eal_alarm_set(STATS_INTERVAL * US_PER_S, print_stats_cb,
> NULL);
> -#else
> - RTE_LOG(INFO, IPSEC, "Stats display disabled\n");
> -#endif /* STATS_INTERVAL */
> + if (stats_interval > 0)
> + rte_eal_alarm_set(stats_interval * US_PER_S,
> + print_stats_cb, NULL);
> + else
> + RTE_LOG(INFO, IPSEC, "Stats display disabled\n");
>
> /* launch per-lcore init on every lcore */
> rte_eal_mp_remote_launch(ipsec_launch_one_lcore, eh_conf,
> CALL_MAIN); diff --git a/examples/ipsec-secgw/ipsec-secgw.h
> b/examples/ipsec-secgw/ipsec-secgw.h
> index f3082a1037..de9f382742 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.h
> +++ b/examples/ipsec-secgw/ipsec-secgw.h
> @@ -6,9 +6,6 @@
>
> #include <stdbool.h>
>
> -#ifndef STATS_INTERVAL
> -#define STATS_INTERVAL 0
> -#endif
>
> #define NB_SOCKETS 4
>
> @@ -144,38 +141,26 @@ is_unprotected_port(uint16_t port_id) static inline
> void core_stats_update_rx(int n) { -#if (STATS_INTERVAL > 0)
> int lcore_id = rte_lcore_id();
> core_statistics[lcore_id].rx += n;
> core_statistics[lcore_id].rx_call++;
> if (n == MAX_PKT_BURST)
> core_statistics[lcore_id].burst_rx += n; -#else
> - RTE_SET_USED(n);
> -#endif /* STATS_INTERVAL */
> }
>
> static inline void
> core_stats_update_tx(int n)
> {
> -#if (STATS_INTERVAL > 0)
> int lcore_id = rte_lcore_id();
> core_statistics[lcore_id].tx += n;
> core_statistics[lcore_id].tx_call++;
> -#else
> - RTE_SET_USED(n);
> -#endif /* STATS_INTERVAL */
> }
>
> static inline void
> core_stats_update_drop(int n)
> {
> -#if (STATS_INTERVAL > 0)
> int lcore_id = rte_lcore_id();
> core_statistics[lcore_id].dropped += n; -#else
> - RTE_SET_USED(n);
> -#endif /* STATS_INTERVAL */
> }
>
> /* helper routine to free bulk of packets */
> --
> 2.25.1
Hi Anoob,
On 9/16/2021 10:30 AM, Anoob Joseph wrote:
> Hi Radu,
>
> Making stats dynamic would have a perf cost. That was the reason it was introduced as a compile time option. Why do we need it changed?
We changed it for two reasons, for a better UX and for the telemetry
feature that is introduced with patch 3.
>
> Also, it looks like this patch is only disabling stats printing when timeout is 0. I don't see stats collection being conditional. Stats collection would also have perf impact, right?
I don't think there is a measurable performance impact because these are
per burst calls.
Regards,
Radu
>
> Thanks,
> Anoob
>
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Radu Nicolau
>> Sent: Wednesday, September 15, 2021 7:15 PM
>> To: Radu Nicolau <radu.nicolau@intel.com>; Akhil Goyal
>> <gakhil@marvell.com>
>> Cc: dev@dpdk.org; declan.doherty@intel.com;
>> hemant.agrawal@oss.nxp.com
>> Subject: [EXT] [dpdk-dev] [PATCH v2 4/9] examples/ipsec-secgw: add stats
>> interval argument
>>
>> External Email
>>
>> ----------------------------------------------------------------------
>> Add -t for stats screen update interval, disabled by default.
>>
>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
>> ---
>> doc/guides/sample_app_ug/ipsec_secgw.rst | 5 ++++
>> examples/ipsec-secgw/ipsec-secgw.c | 29 ++++++++++++++++--------
>> examples/ipsec-secgw/ipsec-secgw.h | 15 ------------
>> 3 files changed, 25 insertions(+), 24 deletions(-)
>>
>> diff --git a/doc/guides/sample_app_ug/ipsec_secgw.rst
>> b/doc/guides/sample_app_ug/ipsec_secgw.rst
>> index 20bc1e6bc4..0d55e74022 100644
>> --- a/doc/guides/sample_app_ug/ipsec_secgw.rst
>> +++ b/doc/guides/sample_app_ug/ipsec_secgw.rst
>> @@ -127,6 +127,7 @@ The application has a number of command line
>> options::
>> -p PORTMASK -P -u PORTMASK -j FRAMESIZE
>> -l -w REPLAY_WINDOW_SIZE -e -a
>> -c SAD_CACHE_SIZE
>> + -t STATISTICS_INTERVAL
>> -s NUMBER_OF_MBUFS_IN_PACKET_POOL
>> -f CONFIG_FILE_PATH
>> --config (port,queue,lcore)[,(port,queue,lcore)]
>> @@ -176,6 +177,10 @@ Where:
>> Zero value disables cache.
>> Default value: 128.
>>
>> +* ``-t``: specifies the statistics screen update interval. If set to zero or
>> + omitted statistics screen is disabled.
>> + Default value: 0.
>> +
>> * ``-s``: sets number of mbufs in packet pool, if not provided number of
>> mbufs
>> will be calculated based on number of cores, eth ports and crypto queues.
>>
>> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-
>> secgw/ipsec-secgw.c
>> index 265fff4bef..60b25be872 100644
>> --- a/examples/ipsec-secgw/ipsec-secgw.c
>> +++ b/examples/ipsec-secgw/ipsec-secgw.c
>> @@ -181,6 +181,7 @@ static uint32_t frag_tbl_sz; static uint32_t
>> frame_buf_size = RTE_MBUF_DEFAULT_BUF_SIZE; static uint32_t mtu_size
>> = RTE_ETHER_MTU; static uint64_t frag_ttl_ns = MAX_FRAG_TTL_NS;
>> +static uint32_t stats_interval;
>>
>> /* application wide librte_ipsec/SA parameters */ struct app_sa_prm
>> app_sa_prm = { @@ -292,7 +293,6 @@ adjust_ipv6_pktlen(struct rte_mbuf
>> *m, const struct rte_ipv6_hdr *iph,
>> }
>> }
>>
>> -#if (STATS_INTERVAL > 0)
>>
>> /* Print out statistics on packet distribution */ static void @@ -352,9 +352,8
>> @@ print_stats_cb(__rte_unused void *param)
>> total_packets_dropped);
>>
>> printf("\n=============================================
>> =======\n");
>>
>> - rte_eal_alarm_set(STATS_INTERVAL * US_PER_S, print_stats_cb,
>> NULL);
>> + rte_eal_alarm_set(stats_interval * US_PER_S, print_stats_cb, NULL);
>> }
>> -#endif /* STATS_INTERVAL */
>>
>> static inline void
>> prepare_one_packet(struct rte_mbuf *pkt, struct ipsec_traffic *t) @@ -
>> 1435,6 +1434,7 @@ print_usage(const char *prgname)
>> " [-e]"
>> " [-a]"
>> " [-c]"
>> + " [-t STATS_INTERVAL]"
>> " [-s NUMBER_OF_MBUFS_IN_PKT_POOL]"
>> " -f CONFIG_FILE"
>> " --config (port,queue,lcore)[,(port,queue,lcore)]"
>> @@ -1459,6 +1459,8 @@ print_usage(const char *prgname)
>> " -a enables SA SQN atomic behaviour\n"
>> " -c specifies inbound SAD cache size,\n"
>> " zero value disables the cache (default value: 128)\n"
>> + " -t specifies statistics screen update interval,\n"
>> + " zero disables statistics screen (default value: 0)\n"
>> " -s number of mbufs in packet pool, if not specified
>> number\n"
>> " of mbufs will be calculated based on number of cores,\n"
>> " ports and crypto queues\n"
>> @@ -1666,7 +1668,7 @@ parse_args(int32_t argc, char **argv, struct
>> eh_conf *eh_conf)
>>
>> argvopt = argv;
>>
>> - while ((opt = getopt_long(argc, argvopt, "aelp:Pu:f:j:w:c:s:",
>> + while ((opt = getopt_long(argc, argvopt, "aelp:Pu:f:j:w:c:t:s:",
>> lgopts, &option_index)) != EOF) {
>>
>> switch (opt) {
>> @@ -1747,6 +1749,15 @@ parse_args(int32_t argc, char **argv, struct
>> eh_conf *eh_conf)
>> }
>> app_sa_prm.cache_sz = ret;
>> break;
>> + case 't':
>> + ret = parse_decimal(optarg);
>> + if (ret < 0) {
>> + printf("Invalid interval value: %s\n", optarg);
>> + print_usage(prgname);
>> + return -1;
>> + }
>> + stats_interval = ret;
>> + break;
>> case CMD_LINE_OPT_CONFIG_NUM:
>> ret = parse_config(optarg);
>> if (ret) {
>> @@ -3350,11 +3361,11 @@ main(int32_t argc, char **argv)
>>
>> check_all_ports_link_status(enabled_port_mask);
>>
>> -#if (STATS_INTERVAL > 0)
>> - rte_eal_alarm_set(STATS_INTERVAL * US_PER_S, print_stats_cb,
>> NULL);
>> -#else
>> - RTE_LOG(INFO, IPSEC, "Stats display disabled\n");
>> -#endif /* STATS_INTERVAL */
>> + if (stats_interval > 0)
>> + rte_eal_alarm_set(stats_interval * US_PER_S,
>> + print_stats_cb, NULL);
>> + else
>> + RTE_LOG(INFO, IPSEC, "Stats display disabled\n");
>>
>> /* launch per-lcore init on every lcore */
>> rte_eal_mp_remote_launch(ipsec_launch_one_lcore, eh_conf,
>> CALL_MAIN); diff --git a/examples/ipsec-secgw/ipsec-secgw.h
>> b/examples/ipsec-secgw/ipsec-secgw.h
>> index f3082a1037..de9f382742 100644
>> --- a/examples/ipsec-secgw/ipsec-secgw.h
>> +++ b/examples/ipsec-secgw/ipsec-secgw.h
>> @@ -6,9 +6,6 @@
>>
>> #include <stdbool.h>
>>
>> -#ifndef STATS_INTERVAL
>> -#define STATS_INTERVAL 0
>> -#endif
>>
>> #define NB_SOCKETS 4
>>
>> @@ -144,38 +141,26 @@ is_unprotected_port(uint16_t port_id) static inline
>> void core_stats_update_rx(int n) { -#if (STATS_INTERVAL > 0)
>> int lcore_id = rte_lcore_id();
>> core_statistics[lcore_id].rx += n;
>> core_statistics[lcore_id].rx_call++;
>> if (n == MAX_PKT_BURST)
>> core_statistics[lcore_id].burst_rx += n; -#else
>> - RTE_SET_USED(n);
>> -#endif /* STATS_INTERVAL */
>> }
>>
>> static inline void
>> core_stats_update_tx(int n)
>> {
>> -#if (STATS_INTERVAL > 0)
>> int lcore_id = rte_lcore_id();
>> core_statistics[lcore_id].tx += n;
>> core_statistics[lcore_id].tx_call++;
>> -#else
>> - RTE_SET_USED(n);
>> -#endif /* STATS_INTERVAL */
>> }
>>
>> static inline void
>> core_stats_update_drop(int n)
>> {
>> -#if (STATS_INTERVAL > 0)
>> int lcore_id = rte_lcore_id();
>> core_statistics[lcore_id].dropped += n; -#else
>> - RTE_SET_USED(n);
>> -#endif /* STATS_INTERVAL */
>> }
>>
>> /* helper routine to free bulk of packets */
>> --
>> 2.25.1
Hi Radu,
Please see inline.
Thanks,
Anoob
> Subject: Re: [EXT] [dpdk-dev] [PATCH v2 4/9] examples/ipsec-secgw: add
> stats interval argument
>
> Hi Anoob,
>
> On 9/16/2021 10:30 AM, Anoob Joseph wrote:
> > Hi Radu,
> >
> > Making stats dynamic would have a perf cost. That was the reason it was
> introduced as a compile time option. Why do we need it changed?
> We changed it for two reasons, for a better UX and for the telemetry feature
> that is introduced with patch 3.
> >
> > Also, it looks like this patch is only disabling stats printing when timeout is 0.
> I don't see stats collection being conditional. Stats collection would also have
> perf impact, right?
> I don't think there is a measurable performance impact because these are
> per burst calls.
[Anoob] Agreed. If it was done intentionally, then I don’t see any problem.
Acked-by: Anoob Joseph <anoobj@marvell.com>
[snip]
@@ -127,6 +127,7 @@ The application has a number of command line options::
-p PORTMASK -P -u PORTMASK -j FRAMESIZE
-l -w REPLAY_WINDOW_SIZE -e -a
-c SAD_CACHE_SIZE
+ -t STATISTICS_INTERVAL
-s NUMBER_OF_MBUFS_IN_PACKET_POOL
-f CONFIG_FILE_PATH
--config (port,queue,lcore)[,(port,queue,lcore)]
@@ -176,6 +177,10 @@ Where:
Zero value disables cache.
Default value: 128.
+* ``-t``: specifies the statistics screen update interval. If set to zero or
+ omitted statistics screen is disabled.
+ Default value: 0.
+
* ``-s``: sets number of mbufs in packet pool, if not provided number of mbufs
will be calculated based on number of cores, eth ports and crypto queues.
@@ -181,6 +181,7 @@ static uint32_t frag_tbl_sz;
static uint32_t frame_buf_size = RTE_MBUF_DEFAULT_BUF_SIZE;
static uint32_t mtu_size = RTE_ETHER_MTU;
static uint64_t frag_ttl_ns = MAX_FRAG_TTL_NS;
+static uint32_t stats_interval;
/* application wide librte_ipsec/SA parameters */
struct app_sa_prm app_sa_prm = {
@@ -292,7 +293,6 @@ adjust_ipv6_pktlen(struct rte_mbuf *m, const struct rte_ipv6_hdr *iph,
}
}
-#if (STATS_INTERVAL > 0)
/* Print out statistics on packet distribution */
static void
@@ -352,9 +352,8 @@ print_stats_cb(__rte_unused void *param)
total_packets_dropped);
printf("\n====================================================\n");
- rte_eal_alarm_set(STATS_INTERVAL * US_PER_S, print_stats_cb, NULL);
+ rte_eal_alarm_set(stats_interval * US_PER_S, print_stats_cb, NULL);
}
-#endif /* STATS_INTERVAL */
static inline void
prepare_one_packet(struct rte_mbuf *pkt, struct ipsec_traffic *t)
@@ -1435,6 +1434,7 @@ print_usage(const char *prgname)
" [-e]"
" [-a]"
" [-c]"
+ " [-t STATS_INTERVAL]"
" [-s NUMBER_OF_MBUFS_IN_PKT_POOL]"
" -f CONFIG_FILE"
" --config (port,queue,lcore)[,(port,queue,lcore)]"
@@ -1459,6 +1459,8 @@ print_usage(const char *prgname)
" -a enables SA SQN atomic behaviour\n"
" -c specifies inbound SAD cache size,\n"
" zero value disables the cache (default value: 128)\n"
+ " -t specifies statistics screen update interval,\n"
+ " zero disables statistics screen (default value: 0)\n"
" -s number of mbufs in packet pool, if not specified number\n"
" of mbufs will be calculated based on number of cores,\n"
" ports and crypto queues\n"
@@ -1666,7 +1668,7 @@ parse_args(int32_t argc, char **argv, struct eh_conf *eh_conf)
argvopt = argv;
- while ((opt = getopt_long(argc, argvopt, "aelp:Pu:f:j:w:c:s:",
+ while ((opt = getopt_long(argc, argvopt, "aelp:Pu:f:j:w:c:t:s:",
lgopts, &option_index)) != EOF) {
switch (opt) {
@@ -1747,6 +1749,15 @@ parse_args(int32_t argc, char **argv, struct eh_conf *eh_conf)
}
app_sa_prm.cache_sz = ret;
break;
+ case 't':
+ ret = parse_decimal(optarg);
+ if (ret < 0) {
+ printf("Invalid interval value: %s\n", optarg);
+ print_usage(prgname);
+ return -1;
+ }
+ stats_interval = ret;
+ break;
case CMD_LINE_OPT_CONFIG_NUM:
ret = parse_config(optarg);
if (ret) {
@@ -3350,11 +3361,11 @@ main(int32_t argc, char **argv)
check_all_ports_link_status(enabled_port_mask);
-#if (STATS_INTERVAL > 0)
- rte_eal_alarm_set(STATS_INTERVAL * US_PER_S, print_stats_cb, NULL);
-#else
- RTE_LOG(INFO, IPSEC, "Stats display disabled\n");
-#endif /* STATS_INTERVAL */
+ if (stats_interval > 0)
+ rte_eal_alarm_set(stats_interval * US_PER_S,
+ print_stats_cb, NULL);
+ else
+ RTE_LOG(INFO, IPSEC, "Stats display disabled\n");
/* launch per-lcore init on every lcore */
rte_eal_mp_remote_launch(ipsec_launch_one_lcore, eh_conf, CALL_MAIN);
@@ -6,9 +6,6 @@
#include <stdbool.h>
-#ifndef STATS_INTERVAL
-#define STATS_INTERVAL 0
-#endif
#define NB_SOCKETS 4
@@ -144,38 +141,26 @@ is_unprotected_port(uint16_t port_id)
static inline void
core_stats_update_rx(int n)
{
-#if (STATS_INTERVAL > 0)
int lcore_id = rte_lcore_id();
core_statistics[lcore_id].rx += n;
core_statistics[lcore_id].rx_call++;
if (n == MAX_PKT_BURST)
core_statistics[lcore_id].burst_rx += n;
-#else
- RTE_SET_USED(n);
-#endif /* STATS_INTERVAL */
}
static inline void
core_stats_update_tx(int n)
{
-#if (STATS_INTERVAL > 0)
int lcore_id = rte_lcore_id();
core_statistics[lcore_id].tx += n;
core_statistics[lcore_id].tx_call++;
-#else
- RTE_SET_USED(n);
-#endif /* STATS_INTERVAL */
}
static inline void
core_stats_update_drop(int n)
{
-#if (STATS_INTERVAL > 0)
int lcore_id = rte_lcore_id();
core_statistics[lcore_id].dropped += n;
-#else
- RTE_SET_USED(n);
-#endif /* STATS_INTERVAL */
}
/* helper routine to free bulk of packets */