Hi Ferruh From: Ferruh Yigit > 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 <do...@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?
Please see my comments below regard it. > <...> > > > --- 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? +1 > > +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. +1 > > +{ > > + 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? Probably Yes Ferruh, I will add details, maybe it will be clearer: As described well in rte_flow_get_aged_flows API description, there are 2 suggested options for the application: 1. To call rte_flow_get_aged_flows from the AGED event callback in order to get the aged flow contexts of the triggered port. 2. To call rte_flow_get_aged_flows in any time application wants. It is probably depends in the way the application wants to synchronize flow APIs calls. The application just gets the information of the aged-out flows context from the above API and can do any flow operation for it, and yes, the most expected case is to destroy the flows. Bill added 2 testpmd commands: 1. To use rte_flow_get_aged_flows and to print a list of all the aged-out flows (with option to destroy them directly by the command). 2. A Boolean to indicate the application whether to notify the testpmd user about new aged-out flows(by print). By this way, the testpmd user can use the 2 approaches with minimum testpmd flow management change. So, the Boolean var is more like "trigger testpmd user notification", not like regular verbose options. Matan > > <...> > > > --- 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.