[dpdk-dev,v2] net/i40e: new API to add VF MAC address from PF

Message ID 1500991766-4430-1-git-send-email-wenzhuo.lu@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Wenzhuo Lu July 25, 2017, 2:09 p.m. UTC
  Currently, on i40e the parameter 'pool' of API
rte_eth_dev_mac_addr_add means the VMDq pool, not VF.
So, it's wrong to use it to set the VF MAC address.
As this API is also used by the VMDq example, ideally
we need a parameter to tell the pool is VMDq or VF.
But it's hard to change it because of the ABI change
concern.
Now the solution is to provide a new API, users can
call it to add VF MAC address from PF on i40e.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
v2:
 - fixed the compiling issue without i40e pmd.

 app/test-pmd/cmdline.c                    | 11 ++++++--
 drivers/net/i40e/rte_pmd_i40e.c           | 44 +++++++++++++++++++++++++++++++
 drivers/net/i40e/rte_pmd_i40e.h           | 20 ++++++++++++++
 drivers/net/i40e/rte_pmd_i40e_version.map |  1 +
 4 files changed, 74 insertions(+), 2 deletions(-)
  

Comments

Ferruh Yigit Aug. 17, 2017, 1:05 p.m. UTC | #1
On 7/25/2017 3:09 PM, Wenzhuo Lu wrote:
> Currently, on i40e the parameter 'pool' of API
> rte_eth_dev_mac_addr_add means the VMDq pool, not VF.

The argument "pool" in rte_eth_dev_mac_addr_add() IS VMDq pool.
This is not just for i40e, this is by API definition.

> So, it's wrong to use it to set the VF MAC address.

Agreed, it seems testpmd function cmd_vf_mac_addr_parsed() implemented
wrong.

> As this API is also used by the VMDq example, ideally
> we need a parameter to tell the pool is VMDq or VF.
> But it's hard to change it because of the ABI change
> concern.

I think we shouldn't NOT update rte_eth_dev_mac_addr_add() API, it is
not wrong.

But should fix testpmd ""mac_addr add port <port_id> vf <vf_id>
<mac_addr>" command.

Can you please update this patch as two patches:

1- i40e rte_pmd_i40e_add_vf_mac_addr() API

2- Fix testpmd function, instead of inserting new i40e API replace it
with wrong rte_eth_dev_mac_addr_add() call.
And I guess bnxt driver also has API to add VF MAC, can you add that one
too?

> Now the solution is to provide a new API, users can
> call it to add VF MAC address from PF on i40e.
> 
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

<...>
  
Wenzhuo Lu Aug. 17, 2017, 1:36 p.m. UTC | #2
Hi Ferruh,

> -----Original Message-----

> From: Yigit, Ferruh

> Sent: Thursday, August 17, 2017 9:05 AM

> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; dev@dpdk.org

> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>;

> Stephen Hurd <stephen.hurd@broadcom.com>; Ajit Khaparde

> <ajit.khaparde@broadcom.com>

> Subject: Re: [dpdk-dev] [PATCH v2] net/i40e: new API to add VF MAC address

> from PF

> 

> On 7/25/2017 3:09 PM, Wenzhuo Lu wrote:

> > Currently, on i40e the parameter 'pool' of API

> > rte_eth_dev_mac_addr_add means the VMDq pool, not VF.

> 

> The argument "pool" in rte_eth_dev_mac_addr_add() IS VMDq pool.

> This is not just for i40e, this is by API definition.

> 

> > So, it's wrong to use it to set the VF MAC address.

> 

> Agreed, it seems testpmd function cmd_vf_mac_addr_parsed() implemented

> wrong.

> 

> > As this API is also used by the VMDq example, ideally we need a

> > parameter to tell the pool is VMDq or VF.

> > But it's hard to change it because of the ABI change concern.

> 

> I think we shouldn't NOT update rte_eth_dev_mac_addr_add() API, it is not

> wrong.

Agree :)

> 

> But should fix testpmd ""mac_addr add port <port_id> vf <vf_id>

> <mac_addr>" command.

> 

> Can you please update this patch as two patches:

> 

> 1- i40e rte_pmd_i40e_add_vf_mac_addr() API

> 

> 2- Fix testpmd function, instead of inserting new i40e API replace it with

> wrong rte_eth_dev_mac_addr_add() call.

> And I guess bnxt driver also has API to add VF MAC, can you add that one too?

Thanks for the suggestion. Will send a new version.

> 

> > Now the solution is to provide a new API, users can call it to add VF

> > MAC address from PF on i40e.

> >

> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

> 

