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

[libvirt] [PATCH 2/2] Add tx_alg attribute to interface XML for virtio backend



qemu's virtio-net-pci driver allows setting the algorithm used for tx
packets to either "bh" or "timer". I don't know exactly how these
algorithms differ, but someone has requested the ability to choose
between them in libvirt's domain XML. See:

   https://bugzilla.redhat.com/show_bug.cgi?id=629662

This patch provides that knob, by adding a new attribute "tx_alg" to
the <driver> element that can be placed inside any <interface> element
in a domain definition. It's use would be something like this:

    <interface ...>
      ...
      <model type='virtio'/>
      <driver tx_alg='bh'/>
      ...
    </interface>

I chose to put this setting as an attribute to <driver> rather than as
a sub-element to <tune> because it is specific to the virtio-net
driver, not something that is generally usable by all network drivers.
(note that this is the same placement as the "driver name=..."
attribute used to choose kernel vs. userland backend for the
virtio-net driver.) This is a bit troublesome to me, because I can see
lots of new virtio options that could potentially be requested (just
run "qemu-kvm -device virtio-net-pci,?" on a qemu that's version
0.13.0 or newer, and compare that output to potential tunable items in
"-device e1000,?" or "-net tap,..."), so the attribute list could
potentially get quite long (which is something I was concerned about
when I first added the option to select kernel vs. userland backend,
but didn't realize just how many virtio-specific options there were).

Actually adding the tx=xxx option to the qemu commandline is only done
if the version of qemu being used advertises it in the output of

    qemu -device virtio-net-pci,?

If the option isn't listed there, the config item is ignored (should
it instead generate a warning log? error out and prevent the domain
from starting?)
---
 src/conf/domain_conf.c       |   26 +++++++++++++++++++++++++-
 src/conf/domain_conf.h       |   11 +++++++++++
 src/libvirt_private.syms     |    1 +
 src/qemu/qemu_capabilities.c |    3 +++
 src/qemu/qemu_capabilities.h |    1 +
 src/qemu/qemu_command.c      |   13 +++++++++++++
 6 files changed, 54 insertions(+), 1 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 84b866b..8f0f057 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -198,6 +198,11 @@ VIR_ENUM_IMPL(virDomainNetBackend, VIR_DOMAIN_NET_BACKEND_TYPE_LAST,
               "qemu",
               "vhost")
 
