[dpdk-dev,4/4] ixgbe: VF RSS reta query and update

Message ID 1443426751-4906-5-git-send-email-wenzhuo.lu@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Wenzhuo Lu Sept. 28, 2015, 7:52 a.m. UTC
  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(+)
  

Comments

Ananyev, Konstantin Oct. 15, 2015, 10:57 p.m. UTC | #1
> -----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
  
Wenzhuo Lu Oct. 16, 2015, 1:21 a.m. UTC | #2
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
  

Patch

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,