netdev - Re: [RFC PATCH 05/12] ixgbe: Add SR-IOV features to main module

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: <20091210010416.GJ4163@verge.net.au>
Date:	Thu, 10 Dec 2009 12:04:17 +1100
From:	Simon Horman <horms@...ge.net.au>
To:	"Rose, Gregory V" <gregory.v.rose@...el.com>
Cc:	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"gospo@...hat.com" <gospo@...hat.com>,
	"Waskiewicz Jr, Peter P" <peter.p.waskiewicz.jr@...el.com>
Subject: Re: [RFC PATCH 05/12] ixgbe: Add SR-IOV features to main module

On Wed, Dec 09, 2009 at 04:49:26PM -0800, Rose, Gregory V wrote:
> >-----Original Message-----
> >From: Simon Horman [mailto:horms@...ge.net.au]
> >Sent: Wednesday, December 09, 2009 4:46 PM
> >To: Kirsher, Jeffrey T
> >Cc: netdev@...r.kernel.org; gospo@...hat.com; Rose, Gregory V;
> >Waskiewicz Jr, Peter P
> >Subject: Re: [RFC PATCH 05/12] ixgbe: Add SR-IOV features to main module
> >
> >> @@ -5767,6 +5954,52 @@ static int __devinit ixgbe_probe(struct pci_dev
> >*pdev,
> >>  		goto err_sw_init;
> >>  	}
> >>
> >> +#ifdef CONFIG_PCI_IOV
> >> +	if (hw->mac.type == ixgbe_mac_82599EB) {
> >> +		/* The 82599 supports up to 64 VFs per physical function
> >> +		 * but this implementation limits allocation to 63 so that
> >> +		 * basic networking resources are still available to the
> >> +		 * physical function
> >> +		 */
> >> +		adapter->num_vfs = (max_vfs > 63) ? 63 : max_vfs;
> >> +		if (adapter->num_vfs) {
> >> +			adapter->flags |= IXGBE_FLAG_SRIOV_ENABLED;
> >> +			err = pci_enable_sriov(pdev, adapter->num_vfs);
> >> +			if (err) {
> >> +				DPRINTK(PROBE, ERR,
> >> +					"Failed to enable PCI sriov: %d\n",
> >> +					err);
> >> +				adapter->flags &= ~IXGBE_FLAG_SRIOV_ENABLED;
> >> +				adapter->num_vfs = 0;
> >> +			}
> >> +		}
> >> +		/* If call to enable VFs succeeded then allocate memory
> >> +		 * for per VF control structures.
> >> +		 */
> >> +		if (adapter->flags & IXGBE_FLAG_SRIOV_ENABLED) {
> >> +			adapter->vfinfo =
> >> +				kcalloc(adapter->num_vfs,
> >> +					sizeof(struct vf_data_storage),
> >> +					GFP_KERNEL);
> >> +			if (!adapter->vfinfo) {
> >> +				/* Oh oh */
> >> +				DPRINTK(PROBE, ERR,
> >> +					"Unable to allocate memory for VF "
> >> +					"Data Storage - SRIOV disabled\n");
> >> +				adapter->flags &= ~IXGBE_FLAG_SRIOV_ENABLED;
> >> +				adapter->num_vfs = 0;
> >
> >A call to pci_disable_sriov(pdev) is needed here?
> [Rose, Gregory V] 
> 
> Good catch.

Thanks. Actually I only noticed while doing the error paths
for my suggestion below.

> >> +			} else {
> >> +				/* Now that we're sure SR-IOV is enabled
> >> +				 * set up the mailbox parameters
> >> +				 */
> >> +				ixgbe_init_mbx_params_pf(hw);
> >> +				memcpy(&hw->mbx.ops, ii->mbx_ops,
> >> +				       sizeof(hw->mbx.ops));
> >> +			}
> >> +		}
> >> +	}
> >> +
> >> +#endif /* CONFIG_PCI_IOV */
> >>  	netdev->features = NETIF_F_SG |
> >>  	                   NETIF_F_IP_CSUM |
> >>  	                   NETIF_F_HW_VLAN_TX |
> >
> >I wonder if it would be easier on the eyes to break the hunk above out
> >into
> >a ixgbe_probe_vf() function. Something like (compile tested only, I
> >don't
> >have an 82599):
> [Rose, Gregory V] 
> 
> I think that was done with the igb driver.  It's a good suggestion.

Yes, something like that is in igb.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html