[dpdk-dev] [PATCH v2 1/3] ether: Add API to support setting TX rate for queue and VF
Hi Changchun, 2014-06-05 03:30, Ouyang, Changchun: > As we can see below, There are already 4 existing functions for vf in the > header file: > rte_ethdev.h:int rte_eth_dev_set_vf_rxmode(uint8_t port, uint16_t vf, > uint16_t rx_mode, > rte_ethdev.h:rte_eth_dev_set_vf_tx(uint8_t port,uint16_t vf, uint8_t on); > rte_ethdev.h:rte_eth_dev_set_vf_rx(uint8_t port,uint16_t vf, uint8_t on); > rte_ethdev.h:rte_eth_dev_set_vf_vlan_filter(uint8_t port, uint16_t vlan_id, > > So do we have plan to move them or remove them as they are all for VF > specifically? I'm not very happy with these functions. I feel it's too specific because only related to igb/ixgbe features. We should try to isolate these things elsewhere. The ethdev API should be generic. It's not the right time to debate about this but I'd like to have such cleanup in roadmap. > If no, why we can accept those functions, but not accept the > rte_eth_set_vf_rate_limit? :-) Let's accept it while thinking together to a future nice cleanup. -- Thomas
[dpdk-dev] [PATCH v2 1/3] ether: Add API to support setting TX rate for queue and VF
Hi Thomas, As we can see below, There are already 4 existing functions for vf in the header file: rte_ethdev.h:int rte_eth_dev_set_vf_rxmode(uint8_t port, uint16_t vf, uint16_t rx_mode, rte_ethdev.h:rte_eth_dev_set_vf_tx(uint8_t port,uint16_t vf, uint8_t on); rte_ethdev.h:rte_eth_dev_set_vf_rx(uint8_t port,uint16_t vf, uint8_t on); rte_ethdev.h:rte_eth_dev_set_vf_vlan_filter(uint8_t port, uint16_t vlan_id, So do we have plan to move them or remove them as they are all for VF specifically? If no, why we can accept those functions, but not accept the rte_eth_set_vf_rate_limit? :-) I have 2 new api in this patch, the rte_eth_set_queue_rate_limit is more generic, whose Argument only have port and queue. but PRC customer has the requirement of API function to limit the vf tx rate, so personally I think rte_eth_set_vf_rate_limit is necessary for them. Thanks and regards, Changchun -Original Message- From: Thomas Monjalon [mailto:thomas.monja...@6wind.com] Sent: Wednesday, May 28, 2014 6:48 AM To: Ouyang, Changchun Cc: dev at dpdk.org Subject: Re: [dpdk-dev] [PATCH v2 1/3] ether: Add API to support setting TX rate for queue and VF Hi Changchun, 2014-05-26 15:45, Ouyang Changchun: > /** > + * Set the rate limitation for a queue on an Ethernet device. > + * > + * @param port_id > + * The port identifier of the Ethernet device. > + * @param queue_idx > + * The queue id. > + * @param tx_rate > + * The tx rate allocated from the total link speed for this queue. > + * @return > + * - (0) if successful. > + * - (-ENOTSUP) if hardware doesn't support this feature. > + * - (-ENODEV) if *port_id* invalid. > + * - (-EINVAL) if bad parameter. > + */ > +int rte_eth_set_queue_rate_limit(uint8_t port_id, uint16_t queue_idx, > + uint16_t tx_rate); > + > +/** > + * Set the rate limitation for a vf on an Ethernet device. > + * > + * @param port_id > + * The port identifier of the Ethernet device. > + * @param vf > + * VF id. > + * @param tx_rate > + * The tx rate allocated from the total link speed for this VF id. > + * @param q_msk > + * The queue mask which need to set the rate. > + * @return > + * - (0) if successful. > + * - (-ENOTSUP) if hardware doesn't support this feature. > + * - (-ENODEV) if *port_id* invalid. > + * - (-EINVAL) if bad parameter. > + */ > +int rte_eth_set_vf_rate_limit(uint8_t port_id, uint16_t vf, > + uint16_t tx_rate, uint64_t q_msk); You are defining an API function specifically for VF. It's not generic and shouldn't appear in the API. We now have to be careful about the API and try to build a robust generic API which could become stable. Is it possible to imagine another API where only port and queue parameters are required? Thanks -- Thomas
[dpdk-dev] [PATCH v2 1/3] ether: Add API to support setting TX rate for queue and VF
Hi Changchun, 2014-05-26 15:45, Ouyang Changchun: > /** > + * Set the rate limitation for a queue on an Ethernet device. > + * > + * @param port_id > + * The port identifier of the Ethernet device. > + * @param queue_idx > + * The queue id. > + * @param tx_rate > + * The tx rate allocated from the total link speed for this queue. > + * @return > + * - (0) if successful. > + * - (-ENOTSUP) if hardware doesn't support this feature. > + * - (-ENODEV) if *port_id* invalid. > + * - (-EINVAL) if bad parameter. > + */ > +int rte_eth_set_queue_rate_limit(uint8_t port_id, uint16_t queue_idx, > + uint16_t tx_rate); > + > +/** > + * Set the rate limitation for a vf on an Ethernet device. > + * > + * @param port_id > + * The port identifier of the Ethernet device. > + * @param vf > + * VF id. > + * @param tx_rate > + * The tx rate allocated from the total link speed for this VF id. > + * @param q_msk > + * The queue mask which need to set the rate. > + * @return > + * - (0) if successful. > + * - (-ENOTSUP) if hardware doesn't support this feature. > + * - (-ENODEV) if *port_id* invalid. > + * - (-EINVAL) if bad parameter. > + */ > +int rte_eth_set_vf_rate_limit(uint8_t port_id, uint16_t vf, > + uint16_t tx_rate, uint64_t q_msk); You are defining an API function specifically for VF. It's not generic and shouldn't appear in the API. We now have to be careful about the API and try to build a robust generic API which could become stable. Is it possible to imagine another API where only port and queue parameters are required? Thanks -- Thomas
[dpdk-dev] [PATCH v2 1/3] ether: Add API to support setting TX rate for queue and VF
This patch adds API to support setting TX rate for a queue and a VF.
Signed-off-by: Ouyang Changchun
---
lib/librte_ether/rte_ethdev.c | 71 +++
lib/librte_ether/rte_ethdev.h | 51 +++
2 files changed, 122 insertions(+)
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index a5727dd..1ea61e1 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1913,6 +1913,77 @@ rte_eth_dev_set_vf_vlan_filter(uint8_t port_id, uint16_t
vlan_id,
vf_mask,vlan_on);
}
+int rte_eth_set_queue_rate_limit(uint8_t port_id, uint16_t queue_idx,
+ uint16_t tx_rate)
+{
+ struct rte_eth_dev *dev;
+ struct rte_eth_dev_info dev_info;
+ struct rte_eth_link link;
+
+ if (port_id >= nb_ports) {
+ PMD_DEBUG_TRACE("set queue rate limit:invalid port id=%d\n",
+ port_id);
+ return -ENODEV;
+ }
+
+ dev = _eth_devices[port_id];
+ rte_eth_dev_info_get(port_id, _info);
+ link = dev->data->dev_link;
+
+ if (queue_idx > dev_info.max_tx_queues) {
+ PMD_DEBUG_TRACE("set queue rate limit:port %d: "
+ "invalid queue id=%d\n", port_id, queue_idx);
+ return -EINVAL;
+ }
+
+ if (tx_rate > link.link_speed) {
+ PMD_DEBUG_TRACE("set queue rate limit:invalid tx_rate=%d, "
+ "bigger than link speed= %d\n",
+ tx_rate, link_speed);
+ return -EINVAL;
+ }
+
+ FUNC_PTR_OR_ERR_RET(*dev->dev_ops->set_queue_rate_limit, -ENOTSUP);
+ return (*dev->dev_ops->set_queue_rate_limit)(dev, queue_idx, tx_rate);
+}
+
+int rte_eth_set_vf_rate_limit(uint8_t port_id, uint16_t vf, uint16_t tx_rate,
+ uint64_t q_msk)
+{
+ struct rte_eth_dev *dev;
+ struct rte_eth_dev_info dev_info;
+ struct rte_eth_link link;
+
+ if (q_msk == 0)
+ return 0;
+
+ if (port_id >= nb_ports) {
+ PMD_DEBUG_TRACE("set VF rate limit:invalid port id=%d\n",
+ port_id);
+ return -ENODEV;
+ }
+
+ dev = _eth_devices[port_id];
+ rte_eth_dev_info_get(port_id, _info);
+ link = dev->data->dev_link;
+
+ if (vf > dev_info.max_vfs) {
+ PMD_DEBUG_TRACE("set VF rate limit:port %d: "
+ "invalid vf id=%d\n", port_id, vf);
+ return -EINVAL;
+ }
+
+ if (tx_rate > link.link_speed) {
+ PMD_DEBUG_TRACE("set VF rate limit:invalid tx_rate=%d, "
+ "bigger than link speed= %d\n",
+ tx_rate, link_speed);
+ return -EINVAL;
+ }
+
+ FUNC_PTR_OR_ERR_RET(*dev->dev_ops->set_vf_rate_limit, -ENOTSUP);
+ return (*dev->dev_ops->set_vf_rate_limit)(dev, vf, tx_rate, q_msk);
+}
+
int
rte_eth_mirror_rule_set(uint8_t port_id,
struct rte_eth_vmdq_mirror_conf *mirror_conf,
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index d5ea46b..445d40a 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1012,6 +1012,17 @@ typedef int (*eth_set_vf_vlan_filter_t)(struct
rte_eth_dev *dev,
uint8_t vlan_on);
/**< @internal Set VF VLAN pool filter */
+typedef int (*eth_set_queue_rate_limit_t)(struct rte_eth_dev *dev,
+ uint16_t queue_idx,
+ uint16_t tx_rate);
+/**< @internal Set queue TX rate */
+
+typedef int (*eth_set_vf_rate_limit_t)(struct rte_eth_dev *dev,
+ uint16_t vf,
+ uint16_t tx_rate,
+ uint64_t q_msk);
+/**< @internal Set VF TX rate */
+
typedef int (*eth_mirror_rule_set_t)(struct rte_eth_dev *dev,
struct rte_eth_vmdq_mirror_conf *mirror_conf,
uint8_t rule_id,
@@ -1119,6 +1130,8 @@ struct eth_dev_ops {
eth_set_vf_rx_tset_vf_rx; /**< enable/disable a VF receive
*/
eth_set_vf_tx_tset_vf_tx; /**< enable/disable a VF
transmit */
eth_set_vf_vlan_filter_t set_vf_vlan_filter; /**< Set VF VLAN filter
*/
+ eth_set_queue_rate_limit_t set_queue_rate_limit; /**< Set queue rate
limit */
+ eth_set_vf_rate_limit_tset_vf_rate_limit; /**< Set VF rate limit
*/
/** Add a signature filter. */
fdir_add_signature_filter_t fdir_add_signature_filter;
@@ -2561,6 +2574,44 @@ int rte_eth_mirror_rule_reset(uint8_t port_id,
uint8_t rule_id);
/**
+ * Set the rate limitation for a queue on an
