From: Ferruh Yigit <ferruh.yigit@amd.com>
To: "Singh, Aman Deep" <aman.deep.singh@intel.com>,
Chengwen Feng <fengchengwen@huawei.com>,
thomas@monjalon.net, andrew.rybchenko@oktetlabs.ru
Cc: dev@dpdk.org, liudongdong3@huawei.com, yisen.zhuang@huawei.com,
yuying.zhang@intel.com
Subject: Re: [PATCH 2/2] app/testpmd: add disable-flow-flush parameter
Date: Wed, 25 Jan 2023 18:34:31 +0000 [thread overview]
Message-ID: <ae1f7a39-4aa7-50ea-d333-bceb98c45fdf@amd.com> (raw)
In-Reply-To: <bde7035f-35ff-7bc7-d98d-fcdd7be75120@intel.com>
On 1/23/2023 3:01 PM, Singh, Aman Deep wrote:
>
> On 12/15/2022 7:11 AM, Chengwen Feng wrote:
>> This patch adds "--disable-flow-flush" parameter, which could used to
>> disable port flow flush when stop port. It allows testing keep flow
>> rules or shared flow objects across restart.
>>
>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>> ---
>> app/test-pmd/parameters.c | 4 ++++
>> app/test-pmd/testpmd.c | 7 ++++++-
>> app/test-pmd/testpmd.h | 1 +
>> doc/guides/testpmd_app_ug/run_app.rst | 5 +++++
>> 4 files changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
>> index d597c209ba..e734ad9a02 100644
>> --- a/app/test-pmd/parameters.c
>> +++ b/app/test-pmd/parameters.c
>> @@ -175,6 +175,7 @@ usage(char* progname)
>> "disable print of designated event or all of them.\n");
>> printf(" --flow-isolate-all: "
>> "requests flow API isolated mode on all ports at initialization time.\n");
>> + printf(" --disable-flow-flush: disable port flow flush when stop port.\n");
>> printf(" --tx-offloads=0xXXXXXXXX: hexadecimal bitmask of TX queue offloads\n");
>> printf(" --rx-offloads=0xXXXXXXXX: hexadecimal bitmask of RX queue offloads\n");
>> printf(" --hot-plug: enable hot plug for device.\n");
>> @@ -667,6 +668,7 @@ launch_args_parse(int argc, char** argv)
>> { "rxfreet", 1, 0, 0 },
>> { "no-flush-rx", 0, 0, 0 },
>> { "flow-isolate-all", 0, 0, 0 },
>> + { "disable-flow-flush", 0, 0, 0 },
>> { "rxoffs", 1, 0, 0 },
>> { "rxpkts", 1, 0, 0 },
>> { "rxhdrs", 1, 0, 0 },
>> @@ -1330,6 +1332,8 @@ launch_args_parse(int argc, char** argv)
>> rmv_interrupt = 0;
>> if (!strcmp(lgopts[opt_idx].name, "flow-isolate-all"))
>> flow_isolate_all = 1;
>> + if (!strcmp(lgopts[opt_idx].name, "disable-flow-flush"))
>> + no_flow_flush = 1;
>> if (!strcmp(lgopts[opt_idx].name, "tx-offloads")) {
>> char *end = NULL;
>> n = strtoull(optarg, &end, 16);
>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>> index 134d79a555..ea78f8982d 100644
>> --- a/app/test-pmd/testpmd.c
>> +++ b/app/test-pmd/testpmd.c
>> @@ -383,6 +383,11 @@ uint8_t no_flush_rx = 0; /* flush by default */
>> */
>> uint8_t flow_isolate_all;
>>
>> +/*
>> + * Disable port flow flush when stop port.
>> + */
>> +uint8_t no_flow_flush = 0; /* do flow flush by default */
>> +
>> /*
>> * Avoids to check link status when starting/stopping a port.
>> */
>> @@ -3246,7 +3251,7 @@ stop_port(portid_t pid)
>> }
>> }
>>
>> - if (port->flow_list)
>> + if (port->flow_list && !no_flow_flush)
>> port_flow_flush(pi);
>
> In case stop_port() is called through pmd_test_exit(), should the flag
> be disabled for flush to happen.
>
In case of 'pmd_test_exit()', 'close_port()' will be called after port
stop which will make port unusable anyway.
As long as there is no memory leak in that path, I think no need to for
flow flush.
@Chengwen, can you please check and confirm there won't be any resource
leak in case of 'no_flow_flush' is set and testpmd terminated via
'pmd_test_exit()' call?
And rest looks good to me.
>>
>> ret = eth_dev_stop_mp(pi);
>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
>> index 7d24d25970..ab4733522d 100644
>> --- a/app/test-pmd/testpmd.h
>> +++ b/app/test-pmd/testpmd.h
>> @@ -476,6 +476,7 @@ extern uint8_t numa_support; /**< set by "--numa" parameter */
>> extern uint16_t port_topology; /**< set by "--port-topology" parameter */
>> extern uint8_t no_flush_rx; /**<set by "--no-flush-rx" parameter */
>> extern uint8_t flow_isolate_all; /**< set by "--flow-isolate-all */
>> +extern uint8_t no_flow_flush; /**< set by "--disable-flow-flush" parameter */
>> extern uint8_t mp_alloc_type;
>> /**< set by "--mp-anon" or "--mp-alloc" parameter */
>> extern uint32_t eth_link_speed;
>> diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
>> index 074f910fc9..3ec3d4f5e6 100644
>> --- a/doc/guides/testpmd_app_ug/run_app.rst
>> +++ b/doc/guides/testpmd_app_ug/run_app.rst
>> @@ -436,6 +436,11 @@ The command line options are:
>>
>> Ports that do not support this mode are automatically discarded.
>>
>> +* ``--disable-flow-flush``
>> +
>> + Disable port flow flush when stop port. This allows testing keep flow rules
>> + or shared flow objects across restart.
>> +
>> * ``--tx-offloads=0xXXXXXXXX``
>>
>> Set the hexadecimal bitmask of TX queue offloads.
next prev parent reply other threads:[~2023-01-25 18:34 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-15 1:41 [PATCH 0/2] net/hns3: support rule keep capability Chengwen Feng
2022-12-15 1:41 ` [PATCH 1/2] net/hns3: declare " Chengwen Feng
2022-12-15 1:56 ` Dongdong Liu
2022-12-15 1:41 ` [PATCH 2/2] app/testpmd: add disable-flow-flush parameter Chengwen Feng
2023-01-23 15:01 ` Singh, Aman Deep
2023-01-25 18:34 ` Ferruh Yigit [this message]
2023-01-25 18:37 ` Ferruh Yigit
2023-01-26 12:29 ` Ori Kam
2023-01-31 7:55 ` Singh, Aman Deep
2023-01-31 17:05 ` [PATCH 0/2] net/hns3: support rule keep capability Ferruh Yigit
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=ae1f7a39-4aa7-50ea-d333-bceb98c45fdf@amd.com \
--to=ferruh.yigit@amd.com \
--cc=aman.deep.singh@intel.com \
--cc=andrew.rybchenko@oktetlabs.ru \
--cc=dev@dpdk.org \
--cc=fengchengwen@huawei.com \
--cc=liudongdong3@huawei.com \
--cc=thomas@monjalon.net \
--cc=yisen.zhuang@huawei.com \
--cc=yuying.zhang@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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).