[PATCH] net/igc: fix segmentation fault in secondary dpdk-symmetric_mp

[PATCH] net/igc: fix segmentation fault in secondary dpdk-symmetric_mp

[zhichaox.zeng]

From: Zhichao Zeng <zhichaox.zeng@intel.com>

In the secondary dpdk-symmetric_mp process, the
"smp_port_init" was skipped, which cause some function
pointers not to be initialized, and a segmentation fault
occurred when calling these function pointers.

This patch assigns initial values to rx_pkt_burst,
tx_pkt_burst and tx_pkt_prepare pointers to avoid
calling null function pointer.

Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>
---
 drivers/net/igc/igc_ethdev.c | 3 +++
 drivers/net/igc/igc_ethdev.h | 5 +++++
 drivers/net/igc/igc_txrx.c   | 6 +++---
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/net/igc/igc_ethdev.c b/drivers/net/igc/igc_ethdev.c
index b9933b395d..dd4a028ea1 100644
--- a/drivers/net/igc/igc_ethdev.c
+++ b/drivers/net/igc/igc_ethdev.c
@@ -1234,6 +1234,9 @@ eth_igc_dev_init(struct rte_eth_dev *dev)
 	dev->rx_queue_count = eth_igc_rx_queue_count;
 	dev->rx_descriptor_status = eth_igc_rx_descriptor_status;
 	dev->tx_descriptor_status = eth_igc_tx_descriptor_status;
+	dev->rx_pkt_burst = &igc_recv_pkts;
+	dev->tx_pkt_burst = &igc_xmit_pkts;
+	dev->tx_pkt_prepare = &eth_igc_prep_pkts;
 
 	/*
 	 * for secondary processes, we don't initialize any further as primary
diff --git a/drivers/net/igc/igc_ethdev.h b/drivers/net/igc/igc_ethdev.h
index f56cad79e9..2fa3d51bba 100644
--- a/drivers/net/igc/igc_ethdev.h
+++ b/drivers/net/igc/igc_ethdev.h
@@ -263,6 +263,11 @@ struct igc_adapter {
 #define IGC_DEV_PRIVATE_FLOW_LIST(_dev) \
 	(&((struct igc_adapter *)(_dev)->data->dev_private)->flow_list)
 
+uint16_t igc_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts);
+uint16_t igc_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts);
+uint16_t eth_igc_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
+								uint16_t nb_pkts);
+
 static inline void
 igc_read_reg_check_set_bits(struct igc_hw *hw, uint32_t reg, uint32_t bits)
 {
diff --git a/drivers/net/igc/igc_txrx.c b/drivers/net/igc/igc_txrx.c
index e48d5df11a..753ac8a411 100644
--- a/drivers/net/igc/igc_txrx.c
+++ b/drivers/net/igc/igc_txrx.c
@@ -345,7 +345,7 @@ rx_desc_get_pkt_info(struct igc_rx_queue *rxq, struct rte_mbuf *rxm,
 	rxm->packet_type = rx_desc_pkt_info_to_pkt_type(pkt_info);
 }
 
-static uint16_t
+uint16_t
 igc_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 {
 	struct igc_rx_queue * const rxq = rx_queue;
@@ -1397,7 +1397,7 @@ eth_igc_rx_queue_setup(struct rte_eth_dev *dev,
 }
 
 /* prepare packets for transmit */
-static uint16_t
+uint16_t
 eth_igc_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
 		uint16_t nb_pkts)
 {
@@ -1604,7 +1604,7 @@ tx_desc_cksum_flags_to_olinfo(uint64_t ol_flags)
 	return tmp;
 }
 
