DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/i40e: new API to add VF MAC address from PF
@ 2017-07-24 20:51 Wenzhuo Lu
  2017-07-25 14:09 ` [dpdk-dev] [PATCH v2] " Wenzhuo Lu
  2017-08-17 18:33 ` [dpdk-dev] [PATCH v3 0/2] fix adding VF MAC in testpmd Wenzhuo Lu
  0 siblings, 2 replies; 11+ messages in thread
From: Wenzhuo Lu @ 2017-07-24 20:51 UTC (permalink / raw)
  To: dev; +Cc: jingjing.wu, beilei.xing, Wenzhuo Lu

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>
---
 app/test-pmd/cmdline.c                    |  9 +++++--
 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, 72 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index b1b36c1..2d2a56e 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -7211,9 +7211,14 @@ 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;
+
+	ret = rte_pmd_i40e_add_vf_mac_addr(res->port_num, res->vf_num,
+					   &res->address);
+	if (ret == -ENOTSUP)
 		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;
-- 
1.9.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [dpdk-dev] [PATCH v2] net/i40e: new API to add VF MAC address from PF
  2017-07-24 20:51 [dpdk-dev] [PATCH] net/i40e: new API to add VF MAC address from PF Wenzhuo Lu
@ 2017-07-25 14:09 ` Wenzhuo Lu
  2017-08-17 13:05   ` Ferruh Yigit
  2017-08-17 18:33 ` [dpdk-dev] [PATCH v3 0/2] fix adding VF MAC in testpmd Wenzhuo Lu
  1 sibling, 1 reply; 11+ messages in thread
From: Wenzhuo Lu @ 2017-07-25 14:09 UTC (permalink / raw)
  To: dev; +Cc: jingjing.wu, beilei.xing, Wenzhuo Lu

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(-)

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;
-- 
1.9.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v2] net/i40e: new API to add VF MAC address from PF
  2017-07-25 14:09 ` [dpdk-dev] [PATCH v2] " Wenzhuo Lu
@ 2017-08-17 13:05   ` Ferruh Yigit
  2017-08-17 13:36     ` Lu, Wenzhuo
  0 siblings, 1 reply; 11+ messages in thread
From: Ferruh Yigit @ 2017-08-17 13:05 UTC (permalink / raw)
  To: Wenzhuo Lu, dev; +Cc: jingjing.wu, beilei.xing, Stephen Hurd, Ajit Khaparde

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>

<...>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v2] net/i40e: new API to add VF MAC address from PF
  2017-08-17 13:05   ` Ferruh Yigit
@ 2017-08-17 13:36     ` Lu, Wenzhuo
  0 siblings, 0 replies; 11+ messages in thread
From: Lu, Wenzhuo @ 2017-08-17 13:36 UTC (permalink / raw)
  To: Yigit, Ferruh, dev
  Cc: Wu, Jingjing, Xing, Beilei, Stephen Hurd, Ajit Khaparde

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [dpdk-dev] [PATCH v3 0/2] fix adding VF MAC in testpmd
  2017-07-24 20:51 [dpdk-dev] [PATCH] net/i40e: new API to add VF MAC address from PF Wenzhuo Lu
  2017-07-25 14:09 ` [dpdk-dev] [PATCH v2] " Wenzhuo Lu
@ 2017-08-17 18:33 ` Wenzhuo Lu
  2017-08-17 18:33   ` [dpdk-dev] [PATCH v3 1/2] net/i40e: new API to add VF MAC address from PF Wenzhuo Lu
                     ` (2 more replies)
  1 sibling, 3 replies; 11+ messages in thread
From: Wenzhuo Lu @ 2017-08-17 18:33 UTC (permalink / raw)
  To: dev; +Cc: Wenzhuo Lu

In testpmd, it's wrong to call rte_eth_dev_mac_addr_add to add a MAC
address for a VF. Because this API is used to add a MAC address for
a VMDq pool.

v3:
 - split the patch to 2. One for adding a new API. One for fixing the CLI.

