DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v2 0/3] Support setting TX rate for queue and VF
@ 2014-05-26  7:45 Ouyang Changchun
  2014-05-26  7:45 ` [dpdk-dev] [PATCH v2 1/3] ether: Add API to support " Ouyang Changchun
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Ouyang Changchun @ 2014-05-26  7:45 UTC (permalink / raw)
  To: dev

This patch v2 fixes some errors and warnings reported by checkpatch.pl.
 
This patch series also contain the 3 items:
1. Add API to support setting TX rate for a queue or a VF.
2. Implement the functionality of setting TX rate for queue or VF in IXGBE PMD.
3. Add commands in testpmd to test the functionality of setting TX rate for queue or VF.

Ouyang Changchun (3):
  Add API to support set TX rate for a queue and VF.
  Implement the functionality of setting TX rate for queue or VF in
    IXGBE PMD.
  Add commands in testpmd to test the functionality of setting TX rate
    for queue or VF.

 app/test-pmd/cmdline.c              | 159 +++++++++++++++++++++++++++++++++++-
 app/test-pmd/config.c               |  47 +++++++++++
 app/test-pmd/testpmd.h              |   3 +
 lib/librte_ether/rte_ethdev.c       |  71 ++++++++++++++++
 lib/librte_ether/rte_ethdev.h       |  51 ++++++++++++
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 122 +++++++++++++++++++++++++++
 lib/librte_pmd_ixgbe/ixgbe_ethdev.h |  13 ++-
 7 files changed, 462 insertions(+), 4 deletions(-)

-- 
1.9.0

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

* [dpdk-dev] [PATCH v2 1/3] ether: Add API to support setting TX rate for queue and VF
  2014-05-26  7:45 [dpdk-dev] [PATCH v2 0/3] Support setting TX rate for queue and VF Ouyang Changchun
@ 2014-05-26  7:45 ` Ouyang Changchun
  2014-05-27 22:47   ` Thomas Monjalon
  2014-05-26  7:45 ` [dpdk-dev] [PATCH v2 2/3] ixgbe: Implement the functionality of setting TX rate for queue or VF in IXGBE PMD Ouyang Changchun
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Ouyang Changchun @ 2014-05-26  7:45 UTC (permalink / raw)
  To: dev

This patch adds API to support setting TX rate for a queue and a VF.

Signed-off-by: Ouyang Changchun <changchun.ouyang@intel.com>
---
 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 = &rte_eth_devices[port_id];
