* [dpdk-dev] [PATCH 0/7] ethdev: change allmulticast controls to return status
@ 2019-09-09 12:13 Andrew Rybchenko
2019-09-09 12:13 ` [dpdk-dev] [PATCH 1/7] ethdev: change allmulticast mode controllers to return errors Andrew Rybchenko
` (6 more replies)
0 siblings, 7 replies; 18+ messages in thread
From: Andrew Rybchenko @ 2019-09-09 12:13 UTC (permalink / raw)
To: Thomas Monjalon, Ferruh Yigit, Rasesh Mody, Shahed Shaikh, Wenzhuo Lu
Cc: dev, Ivan Ilchenko
It is the fourth patch series to get rid of void returning functions
in ethdev in accordance with deprecation notice [1].
It should be applied on top of [2] and [3].
Merge conflicts with [4] are trivial (release notes only).
Functions which return void are bad since they do not provide explicit
information to the caller if everything is OK or not.
It is especially painful in the case of all-multicast mode since it is
not always supported, there are real cases when it fails to apply and
it affects traffic which is received by the port.
Driver maintainrs are encouraged to review the patch which changes
driver callbacks prototype. Changes are not always trivial when I tried
to provide real status of the operation. I used -EAGAIN when I failed
to choose better error code.
The following two drivers ignore status of internal functions and
definitely could be improved:
net/bnx2x: bnx2x_set_rx_mode() can report failure, but it is used in
other places and should be updated carefully.
net/igbvf: e1000_promisc_set_vf() provides return status, but it is
unclear how handle it properly.
[1] https://patches.dpdk.org/patch/56969/
[2] https://patches.dpdk.org/project/dpdk/list/?series=6279
[3] https://patches.dpdk.org/project/dpdk/list/?series=6334
[4] https://patches.dpdk.org/project/dpdk/list/?series=6308
Andrew Rybchenko (1):
ethdev: do nothing if all-multicast mode is applied again
Ivan Ilchenko (6):
ethdev: change allmulticast mode controllers to return errors
net/failsafe: check code of allmulticast mode switch
net/bonding: check code of allmulticast mode switch
ethdev: change allmulticast callbacks to return status
app/testpmd: check code of allmulticast mode switch
examples/ipv4_multicast: check allmulticast enable status
app/test-pmd/cmdline.c | 10 +---
app/test-pmd/testpmd.h | 1 +
app/test-pmd/util.c | 16 ++++++
doc/guides/rel_notes/deprecation.rst | 1 -
doc/guides/rel_notes/release_19_11.rst | 4 ++
drivers/net/atlantic/atl_ethdev.c | 14 +++--
drivers/net/axgbe/axgbe_ethdev.c | 16 ++++--
drivers/net/bnx2x/bnx2x_ethdev.c | 8 ++-
drivers/net/bnxt/bnxt_ethdev.c | 26 +++++++--
drivers/net/bonding/rte_eth_bond_8023ad.c | 13 ++++-
drivers/net/bonding/rte_eth_bond_pmd.c | 67 ++++++++++++++++++++---
drivers/net/cxgbe/cxgbe_ethdev.c | 12 ++--
drivers/net/cxgbe/cxgbe_pfvf.h | 4 +-
drivers/net/dpaa/dpaa_ethdev.c | 8 ++-
drivers/net/dpaa2/dpaa2_ethdev.c | 14 +++--
drivers/net/e1000/em_ethdev.c | 14 +++--
drivers/net/e1000/igb_ethdev.c | 26 ++++++---
drivers/net/enetc/enetc_ethdev.c | 10 +++-
drivers/net/enic/enic_ethdev.c | 22 ++++++--
drivers/net/failsafe/failsafe_ether.c | 8 ++-
drivers/net/failsafe/failsafe_ops.c | 52 ++++++++++++++++--
drivers/net/fm10k/fm10k_ethdev.c | 28 ++++++----
drivers/net/i40e/i40e_ethdev.c | 22 +++++---
drivers/net/i40e/i40e_ethdev_vf.c | 20 +++++--
drivers/net/i40e/i40e_vf_representor.c | 8 +--
drivers/net/iavf/iavf_ethdev.c | 20 +++++--
drivers/net/ice/ice_ethdev.c | 30 +++++++---
drivers/net/ipn3ke/ipn3ke_ethdev.h | 4 +-
drivers/net/ipn3ke/ipn3ke_representor.c | 8 ++-
drivers/net/ixgbe/ixgbe_ethdev.c | 53 ++++++++++++++----
drivers/net/liquidio/lio_ethdev.c | 12 ++--
drivers/net/mlx4/mlx4.h | 4 +-
drivers/net/mlx4/mlx4_ethdev.c | 14 +++--
drivers/net/mlx5/mlx5.h | 4 +-
drivers/net/mlx5/mlx5_rxmode.c | 38 ++++++++++---
drivers/net/mvpp2/mrvl_ethdev.c | 28 +++++++---
drivers/net/netvsc/hn_ethdev.c | 8 +--
drivers/net/netvsc/hn_var.h | 4 +-
drivers/net/netvsc/hn_vf.c | 8 +--
drivers/net/nfb/nfb_rxmode.c | 10 +++-
drivers/net/nfb/nfb_rxmode.h | 8 ++-
drivers/net/octeontx2/otx2_ethdev.h | 4 +-
drivers/net/octeontx2/otx2_ethdev_ops.c | 8 ++-
drivers/net/qede/qede_ethdev.c | 17 ++++--
drivers/net/sfc/sfc_ethdev.c | 8 +--
drivers/net/szedata2/rte_eth_szedata2.c | 6 +-
drivers/net/tap/rte_eth_tap.c | 52 ++++++++++++++----
drivers/net/virtio/virtio_ethdev.c | 24 +++++---
drivers/net/vmxnet3/vmxnet3_ethdev.c | 12 ++--
examples/ipv4_multicast/main.c | 6 +-
lib/librte_ethdev/rte_ethdev.c | 61 ++++++++++++++++-----
lib/librte_ethdev/rte_ethdev.h | 14 ++++-
lib/librte_ethdev/rte_ethdev_core.h | 4 +-
53 files changed, 654 insertions(+), 239 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 18+ messages in thread* [dpdk-dev] [PATCH 1/7] ethdev: change allmulticast mode controllers to return errors 2019-09-09 12:13 [dpdk-dev] [PATCH 0/7] ethdev: change allmulticast controls to return status Andrew Rybchenko @ 2019-09-09 12:13 ` Andrew Rybchenko 2019-09-09 12:13 ` [dpdk-dev] [PATCH 2/7] net/failsafe: check code of allmulticast mode switch Andrew Rybchenko ` (5 subsequent siblings) 6 siblings, 0 replies; 18+ messages in thread From: Andrew Rybchenko @ 2019-09-09 12:13 UTC (permalink / raw) To: Neil Horman, John McNamara, Marko Kovacevic, Thomas Monjalon, Ferruh Yigit Cc: dev, Ivan Ilchenko From: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru> Change rte_eth_allmulticast_enable()/rte_eth_allmulticast_disable() return value from void to int and return negative errno values in case of error conditions. Modify usage of these functions across the ethdev according to new return type. Signed-off-by: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> --- doc/guides/rel_notes/deprecation.rst | 1 - doc/guides/rel_notes/release_19_11.rst | 4 +++ lib/librte_ethdev/rte_ethdev.c | 37 +++++++++++++++++++------- lib/librte_ethdev/rte_ethdev.h | 14 ++++++++-- 4 files changed, 43 insertions(+), 13 deletions(-) diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst index b2e0a1fc7c..0d8e231ef5 100644 --- a/doc/guides/rel_notes/deprecation.rst +++ b/doc/guides/rel_notes/deprecation.rst @@ -88,7 +88,6 @@ Deprecation Notices negative errno values to indicate various error conditions (e.g. invalid port ID, unsupported operation, failed operation): - - ``rte_eth_allmulticast_enable`` and ``rte_eth_allmulticast_disable`` - ``rte_eth_link_get`` and ``rte_eth_link_get_nowait`` - ``rte_eth_dev_stop`` - ``rte_eth_dev_close`` diff --git a/doc/guides/rel_notes/release_19_11.rst b/doc/guides/rel_notes/release_19_11.rst index 5299157df7..a79e6a08e6 100644 --- a/doc/guides/rel_notes/release_19_11.rst +++ b/doc/guides/rel_notes/release_19_11.rst @@ -101,6 +101,10 @@ API Changes ``rte_eth_promiscuous_disable`` return value from ``void`` to ``int`` to provide a way to report various error conditions. +* ethdev: changed ``rte_eth_allmulticast_enable`` and + ``rte_eth_allmulticast_disable`` return value from ``void`` to ``int`` to + provide a way to report various error conditions. + ABI Changes ----------- diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c index 98fe533c5a..577640fdbe 100644 --- a/lib/librte_ethdev/rte_ethdev.c +++ b/lib/librte_ethdev/rte_ethdev.c @@ -1416,10 +1416,23 @@ rte_eth_dev_config_restore(struct rte_eth_dev *dev, } /* replay all multicast configuration */ - if (rte_eth_allmulticast_get(port_id) == 1) - rte_eth_allmulticast_enable(port_id); - else if (rte_eth_allmulticast_get(port_id) == 0) - rte_eth_allmulticast_disable(port_id); + if (rte_eth_allmulticast_get(port_id) == 1) { + ret = rte_eth_allmulticast_enable(port_id); + if (ret != 0 && ret != -ENOTSUP) { + RTE_ETHDEV_LOG(ERR, + "Failed to enable allmulticast mode for device (port %u): %s\n", + port_id, rte_strerror(-ret)); + return ret; + } + } else if (rte_eth_allmulticast_get(port_id) == 0) { + ret = rte_eth_allmulticast_disable(port_id); + if (ret != 0 && ret != -ENOTSUP) { + RTE_ETHDEV_LOG(ERR, + "Failed to disable allmulticast mode for device (port %u): %s\n", + port_id, rte_strerror(-ret)); + return ret; + } + } return 0; } @@ -1945,30 +1958,34 @@ rte_eth_promiscuous_get(uint16_t port_id) return dev->data->promiscuous; } -void +int rte_eth_allmulticast_enable(uint16_t port_id) { struct rte_eth_dev *dev; - RTE_ETH_VALID_PORTID_OR_RET(port_id); + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; - RTE_FUNC_PTR_OR_RET(*dev->dev_ops->allmulticast_enable); + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->allmulticast_enable, -ENOTSUP); (*dev->dev_ops->allmulticast_enable)(dev); dev->data->all_multicast = 1; + + return 0; } -void +int rte_eth_allmulticast_disable(uint16_t port_id) { struct rte_eth_dev *dev; - RTE_ETH_VALID_PORTID_OR_RET(port_id); + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; - RTE_FUNC_PTR_OR_RET(*dev->dev_ops->allmulticast_disable); + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->allmulticast_disable, -ENOTSUP); dev->data->all_multicast = 0; (*dev->dev_ops->allmulticast_disable)(dev); + + return 0; } int diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h index f07a829b29..d24e3f9a24 100644 --- a/lib/librte_ethdev/rte_ethdev.h +++ b/lib/librte_ethdev/rte_ethdev.h @@ -2060,16 +2060,26 @@ int rte_eth_promiscuous_get(uint16_t port_id); * * @param port_id * The port identifier of the Ethernet device. + * @return + * - (0) if successful. + * - (-ENOTSUP) if support for allmulticast_enable() does not exist + * for the device. + * - (-ENODEV) if *port_id* invalid. */ -void rte_eth_allmulticast_enable(uint16_t port_id); +int rte_eth_allmulticast_enable(uint16_t port_id); /** * Disable the receipt of all multicast frames by an Ethernet device. * * @param port_id * The port identifier of the Ethernet device. + * @return + * - (0) if successful. + * - (-ENOTSUP) if support for allmulticast_disable() does not exist + * for the device. + * - (-ENODEV) if *port_id* invalid. */ -void rte_eth_allmulticast_disable(uint16_t port_id); +int rte_eth_allmulticast_disable(uint16_t port_id); /** * Return the value of allmulticast mode for an Ethernet device. -- 2.17.1 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [dpdk-dev] [PATCH 2/7] net/failsafe: check code of allmulticast mode switch 2019-09-09 12:13 [dpdk-dev] [PATCH 0/7] ethdev: change allmulticast controls to return status Andrew Rybchenko 2019-09-09 12:13 ` [dpdk-dev] [PATCH 1/7] ethdev: change allmulticast mode controllers to return errors Andrew Rybchenko @ 2019-09-09 12:13 ` Andrew Rybchenko 2019-09-09 12:56 ` Gaëtan Rivet 2019-09-09 12:13 ` [dpdk-dev] [PATCH 3/7] net/bonding: " Andrew Rybchenko ` (4 subsequent siblings) 6 siblings, 1 reply; 18+ messages in thread From: Andrew Rybchenko @ 2019-09-09 12:13 UTC (permalink / raw) To: Gaetan Rivet; +Cc: dev, Ivan Ilchenko From: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru> rte_eth_allmulticast_enable()/rte_eth_allmulticast_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 all-multicast mode consistent across all active devices in the case of failure. Signed-off-by: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru> Signed-off-by: Andrew Rybchenko <arybchenko@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 bd38f1c1e4..93deacd134 100644 --- a/drivers/net/failsafe/failsafe_ether.c +++ b/drivers/net/failsafe/failsafe_ether.c @@ -140,9 +140,13 @@ fs_eth_dev_conf_apply(struct rte_eth_dev *dev, if (dev->data->all_multicast != edev->data->all_multicast) { DEBUG("Configuring all_multicast"); if (dev->data->all_multicast) - rte_eth_allmulticast_enable(PORT_ID(sdev)); + ret = rte_eth_allmulticast_enable(PORT_ID(sdev)); else - rte_eth_allmulticast_disable(PORT_ID(sdev)); + ret = rte_eth_allmulticast_disable(PORT_ID(sdev)); + if (ret != 0) { + ERROR("Failed to apply allmulticast mode"); + return ret; + } } else { DEBUG("all_multicast already set"); } diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c index fee783ad07..b90fa8676c 100644 --- a/drivers/net/failsafe/failsafe_ops.c +++ b/drivers/net/failsafe/failsafe_ops.c @@ -723,10 +723,28 @@ fs_allmulticast_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_allmulticast_enable(PORT_ID(sdev)); + FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { + ret = rte_eth_allmulticast_enable(PORT_ID(sdev)); + ret = fs_err(sdev, ret); + if (ret != 0) { + ERROR("All-multicast 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_allmulticast_disable(PORT_ID(sdev)); + ret = fs_err(sdev, ret); + if (ret != 0) + ERROR("All-multicast mode disable to rollback failed for subdevice %d", + PORT_ID(sdev)); + } + } fs_unlock(dev, 0); } @@ -735,10 +753,28 @@ fs_allmulticast_disable(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_allmulticast_disable(PORT_ID(sdev)); + FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { + ret = rte_eth_allmulticast_disable(PORT_ID(sdev)); + ret = fs_err(sdev, ret); + if (ret != 0) { + ERROR("All-multicast mode disable 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_allmulticast_enable(PORT_ID(sdev)); + ret = fs_err(sdev, ret); + if (ret != 0) + ERROR("All-multicast mode enable to rollback failed for subdevice %d", + PORT_ID(sdev)); + } + } fs_unlock(dev, 0); } -- 2.17.1 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [dpdk-dev] [PATCH 2/7] net/failsafe: check code of allmulticast mode switch 2019-09-09 12:13 ` [dpdk-dev] [PATCH 2/7] net/failsafe: check code of allmulticast mode switch Andrew Rybchenko @ 2019-09-09 12:56 ` Gaëtan Rivet 2019-09-13 21:04 ` Andrew Rybchenko 0 siblings, 1 reply; 18+ messages in thread From: Gaëtan Rivet @ 2019-09-09 12:56 UTC (permalink / raw) To: Andrew Rybchenko; +Cc: dev, Ivan Ilchenko On Mon, Sep 09, 2019 at 01:13:04PM +0100, Andrew Rybchenko wrote: > From: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru> > > rte_eth_allmulticast_enable()/rte_eth_allmulticast_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 all-multicast mode consistent across all active > devices in the case of failure. > > Signed-off-by: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru> > Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> A small nit below, but otherwise Acked-by: Gaetan Rivet <gaetan.rivet@6wind.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_ops.c b/drivers/net/failsafe/failsafe_ops.c > index fee783ad07..b90fa8676c 100644 > --- a/drivers/net/failsafe/failsafe_ops.c > +++ b/drivers/net/failsafe/failsafe_ops.c > @@ -723,10 +723,28 @@ fs_allmulticast_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_allmulticast_enable(PORT_ID(sdev)); > + FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { > + ret = rte_eth_allmulticast_enable(PORT_ID(sdev)); > + ret = fs_err(sdev, ret); > + if (ret != 0) { > + ERROR("All-multicast 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_allmulticast_disable(PORT_ID(sdev)); > + ret = fs_err(sdev, ret); > + if (ret != 0) > + ERROR("All-multicast mode disable to rollback failed for subdevice %d", I'd formulate the error as such: "All-multicast mode disable during rollback failed for subdevice %d" That may not be the best either, but the current format seems a little off? I could be wrong. > + PORT_ID(sdev)); > + } > + } > fs_unlock(dev, 0); > } > > @@ -735,10 +753,28 @@ fs_allmulticast_disable(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_allmulticast_disable(PORT_ID(sdev)); > + FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { > + ret = rte_eth_allmulticast_disable(PORT_ID(sdev)); > + ret = fs_err(sdev, ret); > + if (ret != 0) { > + ERROR("All-multicast mode disable 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_allmulticast_enable(PORT_ID(sdev)); > + ret = fs_err(sdev, ret); > + if (ret != 0) > + ERROR("All-multicast mode enable to rollback failed for subdevice %d", Same as above. > + PORT_ID(sdev)); > + } > + } > fs_unlock(dev, 0); > } > > -- > 2.17.1 > -- Gaëtan Rivet 6WIND ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [dpdk-dev] [PATCH 2/7] net/failsafe: check code of allmulticast mode switch 2019-09-09 12:56 ` Gaëtan Rivet @ 2019-09-13 21:04 ` Andrew Rybchenko 0 siblings, 0 replies; 18+ messages in thread From: Andrew Rybchenko @ 2019-09-13 21:04 UTC (permalink / raw) To: Gaëtan Rivet; +Cc: dev, Ivan Ilchenko On 9/9/19 3:56 PM, Gaëtan Rivet wrote: > On Mon, Sep 09, 2019 at 01:13:04PM +0100, Andrew Rybchenko wrote: >> From: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru> >> >> rte_eth_allmulticast_enable()/rte_eth_allmulticast_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 all-multicast mode consistent across all active >> devices in the case of failure. >> >> Signed-off-by: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru> >> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> > A small nit below, but otherwise > Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com> Thanks, will fix it in the next version and similar changes in promiscuous mode patches (to have no identical logs). ^ permalink raw reply [flat|nested] 18+ messages in thread
* [dpdk-dev] [PATCH 3/7] net/bonding: check code of allmulticast mode switch 2019-09-09 12:13 [dpdk-dev] [PATCH 0/7] ethdev: change allmulticast controls to return status Andrew Rybchenko 2019-09-09 12:13 ` [dpdk-dev] [PATCH 1/7] ethdev: change allmulticast mode controllers to return errors Andrew Rybchenko 2019-09-09 12:13 ` [dpdk-dev] [PATCH 2/7] net/failsafe: check code of allmulticast mode switch Andrew Rybchenko @ 2019-09-09 12:13 ` Andrew Rybchenko 2019-09-09 12:13 ` [dpdk-dev] [PATCH 4/7] ethdev: change allmulticast callbacks to return status Andrew Rybchenko ` (3 subsequent siblings) 6 siblings, 0 replies; 18+ messages in thread From: Andrew Rybchenko @ 2019-09-09 12:13 UTC (permalink / raw) To: Chas Williams; +Cc: dev, Ivan Ilchenko From: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru> rte_eth_allmulticast_enable()/rte_eth_allmulticast_disable() return value was changed from void to int, so this patch modify usage of these functions across net/bonding according to new return type. Signed-off-by: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> --- drivers/net/bonding/rte_eth_bond_8023ad.c | 13 +++++++-- drivers/net/bonding/rte_eth_bond_pmd.c | 33 +++++++++++++++++++---- 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c b/drivers/net/bonding/rte_eth_bond_8023ad.c index 7189a84585..e64fb6e41d 100644 --- a/drivers/net/bonding/rte_eth_bond_8023ad.c +++ b/drivers/net/bonding/rte_eth_bond_8023ad.c @@ -915,7 +915,12 @@ bond_mode_8023ad_register_lacp_mac(uint16_t slave_id) { int ret; - rte_eth_allmulticast_enable(slave_id); + ret = rte_eth_allmulticast_enable(slave_id); + if (ret != 0) { + RTE_BOND_LOG(ERR, + "failed to enable allmulti mode for port %u: %s", + slave_id, rte_strerror(-ret)); + } if (rte_eth_allmulticast_get(slave_id)) { RTE_BOND_LOG(DEBUG, "forced allmulti for port %u", slave_id); @@ -949,7 +954,11 @@ bond_mode_8023ad_unregister_lacp_mac(uint16_t slave_id) switch (bond_mode_8023ad_ports[slave_id].forced_rx_flags) { case BOND_8023AD_FORCED_ALLMULTI: RTE_BOND_LOG(DEBUG, "unset allmulti for port %u", slave_id); - rte_eth_allmulticast_disable(slave_id); + ret = rte_eth_allmulticast_disable(slave_id); + if (ret != 0) + RTE_BOND_LOG(ERR, + "failed to disable allmulti mode for port %u: %s", + slave_id, rte_strerror(-ret)); break; case BOND_8023AD_FORCED_PROMISC: diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c index f9b7b595df..f2ef4c3341 100644 --- a/drivers/net/bonding/rte_eth_bond_pmd.c +++ b/drivers/net/bonding/rte_eth_bond_pmd.c @@ -2601,6 +2601,8 @@ bond_ethdev_allmulticast_enable(struct rte_eth_dev *eth_dev) { struct bond_dev_private *internals = eth_dev->data->dev_private; int i; + int ret; + uint16_t port_id; switch (internals->mode) { /* allmulti mode is propagated to all slaves */ @@ -2609,9 +2611,13 @@ bond_ethdev_allmulticast_enable(struct rte_eth_dev *eth_dev) case BONDING_MODE_BROADCAST: case BONDING_MODE_8023AD: for (i = 0; i < internals->slave_count; i++) { - uint16_t port_id = internals->slaves[i].port_id; + port_id = internals->slaves[i].port_id; - rte_eth_allmulticast_enable(port_id); + ret = rte_eth_allmulticast_enable(port_id); + if (ret != 0) + RTE_BOND_LOG(ERR, + "Failed to enable allmulti mode for port %u: %s", + port_id, rte_strerror(-ret)); } break; /* allmulti mode is propagated only to primary slave */ @@ -2622,7 +2628,12 @@ bond_ethdev_allmulticast_enable(struct rte_eth_dev *eth_dev) /* Do not touch allmulti when there cannot be primary ports */ if (internals->slave_count == 0) break; - rte_eth_allmulticast_enable(internals->current_primary_port); + port_id = internals->current_primary_port; + ret = rte_eth_allmulticast_enable(port_id); + if (ret != 0) + RTE_BOND_LOG(ERR, + "Failed to enable allmulti mode for port %u: %s", + port_id, rte_strerror(-ret)); } } @@ -2631,6 +2642,8 @@ bond_ethdev_allmulticast_disable(struct rte_eth_dev *eth_dev) { struct bond_dev_private *internals = eth_dev->data->dev_private; int i; + int ret; + uint16_t port_id; switch (internals->mode) { /* allmulti mode is propagated to all slaves */ @@ -2645,7 +2658,12 @@ bond_ethdev_allmulticast_disable(struct rte_eth_dev *eth_dev) bond_mode_8023ad_ports[port_id].forced_rx_flags == BOND_8023AD_FORCED_ALLMULTI) continue; - rte_eth_allmulticast_disable(port_id); + + ret = rte_eth_allmulticast_disable(port_id); + if (ret != 0) + RTE_BOND_LOG(ERR, + "Failed to disable allmulti mode for port %u: %s", + port_id, rte_strerror(-ret)); } break; /* allmulti mode is propagated only to primary slave */ @@ -2656,7 +2674,12 @@ bond_ethdev_allmulticast_disable(struct rte_eth_dev *eth_dev) /* Do not touch allmulti when there cannot be primary ports */ if (internals->slave_count == 0) break; - rte_eth_allmulticast_disable(internals->current_primary_port); + port_id = internals->current_primary_port; + ret = rte_eth_allmulticast_disable(port_id); + if (ret != 0) + RTE_BOND_LOG(ERR, + "Failed to disable allmulti mode for port %u: %s", + port_id, rte_strerror(-ret)); } } -- 2.17.1 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [dpdk-dev] [PATCH 4/7] ethdev: change allmulticast callbacks to return status 2019-09-09 12:13 [dpdk-dev] [PATCH 0/7] ethdev: change allmulticast controls to return status Andrew Rybchenko ` (2 preceding siblings ...) 2019-09-09 12:13 ` [dpdk-dev] [PATCH 3/7] net/bonding: " Andrew Rybchenko @ 2019-09-09 12:13 ` Andrew Rybchenko 2019-09-16 7:03 ` Hyong Youb Kim (hyonkim) 2019-09-24 8:27 ` Ferruh Yigit 2019-09-09 12:13 ` [dpdk-dev] [PATCH 5/7] ethdev: do nothing if all-multicast mode is applied again Andrew Rybchenko ` (2 subsequent siblings) 6 siblings, 2 replies; 18+ messages in thread From: Andrew Rybchenko @ 2019-09-09 12:13 UTC (permalink / raw) To: Igor Russkikh, Pavel Belous, Ravi Kumar, Rasesh Mody, Shahed Shaikh, Ajit Khaparde, Somnath Kotur, Chas Williams, Rahul Lakkireddy, Hemant Agrawal, Sachin Saxena, Wenzhuo Lu, Gagandeep Singh, John Daley, Hyong Youb Kim, Gaetan Rivet, Qi Zhang, Xiao Wang, Beilei Xing, Jingjing Wu, Qiming Yang, Rosen Xu, Konstantin Ananyev, Shijith Thotton, Srisivasubramanian Srinivasan, Matan Azrad, Shahaf Shuler, Yongseok Koh, Viacheslav Ovsiienko, Tomasz Duszynski, Liron Himi, Stephen Hemminger, K. Y. Srinivasan, Haiyang Zhang, Rastislav Cernay, Jan Remes, Jerin Jacob, Nithin Dabilpuram, Kiran Kumar K, Keith Wiles, Maxime Coquelin, Tiwei Bie, Zhihong Wang, Yong Wang, Thomas Monjalon, Ferruh Yigit Cc: dev, Ivan Ilchenko From: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru> Enabling/disabling of allmulticast mode is not always successful and it should be taken into account to be able to handle it properly. When correct return status is unclear from driver code, -EAGAIN is used. Signed-off-by: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> --- drivers/net/atlantic/atl_ethdev.c | 14 ++++--- drivers/net/axgbe/axgbe_ethdev.c | 16 +++++--- drivers/net/bnx2x/bnx2x_ethdev.c | 8 +++- drivers/net/bnxt/bnxt_ethdev.c | 26 +++++++++--- drivers/net/bonding/rte_eth_bond_pmd.c | 38 +++++++++++++++--- drivers/net/cxgbe/cxgbe_ethdev.c | 12 +++--- drivers/net/cxgbe/cxgbe_pfvf.h | 4 +- drivers/net/dpaa/dpaa_ethdev.c | 8 +++- drivers/net/dpaa2/dpaa2_ethdev.c | 14 ++++--- drivers/net/e1000/em_ethdev.c | 14 ++++--- drivers/net/e1000/igb_ethdev.c | 26 +++++++----- drivers/net/enetc/enetc_ethdev.c | 10 +++-- drivers/net/enic/enic_ethdev.c | 22 +++++++--- drivers/net/failsafe/failsafe_ops.c | 8 +++- drivers/net/fm10k/fm10k_ethdev.c | 28 ++++++++----- drivers/net/i40e/i40e_ethdev.c | 22 ++++++---- drivers/net/i40e/i40e_ethdev_vf.c | 20 +++++++--- drivers/net/i40e/i40e_vf_representor.c | 8 ++-- drivers/net/iavf/iavf_ethdev.c | 20 +++++++--- drivers/net/ice/ice_ethdev.c | 30 ++++++++++---- drivers/net/ipn3ke/ipn3ke_ethdev.h | 4 +- drivers/net/ipn3ke/ipn3ke_representor.c | 8 +++- drivers/net/ixgbe/ixgbe_ethdev.c | 53 ++++++++++++++++++++----- drivers/net/liquidio/lio_ethdev.c | 12 +++--- drivers/net/mlx4/mlx4.h | 4 +- drivers/net/mlx4/mlx4_ethdev.c | 14 +++++-- drivers/net/mlx5/mlx5.h | 4 +- drivers/net/mlx5/mlx5_rxmode.c | 38 ++++++++++++++---- drivers/net/mvpp2/mrvl_ethdev.c | 28 +++++++++---- drivers/net/netvsc/hn_ethdev.c | 8 ++-- drivers/net/netvsc/hn_var.h | 4 +- drivers/net/netvsc/hn_vf.c | 8 ++-- drivers/net/nfb/nfb_rxmode.c | 10 +++-- drivers/net/nfb/nfb_rxmode.h | 8 +++- drivers/net/octeontx2/otx2_ethdev.h | 4 +- drivers/net/octeontx2/otx2_ethdev_ops.c | 8 +++- drivers/net/qede/qede_ethdev.c | 17 +++++--- drivers/net/sfc/sfc_ethdev.c | 8 ++-- drivers/net/szedata2/rte_eth_szedata2.c | 6 ++- drivers/net/tap/rte_eth_tap.c | 52 +++++++++++++++++++----- drivers/net/virtio/virtio_ethdev.c | 24 +++++++---- drivers/net/vmxnet3/vmxnet3_ethdev.c | 12 ++++-- lib/librte_ethdev/rte_ethdev.c | 18 ++++++--- lib/librte_ethdev/rte_ethdev_core.h | 4 +- 44 files changed, 497 insertions(+), 207 deletions(-) diff --git a/drivers/net/atlantic/atl_ethdev.c b/drivers/net/atlantic/atl_ethdev.c index 5018529da1..e463d24d27 100644 --- a/drivers/net/atlantic/atl_ethdev.c +++ b/drivers/net/atlantic/atl_ethdev.c @@ -26,8 +26,8 @@ static void atl_dev_close(struct rte_eth_dev *dev); static int atl_dev_reset(struct rte_eth_dev *dev); static int atl_dev_promiscuous_enable(struct rte_eth_dev *dev); static int atl_dev_promiscuous_disable(struct rte_eth_dev *dev); -static void atl_dev_allmulticast_enable(struct rte_eth_dev *dev); -static void atl_dev_allmulticast_disable(struct rte_eth_dev *dev); +static int atl_dev_allmulticast_enable(struct rte_eth_dev *dev); +static int atl_dev_allmulticast_disable(struct rte_eth_dev *dev); static int atl_dev_link_update(struct rte_eth_dev *dev, int wait); static int atl_dev_xstats_get_names(struct rte_eth_dev *dev __rte_unused, @@ -1227,23 +1227,27 @@ atl_dev_promiscuous_disable(struct rte_eth_dev *dev) return 0; } -static void +static int atl_dev_allmulticast_enable(struct rte_eth_dev *dev) { struct aq_hw_s *hw = ATL_DEV_PRIVATE_TO_HW(dev->data->dev_private); hw_atl_rpfl2_accept_all_mc_packets_set(hw, true); + + return 0; } -static void +static int atl_dev_allmulticast_disable(struct rte_eth_dev *dev) { struct aq_hw_s *hw = ATL_DEV_PRIVATE_TO_HW(dev->data->dev_private); if (dev->data->promiscuous == 1) - return; /* must remain in all_multicast mode */ + return 0; /* must remain in all_multicast mode */ hw_atl_rpfl2_accept_all_mc_packets_set(hw, false); + + return 0; } /** diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c index c43b5bfb6f..58ae9bd80a 100644 --- a/drivers/net/axgbe/axgbe_ethdev.c +++ b/drivers/net/axgbe/axgbe_ethdev.c @@ -17,8 +17,8 @@ static void axgbe_dev_interrupt_handler(void *param); static void axgbe_dev_close(struct rte_eth_dev *dev); static int axgbe_dev_promiscuous_enable(struct rte_eth_dev *dev); static int axgbe_dev_promiscuous_disable(struct rte_eth_dev *dev); -static void axgbe_dev_allmulticast_enable(struct rte_eth_dev *dev); -static void axgbe_dev_allmulticast_disable(struct rte_eth_dev *dev); +static int axgbe_dev_allmulticast_enable(struct rte_eth_dev *dev); +static int axgbe_dev_allmulticast_disable(struct rte_eth_dev *dev); static int axgbe_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete); static int axgbe_dev_stats_get(struct rte_eth_dev *dev, @@ -260,7 +260,7 @@ axgbe_dev_promiscuous_disable(struct rte_eth_dev *dev) return 0; } -static void +static int axgbe_dev_allmulticast_enable(struct rte_eth_dev *dev) { struct axgbe_port *pdata = dev->data->dev_private; @@ -268,11 +268,13 @@ axgbe_dev_allmulticast_enable(struct rte_eth_dev *dev) PMD_INIT_FUNC_TRACE(); if (AXGMAC_IOREAD_BITS(pdata, MAC_PFR, PM)) - return; + return 0; AXGMAC_IOWRITE_BITS(pdata, MAC_PFR, PM, 1); + + return 0; } -static void +static int axgbe_dev_allmulticast_disable(struct rte_eth_dev *dev) { struct axgbe_port *pdata = dev->data->dev_private; @@ -280,8 +282,10 @@ axgbe_dev_allmulticast_disable(struct rte_eth_dev *dev) PMD_INIT_FUNC_TRACE(); if (!AXGMAC_IOREAD_BITS(pdata, MAC_PFR, PM)) - return; + return 0; AXGMAC_IOWRITE_BITS(pdata, MAC_PFR, PM, 0); + + return 0; } /* return 0 means link status changed, -1 means not changed */ diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c index 07168e9a8a..20b045ff87 100644 --- a/drivers/net/bnx2x/bnx2x_ethdev.c +++ b/drivers/net/bnx2x/bnx2x_ethdev.c @@ -320,7 +320,7 @@ bnx2x_promisc_disable(struct rte_eth_dev *dev) return 0; } -static void +static int bnx2x_dev_allmulticast_enable(struct rte_eth_dev *dev) { struct bnx2x_softc *sc = dev->data->dev_private; @@ -330,9 +330,11 @@ bnx2x_dev_allmulticast_enable(struct rte_eth_dev *dev) if (rte_eth_promiscuous_get(dev->data->port_id) == 1) sc->rx_mode = BNX2X_RX_MODE_ALLMULTI_PROMISC; bnx2x_set_rx_mode(sc); + + return 0; } -static void +static int bnx2x_dev_allmulticast_disable(struct rte_eth_dev *dev) { struct bnx2x_softc *sc = dev->data->dev_private; @@ -342,6 +344,8 @@ bnx2x_dev_allmulticast_disable(struct rte_eth_dev *dev) if (rte_eth_promiscuous_get(dev->data->port_id) == 1) sc->rx_mode = BNX2X_RX_MODE_PROMISC; bnx2x_set_rx_mode(sc); + + return 0; } static int diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c index 7fff5d5b8f..eb8701131a 100644 --- a/drivers/net/bnxt/bnxt_ethdev.c +++ b/drivers/net/bnxt/bnxt_ethdev.c @@ -1048,32 +1048,46 @@ static int bnxt_promiscuous_disable_op(struct rte_eth_dev *eth_dev) return rc; } -static void bnxt_allmulticast_enable_op(struct rte_eth_dev *eth_dev) +static int bnxt_allmulticast_enable_op(struct rte_eth_dev *eth_dev) { struct bnxt *bp = eth_dev->data->dev_private; struct bnxt_vnic_info *vnic; + uint32_t old_flags; + int rc; if (bp->vnic_info == NULL) - return; + return 0; vnic = &bp->vnic_info[0]; + old_flags = vnic->flags; vnic->flags |= BNXT_VNIC_INFO_ALLMULTI; - bnxt_hwrm_cfa_l2_set_rx_mask(bp, vnic, 0, NULL); + rc = bnxt_hwrm_cfa_l2_set_rx_mask(bp, vnic, 0, NULL); + if (rc != 0) + vnic->flags = old_flags; + + return rc; } -static void bnxt_allmulticast_disable_op(struct rte_eth_dev *eth_dev) +static int bnxt_allmulticast_disable_op(struct rte_eth_dev *eth_dev) { struct bnxt *bp = eth_dev->data->dev_private; struct bnxt_vnic_info *vnic; + uint32_t old_flags; + int rc; if (bp->vnic_info == NULL) - return; + return 0; vnic = &bp->vnic_info[0]; + old_flags = vnic->flags; vnic->flags &= ~BNXT_VNIC_INFO_ALLMULTI; - bnxt_hwrm_cfa_l2_set_rx_mask(bp, vnic, 0, NULL); + rc = bnxt_hwrm_cfa_l2_set_rx_mask(bp, vnic, 0, NULL); + if (rc != 0) + vnic->flags = old_flags; + + return rc; } /* Return bnxt_rx_queue pointer corresponding to a given rxq. */ diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c index f2ef4c3341..4b2affd53e 100644 --- a/drivers/net/bonding/rte_eth_bond_pmd.c +++ b/drivers/net/bonding/rte_eth_bond_pmd.c @@ -2596,12 +2596,12 @@ bond_ethdev_promiscuous_disable(struct rte_eth_dev *dev) return ret; } -static void +static int bond_ethdev_allmulticast_enable(struct rte_eth_dev *eth_dev) { struct bond_dev_private *internals = eth_dev->data->dev_private; int i; - int ret; + int ret = 0; uint16_t port_id; switch (internals->mode) { @@ -2609,7 +2609,9 @@ bond_ethdev_allmulticast_enable(struct rte_eth_dev *eth_dev) case BONDING_MODE_ROUND_ROBIN: case BONDING_MODE_BALANCE: case BONDING_MODE_BROADCAST: - case BONDING_MODE_8023AD: + case BONDING_MODE_8023AD: { + unsigned int slave_ok = 0; + for (i = 0; i < internals->slave_count; i++) { port_id = internals->slaves[i].port_id; @@ -2618,8 +2620,17 @@ bond_ethdev_allmulticast_enable(struct rte_eth_dev *eth_dev) RTE_BOND_LOG(ERR, "Failed to enable allmulti mode for port %u: %s", port_id, rte_strerror(-ret)); + else + slave_ok++; } + /* + * Report success if operation is successful on at least + * on one slave. Otherwise return last error code. + */ + if (slave_ok > 0) + ret = 0; break; + } /* allmulti mode is propagated only to primary slave */ case BONDING_MODE_ACTIVE_BACKUP: case BONDING_MODE_TLB: @@ -2635,14 +2646,16 @@ bond_ethdev_allmulticast_enable(struct rte_eth_dev *eth_dev) "Failed to enable allmulti mode for port %u: %s", port_id, rte_strerror(-ret)); } + + return ret; } -static void +static int bond_ethdev_allmulticast_disable(struct rte_eth_dev *eth_dev) { struct bond_dev_private *internals = eth_dev->data->dev_private; int i; - int ret; + int ret = 0; uint16_t port_id; switch (internals->mode) { @@ -2650,7 +2663,9 @@ bond_ethdev_allmulticast_disable(struct rte_eth_dev *eth_dev) case BONDING_MODE_ROUND_ROBIN: case BONDING_MODE_BALANCE: case BONDING_MODE_BROADCAST: - case BONDING_MODE_8023AD: + case BONDING_MODE_8023AD: { + unsigned int slave_ok = 0; + for (i = 0; i < internals->slave_count; i++) { uint16_t port_id = internals->slaves[i].port_id; @@ -2664,8 +2679,17 @@ bond_ethdev_allmulticast_disable(struct rte_eth_dev *eth_dev) RTE_BOND_LOG(ERR, "Failed to disable allmulti mode for port %u: %s", port_id, rte_strerror(-ret)); + else + slave_ok++; } + /* + * Report success if operation is successful on at least + * on one slave. Otherwise return last error code. + */ + if (slave_ok > 0) + ret = 0; break; + } /* allmulti mode is propagated only to primary slave */ case BONDING_MODE_ACTIVE_BACKUP: case BONDING_MODE_TLB: @@ -2681,6 +2705,8 @@ bond_ethdev_allmulticast_disable(struct rte_eth_dev *eth_dev) "Failed to disable allmulti mode for port %u: %s", port_id, rte_strerror(-ret)); } + + return ret; } static void diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c index be001a0d24..996d61624b 100644 --- a/drivers/net/cxgbe/cxgbe_ethdev.c +++ b/drivers/net/cxgbe/cxgbe_ethdev.c @@ -168,26 +168,26 @@ int cxgbe_dev_promiscuous_disable(struct rte_eth_dev *eth_dev) 0, -1, 1, -1, false); } -void cxgbe_dev_allmulticast_enable(struct rte_eth_dev *eth_dev) +int cxgbe_dev_allmulticast_enable(struct rte_eth_dev *eth_dev) { struct port_info *pi = eth_dev->data->dev_private; struct adapter *adapter = pi->adapter; /* TODO: address filters ?? */ - t4_set_rxmode(adapter, adapter->mbox, pi->viid, -1, - -1, 1, 1, -1, false); + return t4_set_rxmode(adapter, adapter->mbox, pi->viid, -1, + -1, 1, 1, -1, false); } -void cxgbe_dev_allmulticast_disable(struct rte_eth_dev *eth_dev) +int cxgbe_dev_allmulticast_disable(struct rte_eth_dev *eth_dev) { struct port_info *pi = eth_dev->data->dev_private; struct adapter *adapter = pi->adapter; /* TODO: address filters ?? */ - t4_set_rxmode(adapter, adapter->mbox, pi->viid, -1, - -1, 0, 1, -1, false); + return t4_set_rxmode(adapter, adapter->mbox, pi->viid, -1, + -1, 0, 1, -1, false); } int cxgbe_dev_link_update(struct rte_eth_dev *eth_dev, diff --git a/drivers/net/cxgbe/cxgbe_pfvf.h b/drivers/net/cxgbe/cxgbe_pfvf.h index bfa07ba555..3a6ab416f6 100644 --- a/drivers/net/cxgbe/cxgbe_pfvf.h +++ b/drivers/net/cxgbe/cxgbe_pfvf.h @@ -14,8 +14,8 @@ int cxgbe_dev_info_get(struct rte_eth_dev *eth_dev, struct rte_eth_dev_info *device_info); int cxgbe_dev_promiscuous_enable(struct rte_eth_dev *eth_dev); int cxgbe_dev_promiscuous_disable(struct rte_eth_dev *eth_dev); -void cxgbe_dev_allmulticast_enable(struct rte_eth_dev *eth_dev); -void cxgbe_dev_allmulticast_disable(struct rte_eth_dev *eth_dev); +int cxgbe_dev_allmulticast_enable(struct rte_eth_dev *eth_dev); +int cxgbe_dev_allmulticast_disable(struct rte_eth_dev *eth_dev); int cxgbe_mac_addr_set(struct rte_eth_dev *dev, struct rte_ether_addr *addr); int cxgbe_dev_configure(struct rte_eth_dev *eth_dev); int cxgbe_dev_tx_queue_setup(struct rte_eth_dev *eth_dev, uint16_t queue_idx, diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c index ad28c110d7..7b668fb4ac 100644 --- a/drivers/net/dpaa/dpaa_ethdev.c +++ b/drivers/net/dpaa/dpaa_ethdev.c @@ -536,22 +536,26 @@ static int dpaa_eth_promiscuous_disable(struct rte_eth_dev *dev) return 0; } -static void dpaa_eth_multicast_enable(struct rte_eth_dev *dev) +static int dpaa_eth_multicast_enable(struct rte_eth_dev *dev) { struct dpaa_if *dpaa_intf = dev->data->dev_private; PMD_INIT_FUNC_TRACE(); fman_if_set_mcast_filter_table(dpaa_intf->fif); + + return 0; } -static void dpaa_eth_multicast_disable(struct rte_eth_dev *dev) +static int dpaa_eth_multicast_disable(struct rte_eth_dev *dev) { struct dpaa_if *dpaa_intf = dev->data->dev_private; PMD_INIT_FUNC_TRACE(); fman_if_reset_mcast_filter_table(dpaa_intf->fif); + + return 0; } static diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c index d9cc2c3510..6e30326d0b 100644 --- a/drivers/net/dpaa2/dpaa2_ethdev.c +++ b/drivers/net/dpaa2/dpaa2_ethdev.c @@ -1043,7 +1043,7 @@ dpaa2_dev_promiscuous_disable( return ret; } -static void +static int dpaa2_dev_allmulticast_enable( struct rte_eth_dev *dev) { @@ -1055,15 +1055,17 @@ dpaa2_dev_allmulticast_enable( if (dpni == NULL) { DPAA2_PMD_ERR("dpni is NULL"); - return; + return -ENODEV; } ret = dpni_set_multicast_promisc(dpni, CMD_PRI_LOW, priv->token, true); if (ret < 0) DPAA2_PMD_ERR("Unable to enable multicast mode %d", ret); + + return ret; } -static void +static int dpaa2_dev_allmulticast_disable(struct rte_eth_dev *dev) { int ret; @@ -1074,16 +1076,18 @@ dpaa2_dev_allmulticast_disable(struct rte_eth_dev *dev) if (dpni == NULL) { DPAA2_PMD_ERR("dpni is NULL"); - return; + return -ENODEV; } /* must remain on for all promiscuous */ if (dev->data->promiscuous == 1) - return; + return 0; ret = dpni_set_multicast_promisc(dpni, CMD_PRI_LOW, priv->token, false); if (ret < 0) DPAA2_PMD_ERR("Unable to disable multicast mode %d", ret); + + return ret; } static int diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c index b23e840376..f258eb5988 100644 --- a/drivers/net/e1000/em_ethdev.c +++ b/drivers/net/e1000/em_ethdev.c @@ -37,8 +37,8 @@ static void eth_em_stop(struct rte_eth_dev *dev); static void eth_em_close(struct rte_eth_dev *dev); static int eth_em_promiscuous_enable(struct rte_eth_dev *dev); static int eth_em_promiscuous_disable(struct rte_eth_dev *dev); -static void eth_em_allmulticast_enable(struct rte_eth_dev *dev); -static void eth_em_allmulticast_disable(struct rte_eth_dev *dev); +static int eth_em_allmulticast_enable(struct rte_eth_dev *dev); +static int eth_em_allmulticast_disable(struct rte_eth_dev *dev); static int eth_em_link_update(struct rte_eth_dev *dev, int wait_to_complete); static int eth_em_stats_get(struct rte_eth_dev *dev, @@ -1295,7 +1295,7 @@ eth_em_promiscuous_disable(struct rte_eth_dev *dev) return 0; } -static void +static int eth_em_allmulticast_enable(struct rte_eth_dev *dev) { struct e1000_hw *hw = @@ -1305,9 +1305,11 @@ eth_em_allmulticast_enable(struct rte_eth_dev *dev) rctl = E1000_READ_REG(hw, E1000_RCTL); rctl |= E1000_RCTL_MPE; E1000_WRITE_REG(hw, E1000_RCTL, rctl); + + return 0; } -static void +static int eth_em_allmulticast_disable(struct rte_eth_dev *dev) { struct e1000_hw *hw = @@ -1315,10 +1317,12 @@ eth_em_allmulticast_disable(struct rte_eth_dev *dev) uint32_t rctl; if (dev->data->promiscuous == 1) - return; /* must remain in all_multicast mode */ + return 0; /* must remain in all_multicast mode */ rctl = E1000_READ_REG(hw, E1000_RCTL); rctl &= (~E1000_RCTL_MPE); E1000_WRITE_REG(hw, E1000_RCTL, rctl); + + return 0; } static int diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c index a9f6de5d52..fd80490ccc 100644 --- a/drivers/net/e1000/igb_ethdev.c +++ b/drivers/net/e1000/igb_ethdev.c @@ -81,8 +81,8 @@ static void eth_igb_close(struct rte_eth_dev *dev); static int eth_igb_reset(struct rte_eth_dev *dev); static int eth_igb_promiscuous_enable(struct rte_eth_dev *dev); static int eth_igb_promiscuous_disable(struct rte_eth_dev *dev); -static void eth_igb_allmulticast_enable(struct rte_eth_dev *dev); -static void eth_igb_allmulticast_disable(struct rte_eth_dev *dev); +static int eth_igb_allmulticast_enable(struct rte_eth_dev *dev); +static int eth_igb_allmulticast_disable(struct rte_eth_dev *dev); static int eth_igb_link_update(struct rte_eth_dev *dev, int wait_to_complete); static int eth_igb_stats_get(struct rte_eth_dev *dev, @@ -158,8 +158,8 @@ static void igbvf_dev_stop(struct rte_eth_dev *dev); static void igbvf_dev_close(struct rte_eth_dev *dev); static int igbvf_promiscuous_enable(struct rte_eth_dev *dev); static int igbvf_promiscuous_disable(struct rte_eth_dev *dev); -static void igbvf_allmulticast_enable(struct rte_eth_dev *dev); -static void igbvf_allmulticast_disable(struct rte_eth_dev *dev); +static int igbvf_allmulticast_enable(struct rte_eth_dev *dev); +static int igbvf_allmulticast_disable(struct rte_eth_dev *dev); static int eth_igbvf_link_update(struct e1000_hw *hw); static int eth_igbvf_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *rte_stats); @@ -2551,7 +2551,7 @@ eth_igb_promiscuous_disable(struct rte_eth_dev *dev) return 0; } -static void +static int eth_igb_allmulticast_enable(struct rte_eth_dev *dev) { struct e1000_hw *hw = @@ -2561,9 +2561,11 @@ eth_igb_allmulticast_enable(struct rte_eth_dev *dev) rctl = E1000_READ_REG(hw, E1000_RCTL); rctl |= E1000_RCTL_MPE; E1000_WRITE_REG(hw, E1000_RCTL, rctl); + + return 0; } -static void +static int eth_igb_allmulticast_disable(struct rte_eth_dev *dev) { struct e1000_hw *hw = @@ -2571,10 +2573,12 @@ eth_igb_allmulticast_disable(struct rte_eth_dev *dev) uint32_t rctl; if (dev->data->promiscuous == 1) - return; /* must remain in all_multicast mode */ + return 0; /* must remain in all_multicast mode */ rctl = E1000_READ_REG(hw, E1000_RCTL); rctl &= (~E1000_RCTL_MPE); E1000_WRITE_REG(hw, E1000_RCTL, rctl); + + return 0; } static int @@ -3419,7 +3423,7 @@ igbvf_promiscuous_disable(struct rte_eth_dev *dev) return 0; } -static void +static int igbvf_allmulticast_enable(struct rte_eth_dev *dev) { struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private); @@ -3427,9 +3431,11 @@ igbvf_allmulticast_enable(struct rte_eth_dev *dev) /* In promiscuous mode multicast promisc already set */ if (dev->data->promiscuous == 0) e1000_promisc_set_vf(hw, e1000_promisc_multicast); + + return 0; } -static void +static int igbvf_allmulticast_disable(struct rte_eth_dev *dev) { struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private); @@ -3437,6 +3443,8 @@ igbvf_allmulticast_disable(struct rte_eth_dev *dev) /* In promiscuous mode leave multicast promisc enabled */ if (dev->data->promiscuous == 0) e1000_promisc_set_vf(hw, e1000_promisc_disabled); + + return 0; } static int igbvf_set_vfta(struct e1000_hw *hw, uint16_t vid, bool on) diff --git a/drivers/net/enetc/enetc_ethdev.c b/drivers/net/enetc/enetc_ethdev.c index 1ec66d0baf..8ea409c521 100644 --- a/drivers/net/enetc/enetc_ethdev.c +++ b/drivers/net/enetc/enetc_ethdev.c @@ -561,7 +561,7 @@ enetc_promiscuous_disable(struct rte_eth_dev *dev) return 0; } -static void +static int enetc_allmulticast_enable(struct rte_eth_dev *dev) { struct enetc_eth_hw *hw = @@ -575,9 +575,11 @@ enetc_allmulticast_enable(struct rte_eth_dev *dev) psipmr |= ENETC_PSIPMR_SET_MP(0); enetc_port_wr(enetc_hw, ENETC_PSIPMR, psipmr); + + return 0; } -static void +static int enetc_allmulticast_disable(struct rte_eth_dev *dev) { struct enetc_eth_hw *hw = @@ -586,13 +588,15 @@ enetc_allmulticast_disable(struct rte_eth_dev *dev) uint32_t psipmr = 0; if (dev->data->promiscuous == 1) - return; /* must remain in all_multicast mode */ + return 0; /* must remain in all_multicast mode */ /* Setting to disable all multicast mode for SI0*/ psipmr = enetc_port_rd(enetc_hw, ENETC_PSIPMR) & ~(ENETC_PSIPMR_SET_MP(0)); enetc_port_wr(enetc_hw, ENETC_PSIPMR, psipmr); + + return 0; } static int diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c index 5d48930a9d..e12ca213ae 100644 --- a/drivers/net/enic/enic_ethdev.c +++ b/drivers/net/enic/enic_ethdev.c @@ -638,28 +638,38 @@ static int enicpmd_dev_promiscuous_disable(struct rte_eth_dev *eth_dev) return ret; } -static void enicpmd_dev_allmulticast_enable(struct rte_eth_dev *eth_dev) +static int enicpmd_dev_allmulticast_enable(struct rte_eth_dev *eth_dev) { struct enic *enic = pmd_priv(eth_dev); + int ret; if (rte_eal_process_type() != RTE_PROC_PRIMARY) - return; + return -ENOTSUP; ENICPMD_FUNC_TRACE(); enic->allmulti = 1; - enic_add_packet_filter(enic); + ret = enic_add_packet_filter(enic); + if (ret != 0) + enic->allmulti = 0; + + return ret; } -static void enicpmd_dev_allmulticast_disable(struct rte_eth_dev *eth_dev) +static int enicpmd_dev_allmulticast_disable(struct rte_eth_dev *eth_dev) { struct enic *enic = pmd_priv(eth_dev); + int ret; if (rte_eal_process_type() != RTE_PROC_PRIMARY) - return; + return -ENOTSUP; ENICPMD_FUNC_TRACE(); enic->allmulti = 0; - enic_add_packet_filter(enic); + ret = enic_add_packet_filter(enic); + if (ret != 0) + enic->allmulti = 1; + + return ret; } static int enicpmd_add_mac_addr(struct rte_eth_dev *eth_dev, diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c index b90fa8676c..e40072610a 100644 --- a/drivers/net/failsafe/failsafe_ops.c +++ b/drivers/net/failsafe/failsafe_ops.c @@ -718,7 +718,7 @@ fs_promiscuous_disable(struct rte_eth_dev *dev) return ret; } -static void +static int fs_allmulticast_enable(struct rte_eth_dev *dev) { struct sub_device *sdev; @@ -746,9 +746,11 @@ fs_allmulticast_enable(struct rte_eth_dev *dev) } } fs_unlock(dev, 0); + + return ret; } -static void +static int fs_allmulticast_disable(struct rte_eth_dev *dev) { struct sub_device *sdev; @@ -776,6 +778,8 @@ fs_allmulticast_disable(struct rte_eth_dev *dev) } } fs_unlock(dev, 0); + + return ret; } static int diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c index f0f6290089..e9c9f1b619 100644 --- a/drivers/net/fm10k/fm10k_ethdev.c +++ b/drivers/net/fm10k/fm10k_ethdev.c @@ -46,8 +46,8 @@ int fm10k_logtype_driver; static void fm10k_close_mbx_service(struct fm10k_hw *hw); static int fm10k_dev_promiscuous_enable(struct rte_eth_dev *dev); static int fm10k_dev_promiscuous_disable(struct rte_eth_dev *dev); -static void fm10k_dev_allmulticast_enable(struct rte_eth_dev *dev); -static void fm10k_dev_allmulticast_disable(struct rte_eth_dev *dev); +static int fm10k_dev_allmulticast_enable(struct rte_eth_dev *dev); +static int fm10k_dev_allmulticast_disable(struct rte_eth_dev *dev); static inline int fm10k_glort_valid(struct fm10k_hw *hw); static int fm10k_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on); @@ -964,7 +964,7 @@ fm10k_dev_promiscuous_disable(struct rte_eth_dev *dev) return 0; } -static void +static int fm10k_dev_allmulticast_enable(struct rte_eth_dev *dev) { struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private); @@ -974,7 +974,7 @@ fm10k_dev_allmulticast_enable(struct rte_eth_dev *dev) /* Return if it didn't acquire valid glort range */ if ((hw->mac.type == fm10k_mac_pf) && !fm10k_glort_valid(hw)) - return; + return 0; /* If promiscuous mode is enabled, it doesn't make sense to enable * allmulticast and disable promiscuous since fm10k only can select @@ -983,7 +983,7 @@ fm10k_dev_allmulticast_enable(struct rte_eth_dev *dev) if (dev->data->promiscuous) { PMD_INIT_LOG(INFO, "Promiscuous mode is enabled, "\ "needn't enable allmulticast"); - return; + return 0; } fm10k_mbx_lock(hw); @@ -991,11 +991,15 @@ fm10k_dev_allmulticast_enable(struct rte_eth_dev *dev) FM10K_XCAST_MODE_ALLMULTI); fm10k_mbx_unlock(hw); - if (status != FM10K_SUCCESS) + if (status != FM10K_SUCCESS) { PMD_INIT_LOG(ERR, "Failed to enable allmulticast mode"); + return -EAGAIN; + } + + return 0; } -static void +static int fm10k_dev_allmulticast_disable(struct rte_eth_dev *dev) { struct fm10k_hw *hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private); @@ -1005,12 +1009,12 @@ fm10k_dev_allmulticast_disable(struct rte_eth_dev *dev) /* Return if it didn't acquire valid glort range */ if ((hw->mac.type == fm10k_mac_pf) && !fm10k_glort_valid(hw)) - return; + return 0; if (dev->data->promiscuous) { PMD_INIT_LOG(ERR, "Failed to disable allmulticast mode "\ "since promisc mode is enabled"); - return; + return -EINVAL; } fm10k_mbx_lock(hw); @@ -1019,8 +1023,12 @@ fm10k_dev_allmulticast_disable(struct rte_eth_dev *dev) FM10K_XCAST_MODE_NONE); fm10k_mbx_unlock(hw); - if (status != FM10K_SUCCESS) + if (status != FM10K_SUCCESS) { PMD_INIT_LOG(ERR, "Failed to disable allmulticast mode"); + return -EAGAIN; + } + + return 0; } static void diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c index 79bd3a70c9..dcba40082a 100644 --- a/drivers/net/i40e/i40e_ethdev.c +++ b/drivers/net/i40e/i40e_ethdev.c @@ -225,8 +225,8 @@ static void i40e_dev_close(struct rte_eth_dev *dev); static int i40e_dev_reset(struct rte_eth_dev *dev); static int i40e_dev_promiscuous_enable(struct rte_eth_dev *dev); static int i40e_dev_promiscuous_disable(struct rte_eth_dev *dev); -static void i40e_dev_allmulticast_enable(struct rte_eth_dev *dev); -static void i40e_dev_allmulticast_disable(struct rte_eth_dev *dev); +static int i40e_dev_allmulticast_enable(struct rte_eth_dev *dev); +static int i40e_dev_allmulticast_disable(struct rte_eth_dev *dev); static int i40e_dev_set_link_up(struct rte_eth_dev *dev); static int i40e_dev_set_link_down(struct rte_eth_dev *dev); static int i40e_dev_stats_get(struct rte_eth_dev *dev, @@ -2624,7 +2624,7 @@ i40e_dev_promiscuous_disable(struct rte_eth_dev *dev) return 0; } -static void +static int i40e_dev_allmulticast_enable(struct rte_eth_dev *dev) { struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private); @@ -2633,11 +2633,15 @@ i40e_dev_allmulticast_enable(struct rte_eth_dev *dev) int ret; ret = i40e_aq_set_vsi_multicast_promiscuous(hw, vsi->seid, TRUE, NULL); - if (ret != I40E_SUCCESS) + if (ret != I40E_SUCCESS) { PMD_DRV_LOG(ERR, "Failed to enable multicast promiscuous"); + return -EAGAIN; + } + + return 0; } -static void +static int i40e_dev_allmulticast_disable(struct rte_eth_dev *dev) { struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private); @@ -2646,12 +2650,16 @@ i40e_dev_allmulticast_disable(struct rte_eth_dev *dev) int ret; if (dev->data->promiscuous == 1) - return; /* must remain in all_multicast mode */ + return 0; /* must remain in all_multicast mode */ ret = i40e_aq_set_vsi_multicast_promiscuous(hw, vsi->seid, FALSE, NULL); - if (ret != I40E_SUCCESS) + if (ret != I40E_SUCCESS) { PMD_DRV_LOG(ERR, "Failed to disable multicast promiscuous"); + return -EAGAIN; + } + + return 0; } /* diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c index 2bbbacf00b..20125234c4 100644 --- a/drivers/net/i40e/i40e_ethdev_vf.c +++ b/drivers/net/i40e/i40e_ethdev_vf.c @@ -94,8 +94,8 @@ static void i40evf_dev_close(struct rte_eth_dev *dev); static int i40evf_dev_reset(struct rte_eth_dev *dev); static int i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev); static int i40evf_dev_promiscuous_disable(struct rte_eth_dev *dev); -static void i40evf_dev_allmulticast_enable(struct rte_eth_dev *dev); -static void i40evf_dev_allmulticast_disable(struct rte_eth_dev *dev); +static int i40evf_dev_allmulticast_enable(struct rte_eth_dev *dev); +static int i40evf_dev_allmulticast_disable(struct rte_eth_dev *dev); static int i40evf_init_vlan(struct rte_eth_dev *dev); static int i40evf_dev_rx_queue_start(struct rte_eth_dev *dev, uint16_t rx_queue_id); @@ -2194,7 +2194,7 @@ i40evf_dev_promiscuous_disable(struct rte_eth_dev *dev) return ret; } -static void +static int i40evf_dev_allmulticast_enable(struct rte_eth_dev *dev) { struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private); @@ -2202,14 +2202,18 @@ i40evf_dev_allmulticast_enable(struct rte_eth_dev *dev) /* If enabled, just return */ if (vf->promisc_multicast_enabled) - return; + return 0; ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 1); if (ret == 0) vf->promisc_multicast_enabled = TRUE; + else + ret = -EAGAIN; + + return ret; } -static void +static int i40evf_dev_allmulticast_disable(struct rte_eth_dev *dev) { struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private); @@ -2217,11 +2221,15 @@ i40evf_dev_allmulticast_disable(struct rte_eth_dev *dev) /* If enabled, just return */ if (!vf->promisc_multicast_enabled) - return; + return 0; ret = i40evf_config_promisc(dev, vf->promisc_unicast_enabled, 0); if (ret == 0) vf->promisc_multicast_enabled = FALSE; + else + ret = -EAGAIN; + + return ret; } static int diff --git a/drivers/net/i40e/i40e_vf_representor.c b/drivers/net/i40e/i40e_vf_representor.c index 7f69e27a24..5be2945d7e 100644 --- a/drivers/net/i40e/i40e_vf_representor.c +++ b/drivers/net/i40e/i40e_vf_representor.c @@ -294,22 +294,22 @@ i40e_vf_representor_promiscuous_disable(struct rte_eth_dev *ethdev) representor->vf_id, 0); } -static void +static int i40e_vf_representor_allmulticast_enable(struct rte_eth_dev *ethdev) { struct i40e_vf_representor *representor = ethdev->data->dev_private; - rte_pmd_i40e_set_vf_multicast_promisc( + return rte_pmd_i40e_set_vf_multicast_promisc( representor->adapter->eth_dev->data->port_id, representor->vf_id, 1); } -static void +static int i40e_vf_representor_allmulticast_disable(struct rte_eth_dev *ethdev) { struct i40e_vf_representor *representor = ethdev->data->dev_private; - rte_pmd_i40e_set_vf_multicast_promisc( + return rte_pmd_i40e_set_vf_multicast_promisc( representor->adapter->eth_dev->data->port_id, representor->vf_id, 0); } diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c index 22a88c88dd..e879040646 100644 --- a/drivers/net/iavf/iavf_ethdev.c +++ b/drivers/net/iavf/iavf_ethdev.c @@ -45,8 +45,8 @@ static int iavf_dev_stats_get(struct rte_eth_dev *dev, static void iavf_dev_stats_reset(struct rte_eth_dev *dev); static int iavf_dev_promiscuous_enable(struct rte_eth_dev *dev); static int iavf_dev_promiscuous_disable(struct rte_eth_dev *dev); -static void iavf_dev_allmulticast_enable(struct rte_eth_dev *dev); -static void iavf_dev_allmulticast_disable(struct rte_eth_dev *dev); +static int iavf_dev_allmulticast_enable(struct rte_eth_dev *dev); +static int iavf_dev_allmulticast_disable(struct rte_eth_dev *dev); static int iavf_dev_add_mac_addr(struct rte_eth_dev *dev, struct rte_ether_addr *addr, uint32_t index, @@ -674,7 +674,7 @@ iavf_dev_promiscuous_disable(struct rte_eth_dev *dev) return ret; } -static void +static int iavf_dev_allmulticast_enable(struct rte_eth_dev *dev) { struct iavf_adapter *adapter = @@ -683,14 +683,18 @@ iavf_dev_allmulticast_enable(struct rte_eth_dev *dev) int ret; if (vf->promisc_multicast_enabled) - return; + return 0; ret = iavf_config_promisc(adapter, vf->promisc_unicast_enabled, TRUE); if (!ret) vf->promisc_multicast_enabled = TRUE; + else + ret = -EAGAIN; + + return ret; } -static void +static int iavf_dev_allmulticast_disable(struct rte_eth_dev *dev) { struct iavf_adapter *adapter = @@ -699,11 +703,15 @@ iavf_dev_allmulticast_disable(struct rte_eth_dev *dev) int ret; if (!vf->promisc_multicast_enabled) - return; + return 0; ret = iavf_config_promisc(adapter, vf->promisc_unicast_enabled, FALSE); if (!ret) vf->promisc_multicast_enabled = FALSE; + else + ret = -EAGAIN; + + return ret; } static int diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c index eecf9c86c2..ac97efca97 100644 --- a/drivers/net/ice/ice_ethdev.c +++ b/drivers/net/ice/ice_ethdev.c @@ -60,8 +60,8 @@ static int ice_rss_hash_conf_get(struct rte_eth_dev *dev, struct rte_eth_rss_conf *rss_conf); static int ice_promisc_enable(struct rte_eth_dev *dev); static int ice_promisc_disable(struct rte_eth_dev *dev); -static void ice_allmulti_enable(struct rte_eth_dev *dev); -static void ice_allmulti_disable(struct rte_eth_dev *dev); +static int ice_allmulti_enable(struct rte_eth_dev *dev); +static int ice_allmulti_disable(struct rte_eth_dev *dev); static int ice_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on); @@ -3022,7 +3022,7 @@ ice_promisc_disable(struct rte_eth_dev *dev) return ret; } -static void +static int ice_allmulti_enable(struct rte_eth_dev *dev) { struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private); @@ -3030,15 +3030,26 @@ ice_allmulti_enable(struct rte_eth_dev *dev) struct ice_vsi *vsi = pf->main_vsi; enum ice_status status; uint8_t pmask; + int ret = 0; pmask = ICE_PROMISC_MCAST_RX | ICE_PROMISC_MCAST_TX; status = ice_set_vsi_promisc(hw, vsi->idx, pmask, 0); - if (status != ICE_SUCCESS) + + switch (status) { + case ICE_ERR_ALREADY_EXISTS: + PMD_DRV_LOG(DEBUG, "Allmulti has already been enabled"); + case ICE_SUCCESS: + break; + default: PMD_DRV_LOG(ERR, "Failed to enable allmulti, err=%d", status); + ret = -EAGAIN; + } + + return ret; } -static void +static int ice_allmulti_disable(struct rte_eth_dev *dev) { struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private); @@ -3046,15 +3057,20 @@ ice_allmulti_disable(struct rte_eth_dev *dev) struct ice_vsi *vsi = pf->main_vsi; enum ice_status status; uint8_t pmask; + int ret; if (dev->data->promiscuous == 1) - return; /* must remain in all_multicast mode */ + return 0; /* must remain in all_multicast mode */ pmask = ICE_PROMISC_MCAST_RX | ICE_PROMISC_MCAST_TX; status = ice_clear_vsi_promisc(hw, vsi->idx, pmask, 0); - if (status != ICE_SUCCESS) + if (status != ICE_SUCCESS) { PMD_DRV_LOG(ERR, "Failed to clear allmulti, err=%d", status); + ret = -EAGAIN; + } + + return ret; } static int ice_rx_queue_intr_enable(struct rte_eth_dev *dev, diff --git a/drivers/net/ipn3ke/ipn3ke_ethdev.h b/drivers/net/ipn3ke/ipn3ke_ethdev.h index 830e717970..6f0dfd98a3 100644 --- a/drivers/net/ipn3ke/ipn3ke_ethdev.h +++ b/drivers/net/ipn3ke/ipn3ke_ethdev.h @@ -543,9 +543,9 @@ int ipn3ke_rpst_promiscuous_enable(struct rte_eth_dev *ethdev); int ipn3ke_rpst_promiscuous_disable(struct rte_eth_dev *ethdev); -void +int ipn3ke_rpst_allmulticast_enable(struct rte_eth_dev *ethdev); -void +int ipn3ke_rpst_allmulticast_disable(struct rte_eth_dev *ethdev); int ipn3ke_rpst_mac_addr_set(struct rte_eth_dev *ethdev, diff --git a/drivers/net/ipn3ke/ipn3ke_representor.c b/drivers/net/ipn3ke/ipn3ke_representor.c index 9079073c95..6eb70236ad 100644 --- a/drivers/net/ipn3ke/ipn3ke_representor.c +++ b/drivers/net/ipn3ke/ipn3ke_representor.c @@ -2672,7 +2672,7 @@ ipn3ke_rpst_promiscuous_disable(struct rte_eth_dev *ethdev) return 0; } -void +int ipn3ke_rpst_allmulticast_enable(struct rte_eth_dev *ethdev) { struct ipn3ke_hw *hw = IPN3KE_DEV_PRIVATE_TO_HW(ethdev); @@ -2696,9 +2696,11 @@ ipn3ke_rpst_allmulticast_enable(struct rte_eth_dev *ethdev) rpst->port_id, 0); } + + return 0; } -void +int ipn3ke_rpst_allmulticast_disable(struct rte_eth_dev *ethdev) { struct ipn3ke_hw *hw = IPN3KE_DEV_PRIVATE_TO_HW(ethdev); @@ -2722,6 +2724,8 @@ ipn3ke_rpst_allmulticast_disable(struct rte_eth_dev *ethdev) rpst->port_id, 0); } + + return 0; } int diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c index 023b267d74..5f98497ae1 100644 --- a/drivers/net/ixgbe/ixgbe_ethdev.c +++ b/drivers/net/ixgbe/ixgbe_ethdev.c @@ -151,8 +151,8 @@ static void ixgbe_dev_close(struct rte_eth_dev *dev); static int ixgbe_dev_reset(struct rte_eth_dev *dev); static int ixgbe_dev_promiscuous_enable(struct rte_eth_dev *dev); static int ixgbe_dev_promiscuous_disable(struct rte_eth_dev *dev); -static void ixgbe_dev_allmulticast_enable(struct rte_eth_dev *dev); -static void ixgbe_dev_allmulticast_disable(struct rte_eth_dev *dev); +static int ixgbe_dev_allmulticast_enable(struct rte_eth_dev *dev); +static int ixgbe_dev_allmulticast_disable(struct rte_eth_dev *dev); static int ixgbe_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete); static int ixgbe_dev_stats_get(struct rte_eth_dev *dev, @@ -272,8 +272,8 @@ static void ixgbevf_set_ivar_map(struct ixgbe_hw *hw, int8_t direction, static void ixgbevf_configure_msix(struct rte_eth_dev *dev); static int ixgbevf_dev_promiscuous_enable(struct rte_eth_dev *dev); static int ixgbevf_dev_promiscuous_disable(struct rte_eth_dev *dev); -static void ixgbevf_dev_allmulticast_enable(struct rte_eth_dev *dev); -static void ixgbevf_dev_allmulticast_disable(struct rte_eth_dev *dev); +static int ixgbevf_dev_allmulticast_enable(struct rte_eth_dev *dev); +static int ixgbevf_dev_allmulticast_disable(struct rte_eth_dev *dev); /* For Eth VMDQ APIs support */ static int ixgbe_uc_hash_table_set(struct rte_eth_dev *dev, struct @@ -4211,7 +4211,7 @@ ixgbe_dev_promiscuous_disable(struct rte_eth_dev *dev) return 0; } -static void +static int ixgbe_dev_allmulticast_enable(struct rte_eth_dev *dev) { struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private); @@ -4220,20 +4220,24 @@ ixgbe_dev_allmulticast_enable(struct rte_eth_dev *dev) fctrl = IXGBE_READ_REG(hw, IXGBE_FCTRL); fctrl |= IXGBE_FCTRL_MPE; IXGBE_WRITE_REG(hw, IXGBE_FCTRL, fctrl); + + return 0; } -static void +static int ixgbe_dev_allmulticast_disable(struct rte_eth_dev *dev) { struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private); uint32_t fctrl; if (dev->data->promiscuous == 1) - return; /* must remain in all_multicast mode */ + return 0; /* must remain in all_multicast mode */ fctrl = IXGBE_READ_REG(hw, IXGBE_FCTRL); fctrl &= (~IXGBE_FCTRL_MPE); IXGBE_WRITE_REG(hw, IXGBE_FCTRL, fctrl); + + return 0; } /** @@ -8448,20 +8452,47 @@ ixgbevf_dev_promiscuous_disable(struct rte_eth_dev *dev) return ret; } -static void +static int ixgbevf_dev_allmulticast_enable(struct rte_eth_dev *dev) { struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private); + int ret; + int mode = IXGBEVF_XCAST_MODE_ALLMULTI; + + switch (hw->mac.ops.update_xcast_mode(hw, mode)) { + case IXGBE_SUCCESS: + ret = 0; + break; + case IXGBE_ERR_FEATURE_NOT_SUPPORTED: + ret = -ENOTSUP; + break; + default: + ret = -EAGAIN; + break; + } - hw->mac.ops.update_xcast_mode(hw, IXGBEVF_XCAST_MODE_ALLMULTI); + return ret; } -static void +static int ixgbevf_dev_allmulticast_disable(struct rte_eth_dev *dev) { struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private); + int ret; - hw->mac.ops.update_xcast_mode(hw, IXGBEVF_XCAST_MODE_MULTI); + switch (hw->mac.ops.update_xcast_mode(hw, IXGBEVF_XCAST_MODE_MULTI)) { + case IXGBE_SUCCESS: + ret = 0; + break; + case IXGBE_ERR_FEATURE_NOT_SUPPORTED: + ret = -ENOTSUP; + break; + default: + ret = -EAGAIN; + break; + } + + return ret; } static void ixgbevf_mbx_process(struct rte_eth_dev *dev) diff --git a/drivers/net/liquidio/lio_ethdev.c b/drivers/net/liquidio/lio_ethdev.c index e1eaf9eb8a..c7b0a766e1 100644 --- a/drivers/net/liquidio/lio_ethdev.c +++ b/drivers/net/liquidio/lio_ethdev.c @@ -1044,7 +1044,7 @@ lio_dev_promiscuous_disable(struct rte_eth_dev *eth_dev) return lio_change_dev_flag(eth_dev); } -static void +static int lio_dev_allmulticast_enable(struct rte_eth_dev *eth_dev) { struct lio_device *lio_dev = LIO_DEV(eth_dev); @@ -1052,14 +1052,14 @@ lio_dev_allmulticast_enable(struct rte_eth_dev *eth_dev) if (!lio_dev->intf_open) { lio_dev_err(lio_dev, "Port %d down, can't enable multicast\n", lio_dev->port_id); - return; + return -EAGAIN; } lio_dev->ifflags |= LIO_IFFLAG_ALLMULTI; - lio_change_dev_flag(eth_dev); + return lio_change_dev_flag(eth_dev); } -static void +static int lio_dev_allmulticast_disable(struct rte_eth_dev *eth_dev) { struct lio_device *lio_dev = LIO_DEV(eth_dev); @@ -1067,11 +1067,11 @@ lio_dev_allmulticast_disable(struct rte_eth_dev *eth_dev) if (!lio_dev->intf_open) { lio_dev_err(lio_dev, "Port %d down, can't disable multicast\n", lio_dev->port_id); - return; + return -EAGAIN; } lio_dev->ifflags &= ~LIO_IFFLAG_ALLMULTI; - lio_change_dev_flag(eth_dev); + return lio_change_dev_flag(eth_dev); } static void diff --git a/drivers/net/mlx4/mlx4.h b/drivers/net/mlx4/mlx4.h index 21517d70a2..59f315b899 100644 --- a/drivers/net/mlx4/mlx4.h +++ b/drivers/net/mlx4/mlx4.h @@ -207,8 +207,8 @@ int mlx4_dev_set_link_down(struct rte_eth_dev *dev); int mlx4_dev_set_link_up(struct rte_eth_dev *dev); int mlx4_promiscuous_enable(struct rte_eth_dev *dev); int mlx4_promiscuous_disable(struct rte_eth_dev *dev); -void mlx4_allmulticast_enable(struct rte_eth_dev *dev); -void mlx4_allmulticast_disable(struct rte_eth_dev *dev); +int mlx4_allmulticast_enable(struct rte_eth_dev *dev); +int mlx4_allmulticast_disable(struct rte_eth_dev *dev); void mlx4_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index); int mlx4_mac_addr_add(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr, uint32_t index, uint32_t vmdq); diff --git a/drivers/net/mlx4/mlx4_ethdev.c b/drivers/net/mlx4/mlx4_ethdev.c index c8a73bc1f4..c01303f856 100644 --- a/drivers/net/mlx4/mlx4_ethdev.c +++ b/drivers/net/mlx4/mlx4_ethdev.c @@ -414,11 +414,14 @@ mlx4_promiscuous_disable(struct rte_eth_dev *dev) * * @param dev * Pointer to Ethernet device structure. + * + * @return + * 0 on success, a negative errno value otherwise and rte_errno is set. */ -void +int mlx4_allmulticast_enable(struct rte_eth_dev *dev) { - mlx4_rxmode_toggle(dev, RXMODE_TOGGLE_ALLMULTI_ON); + return mlx4_rxmode_toggle(dev, RXMODE_TOGGLE_ALLMULTI_ON); } /** @@ -426,11 +429,14 @@ mlx4_allmulticast_enable(struct rte_eth_dev *dev) * * @param dev * Pointer to Ethernet device structure. + * + * @return + * 0 on success, a negative errno value otherwise and rte_errno is set. */ -void +int mlx4_allmulticast_disable(struct rte_eth_dev *dev) { - mlx4_rxmode_toggle(dev, RXMODE_TOGGLE_ALLMULTI_OFF); + return mlx4_rxmode_toggle(dev, RXMODE_TOGGLE_ALLMULTI_OFF); } /** diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index 11d540c3a5..bab8a8d672 100644 --- a/drivers/net/mlx5/mlx5.h +++ b/drivers/net/mlx5/mlx5.h @@ -754,8 +754,8 @@ int mlx5_dev_rss_reta_update(struct rte_eth_dev *dev, int mlx5_promiscuous_enable(struct rte_eth_dev *dev); int mlx5_promiscuous_disable(struct rte_eth_dev *dev); -void mlx5_allmulticast_enable(struct rte_eth_dev *dev); -void mlx5_allmulticast_disable(struct rte_eth_dev *dev); +int mlx5_allmulticast_enable(struct rte_eth_dev *dev); +int mlx5_allmulticast_disable(struct rte_eth_dev *dev); /* mlx5_stats.c */ diff --git a/drivers/net/mlx5/mlx5_rxmode.c b/drivers/net/mlx5/mlx5_rxmode.c index c862fc9520..b5752efb1b 100644 --- a/drivers/net/mlx5/mlx5_rxmode.c +++ b/drivers/net/mlx5/mlx5_rxmode.c @@ -103,8 +103,11 @@ mlx5_promiscuous_disable(struct rte_eth_dev *dev) * * @param dev * Pointer to Ethernet device structure. + * + * @return + * 0 on success, a negative errno value otherwise and rte_errno is set. */ -void +int mlx5_allmulticast_enable(struct rte_eth_dev *dev) { struct mlx5_priv *priv = dev->data->dev_private; @@ -116,14 +119,23 @@ mlx5_allmulticast_enable(struct rte_eth_dev *dev) "port %u cannot enable allmulticast mode" " in flow isolation mode", dev->data->port_id); - return; + return 0; + } + if (priv->config.vf) { + ret = mlx5_nl_allmulti(dev, 1); + if (ret) + goto error; } - if (priv->config.vf) - mlx5_nl_allmulti(dev, 1); ret = mlx5_traffic_restart(dev); if (ret) DRV_LOG(ERR, "port %u cannot enable allmulicast mode: %s", dev->data->port_id, strerror(rte_errno)); +error: + /* + * rte_eth_allmulticast_enable() rollback + * dev->data->all_multicast in the case of failure. + */ + return ret; } /** @@ -131,18 +143,30 @@ mlx5_allmulticast_enable(struct rte_eth_dev *dev) * * @param dev * Pointer to Ethernet device structure. + * + * @return + * 0 on success, a negative errno value otherwise and rte_errno is set. */ -void +int mlx5_allmulticast_disable(struct rte_eth_dev *dev) { struct mlx5_priv *priv = dev->data->dev_private; int ret; dev->data->all_multicast = 0; - if (priv->config.vf) - mlx5_nl_allmulti(dev, 0); + if (priv->config.vf) { + ret = mlx5_nl_allmulti(dev, 0); + if (ret) + goto error; + } ret = mlx5_traffic_restart(dev); if (ret) DRV_LOG(ERR, "port %u cannot disable allmulicast mode: %s", dev->data->port_id, strerror(rte_errno)); +error: + /* + * rte_eth_allmulticast_disable() rollback + * dev->data->all_multicast in the case of failure. + */ + return ret; } diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index 7babc891ae..963ff7dad6 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -1024,22 +1024,29 @@ mrvl_promiscuous_enable(struct rte_eth_dev *dev) * * @param dev * Pointer to Ethernet device structure. + * + * @return + * 0 on success, negative error value otherwise. */ -static void +static int mrvl_allmulticast_enable(struct rte_eth_dev *dev) { struct mrvl_priv *priv = dev->data->dev_private; int ret; if (!priv->ppio) - return; + return 0; if (priv->isolated) - return; + return 0; ret = pp2_ppio_set_mc_promisc(priv->ppio, 1); - if (ret) + if (ret) { MRVL_LOG(ERR, "Failed enable all-multicast mode"); + return -EAGAIN; + } + + return 0; } /** @@ -1074,19 +1081,26 @@ mrvl_promiscuous_disable(struct rte_eth_dev *dev) * * @param dev * Pointer to Ethernet device structure. + * + * @return + * 0 on success, negative error value otherwise. */ -static void +static int mrvl_allmulticast_disable(struct rte_eth_dev *dev) { struct mrvl_priv *priv = dev->data->dev_private; int ret; if (!priv->ppio) - return; + return 0; ret = pp2_ppio_set_mc_promisc(priv->ppio, 0); - if (ret) + if (ret) { MRVL_LOG(ERR, "Failed to disable all-multicast mode"); + return -EAGAIN; + } + + return 0; } /** diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c index d04a6c8acb..eb86134ce7 100644 --- a/drivers/net/netvsc/hn_ethdev.c +++ b/drivers/net/netvsc/hn_ethdev.c @@ -438,7 +438,7 @@ hn_dev_promiscuous_disable(struct rte_eth_dev *dev) return hn_vf_promiscuous_disable(dev); } -static void +static int hn_dev_allmulticast_enable(struct rte_eth_dev *dev) { struct hn_data *hv = dev->data->dev_private; @@ -446,17 +446,17 @@ hn_dev_allmulticast_enable(struct rte_eth_dev *dev) hn_rndis_set_rxfilter(hv, NDIS_PACKET_TYPE_DIRECTED | NDIS_PACKET_TYPE_ALL_MULTICAST | NDIS_PACKET_TYPE_BROADCAST); - hn_vf_allmulticast_enable(dev); + return hn_vf_allmulticast_enable(dev); } -static void +static int hn_dev_allmulticast_disable(struct rte_eth_dev *dev) { struct hn_data *hv = dev->data->dev_private; hn_rndis_set_rxfilter(hv, NDIS_PACKET_TYPE_DIRECTED | NDIS_PACKET_TYPE_BROADCAST); - hn_vf_allmulticast_disable(dev); + return hn_vf_allmulticast_disable(dev); } static int diff --git a/drivers/net/netvsc/hn_var.h b/drivers/net/netvsc/hn_var.h index 01f2276482..b72803a8b0 100644 --- a/drivers/net/netvsc/hn_var.h +++ b/drivers/net/netvsc/hn_var.h @@ -212,8 +212,8 @@ void hn_vf_reset(struct rte_eth_dev *dev); void hn_vf_stop(struct rte_eth_dev *dev); void hn_vf_close(struct rte_eth_dev *dev); -void hn_vf_allmulticast_enable(struct rte_eth_dev *dev); -void hn_vf_allmulticast_disable(struct rte_eth_dev *dev); +int hn_vf_allmulticast_enable(struct rte_eth_dev *dev); +int hn_vf_allmulticast_disable(struct rte_eth_dev *dev); int hn_vf_promiscuous_enable(struct rte_eth_dev *dev); int hn_vf_promiscuous_disable(struct rte_eth_dev *dev); int hn_vf_mc_addr_list(struct rte_eth_dev *dev, diff --git a/drivers/net/netvsc/hn_vf.c b/drivers/net/netvsc/hn_vf.c index d133438bbd..695f680216 100644 --- a/drivers/net/netvsc/hn_vf.c +++ b/drivers/net/netvsc/hn_vf.c @@ -400,14 +400,14 @@ void hn_vf_stats_reset(struct rte_eth_dev *dev) VF_ETHDEV_FUNC(dev, rte_eth_stats_reset); } -void hn_vf_allmulticast_enable(struct rte_eth_dev *dev) +int hn_vf_allmulticast_enable(struct rte_eth_dev *dev) { - VF_ETHDEV_FUNC(dev, rte_eth_allmulticast_enable); + VF_ETHDEV_FUNC_RET_STATUS(dev, rte_eth_allmulticast_enable); } -void hn_vf_allmulticast_disable(struct rte_eth_dev *dev) +int hn_vf_allmulticast_disable(struct rte_eth_dev *dev) { - VF_ETHDEV_FUNC(dev, rte_eth_allmulticast_disable); + VF_ETHDEV_FUNC_RET_STATUS(dev, rte_eth_allmulticast_disable); } int hn_vf_promiscuous_enable(struct rte_eth_dev *dev) diff --git a/drivers/net/nfb/nfb_rxmode.c b/drivers/net/nfb/nfb_rxmode.c index 17708c84c6..3327c8272b 100644 --- a/drivers/net/nfb/nfb_rxmode.c +++ b/drivers/net/nfb/nfb_rxmode.c @@ -59,7 +59,7 @@ nfb_eth_promiscuous_get(struct rte_eth_dev *dev) return (status.mac_filter == RXMAC_MAC_FILTER_PROMISCUOUS); } -void +int nfb_eth_allmulticast_enable(struct rte_eth_dev *dev) { struct pmd_internals *internals = (struct pmd_internals *) @@ -70,9 +70,11 @@ nfb_eth_allmulticast_enable(struct rte_eth_dev *dev) nc_rxmac_mac_filter_enable(internals->rxmac[i], RXMAC_MAC_FILTER_TABLE_BCAST_MCAST); } + + return 0; } -void +int nfb_eth_allmulticast_disable(struct rte_eth_dev *dev) { struct pmd_internals *internals = (struct pmd_internals *) @@ -82,12 +84,14 @@ nfb_eth_allmulticast_disable(struct rte_eth_dev *dev) /* if multicast is not enabled do nothing */ if (!nfb_eth_allmulticast_get(dev)) - return; + return 0; for (i = 0; i < internals->max_rxmac; ++i) { nc_rxmac_mac_filter_enable(internals->rxmac[i], internals->rx_filter_original); } + + return 0; } int diff --git a/drivers/net/nfb/nfb_rxmode.h b/drivers/net/nfb/nfb_rxmode.h index 1d5bafa98a..5a29e5ffee 100644 --- a/drivers/net/nfb/nfb_rxmode.h +++ b/drivers/net/nfb/nfb_rxmode.h @@ -57,8 +57,10 @@ nfb_eth_allmulticast_get(struct rte_eth_dev *dev); * * @param dev * Pointer to Ethernet device structure. + * + * @return always 0 */ -void +int nfb_eth_allmulticast_enable(struct rte_eth_dev *dev); /** @@ -66,8 +68,10 @@ nfb_eth_allmulticast_enable(struct rte_eth_dev *dev); * * @param dev * Pointer to Ethernet device structure. + * + * @return always 0 */ -void +int nfb_eth_allmulticast_disable(struct rte_eth_dev *dev); #endif /* _NFB_RXMODE_H_ */ diff --git a/drivers/net/octeontx2/otx2_ethdev.h b/drivers/net/octeontx2/otx2_ethdev.h index 8814622e43..1203c54636 100644 --- a/drivers/net/octeontx2/otx2_ethdev.h +++ b/drivers/net/octeontx2/otx2_ethdev.h @@ -381,8 +381,8 @@ int otx2_nix_rx_descriptor_status(void *rx_queue, uint16_t offset); void otx2_nix_promisc_config(struct rte_eth_dev *eth_dev, int en); int otx2_nix_promisc_enable(struct rte_eth_dev *eth_dev); int otx2_nix_promisc_disable(struct rte_eth_dev *eth_dev); -void otx2_nix_allmulticast_enable(struct rte_eth_dev *eth_dev); -void otx2_nix_allmulticast_disable(struct rte_eth_dev *eth_dev); +int otx2_nix_allmulticast_enable(struct rte_eth_dev *eth_dev); +int otx2_nix_allmulticast_disable(struct rte_eth_dev *eth_dev); int otx2_nix_tx_queue_start(struct rte_eth_dev *eth_dev, uint16_t qidx); int otx2_nix_tx_queue_stop(struct rte_eth_dev *eth_dev, uint16_t qidx); uint64_t otx2_nix_rxq_mbuf_setup(struct otx2_eth_dev *dev, uint16_t port_id); diff --git a/drivers/net/octeontx2/otx2_ethdev_ops.c b/drivers/net/octeontx2/otx2_ethdev_ops.c index 5a97a090ae..26eef6bd2a 100644 --- a/drivers/net/octeontx2/otx2_ethdev_ops.c +++ b/drivers/net/octeontx2/otx2_ethdev_ops.c @@ -167,16 +167,20 @@ nix_allmulticast_config(struct rte_eth_dev *eth_dev, int en) otx2_mbox_process(mbox); } -void +int otx2_nix_allmulticast_enable(struct rte_eth_dev *eth_dev) { nix_allmulticast_config(eth_dev, 1); + + return 0; } -void +int otx2_nix_allmulticast_disable(struct rte_eth_dev *eth_dev) { nix_allmulticast_config(eth_dev, 0); + + return 0; } void diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c index cfca6c4bc7..5409bae019 100644 --- a/drivers/net/qede/qede_ethdev.c +++ b/drivers/net/qede/qede_ethdev.c @@ -1744,25 +1744,32 @@ static void qede_reset_stats(struct rte_eth_dev *eth_dev) qede_reset_queue_stats(qdev, false); } -static void qede_allmulticast_enable(struct rte_eth_dev *eth_dev) +static int qede_allmulticast_enable(struct rte_eth_dev *eth_dev) { enum qed_filter_rx_mode_type type = QED_FILTER_RX_MODE_TYPE_MULTI_PROMISC; + enum _ecore_status_t ecore_status; if (rte_eth_promiscuous_get(eth_dev->data->port_id) == 1) type |= QED_FILTER_RX_MODE_TYPE_PROMISC; - qed_configure_filter_rx_mode(eth_dev, type); + ecore_status = qed_configure_filter_rx_mode(eth_dev, type); + + return ecore_status >= ECORE_SUCCESS ? 0 : -EAGAIN; } -static void qede_allmulticast_disable(struct rte_eth_dev *eth_dev) +static int qede_allmulticast_disable(struct rte_eth_dev *eth_dev) { + enum _ecore_status_t ecore_status; + if (rte_eth_promiscuous_get(eth_dev->data->port_id) == 1) - qed_configure_filter_rx_mode(eth_dev, + ecore_status = qed_configure_filter_rx_mode(eth_dev, QED_FILTER_RX_MODE_TYPE_PROMISC); else - qed_configure_filter_rx_mode(eth_dev, + ecore_status = qed_configure_filter_rx_mode(eth_dev, QED_FILTER_RX_MODE_TYPE_REGULAR); + + return ecore_status >= ECORE_SUCCESS ? 0 : -EAGAIN; } static int diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c index 5faf14b674..8a3fcaa57e 100644 --- a/drivers/net/sfc/sfc_ethdev.c +++ b/drivers/net/sfc/sfc_ethdev.c @@ -414,16 +414,16 @@ sfc_dev_promisc_disable(struct rte_eth_dev *dev) return sfc_dev_filter_set(dev, SFC_DEV_FILTER_MODE_PROMISC, B_FALSE); } -static void +static int sfc_dev_allmulti_enable(struct rte_eth_dev *dev) { - sfc_dev_filter_set(dev, SFC_DEV_FILTER_MODE_ALLMULTI, B_TRUE); + return sfc_dev_filter_set(dev, SFC_DEV_FILTER_MODE_ALLMULTI, B_TRUE); } -static void +static int sfc_dev_allmulti_disable(struct rte_eth_dev *dev) { - sfc_dev_filter_set(dev, SFC_DEV_FILTER_MODE_ALLMULTI, B_FALSE); + return sfc_dev_filter_set(dev, SFC_DEV_FILTER_MODE_ALLMULTI, B_FALSE); } static int diff --git a/drivers/net/szedata2/rte_eth_szedata2.c b/drivers/net/szedata2/rte_eth_szedata2.c index 2f3811b67f..0ca52e28e2 100644 --- a/drivers/net/szedata2/rte_eth_szedata2.c +++ b/drivers/net/szedata2/rte_eth_szedata2.c @@ -1360,16 +1360,18 @@ eth_promiscuous_disable(struct rte_eth_dev *dev __rte_unused) return -ENOTSUP; } -static void +static int eth_allmulticast_enable(struct rte_eth_dev *dev __rte_unused) { PMD_DRV_LOG(WARNING, "Enabling allmulticast mode is not supported."); + return -ENOTSUP; } -static void +static int eth_allmulticast_disable(struct rte_eth_dev *dev __rte_unused) { PMD_DRV_LOG(WARNING, "Disabling allmulticast mode is not supported."); + return -ENOTSUP; } static const struct eth_dev_ops ops = { diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c index 41612ce838..ef62a8eea9 100644 --- a/drivers/net/tap/rte_eth_tap.c +++ b/drivers/net/tap/rte_eth_tap.c @@ -1156,28 +1156,60 @@ tap_promisc_disable(struct rte_eth_dev *dev) return 0; } -static void +static int tap_allmulti_enable(struct rte_eth_dev *dev) { struct pmd_internals *pmd = dev->data->dev_private; struct ifreq ifr = { .ifr_flags = IFF_ALLMULTI }; + int ret; - dev->data->all_multicast = 1; - tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 1, LOCAL_AND_REMOTE); - if (pmd->remote_if_index && !pmd->flow_isolate) - tap_flow_implicit_create(pmd, TAP_REMOTE_ALLMULTI); + ret = tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 1, LOCAL_AND_REMOTE); + if (ret != 0) + return ret; + + if (pmd->remote_if_index && !pmd->flow_isolate) { + dev->data->all_multicast = 1; + ret = tap_flow_implicit_create(pmd, TAP_REMOTE_ALLMULTI); + if (ret != 0) { + /* Rollback allmulti flag */ + tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 0, LOCAL_AND_REMOTE); + /* + * rte_eth_dev_allmulticast_enable() rollback + * dev->data->all_multicast in the case of failure. + */ + return ret; + } + } + + return 0; } -static void +static int tap_allmulti_disable(struct rte_eth_dev *dev) { struct pmd_internals *pmd = dev->data->dev_private; struct ifreq ifr = { .ifr_flags = IFF_ALLMULTI }; + int ret; - dev->data->all_multicast = 0; - tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 0, LOCAL_AND_REMOTE); - if (pmd->remote_if_index && !pmd->flow_isolate) - tap_flow_implicit_destroy(pmd, TAP_REMOTE_ALLMULTI); + ret = tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 0, LOCAL_AND_REMOTE); + if (ret != 0) + return ret; + + if (pmd->remote_if_index && !pmd->flow_isolate) { + dev->data->all_multicast = 0; + ret = tap_flow_implicit_destroy(pmd, TAP_REMOTE_ALLMULTI); + if (ret != 0) { + /* Rollback allmulti flag */ + tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 1, LOCAL_AND_REMOTE); + /* + * rte_eth_dev_allmulticast_disable() rollback + * dev->data->all_multicast in the case of failure. + */ + return ret; + } + } + + return 0; } static int diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c index 1ba4aa37e8..2ae438105a 100644 --- a/drivers/net/virtio/virtio_ethdev.c +++ b/drivers/net/virtio/virtio_ethdev.c @@ -43,8 +43,8 @@ static int virtio_dev_start(struct rte_eth_dev *dev); static void virtio_dev_stop(struct rte_eth_dev *dev); static int virtio_dev_promiscuous_enable(struct rte_eth_dev *dev); static int virtio_dev_promiscuous_disable(struct rte_eth_dev *dev); -static void virtio_dev_allmulticast_enable(struct rte_eth_dev *dev); -static void virtio_dev_allmulticast_disable(struct rte_eth_dev *dev); +static int virtio_dev_allmulticast_enable(struct rte_eth_dev *dev); +static int virtio_dev_allmulticast_disable(struct rte_eth_dev *dev); static int virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info); static int virtio_dev_link_update(struct rte_eth_dev *dev, @@ -800,7 +800,7 @@ virtio_dev_promiscuous_disable(struct rte_eth_dev *dev) return 0; } -static void +static int virtio_dev_allmulticast_enable(struct rte_eth_dev *dev) { struct virtio_hw *hw = dev->data->dev_private; @@ -810,7 +810,7 @@ virtio_dev_allmulticast_enable(struct rte_eth_dev *dev) if (!vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_RX)) { PMD_INIT_LOG(INFO, "host does not support rx control"); - return; + return -ENOTSUP; } ctrl.hdr.class = VIRTIO_NET_CTRL_RX; @@ -819,11 +819,15 @@ virtio_dev_allmulticast_enable(struct rte_eth_dev *dev) dlen[0] = 1; ret = virtio_send_command(hw->cvq, &ctrl, dlen, 1); - if (ret) + if (ret) { PMD_INIT_LOG(ERR, "Failed to enable allmulticast"); + return -EAGAIN; + } + + return 0; } -static void +static int virtio_dev_allmulticast_disable(struct rte_eth_dev *dev) { struct virtio_hw *hw = dev->data->dev_private; @@ -833,7 +837,7 @@ virtio_dev_allmulticast_disable(struct rte_eth_dev *dev) if (!vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_RX)) { PMD_INIT_LOG(INFO, "host does not support rx control"); - return; + return -ENOTSUP; } ctrl.hdr.class = VIRTIO_NET_CTRL_RX; @@ -842,8 +846,12 @@ virtio_dev_allmulticast_disable(struct rte_eth_dev *dev) dlen[0] = 1; ret = virtio_send_command(hw->cvq, &ctrl, dlen, 1); - if (ret) + if (ret) { PMD_INIT_LOG(ERR, "Failed to disable allmulticast"); + return -EAGAIN; + } + + return 0; } #define VLAN_TAG_LEN 4 /* 802.3ac tag (not DMA'd) */ diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c index 8bfe16c482..22438d7a7d 100644 --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c @@ -67,8 +67,8 @@ static void vmxnet3_dev_close(struct rte_eth_dev *dev); static void vmxnet3_dev_set_rxmode(struct vmxnet3_hw *hw, uint32_t feature, int set); static int vmxnet3_dev_promiscuous_enable(struct rte_eth_dev *dev); static int vmxnet3_dev_promiscuous_disable(struct rte_eth_dev *dev); -static void vmxnet3_dev_allmulticast_enable(struct rte_eth_dev *dev); -static void vmxnet3_dev_allmulticast_disable(struct rte_eth_dev *dev); +static int vmxnet3_dev_allmulticast_enable(struct rte_eth_dev *dev); +static int vmxnet3_dev_allmulticast_disable(struct rte_eth_dev *dev); static int __vmxnet3_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete); static int vmxnet3_dev_link_update(struct rte_eth_dev *dev, @@ -1297,21 +1297,25 @@ vmxnet3_dev_promiscuous_disable(struct rte_eth_dev *dev) } /* Allmulticast supported only if Vmxnet3_DriverShared is initialized in adapter */ -static void +static int vmxnet3_dev_allmulticast_enable(struct rte_eth_dev *dev) { struct vmxnet3_hw *hw = dev->data->dev_private; vmxnet3_dev_set_rxmode(hw, VMXNET3_RXM_ALL_MULTI, 1); + + return 0; } /* Allmulticast supported only if Vmxnet3_DriverShared is initialized in adapter */ -static void +static int vmxnet3_dev_allmulticast_disable(struct rte_eth_dev *dev) { struct vmxnet3_hw *hw = dev->data->dev_private; vmxnet3_dev_set_rxmode(hw, VMXNET3_RXM_ALL_MULTI, 0); + + return 0; } /* Enable/disable filter on vlan */ diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c index 577640fdbe..8115226c91 100644 --- a/lib/librte_ethdev/rte_ethdev.c +++ b/lib/librte_ethdev/rte_ethdev.c @@ -1962,30 +1962,38 @@ int rte_eth_allmulticast_enable(uint16_t port_id) { struct rte_eth_dev *dev; + uint8_t old_allmulticast; + int diag; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->allmulticast_enable, -ENOTSUP); - (*dev->dev_ops->allmulticast_enable)(dev); - dev->data->all_multicast = 1; + old_allmulticast = dev->data->all_multicast; + diag = (*dev->dev_ops->allmulticast_enable)(dev); + dev->data->all_multicast = (diag == 0) ? 1 : old_allmulticast; - return 0; + return eth_err(port_id, diag); } int rte_eth_allmulticast_disable(uint16_t port_id) { struct rte_eth_dev *dev; + uint8_t old_allmulticast; + int diag; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->allmulticast_disable, -ENOTSUP); + old_allmulticast = dev->data->all_multicast; dev->data->all_multicast = 0; - (*dev->dev_ops->allmulticast_disable)(dev); + diag = (*dev->dev_ops->allmulticast_disable)(dev); + if (diag != 0) + dev->data->all_multicast = old_allmulticast; - return 0; + return eth_err(port_id, diag); } int diff --git a/lib/librte_ethdev/rte_ethdev_core.h b/lib/librte_ethdev/rte_ethdev_core.h index 6322348d17..8d5c67a418 100644 --- a/lib/librte_ethdev/rte_ethdev_core.h +++ b/lib/librte_ethdev/rte_ethdev_core.h @@ -58,10 +58,10 @@ typedef int (*eth_promiscuous_enable_t)(struct rte_eth_dev *dev); typedef int (*eth_promiscuous_disable_t)(struct rte_eth_dev *dev); /**< @internal Function used to disable the RX promiscuous mode of an Ethernet device. */ -typedef void (*eth_allmulticast_enable_t)(struct rte_eth_dev *dev); +typedef int (*eth_allmulticast_enable_t)(struct rte_eth_dev *dev); /**< @internal Enable the receipt of all multicast packets by an Ethernet device. */ -typedef void (*eth_allmulticast_disable_t)(struct rte_eth_dev *dev); +typedef int (*eth_allmulticast_disable_t)(struct rte_eth_dev *dev); /**< @internal Disable the receipt of all multicast packets by an Ethernet device. */ typedef int (*eth_link_update_t)(struct rte_eth_dev *dev, -- 2.17.1 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [dpdk-dev] [PATCH 4/7] ethdev: change allmulticast callbacks to return status 2019-09-09 12:13 ` [dpdk-dev] [PATCH 4/7] ethdev: change allmulticast callbacks to return status Andrew Rybchenko @ 2019-09-16 7:03 ` Hyong Youb Kim (hyonkim) 2019-09-16 7:29 ` Andrew Rybchenko 2019-09-24 8:27 ` Ferruh Yigit 1 sibling, 1 reply; 18+ messages in thread From: Hyong Youb Kim (hyonkim) @ 2019-09-16 7:03 UTC (permalink / raw) To: Andrew Rybchenko, Igor Russkikh, Pavel Belous, Ravi Kumar, Rasesh Mody, Shahed Shaikh, Ajit Khaparde, Somnath Kotur, Chas Williams, Rahul Lakkireddy, Hemant Agrawal, Sachin Saxena, Wenzhuo Lu, Gagandeep Singh, John Daley (johndale), Gaetan Rivet, Qi Zhang, Xiao Wang, Beilei Xing, Jingjing Wu, Qiming Yang, Rosen Xu, Konstantin Ananyev, Shijith Thotton, Srisivasubramanian Srinivasan, Matan Azrad, Shahaf Shuler, Yongseok Koh, Viacheslav Ovsiienko, Tomasz Duszynski, Liron Himi, Stephen Hemminger, K. Y. Srinivasan, Haiyang Zhang, Rastislav Cernay, Jan Remes, Jerin Jacob, Nithin Dabilpuram, Kiran Kumar K, Keith Wiles, Maxime Coquelin, Tiwei Bie, Zhihong Wang, Yong Wang, Thomas Monjalon, Ferruh Yigit Cc: dev, Ivan Ilchenko > -----Original Message----- > From: Andrew Rybchenko <arybchenko@solarflare.com> > Sent: Monday, September 9, 2019 9:13 PM [...] > Subject: [PATCH 4/7] ethdev: change allmulticast callbacks to return status > > From: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru> > > Enabling/disabling of allmulticast mode is not always successful and > it should be taken into account to be able to handle it properly. > > When correct return status is unclear from driver code, -EAGAIN is used. > > Signed-off-by: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru> > Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> > --- [...] > drivers/net/enic/enic_ethdev.c | 22 +++++++--- [...] > diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c > index 5d48930a9d..e12ca213ae 100644 > --- a/drivers/net/enic/enic_ethdev.c > +++ b/drivers/net/enic/enic_ethdev.c > @@ -638,28 +638,38 @@ static int enicpmd_dev_promiscuous_disable(struct > rte_eth_dev *eth_dev) > return ret; > } > > -static void enicpmd_dev_allmulticast_enable(struct rte_eth_dev *eth_dev) > +static int enicpmd_dev_allmulticast_enable(struct rte_eth_dev *eth_dev) > { > struct enic *enic = pmd_priv(eth_dev); > + int ret; > > if (rte_eal_process_type() != RTE_PROC_PRIMARY) > - return; > + return -ENOTSUP; Hi Andrew, If you are making v2, could you make this to return -E_RTE_SECONDARY, as in the promisc patch? > > ENICPMD_FUNC_TRACE(); > enic->allmulti = 1; > - enic_add_packet_filter(enic); > + ret = enic_add_packet_filter(enic); > + if (ret != 0) > + enic->allmulti = 0; > + > + return ret; > } > > -static void enicpmd_dev_allmulticast_disable(struct rte_eth_dev *eth_dev) > +static int enicpmd_dev_allmulticast_disable(struct rte_eth_dev *eth_dev) > { > struct enic *enic = pmd_priv(eth_dev); > + int ret; > > if (rte_eal_process_type() != RTE_PROC_PRIMARY) > - return; > + return -ENOTSUP; Here too. I tested all 5 series (promisc, allmulti, ...) with enic, and my test cases all passed. Acked-by: Hyong Youb Kim <hyonkim@cisco.com> Thanks. -Hyong > > ENICPMD_FUNC_TRACE(); > enic->allmulti = 0; > - enic_add_packet_filter(enic); > + ret = enic_add_packet_filter(enic); > + if (ret != 0) > + enic->allmulti = 1; > + > + return ret; > } > > static int enicpmd_add_mac_addr(struct rte_eth_dev *eth_dev, ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [dpdk-dev] [PATCH 4/7] ethdev: change allmulticast callbacks to return status 2019-09-16 7:03 ` Hyong Youb Kim (hyonkim) @ 2019-09-16 7:29 ` Andrew Rybchenko 0 siblings, 0 replies; 18+ messages in thread From: Andrew Rybchenko @ 2019-09-16 7:29 UTC (permalink / raw) To: Hyong Youb Kim (hyonkim), Igor Russkikh, Pavel Belous, Ravi Kumar, Rasesh Mody, Shahed Shaikh, Ajit Khaparde, Somnath Kotur, Chas Williams, Rahul Lakkireddy, Hemant Agrawal, Sachin Saxena, Wenzhuo Lu, Gagandeep Singh, John Daley (johndale), Gaetan Rivet, Qi Zhang, Xiao Wang, Beilei Xing, Jingjing Wu, Qiming Yang, Rosen Xu, Konstantin Ananyev, Shijith Thotton, Srisivasubramanian Srinivasan, Matan Azrad, Shahaf Shuler, Yongseok Koh, Viacheslav Ovsiienko, Tomasz Duszynski, Liron Himi, Stephen Hemminger, K. Y. Srinivasan, Haiyang Zhang, Rastislav Cernay, Jan Remes, Jerin Jacob, Nithin Dabilpuram, Kiran Kumar K, Keith Wiles, Maxime Coquelin, Tiwei Bie, Zhihong Wang, Yong Wang, Thomas Monjalon, Ferruh Yigit Cc: dev, Ivan Ilchenko Hi Hyong, On 9/16/19 10:03 AM, Hyong Youb Kim (hyonkim) wrote: >> -----Original Message----- >> From: Andrew Rybchenko <arybchenko@solarflare.com> >> Sent: Monday, September 9, 2019 9:13 PM > [...] >> Subject: [PATCH 4/7] ethdev: change allmulticast callbacks to return status >> >> From: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru> >> >> Enabling/disabling of allmulticast mode is not always successful and >> it should be taken into account to be able to handle it properly. >> >> When correct return status is unclear from driver code, -EAGAIN is used. >> >> Signed-off-by: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru> >> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> >> --- > [...] >> drivers/net/enic/enic_ethdev.c | 22 +++++++--- > [...] >> diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c >> index 5d48930a9d..e12ca213ae 100644 >> --- a/drivers/net/enic/enic_ethdev.c >> +++ b/drivers/net/enic/enic_ethdev.c >> @@ -638,28 +638,38 @@ static int enicpmd_dev_promiscuous_disable(struct >> rte_eth_dev *eth_dev) >> return ret; >> } >> >> -static void enicpmd_dev_allmulticast_enable(struct rte_eth_dev *eth_dev) >> +static int enicpmd_dev_allmulticast_enable(struct rte_eth_dev *eth_dev) >> { >> struct enic *enic = pmd_priv(eth_dev); >> + int ret; >> >> if (rte_eal_process_type() != RTE_PROC_PRIMARY) >> - return; >> + return -ENOTSUP; > Hi Andrew, > > If you are making v2, could you make this to return -E_RTE_SECONDARY, > as in the promisc patch? Yes, of course. I'll do in v2. >> ENICPMD_FUNC_TRACE(); >> enic->allmulti = 1; >> - enic_add_packet_filter(enic); >> + ret = enic_add_packet_filter(enic); >> + if (ret != 0) >> + enic->allmulti = 0; >> + >> + return ret; >> } >> >> -static void enicpmd_dev_allmulticast_disable(struct rte_eth_dev *eth_dev) >> +static int enicpmd_dev_allmulticast_disable(struct rte_eth_dev *eth_dev) >> { >> struct enic *enic = pmd_priv(eth_dev); >> + int ret; >> >> if (rte_eal_process_type() != RTE_PROC_PRIMARY) >> - return; >> + return -ENOTSUP; > Here too. > > I tested all 5 series (promisc, allmulti, ...) with enic, and my test > cases all passed. > > Acked-by: Hyong Youb Kim <hyonkim@cisco.com> Thanks for the testing and review, Andrew. > Thanks. > -Hyong > >> ENICPMD_FUNC_TRACE(); >> enic->allmulti = 0; >> - enic_add_packet_filter(enic); >> + ret = enic_add_packet_filter(enic); >> + if (ret != 0) >> + enic->allmulti = 1; >> + >> + return ret; >> } >> >> static int enicpmd_add_mac_addr(struct rte_eth_dev *eth_dev, ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [dpdk-dev] [PATCH 4/7] ethdev: change allmulticast callbacks to return status 2019-09-09 12:13 ` [dpdk-dev] [PATCH 4/7] ethdev: change allmulticast callbacks to return status Andrew Rybchenko 2019-09-16 7:03 ` Hyong Youb Kim (hyonkim) @ 2019-09-24 8:27 ` Ferruh Yigit 2019-09-24 12:14 ` Andrew Rybchenko 1 sibling, 1 reply; 18+ messages in thread From: Ferruh Yigit @ 2019-09-24 8:27 UTC (permalink / raw) To: Andrew Rybchenko, Igor Russkikh, Pavel Belous, Ravi Kumar, Rasesh Mody, Shahed Shaikh, Ajit Khaparde, Somnath Kotur, Chas Williams, Rahul Lakkireddy, Hemant Agrawal, Sachin Saxena, Wenzhuo Lu, Gagandeep Singh, John Daley, Hyong Youb Kim, Gaetan Rivet, Qi Zhang, Xiao Wang, Beilei Xing, Jingjing Wu, Qiming Yang, Rosen Xu, Konstantin Ananyev, Shijith Thotton, Srisivasubramanian Srinivasan, Matan Azrad, Shahaf Shuler, Yongseok Koh, Viacheslav Ovsiienko, Tomasz Duszynski, Liron Himi, Stephen Hemminger, K. Y. Srinivasan, Haiyang Zhang, Rastislav Cernay, Jan Remes, Jerin Jacob, Nithin Dabilpuram, Kiran Kumar K, Keith Wiles, Maxime Coquelin, Tiwei Bie, Zhihong Wang, Yong Wang, Thomas Monjalon Cc: dev, Ivan Ilchenko On 9/9/2019 1:13 PM, Andrew Rybchenko wrote: > From: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru> > > Enabling/disabling of allmulticast mode is not always successful and > it should be taken into account to be able to handle it properly. > > When correct return status is unclear from driver code, -EAGAIN is used. > > Signed-off-by: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru> > Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> <...> > @@ -1227,23 +1227,27 @@ atl_dev_promiscuous_disable(struct rte_eth_dev *dev) > return 0; > } > > -static void > +static int > atl_dev_allmulticast_enable(struct rte_eth_dev *dev) > { > struct aq_hw_s *hw = ATL_DEV_PRIVATE_TO_HW(dev->data->dev_private); > > hw_atl_rpfl2_accept_all_mc_packets_set(hw, true); > + > + return 0; > } > > -static void > +static int > atl_dev_allmulticast_disable(struct rte_eth_dev *dev) > { > struct aq_hw_s *hw = ATL_DEV_PRIVATE_TO_HW(dev->data->dev_private); > > if (dev->data->promiscuous == 1) > - return; /* must remain in all_multicast mode */ > + return 0; /* must remain in all_multicast mode */ What about making this change in the API level, so all dev_ops not need to do the similar check. Indeed I can see you are already adding this to API in following patches, but we can document and guarantee this behavior in API level, so driver developers can know and rely on this behavior, what do you think? And this can be a general guideline for all enable/disable APIs. If the feature already enabled, detect in the API and don't call underlying enable dev_ops, same for the disable. This saves doing the checks by multiple drivers. <...> > @@ -58,10 +58,10 @@ typedef int (*eth_promiscuous_enable_t)(struct rte_eth_dev *dev); > typedef int (*eth_promiscuous_disable_t)(struct rte_eth_dev *dev); > /**< @internal Function used to disable the RX promiscuous mode of an Ethernet device. */ > > -typedef void (*eth_allmulticast_enable_t)(struct rte_eth_dev *dev); > +typedef int (*eth_allmulticast_enable_t)(struct rte_eth_dev *dev); > /**< @internal Enable the receipt of all multicast packets by an Ethernet device. */ > > -typedef void (*eth_allmulticast_disable_t)(struct rte_eth_dev *dev); > +typedef int (*eth_allmulticast_disable_t)(struct rte_eth_dev *dev); > /**< @internal Disable the receipt of all multicast packets by an Ethernet device. */ Can you please document what a driver should return? As done in other patches. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [dpdk-dev] [PATCH 4/7] ethdev: change allmulticast callbacks to return status 2019-09-24 8:27 ` Ferruh Yigit @ 2019-09-24 12:14 ` Andrew Rybchenko 0 siblings, 0 replies; 18+ messages in thread From: Andrew Rybchenko @ 2019-09-24 12:14 UTC (permalink / raw) To: Ferruh Yigit, Igor Russkikh, Pavel Belous, Ravi Kumar, Rasesh Mody, Shahed Shaikh, Ajit Khaparde, Somnath Kotur, Chas Williams, Rahul Lakkireddy, Hemant Agrawal, Sachin Saxena, Wenzhuo Lu, Gagandeep Singh, John Daley, Hyong Youb Kim, Gaetan Rivet, Qi Zhang, Xiao Wang, Beilei Xing, Jingjing Wu, Qiming Yang, Rosen Xu, Konstantin Ananyev, Shijith Thotton, Srisivasubramanian Srinivasan, Matan Azrad, Shahaf Shuler, Yongseok Koh, Viacheslav Ovsiienko, Tomasz Duszynski, Liron Himi, Stephen Hemminger, K. Y. Srinivasan, Haiyang Zhang, Rastislav Cernay, Jan Remes, Jerin Jacob, Nithin Dabilpuram, Kiran Kumar K, Keith Wiles, Maxime Coquelin, Tiwei Bie, Zhihong Wang, Yong Wang, Thomas Monjalon Cc: dev, Ivan Ilchenko On 9/24/19 11:27 AM, Ferruh Yigit wrote: > On 9/9/2019 1:13 PM, Andrew Rybchenko wrote: >> From: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru> >> >> Enabling/disabling of allmulticast mode is not always successful and >> it should be taken into account to be able to handle it properly. >> >> When correct return status is unclear from driver code, -EAGAIN is used. >> >> Signed-off-by: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru> >> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> > <...> > >> @@ -1227,23 +1227,27 @@ atl_dev_promiscuous_disable(struct rte_eth_dev *dev) >> return 0; >> } >> >> -static void >> +static int >> atl_dev_allmulticast_enable(struct rte_eth_dev *dev) >> { >> struct aq_hw_s *hw = ATL_DEV_PRIVATE_TO_HW(dev->data->dev_private); >> >> hw_atl_rpfl2_accept_all_mc_packets_set(hw, true); >> + >> + return 0; >> } >> >> -static void >> +static int >> atl_dev_allmulticast_disable(struct rte_eth_dev *dev) >> { >> struct aq_hw_s *hw = ATL_DEV_PRIVATE_TO_HW(dev->data->dev_private); >> >> if (dev->data->promiscuous == 1) >> - return; /* must remain in all_multicast mode */ >> + return 0; /* must remain in all_multicast mode */ > What about making this change in the API level, so all dev_ops not need to do > the similar check. > Indeed I can see you are already adding this to API in following patches, but we > can document and guarantee this behavior in API level, so driver developers can > know and rely on this behavior, what do you think? > > And this can be a general guideline for all enable/disable APIs. If the feature > already enabled, detect in the API and don't call underlying enable dev_ops, > same for the disable. This saves doing the checks by multiple drivers. I'm a bit confused. It is all-multicast disable callback which check promiscuous mode and it relies on fact that promiscuous mode dominates. However, a driver could still have own settings which are used on promiscuous enable/disable to configure HW consistently. If we're talking about all-multicast mode check here, in general I agree, but there is rte_eth_dev_config_restore() which calls ops function to replay current settings. If driver cares about it on start itself, the check still makes sense. I would prefer to require it from drivers to apply correct settings on startup and remove rte_eth_dev_config_restore() function completely. It would allow to remove such checks from driver callbacks. However, I understand that current solution is generic and still makes sense. > <...> > >> @@ -58,10 +58,10 @@ typedef int (*eth_promiscuous_enable_t)(struct rte_eth_dev *dev); >> typedef int (*eth_promiscuous_disable_t)(struct rte_eth_dev *dev); >> /**< @internal Function used to disable the RX promiscuous mode of an Ethernet device. */ >> >> -typedef void (*eth_allmulticast_enable_t)(struct rte_eth_dev *dev); >> +typedef int (*eth_allmulticast_enable_t)(struct rte_eth_dev *dev); >> /**< @internal Enable the receipt of all multicast packets by an Ethernet device. */ >> >> -typedef void (*eth_allmulticast_disable_t)(struct rte_eth_dev *dev); >> +typedef int (*eth_allmulticast_disable_t)(struct rte_eth_dev *dev); >> /**< @internal Disable the receipt of all multicast packets by an Ethernet device. */ > Can you please document what a driver should return? As done in other patches. Yes, will do in v2. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [dpdk-dev] [PATCH 5/7] ethdev: do nothing if all-multicast mode is applied again 2019-09-09 12:13 [dpdk-dev] [PATCH 0/7] ethdev: change allmulticast controls to return status Andrew Rybchenko ` (3 preceding siblings ...) 2019-09-09 12:13 ` [dpdk-dev] [PATCH 4/7] ethdev: change allmulticast callbacks to return status Andrew Rybchenko @ 2019-09-09 12:13 ` Andrew Rybchenko 2019-09-24 8:36 ` Ferruh Yigit 2019-09-09 12:13 ` [dpdk-dev] [PATCH 6/7] app/testpmd: check code of allmulticast mode switch Andrew Rybchenko 2019-09-09 12:13 ` [dpdk-dev] [PATCH 7/7] examples/ipv4_multicast: check allmulticast enable status Andrew Rybchenko 6 siblings, 1 reply; 18+ messages in thread From: Andrew Rybchenko @ 2019-09-09 12:13 UTC (permalink / raw) To: Thomas Monjalon, Ferruh Yigit; +Cc: dev Since driver callbacks return status code now, there is no necessity to enable or disable all-multicast mode once again if it is already successfully enabled or disabled. Configuration restore at startup tries to ensure that configured all-multicast mode is applied and start will return error if it fails. Also it avoids theoretical cases when already configured all-multicast mode is applied once again and fails. In this cases it is unclear which value should be reported on get (configured or opposite). Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> --- lib/librte_ethdev/rte_ethdev.c | 40 ++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c index 8115226c91..e1921e8225 100644 --- a/lib/librte_ethdev/rte_ethdev.c +++ b/lib/librte_ethdev/rte_ethdev.c @@ -1416,16 +1416,22 @@ rte_eth_dev_config_restore(struct rte_eth_dev *dev, } /* replay all multicast configuration */ - if (rte_eth_allmulticast_get(port_id) == 1) { - ret = rte_eth_allmulticast_enable(port_id); + /* + * use callbacks directly since we don't need port_id check and + * would like to bypass the same value set + */ + if (rte_eth_allmulticast_get(port_id) == 1 && + *dev->dev_ops->allmulticast_enable != NULL) { + ret = (*dev->dev_ops->allmulticast_enable)(dev); if (ret != 0 && ret != -ENOTSUP) { RTE_ETHDEV_LOG(ERR, "Failed to enable allmulticast mode for device (port %u): %s\n", port_id, rte_strerror(-ret)); return ret; } - } else if (rte_eth_allmulticast_get(port_id) == 0) { - ret = rte_eth_allmulticast_disable(port_id); + } else if (rte_eth_allmulticast_get(port_id) == 0 && + *dev->dev_ops->allmulticast_disable != NULL) { + ret = (*dev->dev_ops->allmulticast_disable)(dev); if (ret != 0 && ret != -ENOTSUP) { RTE_ETHDEV_LOG(ERR, "Failed to disable allmulticast mode for device (port %u): %s\n", @@ -1962,16 +1968,17 @@ int rte_eth_allmulticast_enable(uint16_t port_id) { struct rte_eth_dev *dev; - uint8_t old_allmulticast; - int diag; + int diag = 0; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->allmulticast_enable, -ENOTSUP); - old_allmulticast = dev->data->all_multicast; - diag = (*dev->dev_ops->allmulticast_enable)(dev); - dev->data->all_multicast = (diag == 0) ? 1 : old_allmulticast; + + if (dev->data->all_multicast == 0) { + diag = (*dev->dev_ops->allmulticast_enable)(dev); + dev->data->all_multicast = (diag == 0) ? 1 : 0; + } return eth_err(port_id, diag); } @@ -1980,18 +1987,19 @@ int rte_eth_allmulticast_disable(uint16_t port_id) { struct rte_eth_dev *dev; - uint8_t old_allmulticast; - int diag; + int diag = 0; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->allmulticast_disable, -ENOTSUP); - old_allmulticast = dev->data->all_multicast; - dev->data->all_multicast = 0; - diag = (*dev->dev_ops->allmulticast_disable)(dev); - if (diag != 0) - dev->data->all_multicast = old_allmulticast; + + if (dev->data->all_multicast == 1) { + dev->data->all_multicast = 0; + diag = (*dev->dev_ops->allmulticast_disable)(dev); + if (diag != 0) + dev->data->all_multicast = 1; + } return eth_err(port_id, diag); } -- 2.17.1 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [dpdk-dev] [PATCH 5/7] ethdev: do nothing if all-multicast mode is applied again 2019-09-09 12:13 ` [dpdk-dev] [PATCH 5/7] ethdev: do nothing if all-multicast mode is applied again Andrew Rybchenko @ 2019-09-24 8:36 ` Ferruh Yigit 2019-09-24 8:54 ` Andrew Rybchenko 0 siblings, 1 reply; 18+ messages in thread From: Ferruh Yigit @ 2019-09-24 8:36 UTC (permalink / raw) To: Andrew Rybchenko, Thomas Monjalon; +Cc: dev On 9/9/2019 1:13 PM, Andrew Rybchenko wrote: > Since driver callbacks return status code now, there is no necessity > to enable or disable all-multicast mode once again if it is already > successfully enabled or disabled. > > Configuration restore at startup tries to ensure that configured > all-multicast mode is applied and start will return error if it fails. > > Also it avoids theoretical cases when already configured all-multicast > mode is applied once again and fails. In this cases it is unclear > which value should be reported on get (configured or opposite). > > Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> > --- > lib/librte_ethdev/rte_ethdev.c | 40 ++++++++++++++++++++-------------- > 1 file changed, 24 insertions(+), 16 deletions(-) > > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c > index 8115226c91..e1921e8225 100644 > --- a/lib/librte_ethdev/rte_ethdev.c > +++ b/lib/librte_ethdev/rte_ethdev.c > @@ -1416,16 +1416,22 @@ rte_eth_dev_config_restore(struct rte_eth_dev *dev, > } > > /* replay all multicast configuration */ > - if (rte_eth_allmulticast_get(port_id) == 1) { > - ret = rte_eth_allmulticast_enable(port_id); > + /* > + * use callbacks directly since we don't need port_id check and > + * would like to bypass the same value set > + */ > + if (rte_eth_allmulticast_get(port_id) == 1 && > + *dev->dev_ops->allmulticast_enable != NULL) { > + ret = (*dev->dev_ops->allmulticast_enable)(dev); I am for using the API here, it is more abstract instead of adding the dev_ops null checks etc. Will there be any downside to use the API? <...> > @@ -1962,16 +1968,17 @@ int > rte_eth_allmulticast_enable(uint16_t port_id) > { > struct rte_eth_dev *dev; > - uint8_t old_allmulticast; > - int diag; > + int diag = 0; > > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > dev = &rte_eth_devices[port_id]; > > RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->allmulticast_enable, -ENOTSUP); > - old_allmulticast = dev->data->all_multicast; > - diag = (*dev->dev_ops->allmulticast_enable)(dev); > - dev->data->all_multicast = (diag == 0) ? 1 : old_allmulticast; > + > + if (dev->data->all_multicast == 0) { What about adding this check even before 'allmulticast_enable' check, so if the multicast is already enabled why bother having dev_ops or not: if (dev->data->all_multicast == 1) return eth_err(port_id, diag); > + diag = (*dev->dev_ops->allmulticast_enable)(dev); > + dev->data->all_multicast = (diag == 0) ? 1 : 0; > + } > > return eth_err(port_id, diag); > } > @@ -1980,18 +1987,19 @@ int > rte_eth_allmulticast_disable(uint16_t port_id) > { > struct rte_eth_dev *dev; > - uint8_t old_allmulticast; > - int diag; > + int diag = 0; > > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > dev = &rte_eth_devices[port_id]; > > RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->allmulticast_disable, -ENOTSUP); > - old_allmulticast = dev->data->all_multicast; > - dev->data->all_multicast = 0; > - diag = (*dev->dev_ops->allmulticast_disable)(dev); > - if (diag != 0) > - dev->data->all_multicast = old_allmulticast; > + > + if (dev->data->all_multicast == 1) { Same comment with above, can we move this check above.. > + dev->data->all_multicast = 0; > + diag = (*dev->dev_ops->allmulticast_disable)(dev); > + if (diag != 0) > + dev->data->all_multicast = 1; > + } > > return eth_err(port_id, diag); > } > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [dpdk-dev] [PATCH 5/7] ethdev: do nothing if all-multicast mode is applied again 2019-09-24 8:36 ` Ferruh Yigit @ 2019-09-24 8:54 ` Andrew Rybchenko 2019-09-24 11:03 ` Ferruh Yigit 0 siblings, 1 reply; 18+ messages in thread From: Andrew Rybchenko @ 2019-09-24 8:54 UTC (permalink / raw) To: Ferruh Yigit, Thomas Monjalon; +Cc: dev On 9/24/19 11:36 AM, Ferruh Yigit wrote: > On 9/9/2019 1:13 PM, Andrew Rybchenko wrote: >> Since driver callbacks return status code now, there is no necessity >> to enable or disable all-multicast mode once again if it is already >> successfully enabled or disabled. >> >> Configuration restore at startup tries to ensure that configured >> all-multicast mode is applied and start will return error if it fails. >> >> Also it avoids theoretical cases when already configured all-multicast >> mode is applied once again and fails. In this cases it is unclear >> which value should be reported on get (configured or opposite). >> >> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> >> --- >> lib/librte_ethdev/rte_ethdev.c | 40 ++++++++++++++++++++-------------- >> 1 file changed, 24 insertions(+), 16 deletions(-) >> >> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c >> index 8115226c91..e1921e8225 100644 >> --- a/lib/librte_ethdev/rte_ethdev.c >> +++ b/lib/librte_ethdev/rte_ethdev.c >> @@ -1416,16 +1416,22 @@ rte_eth_dev_config_restore(struct rte_eth_dev *dev, >> } >> >> /* replay all multicast configuration */ >> - if (rte_eth_allmulticast_get(port_id) == 1) { >> - ret = rte_eth_allmulticast_enable(port_id); >> + /* >> + * use callbacks directly since we don't need port_id check and >> + * would like to bypass the same value set >> + */ >> + if (rte_eth_allmulticast_get(port_id) == 1 && >> + *dev->dev_ops->allmulticast_enable != NULL) { >> + ret = (*dev->dev_ops->allmulticast_enable)(dev); > I am for using the API here, it is more abstract instead of adding the dev_ops > null checks etc. Will there be any downside to use the API? API does not call operation if value matches and it will exactly match here for sure since we just get it and applying once again. Can't say that I like usage callback directly here, but it looks acceptable for me. I've tried to clarify why it is done this way in the comment above. > <...> > >> @@ -1962,16 +1968,17 @@ int >> rte_eth_allmulticast_enable(uint16_t port_id) >> { >> struct rte_eth_dev *dev; >> - uint8_t old_allmulticast; >> - int diag; >> + int diag = 0; >> >> RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); >> dev = &rte_eth_devices[port_id]; >> >> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->allmulticast_enable, -ENOTSUP); >> - old_allmulticast = dev->data->all_multicast; >> - diag = (*dev->dev_ops->allmulticast_enable)(dev); >> - dev->data->all_multicast = (diag == 0) ? 1 : old_allmulticast; >> + >> + if (dev->data->all_multicast == 0) { > What about adding this check even before 'allmulticast_enable' check, so if the > multicast is already enabled why bother having dev_ops or not: > > if (dev->data->all_multicast == 1) > return eth_err(port_id, diag); Yes, it is a good idea. If so, similar fix up will be required for promiscuous mode. >> + diag = (*dev->dev_ops->allmulticast_enable)(dev); >> + dev->data->all_multicast = (diag == 0) ? 1 : 0; >> + } >> >> return eth_err(port_id, diag); >> } >> @@ -1980,18 +1987,19 @@ int >> rte_eth_allmulticast_disable(uint16_t port_id) >> { >> struct rte_eth_dev *dev; >> - uint8_t old_allmulticast; >> - int diag; >> + int diag = 0; >> >> RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); >> dev = &rte_eth_devices[port_id]; >> >> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->allmulticast_disable, -ENOTSUP); >> - old_allmulticast = dev->data->all_multicast; >> - dev->data->all_multicast = 0; >> - diag = (*dev->dev_ops->allmulticast_disable)(dev); >> - if (diag != 0) >> - dev->data->all_multicast = old_allmulticast; >> + >> + if (dev->data->all_multicast == 1) { > Same comment with above, can we move this check above.. Yes, will fix in the next version as well. Thanks for review. >> + dev->data->all_multicast = 0; >> + diag = (*dev->dev_ops->allmulticast_disable)(dev); >> + if (diag != 0) >> + dev->data->all_multicast = 1; >> + } >> >> return eth_err(port_id, diag); >> } >> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [dpdk-dev] [PATCH 5/7] ethdev: do nothing if all-multicast mode is applied again 2019-09-24 8:54 ` Andrew Rybchenko @ 2019-09-24 11:03 ` Ferruh Yigit 2019-09-24 11:50 ` Andrew Rybchenko 0 siblings, 1 reply; 18+ messages in thread From: Ferruh Yigit @ 2019-09-24 11:03 UTC (permalink / raw) To: Andrew Rybchenko, Thomas Monjalon; +Cc: dev On 9/24/2019 9:54 AM, Andrew Rybchenko wrote: > On 9/24/19 11:36 AM, Ferruh Yigit wrote: >> On 9/9/2019 1:13 PM, Andrew Rybchenko wrote: >>> Since driver callbacks return status code now, there is no necessity >>> to enable or disable all-multicast mode once again if it is already >>> successfully enabled or disabled. >>> >>> Configuration restore at startup tries to ensure that configured >>> all-multicast mode is applied and start will return error if it fails. >>> >>> Also it avoids theoretical cases when already configured all-multicast >>> mode is applied once again and fails. In this cases it is unclear >>> which value should be reported on get (configured or opposite). >>> >>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> >>> --- >>> lib/librte_ethdev/rte_ethdev.c | 40 ++++++++++++++++++++-------------- >>> 1 file changed, 24 insertions(+), 16 deletions(-) >>> >>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c >>> index 8115226c91..e1921e8225 100644 >>> --- a/lib/librte_ethdev/rte_ethdev.c >>> +++ b/lib/librte_ethdev/rte_ethdev.c >>> @@ -1416,16 +1416,22 @@ rte_eth_dev_config_restore(struct rte_eth_dev *dev, >>> } >>> >>> /* replay all multicast configuration */ >>> - if (rte_eth_allmulticast_get(port_id) == 1) { >>> - ret = rte_eth_allmulticast_enable(port_id); >>> + /* >>> + * use callbacks directly since we don't need port_id check and >>> + * would like to bypass the same value set >>> + */ >>> + if (rte_eth_allmulticast_get(port_id) == 1 && >>> + *dev->dev_ops->allmulticast_enable != NULL) { >>> + ret = (*dev->dev_ops->allmulticast_enable)(dev); >> I am for using the API here, it is more abstract instead of adding the dev_ops >> null checks etc. Will there be any downside to use the API? > > API does not call operation if value matches and it will exactly match here > for sure since we just get it and applying once again. Ah, right, we need it here as you explained. Thx. > Can't say that I like usage callback directly here, but it looks acceptable > for me. I've tried to clarify why it is done this way in the comment above. > >> <...> >> >>> @@ -1962,16 +1968,17 @@ int >>> rte_eth_allmulticast_enable(uint16_t port_id) >>> { >>> struct rte_eth_dev *dev; >>> - uint8_t old_allmulticast; >>> - int diag; >>> + int diag = 0; >>> >>> RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); >>> dev = &rte_eth_devices[port_id]; >>> >>> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->allmulticast_enable, -ENOTSUP); >>> - old_allmulticast = dev->data->all_multicast; >>> - diag = (*dev->dev_ops->allmulticast_enable)(dev); >>> - dev->data->all_multicast = (diag == 0) ? 1 : old_allmulticast; >>> + >>> + if (dev->data->all_multicast == 0) { >> What about adding this check even before 'allmulticast_enable' check, so if the >> multicast is already enabled why bother having dev_ops or not: >> >> if (dev->data->all_multicast == 1) >> return eth_err(port_id, diag); > > Yes, it is a good idea. If so, similar fix up will be required for > promiscuous mode. > >>> + diag = (*dev->dev_ops->allmulticast_enable)(dev); >>> + dev->data->all_multicast = (diag == 0) ? 1 : 0; >>> + } >>> >>> return eth_err(port_id, diag); >>> } >>> @@ -1980,18 +1987,19 @@ int >>> rte_eth_allmulticast_disable(uint16_t port_id) >>> { >>> struct rte_eth_dev *dev; >>> - uint8_t old_allmulticast; >>> - int diag; >>> + int diag = 0; >>> >>> RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); >>> dev = &rte_eth_devices[port_id]; >>> >>> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->allmulticast_disable, -ENOTSUP); >>> - old_allmulticast = dev->data->all_multicast; >>> - dev->data->all_multicast = 0; >>> - diag = (*dev->dev_ops->allmulticast_disable)(dev); >>> - if (diag != 0) >>> - dev->data->all_multicast = old_allmulticast; >>> + >>> + if (dev->data->all_multicast == 1) { >> Same comment with above, can we move this check above.. > > Yes, will fix in the next version as well. Thanks for review. > >>> + dev->data->all_multicast = 0; >>> + diag = (*dev->dev_ops->allmulticast_disable)(dev); >>> + if (diag != 0) >>> + dev->data->all_multicast = 1; >>> + } >>> >>> return eth_err(port_id, diag); >>> } >>> > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [dpdk-dev] [PATCH 5/7] ethdev: do nothing if all-multicast mode is applied again 2019-09-24 11:03 ` Ferruh Yigit @ 2019-09-24 11:50 ` Andrew Rybchenko 0 siblings, 0 replies; 18+ messages in thread From: Andrew Rybchenko @ 2019-09-24 11:50 UTC (permalink / raw) To: Ferruh Yigit, Thomas Monjalon; +Cc: dev On 9/24/19 2:03 PM, Ferruh Yigit wrote: > On 9/24/2019 9:54 AM, Andrew Rybchenko wrote: >> On 9/24/19 11:36 AM, Ferruh Yigit wrote: >>> On 9/9/2019 1:13 PM, Andrew Rybchenko wrote: >>>> Since driver callbacks return status code now, there is no necessity >>>> to enable or disable all-multicast mode once again if it is already >>>> successfully enabled or disabled. >>>> >>>> Configuration restore at startup tries to ensure that configured >>>> all-multicast mode is applied and start will return error if it fails. >>>> >>>> Also it avoids theoretical cases when already configured all-multicast >>>> mode is applied once again and fails. In this cases it is unclear >>>> which value should be reported on get (configured or opposite). >>>> >>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> >>>> --- >>>> lib/librte_ethdev/rte_ethdev.c | 40 ++++++++++++++++++++-------------- >>>> 1 file changed, 24 insertions(+), 16 deletions(-) >>>> >>>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c >>>> index 8115226c91..e1921e8225 100644 >>>> --- a/lib/librte_ethdev/rte_ethdev.c >>>> +++ b/lib/librte_ethdev/rte_ethdev.c >>>> @@ -1416,16 +1416,22 @@ rte_eth_dev_config_restore(struct rte_eth_dev *dev, >>>> } >>>> >>>> /* replay all multicast configuration */ >>>> - if (rte_eth_allmulticast_get(port_id) == 1) { >>>> - ret = rte_eth_allmulticast_enable(port_id); >>>> + /* >>>> + * use callbacks directly since we don't need port_id check and >>>> + * would like to bypass the same value set >>>> + */ >>>> + if (rte_eth_allmulticast_get(port_id) == 1 && >>>> + *dev->dev_ops->allmulticast_enable != NULL) { >>>> + ret = (*dev->dev_ops->allmulticast_enable)(dev); >>> I am for using the API here, it is more abstract instead of adding the dev_ops >>> null checks etc. Will there be any downside to use the API? >> API does not call operation if value matches and it will exactly match here >> for sure since we just get it and applying once again. > Ah, right, we need it here as you explained. Thx. In fact, I think eth_err() is required to handle callback return value. I'll add it in v2. >> Can't say that I like usage callback directly here, but it looks acceptable >> for me. I've tried to clarify why it is done this way in the comment above. >> >>> <...> >>> >>>> @@ -1962,16 +1968,17 @@ int >>>> rte_eth_allmulticast_enable(uint16_t port_id) >>>> { >>>> struct rte_eth_dev *dev; >>>> - uint8_t old_allmulticast; >>>> - int diag; >>>> + int diag = 0; >>>> >>>> RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); >>>> dev = &rte_eth_devices[port_id]; >>>> >>>> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->allmulticast_enable, -ENOTSUP); >>>> - old_allmulticast = dev->data->all_multicast; >>>> - diag = (*dev->dev_ops->allmulticast_enable)(dev); >>>> - dev->data->all_multicast = (diag == 0) ? 1 : old_allmulticast; >>>> + >>>> + if (dev->data->all_multicast == 0) { >>> What about adding this check even before 'allmulticast_enable' check, so if the >>> multicast is already enabled why bother having dev_ops or not: >>> >>> if (dev->data->all_multicast == 1) >>> return eth_err(port_id, diag); >> Yes, it is a good idea. If so, similar fix up will be required for >> promiscuous mode. I'd prefer to return 0 directly. >>>> + diag = (*dev->dev_ops->allmulticast_enable)(dev); >>>> + dev->data->all_multicast = (diag == 0) ? 1 : 0; >>>> + } >>>> >>>> return eth_err(port_id, diag); >>>> } >>>> @@ -1980,18 +1987,19 @@ int >>>> rte_eth_allmulticast_disable(uint16_t port_id) >>>> { >>>> struct rte_eth_dev *dev; >>>> - uint8_t old_allmulticast; >>>> - int diag; >>>> + int diag = 0; >>>> >>>> RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); >>>> dev = &rte_eth_devices[port_id]; >>>> >>>> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->allmulticast_disable, -ENOTSUP); >>>> - old_allmulticast = dev->data->all_multicast; >>>> - dev->data->all_multicast = 0; >>>> - diag = (*dev->dev_ops->allmulticast_disable)(dev); >>>> - if (diag != 0) >>>> - dev->data->all_multicast = old_allmulticast; >>>> + >>>> + if (dev->data->all_multicast == 1) { >>> Same comment with above, can we move this check above.. >> Yes, will fix in the next version as well. Thanks for review. >> >>>> + dev->data->all_multicast = 0; >>>> + diag = (*dev->dev_ops->allmulticast_disable)(dev); >>>> + if (diag != 0) >>>> + dev->data->all_multicast = 1; >>>> + } >>>> >>>> return eth_err(port_id, diag); >>>> } >>>> ^ permalink raw reply [flat|nested] 18+ messages in thread
* [dpdk-dev] [PATCH 6/7] app/testpmd: check code of allmulticast mode switch 2019-09-09 12:13 [dpdk-dev] [PATCH 0/7] ethdev: change allmulticast controls to return status Andrew Rybchenko ` (4 preceding siblings ...) 2019-09-09 12:13 ` [dpdk-dev] [PATCH 5/7] ethdev: do nothing if all-multicast mode is applied again Andrew Rybchenko @ 2019-09-09 12:13 ` Andrew Rybchenko 2019-09-09 12:13 ` [dpdk-dev] [PATCH 7/7] examples/ipv4_multicast: check allmulticast enable status Andrew Rybchenko 6 siblings, 0 replies; 18+ messages in thread From: Andrew Rybchenko @ 2019-09-09 12:13 UTC (permalink / raw) To: Wenzhuo Lu, Jingjing Wu, Bernard Iremonger; +Cc: dev, Ivan Ilchenko From: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru> rte_eth_allmulticast_enable()/rte_eth_allmulticast_disable() return value was changed from void to int, so this patch modify usage of these functions across app/test-pmd according to new return type. Signed-off-by: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> --- app/test-pmd/cmdline.c | 10 ++-------- app/test-pmd/testpmd.h | 1 + app/test-pmd/util.c | 16 ++++++++++++++++ 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index 6b9444f42d..dd4e6e6021 100644 --- a/app/test-pmd/cmdline.c +++ b/app/test-pmd/cmdline.c @@ -6603,17 +6603,11 @@ static void cmd_set_allmulti_mode_parsed(void *parsed_result, /* all ports */ if (allports) { RTE_ETH_FOREACH_DEV(i) { - if (enable) - rte_eth_allmulticast_enable(i); - else - rte_eth_allmulticast_disable(i); + eth_set_allmulticast_mode(i, enable); } } else { - if (enable) - rte_eth_allmulticast_enable(res->port_num); - else - rte_eth_allmulticast_disable(res->port_num); + eth_set_allmulticast_mode(res->port_num, enable); } } diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index ab93062923..f1529696f5 100644 --- a/app/test-pmd/testpmd.h +++ b/app/test-pmd/testpmd.h @@ -825,6 +825,7 @@ void setup_gso(const char *mode, portid_t port_id); int eth_dev_info_get_print_err(uint16_t port_id, struct rte_eth_dev_info *dev_info); void eth_set_promisc_mode(uint16_t port_id, int enable); +void eth_set_allmulticast_mode(uint16_t port, int enable); /* Functions to manage the set of filtered Multicast MAC addresses */ diff --git a/app/test-pmd/util.c b/app/test-pmd/util.c index 4626751343..1aec5d755b 100644 --- a/app/test-pmd/util.c +++ b/app/test-pmd/util.c @@ -261,3 +261,19 @@ eth_set_promisc_mode(uint16_t port, int enable) enable ? "enabling" : "disabling", port, rte_strerror(-ret)); } + +void +eth_set_allmulticast_mode(uint16_t port, int enable) +{ + int ret; + + if (enable) + ret = rte_eth_allmulticast_enable(port); + else + ret = rte_eth_allmulticast_disable(port); + + if (ret != 0) + printf("Error during %s all-multicast mode for port %u: %s\n", + enable ? "enabling" : "disabling", + port, rte_strerror(-ret)); +} -- 2.17.1 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [dpdk-dev] [PATCH 7/7] examples/ipv4_multicast: check allmulticast enable status 2019-09-09 12:13 [dpdk-dev] [PATCH 0/7] ethdev: change allmulticast controls to return status Andrew Rybchenko ` (5 preceding siblings ...) 2019-09-09 12:13 ` [dpdk-dev] [PATCH 6/7] app/testpmd: check code of allmulticast mode switch Andrew Rybchenko @ 2019-09-09 12:13 ` Andrew Rybchenko 6 siblings, 0 replies; 18+ messages in thread From: Andrew Rybchenko @ 2019-09-09 12:13 UTC (permalink / raw) To: Marko Kovacevic, Ori Kam, Bruce Richardson, Pablo de Lara, Radu Nicolau, Akhil Goyal, Tomasz Kantecki Cc: dev, Ivan Ilchenko From: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru> rte_eth_allmulticast_enable()/rte_eth_allmulticast_disable() return value was changed from void to int, so this patch modify usage of these functions across examples/ipv4_multicast according to new return type. Signed-off-by: Ivan Ilchenko <Ivan.Ilchenko@oktetlabs.ru> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com> --- examples/ipv4_multicast/main.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/examples/ipv4_multicast/main.c b/examples/ipv4_multicast/main.c index 1ee3b61d1a..8fd19f4cf6 100644 --- a/examples/ipv4_multicast/main.c +++ b/examples/ipv4_multicast/main.c @@ -770,7 +770,11 @@ main(int argc, char **argv) qconf->tx_queue_id[portid] = queueid; queueid++; } - rte_eth_allmulticast_enable(portid); + ret = rte_eth_allmulticast_enable(portid); + if (ret < 0) + rte_exit(EXIT_FAILURE, + "rte_eth_allmulticast_enable: err=%d, port=%d\n", + ret, portid); /* Start device */ ret = rte_eth_dev_start(portid); if (ret < 0) -- 2.17.1 ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2019-09-24 12:14 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-09 12:13 [dpdk-dev] [PATCH 0/7] ethdev: change allmulticast controls to return status Andrew Rybchenko 2019-09-09 12:13 ` [dpdk-dev] [PATCH 1/7] ethdev: change allmulticast mode controllers to return errors Andrew Rybchenko 2019-09-09 12:13 ` [dpdk-dev] [PATCH 2/7] net/failsafe: check code of allmulticast mode switch Andrew Rybchenko 2019-09-09 12:56 ` Gaëtan Rivet 2019-09-13 21:04 ` Andrew Rybchenko 2019-09-09 12:13 ` [dpdk-dev] [PATCH 3/7] net/bonding: " Andrew Rybchenko 2019-09-09 12:13 ` [dpdk-dev] [PATCH 4/7] ethdev: change allmulticast callbacks to return status Andrew Rybchenko 2019-09-16 7:03 ` Hyong Youb Kim (hyonkim) 2019-09-16 7:29 ` Andrew Rybchenko 2019-09-24 8:27 ` Ferruh Yigit 2019-09-24 12:14 ` Andrew Rybchenko 2019-09-09 12:13 ` [dpdk-dev] [PATCH 5/7] ethdev: do nothing if all-multicast mode is applied again Andrew Rybchenko 2019-09-24 8:36 ` Ferruh Yigit 2019-09-24 8:54 ` Andrew Rybchenko 2019-09-24 11:03 ` Ferruh Yigit 2019-09-24 11:50 ` Andrew Rybchenko 2019-09-09 12:13 ` [dpdk-dev] [PATCH 6/7] app/testpmd: check code of allmulticast mode switch Andrew Rybchenko 2019-09-09 12:13 ` [dpdk-dev] [PATCH 7/7] examples/ipv4_multicast: check allmulticast enable status Andrew Rybchenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).