DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/4] RSS enhancement on Intel x550 NIC
@ 2015-09-28  7:52 Wenzhuo Lu
  2015-09-28  7:52 ` [dpdk-dev] [PATCH 1/4] ixgbe: 512 entries RSS table on x550 Wenzhuo Lu
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Wenzhuo Lu @ 2015-09-28  7:52 UTC (permalink / raw)
  To: dev

This patch set implements the RSS enhancement on x550.
The enhancement includes, the PF RSS redirection table is enlarged
from 128 entries to 512 entries, the VF doesn't share the same
registers with PF and per VF RSS redirection table is provided.

Wenzhuo Lu (4):
  ixgbe: 512 entries RSS table on x550
  ixgbe: VF RSS config on x550
  ixgbe: VF RSS hash query and update
  ixgbe: VF RSS reta query and update

 drivers/net/ixgbe/ixgbe_ethdev.c | 211 +++++++++++++++++++++++++++++++++---
 drivers/net/ixgbe/ixgbe_ethdev.h |   6 ++
 drivers/net/ixgbe/ixgbe_rxtx.c   | 224 +++++++++++++++++++++++++++++++++++++--
 3 files changed, 423 insertions(+), 18 deletions(-)

-- 
1.9.3

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

* [dpdk-dev] [PATCH 1/4] ixgbe: 512 entries RSS table on x550
  2015-09-28  7:52 [dpdk-dev] [PATCH 0/4] RSS enhancement on Intel x550 NIC Wenzhuo Lu
@ 2015-09-28  7:52 ` Wenzhuo Lu
  2015-10-15 22:19   ` Ananyev, Konstantin
  2015-09-28  7:52 ` [dpdk-dev] [PATCH 2/4] ixgbe: VF RSS config " Wenzhuo Lu
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Wenzhuo Lu @ 2015-09-28  7:52 UTC (permalink / raw)
  To: dev

Comparing with the older NICs, x550's RSS redirection table is enlarged to 512
entries. As the original code is for the NICs which have a 128 entries RSS table,
it means only part of the RSS table is set on x550. So, RSS cannot work as
expected on x550, it doesn't redirect the packets evenly.
This patch configs the entries beyond 128 on x550 to let RSS work well, and also
update the query and update functions to support 512 entries.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 106 ++++++++++++++++++++++++++++++++++-----
 drivers/net/ixgbe/ixgbe_rxtx.c   |  18 +++++--
 2 files changed, 109 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index ec2918c..a1ef26f 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -2397,7 +2397,17 @@ ixgbe_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 				ETH_TXQ_FLAGS_NOOFFLOADS,
 	};
 	dev_info->hash_key_size = IXGBE_HKEY_MAX_INDEX * sizeof(uint32_t);
-	dev_info->reta_size = ETH_RSS_RETA_SIZE_128;
+
+	switch (hw->mac.type) {
+	case ixgbe_mac_X550:
+	case ixgbe_mac_X550EM_x:
+		dev_info->reta_size = ETH_RSS_RETA_SIZE_512;
+		break;
+	default:
+		dev_info->reta_size = ETH_RSS_RETA_SIZE_128;
+		break;
+	}
+
 	dev_info->flow_type_rss_offloads = IXGBE_RSS_OFFLOAD_ALL;
 }
 
