From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Wang, Xiao W" Subject: Re: [PATCH] net/fm10k: fix secondary process crash Date: Sat, 1 Apr 2017 00:20:53 +0000 Message-ID: References: <1490673535-104255-1-git-send-email-xiao.w.wang@intel.com> <4341B239C0EFF9468EE453F9E9F4604D3C620539@shsmsx102.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" , "stable@dpdk.org" To: "Chen, Jing D" Return-path: In-Reply-To: <4341B239C0EFF9468EE453F9E9F4604D3C620539@shsmsx102.ccr.corp.intel.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Mark, > -----Original Message----- > From: Chen, Jing D > Sent: Friday, March 31, 2017 9:30 AM > To: Wang, Xiao W > Cc: dev@dpdk.org; stable@dpdk.org > Subject: RE: [PATCH] net/fm10k: fix secondary process crash >=20 >=20 >=20 > > -----Original Message----- > > From: Wang, Xiao W > > Sent: Tuesday, March 28, 2017 11:59 AM > > To: Chen, Jing D > > Cc: dev@dpdk.org; Wang, Xiao W ; > stable@dpdk.org > > Subject: [PATCH] net/fm10k: fix secondary process crash > > > > If the primary process has initialized all the queues to vector pmd mod= e, > the > > secondary process should not use scalar code path, because the per queu= e > 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 > > --- > > 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 =3D 1; > > uint16_t tx_ftag_en =3D 0; > > > > + if (rte_eal_process_type() !=3D RTE_PROC_PRIMARY) { > > + /* primary process has set the ftag flag and txq_flags */ > > + txq =3D dev->data->tx_queues[0]; > > + if (fm10k_tx_vec_condition_check(txq)) { > > + dev->tx_pkt_burst =3D fm10k_xmit_pkts; > > + dev->tx_pkt_prepare =3D fm10k_prep_pkts; > > + PMD_INIT_LOG(DEBUG, "Use regular Tx func"); > > + } else { > > + PMD_INIT_LOG(DEBUG, "Use vector Tx func"); > > + dev->tx_pkt_burst =3D fm10k_xmit_pkts_vec; > > + dev->tx_pkt_prepare =3D NULL; > > + } > > + return; > > + } > > + >=20 > 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 =3D 0; i < dev->data->nb_tx_queues; i++) { txq =3D 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 >=20 > > if (fm10k_check_ftag(dev->device->devargs)) > > tx_ftag_en =3D 1; > > > > @@ -2810,6 +2825,9 @@ static void __attribute__((cold)) > > else > > PMD_INIT_LOG(DEBUG, "Use regular Rx func"); > > > > + if (rte_eal_process_type() !=3D RTE_PROC_PRIMARY) > > + return; > > + > > for (i =3D 0; i < dev->data->nb_rx_queues; i++) { > > struct fm10k_rx_queue *rxq =3D dev->data->rx_queues[i]; > > > > @@ -2856,9 +2874,15 @@ static void __attribute__((cold)) > > dev->tx_pkt_burst =3D &fm10k_xmit_pkts; > > dev->tx_pkt_prepare =3D &fm10k_prep_pkts; > > > > - /* only initialize in the primary process */ > > - if (rte_eal_process_type() !=3D 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() !=3D 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 |=3D RTE_ETH_DEV_DETACHABLE; > > -- > > 1.8.3.1