[Intel-wired-lan] [PATCH] igb: fix NULL derefs due to skipped SR-IOV enabling
Stefan Assmann
sassmann at kpanic.de
Mon Oct 19 10:50:39 UTC 2015
On 19.10.2015 12:23, Jan Beulich wrote:
> The combined effect of commits 6423fc3416 ("igb: do not re-init SR-IOV
> during probe") and ceee3450b3 ("igb: make sure SR-IOV init uses the
> right number of queues") causes VFs no longer getting set up, leading
> to NULL pointer dereferences due to the adapter's ->vf_data being NULL
> while ->vfs_allocated_count is non-zero. The first commit not only
> neglected the side effect of igb_sriov_reinit() that the second commit
> tried to account for, but also that of setting IGB_FLAG_HAS_MSIX,
> without which igb_enable_sriov() is effectively a no-op. Calling
> igb_{,re}set_interrupt_capability() as done here seems to address this,
> but I'm not sure whether this is better than sinply reverting the other
> two commits.
Jan,
this should already be fixed by the following net-next commit
cbfe360a1541a32e9e28f8f8ac925d2b7979d767 igb: assume MSI-X interrupts during initialization
Stefan
>
> Signed-off-by: Jan Beulich <jbeulich at suse.com>
> Cc: Stefan Assmann <sassmann at kpanic.de>
> Cc: Todd Fujinaka <todd.fujinaka at intel.com>
> Cc: Jesse Brandeburg <jesse.brandeburg at intel.com>
> Cc: Shannon Nelson <shannon.nelson at intel.com>
> Cc: Carolyn Wyborny <carolyn.wyborny at intel.com>
> Cc: Don Skidmore <donald.c.skidmore at intel.com>
> Cc: Matthew Vick <matthew.vick at intel.com>
> Cc: John Ronciak <john.ronciak at intel.com>
> Cc: Mitch Williams <mitch.a.williams at intel.com>
> ---
> As a side note, while looking at all the interactions here it seems
> suspicious to me that igb_request_msix() failure does not lead to
> SR-IOV being disabled on the device, despite IGB_FLAG_HAS_MSIX being
> set being a prereq to it as explained above.
> ---
> drivers/net/ethernet/intel/igb/igb_main.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> --- 4.3-rc6/drivers/net/ethernet/intel/igb/igb_main.c
> +++ 4.3-rc6-IGB-VF-setup/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -2856,6 +2856,14 @@ static void igb_probe_vfs(struct igb_ada
> if ((hw->mac.type == e1000_i210) || (hw->mac.type == e1000_i211))
> return;
>
> + /*
> + * Of the below we really only want the effect of getting
> + * IGB_FLAG_HAS_MSIX set (if available), without which
> + * igb_enable_sriov() has no effect.
> + */
> + igb_set_interrupt_capability(adapter, true);
> + igb_reset_interrupt_capability(adapter);
> +
> pci_sriov_set_totalvfs(pdev, 7);
> igb_enable_sriov(pdev, max_vfs);
>
>
>
>
More information about the Intel-wired-lan
mailing list