From: Matan Azrad <matan@mellanox.com>
To: Adrien Mazarguil <adrien.mazarguil@6wind.com>,
Shahaf Shuler <shahafs@mellanox.com>
Cc: Mordechay Haimovsky <motih@mellanox.com>,
"dev@dpdk.org" <dev@dpdk.org>,
"stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH v3] net/mlx4: fix dev rmv not detected after port stop
Date: Wed, 31 Jan 2018 10:08:06 +0000 [thread overview]
Message-ID: <AM4PR0501MB2657356B2CA46A039B96F7F6D2FB0@AM4PR0501MB2657.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <20180131091513.GS4256@6wind.com>
Hi all
From: Adrien Mazarguil
> On Tue, Jan 30, 2018 at 08:37:06PM +0000, Shahaf Shuler wrote:
> > Tuesday, January 30, 2018 11:40 AM, Adrien Mazarguil:
> > > Unfortunately I didn't get a chance to review this patch before it was
> applied.
> > > I'm not sure a stopped port is supposed to report events
> > > (interrupts). Will applications expect them to occur at this point?
> >
> > Why not?
> >
> > Stopped port is still counted as attached. The fact the application stopped
> the packet receive on it doesn't mean it should not receive a sync events
> (such as the remove event).
> > async events, by definition, are not related to traffic being flows through
> the port.
>
> My comment is based on my understanding of rte_eth_dev_stop(), which is
> a device (or port) is completely stopped, in a suspended state and no
> interrupts shall occur, as a means for applications to temporarily not be
> bothered by them until restarted.
Stopping traffic is not saying that the application is not interesting in the device,
I think that you mean to dev_close().
Any event may still be usable for application between dev_stop() to dev_start(),
especially RMV or LCS can still be interested.
> Think about it that way: applications do not want to get interrupts
> immediately after the device is initialized, because they might not be ready
> to process them at this point. An explicit call to rte_eth_dev_start() tells the
> PMD when it's OK to do so. The converse is rte_eth_dev_stop().
So, they can delay the event registration to the time they interesting in the events.
And use event unregister when they are not interesting in it anymore.
> Stopping traffic can already be achieved by not polling from the application
> side, calling rte_eth_dev_[rt]x_queue_stop() and/or toggling RX/TX
> interrupts through rte_eth_dev_[rt]x_intr_enable(). rte_eth_dev_stop()
> provides lower-level device control.
I think it makes sense only for Rx interrupt which is traffic oriented(like stop and start).
> Perhaps documentation is not clear, however that's how LSC seems
> implemented in all PMDs; it gets disabled after rte_eth_dev_stop() and one
> should explicitly use rte_eth_link_get() to retrieve link status afterward. I
> think RMV should behave similarly with rte_eth_dev_is_removed().
> Adapting fail-safe should be easier than modifying all the remaining PMDs.
Or maybe PMDs which do it make mistakes.
Matan.
> > > In my opinion it's not a fix, as in, it doesn't address an issue
> > > introduced by the mentioned patch whose behavior was correct.
> > >
> > > It's probably too late to change it now and it does address an issue
> > > seen with a use case involving this PMD, however I think the
> > > fail-safe PMD could as well poll using the recently-added
> > > rte_eth_dev_is_removed() when it's aware the underlying port is
> stopped instead of expecting interrupts.
> > >
> > > --
> > > Adrien Mazarguil
> > > 6WIND
>
> --
> Adrien Mazarguil
> 6WIND
next prev parent reply other threads:[~2018-01-31 10:08 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1516356548-15057-1-git-send-email-motih@mellanox.com>
2018-01-19 10:16 ` [dpdk-stable] [PATCH v2] net/mlx4: fix port start fail after device removal Moti Haimovsky
2018-01-28 13:54 ` Shahaf Shuler
2018-01-29 8:34 ` [dpdk-stable] [PATCH v3] net/mlx4: fix dev rmv not detected after port stop Moti Haimovsky
2018-01-29 10:54 ` Shahaf Shuler
2018-01-30 9:21 ` Thomas Monjalon
2018-01-30 9:39 ` Adrien Mazarguil
2018-01-30 20:37 ` Shahaf Shuler
2018-01-31 9:15 ` Adrien Mazarguil
2018-01-31 9:54 ` [dpdk-stable] [dpdk-dev] " Gaëtan Rivet
2018-01-31 10:08 ` Matan Azrad [this message]
2018-01-31 10:43 ` Adrien Mazarguil
2018-01-31 13:44 ` Matan Azrad
2018-01-31 14:31 ` Adrien Mazarguil
2018-01-31 17:07 ` Matan Azrad
2018-02-02 19:53 ` Adrien Mazarguil
2018-02-03 19:42 ` 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=AM4PR0501MB2657356B2CA46A039B96F7F6D2FB0@AM4PR0501MB2657.eurprd05.prod.outlook.com \
--to=matan@mellanox.com \
--cc=adrien.mazarguil@6wind.com \
--cc=dev@dpdk.org \
--cc=motih@mellanox.com \
--cc=shahafs@mellanox.com \
--cc=stable@dpdk.org \
/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).