DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/i40e: fix core dumped when setting txq or rxq to 0 in VF
@ 2019-06-21  4:34 Andy Pei
  2019-06-21  8:26 ` [dpdk-dev] [PATCH v2] " Andy Pei
  2019-06-21 11:42 ` [dpdk-dev] [PATCH] " Ye Xiaolong
  0 siblings, 2 replies; 10+ messages in thread
From: Andy Pei @ 2019-06-21  4:34 UTC (permalink / raw)
  To: dev
  Cc: andy.pei, helin.zhang, stable, roy.fan.zhang, qi.z.zhang,
	jingjing.wu, beilei.xing, ferruh.yigit, rosen.xu

Testpmd stucked and core dumped
when set invalid VF queue number.
This patch fix this issue.

Fixes: d6b19729093e ("i40evf: support configurable crc stripping")
Cc: helin.zhang@intel.com
Cc: stable@dpdk.org

Signed-off-by: Andy Pei <andy.pei@intel.com>
---
Cc: roy.fan.zhang@intel.com
Cc: qi.z.zhang@intel.com
Cc: jingjing.wu@intel.com
Cc: beilei.xing@intel.com
Cc: ferruh.yigit@intel.com
Cc: rosen.xu@intel.com

 drivers/net/i40e/i40e_ethdev_vf.c | 35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 63dbe14..7096cc5 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -573,7 +573,7 @@ struct rte_i40evf_xstats_name_off {
 {
 	txq_info->vsi_id = vsi_id;
 	txq_info->queue_id = queue_id;
-	if (queue_id < nb_txq) {
+	if ((queue_id < nb_txq) && txq) {
 		txq_info->ring_len = txq->nb_tx_desc;
 		txq_info->dma_ring_addr = txq->tx_ring_phys_addr;
 	}
@@ -590,7 +590,7 @@ struct rte_i40evf_xstats_name_off {
 	rxq_info->vsi_id = vsi_id;
 	rxq_info->queue_id = queue_id;
 	rxq_info->max_pkt_size = max_pkt_size;
-	if (queue_id < nb_rxq) {
+	if ((queue_id < nb_rxq) && rxq) {
 		rxq_info->ring_len = rxq->nb_rx_desc;
 		rxq_info->dma_ring_addr = rxq->rx_ring_phys_addr;
 		rxq_info->databuffer_size =
@@ -622,11 +622,32 @@ struct rte_i40evf_xstats_name_off {
 	vc_vqci->num_queue_pairs = nb_qp;
 
 	for (i = 0, vc_qpi = vc_vqci->qpair; i < nb_qp; i++, vc_qpi++) {
-		i40evf_fill_virtchnl_vsi_txq_info(&vc_qpi->txq,
-			vc_vqci->vsi_id, i, dev->data->nb_tx_queues, txq[i]);
-		i40evf_fill_virtchnl_vsi_rxq_info(&vc_qpi->rxq,
-			vc_vqci->vsi_id, i, dev->data->nb_rx_queues,
-					vf->max_pkt_len, rxq[i]);
+		if (!txq)
+			i40evf_fill_virtchnl_vsi_txq_info(&vc_qpi->txq,
+							vc_vqci->vsi_id,
+							i,
+							dev->data->nb_tx_queues,
+							NULL);
+		else
+			i40evf_fill_virtchnl_vsi_txq_info(&vc_qpi->txq,
+							vc_vqci->vsi_id,
+							i,
+							dev->data->nb_tx_queues,
+							txq[i]);
+		if (!rxq)
+			i40evf_fill_virtchnl_vsi_rxq_info(&vc_qpi->rxq,
+							vc_vqci->vsi_id,
+							i,
+							dev->data->nb_rx_queues,
+							vf->max_pkt_len,
+							NULL);
+		else
+			i40evf_fill_virtchnl_vsi_rxq_info(&vc_qpi->rxq,
+							vc_vqci->vsi_id,
+							i,
+							dev->data->nb_rx_queues,
+							vf->max_pkt_len,
+							rxq[i]);
 	}
 	memset(&args, 0, sizeof(args));
 	args.ops = VIRTCHNL_OP_CONFIG_VSI_QUEUES;
-- 
1.8.3.1


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

* Re: [dpdk-dev] [PATCH] net/i40e: fix core dumped when setting txq or rxq to 0 in VF
  2019-06-21 11:42 ` [dpdk-dev] [PATCH] " Ye Xiaolong
@ 2019-06-21  8:17   ` Pei, Andy
  2019-06-21  9:02   ` Pei, Andy
  1 sibling, 0 replies; 10+ messages in thread
From: Pei, Andy @ 2019-06-21  8:17 UTC (permalink / raw)
  To: Ye, Xiaolong
  Cc: dev, Zhang, Helin, stable, Zhang, Roy Fan, Zhang, Qi Z, Wu,
	Jingjing, Xing, Beilei, Yigit, Ferruh, Xu, Rosen

HI Xiaolong,

Will do in this way in V2.



Thanks 

-----Original Message-----
From: Ye, Xiaolong 
Sent: Friday, June 21, 2019 7:42 PM
To: Pei, Andy <andy.pei@intel.com>
Cc: dev@dpdk.org; Zhang, Helin <helin.zhang@intel.com>; stable@dpdk.org; Zhang, Roy Fan <roy.fan.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Xu, Rosen <rosen.xu@intel.com>
Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix core dumped when setting txq or rxq to 0 in VF

On 06/21, Andy Pei wrote:
>Testpmd stucked and core dumped
>when set invalid VF queue number.
>This patch fix this issue.

It's better to wrap the commit message within 72 characters. Perhaps something
like:

Testpmd would stuck and result in core dump when user specifies an invalid VF queue number. This patch fixes this issue.

What do you think?
>
>Fixes: d6b19729093e ("i40evf: support configurable crc stripping")
>Cc: helin.zhang@intel.com
>Cc: stable@dpdk.org
>
>Signed-off-by: Andy Pei <andy.pei@intel.com>
>---
>Cc: roy.fan.zhang@intel.com
>Cc: qi.z.zhang@intel.com
>Cc: jingjing.wu@intel.com
>Cc: beilei.xing@intel.com
>Cc: ferruh.yigit@intel.com
>Cc: rosen.xu@intel.com
>
> drivers/net/i40e/i40e_ethdev_vf.c | 35 
> ++++++++++++++++++++++++++++-------
> 1 file changed, 28 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/net/i40e/i40e_ethdev_vf.c 
>b/drivers/net/i40e/i40e_ethdev_vf.c
>index 63dbe14..7096cc5 100644
>--- a/drivers/net/i40e/i40e_ethdev_vf.c
>+++ b/drivers/net/i40e/i40e_ethdev_vf.c
>@@ -573,7 +573,7 @@ struct rte_i40evf_xstats_name_off {  {
> 	txq_info->vsi_id = vsi_id;
> 	txq_info->queue_id = queue_id;
>-	if (queue_id < nb_txq) {
>+	if ((queue_id < nb_txq) && txq) {
> 		txq_info->ring_len = txq->nb_tx_desc;
> 		txq_info->dma_ring_addr = txq->tx_ring_phys_addr;
> 	}
>@@ -590,7 +590,7 @@ struct rte_i40evf_xstats_name_off {
> 	rxq_info->vsi_id = vsi_id;
> 	rxq_info->queue_id = queue_id;
> 	rxq_info->max_pkt_size = max_pkt_size;
>-	if (queue_id < nb_rxq) {
>+	if ((queue_id < nb_rxq) && rxq) {
> 		rxq_info->ring_len = rxq->nb_rx_desc;
> 		rxq_info->dma_ring_addr = rxq->rx_ring_phys_addr;
> 		rxq_info->databuffer_size =
>@@ -622,11 +622,32 @@ struct rte_i40evf_xstats_name_off {
> 	vc_vqci->num_queue_pairs = nb_qp;
> 
> 	for (i = 0, vc_qpi = vc_vqci->qpair; i < nb_qp; i++, vc_qpi++) {
>-		i40evf_fill_virtchnl_vsi_txq_info(&vc_qpi->txq,
>-			vc_vqci->vsi_id, i, dev->data->nb_tx_queues, txq[i]);
>-		i40evf_fill_virtchnl_vsi_rxq_info(&vc_qpi->rxq,
>-			vc_vqci->vsi_id, i, dev->data->nb_rx_queues,
>-					vf->max_pkt_len, rxq[i]);
>+		if (!txq)
>+			i40evf_fill_virtchnl_vsi_txq_info(&vc_qpi->txq,
>+							vc_vqci->vsi_id,
>+							i,
>+							dev->data->nb_tx_queues,
>+							NULL);
>+		else
>+			i40evf_fill_virtchnl_vsi_txq_info(&vc_qpi->txq,
>+							vc_vqci->vsi_id,
>+							i,
>+							dev->data->nb_tx_queues,
>+							txq[i]);
>+		if (!rxq)
>+			i40evf_fill_virtchnl_vsi_rxq_info(&vc_qpi->rxq,
>+							vc_vqci->vsi_id,
>+							i,
>+							dev->data->nb_rx_queues,
>+							vf->max_pkt_len,
>+							NULL);
>+		else
>+			i40evf_fill_virtchnl_vsi_rxq_info(&vc_qpi->rxq,
>+							vc_vqci->vsi_id,
>+							i,
>+							dev->data->nb_rx_queues,
>+							vf->max_pkt_len,
>+							rxq[i]);

No need to use one line for each parameter, I think you can still use the old format.


Thanks,
Xiaolong
> 	}
> 	memset(&args, 0, sizeof(args));
> 	args.ops = VIRTCHNL_OP_CONFIG_VSI_QUEUES;
>--
>1.8.3.1
>

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

* [dpdk-dev] [PATCH v2] net/i40e: fix core dumped when setting txq or rxq to 0 in VF
  2019-06-21  4:34 [dpdk-dev] [PATCH] net/i40e: fix core dumped when setting txq or rxq to 0 in VF Andy Pei
@ 2019-06-21  8:26 ` Andy Pei
  2019-06-21  9:23   ` [dpdk-dev] [PATCH v3] " Andy Pei
  2019-06-21 11:42 ` [dpdk-dev] [PATCH] " Ye Xiaolong
  1 sibling, 1 reply; 10+ messages in thread
From: Andy Pei @ 2019-06-21  8:26 UTC (permalink / raw)
  To: dev
  Cc: andy.pei, helin.zhang, stable, roy.fan.zhang, qi.z.zhang,
	jingjing.wu, beilei.xing, ferruh.yigit, rosen.xu, xiaolong.ye

Testpmd would stuck and result in core dump when user specifies an
invalid VF queue number. This patch fixes this issue.

Fixes: d6b19729093e ("i40evf: support configurable crc stripping")
Cc: helin.zhang@intel.com
Cc: stable@dpdk.org

Signed-off-by: Andy Pei <andy.pei@intel.com>
---
Cc: roy.fan.zhang@intel.com
Cc: qi.z.zhang@intel.com
Cc: jingjing.wu@intel.com
Cc: beilei.xing@intel.com
Cc: ferruh.yigit@intel.com
Cc: rosen.xu@intel.com
Cc: xiaolong.ye@intel.com

 drivers/net/i40e/i40e_ethdev_vf.c | 35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 63dbe14..35df59e 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -573,7 +573,7 @@ struct rte_i40evf_xstats_name_off {
 {
 	txq_info->vsi_id = vsi_id;
 	txq_info->queue_id = queue_id;
-	if (queue_id < nb_txq) {
+	if (queue_id < nb_txq && txq) {
 		txq_info->ring_len = txq->nb_tx_desc;
 		txq_info->dma_ring_addr = txq->tx_ring_phys_addr;
 	}
@@ -590,7 +590,7 @@ struct rte_i40evf_xstats_name_off {
 	rxq_info->vsi_id = vsi_id;
 	rxq_info->queue_id = queue_id;
 	rxq_info->max_pkt_size = max_pkt_size;
-	if (queue_id < nb_rxq) {
+	if (queue_id < nb_rxq && rxq) {
 		rxq_info->ring_len = rxq->nb_rx_desc;
 		rxq_info->dma_ring_addr = rxq->rx_ring_phys_addr;
 		rxq_info->databuffer_size =
@@ -622,11 +622,32 @@ struct rte_i40evf_xstats_name_off {
 	vc_vqci->num_queue_pairs = nb_qp;
 
 	for (i = 0, vc_qpi = vc_vqci->qpair; i < nb_qp; i++, vc_qpi++) {
-		i40evf_fill_virtchnl_vsi_txq_info(&vc_qpi->txq,
-			vc_vqci->vsi_id, i, dev->data->nb_tx_queues, txq[i]);
-		i40evf_fill_virtchnl_vsi_rxq_info(&vc_qpi->rxq,
-			vc_vqci->vsi_id, i, dev->data->nb_rx_queues,
-					vf->max_pkt_len, rxq[i]);
+		if (!txq)
+			i40evf_fill_virtchnl_vsi_txq_info(&vc_qpi->txq,
+							vc_vqci->vsi_id,
+							i,
+							dev->data->nb_tx_queues,
+							NULL);
+		else
+			i40evf_fill_virtchnl_vsi_txq_info(&vc_qpi->txq,
+							vc_vqci->vsi_id,
+							i,
+							dev->data->nb_tx_queues,
+							txq[i]);
+		if (!rxq)
+			i40evf_fill_virtchnl_vsi_rxq_info(&vc_qpi->rxq,
+							vc_vqci->vsi_id,
+							i,
+							dev->data->nb_rx_queues,
+							vf->max_pkt_len,
+							NULL);
+		else
+			i40evf_fill_virtchnl_vsi_rxq_info(&vc_qpi->rxq,
+							vc_vqci->vsi_id,
+							i,
+							dev->data->nb_rx_queues,
+							vf->max_pkt_len,
+							rxq[i]);
 	}
 	memset(&args, 0, sizeof(args));
 	args.ops = VIRTCHNL_OP_CONFIG_VSI_QUEUES;
-- 
1.8.3.1


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

* Re: [dpdk-dev] [PATCH] net/i40e: fix core dumped when setting txq or rxq to 0 in VF
  2019-06-21 11:42 ` [dpdk-dev] [PATCH] " Ye Xiaolong
  2019-06-21  8:17   ` Pei, Andy
@ 2019-06-21  9:02   ` Pei, Andy
  1 sibling, 0 replies; 10+ messages in thread
From: Pei, Andy @ 2019-06-21  9:02 UTC (permalink / raw)
  To: Ye, Xiaolong
  Cc: dev, Zhang, Helin, stable, Zhang, Roy Fan, Zhang, Qi Z, Wu,
	Jingjing, Xing, Beilei, Yigit, Ferruh, Xu, Rosen

HI

-----Original Message-----
From: Ye, Xiaolong 
Sent: Friday, June 21, 2019 7:42 PM
To: Pei, Andy <andy.pei@intel.com>
Cc: dev@dpdk.org; Zhang, Helin <helin.zhang@intel.com>; stable@dpdk.org; Zhang, Roy Fan <roy.fan.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Xu, Rosen <rosen.xu@intel.com>
Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix core dumped when setting txq or rxq to 0 in VF

On 06/21, Andy Pei wrote:
>Testpmd stucked and core dumped
>when set invalid VF queue number.
>This patch fix this issue.

It's better to wrap the commit message within 72 characters. Perhaps something
like:

Testpmd would stuck and result in core dump when user specifies an invalid VF queue number. This patch fixes this issue.

What do you think?
>
>Fixes: d6b19729093e ("i40evf: support configurable crc stripping")
>Cc: helin.zhang@intel.com
>Cc: stable@dpdk.org
>
>Signed-off-by: Andy Pei <andy.pei@intel.com>
>---
>Cc: roy.fan.zhang@intel.com
>Cc: qi.z.zhang@intel.com
>Cc: jingjing.wu@intel.com
>Cc: beilei.xing@intel.com
>Cc: ferruh.yigit@intel.com
>Cc: rosen.xu@intel.com
>
> drivers/net/i40e/i40e_ethdev_vf.c | 35 
> ++++++++++++++++++++++++++++-------
> 1 file changed, 28 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/net/i40e/i40e_ethdev_vf.c 
>b/drivers/net/i40e/i40e_ethdev_vf.c
>index 63dbe14..7096cc5 100644
>--- a/drivers/net/i40e/i40e_ethdev_vf.c
>+++ b/drivers/net/i40e/i40e_ethdev_vf.c
>@@ -573,7 +573,7 @@ struct rte_i40evf_xstats_name_off {  {
> 	txq_info->vsi_id = vsi_id;
> 	txq_info->queue_id = queue_id;
>-	if (queue_id < nb_txq) {
>+	if ((queue_id < nb_txq) && txq) {
> 		txq_info->ring_len = txq->nb_tx_desc;
> 		txq_info->dma_ring_addr = txq->tx_ring_phys_addr;
> 	}
>@@ -590,7 +590,7 @@ struct rte_i40evf_xstats_name_off {
> 	rxq_info->vsi_id = vsi_id;
> 	rxq_info->queue_id = queue_id;
> 	rxq_info->max_pkt_size = max_pkt_size;
>-	if (queue_id < nb_rxq) {
>+	if ((queue_id < nb_rxq) && rxq) {
> 		rxq_info->ring_len = rxq->nb_rx_desc;
> 		rxq_info->dma_ring_addr = rxq->rx_ring_phys_addr;
> 		rxq_info->databuffer_size =
>@@ -622,11 +622,32 @@ struct rte_i40evf_xstats_name_off {
> 	vc_vqci->num_queue_pairs = nb_qp;
> 
> 	for (i = 0, vc_qpi = vc_vqci->qpair; i < nb_qp; i++, vc_qpi++) {
>-		i40evf_fill_virtchnl_vsi_txq_info(&vc_qpi->txq,
>-			vc_vqci->vsi_id, i, dev->data->nb_tx_queues, txq[i]);
>-		i40evf_fill_virtchnl_vsi_rxq_info(&vc_qpi->rxq,
>-			vc_vqci->vsi_id, i, dev->data->nb_rx_queues,
>-					vf->max_pkt_len, rxq[i]);
>+		if (!txq)
>+			i40evf_fill_virtchnl_vsi_txq_info(&vc_qpi->txq,
>+							vc_vqci->vsi_id,
>+							i,
>+							dev->data->nb_tx_queues,
>+							NULL);
>+		else
>+			i40evf_fill_virtchnl_vsi_txq_info(&vc_qpi->txq,
>+							vc_vqci->vsi_id,
>+							i,
>+							dev->data->nb_tx_queues,
>+							txq[i]);
>+		if (!rxq)
>+			i40evf_fill_virtchnl_vsi_rxq_info(&vc_qpi->rxq,
>+							vc_vqci->vsi_id,
>+							i,
>+							dev->data->nb_rx_queues,
>+							vf->max_pkt_len,
>+							NULL);
>+		else
>+			i40evf_fill_virtchnl_vsi_rxq_info(&vc_qpi->rxq,
>+							vc_vqci->vsi_id,
>+							i,
>+							dev->data->nb_rx_queues,
>+							vf->max_pkt_len,
>+							rxq[i]);

No need to use one line for each parameter, I think you can still use the old format.

A tab is added at the beginning of the line, so the old format will exceed 80 characters.
So I suggests for use a new format but not one line for each parameter.
A new line comes if the upper line is more than 80 characters.
I will do this in v3

Thanks,
Xiaolong
> 	}
> 	memset(&args, 0, sizeof(args));
> 	args.ops = VIRTCHNL_OP_CONFIG_VSI_QUEUES;
>--
>1.8.3.1
>

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

* [dpdk-dev] [PATCH v3] net/i40e: fix core dumped when setting txq or rxq to 0 in VF
  2019-06-21  8:26 ` [dpdk-dev] [PATCH v2] " Andy Pei
@ 2019-06-21  9:23   ` Andy Pei
  2019-06-30  0:04     ` Zhang, Qi Z
  2019-07-04  1:38     ` [dpdk-dev] [PATCH v4] " Andy Pei
  0 siblings, 2 replies; 10+ messages in thread
From: Andy Pei @ 2019-06-21  9:23 UTC (permalink / raw)
  To: dev
  Cc: andy.pei, helin.zhang, stable, roy.fan.zhang, qi.z.zhang,
	jingjing.wu, beilei.xing, ferruh.yigit, rosen.xu, xiaolong.ye

Testpmd would stuck and result in core dump when user specifies an
invalid VF queue number. This patch fixes this issue.

Fixes: d6b19729093e ("i40evf: support configurable crc stripping")
Cc: helin.zhang@intel.com
Cc: stable@dpdk.org

Signed-off-by: Andy Pei <andy.pei@intel.com>
---
v3:
* no need to use a new line for each parameter when call envoke a
  function. A new line comes when the current line is more than
  80 characters.

v2:
 * modify commit meaasage so one line contains not more than 72
   characters.
 * delete unnecessary parentheses around 'queue_id < nb_txq'
 * delete unnecessary parentheses around 'queue_id < nb_rxq'

Cc: roy.fan.zhang@intel.com
Cc: qi.z.zhang@intel.com
Cc: jingjing.wu@intel.com
Cc: beilei.xing@intel.com
Cc: ferruh.yigit@intel.com
Cc: rosen.xu@intel.com
Cc: xiaolong.ye@intel.com

 drivers/net/i40e/i40e_ethdev_vf.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 63dbe14..41097fd 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -573,7 +573,7 @@ struct rte_i40evf_xstats_name_off {
 {
 	txq_info->vsi_id = vsi_id;
 	txq_info->queue_id = queue_id;
-	if (queue_id < nb_txq) {
+	if (queue_id < nb_txq && txq) {
 		txq_info->ring_len = txq->nb_tx_desc;
 		txq_info->dma_ring_addr = txq->tx_ring_phys_addr;
 	}
@@ -590,7 +590,7 @@ struct rte_i40evf_xstats_name_off {
 	rxq_info->vsi_id = vsi_id;
 	rxq_info->queue_id = queue_id;
 	rxq_info->max_pkt_size = max_pkt_size;
-	if (queue_id < nb_rxq) {
+	if (queue_id < nb_rxq && rxq) {
 		rxq_info->ring_len = rxq->nb_rx_desc;
 		rxq_info->dma_ring_addr = rxq->rx_ring_phys_addr;
 		rxq_info->databuffer_size =
@@ -622,11 +622,22 @@ struct rte_i40evf_xstats_name_off {
 	vc_vqci->num_queue_pairs = nb_qp;
 
 	for (i = 0, vc_qpi = vc_vqci->qpair; i < nb_qp; i++, vc_qpi++) {
-		i40evf_fill_virtchnl_vsi_txq_info(&vc_qpi->txq,
-			vc_vqci->vsi_id, i, dev->data->nb_tx_queues, txq[i]);
-		i40evf_fill_virtchnl_vsi_rxq_info(&vc_qpi->rxq,
-			vc_vqci->vsi_id, i, dev->data->nb_rx_queues,
-					vf->max_pkt_len, rxq[i]);
+		if (!txq)
+			i40evf_fill_virtchnl_vsi_txq_info(&vc_qpi->txq,
+				vc_vqci->vsi_id, i, dev->data->nb_tx_queues,
+				NULL);
+		else
+			i40evf_fill_virtchnl_vsi_txq_info(&vc_qpi->txq,
+				vc_vqci->vsi_id, i, dev->data->nb_tx_queues,
+				txq[i]);
+		if (!rxq)
+			i40evf_fill_virtchnl_vsi_rxq_info(&vc_qpi->rxq,
+				vc_vqci->vsi_id, i, dev->data->nb_rx_queues,
+				vf->max_pkt_len, NULL);
+		else
+			i40evf_fill_virtchnl_vsi_rxq_info(&vc_qpi->rxq,
+				vc_vqci->vsi_id, i, dev->data->nb_rx_queues,
+				vf->max_pkt_len, rxq[i]);
 	}
 	memset(&args, 0, sizeof(args));
 	args.ops = VIRTCHNL_OP_CONFIG_VSI_QUEUES;
-- 
1.8.3.1


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

* Re: [dpdk-dev] [PATCH] net/i40e: fix core dumped when setting txq or rxq to 0 in VF
  2019-06-21  4:34 [dpdk-dev] [PATCH] net/i40e: fix core dumped when setting txq or rxq to 0 in VF Andy Pei
  2019-06-21  8:26 ` [dpdk-dev] [PATCH v2] " Andy Pei
@ 2019-06-21 11:42 ` Ye Xiaolong
  2019-06-21  8:17   ` Pei, Andy
  2019-06-21  9:02   ` Pei, Andy
  1 sibling, 2 replies; 10+ messages in thread
From: Ye Xiaolong @ 2019-06-21 11:42 UTC (permalink / raw)
  To: Andy Pei
  Cc: dev, helin.zhang, stable, roy.fan.zhang, qi.z.zhang, jingjing.wu,
	beilei.xing, ferruh.yigit, rosen.xu

On 06/21, Andy Pei wrote:
>Testpmd stucked and core dumped
>when set invalid VF queue number.
>This patch fix this issue.

It's better to wrap the commit message within 72 characters. Perhaps something 
like:

Testpmd would stuck and result in core dump when user specifies an 
invalid VF queue number. This patch fixes this issue.

What do you think?
>
>Fixes: d6b19729093e ("i40evf: support configurable crc stripping")
>Cc: helin.zhang@intel.com
>Cc: stable@dpdk.org
>
>Signed-off-by: Andy Pei <andy.pei@intel.com>
>---
>Cc: roy.fan.zhang@intel.com
>Cc: qi.z.zhang@intel.com
>Cc: jingjing.wu@intel.com
>Cc: beilei.xing@intel.com
>Cc: ferruh.yigit@intel.com
>Cc: rosen.xu@intel.com
>
> drivers/net/i40e/i40e_ethdev_vf.c | 35 ++++++++++++++++++++++++++++-------
> 1 file changed, 28 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
>index 63dbe14..7096cc5 100644
>--- a/drivers/net/i40e/i40e_ethdev_vf.c
>+++ b/drivers/net/i40e/i40e_ethdev_vf.c
>@@ -573,7 +573,7 @@ struct rte_i40evf_xstats_name_off {
> {
> 	txq_info->vsi_id = vsi_id;
> 	txq_info->queue_id = queue_id;
>-	if (queue_id < nb_txq) {
>+	if ((queue_id < nb_txq) && txq) {
> 		txq_info->ring_len = txq->nb_tx_desc;
> 		txq_info->dma_ring_addr = txq->tx_ring_phys_addr;
> 	}
>@@ -590,7 +590,7 @@ struct rte_i40evf_xstats_name_off {
> 	rxq_info->vsi_id = vsi_id;
> 	rxq_info->queue_id = queue_id;
> 	rxq_info->max_pkt_size = max_pkt_size;
>-	if (queue_id < nb_rxq) {
>+	if ((queue_id < nb_rxq) && rxq) {
> 		rxq_info->ring_len = rxq->nb_rx_desc;
> 		rxq_info->dma_ring_addr = rxq->rx_ring_phys_addr;
> 		rxq_info->databuffer_size =
>@@ -622,11 +622,32 @@ struct rte_i40evf_xstats_name_off {
> 	vc_vqci->num_queue_pairs = nb_qp;
> 
> 	for (i = 0, vc_qpi = vc_vqci->qpair; i < nb_qp; i++, vc_qpi++) {
>-		i40evf_fill_virtchnl_vsi_txq_info(&vc_qpi->txq,
>-			vc_vqci->vsi_id, i, dev->data->nb_tx_queues, txq[i]);
>-		i40evf_fill_virtchnl_vsi_rxq_info(&vc_qpi->rxq,
>-			vc_vqci->vsi_id, i, dev->data->nb_rx_queues,
>-					vf->max_pkt_len, rxq[i]);
>+		if (!txq)
>+			i40evf_fill_virtchnl_vsi_txq_info(&vc_qpi->txq,
>+							vc_vqci->vsi_id,
>+							i,
>+							dev->data->nb_tx_queues,
>+							NULL);
>+		else
>+			i40evf_fill_virtchnl_vsi_txq_info(&vc_qpi->txq,
>+							vc_vqci->vsi_id,
>+							i,
>+							dev->data->nb_tx_queues,
>+							txq[i]);
>+		if (!rxq)
>+			i40evf_fill_virtchnl_vsi_rxq_info(&vc_qpi->rxq,
>+							vc_vqci->vsi_id,
>+							i,
>+							dev->data->nb_rx_queues,
>+							vf->max_pkt_len,
>+							NULL);
>+		else
>+			i40evf_fill_virtchnl_vsi_rxq_info(&vc_qpi->rxq,
>+							vc_vqci->vsi_id,
>+							i,
>+							dev->data->nb_rx_queues,
>+							vf->max_pkt_len,
>+							rxq[i]);

No need to use one line for each parameter, I think you can still use the old
format.


Thanks,
Xiaolong
> 	}
> 	memset(&args, 0, sizeof(args));
> 	args.ops = VIRTCHNL_OP_CONFIG_VSI_QUEUES;
>-- 
>1.8.3.1
>

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

* Re: [dpdk-dev] [PATCH v3] net/i40e: fix core dumped when setting txq or rxq to 0 in VF
  2019-06-21  9:23   ` [dpdk-dev] [PATCH v3] " Andy Pei
@ 2019-06-30  0:04     ` Zhang, Qi Z
  2019-07-04  1:38     ` [dpdk-dev] [PATCH v4] " Andy Pei
  1 sibling, 0 replies; 10+ messages in thread
From: Zhang, Qi Z @ 2019-06-30  0:04 UTC (permalink / raw)
  To: Pei, Andy, dev
  Cc: Zhang, Helin, stable, Zhang, Roy Fan, Wu, Jingjing, Xing, Beilei,
	Yigit, Ferruh, Xu, Rosen, Ye, Xiaolong

Hi Andy:

> -----Original Message-----
> From: Pei, Andy
> Sent: Friday, June 21, 2019 5:23 PM
> To: dev@dpdk.org
> Cc: Pei, Andy <andy.pei@intel.com>; Zhang, Helin <helin.zhang@intel.com>;
> stable@dpdk.org; Zhang, Roy Fan <roy.fan.zhang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Xu, Rosen
> <rosen.xu@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>
> Subject: [PATCH v3] net/i40e: fix core dumped when setting txq or rxq to 0 in
> VF
> 
> Testpmd would stuck and result in core dump when user specifies an invalid
> VF queue number. This patch fixes this issue.

Is that the case, dev_start will core dump due to violate memory access if rxq or txq number is 0?
It's better to have a more specific description of the issue..

> 
> Fixes: d6b19729093e ("i40evf: support configurable crc stripping")
> Cc: helin.zhang@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Andy Pei <andy.pei@intel.com>
> ---
> v3:
> * no need to use a new line for each parameter when call envoke a
>   function. A new line comes when the current line is more than
>   80 characters.
> 
> v2:
>  * modify commit meaasage so one line contains not more than 72
>    characters.
>  * delete unnecessary parentheses around 'queue_id < nb_txq'
>  * delete unnecessary parentheses around 'queue_id < nb_rxq'
> 
> Cc: roy.fan.zhang@intel.com
> Cc: qi.z.zhang@intel.com
> Cc: jingjing.wu@intel.com
> Cc: beilei.xing@intel.com
> Cc: ferruh.yigit@intel.com
> Cc: rosen.xu@intel.com
> Cc: xiaolong.ye@intel.com
> 
>  drivers/net/i40e/i40e_ethdev_vf.c | 25 ++++++++++++++++++-------
>  1 file changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> b/drivers/net/i40e/i40e_ethdev_vf.c
> index 63dbe14..41097fd 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -573,7 +573,7 @@ struct rte_i40evf_xstats_name_off {  {
>  	txq_info->vsi_id = vsi_id;
>  	txq_info->queue_id = queue_id;
> -	if (queue_id < nb_txq) {
> +	if (queue_id < nb_txq && txq) {
>  		txq_info->ring_len = txq->nb_tx_desc;
>  		txq_info->dma_ring_addr = txq->tx_ring_phys_addr;
>  	}
> @@ -590,7 +590,7 @@ struct rte_i40evf_xstats_name_off {
>  	rxq_info->vsi_id = vsi_id;
>  	rxq_info->queue_id = queue_id;
>  	rxq_info->max_pkt_size = max_pkt_size;
> -	if (queue_id < nb_rxq) {
> +	if (queue_id < nb_rxq && rxq) {
>  		rxq_info->ring_len = rxq->nb_rx_desc;
>  		rxq_info->dma_ring_addr = rxq->rx_ring_phys_addr;
>  		rxq_info->databuffer_size =
> @@ -622,11 +622,22 @@ struct rte_i40evf_xstats_name_off {
>  	vc_vqci->num_queue_pairs = nb_qp;
> 
>  	for (i = 0, vc_qpi = vc_vqci->qpair; i < nb_qp; i++, vc_qpi++) {
> -		i40evf_fill_virtchnl_vsi_txq_info(&vc_qpi->txq,
> -			vc_vqci->vsi_id, i, dev->data->nb_tx_queues, txq[i]);
> -		i40evf_fill_virtchnl_vsi_rxq_info(&vc_qpi->rxq,
> -			vc_vqci->vsi_id, i, dev->data->nb_rx_queues,
> -					vf->max_pkt_len, rxq[i]);
> +		if (!txq)
> +			i40evf_fill_virtchnl_vsi_txq_info(&vc_qpi->txq,
> +				vc_vqci->vsi_id, i, dev->data->nb_tx_queues,
> +				NULL);
> +		else
> +			i40evf_fill_virtchnl_vsi_txq_info(&vc_qpi->txq,
> +				vc_vqci->vsi_id, i, dev->data->nb_tx_queues,
> +				txq[i]);

Seems, we can simply to !txq?txq:txq[i] to avoid some duplicate, right?

> +		if (!rxq)
> +			i40evf_fill_virtchnl_vsi_rxq_info(&vc_qpi->rxq,
> +				vc_vqci->vsi_id, i, dev->data->nb_rx_queues,
> +				vf->max_pkt_len, NULL);
> +		else
> +			i40evf_fill_virtchnl_vsi_rxq_info(&vc_qpi->rxq,
> +				vc_vqci->vsi_id, i, dev->data->nb_rx_queues,
> +				vf->max_pkt_len, rxq[i]);
>  	}
>  	memset(&args, 0, sizeof(args));
>  	args.ops = VIRTCHNL_OP_CONFIG_VSI_QUEUES;
> --
> 1.8.3.1


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

* [dpdk-dev] [PATCH v4] net/i40e: fix core dumped when setting txq or rxq to 0 in VF
  2019-06-21  9:23   ` [dpdk-dev] [PATCH v3] " Andy Pei
  2019-06-30  0:04     ` Zhang, Qi Z
@ 2019-07-04  1:38     ` Andy Pei
  2019-07-04  6:17       ` Zhang, Qi Z
  1 sibling, 1 reply; 10+ messages in thread
From: Andy Pei @ 2019-07-04  1:38 UTC (permalink / raw)
  To: dev
  Cc: andy.pei, helin.zhang, stable, roy.fan.zhang, qi.z.zhang,
	jingjing.wu, beilei.xing, ferruh.yigit, rosen.xu, xiaolong.ye

Testpmd would stuck and result in core dump when user specifies an
invalid VF queue number, for example when setting txq or rxq to 0.
When txq or rxq is set to 0, pointer of pointer rxq or txq in
function i40evf_configure_vsi_queues is NULL. The usage of txq[i]
or rxq[0] is valid. This patch fixes this issue.

Fixes: d6b19729093e ("i40evf: support configurable crc stripping")
Cc: helin.zhang@intel.com
Cc: stable@dpdk.org

Signed-off-by: Andy Pei <andy.pei@intel.com>
---
v4:
* use "txq ? txq[i] : txq" to avoid some duplicate.
* a more specific description of the issue in the commit message.

v3:
* no need to use a new line for each parameter when call envoke a
  function. A new line comes when the current line is more than
  80 characters.

    v2:
* modify commit meaasage so one line contains not more than 72
  characters.
* delete unnecessary parentheses around 'queue_id < nb_txq'
* delete unnecessary parentheses around 'queue_id < nb_rxq'

Cc: roy.fan.zhang@intel.com
Cc: qi.z.zhang@intel.com
Cc: jingjing.wu@intel.com
Cc: beilei.xing@intel.com
Cc: ferruh.yigit@intel.com
Cc: rosen.xu@intel.com
Cc: xiaolong.ye@intel.com

 drivers/net/i40e/i40e_ethdev_vf.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index d922a84..5be32b0 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -573,7 +573,7 @@ struct rte_i40evf_xstats_name_off {
 {
 	txq_info->vsi_id = vsi_id;
 	txq_info->queue_id = queue_id;
-	if (queue_id < nb_txq) {
+	if (queue_id < nb_txq && txq) {
 		txq_info->ring_len = txq->nb_tx_desc;
 		txq_info->dma_ring_addr = txq->tx_ring_phys_addr;
 	}
@@ -590,7 +590,7 @@ struct rte_i40evf_xstats_name_off {
 	rxq_info->vsi_id = vsi_id;
 	rxq_info->queue_id = queue_id;
 	rxq_info->max_pkt_size = max_pkt_size;
-	if (queue_id < nb_rxq) {
+	if (queue_id < nb_rxq && rxq) {
 		rxq_info->ring_len = rxq->nb_rx_desc;
 		rxq_info->dma_ring_addr = rxq->rx_ring_phys_addr;
 		rxq_info->databuffer_size =
@@ -623,10 +623,11 @@ struct rte_i40evf_xstats_name_off {
 
 	for (i = 0, vc_qpi = vc_vqci->qpair; i < nb_qp; i++, vc_qpi++) {
 		i40evf_fill_virtchnl_vsi_txq_info(&vc_qpi->txq,
-			vc_vqci->vsi_id, i, dev->data->nb_tx_queues, txq[i]);
+			vc_vqci->vsi_id, i, dev->data->nb_tx_queues,
+			txq ? txq[i] : NULL);
 		i40evf_fill_virtchnl_vsi_rxq_info(&vc_qpi->rxq,
 			vc_vqci->vsi_id, i, dev->data->nb_rx_queues,
-					vf->max_pkt_len, rxq[i]);
+			vf->max_pkt_len, rxq ? rxq[i] : NULL);
 	}
 	memset(&args, 0, sizeof(args));
 	args.ops = VIRTCHNL_OP_CONFIG_VSI_QUEUES;
-- 
1.8.3.1


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

* Re: [dpdk-dev] [PATCH v4] net/i40e: fix core dumped when setting txq or rxq to 0 in VF
  2019-07-04  1:38     ` [dpdk-dev] [PATCH v4] " Andy Pei
@ 2019-07-04  6:17       ` Zhang, Qi Z
  2019-07-04  6:20         ` Pei, Andy
  0 siblings, 1 reply; 10+ messages in thread
From: Zhang, Qi Z @ 2019-07-04  6:17 UTC (permalink / raw)
  To: Pei, Andy, dev
  Cc: Zhang, Helin, stable, Zhang, Roy Fan, Wu, Jingjing, Xing, Beilei,
	Yigit, Ferruh, Xu, Rosen, Ye, Xiaolong



> -----Original Message-----
> From: Pei, Andy
> Sent: Thursday, July 4, 2019 9:39 AM
> To: dev@dpdk.org
> Cc: Pei, Andy <andy.pei@intel.com>; Zhang, Helin <helin.zhang@intel.com>;
> stable@dpdk.org; Zhang, Roy Fan <roy.fan.zhang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Xu, Rosen
> <rosen.xu@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>
> Subject: [PATCH v4] net/i40e: fix core dumped when setting txq or rxq to 0 in
> VF
> 
> Testpmd would stuck and result in core dump when user specifies an invalid
> VF queue number, for example when setting txq or rxq to 0.
> When txq or rxq is set to 0, pointer of pointer rxq or txq in function
> i40evf_configure_vsi_queues is NULL. The usage of txq[i] or rxq[0] is valid. This
> patch fixes this issue.
> 
> Fixes: d6b19729093e ("i40evf: support configurable crc stripping")
> Cc: helin.zhang@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Andy Pei <andy.pei@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi

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

* Re: [dpdk-dev] [PATCH v4] net/i40e: fix core dumped when setting txq or rxq to 0 in VF
  2019-07-04  6:17       ` Zhang, Qi Z
@ 2019-07-04  6:20         ` Pei, Andy
  0 siblings, 0 replies; 10+ messages in thread
From: Pei, Andy @ 2019-07-04  6:20 UTC (permalink / raw)
  To: Zhang, Qi Z, dev
  Cc: Zhang, Helin, stable, Zhang, Roy Fan, Wu, Jingjing, Xing, Beilei,
	Yigit, Ferruh, Xu, Rosen, Ye, Xiaolong

Thanks Qi

-----Original Message-----
From: Zhang, Qi Z 
Sent: Thursday, July 4, 2019 2:18 PM
To: Pei, Andy <andy.pei@intel.com>; dev@dpdk.org
Cc: Zhang, Helin <helin.zhang@intel.com>; stable@dpdk.org; Zhang, Roy Fan <roy.fan.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Xu, Rosen <rosen.xu@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>
Subject: RE: [PATCH v4] net/i40e: fix core dumped when setting txq or rxq to 0 in VF



> -----Original Message-----
> From: Pei, Andy
> Sent: Thursday, July 4, 2019 9:39 AM
> To: dev@dpdk.org
> Cc: Pei, Andy <andy.pei@intel.com>; Zhang, Helin 
> <helin.zhang@intel.com>; stable@dpdk.org; Zhang, Roy Fan 
> <roy.fan.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, 
> Jingjing <jingjing.wu@intel.com>; Xing, Beilei 
> <beilei.xing@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Xu, 
> Rosen <rosen.xu@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>
> Subject: [PATCH v4] net/i40e: fix core dumped when setting txq or rxq 
> to 0 in VF
> 
> Testpmd would stuck and result in core dump when user specifies an 
> invalid VF queue number, for example when setting txq or rxq to 0.
> When txq or rxq is set to 0, pointer of pointer rxq or txq in function 
> i40evf_configure_vsi_queues is NULL. The usage of txq[i] or rxq[0] is 
> valid. This patch fixes this issue.
> 
> Fixes: d6b19729093e ("i40evf: support configurable crc stripping")
> Cc: helin.zhang@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Andy Pei <andy.pei@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi

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

end of thread, other threads:[~2019-07-04  6:20 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-21  4:34 [dpdk-dev] [PATCH] net/i40e: fix core dumped when setting txq or rxq to 0 in VF Andy Pei
2019-06-21  8:26 ` [dpdk-dev] [PATCH v2] " Andy Pei
2019-06-21  9:23   ` [dpdk-dev] [PATCH v3] " Andy Pei
2019-06-30  0:04     ` Zhang, Qi Z
2019-07-04  1:38     ` [dpdk-dev] [PATCH v4] " Andy Pei
2019-07-04  6:17       ` Zhang, Qi Z
2019-07-04  6:20         ` Pei, Andy
2019-06-21 11:42 ` [dpdk-dev] [PATCH] " Ye Xiaolong
2019-06-21  8:17   ` Pei, Andy
2019-06-21  9:02   ` Pei, Andy

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