From: "Gaëtan Rivet" <gaetan.rivet@6wind.com>
To: Matan Azrad <matan@mellanox.com>
Cc: Adrien Mazarguil <adrien.mazarguil@6wind.com>,
Thomas Monjalon <thomas@monjalon.net>,
dev@dpdk.org, stable@dpdk.org
Subject: Re: [dpdk-stable] [PATCH v2 4/4] net/failsafe: fix removed device handling
Date: Wed, 13 Dec 2017 16:16:41 +0100 [thread overview]
Message-ID: <20171213151641.g42zr7zupbsdgxsv@bidouze.vm.6wind.com> (raw)
In-Reply-To: <1513175370-16583-5-git-send-email-matan@mellanox.com>
Hi Matan,
On Wed, Dec 13, 2017 at 02:29:30PM +0000, Matan Azrad wrote:
> There is time between the physical removal of the device until
> sub-device PMDs get a RMV interrupt. At this time DPDK PMDs and
> applications still don't know about the removal and may call sub-device
> control operation which should return an error.
>
> In previous code this error is reported to the application contrary to
> fail-safe principle that the app should not be aware of device removal.
>
> Add an removal check in each relevant control command error flow and
> prevent an error report to application when the sub-device is removed.
>
> Fixes: a46f8d5 ("net/failsafe: add fail-safe PMD")
> Fixes: b737a1e ("net/failsafe: support flow API")
> Cc: stable@dpdk.org
>
This patch is not a fix.
It relies on an eth_dev API evolution. Without this evolution,
this patch is meaningless and would break compilation if backported in
stable branch.
Please remove those tags.
> Signed-off-by: Matan Azrad <matan@mellanox.com>
> ---
> drivers/net/failsafe/failsafe_flow.c | 18 ++++++++++-------
> drivers/net/failsafe/failsafe_ops.c | 34 ++++++++++++++++++++++-----------
> drivers/net/failsafe/failsafe_private.h | 10 ++++++++++
> 3 files changed, 44 insertions(+), 18 deletions(-)
< ... >
> +/*
> + * Check if sub device was removed.
> + */
> +static inline int
> +fs_is_removed(struct sub_device *sdev)
> +{
> + if (sdev->remove == 1 || rte_eth_dev_is_removed(PORT_ID(sdev)) != 0)
> + return 1;
> + return 0;
> +}
Have you considered adding this check within the subdev iterator itself?
I think it would prevent you from having to add it to each return value
checks.
It is still MT-unsafe anyway.
--
Gaëtan Rivet
6WIND
next prev parent reply other threads:[~2017-12-13 15:16 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1509637324-13525-1-git-send-email-matan@mellanox.com>
2017-11-02 15:42 ` [dpdk-stable] [PATCH 1/3] net/failsafe: fix removal handling lack Matan Azrad
2017-11-06 8:19 ` Gaëtan Rivet
[not found] ` <1513175370-16583-1-git-send-email-matan@mellanox.com>
2017-12-13 14:29 ` [dpdk-stable] [PATCH v2 4/4] net/failsafe: fix removed device handling Matan Azrad
2017-12-13 15:16 ` Gaëtan Rivet [this message]
2017-12-13 15:48 ` Matan Azrad
2017-12-13 16:09 ` Gaëtan Rivet
2017-12-13 17:09 ` Thomas Monjalon
2017-12-14 10:40 ` Matan Azrad
2017-12-13 21:55 ` Gaëtan Rivet
2017-12-14 10:40 ` Matan Azrad
2017-12-14 10:48 ` Gaëtan Rivet
2017-12-14 13:07 ` Matan Azrad
2017-12-14 13:27 ` Gaëtan Rivet
2017-12-14 14:43 ` Matan Azrad
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=20171213151641.g42zr7zupbsdgxsv@bidouze.vm.6wind.com \
--to=gaetan.rivet@6wind.com \
--cc=adrien.mazarguil@6wind.com \
--cc=dev@dpdk.org \
--cc=matan@mellanox.com \
--cc=stable@dpdk.org \
--cc=thomas@monjalon.net \
/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).