[v4] net/igc: add support for secondary processes
Checks
Commit Message
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(-)
Comments
> -----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
>> 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
> -----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
@@ -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;
@@ -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;
@@ -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