> <...>
  

Patch

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index b1b36c1..87257ad 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -7211,9 +7211,16 @@  static void cmd_vf_mac_addr_parsed(void *parsed_result,
 	struct cmd_vf_mac_addr_result *res = parsed_result;
 	int ret = 0;
 
-	if (strcmp(res->what, "add") == 0)
+	if (strcmp(res->what, "add") != 0)
+		return;
+
+#ifdef RTE_LIBRTE_I40E_PMD
+	ret = rte_pmd_i40e_add_vf_mac_addr(res->port_num, res->vf_num,
+					   &res->address);
+	if (ret == -ENOTSUP)
+#endif
 		ret = rte_eth_dev_mac_addr_add(res->port_num,
-					&res->address, res->vf_num);
+					       &res->address, res->vf_num);
 	if(ret < 0)
 		printf("vf_mac_addr_cmd error: (%s)\n", strerror(-ret));
 
diff --git a/drivers/net/i40e/rte_pmd_i40e.c b/drivers/net/i40e/rte_pmd_i40e.c
index f12b7f4..8877239 100644
--- a/drivers/net/i40e/rte_pmd_i40e.c
+++ b/drivers/net/i40e/rte_pmd_i40e.c
@@ -2115,3 +2115,47 @@  int rte_pmd_i40e_ptype_mapping_replace(uint8_t port,
 
 	return 0;
 }
+
+int
+rte_pmd_i40e_add_vf_mac_addr(uint8_t port, uint16_t vf_id,
+			     struct ether_addr *mac_addr)
+{
+	struct rte_eth_dev *dev;
+	struct i40e_pf_vf *vf;
+	struct i40e_vsi *vsi;
+	struct i40e_pf *pf;
+	struct i40e_mac_filter_info mac_filter;
+	int ret;
+
+	if (i40e_validate_mac_addr((u8 *)mac_addr) != I40E_SUCCESS)
+		return -EINVAL;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
+
+	dev = &rte_eth_devices[port];
+
+	if (!is_i40e_supported(dev))
+		return -ENOTSUP;
+
+	pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+
+	if (vf_id >= pf->vf_num || !pf->vfs)
+		return -EINVAL;
+
+	vf = &pf->vfs[vf_id];
+	vsi = vf->vsi;
+	if (!vsi) {
+		PMD_DRV_LOG(ERR, "Invalid VSI.");
+		return -EINVAL;
+	}
+
+	mac_filter.filter_type = RTE_MACVLAN_PERFECT_MATCH;
+	ether_addr_copy(mac_addr, &mac_filter.mac_addr);
+	ret = i40e_vsi_add_mac(vsi, &mac_filter);
+	if (ret != I40E_SUCCESS) {
+		PMD_DRV_LOG(ERR, "Failed to add MAC filter.");
+		return -1;
+	}
+
+	return 0;
+}
diff --git a/drivers/net/i40e/rte_pmd_i40e.h b/drivers/net/i40e/rte_pmd_i40e.h
index 356fa89..155b7e8 100644
--- a/drivers/net/i40e/rte_pmd_i40e.h
+++ b/drivers/net/i40e/rte_pmd_i40e.h
@@ -637,4 +637,24 @@  int rte_pmd_i40e_ptype_mapping_replace(uint8_t port,
 				       uint8_t mask,
 				       uint32_t pkt_type);
 
+/**
+ * Add a VF MAC address.
+ *
+ * Add more MAC address for VF. The existing MAC addresses
+ * are still effective.
+ *
+ * @param port
+ *   The port identifier of the Ethernet device.
+ * @param vf_id
+ *   VF id.
+ * @param mac_addr
+ *   VF MAC address.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if *port* invalid.
+ *   - (-EINVAL) if *vf* or *mac_addr* is invalid.
+ */
+int rte_pmd_i40e_add_vf_mac_addr(uint8_t port, uint16_t vf_id,
+				 struct ether_addr *mac_addr);
+
 #endif /* _PMD_I40E_H_ */
diff --git a/drivers/net/i40e/rte_pmd_i40e_version.map b/drivers/net/i40e/rte_pmd_i40e_version.map
index 20cc980..84c6ff1 100644
--- a/drivers/net/i40e/rte_pmd_i40e_version.map
+++ b/drivers/net/i40e/rte_pmd_i40e_version.map
@@ -42,6 +42,7 @@  DPDK_17.05 {
 DPDK_17.08 {
 	global:
 
+	rte_pmd_i40e_add_vf_mac_addr;
 	rte_pmd_i40e_get_ddp_info;
 
 } DPDK_17.05;