diff mbox series

[v1,11/38] net/mvpp2: align checking order

Message ID 20201202101212.4717-12-lironh@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: Jerin Jacob
Headers
Series net/mvpp2: misc updates |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Liron Himi Dec. 2, 2020, 10:11 a.m. UTC 
  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(-)

Comments

Michael Shamis Dec. 23, 2020, 9:41 a.m. UTC | #1 
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
Jerin Jacob Jan. 11, 2021, 2:57 p.m. UTC | #2 
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
>
Liron Himi Jan. 18, 2021, 8:01 p.m. UTC | #3 
-----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 mbox series

Patch

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);
 }