@@ -3205,14 +3215,29 @@ ixgbe_dev_rss_reta_update(struct rte_eth_dev *dev,
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
 	PMD_INIT_FUNC_TRACE();
-	if (reta_size != ETH_RSS_RETA_SIZE_128) {
-		PMD_DRV_LOG(ERR, "The size of hash lookup table configured "
-			"(%d) doesn't match the number hardware can supported "
-			"(%d)\n", reta_size, ETH_RSS_RETA_SIZE_128);
-		return -EINVAL;
+	switch (hw->mac.type) {
+	case ixgbe_mac_X550:
+	case ixgbe_mac_X550EM_x:
+		if (reta_size != ETH_RSS_RETA_SIZE_512) {
+			PMD_DRV_LOG(ERR, "The size of hash lookup table "
+					"configured (%d) doesn't match the "
+					"number hardware can supported (%d)\n",
+					reta_size, ETH_RSS_RETA_SIZE_512);
+			return -EINVAL;
+		}
+		break;
+	default:
+		if (reta_size != ETH_RSS_RETA_SIZE_128) {
+			PMD_DRV_LOG(ERR, "The size of hash lookup table "
+					"configured (%d) doesn't match the "
+					"number hardware can supported (%d)\n",
+					reta_size, ETH_RSS_RETA_SIZE_128);
+			return -EINVAL;
+		}
+		break;
 	}
 
-	for (i = 0; i < reta_size; i += IXGBE_4_BIT_WIDTH) {
+	for (i = 0; i < ETH_RSS_RETA_SIZE_128; i += IXGBE_4_BIT_WIDTH) {
 		idx = i / RTE_RETA_GROUP_SIZE;
 		shift = i % RTE_RETA_GROUP_SIZE;
 		mask = (uint8_t)((reta_conf[idx].mask >> shift) &
@@ -3234,6 +3259,30 @@ ixgbe_dev_rss_reta_update(struct rte_eth_dev *dev,
 		IXGBE_WRITE_REG(hw, IXGBE_RETA(i >> 2), reta);
 	}
 
+	for (i = ETH_RSS_RETA_SIZE_128; i < reta_size; i += IXGBE_4_BIT_WIDTH) {
+		idx = i / RTE_RETA_GROUP_SIZE;
+		shift = i % RTE_RETA_GROUP_SIZE;
+		mask = (uint8_t)((reta_conf[idx].mask >> shift) &
+						IXGBE_4_BIT_MASK);
+		if (!mask)
+			continue;
+		if (mask == IXGBE_4_BIT_MASK)
+			r = 0;
+		else
+			r = IXGBE_READ_REG(hw,
+				IXGBE_ERETA((i - ETH_RSS_RETA_SIZE_128) >> 2));
+		for (j = 0, reta = 0; j < IXGBE_4_BIT_WIDTH; j++) {
+			if (mask & (0x1 << j))
+				reta |= reta_conf[idx].reta[shift + j] <<
+							(CHAR_BIT * j);
+			else
+				reta |= r & (IXGBE_8_BIT_MASK <<
+						(CHAR_BIT * j));
+		}
+		IXGBE_WRITE_REG(hw,
+			IXGBE_ERETA((i - ETH_RSS_RETA_SIZE_128) >> 2), reta);
+	}
+
 	return 0;
 }
 
@@ -3248,11 +3297,26 @@ ixgbe_dev_rss_reta_query(struct rte_eth_dev *dev,
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
 	PMD_INIT_FUNC_TRACE();
-	if (reta_size != ETH_RSS_RETA_SIZE_128) {
-		PMD_DRV_LOG(ERR, "The size of hash lookup table configured "
-			"(%d) doesn't match the number hardware can supported "
-				"(%d)\n", reta_size, ETH_RSS_RETA_SIZE_128);
-		return -EINVAL;
+	switch (hw->mac.type) {
+	case ixgbe_mac_X550:
+	case ixgbe_mac_X550EM_x:
+		if (reta_size != ETH_RSS_RETA_SIZE_512) {
+			PMD_DRV_LOG(ERR, "The size of hash lookup table "
+					" configured (%d) doesn't match the "
+					"number hardware can supported (%d)\n",
+					reta_size, ETH_RSS_RETA_SIZE_512);
+			return -EINVAL;
+		}
+		break;
+	default:
+		if (reta_size != ETH_RSS_RETA_SIZE_128) {
+			PMD_DRV_LOG(ERR, "The size of hash lookup table "
+					" configured (%d) doesn't match the "
+					"number hardware can supported (%d)\n",
+					reta_size, ETH_RSS_RETA_SIZE_128);
+			return -EINVAL;
+		}
+		break;
 	}
 
 	for (i = 0; i < ETH_RSS_RETA_SIZE_128; i += IXGBE_4_BIT_WIDTH) {
@@ -3272,6 +3336,24 @@ ixgbe_dev_rss_reta_query(struct rte_eth_dev *dev,
 		}
 	}
 
+	for (i = ETH_RSS_RETA_SIZE_128; i < reta_size; i += IXGBE_4_BIT_WIDTH) {
+		idx = i / RTE_RETA_GROUP_SIZE;
+		shift = i % RTE_RETA_GROUP_SIZE;
+		mask = (uint8_t)((reta_conf[idx].mask >> shift) &
+						IXGBE_4_BIT_MASK);
+		if (!mask)
+			continue;
+
+		reta = IXGBE_READ_REG(hw,
+				IXGBE_ERETA((i - ETH_RSS_RETA_SIZE_128) >> 2));
+		for (j = 0; j < IXGBE_4_BIT_WIDTH; j++) {
+			if (mask & (0x1 << j))
+				reta_conf[idx].reta[shift + j] =
+					((reta >> (CHAR_BIT * j)) &
+						IXGBE_8_BIT_MASK);
+		}
+	}
+
 	return 0;
 }
 
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index a598a72..a746ae7 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -2790,7 +2790,7 @@ ixgbe_rss_configure(struct rte_eth_dev *dev)
 {
 	struct rte_eth_rss_conf rss_conf;
 	struct ixgbe_hw *hw;
-	uint32_t reta;
+	uint32_t reta = 0;
 	uint16_t i;
 	uint16_t j;
 
@@ -2802,8 +2802,7 @@ ixgbe_rss_configure(struct rte_eth_dev *dev)
 	 * The byte-swap is needed because NIC registers are in
 	 * little-endian order.
 	 */
-	reta = 0;
-	for (i = 0, j = 0; i < 128; i++, j++) {
+	for (i = 0, j = 0; i < ETH_RSS_RETA_SIZE_128; i++, j++) {
 		if (j == dev->data->nb_rx_queues)
 			j = 0;
 		reta = (reta << 8) | j;
@@ -2812,6 +2811,19 @@ ixgbe_rss_configure(struct rte_eth_dev *dev)
 					rte_bswap32(reta));
 	}
 
+	if ((hw->mac.type == ixgbe_mac_X550) ||
+		(hw->mac.type == ixgbe_mac_X550EM_x)) {
+		for (i = 0, j = 0; i < (ETH_RSS_RETA_SIZE_512 - ETH_RSS_RETA_SIZE_128);
+			i++, j++) {
+			if (j == dev->data->nb_rx_queues)
+				j = 0;
+			reta = (reta << 8) | j;
+			if ((i & 3) == 3)
+				IXGBE_WRITE_REG(hw, IXGBE_ERETA(i >> 2),
+						rte_bswap32(reta));
+		}
+	}
+
 	/*
 	 * Configure the RSS key and the RSS protocols used to compute
 	 * the RSS hash of input packets.
-- 
1.9.3

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

* [dpdk-dev] [PATCH 2/4] ixgbe: VF RSS config on x550
  2015-09-28  7:52 [dpdk-dev] [PATCH 0/4] RSS enhancement on Intel x550 NIC Wenzhuo Lu
  2015-09-28  7:52 ` [dpdk-dev] [PATCH 1/4] ixgbe: 512 entries RSS table on x550 Wenzhuo Lu
@ 2015-09-28  7:52 ` Wenzhuo Lu
  2015-10-15 22:42   ` Ananyev, Konstantin
  2015-09-28  7:52 ` [dpdk-dev] [PATCH 3/4] ixgbe: VF RSS hash query and update Wenzhuo Lu
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Wenzhuo Lu @ 2015-09-28  7:52 UTC (permalink / raw)
  To: dev

On x550, there're separate registers provided for VF RSS while on the other
10G NICs, for example, 82599, VF and PF share the same registers.
This patch lets x550 use the VF specific registers when doing RSS configuration
on VF. The behavior of other 10G NICs doesn't change.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 drivers/net/ixgbe/ixgbe_rxtx.c | 111 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 108 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index a746ae7..4a2d24a 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -2838,6 +2838,107 @@ ixgbe_rss_configure(struct rte_eth_dev *dev)
 	ixgbe_hw_rss_hash_set(hw, &rss_conf);
 }
 
+static void
+ixgbevf_rss_disable_x550(struct rte_eth_dev *dev)
+{
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t vfmrqc;
+
+	vfmrqc = IXGBE_READ_REG(hw, IXGBE_VFMRQC);
+	vfmrqc &= ~IXGBE_MRQC_RSSEN;
+	IXGBE_WRITE_REG(hw, IXGBE_VFMRQC, vfmrqc);
+}
+
+static void
+ixgbevf_hw_rss_hash_set_x550(struct ixgbe_hw *hw,
+			struct rte_eth_rss_conf *rss_conf)
+{
+	uint8_t  *hash_key;
+	uint32_t vfmrqc;
+	uint32_t rss_key;
+	uint64_t rss_hf;
+	uint16_t i;
+
+	hash_key = rss_conf->rss_key;
+	if (hash_key != NULL) {
+		/* Fill in RSS hash key */
+		for (i = 0; i < 10; i++) {
+			rss_key  = hash_key[(i * 4)];
+			rss_key |= hash_key[(i * 4) + 1] << 8;
+			rss_key |= hash_key[(i * 4) + 2] << 16;
+			rss_key |= hash_key[(i * 4) + 3] << 24;
+			IXGBE_WRITE_REG_ARRAY(hw, IXGBE_VFRSSRK(0), i, rss_key);
+		}
+	}
+
+	/* Set configured hashing protocols in VFMRQC register */
+	rss_hf = rss_conf->rss_hf;
+	vfmrqc = IXGBE_MRQC_RSSEN; /* Enable RSS */
+	if (rss_hf & ETH_RSS_IPV4)
+		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV4;
+	if (rss_hf & ETH_RSS_NONFRAG_IPV4_TCP)
+		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV4_TCP;
+	if (rss_hf & ETH_RSS_IPV6)
+		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV6;
+	if (rss_hf & ETH_RSS_IPV6_EX)
+		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV6_EX;
+	if (rss_hf & ETH_RSS_NONFRAG_IPV6_TCP)
+		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV6_TCP;
+	if (rss_hf & ETH_RSS_IPV6_TCP_EX)
+		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV6_EX_TCP;
+	if (rss_hf & ETH_RSS_NONFRAG_IPV4_UDP)
+		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV4_UDP;
+	if (rss_hf & ETH_RSS_NONFRAG_IPV6_UDP)
+		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV6_UDP;
+	if (rss_hf & ETH_RSS_IPV6_UDP_EX)
+		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV6_EX_UDP;
+	IXGBE_WRITE_REG(hw, IXGBE_VFMRQC, vfmrqc);
+}
+
+static void
+ixgbevf_rss_configure_x550(struct rte_eth_dev *dev)
+{
+	struct rte_eth_rss_conf rss_conf;
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t reta = 0;
+	uint16_t i;
+	uint16_t j;
+
+	PMD_INIT_FUNC_TRACE();
+
+	if (dev->data->dev_conf.rxmode.mq_mode != ETH_MQ_RX_RSS) {
+		ixgbevf_rss_disable_x550(dev);
+		PMD_DRV_LOG(DEBUG, "RSS not configured\n");
+		return;
+	}
+	/*
+	 * Fill in redirection table
+	 * The byte-swap is needed because NIC registers are in
+	 * little-endian order.
+	 */
+	for (i = 0, j = 0; i < ETH_RSS_RETA_SIZE_64; i++, j++) {
+		if (j == dev->data->nb_rx_queues)
+			j = 0;
+		reta = (reta << 8) | j;
+		if ((i & 3) == 3)
+			IXGBE_WRITE_REG(hw, IXGBE_VFRETA(i >> 2),
+					rte_bswap32(reta));
+	}
+
+	/*
+	 * Configure the RSS key and the RSS protocols used to compute
+	 * the RSS hash of input packets.
+	 */
+	rss_conf = dev->data->dev_conf.rx_adv_conf.rss_conf;
+	if ((rss_conf.rss_hf & IXGBE_RSS_OFFLOAD_ALL) == 0) {
+		ixgbevf_rss_disable_x550(dev);
+		return;
+	}
+	if (rss_conf.rss_key == NULL)
+		rss_conf.rss_key = rss_intel_key; /* Default hash key */
+	ixgbevf_hw_rss_hash_set_x550(hw, &rss_conf);
+}
+
 #define NUM_VFTA_REGISTERS 128
 #define NIC_RX_BUFFER_SIZE 0x200
 #define X550_RX_BUFFER_SIZE 0x180
@@ -3621,12 +3722,16 @@ ixgbe_alloc_rx_queue_mbufs(struct ixgbe_rx_queue *rxq)
 static int
 ixgbe_config_vf_rss(struct rte_eth_dev *dev)
 {
-	struct ixgbe_hw *hw;
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	uint32_t mrqc;
 
-	ixgbe_rss_configure(dev);
+	if (hw->mac.type == ixgbe_mac_X550_vf ||
+		hw->mac.type == ixgbe_mac_X550EM_x_vf) {
+		ixgbevf_rss_configure_x550(dev);
+		return 0;
+	}
 
-	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	ixgbe_rss_configure(dev);
 
 	/* MRQC: enable VF RSS */
 	mrqc = IXGBE_READ_REG(hw, IXGBE_MRQC);
-- 
1.9.3

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

* [dpdk-dev] [PATCH 3/4] ixgbe: VF RSS hash query and update
  2015-09-28  7:52 [dpdk-dev] [PATCH 0/4] RSS enhancement on Intel x550 NIC Wenzhuo Lu
  2015-09-28  7:52 ` [dpdk-dev] [PATCH 1/4] ixgbe: 512 entries RSS table on x550 Wenzhuo Lu
  2015-09-28  7:52 ` [dpdk-dev] [PATCH 2/4] ixgbe: VF RSS config " Wenzhuo Lu
@ 2015-09-28  7:52 ` Wenzhuo Lu
  2015-10-15 22:53   ` [dpdk-dev] FW: " Ananyev, Konstantin
  2015-09-28  7:52 ` [dpdk-dev] [PATCH 4/4] ixgbe: VF RSS reta " Wenzhuo Lu
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Wenzhuo Lu @ 2015-09-28  7:52 UTC (permalink / raw)
  To: dev

This patch implements the VF RSS hash query and update function on 10G
NICs. But the update function is only provided for x550. Because the
other NICs don't have the separate registers for VF, we don't want to
let a VF NIC change the shared RSS hash registers. It may cause PF and
other VF NICs' behavior change without being noticed.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c |  2 +
 drivers/net/ixgbe/ixgbe_ethdev.h |  6 +++
 drivers/net/ixgbe/ixgbe_rxtx.c   | 95 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 103 insertions(+)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index a1ef26f..5e50ee6 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -497,6 +497,8 @@ static const struct eth_dev_ops ixgbevf_eth_dev_ops = {
 	.mac_addr_set         = ixgbevf_set_default_mac_addr,
 	.get_reg_length       = ixgbevf_get_reg_length,
 	.get_reg              = ixgbevf_get_regs,
+	.rss_hash_update      = ixgbevf_dev_rss_hash_update,
+	.rss_hash_conf_get    = ixgbevf_dev_rss_hash_conf_get,
 };
 
 /* store statistics names and its offset in stats structure */
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index c3d4f4f..30952b6 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -377,6 +377,12 @@ int ixgbe_dev_rss_hash_update(struct rte_eth_dev *dev,
 int ixgbe_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
 				struct rte_eth_rss_conf *rss_conf);
 
+int ixgbevf_dev_rss_hash_update(struct rte_eth_dev *dev,
+				struct rte_eth_rss_conf *rss_conf);
+
+int ixgbevf_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
+				  struct rte_eth_rss_conf *rss_conf);
+
 /*
  * Flow director function prototypes
  */
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 4a2d24a..5b64ece 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -2895,6 +2895,101 @@ ixgbevf_hw_rss_hash_set_x550(struct ixgbe_hw *hw,
 	IXGBE_WRITE_REG(hw, IXGBE_VFMRQC, vfmrqc);
 }
 
+int
+ixgbevf_dev_rss_hash_update(struct rte_eth_dev *dev,
+			    struct rte_eth_rss_conf *rss_conf)
+{
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t vfmrqc;
+	uint64_t rss_hf;
+
+	if (hw->mac.type != ixgbe_mac_X550_vf &&
+		hw->mac.type != ixgbe_mac_X550EM_x_vf) {
+		PMD_DRV_LOG(ERR, "RSS hash update is not supported on this "
+			"VF NIC.");
+		return -ENOTSUP;
+	}
+
+	/*
+	 * There's hardware limitation:
+	 *     "RSS enabling cannot be done dynamically while it must be
+	 *      preceded by a software reset"
+	 * Before changing anything, first check that the update RSS operation
+	 * does not attempt to disable RSS, if RSS was enabled at
+	 * initialization time, or does not attempt to enable RSS, if RSS was
+	 * disabled at initialization time.
+	 */
+	rss_hf = rss_conf->rss_hf & IXGBE_RSS_OFFLOAD_ALL;
+	vfmrqc = IXGBE_READ_REG(hw, IXGBE_VFMRQC);
+	if (!(vfmrqc & IXGBE_MRQC_RSSEN)) { /* RSS disabled */
+		if (rss_hf != 0) /* Enable RSS */
+			return -(EINVAL);
+		return 0; /* Nothing to do */
+	}
+	/* RSS enabled */
+	if (rss_hf == 0) /* Disable RSS */
+		return -(EINVAL);
+	ixgbevf_hw_rss_hash_set_x550(hw, rss_conf);
+	return 0;
+}
+
+int
+ixgbevf_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
+			      struct rte_eth_rss_conf *rss_conf)
+{
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint8_t *hash_key;
+	uint32_t vfmrqc;
+	uint32_t rss_key;
+	uint64_t rss_hf;
+	uint16_t i;
+
+	if (hw->mac.type != ixgbe_mac_X550_vf &&
+		hw->mac.type != ixgbe_mac_X550EM_x_vf) {
+		return ixgbe_dev_rss_hash_conf_get(dev, rss_conf);
+	}
+
+	hash_key = rss_conf->rss_key;
+	if (hash_key != NULL) {
+		/* Return RSS hash key */
+		for (i = 0; i < 10; i++) {
+			rss_key = IXGBE_READ_REG_ARRAY(hw, IXGBE_VFRSSRK(0), i);
+			hash_key[(i * 4)] = rss_key & 0x000000FF;
+			hash_key[(i * 4) + 1] = (rss_key >> 8) & 0x000000FF;
+			hash_key[(i * 4) + 2] = (rss_key >> 16) & 0x000000FF;
+			hash_key[(i * 4) + 3] = (rss_key >> 24) & 0x000000FF;
+		}
+	}
+
+	/* Get RSS functions configured in VFMRQC register */
+	vfmrqc = IXGBE_READ_REG(hw, IXGBE_VFMRQC);
+	if ((vfmrqc & IXGBE_MRQC_RSSEN) == 0) { /* RSS is disabled */
+		rss_conf->rss_hf = 0;
+		return 0;
+	}
+	rss_hf = 0;
+	if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV4)
+		rss_hf |= ETH_RSS_IPV4;
+	if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV4_TCP)
+		rss_hf |= ETH_RSS_NONFRAG_IPV4_TCP;
+	if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV6)
+		rss_hf |= ETH_RSS_IPV6;
+	if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV6_EX)
+		rss_hf |= ETH_RSS_IPV6_EX;
+	if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV6_TCP)
+		rss_hf |= ETH_RSS_NONFRAG_IPV6_TCP;
+	if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV6_EX_TCP)
+		rss_hf |= ETH_RSS_IPV6_TCP_EX;
+	if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV4_UDP)
+		rss_hf |= ETH_RSS_NONFRAG_IPV4_UDP;
+	if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV6_UDP)
+		rss_hf |= ETH_RSS_NONFRAG_IPV6_UDP;
+	if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV6_EX_UDP)
+		rss_hf |= ETH_RSS_IPV6_UDP_EX;
+	rss_conf->rss_hf = rss_hf;
+	return 0;
+}
+
 static void
 ixgbevf_rss_configure_x550(struct rte_eth_dev *dev)
 {
-- 
1.9.3

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

* [dpdk-dev] [PATCH 4/4] ixgbe: VF RSS reta query and update
  2015-09-28  7:52 [dpdk-dev] [PATCH 0/4] RSS enhancement on Intel x550 NIC Wenzhuo Lu
                   ` (2 preceding siblings ...)
  2015-09-28  7:52 ` [dpdk-dev] [PATCH 3/4] ixgbe: VF RSS hash query and update Wenzhuo Lu
@ 2015-09-28  7:52 ` Wenzhuo Lu
  2015-10-15 22:57   ` Ananyev, Konstantin
  2015-10-16 10:04 ` [dpdk-dev] [PATCH 0/4] RSS enhancement on Intel x550 NIC Nélio Laranjeiro
  2015-10-16 13:05 ` [dpdk-dev] [PATCH v2 " Wenzhuo Lu
  5 siblings, 1 reply; 18+ messages in thread
From: Wenzhuo Lu @ 2015-09-28  7:52 UTC (permalink / raw)
  To: dev

This patch implements the VF RSS redirection table query and update function
on 10G NICs. But the update function is only provided for x550. Because the
other NICs don't have the separate registers for VF, we don't want to let a
VF NIC change the shared RSS reta registers. It may cause PF and other VF NICs'
behavior change without being noticed.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 103 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 103 insertions(+)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 5e50ee6..44baadf 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -326,6 +326,13 @@ static int ixgbe_timesync_read_rx_timestamp(struct rte_eth_dev *dev,
 static int ixgbe_timesync_read_tx_timestamp(struct rte_eth_dev *dev,
 					    struct timespec *timestamp);
 
+static int ixgbevf_dev_rss_reta_update(struct rte_eth_dev *dev,
+				struct rte_eth_rss_reta_entry64 *reta_conf,
+				uint16_t reta_size);
+static int ixgbevf_dev_rss_reta_query(struct rte_eth_dev *dev,
+				struct rte_eth_rss_reta_entry64 *reta_conf,
+				uint16_t reta_size);
+
 /*
  * Define VF Stats MACRO for Non "cleared on read" register
  */
@@ -497,6 +504,8 @@ static const struct eth_dev_ops ixgbevf_eth_dev_ops = {
 	.mac_addr_set         = ixgbevf_set_default_mac_addr,
 	.get_reg_length       = ixgbevf_get_reg_length,
 	.get_reg              = ixgbevf_get_regs,
+	.reta_update          = ixgbevf_dev_rss_reta_update,
+	.reta_query           = ixgbevf_dev_rss_reta_query,
 	.rss_hash_update      = ixgbevf_dev_rss_hash_update,
 	.rss_hash_conf_get    = ixgbevf_dev_rss_hash_conf_get,
 };
@@ -5557,6 +5566,100 @@ ixgbe_set_eeprom(struct rte_eth_dev *dev,
 	return eeprom->ops.write_buffer(hw,  first, length, data);
 }
 
+static int
+ixgbevf_dev_rss_reta_update(struct rte_eth_dev *dev,
+			struct rte_eth_rss_reta_entry64 *reta_conf,
+			uint16_t reta_size)
+{
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t reta, r;
+	uint16_t i, j;
+	uint16_t idx, shift;
+	uint8_t mask;
+
+	if (hw->mac.type != ixgbe_mac_X550_vf &&
+		hw->mac.type != ixgbe_mac_X550EM_x_vf) {
+		PMD_DRV_LOG(ERR, "RSS reta update is not supported on this "
+			"VF NIC.");
+		return -ENOTSUP;
+	}
+
+	if (reta_size != ETH_RSS_RETA_SIZE_64) {
+		PMD_DRV_LOG(ERR, "The size of hash lookup table configured "
+			"(%d) doesn't match the number of hardware can "
+			"support (%d)\n", reta_size, ETH_RSS_RETA_SIZE_64);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < reta_size; i += IXGBE_4_BIT_WIDTH) {
+		idx = i / RTE_RETA_GROUP_SIZE;
+		shift = i % RTE_RETA_GROUP_SIZE;
+		mask = (uint8_t)((reta_conf[idx].mask >> shift) &
+				IXGBE_4_BIT_WIDTH);
+		if (!mask)
+			continue;
+		if (mask == IXGBE_4_BIT_WIDTH)
+			r = 0;
+		else
+			r = IXGBE_READ_REG(hw, IXGBE_VFRETA(i >> 2));
+
+		for (j = 0, reta = 0; j < IXGBE_4_BIT_WIDTH; j++) {
+			if (mask & (0x1 << j))
+				reta |= reta_conf[idx].reta[shift + j] <<
+					(CHAR_BIT * j);
+			else
+				reta |= r &
+					(IXGBE_8_BIT_MASK << (CHAR_BIT * j));
+		}
+		IXGBE_WRITE_REG(hw, IXGBE_VFRETA(i >> 2), reta);
+	}
+
+	return 0;
+}
+
+static int
+ixgbevf_dev_rss_reta_query(struct rte_eth_dev *dev,
+			struct rte_eth_rss_reta_entry64 *reta_conf,
+			uint16_t reta_size)
+{
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t reta;
+	uint16_t i, j;
+	uint16_t idx, shift;
+	uint8_t mask;
+
+	if (hw->mac.type != ixgbe_mac_X550_vf &&
+		hw->mac.type != ixgbe_mac_X550EM_x_vf) {
+		return ixgbe_dev_rss_reta_query(dev, reta_conf, reta_size);
+	}
+
+	if (reta_size != ETH_RSS_RETA_SIZE_64) {
+		PMD_DRV_LOG(ERR, "The size of hash lookup table configured "
+			"(%d) doesn't match the number of hardware can "
+			"support (%d)\n", reta_size, ETH_RSS_RETA_SIZE_64);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < reta_size; i += IXGBE_4_BIT_WIDTH) {
+		idx = i / RTE_RETA_GROUP_SIZE;
+		shift = i % RTE_RETA_GROUP_SIZE;
+		mask = (uint8_t)((reta_conf[idx].mask >> shift) &
+					IXGBE_4_BIT_MASK);
+		if (!mask)
+			continue;
+
+		reta = IXGBE_READ_REG(hw, IXGBE_VFRETA(i >> 2));
+		for (j = 0; j < IXGBE_4_BIT_WIDTH; j++) {
+			if (mask & (0x1 << j))
+				reta_conf[idx].reta[shift + j] =
+					((reta >> (CHAR_BIT * j)) &
+						IXGBE_8_BIT_MASK);
+		}
+	}
+
+	return 0;
+}
+
 static struct rte_driver rte_ixgbe_driver = {
 	.type = PMD_PDEV,
 	.init = rte_ixgbe_pmd_init,
-- 
1.9.3

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

* Re: [dpdk-dev] [PATCH 1/4] ixgbe: 512 entries RSS table on x550
  2015-09-28  7:52 ` [dpdk-dev] [PATCH 1/4] ixgbe: 512 entries RSS table on x550 Wenzhuo Lu
@ 2015-10-15 22:19   ` Ananyev, Konstantin
  0 siblings, 0 replies; 18+ messages in thread
From: Ananyev, Konstantin @ 2015-10-15 22:19 UTC (permalink / raw)
  To: Lu, Wenzhuo, dev

Hi Wenzhuo,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Monday, September 28, 2015 8:52 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 1/4] ixgbe: 512 entries RSS table on x550
> 
> Comparing with the older NICs, x550's RSS redirection table is enlarged to 512
> entries. As the original code is for the NICs which have a 128 entries RSS table,
> it means only part of the RSS table is set on x550. So, RSS cannot work as
> expected on x550, it doesn't redirect the packets evenly.
> This patch configs the entries beyond 128 on x550 to let RSS work well, and also
> update the query and update functions to support 512 entries.
> 
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 106 ++++++++++++++++++++++++++++++++++-----
>  drivers/net/ixgbe/ixgbe_rxtx.c   |  18 +++++--
>  2 files changed, 109 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index ec2918c..a1ef26f 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -2397,7 +2397,17 @@ ixgbe_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>  				ETH_TXQ_FLAGS_NOOFFLOADS,
>  	};
>  	dev_info->hash_key_size = IXGBE_HKEY_MAX_INDEX * sizeof(uint32_t);
> -	dev_info->reta_size = ETH_RSS_RETA_SIZE_128;
> +
> +	switch (hw->mac.type) {
> +	case ixgbe_mac_X550:
> +	case ixgbe_mac_X550EM_x:
> +		dev_info->reta_size = ETH_RSS_RETA_SIZE_512;
> +		break;
> +	default:
> +		dev_info->reta_size = ETH_RSS_RETA_SIZE_128;
> +		break;
> +	}
> +

Instead of adding switch(hw->mac_type) in dozen of places, why
not to create a new function: get_reta_size(hw) and use it whenever it is needed?

>  	dev_info->flow_type_rss_offloads = IXGBE_RSS_OFFLOAD_ALL;
>  }
> 
> @@ -3205,14 +3215,29 @@ ixgbe_dev_rss_reta_update(struct rte_eth_dev *dev,
>  	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> 
>  	PMD_INIT_FUNC_TRACE();
> -	if (reta_size != ETH_RSS_RETA_SIZE_128) {
> -		PMD_DRV_LOG(ERR, "The size of hash lookup table configured "
> -			"(%d) doesn't match the number hardware can supported "
> -			"(%d)\n", reta_size, ETH_RSS_RETA_SIZE_128);
> -		return -EINVAL;
> +	switch (hw->mac.type) {
> +	case ixgbe_mac_X550:
> +	case ixgbe_mac_X550EM_x:
> +		if (reta_size != ETH_RSS_RETA_SIZE_512) {
> +			PMD_DRV_LOG(ERR, "The size of hash lookup table "
> +					"configured (%d) doesn't match the "
> +					"number hardware can supported (%d)\n",
> +					reta_size, ETH_RSS_RETA_SIZE_512);
> +			return -EINVAL;
> +		}
> +		break;
> +	default:
> +		if (reta_size != ETH_RSS_RETA_SIZE_128) {
> +			PMD_DRV_LOG(ERR, "The size of hash lookup table "
> +					"configured (%d) doesn't match the "
> +					"number hardware can supported (%d)\n",
> +					reta_size, ETH_RSS_RETA_SIZE_128);
> +			return -EINVAL;
> +		}
> +		break;
>  	}
> 
> -	for (i = 0; i < reta_size; i += IXGBE_4_BIT_WIDTH) {
> +	for (i = 0; i < ETH_RSS_RETA_SIZE_128; i += IXGBE_4_BIT_WIDTH) {
>  		idx = i / RTE_RETA_GROUP_SIZE;
>  		shift = i % RTE_RETA_GROUP_SIZE;
>  		mask = (uint8_t)((reta_conf[idx].mask >> shift) &
> @@ -3234,6 +3259,30 @@ ixgbe_dev_rss_reta_update(struct rte_eth_dev *dev,
>  		IXGBE_WRITE_REG(hw, IXGBE_RETA(i >> 2), reta);
>  	}
> 
> +	for (i = ETH_RSS_RETA_SIZE_128; i < reta_size; i += IXGBE_4_BIT_WIDTH) {
> +		idx = i / RTE_RETA_GROUP_SIZE;
> +		shift = i % RTE_RETA_GROUP_SIZE;
> +		mask = (uint8_t)((reta_conf[idx].mask >> shift) &
> +						IXGBE_4_BIT_MASK);
> +		if (!mask)
> +			continue;
> +		if (mask == IXGBE_4_BIT_MASK)
> +			r = 0;
> +		else
> +			r = IXGBE_READ_REG(hw,
> +				IXGBE_ERETA((i - ETH_RSS_RETA_SIZE_128) >> 2));
> +		for (j = 0, reta = 0; j < IXGBE_4_BIT_WIDTH; j++) {
> +			if (mask & (0x1 << j))
> +				reta |= reta_conf[idx].reta[shift + j] <<
> +							(CHAR_BIT * j);
> +			else
> +				reta |= r & (IXGBE_8_BIT_MASK <<
> +						(CHAR_BIT * j));
> +		}
> +		IXGBE_WRITE_REG(hw,
> +			IXGBE_ERETA((i - ETH_RSS_RETA_SIZE_128) >> 2), reta);
> +	}
> +

Is there any good reason to create a second loop and duplicate loops body?
Why not just:

if (reta_size != get_reta_size(hw) {return -EINVAL;}
for (i=0; I != reta_size; i+=...) {...}
?  

>  	return 0;
>  }
> 
> @@ -3248,11 +3297,26 @@ ixgbe_dev_rss_reta_query(struct rte_eth_dev *dev,
>  	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> 
>  	PMD_INIT_FUNC_TRACE();
> -	if (reta_size != ETH_RSS_RETA_SIZE_128) {
> -		PMD_DRV_LOG(ERR, "The size of hash lookup table configured "
> -			"(%d) doesn't match the number hardware can supported "
> -				"(%d)\n", reta_size, ETH_RSS_RETA_SIZE_128);
> -		return -EINVAL;
> +	switch (hw->mac.type) {
> +	case ixgbe_mac_X550:
> +	case ixgbe_mac_X550EM_x:
> +		if (reta_size != ETH_RSS_RETA_SIZE_512) {
> +			PMD_DRV_LOG(ERR, "The size of hash lookup table "
> +					" configured (%d) doesn't match the "
> +					"number hardware can supported (%d)\n",
> +					reta_size, ETH_RSS_RETA_SIZE_512);
> +			return -EINVAL;
> +		}
> +		break;
> +	default:
> +		if (reta_size != ETH_RSS_RETA_SIZE_128) {
> +			PMD_DRV_LOG(ERR, "The size of hash lookup table "
> +					" configured (%d) doesn't match the "
> +					"number hardware can supported (%d)\n",
> +					reta_size, ETH_RSS_RETA_SIZE_128);
> +			return -EINVAL;
> +		}
> +		break;
>  	}
> 
>  	for (i = 0; i < ETH_RSS_RETA_SIZE_128; i += IXGBE_4_BIT_WIDTH) {
> @@ -3272,6 +3336,24 @@ ixgbe_dev_rss_reta_query(struct rte_eth_dev *dev,
>  		}
>  	}
> 
> +	for (i = ETH_RSS_RETA_SIZE_128; i < reta_size; i += IXGBE_4_BIT_WIDTH) {
> +		idx = i / RTE_RETA_GROUP_SIZE;
> +		shift = i % RTE_RETA_GROUP_SIZE;
> +		mask = (uint8_t)((reta_conf[idx].mask >> shift) &
> +						IXGBE_4_BIT_MASK);
> +		if (!mask)
> +			continue;
> +
> +		reta = IXGBE_READ_REG(hw,
> +				IXGBE_ERETA((i - ETH_RSS_RETA_SIZE_128) >> 2));
> +		for (j = 0; j < IXGBE_4_BIT_WIDTH; j++) {
> +			if (mask & (0x1 << j))
> +				reta_conf[idx].reta[shift + j] =
> +					((reta >> (CHAR_BIT * j)) &
> +						IXGBE_8_BIT_MASK);
> +		}
> +	}
> +

Same as above - don't see any need to create an extra loop with identical body.
Pls merge them into one.

>  	return 0;
>  }
> 
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index a598a72..a746ae7 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -2790,7 +2790,7 @@ ixgbe_rss_configure(struct rte_eth_dev *dev)
>  {
>  	struct rte_eth_rss_conf rss_conf;
>  	struct ixgbe_hw *hw;
> -	uint32_t reta;
> +	uint32_t reta = 0;
>  	uint16_t i;
>  	uint16_t j;
> 
> @@ -2802,8 +2802,7 @@ ixgbe_rss_configure(struct rte_eth_dev *dev)
>  	 * The byte-swap is needed because NIC registers are in
>  	 * little-endian order.
>  	 */
> -	reta = 0;

What was wrong with that one?

> -	for (i = 0, j = 0; i < 128; i++, j++) {
> +	for (i = 0, j = 0; i < ETH_RSS_RETA_SIZE_128; i++, j++) {
>  		if (j == dev->data->nb_rx_queues)
>  			j = 0;
>  		reta = (reta << 8) | j;
> @@ -2812,6 +2811,19 @@ ixgbe_rss_configure(struct rte_eth_dev *dev)
>  					rte_bswap32(reta));
>  	}
> 
> +	if ((hw->mac.type == ixgbe_mac_X550) ||
> +		(hw->mac.type == ixgbe_mac_X550EM_x)) {
> +		for (i = 0, j = 0; i < (ETH_RSS_RETA_SIZE_512 - ETH_RSS_RETA_SIZE_128);
> +			i++, j++) {
> +			if (j == dev->data->nb_rx_queues)
> +				j = 0;
> +			reta = (reta << 8) | j;
> +			if ((i & 3) == 3)
> +				IXGBE_WRITE_REG(hw, IXGBE_ERETA(i >> 2),
> +						rte_bswap32(reta));
> +		}
> +	}
> +

Same as above.
Seems no need to have 2 identical loops, pls merge into one.

Konstantin

>  	/*
>  	 * Configure the RSS key and the RSS protocols used to compute
>  	 * the RSS hash of input packets.
> --
> 1.9.3

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

* Re: [dpdk-dev] [PATCH 2/4] ixgbe: VF RSS config on x550
  2015-09-28  7:52 ` [dpdk-dev] [PATCH 2/4] ixgbe: VF RSS config " Wenzhuo Lu
@ 2015-10-15 22:42   ` Ananyev, Konstantin
  0 siblings, 0 replies; 18+ messages in thread
From: Ananyev, Konstantin @ 2015-10-15 22:42 UTC (permalink / raw)
  To: Lu, Wenzhuo, dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Monday, September 28, 2015 8:52 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 2/4] ixgbe: VF RSS config on x550
> 
> On x550, there're separate registers provided for VF RSS while on the other
> 10G NICs, for example, 82599, VF and PF share the same registers.
> This patch lets x550 use the VF specific registers when doing RSS configuration
> on VF. The behavior of other 10G NICs doesn't change.
> 
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> ---
>  drivers/net/ixgbe/ixgbe_rxtx.c | 111 +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 108 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index a746ae7..4a2d24a 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -2838,6 +2838,107 @@ ixgbe_rss_configure(struct rte_eth_dev *dev)
>  	ixgbe_hw_rss_hash_set(hw, &rss_conf);
>  }
> 
> +static void
> +ixgbevf_rss_disable_x550(struct rte_eth_dev *dev)
> +{
> +	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	uint32_t vfmrqc;
> +
> +	vfmrqc = IXGBE_READ_REG(hw, IXGBE_VFMRQC);
> +	vfmrqc &= ~IXGBE_MRQC_RSSEN;
> +	IXGBE_WRITE_REG(hw, IXGBE_VFMRQC, vfmrqc);
> +}
> +
> +static void
> +ixgbevf_hw_rss_hash_set_x550(struct ixgbe_hw *hw,
> +			struct rte_eth_rss_conf *rss_conf)
> +{
> +	uint8_t  *hash_key;
> +	uint32_t vfmrqc;
> +	uint32_t rss_key;
> +	uint64_t rss_hf;
> +	uint16_t i;
> +
> +	hash_key = rss_conf->rss_key;
> +	if (hash_key != NULL) {
> +		/* Fill in RSS hash key */
> +		for (i = 0; i < 10; i++) {
> +			rss_key  = hash_key[(i * 4)];
> +			rss_key |= hash_key[(i * 4) + 1] << 8;
> +			rss_key |= hash_key[(i * 4) + 2] << 16;
> +			rss_key |= hash_key[(i * 4) + 3] << 24;
> +			IXGBE_WRITE_REG_ARRAY(hw, IXGBE_VFRSSRK(0), i, rss_key);
> +		}
> +	}
> +
> +	/* Set configured hashing protocols in VFMRQC register */
> +	rss_hf = rss_conf->rss_hf;
> +	vfmrqc = IXGBE_MRQC_RSSEN; /* Enable RSS */
> +	if (rss_hf & ETH_RSS_IPV4)
> +		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV4;
> +	if (rss_hf & ETH_RSS_NONFRAG_IPV4_TCP)
> +		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV4_TCP;
> +	if (rss_hf & ETH_RSS_IPV6)
> +		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV6;
> +	if (rss_hf & ETH_RSS_IPV6_EX)
> +		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV6_EX;
> +	if (rss_hf & ETH_RSS_NONFRAG_IPV6_TCP)
> +		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV6_TCP;
> +	if (rss_hf & ETH_RSS_IPV6_TCP_EX)
> +		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV6_EX_TCP;
> +	if (rss_hf & ETH_RSS_NONFRAG_IPV4_UDP)
> +		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV4_UDP;
> +	if (rss_hf & ETH_RSS_NONFRAG_IPV6_UDP)
> +		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV6_UDP;
> +	if (rss_hf & ETH_RSS_IPV6_UDP_EX)
> +		vfmrqc |= IXGBE_MRQC_RSS_FIELD_IPV6_EX_UDP;
> +	IXGBE_WRITE_REG(hw, IXGBE_VFMRQC, vfmrqc);
> +}

This function is identical to ixgbe_hw_rss_hash_set(), except the 2 HW register sets it updates:
IXGBE_VFMRQC vs IXGBE_MRQC and IXGBE_VFRSSRK(0) vs IXGBE_RSSRK(0).
Plus they botha re static functions, so why not addresses of these 2 HW registers as extra parameters
to the ixgbe_hw_rss_hash_set() and use it in all places.
That way you'll avoid quite big code duplication.

> +
> +static void
> +ixgbevf_rss_configure_x550(struct rte_eth_dev *dev)
> +{
> +	struct rte_eth_rss_conf rss_conf;
> +	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	uint32_t reta = 0;
> +	uint16_t i;
> +	uint16_t j;
> +
> +	PMD_INIT_FUNC_TRACE();
> +
> +	if (dev->data->dev_conf.rxmode.mq_mode != ETH_MQ_RX_RSS) {
> +		ixgbevf_rss_disable_x550(dev);
> +		PMD_DRV_LOG(DEBUG, "RSS not configured\n");
> +		return;
> +	}
> +	/*
> +	 * Fill in redirection table
> +	 * The byte-swap is needed because NIC registers are in
> +	 * little-endian order.
> +	 */
> +	for (i = 0, j = 0; i < ETH_RSS_RETA_SIZE_64; i++, j++) {
> +		if (j == dev->data->nb_rx_queues)
> +			j = 0;
> +		reta = (reta << 8) | j;
> +		if ((i & 3) == 3)
> +			IXGBE_WRITE_REG(hw, IXGBE_VFRETA(i >> 2),
> +					rte_bswap32(reta));
> +	}
> +
> +	/*
> +	 * Configure the RSS key and the RSS protocols used to compute
> +	 * the RSS hash of input packets.
> +	 */
> +	rss_conf = dev->data->dev_conf.rx_adv_conf.rss_conf;
> +	if ((rss_conf.rss_hf & IXGBE_RSS_OFFLOAD_ALL) == 0) {
> +		ixgbevf_rss_disable_x550(dev);
> +		return;
> +	}
> +	if (rss_conf.rss_key == NULL)
> +		rss_conf.rss_key = rss_intel_key; /* Default hash key */
> +	ixgbevf_hw_rss_hash_set_x550(hw, &rss_conf);
> +}
> +

Same comment as above: > 90%of that function is just copy & paste of ixgbe_rss_configure().
Pls find a way to unify them and avoid unnecessary code duplication and growth.

>  #define NUM_VFTA_REGISTERS 128
>  #define NIC_RX_BUFFER_SIZE 0x200
>  #define X550_RX_BUFFER_SIZE 0x180
> @@ -3621,12 +3722,16 @@ ixgbe_alloc_rx_queue_mbufs(struct ixgbe_rx_queue *rxq)
>  static int
>  ixgbe_config_vf_rss(struct rte_eth_dev *dev)
>  {
> -	struct ixgbe_hw *hw;
> +	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>  	uint32_t mrqc;
> 
> -	ixgbe_rss_configure(dev);
> +	if (hw->mac.type == ixgbe_mac_X550_vf ||
> +		hw->mac.type == ixgbe_mac_X550EM_x_vf) {
> +		ixgbevf_rss_configure_x550(dev);
> +		return 0;
> +	}
> 
> -	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	ixgbe_rss_configure(dev);
> 
>  	/* MRQC: enable VF RSS */
>  	mrqc = IXGBE_READ_REG(hw, IXGBE_MRQC);
> --
> 1.9.3

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

* [dpdk-dev] FW:  [PATCH 3/4] ixgbe: VF RSS hash query and update
  2015-09-28  7:52 ` [dpdk-dev] [PATCH 3/4] ixgbe: VF RSS hash query and update Wenzhuo Lu
@ 2015-10-15 22:53   ` Ananyev, Konstantin
  0 siblings, 0 replies; 18+ messages in thread
From: Ananyev, Konstantin @ 2015-10-15 22:53 UTC (permalink / raw)
  To: Lu, Wenzhuo; +Cc: dev




> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Monday, September 28, 2015 8:53 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 3/4] ixgbe: VF RSS hash query and update
> 
> This patch implements the VF RSS hash query and update function on 10G
> NICs. But the update function is only provided for x550. Because the
> other NICs don't have the separate registers for VF, we don't want to
> let a VF NIC change the shared RSS hash registers. It may cause PF and
> other VF NICs' behavior change without being noticed.
> 
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c |  2 +
>  drivers/net/ixgbe/ixgbe_ethdev.h |  6 +++
>  drivers/net/ixgbe/ixgbe_rxtx.c   | 95 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 103 insertions(+)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index a1ef26f..5e50ee6 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -497,6 +497,8 @@ static const struct eth_dev_ops ixgbevf_eth_dev_ops = {
>  	.mac_addr_set         = ixgbevf_set_default_mac_addr,
>  	.get_reg_length       = ixgbevf_get_reg_length,
>  	.get_reg              = ixgbevf_get_regs,
> +	.rss_hash_update      = ixgbevf_dev_rss_hash_update,
> +	.rss_hash_conf_get    = ixgbevf_dev_rss_hash_conf_get,
>  };
> 
>  /* store statistics names and its offset in stats structure */
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
> index c3d4f4f..30952b6 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> @@ -377,6 +377,12 @@ int ixgbe_dev_rss_hash_update(struct rte_eth_dev *dev,
>  int ixgbe_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
>  				struct rte_eth_rss_conf *rss_conf);
> 
> +int ixgbevf_dev_rss_hash_update(struct rte_eth_dev *dev,
> +				struct rte_eth_rss_conf *rss_conf);
> +
> +int ixgbevf_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
> +				  struct rte_eth_rss_conf *rss_conf);
> +
>  /*
>   * Flow director function prototypes
>   */
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index 4a2d24a..5b64ece 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -2895,6 +2895,101 @@ ixgbevf_hw_rss_hash_set_x550(struct ixgbe_hw *hw,
>  	IXGBE_WRITE_REG(hw, IXGBE_VFMRQC, vfmrqc);
>  }
> 
> +int
> +ixgbevf_dev_rss_hash_update(struct rte_eth_dev *dev,
> +			    struct rte_eth_rss_conf *rss_conf)
> +{
> +	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	uint32_t vfmrqc;
> +	uint64_t rss_hf;
> +
> +	if (hw->mac.type != ixgbe_mac_X550_vf &&
> +		hw->mac.type != ixgbe_mac_X550EM_x_vf) {
> +		PMD_DRV_LOG(ERR, "RSS hash update is not supported on this "
> +			"VF NIC.");
> +		return -ENOTSUP;
> +	}
> +
> +	/*
> +	 * There's hardware limitation:
> +	 *     "RSS enabling cannot be done dynamically while it must be
> +	 *      preceded by a software reset"
> +	 * Before changing anything, first check that the update RSS operation
> +	 * does not attempt to disable RSS, if RSS was enabled at
> +	 * initialization time, or does not attempt to enable RSS, if RSS was
> +	 * disabled at initialization time.
> +	 */
> +	rss_hf = rss_conf->rss_hf & IXGBE_RSS_OFFLOAD_ALL;
> +	vfmrqc = IXGBE_READ_REG(hw, IXGBE_VFMRQC);
> +	if (!(vfmrqc & IXGBE_MRQC_RSSEN)) { /* RSS disabled */
> +		if (rss_hf != 0) /* Enable RSS */
> +			return -(EINVAL);
> +		return 0; /* Nothing to do */
> +	}
> +	/* RSS enabled */
> +	if (rss_hf == 0) /* Disable RSS */
> +		return -(EINVAL);
> +	ixgbevf_hw_rss_hash_set_x550(hw, rss_conf);
> +	return 0;
> +}

Same comment as before: this function is just copy & paste ixgbe_dev_rss_hash_update(),
the only difference is the HW register: VFMRQC vs MRQC.
Pls unify - create a common static function that would be called by both ixgbe_dev_rss_hash_update()
and ixgbevf_dev_rss_hash_update(), or something. 

> +
> +int
> +ixgbevf_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
> +			      struct rte_eth_rss_conf *rss_conf)
> +{
> +	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	uint8_t *hash_key;
> +	uint32_t vfmrqc;
> +	uint32_t rss_key;
> +	uint64_t rss_hf;
> +	uint16_t i;
> +
> +	if (hw->mac.type != ixgbe_mac_X550_vf &&
> +		hw->mac.type != ixgbe_mac_X550EM_x_vf) {
> +		return ixgbe_dev_rss_hash_conf_get(dev, rss_conf);
> +	}
> +
> +	hash_key = rss_conf->rss_key;
> +	if (hash_key != NULL) {
> +		/* Return RSS hash key */
> +		for (i = 0; i < 10; i++) {
> +			rss_key = IXGBE_READ_REG_ARRAY(hw, IXGBE_VFRSSRK(0), i);
> +			hash_key[(i * 4)] = rss_key & 0x000000FF;
> +			hash_key[(i * 4) + 1] = (rss_key >> 8) & 0x000000FF;
> +			hash_key[(i * 4) + 2] = (rss_key >> 16) & 0x000000FF;
> +			hash_key[(i * 4) + 3] = (rss_key >> 24) & 0x000000FF;
> +		}
> +	}
> +
> +	/* Get RSS functions configured in VFMRQC register */
> +	vfmrqc = IXGBE_READ_REG(hw, IXGBE_VFMRQC);
> +	if ((vfmrqc & IXGBE_MRQC_RSSEN) == 0) { /* RSS is disabled */
> +		rss_conf->rss_hf = 0;
> +		return 0;
> +	}
> +	rss_hf = 0;
> +	if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV4)
> +		rss_hf |= ETH_RSS_IPV4;
> +	if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV4_TCP)
> +		rss_hf |= ETH_RSS_NONFRAG_IPV4_TCP;
> +	if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV6)
> +		rss_hf |= ETH_RSS_IPV6;
> +	if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV6_EX)
> +		rss_hf |= ETH_RSS_IPV6_EX;
> +	if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV6_TCP)
> +		rss_hf |= ETH_RSS_NONFRAG_IPV6_TCP;
> +	if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV6_EX_TCP)
> +		rss_hf |= ETH_RSS_IPV6_TCP_EX;
> +	if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV4_UDP)
> +		rss_hf |= ETH_RSS_NONFRAG_IPV4_UDP;
> +	if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV6_UDP)
> +		rss_hf |= ETH_RSS_NONFRAG_IPV6_UDP;
> +	if (vfmrqc & IXGBE_MRQC_RSS_FIELD_IPV6_EX_UDP)
> +		rss_hf |= ETH_RSS_IPV6_UDP_EX;
> +	rss_conf->rss_hf = rss_hf;
> +	return 0;
> +}

Still the same: this is a clone of ixgbe_dev_rss_hash_conf_get() except the HW regsiters names.
No need to create another copy of identical copy.
Pls unify.

Konstantin

> +
>  static void
>  ixgbevf_rss_configure_x550(struct rte_eth_dev *dev)
>  {
> --
> 1.9.3

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

* Re: [dpdk-dev] [PATCH 4/4] ixgbe: VF RSS reta query and update
  2015-09-28  7:52 ` [dpdk-dev] [PATCH 4/4] ixgbe: VF RSS reta " Wenzhuo Lu
@ 2015-10-15 22:57   ` Ananyev, Konstantin
  2015-10-16  1:21     ` Lu, Wenzhuo
  0 siblings, 1 reply; 18+ messages in thread
From: Ananyev, Konstantin @ 2015-10-15 22:57 UTC (permalink / raw)
  To: Lu, Wenzhuo, dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Monday, September 28, 2015 8:53 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 4/4] ixgbe: VF RSS reta query and update
> 
> This patch implements the VF RSS redirection table query and update function
> on 10G NICs. But the update function is only provided for x550. Because the
> other NICs don't have the separate registers for VF, we don't want to let a
> VF NIC change the shared RSS reta registers. It may cause PF and other VF NICs'
> behavior change without being noticed.
> 
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 103 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 103 insertions(+)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 5e50ee6..44baadf 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -326,6 +326,13 @@ static int ixgbe_timesync_read_rx_timestamp(struct rte_eth_dev *dev,
>  static int ixgbe_timesync_read_tx_timestamp(struct rte_eth_dev *dev,
>  					    struct timespec *timestamp);
> 
> +static int ixgbevf_dev_rss_reta_update(struct rte_eth_dev *dev,
> +				struct rte_eth_rss_reta_entry64 *reta_conf,
> +				uint16_t reta_size);
> +static int ixgbevf_dev_rss_reta_query(struct rte_eth_dev *dev,
> +				struct rte_eth_rss_reta_entry64 *reta_conf,
> +				uint16_t reta_size);
> +
>  /*
>   * Define VF Stats MACRO for Non "cleared on read" register
>   */
> @@ -497,6 +504,8 @@ static const struct eth_dev_ops ixgbevf_eth_dev_ops = {
>  	.mac_addr_set         = ixgbevf_set_default_mac_addr,
>  	.get_reg_length       = ixgbevf_get_reg_length,
>  	.get_reg              = ixgbevf_get_regs,
> +	.reta_update          = ixgbevf_dev_rss_reta_update,
> +	.reta_query           = ixgbevf_dev_rss_reta_query,
>  	.rss_hash_update      = ixgbevf_dev_rss_hash_update,
>  	.rss_hash_conf_get    = ixgbevf_dev_rss_hash_conf_get,
>  };
> @@ -5557,6 +5566,100 @@ ixgbe_set_eeprom(struct rte_eth_dev *dev,
>  	return eeprom->ops.write_buffer(hw,  first, length, data);
>  }
> 
> +static int
> +ixgbevf_dev_rss_reta_update(struct rte_eth_dev *dev,
> +			struct rte_eth_rss_reta_entry64 *reta_conf,
> +			uint16_t reta_size)
> +{
> +	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	uint32_t reta, r;
> +	uint16_t i, j;
> +	uint16_t idx, shift;
> +	uint8_t mask;
> +
> +	if (hw->mac.type != ixgbe_mac_X550_vf &&
> +		hw->mac.type != ixgbe_mac_X550EM_x_vf) {
> +		PMD_DRV_LOG(ERR, "RSS reta update is not supported on this "
> +			"VF NIC.");
> +		return -ENOTSUP;
> +	}
> +
> +	if (reta_size != ETH_RSS_RETA_SIZE_64) {
> +		PMD_DRV_LOG(ERR, "The size of hash lookup table configured "
> +			"(%d) doesn't match the number of hardware can "
> +			"support (%d)\n", reta_size, ETH_RSS_RETA_SIZE_64);
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i < reta_size; i += IXGBE_4_BIT_WIDTH) {
> +		idx = i / RTE_RETA_GROUP_SIZE;
> +		shift = i % RTE_RETA_GROUP_SIZE;
> +		mask = (uint8_t)((reta_conf[idx].mask >> shift) &
> +				IXGBE_4_BIT_WIDTH);
> +		if (!mask)
> +			continue;
> +		if (mask == IXGBE_4_BIT_WIDTH)
> +			r = 0;
> +		else
> +			r = IXGBE_READ_REG(hw, IXGBE_VFRETA(i >> 2));
> +
> +		for (j = 0, reta = 0; j < IXGBE_4_BIT_WIDTH; j++) {
> +			if (mask & (0x1 << j))
> +				reta |= reta_conf[idx].reta[shift + j] <<
> +					(CHAR_BIT * j);
> +			else
> +				reta |= r &
> +					(IXGBE_8_BIT_MASK << (CHAR_BIT * j));
> +		}
> +		IXGBE_WRITE_REG(hw, IXGBE_VFRETA(i >> 2), reta);
> +	}
> +
> +	return 0;
> +}
> +
> +static int
> +ixgbevf_dev_rss_reta_query(struct rte_eth_dev *dev,
> +			struct rte_eth_rss_reta_entry64 *reta_conf,
> +			uint16_t reta_size)
> +{
> +	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	uint32_t reta;
> +	uint16_t i, j;
> +	uint16_t idx, shift;
> +	uint8_t mask;
> +
> +	if (hw->mac.type != ixgbe_mac_X550_vf &&
> +		hw->mac.type != ixgbe_mac_X550EM_x_vf) {
> +		return ixgbe_dev_rss_reta_query(dev, reta_conf, reta_size);
> +	}
> +
> +	if (reta_size != ETH_RSS_RETA_SIZE_64) {
> +		PMD_DRV_LOG(ERR, "The size of hash lookup table configured "
> +			"(%d) doesn't match the number of hardware can "
> +			"support (%d)\n", reta_size, ETH_RSS_RETA_SIZE_64);
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i < reta_size; i += IXGBE_4_BIT_WIDTH) {
> +		idx = i / RTE_RETA_GROUP_SIZE;
> +		shift = i % RTE_RETA_GROUP_SIZE;
> +		mask = (uint8_t)((reta_conf[idx].mask >> shift) &
> +					IXGBE_4_BIT_MASK);
> +		if (!mask)
> +			continue;
> +
> +		reta = IXGBE_READ_REG(hw, IXGBE_VFRETA(i >> 2));
> +		for (j = 0; j < IXGBE_4_BIT_WIDTH; j++) {
> +			if (mask & (0x1 << j))
> +				reta_conf[idx].reta[shift + j] =
> +					((reta >> (CHAR_BIT * j)) &
> +						IXGBE_8_BIT_MASK);
> +		}
> +	}
> +
> +	return 0;
> +}
> +

Same as for other 3 patches in that series: >90% of the code is just copy & paste of existing one,
with different HW registers name and reta_size.
Pls unify.
Konstantin

>  static struct rte_driver rte_ixgbe_driver = {
>  	.type = PMD_PDEV,
>  	.init = rte_ixgbe_pmd_init,
> --
> 1.9.3

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

* Re: [dpdk-dev] [PATCH 4/4] ixgbe: VF RSS reta query and update
  2015-10-15 22:57   ` Ananyev, Konstantin
@ 2015-10-16  1:21     ` Lu, Wenzhuo
  0 siblings, 0 replies; 18+ messages in thread
From: Lu, Wenzhuo @ 2015-10-16  1:21 UTC (permalink / raw)
  To: Ananyev, Konstantin, dev

Hi Konstantin,

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Friday, October 16, 2015 6:58 AM
> To: Lu, Wenzhuo; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH 4/4] ixgbe: VF RSS reta query and update
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
> > Sent: Monday, September 28, 2015 8:53 AM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH 4/4] ixgbe: VF RSS reta query and update
> >
> > This patch implements the VF RSS redirection table query and update
> > function on 10G NICs. But the update function is only provided for
> > x550. Because the other NICs don't have the separate registers for VF,
> > we don't want to let a VF NIC change the shared RSS reta registers. It may
> cause PF and other VF NICs'
> > behavior change without being noticed.
> >
> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > ---
> >  drivers/net/ixgbe/ixgbe_ethdev.c | 103
> > +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 103 insertions(+)
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > b/drivers/net/ixgbe/ixgbe_ethdev.c
> > index 5e50ee6..44baadf 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > @@ -326,6 +326,13 @@ static int
> > ixgbe_timesync_read_rx_timestamp(struct rte_eth_dev *dev,  static int
> ixgbe_timesync_read_tx_timestamp(struct rte_eth_dev *dev,
> >  					    struct timespec *timestamp);
> >
> > +static int ixgbevf_dev_rss_reta_update(struct rte_eth_dev *dev,
> > +				struct rte_eth_rss_reta_entry64 *reta_conf,
> > +				uint16_t reta_size);
> > +static int ixgbevf_dev_rss_reta_query(struct rte_eth_dev *dev,
> > +				struct rte_eth_rss_reta_entry64 *reta_conf,
> > +				uint16_t reta_size);
> > +
> >  /*
> >   * Define VF Stats MACRO for Non "cleared on read" register
> >   */
> > @@ -497,6 +504,8 @@ static const struct eth_dev_ops ixgbevf_eth_dev_ops
> = {
> >  	.mac_addr_set         = ixgbevf_set_default_mac_addr,
> >  	.get_reg_length       = ixgbevf_get_reg_length,
> >  	.get_reg              = ixgbevf_get_regs,
> > +	.reta_update          = ixgbevf_dev_rss_reta_update,
> > +	.reta_query           = ixgbevf_dev_rss_reta_query,
> >  	.rss_hash_update      = ixgbevf_dev_rss_hash_update,
> >  	.rss_hash_conf_get    = ixgbevf_dev_rss_hash_conf_get,
> >  };
> > @@ -5557,6 +5566,100 @@ ixgbe_set_eeprom(struct rte_eth_dev *dev,
> >  	return eeprom->ops.write_buffer(hw,  first, length, data);  }
> >
> > +static int
> > +ixgbevf_dev_rss_reta_update(struct rte_eth_dev *dev,
> > +			struct rte_eth_rss_reta_entry64 *reta_conf,
> > +			uint16_t reta_size)
> > +{
> > +	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> > +	uint32_t reta, r;
> > +	uint16_t i, j;
> > +	uint16_t idx, shift;
> > +	uint8_t mask;
> > +
> > +	if (hw->mac.type != ixgbe_mac_X550_vf &&
> > +		hw->mac.type != ixgbe_mac_X550EM_x_vf) {
> > +		PMD_DRV_LOG(ERR, "RSS reta update is not supported on this "
> > +			"VF NIC.");
> > +		return -ENOTSUP;
> > +	}
> > +
> > +	if (reta_size != ETH_RSS_RETA_SIZE_64) {
> > +		PMD_DRV_LOG(ERR, "The size of hash lookup table configured
> "
> > +			"(%d) doesn't match the number of hardware can "
> > +			"support (%d)\n", reta_size, ETH_RSS_RETA_SIZE_64);
> > +		return -EINVAL;
> > +	}
> > +
> > +	for (i = 0; i < reta_size; i += IXGBE_4_BIT_WIDTH) {
> > +		idx = i / RTE_RETA_GROUP_SIZE;
> > +		shift = i % RTE_RETA_GROUP_SIZE;
> > +		mask = (uint8_t)((reta_conf[idx].mask >> shift) &
> > +				IXGBE_4_BIT_WIDTH);
> > +		if (!mask)
> > +			continue;
> > +		if (mask == IXGBE_4_BIT_WIDTH)
> > +			r = 0;
> > +		else
> > +			r = IXGBE_READ_REG(hw, IXGBE_VFRETA(i >> 2));
> > +
> > +		for (j = 0, reta = 0; j < IXGBE_4_BIT_WIDTH; j++) {
> > +			if (mask & (0x1 << j))
> > +				reta |= reta_conf[idx].reta[shift + j] <<
> > +					(CHAR_BIT * j);
> > +			else
> > +				reta |= r &
> > +					(IXGBE_8_BIT_MASK << (CHAR_BIT * j));
> > +		}
> > +		IXGBE_WRITE_REG(hw, IXGBE_VFRETA(i >> 2), reta);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int
> > +ixgbevf_dev_rss_reta_query(struct rte_eth_dev *dev,
> > +			struct rte_eth_rss_reta_entry64 *reta_conf,
> > +			uint16_t reta_size)
> > +{
> > +	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> > +	uint32_t reta;
> > +	uint16_t i, j;
> > +	uint16_t idx, shift;
> > +	uint8_t mask;
> > +
> > +	if (hw->mac.type != ixgbe_mac_X550_vf &&
> > +		hw->mac.type != ixgbe_mac_X550EM_x_vf) {
> > +		return ixgbe_dev_rss_reta_query(dev, reta_conf, reta_size);
> > +	}
> > +
> > +	if (reta_size != ETH_RSS_RETA_SIZE_64) {
> > +		PMD_DRV_LOG(ERR, "The size of hash lookup table configured
> "
> > +			"(%d) doesn't match the number of hardware can "
> > +			"support (%d)\n", reta_size, ETH_RSS_RETA_SIZE_64);
> > +		return -EINVAL;
> > +	}
> > +
> > +	for (i = 0; i < reta_size; i += IXGBE_4_BIT_WIDTH) {
> > +		idx = i / RTE_RETA_GROUP_SIZE;
> > +		shift = i % RTE_RETA_GROUP_SIZE;
> > +		mask = (uint8_t)((reta_conf[idx].mask >> shift) &
> > +					IXGBE_4_BIT_MASK);
> > +		if (!mask)
> > +			continue;
> > +
> > +		reta = IXGBE_READ_REG(hw, IXGBE_VFRETA(i >> 2));
> > +		for (j = 0; j < IXGBE_4_BIT_WIDTH; j++) {
> > +			if (mask & (0x1 << j))
> > +				reta_conf[idx].reta[shift + j] =
> > +					((reta >> (CHAR_BIT * j)) &
> > +						IXGBE_8_BIT_MASK);
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> 
> Same as for other 3 patches in that series: >90% of the code is just copy & paste
> of existing one, with different HW registers name and reta_size.
> Pls unify.
> Konstantin
Thanks. I'll try to condense the code. And the same for the other 3 patches.

> 
> >  static struct rte_driver rte_ixgbe_driver = {
> >  	.type = PMD_PDEV,
> >  	.init = rte_ixgbe_pmd_init,
> > --
> > 1.9.3

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

* Re: [dpdk-dev] [PATCH 0/4] RSS enhancement on Intel x550 NIC
  2015-09-28  7:52 [dpdk-dev] [PATCH 0/4] RSS enhancement on Intel x550 NIC Wenzhuo Lu
                   ` (3 preceding siblings ...)
  2015-09-28  7:52 ` [dpdk-dev] [PATCH 4/4] ixgbe: VF RSS reta " Wenzhuo Lu
@ 2015-10-16 10:04 ` Nélio Laranjeiro
  2015-10-16 13:05 ` [dpdk-dev] [PATCH v2 " Wenzhuo Lu
  5 siblings, 0 replies; 18+ messages in thread
From: Nélio Laranjeiro @ 2015-10-16 10:04 UTC (permalink / raw)
  To: Wenzhuo Lu; +Cc: dev

On Mon, Sep 28, 2015 at 03:52:27PM +0800, Wenzhuo Lu wrote:
> This patch set implements the RSS enhancement on x550.
> The enhancement includes, the PF RSS redirection table is enlarged
> from 128 entries to 512 entries, the VF doesn't share the same
> registers with PF and per VF RSS redirection table is provided.
>[...] 
 
Hi Wenzhuo, 

We should discuss about this API for a future release of DPDK because
this one lacks in flexibility.  Some other NICs have indirection tables
with a different/configurable size, and the current API does not help
to manage it.

For ConnectX-4 I have made a lot of hacks to avoid changing the DPDK
API, "[dpdk-dev] [PATCH 0/3] Add RETA configuration to MLX5".
http://dpdk.org/ml/archives/dev/2015-October/024681.html

>From a user point of view, to update the RETA table, the API expects the
user to know the size of it to update or query.  With your patchset,
Intel have two indirection table sizes now, with Mellanox ConnectX-4, I
fixed to the size of 512 entries because it is not fixed by default.

How about discussing this in a separate thread?

Regards,

-- 
Nélio Laranjeiro
6WIND

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

* [dpdk-dev] [PATCH v2 0/4] RSS enhancement on Intel x550 NIC
  2015-09-28  7:52 [dpdk-dev] [PATCH 0/4] RSS enhancement on Intel x550 NIC Wenzhuo Lu
                   ` (4 preceding siblings ...)
  2015-10-16 10:04 ` [dpdk-dev] [PATCH 0/4] RSS enhancement on Intel x550 NIC Nélio Laranjeiro
@ 2015-10-16 13:05 ` Wenzhuo Lu
  2015-10-16 13:05   ` [dpdk-dev] [PATCH v2 1/4] ixgbe: 512 entries RSS table on x550 Wenzhuo Lu
                     ` (4 more replies)
  5 siblings, 5 replies; 18+ messages in thread
From: Wenzhuo Lu @ 2015-10-16 13:05 UTC (permalink / raw)
  To: dev

This patch set implements the RSS enhancement on x550.
The enhancement includes, the PF RSS redirection table is enlarged
from 128 entries to 512 entries, the VF doesn't share the same
registers with PF and per VF RSS redirection table is provided.

V2:
Condense the code.
Add release notes.

Wenzhuo Lu (4):
  ixgbe: 512 entries RSS table on x550
  ixgbe: VF RSS config on x550
  ixgbe: VF RSS reta/hash query and update
  doc: release notes update for RSS enhancement

 doc/guides/rel_notes/release_2_2.rst |   5 ++
 drivers/net/ixgbe/ixgbe_ethdev.c     | 102 +++++++++++++++++++++++++++++++----
 drivers/net/ixgbe/ixgbe_ethdev.h     |  10 ++++
 drivers/net/ixgbe/ixgbe_rxtx.c       |  43 +++++++++++----
 4 files changed, 142 insertions(+), 18 deletions(-)

-- 
1.9.3

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

* [dpdk-dev] [PATCH v2 1/4] ixgbe: 512 entries RSS table on x550
  2015-10-16 13:05 ` [dpdk-dev] [PATCH v2 " Wenzhuo Lu
@ 2015-10-16 13:05   ` Wenzhuo Lu
  2015-10-16 13:05   ` [dpdk-dev] [PATCH v2 2/4] ixgbe: VF RSS config " Wenzhuo Lu
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 18+ messages in thread
From: Wenzhuo Lu @ 2015-10-16 13:05 UTC (permalink / raw)
  To: dev

Comparing with the older NICs, x550's RSS redirection table is enlarged to 512
entries. As the original code is for the NICs which have a 128 entries RSS table,
it means only part of the RSS table is set on x550. So, RSS cannot work as
expected on x550, it doesn't redirect the packets evenly.
This patch configs the entries beyond 128 on x550 to let RSS work well, and also
update the query and update functions to support 512 entries.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 57 +++++++++++++++++++++++++++++++++-------
 drivers/net/ixgbe/ixgbe_ethdev.h |  4 +++
 drivers/net/ixgbe/ixgbe_rxtx.c   | 10 +++++--
 3 files changed, 60 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index ec2918c..480891d 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -2397,7 +2397,7 @@ ixgbe_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 				ETH_TXQ_FLAGS_NOOFFLOADS,
 	};
 	dev_info->hash_key_size = IXGBE_HKEY_MAX_INDEX * sizeof(uint32_t);
-	dev_info->reta_size = ETH_RSS_RETA_SIZE_128;
+	dev_info->reta_size = ixgbe_reta_size_get(hw->mac.type);
 	dev_info->flow_type_rss_offloads = IXGBE_RSS_OFFLOAD_ALL;
 }
 
@@ -3203,12 +3203,15 @@ ixgbe_dev_rss_reta_update(struct rte_eth_dev *dev,
 	uint32_t reta, r;
 	uint16_t idx, shift;
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint16_t sp_reta_size;
+	uint32_t reta_reg;
 
 	PMD_INIT_FUNC_TRACE();
-	if (reta_size != ETH_RSS_RETA_SIZE_128) {
+	sp_reta_size = ixgbe_reta_size_get(hw->mac.type);
+	if (reta_size != sp_reta_size) {
 		PMD_DRV_LOG(ERR, "The size of hash lookup table configured "
 			"(%d) doesn't match the number hardware can supported "
-			"(%d)\n", reta_size, ETH_RSS_RETA_SIZE_128);
+			"(%d)\n", reta_size, sp_reta_size);
 		return -EINVAL;
 	}
 
@@ -3219,10 +3222,11 @@ ixgbe_dev_rss_reta_update(struct rte_eth_dev *dev,
 						IXGBE_4_BIT_MASK);
 		if (!mask)
 			continue;
+		reta_reg = ixgbe_reta_reg_get(hw->mac.type, i);
 		if (mask == IXGBE_4_BIT_MASK)
 			r = 0;
 		else
-			r = IXGBE_READ_REG(hw, IXGBE_RETA(i >> 2));
+			r = IXGBE_READ_REG(hw, reta_reg);
 		for (j = 0, reta = 0; j < IXGBE_4_BIT_WIDTH; j++) {
 			if (mask & (0x1 << j))
 				reta |= reta_conf[idx].reta[shift + j] <<
@@ -3231,7 +3235,7 @@ ixgbe_dev_rss_reta_update(struct rte_eth_dev *dev,
 				reta |= r & (IXGBE_8_BIT_MASK <<
 						(CHAR_BIT * j));
 		}
-		IXGBE_WRITE_REG(hw, IXGBE_RETA(i >> 2), reta);
+		IXGBE_WRITE_REG(hw, reta_reg, reta);
 	}
 
 	return 0;
@@ -3246,16 +3250,19 @@ ixgbe_dev_rss_reta_query(struct rte_eth_dev *dev,
 	uint32_t reta;
 	uint16_t idx, shift;
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint16_t sp_reta_size;
+	uint32_t reta_reg;
 
 	PMD_INIT_FUNC_TRACE();
-	if (reta_size != ETH_RSS_RETA_SIZE_128) {
+	sp_reta_size = ixgbe_reta_size_get(hw->mac.type);
+	if (reta_size != sp_reta_size) {
 		PMD_DRV_LOG(ERR, "The size of hash lookup table configured "
 			"(%d) doesn't match the number hardware can supported "
-				"(%d)\n", reta_size, ETH_RSS_RETA_SIZE_128);
+			"(%d)\n", reta_size, sp_reta_size);
 		return -EINVAL;
 	}
 
-	for (i = 0; i < ETH_RSS_RETA_SIZE_128; i += IXGBE_4_BIT_WIDTH) {
+	for (i = 0; i < reta_size; i += IXGBE_4_BIT_WIDTH) {
 		idx = i / RTE_RETA_GROUP_SIZE;
 		shift = i % RTE_RETA_GROUP_SIZE;
 		mask = (uint8_t)((reta_conf[idx].mask >> shift) &
@@ -3263,7 +3270,8 @@ ixgbe_dev_rss_reta_query(struct rte_eth_dev *dev,
 		if (!mask)
 			continue;
 
-		reta = IXGBE_READ_REG(hw, IXGBE_RETA(i >> 2));
+		reta_reg = ixgbe_reta_reg_get(hw->mac.type, i);
+		reta = IXGBE_READ_REG(hw, reta_reg);
 		for (j = 0; j < IXGBE_4_BIT_WIDTH; j++) {
 			if (mask & (0x1 << j))
 				reta_conf[idx].reta[shift + j] =
@@ -5473,6 +5481,37 @@ ixgbe_set_eeprom(struct rte_eth_dev *dev,
 	return eeprom->ops.write_buffer(hw,  first, length, data);
 }
 
+uint16_t
+ixgbe_reta_size_get(enum ixgbe_mac_type mac_type) {
+	switch (mac_type) {
+	case ixgbe_mac_X550:
+	case ixgbe_mac_X550EM_x:
+		return ETH_RSS_RETA_SIZE_512;
+	case ixgbe_mac_X550_vf:
+	case ixgbe_mac_X550EM_x_vf:
+		return ETH_RSS_RETA_SIZE_64;
+	default:
+		return ETH_RSS_RETA_SIZE_128;
+	}
+}
+
+uint32_t
+ixgbe_reta_reg_get(enum ixgbe_mac_type mac_type, uint16_t reta_idx) {
+	switch (mac_type) {
+	case ixgbe_mac_X550:
+	case ixgbe_mac_X550EM_x:
+		if (reta_idx < ETH_RSS_RETA_SIZE_128)
+			return IXGBE_RETA(reta_idx >> 2);
+		else
+			return IXGBE_ERETA((reta_idx - ETH_RSS_RETA_SIZE_128) >> 2);
+	case ixgbe_mac_X550_vf:
+	case ixgbe_mac_X550EM_x_vf:
+		return IXGBE_VFRETA(reta_idx >> 2);
+	default:
+		return IXGBE_RETA(reta_idx >> 2);
+	}
+}
+
 static struct rte_driver rte_ixgbe_driver = {
 	.type = PMD_PDEV,
 	.init = rte_ixgbe_pmd_init,
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index c3d4f4f..0c669cd 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -377,6 +377,10 @@ int ixgbe_dev_rss_hash_update(struct rte_eth_dev *dev,
 int ixgbe_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
 				struct rte_eth_rss_conf *rss_conf);
 
+uint16_t ixgbe_reta_size_get(enum ixgbe_mac_type mac_type);
+
+uint32_t ixgbe_reta_reg_get(enum ixgbe_mac_type mac_type, uint16_t reta_idx);
+
 /*
  * Flow director function prototypes
  */
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index a598a72..494a6be 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -2793,22 +2793,28 @@ ixgbe_rss_configure(struct rte_eth_dev *dev)
 	uint32_t reta;
 	uint16_t i;
 	uint16_t j;
+	uint16_t sp_reta_size;
+	uint32_t reta_reg;
 
 	PMD_INIT_FUNC_TRACE();
 	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
+	sp_reta_size = ixgbe_reta_size_get(hw->mac.type);
+
 	/*
 	 * Fill in redirection table
 	 * The byte-swap is needed because NIC registers are in
 	 * little-endian order.
 	 */
 	reta = 0;
-	for (i = 0, j = 0; i < 128; i++, j++) {
+	for (i = 0, j = 0; i < sp_reta_size; i++, j++) {
+		reta_reg = ixgbe_reta_reg_get(hw->mac.type, i);
+
 		if (j == dev->data->nb_rx_queues)
 			j = 0;
 		reta = (reta << 8) | j;
 		if ((i & 3) == 3)
-			IXGBE_WRITE_REG(hw, IXGBE_RETA(i >> 2),
+			IXGBE_WRITE_REG(hw, reta_reg,
 					rte_bswap32(reta));
 	}
 
-- 
1.9.3

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

* [dpdk-dev] [PATCH v2 2/4] ixgbe: VF RSS config on x550
  2015-10-16 13:05 ` [dpdk-dev] [PATCH v2 " Wenzhuo Lu
  2015-10-16 13:05   ` [dpdk-dev] [PATCH v2 1/4] ixgbe: 512 entries RSS table on x550 Wenzhuo Lu
@ 2015-10-16 13:05   ` Wenzhuo Lu
  2015-10-16 13:05   ` [dpdk-dev] [PATCH v2 3/4] ixgbe: VF RSS reta/hash query and update Wenzhuo Lu
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 18+ messages in thread
From: Wenzhuo Lu @ 2015-10-16 13:05 UTC (permalink / raw)
  To: dev

On x550, there're separate registers provided for VF RSS while on the other
10G NICs, for example, 82599, VF and PF share the same registers.
This patch lets x550 use the VF specific registers when doing RSS configuration
on VF. The behavior of other 10G NICs doesn't change.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 22 ++++++++++++++++++++++
 drivers/net/ixgbe/ixgbe_ethdev.h |  4 ++++
 drivers/net/ixgbe/ixgbe_rxtx.c   | 15 +++++++++++----
 3 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 480891d..e2fbcfc 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -5512,6 +5512,28 @@ ixgbe_reta_reg_get(enum ixgbe_mac_type mac_type, uint16_t reta_idx) {
 	}
 }
 
+uint32_t
+ixgbe_mrqc_reg_get(enum ixgbe_mac_type mac_type) {
+	switch (mac_type) {
+	case ixgbe_mac_X550_vf:
+	case ixgbe_mac_X550EM_x_vf:
+		return IXGBE_VFMRQC;
+	default:
+		return IXGBE_MRQC;
+	}
+}
+
+uint32_t
+ixgbe_rssrk_reg_get(enum ixgbe_mac_type mac_type, uint8_t i) {
+	switch (mac_type) {
+	case ixgbe_mac_X550_vf:
+	case ixgbe_mac_X550EM_x_vf:
+		return IXGBE_VFRSSRK(i);
+	default:
+		return IXGBE_RSSRK(i);
+	}
+}
+
 static struct rte_driver rte_ixgbe_driver = {
 	.type = PMD_PDEV,
 	.init = rte_ixgbe_pmd_init,
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 0c669cd..441a17f 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -381,6 +381,10 @@ uint16_t ixgbe_reta_size_get(enum ixgbe_mac_type mac_type);
 
 uint32_t ixgbe_reta_reg_get(enum ixgbe_mac_type mac_type, uint16_t reta_idx);
 
+uint32_t ixgbe_mrqc_reg_get(enum ixgbe_mac_type mac_type);
+
+uint32_t ixgbe_rssrk_reg_get(enum ixgbe_mac_type mac_type, uint8_t i);
+
 /*
  * Flow director function prototypes
  */
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 494a6be..0b8ca18 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -2647,11 +2647,13 @@ ixgbe_rss_disable(struct rte_eth_dev *dev)
 {
 	struct ixgbe_hw *hw;
 	uint32_t mrqc;
+	uint32_t mrqc_reg;
 
 	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	mrqc = IXGBE_READ_REG(hw, IXGBE_MRQC);
+	mrqc_reg = ixgbe_mrqc_reg_get(hw->mac.type);
+	mrqc = IXGBE_READ_REG(hw, mrqc_reg);
 	mrqc &= ~IXGBE_MRQC_RSSEN;
-	IXGBE_WRITE_REG(hw, IXGBE_MRQC, mrqc);
+	IXGBE_WRITE_REG(hw, mrqc_reg, mrqc);
 }
 
 static void
@@ -2662,6 +2664,11 @@ ixgbe_hw_rss_hash_set(struct ixgbe_hw *hw, struct rte_eth_rss_conf *rss_conf)
 	uint32_t rss_key;
 	uint64_t rss_hf;
 	uint16_t i;
+	uint32_t mrqc_reg;
+	uint32_t rssrk_reg;
+
+	mrqc_reg = ixgbe_mrqc_reg_get(hw->mac.type);
+	rssrk_reg = ixgbe_rssrk_reg_get(hw->mac.type, 0);
 
 	hash_key = rss_conf->rss_key;
 	if (hash_key != NULL) {
@@ -2671,7 +2678,7 @@ ixgbe_hw_rss_hash_set(struct ixgbe_hw *hw, struct rte_eth_rss_conf *rss_conf)
 			rss_key |= hash_key[(i * 4) + 1] << 8;
 			rss_key |= hash_key[(i * 4) + 2] << 16;
 			rss_key |= hash_key[(i * 4) + 3] << 24;
-			IXGBE_WRITE_REG_ARRAY(hw, IXGBE_RSSRK(0), i, rss_key);
+			IXGBE_WRITE_REG_ARRAY(hw, rssrk_reg, i, rss_key);
 		}
 	}
 
@@ -2696,7 +2703,7 @@ ixgbe_hw_rss_hash_set(struct ixgbe_hw *hw, struct rte_eth_rss_conf *rss_conf)
 		mrqc |= IXGBE_MRQC_RSS_FIELD_IPV6_UDP;
 	if (rss_hf & ETH_RSS_IPV6_UDP_EX)
 		mrqc |= IXGBE_MRQC_RSS_FIELD_IPV6_EX_UDP;
-	IXGBE_WRITE_REG(hw, IXGBE_MRQC, mrqc);
+	IXGBE_WRITE_REG(hw, mrqc_reg, mrqc);
 }
 
 int
-- 
1.9.3

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

* [dpdk-dev] [PATCH v2 3/4] ixgbe: VF RSS reta/hash query and update
  2015-10-16 13:05 ` [dpdk-dev] [PATCH v2 " Wenzhuo Lu
  2015-10-16 13:05   ` [dpdk-dev] [PATCH v2 1/4] ixgbe: 512 entries RSS table on x550 Wenzhuo Lu
  2015-10-16 13:05   ` [dpdk-dev] [PATCH v2 2/4] ixgbe: VF RSS config " Wenzhuo Lu
@ 2015-10-16 13:05   ` Wenzhuo Lu
  2015-10-16 13:05   ` [dpdk-dev] [PATCH v2 4/4] doc: release notes update for RSS enhancement Wenzhuo Lu
  2015-10-16 15:10   ` [dpdk-dev] [PATCH v2 0/4] RSS enhancement on Intel x550 NIC Ananyev, Konstantin
  4 siblings, 0 replies; 18+ messages in thread
From: Wenzhuo Lu @ 2015-10-16 13:05 UTC (permalink / raw)
  To: dev

This patch implements the VF RSS reta/hash query and update function
on 10G NICs. But the update function is only provided for x550. Because
the other NICs don't have the separate registers for VF, we don't want
to let a VF NIC change the shared RSS reta/hash registers. It may cause
PF and other VF NICs' behavior change without being noticed.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 23 +++++++++++++++++++++++
 drivers/net/ixgbe/ixgbe_ethdev.h |  2 ++
 drivers/net/ixgbe/ixgbe_rxtx.c   | 18 +++++++++++++++---
 3 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index e2fbcfc..f4d823e 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -497,6 +497,10 @@ static const struct eth_dev_ops ixgbevf_eth_dev_ops = {
 	.mac_addr_set         = ixgbevf_set_default_mac_addr,
 	.get_reg_length       = ixgbevf_get_reg_length,
 	.get_reg              = ixgbevf_get_regs,
+	.reta_update          = ixgbe_dev_rss_reta_update,
+	.reta_query           = ixgbe_dev_rss_reta_query,
+	.rss_hash_update      = ixgbe_dev_rss_hash_update,
+	.rss_hash_conf_get    = ixgbe_dev_rss_hash_conf_get,
 };
 
 /* store statistics names and its offset in stats structure */
@@ -3207,6 +3211,13 @@ ixgbe_dev_rss_reta_update(struct rte_eth_dev *dev,
 	uint32_t reta_reg;
 
 	PMD_INIT_FUNC_TRACE();
+
+	if (!ixgbe_rss_update_sp(hw->mac.type)) {
+		PMD_DRV_LOG(ERR, "RSS reta update is not supported on this "
+			"NIC.");
+		return -ENOTSUP;
+	}
+
 	sp_reta_size = ixgbe_reta_size_get(hw->mac.type);
 	if (reta_size != sp_reta_size) {
 		PMD_DRV_LOG(ERR, "The size of hash lookup table configured "
@@ -5534,6 +5545,18 @@ ixgbe_rssrk_reg_get(enum ixgbe_mac_type mac_type, uint8_t i) {
 	}
 }
 
+bool
+ixgbe_rss_update_sp(enum ixgbe_mac_type mac_type) {
+	switch (mac_type) {
+	case ixgbe_mac_82599_vf:
+	case ixgbe_mac_X540_vf:
+		return 0;
+	default:
+		return 1;
+	}
+}
+
+
 static struct rte_driver rte_ixgbe_driver = {
 	.type = PMD_PDEV,
 	.init = rte_ixgbe_pmd_init,
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 441a17f..bfc1c01 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -385,6 +385,8 @@ uint32_t ixgbe_mrqc_reg_get(enum ixgbe_mac_type mac_type);
 
 uint32_t ixgbe_rssrk_reg_get(enum ixgbe_mac_type mac_type, uint8_t i);
 
+bool ixgbe_rss_update_sp(enum ixgbe_mac_type mac_type);
+
 /*
  * Flow director function prototypes
  */
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 0b8ca18..1158562 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -2713,9 +2713,17 @@ ixgbe_dev_rss_hash_update(struct rte_eth_dev *dev,
 	struct ixgbe_hw *hw;
 	uint32_t mrqc;
 	uint64_t rss_hf;
+	uint32_t mrqc_reg;
 
 	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
+	if (!ixgbe_rss_update_sp(hw->mac.type)) {
+		PMD_DRV_LOG(ERR, "RSS hash update is not supported on this "
+			"NIC.");
+		return -ENOTSUP;
+	}
+	mrqc_reg = ixgbe_mrqc_reg_get(hw->mac.type);
+
 	/*
 	 * Excerpt from section 7.1.2.8 Receive-Side Scaling (RSS):
 	 *     "RSS enabling cannot be done dynamically while it must be
@@ -2726,7 +2734,7 @@ ixgbe_dev_rss_hash_update(struct rte_eth_dev *dev,
 	 * disabled at initialization time.
 	 */
 	rss_hf = rss_conf->rss_hf & IXGBE_RSS_OFFLOAD_ALL;
-	mrqc = IXGBE_READ_REG(hw, IXGBE_MRQC);
+	mrqc = IXGBE_READ_REG(hw, mrqc_reg);
 	if (!(mrqc & IXGBE_MRQC_RSSEN)) { /* RSS disabled */
 		if (rss_hf != 0) /* Enable RSS */
 			return -(EINVAL);
@@ -2749,13 +2757,17 @@ ixgbe_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
 	uint32_t rss_key;
 	uint64_t rss_hf;
 	uint16_t i;
+	uint32_t mrqc_reg;
+	uint32_t rssrk_reg;
 
 	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	mrqc_reg = ixgbe_mrqc_reg_get(hw->mac.type);
+	rssrk_reg = ixgbe_rssrk_reg_get(hw->mac.type, 0);
 	hash_key = rss_conf->rss_key;
 	if (hash_key != NULL) {
 		/* Return RSS hash key */
 		for (i = 0; i < 10; i++) {
-			rss_key = IXGBE_READ_REG_ARRAY(hw, IXGBE_RSSRK(0), i);
+			rss_key = IXGBE_READ_REG_ARRAY(hw, rssrk_reg, i);
 			hash_key[(i * 4)] = rss_key & 0x000000FF;
 			hash_key[(i * 4) + 1] = (rss_key >> 8) & 0x000000FF;
 			hash_key[(i * 4) + 2] = (rss_key >> 16) & 0x000000FF;
@@ -2764,7 +2776,7 @@ ixgbe_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
 	}
 
 	/* Get RSS functions configured in MRQC register */
-	mrqc = IXGBE_READ_REG(hw, IXGBE_MRQC);
+	mrqc = IXGBE_READ_REG(hw, mrqc_reg);
 	if ((mrqc & IXGBE_MRQC_RSSEN) == 0) { /* RSS is disabled */
 		rss_conf->rss_hf = 0;
 		return 0;
-- 
1.9.3

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

* [dpdk-dev] [PATCH v2 4/4] doc: release notes update for RSS enhancement
  2015-10-16 13:05 ` [dpdk-dev] [PATCH v2 " Wenzhuo Lu
                     ` (2 preceding siblings ...)
  2015-10-16 13:05   ` [dpdk-dev] [PATCH v2 3/4] ixgbe: VF RSS reta/hash query and update Wenzhuo Lu
@ 2015-10-16 13:05   ` Wenzhuo Lu
  2015-10-16 15:10   ` [dpdk-dev] [PATCH v2 0/4] RSS enhancement on Intel x550 NIC Ananyev, Konstantin
  4 siblings, 0 replies; 18+ messages in thread
From: Wenzhuo Lu @ 2015-10-16 13:05 UTC (permalink / raw)
  To: dev

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 doc/guides/rel_notes/release_2_2.rst | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/guides/rel_notes/release_2_2.rst b/doc/guides/rel_notes/release_2_2.rst
index 5687676..d9d2a3d 100644
--- a/doc/guides/rel_notes/release_2_2.rst
+++ b/doc/guides/rel_notes/release_2_2.rst
@@ -4,6 +4,11 @@ DPDK Release 2.2
 New Features
 ------------
 
+* **ixgbe: RSS enhancement on Intel x550 NIC**
+
+  Support 512 entries RSS redirection table.
+  Support per VF RSS redirection table.
+
 
 Resolved Issues
 ---------------
-- 
1.9.3

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

* Re: [dpdk-dev] [PATCH v2 0/4] RSS enhancement on Intel x550 NIC
  2015-10-16 13:05 ` [dpdk-dev] [PATCH v2 " Wenzhuo Lu
                     ` (3 preceding siblings ...)
  2015-10-16 13:05   ` [dpdk-dev] [PATCH v2 4/4] doc: release notes update for RSS enhancement Wenzhuo Lu
@ 2015-10-16 15:10   ` Ananyev, Konstantin
  2015-10-28 17:22     ` Thomas Monjalon
  4 siblings, 1 reply; 18+ messages in thread
From: Ananyev, Konstantin @ 2015-10-16 15:10 UTC (permalink / raw)
  To: Lu, Wenzhuo, dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Friday, October 16, 2015 2:06 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v2 0/4] RSS enhancement on Intel x550 NIC
> 
> This patch set implements the RSS enhancement on x550.
> The enhancement includes, the PF RSS redirection table is enlarged
> from 128 entries to 512 entries, the VF doesn't share the same
> registers with PF and per VF RSS redirection table is provided.
> 
> V2:
> Condense the code.
> Add release notes.
> 
> Wenzhuo Lu (4):
>   ixgbe: 512 entries RSS table on x550
>   ixgbe: VF RSS config on x550
>   ixgbe: VF RSS reta/hash query and update
>   doc: release notes update for RSS enhancement
> 
>  doc/guides/rel_notes/release_2_2.rst |   5 ++
>  drivers/net/ixgbe/ixgbe_ethdev.c     | 102 +++++++++++++++++++++++++++++++----
>  drivers/net/ixgbe/ixgbe_ethdev.h     |  10 ++++
>  drivers/net/ixgbe/ixgbe_rxtx.c       |  43 +++++++++++----
>  4 files changed, 142 insertions(+), 18 deletions(-)
> 
> --

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Great work :)

> 1.9.3

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

* Re: [dpdk-dev] [PATCH v2 0/4] RSS enhancement on Intel x550 NIC
  2015-10-16 15:10   ` [dpdk-dev] [PATCH v2 0/4] RSS enhancement on Intel x550 NIC Ananyev, Konstantin
@ 2015-10-28 17:22     ` Thomas Monjalon
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Monjalon @ 2015-10-28 17:22 UTC (permalink / raw)
  To: Lu, Wenzhuo; +Cc: dev

> > This patch set implements the RSS enhancement on x550.
> > The enhancement includes, the PF RSS redirection table is enlarged
> > from 128 entries to 512 entries, the VF doesn't share the same
> > registers with PF and per VF RSS redirection table is provided.
> > 
> > V2:
> > Condense the code.
> > Add release notes.
> > 
> > Wenzhuo Lu (4):
> >   ixgbe: 512 entries RSS table on x550
> >   ixgbe: VF RSS config on x550
> >   ixgbe: VF RSS reta/hash query and update
> >   doc: release notes update for RSS enhancement
> 
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> Great work :)

Applied, thanks
The doc patch is split and merged in previous commits.

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

end of thread, other threads:[~2015-10-28 17:24 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-28  7:52 [dpdk-dev] [PATCH 0/4] RSS enhancement on Intel x550 NIC Wenzhuo Lu
2015-09-28  7:52 ` [dpdk-dev] [PATCH 1/4] ixgbe: 512 entries RSS table on x550 Wenzhuo Lu
2015-10-15 22:19   ` Ananyev, Konstantin
2015-09-28  7:52 ` [dpdk-dev] [PATCH 2/4] ixgbe: VF RSS config " Wenzhuo Lu
2015-10-15 22:42   ` Ananyev, Konstantin
2015-09-28  7:52 ` [dpdk-dev] [PATCH 3/4] ixgbe: VF RSS hash query and update Wenzhuo Lu
2015-10-15 22:53   ` [dpdk-dev] FW: " Ananyev, Konstantin
2015-09-28  7:52 ` [dpdk-dev] [PATCH 4/4] ixgbe: VF RSS reta " Wenzhuo Lu
2015-10-15 22:57   ` Ananyev, Konstantin
2015-10-16  1:21     ` Lu, Wenzhuo
2015-10-16 10:04 ` [dpdk-dev] [PATCH 0/4] RSS enhancement on Intel x550 NIC Nélio Laranjeiro
2015-10-16 13:05 ` [dpdk-dev] [PATCH v2 " Wenzhuo Lu
2015-10-16 13:05   ` [dpdk-dev] [PATCH v2 1/4] ixgbe: 512 entries RSS table on x550 Wenzhuo Lu
2015-10-16 13:05   ` [dpdk-dev] [PATCH v2 2/4] ixgbe: VF RSS config " Wenzhuo Lu
2015-10-16 13:05   ` [dpdk-dev] [PATCH v2 3/4] ixgbe: VF RSS reta/hash query and update Wenzhuo Lu
2015-10-16 13:05   ` [dpdk-dev] [PATCH v2 4/4] doc: release notes update for RSS enhancement Wenzhuo Lu
2015-10-16 15:10   ` [dpdk-dev] [PATCH v2 0/4] RSS enhancement on Intel x550 NIC Ananyev, Konstantin
2015-10-28 17:22     ` 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).