New commands to configure IOV features
Subsystems (4)
I've tried to figure out if there was a standard interface (ethtool/iproute) through which a user could configure the number of vfs in his system.
I've seen the RFC suggested in http://markmail.org/thread/qblfcv7zbxsxp7q6,
and http://markmail.org/thread/fw54dcppmxuxoe6n, but failed to see any later references to it (commits or further discussion on this topic).
How exactly are things standing with these RFCs? Were they abandoned?
Thanks,
Yuval
Quoted 10 lines · On Mon, 7 May 2012 14:17:54 +0300 Yuval Mintz <[email protected]> wrote:
I've tried to figure out if there was a standard interface (ethtool/iproute) through which a user could configure the number of vfs in his system.
I've seen the RFC suggested in
http://markmail.org/thread/qblfcv7zbxsxp7q6, and
http://markmail.org/thread/fw54dcppmxuxoe6n, but failed to see any later references to it (commits or further discussion on this topic).How exactly are things standing with these RFCs? Were they abandoned?
The only way to configure the number of VFs continues to be through the max_vfs module parameter. I've got a patch to do it through ethtool sitting on the back burner but due to other requirements of my day job I've not been able to work on it since last fall.
- Greg
Quoted 8 lines
Thanks,
Yuval--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 12 lines · Greg Rose
Quoted 9 lines · On Mon, 7 May 2012 14:17:54 +0300 Yuval Mintz <[email protected]> wrote:
I've tried to figure out if there was a standard interface (ethtool/iproute) through which a user could configure the number of vfs in his system.
I've seen the RFC suggested in
http://markmail.org/thread/qblfcv7zbxsxp7q6, and
http://markmail.org/thread/fw54dcppmxuxoe6n, but failed to see any later references to it (commits or further discussion on this topic).
Quoted 8 lines
Quoted 1 line
How exactly are things standing with these RFCs? Were they abandoned?
The only way to configure the number of VFs continues to be through the
max_vfsmodule parameter. I've got a patch to do it through ethtool sitting on the back burner but due to other requirements of my day job I've not been able to work on it since last fall.- Greg
Hi Ben,
If I want to pick the RFCs and add support for configuring the number of VFs - do you think ethtool's the right place for such added support?
I'm asking since as far as I can see, ethtool (today) doesn't contain any features related to virtual functions.
Thanks,
Yuval
Quoted 24 lines · Yuval Mintz
Quoted 12 lines · Greg Rose
Quoted 9 lines · On Mon, 7 May 2012 14:17:54 +0300 Yuval Mintz <[email protected]> wrote:
I've tried to figure out if there was a standard interface (ethtool/iproute) through which a user could configure the number of vfs in his system.
I've seen the RFC suggested in
http://markmail.org/thread/qblfcv7zbxsxp7q6, and
http://markmail.org/thread/fw54dcppmxuxoe6n, but failed to see any later references to it (commits or further discussion on this topic).Quoted 7 lines
Quoted 1 line
How exactly are things standing with these RFCs? Were they abandoned?
The only way to configure the number of VFs continues to be through the
max_vfsmodule parameter. I've got a patch to do it through ethtool sitting on the back burner but due to other requirements of my day job I've not been able to work on it since last fall.- Greg
Hi Ben,
Quoted 8 lines
If I want to pick the RFCs and add support for configuring the number of VFs - do you think ethtool's the right place for such added support?
I'm asking since as far as I can see, ethtool (today) doesn't contain any features related to virtual functions.
Thanks,
Yuval
I think the issue is that any class of PCI device could theoretically support SR-IOV. For example there could be a storage controller out there that supports spawning VFs, and an ethtool solution wouldn't work for a device like that. Personally what I would like to see is a solution that is more focused on the PCI side of the network adapters instead of the network side when it comes to enabling VFs.
Thanks,
Alex
Quoted 26 lines · On Tue, 26 Jun 2012 15:21:55 +0300 Yuval Mintz <[email protected]> wrote:
Quoted 16 lines · Greg Rose
Quoted 4 lines · On Mon, 7 May 2012 14:17:54 +0300 Yuval Mintz <[email protected]> wrote:
I've tried to figure out if there was a standard interface (ethtool/iproute) through which a user could configure the number of vfs in his system.
Quoted 8 lines
I've seen the RFC suggested in
http://markmail.org/thread/qblfcv7zbxsxp7q6, and
http://markmail.org/thread/fw54dcppmxuxoe6n, but failed to see any later references to it (commits or further discussion on this topic).How exactly are things standing with these RFCs? Were they abandoned?
Quoted 6 lines
The only way to configure the number of VFs continues to be through the
max_vfsmodule parameter. I've got a patch to do it through ethtool sitting on the back burner but due to other requirements of my day job I've not been able to work on it since last fall.- Greg
Quoted 8 lines
Hi Ben,
If I want to pick the RFCs and add support for configuring the number of VFs - do you think ethtool's the right place for such added support?
I'm asking since as far as I can see, ethtool (today) doesn't contain any features related to virtual functions.
I think a PCI utility tool would be better, SR-IOV is not limited to network devices. That's one of the reasons I dropped the RFC. I haven't gotten back to the idea since then due to my day job keeping me pretty busy.
- Greg
Quoted 31 lines
Quoted 22 lines
Quoted 14 lines
Quoted 5 lines
I've tried to figure out if there was a standard interface (ethtool/iproute) through which a user could configure the number of vfs in his system.
Quoted 8 lines
I've seen the RFC suggested in
http://markmail.org/thread/qblfcv7zbxsxp7q6, and
http://markmail.org/thread/fw54dcppmxuxoe6n, but failed to see any later references to it (commits or further discussion on this topic).How exactly are things standing with these RFCs? Were they abandoned?
Quoted 6 lines
The only way to configure the number of VFs continues to be through the
max_vfsmodule parameter. I've got a patch to do it through ethtool sitting on the back burner but due to other requirements of my day job I've not been able to work on it since last fall.- Greg
Quoted 8 lines
Hi Ben,
If I want to pick the RFCs and add support for configuring the number of VFs - do you think ethtool's the right place for such added support?
I'm asking since as far as I can see, ethtool (today) doesn't contain any features related to virtual functions.
Quoted 6 lines
I think a PCI utility tool would be better, SR-IOV is not limited to network devices. That's one of the reasons I dropped the RFC. I haven't gotten back to the idea since then due to my day job keeping me pretty busy.
- Greg
I'm adding the linux-pci guys, since they (obviously) are the experts in these topics.
Is there any PCI utility (or at least an existing kernel API) which is suitable for adding this sort of functionality?
Thanks,
Yuval
Quoted 35 lines · Greg Rose
Quoted 32 lines · On Tue, 26 Jun 2012 15:21:55 +0300 Yuval Mintz <[email protected]> wrote:
Quoted 16 lines · Greg Rose
Quoted 4 lines · On Mon, 7 May 2012 14:17:54 +0300 Yuval Mintz <[email protected]> wrote:
I've tried to figure out if there was a standard interface (ethtool/iproute) through which a user could configure the number of vfs in his system.
Quoted 8 lines
I've seen the RFC suggested in
http://markmail.org/thread/qblfcv7zbxsxp7q6, and
http://markmail.org/thread/fw54dcppmxuxoe6n, but failed to see any later references to it (commits or further discussion on this topic).How exactly are things standing with these RFCs? Were they abandoned?
Quoted 6 lines
The only way to configure the number of VFs continues to be through the
max_vfsmodule parameter. I've got a patch to do it through ethtool sitting on the back burner but due to other requirements of my day job I've not been able to work on it since last fall.- Greg
Hi Ben,
If I want to pick the RFCs and add support for configuring the number of VFs - do you think ethtool's the right place for such added support?
Quoted 7 lines
Quoted 2 lines
I'm asking since as far as I can see, ethtool (today) doesn't contain any features related to virtual functions.
I think a PCI utility tool would be better, SR-IOV is not limited to network devices. That's one of the reasons I dropped the RFC. I haven't gotten back to the idea since then due to my day job keeping me pretty busy.
For what it's worth, I agree with this.
Ben.
Signature · 3 lines
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.
Quoted 22 lines · Ben Hutchings
Quoted 21 lines · Greg Rose
Quoted 17 lines · On Tue, 26 Jun 2012 15:21:55 +0300 Yuval Mintz<[email protected]> wrote:
Quoted 6 lines · Greg Rose
The only way to configure the number of VFs continues to be through the
max_vfsmodule parameter. I've got a patch to do it through ethtool sitting on the back burner but due to other requirements of my day job I've not been able to work on it since last fall.- Greg
Hi Ben,
If I want to pick the RFCs and add support for configuring the number of VFs - do you think ethtool's the right place for such added support?
I'm asking since as far as I can see, ethtool (today) doesn't contain any features related to virtual functions.
Quoted 6 lines
Quoted 4 lines
I think a PCI utility tool would be better, SR-IOV is not limited to network devices. That's one of the reasons I dropped the RFC. I haven't gotten back to the idea since then due to my day job keeping me pretty busy.
For what it's worth, I agree with this.
I'd love to have the ability to set the number of VFs on the fly. From my perspective it would be ideal if this could be exported via /sys or something rather than a standalone tool using a binary API. It's fewer packages for userspace to deal with as well as being easily scripted.
Chris
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
If I want to pick the RFCs and add support for configuring the number of VFs - do you think ethtool's the right place for such added support?
Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
I think a PCI utility tool would be better, SR-IOV is not limited to network devices. That's one of the reasons I dropped the RFC. I haven't gotten back to the idea since then due to my day job keeping me pretty busy.
For what it's worth, I agree with this.
From my perspective it would be ideal if this could be exported via /sys or something
Well, obviously unless there was a sudden change in our stance regarding sysfs we will not head that way.
This thread got no replies from the pci community, and I'm unfamiliar with such a tool.
Dave, What's your stance in the matter - do you wish us to continue pursuing some pci tool (which might or might not exist), or instead work on a networking solution to this issue?
Do you happen to know such a tool?
Thanks,
Yuval
Quoted 28 lines · Yuval Mintz
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
If I want to pick the RFCs and add support for configuring the number of VFs - do you think ethtool's the right place for such added support?
Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
I think a PCI utility tool would be better, SR-IOV is not limited to network devices. That's one of the reasons I dropped the RFC. I haven't gotten back to the idea since then due to my day job keeping me pretty busy.
For what it's worth, I agree with this.
From my perspective it would be ideal if this could be exported via /sys or something
Well, obviously unless there was a sudden change in our stance regarding sysfs we will not head that way.
This thread got no replies from the pci community, and I'm unfamiliar with such a tool.
Dave, What's your stance in the matter - do you wish us to continue pursuing some pci tool (which might or might not exist), or instead work on a networking solution to this issue?
Do you happen to know such a tool?
Quoted 7 lines
Thanks,
Yuval--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Yuval (et. al.),
Not seeing the original thread on netdev,
I just had a recent discussion w/Greg Rose about providing sysfs-based, VF enable/disable methods.
I was told that historically, VF enablement started as a sysfs-based function, was debated and pushed toward a device/driver-specific method,
as it is implemented today. Now, with some experience with SRIOV and
its use in the virtualization space, the discussion has renewed as to whether a sysfs-based enable/disable method should be resurrected, so it provides a more generic method for virtualization tools/api's to manage SRIOV/VF devices.
I was hoping to discuss this topic with a number of folks at LinuxCon/Plumbers/KS when the PCI mini-summit is held, to gain further insight, or be brought up to speed on past history, and review current uses/status of VFs.
WRT SRIOV-nic devices, the thinking goes that protocol-level parameters associated with VFs should use protocol-specific interfaces, e.g., ethtool, ip link set, etc. for Ethernet VFs.
Thus, the various protocol control functions/tools should
be used to control VF parameters, as one would for a physical device of that protocol/class.
- Don
Quoted 6 lines · Don Dutile
WRT SRIOV-nic devices, the thinking goes that protocol-level parameters associated with VFs should use protocol-specific interfaces, e.g., ethtool, ip link set, etc. for Ethernet VFs.
Thus, the various protocol control functions/tools should
be used to control VF parameters, as one would for a physical device of that protocol/class.
It seems to me that the mere act of creating one or more VFs is something generic, applicable to all devices that are capable of it. The details of configuring those VFs can then be handled by protocol-specific interfaces.
I'm not too worried about the exact mechanism of doing it, as long as it's ultimately scriptable--that is, if it's a C API then it would be appreciated if there was a standard tool to call that implements it. From that perspective a sysfs-based interface is ideal since it is directly scriptable.
Chris
David Miller
maintainerFrom: Chris Friesen <[email protected]>
Date: Tue, 17 Jul 2012 15:08:45 -0600
Quoted 2 lines
From that perspective a sysfs-based interface is ideal since it is directly scriptable.
As is anything ethtool or netlink based, since we have 'ethtool' and 'ip' for scripting.
Quoted 8 lines · David Miller
From: Chris Friesen<[email protected]>
Date: Tue, 17 Jul 2012 15:08:45 -0600Quoted 2 lines
From that perspective a sysfs-based interface is ideal since it is
directly scriptable.As is anything ethtool or netlink based, since we have 'ethtool' and 'ip' for scripting.
I'm not picky...whatever works.
To me the act of creating virtual functions seems generic enough (I'm aware of SR-IOV capable storage controllers, I'm sure there is other hardware as well) that ethtool/ip don't really seem like the most appropriate tools for the job.
I would have thought it would make more sense as a generic PCI functionality, in which case I'm not aware of an existing binary tool that would be a logical choice to extend.
Chris
Quoted 7 lines · Chris Friesen
Quoted 6 lines · David Miller
From: Chris Friesen<[email protected]>
Date: Tue, 17 Jul 2012 15:08:45 -0600Quoted 2 lines
From that perspective a sysfs-based interface is ideal since it is directly scriptable.
Quoted 7 lines
Quoted 2 lines
As is anything ethtool or netlink based, since we have 'ethtool' and 'ip' for scripting.
I'm not picky...whatever works.
To me the act of creating virtual functions seems generic enough (I'm aware of SR-IOV capable storage controllers, I'm sure there is other hardware as well) that ethtool/ip don't really seem like the most appropriate tools for the job.
Yes, and then there are 'other network' controllers too ... IB which I don't know if it adheres to ethtool, since it's not an Ethernet device ... isn't that why they call it Infiniband ... ;-) )
In the telecom space, they use NTBs and PCI as a 'network' ... I know, not common in Linux space, and VFs in that space aren't being discussed (that I've ever heard), but another example where 'network' != Ethernet, so ethtool doesn't solve PCI-level configuration/use.
So, VFs are a PCI defined entity, so their enable/disablement should be handled by PCI.
Conversely, when dealing with networking attributes of a PCI-VF ethernet-nic, networking tools should be used, not PCI tools.
Quoted 3 lines
I would have thought it would make more sense as a generic PCI functionality, in which case I'm not aware of an existing binary tool that would be a logical choice to extend.
Chris
Quoted 15 lines · Don Dutile
Quoted 7 lines · Chris Friesen
Quoted 6 lines · David Miller
From: Chris Friesen<[email protected]>
Date: Tue, 17 Jul 2012 15:08:45 -0600Quoted 2 lines
From that perspective a sysfs-based interface is ideal since it is directly scriptable.
Quoted 7 lines
Quoted 2 lines
As is anything ethtool or netlink based, since we have 'ethtool' and 'ip' for scripting.
I'm not picky...whatever works.
To me the act of creating virtual functions seems generic enough (I'm aware of SR-IOV capable storage controllers, I'm sure there is other hardware as well) that ethtool/ip don't really seem like the most appropriate tools for the job.
Quoted 8 lines
Yes, and then there are 'other network' controllers too ... IB which I don't know if it adheres to ethtool, since it's not an Ethernet device ... isn't that why they call it Infiniband ... ;-) ) In the telecom space, they use NTBs and PCI as a 'network' ... I know, not common in Linux space, and VFs in that space aren't being discussed (that I've ever heard), but another example where 'network' != Ethernet, so ethtool doesn't solve PCI-level configuration/use.
[...]
The ethtool API is typically used for net device operations that can be largely devolved to individual drivers, and which the network stack can mostly ignore (though offload features are an historical exception to this). It started with Ethernet link settings, but many operations are applicable (and implemented by) other types of network device.
Ben.
Signature · 3 lines
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.
Quoted 6 lines · Ben Hutchings
The ethtool API is typically used for net device operations that can be largely devolved to individual drivers, and which the network stack can mostly ignore (though offload features are an historical exception to this). It started with Ethernet link settings, but many operations are applicable (and implemented by) other types of network device.
That (potentially) accounts for all network devices, but it leaves all the other devices that could export virtual functions.
Why should I need to use a different API to enable virtual functions on my network device and my storage controller? (And why should "ethtool" or "ip" care that it's a virtual function?)
What Don and I are suggesting is that the concept of virtual functions is a PCI thing, so it should be dealt with at the PCI layer. Regardless of the type of device the export of virtual functions is conceptually the same thing, so it should use the same API.
Once the device exists, then domain-specific APIs would be used to configure it the same way that they would configure a physical device.
Chris
Quoted 8 lines · Chris Friesen
Quoted 6 lines · Ben Hutchings
The ethtool API is typically used for net device operations that can be largely devolved to individual drivers, and which the network stack can mostly ignore (though offload features are an historical exception to this). It started with Ethernet link settings, but many operations are applicable (and implemented by) other types of network device.
Quoted 5 lines
That (potentially) accounts for all network devices, but it leaves all the other devices that could export virtual functions.
Why should I need to use a different API to enable virtual functions on my network device and my storage controller?
Indeed; I was merely making the point that it would be quite valid to use that means for setting VF network parameters for any network device that supports IOV.
Quoted 1 line
(And why should "ethtool" or "ip" care that it's a virtual function?)
VFs may be assigned to a guest which is not fully trusted by the hypervisor or privileged domain. (This can sometimes be true for PFs too, depending on the capabilities of the hypervisor and guest OS.) Some configuration may therefore need to be done via a trusted PF.
Quoted 7 lines
What Don and I are suggesting is that the concept of virtual functions is a PCI thing, so it should be dealt with at the PCI layer. Regardless of the type of device the export of virtual functions is conceptually the same thing, so it should use the same API.
Once the device exists, then domain-specific APIs would be used to configure it the same way that they would configure a physical device.
To an extent, but not entirely.
Currently, the assigned MAC address and (optional) VLAN tag for each networking VF are configured via the PF net device (though this is done though the rtnetlink API rather than ethtool).
Ben.
Signature · 3 lines
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.
Quoted 12 lines · Ben Hutchings
Quoted 11 lines · Chris Friesen
Quoted 6 lines · Ben Hutchings
The ethtool API is typically used for net device operations that can be largely devolved to individual drivers, and which the network stack can mostly ignore (though offload features are an historical exception to this). It started with Ethernet link settings, but many operations are applicable (and implemented by) other types of network device.
That (potentially) accounts for all network devices, but it leaves all the other devices that could export virtual functions.
Quoted 7 lines
Quoted 2 lines
Why should I need to use a different API to enable virtual functions on my network device and my storage controller?
Indeed; I was merely making the point that it would be quite valid to use that means for setting VF network parameters for any network device that supports IOV.
Yes, I read Ben's reply as supporting the proposition of VF enablement at the PCI level.
Quoted 7 lines
Quoted 1 line
(And why should "ethtool" or "ip" care that it's a virtual function?)
VFs may be assigned to a guest which is not fully trusted by the hypervisor or privileged domain. (This can sometimes be true for PFs too, depending on the capabilities of the hypervisor and guest OS.) Some configuration may therefore need to be done via a trusted PF.
Correct! The security domain (for KVM) is the host, thus, the host assignes VF attributes *before* they are given to the guest.... The guest is just a consumer, at least that's been my experience with VF devices to date, but I could see how an improper VF design could allow untrusted/guest (ethtool/netlink) ops on the VF.
Quoted 5 lines
Quoted 5 lines
What Don and I are suggesting is that the concept of virtual functions is a PCI thing, so it should be dealt with at the PCI layer. Regardless of the type of device the export of virtual functions is conceptually the same thing, so it should use the same API.
Quoted 8 lines
Quoted 2 lines
Once the device exists, then domain-specific APIs would be used to configure it the same way that they would configure a physical device.
To an extent, but not entirely.
Currently, the assigned MAC address and (optional) VLAN tag for each networking VF are configured via the PF net device (though this is done though the rtnetlink API rather than ethtool).
Yes, through the PF, which is suppose to remain in the trusted host/hypervisor domain. (Do a 'man ip' on RHEL6 and look at 'ip link set' where it then mentions the parameter 'vf'.).
Quoted 3 lines
Ben.
Quoted 10 lines · Ben Hutchings
Quoted 2 lines · Chris Friesen
Once the device exists, then domain-specific APIs would be used to configure it the same way that they would configure a physical device.
To an extent, but not entirely.
Currently, the assigned MAC address and (optional) VLAN tag for each networking VF are configured via the PF net device (though this is done though the rtnetlink API rather than ethtool).
I actually have a use-case where the guest needs to be able to modify the MAC addresses of network devices that are actually VFs.
The guest is bonding the network devices together, so the bonding driver in the guest expects to be able to set all the slaves to the same MAC address.
As I read the ixgbe driver, this should be possible as long as the host hasn't explicitly set the MAC address of the VF. Is that correct?
Chris
Quoted 23 lines
-----Original Message-----
From: Chris Friesen [mailto:[email protected]]
Sent: Friday, July 20, 2012 4:42 PM
To: Ben Hutchings
Cc: Don Dutile; David Miller; [email protected]; Rose, Gregory V; [email protected]; [email protected]
Subject: Re: New commands to configure IOV featuresQuoted 10 lines · Ben Hutchings
Quoted 2 lines · Chris Friesen
Once the device exists, then domain-specific APIs would be used to configure it the same way that they would configure a physical device.
To an extent, but not entirely.
Currently, the assigned MAC address and (optional) VLAN tag for each networking VF are configured via the PF net device (though this is done though the rtnetlink API rather than ethtool).
I actually have a use-case where the guest needs to be able to modify the MAC addresses of network devices that are actually VFs.
Quoted 6 lines
The guest is bonding the network devices together, so the bonding driver in the guest expects to be able to set all the slaves to the same MAC address.
As I read the ixgbe driver, this should be possible as long as the host hasn't explicitly set the MAC address of the VF. Is that correct?
That is correct.
- Greg
Intel Corp.
LAN Access Division
Quoted 12 lines · Chris Friesen
Quoted 10 lines · Ben Hutchings
Quoted 2 lines · Chris Friesen
Once the device exists, then domain-specific APIs would be used to configure it the same way that they would configure a physical device.
To an extent, but not entirely.
Currently, the assigned MAC address and (optional) VLAN tag for each networking VF are configured via the PF net device (though this is done though the rtnetlink API rather than ethtool).
Quoted 7 lines
I actually have a use-case where the guest needs to be able to modify the MAC addresses of network devices that are actually VFs.
The guest is bonding the network devices together, so the bonding driver in the guest expects to be able to set all the slaves to the same MAC address.
As I read the ixgbe driver, this should be possible as long as the host hasn't explicitly set the MAC address of the VF. Is that correct?
Chris
Interesting tug of war: hypervisors will want to set the macaddrs for security reasons,
some guests may want to set macaddr for (valid?) config reasons.
Quoted 6 lines · Don Dutile
Quoted 5 lines · Chris Friesen
I actually have a use-case where the guest needs to be able to modify the MAC addresses of network devices that are actually VFs.
The guest is bonding the network devices together, so the bonding driver in the guest expects to be able to set all the slaves to the same MAC address.
Quoted 8 lines
Quoted 3 lines
As I read the ixgbe driver, this should be possible as long as the host hasn't explicitly set the MAC address of the VF. Is that correct?
Chris
Interesting tug of war: hypervisors will want to set the macaddrs for security reasons,
some guests may want to set macaddr for
(valid?) config reasons.
In our case we have control over both guest an host anyways, so it's less of a security issue. In the general case though I could see it being an interesting problem.
Back to the original discussion though--has anyone got any ideas about the best way to trigger runtime creation of VFs? I don't know what the binary APIs looks like, but via sysfs I could see something like
echo number_of_new_vfs_to_create > /sys/bus/pci/devices/<address>/create_vfs
Something else that occurred to me--is there buy-in from driver maintainers? I know the Intel ethernet drivers (what I'm most familiar with) would need to be substantially modified to support on-the-fly addition of new vfs. Currently they assume that the number of vfs is known at module init time.
Chris
Quoted 33 lines
-----Original Message-----
From: Chris Friesen [mailto:[email protected]]
Sent: Monday, July 23, 2012 8:10 AM
To: Don Dutile
Cc: Ben Hutchings; David Miller; [email protected]; Rose, Gregory V; [email protected]; [email protected]
Subject: Re: New commands to configure IOV featuresQuoted 12 lines · Don Dutile
Quoted 11 lines · Chris Friesen
I actually have a use-case where the guest needs to be able to modify the MAC addresses of network devices that are actually VFs.
The guest is bonding the network devices together, so the bonding driver in the guest expects to be able to set all the slaves to the same MAC address.
As I read the ixgbe driver, this should be possible as long as the host hasn't explicitly set the MAC address of the VF. Is that correct?
Quoted 7 lines
Quoted 1 line
Chris
Interesting tug of war: hypervisors will want to set the macaddrs for security reasons,
some guests may want to set macaddr for
(valid?) config reasons.In our case we have control over both guest an host anyways, so it's less of a security issue. In the general case though I could see it being an interesting problem.
Quoted 6 lines
Back to the original discussion though--has anyone got any ideas about the best way to trigger runtime creation of VFs? I don't know what the binary APIs looks like, but via sysfs I could see something like
echo
number_of_new_vfs_to_create>
/sys/bus/pci/devices/<address>/create_vfs
The original proposals for creation and management of virtual functions were very much along these lines. However, at the time most of the distributions that used virtualization were based upon the 2.6.18 kernel. Red Hat Enterprise Linux 5.x and Citrix Xen Server were both using kernels derived from the 2.6.18 kernel. In order to implement a sysfs based management approach we would have had to break the 2.6.18 kernel ABI, which is of course a non-starter. We were able to implement an SR-IOV solution without breaking the ABI but it required use of a module parameter to inform the PF driver of how many VFs it should create.
This was fine during the first couple of years in which not many folks were using SR-IOV outside of a lab and the number of platforms that supported SR-IOV was very limited. As an experimental solution it has worked pretty well.
The last year and a half or so we have seen SR-IOV go from an experimental technology to one that is becoming increasingly deployed in real world applications and now the limitations of that original approach are more apparent.
Quoted 3 lines
Something else that occurred to me--is there buy-in from driver maintainers?
That's a good question. I've seen a lot of resistance to using sysfs based interfaces in drivers but usually that is when a driver wants to implement a private interface that other drivers wouldn't want or be able to use. I'm less sure about a sysfs solution that could be deployed by all SR-IOV capable devices of any type, be they Ethernet controllers, SCSI controllers, etc. I'm not sure what the objection would be in case of a general purpose solution, however I've been told that in general sysfs based solutions are often frowned upon by kernel maintainers. Perhaps that is because many of them are not generic solutions to a well defined problem.
I know the Intel ethernet drivers (what I'm most familiar
Quoted 2 lines
with) would need to be substantially modified to support on-the-fly addition of new vfs.
Actually, it wouldn't be that bad. But yes, there'd have to be some way for a driver to register a callback routine with the PCI interface so that it could be notified when changes have been made to the SR-IOV configuration of the device. This would require a new API, and some driver changes. In the case of the Intel drivers it wouldn't be too intrusive and it would definitely help us to meet some customer requirements. The current model using a module parameter forces all ports controlled by a PF driver to use the same number of VFs per function. This is clunky and there are a lot of users that would like the ability to assign differing numbers of VFs to respective physical functions.
I should also note that we need to be careful about what we mean by the phrase "support on-the-fly addition of new vfs".
You cannot just add VFs without first tearing down the VFs that are currently allocated. This is a limitation of the PCIE SR-IOV spec IIRC, and in any case it is true with Intel SR-IOV capable devices. The numVFs parameter in the SR-IOV capability structure is not writeable while the VF enable bit is set. To change that value you must first clear the VF enable bit and when you do that all your current VFs cease to exist. You can then write a different number of VFs and re-enable them but during a short interval the VFs that were already there are destroyed and completely reset when they come back on-line.
- Greg
Quoted 29 lines · On Mon, 23 Jul 2012 09:09:38 -0600 Chris Friesen <[email protected]>…
Quoted 12 lines · Don Dutile
Quoted 11 lines · Chris Friesen
I actually have a use-case where the guest needs to be able to modify the MAC addresses of network devices that are actually VFs.
The guest is bonding the network devices together, so the bonding driver in the guest expects to be able to set all the slaves to the same MAC address.
As I read the ixgbe driver, this should be possible as long as the host hasn't explicitly set the MAC address of the VF. Is that correct?
Quoted 7 lines
Quoted 1 line
Chris
Interesting tug of war: hypervisors will want to set the macaddrs for security reasons,
some guests may want to set macaddr for
(valid?) config reasons.In our case we have control over both guest an host anyways, so it's less of a security issue. In the general case though I could see it being an interesting problem.
Back to the original discussion though--has anyone got any ideas about the best way to trigger runtime creation of VFs? I don't know what the binary APIs looks like, but via sysfs I could see something like
Quoted 9 lines
echo
number_of_new_vfs_to_create>
/sys/bus/pci/devices/<address>/create_vfsSomething else that occurred to me--is there buy-in from driver maintainers? I know the Intel ethernet drivers (what I'm most familiar with) would need to be substantially modified to support on-the-fly addition of new vfs. Currently they assume that the number of vfs is known at module init time.
Why couldn't rtnl_link_ops be used for this. It is already the preferred interface to create vlan's, bond devices, and other virtual devices? The one issue is that do the created VF's exist in kernel as devices or only visible to guest?
Quoted 52 lines
-----Original Message-----
From: Stephen Hemminger [mailto:[email protected]]
Sent: Monday, July 23, 2012 11:36 AM
To: Chris Friesen
Cc: Don Dutile; Ben Hutchings; David Miller; [email protected]; Rose, Gregory V; [email protected]; [email protected]
Subject: Re: New commands to configure IOV featuresQuoted 6 lines · On Mon, 23 Jul 2012 09:09:38 -0600 Chris Friesen <[email protected]>…
Quoted 5 lines · Don Dutile
Quoted 4 lines · Chris Friesen
I actually have a use-case where the guest needs to be able to modify the MAC addresses of network devices that are actually VFs.
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
The guest is bonding the network devices together, so the bonding driver in the guest expects to be able to set all the slaves to the same MAC address.
As I read the ixgbe driver, this should be possible as long as the host hasn't explicitly set the MAC address of the VF. Is that
correct?
Quoted 20 lines
Quoted 8 lines
Quoted 2 lines
Chris
Interesting tug of war: hypervisors will want to set the macaddrs for security reasons,
some guests may want to set macaddr for
(valid?) config reasons.In our case we have control over both guest an host anyways, so it's less of a security issue. In the general case though I could see it being an interesting problem.
Back to the original discussion though--has anyone got any ideas about the best way to trigger runtime creation of VFs? I don't know what the binary APIs looks like, but via sysfs I could see something like
echo
number_of_new_vfs_to_create>
/sys/bus/pci/devices/<address>/create_vfsQuoted 7 lines
Something else that occurred to me--is there buy-in from driver maintainers? I know the Intel ethernet drivers (what I'm most familiar
with) would need to be substantially modified to support on-the-fly addition of new vfs. Currently they assume that the number of vfs is known at module init time.
Quoted 4 lines
Why couldn't
rtnl_link_opsbe used for this. It is already the preferred interface to create vlan's, bond devices, and other virtual devices? The one issue is that do the created VF's exist in kernel as devices or only visible to guest?
I would say that rtnl_link_ops are network oriented and not appropriate for something like a storage controller or graphics device, which are two other common SR-IOV capable devices.
I think it should be oriented toward the PCIe interface and subsystems in the kernel.
- Greg
Quoted 14 lines
Quoted 14 lines
Quoted 13 lines
Back to the original discussion though--has anyone got any ideas about the best way to trigger runtime creation of VFs? I don't know what the binary APIs looks like, but via sysfs I could see something like
echo
number_of_new_vfs_to_create>
/sys/bus/pci/devices/<address>/create_vfsSomething else that occurred to me--is there buy-in from driver maintainers? I know the Intel ethernet drivers (what I'm most familiar
with) would need to be substantially modified to support on-the-fly addition of new vfs. Currently they assume that the number of vfs is known at module init time.
Quoted 6 lines
Quoted 4 lines
Why couldn't
rtnl_link_opsbe used for this. It is already the preferred interface to create vlan's, bond devices, and other virtual devices? The one issue is that do the created VF's exist in kernel as devices or only visible to guest?I would say that
rtnl_link_opsare network oriented and not appropriate for something like a storage controller or graphics device, which are two other common SR-IOV capable devices.
Hi Dave,
We're currently fine-tuning our SRIOV support, which we will shortly send upstream.
We've encountered a problem though - all drivers currently supporting SRIOV do so with the usage of a module param: e.g., 'max_vfs' for ixgbe, 'num_vfs' for benet, etc.
The SRIOV feature is disabled by default on all the drivers; it can only be enabled via usage of the module param.
We don't want the lack of SRIOV module param in the bnx2x driver to be the bottle-neck when we'll submit the SRIOV feature upstream, and we also don't want to enable SRIOV by default (following the same logic of other drivers; most users don't use SRIOV and it would strain their resources).
As we see it, there are several possible ways of solving the issue:
1. Use some network-tool (e.g., ethtool).
2. Implement a standard sysfs interface for PCIe devices, as SRIOV is not solely network-related (this should be done via the PCI linux tree).
3. Implement a module param in our bnx2x code.
We would like to know what's your preferred method for solving this issue, and to hear if you have another (better?) method by which we can add this kind of support.
Thanks,
Yuval Mintz
Quoted 41 lines · On Wed, 19 Sep 2012 14:07:19 +0300 Yuval Mintz <[email protected]> wrote:
Quoted 15 lines
Quoted 15 lines
Quoted 14 lines
Back to the original discussion though--has anyone got any ideas about the best way to trigger runtime creation of VFs? I don't know what the binary APIs looks like, but via sysfs I could see something like
echo
number_of_new_vfs_to_create>
/sys/bus/pci/devices/<address>/create_vfsSomething else that occurred to me--is there buy-in from driver maintainers? I know the Intel ethernet drivers (what I'm most familiar
with) would need to be substantially modified to support on-the-fly addition of new vfs. Currently they assume that the number of vfs is known at module init time.Quoted 8 lines
Quoted 4 lines
Why couldn't
rtnl_link_opsbe used for this. It is already the preferred interface to create vlan's, bond devices, and other virtual devices? The one issue is that do the created VF's exist in kernel as devices or only visible to guest?I would say that
rtnl_link_opsare network oriented and not appropriate for something like a storage controller or graphics device, which are two other common SR-IOV capable devices.Hi Dave,
We're currently fine-tuning our SRIOV support, which we will shortly send upstream.
We've encountered a problem though - all drivers currently supporting SRIOV do so with the usage of a module param: e.g., '
max_vfs' for ixgbe, 'num_vfs' for benet, etc.
The SRIOV feature is disabled by default on all the drivers; it can only be enabled via usage of the module param.We don't want the lack of SRIOV module param in the bnx2x driver to be the bottle-neck when we'll submit the SRIOV feature upstream, and we also don't want to enable SRIOV by default (following the same logic of other drivers; most users don't use SRIOV and it would strain their resources).
Quoted 5 lines
As we see it, there are several possible ways of solving the issue:
1. Use some network-tool (e.g., ethtool).
2. Implement a standard sysfs interface for PCIe devices, as SRIOV is not solely network-related (this should be done via the PCI linux tree).
I was not able to attend the Linux conference held at the end of August myself but coworkers of mine here at Intel informed that method 2 here seems to be the preferred approach. Perhaps some folks who attended the the conference can chime in with more specifics.
- Greg
LAN Access Division
Intel Corp.
Quoted 14 lines
3. Implement a module param in our bnx2x code.
We would like to know what's your preferred method for solving this issue, and to hear if you have another (better?) method by which we can add this kind of support.
Thanks,
Yuval Mintz--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 2 lines · Greg Rose
On Wed, 19 Sep 2012 14:07:19 +0300
Yuval Mintz <[email protected]> wrote:
[...]
Quoted 12 lines
Quoted 12 lines
We've encountered a problem though - all drivers currently supporting SRIOV do so with the usage of a module param: e.g., '
max_vfs' for ixgbe, 'num_vfs' for benet, etc.
The SRIOV feature is disabled by default on all the drivers; it can only be enabled via usage of the module param.We don't want the lack of SRIOV module param in the bnx2x driver to be the bottle-neck when we'll submit the SRIOV feature upstream, and we also don't want to enable SRIOV by default (following the same logic of other drivers; most users don't use SRIOV and it would strain their resources).
Quoted 5 lines
Quoted 5 lines
As we see it, there are several possible ways of solving the issue:
1. Use some network-tool (e.g., ethtool).
2. Implement a standard sysfs interface for PCIe devices, as SRIOV is not solely network-related (this should be done via the PCI linux tree).
There is another interim option, which is to put this setting somewhere in NVRAM on the device. This solves the one-size-for-all-devices problem, though obviously not the inconsistent-interface problem.
Quoted 4 lines
I was not able to attend the Linux conference held at the end of August myself but coworkers of mine here at Intel informed that method 2 here seems to be the preferred approach. Perhaps some folks who attended the the conference can chime in with more specifics.
There really wasn't much more specific discussion. Bjorn's summary of the mini-summit <http://lwn.net/Articles/514113/> says:
Quoted 8 lines
SR-IOV Management
Currently drivers implement module parameters like "
max_vfs". This means all devices claimed by the driver get the same number of VFs, and you can't change anything without unloading and reloading the driver.Consensus that we should try to implement a knob for this in sysfs so it can be generic (not in each driver) and set individually for each device.
I don't think any implementation has been posted to the linux-pci list yet...?
Ben.
Signature · 3 lines
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.
Quoted 11 lines · Ben Hutchings
Quoted 4 lines · Greg Rose
I was not able to attend the Linux conference held at the end of August myself but coworkers of mine here at Intel informed that method 2 here seems to be the preferred approach. Perhaps some folks who attended the the conference can chime in with more specifics.
There really wasn't much more specific discussion. Bjorn's summary of the mini-summit <http://lwn.net/Articles/514113/> says:
Quoted 2 lines
SR-IOV Management
Quoted 8 lines
Quoted 6 lines
Currently drivers implement module parameters like "
max_vfs". This means all devices claimed by the driver get the same number of VFs, and you can't change anything without unloading and reloading the driver.Consensus that we should try to implement a knob for this in sysfs so it can be generic (not in each driver) and set individually for each device.
I don't think any implementation has been posted to the linux-pci list
please check attached three patches...
Thanks
Yinghai
Subject: [PATCH] PCI: Add pci_dev_type
later use it for visiable attribute control.
--- drivers/pci/pci-sysfs.c | 24 ++++++++++++++++++++++++ drivers/pci/pci.h | 1 + drivers/pci/probe.c | 1 + 3 files changed, 26 insertions(+)
drivers/pci/pci-sysfs.c +24 -0
@@ -1411,3 +1411,27 @@ static int __init pci_sysfs_init(void) }late_initcall (pci_sysfs_init); + +static struct attribute *pci_dev_dev_attrs[] = { +NULL , +}; + +static umode_t pci_dev_attrs_are_visible (struct kobject *kobj, +struct attribute *a,int n) +{ +return a->mode; +} + +static struct attribute_group pci_dev_attr_group = { + .attrs = pci_dev_dev_attrs, + .is_visible = pci_dev_attrs_are_visible, +}; + +static const struct attribute_group *pci_dev_attr_groups[] = { + &pci_dev_attr_group, +NULL , +}; + +struct device_type pci_dev_type = { + .groups = pci_dev_attr_groups, +};
drivers/pci/pci.h +1 -0
@@ -157,6 +157,7 @@ static inline int pci_no_d1d2(struct pci }extern struct device_attribute pci_dev_attrs[];extern struct device_attribute pcibus_dev_attrs[]; +extern struct device_type pci_dev_type;#ifdef CONFIG_HOTPLUG extern struct bus_attribute pci_bus_attrs[];#else
drivers/pci/probe.c +1 -0
@@ -975,6 +975,7 @@ int pci_setup_device(struct pci_dev *dev dev->sysdata = dev->bus->sysdata; dev->dev.parent = dev->bus->bridge; dev->dev.bus = &pci_bus_type; + dev->dev.type = &pci_dev_type; dev->hdr_type = hdr_type &0x7f ; dev->multifunction = !!(hdr_type &0x80 ); dev->error_state = pci_channel_io_normal;
Subject: [PATCH 10/10] PCI, sys: Use is_visable() with boot_vga attribute for pci_dev
That could let pci_create_sysfs_dev_files more simple.
also fix possible fix memleak during removing path.
--- drivers/pci/pci-sysfs.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)
drivers/pci/pci-sysfs.c +10 -11
@@ -1304,29 +1304,20 @@ int __must_check pci_create_sysfs_dev_fi pdev->rom_attr = attr; } -if ((pdev->class >>8 ) ==PCI_CLASS_DISPLAY_VGA ) { - retval =device_create_file (&pdev->dev, &vga_attr); -if (retval) - goto err_rom_file; - } -/* add platform-specific attributes */ retval =pcibios_add_platform_entries (pdev);if (retval) - goto err_vga_file; + goto err_rom_file;/* add sysfs entries for various capabilities */ retval =pci_create_capabilities_sysfs (pdev);if (retval) - goto err_vga_file; + goto err_rom_file;pci_create_firmware_label_files (pdev);return 0 ; -err_vga_file: -if ((pdev->class >>8 ) ==PCI_CLASS_DISPLAY_VGA ) -device_remove_file (&pdev->dev, &vga_attr); err_rom_file:if (rom_size) {sysfs_remove_bin_file (&pdev->dev.kobj, pdev->rom_attr);
@@ -1414,12 +1405,20 @@ static int __init pci_sysfs_init(void)late_initcall (pci_sysfs_init);static struct attribute *pci_dev_dev_attrs[] = { + &vga_attr.attr,NULL , };static umode_t pci_dev_attrs_are_visible (struct kobject *kobj,struct attribute *a,int n) { + struct device *dev = container_of(kobj, struct device, kobj); +struct pci_dev *pdev =to_pci_dev (dev); + +if (a == &vga_attr.attr) +if ((pdev->class >>8 ) !=PCI_CLASS_DISPLAY_VGA ) +return 0 ; +return a->mode; }
Subject: [PATCH] PCI: Add max_vfs in sysfs per pci device where supports
driver later need to check dev->max_vfs in /sys
--- drivers/pci/pci-sysfs.c | 29 +++++++++++++++++++++++++++++ include/linux/pci.h | 1 + 2 files changed, 30 insertions(+)
drivers/pci/pci-sysfs.c +29 -0
@@ -459,6 +459,30 @@ boot_vga_show(struct device *dev, struct }struct device_attribute vga_attr =__ATTR_RO (boot_vga); +static ssize_t +max_vfs_show (struct device *dev,struct device_attribute *attr,char *buf) +{ +struct pci_dev *pdev =to_pci_dev (dev); + +return sprintf (buf,"%u\n" , pdev->max_vfs); +} +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; + +return count; +} +struct device_attribute max_vfs_attr = +__ATTR (max_vfs,0644 , max_vfs_show, max_vfs_store); +static void pci_config_pm_runtime_get (struct pci_dev *pdev) {
@@ -1406,6 +1430,7 @@ late_initcall(pci_sysfs_init);static struct attribute *pci_dev_dev_attrs[] = { &vga_attr.attr, + &max_vfs_attr.attr,NULL , };
@@ -1419,6 +1444,10 @@ static umode_t pci_dev_attrs_are_visibleif ((pdev->class >>8 ) !=PCI_CLASS_DISPLAY_VGA )return 0 ; +if (a == &max_vfs_attr.attr) +if (!pdev->is_physfn) +return 0 ; +return a->mode; }
include/linux/pci.h +1 -0
@@ -359,6 +359,7 @@ struct pci_dev { unsigned int broken_intx_masking:1; unsigned int io_window_1k:1; /* Intel P2P bridge 1K I/O windows */pci_dev_flags_t dev_flags; +unsigned int max_vfs;atomic_t enable_cnt;/* pci_enable_device has been called */ u32 saved_config_space[16 ];/* config space saved at suspend time */
Quoted 23 lines · Yinghai Lu
Quoted 11 lines · Ben Hutchings
Quoted 4 lines · Greg Rose
I was not able to attend the Linux conference held at the end of August myself but coworkers of mine here at Intel informed that method 2 here seems to be the preferred approach. Perhaps some folks who attended the the conference can chime in with more specifics.
There really wasn't much more specific discussion. Bjorn's summary of the mini-summit <http://lwn.net/Articles/514113/> says:
Quoted 2 lines
SR-IOV Management
Quoted 8 lines
Quoted 6 lines
Currently drivers implement module parameters like "
max_vfs". This means all devices claimed by the driver get the same number of VFs, and you can't change anything without unloading and reloading the driver.Consensus that we should try to implement a knob for this in sysfs so it can be generic (not in each driver) and set individually for each device.
I don't think any implementation has been posted to the linux-pci list
please check attached three patches...
[...]
Quoted 17 lines
drivers/pci/pci-sysfs.c +11
-0@@ -459,6 +459,30 @@ boot_vga_show(struct device *dev, struct }struct device_attribute vga_attr =__ATTR_RO (boot_vga); +static ssize_t +max_vfs_show (struct device *dev,struct device_attribute *attr,char *buf) +{ +struct pci_dev *pdev =to_pci_dev (dev); + +return sprintf (buf,"%u\n" , pdev->max_vfs); +} +static ssize_t +max_vfs_store (struct device *dev,struct device_attribute *attr, +const char *buf,size_t count) +{
drivers/pci/pci-sysfs.c boot_vga_show(struct device *dev, struct · line 482
Quoted 10 lines
drivers/pci/pci-sysfs.c +10
-0+unsigned long val; +struct pci_dev *pdev =to_pci_dev (dev); + +if (strict_strtoul (buf,0 , &val) <0 ) +return -EINVAL ; + + pdev->max_vfs = val; + +return count; +}
[...]
Then what would actually trigger creation of the VFs? There's no way we can assume that some sysfs attribute will be written before the PF driver is loaded (what if it's built-in?). I thought the idea was to add a driver callback that would be called when the sysfs attribute was written.
Ben.
Signature · 3 lines
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.
Quoted 16 lines · Ben Hutchings
Quoted 13 lines · Yinghai Lu
(quoted diff) +13
-0+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; + + return count; +}[...]
Quoted 5 lines
Then what would actually trigger creation of the VFs? There's no way we can assume that some sysfs attribute will be written before the PF driver is loaded (what if it's built-in?). I thought the idea was to add a driver callback that would be called when the sysfs attribute was written.
could just stop the device and add it back again?
just like updated patch.
Thanks
Yinghai
Subject: [PATCH] PCI: Add max_vfs in sysfs per pci device where supports
driver later need to check dev->max_vfs in /sys
--- drivers/pci/pci-sysfs.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++ drivers/pci/remove.c | 2 - include/linux/pci.h | 2 + 3 files changed, 60 insertions(+), 1 deletion(-)
drivers/pci/pci-sysfs.c +57 -0
@@ -459,6 +459,58 @@ boot_vga_show(struct device *dev, struct }struct device_attribute vga_attr =__ATTR_RO (boot_vga); +static ssize_t +max_vfs_show (struct device *dev,struct device_attribute *attr,char *buf) +{ +struct pci_dev *pdev =to_pci_dev (dev); + +return sprintf (buf,"%u\n" , pdev->max_vfs); +} + +static void rebind_callback (struct device *dev) +{ +struct pci_dev *pdev =to_pci_dev (dev); +int err; + +mutex_lock (&pci_remove_rescan_mutex); +pci_stop_bus_device (pdev); + err =pci_bus_add_device (pdev); +if (!err) { +struct pci_bus *child = pdev->subordinate; + +if (child) { +pci_bus_add_devices (child); +if (!child->is_added) +pci_bus_add_child (child); + } + } +mutex_unlock (&pci_remove_rescan_mutex); +} +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) { +int err; + err =device_schedule_callback (dev, rebind_callback); + +if (err) +return err; + } + +return count; +} +struct device_attribute max_vfs_attr = +__ATTR (max_vfs,0644 , max_vfs_show, max_vfs_store); +static void pci_config_pm_runtime_get (struct pci_dev *pdev) {
@@ -1406,6 +1458,7 @@ late_initcall(pci_sysfs_init);static struct attribute *pci_dev_dev_attrs[] = { &vga_attr.attr, + &max_vfs_attr.attr,NULL , };
@@ -1419,6 +1472,10 @@ static umode_t pci_dev_attrs_are_visibleif ((pdev->class >>8 ) !=PCI_CLASS_DISPLAY_VGA )return 0 ; +if (a == &max_vfs_attr.attr) +if (!pdev->is_physfn) +return 0 ; +return a->mode; }
include/linux/pci.h +2 -0
@@ -359,6 +359,7 @@ struct pci_dev { unsigned int broken_intx_masking:1; unsigned int io_window_1k:1; /* Intel P2P bridge 1K I/O windows */pci_dev_flags_t dev_flags; +unsigned int max_vfs;atomic_t enable_cnt;/* pci_enable_device has been called */ u32 saved_config_space[16 ];/* config space saved at suspend time */
@@ -812,6 +813,7 @@ u8 pci_common_swizzle(struct pci_dev *deextern struct pci_dev *pci_dev_get (struct pci_dev *dev);extern void pci_dev_put (struct pci_dev *dev);extern void pci_remove_bus (struct pci_bus *b); +void pci_stop_bus_device (struct pci_dev *dev);extern void pci_stop_and_remove_bus_device (struct pci_dev *dev);void pci_stop_root_bus (struct pci_bus *bus);void pci_remove_root_bus (struct pci_bus *bus);
drivers/pci/remove.c +1 -1
@@ -55,7 +55,7 @@ void pci_remove_bus(struct pci_bus *bus) }EXPORT_SYMBOL (pci_remove_bus); -static void pci_stop_bus_device (struct pci_dev *dev) +void pci_stop_bus_device (struct pci_dev *dev) {struct pci_bus *bus = dev->subordinate;struct pci_dev *child, *tmp;
Quoted 18 lines · Yinghai Lu
Quoted 16 lines · Ben Hutchings
Quoted 13 lines · Yinghai Lu
drivers/pci/pci-sysfs.c +13
-0+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; + +return count; +}[...]
Quoted 7 lines
Quoted 5 lines
Then what would actually trigger creation of the VFs? There's no way we can assume that some sysfs attribute will be written before the PF driver is loaded (what if it's built-in?). I thought the idea was to add a driver callback that would be called when the sysfs attribute was written.
could just stop the device and add it back again?
This is highly disruptive and I think it would be totally unacceptable for at least networking devices.
Ben.
Quoted 5 lines
just like updated patch.
Thanks
Yinghai
Signature · 3 lines
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.
Quoted 5 lines · Ben Hutchings
Quoted 1 line · Yinghai Lu
could just stop the device and add it back again?
This is highly disruptive and I think it would be totally unacceptable for at least networking devices.
so you want PF network device continue work while enable VF on it?
Yinghai
Quoted 10 lines · Yinghai Lu
Quoted 5 lines · Ben Hutchings
Quoted 1 line · Yinghai Lu
could just stop the device and add it back again?
This is highly disruptive and I think it would be totally unacceptable for at least networking devices.
so you want PF network device continue work while enable VF on it?
Yes - normal scenario a system is installed thru automation over a network. The default initrd for installer does not configure VFs. Post install, a management tool like Openstack Quantum or others are used to enable VFs. If the PF NIC happens to be the communication path
- disrupting the PF during a remote configuration cycle can leave you blind and the system unreachable.
-subhendu
Quoted 32 lines
-----Original Message-----
From: Ben Hutchings [mailto:[email protected]]
Sent: Wednesday, September 19, 2012 6:23 PM
To: Yinghai Lu
Cc: Bjorn Helgaas; Rose, Gregory V; Yuval Mintz; [email protected]; [email protected]; Ariel Elior; Eilon Greenstein; linux-pci
Subject: Re: New commands to configure IOV featuresQuoted 23 lines · Yinghai Lu
Quoted 20 lines · Ben Hutchings
drivers/pci/pci-sysfs.c boot_vga_show(struct device *dev, struct · line 510
Quoted 12 lines · Yinghai Lu
drivers/pci/pci-sysfs.c +12
-0+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; + +return count; +}[...]
Then what would actually trigger creation of the VFs? There's no way we can assume that some sysfs attribute will be written before the PF driver is loaded (what if it's built-in?). I thought the idea was to add a driver callback that would be called when the sysfs attribute was written.
Quoted 4 lines
Quoted 1 line
could just stop the device and add it back again?
This is highly disruptive and I think it would be totally unacceptable for at least networking devices.
Agreed.
We need the driver callback.
- Greg
Quoted 11 lines · Gregory V
Quoted 7 lines
-----Original Message-----
From: Ben Hutchings [mailto:[email protected]]Quoted 2 lines
could just stop the device and add it back again?
This is highly disruptive and I think it would be totally unacceptable for at least networking devices.
Agreed.
We need the driver callback.
ok, something like attached patches?
ixgbe change need more cleanup from ixgbe guys.
-Yinghai
Subject: [PATCH] PCI: Add pci_dev_type
later use it for visiable attribute control.
--- drivers/pci/pci-sysfs.c | 24 ++++++++++++++++++++++++ drivers/pci/pci.h | 1 + drivers/pci/probe.c | 1 + 3 files changed, 26 insertions(+)
drivers/pci/pci-sysfs.c +24 -0
@@ -1412,3 +1412,27 @@ static int __init pci_sysfs_init(void) }late_initcall (pci_sysfs_init); + +static struct attribute *pci_dev_dev_attrs[] = { +NULL , +}; + +static umode_t pci_dev_attrs_are_visible (struct kobject *kobj, +struct attribute *a,int n) +{ +return a->mode; +} + +static struct attribute_group pci_dev_attr_group = { + .attrs = pci_dev_dev_attrs, + .is_visible = pci_dev_attrs_are_visible, +}; + +static const struct attribute_group *pci_dev_attr_groups[] = { + &pci_dev_attr_group, +NULL , +}; + +struct device_type pci_dev_type = { + .groups = pci_dev_attr_groups, +};
drivers/pci/pci.h +1 -0
@@ -158,6 +158,7 @@ static inline int pci_no_d1d2(struct pci }extern struct device_attribute pci_dev_attrs[];extern struct device_attribute pcibus_dev_attrs[]; +extern struct device_type pci_dev_type;#ifdef CONFIG_HOTPLUG extern struct bus_attribute pci_bus_attrs[];#else
drivers/pci/probe.c +1 -0
@@ -1277,6 +1277,7 @@ int pci_setup_device(struct pci_dev *dev dev->sysdata = dev->bus->sysdata; dev->dev.parent = dev->bus->bridge; dev->dev.bus = &pci_bus_type; + dev->dev.type = &pci_dev_type; dev->hdr_type = hdr_type &0x7f ; dev->multifunction = !!(hdr_type &0x80 ); dev->error_state = pci_channel_io_normal;
Subject: [PATCH 10/10] PCI, sys: Use is_visable() with boot_vga attribute for pci_dev
That could let pci_create_sysfs_dev_files more simple.
also fix possible fix memleak during removing path.
--- drivers/pci/pci-sysfs.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)
drivers/pci/pci-sysfs.c +10 -11
@@ -1304,29 +1304,20 @@ int __must_check pci_create_sysfs_dev_fi pdev->rom_attr = attr; } -if ((pdev->class >>8 ) ==PCI_CLASS_DISPLAY_VGA ) { - retval =device_create_file (&pdev->dev, &vga_attr); -if (retval) - goto err_rom_file; - } -/* add platform-specific attributes */ retval =pcibios_add_platform_entries (pdev);if (retval) - goto err_vga_file; + goto err_rom_file;/* add sysfs entries for various capabilities */ retval =pci_create_capabilities_sysfs (pdev);if (retval) - goto err_vga_file; + goto err_rom_file;pci_create_firmware_label_files (pdev);return 0 ; -err_vga_file: -if ((pdev->class >>8 ) ==PCI_CLASS_DISPLAY_VGA ) -device_remove_file (&pdev->dev, &vga_attr); err_rom_file:if (rom_size) {sysfs_remove_bin_file (&pdev->dev.kobj, pdev->rom_attr);
@@ -1414,12 +1405,20 @@ static int __init pci_sysfs_init(void)late_initcall (pci_sysfs_init);static struct attribute *pci_dev_dev_attrs[] = { + &vga_attr.attr,NULL , };static umode_t pci_dev_attrs_are_visible (struct kobject *kobj,struct attribute *a,int n) { + struct device *dev = container_of(kobj, struct device, kobj); +struct pci_dev *pdev =to_pci_dev (dev); + +if (a == &vga_attr.attr) +if ((pdev->class >>8 ) !=PCI_CLASS_DISPLAY_VGA ) +return 0 ; +return a->mode; }
--- include/linux/pci.h | 1 + 1 file changed, 1 insertion(+)
include/linux/pci.h +1 -0
@@ -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);
Subject: [PATCH] PCI: Add max_vfs in sysfs per pci device where supports
driver later need to check dev->max_vfs in /sys
--- drivers/pci/pci-sysfs.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/pci.h | 1 2 files changed, 54 insertions(+)
drivers/pci/pci-sysfs.c +53 -0
71 lines not on this page · raw message
include/linux/pci.h +1 -0
@@ -359,6 +359,7 @@ struct pci_dev { unsigned int broken_intx_masking:1; unsigned int io_window_1k:1; /* Intel P2P bridge 1K I/O windows */pci_dev_flags_t dev_flags; +unsigned int max_vfs;atomic_t enable_cnt;/* pci_enable_device has been called */ u32 saved_config_space[16 ];/* config space saved at suspend time */
--- drivers/net/ethernet/intel/ixgbe/ixgbe.h | 2 + drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 43 ++++++++++++++++++++------ 2 files changed, 36 insertions(+), 9 deletions(-)
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +34 -9
73 lines not on this page · raw message
drivers/net/ethernet/intel/ixgbe/ixgbe.h +2 -0
@@ -571,6 +571,8 @@ struct ixgbe_adapter {u32 interrupt_event;u32 led_reg; +struct ixgbe_info *ixgbe_info; +#ifdef CONFIG_IXGBE_PTP struct ptp_clock *ptp_clock;struct ptp_clock_info ptp_caps;
On Thu, 2012-09-20 at 22:50 -0700, Yinghai Lu wrote:
[...]
include/linux/pci.h struct pci_driver { · line 669
Quoted 12 lines
Index:
linux-2.6/include/linux/pci.h
===================================================================include/linux/pci.h +1
-0@@ -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.
Quoted 5 lines
Subject: [PATCH] PCI: Add
max_vfsin sysfs per pci device where supportsdriver later need to check
dev->max_vfsin /sysSigned-off-by Yinghai Lu <[email protected]>
[...]
include/linux/pci.h
Quoted 13 lines
include/linux/pci.h +13
-0+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?
include/linux/pci.h
Quoted 2 lines
include/linux/pci.h +2
-0+int err; + err =device_schedule_callback (dev, max_vfs_callback);
Any particular reason this should be a callback?
[...]
Quoted 20 lines
Index:
linux-2.6/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
===================================================================drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +0
-7@@ -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,
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c static int __devinit ixgbe_sw_init(struc · line 4524
Quoted 5 lines
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +1
-1@@ -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);
Quoted 33 lines
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +18
-1#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 ; +
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c static const struct pci_error_handlers i · line 7742
Quoted 10 lines
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +10
-0+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.
Quoted 4 lines
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +4
-0+ } +#endif +} +
[...]
Signature · 3 lines
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.
Quoted 27 lines · Ben Hutchings
On Thu, 2012-09-20 at 22:50 -0700, Yinghai Lu wrote:
[...]include/linux/pci.h struct pci_driver { · line 669
Quoted 12 lines
Index:
linux-2.6/include/linux/pci.h
===================================================================include/linux/pci.h +1
-0@@ -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.Quoted 5 lines
Subject: [PATCH] PCI: Add
max_vfsin sysfs per pci device where supportsdriver later need to check
dev->max_vfsin /sysSigned-off-by Yinghai Lu <[email protected]>[...]
Quoted 15 lines
include/linux/pci.h
Quoted 13 lines
include/linux/pci.h +13
-0+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?
should be removed.
Quoted 5 lines
include/linux/pci.h
Quoted 3 lines
include/linux/pci.h +2
-0+int err; + err =device_schedule_callback (dev, max_vfs_callback);Any particular reason this should be a callback?
no, just copied that from bus rescan.
Quoted 31 lines
[...]
Quoted 20 lines
Index:
linux-2.6/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
===================================================================drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +0
-7@@ -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,
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c static int __devinit ixgbe_sw_init(struc · line 4524
Quoted 5 lines
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +1
-1@@ -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.
Quoted 2 lines
So I think the correct assignment is:
adapter->num_vfs = min(pdev->max_vfs, 63);
this will treat >63 as 63. old one treat > 63 as 0 aka disabled.
looks your version is more reasonable...
Quoted 41 lines
Quoted 41 lines
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +25
-1#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;
Quoted 8 lines
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c static const struct pci_error_handlers i · line 7742
Quoted 3 lines
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +3
-0+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_vfsand maybe report an error), but I'm not sure about that.
should still call set_max_vfs, and let it set finally valid max_vfs.
pci driver should know better which max_vfs is better for exact ...
-Yinghai
Quoted 12 lines · Yinghai Lu
Quoted 10 lines · Gregory V
Quoted 7 lines
-----Original Message-----
From: Ben Hutchings [mailto:[email protected]]Quoted 2 lines
could just stop the device and add it back again?
This is highly disruptive and I think it would be totally unacceptable for at least networking devices.
Agreed.
Quoted 7 lines
Quoted 1 line
We need the driver callback.
ok, something like attached patches?
ixgbe change need more cleanup from ixgbe guys.
-Yinghai
yuk, no.
I have a set of patches almost done.
It provides 3 sysfs files (enable, disable, num_max_vfs); callbacks for enable & disable.
i'm tied up until Monday on RHEL6, then I'll switch gears & post a set of patches.
Quoted 7 lines · Don Dutile
Quoted 5 lines
ok, something like attached patches?
ixgbe change need more cleanup from ixgbe guys.
yuk, no.
I have a set of patches almost done.
good. includes update to those network drivers?
Quoted 2 lines
It provides 3 sysfs files (enable, disable,
num_max_vfs); callbacks for enable & disable.
why using three? only pass max_vfs should be enough...
aka pass 0 mean disabling, pass other value mean enabling.
Quoted 3 lines
i'm tied up until Monday on RHEL6, then I'll switch gears & post a set of patches.
so that is your employer 'sinternal policy? for RHEL 6 kernel first, then upstream kernel?
thought RH only accept backporting only patches get into upstream already.
-Yinghai
Quoted 9 lines · Yinghai Lu
Quoted 7 lines · Don Dutile
Quoted 5 lines
ok, something like attached patches?
ixgbe change need more cleanup from ixgbe guys.
yuk, no.
I have a set of patches almost done.
Quoted 9 lines
good. includes update to those network drivers?
Quoted 2 lines
It provides 3 sysfs files (enable, disable,
num_max_vfs); callbacks for enable& disable.why using three? only pass
max_vfsshould be enough...aka pass 0 mean disabling, pass other value mean enabling.
could do that. but I wouldn't use 'max_vfs'; I would recommend 'num_vfs', as max implies, er, um, the maximum, and what one wants is to be able to enable a number from 1->max_vfs. 'max_vfs' will be provided by another file.
Quoted 6 lines
Quoted 3 lines
i'm tied up until Monday on RHEL6, then I'll switch gears& post a set of patches.
so that is your employer 'sinternal policy? for RHEL 6 kernel first, then upstream kernel?
No, I have deadlines for RHEL6 for *other work* until Monday. After that, I can re-focus on upstream work. Some of us actually have other work than just upstream.... crazy talk, I know! ;-)
Quoted 2 lines
thought RH only accept backporting only patches get into upstream already.
yup. 'upstream first' is the rule at RH before it gets into a RHEL release... and sometimes that isn't sufficient!
Have a beer, enjoy the weekend, posting forthcoming next week!
Quoted 1 line
-Yinghai
Quoted 12 lines
Quoted 7 lines
Quoted 2 lines
It provides 3 sysfs files (enable, disable,
num_max_vfs); callbacks for enable& disable.why using three? only pass
max_vfsshould be enough...aka pass 0 mean disabling, pass other value mean enabling.
could do that. but I wouldn't use '
max_vfs'; I would recommend 'num_vfs', as max implies, er, um, the maximum, and what one wants is to be able to enable a number from 1->max_vfs. 'max_vfs' will be provided by another file.
If we're at it, how do you suggest the configurations of the VFs resources/properties should be made?
Notice I'm asking about properties which relate to all PCI devices, E.g., number of interrupts assigned to each VF (for VF RSS).
(For properties which relate only to networking devices, we could use standard network API such as netlink).
Perhaps given a configurable property that applies to all PCI devices, once SRIOV is enabled and the VFs are created, there would be an additional sysfs node under the VFs through which the user could configure that property.
Quoted 13 lines · Yuval Mintz
Quoted 12 lines
Quoted 7 lines
Quoted 2 lines
It provides 3 sysfs files (enable, disable,
num_max_vfs);
callbacks for enable& disable.why using three? only pass
max_vfsshould be enough...aka pass 0 mean disabling, pass other value mean enabling.
could do that. but I wouldn't use '
max_vfs'; I would recommend 'num_vfs', as max implies, er, um, the maximum, and what one wants is to be able to enable a number from 1->max_vfs. 'max_vfs' will be provided by another file.
Quoted 5 lines
If we're at it, how do you suggest the configurations of the VFs resources/properties should be made?
Notice I'm asking about properties which relate to all PCI devices, E.g., number of interrupts assigned to each VF (for VF RSS).
This issue/question is indep of VFs & SRIOV.
It's a generic problem for PFs and how many (msi) intr's a device is given, based on its request, i.e., system provides 8 MSI per cpu per device ... how are those 8 assigned/used by the device? ... how to free 4 of the 8 if one device doesn't use them, and another
device could use them.. cache/memory affinity to intr's.... the list
goes on...
Quoted 4 lines
(For properties which relate only to networking devices, we could use standard network API such as netlink).
Perhaps given a configurable property that applies to all PCI devices,
A VF is 'just another PCI device', so configurable properties for all PCI devices should encompass VF/SRIOV devices equally.
Quoted 9 lines
once SRIOV is enabled and the VFs are created, there would be an additional sysfs node under the VFs through which the user could configure that property.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
yuk, no.
I have a set of patches almost done.
i'm tied up until Monday on RHEL6, then I'll switch gears& post a set of patches.
hi Don - anything new about this incoming patch-series?
Quoted 6 lines
Quoted 3 lines
so that is your employer 'sinternal policy? for RHEL 6 kernel first, then upstream kernel?
No, I have deadlines for RHEL6 for *other work* until Monday. After that, I can re-focus on upstream work. Some of us actually have other work than just upstream.... crazy talk, I know! ;-)
Quoted 5 lines · Yuval Mintz
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
yuk, no.
I have a set of patches almost done.
i'm tied up until Monday on RHEL6, then I'll switch gears& post a set of
patches.
Quoted 10 lines
hi Don - anything new about this incoming patch-series?
Quoted 6 lines
Quoted 3 lines
so that is your employer 'sinternal policy? for RHEL 6 kernel first, then upstream kernel?
No, I have deadlines for RHEL6 for *other work* until Monday. After that, I can re-focus on upstream work. Some of us actually have other work than just upstream.... crazy talk, I know! ;-)
I should have the sysfs-level patches done & posted today. Need another day or 2 to get a driver refactored to use it (ixgbe).
Although the patches will enable per-device sriov enablement/disablement, further thought on tools that will stack on top of it, probably will need to augment a new pci sysfs directory that tools can find sriov devices quicker/easier w/o traversing the entire /sys/bus/pci/devices tree searching for a 'sriov_max_vfs' file, but, I can do that as a follow-on patch. I'm thinking of what is done now with virtfn<X> linked files in the pf, but the inverse link in a directory, like /sys/bus/pci/sriov.
Then there's the additional patches needed to do an 'all', or <DOMAIN:B:D.F> qualified enable/disable from a per-sysfs-driver perspective. ... fun stuff!
David Miller
maintainerFrom: "Yuval Mintz" <[email protected]>
Date: Wed, 19 Sep 2012 14:07:19 +0300
Quoted 1 line
3. Implement a module param in our bnx2x code.
Just because I was a moron and didn't notice the other cases doesn't mean I should let you guys do it too.
So I just want to say that #3 is absolutely not an option.
Quoted 29 lines
-----Original Message-----
From: Don Dutile [mailto:[email protected]]
Sent: Monday, July 23, 2012 7:04 AM
To: Chris Friesen
Cc: Ben Hutchings; David Miller; [email protected]; Rose, Gregory V; [email protected]; [email protected]
Subject: Re: New commands to configure IOV featuresQuoted 6 lines · Chris Friesen
Quoted 5 lines · Ben Hutchings
Quoted 2 lines · Chris Friesen
Once the device exists, then domain-specific APIs would be used to configure it the same way that they would configure a physical device.
Quoted 7 lines
Quoted 5 lines
To an extent, but not entirely.
Currently, the assigned MAC address and (optional) VLAN tag for each networking VF are configured via the PF net device (though this is done though the rtnetlink API rather than ethtool).
I actually have a use-case where the guest needs to be able to modify
the MAC addresses of network devices that are actually VFs.
Quoted 2 lines
The guest is bonding the network devices together, so the bonding driver
in the guest expects to be able to set all the slaves to the same MAC address.
Quoted 2 lines
As I read the ixgbe driver, this should be possible as long as the host
Quoted 8 lines
hasn't explicitly set the MAC address of the VF. Is that correct?
Quoted 2 lines
Chris
Interesting tug of war: hypervisors will want to set the macaddrs for security reasons,
some guests may want to set macaddr for (valid?)
config reasons.
It is a matter of trust. The ability to set your own MAC address filters is a potential security issue, so host administrators have the ability to determine whether they trust the VF (and implicitly, the domain in which the VF resides). There is also a sort of half-way solution. By turning off anti-spoofing you can allow the VF to use source MAC addresses that are not actually assigned in HW filters. This was done to support some bonding scenarios where the VF will need to transmit with a different source address.
Many applications using SR-IOV are embedded devices such as switches, edge relay devices, IP forwarding/filtering appliances, routers, etc. More often than not the host administrator can trust domains that the VFs are assigned to because those domains are completely under the control of the local host. In those cases the VFs are trusted and can be allowed to set their own MAC filters and use any source MAC address they please.
Other applications might need to assign VF devices to non-trusted domains. Perhaps a service provider has leased a virtual machine domain to a subscriber who has purchased QoS levels that can only be met with the performance levels available with SR-IOV VF devices. Other scenarios exist. In these cases it is worthwhile to be able to restrict the VF's ability to set MAC filters and use source MAD addresses not assigned to it.
Rather than a tug of war I just view it as balancing security concerns with levels of additional capability and functionality. That goes on all the time.
- Greg