[dpdk-dev] [PATCH 2/2] net/mlx5: panic when destroying a queue in use

Adrien Mazarguil adrien.mazarguil at 6wind.com
Wed Apr 12 19:04:28 CEST 2017


On Wed, Apr 12, 2017 at 05:48:40PM +0100, Ferruh Yigit wrote:
> On 4/11/2017 4:21 PM, Nelio Laranjeiro wrote:
> > Since the queue release API does not allow failures (return value is void)
> > and the flow API does not allow a queue to be released as long as a flow
> > rule depends on it, the only rational decision to avoid undefined behavior
> > is to panic in this situation.
> > 
> > Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro at 6wind.com>
> > Acked-by: Adrien Mazarguil <adrien.mazarguil at 6wind.com>
> 
> <...>
> 
> > @@ -1248,6 +1249,9 @@ mlx5_rx_queue_release(void *dpdk_rxq)
> >  	rxq_ctrl = container_of(rxq, struct rxq_ctrl, rxq);
> >  	priv = rxq_ctrl->priv;
> >  	priv_lock(priv);
> > +	if (priv_flow_rxq_in_use(priv, rxq))
> > +		rte_panic("Rx queue %p is still used by a flow and cannot be"
> > +			  " removed\n", (void *)rxq_ctrl);
> 
> Actually, this adds exit code to the PMD, not sure this is good idea.
> 
> There was a patch to remove them from libraries. The exit decision
> should be belong to the application, not to the PMD, what do you think?

I think rte_panic() makes sense in such cases. It's a voluntary crash
similar to assert() (BUG_ON() for kernel folks) to avoids memory corruption
and other nasty things which unfortunately can occur when applications
happen to do things in the wrong order.

In this specific case, it is caused by shortcomings in the current API,
namely the lack of a return value for this function, which cannot be changed
at this stage. This is not a fix as there is not really a commit to blame.

> 
> >  	for (i = 0; (i != priv->rxqs_n); ++i)
> >  		if ((*priv->rxqs)[i] == rxq) {
> >  			DEBUG("%p: removing RX queue %p from list",
> > 
> 

-- 
Adrien Mazarguil
6WIND


More information about the dev mailing list