net/mlx5: fix the empty flow error structure

Message ID 1587108644-456343-1-git-send-email-bingz@mellanox.com (mailing list archive)
State Accepted, archived
Delegated to: Raslan Darawsheh
Headers
Series net/mlx5: fix the empty flow error structure |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-nxp-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS

Commit Message

Bing Zhao April 17, 2020, 7:30 a.m. UTC
  The output flow error parameter is used to indicate the detailed
reason of the failure when calling a rte_flow_* interface. Even
though sometimes the application will not check it or use it, the PMD
must fill it in the failure branch before returning. Or else, some
dirty value in the stack, heap will be accessed as a pointer and then
cause a crash.
In this case, when a port is stopped, it is not allowed to insert a
flow from application. The detailed error information should be
filled. If the application needs to check the detailed error reason,
it will get the information but not result in any crash.

Fixes: 6ec024870715 ("net/mlx5: check device stat before creating flow")

Signed-off-by: Bing Zhao <bingz@mellanox.com>
Acked-by: Ori Kam <orika@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
  

Comments

Raslan Darawsheh April 29, 2020, 9:44 a.m. UTC | #1
Hi,

> -----Original Message-----
> From: Bing Zhao <bingz@mellanox.com>
> Sent: Friday, April 17, 2020 10:31 AM
> To: Ori Kam <orika@mellanox.com>; Slava Ovsiienko
> <viacheslavo@mellanox.com>; Raslan Darawsheh <rasland@mellanox.com>
> Cc: Matan Azrad <matan@mellanox.com>; dev@dpdk.org
> Subject: [PATCH] net/mlx5: fix the empty flow error structure
> 
> The output flow error parameter is used to indicate the detailed
> reason of the failure when calling a rte_flow_* interface. Even
> though sometimes the application will not check it or use it, the PMD
> must fill it in the failure branch before returning. Or else, some
> dirty value in the stack, heap will be accessed as a pointer and then
> cause a crash.
> In this case, when a port is stopped, it is not allowed to insert a
> flow from application. The detailed error information should be
> filled. If the application needs to check the detailed error reason,
> it will get the information but not result in any crash.
> 
> Fixes: 6ec024870715 ("net/mlx5: check device stat before creating flow")
Fixed Fixes line sha
> 
> Signed-off-by: Bing Zhao <bingz@mellanox.com>
> Acked-by: Ori Kam <orika@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_flow.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index c529aa3..6a5522c 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -4490,9 +4490,12 @@ struct rte_flow *
>  	 * are not affected.
>  	 */
>  	if (unlikely(!dev->data->dev_started)) {
> -		rte_errno = ENODEV;
>  		DRV_LOG(DEBUG, "port %u is not started when "
>  			"inserting a flow", dev->data->port_id);
> +		rte_flow_error_set(error, ENODEV,
> +				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> +				   NULL,
> +				   "port not started");
>  		return NULL;
>  	}
>  	return (void *)(uintptr_t)flow_list_create(dev, &priv->flows,
> --
> 1.8.3.1

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh
  

Patch

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index c529aa3..6a5522c 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -4490,9 +4490,12 @@  struct rte_flow *
 	 * are not affected.
 	 */
 	if (unlikely(!dev->data->dev_started)) {
-		rte_errno = ENODEV;
 		DRV_LOG(DEBUG, "port %u is not started when "
 			"inserting a flow", dev->data->port_id);
+		rte_flow_error_set(error, ENODEV,
+				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				   NULL,
+				   "port not started");
 		return NULL;
 	}
 	return (void *)(uintptr_t)flow_list_create(dev, &priv->flows,