+VIR_ENUM_IMPL(virDomainNetVirtioTxAlg, VIR_DOMAIN_NET_BACKEND_TYPE_LAST,
+              "default",
+              "bh",
+              "timer")
+
 VIR_ENUM_IMPL(virDomainChrChannelTarget,
               VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST,
               "guestfwd",
@@ -2477,6 +2482,7 @@ virDomainNetDefParseXML(virCapsPtr caps,
     char *port = NULL;
     char *model = NULL;
     char *backend = NULL;
+    char *tx_alg = NULL;
     char *filter = NULL;
     char *internal = NULL;
     char *devaddr = NULL;
@@ -2565,6 +2571,7 @@ virDomainNetDefParseXML(virCapsPtr caps,
                 model = virXMLPropString(cur, "type");
             } else if (xmlStrEqual (cur->name, BAD_CAST "driver")) {
                 backend = virXMLPropString(cur, "name");
+                tx_alg = virXMLPropString(cur, "tx_alg");
             } else if (xmlStrEqual (cur->name, BAD_CAST "filterref")) {
                 filter = virXMLPropString(cur, "filter");
                 VIR_FREE(filterparams);
@@ -2769,6 +2776,18 @@ virDomainNetDefParseXML(virCapsPtr caps,
             }
             def->driver.virtio.name = name;
         }
+        if (tx_alg != NULL) {
+            int alg;
+            if (((alg = virDomainNetVirtioTxAlgTypeFromString(tx_alg)) < 0) ||
+                (alg == VIR_DOMAIN_NET_VIRTIO_TX_ALG_DEFAULT)) {
+                virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+                                     _("Unknown interface <driver tx_alg='%s'> "
+                                       "has been specified"),
+                                     tx_alg);
+                goto error;
+            }
+            def->driver.virtio.tx_alg = alg;
+        }
     }
 
     if (filter != NULL) {
@@ -2808,6 +2827,7 @@ cleanup:
     VIR_FREE(bridge);
     VIR_FREE(model);
     VIR_FREE(backend);
+    VIR_FREE(tx_alg);
     VIR_FREE(filter);
     VIR_FREE(type);
     VIR_FREE(internal);
@@ -6808,12 +6828,16 @@ virDomainNetDefFormat(virBufferPtr buf,
         virBufferEscapeString(buf, "      <model type='%s'/>\n",
                               def->model);
         if (STREQ(def->model, "virtio") &&
-            def->driver.virtio.name) {
+            (def->driver.virtio.name || def->driver.virtio.tx_alg)) {
             virBufferAddLit(buf, "      <driver");
             if (def->driver.virtio.name) {
                 virBufferVSprintf(buf, " name='%s'",
                                   virDomainNetBackendTypeToString(def->driver.virtio.name));
             }
+            if (def->driver.virtio.tx_alg) {
+                virBufferVSprintf(buf, " tx_alg='%s'",
+                                  virDomainNetVirtioTxAlgTypeToString(def->driver.virtio.tx_alg));
+            }
             virBufferAddLit(buf, "/>\n");
         }
     }
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 44d0a4b..b3a02f4 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -321,6 +321,15 @@ enum virDomainNetBackendType {
     VIR_DOMAIN_NET_BACKEND_TYPE_LAST,
 };
 
+/* the TX algorithm used for virtio interfaces */
+enum virDomainNetVirtioTxAlgType {
+    VIR_DOMAIN_NET_VIRTIO_TX_ALG_DEFAULT, /* default for this version of qemu */
+    VIR_DOMAIN_NET_VIRTIO_TX_ALG_BH,
+    VIR_DOMAIN_NET_VIRTIO_TX_ALG_TIMER,
+
+    VIR_DOMAIN_NET_VIRTIO_TX_ALG_LAST,
+};
+
 /* the mode type for macvtap devices */
 enum virDomainNetdevMacvtapType {
     VIR_DOMAIN_NETDEV_MACVTAP_MODE_VEPA,
@@ -341,6 +350,7 @@ struct _virDomainNetDef {
     union {
         struct {
             enum virDomainNetBackendType name; /* which driver backend to use */
+            enum virDomainNetVirtioTxAlgType tx_alg;
         } virtio;
     } driver;
     union {
@@ -1367,6 +1377,7 @@ VIR_ENUM_DECL(virDomainFS)
 VIR_ENUM_DECL(virDomainFSAccessMode)
 VIR_ENUM_DECL(virDomainNet)
 VIR_ENUM_DECL(virDomainNetBackend)
+VIR_ENUM_DECL(virDomainNetVirtioTxAlg)
 VIR_ENUM_DECL(virDomainChrDevice)
 VIR_ENUM_DECL(virDomainChrChannelTarget)
 VIR_ENUM_DECL(virDomainChrConsoleTarget)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 88e270c..dd44749 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -268,6 +268,7 @@ virDomainMemballoonModelTypeFromString;
 virDomainMemballoonModelTypeToString;
 virDomainNetDefFree;
 virDomainNetTypeToString;
+virDomainNetVirtioTxAlgTypeToString;
 virDomainObjAssignDef;
 virDomainObjSetDefTransient;
 virDomainObjGetPersistentDef;
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 0e1f79c..935c669 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1063,6 +1063,7 @@ qemuCapsExtractDeviceStr(const char *qemu,
                                "-device", "?",
                                "-device", "pci-assign,?",
                                "-device", "virtio-blk-pci,?",
+                               "-device", "virtio-net-pci,?",
                                NULL);
     virCommandAddEnvPassCommon(cmd);
     /* qemu -help goes to stdout, but qemu -device ? goes to stderr.  */
@@ -1104,6 +1105,8 @@ qemuCapsParseDeviceStr(const char *str, unsigned long long *flags)
         if (strstr(str, "pci-assign.bootindex"))
             *flags |= QEMUD_CMD_FLAG_PCI_BOOTINDEX;
     }
+    if (strstr(str, "virtio-net-pci.tx="))
+        *flags |= QEMUD_CMD_FLAG_VIRTIO_TX_ALG;
 
     return 0;
 }
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index dd39b3b..c29d914 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -92,6 +92,7 @@ enum qemuCapsFlags {
     QEMUD_CMD_FLAG_CCID_PASSTHRU = (1LL << 55), /* -device ccid-card-passthru */
     QEMUD_CMD_FLAG_CHARDEV_SPICEVMC = (1LL << 56), /* newer -chardev spicevmc */
     QEMUD_CMD_FLAG_DEVICE_SPICEVMC = (1LL << 57), /* older -device spicevmc*/
+    QEMUD_CMD_FLAG_VIRTIO_TX_ALG = (1LL << 58), /* -device virtio-net-pci,tx=string */
 };
 
 virCapsPtr qemuCapsInit(virCapsPtr old_caps);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1b213da..1d6f20c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1577,16 +1577,29 @@ qemuBuildNicDevStr(virDomainNetDefPtr net,
 {
     virBuffer buf = VIR_BUFFER_INITIALIZER;
     const char *nic;
+    bool usingVirtio = false;
 
     if (!net->model) {
         nic = "rtl8139";
     } else if (STREQ(net->model, "virtio")) {
         nic = "virtio-net-pci";
+        usingVirtio = true;
     } else {
         nic = net->model;
     }
 
     virBufferAdd(&buf, nic, strlen(nic));
+    if (usingVirtio && net->driver.virtio.tx_alg) {
+        if (qemuCmdFlags & QEMUD_CMD_FLAG_VIRTIO_TX_ALG) {
+            virBufferVSprintf(&buf, ",tx=%s",
+                              virDomainNetVirtioTxAlgTypeToString(net->driver.virtio.tx_alg));
+        } else {
+            /* What should we do if the option isn't available? log a
+             * warning? prevent the domain from starting? Ignore it?
+             * Right now we're ignoring it.
+             */
+        }
+    }
     if (vlan == -1)
         virBufferVSprintf(&buf, ",netdev=host%s", net->info.alias);
     else
-- 
1.7.3.4


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