All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net/fm10k: fix secondary process crash
@ 2017-03-28  3:58 Xiao Wang
  2017-03-31  1:30 ` Chen, Jing D
  2017-04-01  3:22 ` Chen, Jing D
  0 siblings, 2 replies; 5+ messages in thread
From: Xiao Wang @ 2017-03-28  3:58 UTC (permalink / raw)
  To: jing.d.chen; +Cc: dev, Xiao Wang, stable

If the primary process has initialized all the queues to vector
pmd mode, the secondary process should not use scalar code path,
because the per queue data structures haven't been prepared for
that, e.g. txq->ops is for vector Tx rather than scalar Tx.

Fixes: a6ce64a97520 ("fm10k: introduce vector driver")
Cc: stable@dpdk.org

Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
---
 drivers/net/fm10k/fm10k_ethdev.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 388f929..680d617 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -2750,6 +2750,21 @@ static void __attribute__((cold))
 	int use_sse = 1;
 	uint16_t tx_ftag_en = 0;
 
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
+		/* primary process has set the ftag flag and txq_flags */
+		txq = dev->data->tx_queues[0];
+		if (fm10k_tx_vec_condition_check(txq)) {
+			dev->tx_pkt_burst = fm10k_xmit_pkts;
+			dev->tx_pkt_prepare = fm10k_prep_pkts;
+			PMD_INIT_LOG(DEBUG, "Use regular Tx func");
+		} else {
+			PMD_INIT_LOG(DEBUG, "Use vector Tx func");
+			dev->tx_pkt_burst = fm10k_xmit_pkts_vec;
+			dev->tx_pkt_prepare = NULL;
+		}
+		return;
+	}
+
 	if (fm10k_check_ftag(dev->device->devargs))
 		tx_ftag_en = 1;
 
@@ -2810,6 +2825,9 @@ static void __attribute__((cold))
 	else
 		PMD_INIT_LOG(DEBUG, "Use regular Rx func");
 
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return;
+
 	for (i = 0; i < dev->data->nb_rx_queues; i++) {
 		struct fm10k_rx_queue *rxq = dev->data->rx_queues[i];
 
@@ -2856,9 +2874,15 @@ static void __attribute__((cold))
 	dev->tx_pkt_burst = &fm10k_xmit_pkts;
 	dev->tx_pkt_prepare = &fm10k_prep_pkts;
 
-	/* only initialize in the primary process */
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+	/*
+	 * Primary process does the whole initialization, for secondary
+	 * processes, we just select the same Rx and Tx function as primary.
+	 */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
+		fm10k_set_rx_function(dev);
+		fm10k_set_tx_function(dev);
 		return 0;
+	}
 
 	rte_eth_copy_pci_info(dev, pdev);
 	dev->data->dev_flags |= RTE_ETH_DEV_DETACHABLE;
-- 
1.8.3.1

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

* Re: [PATCH] net/fm10k: fix secondary process crash
  2017-03-28  3:58 [PATCH] net/fm10k: fix secondary process crash Xiao Wang
