[Intel-wired-lan] [PATCH] ixgbe: Remove bimodal SR-IOV disabling
Rose, Gregory V
gregory.v.rose at intel.com
Fri Jul 10 21:36:09 UTC 2015
> -----Original Message-----
> From: Alex Williamson [mailto:alex.williamson at redhat.com]
> Sent: Friday, July 10, 2015 2:32 PM
> To: intel-wired-lan at lists.osuosl.org; Kirsher, Jeffrey T
> Cc: netdev at vger.kernel.org; linux-kernel at vger.kernel.org; Rose, Gregory V
> Subject: [PATCH] ixgbe: Remove bimodal SR-IOV disabling
>
> When unbinding an SR-IOV device with VFs configured from ixgbe, the driver
> behaves in one of two ways. If max_vfs was specified, the SR-IOV state is
> disabled, removing the VFs. The occurs regardless of whether the VF count
> was later modified through sysfs. If however max_vfs is zero, such as by
> not specifying the module parameter, the VFs persist after the PF is
> unbound from ixgbe. If the PF is then bound to vfio-pci to be assigned to
> a VM, the PF is non-functional.
>
> >From the comment, commit da36b64736cf ("ixgbe: Implement PCI SR-IOV
> sysfs callback operation") clearly intended this alternate behavior, but
> probably didn't realize the PF doesn't work in this mode.
>
> This bimodal behavior is confusing to users and results in a state where
> the PF is broken for other uses unless the user sets sriov_numvfs to zero
> prior to unbinding the device. Remove this behavior so that VFs are
> removed and the PF is functional for other uses after unbind, regardless
> of the way VFs are enabled.
>
> Signed-off-by: Alex Williamson <alex.williamson at redhat.com>
> Cc: Greg Rose <gregory.v.rose at intel.com>
> Cc: Jeff Kirsher <jeffrey.t.kirsher at intel.com>
> ---
>
> I can only think that not disabling SR-IOV was meant to enable some sort
> of persistence for VFs, but that's probably better accomplished with
> either udev rules and/or modprobe.d install scripts.
>
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 5be12a0..de04e3e 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -8810,12 +8810,7 @@ static void ixgbe_remove(struct pci_dev *pdev)
> unregister_netdev(netdev);
>
> #ifdef CONFIG_PCI_IOV
> - /*
> - * Only disable SR-IOV on unload if the user specified the now
> - * deprecated max_vfs module parameter.
> - */
> - if (max_vfs)
> - ixgbe_disable_sriov(adapter);
> + ixgbe_disable_sriov(adapter);
> #endif
> ixgbe_clear_interrupt_scheme(adapter);
>
Please remove max_vfs module parameter - it is deprecated and should be removed from upstream builds. Dave let us get away with a kernel module a few years ago because the other necessary infrastructure to enable SR-IOV virtual functions via the PCIe interface was not available. Now that it's there it should be removed and vendors/end users should be forced to move away from this.
Thanks,
- Greg Rose
Intel Corp.
Networking Division
More information about the Intel-wired-lan
mailing list