Re: [PATCH] app/testpmd: fix use of indirect action after port close

["Singh, Aman Deep" <aman.deep.singh@intel.com>]

Hi Dmitry,

On 3/7/2022 10:18 PM, Dmitry Kozlyuk wrote:
> When a port was closed, indirect actions could remain
> with their handles no longer valid.
> If a newly attached device was assigned the same ID as the closed port,
> those indirect actions became accessible again.
> Any attempt to use them resulted in an undefined behavior.
> Automatically flush indirect actions when a port is closed.
>
> Fixes: 4b61b8774be9 ("ethdev: introduce indirect flow action")
> Cc: stable@dpdk.org
>
> Signed-off-by: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>
> ---
From: guides/prog_guide/rte_flow.rst
/"If ``RTE_ETH_DEV_CAPA_FLOW_SHARED_OBJECT_KEEP`` is advertised,//
//this means that the PMD can keep at least some indirect actions//
//across device stop and start.

/Please check, if we are inline with the guidelines given in the section.
/
/
>   app/test-pmd/config.c  | 31 +++++++++++++++++++++++++++++++
>   app/test-pmd/testpmd.c |  1 +
>   app/test-pmd/testpmd.h |  1 +
>   3 files changed, 33 insertions(+)
>
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index cc8e7aa138..04baa319e0 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1844,6 +1844,37 @@ port_action_handle_destroy(portid_t port_id,
>   	return ret;
>   }
>   
> +int
> +port_action_handle_flush(portid_t port_id)
> +{
> +	struct rte_port *port;
> +	struct port_indirect_action **tmp;
> +	int ret = 0;
> +
> +	if (port_id_is_invalid(port_id, ENABLED_WARN) ||
> +	    port_id == (portid_t)RTE_PORT_ALL)
> +		return -EINVAL;
> +	port = &ports[port_id];
> +	tmp = &port->actions_list;
> +	while (*tmp != NULL) {
> +		struct rte_flow_error error;
> +		struct port_indirect_action *pia = *tmp;
> +
> +		/* Poisoning to make sure PMDs update it in case of error. */
> +		memset(&error, 0x44, sizeof(error));
> +		if (pia->handle != NULL &&
> +		    rte_flow_action_handle_destroy
> +					(port_id, pia->handle, &error) != 0) {
> +			printf("Indirect action #%u not destroyed\n", pia->id);
> +			ret = port_flow_complain(&error);
> +			tmp = &pia->next;
> +		} else {
> +			*tmp = pia->next;
> +			free(pia);
> +		}
> +	}
> +	return ret;
> +}
>   
>   /** Get indirect action by port + id */
>   struct rte_flow_action_handle *
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index fe2ce19f99..f07f76f8f5 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -3139,6 +3139,7 @@ close_port(portid_t pid)
>   		if (is_proc_primary()) {
>   			port_flow_flush(pi);
>   			port_flex_item_flush(pi);
> +			port_action_handle_flush(pi);
>   			rte_eth_dev_close(pi);
>   		}
>   
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index 31f766c965..e25663e5b3 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -907,6 +907,7 @@ int port_action_handle_create(portid_t port_id, uint32_t id,
>   			      const struct rte_flow_action *action);
>   int port_action_handle_destroy(portid_t port_id,
>   			       uint32_t n, const uint32_t *action);
> +int port_action_handle_flush(portid_t port_id);
>   struct rte_flow_action_handle *port_action_handle_get_by_id(portid_t port_id,
>   							    uint32_t id);
>   int port_action_handle_update(portid_t port_id, uint32_t id,