diff mbox series

[v5] net/i40e: fix vf runtime queues rss config

Message ID 1565692851-36962-1-git-send-email-xiao.zhang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Qi Zhang
Headers
Series [v5] net/i40e: fix vf runtime queues rss config |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Xiao Zhang Aug. 13, 2019, 10:40 a.m. UTC 
  I40evf queue can not work properly with kernel pf driver. Eg. when
configure 8 queues pair, only 4 queues can receive packets, and half
packets will be lost if using 2 queues pair.
This issue is caused by misconfiguration of look up table, use aq
command to setup the lut to make it work properly.

Fixes: cea7a51c1750 ("i40evf: support RSS")
Cc: stable@dpdk.org

Signed-off-by: Xiao Zhang <xiao.zhang@intel.com>
---
v5 fix compile issue
v4 move local variable definition to the begin of the function
v3 move LUT configuration in to i40evf_configure_rss
v2 change for loop format to avoid build patch issue
---
 drivers/net/i40e/i40e_ethdev_vf.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

Comments

Xing, Beilei Aug. 13, 2019, 2:21 a.m. UTC | #1 
> -----Original Message-----
> From: Zhang, Xiao
> Sent: Tuesday, August 13, 2019 6:41 PM
> To: dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Xiao <xiao.zhang@intel.com>;
> stable@dpdk.org
> Subject: [v5] net/i40e: fix vf runtime queues rss config
> 
> I40evf queue can not work properly with kernel pf driver. Eg. when configure
> 8 queues pair, only 4 queues can receive packets, and half packets will be lost
> if using 2 queues pair.
> This issue is caused by misconfiguration of look up table, use aq command to
> setup the lut to make it work properly.
> 
> Fixes: cea7a51c1750 ("i40evf: support RSS")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Xiao Zhang <xiao.zhang@intel.com>

Acked-by: Beilei Xing <beilei.xing@intel.com>
Xiaolong Ye Aug. 13, 2019, 6:28 a.m. UTC | #2 
Hi, Xiao

On 08/13, Xiao Zhang wrote:
>I40evf queue can not work properly with kernel pf driver. Eg. when
>configure 8 queues pair, only 4 queues can receive packets, and half
>packets will be lost if using 2 queues pair.
>This issue is caused by misconfiguration of look up table, use aq
>command to setup the lut to make it work properly.

So the original code of lookup table configuration is problematic? Can we just
remove them?

Thanks,
Xiaolong

