Re: [dpdk-dev] [EXT] Re: [PATCH v1 11/38] net/mvpp2: align checking order
From: Liron Himi <hidden>
Date: 2021-01-18 20:02:03
-----Original Message----- From: Jerin Jacob <redacted> Sent: Monday, 11 January 2021 16:57 To: Liron Himi <redacted> Cc: Jerin Jacob Kollanukkaran <redacted>; dpdk-dev <redacted>; Yuri Chipchev <redacted> 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 [off-list ref] wrote:
quoted hunk ↗ jump to hunk
From: Yuri Chipchev <redacted> first check for 'isolated' and then for '!ppio' Signed-off-by: Yuri Chipchev <redacted> Reviewed-by: Liron Himi <redacted> --- drivers/net/mvpp2/mrvl_ethdev.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-)diff --git a/drivers/net/mvpp2/mrvl_ethdev.cb/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.
quoted hunk ↗ jump to hunk
- 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_tvlan_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