Wenzhuo Lu (2):
  net/i40e: new API to add VF MAC address from PF
  app/testpmd: fix wrong API of adding VF MAC

 app/test-pmd/cmdline.c                    | 19 ++++++++++---
 drivers/net/i40e/rte_pmd_i40e.c           | 44 +++++++++++++++++++++++++++++++
 drivers/net/i40e/rte_pmd_i40e.h           | 20 ++++++++++++++
 drivers/net/i40e/rte_pmd_i40e_version.map |  7 +++++
 4 files changed, 86 insertions(+), 4 deletions(-)

-- 
1.9.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [dpdk-dev] [PATCH v3 1/2] net/i40e: new API to add VF MAC address from PF
  2017-08-17 18:33 ` [dpdk-dev] [PATCH v3 0/2] fix adding VF MAC in testpmd Wenzhuo Lu
@ 2017-08-17 18:33   ` Wenzhuo Lu
  2017-08-18  0:32     ` Stephen Hemminger
  2017-08-17 18:33   ` [dpdk-dev] [PATCH v3 2/2] app/testpmd: fix wrong API of adding VF MAC Wenzhuo Lu
  2017-08-28  9:53   ` [dpdk-dev] [PATCH v3 0/2] fix adding VF MAC in testpmd Ferruh Yigit
  2 siblings, 1 reply; 11+ messages in thread
From: Wenzhuo Lu @ 2017-08-17 18:33 UTC (permalink / raw)
  To: dev; +Cc: Wenzhuo Lu

Currently, rte_eth_dev_mac_addr_add is used by a
testpmd CLI to add a MAC address for VF. But the
parameter 'pool' of this API means the VMDq pool,
not VF.
So, it's wrong to use it to add the VF MAC address.
This patch provides a new API that can be used to
add VF MAC address on i40e.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 drivers/net/i40e/rte_pmd_i40e.c           | 44 +++++++++++++++++++++++++++++++
 drivers/net/i40e/rte_pmd_i40e.h           | 20 ++++++++++++++
 drivers/net/i40e/rte_pmd_i40e_version.map |  7 +++++
 3 files changed, 71 insertions(+)

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..ef8882b 100644
--- a/drivers/net/i40e/rte_pmd_i40e_version.map
+++ b/drivers/net/i40e/rte_pmd_i40e_version.map
@@ -45,3 +45,10 @@ DPDK_17.08 {
 	rte_pmd_i40e_get_ddp_info;
 
 } DPDK_17.05;
+
+DPDK_17.11 {
+	global:
+
+	rte_pmd_i40e_add_vf_mac_addr;
+
+} DPDK_17.08;
-- 
1.9.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [dpdk-dev] [PATCH v3 2/2] app/testpmd: fix wrong API of adding VF MAC
  2017-08-17 18:33 ` [dpdk-dev] [PATCH v3 0/2] fix adding VF MAC in testpmd Wenzhuo Lu
  2017-08-17 18:33   ` [dpdk-dev] [PATCH v3 1/2] net/i40e: new API to add VF MAC address from PF Wenzhuo Lu
@ 2017-08-17 18:33   ` Wenzhuo Lu
  2017-08-28  9:53   ` [dpdk-dev] [PATCH v3 0/2] fix adding VF MAC in testpmd Ferruh Yigit
  2 siblings, 0 replies; 11+ messages in thread
From: Wenzhuo Lu @ 2017-08-17 18:33 UTC (permalink / raw)
  To: dev; +Cc: Wenzhuo Lu

When adding a VF MAC address, rte_eth_dev_mac_addr_add
is called. It's not right, because this API is used to
add a MAC address for a VMDq pool not a VF. Although it
can work on ixgbe as VMDq pool and VF mean the same thing
on ixgbe.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 app/test-pmd/cmdline.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index cd8c358..0144191 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -7209,11 +7209,22 @@ static void cmd_vf_mac_addr_parsed(void *parsed_result,
 		__attribute__((unused)) void *data)
 {
 	struct cmd_vf_mac_addr_result *res = parsed_result;
-	int ret = 0;
+	int ret = -ENOTSUP;
+
+	if (strcmp(res->what, "add") != 0)
+		return;
+
+#ifdef RTE_LIBRTE_I40E_PMD
+	if (ret == -ENOTSUP)
+		ret = rte_pmd_i40e_add_vf_mac_addr(res->port_num, res->vf_num,
+						   &res->address);
+#endif
+#ifdef RTE_LIBRTE_BNXT_PMD
+	if (ret == -ENOTSUP)
+		ret = rte_pmd_bnxt_mac_addr_add(res->port_num, &res->address,
+						res->vf_num);
+#endif
 
-	if (strcmp(res->what, "add") == 0)
-		ret = rte_eth_dev_mac_addr_add(res->port_num,
-					&res->address, res->vf_num);
 	if(ret < 0)
 		printf("vf_mac_addr_cmd error: (%s)\n", strerror(-ret));
 
-- 
1.9.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/2] net/i40e: new API to add VF MAC address from PF
  2017-08-17 18:33   ` [dpdk-dev] [PATCH v3 1/2] net/i40e: new API to add VF MAC address from PF Wenzhuo Lu