-static uint16_t
+uint16_t
 igc_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 {
 	struct igc_tx_queue * const txq = tx_queue;
-- 
2.25.1

RE: [PATCH] net/igc: fix segmentation fault in secondary dpdk-symmetric_mp

[Guo, Junfeng]

> -----Original Message-----
> From: Zeng, ZhichaoX <zhichaox.zeng@intel.com>
> Sent: Wednesday, June 15, 2022 15:39
> To: dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; Zeng, ZhichaoX
> <zhichaox.zeng@intel.com>; Guo, Junfeng <junfeng.guo@intel.com>; Su,
> Simei <simei.su@intel.com>
> Subject: [PATCH] net/igc: fix segmentation fault in secondary dpdk-
> symmetric_mp
> 
> From: Zhichao Zeng <zhichaox.zeng@intel.com>
> 
> In the secondary dpdk-symmetric_mp process, the
> "smp_port_init" was skipped, which cause some function
> pointers not to be initialized, and a segmentation fault
> occurred when calling these function pointers.
> 
> This patch assigns initial values to rx_pkt_burst,
> tx_pkt_burst and tx_pkt_prepare pointers to avoid
> calling null function pointer.
> 
> Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>
> ---
>  drivers/net/igc/igc_ethdev.c | 3 +++
>  drivers/net/igc/igc_ethdev.h | 5 +++++
>  drivers/net/igc/igc_txrx.c   | 6 +++---
>  3 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/igc/igc_ethdev.c b/drivers/net/igc/igc_ethdev.c
> index b9933b395d..dd4a028ea1 100644
> --- a/drivers/net/igc/igc_ethdev.c
> +++ b/drivers/net/igc/igc_ethdev.c
> @@ -1234,6 +1234,9 @@ eth_igc_dev_init(struct rte_eth_dev *dev)
>  	dev->rx_queue_count = eth_igc_rx_queue_count;
>  	dev->rx_descriptor_status = eth_igc_rx_descriptor_status;
>  	dev->tx_descriptor_status = eth_igc_tx_descriptor_status;
> +	dev->rx_pkt_burst = &igc_recv_pkts;
> +	dev->tx_pkt_burst = &igc_xmit_pkts;
> +	dev->tx_pkt_prepare = &eth_igc_prep_pkts;

Maybe the parameter & are not needed here? you can double check for this when build.

> 
>  	/*
>  	 * for secondary processes, we don't initialize any further as
> primary
> diff --git a/drivers/net/igc/igc_ethdev.h b/drivers/net/igc/igc_ethdev.h
> index f56cad79e9..2fa3d51bba 100644
> --- a/drivers/net/igc/igc_ethdev.h
> +++ b/drivers/net/igc/igc_ethdev.h
> @@ -263,6 +263,11 @@ struct igc_adapter {
>  #define IGC_DEV_PRIVATE_FLOW_LIST(_dev) \
>  	(&((struct igc_adapter *)(_dev)->data->dev_private)->flow_list)
> 
> +uint16_t igc_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
> uint16_t nb_pkts);
> +uint16_t igc_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
> uint16_t nb_pkts);
> +uint16_t eth_igc_prep_pkts(__rte_unused void *tx_queue, struct
> rte_mbuf **tx_pkts,
> +
> 	uint16_t nb_pkts);
> +

These functions are tx/rx related, it would be better to put them into igc_txrx.h.

>  static inline void
>  igc_read_reg_check_set_bits(struct igc_hw *hw, uint32_t reg, uint32_t
> bits)
>  {
> diff --git a/drivers/net/igc/igc_txrx.c b/drivers/net/igc/igc_txrx.c
> index e48d5df11a..753ac8a411 100644
> --- a/drivers/net/igc/igc_txrx.c
> +++ b/drivers/net/igc/igc_txrx.c
> @@ -345,7 +345,7 @@ rx_desc_get_pkt_info(struct igc_rx_queue *rxq,
> struct rte_mbuf *rxm,
>  	rxm->packet_type = rx_desc_pkt_info_to_pkt_type(pkt_info);
>  }
> 
> -static uint16_t
> +uint16_t
>  igc_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t
> nb_pkts)
>  {
>  	struct igc_rx_queue * const rxq = rx_queue;
> @@ -1397,7 +1397,7 @@ eth_igc_rx_queue_setup(struct rte_eth_dev
> *dev,
>  }
> 
>  /* prepare packets for transmit */
> -static uint16_t
> +uint16_t
>  eth_igc_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf
> **tx_pkts,
>  		uint16_t nb_pkts)
>  {
> @@ -1604,7 +1604,7 @@ tx_desc_cksum_flags_to_olinfo(uint64_t
> ol_flags)
>  	return tmp;
>  }
> 
> -static uint16_t
> +uint16_t
>  igc_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t
> nb_pkts)
>  {
>  	struct igc_tx_queue * const txq = tx_queue;
> --
> 2.25.1

[PATCH v2] net/igc: fix segmentation fault in secondary dpdk-symmetric_mp

[zhichaox.zeng]

From: Zhichao Zeng <zhichaox.zeng@intel.com>

In the secondary dpdk-symmetric_mp process, the "smp_port_init" was
skipped, which cause some function pointers not to be initialized,
and a segmentation fault occurred when calling these function pointers.

This patch assigns initial values to rx_pkt_burst, tx_pkt_burst
and tx_pkt_prepare pointers to avoid calling null function pointer.

Fixes: 66fde1b943eb ("net/igc: add skeleton")
Cc: alvinx.zhang@intel.com
Cc: stable@dpdk.org

Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>

---
v2:
remove unnecessary parameters, move declaration to relevant header file
---
 drivers/net/igc/igc_ethdev.c | 3 +++
 drivers/net/igc/igc_txrx.c   | 6 +++---
 drivers/net/igc/igc_txrx.h   | 4 ++++
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/net/igc/igc_ethdev.c b/drivers/net/igc/igc_ethdev.c
index b9933b395d..25fb91bfec 100644
--- a/drivers/net/igc/igc_ethdev.c
+++ b/drivers/net/igc/igc_ethdev.c
@@ -1234,6 +1234,9 @@ eth_igc_dev_init(struct rte_eth_dev *dev)
 	dev->rx_queue_count = eth_igc_rx_queue_count;
 	dev->rx_descriptor_status = eth_igc_rx_descriptor_status;
 	dev->tx_descriptor_status = eth_igc_tx_descriptor_status;
+	dev->rx_pkt_burst = igc_recv_pkts;
+	dev->tx_pkt_burst = igc_xmit_pkts;
+	dev->tx_pkt_prepare = eth_igc_prep_pkts;
 
 	/*
 	 * for secondary processes, we don't initialize any further as primary
diff --git a/drivers/net/igc/igc_txrx.c b/drivers/net/igc/igc_txrx.c
index e48d5df11a..753ac8a411 100644
--- a/drivers/net/igc/igc_txrx.c
+++ b/drivers/net/igc/igc_txrx.c
@@ -345,7 +345,7 @@ rx_desc_get_pkt_info(struct igc_rx_queue *rxq, struct rte_mbuf *rxm,
 	rxm->packet_type = rx_desc_pkt_info_to_pkt_type(pkt_info);
 }
 
-static uint16_t
+uint16_t
 igc_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 {
 	struct igc_rx_queue * const rxq = rx_queue;
@@ -1397,7 +1397,7 @@ eth_igc_rx_queue_setup(struct rte_eth_dev *dev,
 }
 
 /* prepare packets for transmit */
-static uint16_t
+uint16_t
 eth_igc_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
 		uint16_t nb_pkts)
 {
@@ -1604,7 +1604,7 @@ tx_desc_cksum_flags_to_olinfo(uint64_t ol_flags)
 	return tmp;
 }
 
