On 12/02/2011 11:30 AM, Shradha Shah wrote:
On 11/30/2011 01:29 AM, Laine Stump wrote:
On 11/29/2011 02:53 PM, Daniel P. Berrange wrote:
On Tue, Nov 29, 2011 at 03:46:13PM +0000, Shradha Shah wrote:
Interface Pools and Passthrough mode:
Current Method:
The passthrough mode uses a macvtap a direct connection to connect each guest to the network. The physical interface to be used is picked from among those listed in<interface> sub elements of the<forward> element.
The current specification for<forward> extends to allow 0 or more<interface> sub-elements:
Example:
<forward mode='passthrough' dev='eth10'/>
<interface dev='eth10'/>
<interface dev='eth12'/>
<interface dev='eth18'/>
<interface dev='eth20'/>
</forward>
However with an ethernet card with 64 VF's or more, the above method gets tedious on the system.
On the other hand, just parameterizing a string (eth%d) is inadequate, eg, when there are multiple non-contiguous ranges.
Heh. You've gone through some of the same thought process I went through when I wrote the original code :-)
Proposed Method:
The 5 patches provided along with this introductory e-mail
i) Introduce a structure to store the state of all the virtual functions attached to each physical function
ii) Find a free virtual function given the physical function.
The forward specification will hence only mention the physical function as the interface sub element:
Example:
<forward mode='passthrough' dev='eth2'/>
<interface dev='eth2'/>
</forward>
I can see what you mean about it being tedious to construct the config with
all 64 (or more) VF's listed, but this proposed change has a couple of issues
First of all, the change you describe would be a semantic change in the
network XML, which would break compatibility with previous releases of
libvirt. Since we consider XML to be poart of our long term ABI stability
guarantee I don't think we can do the change.
Ignoring the ABI issue, I'm concerned that as we get PFs with an increasingly
large number of VFs, we may well *not* want to associate all VFs with a single
virtual network definition. eg, we might wna to put 32 VFs in one network and
32 VFs in another network. Or if we have 2 PFs, we might want to interleave
VFs from several PFs across virtual networks. If all we can do is list the
PF in the XML, we loose significant flexibility in how VFs are assigned.
My first concern too when I saw the patch was the semantic change (but also the loss of flexibility), which is obviously a no-go. It's a convenient capability to have though, so it would be nice to get it in somehow. What if we allowed including all the VFs associated with a PF by adding an extra attribute? e.g.:
<interface dev='eth10' type='sriov'/>
(or whatever is more appropriate in place of "sriov"). Or possibly a different element type could be used:
<pf dev='eth10'/>
(didn't want to spend time thinking of a better name than "pf"...).
At the time the network is created, this would cause libvirt to get the list of all VFs for the given PF and put them into the pool. This could be used instead of, or in combination with, the existing<interface dev='eth1'/> form. Thus the existing semantics would be preserved, the flexibility of specifying individual devices would be retained, and the desired convenience of adding all VFs of a PF with a single line would be added.
I completely agree with this method, I can work on this cause next week. May I ask which method you would suggest I go forward with,
1)<interface dev='eth10' type='sriov'/>
2) Or possibly a different element type could be used:<pf dev='eth10'/>
I don't have a preference. I was hoping that Dan (or somebody else)
would :-) (Also, don't infer from my mail that this suggestion is the
way to go - my own ideas have been shot down on libvir-list before as well.)
Laine and Daniel,
Many thanks for reviewing my patch series.
I do understand the point of loss of flexibility when we want to interleave Vfs from several PFs across virtual networks.
Regarding the changes to the network semantics, I am a little bit confused,
1) We do not require a change to the XML schema in order to support the patch series
Additions to the schema are fine.
2) Is the change required to support the modifications to the structures 'virNetworkForwardVfDef' and 'virNetworkForwardIfDef'? OR
3) Would the change be required because of the compatibility issue with interface pools?
The problem we're concerned about is that the meaning of:
<forward mode='passthrough'>
<interface dev='eth0'/>
</forward>
changes. With existing code, that means "a pool with a single ethernet
interface named eth0"; with your patches, it means "a pool with several
ethernet interfaces ethX-ethY (which are the VFs associated with the PF
named eth0). This isn't acceptable because libvirt guarantees 100% API
stability across releases.
It would be acceptable, though (in my opinion) to have the new behavior
occur based on the presence of an extra attribute, or a different element.
If the concern is the compatibility issue, the patch series I have submitted takes this into consideration and will still work as before if interface pools are mentioned instead of just the PF.
Not having an SRIOV-capable NIC to toy with (yet! I have one on order),
I was unable to directly try out your code, and didn't go through it
line by line, but based on your description it seems that an interface
definition that is currently valid (my example above) would have a
different interpretation after your patches are applied. Is that not the
case?
I completely agree with Laine's suggestion in the previous thread, I can work on this cause next week. May I ask for suggestions as to which method I should go forward with,
1)<interface dev='eth10' type='sriov'/>
2) Or possibly a different element type could be used:<pf dev='eth10'/>
You may want to wait until Monday to see if Dan weighs in on this one
way or the other (or with a better idea) (he's traveling at the moment).
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list