[v2] net/iavf: fix iavf crashed on dev_stop when running in multi-process mode

Message ID 20220414092902.176462-1-ke1x.zhang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Qi Zhang
Headers
Series [v2] net/iavf: fix iavf crashed on dev_stop when running in multi-process mode |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/github-robot: build success github build: passed
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/intel-Testing success Testing PASS

Commit Message

Zhang, Ke1X April 14, 2022, 9:29 a.m. UTC
  In the multi process environment, the sub process
operates on the shared memory and changes the
function pointer of the main process, resulting in
the failure to find the address of the function when
main process releasing, resulting in crash.

Signed-off-by: Ke Zhang <ke1x.zhang@intel.com>
---
 drivers/net/iavf/iavf_rxtx.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)
  

Comments

Qi Zhang April 18, 2022, 6:41 a.m. UTC | #1
> -----Original Message-----
> From: Ke Zhang <ke1x.zhang@intel.com>
> Sent: Thursday, April 14, 2022 5:29 PM
> To: Li, Xiaoyun <xiaoyun.li@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> Xing, Beilei <beilei.xing@intel.com>; dev@dpdk.org
> Cc: Zhang, Ke1X <ke1x.zhang@intel.com>
> Subject: [PATCH v2] net/iavf: fix iavf crashed on dev_stop when running in
> multi-process mode

It's not a good practice to take the failure test case as the title, please describe what actually the patch fixed.

e.g.:
fix mbuf release function point corrupt in multi-process

> 
> In the multi process environment, the sub process operates on the shared
> memory and changes the function pointer of the main process, resulting in
> the failure to find the address of the function when main process releasing,
> resulting in crash.
> 
> Signed-off-by: Ke Zhang <ke1x.zhang@intel.com>
> ---
>  drivers/net/iavf/iavf_rxtx.c | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/iavf/iavf_rxtx.c b/drivers/net/iavf/iavf_rxtx.c index
> 16e8d021f9..1cef985fcc 100644
> --- a/drivers/net/iavf/iavf_rxtx.c
> +++ b/drivers/net/iavf/iavf_rxtx.c
> @@ -2822,12 +2822,12 @@ iavf_set_rx_function(struct rte_eth_dev *dev)
>  		if (vf->vf_res->vf_cap_flags &
>  			VIRTCHNL_VF_OFFLOAD_RX_FLEX_DESC)
>  			use_flex = true;
> -
> -		for (i = 0; i < dev->data->nb_rx_queues; i++) {
> -			rxq = dev->data->rx_queues[i];
> -			(void)iavf_rxq_vec_setup(rxq);
> +		if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> +			for (i = 0; i < dev->data->nb_rx_queues; i++) {
> +				rxq = dev->data->rx_queues[i];
> +				(void)iavf_rxq_vec_setup(rxq);
> +			}
>  		}

Now, you force rxq->ops only can be owned by primary process, which is not necessary.
Its better we still allow a secondary process to stop a queue (which will call rxq->ops)
  

Patch

diff --git a/drivers/net/iavf/iavf_rxtx.c b/drivers/net/iavf/iavf_rxtx.c
index 16e8d021f9..1cef985fcc 100644
--- a/drivers/net/iavf/iavf_rxtx.c
+++ b/drivers/net/iavf/iavf_rxtx.c
@@ -2822,12 +2822,12 @@  iavf_set_rx_function(struct rte_eth_dev *dev)
 		if (vf->vf_res->vf_cap_flags &
 			VIRTCHNL_VF_OFFLOAD_RX_FLEX_DESC)
 			use_flex = true;
-
-		for (i = 0; i < dev->data->nb_rx_queues; i++) {
-			rxq = dev->data->rx_queues[i];
-			(void)iavf_rxq_vec_setup(rxq);
+		if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+			for (i = 0; i < dev->data->nb_rx_queues; i++) {
+				rxq = dev->data->rx_queues[i];
+				(void)iavf_rxq_vec_setup(rxq);
+			}
 		}
-
 		if (dev->data->scattered_rx) {
 			if (!use_avx512) {
 				PMD_DRV_LOG(DEBUG,
@@ -3002,20 +3002,21 @@  iavf_set_tx_function(struct rte_eth_dev *dev)
 		}
 #endif
 
-		for (i = 0; i < dev->data->nb_tx_queues; i++) {
-			txq = dev->data->tx_queues[i];
-			if (!txq)
-				continue;
+		if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+			for (i = 0; i < dev->data->nb_tx_queues; i++) {
+				txq = dev->data->tx_queues[i];
+				if (!txq)
+					continue;
 #ifdef CC_AVX512_SUPPORT
-			if (use_avx512)
-				iavf_txq_vec_setup_avx512(txq);
-			else
-				iavf_txq_vec_setup(txq);
+				if (use_avx512)
+					iavf_txq_vec_setup_avx512(txq);
+				else
+					iavf_txq_vec_setup(txq);
 #else
-			iavf_txq_vec_setup(txq);
+				iavf_txq_vec_setup(txq);
 #endif
+			}
 		}
-
 		return;
 	}