-static uint16_t
+uint16_t
 igc_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 {
 	struct igc_tx_queue * const txq = tx_queue;
diff --git a/drivers/net/igc/igc_txrx.h b/drivers/net/igc/igc_txrx.h
index 535108a868..a5240df9d7 100644
--- a/drivers/net/igc/igc_txrx.h
+++ b/drivers/net/igc/igc_txrx.h
@@ -49,6 +49,10 @@ void eth_igc_txq_info_get(struct rte_eth_dev *dev, uint16_t queue_id,
 	struct rte_eth_txq_info *qinfo);
 void eth_igc_vlan_strip_queue_set(struct rte_eth_dev *dev,
 			uint16_t rx_queue_id, int on);
+uint16_t igc_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts);
+uint16_t igc_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts);
+uint16_t eth_igc_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
+	uint16_t nb_pkts);
 #ifdef __cplusplus
 }
 #endif
-- 
2.25.1

RE: [PATCH v2] net/igc: fix segmentation fault in secondary dpdk-symmetric_mp

[Zhang, Qi Z]

> -----Original Message-----
> From: zhichaox.zeng@intel.com <zhichaox.zeng@intel.com>
> Sent: Wednesday, June 22, 2022 6:49 PM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zeng, ZhichaoX
> <zhichaox.zeng@intel.com>; alvinx.zhang@intel.com; Guo, Junfeng
> <junfeng.guo@intel.com>; Su, Simei <simei.su@intel.com>; Ferruh Yigit
> <ferruh.yigit@intel.com>
> Subject: [PATCH v2] net/igc: fix segmentation fault in secondary dpdk-
> symmetric_mp
> 
> From: Zhichao Zeng <zhichaox.zeng@intel.com>
> 
> In the secondary dpdk-symmetric_mp process, the "smp_port_init" was
> skipped, which cause some function pointers not to be initialized, and a
> segmentation fault occurred when calling these function pointers.
> 
> This patch assigns initial values to rx_pkt_burst, tx_pkt_burst and
> tx_pkt_prepare pointers to avoid calling null function pointer.
> 
> Fixes: 66fde1b943eb ("net/igc: add skeleton")
> Cc: alvinx.zhang@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>
> 
> ---
> v2:
> remove unnecessary parameters, move declaration to relevant header file
> ---
>  drivers/net/igc/igc_ethdev.c | 3 +++
>  drivers/net/igc/igc_txrx.c   | 6 +++---
>  drivers/net/igc/igc_txrx.h   | 4 ++++
>  3 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/igc/igc_ethdev.c b/drivers/net/igc/igc_ethdev.c index
> b9933b395d..25fb91bfec 100644
> --- a/drivers/net/igc/igc_ethdev.c
> +++ b/drivers/net/igc/igc_ethdev.c
> @@ -1234,6 +1234,9 @@ eth_igc_dev_init(struct rte_eth_dev *dev)
>  	dev->rx_queue_count = eth_igc_rx_queue_count;
>  	dev->rx_descriptor_status = eth_igc_rx_descriptor_status;
>  	dev->tx_descriptor_status = eth_igc_tx_descriptor_status;
> +	dev->rx_pkt_burst = igc_recv_pkts;
> +	dev->tx_pkt_burst = igc_xmit_pkts;
> +	dev->tx_pkt_prepare = eth_igc_prep_pkts;

