Re: [dpdk-dev] [PATCH] librte_ether: fix crashes in rte_ethdev functions.

["Iremonger, Bernard" <bernard.iremonger@intel.com>]

Hi Michael,

> -----Original Message-----
> From: Qiu, Michael
> Sent: Monday, December 21, 2015 9:03 AM
> To: Iremonger, Bernard <bernard.iremonger@intel.com>; dev@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@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);
> > +
> 
> 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.