Re: [RFC PATCH 0/7] vfio/pci: SR-IOV support

[Jerin Jacob]

On Tue, Feb 11, 2020 at 10:36 PM Alex Williamson
<alex.williamson@xxxxxxxxxx> wrote:
>
> On Tue, 11 Feb 2020 16:48:47 +0530
> Jerin Jacob <jerinjacobk@xxxxxxxxx> wrote:
>
> > On Wed, Feb 5, 2020 at 4:35 AM Alex Williamson
> > <alex.williamson@xxxxxxxxxx> wrote:
> > >
> > > There seems to be an ongoing desire to use userspace, vfio-based
> > > drivers for both SR-IOV PF and VF devices.  The fundamental issue
> > > with this concept is that the VF is not fully independent of the PF
> > > driver.  Minimally the PF driver might be able to deny service to the
> > > VF, VF data paths might be dependent on the state of the PF device,
> > > or the PF my have some degree of ability to inspect or manipulate the
> > > VF data.  It therefore would seem irresponsible to unleash VFs onto
> > > the system, managed by a user owned PF.
> > >
> > > We address this in a few ways in this series.  First, we can use a bus
> > > notifier and the driver_override facility to make sure VFs are bound
> > > to the vfio-pci driver by default.  This should eliminate the chance
> > > that a VF is accidentally bound and used by host drivers.  We don't
> > > however remove the ability for a host admin to change this override.
> > >
> > > The next issue we need to address is how we let userspace drivers
> > > opt-in to this participation with the PF driver.  We do not want an
> > > admin to be able to unwittingly assign one of these VFs to a tenant
> > > that isn't working in collaboration with the PF driver.  We could use
> > > IOMMU grouping, but this seems to push too far towards tightly coupled
> > > PF and VF drivers.  This series introduces a "VF token", implemented
> > > as a UUID, as a shared secret between PF and VF drivers.  The token
> > > needs to be set by the PF driver and used as part of the device
> > > matching by the VF driver.  Provisions in the code also account for
> > > restarting the PF driver with active VF drivers, requiring the PF to
> > > use the current token to re-gain access to the PF.
> >
> > Thanks Alex for the series. DPDK realizes this use-case through, an out of
> > tree igb_uio module, for non VFIO devices. Supporting this use case, with
> > VFIO, will be a great enhancement for DPDK as we are planning to
> > get rid of out of tree modules any focus only on userspace aspects.
> >
> > From the DPDK perspective, we have following use-cases
> >
> > 1) VF representer or OVS/vSwitch  use cases where
> > DPDK PF acts as an HW switch to steer traffic to VF
> > using the rte_flow library backed by HW CAMs.
> >
> > 2) Unlike, other PCI class of devices, Network class of PCIe devices
> > would have additional
> > capability on the PF devices such as promiscuous mode support etc
> > leverage that in DPDK
> > PF and VF use cases.
> >
> > That would boil down to the use of the following topology.
> > a)  PF bound to DPDK/VFIO  and  VF bound to Linux
> > b)  PF bound to DPDK/VFIO  and  VF bound to DPDK/VFIO
> >
> > Tested the use case (a) and it works this patch. Tested use case(b), it
> > works with patch provided both PF and VF under the same application.
> >
> > Regarding the use case where  PF bound to DPDK/VFIO and
> > VF bound to DPDK/VFIO are _two different_ processes then sharing the UUID
> > will be a little tricky thing in terms of usage. But if that is the
> > purpose of bringing
> > UUID to the equation then it fine.
> >
> > Overall this series looks good to me.  We can test the next non-RFC
> > series and give
> > Tested-by by after testing with DPDK.
>
> Thanks Jerin, that's great feedback.  For case b), it is rather the
> intention of the shared VF token proposed here that it imposes some
> small barrier in validating the collaboration between the PF and VF
> drivers.  In a trusted environment, a common UUID might be exposed in a
> shared file and the same token could be used by all PFs and VFs on the
> system, or datacenter.  The goal is simply to make sure the
> collaboration is explicit, I don't want to be fielding support issues
> from users assigning PFs and VFs to unrelated VM instances or
> unintentionally creating your scenario a) configuration.

Yes. Makes sense from kernel PoV.

DPDK side, probably we will end in hardcoded UUID value.

The tricky part would DPDK PF and QEMU VF integration case.
I am not sure about that integration( a command-line option for UUID) or
something more sophisticated orchestration. Anyway, it is clear from
kernel side,
Something needs to be sorted with the QEMU community.

> With the positive response from you and Thomas, I'll post a non-RFC
> version and barring any blockers maybe we can get this in for the v5.7
> kernel.  Thanks,

Great.

>
> Alex
>