7:43 p.m.
Ján Tomko (5):
conf: split out virtio net driver formatting
Add virSwitch to data types
conf: remove redundant local variable
conf: add options for disabling segment offloading
qemu: wire up virtio-net segment offloading options
docs/formatdomain.html.in | 27 ++++
docs/schemas/basictypes.rng | 6 +
docs/schemas/domaincommon.rng | 25 +++
src/conf/domain_conf.c | 176 ++++++++++++++++-----
src/conf/domain_conf.h | 5 +
src/qemu/qemu_command.c | 20 +++
.../qemuxml2argv-net-virtio-disable-offloads.args | 8 +
.../qemuxml2argv-net-virtio-disable-offloads.xml | 32 ++++
tests/qemuxml2argvtest.c | 2 +
tests/qemuxml2xmltest.c | 1 +
10 files changed, 262 insertions(+), 40 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml
--
1.8.5.5
7:43 p.m.
New subject: [libvirt] [PATCH 1/5] conf: split out virtio net driver formatting
Instead of checking upfront if the <driver> element will be needed
in a big condition, just format all the attributes into a string
and output the <driver> element if the string is not empty.
---
src/conf/domain_conf.c | 68 ++++++++++++++++++++++++++++++++------------------
1 file changed, 44 insertions(+), 24 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a2a7d92..fcf7fb6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16401,6 +16401,41 @@ virDomainActualNetDefFormat(virBufferPtr buf,
return 0;
}
+
+static int
+virDomainVirtioNetDriverFormat(char **outstr,
+ virDomainNetDefPtr def)
+{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ if (def->driver.virtio.name) {
+ virBufferAsprintf(&buf, "name='%s' ",
+ virDomainNetBackendTypeToString(def->driver.virtio.name));
+ }
+ if (def->driver.virtio.txmode) {
+ virBufferAsprintf(&buf, "txmode='%s' ",
+
virDomainNetVirtioTxModeTypeToString(def->driver.virtio.txmode));
+ }
+ if (def->driver.virtio.ioeventfd) {
+ virBufferAsprintf(&buf, "ioeventfd='%s' ",
+
virTristateSwitchTypeToString(def->driver.virtio.ioeventfd));
+ }
+ if (def->driver.virtio.event_idx) {
+ virBufferAsprintf(&buf, "event_idx='%s' ",
+
virTristateSwitchTypeToString(def->driver.virtio.event_idx));
+ }
+ if (def->driver.virtio.queues)
+ virBufferAsprintf(&buf, "queues='%u' ",
def->driver.virtio.queues);
+
+ virBufferTrim(&buf, " ", -1);
+
+ if (virBufferCheckError(&buf) < 0)
+ return -1;
+
+ *outstr = virBufferContentAndReset(&buf);
+ return 0;
+}
+
+
int
virDomainNetDefFormat(virBufferPtr buf,
virDomainNetDefPtr def,
@@ -16576,30 +16611,15 @@ virDomainNetDefFormat(virBufferPtr buf,
if (def->model) {
virBufferEscapeString(buf, "<model type='%s'/>\n",
def->model);
- if (STREQ(def->model, "virtio") &&
- (def->driver.virtio.name || def->driver.virtio.txmode ||
- def->driver.virtio.ioeventfd || def->driver.virtio.event_idx ||
- def->driver.virtio.queues)) {
- virBufferAddLit(buf, "<driver");
- if (def->driver.virtio.name) {
- virBufferAsprintf(buf, " name='%s'",
-
virDomainNetBackendTypeToString(def->driver.virtio.name));
- }
- if (def->driver.virtio.txmode) {
- virBufferAsprintf(buf, " txmode='%s'",
-
virDomainNetVirtioTxModeTypeToString(def->driver.virtio.txmode));
- }
- if (def->driver.virtio.ioeventfd) {
- virBufferAsprintf(buf, " ioeventfd='%s'",
-
virTristateSwitchTypeToString(def->driver.virtio.ioeventfd));
- }
- if (def->driver.virtio.event_idx) {
- virBufferAsprintf(buf, " event_idx='%s'",
-
virTristateSwitchTypeToString(def->driver.virtio.event_idx));
- }
- if (def->driver.virtio.queues)
- virBufferAsprintf(buf, " queues='%u'",
def->driver.virtio.queues);
- virBufferAddLit(buf, "/>\n");
+ if (STREQ(def->model, "virtio")) {
+ char *str;
+
+ if (virDomainVirtioNetDriverFormat(&str, def) < 0)
+ return -1;
+
+ if (str)
+ virBufferAsprintf(buf, "<driver %s/>\n", str);
+ VIR_FREE(str);
}
}
if (def->filter) {
--
1.8.5.5
5:55 a.m.
New subject: [libvirt] [PATCH 1/5] conf: split out virtio net driver formatting
On 09/11/2014 07:43 AM, Ján Tomko wrote:
Instead of checking upfront if the <driver> element will be
needed
in a big condition, just format all the attributes into a string
and output the <driver> element if the string is not empty.
---
src/conf/domain_conf.c | 68 ++++++++++++++++++++++++++++++++------------------
1 file changed, 44 insertions(+), 24 deletions(-)
ACK
I left some thoughts below, but I think I convinced myself that this was
the way to go - just figured I'd keep the thoughts there though.
John
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a2a7d92..fcf7fb6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16401,6 +16401,41 @@ virDomainActualNetDefFormat(virBufferPtr buf,
return 0;
}
+
+static int
+virDomainVirtioNetDriverFormat(char **outstr,
+ virDomainNetDefPtr def)
Could perhaps have been:
static char *
virDomainVirtioNetDriverFormat(virDomainNetDefPtr def)
+{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ if (def->driver.virtio.name) {
+ virBufferAsprintf(&buf, "name='%s' ",
+ virDomainNetBackendTypeToString(def->driver.virtio.name));
+ }
+ if (def->driver.virtio.txmode) {
+ virBufferAsprintf(&buf, "txmode='%s' ",
+
virDomainNetVirtioTxModeTypeToString(def->driver.virtio.txmode));
+ }
+ if (def->driver.virtio.ioeventfd) {
+ virBufferAsprintf(&buf, "ioeventfd='%s' ",
+
virTristateSwitchTypeToString(def->driver.virtio.ioeventfd));
+ }
+ if (def->driver.virtio.event_idx) {
+ virBufferAsprintf(&buf, "event_idx='%s' ",
+
virTristateSwitchTypeToString(def->driver.virtio.event_idx));
+ }
+ if (def->driver.virtio.queues)
+ virBufferAsprintf(&buf, "queues='%u' ",
def->driver.virtio.queues);
+
+ virBufferTrim(&buf, " ", -1);
+
+ if (virBufferCheckError(&buf) < 0)
+ return -1;
So an 'error' here is more than likely a memory one (similar to old
code), but the only difference here is you're checking and failing
because of that; whereas, the old code didn't fail right away... Or at
least until the similar error checking was done on the buffer.
Hmm. so I guess this is why you took the approach of passing a string
address to return your string buffer into. This will just cause a bit
faster failure (more than likely) from the
+
+ *outstr = virBufferContentAndReset(&buf);
and return virBufferContentAndReset(&buf);
+ return 0;
+}
+
+
int
virDomainNetDefFormat(virBufferPtr buf,
virDomainNetDefPtr def,
@@ -16576,30 +16611,15 @@ virDomainNetDefFormat(virBufferPtr buf,
if (def->model) {
virBufferEscapeString(buf, "<model type='%s'/>\n",
def->model);
- if (STREQ(def->model, "virtio") &&
- (def->driver.virtio.name || def->driver.virtio.txmode ||
- def->driver.virtio.ioeventfd || def->driver.virtio.event_idx ||
- def->driver.virtio.queues)) {
- virBufferAddLit(buf, "<driver");
- if (def->driver.virtio.name) {
- virBufferAsprintf(buf, " name='%s'",
-
virDomainNetBackendTypeToString(def->driver.virtio.name));
- }
- if (def->driver.virtio.txmode) {
- virBufferAsprintf(buf, " txmode='%s'",
-
virDomainNetVirtioTxModeTypeToString(def->driver.virtio.txmode));
- }
- if (def->driver.virtio.ioeventfd) {
- virBufferAsprintf(buf, " ioeventfd='%s'",
-
virTristateSwitchTypeToString(def->driver.virtio.ioeventfd));
- }
- if (def->driver.virtio.event_idx) {
- virBufferAsprintf(buf, " event_idx='%s'",
-
virTristateSwitchTypeToString(def->driver.virtio.event_idx));
- }
- if (def->driver.virtio.queues)
- virBufferAsprintf(buf, " queues='%u'",
def->driver.virtio.queues);
- virBufferAddLit(buf, "/>\n");
+ if (STREQ(def->model, "virtio")) {
+ char *str;
+
+ if (virDomainVirtioNetDriverFormat(&str, def) < 0)
+ return -1;
+
+ if (str)
+ virBufferAsprintf(buf, "<driver %s/>\n", str);
Of course if you went with return char * option, it'd be
if ((str = virDomainVirtioNetDriverFormat(def)))
virBufferAsprintf(buf, "<driver %s/>\n", str);
+ VIR_FREE(str);
}
}
if (def->filter) {
7:33 p.m.
New subject: [libvirt] [PATCH 1/5] conf: split out virtio net driver formatting
On 09/15/2014 11:55 PM, John Ferlan wrote:
On 09/11/2014 07:43 AM, Ján Tomko wrote:
> Instead of checking upfront if the <driver> element will be needed
> in a big condition, just format all the attributes into a string
> and output the <driver> element if the string is not empty.
> ---
> src/conf/domain_conf.c | 68 ++++++++++++++++++++++++++++++++------------------
> 1 file changed, 44 insertions(+), 24 deletions(-)
>
ACK
I left some thoughts below, but I think I convinced myself that this was
the way to go - just figured I'd keep the thoughts there though.
John
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a2a7d92..fcf7fb6 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -16401,6 +16401,41 @@ virDomainActualNetDefFormat(virBufferPtr buf,
> return 0;
> }
>
> +
> +static int
> +virDomainVirtioNetDriverFormat(char **outstr,
> + virDomainNetDefPtr def)
Could perhaps have been:
static char *
virDomainVirtioNetDriverFormat(virDomainNetDefPtr def)
> +{
> + virBuffer buf = VIR_BUFFER_INITIALIZER;
> + if (def->driver.virtio.name) {
> + virBufferAsprintf(&buf, "name='%s' ",
> +
virDomainNetBackendTypeToString(def->driver.virtio.name));
> + }
> + if (def->driver.virtio.txmode) {
> + virBufferAsprintf(&buf, "txmode='%s' ",
> +
virDomainNetVirtioTxModeTypeToString(def->driver.virtio.txmode));
> + }
> + if (def->driver.virtio.ioeventfd) {
> + virBufferAsprintf(&buf, "ioeventfd='%s' ",
> +
virTristateSwitchTypeToString(def->driver.virtio.ioeventfd));
> + }
> + if (def->driver.virtio.event_idx) {
> + virBufferAsprintf(&buf, "event_idx='%s' ",
> +
virTristateSwitchTypeToString(def->driver.virtio.event_idx));
> + }
> + if (def->driver.virtio.queues)
> + virBufferAsprintf(&buf, "queues='%u' ",
def->driver.virtio.queues);
> +
> + virBufferTrim(&buf, " ", -1);
> +
> + if (virBufferCheckError(&buf) < 0)
> + return -1;
So an 'error' here is more than likely a memory one (similar to old
code), but the only difference here is you're checking and failing
because of that; whereas, the old code didn't fail right away... Or at
least until the similar error checking was done on the buffer.
Hmm. so I guess this is why you took the approach of passing a string
address to return your string buffer into. This will just cause a bit
faster failure (more than likely) from the
I wanted to get rid of the giant condition, failing sooner is just a side effect.
> +
> + *outstr = virBufferContentAndReset(&buf);
and return virBufferContentAndReset(&buf);
> + return 0;
> +}
> +
> +
> int
> virDomainNetDefFormat(virBufferPtr buf,
> virDomainNetDefPtr def,
> @@ -16576,30 +16611,15 @@ virDomainNetDefFormat(virBufferPtr buf,
> if (def->model) {
> virBufferEscapeString(buf, "<model type='%s'/>\n",
> def->model);
> - if (STREQ(def->model, "virtio") &&
> - (def->driver.virtio.name || def->driver.virtio.txmode ||
> - def->driver.virtio.ioeventfd || def->driver.virtio.event_idx ||
> - def->driver.virtio.queues)) {
> - virBufferAddLit(buf, "<driver");
> - if (def->driver.virtio.name) {
> - virBufferAsprintf(buf, " name='%s'",
> -
virDomainNetBackendTypeToString(def->driver.virtio.name));
> - }
> - if (def->driver.virtio.txmode) {
> - virBufferAsprintf(buf, " txmode='%s'",
> -
virDomainNetVirtioTxModeTypeToString(def->driver.virtio.txmode));
> - }
> - if (def->driver.virtio.ioeventfd) {
> - virBufferAsprintf(buf, " ioeventfd='%s'",
> -
virTristateSwitchTypeToString(def->driver.virtio.ioeventfd));
> - }
> - if (def->driver.virtio.event_idx) {
> - virBufferAsprintf(buf, " event_idx='%s'",
> -
virTristateSwitchTypeToString(def->driver.virtio.event_idx));
> - }
> - if (def->driver.virtio.queues)
> - virBufferAsprintf(buf, " queues='%u'",
def->driver.virtio.queues);
> - virBufferAddLit(buf, "/>\n");
> + if (STREQ(def->model, "virtio")) {
> + char *str;
> +
> + if (virDomainVirtioNetDriverFormat(&str, def) < 0)
> + return -1;
> +
> + if (str)
> + virBufferAsprintf(buf, "<driver %s/>\n", str);
Of course if you went with return char * option, it'd be
if ((str = virDomainVirtioNetDriverFormat(def)))
virBufferAsprintf(buf, "<driver %s/>\n", str);
We wouldn't know if str is NULL because the driver has no special options, or
if it's because we were out of memory. In the unlikely event of
virDomainVirtioNetDriverFormat getting OOM error and the other virBuffer*
calls succeeding, the <driver> element would not get formatted, but we
wouldn't report an error either.
Jan
7:43 p.m.
New subject: [libvirt] [PATCH 2/5] Add virSwitch to data types
Just to make this series work until Martin pushes his more complete
cleanup:
https://www.redhat.com/archives/libvir-list/2014-September/msg00391.html
---
docs/schemas/basictypes.rng | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
index 75d5238..3e90262 100644
--- a/docs/schemas/basictypes.rng
+++ b/docs/schemas/basictypes.rng
@@ -446,4 +446,10 @@
</optional>
</define>
+ <define name="virSwitch">
+ <choice>
+ <value>on</value>
+ <value>off</value>
+ </choice>
+ </define>
</grammar>
--
1.8.5.5
5:56 a.m.
New subject: [libvirt] [PATCH 2/5] Add virSwitch to data types
On 09/11/2014 07:43 AM, Ján Tomko wrote:
Just to make this series work until Martin pushes his more complete
cleanup:
https://www.redhat.com/archives/libvir-list/2014-September/msg00391.html
---
docs/schemas/basictypes.rng | 6 ++++++
1 file changed, 6 insertions(+)
Looks like this goes away with Martin's virOnOff
John
diff --git a/docs/schemas/basictypes.rng
b/docs/schemas/basictypes.rng
index 75d5238..3e90262 100644
--- a/docs/schemas/basictypes.rng
+++ b/docs/schemas/basictypes.rng
@@ -446,4 +446,10 @@
</optional>
</define>
+ <define name="virSwitch">
+ <choice>
+ <value>on</value>
+ <value>off</value>
+ </choice>
+ </define>
</grammar>
7:43 p.m.
New subject: [libvirt] [PATCH 3/5] conf: remove redundant local variable
Use just one int variable for all the FromString calls.
---
src/conf/domain_conf.c | 27 +++++++++++----------------
1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fcf7fb6..0fdfa6f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6904,7 +6904,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
virNWFilterHashTablePtr filterparams = NULL;
virDomainActualNetDefPtr actual = NULL;
xmlNodePtr oldnode = ctxt->node;
- int ret;
+ int ret, val;
if (VIR_ALLOC(def) < 0)
return NULL;
@@ -7248,13 +7248,12 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
}
if (mode != NULL) {
- int m;
- if ((m = virNetDevMacVLanModeTypeFromString(mode)) < 0) {
+ if ((val = virNetDevMacVLanModeTypeFromString(mode)) < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Unknown mode has been specified"));
goto error;
}
- def->data.direct.mode = m;
+ def->data.direct.mode = val;
} else {
def->data.direct.mode = VIR_NETDEV_MACVLAN_MODE_VEPA;
}
@@ -7329,31 +7328,28 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
if (def->type != VIR_DOMAIN_NET_TYPE_HOSTDEV &&
STREQ_NULLABLE(def->model, "virtio")) {
if (backend != NULL) {
- int name;
- if ((name = virDomainNetBackendTypeFromString(backend)) < 0 ||
- name == VIR_DOMAIN_NET_BACKEND_TYPE_DEFAULT) {
+ if ((val = virDomainNetBackendTypeFromString(backend)) < 0 ||
+ val == VIR_DOMAIN_NET_BACKEND_TYPE_DEFAULT) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Unknown interface <driver name='%s'>
"
"has been specified"),
backend);
goto error;
}
- def->driver.virtio.name = name;
+ def->driver.virtio.name = val;
}
if (txmode != NULL) {
- int m;
- if ((m = virDomainNetVirtioTxModeTypeFromString(txmode)) < 0 ||
- m == VIR_DOMAIN_NET_VIRTIO_TX_MODE_DEFAULT) {
+ if ((val = virDomainNetVirtioTxModeTypeFromString(txmode)) < 0 ||
+ val == VIR_DOMAIN_NET_VIRTIO_TX_MODE_DEFAULT) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Unknown interface <driver
txmode='%s'> "
"has been specified"),
txmode);
goto error;
}
- def->driver.virtio.txmode = m;
+ def->driver.virtio.txmode = val;
}
if (ioeventfd) {
- int val;
if ((val = virTristateSwitchTypeFromString(ioeventfd)) <= 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unknown interface ioeventfd mode
'%s'"),
@@ -7363,14 +7359,13 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
def->driver.virtio.ioeventfd = val;
}
if (event_idx) {
- int idx;
- if ((idx = virTristateSwitchTypeFromString(event_idx)) <= 0) {
+ if ((val = virTristateSwitchTypeFromString(event_idx)) <= 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unknown interface event_idx mode
'%s'"),
event_idx);
goto error;
}
- def->driver.virtio.event_idx = idx;
+ def->driver.virtio.event_idx = val;
}
if (queues) {
unsigned int q;
--
1.8.5.5
5:56 a.m.
New subject: [libvirt] [PATCH 3/5] conf: remove redundant local variable
On 09/11/2014 07:43 AM, Ján Tomko wrote:
Use just one int variable for all the FromString calls.
---
src/conf/domain_conf.c | 27 +++++++++++----------------
1 file changed, 11 insertions(+), 16 deletions(-)
ACK
7:43 p.m.
New subject: [libvirt] [PATCH 4/5] conf: add options for disabling segment offloading
Add the following attributes:
csum, gso, guest_tso4, guest_tso6, guest_ecn
to the <driver> element of network interface
which control the virtio-net device properties
of the same names.
---
docs/formatdomain.html.in | 27 ++++++++
docs/schemas/domaincommon.rng | 25 +++++++
src/conf/domain_conf.c | 81 ++++++++++++++++++++++
src/conf/domain_conf.h | 5 ++
.../qemuxml2argv-net-virtio-disable-offloads.xml | 32 +++++++++
tests/qemuxml2xmltest.c | 1 +
6 files changed, 171 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index a2ea758..5b2758a 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3847,6 +3847,12 @@ qemu-kvm -net nic,model=? /dev/null
<model type='virtio'/>
<b><driver name='vhost' txmode='iothread'
ioeventfd='on' event_idx='off' queues='5'/></b>
</interface>
+ <interface type='network'>
+ <source network='default'/>
+ <target dev='vnet2'/>
+ <model type='virtio'/>
+ <b><driver csum='off' gso='off'
guest_tso4='off' guest_tso6='off'
guest_ecn='off'/></b>
+ </interface>
</devices>
...</pre>
@@ -3949,6 +3955,27 @@ qemu-kvm -net nic,model=? /dev/null
processor, resulting in much higher throughput.
<span class="since">Since 1.0.6 (QEMU and KVM only)</span>
</dd>
+ <dt><code>csum</code></dt>
+ <dd>
+ The <code>csum</code> attribute with possible values
<code>on</code>
+ and <code>off</code> controls host-side support for packets with
+ partial checksum values.
+ <span class="since">Since 1.2.9 (QEMU
only)</span><br/><br/>
+
+ <b>In general you should leave this option alone, unless you
+ are very certain you know what you are doing.</b>
+ </dd>
+ <dt>segment offloading options</dt>
+ <dd>
+ The attributes <code>gso</code>,
<code>guest_tso4</code>,
+ <code>guest_tso6</code>, <code>guest_ecn</code> with
possible
+ values of <code>on</code> and <code>off</code> can be
used
+ to tune segment offloading.
+ <span class="since">Since 1.2.9 (QEMU
only)</span><br/><br/>
+
+ <b>In general you should leave this option alone, unless you
+ are very certain you know what you are doing.</b>
+ </dd>
</dl>
<h5><a name="elementsNICSTargetOverride">Overriding the target
element</a></h5>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 6ae940a..c5b092f 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2373,6 +2373,31 @@
<optional>
<ref name="event_idx"/>
</optional>
+ <optional>
+ <attribute name="csum">
+ <ref name="virSwitch"/>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name="gso">
+ <ref name="virSwitch"/>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name="guest_tso4">
+ <ref name="virSwitch"/>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name="guest_tso6">
+ <ref name="virSwitch"/>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name="guest_ecn">
+ <ref name="virSwitch"/>
+ </attribute>
+ </optional>
</group>
</choice>
<empty/>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0fdfa6f..cc67e35 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6892,6 +6892,11 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
char *ioeventfd = NULL;
char *event_idx = NULL;
char *queues = NULL;
+ char *csum = NULL;
+ char *gso = NULL;
+ char *guest_tso4 = NULL;
+ char *guest_tso6 = NULL;
+ char *guest_ecn = NULL;
char *filter = NULL;
char *internal = NULL;
char *devaddr = NULL;
@@ -7015,6 +7020,11 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
ioeventfd = virXMLPropString(cur, "ioeventfd");
event_idx = virXMLPropString(cur, "event_idx");
queues = virXMLPropString(cur, "queues");
+ csum = virXMLPropString(cur, "csum");
+ gso = virXMLPropString(cur, "gso");
+ guest_tso4 = virXMLPropString(cur, "guest_tso4");
+ guest_tso6 = virXMLPropString(cur, "guest_tso6");
+ guest_ecn = virXMLPropString(cur, "guest_ecn");
} else if (xmlStrEqual(cur->name, BAD_CAST "filterref")) {
if (filter) {
virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -7377,6 +7387,51 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
}
def->driver.virtio.queues = q;
}
+ if (csum) {
+ if ((val = virTristateSwitchTypeFromString(csum)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown interface csum mode '%s'"),
+ csum);
+ goto error;
+ }
+ def->driver.virtio.csum = val;
+ }
+ if (gso) {
+ if ((val = virTristateSwitchTypeFromString(gso)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown interface gso mode '%s'"),
+ gso);
+ goto error;
+ }
+ def->driver.virtio.gso = val;
+ }
+ if (guest_tso4) {
+ if ((val = virTristateSwitchTypeFromString(guest_tso4)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown interface guest_tso4 mode
'%s'"),
+ guest_tso4);
+ goto error;
+ }
+ def->driver.virtio.guest_tso4 = val;
+ }
+ if (guest_tso6) {
+ if ((val = virTristateSwitchTypeFromString(guest_tso6)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown interface guest_tso6 mode
'%s'"),
+ guest_tso6);
+ goto error;
+ }
+ def->driver.virtio.guest_tso6 = val;
+ }
+ if (guest_ecn) {
+ if ((val = virTristateSwitchTypeFromString(guest_ecn)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown interface guest_ecn mode
'%s'"),
+ guest_ecn);
+ goto error;
+ }
+ def->driver.virtio.guest_ecn = val;
+ }
}
def->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DEFAULT;
@@ -7434,6 +7489,11 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
VIR_FREE(ioeventfd);
VIR_FREE(event_idx);
VIR_FREE(queues);
+ VIR_FREE(csum);
+ VIR_FREE(gso);
+ VIR_FREE(guest_tso4);
+ VIR_FREE(guest_tso6);
+ VIR_FREE(guest_ecn);
VIR_FREE(filter);
VIR_FREE(type);
VIR_FREE(internal);
@@ -16421,6 +16481,27 @@ virDomainVirtioNetDriverFormat(char **outstr,
if (def->driver.virtio.queues)
virBufferAsprintf(&buf, "queues='%u' ",
def->driver.virtio.queues);
+ if (def->driver.virtio.csum) {
+ virBufferAsprintf(&buf, "csum='%s' ",
+ virTristateSwitchTypeToString(def->driver.virtio.csum));
+ }
+ if (def->driver.virtio.gso) {
+ virBufferAsprintf(&buf, "gso='%s' ",
+ virTristateSwitchTypeToString(def->driver.virtio.gso));
+ }
+ if (def->driver.virtio.guest_tso4) {
+ virBufferAsprintf(&buf, "guest_tso4='%s' ",
+
virTristateSwitchTypeToString(def->driver.virtio.guest_tso4));
+ }
+ if (def->driver.virtio.guest_tso6) {
+ virBufferAsprintf(&buf, "guest_tso6='%s' ",
+
virTristateSwitchTypeToString(def->driver.virtio.guest_tso6));
+ }
+ if (def->driver.virtio.guest_ecn) {
+ virBufferAsprintf(&buf, "guest_ecn='%s' ",
+
virTristateSwitchTypeToString(def->driver.virtio.guest_ecn));
+ }
+
virBufferTrim(&buf, " ", -1);
if (virBufferCheckError(&buf) < 0)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index efae2f5..99a6aa9 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -895,6 +895,11 @@ struct _virDomainNetDef {
virTristateSwitch ioeventfd;
virTristateSwitch event_idx;
unsigned int queues; /* Multiqueue virtio-net */
+ virTristateSwitch csum;
+ virTristateSwitch gso;
+ virTristateSwitch guest_tso4;
+ virTristateSwitch guest_tso6;
+ virTristateSwitch guest_ecn;
} virtio;
} driver;
union {
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml
b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml
new file mode 100644
index 0000000..5603604
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml
@@ -0,0 +1,32 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest7'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <controller type='usb' index='0'/>
+ <controller type='ide' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <interface type='user'>
+ <mac address='00:22:44:66:88:aa'/>
+ <model type='virtio'/>
+ <driver csum='off' gso='off' guest_tso4='off'
guest_tso6='off' guest_ecn='off'/>
+ </interface>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 34cdb97..183c13f 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -260,6 +260,7 @@ mymain(void)
DO_TEST("net-user");
DO_TEST("net-virtio");
DO_TEST("net-virtio-device");
+ DO_TEST("net-virtio-disable-offloads");
DO_TEST("net-eth");
DO_TEST("net-eth-ifname");
DO_TEST("net-virtio-network-portgroup");
--
1.8.5.5
6:24 a.m.
New subject: [libvirt] [PATCH 4/5] conf: add options for disabling segment offloading
On 09/11/2014 07:43 AM, Ján Tomko wrote:
Add the following attributes:
csum, gso, guest_tso4, guest_tso6, guest_ecn
to the <driver> element of network interface
which control the virtio-net device properties
of the same names.
---
docs/formatdomain.html.in | 27 ++++++++
docs/schemas/domaincommon.rng | 25 +++++++
src/conf/domain_conf.c | 81 ++++++++++++++++++++++
src/conf/domain_conf.h | 5 ++
.../qemuxml2argv-net-virtio-disable-offloads.xml | 32 +++++++++
tests/qemuxml2xmltest.c | 1 +
6 files changed, 171 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index a2ea758..5b2758a 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3847,6 +3847,12 @@ qemu-kvm -net nic,model=? /dev/null
<model type='virtio'/>
<b><driver name='vhost' txmode='iothread'
ioeventfd='on' event_idx='off' queues='5'/></b>
</interface>
+ <interface type='network'>
+ <source network='default'/>
+ <target dev='vnet2'/>
+ <model type='virtio'/>
+ <b><driver csum='off' gso='off'
guest_tso4='off' guest_tso6='off'
guest_ecn='off'/></b>
+ </interface>
This doesn't require a driver name='' value?
</devices>
...</pre>
@@ -3949,6 +3955,27 @@ qemu-kvm -net nic,model=? /dev/null
processor, resulting in much higher throughput.
<span class="since">Since 1.0.6 (QEMU and KVM
only)</span>
</dd>
+ <dt><code>csum</code></dt>
+ <dd>
+ The <code>csum</code> attribute with possible values
<code>on</code>
+ and <code>off</code> controls host-side support for packets with
+ partial checksum values.
+ <span class="since">Since 1.2.9 (QEMU
only)</span><br/><br/>
+
+ <b>In general you should leave this option alone, unless you
+ are very certain you know what you are doing.</b>
+ </dd>
+ <dt>segment offloading options</dt>
+ <dd>
+ The attributes <code>gso</code>,
<code>guest_tso4</code>,
+ <code>guest_tso6</code>, <code>guest_ecn</code> with
possible
+ values of <code>on</code> and <code>off</code> can be
used
+ to tune segment offloading.
+ <span class="since">Since 1.2.9 (QEMU
only)</span><br/><br/>
+
+ <b>In general you should leave this option alone, unless you
+ are very certain you know what you are doing.</b>
s/this option/these options/
[oh - I'm having a flashback to something similar I had to do at my
former employer with their virtualization switch software... these got
enabled by some application and caused shall we say significant
performance issues, especially for older drivers. That particular
software was loaded/started after the virtualization network switch and
thus reset what our code had done... Bugger to find as it embedded in
some output from some network command that was executed rarely in our
vswitch network daemon layer]
Anyway, I understand it's desired to not say much about them, but is
there any need to say what the defaults are? Furthermore, does one
domain's setting affect other domains? I guess my curiosity is more is
this a domain function or an interface (host) setting. We may want to
indicate that here... Is the difference dependent upon the driver name?
I know from my previous experience it had a wider affect, but that code
had a very different implementation. The vswitch was separate from the
guest as a host process to which guests could configure ports. The
vswitch software had the configuration magic to the lower level network
driver(s) which is where the tso/cko, etc. were managed in the kernel.
The equivalent of 'em0', 'eth0', etc - the physical NIC. A vswitch was
"tied" to a particular physical NIC. So any changes to the pNIC cast a
wide net, which is why "other" software setting TSO/CKO options on the
same pNIC our vswitch used after our code disabled it caused all sorts
of issues. (Just so you understand why I'm asking the question).
+ </dd>
</dl>
<h5><a name="elementsNICSTargetOverride">Overriding the target
element</a></h5>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 6ae940a..c5b092f 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2373,6 +2373,31 @@
<optional>
<ref name="event_idx"/>
</optional>
+ <optional>
+ <attribute name="csum">
+ <ref name="virSwitch"/>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name="gso">
+ <ref name="virSwitch"/>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name="guest_tso4">
+ <ref name="virSwitch"/>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name="guest_tso6">
+ <ref name="virSwitch"/>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name="guest_ecn">
+ <ref name="virSwitch"/>
+ </attribute>
+ </optional>
s/virSwitch/virOnOff/g
The rest looks fine... ACK with the nits fixed...
John
</group>
</choice>
<empty/>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0fdfa6f..cc67e35 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6892,6 +6892,11 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
char *ioeventfd = NULL;
char *event_idx = NULL;
char *queues = NULL;
+ char *csum = NULL;
+ char *gso = NULL;
+ char *guest_tso4 = NULL;
+ char *guest_tso6 = NULL;
+ char *guest_ecn = NULL;
char *filter = NULL;
char *internal = NULL;
char *devaddr = NULL;
@@ -7015,6 +7020,11 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
ioeventfd = virXMLPropString(cur, "ioeventfd");
event_idx = virXMLPropString(cur, "event_idx");
queues = virXMLPropString(cur, "queues");
+ csum = virXMLPropString(cur, "csum");
+ gso = virXMLPropString(cur, "gso");
+ guest_tso4 = virXMLPropString(cur, "guest_tso4");
+ guest_tso6 = virXMLPropString(cur, "guest_tso6");
+ guest_ecn = virXMLPropString(cur, "guest_ecn");
} else if (xmlStrEqual(cur->name, BAD_CAST "filterref")) {
if (filter) {
virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -7377,6 +7387,51 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
}
def->driver.virtio.queues = q;
}
+ if (csum) {
+ if ((val = virTristateSwitchTypeFromString(csum)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown interface csum mode '%s'"),
+ csum);
+ goto error;
+ }
+ def->driver.virtio.csum = val;
+ }
+ if (gso) {
+ if ((val = virTristateSwitchTypeFromString(gso)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown interface gso mode '%s'"),
+ gso);
+ goto error;
+ }
+ def->driver.virtio.gso = val;
+ }
+ if (guest_tso4) {
+ if ((val = virTristateSwitchTypeFromString(guest_tso4)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown interface guest_tso4 mode
'%s'"),
+ guest_tso4);
+ goto error;
+ }
+ def->driver.virtio.guest_tso4 = val;
+ }
+ if (guest_tso6) {
+ if ((val = virTristateSwitchTypeFromString(guest_tso6)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown interface guest_tso6 mode
'%s'"),
+ guest_tso6);
+ goto error;
+ }
+ def->driver.virtio.guest_tso6 = val;
+ }
+ if (guest_ecn) {
+ if ((val = virTristateSwitchTypeFromString(guest_ecn)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown interface guest_ecn mode
'%s'"),
+ guest_ecn);
+ goto error;
+ }
+ def->driver.virtio.guest_ecn = val;
+ }
}
def->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DEFAULT;
@@ -7434,6 +7489,11 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
VIR_FREE(ioeventfd);
VIR_FREE(event_idx);
VIR_FREE(queues);
+ VIR_FREE(csum);
+ VIR_FREE(gso);
+ VIR_FREE(guest_tso4);
+ VIR_FREE(guest_tso6);
+ VIR_FREE(guest_ecn);
VIR_FREE(filter);
VIR_FREE(type);
VIR_FREE(internal);
@@ -16421,6 +16481,27 @@ virDomainVirtioNetDriverFormat(char **outstr,
if (def->driver.virtio.queues)
virBufferAsprintf(&buf, "queues='%u' ",
def->driver.virtio.queues);
+ if (def->driver.virtio.csum) {
+ virBufferAsprintf(&buf, "csum='%s' ",
+ virTristateSwitchTypeToString(def->driver.virtio.csum));
+ }
+ if (def->driver.virtio.gso) {
+ virBufferAsprintf(&buf, "gso='%s' ",
+ virTristateSwitchTypeToString(def->driver.virtio.gso));
+ }
+ if (def->driver.virtio.guest_tso4) {
+ virBufferAsprintf(&buf, "guest_tso4='%s' ",
+
virTristateSwitchTypeToString(def->driver.virtio.guest_tso4));
+ }
+ if (def->driver.virtio.guest_tso6) {
+ virBufferAsprintf(&buf, "guest_tso6='%s' ",
+
virTristateSwitchTypeToString(def->driver.virtio.guest_tso6));
+ }
+ if (def->driver.virtio.guest_ecn) {
+ virBufferAsprintf(&buf, "guest_ecn='%s' ",
+
virTristateSwitchTypeToString(def->driver.virtio.guest_ecn));
+ }
+
virBufferTrim(&buf, " ", -1);
if (virBufferCheckError(&buf) < 0)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index efae2f5..99a6aa9 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -895,6 +895,11 @@ struct _virDomainNetDef {
virTristateSwitch ioeventfd;
virTristateSwitch event_idx;
unsigned int queues; /* Multiqueue virtio-net */
+ virTristateSwitch csum;
+ virTristateSwitch gso;
+ virTristateSwitch guest_tso4;
+ virTristateSwitch guest_tso6;
+ virTristateSwitch guest_ecn;
} virtio;
} driver;
union {
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml
b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml
new file mode 100644
index 0000000..5603604
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml
@@ -0,0 +1,32 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest7'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <controller type='usb' index='0'/>
+ <controller type='ide' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <interface type='user'>
+ <mac address='00:22:44:66:88:aa'/>
+ <model type='virtio'/>
+ <driver csum='off' gso='off' guest_tso4='off'
guest_tso6='off' guest_ecn='off'/>
+ </interface>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 34cdb97..183c13f 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -260,6 +260,7 @@ mymain(void)
DO_TEST("net-user");
DO_TEST("net-virtio");
DO_TEST("net-virtio-device");
+ DO_TEST("net-virtio-disable-offloads");
DO_TEST("net-eth");
DO_TEST("net-eth-ifname");
DO_TEST("net-virtio-network-portgroup");
3:43 p.m.
New subject: [libvirt] [PATCH 4/5] conf: add options for disabling segment offloading
On 09/16/2014 12:24 AM, John Ferlan wrote:
On 09/11/2014 07:43 AM, Ján Tomko wrote:
> Add the following attributes:
> csum, gso, guest_tso4, guest_tso6, guest_ecn
> to the <driver> element of network interface
> which control the virtio-net device properties
> of the same names.
> ---
> docs/formatdomain.html.in | 27 ++++++++
> docs/schemas/domaincommon.rng | 25 +++++++
> src/conf/domain_conf.c | 81 ++++++++++++++++++++++
> src/conf/domain_conf.h | 5 ++
> .../qemuxml2argv-net-virtio-disable-offloads.xml | 32 +++++++++
> tests/qemuxml2xmltest.c | 1 +
> 6 files changed, 171 insertions(+)
> create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index a2ea758..5b2758a 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -3847,6 +3847,12 @@ qemu-kvm -net nic,model=? /dev/null
> <model type='virtio'/>
> <b><driver name='vhost' txmode='iothread'
ioeventfd='on' event_idx='off' queues='5'/></b>
> </interface>
> + <interface type='network'>
> + <source network='default'/>
> + <target dev='vnet2'/>
> + <model type='virtio'/>
> + <b><driver csum='off' gso='off'
guest_tso4='off' guest_tso6='off'
guest_ecn='off'/></b>
> + </interface>
This doesn't require a driver name='' value?
AFAIK only queues require name='vhost'.
> </devices>
> ...</pre>
>
> @@ -3949,6 +3955,27 @@ qemu-kvm -net nic,model=? /dev/null
> processor, resulting in much higher throughput.
> <span class="since">Since 1.0.6 (QEMU and KVM
only)</span>
> </dd>
> + <dt><code>csum</code></dt>
> + <dd>
> + The <code>csum</code> attribute with possible values
<code>on</code>
> + and <code>off</code> controls host-side support for packets
with
> + partial checksum values.
> + <span class="since">Since 1.2.9 (QEMU
only)</span><br/><br/>
> +
> + <b>In general you should leave this option alone, unless you
> + are very certain you know what you are doing.</b>
> + </dd>
> + <dt>segment offloading options</dt>
> + <dd>
> + The attributes <code>gso</code>,
<code>guest_tso4</code>,
> + <code>guest_tso6</code>, <code>guest_ecn</code> with
possible
> + values of <code>on</code> and <code>off</code> can
be used
> + to tune segment offloading.
> + <span class="since">Since 1.2.9 (QEMU
only)</span><br/><br/>
> +
> + <b>In general you should leave this option alone, unless you
> + are very certain you know what you are doing.</b>
s/this option/these options/
[oh - I'm having a flashback to something similar I had to do at my
former employer with their virtualization switch software... these got
enabled by some application and caused shall we say significant
performance issues, especially for older drivers. That particular
software was loaded/started after the virtualization network switch and
thus reset what our code had done... Bugger to find as it embedded in
some output from some network command that was executed rarely in our
vswitch network daemon layer]
Anyway, I understand it's desired to not say much about them, but is
there any need to say what the defaults are? Furthermore, does one
domain's setting affect other domains? I guess my curiosity is more is
this a domain function or an interface (host) setting. We may want to
indicate that here... Is the difference dependent upon the driver name?
This is a per (guest) interface setting, I'm not aware of it affecting other
interfaces or domains. Perhaps the 'leave this option alone' was too harsh.
I'll add the defaults before pushing/sending another version.
Jan
I know from my previous experience it had a wider affect, but that
code
had a very different implementation. The vswitch was separate from the
guest as a host process to which guests could configure ports. The
vswitch software had the configuration magic to the lower level network
driver(s) which is where the tso/cko, etc. were managed in the kernel.
The equivalent of 'em0', 'eth0', etc - the physical NIC. A vswitch was
"tied" to a particular physical NIC. So any changes to the pNIC cast a
wide net, which is why "other" software setting TSO/CKO options on the
same pNIC our vswitch used after our code disabled it caused all sorts
of issues. (Just so you understand why I'm asking the question).
> + </dd>
> </dl>
>
> <h5><a name="elementsNICSTargetOverride">Overriding the
target element</a></h5>
6:30 a.m.
New subject: [libvirt] [PATCH 4/5] conf: add options for disabling segment offloading
On 09/11/2014 05:43 AM, Ján Tomko wrote:
Add the following attributes:
csum, gso, guest_tso4, guest_tso6, guest_ecn
to the <driver> element of network interface
which control the virtio-net device properties
of the same names.
---
docs/formatdomain.html.in | 27 ++++++++
docs/schemas/domaincommon.rng | 25 +++++++
src/conf/domain_conf.c | 81 ++++++++++++++++++++++
src/conf/domain_conf.h | 5 ++
.../qemuxml2argv-net-virtio-disable-offloads.xml | 32 +++++++++
tests/qemuxml2xmltest.c | 1 +
6 files changed, 171 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index a2ea758..5b2758a 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3847,6 +3847,12 @@ qemu-kvm -net nic,model=? /dev/null
<model type='virtio'/>
<b><driver name='vhost' txmode='iothread'
ioeventfd='on' event_idx='off' queues='5'/></b>
</interface>
+ <interface type='network'>
+ <source network='default'/>
+ <target dev='vnet2'/>
+ <model type='virtio'/>
+ <b><driver csum='off' gso='off'
guest_tso4='off' guest_tso6='off'
guest_ecn='off'/></b>
+ </interface>
Are we stuck with names with underscores in our XML? I'm still not sure
if we've come up with the best naming for exposing all these knobs.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:43 p.m.
New subject: [libvirt] [PATCH 4/5] conf: add options for disabling segment offloading
On 09/16/2014 12:30 AM, Eric Blake wrote:
On 09/11/2014 05:43 AM, Ján Tomko wrote:
> Add the following attributes:
> csum, gso, guest_tso4, guest_tso6, guest_ecn
> to the <driver> element of network interface
> which control the virtio-net device properties
> of the same names.
> ---
> docs/formatdomain.html.in | 27 ++++++++
> docs/schemas/domaincommon.rng | 25 +++++++
> src/conf/domain_conf.c | 81 ++++++++++++++++++++++
> src/conf/domain_conf.h | 5 ++
> .../qemuxml2argv-net-virtio-disable-offloads.xml | 32 +++++++++
> tests/qemuxml2xmltest.c | 1 +
> 6 files changed, 171 insertions(+)
> create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index a2ea758..5b2758a 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -3847,6 +3847,12 @@ qemu-kvm -net nic,model=? /dev/null
> <model type='virtio'/>
> <b><driver name='vhost' txmode='iothread'
ioeventfd='on' event_idx='off' queues='5'/></b>
> </interface>
> + <interface type='network'>
> + <source network='default'/>
> + <target dev='vnet2'/>
> + <model type='virtio'/>
> + <b><driver csum='off' gso='off'
guest_tso4='off' guest_tso6='off'
guest_ecn='off'/></b>
> + </interface>
Are we stuck with names with underscores in our XML? I'm still not sure
if we've come up with the best naming for exposing all these knobs.
I'd rather not mix underscores (event_idx) and other word separators in the
same element.
Alternatively, we could do something like:
<driver csum='off' gso='off'>
<guest tso4='off' tso6='off' ecn='off'/>
</driver>
to get rid of the multi-word attributes, but I like the underscores better
because they match QEMU arguments.
Jan
10:57 p.m.
New subject: [libvirt] [PATCH 4/5] conf: add options for disabling segment offloading
On Mon, Sep 15, 2014 at 04:30:46PM -0600, Eric Blake wrote:
On 09/11/2014 05:43 AM, Ján Tomko wrote:
> Add the following attributes:
> csum, gso, guest_tso4, guest_tso6, guest_ecn
> to the <driver> element of network interface
> which control the virtio-net device properties
> of the same names.
> ---
> docs/formatdomain.html.in | 27 ++++++++
> docs/schemas/domaincommon.rng | 25 +++++++
> src/conf/domain_conf.c | 81 ++++++++++++++++++++++
> src/conf/domain_conf.h | 5 ++
> .../qemuxml2argv-net-virtio-disable-offloads.xml | 32 +++++++++
> tests/qemuxml2xmltest.c | 1 +
> 6 files changed, 171 insertions(+)
> create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index a2ea758..5b2758a 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -3847,6 +3847,12 @@ qemu-kvm -net nic,model=? /dev/null
> <model type='virtio'/>
> <b><driver name='vhost' txmode='iothread'
ioeventfd='on' event_idx='off' queues='5'/></b>
> </interface>
> + <interface type='network'>
> + <source network='default'/>
> + <target dev='vnet2'/>
> + <model type='virtio'/>
> + <b><driver csum='off' gso='off'
guest_tso4='off' guest_tso6='off'
guest_ecn='off'/></b>
> + </interface>
Are we stuck with names with underscores in our XML? I'm still not sure
if we've come up with the best naming for exposing all these knobs.
I'm not really convinced having a 'guest_' prefix really buys
us anything here, since there's no naming clash to avoid. Why
don't we just kill the 'guest_' prefixes.
NB, remember that precisely matching QEMU naming is a non-goal,
we should be designing something that makes sense in general.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
11:22 p.m.
New subject: [libvirt] [PATCH 4/5] conf: add options for disabling segment offloading
On 09/17/2014 08:57 AM, Daniel P. Berrange wrote:
>> + <b><driver csum='off'
gso='off' guest_tso4='off' guest_tso6='off'
guest_ecn='off'/></b>
>> + </interface>
>
> Are we stuck with names with underscores in our XML? I'm still not sure
> if we've come up with the best naming for exposing all these knobs.
I'm not really convinced having a 'guest_' prefix really buys
us anything here, since there's no naming clash to avoid. Why
don't we just kill the 'guest_' prefixes.
NB, remember that precisely matching QEMU naming is a non-goal,
we should be designing something that makes sense in general.
I agree; I'd be fine with:
<driver csum='off' gso='off' tso4='off' tso6='off'
ecn='off'/>
with no need for a <guest> sub-structure. Yeah, we'll have to do some
glue logic to translate to qemu names, but that's what libvirt is for.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
11:36 p.m.
New subject: [libvirt] [PATCH 4/5] conf: add options for disabling segment offloading
On 09/17/2014 04:57 PM, Daniel P. Berrange wrote:
On Mon, Sep 15, 2014 at 04:30:46PM -0600, Eric Blake wrote:
> On 09/11/2014 05:43 AM, Ján Tomko wrote:
>> Add the following attributes:
>> csum, gso, guest_tso4, guest_tso6, guest_ecn
>> to the <driver> element of network interface
>> which control the virtio-net device properties
>> of the same names.
>> ---
>> docs/formatdomain.html.in | 27 ++++++++
>> docs/schemas/domaincommon.rng | 25 +++++++
>> src/conf/domain_conf.c | 81 ++++++++++++++++++++++
>> src/conf/domain_conf.h | 5 ++
>> .../qemuxml2argv-net-virtio-disable-offloads.xml | 32 +++++++++
>> tests/qemuxml2xmltest.c | 1 +
>> 6 files changed, 171 insertions(+)
>> create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index a2ea758..5b2758a 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -3847,6 +3847,12 @@ qemu-kvm -net nic,model=? /dev/null
>> <model type='virtio'/>
>> <b><driver name='vhost' txmode='iothread'
ioeventfd='on' event_idx='off' queues='5'/></b>
>> </interface>
>> + <interface type='network'>
>> + <source network='default'/>
>> + <target dev='vnet2'/>
>> + <model type='virtio'/>
>> + <b><driver csum='off' gso='off'
guest_tso4='off' guest_tso6='off'
guest_ecn='off'/></b>
>> + </interface>
>
> Are we stuck with names with underscores in our XML? I'm still not sure
> if we've come up with the best naming for exposing all these knobs.
I'm not really convinced having a 'guest_' prefix really buys
us anything here, since there's no naming clash to avoid. Why
don't we just kill the 'guest_' prefixes.
The clash is in the options I didn't expose:
http://git.qemu.org/?p=qemu.git;a=blob;f=include/hw/virtio/virtio-net.h;h...
because they weren't requested by the (private :() bug 1139364
Jan
11:52 p.m.
New subject: [libvirt] [PATCH 4/5] conf: add options for disabling segment offloading
On Wed, Sep 17, 2014 at 05:36:18PM +0200, Ján Tomko wrote:
On 09/17/2014 04:57 PM, Daniel P. Berrange wrote:
> On Mon, Sep 15, 2014 at 04:30:46PM -0600, Eric Blake wrote:
>> On 09/11/2014 05:43 AM, Ján Tomko wrote:
>>> Add the following attributes:
>>> csum, gso, guest_tso4, guest_tso6, guest_ecn
>>> to the <driver> element of network interface
>>> which control the virtio-net device properties
>>> of the same names.
>>> ---
>>> docs/formatdomain.html.in | 27 ++++++++
>>> docs/schemas/domaincommon.rng | 25 +++++++
>>> src/conf/domain_conf.c | 81
++++++++++++++++++++++
>>> src/conf/domain_conf.h | 5 ++
>>> .../qemuxml2argv-net-virtio-disable-offloads.xml | 32 +++++++++
>>> tests/qemuxml2xmltest.c | 1 +
>>> 6 files changed, 171 insertions(+)
>>> create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml
>>>
>>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>>> index a2ea758..5b2758a 100644
>>> --- a/docs/formatdomain.html.in
>>> +++ b/docs/formatdomain.html.in
>>> @@ -3847,6 +3847,12 @@ qemu-kvm -net nic,model=? /dev/null
>>> <model type='virtio'/>
>>> <b><driver name='vhost'
txmode='iothread' ioeventfd='on' event_idx='off'
queues='5'/></b>
>>> </interface>
>>> + <interface type='network'>
>>> + <source network='default'/>
>>> + <target dev='vnet2'/>
>>> + <model type='virtio'/>
>>> + <b><driver csum='off' gso='off'
guest_tso4='off' guest_tso6='off'
guest_ecn='off'/></b>
>>> + </interface>
>>
>> Are we stuck with names with underscores in our XML? I'm still not sure
>> if we've come up with the best naming for exposing all these knobs.
>
> I'm not really convinced having a 'guest_' prefix really buys
> us anything here, since there's no naming clash to avoid. Why
> don't we just kill the 'guest_' prefixes.
The clash is in the options I didn't expose:
http://git.qemu.org/?p=qemu.git;a=blob;f=include/hw/virtio/virtio-net.h;h...
because they weren't requested by the (private :() bug 1139364
Ah, so this is why you shouldn't take the precise solution requested in
a bug too literally, and instead look at the general picture :-)
So QEMU exposes alot of stuff:
$ qemu-kvm -device virtio-net,?
virtio-net-pci.ioeventfd=on/off
virtio-net-pci.vectors=uint32
virtio-net-pci.indirect_desc=on/off
virtio-net-pci.event_idx=on/off
virtio-net-pci.any_layout=on/off
virtio-net-pci.csum=on/off
virtio-net-pci.guest_csum=on/off
virtio-net-pci.gso=on/off
virtio-net-pci.guest_tso4=on/off
virtio-net-pci.guest_tso6=on/off
virtio-net-pci.guest_ecn=on/off
virtio-net-pci.guest_ufo=on/off
virtio-net-pci.host_tso4=on/off
virtio-net-pci.host_tso6=on/off
virtio-net-pci.host_ecn=on/off
virtio-net-pci.host_ufo=on/off
virtio-net-pci.mrg_rxbuf=on/off
virtio-net-pci.status=on/off
virtio-net-pci.ctrl_vq=on/off
virtio-net-pci.ctrl_rx=on/off
virtio-net-pci.ctrl_vlan=on/off
virtio-net-pci.ctrl_rx_extra=on/off
virtio-net-pci.ctrl_mac_addr=on/off
virtio-net-pci.ctrl_guest_offloads=on/off
virtio-net-pci.mq=on/off
virtio-net-pci.mac=macaddr
virtio-net-pci.vlan=vlan
virtio-net-pci.netdev=netdev
virtio-net-pci.bootindex=int32
virtio-net-pci.x-txtimer=uint32
virtio-net-pci.x-txburst=int32
virtio-net-pci.tx=str
virtio-net-pci.addr=pci-devfn
virtio-net-pci.romfile=str
virtio-net-pci.rombar=uint32
virtio-net-pci.multifunction=on/off
virtio-net-pci.command_serr_enable=on/off
Which to me indicates that Eric's suggestion for sub-elements is a
good idea. eg go for:
<driver>
<guest ..../>
<host ..../>
</driver>
and support the host bits too
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
11:56 p.m.
New subject: [libvirt] [PATCH 4/5] conf: add options for disabling segment offloading
On 09/17/2014 09:52 AM, Daniel P. Berrange wrote:
Ah, so this is why you shouldn't take the precise solution requested in
a bug too literally, and instead look at the general picture :-)
So QEMU exposes alot of stuff:
$ qemu-kvm -device virtio-net,?
virtio-net-pci.guest_tso4=on/off
virtio-net-pci.guest_tso6=on/off
virtio-net-pci.guest_ecn=on/off
virtio-net-pci.guest_ufo=on/off
virtio-net-pci.host_tso4=on/off
virtio-net-pci.host_tso6=on/off
virtio-net-pci.host_ecn=on/off
virtio-net-pci.host_ufo=on/off
Which to me indicates that Eric's suggestion for sub-elements is a
Wasn't my idea, but Jan's.
good idea. eg go for:
<driver>
<guest ..../>
<host ..../>
</driver>
and support the host bits too
but yes, I could live with that, now that I'm seeing how many more
options there are to be exposed.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
7:43 p.m.
New subject: [libvirt] [PATCH 5/5] qemu: wire up virtio-net segment offloading options
---
src/qemu/qemu_command.c | 20 ++++++++++++++++++++
.../qemuxml2argv-net-virtio-disable-offloads.args | 8 ++++++++
tests/qemuxml2argvtest.c | 2 ++
3 files changed, 30 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.args
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 665a590..0b88a48 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4430,6 +4430,26 @@ qemuBuildNicDevStr(virDomainDefPtr def,
virBufferAsprintf(&buf, ",event_idx=%s",
virTristateSwitchTypeToString(net->driver.virtio.event_idx));
}
+ if (net->driver.virtio.csum) {
+ virBufferAsprintf(&buf, ",csum=%s",
+
virTristateSwitchTypeToString(net->driver.virtio.csum));
+ }
+ if (net->driver.virtio.gso) {
+ virBufferAsprintf(&buf, ",gso=%s",
+ virTristateSwitchTypeToString(net->driver.virtio.gso));
+ }
+ if (net->driver.virtio.guest_tso4) {
+ virBufferAsprintf(&buf, ",guest_tso4=%s",
+
virTristateSwitchTypeToString(net->driver.virtio.guest_tso4));
+ }
+ if (net->driver.virtio.guest_tso6) {
+ virBufferAsprintf(&buf, ",guest_tso6=%s",
+
virTristateSwitchTypeToString(net->driver.virtio.guest_tso6));
+ }
+ if (net->driver.virtio.guest_ecn) {
+ virBufferAsprintf(&buf, ",guest_ecn=%s",
+
virTristateSwitchTypeToString(net->driver.virtio.guest_ecn));
+ }
}
if (usingVirtio && vhostfdSize > 1) {
/* As advised at http://www.linux-kvm.org/page/Multiqueue
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.args
b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.args
new file mode 100644
index 0000000..49e8bf7
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.args
@@ -0,0 +1,8 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \
+-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \
+-hda /dev/HostVG/QEMUGuest7 \
+-device virtio-net-pci,csum=off,gso=off,guest_tso4=off,guest_tso6=off\
+,guest_ecn=off,vlan=0,id=net0,mac=00:22:44:66:88:aa,bus=pci.0,addr=0x3 \
+-net user,vlan=0,name=hostnet0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 5c28253..9da1214 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -959,6 +959,8 @@ mymain(void)
DO_TEST("net-virtio", NONE);
DO_TEST("net-virtio-device",
QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_VIRTIO_TX_ALG);
+ DO_TEST("net-virtio-disable-offloads",
+ QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
DO_TEST("net-virtio-netdev",
QEMU_CAPS_DEVICE, QEMU_CAPS_NETDEV, QEMU_CAPS_NODEFCONFIG);
DO_TEST("net-virtio-s390",
--
1.8.5.5
6:25 a.m.
New subject: [libvirt] [PATCH 5/5] qemu: wire up virtio-net segment offloading options
On 09/11/2014 07:43 AM, Ján Tomko wrote:
---
src/qemu/qemu_command.c | 20 ++++++++++++++++++++
.../qemuxml2argv-net-virtio-disable-offloads.args | 8 ++++++++
tests/qemuxml2argvtest.c | 2 ++
3 files changed, 30 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.args
Kind of light on the commit message...
ACK
John
3697
days inactive
3703
days old
19 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Eric Blake -
John Ferlan -
Ján Tomko