| 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
| ||
|
Date: Fri, 21 Sep 2012 18:35:58 +0100
From: Ben Hutchings <bhutchings@...arflare.com>
To: Yinghai Lu <yinghai@...nel.org>
CC: "Rose, Gregory V" <gregory.v.rose@...el.com>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Yuval Mintz <yuvalmin@...adcom.com>,
"davem@...emloft.net" <davem@...emloft.net>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
Ariel Elior <ariele@...adcom.com>,
Eilon Greenstein <eilong@...adcom.com>,
linux-pci <linux-pci@...r.kernel.org>
Subject: Re: New commands to configure IOV features
On Thu, 2012-09-20 at 22:50 -0700, Yinghai Lu wrote:
[...]
> Index: linux-2.6/include/linux/pci.h
> ===================================================================
> --- linux-2.6.orig/include/linux/pci.h
> +++ linux-2.6/include/linux/pci.h
> @@ -663,6 +663,7 @@ struct pci_driver {
> const struct pci_device_id *id_table; /* must be non-NULL for probe to be called */
> int (*probe) (struct pci_dev *dev, const struct pci_device_id *id); /* New device inserted */
> void (*remove) (struct pci_dev *dev); /* Device removed (NULL if not a hot-plug capable driver) */
> + void (*set_max_vfs) (struct pci_dev *dev); /* enable sriov */
> int (*suspend) (struct pci_dev *dev, pm_message_t state); /* Device suspended */
> int (*suspend_late) (struct pci_dev *dev, pm_message_t state);
> int (*resume_early) (struct pci_dev *dev);
Not sure about this; the driver may fail to enable those VFs and it
would be nice to be able to report that failure directly.
That said, this is 'max_vfs' (a limit) and if the driver fails to set up
all the VFs then the *limit* may still be changed.
> Subject: [PATCH] PCI: Add max_vfs in sysfs per pci device where supports
>
> driver later need to check dev->max_vfs in /sys
>
> Signed-off-by: Yinghai Lu <yinghai@...nel.org>
[...]
> +static ssize_t
> +max_vfs_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + unsigned long val;
> + struct pci_dev *pdev = to_pci_dev(dev);
> +
> + if (strict_strtoul(buf, 0, &val) < 0)
> + return -EINVAL;
> +
> + pdev->max_vfs = val;
> +
> + if (pdev->is_added) {
No locking required here?
> + int err;
> + err = device_schedule_callback(dev, max_vfs_callback);
Any particular reason this should be a callback?
[...]
> Index: linux-2.6/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ linux-2.6/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -129,13 +129,6 @@ static struct notifier_block dca_notifie
> };
> #endif
>
> -#ifdef CONFIG_PCI_IOV
> -static unsigned int max_vfs;
> -module_param(max_vfs, uint, 0);
> -MODULE_PARM_DESC(max_vfs,
> - "Maximum number of virtual functions to allocate per
> physical function - default is zero and maximum value is 63");
> -#endif /* CONFIG_PCI_IOV */
> -
> static unsigned int allow_unsupported_sfp;
> module_param(allow_unsupported_sfp, uint, 0);
> MODULE_PARM_DESC(allow_unsupported_sfp,
> @@ -4528,7 +4521,7 @@ static int __devinit ixgbe_sw_init(struc
> #ifdef CONFIG_PCI_IOV
> /* assign number of SR-IOV VFs */
> if (hw->mac.type != ixgbe_mac_82598EB)
> - adapter->num_vfs = (max_vfs > 63) ? 0 : max_vfs;
> + adapter->num_vfs = (pdev->max_vfs > 63) ? 0 : pdev->max_vfs;
We are trying to make all SR-IOV capable drivers work the same, so this
weird limiting behaviour should not be retained.
So I think the correct assignment is:
adapter->num_vfs = min(pdev->max_vfs, 63);
> #endif
> /* enable itr by default in dynamic mode */
> @@ -7249,8 +7242,9 @@ static int __devinit ixgbe_probe(struct
>
> #ifdef CONFIG_PCI_IOV
> ixgbe_enable_sriov(adapter, ii);
> -
> #endif
> + adapter->ixgbe_info = ii;
> +
> netdev->features = NETIF_F_SG |
> NETIF_F_IP_CSUM |
> NETIF_F_IPV6_CSUM |
> @@ -7720,11 +7714,42 @@ static const struct pci_error_handlers i
> .resume = ixgbe_io_resume,
> };
>
> +static void ixgbe_set_max_vfs(struct pci_dev *pdev)
> +{
> +#ifdef CONFIG_PCI_IOV
> + struct ixgbe_adapter *adapter = pci_get_drvdata(pdev);
> + struct net_device *netdev = adapter->netdev;
> + struct ixgbe_hw *hw = &adapter->hw;
> + int num_vfs = 0;
> +
> + /* assign number of SR-IOV VFs */
> + if (hw->mac.type != ixgbe_mac_82598EB)
> + num_vfs = (pdev->max_vfs > 63) ? 0 : pdev->max_vfs;
> +
> + /* no change */
> + if (adapter->num_vfs == num_vfs)
> + return;
> +
> + if (!num_vfs) {
> + /* disable sriov */
> + ixgbe_disable_sriov(adapter);
> + adapter->num_vfs = 0;
> + } else if (!adapter->num_vfs && num_vfs) {
> + /* enable sriov */
> + adapter->num_vfs = num_vfs;
> + ixgbe_enable_sriov(adapter, adapter->ixgbe_info);
> + } else {
> + /* increase or decrease */
Indeed, increase or decrease is not supported either in our PCI API or
in the SR-IOV spec. I think I would prefer the PCI core to filter out
unsupported changes (i.e. not call set_max_vfs and maybe report an
error), but I'm not sure about that.
Ben.
> + }
> +#endif
> +}
> +
[...]
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
--
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
Powered by blists - more mailing lists