+	rte_eth_dev_info_get(port_id, &dev_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 = &rte_eth_devices[port_id];
+	rte_eth_dev_info_get(port_id, &dev_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_t            set_vf_rx;  /**< enable/disable a VF receive */
 	eth_set_vf_tx_t            set_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_t    set_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 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);
+
+/**
  * Initialize bypass logic. This function needs to be called before
  * executing any other bypass API.
  *
-- 
1.9.0

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

* [dpdk-dev] [PATCH v2 2/3] ixgbe: Implement the functionality of setting TX rate for queue or VF in IXGBE PMD
  2014-05-26  7:45 [dpdk-dev] [PATCH v2 0/3] Support setting TX rate for queue and VF Ouyang Changchun
  2014-05-26  7:45 ` [dpdk-dev] [PATCH v2 1/3] ether: Add API to support " Ouyang Changchun
@ 2014-05-26  7:45 ` Ouyang Changchun
  2014-05-26  7:45 ` [dpdk-dev] [PATCH v2 3/3] testpmd: Add commands to test the functionality of setting TX rate for queue or VF Ouyang Changchun
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Ouyang Changchun @ 2014-05-26  7:45 UTC (permalink / raw)
  To: dev

This patch implements the functionality of setting TX rate for queue or VF in IXGBE PMD.

Signed-off-by: Ouyang Changchun <changchun.ouyang@intel.com>
---
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 122 ++++++++++++++++++++++++++++++++++++
 lib/librte_pmd_ixgbe/ixgbe_ethdev.h |  13 +++-
 2 files changed, 132 insertions(+), 3 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
index c9b5fe4..643477a 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
@@ -87,6 +87,8 @@
 #define IXGBE_LINK_UP_CHECK_TIMEOUT   1000 /* ms */
 #define IXGBE_VMDQ_NUM_UC_MAC         4096 /* Maximum nb. of UC MAC addr. */
 
+#define IXGBE_MMW_SIZE_DEFAULT        0x4
+#define IXGBE_MMW_SIZE_JUMBO_FRAME    0x14
 
 #define IXGBEVF_PMD_NAME "rte_ixgbevf_pmd" /* PMD name */
 
@@ -182,6 +184,10 @@ static int ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 static int ixgbe_mirror_rule_reset(struct rte_eth_dev *dev,
 		uint8_t	rule_id);
 
+static int ixgbe_set_queue_rate_limit(struct rte_eth_dev *dev,
+		uint16_t queue_idx, uint16_t tx_rate);
+static int ixgbe_set_vf_rate_limit(struct rte_eth_dev *dev, uint16_t vf,
+		uint16_t tx_rate, uint64_t q_msk);
 /*
  * Define VF Stats MACRO for Non "cleared on read" register
  */
@@ -280,6 +286,8 @@ static struct eth_dev_ops ixgbe_eth_dev_ops = {
 	.set_vf_rx            = ixgbe_set_pool_rx,
 	.set_vf_tx            = ixgbe_set_pool_tx,
 	.set_vf_vlan_filter   = ixgbe_set_pool_vlan_filter,
+	.set_queue_rate_limit = ixgbe_set_queue_rate_limit,
+	.set_vf_rate_limit    = ixgbe_set_vf_rate_limit,
 	.fdir_add_signature_filter    = ixgbe_fdir_add_signature_filter,
 	.fdir_update_signature_filter = ixgbe_fdir_update_signature_filter,
 	.fdir_remove_signature_filter = ixgbe_fdir_remove_signature_filter,
@@ -1288,10 +1296,13 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 {
 	struct ixgbe_hw *hw =
 		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct ixgbe_vf_info *vfinfo =
+		*IXGBE_DEV_PRIVATE_TO_P_VFDATA(dev->data->dev_private);
 	int err, link_up = 0, negotiate = 0;
 	uint32_t speed = 0;
 	int mask = 0;
 	int status;
+	uint16_t vf, idx;
 	
 	PMD_INIT_FUNC_TRACE();
 
@@ -1408,6 +1419,16 @@ skip_link_setup:
 			goto error;
 	}
 
+	/* Restore vf rate limit */
+	if (vfinfo != NULL) {
+		for (vf = 0; vf < dev->pci_dev->max_vfs; vf++)
+			for (idx = 0; idx < IXGBE_MAX_QUEUE_NUM_PER_VF; idx++)
+				if (vfinfo[vf].tx_rate[idx] != 0)
+					ixgbe_set_vf_rate_limit(dev, vf,
+						vfinfo[vf].tx_rate[idx],
+						1 << idx);
+	}
+
 	ixgbe_restore_statistics_mapping(dev);
 
 	return (0);
@@ -3062,6 +3083,107 @@ ixgbe_mirror_rule_reset(struct rte_eth_dev *dev, uint8_t rule_id)
 	return 0;
 }
 
+static int ixgbe_set_queue_rate_limit(struct rte_eth_dev *dev,
+	uint16_t queue_idx, uint16_t tx_rate)
+{
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t rf_dec, rf_int;
+	uint32_t bcnrc_val;
+	uint16_t link_speed = dev->data->dev_link.link_speed;
+
+	if (queue_idx >= hw->mac.max_tx_queues)
+		return -EINVAL;
+
+	if (tx_rate != 0) {
+		/* Calculate the rate factor values to set */
+		rf_int = (uint32_t)link_speed / (uint32_t)tx_rate;
+		rf_dec = (uint32_t)link_speed % (uint32_t)tx_rate;
+		rf_dec = (rf_dec << IXGBE_RTTBCNRC_RF_INT_SHIFT) / tx_rate;
+
+		bcnrc_val = IXGBE_RTTBCNRC_RS_ENA;
+		bcnrc_val |= ((rf_int << IXGBE_RTTBCNRC_RF_INT_SHIFT) &
+				IXGBE_RTTBCNRC_RF_INT_MASK_M);
+		bcnrc_val |= (rf_dec & IXGBE_RTTBCNRC_RF_DEC_MASK);
+	} else {
+		bcnrc_val = 0;
+	}
+
+	/*
+	 * Set global transmit compensation time to the MMW_SIZE in RTTBCNRM
+	 * register. MMW_SIZE=0x014 if 9728-byte jumbo is supported, otherwise
+	 * set as 0x4.
+	 */
+	if ((dev->data->dev_conf.rxmode.jumbo_frame == 1) &&
+		(dev->data->dev_conf.rxmode.max_rx_pkt_len >=
+				IXGBE_MAX_JUMBO_FRAME_SIZE))
+		IXGBE_WRITE_REG(hw, IXGBE_RTTBCNRM,
+			IXGBE_MMW_SIZE_JUMBO_FRAME);
+	else
+		IXGBE_WRITE_REG(hw, IXGBE_RTTBCNRM,
+			IXGBE_MMW_SIZE_DEFAULT);
+
+	/* Set RTTBCNRC of queue X */
+	IXGBE_WRITE_REG(hw, IXGBE_RTTDQSEL, queue_idx);
+	IXGBE_WRITE_REG(hw, IXGBE_RTTBCNRC, bcnrc_val);
+	IXGBE_WRITE_FLUSH(hw);
+
+	return 0;
+}
+
+static int ixgbe_set_vf_rate_limit(struct rte_eth_dev *dev, uint16_t vf,
+	uint16_t tx_rate, uint64_t q_msk)
+{
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct ixgbe_vf_info *vfinfo =
+		*(IXGBE_DEV_PRIVATE_TO_P_VFDATA(dev->data->dev_private));
+	uint8_t  nb_q_per_pool = RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool;
+	uint32_t queue_stride =
+		IXGBE_MAX_RX_QUEUE_NUM / RTE_ETH_DEV_SRIOV(dev).active;
+	uint32_t queue_idx = vf * queue_stride, idx = 0, vf_idx;
+	uint32_t queue_end = queue_idx + nb_q_per_pool - 1;
+	uint16_t total_rate = 0;
+
+	if (queue_end >= hw->mac.max_tx_queues)
+		return -EINVAL;
+
+	if (vfinfo != NULL) {
+		for (vf_idx = 0; vf_idx < dev->pci_dev->max_vfs; vf_idx++) {
+			if (vf_idx == vf)
+				continue;
+			for (idx = 0; idx < RTE_DIM(vfinfo[vf_idx].tx_rate);
+				idx++)
+				total_rate += vfinfo[vf_idx].tx_rate[idx];
+		}
+	} else
+		return -EINVAL;
+
+	/* Store tx_rate for this vf. */
+	for (idx = 0; idx < nb_q_per_pool; idx++) {
+		if (((uint64_t)0x1 << idx) & q_msk) {
+			if (vfinfo[vf].tx_rate[idx] != tx_rate)
+				vfinfo[vf].tx_rate[idx] = tx_rate;
+			total_rate += tx_rate;
+		}
+	}
+
+	if (total_rate > dev->data->dev_link.link_speed) {
+		/*
+		 * Reset stored TX rate of the VF if it causes exceed
+		 * link speed.
+		 */
+		memset(vfinfo[vf].tx_rate, 0, sizeof(vfinfo[vf].tx_rate));
+		return -EINVAL;
+	}
+
+	/* Set RTTBCNRC of each queue/pool for vf X  */
+	for (; queue_idx <= queue_end; queue_idx++) {
+		if (0x1 & q_msk)
+			ixgbe_set_queue_rate_limit(dev, queue_idx, tx_rate);
+		q_msk = q_msk >> 1;
+	}
+
+	return 0;
+}
 static struct rte_driver rte_ixgbe_driver = {
 	.type = PMD_PDEV,
 	.init = rte_ixgbe_pmd_init,
diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.h b/lib/librte_pmd_ixgbe/ixgbe_ethdev.h
index 9d7e93f..4d11105 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.h
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.h
@@ -64,9 +64,16 @@
 
 /* Loopback operation modes */
 /* 82599 specific loopback operation types */
-#define IXGBE_LPBK_82599_NONE		0x0 /* Default value. Loopback is disabled. */
-#define IXGBE_LPBK_82599_TX_RX		0x1 /* Tx->Rx loopback operation is enabled. */
+#define IXGBE_LPBK_82599_NONE   0x0 /* Default value. Loopback is disabled. */
+#define IXGBE_LPBK_82599_TX_RX  0x1 /* Tx->Rx loopback operation is enabled. */
 
+#define IXGBE_MAX_JUMBO_FRAME_SIZE      0x2600 /* Maximum Jumbo frame size. */
+
+#define IXGBE_RTTBCNRC_RF_INT_MASK_BASE 0x000003FF
+#define IXGBE_RTTBCNRC_RF_INT_MASK_M \
+	(IXGBE_RTTBCNRC_RF_INT_MASK_BASE << IXGBE_RTTBCNRC_RF_INT_SHIFT)
+
+#define IXGBE_MAX_QUEUE_NUM_PER_VF  8
 /*
  * Information about the fdir mode.
  */
@@ -125,7 +132,7 @@ struct ixgbe_vf_info {
 	uint16_t default_vf_vlan_id;
 	uint16_t vlans_enabled;
 	bool clear_to_send;
-	uint16_t tx_rate;
+	uint16_t tx_rate[IXGBE_MAX_QUEUE_NUM_PER_VF];
 	uint16_t vlan_count;
 	uint8_t spoofchk_enabled;
 };
-- 
1.9.0

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

* [dpdk-dev] [PATCH v2 3/3] testpmd: Add commands to test the functionality of setting TX rate for queue or VF
  2014-05-26  7:45 [dpdk-dev] [PATCH v2 0/3] Support setting TX rate for queue and VF Ouyang Changchun
  2014-05-26  7:45 ` [dpdk-dev] [PATCH v2 1/3] ether: Add API to support " Ouyang Changchun
  2014-05-26  7:45 ` [dpdk-dev] [PATCH v2 2/3] ixgbe: Implement the functionality of setting TX rate for queue or VF in IXGBE PMD Ouyang Changchun
@ 2014-05-26  7:45 ` Ouyang Changchun
  2014-05-26 22:52 ` [dpdk-dev] [PATCH v2 0/3] Support setting TX rate for queue and VF Neil Horman
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Ouyang Changchun @ 2014-05-26  7:45 UTC (permalink / raw)
  To: dev

This patch adds commands in testpmd to test the functionality of setting TX rate for queue or VF.

Signed-off-by: Ouyang Changchun <changchun.ouyang@intel.com>
---
 app/test-pmd/cmdline.c | 159 ++++++++++++++++++++++++++++++++++++++++++++++++-
 app/test-pmd/config.c  |  47 +++++++++++++++
 app/test-pmd/testpmd.h |   3 +
 3 files changed, 208 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index b3824f9..83b2665 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -342,7 +342,14 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"    BAM:accepts broadcast packets;"
 			"MPE:accepts all multicast packets\n\n"
 			"    Enable/Disable a VF receive mode of a port\n\n"
-			
+
+			"set port (port_id) queue (queue_id) rate (rate_num)\n"
+			"    Set rate limit for a queue of a port\n\n"
+
+			"set port (port_id) vf (vf_id) rate (rate_num) "
+			"queue_mask (queue_mask_value)\n"
+			"    Set rate limit for queues in VF of a port\n\n"
+
 			"set port (port_id) mirror-rule (rule_id)" 
 			"(pool-mirror|vlan-mirror)\n"
 			" (poolmask|vlanid[,vlanid]*) dst-pool (pool_id) (on|off)\n"
@@ -4790,6 +4797,154 @@ cmdline_parse_inst_t cmd_vf_rxvlan_filter = {
 	},
 };
 
+/* *** SET RATE LIMIT FOR A QUEUE OF A PORT *** */
+struct cmd_queue_rate_limit_result {
+	cmdline_fixed_string_t set;
+	cmdline_fixed_string_t port;
+	uint8_t port_num;
+	cmdline_fixed_string_t queue;
+	uint8_t queue_num;
+	cmdline_fixed_string_t rate;
+	uint16_t rate_num;
+};
+
+static void cmd_queue_rate_limit_parsed(void *parsed_result,
+		__attribute__((unused)) struct cmdline *cl,
+		__attribute__((unused)) void *data)
+{
+	struct cmd_queue_rate_limit_result *res = parsed_result;
+	int ret = 0;
+
+	if ((strcmp(res->set, "set") == 0) && (strcmp(res->port, "port") == 0)
+		&& (strcmp(res->queue, "queue") == 0)
+		&& (strcmp(res->rate, "rate") == 0))
+		ret = set_queue_rate_limit(res->port_num, res->queue_num,
+					res->rate_num);
+	if (ret < 0)
+		printf("queue_rate_limit_cmd error: (%s)\n", strerror(-ret));
+
+}
+
+cmdline_parse_token_string_t cmd_queue_rate_limit_set =
+	TOKEN_STRING_INITIALIZER(struct cmd_queue_rate_limit_result,
+				set, "set");
+cmdline_parse_token_string_t cmd_queue_rate_limit_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_queue_rate_limit_result,
+				port, "port");
+cmdline_parse_token_num_t cmd_queue_rate_limit_portnum =
+	TOKEN_NUM_INITIALIZER(struct cmd_queue_rate_limit_result,
+				port_num, UINT8);
+cmdline_parse_token_string_t cmd_queue_rate_limit_queue =
+	TOKEN_STRING_INITIALIZER(struct cmd_queue_rate_limit_result,
+				queue, "queue");
+cmdline_parse_token_num_t cmd_queue_rate_limit_queuenum =
+	TOKEN_NUM_INITIALIZER(struct cmd_queue_rate_limit_result,
+				queue_num, UINT8);
+cmdline_parse_token_string_t cmd_queue_rate_limit_rate =
+	TOKEN_STRING_INITIALIZER(struct cmd_queue_rate_limit_result,
+				rate, "rate");
+cmdline_parse_token_num_t cmd_queue_rate_limit_ratenum =
+	TOKEN_NUM_INITIALIZER(struct cmd_queue_rate_limit_result,
+				rate_num, UINT16);
+
+cmdline_parse_inst_t cmd_queue_rate_limit = {
+	.f = cmd_queue_rate_limit_parsed,
+	.data = (void *)0,
+	.help_str = "set port X queue Y rate Z:(X = port number,"
+	"Y = queue number,Z = rate number)set rate limit for a queue on port X",
+	.tokens = {
+		(void *)&cmd_queue_rate_limit_set,
+		(void *)&cmd_queue_rate_limit_port,
+		(void *)&cmd_queue_rate_limit_portnum,
+		(void *)&cmd_queue_rate_limit_queue,
+		(void *)&cmd_queue_rate_limit_queuenum,
+		(void *)&cmd_queue_rate_limit_rate,
+		(void *)&cmd_queue_rate_limit_ratenum,
+		NULL,
+	},
+};
+
+
+/* *** SET RATE LIMIT FOR A VF OF A PORT *** */
+struct cmd_vf_rate_limit_result {
+	cmdline_fixed_string_t set;
+	cmdline_fixed_string_t port;
+	uint8_t port_num;
+	cmdline_fixed_string_t vf;
+	uint8_t vf_num;
+	cmdline_fixed_string_t rate;
+	uint16_t rate_num;
+	cmdline_fixed_string_t q_msk;
+	uint64_t q_msk_val;
+};
+
+static void cmd_vf_rate_limit_parsed(void *parsed_result,
+		__attribute__((unused)) struct cmdline *cl,
+		__attribute__((unused)) void *data)
+{
+	struct cmd_vf_rate_limit_result *res = parsed_result;
+	int ret = 0;
+
+	if ((strcmp(res->set, "set") == 0) && (strcmp(res->port, "port") == 0)
+		&& (strcmp(res->vf, "vf") == 0)
+		&& (strcmp(res->rate, "rate") == 0)
+		&& (strcmp(res->q_msk, "queue_mask") == 0))
+		ret = set_vf_rate_limit(res->port_num, res->vf_num,
+					res->rate_num, res->q_msk_val);
+	if (ret < 0)
+		printf("vf_rate_limit_cmd error: (%s)\n", strerror(-ret));
+
+}
+
+cmdline_parse_token_string_t cmd_vf_rate_limit_set =
+	TOKEN_STRING_INITIALIZER(struct cmd_vf_rate_limit_result,
+				set, "set");
+cmdline_parse_token_string_t cmd_vf_rate_limit_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_vf_rate_limit_result,
+				port, "port");
+cmdline_parse_token_num_t cmd_vf_rate_limit_portnum =
+	TOKEN_NUM_INITIALIZER(struct cmd_vf_rate_limit_result,
+				port_num, UINT8);
+cmdline_parse_token_string_t cmd_vf_rate_limit_vf =
+	TOKEN_STRING_INITIALIZER(struct cmd_vf_rate_limit_result,
+				vf, "vf");
+cmdline_parse_token_num_t cmd_vf_rate_limit_vfnum =
+	TOKEN_NUM_INITIALIZER(struct cmd_vf_rate_limit_result,
+				vf_num, UINT8);
+cmdline_parse_token_string_t cmd_vf_rate_limit_rate =
+	TOKEN_STRING_INITIALIZER(struct cmd_vf_rate_limit_result,
+				rate, "rate");
+cmdline_parse_token_num_t cmd_vf_rate_limit_ratenum =
+	TOKEN_NUM_INITIALIZER(struct cmd_vf_rate_limit_result,
+				rate_num, UINT16);
+cmdline_parse_token_string_t cmd_vf_rate_limit_q_msk =
+	TOKEN_STRING_INITIALIZER(struct cmd_vf_rate_limit_result,
+				q_msk, "queue_mask");
+cmdline_parse_token_num_t cmd_vf_rate_limit_q_msk_val =
+	TOKEN_NUM_INITIALIZER(struct cmd_vf_rate_limit_result,
+				q_msk_val, UINT64);
+
+cmdline_parse_inst_t cmd_vf_rate_limit = {
+	.f = cmd_vf_rate_limit_parsed,
+	.data = (void *)0,
+	.help_str = "set port X vf Y rate Z queue_mask V:(X = port number,"
+	"Y = VF number,Z = rate number, V = queue mask value)set rate limit "
+	"for queues of VF on port X",
+	.tokens = {
+		(void *)&cmd_vf_rate_limit_set,
+		(void *)&cmd_vf_rate_limit_port,
+		(void *)&cmd_vf_rate_limit_portnum,
+		(void *)&cmd_vf_rate_limit_vf,
+		(void *)&cmd_vf_rate_limit_vfnum,
+		(void *)&cmd_vf_rate_limit_rate,
+		(void *)&cmd_vf_rate_limit_ratenum,
+		(void *)&cmd_vf_rate_limit_q_msk,
+		(void *)&cmd_vf_rate_limit_q_msk_val,
+		NULL,
+	},
+};
+
+
 /* *** CONFIGURE VM MIRROR VLAN/POOL RULE *** */
 struct cmd_set_mirror_mask_result {
 	cmdline_fixed_string_t set;
@@ -5230,6 +5385,8 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_vf_mac_addr_filter ,
 	(cmdline_parse_inst_t *)&cmd_set_vf_traffic,
 	(cmdline_parse_inst_t *)&cmd_vf_rxvlan_filter,
+	(cmdline_parse_inst_t *)&cmd_queue_rate_limit,
+	(cmdline_parse_inst_t *)&cmd_vf_rate_limit,
 	(cmdline_parse_inst_t *)&cmd_set_mirror_mask,
 	(cmdline_parse_inst_t *)&cmd_set_mirror_link,
 	(cmdline_parse_inst_t *)&cmd_reset_mirror_rule,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 20ad0a8..002bc50 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1731,3 +1731,50 @@ set_vf_rx_vlan(portid_t port_id, uint16_t vlan_id, uint64_t vf_mask, uint8_t on)
 	       "diag=%d\n", port_id, diag);
 }
 
+int
+set_queue_rate_limit(portid_t port_id, uint16_t queue_idx, uint16_t rate)
+{
+	int diag;
+	struct rte_eth_link link;
+
+	if (port_id_is_invalid(port_id))
+		return 1;
+	rte_eth_link_get_nowait(port_id, &link);
+	if (rate > link.link_speed) {
+		printf("Invalid rate value:%u bigger than link speed: %u\n",
+			rate, link.link_speed);
+		return 1;
+	}
+	diag = rte_eth_set_queue_rate_limit(port_id, queue_idx, rate);
+	if (diag == 0)
+		return diag;
+	printf("rte_eth_set_queue_rate_limit for port_id=%d failed diag=%d\n",
+		port_id, diag);
+	return diag;
+}
+
+int
+set_vf_rate_limit(portid_t port_id, uint16_t vf, uint16_t rate, uint64_t q_msk)
+{
+	int diag;
+	struct rte_eth_link link;
+
+	if (q_msk == 0)
+		return 0;
+
+	if (port_id_is_invalid(port_id))
+		return 1;
+	rte_eth_link_get_nowait(port_id, &link);
+	if (rate > link.link_speed) {
+		printf("Invalid rate value:%u bigger than link speed: %u\n",
+			rate, link.link_speed);
+		return 1;
+	}
+	diag = rte_eth_set_vf_rate_limit(port_id, vf, rate, q_msk);
+	if (diag == 0)
+		return diag;
+	printf("rte_eth_set_vf_rate_limit for port_id=%d failed diag=%d\n",
+		port_id, diag);
+	return diag;
+}
+
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 2bdb1a2..f3f8bc2 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -528,6 +528,9 @@ void set_vf_traffic(portid_t port_id, uint8_t is_rx, uint16_t vf, uint8_t on);
 void set_vf_rx_vlan(portid_t port_id, uint16_t vlan_id, 
 		uint64_t vf_mask, uint8_t on);
 
