> -----Original Message----- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Iremonger, Bernard > Sent: Monday, December 21, 2015 11:40 AM > To: Qiu, Michael; dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH] librte_ether: fix crashes in rte_ethdev > functions. > > Hi Michael, > > > -----Original Message----- > > From: Qiu, Michael > > Sent: Monday, December 21, 2015 9:03 AM > > To: Iremonger, Bernard <bernard.iremonger at intel.com>; dev at dpdk.org > > Subject: Re: [dpdk-dev] [PATCH] librte_ether: fix crashes in rte_ethdev > > functions. > > > > On 2015/12/18 1:24, Bernard Iremonger wrote: > > > The nb_rx_queues and nb_tx_queues are initialised before the tx_queue > > > and rx_queue arrays are allocated. The arrays are allocated when the > > > ethdev port is started. > > > > > > If any of the following functions are called before the ethdev port is > > > started there is a segmentation fault: > > > > > > rte_eth_stats_get > > > rte_eth_stats_reset > > > rte_eth_xstats_get > > > rte_eth_xstats_reset > > > > > > Fixes: af75078fece3 ("first public release") > > > Fixes: ce757f5c9a4d ("ethdev: new method to retrieve extended > > > statistics") > > > Fixes: d4fef8b0d5e5 ("ethdev: expose generic and driver specific stats > > > in xstats") > > > Signed-off-by: Bernard Iremonger <bernard.iremonger at intel.com> > > > --- > > > lib/librte_ether/rte_ethdev.c | 16 ++++++++++++---- > > > 1 file changed, 12 insertions(+), 4 deletions(-) > > > > > > diff --git a/lib/librte_ether/rte_ethdev.c > > > b/lib/librte_ether/rte_ethdev.c index ed971b4..a0ee84d 100644 > > > --- a/lib/librte_ether/rte_ethdev.c > > > +++ b/lib/librte_ether/rte_ethdev.c > > > @@ -1441,7 +1441,10 @@ rte_eth_stats_get(uint8_t port_id, struct > > rte_eth_stats *stats) > > > memset(stats, 0, sizeof(*stats)); > > > > > > RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->stats_get, - > > ENOTSUP); > > > - (*dev->dev_ops->stats_get)(dev, stats); > > > + > > > + if (dev->data->dev_started) > > > + (*dev->dev_ops->stats_get)(dev, stats); > > > +
So why it would be no possible now to get statistics on the stopped device? Konstantin > > > > My question is should we mark an error or a warning here and return an > > error so that the caller knows what happens? > > > > Thanks, > > Michael > > <snip> > > In other cases in rte_ethdev.c where there is a check on > "dev->data->dev_started" there is a RTE_PMD_DEBUG_TRACE() line. I will > add a RTE_PMD_DEBUG_TRACE() line. > The rte_eth_stats_reset() and rte_eth_xstats_reset() functions return void. > Not sure if an error is required for the rte_eth_stats_get() and > rte_eth_xstats_get() functions as the stats information returned is all > zero's at present. > > Regards, > > Bernard.
