On 05/10/2011 05:59 AM, D. Herrendoerfer wrote: > Hi all, > > starting with kernel 2.6.38 macvtap supports a 'passthru' mode for > attaching virtual functions > of a SRIOV capable network card directly to a VM. > This patch adds the capability to configure such a device. > > Signed-off-by: Dirk Herrendoerfer <d.herrendoerfer at herrendoerfer.name> > > diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng > index 7163c6e..e2a66ae 100644 > --- a/docs/schemas/domain.rng > +++ b/docs/schemas/domain.rng > @@ -2351,7 +2351,7 @@ > </define> > <define name="bridgeMode"> > <data type="string"> > - <param name="pattern">(vepa|bridge|private)</param> > + <param name="pattern">(vepa|bridge|private|passthru)</param> > </data> > </define> > <define name="addrMAC"> Missing corresponding documentation in docs/formatdomain.html.in to describe the new mode. What happens if you try to use this mode but you have an older kernel? Do we need to add any sanity checking to give a saner error message than just blindly trying the string and reporting back whatever message gets returned about an unknown argument? The rest of this patch looks okay to me (with my disclaimer that I've never used an SRIOV card), but I'd rather wait for a v2 than push this without documentation.

On 05/11/2011 08:21 AM, Daniel P. Berrange wrote: >On Tue, May 10, 2011 at 04:21:29PM -0600, Eric Blake wrote: >>On 05/10/2011 05:59 AM, D. Herrendoerfer wrote: >>>Hi all, >>> >>>starting with kernel 2.6.38 macvtap supports a 'passthru' mode for >>>attaching virtual functions >>>of a SRIOV capable network card directly to a VM. >>>This patch adds the capability to configure such a device. >>> >>>Signed-off-by: Dirk Herrendoerfer<d.herrendoerfer at herrendoerfer.name> >>> >>>diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng >>>index 7163c6e..e2a66ae 100644 >>>--- a/docs/schemas/domain.rng >>>+++ b/docs/schemas/domain.rng >>>@@ -2351,7 +2351,7 @@ >>> </define> >>> <define name="bridgeMode"> >>> <data type="string"> >>>-<param name="pattern">(vepa|bridge|private)</param> >>>+<param name="pattern">(vepa|bridge|private|passthru)</param> >>> </data> >>> </define> >>> <define name="addrMAC"> >>Missing corresponding documentation in docs/formatdomain.html.in to >>describe the new mode. >> >>What happens if you try to use this mode but you have an older kernel? >>Do we need to add any sanity checking to give a saner error message than >>just blindly trying the string and reporting back whatever message gets >>returned about an unknown argument? >> >>The rest of this patch looks okay to me (with my disclaimer that I've >>never used an SRIOV card), but I'd rather wait for a v2 than push this >>without documentation. >The kernel GIT commit actually has some useful info > > >commit eb06acdc85585f28864261f28659157848762ee4 >Author: Sridhar Samudrala&lt;sri(a)us.ibm.com&gt; >Date: Thu Oct 28 13:10:50 2010 +0000 > > macvlan: Introduce 'passthru' mode to takeover the underlying device > > With the current default 'vepa' mode, a KVM guest using virtio with > macvtap backend has the following limitations. > - cannot change/add a mac address on the guest virtio-net > - cannot create a vlan device on the guest virtio-net > - cannot enable promiscuous mode on guest virtio-net I had read that commit notice earlier, and didn't understand the first item - which MAC address needs to be changed? Doesn't the guest have its own MAC address, unrelated to the MAC address of the underlying device on the host?

This patch includes the documentation part. The error returned by libvirt when a using an older kernel is imho sufficient to point to the macvtap setup. Apart from that I doubt that someone will use this feature by chance. But still - I believe the error returned is detailed enough for now.

Signed-off-by: Dirk Herrendoerfer <d.herrendoerfer at herrendoerfer.name> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 989dcf6..3f6bec8 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1429,6 +1429,12 @@ external router or gateway and that device sends them back to the host. This procedure is followed if either the source or destination device is in <code>private</code> mode.</dd> + <dt><code>passthru</code></dt> + <dd>This feature allows to attach a virtual function of a SRIOV capable

+ NIC directly to a VM without loosing the migration capability.

+ All packets are sent to the VF/IF of the configured network device. + Depending on the capabilities of the device additional prerequisites or + limitations may apply. <span class="since">(Since Linux 2.6.38.)</span></dd>

</dl> <pre> diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 7163c6e..e2a66ae 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -2351,7 +2351,7 @@ </define> <define name="bridgeMode"> <data type="string"> - <param name="pattern">(vepa|bridge|private)</param> + <param name="pattern">(vepa|bridge|private|passthru)</param>

</data> </define> <define name="addrMAC"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d3efec6..4c4bfad 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -383,7 +383,8 @@ VIR_ENUM_IMPL(virDomainSeclabel, VIR_DOMAIN_SECLABEL_LAST, VIR_ENUM_IMPL(virDomainNetdevMacvtap, VIR_DOMAIN_NETDEV_MACVTAP_MODE_LAST, "vepa", "private", - "bridge") + "bridge", + "passthru" )

VIR_ENUM_IMPL(virVirtualPort, VIR_VIRTUALPORT_TYPE_LAST, "none", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a0f820c..70943f9 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -335,6 +335,7 @@ enum virDomainNetdevMacvtapType { VIR_DOMAIN_NETDEV_MACVTAP_MODE_VEPA, VIR_DOMAIN_NETDEV_MACVTAP_MODE_PRIVATE, VIR_DOMAIN_NETDEV_MACVTAP_MODE_BRIDGE, + VIR_DOMAIN_NETDEV_MACVTAP_MODE_PASSTHRU,

VIR_DOMAIN_NETDEV_MACVTAP_MODE_LAST, }; diff --git a/src/util/macvtap.c b/src/util/macvtap.c index a7af0cb..1b274cc 100644 --- a/src/util/macvtap.c +++ b/src/util/macvtap.c @@ -473,6 +473,9 @@ macvtapModeFromInt(enum virDomainNetdevMacvtapType mode) case VIR_DOMAIN_NETDEV_MACVTAP_MODE_BRIDGE: return MACVLAN_MODE_BRIDGE; + case VIR_DOMAIN_NETDEV_MACVTAP_MODE_PASSTHRU: + return MACVLAN_MODE_PASSTHRU;

On Tue, May 17, 2011 at 08:19:17PM -0600, Eric Blake wrote: > On 05/17/2011 05:26 AM, D. Herrendoerfer wrote: >> This patch includes the documentation part. >> >> The error returned by libvirt when a using an older kernel >> is imho sufficient to point to the macvtap setup. Apart from that >> I doubt that someone will use this feature by chance. But still - I >> believe the error returned is detailed enough for now. > > Fair enough for me. > > Your patch came through rather mangled by your mailer; sometimes, > it is > worth sending a patch to yourself as a trial run to see if others > would > have a hard time running 'git am' on the resulting email, then figure > out how to tune 'git send-email' to do what you wanted for sending > the > patch. The git mailing list or IRC channel is a good resource for > figuring out configuration emails that make patch submission easier. > >> >> Signed-off-by: Dirk Herrendoerfer <d.herrendoerfer at >> herrendoerfer.name> >> >> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in >> index 989dcf6..3f6bec8 100644 >> --- a/docs/formatdomain.html.in >> +++ b/docs/formatdomain.html.in >> @@ -1429,6 +1429,12 @@ >> external router or gateway and that device sends them back to >> the >> host. This procedure is followed if either the source or >> destination >> device is in <code>private</code> mode.</dd> >> + <dt><code>passthru</code></dt> >> + <dd>This feature allows to attach a virtual function of a >> SRIOV >> capable > > s/allows to attach/attaches/ > >> + NIC directly to a VM without loosing the migration >> capability. > > s/loosing/losing/ > >> + All packets are sent to the VF/IF of the configured network >> device. >> + Depending on the capabilities of the device additional >> prerequisites or >> + limitations may apply. <span class="since">(Since Linux >> 2.6.38.)</span></dd> > > Here's where part of the patch botching showed up. Also, when > possible, > we like to keep lines within 80 columns. > > The <span class="since"> annotations are for when a feature was > added to > libvirt (0.9.2 in this case), not when it was added elsewhere. > >> </dl> >> >> <pre> >> diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng >> index 7163c6e..e2a66ae 100644 >> --- a/docs/schemas/domain.rng >> +++ b/docs/schemas/domain.rng >> @@ -2351,7 +2351,7 @@ >> </define> >> <define name="bridgeMode"> >> <data type="string"> >> - <param name="pattern">(vepa|bridge|private)</param> >> + <param name="pattern">(vepa|bridge|private|passthru)</param> > > We already have other cases of "passthrough" in our XML schema; > let's be > consistent and avoid abbreviations in the public interface. > >> </data> >> </define> >> <define name="addrMAC"> >> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >> index d3efec6..4c4bfad 100644 >> --- a/src/conf/domain_conf.c >> +++ b/src/conf/domain_conf.c >> @@ -383,7 +383,8 @@ VIR_ENUM_IMPL(virDomainSeclabel, >> VIR_DOMAIN_SECLABEL_LAST, >> VIR_ENUM_IMPL(virDomainNetdevMacvtap, >> VIR_DOMAIN_NETDEV_MACVTAP_MODE_LAST, >> "vepa", >> "private", >> - "bridge") >> + "bridge", >> + "passthru" ) > > Odd spacing, and same spelling issue. > >> >> VIR_ENUM_IMPL(virVirtualPort, VIR_VIRTUALPORT_TYPE_LAST, >> "none", >> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h >> index a0f820c..70943f9 100644 >> --- a/src/conf/domain_conf.h >> +++ b/src/conf/domain_conf.h >> @@ -335,6 +335,7 @@ enum virDomainNetdevMacvtapType { >> VIR_DOMAIN_NETDEV_MACVTAP_MODE_VEPA, >> VIR_DOMAIN_NETDEV_MACVTAP_MODE_PRIVATE, >> VIR_DOMAIN_NETDEV_MACVTAP_MODE_BRIDGE, >> + VIR_DOMAIN_NETDEV_MACVTAP_MODE_PASSTHRU, > > But here, the abbreviation is fine (it's internal to the code). > >> >> VIR_DOMAIN_NETDEV_MACVTAP_MODE_LAST, >> }; >> diff --git a/src/util/macvtap.c b/src/util/macvtap.c >> index a7af0cb..1b274cc 100644 >> --- a/src/util/macvtap.c >> +++ b/src/util/macvtap.c >> @@ -473,6 +473,9 @@ macvtapModeFromInt(enum >> virDomainNetdevMacvtapType >> mode) >> case VIR_DOMAIN_NETDEV_MACVTAP_MODE_BRIDGE: >> return MACVLAN_MODE_BRIDGE; >> >> + case VIR_DOMAIN_NETDEV_MACVTAP_MODE_PASSTHRU: >> + return MACVLAN_MODE_PASSTHRU; > > This fails to compile for me on kernel-2.6.35.13-91.fc14.x86_64, > but I > see on rawhide that this is 8, so we need a #define fallback value. > Shoot, in rawhide, it's an enum rather than a #define, so we need a > configure check rather than a simpler preprocessor check. > > This is what I'll probably push. > > > From f977a09221757826ee91dd966e8d726b0bc22501 Mon Sep 17 00:00:00 > 2001 > From: Dirk Herrendorefer <d.herrendoerfer(a)herrendoerfer.name&gt; > Date: Tue, 17 May 2011 13:26:09 +0200 > Subject: [PATCH] Add support for 'passthru' mode for direct network > interfaces > > starting with kernel 2.6.38 macvtap supports a 'passthru' mode for > attaching virtual functions of a SRIOV capable network card > directly to > a VM. > This patch adds the capability to configure such a device. > > Signed-off-by: Dirk Herrendoerfer > <d.herrendoerfer(a)herrendoerfer.name&gt; > --- > AUTHORS | 1 + > configure.ac | 7 +++++++ > docs/formatdomain.html.in | 7 +++++++ > docs/schemas/domain.rng | 2 +- > src/conf/domain_conf.c | 3 ++- > src/conf/domain_conf.h | 1 + > src/util/macvtap.c | 12 ++++++++++-- > src/util/macvtap.h | 1 + > 8 files changed, 30 insertions(+), 4 deletions(-) > > diff --git a/AUTHORS b/AUTHORS > index 1bb1f0f..a1e93db 100644 > --- a/AUTHORS > +++ b/AUTHORS > @@ -170,6 +170,7 @@ Patches have also been contributed by: > index 233e4af..8db3226 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -2240,6 +2240,13 @@ if test "$with_macvtap" != "no" ; then > fi > AM_CONDITIONAL([WITH_MACVTAP], [test "$with_macvtap" = "yes"]) > AC_MSG_RESULT([$with_macvtap]) > +if test "$with_macvtap" = yes; then > + AC_CHECK_DECLS([MACVLAN_MODE_PASSTHRU], [], [], [[ > + #include <sys/socket.h> > + #include <linux/if_link.h> > + ]]) > +fi > + > > AC_ARG_WITH([virtualport], > AC_HELP_STRING([--with-virtualport],[enable virtual port support > @<:@default=check@:>@]), > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in > index 989dcf6..70a120b 100644 > --- a/docs/formatdomain.html.in > +++ b/docs/formatdomain.html.in > @@ -1429,6 +1429,13 @@ > external router or gateway and that device sends them back to > the > host. This procedure is followed if either the source or > destination > device is in <code>private</code> mode.</dd> > + <dt><code>passthru</code></dt> > + <dd>This feature attaches a virtual function of a SRIOV > capable > + NIC directly to a VM without losing the migration capability. > + All packets are sent to the VF/IF of the configured network > device. > + Depending on the capabilities of the device additional > prerequisites or > + limitations may apply; for example, on Linux this requires > + kernel 2.6.38 or newer. <span class="since">Since 0.9.2</ > span></dd> > </dl> > > <pre> > diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng > index 7163c6e..b252547 100644 > --- a/docs/schemas/domain.rng > +++ b/docs/schemas/domain.rng > @@ -2351,7 +2351,7 @@ > </define> > <define name="bridgeMode"> > <data type="string"> > - <param name="pattern">(vepa|bridge|private)</param> > + <param name="pattern">(vepa|bridge|private|passthrough)</ > param> > </data> > </define> > <define name="addrMAC"> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 498438a..3298c80 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -433,7 +433,8 @@ VIR_ENUM_IMPL(virDomainSeclabel, > VIR_DOMAIN_SECLABEL_LAST, > VIR_ENUM_IMPL(virDomainNetdevMacvtap, > VIR_DOMAIN_NETDEV_MACVTAP_MODE_LAST, > "vepa", > "private", > - "bridge") > + "bridge", > + "passthrough") > > VIR_ENUM_IMPL(virVirtualPort, VIR_VIRTUALPORT_TYPE_LAST, > "none", > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index fe42f21..5fe31d4 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -335,6 +335,7 @@ enum virDomainNetdevMacvtapType { > VIR_DOMAIN_NETDEV_MACVTAP_MODE_VEPA, > VIR_DOMAIN_NETDEV_MACVTAP_MODE_PRIVATE, > VIR_DOMAIN_NETDEV_MACVTAP_MODE_BRIDGE, > + VIR_DOMAIN_NETDEV_MACVTAP_MODE_PASSTHRU, > > VIR_DOMAIN_NETDEV_MACVTAP_MODE_LAST, > }; > diff --git a/src/util/macvtap.c b/src/util/macvtap.c > index a7af0cb..068638e 100644 > --- a/src/util/macvtap.c > +++ b/src/util/macvtap.c > @@ -1,5 +1,5 @@ > /* > - * Copyright (C) 2010 Red Hat, Inc. > + * Copyright (C) 2010-2011 Red Hat, Inc. > * Copyright (C) 2010 IBM Corporation > * > * This library is free software; you can redistribute it and/or > @@ -45,6 +45,11 @@ > > # include <netlink/msg.h> > > +/* Older kernels lacked this enum value. */ > +# if !HAVE_DECL_MACVLAN_MODE_PASSTHRU > +# define MACVLAN_MODE_PASSTHRU 8 > +# endif > + > #endif /* WITH_MACVTAP || WITH_VIRTUALPORT */ > > #include "util.h" > @@ -473,6 +478,9 @@ macvtapModeFromInt(enum > virDomainNetdevMacvtapType mode) > case VIR_DOMAIN_NETDEV_MACVTAP_MODE_BRIDGE: > return MACVLAN_MODE_BRIDGE; > > + case VIR_DOMAIN_NETDEV_MACVTAP_MODE_PASSTHRU: > + return MACVLAN_MODE_PASSTHRU; > + > case VIR_DOMAIN_NETDEV_MACVTAP_MODE_VEPA: > default: > return MACVLAN_MODE_VEPA; > @@ -546,7 +554,7 @@ configMacvtapTap(int tapfd, int vnet_hdr) > * be NULL if this function is supposed to choose a name > * @macaddress: The MAC address for the macvtap device > * @linkdev: The interface name of the NIC to connect to the external > bridge > - * @mode: int describing the mode for 'bridge', 'vepa' or 'private'. > + * @mode: int describing the mode for 'bridge', 'vepa', 'private' or > 'passthru'. > * @vnet_hdr: 1 to enable IFF_VNET_HDR, 0 to disable it > * @vmuuid: The UUID of the VM the macvtap belongs to > * @virtPortProfile: pointer to object holding the virtual port > profile > data > diff --git a/src/util/macvtap.h b/src/util/macvtap.h > index 54205c7..c45dd13 100644 > --- a/src/util/macvtap.h > +++ b/src/util/macvtap.h > @@ -92,6 +92,7 @@ void delMacvtap(const char *ifname, > # define MACVTAP_MODE_PRIVATE_STR "private" > # define MACVTAP_MODE_VEPA_STR "vepa" > # define MACVTAP_MODE_BRIDGE_STR "bridge" > +# define MACVTAP_MODE_PASSTHRU_STR "passthru" > > int vpAssociatePortProfileId(const char *macvtap_ifname, > const unsigned char *macvtap_macaddr, That patch looks fine to me, ACK, The only unresolved uncertainty to me is what happen if you try to use it on a kernel without the support ?

Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On 05/18/2011 05:32 AM, D. Herrendoerfer wrote: >>>> destination >>>> device is in <code>private</code> mode.</dd> >>>> + <dt><code>passthru</code></dt> >>>> + <dd>This feature allows to attach a virtual function of a >>>> SRIOV >>>> capable >>> >> That patch looks fine to me, ACK, >> The only unresolved uncertainty to me is what happen if you try to >> use >> it on a kernel without the support ? >> > > This looks fine, except in formatdomain.html.in the feature is > still called > passthru. > > Used on a system without passthru support this raises an error > indicating a > failed macvtap setup. I fixed that one last place, and found one more issue: > > --- a/src/util/macvtap.h > +++ b/src/util/macvtap.h > @@ -92,6 +92,7 @@ void delMacvtap(const char *ifname, > # define MACVTAP_MODE_PRIVATE_STR "private" > # define MACVTAP_MODE_VEPA_STR "vepa" > # define MACVTAP_MODE_BRIDGE_STR "bridge" > +# define MACVTAP_MODE_PASSTHRU_STR "passthru" No one was using these defines (they were leftovers from before we switched to virEnum), so I deleted them. Now pushed. Thanks for the submission! -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org