@ 2017-08-18  0:32     ` Stephen Hemminger
  2017-08-18 16:43       ` Ferruh Yigit
  0 siblings, 1 reply; 11+ messages in thread
From: Stephen Hemminger @ 2017-08-18  0:32 UTC (permalink / raw)
  To: Wenzhuo Lu; +Cc: dev

On Fri, 18 Aug 2017 02:33:42 +0800
Wenzhuo Lu <wenzhuo.lu@intel.com> wrote:

> Currently, rte_eth_dev_mac_addr_add is used by a
> testpmd CLI to add a MAC address for VF. But the
> parameter 'pool' of this API means the VMDq pool,
> not VF.
> So, it's wrong to use it to add the VF MAC address.
> This patch provides a new API that can be used to
> add VF MAC address on i40e.
> 
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

What do other drivers do?
Sorry, a driver specific API is (almost) always the wrong solution.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/2] net/i40e: new API to add VF MAC address from PF
  2017-08-18  0:32     ` Stephen Hemminger
@ 2017-08-18 16:43       ` Ferruh Yigit
  2017-08-19 12:52         ` Lu, Wenzhuo
  0 siblings, 1 reply; 11+ messages in thread
From: Ferruh Yigit @ 2017-08-18 16:43 UTC (permalink / raw)
  To: Stephen Hemminger, Wenzhuo Lu; +Cc: dev

On 8/18/2017 1:32 AM, Stephen Hemminger wrote:
> On Fri, 18 Aug 2017 02:33:42 +0800
> Wenzhuo Lu <wenzhuo.lu@intel.com> wrote:
> 
>> Currently, rte_eth_dev_mac_addr_add is used by a
>> testpmd CLI to add a MAC address for VF. But the
>> parameter 'pool' of this API means the VMDq pool,
>> not VF.
>> So, it's wrong to use it to add the VF MAC address.
>> This patch provides a new API that can be used to
>> add VF MAC address on i40e.
>>
>> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> 
> What do other drivers do?

This is adding VF MAC through PF, not all PMDs support this.

> Sorry, a driver specific API is (almost) always the wrong solution.

Currently there are set of device specific APIs [1].

The motivation was to be able to expose NIC specific features,
abstraction layer should not prevent using HW features; while keeping
abstraction clean for common majority, not having APIs valid only for a
single NIC.

But with number of the device specific APIs increasing, it may be time
to bring postponed discussion to life on how to manage them.

Recent ioclt or staging dev_ops was a good tart:
http://dpdk.org/ml/archives/dev/2017-August/thread.html#72423


[1]
Physical PMDs with device specific API: i40e, ixgbe, bnxt
Virtual PMDs with device specific API: ring, vhost, bonding, xenvirt

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/2] net/i40e: new API to add VF MAC address from PF
  2017-08-18 16:43       ` Ferruh Yigit
