[dpdk-dev] [RFC] lib/librte_ether: add a return value for MAC set ops

[Wenzhuo Lu]

Setting the default MAC address may fail on many NICs.
But the ops return void. So, even it failed, RTE changes
the MAC address and APP doesn't know the failure.

It's not real patch, just show the idea to add a return
value for the ops.

BTW,
Seems we should do the same thing for
rte_eth_dev_mac_addr_remove as it also has chance to fail
in PMD layer.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu at intel.com>
---
 drivers/net/i40e/i40e_ethdev_vf.c  | 12 +++++++-----
 lib/librte_ether/rte_ethdev.c      |  7 +++++--
 lib/librte_ether/rte_ethdev.h      |  1 +
 lib/librte_ether/rte_ethdev_core.h |  2 +-
 4 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 6ac3f8c..1d3898b 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -120,8 +120,8 @@ static int i40evf_dev_rss_hash_update(struct rte_eth_dev *dev,
 static int i40evf_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
 					struct rte_eth_rss_conf *rss_conf);
 static int i40evf_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
-static void i40evf_set_default_mac_addr(struct rte_eth_dev *dev,
-					struct ether_addr *mac_addr);
+static int i40evf_set_default_mac_addr(struct rte_eth_dev *dev,
+				       struct ether_addr *mac_addr);
 static int
 i40evf_dev_rx_queue_intr_enable(struct rte_eth_dev *dev, uint16_t queue_id);
 static int
@@ -2655,7 +2655,7 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev)
 	return ret;
 }
 
-static void
+static int
 i40evf_set_default_mac_addr(struct rte_eth_dev *dev,
 			    struct ether_addr *mac_addr)
 {
@@ -2664,15 +2664,17 @@ static int eth_i40evf_pci_remove(struct rte_pci_device *pci_dev)
 
 	if (!is_valid_assigned_ether_addr(mac_addr)) {
 		PMD_DRV_LOG(ERR, "Tried to set invalid MAC address.");
-		return;
+		return -1;
 	}
 
 	if (vf->flags & I40E_FLAG_VF_MAC_BY_PF)
-		return;
+		return -1;
 
 	i40evf_del_mac_addr_by_addr(dev, (struct ether_addr *)hw->mac.addr);
 
 	i40evf_add_mac_addr(dev, mac_addr, 0, 0);
 
 	ether_addr_copy(mac_addr, (struct ether_addr *)hw->mac.addr);
+
+	return 0;
 }
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index f285ba2..869c960 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -2816,6 +2816,7 @@ struct rte_eth_dev *
 rte_eth_dev_default_mac_addr_set(uint16_t port_id, struct ether_addr *addr)
 {
 	struct rte_eth_dev *dev;
+	int ret;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 
@@ -2825,11 +2826,13 @@ struct rte_eth_dev *
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mac_addr_set, -ENOTSUP);
 
+	ret = (*dev->dev_ops->mac_addr_set)(dev, addr);
+	if (ret)
+		return -EPERM;
+
 	/* Update default address in NIC data structure */
 	ether_addr_copy(addr, &dev->data->mac_addrs[0]);
 
-	(*dev->dev_ops->mac_addr_set)(dev, addr);
-
 	return 0;
 }
 
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index ccf4a15..e3355cc 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -2616,6 +2616,7 @@ int rte_eth_dev_mac_addr_add(uint16_t port_id, struct ether_addr *mac_addr,
  *   - (-ENOTSUP) if hardware doesn't support.
  *   - (-ENODEV) if *port* invalid.
  *   - (-EINVAL) if MAC address is invalid.
+ *   - (-EPERM) if the default MAC address cannot be changed.
  */
 int rte_eth_dev_default_mac_addr_set(uint16_t port_id,
 		struct ether_addr *mac_addr);
diff --git a/lib/librte_ether/rte_ethdev_core.h b/lib/librte_ether/rte_ethdev_core.h
index f44b40e..91fa1c0 100644
--- a/lib/librte_ether/rte_ethdev_core.h
+++ b/lib/librte_ether/rte_ethdev_core.h
@@ -251,7 +251,7 @@ typedef int (*eth_mac_addr_add_t)(struct rte_eth_dev *dev,
 				  uint32_t vmdq);
 /**< @internal Set a MAC address into Receive Address Address Register */
 
-typedef void (*eth_mac_addr_set_t)(struct rte_eth_dev *dev,
+typedef int (*eth_mac_addr_set_t)(struct rte_eth_dev *dev,
 				  struct ether_addr *mac_addr);
 /**< @internal Set a MAC address into Receive Address Address Register */
 
-- 
1.9.3