If you already assign the function point in dev_init, you should remove the following redundant code.

[PATCH v3] net/igc: move the initialization of data path into dev_init

[zhichaox.zeng]

From: Zhichao Zeng <zhichaox.zeng@intel.com>

The upper-layer application usually call the common interface "dev_init"
to initialize the data path, but in the igc driver, the initialization
of data path is in "igc_rx_init" and "eth_igc_tx_queue_setup", while in
other drivers it is in "dev_init". So when upper-layer application
calling these function pointers will occur segmentation faults.

This patch moves the initialization of data path into "eth_igc_dev_init"
to avoid segmentation faults, which is consistent with other drivers.

Fixes: a5aeb2b9e225 ("net/igc: support Rx and Tx")
Cc: alvinx.zhang@intel.com
Cc: stable@dpdk.org

Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>

---
v2:
remove unnecessary parameters, move declaration to relevant header file
---
v3:
remove redundant code, optimize commit log
---
 drivers/net/igc/igc_ethdev.c |  3 +++
 drivers/net/igc/igc_txrx.c   | 10 +++-------
 drivers/net/igc/igc_txrx.h   |  4 ++++
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/net/igc/igc_ethdev.c b/drivers/net/igc/igc_ethdev.c
index b9933b395d..25fb91bfec 100644
--- a/drivers/net/igc/igc_ethdev.c
+++ b/drivers/net/igc/igc_ethdev.c
@@ -1234,6 +1234,9 @@ eth_igc_dev_init(struct rte_eth_dev *dev)
 	dev->rx_queue_count = eth_igc_rx_queue_count;
 	dev->rx_descriptor_status = eth_igc_rx_descriptor_status;
 	dev->tx_descriptor_status = eth_igc_tx_descriptor_status;