@ 2017-08-19 12:52         ` Lu, Wenzhuo
  0 siblings, 0 replies; 11+ messages in thread
From: Lu, Wenzhuo @ 2017-08-19 12:52 UTC (permalink / raw)
  To: Yigit, Ferruh, Stephen Hemminger; +Cc: dev

Hi,

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Friday, August 18, 2017 12:43 PM
> To: Stephen Hemminger <stephen@networkplumber.org>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 1/2] net/i40e: new API to add VF MAC
> address from PF
> 
> On 8/18/2017 1:32 AM, Stephen Hemminger wrote:
> > On Fri, 18 Aug 2017 02:33:42 +0800
> > Wenzhuo Lu <wenzhuo.lu@intel.com> wrote:
> >
> >> Currently, rte_eth_dev_mac_addr_add is used by a testpmd CLI to add a
> >> MAC address for VF. But the parameter 'pool' of this API means the
> >> VMDq pool, not VF.
> >> So, it's wrong to use it to add the VF MAC address.
> >> This patch provides a new API that can be used to add VF MAC address
> >> on i40e.
> >>
> >> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> >
> > What do other drivers do?
> 
> This is adding VF MAC through PF, not all PMDs support this.
> 
> > Sorry, a driver specific API is (almost) always the wrong solution.
> 
> Currently there are set of device specific APIs [1].
> 
> The motivation was to be able to expose NIC specific features, abstraction
> layer should not prevent using HW features; while keeping abstraction clean
> for common majority, not having APIs valid only for a single NIC.
> 
> But with number of the device specific APIs increasing, it may be time to
> bring postponed discussion to life on how to manage them.
> 
> Recent ioclt or staging dev_ops was a good tart:
> http://dpdk.org/ml/archives/dev/2017-August/thread.html#72423
> 
> 
> [1]
> Physical PMDs with device specific API: i40e, ixgbe, bnxt Virtual PMDs with
> device specific API: ring, vhost, bonding, xenvirt
This patch follows the existing bnxt example to implement the same function on i40e.

Totally agree the private API is not a good idea. To my opinion, as many same functions are implemented on ixgbe, i40e, bnxt, it's them time to make them public. Let me send some RFC later.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v3 0/2] fix adding VF MAC in testpmd
  2017-08-17 18:33 ` [dpdk-dev] [PATCH v3 0/2] fix adding VF MAC in testpmd Wenzhuo Lu
  2017-08-17 18:33   ` [dpdk-dev] [PATCH v3 1/2] net/i40e: new API to add VF MAC address from PF Wenzhuo Lu
  2017-08-17 18:33   ` [dpdk-dev] [PATCH v3 2/2] app/testpmd: fix wrong API of adding VF MAC Wenzhuo Lu
@ 2017-08-28  9:53   ` Ferruh Yigit
  2 siblings, 0 replies; 11+ messages in thread
From: Ferruh Yigit @ 2017-08-28  9:53 UTC (permalink / raw)
  To: Wenzhuo Lu, dev

On 8/17/2017 7:33 PM, Wenzhuo Lu wrote:
> In testpmd, it's wrong to call rte_eth_dev_mac_addr_add to add a MAC
> address for a VF. Because this API is used to add a MAC address for
> a VMDq pool.
> 
> v3:
>  - split the patch to 2. One for adding a new API. One for fixing the CLI.
> 
> Wenzhuo Lu (2):
>   net/i40e: new API to add VF MAC address from PF
>   app/testpmd: fix wrong API of adding VF MAC

Series applied to dpdk-next-net/master, thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-08-28  9:53 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-24 20:51 [dpdk-dev] [PATCH] net/i40e: new API to add VF MAC address from PF Wenzhuo Lu
2017-07-25 14:09 ` [dpdk-dev] [PATCH v2] " Wenzhuo Lu
2017-08-17 13:05   ` Ferruh Yigit
2017-08-17 13:36     ` Lu, Wenzhuo
2017-08-17 18:33 ` [dpdk-dev] [PATCH v3 0/2] fix adding VF MAC in testpmd Wenzhuo Lu
2017-08-17 18:33   ` [dpdk-dev] [PATCH v3 1/2] net/i40e: new API to add VF MAC address from PF Wenzhuo Lu
2017-08-18  0:32     ` Stephen Hemminger
2017-08-18 16:43       ` Ferruh Yigit
2017-08-19 12:52         ` Lu, Wenzhuo
2017-08-17 18:33   ` [dpdk-dev] [PATCH v3 2/2] app/testpmd: fix wrong API of adding VF MAC Wenzhuo Lu
2017-08-28  9:53   ` [dpdk-dev] [PATCH v3 0/2] fix adding VF MAC in testpmd Ferruh Yigit

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).