DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Bill Zhou <dongz@mellanox.com>,
	wenzhuo.lu@intel.com, jingjing.wu@intel.com,
	bernard.iremonger@intel.com, orika@mellanox.com,
	john.mcnamara@intel.com, marko.kovacevic@intel.com,
	matan@mellanox.com
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2] app/testpmd: support flow aging
Date: Thu, 30 Apr 2020 23:43:19 +0100
Message-ID: <5fc8bc00-e4b1-b2c4-1caf-03dea0456f4c@intel.com> (raw)
In-Reply-To: <20200430155345.1384-1-dongz@mellanox.com>

On 4/30/2020 4:53 PM, Bill Zhou wrote:
> Currently, there is no way to check the aging event or to get the current
> aged flows in testpmd, this patch include those implements, it's included:
> - Registering aging event when the testpmd application start, add new
>   command to control if the event expose to the applications. If it's be
>   set, when new flow be checked age out, there will be one-line output log.
> - Add new command to list all aged flows, meanwhile, we can set parameter
>   to destroy it.
> 
> Signed-off-by: Bill Zhou <dongz@mellanox.com>
> ---
> v2: Update the way of registering aging event, add new command to control
> if the event need be print or not. Update the output of the delete aged
> flow command format.

<...>

> @@ -19387,6 +19394,44 @@ cmdline_parse_inst_t cmd_showport_macs = {
>  	},
>  };
>  
> +/* Enable/Disable flow aging event log */
> +struct cmd_set_aged_flow_event_log_result {
> +	cmdline_fixed_string_t set;
> +	cmdline_fixed_string_t keyword;
> +	cmdline_fixed_string_t enable;
> +};
> +cmdline_parse_token_string_t cmd_set_aged_flow_event_log_set =
> +	TOKEN_STRING_INITIALIZER(struct cmd_set_aged_flow_event_log_result,
> +		set, "set");
> +cmdline_parse_token_string_t cmd_set_aged_flow_event_log_keyword =
> +	TOKEN_STRING_INITIALIZER(struct cmd_set_aged_flow_event_log_result,
> +		keyword, "aged_flow_event_log");
> +cmdline_parse_token_string_t cmd_set_aged_flow_event_log_enable =
> +	TOKEN_STRING_INITIALIZER(struct cmd_set_aged_flow_event_log_result,
> +		enable, "on#off");
> +
> +static void
> +cmd_set_aged_flow_event_log_parsed(void *parsed_result,
> +				__rte_unused struct cmdline *cl,
> +				__rte_unused void *data)
> +{
> +	struct cmd_set_aged_flow_event_log_result *res = parsed_result;
> +	int enable = (strcmp(res->enable, "on") == 0) ? 1 : 0;
> +	update_aging_event_log_status(enable);
> +}
> +
> +cmdline_parse_inst_t cmd_set_aged_flow_event_log = {
> +	.f = cmd_set_aged_flow_event_log_parsed,
> +	.data = NULL,
> +	.help_str = "set aged_flow_event_log on|off",

What do you think "set aged_flow_verbose on|off" to be more similar to existing
verbose command?

<...>

> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -3068,6 +3068,21 @@ rmv_port_callback(void *arg)
>  		start_packet_forwarding(0);
>  }
>  
> +static int aged_flow_event_enable;

Other global config values are at the beginning of the file, with a comment on
them, can you do same with variable?

> +void update_aging_event_log_status(int enable)
> +{
> +	aged_flow_event_enable = enable;
> +}
> +
> +int aging_event_output(uint16_t portid)

This can be a 'static' function.

> +{
> +	if (aged_flow_event_enable) {
> +		printf("port %u RTE_ETH_EVENT_FLOW_AGED triggered\n", portid);
> +		fflush(stdout);
> +	}
> +	return 0;
> +}
> +
>  /* This function is used by the interrupt thread */
>  static int
>  eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
> @@ -3098,6 +3113,8 @@ eth_event_callback(portid_t port_id, enum rte_eth_event_type type, void *param,
>  				rmv_port_callback, (void *)(intptr_t)port_id))
>  			fprintf(stderr, "Could not set up deferred device removal\n");
>  		break;
> +	case RTE_ETH_EVENT_FLOW_AGED:
> +		aging_event_output(port_id);

Can't this provide more information than port_id, like flow id etc, what
event_process function provides? can we print it too?
And what is the intended usage in real application, when flow aged event
occurred, should application destroy the flow? So it should know the flow, right?

<...>

> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -1062,6 +1062,21 @@ Where:
>  
>     Check the NIC Datasheet for hardware limits.
>  
> +aged flow event log set
> +~~~~~~~~~~~~~~~~~~~~~~~
> +
> +Currently, the flow aging event is registered when the testpmd application start, if new flow be checked
> +age out, there will be one output log for it. But some applications do not interest this event, use this
> +command can set if the event expose to the applications::
> +
> +   testpmd> set aged_flow_event_log (on|off)
> +
> +For examine::
> +
> +   testpmd> set aged_flow_event_log on
> +   testpmd> port 0 RTE_ETH_EVENT_FLOW_AGED triggered
> +   testpmd> set aged_flow_event_log off
> +

This can be moved below the "set verbose" command since they are in similar group.

  reply	other threads:[~2020-04-30 22:43 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-24 10:55 [dpdk-dev] [PATCH] " Bill Zhou
2020-04-24 16:25 ` Ferruh Yigit
2020-04-26  7:23   ` Bill Zhou
2020-04-27 14:13     ` Ferruh Yigit
2020-04-27 15:12       ` Matan Azrad
2020-04-30 22:26         ` Ferruh Yigit
2020-04-30 15:53 ` [dpdk-dev] [PATCH v2] " Bill Zhou
2020-04-30 22:43   ` Ferruh Yigit [this message]
2020-05-01  6:51     ` Matan Azrad
2020-05-01  9:27       ` Ferruh Yigit
2020-05-01 11:28         ` Matan Azrad
2020-05-01 11:54           ` Ferruh Yigit
2020-05-01 12:45             ` Matan Azrad
2020-05-01 13:38               ` Ferruh Yigit
2020-05-01 15:14                 ` Matan Azrad
2020-05-01 15:44                   ` Ferruh Yigit
2020-05-02 14:00   ` [dpdk-dev] [PATCH v3] " Bill Zhou
2020-05-03  8:59     ` [dpdk-dev] [PATCH v4] " Bill Zhou
2020-05-03  9:46       ` Matan Azrad
2020-05-03 14:58       ` Ori Kam
2020-05-05  8:37       ` Ferruh Yigit
2020-05-05  9:11         ` Matan Azrad
2020-05-05  9:23           ` Ferruh Yigit
2020-05-05  9:49       ` [dpdk-dev] [PATCH v5] " Bill Zhou
2020-05-05 10:09         ` Ori Kam
2020-05-05 15:11           ` Ferruh Yigit
2020-05-06  8:04             ` Matan Azrad

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5fc8bc00-e4b1-b2c4-1caf-03dea0456f4c@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=bernard.iremonger@intel.com \
    --cc=dev@dpdk.org \
    --cc=dongz@mellanox.com \
    --cc=jingjing.wu@intel.com \
    --cc=john.mcnamara@intel.com \
    --cc=marko.kovacevic@intel.com \
    --cc=matan@mellanox.com \
    --cc=orika@mellanox.com \
    --cc=wenzhuo.lu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git