[dpdk-dev,v5,1/3] librte_ether: add API for VF management

Message ID 1475158591-2243-2-git-send-email-bernard.iremonger@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Iremonger, Bernard Sept. 29, 2016, 2:16 p.m. UTC
  Add new API function to configure and manage VF's on a NIC.

add rte_eth_dev_set_vf_vlan_stripq function.

Signed-off-by: azelezniak <alexz@att.com>
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 lib/librte_ether/rte_ethdev.c          | 27 +++++++++++++++++++++++++++
 lib/librte_ether/rte_ethdev.h          | 23 ++++++++++++++++++++++-
 lib/librte_ether/rte_ether_version.map |  6 ++++++
 3 files changed, 55 insertions(+), 1 deletion(-)
  

Comments

Thomas Monjalon Sept. 29, 2016, 2:30 p.m. UTC | #1
2016-09-29 15:16, Bernard Iremonger:
> Add new API function to configure and manage VF's on a NIC.
> 
> add rte_eth_dev_set_vf_vlan_stripq function.
> 
> Signed-off-by: azelezniak <alexz@att.com>

We need the full name of azelezniak.

> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
[...]
> +int
> +rte_eth_dev_set_vf_vlan_stripq(uint8_t port, uint16_t vf, int on);

Why keeping this function in ethdev?

I think it would be more consistent to have also existing VF functions
moving from ethdev to rte_pmd_ixgbe.h.
You cannot remove them, but you can create their ixgbe-specific version
and announce that the ethdev ones are deprecated.
  
Iremonger, Bernard Sept. 29, 2016, 3:16 p.m. UTC | #2
Hi Thomas,

<snip>

> Subject: Re: [dpdk-dev] [PATCH v5 1/3] librte_ether: add API for VF
> management
> 
> 2016-09-29 15:16, Bernard Iremonger:
> > Add new API function to configure and manage VF's on a NIC.
> >
> > add rte_eth_dev_set_vf_vlan_stripq function.
> >
> > Signed-off-by: azelezniak <alexz@att.com>
> 
> We need the full name of azelezniak.

It is Alex Zelezniak, I will update the commit messages.

> > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> [...]
> > +int
> > +rte_eth_dev_set_vf_vlan_stripq(uint8_t port, uint16_t vf, int on);
> 
> Why keeping this function in ethdev?

This function is using an existing API in the eth_dev_ops structure.

dev->dev_ops->vlan_strip_queue_set

The vlan_strip_queue_set API is used by  the i40e, ixgbe and mlx5 PMD's.

> I think it would be more consistent to have also existing VF functions moving
> from ethdev to rte_pmd_ixgbe.h.
> You cannot remove them, but you can create their ixgbe-specific version and
> announce that the ethdev ones are deprecated.

There are 5 existing VF functions which are only used by ixgbe PMD at present.
It would make sense to create ixgbe-specific versions, however I think this should be done in a separate patchset.

Regards,

Bernard
  
Thomas Monjalon Sept. 29, 2016, 4:19 p.m. UTC | #3
2016-09-29 15:16, Iremonger, Bernard:
> > 2016-09-29 15:16, Bernard Iremonger:
> > > +int
> > > +rte_eth_dev_set_vf_vlan_stripq(uint8_t port, uint16_t vf, int on);
> > 
> > Why keeping this function in ethdev?
> 
> This function is using an existing API in the eth_dev_ops structure.
> 
> dev->dev_ops->vlan_strip_queue_set
> 
> The vlan_strip_queue_set API is used by  the i40e, ixgbe and mlx5 PMD's.

OK but it was not used to control VF from PF.
This line:
	(*dev->dev_ops->vlan_strip_queue_set)(dev, q + vf * queues_per_pool, on);
seems Intel specific.
Please keep "VF from PF" outside of ethdev for 16.11.

> > I think it would be more consistent to have also existing VF functions moving
> > from ethdev to rte_pmd_ixgbe.h.
> > You cannot remove them, but you can create their ixgbe-specific version and
> > announce that the ethdev ones are deprecated.
> 
> There are 5 existing VF functions which are only used by ixgbe PMD at present.
> It would make sense to create ixgbe-specific versions, however I think this should be done in a separate patchset.

Yes it can be a separate patchset for RC2 of course.
  
Iremonger, Bernard Sept. 29, 2016, 4:38 p.m. UTC | #4
Hi Thomas,

