Re: [dpdk-dev] [PATCH v4] app/testpmd: support flow aging

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 5/5/2020 10:11 AM, Matan Azrad wrote:
> 
> 
> From: Ferruh Yigit:
>> On 5/3/2020 9:59 AM, 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:
>>>
>>> - Add new item "flow_aged" to the current print event command
>> arguments.
>>> - 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.
>>> v3: Change the command from only set aged flow output to set one
>>> gloable verbose bitmap for all events output.
>>> v4: Add the event output to current global print event arguments.
>>
>> <...>
>>
>>> @@ -187,9 +187,9 @@ usage(char* progname)
>>>  	printf("  --no-rmv-interrupt: disable device removal interrupt.\n");
>>>  	printf("  --bitrate-stats=N: set the logical core N to perform "
>>>  		"bit-rate calculation.\n");
>>> -	printf("  --print-event
>> <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|all
>>> : "
>>> +	printf("  --print-event
>> <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|flo
>> w_aged|all>: "
>>>  	       "enable print of designated event or all of them.\n");
>>> -	printf("  --mask-event
>> <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|all
>>> : "
>>> +	printf("  --mask-event
>> <unknown|intr_lsc|queue_state|intr_reset|vf_mbox|macsec|intr_rmv|flo
>> w_aged|all>: "
>>
>> +1 to '--print-event', can you please update the documentation for the
>> change?
> 
>  +1
> 
>> <...>
>>
>>> @@ -388,7 +389,8 @@ uint32_t event_print_mask = (UINT32_C(1) <<
>> RTE_ETH_EVENT_UNKNOWN) |
>>>  			    (UINT32_C(1) << RTE_ETH_EVENT_INTR_RESET) |
>>>  			    (UINT32_C(1) << RTE_ETH_EVENT_IPSEC) |
>>>  			    (UINT32_C(1) << RTE_ETH_EVENT_MACSEC) |
>>> -			    (UINT32_C(1) << RTE_ETH_EVENT_INTR_RMV);
>>> +			    (UINT32_C(1) << RTE_ETH_EVENT_INTR_RMV) |
>>> +			    (UINT32_C(1) << RTE_ETH_EVENT_FLOW_AGED);
>>
>> This is enabling the event logging by default, we are turning back to original
>> point, since '--print-event' can be used to enable it, can you please leave it
>> out by default?
> 
> The aged event is triggered only when there is at least one flow which is configured with AGE action,
> Don't you think it should be printed by default if the user requested aging in a flow?
> 
Good point, OK to keep as it is.
So only above doc update please and feel free to keep existing acks.