@ 2017-03-31  1:30 ` Chen, Jing D
  2017-04-01  0:20   ` Wang, Xiao W
  2017-04-01  3:22 ` Chen, Jing D
  1 sibling, 1 reply; 5+ messages in thread
From: Chen, Jing D @ 2017-03-31  1:30 UTC (permalink / raw)
  To: Wang, Xiao W; +Cc: dev, stable



> -----Original Message-----
> From: Wang, Xiao W
> Sent: Tuesday, March 28, 2017 11:59 AM
> To: Chen, Jing D <jing.d.chen@intel.com>
> Cc: dev@dpdk.org; Wang, Xiao W <xiao.w.wang@intel.com>; stable@dpdk.org
> Subject: [PATCH] net/fm10k: fix secondary process crash
> 
> If the primary process has initialized all the queues to vector pmd mode, the
> secondary process should not use scalar code path, because the per queue data
> structures haven't been prepared for that, e.g. txq->ops is for vector Tx rather
> than scalar Tx.
> 
> Fixes: a6ce64a97520 ("fm10k: introduce vector driver")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
> ---
>  drivers/net/fm10k/fm10k_ethdev.c | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/fm10k/fm10k_ethdev.c
> b/drivers/net/fm10k/fm10k_ethdev.c
> index 388f929..680d617 100644
> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> @@ -2750,6 +2750,21 @@ static void __attribute__((cold))
>  	int use_sse = 1;
>  	uint16_t tx_ftag_en = 0;
> 
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
> +		/* primary process has set the ftag flag and txq_flags */
> +		txq = dev->data->tx_queues[0];
> +		if (fm10k_tx_vec_condition_check(txq)) {
> +			dev->tx_pkt_burst = fm10k_xmit_pkts;
> +			dev->tx_pkt_prepare = fm10k_prep_pkts;
> +			PMD_INIT_LOG(DEBUG, "Use regular Tx func");
> +		} else {
> +			PMD_INIT_LOG(DEBUG, "Use vector Tx func");
> +			dev->tx_pkt_burst = fm10k_xmit_pkts_vec;
> +			dev->tx_pkt_prepare = NULL;
> +		}
> +		return;
> +	}
> +

Why we need to check process type? What would happen if no changes made here?

>  	if (fm10k_check_ftag(dev->device->devargs))
>  		tx_ftag_en = 1;
> 
> @@ -2810,6 +2825,9 @@ static void __attribute__((cold))
>  	else
>  		PMD_INIT_LOG(DEBUG, "Use regular Rx func");
> 
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> +		return;
> +
>  	for (i = 0; i < dev->data->nb_rx_queues; i++) {
>  		struct fm10k_rx_queue *rxq = dev->data->rx_queues[i];
> 
> @@ -2856,9 +2874,15 @@ static void __attribute__((cold))
>  	dev->tx_pkt_burst = &fm10k_xmit_pkts;
>  	dev->tx_pkt_prepare = &fm10k_prep_pkts;
> 
> -	/* only initialize in the primary process */
> -	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> +	/*
> +	 * Primary process does the whole initialization, for secondary
> +	 * processes, we just select the same Rx and Tx function as primary.
> +	 */
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
> +		fm10k_set_rx_function(dev);
> +		fm10k_set_tx_function(dev);
>  		return 0;
> +	}
> 
>  	rte_eth_copy_pci_info(dev, pdev);
>  	dev->data->dev_flags |= RTE_ETH_DEV_DETACHABLE;
> --
> 1.8.3.1

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

* Re: [PATCH] net/fm10k: fix secondary process crash
  2017-03-31  1:30 ` Chen, Jing D
@ 2017-04-01  0:20   ` Wang, Xiao W
  0 siblings, 0 replies; 5+ messages in thread
From: Wang, Xiao W @ 2017-04-01  0:20 UTC (permalink / raw)
  To: Chen, Jing D; +Cc: dev, stable

Hi Mark,

> -----Original Message-----
> From: Chen, Jing D
> Sent: Friday, March 31, 2017 9:30 AM
> To: Wang, Xiao W <xiao.w.wang@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [PATCH] net/fm10k: fix secondary process crash
> 
> 
> 
> > -----Original Message-----
> > From: Wang, Xiao W
> > Sent: Tuesday, March 28, 2017 11:59 AM
> > To: Chen, Jing D <jing.d.chen@intel.com>
> > Cc: dev@dpdk.org; Wang, Xiao W <xiao.w.wang@intel.com>;
> stable@dpdk.org
> > Subject: [PATCH] net/fm10k: fix secondary process crash
> >
> > If the primary process has initialized all the queues to vector pmd mode,
> the
> > secondary process should not use scalar code path, because the per queue
> data
> > structures haven't been prepared for that, e.g. txq->ops is for vector Tx
> rather
> > than scalar Tx.
> >
> > Fixes: a6ce64a97520 ("fm10k: introduce vector driver")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
> > ---
> >  drivers/net/fm10k/fm10k_ethdev.c | 28 ++++++++++++++++++++++++++--
> >  1 file changed, 26 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/fm10k/fm10k_ethdev.c
> > b/drivers/net/fm10k/fm10k_ethdev.c
> > index 388f929..680d617 100644
> > --- a/drivers/net/fm10k/fm10k_ethdev.c
> > +++ b/drivers/net/fm10k/fm10k_ethdev.c
> > @@ -2750,6 +2750,21 @@ static void __attribute__((cold))
> >  	int use_sse = 1;
> >  	uint16_t tx_ftag_en = 0;
> >
> > +	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
> > +		/* primary process has set the ftag flag and txq_flags */
> > +		txq = dev->data->tx_queues[0];
> > +		if (fm10k_tx_vec_condition_check(txq)) {
> > +			dev->tx_pkt_burst = fm10k_xmit_pkts;
> > +			dev->tx_pkt_prepare = fm10k_prep_pkts;
> > +			PMD_INIT_LOG(DEBUG, "Use regular Tx func");
> > +		} else {
> > +			PMD_INIT_LOG(DEBUG, "Use vector Tx func");
> > +			dev->tx_pkt_burst = fm10k_xmit_pkts_vec;
> > +			dev->tx_pkt_prepare = NULL;
> > +		}
> > +		return;
> > +	}
> > +
> 
> Why we need to check process type? What would happen if no changes
> made here?

If no change, then this function will re-set some fields of txq structure.
e.g.       for (i = 0; i < dev->data->nb_tx_queues; i++) {
            		txq = dev->data->tx_queues[i];
            		fm10k_txq_vec_setup(txq);
       	}
Though these fields would be re-set to the same value, it doesn't look good. In secondary, we just read the queues and do not write them.

Best Regards,
Xiao
> 
> >  	if (fm10k_check_ftag(dev->device->devargs))
> >  		tx_ftag_en = 1;
> >
> > @@ -2810,6 +2825,9 @@ static void __attribute__((cold))
> >  	else
> >  		PMD_INIT_LOG(DEBUG, "Use regular Rx func");
> >
> > +	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> > +		return;
> > +
> >  	for (i = 0; i < dev->data->nb_rx_queues; i++) {
> >  		struct fm10k_rx_queue *rxq = dev->data->rx_queues[i];
> >
> > @@ -2856,9 +2874,15 @@ static void __attribute__((cold))
> >  	dev->tx_pkt_burst = &fm10k_xmit_pkts;
> >  	dev->tx_pkt_prepare = &fm10k_prep_pkts;
> >
> > -	/* only initialize in the primary process */
> > -	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> > +	/*
> > +	 * Primary process does the whole initialization, for secondary
> > +	 * processes, we just select the same Rx and Tx function as primary.
> > +	 */
> > +	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
> > +		fm10k_set_rx_function(dev);
> > +		fm10k_set_tx_function(dev);
> >  		return 0;
> > +	}
> >
> >  	rte_eth_copy_pci_info(dev, pdev);
> >  	dev->data->dev_flags |= RTE_ETH_DEV_DETACHABLE;
> > --
> > 1.8.3.1

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

* Re: [PATCH] net/fm10k: fix secondary process crash
  2017-03-28  3:58 [PATCH] net/fm10k: fix secondary process crash Xiao Wang
  2017-03-31  1:30 ` Chen, Jing D
