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

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,