+	dev->rx_pkt_burst = igc_recv_pkts;
+	dev->tx_pkt_burst = igc_xmit_pkts;
+	dev->tx_pkt_prepare = eth_igc_prep_pkts;
 
 	/*
 	 * for secondary processes, we don't initialize any further as primary
diff --git a/drivers/net/igc/igc_txrx.c b/drivers/net/igc/igc_txrx.c
index e48d5df11a..f38c5e7863 100644
--- a/drivers/net/igc/igc_txrx.c
+++ b/drivers/net/igc/igc_txrx.c
@@ -345,7 +345,7 @@ rx_desc_get_pkt_info(struct igc_rx_queue *rxq, struct rte_mbuf *rxm,
 	rxm->packet_type = rx_desc_pkt_info_to_pkt_type(pkt_info);
 }
 
-static uint16_t
+uint16_t
 igc_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 {
 	struct igc_rx_queue * const rxq = rx_queue;
@@ -1071,8 +1071,6 @@ igc_rx_init(struct rte_eth_dev *dev)
 	uint16_t i;
 	int ret;
 
-	dev->rx_pkt_burst = igc_recv_pkts;
-
 	/*
 	 * Make sure receives are disabled while setting
 	 * up the descriptor ring.
@@ -1397,7 +1395,7 @@ eth_igc_rx_queue_setup(struct rte_eth_dev *dev,
 }
 
 /* prepare packets for transmit */
-static uint16_t
+uint16_t
 eth_igc_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
 		uint16_t nb_pkts)
 {
@@ -1604,7 +1602,7 @@ tx_desc_cksum_flags_to_olinfo(uint64_t ol_flags)
 	return tmp;
 }
 
-static uint16_t
+uint16_t
 igc_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 {
 	struct igc_tx_queue * const txq = tx_queue;
@@ -2030,8 +2028,6 @@ int eth_igc_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 		txq->sw_ring, txq->tx_ring, txq->tx_ring_phys_addr);
 
 	igc_reset_tx_queue(txq);
-	dev->tx_pkt_burst = igc_xmit_pkts;
-	dev->tx_pkt_prepare = &eth_igc_prep_pkts;
 	dev->data->tx_queues[queue_idx] = txq;
 	txq->offloads = tx_conf->offloads;
 
diff --git a/drivers/net/igc/igc_txrx.h b/drivers/net/igc/igc_txrx.h
index 535108a868..a5240df9d7 100644
--- a/drivers/net/igc/igc_txrx.h
+++ b/drivers/net/igc/igc_txrx.h
@@ -49,6 +49,10 @@ void eth_igc_txq_info_get(struct rte_eth_dev *dev, uint16_t queue_id,
 	struct rte_eth_txq_info *qinfo);
 void eth_igc_vlan_strip_queue_set(struct rte_eth_dev *dev,
 			uint16_t rx_queue_id, int on);
+uint16_t igc_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts);
+uint16_t igc_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts);
+uint16_t eth_igc_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
+	uint16_t nb_pkts);
 #ifdef __cplusplus
 }
 #endif
-- 
2.25.1

RE: [PATCH v3] net/igc: move the initialization of data path into dev_init

[Zhang, Qi Z]

> -----Original Message-----
> From: Zeng, ZhichaoX <zhichaox.zeng@intel.com>
> Sent: Tuesday, June 28, 2022 2:21 PM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Zeng, ZhichaoX <zhichaox.zeng@intel.com>;
> alvinx.zhang@intel.com; Guo, Junfeng <junfeng.guo@intel.com>; Su, Simei
> <simei.su@intel.com>; Ferruh Yigit <ferruh.yigit@intel.com>
> Subject: [PATCH v3] net/igc: move the initialization of data path into dev_init
> 
> From: Zhichao Zeng <zhichaox.zeng@intel.com>
> 
> The upper-layer application usually call the common interface "dev_init"
> to initialize the data path, but in the igc driver, the initialization of data path is in
> "igc_rx_init" and "eth_igc_tx_queue_setup", while in other drivers it is in
> "dev_init". So when upper-layer application calling these function pointers will
> occur segmentation faults.

