netdev - RE: [PATCH] ixgbe: Remove bimodal SR-IOV disabling

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: <C5551D9AAB213A418B7FD5E4A6F30A0789336065@ORSMSX108.amr.corp.intel.com>
Date:	Fri, 10 Jul 2015 21:36:09 +0000
From:	"Rose, Gregory V" <gregory.v.rose@...el.com>
To:	Alex Williamson <alex.williamson@...hat.com>,
	"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] ixgbe: Remove bimodal SR-IOV disabling


> -----Original Message-----
> From: Alex Williamson [mailto:alex.williamson@...hat.com]
> Sent: Friday, July 10, 2015 2:32 PM
> To: intel-wired-lan@...ts.osuosl.org; Kirsher, Jeffrey T
> Cc: netdev@...r.kernel.org; linux-kernel@...r.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@...hat.com>
> Cc: Greg Rose <gregory.v.rose@...el.com>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@...el.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