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

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

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);
>