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