+int set_queue_rate_limit(portid_t port_id, uint16_t queue_idx, uint16_t rate);
+int set_vf_rate_limit(portid_t port_id, uint16_t vf, uint16_t rate,
+				uint64_t q_msk);
 /*
  * Work-around of a compilation error with ICC on invocations of the
  * rte_be_to_cpu_16() function.
-- 
1.9.0

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

* Re: [dpdk-dev] [PATCH v2 0/3] Support setting TX rate for queue and VF
  2014-05-26  7:45 [dpdk-dev] [PATCH v2 0/3] Support setting TX rate for queue and VF Ouyang Changchun
                   ` (2 preceding siblings ...)
  2014-05-26  7:45 ` [dpdk-dev] [PATCH v2 3/3] testpmd: Add commands to test the functionality of setting TX rate for queue or VF Ouyang Changchun
@ 2014-05-26 22:52 ` Neil Horman
  2014-06-05  3:11   ` Ouyang, Changchun
       [not found] ` <F52918179C57134FAEC9EA62FA2F9625117C89F0@shsmsx102.ccr.corp.intel.com>
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Neil Horman @ 2014-05-26 22:52 UTC (permalink / raw)
  To: Ouyang Changchun; +Cc: dev

On Mon, May 26, 2014 at 03:45:28PM +0800, Ouyang Changchun wrote:
> This patch v2 fixes some errors and warnings reported by checkpatch.pl.
>  
> This patch series also contain the 3 items:
> 1. Add API to support setting TX rate for a queue or a VF.
> 2. Implement the functionality of setting TX rate for queue or VF in IXGBE PMD.
> 3. Add commands in testpmd to test the functionality of setting TX rate for queue or VF.
> 
> Ouyang Changchun (3):
>   Add API to support set TX rate for a queue and VF.
>   Implement the functionality of setting TX rate for queue or VF in
>     IXGBE PMD.
>   Add commands in testpmd to test the functionality of setting TX rate
>     for queue or VF.
> 
>  app/test-pmd/cmdline.c              | 159 +++++++++++++++++++++++++++++++++++-
>  app/test-pmd/config.c               |  47 +++++++++++
>  app/test-pmd/testpmd.h              |   3 +
>  lib/librte_ether/rte_ethdev.c       |  71 ++++++++++++++++
>  lib/librte_ether/rte_ethdev.h       |  51 ++++++++++++
>  lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 122 +++++++++++++++++++++++++++
>  lib/librte_pmd_ixgbe/ixgbe_ethdev.h |  13 ++-
>  7 files changed, 462 insertions(+), 4 deletions(-)
> 
This seems a bit backwards.  queue rate limiting is rather a generic function,
that doesn't really need to know any details about the hardware, save for its
maximum tx rate, but you're implementaiton requires that it be re-implemented
for each PMD.  Why not just export max tx rates from the PMD and write a generic
queuing libarary to do rate limitation for any PMD?

Neil

> -- 
> 1.9.0
> 
> 

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

* Re: [dpdk-dev] [PATCH v2 1/3] ether: Add API to support setting TX rate for queue and VF
  2014-05-26  7:45 ` [dpdk-dev] [PATCH v2 1/3] ether: Add API to support " Ouyang Changchun
@ 2014-05-27 22:47   ` Thomas Monjalon
  2014-06-05  3:30     ` Ouyang, Changchun
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Monjalon @ 2014-05-27 22:47 UTC (permalink / raw)
  To: Ouyang Changchun; +Cc: dev

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

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

* Re: [dpdk-dev] [PATCH v2 0/3] Support setting TX rate for queue and VF
  2014-05-26 22:52 ` [dpdk-dev] [PATCH v2 0/3] Support setting TX rate for queue and VF Neil Horman
@ 2014-06-05  3:11   ` Ouyang, Changchun
  2014-06-05 11:01     ` Neil Horman
  0 siblings, 1 reply; 14+ messages in thread
From: Ouyang, Changchun @ 2014-06-05  3:11 UTC (permalink / raw)
  To: Neil Horman; +Cc: dev

Hi, Neil

" but you're implementaiton requires that it be re-implemented for each PMD "

[Changchun]: Different PMD(corresponding diff NIC) has different register to set.
It makes sense to me has different implementation to support limit tx rate.
 
" Why not just export max tx rates from the PMD and write a generic queuing libarary to do rate limitation for any PMD?"

[Changchun]: Just export max tx rate is not enough for customer.
I think if we can leverage HW feature to do it, why need a more complicated lib to do so? 

Thanks
Changchun

-----Original Message-----
From: Neil Horman [mailto:nhorman@tuxdriver.com] 
Sent: Tuesday, May 27, 2014 6:53 AM
To: Ouyang, Changchun
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2 0/3] Support setting TX rate for queue and VF

On Mon, May 26, 2014 at 03:45:28PM +0800, Ouyang Changchun wrote:
> This patch v2 fixes some errors and warnings reported by checkpatch.pl.
>  
> This patch series also contain the 3 items:
> 1. Add API to support setting TX rate for a queue or a VF.
> 2. Implement the functionality of setting TX rate for queue or VF in IXGBE PMD.
> 3. Add commands in testpmd to test the functionality of setting TX rate for queue or VF.
> 
> Ouyang Changchun (3):
>   Add API to support set TX rate for a queue and VF.
>   Implement the functionality of setting TX rate for queue or VF in
>     IXGBE PMD.
>   Add commands in testpmd to test the functionality of setting TX rate
>     for queue or VF.
> 
>  app/test-pmd/cmdline.c              | 159 +++++++++++++++++++++++++++++++++++-
>  app/test-pmd/config.c               |  47 +++++++++++
>  app/test-pmd/testpmd.h              |   3 +
>  lib/librte_ether/rte_ethdev.c       |  71 ++++++++++++++++
>  lib/librte_ether/rte_ethdev.h       |  51 ++++++++++++
>  lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 122 +++++++++++++++++++++++++++  
> lib/librte_pmd_ixgbe/ixgbe_ethdev.h |  13 ++-
>  7 files changed, 462 insertions(+), 4 deletions(-)
> 
This seems a bit backwards.  queue rate limiting is rather a generic function, that doesn't really need to know any details about the hardware, save for its maximum tx rate, but you're implementaiton requires that it be re-implemented for each PMD.  Why not just export max tx rates from the PMD and write a generic queuing libarary to do rate limitation for any PMD?

Neil

> --
> 1.9.0
> 
> 

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

* Re: [dpdk-dev] [PATCH v2 1/3] ether: Add API to support setting TX rate for queue and VF
  2014-05-27 22:47   ` Thomas Monjalon
@ 2014-06-05  3:30     ` Ouyang, Changchun
  2014-06-10 23:02       ` Thomas Monjalon
  0 siblings, 1 reply; 14+ messages in thread
From: Ouyang, Changchun @ 2014-06-05  3:30 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

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.monjalon@6wind.com] 
Sent: Wednesday, May 28, 2014 6:48 AM
To: Ouyang, Changchun
Cc: dev@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

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

* Re: [dpdk-dev] [PATCH v2 0/3] Support setting TX rate for queue and VF
       [not found] ` <F52918179C57134FAEC9EA62FA2F9625117C89F0@shsmsx102.ccr.corp.intel.com>
@ 2014-06-05  5:32   ` Liu, Jijiang
  0 siblings, 0 replies; 14+ messages in thread
From: Liu, Jijiang @ 2014-06-05  5:32 UTC (permalink / raw)
  To: Ouyang, Changchun, dev

Acked-by: Jijiang Liu <jijiang.liu@intel.com>

-----Original Message-----
From: Ouyang, Changchun
Sent: Monday, May 26, 2014 3:45 PM
To: dev@dpdk.org
Cc: Ouyang, Changchun
Subject: [PATCH v2 0/3] Support setting TX rate for queue and VF

This patch v2 fixes some errors and warnings reported by checkpatch.pl.
 
This patch series also contain the 3 items:
1. Add API to support setting TX rate for a queue or a VF.
2. Implement the functionality of setting TX rate for queue or VF in IXGBE PMD.
3. Add commands in testpmd to test the functionality of setting TX rate for queue or VF.

Ouyang Changchun (3):
  Add API to support set TX rate for a queue and VF.
  Implement the functionality of setting TX rate for queue or VF in
    IXGBE PMD.
  Add commands in testpmd to test the functionality of setting TX rate
    for queue or VF.

 app/test-pmd/cmdline.c              | 159 +++++++++++++++++++++++++++++++++++-
 app/test-pmd/config.c               |  47 +++++++++++
 app/test-pmd/testpmd.h              |   3 +
 lib/librte_ether/rte_ethdev.c       |  71 ++++++++++++++++
 lib/librte_ether/rte_ethdev.h       |  51 ++++++++++++
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 122 +++++++++++++++++++++++++++  lib/librte_pmd_ixgbe/ixgbe_ethdev.h |  13 ++-
 7 files changed, 462 insertions(+), 4 deletions(-)

--
1.9.0

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

* Re: [dpdk-dev] [PATCH v2 0/3] Support setting TX rate for queue and VF
  2014-06-05  3:11   ` Ouyang, Changchun
@ 2014-06-05 11:01     ` Neil Horman
  0 siblings, 0 replies; 14+ messages in thread
From: Neil Horman @ 2014-06-05 11:01 UTC (permalink / raw)
  To: Ouyang, Changchun; +Cc: dev

On Thu, Jun 05, 2014 at 03:11:46AM +0000, Ouyang, Changchun wrote:
> Hi, Neil
> 
> " but you're implementaiton requires that it be re-implemented for each PMD "
> 
> [Changchun]: Different PMD(corresponding diff NIC) has different register to set.
> It makes sense to me has different implementation to support limit tx rate.
>  
> " Why not just export max tx rates from the PMD and write a generic queuing libarary to do rate limitation for any PMD?"
> 
> [Changchun]: Just export max tx rate is not enough for customer.
> I think if we can leverage HW feature to do it, why need a more complicated lib to do so? 
> 
> Thanks
> Changchun
> 
sorry, I reread my initial comment and see I wasn't being at all clear.  You 
actually do have a nice generic layer for applications to call through in 
rte_ethdev, which is good.  What you don't have is a backoff mechanism.  That is 
to say the return of ENOSUP in rte_eth_dev_set_queue_rate_limit means that your
queuing library doesnt' work for any device that doesn't support hardware rate
limiting (implying none of the virtual devs get to implement this).  Don't you
think you should implement a generic software queue rate limit layer for the
myrriad of devices that cant' do what ixgbe can?

Regards
Neil
 

> -----Original Message-----
> From: Neil Horman [mailto:nhorman@tuxdriver.com] 
> Sent: Tuesday, May 27, 2014 6:53 AM
> To: Ouyang, Changchun
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 0/3] Support setting TX rate for queue and VF
> 
> On Mon, May 26, 2014 at 03:45:28PM +0800, Ouyang Changchun wrote:
> > This patch v2 fixes some errors and warnings reported by checkpatch.pl.
> >  
> > This patch series also contain the 3 items:
> > 1. Add API to support setting TX rate for a queue or a VF.
> > 2. Implement the functionality of setting TX rate for queue or VF in IXGBE PMD.
> > 3. Add commands in testpmd to test the functionality of setting TX rate for queue or VF.
> > 
> > Ouyang Changchun (3):
> >   Add API to support set TX rate for a queue and VF.
> >   Implement the functionality of setting TX rate for queue or VF in
> >     IXGBE PMD.
> >   Add commands in testpmd to test the functionality of setting TX rate
> >     for queue or VF.
> > 
> >  app/test-pmd/cmdline.c              | 159 +++++++++++++++++++++++++++++++++++-
> >  app/test-pmd/config.c               |  47 +++++++++++
> >  app/test-pmd/testpmd.h              |   3 +
> >  lib/librte_ether/rte_ethdev.c       |  71 ++++++++++++++++
> >  lib/librte_ether/rte_ethdev.h       |  51 ++++++++++++
> >  lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 122 +++++++++++++++++++++++++++  
> > lib/librte_pmd_ixgbe/ixgbe_ethdev.h |  13 ++-
> >  7 files changed, 462 insertions(+), 4 deletions(-)
> > 
> This seems a bit backwards.  queue rate limiting is rather a generic function, that doesn't really need to know any details about the hardware, save for its maximum tx rate, but you're implementaiton requires that it be re-implemented for each PMD.  Why not just export max tx rates from the PMD and write a generic queuing libarary to do rate limitation for any PMD?
> 
> Neil
> 
> > --
> > 1.9.0
> > 
> > 
> 

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

* Re: [dpdk-dev] [PATCH v2 0/3] Support setting TX rate for queue and VF
  2014-05-26  7:45 [dpdk-dev] [PATCH v2 0/3] Support setting TX rate for queue and VF Ouyang Changchun
                   ` (4 preceding siblings ...)
       [not found] ` <F52918179C57134FAEC9EA62FA2F9625117C89F0@shsmsx102.ccr.corp.intel.com>
@ 2014-06-05 12:41 ` Cao, Waterman
  2014-06-06  6:52 ` Xie, Huawei
  6 siblings, 0 replies; 14+ messages in thread
From: Cao, Waterman @ 2014-06-05 12:41 UTC (permalink / raw)
  To: Ouyang, Changchun, dev

Tested-by: Waterman Cao <waterman.cao at intel.com>

This patch bug fix has been tested by Intel.
Please see information as the following:
Fedora 20 x86_64, Linux Kernel 3.13.9-200, GCC 4.8.2
Intel Xeon CPU E5-2680 v2 @ 2.80GHz
NIC: Intel Niantic 82599, Intel i350, Intel 82580 and Intel 82576

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

* Re: [dpdk-dev] [PATCH v2 0/3] Support setting TX rate for queue and VF
  2014-05-26  7:45 [dpdk-dev] [PATCH v2 0/3] Support setting TX rate for queue and VF Ouyang Changchun
                   ` (5 preceding siblings ...)
  2014-06-05 12:41 ` Cao, Waterman
@ 2014-06-06  6:52 ` Xie, Huawei
  2014-06-11 14:05   ` Thomas Monjalon
  6 siblings, 1 reply; 14+ messages in thread
From: Xie, Huawei @ 2014-06-06  6:52 UTC (permalink / raw)
  To: Ouyang, Changchun, dev

Acked-by: Huawei Xie <huawei.xie@intel.com>

-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ouyang Changchun
Sent: Monday, May 26, 2014 3:45 PM
To: dev@dpdk.org
Subject: [dpdk-dev] [PATCH v2 0/3] Support setting TX rate for queue and VF

This patch v2 fixes some errors and warnings reported by checkpatch.pl.
 
This patch series also contain the 3 items:
1. Add API to support setting TX rate for a queue or a VF.
2. Implement the functionality of setting TX rate for queue or VF in IXGBE PMD.
3. Add commands in testpmd to test the functionality of setting TX rate for queue or VF.

Ouyang Changchun (3):
  Add API to support set TX rate for a queue and VF.
  Implement the functionality of setting TX rate for queue or VF in
    IXGBE PMD.
  Add commands in testpmd to test the functionality of setting TX rate
    for queue or VF.

 app/test-pmd/cmdline.c              | 159 +++++++++++++++++++++++++++++++++++-
 app/test-pmd/config.c               |  47 +++++++++++
 app/test-pmd/testpmd.h              |   3 +
 lib/librte_ether/rte_ethdev.c       |  71 ++++++++++++++++
 lib/librte_ether/rte_ethdev.h       |  51 ++++++++++++
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 122 +++++++++++++++++++++++++++  lib/librte_pmd_ixgbe/ixgbe_ethdev.h |  13 ++-
 7 files changed, 462 insertions(+), 4 deletions(-)

--
1.9.0

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

* Re: [dpdk-dev] [PATCH v2 1/3] ether: Add API to support setting TX rate for queue and VF
  2014-06-05  3:30     ` Ouyang, Changchun
@ 2014-06-10 23:02       ` Thomas Monjalon
  0 siblings, 0 replies; 14+ messages in thread
From: Thomas Monjalon @ 2014-06-10 23:02 UTC (permalink / raw)
  To: Ouyang, Changchun; +Cc: dev

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

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

* Re: [dpdk-dev] [PATCH v2 0/3] Support setting TX rate for queue and VF
  2014-06-06  6:52 ` Xie, Huawei
@ 2014-06-11 14:05   ` Thomas Monjalon
  0 siblings, 0 replies; 14+ messages in thread
From: Thomas Monjalon @ 2014-06-11 14:05 UTC (permalink / raw)
  To: Ouyang, Changchun; +Cc: dev

> This patch series also contain the 3 items:
> 1. Add API to support setting TX rate for a queue or a VF.
> 2. Implement the functionality of setting TX rate for queue or VF in IXGBE PMD.
> 3. Add commands in testpmd to test the functionality of setting TX rate for queue or VF.

Acked-by: Jijiang Liu <jijiang.liu@intel.com>
Tested-by: Waterman Cao <waterman.cao@intel.com>
Acked-by: Huawei Xie <huawei.xie@intel.com>

Applied for version 1.7.0.

Thanks
-- 
Thomas

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

end of thread, other threads:[~2014-06-11 14:05 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-26  7:45 [dpdk-dev] [PATCH v2 0/3] Support setting TX rate for queue and VF Ouyang Changchun
2014-05-26  7:45 ` [dpdk-dev] [PATCH v2 1/3] ether: Add API to support " Ouyang Changchun
2014-05-27 22:47   ` Thomas Monjalon
2014-06-05  3:30     ` Ouyang, Changchun
2014-06-10 23:02       ` Thomas Monjalon
2014-05-26  7:45 ` [dpdk-dev] [PATCH v2 2/3] ixgbe: Implement the functionality of setting TX rate for queue or VF in IXGBE PMD Ouyang Changchun
2014-05-26  7:45 ` [dpdk-dev] [PATCH v2 3/3] testpmd: Add commands to test the functionality of setting TX rate for queue or VF Ouyang Changchun
2014-05-26 22:52 ` [dpdk-dev] [PATCH v2 0/3] Support setting TX rate for queue and VF Neil Horman
2014-06-05  3:11   ` Ouyang, Changchun
2014-06-05 11:01     ` Neil Horman
     [not found] ` <F52918179C57134FAEC9EA62FA2F9625117C89F0@shsmsx102.ccr.corp.intel.com>
2014-06-05  5:32   ` Liu, Jijiang
2014-06-05 12:41 ` Cao, Waterman
2014-06-06  6:52 ` Xie, Huawei
2014-06-11 14:05   ` Thomas Monjalon

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