NO, for most intel PMD e.g.: i40e and ice, we don't initialize data path in dev_init.

> 
> This patch moves the initialization of data path into "eth_igc_dev_init"
> to avoid segmentation faults, which is consistent with other drivers.

I saw dev->rx_pkt_burst can be overwritten in igc_rx_init, I assume the issue still be exist.

[PATCH v4] net/igc: add support for secondary processes

[zhichaox.zeng]

From: Zhichao Zeng <zhichaox.zeng@intel.com>

The RX function was not specified in the secondary process, causing the
secondary process to segfault in a multi-process environment.

This patch specify RX/TX functions in "dev_init" to support secondary
processes.

Fixes: 66fde1b943eb ("net/igc: add skeleton")
Cc: alvinx.zhang@intel.com
Cc: stable@dpdk.org

Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>

---
v2:
remove unnecessary parameters, move declaration to relevant header file
---
v3:
remove redundant code, optimize commit log
---
v4:
rework patch
---
 drivers/net/igc/igc_ethdev.c | 9 ++++++++-
 drivers/net/igc/igc_txrx.c   | 8 ++++----
 drivers/net/igc/igc_txrx.h   | 6 ++++++
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/net/igc/igc_ethdev.c b/drivers/net/igc/igc_ethdev.c
index b9933b395d..7f221a5d34 100644
--- a/drivers/net/igc/igc_ethdev.c
+++ b/drivers/net/igc/igc_ethdev.c
@@ -1240,8 +1240,15 @@ eth_igc_dev_init(struct rte_eth_dev *dev)
 	 * has already done this work. Only check we don't need a different
 	 * RX function.
 	 */
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
+		dev->rx_pkt_burst = igc_recv_pkts;
+		if (dev->data->scattered_rx)
+			dev->rx_pkt_burst = igc_recv_scattered_pkts;
+
+		dev->tx_pkt_burst = igc_xmit_pkts;
+		dev->tx_pkt_prepare = eth_igc_prep_pkts;
 		return 0;
+	}
 
 	rte_eth_copy_pci_info(dev, pci_dev);
 	dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
diff --git a/drivers/net/igc/igc_txrx.c b/drivers/net/igc/igc_txrx.c
index e48d5df11a..ffd219b0df 100644
--- a/drivers/net/igc/igc_txrx.c
+++ b/drivers/net/igc/igc_txrx.c
@@ -345,7 +345,7 @@ rx_desc_get_pkt_info(struct igc_rx_queue *rxq, struct rte_mbuf *rxm,
 	rxm->packet_type = rx_desc_pkt_info_to_pkt_type(pkt_info);
 }
 
-static uint16_t
+uint16_t
 igc_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 {
 	struct igc_rx_queue * const rxq = rx_queue;
@@ -488,7 +488,7 @@ igc_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 	return nb_rx;
 }
 
-static uint16_t
+uint16_t
 igc_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 			uint16_t nb_pkts)
 {
@@ -1397,7 +1397,7 @@ eth_igc_rx_queue_setup(struct rte_eth_dev *dev,
 }
 
 /* prepare packets for transmit */
-static uint16_t
+uint16_t
 eth_igc_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
 		uint16_t nb_pkts)
 {
@@ -1604,7 +1604,7 @@ tx_desc_cksum_flags_to_olinfo(uint64_t ol_flags)
 	return tmp;
 }
 
