Re: [PATCH v3] net/mlx4: fix dev rmv not detected after port stop

[Adrien Mazarguil <adrien.mazarguil@6wind.com>]

Hi,

On Mon, Jan 29, 2018 at 10:54:16AM +0000, Shahaf Shuler wrote:
> Mordechay Haimovsky, Monday, January 29, 2018 10:35 AM:
> > In failsafe device start can be called for ports/devices that had been plugged
> > out.
> > The mlx4 PMD detects device removal by listening to the device RMV events,
> > when the mlx4 port is being stopped, the PMD no longer listens to these
> > events causing the PMD to stop detecting device removals.
> > This patch fixes this issue by moving installation of the interrupt handler to
> > device configuration, and toggle only the Rx-queue interrupts on start/stop.
> > 
> > Fixes: a6e8b01c3c26 ("net/mlx4: compact interrupt functions")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Moti Haimovsky <motih@mellanox.com>
> > ---
> > V3:
> > Modifications according to review inputs from Shahaf Shuler
> > See: 1516357009-15463-1-git-send-email-motih@mellanox.com
> > 
> > V2:
> > Fixed commit message.
> > ---
> > 
> >  drivers/net/mlx4/mlx4.c      | 10 ++++++++--
> >  drivers/net/mlx4/mlx4.h      |  2 ++
> >  drivers/net/mlx4/mlx4_intr.c | 41
> > ++++++++++++++++++++++++++++++++++++++---
> >  3 files changed, 48 insertions(+), 5 deletions(-)
> > 
> 
> Acked-by: Shahaf Shuler <shahafs@mellanox.com> 
> 
> Adrien - let me know if you see issues with this patch, I want to include it on RC2.

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?

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