DPDK patches and discussions
 help / color / mirror / Atom feed
From: Wenzhuo Lu <wenzhuo.lu@intel.com>
To: dev@dpdk.org
Cc: Wenzhuo Lu <wenzhuo.lu@intel.com>
Subject: [dpdk-dev] [RFC] lib/librte_ether: add a return value for MAC set ops
Date: Thu, 25 Jan 2018 10:46:57 +0800	[thread overview]
Message-ID: <1516848417-77912-1-git-send-email-wenzhuo.lu@intel.com> (raw)

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@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

             reply	other threads:[~2018-01-25  2:44 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-25  2:46 Wenzhuo Lu [this message]
2018-01-25 10:40 ` Olivier Matz
2018-01-26  2:19   ` Lu, Wenzhuo
2018-01-26 16:54     ` Ferruh Yigit
2018-01-29  8:25       ` Olivier Matz
2018-05-22 22:47 ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1516848417-77912-1-git-send-email-wenzhuo.lu@intel.com \
    --to=wenzhuo.lu@intel.com \
    --cc=dev@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).