Re: [Intel-wired-lan] [PATCH net] igb: clean up in all error paths when enabling SR-IOV

[Tony Nguyen <anthony.l.nguyen@intel.com>]

On 8/24/2023 6:07 AM, Corinna Vinschen wrote:
> Question to the Intel folks:
> 
> On Aug 24 11:16, Corinna Vinschen wrote:
>> After commit 50f303496d92 ("igb: Enable SR-IOV after reinit"), removing
>> the igb module could hang or crash (depending on the machine) when the
>> module has been loaded with the max_vfs parameter set to some value != 0.
>>
>> In case of one test machine with a dual port 82580, this hang occured:
>> [...]
>> The reproducer was a simple script:
>>
>>    #!/bin/sh
>>    for i in `seq 1 5`; do
>>      modprobe -rv igb
>>      modprobe -v igb max_vfs=1
>>      sleep 1
>>      modprobe -rv igb
>>    done
>>
>> It turned out that this could only be reproduce on 82580 (quad and
>> dual-port), but not on 82576, i350 and i210.  Further debugging showed
>> that igb_enable_sriov()'s call to pci_enable_sriov() is failing, because
>> dev->is_physfn is 0 on 82580.
> 
> Along these lines, isn't the first and foremost bug that igb_enable_sriov()
> has been called for this NIC at all?  In terms of patches, shouldn't the
> guard expression in igb_probe_vfs()
> 
>          /* Virtualization features not supported on i210 family. */
> 	if ((hw->mac.type == e1000_i210) || (hw->mac.type == e1000_i211))
> 		return;
> 
> get changed to:
> 
>          /* Virtualization features not supported on i210 and 82580 family. */
> 	if ((hw->mac.type == e1000_i210) || (hw->mac.type == e1000_i211) ||
> 	    (hw->mac.type == e1000_82580))
> 		return;

Hi Corinna,

Adding 82580 to this seems like a good change; did you want to submit a 
patch to do this?

Thanks,
Tony
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan