From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>
Cc: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>, dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 2/2] net/mlx5: panic when destroying a queue in use
Date: Wed, 12 Apr 2017 19:04:28 +0200 [thread overview]
Message-ID: <20170412170428.GC3790@6wind.com> (raw)
In-Reply-To: <6cc1a018-3d2b-e51b-97f3-737b93dccc26@intel.com>
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@6wind.com>
> > Acked-by: Adrien Mazarguil <adrien.mazarguil@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
next prev parent reply other threads:[~2017-04-12 17:04 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-11 15:21 [dpdk-dev] [PATCH 1/2] net/mlx5: fix flow queues array allocation Nelio Laranjeiro
2017-04-11 15:21 ` [dpdk-dev] [PATCH 2/2] net/mlx5: panic when destroying a queue in use Nelio Laranjeiro
2017-04-12 16:48 ` Ferruh Yigit
2017-04-12 17:04 ` Adrien Mazarguil [this message]
2017-04-12 15:02 ` [dpdk-dev] [PATCH 1/2] net/mlx5: fix flow queues array allocation Ferruh Yigit
2017-04-12 16:51 ` Ferruh Yigit
2017-04-13 11:00 ` Ferruh Yigit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170412170428.GC3790@6wind.com \
--to=adrien.mazarguil@6wind.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=nelio.laranjeiro@6wind.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).