-static uint16_t
+uint16_t
 igc_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 {
 	struct igc_tx_queue * const txq = tx_queue;
diff --git a/drivers/net/igc/igc_txrx.h b/drivers/net/igc/igc_txrx.h
index 535108a868..02a0a051bb 100644
--- a/drivers/net/igc/igc_txrx.h
+++ b/drivers/net/igc/igc_txrx.h
@@ -49,6 +49,12 @@ void eth_igc_txq_info_get(struct rte_eth_dev *dev, uint16_t queue_id,
 	struct rte_eth_txq_info *qinfo);
 void eth_igc_vlan_strip_queue_set(struct rte_eth_dev *dev,
 			uint16_t rx_queue_id, int on);
+uint16_t igc_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts);
+uint16_t igc_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts);
+uint16_t eth_igc_prep_pkts(__rte_unused void *tx_queue, struct rte_mbuf **tx_pkts,
+	uint16_t nb_pkts);
+uint16_t igc_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
+	uint16_t nb_pkts);
 #ifdef __cplusplus
 }
 #endif
-- 
2.25.1

RE: [PATCH v4] net/igc: add support for secondary processes

[Zhang, Qi Z]

> -----Original Message-----
> From: Zeng, ZhichaoX <zhichaox.zeng@intel.com>
> Sent: Thursday, June 30, 2022 7:04 PM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Zeng, ZhichaoX <zhichaox.zeng@intel.com>;
> alvinx.zhang@intel.com; Guo, Junfeng <junfeng.guo@intel.com>; Su, Simei
> <simei.su@intel.com>; Burakov, Anatoly <anatoly.burakov@intel.com>; Ferruh
> Yigit <ferruh.yigit@intel.com>
> Subject: [PATCH v4] net/igc: add support for secondary processes
> 
> From: Zhichao Zeng <zhichaox.zeng@intel.com>
> 
> The RX function was not specified in the secondary process, causing the
> secondary process to segfault in a multi-process environment.
> 
> This patch specify RX/TX functions in "dev_init" to support secondary processes.
> 
> Fixes: 66fde1b943eb ("net/igc: add skeleton")
> Cc: alvinx.zhang@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>
> 
> ---
> v2:
> remove unnecessary parameters, move declaration to relevant header file
> ---
> v3:
> remove redundant code, optimize commit log
> ---
> v4:
> rework patch
> ---
>  drivers/net/igc/igc_ethdev.c | 9 ++++++++-
>  drivers/net/igc/igc_txrx.c   | 8 ++++----
>  drivers/net/igc/igc_txrx.h   | 6 ++++++
>  3 files changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/igc/igc_ethdev.c b/drivers/net/igc/igc_ethdev.c index
> b9933b395d..7f221a5d34 100644
> --- a/drivers/net/igc/igc_ethdev.c
> +++ b/drivers/net/igc/igc_ethdev.c
> @@ -1240,8 +1240,15 @@ eth_igc_dev_init(struct rte_eth_dev *dev)
>  	 * has already done this work. Only check we don't need a different
>  	 * RX function.
>  	 */
> -	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
> +		dev->rx_pkt_burst = igc_recv_pkts;
> +		if (dev->data->scattered_rx)
> +			dev->rx_pkt_burst = igc_recv_scattered_pkts;

Please removed the redundant code in igc_rx_init

RE: [PATCH v4] net/igc: add support for secondary processes

[Zeng, ZhichaoX]

>> The RX function was not specified in the secondary process, causing 
>> the secondary process to segfault in a multi-process environment.
>> 
>> This patch specify RX/TX functions in "dev_init" to support secondary processes.
>> 
>> Fixes: 66fde1b943eb ("net/igc: add skeleton")
>> Cc: alvinx.zhang@intel.com
>> Cc: stable@dpdk.org
>> 
>> Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>
>> 
>> ---
>> v2:
>> remove unnecessary parameters, move declaration to relevant header 
>> file
>> ---
>> v3:
>> remove redundant code, optimize commit log
>> ---
>> v4:
>> rework patch
>> ---
>>  drivers/net/igc/igc_ethdev.c | 9 ++++++++-
>>  drivers/net/igc/igc_txrx.c   | 8 ++++----
>>  drivers/net/igc/igc_txrx.h   | 6 ++++++
>>  3 files changed, 18 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/net/igc/igc_ethdev.c 
>> b/drivers/net/igc/igc_ethdev.c index
>> b9933b395d..7f221a5d34 100644
>> --- a/drivers/net/igc/igc_ethdev.c
>> +++ b/drivers/net/igc/igc_ethdev.c
>> @@ -1240,8 +1240,15 @@ eth_igc_dev_init(struct rte_eth_dev *dev)
>>  	 * has already done this work. Only check we don't need a different
>>  	 * RX function.
>>  	 */
>> -	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
>> +		dev->rx_pkt_burst = igc_recv_pkts;
>> +		if (dev->data->scattered_rx)
>> +			dev->rx_pkt_burst = igc_recv_scattered_pkts;
>
>Please removed the redundant code in igc_rx_init

