[dpdk-dev] [PATCH] net/i40e: fix core dumped when setting txq or rxq to 0 in VF

[dpdk-dev] [PATCH] net/i40e: fix core dumped when setting txq or rxq to 0 in VF

[Andy Pei]

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

Re: [dpdk-dev] [PATCH] net/i40e: fix core dumped when setting txq or rxq to 0 in VF

[Pei, Andy]

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
>

[dpdk-dev] [PATCH v2] net/i40e: fix core dumped when setting txq or rxq to 0 in VF

[Andy Pei]

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

Re: [dpdk-dev] [PATCH] net/i40e: fix core dumped when setting txq or rxq to 0 in VF

[Pei, Andy]

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
>

[dpdk-dev] [PATCH v3] net/i40e: fix core dumped when setting txq or rxq to 0 in VF

[Andy Pei]

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

Re: [dpdk-dev] [PATCH] net/i40e: fix core dumped when setting txq or rxq to 0 in VF

[Ye Xiaolong]

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
>

Re: [dpdk-dev] [PATCH v3] net/i40e: fix core dumped when setting txq or rxq to 0 in VF

[Zhang, Qi Z]

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

[dpdk-dev] [PATCH v4] net/i40e: fix core dumped when setting txq or rxq to 0 in VF

[Andy Pei]

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

Re: [dpdk-dev] [PATCH v4] net/i40e: fix core dumped when setting txq or rxq to 0 in VF

[Zhang, Qi Z]

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

Re: [dpdk-dev] [PATCH v4] net/i40e: fix core dumped when setting txq or rxq to 0 in VF

[Pei, Andy]

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