[libvirt] [PATCH 0/5] Interface pools and passthrough mode

Laine Stump laine at laine.org
Fri Dec 2 19:31:56 UTC 2011


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).





More information about the libvir-list mailing list