| Message ID | 20201202101212.4717-12-lironh@marvell.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Jerin Jacob |
| Headers | show |
| Series | net/mvpp2: misc updates | expand |
| Context | Check | Description |
|---|---|---|
| ci/checkpatch | success | coding style OK |
Reviewed-by: Michael Shamis <michaelsh@marvell.com> -----Original Message----- From: dev <dev-bounces@dpdk.org> On Behalf Of lironh@marvell.com Sent: Wednesday, December 2, 2020 12:12 PM To: Jerin Jacob Kollanukkaran <jerinj@marvell.com> Cc: dev@dpdk.org; Yuri Chipchev <yuric@marvell.com>; Liron Himi <lironh@marvell.com> Subject: [dpdk-dev] [PATCH v1 11/38] net/mvpp2: align checking order From: Yuri Chipchev <yuric@marvell.com> first check for 'isolated' and then for '!ppio' Signed-off-by: Yuri Chipchev <yuric@marvell.com> Reviewed-by: Liron Himi <lironh@marvell.com> --- drivers/net/mvpp2/mrvl_ethdev.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index 130f5221d..76847b303 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -1007,10 +1007,10 @@ mrvl_promiscuous_enable(struct rte_eth_dev *dev) struct mrvl_priv *priv = dev->data->dev_private; int ret; - if (!priv->ppio) + if (priv->isolated) return 0; - if (priv->isolated) + if (!priv->ppio) return 0; ret = pp2_ppio_set_promisc(priv->ppio, 1); @@ -1037,10 +1037,10 @@ mrvl_allmulticast_enable(struct rte_eth_dev *dev) struct mrvl_priv *priv = dev->data->dev_private; int ret; - if (!priv->ppio) + if (priv->isolated) return 0; - if (priv->isolated) + if (!priv->ppio) return 0; ret = pp2_ppio_set_mc_promisc(priv->ppio, 1); @@ -1067,6 +1067,9 @@ mrvl_promiscuous_disable(struct rte_eth_dev *dev) struct mrvl_priv *priv = dev->data->dev_private; int ret; + if (priv->isolated) + return 0; + if (!priv->ppio) return 0; @@ -1094,6 +1097,9 @@ mrvl_allmulticast_disable(struct rte_eth_dev *dev) struct mrvl_priv *priv = dev->data->dev_private; int ret; + if (priv->isolated) + return 0; + if (!priv->ppio) return 0; @@ -1121,10 +1127,10 @@ mrvl_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index) char buf[RTE_ETHER_ADDR_FMT_SIZE]; int ret; - if (!priv->ppio) + if (priv->isolated) return; - if (priv->isolated) + if (!priv->ppio) return; ret = pp2_ppio_remove_mac_addr(priv->ppio, @@ -1209,12 +1215,12 @@ mrvl_mac_addr_set(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr) struct mrvl_priv *priv = dev->data->dev_private; int ret; - if (!priv->ppio) - return 0; - if (priv->isolated) return -ENOTSUP; + if (!priv->ppio) + return 0; + ret = pp2_ppio_set_mac_addr(priv->ppio, mac_addr->addr_bytes); if (ret) { char buf[RTE_ETHER_ADDR_FMT_SIZE]; @@ -1590,12 +1596,12 @@ mrvl_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on) { struct mrvl_priv *priv = dev->data->dev_private; - if (!priv->ppio) - return -EPERM; - if (priv->isolated) return -ENOTSUP; + if (!priv->ppio) + return 0; + return on ? pp2_ppio_add_vlan(priv->ppio, vlan_id) : pp2_ppio_remove_vlan(priv->ppio, vlan_id); } -- 2.28.0
On Wed, Dec 2, 2020 at 3:46 PM <lironh@marvell.com> wrote: > > From: Yuri Chipchev <yuric@marvell.com> > > first check for 'isolated' and then for '!ppio' > > Signed-off-by: Yuri Chipchev <yuric@marvell.com> > Reviewed-by: Liron Himi <lironh@marvell.com> > --- > drivers/net/mvpp2/mrvl_ethdev.c | 30 ++++++++++++++++++------------ > 1 file changed, 18 insertions(+), 12 deletions(-) > > diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c > index 130f5221d..76847b303 100644 > --- a/drivers/net/mvpp2/mrvl_ethdev.c > +++ b/drivers/net/mvpp2/mrvl_ethdev.c > @@ -1007,10 +1007,10 @@ mrvl_promiscuous_enable(struct rte_eth_dev *dev) > struct mrvl_priv *priv = dev->data->dev_private; > int ret; > > - if (!priv->ppio) > + if (priv->isolated) > return 0; Rather than duplicating these checks in every function. Please move sanity check to a function and reuse it. > > - if (priv->isolated) > + if (!priv->ppio) > return 0; > > ret = pp2_ppio_set_promisc(priv->ppio, 1); > @@ -1037,10 +1037,10 @@ mrvl_allmulticast_enable(struct rte_eth_dev *dev) > struct mrvl_priv *priv = dev->data->dev_private; > int ret; > > - if (!priv->ppio) > + if (priv->isolated) > return 0; > > - if (priv->isolated) > + if (!priv->ppio) > return 0; > > ret = pp2_ppio_set_mc_promisc(priv->ppio, 1); > @@ -1067,6 +1067,9 @@ mrvl_promiscuous_disable(struct rte_eth_dev *dev) > struct mrvl_priv *priv = dev->data->dev_private; > int ret; > > + if (priv->isolated) > + return 0; > + > if (!priv->ppio) > return 0; > > @@ -1094,6 +1097,9 @@ mrvl_allmulticast_disable(struct rte_eth_dev *dev) > struct mrvl_priv *priv = dev->data->dev_private; > int ret; > > + if (priv->isolated) > + return 0; > + > if (!priv->ppio) > return 0; > > @@ -1121,10 +1127,10 @@ mrvl_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index) > char buf[RTE_ETHER_ADDR_FMT_SIZE]; > int ret; > > - if (!priv->ppio) > + if (priv->isolated) > return; > > - if (priv->isolated) > + if (!priv->ppio) > return; > > ret = pp2_ppio_remove_mac_addr(priv->ppio, > @@ -1209,12 +1215,12 @@ mrvl_mac_addr_set(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr) > struct mrvl_priv *priv = dev->data->dev_private; > int ret; > > - if (!priv->ppio) > - return 0; > - > if (priv->isolated) > return -ENOTSUP; > > + if (!priv->ppio) > + return 0; > + > ret = pp2_ppio_set_mac_addr(priv->ppio, mac_addr->addr_bytes); > if (ret) { > char buf[RTE_ETHER_ADDR_FMT_SIZE]; > @@ -1590,12 +1596,12 @@ mrvl_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on) > { > struct mrvl_priv *priv = dev->data->dev_private; > > - if (!priv->ppio) > - return -EPERM; > - > if (priv->isolated) > return -ENOTSUP; > > + if (!priv->ppio) > + return 0; > + > return on ? pp2_ppio_add_vlan(priv->ppio, vlan_id) : > pp2_ppio_remove_vlan(priv->ppio, vlan_id); > } > -- > 2.28.0 >
-----Original Message----- From: Jerin Jacob <jerinjacobk@gmail.com> Sent: Monday, 11 January 2021 16:57 To: Liron Himi <lironh@marvell.com> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; dpdk-dev <dev@dpdk.org>; Yuri Chipchev <yuric@marvell.com> Subject: [EXT] Re: [dpdk-dev] [PATCH v1 11/38] net/mvpp2: align checking order External Email ---------------------------------------------------------------------- On Wed, Dec 2, 2020 at 3:46 PM <lironh@marvell.com> wrote: > > From: Yuri Chipchev <yuric@marvell.com> > > first check for 'isolated' and then for '!ppio' > > Signed-off-by: Yuri Chipchev <yuric@marvell.com> > Reviewed-by: Liron Himi <lironh@marvell.com> > --- > drivers/net/mvpp2/mrvl_ethdev.c | 30 ++++++++++++++++++------------ > 1 file changed, 18 insertions(+), 12 deletions(-) > > diff --git a/drivers/net/mvpp2/mrvl_ethdev.c > b/drivers/net/mvpp2/mrvl_ethdev.c index 130f5221d..76847b303 100644 > --- a/drivers/net/mvpp2/mrvl_ethdev.c > +++ b/drivers/net/mvpp2/mrvl_ethdev.c > @@ -1007,10 +1007,10 @@ mrvl_promiscuous_enable(struct rte_eth_dev *dev) > struct mrvl_priv *priv = dev->data->dev_private; > int ret; > > - if (!priv->ppio) > + if (priv->isolated) > return 0; Rather than duplicating these checks in every function. Please move sanity check to a function and reuse it. [L.H.] yes, currently it looks like they are connected and should be in a separate function. but, the check for the 'ppio' will have more body, so I don't want to put them together. > > - if (priv->isolated) > + if (!priv->ppio) > return 0; > > ret = pp2_ppio_set_promisc(priv->ppio, 1); @@ -1037,10 > +1037,10 @@ mrvl_allmulticast_enable(struct rte_eth_dev *dev) > struct mrvl_priv *priv = dev->data->dev_private; > int ret; > > - if (!priv->ppio) > + if (priv->isolated) > return 0; > > - if (priv->isolated) > + if (!priv->ppio) > return 0; > > ret = pp2_ppio_set_mc_promisc(priv->ppio, 1); @@ -1067,6 > +1067,9 @@ mrvl_promiscuous_disable(struct rte_eth_dev *dev) > struct mrvl_priv *priv = dev->data->dev_private; > int ret; > > + if (priv->isolated) > + return 0; > + > if (!priv->ppio) > return 0; > > @@ -1094,6 +1097,9 @@ mrvl_allmulticast_disable(struct rte_eth_dev *dev) > struct mrvl_priv *priv = dev->data->dev_private; > int ret; > > + if (priv->isolated) > + return 0; > + > if (!priv->ppio) > return 0; > > @@ -1121,10 +1127,10 @@ mrvl_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index) > char buf[RTE_ETHER_ADDR_FMT_SIZE]; > int ret; > > - if (!priv->ppio) > + if (priv->isolated) > return; > > - if (priv->isolated) > + if (!priv->ppio) > return; > > ret = pp2_ppio_remove_mac_addr(priv->ppio, > @@ -1209,12 +1215,12 @@ mrvl_mac_addr_set(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr) > struct mrvl_priv *priv = dev->data->dev_private; > int ret; > > - if (!priv->ppio) > - return 0; > - > if (priv->isolated) > return -ENOTSUP; > > + if (!priv->ppio) > + return 0; > + > ret = pp2_ppio_set_mac_addr(priv->ppio, mac_addr->addr_bytes); > if (ret) { > char buf[RTE_ETHER_ADDR_FMT_SIZE]; @@ -1590,12 > +1596,12 @@ mrvl_vlan_filter_set(struct rte_eth_dev *dev, uint16_t > vlan_id, int on) { > struct mrvl_priv *priv = dev->data->dev_private; > > - if (!priv->ppio) > - return -EPERM; > - > if (priv->isolated) > return -ENOTSUP; > > + if (!priv->ppio) > + return 0; > + > return on ? pp2_ppio_add_vlan(priv->ppio, vlan_id) : > pp2_ppio_remove_vlan(priv->ppio, vlan_id); } > -- > 2.28.0 >
diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c index 130f5221d..76847b303 100644 --- a/drivers/net/mvpp2/mrvl_ethdev.c +++ b/drivers/net/mvpp2/mrvl_ethdev.c @@ -1007,10 +1007,10 @@ mrvl_promiscuous_enable(struct rte_eth_dev *dev) struct mrvl_priv *priv = dev->data->dev_private; int ret; - if (!priv->ppio) + if (priv->isolated) return 0; - if (priv->isolated) + if (!priv->ppio) return 0; ret = pp2_ppio_set_promisc(priv->ppio, 1); @@ -1037,10 +1037,10 @@ mrvl_allmulticast_enable(struct rte_eth_dev *dev) struct mrvl_priv *priv = dev->data->dev_private; int ret; - if (!priv->ppio) + if (priv->isolated) return 0; - if (priv->isolated) + if (!priv->ppio) return 0; ret = pp2_ppio_set_mc_promisc(priv->ppio, 1); @@ -1067,6 +1067,9 @@ mrvl_promiscuous_disable(struct rte_eth_dev *dev) struct mrvl_priv *priv = dev->data->dev_private; int ret; + if (priv->isolated) + return 0; + if (!priv->ppio) return 0; @@ -1094,6 +1097,9 @@ mrvl_allmulticast_disable(struct rte_eth_dev *dev) struct mrvl_priv *priv = dev->data->dev_private; int ret; + if (priv->isolated) + return 0; + if (!priv->ppio) return 0; @@ -1121,10 +1127,10 @@ mrvl_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index) char buf[RTE_ETHER_ADDR_FMT_SIZE]; int ret; - if (!priv->ppio) + if (priv->isolated) return; - if (priv->isolated) + if (!priv->ppio) return; ret = pp2_ppio_remove_mac_addr(priv->ppio, @@ -1209,12 +1215,12 @@ mrvl_mac_addr_set(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr) struct mrvl_priv *priv = dev->data->dev_private; int ret; - if (!priv->ppio) - return 0; - if (priv->isolated) return -ENOTSUP; + if (!priv->ppio) + return 0; + ret = pp2_ppio_set_mac_addr(priv->ppio, mac_addr->addr_bytes); if (ret) { char buf[RTE_ETHER_ADDR_FMT_SIZE]; @@ -1590,12 +1596,12 @@ mrvl_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on) { struct mrvl_priv *priv = dev->data->dev_private; - if (!priv->ppio) - return -EPERM; - if (priv->isolated) return -ENOTSUP; + if (!priv->ppio) + return 0; + return on ? pp2_ppio_add_vlan(priv->ppio, vlan_id) : pp2_ppio_remove_vlan(priv->ppio, vlan_id); }