[PATCH v3] ethdev: Align enable logic handling with disable functions

[PATCH v3] ethdev: Align enable logic handling with disable functions

[Sunyang Wu]

This patch modifies the handling logic of the "enable" related
operations. The key intention is to align it with the processing
approach of the "disable" functions.
Previously, there was an inconsistency in how failure scenarios were
dealt with between the "enable" and "disable" logic. Now, after
adjustment, their behaviors in exceptional cases are made more uniform.
Importantly, this change does not introduce any alteration to the actual
runtime behavior of the functions; it only serves to enhance code
consistency and maintainability, making the overall logic easier to
understand and maintain in the long run.
In this way, we ensure the codebase follows a more cohesive pattern,
reducing potential confusion during future development and maintenance
efforts that could stem from logical disparities.

Signed-off-by: Sunyang Wu <sunyang.wu@jaguarmicro.com>
---
 lib/ethdev/rte_ethdev.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index dd7c00bc94..41f96071e2 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -3018,7 +3018,8 @@ rte_eth_promiscuous_enable(uint16_t port_id)
 		return -ENOTSUP;
 
 	diag = dev->dev_ops->promiscuous_enable(dev);
-	dev->data->promiscuous = (diag == 0) ? 1 : 0;
+	if (diag == 0)
+		dev->data->promiscuous = 1;
 
 	diag = eth_err(port_id, diag);
 
@@ -3086,7 +3087,8 @@ rte_eth_allmulticast_enable(uint16_t port_id)
 	if (dev->dev_ops->allmulticast_enable == NULL)
 		return -ENOTSUP;
 	diag = dev->dev_ops->allmulticast_enable(dev);
-	dev->data->all_multicast = (diag == 0) ? 1 : 0;
+	if (diag == 0)
+		dev->data->all_multicast = 1;
 
 	diag = eth_err(port_id, diag);
 
-- 
2.19.0.rc0.windows.1

Re: [PATCH v3] ethdev: Align enable logic handling with disable functions

[Andrew Rybchenko]

Ack from Morten on v2 is lost.

On 7/22/25 04:09, Sunyang Wu wrote:
> This patch modifies the handling logic of the "enable" related

This patch modifies -> Modify

> operations. The key intention is to align it with the processing
> approach of the "disable" functions.
> Previously, there was an inconsistency in how failure scenarios were
> dealt with between the "enable" and "disable" logic. Now, after
> adjustment, their behaviors in exceptional cases are made more uniform.
> Importantly, this change does not introduce any alteration to the actual
> runtime behavior of the functions; it only serves to enhance code
> consistency and maintainability, making the overall logic easier to
> understand and maintain in the long run.
> In this way, we ensure the codebase follows a more cohesive pattern,
> reducing potential confusion during future development and maintenance
> efforts that could stem from logical disparities.
> 
> Signed-off-by: Sunyang Wu <sunyang.wu@jaguarmicro.com>

Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

> ---
>   lib/ethdev/rte_ethdev.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index dd7c00bc94..41f96071e2 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -3018,7 +3018,8 @@ rte_eth_promiscuous_enable(uint16_t port_id)
>   		return -ENOTSUP;
>   
>   	diag = dev->dev_ops->promiscuous_enable(dev);
> -	dev->data->promiscuous = (diag == 0) ? 1 : 0;
> +	if (diag == 0)
> +		dev->data->promiscuous = 1;

Strictly speaking it changes the behaviour and result could be different
if driver callback modifies the promiscuous or all_multicast itself.
Anyway I agree that the code should be unified and driver should not
touch these flags in data.

>   
>   	diag = eth_err(port_id, diag);
>   
> @@ -3086,7 +3087,8 @@ rte_eth_allmulticast_enable(uint16_t port_id)
>   	if (dev->dev_ops->allmulticast_enable == NULL)
>   		return -ENOTSUP;
>   	diag = dev->dev_ops->allmulticast_enable(dev);
> -	dev->data->all_multicast = (diag == 0) ? 1 : 0;
> +	if (diag == 0)
> +		dev->data->all_multicast = 1;
>   
>   	diag = eth_err(port_id, diag);
>