netdev - Re: [PATCH V9 mlx5-next 09/15] vfio: Define device migration protocol v2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87k0dci989.fsf@redhat.com>
Date:   Wed, 02 Mar 2022 17:56:38 +0100
From:   Cornelia Huck <cohuck@...hat.com>
To:     Alex Williamson <alex.williamson@...hat.com>
Cc:     Jason Gunthorpe <jgg@...dia.com>,
        Yishai Hadas <yishaih@...dia.com>, bhelgaas@...gle.com,
        saeedm@...dia.com, linux-pci@...r.kernel.org, kvm@...r.kernel.org,
        netdev@...r.kernel.org, kuba@...nel.org, leonro@...dia.com,
        kwankhede@...dia.com, mgurtovoy@...dia.com, maorg@...dia.com,
        ashok.raj@...el.com, kevin.tian@...el.com,
        shameerali.kolothum.thodi@...wei.com
Subject: Re: [PATCH V9 mlx5-next 09/15] vfio: Define device migration
 protocol v2

On Wed, Mar 02 2022, Alex Williamson <alex.williamson@...hat.com> wrote:

> On Wed, 02 Mar 2022 17:07:21 +0100
> Cornelia Huck <cohuck@...hat.com> wrote:
>
>> On Wed, Mar 02 2022, Alex Williamson <alex.williamson@...hat.com> wrote:
>> 
>> > On Wed, 2 Mar 2022 10:27:32 -0400
>> > Jason Gunthorpe <jgg@...dia.com> wrote:
>> >  
>> >> On Wed, Mar 02, 2022 at 12:19:20PM +0100, Cornelia Huck wrote:  
>> >> > > +/*
>> >> > > + * vfio_mig_get_next_state - Compute the next step in the FSM
>> >> > > + * @cur_fsm - The current state the device is in
>> >> > > + * @new_fsm - The target state to reach
>> >> > > + * @next_fsm - Pointer to the next step to get to new_fsm
>> >> > > + *
>> >> > > + * Return 0 upon success, otherwise -errno
>> >> > > + * Upon success the next step in the state progression between cur_fsm and
>> >> > > + * new_fsm will be set in next_fsm.    
>> >> > 
>> >> > What about non-success? Can the caller make any assumption about
>> >> > next_fsm in that case? Because...    
>> >> 
>> >> I checked both mlx5 and acc, both properly ignore the next_fsm value
>> >> on error. This oddness aros when Alex asked to return an errno instead
>> >> of the state value.  
>> >
>> > Right, my assertion was that only the driver itself should be able to
>> > transition to the ERROR state.  vfio_mig_get_next_state() should never
>> > advise the driver to go to the error state, it can only report that a
>> > transition is invalid.  The driver may stay in the current state if an
>> > error occurs here, which is why we added the ability to get the device
>> > state.  Thanks,
>> >
>> > Alex  
>> 
>> So, should the function then write anything to next_fsm if it returns
>> -errno? (Maybe I'm misunderstanding.) Or should the caller always expect
>> that something may be written to new_fsm, and simply only look at it if
>> the function returns success?
>
> Note that this function doesn't actually transition the device to
> next_fsm, it's only informing the driver what the next state is.
> Therefore I think it's reasonable to expect that the caller is never
> going to use it's actual internal device state for next_fsm.  So I
> don't really see a case where we need to worry about preserving
> next_fsm in the error condition.  Thanks,
>
> Alex

Yeah, I guess any reasonable caller won't try to pass in their internal
state. Let's hope that any stupid usuage of that interface is caught
during review :)