@ 2017-04-01  3:22 ` Chen, Jing D
  2017-04-03  9:51   ` [dpdk-stable] " Ferruh Yigit
  1 sibling, 1 reply; 5+ messages in thread
From: Chen, Jing D @ 2017-04-01  3:22 UTC (permalink / raw)
  To: Wang, Xiao W; +Cc: dev, stable



> -----Original Message-----
> From: Wang, Xiao W
> Sent: Tuesday, March 28, 2017 11:59 AM
> To: Chen, Jing D <jing.d.chen@intel.com>
> Cc: dev@dpdk.org; Wang, Xiao W <xiao.w.wang@intel.com>;
> stable@dpdk.org
> Subject: [PATCH] net/fm10k: fix secondary process crash
> 
> If the primary process has initialized all the queues to vector pmd mode, the
> secondary process should not use scalar code path, because the per queue
> data structures haven't been prepared for that, e.g. txq->ops is for vector Tx
> rather than scalar Tx.
> 
> Fixes: a6ce64a97520 ("fm10k: introduce vector driver")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
Acked-by : Jing Chen <jing.d.chen@intel.com>

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

* Re: [dpdk-stable] [PATCH] net/fm10k: fix secondary process crash
  2017-04-01  3:22 ` Chen, Jing D
@ 2017-04-03  9:51   ` Ferruh Yigit
  0 siblings, 0 replies; 5+ messages in thread
From: Ferruh Yigit @ 2017-04-03  9:51 UTC (permalink / raw)
  To: Chen, Jing D, Wang, Xiao W; +Cc: dev, stable

On 4/1/2017 4:22 AM, Chen, Jing D wrote:
> 
> 
>> -----Original Message-----
>> From: Wang, Xiao W
>> Sent: Tuesday, March 28, 2017 11:59 AM
>> To: Chen, Jing D <jing.d.chen@intel.com>
>> Cc: dev@dpdk.org; Wang, Xiao W <xiao.w.wang@intel.com>;
>> stable@dpdk.org
>> Subject: [PATCH] net/fm10k: fix secondary process crash
>>
>> If the primary process has initialized all the queues to vector pmd mode, the
>> secondary process should not use scalar code path, because the per queue
>> data structures haven't been prepared for that, e.g. txq->ops is for vector Tx
>> rather than scalar Tx.
>>
>> Fixes: a6ce64a97520 ("fm10k: introduce vector driver")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
> Acked-by : Jing Chen <jing.d.chen@intel.com>

Applied to dpdk-next-net/master, thanks.

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

end of thread, other threads:[~2017-04-03  9:51 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-28  3:58 [PATCH] net/fm10k: fix secondary process crash Xiao Wang
2017-03-31  1:30 ` Chen, Jing D
2017-04-01  0:20   ` Wang, Xiao W
2017-04-01  3:22 ` Chen, Jing D
2017-04-03  9:51   ` [dpdk-stable] " Ferruh Yigit

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.