>
>Fixes: cea7a51c1750 ("i40evf: support RSS")
>Cc: stable@dpdk.org
>
>Signed-off-by: Xiao Zhang <xiao.zhang@intel.com>
>---
>v5 fix compile issue
>v4 move local variable definition to the begin of the function
>v3 move LUT configuration in to i40evf_configure_rss
>v2 change for loop format to avoid build patch issue
>---
> drivers/net/i40e/i40e_ethdev_vf.c | 32 ++++++++++++++++++++++++++------
> 1 file changed, 26 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
>index 308fb98..c77b30c 100644
>--- a/drivers/net/i40e/i40e_ethdev_vf.c
>+++ b/drivers/net/i40e/i40e_ethdev_vf.c
>@@ -2598,7 +2598,10 @@ i40evf_config_rss(struct i40e_vf *vf)
> 	struct i40e_hw *hw = I40E_VF_TO_HW(vf);
> 	struct rte_eth_rss_conf rss_conf;
> 	uint32_t i, j, lut = 0, nb_q = (I40E_VFQF_HLUT_MAX_INDEX + 1) * 4;
>+	uint32_t rss_lut_size = (I40E_VFQF_HLUT1_MAX_INDEX + 1) * 4;
> 	uint16_t num;
>+	uint8_t *lut_info;
>+	int ret;
> 
> 	if (vf->dev_data->dev_conf.rxmode.mq_mode != ETH_MQ_RX_RSS) {
> 		i40evf_disable_rss(vf);
>@@ -2608,12 +2611,29 @@ i40evf_config_rss(struct i40e_vf *vf)
> 
> 	num = RTE_MIN(vf->dev_data->nb_rx_queues, I40E_MAX_QP_NUM_PER_VF);
> 	/* Fill out the look up table */
>-	for (i = 0, j = 0; i < nb_q; i++, j++) {
>-		if (j >= num)
>-			j = 0;
>-		lut = (lut << 8) | j;
>-		if ((i & 3) == 3)
>-			I40E_WRITE_REG(hw, I40E_VFQF_HLUT(i >> 2), lut);
>+	if (!(vf->flags & I40E_FLAG_RSS_AQ_CAPABLE)) {
>+		for (i = 0, j = 0; i < nb_q; i++, j++) {
>+			if (j >= num)
>+				j = 0;
>+			lut = (lut << 8) | j;
>+			if ((i & 3) == 3)
>+				I40E_WRITE_REG(hw, I40E_VFQF_HLUT(i >> 2), lut);
>+		}
>+	} else {
>+		lut_info = rte_zmalloc("i40e_rss_lut", rss_lut_size, 0);
>+		if (!lut_info) {
>+			PMD_DRV_LOG(ERR, "No memory can be allocated");
>+			return -ENOMEM;
>+		}
>+
>+		for (i = 0; i < rss_lut_size; i++)
>+			lut_info[i] = i % vf->num_queue_pairs;
>+
>+		ret = i40evf_set_rss_lut(&vf->vsi, lut_info,
>+					 rss_lut_size);
>+		rte_free(lut_info);
>+		if (ret)
>+			return ret;
> 	}
> 
> 	rss_conf = vf->dev_data->dev_conf.rx_adv_conf.rss_conf;
>-- 
>2.7.4
>
Xiao Zhang Aug. 13, 2019, 7:24 a.m. UTC | #3 
> -----Original Message-----
> From: Ye, Xiaolong
> Sent: Tuesday, August 13, 2019 2:28 PM
> To: Zhang, Xiao <xiao.zhang@intel.com>
> Cc: dev@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; stable@dpdk.org
> Subject: Re: [dpdk-dev] [v5] net/i40e: fix vf runtime queues rss config
> 
> Hi, Xiao
> 
> On 08/13, Xiao Zhang wrote:
> >I40evf queue can not work properly with kernel pf driver. Eg. when
> >configure 8 queues pair, only 4 queues can receive packets, and half
> >packets will be lost if using 2 queues pair.
> >This issue is caused by misconfiguration of look up table, use aq
> >command to setup the lut to make it work properly.
> 
> So the original code of lookup table configuration is problematic? Can we just
> remove them?

The original code does not work with device X722 VF. For other devices using i40evf, the original code works.
The commit message may missed this information.
And the new code only workable for devices capable with AQ command, so we can not remove the original code.

> 
> Thanks,
> Xiaolong
> 
> >
> >Fixes: cea7a51c1750 ("i40evf: support RSS")
> >Cc: stable@dpdk.org
> >
> >Signed-off-by: Xiao Zhang <xiao.zhang@intel.com>
> >---
> >v5 fix compile issue
> >v4 move local variable definition to the begin of the function
> >v3 move LUT configuration in to i40evf_configure_rss
> >v2 change for loop format to avoid build patch issue
> >---
> > drivers/net/i40e/i40e_ethdev_vf.c | 32
> >++++++++++++++++++++++++++------
> > 1 file changed, 26 insertions(+), 6 deletions(-)
> >
> >diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> >b/drivers/net/i40e/i40e_ethdev_vf.c
> >index 308fb98..c77b30c 100644
> >--- a/drivers/net/i40e/i40e_ethdev_vf.c
> >+++ b/drivers/net/i40e/i40e_ethdev_vf.c
> >@@ -2598,7 +2598,10 @@ i40evf_config_rss(struct i40e_vf *vf)
> > 	struct i40e_hw *hw = I40E_VF_TO_HW(vf);
> > 	struct rte_eth_rss_conf rss_conf;
> > 	uint32_t i, j, lut = 0, nb_q = (I40E_VFQF_HLUT_MAX_INDEX + 1) * 4;
> >+	uint32_t rss_lut_size = (I40E_VFQF_HLUT1_MAX_INDEX + 1) * 4;
> > 	uint16_t num;
> >+	uint8_t *lut_info;
> >+	int ret;
> >
> > 	if (vf->dev_data->dev_conf.rxmode.mq_mode != ETH_MQ_RX_RSS) {
> > 		i40evf_disable_rss(vf);
> >@@ -2608,12 +2611,29 @@ i40evf_config_rss(struct i40e_vf *vf)
> >
> > 	num = RTE_MIN(vf->dev_data->nb_rx_queues,
> I40E_MAX_QP_NUM_PER_VF);
> > 	/* Fill out the look up table */
> >-	for (i = 0, j = 0; i < nb_q; i++, j++) {
> >-		if (j >= num)
> >-			j = 0;
> >-		lut = (lut << 8) | j;
> >-		if ((i & 3) == 3)
> >-			I40E_WRITE_REG(hw, I40E_VFQF_HLUT(i >> 2), lut);
> >+	if (!(vf->flags & I40E_FLAG_RSS_AQ_CAPABLE)) {
> >+		for (i = 0, j = 0; i < nb_q; i++, j++) {
> >+			if (j >= num)
> >+				j = 0;
> >+			lut = (lut << 8) | j;
> >+			if ((i & 3) == 3)
> >+				I40E_WRITE_REG(hw, I40E_VFQF_HLUT(i >> 2),
> lut);
> >+		}
> >+	} else {
> >+		lut_info = rte_zmalloc("i40e_rss_lut", rss_lut_size, 0);
> >+		if (!lut_info) {
> >+			PMD_DRV_LOG(ERR, "No memory can be allocated");
> >+			return -ENOMEM;
> >+		}
> >+
> >+		for (i = 0; i < rss_lut_size; i++)
> >+			lut_info[i] = i % vf->num_queue_pairs;
> >+
> >+		ret = i40evf_set_rss_lut(&vf->vsi, lut_info,
> >+					 rss_lut_size);
> >+		rte_free(lut_info);
> >+		if (ret)
> >+			return ret;
> > 	}
> >
> > 	rss_conf = vf->dev_data->dev_conf.rx_adv_conf.rss_conf;
> >--
> >2.7.4
> >
Xiaolong Ye Aug. 13, 2019, 7:37 a.m. UTC | #4 
On 08/13, Zhang, Xiao wrote:
>
>
>> -----Original Message-----
>> From: Ye, Xiaolong
>> Sent: Tuesday, August 13, 2019 2:28 PM
>> To: Zhang, Xiao <xiao.zhang@intel.com>
>> Cc: dev@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; stable@dpdk.org
>> Subject: Re: [dpdk-dev] [v5] net/i40e: fix vf runtime queues rss config
>> 
>> Hi, Xiao
>> 
>> On 08/13, Xiao Zhang wrote:
>> >I40evf queue can not work properly with kernel pf driver. Eg. when
>> >configure 8 queues pair, only 4 queues can receive packets, and half
>> >packets will be lost if using 2 queues pair.
>> >This issue is caused by misconfiguration of look up table, use aq
>> >command to setup the lut to make it work properly.
>> 
>> So the original code of lookup table configuration is problematic? Can we just
>> remove them?
>
>The original code does not work with device X722 VF. For other devices using i40evf, the original code works.
>The commit message may missed this information.
>And the new code only workable for devices capable with AQ command, so we can not remove the original code.

Ok, can you add these info in your commit message and send a new version?

Thanks,
Xiaolong

>
>> 
>> Thanks,
>> Xiaolong
>> 
>> >
>> >Fixes: cea7a51c1750 ("i40evf: support RSS")
>> >Cc: stable@dpdk.org
>> >
>> >Signed-off-by: Xiao Zhang <xiao.zhang@intel.com>
>> >---
>> >v5 fix compile issue
>> >v4 move local variable definition to the begin of the function
>> >v3 move LUT configuration in to i40evf_configure_rss
>> >v2 change for loop format to avoid build patch issue
>> >---
>> > drivers/net/i40e/i40e_ethdev_vf.c | 32
>> >++++++++++++++++++++++++++------
>> > 1 file changed, 26 insertions(+), 6 deletions(-)
>> >
>> >diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
>> >b/drivers/net/i40e/i40e_ethdev_vf.c
>> >index 308fb98..c77b30c 100644
>> >--- a/drivers/net/i40e/i40e_ethdev_vf.c
>> >+++ b/drivers/net/i40e/i40e_ethdev_vf.c
>> >@@ -2598,7 +2598,10 @@ i40evf_config_rss(struct i40e_vf *vf)
>> > 	struct i40e_hw *hw = I40E_VF_TO_HW(vf);
>> > 	struct rte_eth_rss_conf rss_conf;
>> > 	uint32_t i, j, lut = 0, nb_q = (I40E_VFQF_HLUT_MAX_INDEX + 1) * 4;
>> >+	uint32_t rss_lut_size = (I40E_VFQF_HLUT1_MAX_INDEX + 1) * 4;
>> > 	uint16_t num;
>> >+	uint8_t *lut_info;
>> >+	int ret;
>> >
>> > 	if (vf->dev_data->dev_conf.rxmode.mq_mode != ETH_MQ_RX_RSS) {
>> > 		i40evf_disable_rss(vf);
>> >@@ -2608,12 +2611,29 @@ i40evf_config_rss(struct i40e_vf *vf)
>> >
>> > 	num = RTE_MIN(vf->dev_data->nb_rx_queues,
>> I40E_MAX_QP_NUM_PER_VF);
>> > 	/* Fill out the look up table */
>> >-	for (i = 0, j = 0; i < nb_q; i++, j++) {
>> >-		if (j >= num)
>> >-			j = 0;
>> >-		lut = (lut << 8) | j;
>> >-		if ((i & 3) == 3)
>> >-			I40E_WRITE_REG(hw, I40E_VFQF_HLUT(i >> 2), lut);
>> >+	if (!(vf->flags & I40E_FLAG_RSS_AQ_CAPABLE)) {
>> >+		for (i = 0, j = 0; i < nb_q; i++, j++) {
>> >+			if (j >= num)
>> >+				j = 0;
>> >+			lut = (lut << 8) | j;
>> >+			if ((i & 3) == 3)
>> >+				I40E_WRITE_REG(hw, I40E_VFQF_HLUT(i >> 2),
>> lut);
>> >+		}
>> >+	} else {
>> >+		lut_info = rte_zmalloc("i40e_rss_lut", rss_lut_size, 0);
>> >+		if (!lut_info) {
>> >+			PMD_DRV_LOG(ERR, "No memory can be allocated");
>> >+			return -ENOMEM;
>> >+		}
>> >+
>> >+		for (i = 0; i < rss_lut_size; i++)
>> >+			lut_info[i] = i % vf->num_queue_pairs;
>> >+
>> >+		ret = i40evf_set_rss_lut(&vf->vsi, lut_info,
>> >+					 rss_lut_size);
>> >+		rte_free(lut_info);
>> >+		if (ret)
>> >+			return ret;
>> > 	}
>> >
>> > 	rss_conf = vf->dev_data->dev_conf.rx_adv_conf.rss_conf;
>> >--
>> >2.7.4
>> >
Xiao Zhang Aug. 13, 2019, 7:59 a.m. UTC | #5 
> -----Original Message-----
> From: Ye, Xiaolong
> Sent: Tuesday, August 13, 2019 3:37 PM
> To: Zhang, Xiao <xiao.zhang@intel.com>
> Cc: dev@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; stable@dpdk.org
> Subject: Re: [dpdk-dev] [v5] net/i40e: fix vf runtime queues rss config
> 
> On 08/13, Zhang, Xiao wrote:
> >
> >
> >> -----Original Message-----
> >> From: Ye, Xiaolong
> >> Sent: Tuesday, August 13, 2019 2:28 PM
> >> To: Zhang, Xiao <xiao.zhang@intel.com>
> >> Cc: dev@dpdk.org; Xing, Beilei <beilei.xing@intel.com>;
> >> stable@dpdk.org
> >> Subject: Re: [dpdk-dev] [v5] net/i40e: fix vf runtime queues rss
> >> config
> >>
> >> Hi, Xiao
> >>
> >> On 08/13, Xiao Zhang wrote:
> >> >I40evf queue can not work properly with kernel pf driver. Eg. when
> >> >configure 8 queues pair, only 4 queues can receive packets, and half
> >> >packets will be lost if using 2 queues pair.
> >> >This issue is caused by misconfiguration of look up table, use aq
> >> >command to setup the lut to make it work properly.
> >>
> >> So the original code of lookup table configuration is problematic?
> >> Can we just remove them?
> >
> >The original code does not work with device X722 VF. For other devices using
> i40evf, the original code works.
> >The commit message may missed this information.
> >And the new code only workable for devices capable with AQ command, so we
> can not remove the original code.
> 
> Ok, can you add these info in your commit message and send a new version?
> 

Yes, it's ok.

> Thanks,
> Xiaolong
> 
> >
> >>
> >> Thanks,
> >> Xiaolong
> >>
> >> >
> >> >Fixes: cea7a51c1750 ("i40evf: support RSS")
> >> >Cc: stable@dpdk.org
> >> >
> >> >Signed-off-by: Xiao Zhang <xiao.zhang@intel.com>
> >> >---
> >> >v5 fix compile issue
> >> >v4 move local variable definition to the begin of the function
> >> >v3 move LUT configuration in to i40evf_configure_rss
> >> >v2 change for loop format to avoid build patch issue
> >> >---
> >> > drivers/net/i40e/i40e_ethdev_vf.c | 32
> >> >++++++++++++++++++++++++++------
> >> > 1 file changed, 26 insertions(+), 6 deletions(-)
> >> >
> >> >diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> >> >b/drivers/net/i40e/i40e_ethdev_vf.c
> >> >index 308fb98..c77b30c 100644
> >> >--- a/drivers/net/i40e/i40e_ethdev_vf.c
> >> >+++ b/drivers/net/i40e/i40e_ethdev_vf.c
> >> >@@ -2598,7 +2598,10 @@ i40evf_config_rss(struct i40e_vf *vf)
> >> > 	struct i40e_hw *hw = I40E_VF_TO_HW(vf);
> >> > 	struct rte_eth_rss_conf rss_conf;
> >> > 	uint32_t i, j, lut = 0, nb_q = (I40E_VFQF_HLUT_MAX_INDEX + 1) * 4;
> >> >+	uint32_t rss_lut_size = (I40E_VFQF_HLUT1_MAX_INDEX + 1) * 4;
> >> > 	uint16_t num;
> >> >+	uint8_t *lut_info;
> >> >+	int ret;
> >> >
> >> > 	if (vf->dev_data->dev_conf.rxmode.mq_mode != ETH_MQ_RX_RSS) {
> >> > 		i40evf_disable_rss(vf);
> >> >@@ -2608,12 +2611,29 @@ i40evf_config_rss(struct i40e_vf *vf)
> >> >
> >> > 	num = RTE_MIN(vf->dev_data->nb_rx_queues,
> >> I40E_MAX_QP_NUM_PER_VF);
> >> > 	/* Fill out the look up table */
> >> >-	for (i = 0, j = 0; i < nb_q; i++, j++) {
> >> >-		if (j >= num)
> >> >-			j = 0;
> >> >-		lut = (lut << 8) | j;
> >> >-		if ((i & 3) == 3)
> >> >-			I40E_WRITE_REG(hw, I40E_VFQF_HLUT(i >> 2), lut);
> >> >+	if (!(vf->flags & I40E_FLAG_RSS_AQ_CAPABLE)) {
> >> >+		for (i = 0, j = 0; i < nb_q; i++, j++) {
> >> >+			if (j >= num)
> >> >+				j = 0;
> >> >+			lut = (lut << 8) | j;
> >> >+			if ((i & 3) == 3)
> >> >+				I40E_WRITE_REG(hw, I40E_VFQF_HLUT(i >> 2),
> >> lut);
> >> >+		}
> >> >+	} else {
> >> >+		lut_info = rte_zmalloc("i40e_rss_lut", rss_lut_size, 0);
> >> >+		if (!lut_info) {
> >> >+			PMD_DRV_LOG(ERR, "No memory can be allocated");
> >> >+			return -ENOMEM;
> >> >+		}
> >> >+
> >> >+		for (i = 0; i < rss_lut_size; i++)
> >> >+			lut_info[i] = i % vf->num_queue_pairs;
> >> >+
> >> >+		ret = i40evf_set_rss_lut(&vf->vsi, lut_info,
> >> >+					 rss_lut_size);
> >> >+		rte_free(lut_info);
> >> >+		if (ret)
> >> >+			return ret;
> >> > 	}
> >> >
> >> > 	rss_conf = vf->dev_data->dev_conf.rx_adv_conf.rss_conf;
> >> >--
> >> >2.7.4
> >> >
diff mbox series

Patch

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 308fb98..c77b30c 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -2598,7 +2598,10 @@  i40evf_config_rss(struct i40e_vf *vf)
 	struct i40e_hw *hw = I40E_VF_TO_HW(vf);
 	struct rte_eth_rss_conf rss_conf;
 	uint32_t i, j, lut = 0, nb_q = (I40E_VFQF_HLUT_MAX_INDEX + 1) * 4;
+	uint32_t rss_lut_size = (I40E_VFQF_HLUT1_MAX_INDEX + 1) * 4;
 	uint16_t num;
+	uint8_t *lut_info;
+	int ret;
 
 	if (vf->dev_data->dev_conf.rxmode.mq_mode != ETH_MQ_RX_RSS) {
 		i40evf_disable_rss(vf);
@@ -2608,12 +2611,29 @@  i40evf_config_rss(struct i40e_vf *vf)
 
 	num = RTE_MIN(vf->dev_data->nb_rx_queues, I40E_MAX_QP_NUM_PER_VF);
 	/* Fill out the look up table */
-	for (i = 0, j = 0; i < nb_q; i++, j++) {
-		if (j >= num)
-			j = 0;
-		lut = (lut << 8) | j;
-		if ((i & 3) == 3)
-			I40E_WRITE_REG(hw, I40E_VFQF_HLUT(i >> 2), lut);
+	if (!(vf->flags & I40E_FLAG_RSS_AQ_CAPABLE)) {
+		for (i = 0, j = 0; i < nb_q; i++, j++) {
+			if (j >= num)
+				j = 0;
+			lut = (lut << 8) | j;
+			if ((i & 3) == 3)
+				I40E_WRITE_REG(hw, I40E_VFQF_HLUT(i >> 2), lut);
+		}
+	} else {
+		lut_info = rte_zmalloc("i40e_rss_lut", rss_lut_size, 0);
+		if (!lut_info) {
+			PMD_DRV_LOG(ERR, "No memory can be allocated");
+			return -ENOMEM;
+		}
+
+		for (i = 0; i < rss_lut_size; i++)
+			lut_info[i] = i % vf->num_queue_pairs;
+
+		ret = i40evf_set_rss_lut(&vf->vsi, lut_info,
+					 rss_lut_size);
+		rte_free(lut_info);
+		if (ret)
+			return ret;
 	}
 
 	rss_conf = vf->dev_data->dev_conf.rx_adv_conf.rss_conf;