Re: [libvirt] [PATCH] Add support for 'passthru' mode for direct network interfaces

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/