Only the main process will execute "igc_rx_init", and the secondary process will not execute it.
So, the data path of the secondary process is not initialized. 

The code that this patch adds to initialize the data path in "dev_init" will only be executed in the
secondary process. The same code in "igc_rx_init" is not redundant.

May I ask if the commit log is confusing, and should I submit new patch to correct it?

Regards
Zhichao

RE: [PATCH v4] net/igc: add support for secondary processes

[Zhang, Qi Z]

> -----Original Message-----
> From: Zeng, ZhichaoX <zhichaox.zeng@intel.com>
> Sent: Friday, July 1, 2022 11:13 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org; Yang, Qiming <qiming.yang@intel.com>;
> alvinx.zhang@intel.com; Guo, Junfeng <junfeng.guo@intel.com>; Su, Simei
> <simei.su@intel.com>; Burakov, Anatoly <anatoly.burakov@intel.com>;
> Ferruh Yigit <ferruh.yigit@intel.com>
> Subject: RE: [PATCH v4] net/igc: add support for secondary processes
> 
> >> The RX function was not specified in the secondary process, causing
> >> the secondary process to segfault in a multi-process environment.
> >>
> >> This patch specify RX/TX functions in "dev_init" to support secondary
> processes.
> >>
> >> Fixes: 66fde1b943eb ("net/igc: add skeleton")
> >> Cc: alvinx.zhang@intel.com
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>
> >>
> >> ---
> >> v2:
> >> remove unnecessary parameters, move declaration to relevant header
> >> file
> >> ---
> >> v3:
> >> remove redundant code, optimize commit log
> >> ---
> >> v4:
> >> rework patch
> >> ---
> >>  drivers/net/igc/igc_ethdev.c | 9 ++++++++-
> >>  drivers/net/igc/igc_txrx.c   | 8 ++++----
> >>  drivers/net/igc/igc_txrx.h   | 6 ++++++
> >>  3 files changed, 18 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/net/igc/igc_ethdev.c
> >> b/drivers/net/igc/igc_ethdev.c index
> >> b9933b395d..7f221a5d34 100644
> >> --- a/drivers/net/igc/igc_ethdev.c
> >> +++ b/drivers/net/igc/igc_ethdev.c
> >> @@ -1240,8 +1240,15 @@ eth_igc_dev_init(struct rte_eth_dev *dev)
> >>  	 * has already done this work. Only check we don't need a different
> >>  	 * RX function.
> >>  	 */
> >> -	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> >> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
> >> +		dev->rx_pkt_burst = igc_recv_pkts;
> >> +		if (dev->data->scattered_rx)
> >> +			dev->rx_pkt_burst = igc_recv_scattered_pkts;
> >
> >Please removed the redundant code in igc_rx_init
> 
> Only the main process will execute "igc_rx_init", and the secondary process
> will not execute it.
> So, the data path of the secondary process is not initialized.
> 
> The code that this patch adds to initialize the data path in "dev_init" will
> only be executed in the secondary process. The same code in "igc_rx_init" is
> not redundant.

Ok, I missed this point,  the implementation is correct.

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

Applied to dpdk-next-net-intel.

Thanks
Qi

> 
> May I ask if the commit log is confusing, and should I submit new patch to
> correct it?
> 
> Regards
> Zhichao