> Subject: Re: [dpdk-dev] [PATCH v5 1/3] librte_ether: add API for VF
> management
> 
> 2016-09-29 15:16, Iremonger, Bernard:
> > > 2016-09-29 15:16, Bernard Iremonger:
> > > > +int
> > > > +rte_eth_dev_set_vf_vlan_stripq(uint8_t port, uint16_t vf, int
> > > > +on);
> > >
> > > Why keeping this function in ethdev?
> >
> > This function is using an existing API in the eth_dev_ops structure.
> >
> > dev->dev_ops->vlan_strip_queue_set
> >
> > The vlan_strip_queue_set API is used by  the i40e, ixgbe and mlx5 PMD's.
> 
> OK but it was not used to control VF from PF.
> This line:
> 	(*dev->dev_ops->vlan_strip_queue_set)(dev, q + vf *
> queues_per_pool, on); seems Intel specific.
> Please keep "VF from PF" outside of ethdev for 16.11.

I will try calling  (*dev->dev_ops->vlan_strip_queue_set) from an ixgbe-specific function.
Will this be acceptable? 

Regards,

Bernard.
  
Thomas Monjalon Sept. 29, 2016, 4:45 p.m. UTC | #5
2016-09-29 16:38, Iremonger, Bernard:
> Hi Thomas,
> 
> > Subject: Re: [dpdk-dev] [PATCH v5 1/3] librte_ether: add API for VF
> > management
> > 
> > 2016-09-29 15:16, Iremonger, Bernard:
> > > > 2016-09-29 15:16, Bernard Iremonger:
> > > > > +int
> > > > > +rte_eth_dev_set_vf_vlan_stripq(uint8_t port, uint16_t vf, int
> > > > > +on);
> > > >
> > > > Why keeping this function in ethdev?
> > >
> > > This function is using an existing API in the eth_dev_ops structure.
> > >
> > > dev->dev_ops->vlan_strip_queue_set
> > >
> > > The vlan_strip_queue_set API is used by  the i40e, ixgbe and mlx5 PMD's.
> > 
> > OK but it was not used to control VF from PF.
> > This line:
> > 	(*dev->dev_ops->vlan_strip_queue_set)(dev, q + vf *
> > queues_per_pool, on); seems Intel specific.
> > Please keep "VF from PF" outside of ethdev for 16.11.
> 
> I will try calling  (*dev->dev_ops->vlan_strip_queue_set) from an ixgbe-specific function.
> Will this be acceptable? 

I think it is acceptable.
  

Patch

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 382c959..15e21ca 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -2489,6 +2489,33 @@  rte_eth_dev_set_vf_vlan_filter(uint8_t port_id, uint16_t vlan_id,
 						   vf_mask, vlan_on);
 }
 
+int
+rte_eth_dev_set_vf_vlan_stripq(uint8_t port_id, uint16_t vf, int on)
+{
+	struct rte_eth_dev *dev;
+	struct rte_eth_dev_info dev_info;
+	uint16_t queues_per_pool;
+	uint32_t q;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+	dev = &rte_eth_devices[port_id];
+	rte_eth_dev_info_get(port_id, &dev_info);
+
+	if (vf >= dev_info.max_vfs) {
+		RTE_PMD_DEBUG_TRACE("set VF vlan stripq: invalid VF %d\n", vf);
+		return -EINVAL;
+	}
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->vlan_strip_queue_set, -ENOTSUP);
+
+	queues_per_pool = dev_info.vmdq_queue_num/dev_info.max_vmdq_pools;
+
+	for (q = 0; q < queues_per_pool; q++)
+		(*dev->dev_ops->vlan_strip_queue_set)(dev, q + vf * queues_per_pool, on);
+	return 0;
+}
+
 int rte_eth_set_queue_rate_limit(uint8_t port_id, uint16_t queue_idx,
 					uint16_t tx_rate)
 {
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 96575e8..fb2be45 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -3499,7 +3499,28 @@  rte_eth_dev_set_vf_vlan_filter(uint8_t port, uint16_t vlan_id,
 				uint8_t vlan_on);
 
 /**
- * Set a traffic mirroring rule on an Ethernet device
+ * Enable/Disable vf vlan strip for all queues in a pool
+ *
+ * @param port
+ *    The port identifier of the Ethernet device.
+ * @param vf
+ *    ID specifying VF.
+ * @param on
+ *    1 - Enable VF's vlan strip on RX queues.
+ *    0 - Disable VF's vlan strip on RX queues.
+ * @param queues_per_pool
+ *    The number of queues per pool.
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if hardware doesn't support this feature.
+ *   - (-ENODEV) if *port* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+int
+rte_eth_dev_set_vf_vlan_stripq(uint8_t port, uint16_t vf, int on);
+
+/** Set a traffic mirroring rule on an Ethernet device
  *
  * @param port_id
  *   The port identifier of the Ethernet device.
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index 45ddf44..ae44074 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -139,3 +139,9 @@  DPDK_16.07 {
 	rte_eth_dev_get_port_by_name;
 	rte_eth_xstats_get_names;
 } DPDK_16.04;
+
+DPDK_16.11 {
+	global:
+
+	rte_eth_dev_set_vf_vlan_stripq;
+} DPDK_16.07;