RE: [PATCH net v2] iavf: Detach device during reset task

From: Keller, Jacob E
Date: Wed Aug 31 2022 - 16:09:56 EST




> -----Original Message-----
> From: Ivan Vecera <ivecera@xxxxxxxxxx>
> Sent: Wednesday, August 31, 2022 12:06 AM
> To: Laba, SlawomirX <slawomirx.laba@xxxxxxxxx>
> Cc: netdev@xxxxxxxxxxxxxxx; Keller, Jacob E <jacob.e.keller@xxxxxxxxx>;
> Piotrowski, Patryk <patryk.piotrowski@xxxxxxxxx>; Vitaly Grinberg
> <vgrinber@xxxxxxxxxx>; Brandeburg, Jesse <jesse.brandeburg@xxxxxxxxx>;
> Nguyen, Anthony L <anthony.l.nguyen@xxxxxxxxx>; David S. Miller
> <davem@xxxxxxxxxxxxx>; Eric Dumazet <edumazet@xxxxxxxxxx>; Jakub Kicinski
> <kuba@xxxxxxxxxx>; Paolo Abeni <pabeni@xxxxxxxxxx>; Jeff Kirsher
> <jeffrey.t.kirsher@xxxxxxxxx>; moderated list:INTEL ETHERNET DRIVERS <intel-
> wired-lan@xxxxxxxxxxxxxxxx>; open list <linux-kernel@xxxxxxxxxxxxxxx>
> Subject: Re: [PATCH net v2] iavf: Detach device during reset task
>
> On Tue, 30 Aug 2022 20:49:54 +0000
> "Laba, SlawomirX" <slawomirx.laba@xxxxxxxxx> wrote:
>
> > Ivan, what do you think about this flow [1]? Shouldn't it also goto reset_finish
> label?
> >
> > if (i == IAVF_RESET_WAIT_COMPLETE_COUNT) {
> > dev_err(&adapter->pdev->dev, "Reset never finished (%x)\n",
> > reg_val);
> > iavf_disable_vf(adapter);
> > mutex_unlock(&adapter->client_lock);
> > mutex_unlock(&adapter->crit_lock);
> > return; /* Do not attempt to reinit. It's dead, Jim. */
> > }
> >
> > I am concerned that if the reset never finishes and iavf goes into disabled state,
> and then for example if driver reload operation is performed, bad things can
> happen.
>
> I think we should not re-attach device back as the VF is disabled. Detached device
> causes that userspace (user) won't be able to configure associated interface
> that is correct. Driver reload won't cause anything special in this situation
> because during module removal the interface is unregistered.
>
> Thanks,
> Ivan

I agree. It's safe to remove a detached device.

Thanks,
Jake