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 <fengcheng...@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.

Reply via email to