Re: 回复: [PATCH] ethdev: keep promiscuous/allmulti value before enabling

[Thomas Monjalon <thomas@monjalon.net>]

21/07/2025 14:13, Sunyang Wu:
> Hi, Thomas:
> Thanks for your note. The main purpose of this modification is to align the handling logic with the "disable" functions,
> aiming to enhance the overall consistency and maintainability of the code.
> 
> Previously, the handling of failure scenarios in the "enable" related logic differed from that in the "disable" functions. 
> With this adjustment, both will behave more uniformly in exceptional cases, which should reduce potential confusion
> during future development or maintenance caused by inconsistent logic.

In this case, please send a v2 explaining the intent is to align with disable functions,
and saying there is no behavior change.


> 21/07/2025 13:51, Sunyang Wu:
> > The values of the promiscuous and allmulticast variables are set after 
> > calling the driver, according to the return value.
> >
> > Fixes: 400d75818266 ("ethdev: check device promiscuous state") 
> > de5ccf0775ae ("ethdev:do nothing if all-multicast mode is applied
> > again")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Sunyang Wu <sunyang.wu@jaguarmicro.com>
> [...]
> >       diag = dev->dev_ops->promiscuous_enable(dev);
> > -     dev->data->promiscuous = (diag == 0) ? 1 : 0;
> > +     if (diag == 0)
> > +             dev->data->promiscuous = 1;
> 
> I remember seeing this strange behavior of resetting the value if failed.
> And it is done differently in the "disable" functions.
> 
> But it is not so wrong, because if it was enabled, the function returns early.
> So the value changes only if it is successful.
> 
> What is the issue you observe?
> Is it a rework to make the code easier to understand?