Hi,

On Thu, Sep 05, 2019 at 05:10:40PM +0100, Andrew Rybchenko wrote:
> From: Ivan Ilchenko <ivan.ilche...@oktetlabs.ru>
> 
> rte_eth_promiscuous_enable()/rte_eth_promiscuous_disable() return
> value was changed from void to int, so this patch modify usage
> of these functions across net/failsafe according to new return type.
> 
> Try to keep promiscuous mode consistent across all active
> devices in the case of failure.
> 
> Signed-off-by: Ivan Ilchenko <ivan.ilche...@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybche...@solarflare.com>
> ---
>  drivers/net/failsafe/failsafe_ether.c |  8 +++--
>  drivers/net/failsafe/failsafe_ops.c   | 44 ++++++++++++++++++++++++---
>  2 files changed, 46 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/failsafe/failsafe_ether.c 
> b/drivers/net/failsafe/failsafe_ether.c
> index 504c76edb0..bd38f1c1e4 100644
> --- a/drivers/net/failsafe/failsafe_ether.c
> +++ b/drivers/net/failsafe/failsafe_ether.c
> @@ -126,9 +126,13 @@ fs_eth_dev_conf_apply(struct rte_eth_dev *dev,
>       if (dev->data->promiscuous != edev->data->promiscuous) {
>               DEBUG("Configuring promiscuous");
>               if (dev->data->promiscuous)
> -                     rte_eth_promiscuous_enable(PORT_ID(sdev));
> +                     ret = rte_eth_promiscuous_enable(PORT_ID(sdev));
>               else
> -                     rte_eth_promiscuous_disable(PORT_ID(sdev));
> +                     ret = rte_eth_promiscuous_disable(PORT_ID(sdev));
> +             if (ret != 0) {
> +                     ERROR("Failed to apply promiscuous mode");
> +                     return ret;
> +             }
>       } else {
>               DEBUG("promiscuous already set");
>       }
> diff --git a/drivers/net/failsafe/failsafe_ops.c 
> b/drivers/net/failsafe/failsafe_ops.c
> index cc14bc2bcc..cbf143fb3c 100644
> --- a/drivers/net/failsafe/failsafe_ops.c
> +++ b/drivers/net/failsafe/failsafe_ops.c
> @@ -659,11 +659,29 @@ fs_promiscuous_enable(struct rte_eth_dev *dev)
>  {
>       struct sub_device *sdev;
>       uint8_t i;
> +     int ret = 0;
>  
>       fs_lock(dev, 0);
> -     FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE)
> -             rte_eth_promiscuous_enable(PORT_ID(sdev));
> +     FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
> +             ret = rte_eth_promiscuous_enable(PORT_ID(sdev));
> +             if (ret != 0) {
> +                     ERROR("Promiscuous mode enable failed for subdevice %d",
> +                             PORT_ID(sdev));
> +                     break;
> +             }
> +     }
> +     if (ret != 0) {
> +             /* Rollback in the case of failure */
> +             FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
> +                     ret = rte_eth_promiscuous_disable(PORT_ID(sdev));
> +                     if (ret != 0)
> +                             ERROR("Promiscuous mode disable failed for 
> subdevice %d",
> +                                     PORT_ID(sdev));
> +             }
> +     }
>       fs_unlock(dev, 0);
> +
> +     return;

This patch should be applied after the ethdev change to avoid breaking
the build, I think?

You should be able to change the ethdev API, leaving the fail-safe
internals ignore the return value, then apply this patch and fix it.
This way the patchset should not break the build mid-series.

Otherwise good implementation with the rollback.

-- 
Gaëtan Rivet
6WIND

Reply via email to