[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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




On May 18, 2011, at 10:53 AM, Daniel Veillard wrote:

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 herrendoerfer name>
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 herrendoerfer name>
---
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 ?


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.

Dirk.

Daniel

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]