On 09/24/2014 01:24 AM, John Ferlan wrote: > > > On 09/18/2014 10:15 AM, Ján Tomko wrote: >> Add options for tuning segment offloading: >> <driver> >> <host csum='off' gso='off' tso4='off' tso6='off' >> ecn='off' ufo='off'/> >> <guest csum='off' tso4='off' tso6='off' ecn='off' ufo='off'/> >> </driver> >> which control the respective host_ and guest_ properties >> of the virtio-net device. >> --- >> docs/formatdomain.html.in | 24 ++- >> docs/schemas/domaincommon.rng | 44 ++++- >> src/conf/domain_conf.c | 218 ++++++++++++++++++++- >> src/conf/domain_conf.h | 15 ++ >> .../qemuxml2argv-net-virtio-disable-offloads.xml | 35 ++++ >> tests/qemuxml2xmltest.c | 1 + >> 6 files changed, 329 insertions(+), 8 deletions(-) >> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml >> >> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in >> index 8c03ebb..5dd70a4 100644 >> --- a/docs/formatdomain.html.in >> +++ b/docs/formatdomain.html.in >> @@ -3868,7 +3868,11 @@ qemu-kvm -net nic,model=? /dev/null >> <source network='default'/> >> <target dev='vnet1'/> >> <model type='virtio'/> >> - <b><driver name='vhost' txmode='iothread' ioeventfd='on' event_idx='off' queues='5'/></b> >> + <b><driver name='vhost' txmode='iothread' ioeventfd='on' event_idx='off' queues='5'> >> + <host csum='off' gso='off' tso4='off' tso6='off' ecn='off' ufo='off'/> >> + <guest csum='off' tso4='off' tso6='off' ecn='off' ufo='off'/> >> + </driver> >> + </b> >> </interface> >> </devices> >> ...</pre> >> @@ -3972,6 +3976,24 @@ 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>host offloading options</code></dt> > > Should this be <code>host</host> offloading options? > or host TCP options (in some way to indicate that these > are TCP level options). Probably also want your disclaimer > use of these should be for only those that know what they > are doing... > Well ufo is an UDP option. > >> + <dd> >> + The <code>csum</code>, <code>gso</code>, <code>tso4</code>, >> + <code>tso6</code>, <code>ecn</code>, <code>ufo</code> > > Should read: ecn, and ufo > > Should you "spell out" what each translates to? > > csum (checksums), gso (generic segmentation offload), > tso (tcp segmentation offload v4 and v6), ecn (congestion > notification), and ufo (UDP fragment offloads) > I thought not spelling them out was the equivalent of the "only use this if you know what you're doing" disclaimer. > > >> + attributes with possible values <code>on</code> >> + and <code>off</code> can be used to turn host offloads off. > > s/turn host offloads off/turn off host TCP options/ > > Saying "offloads off" aloud just doesn't seem right. > >> + By default, the supported offloads are enabled by QEMU. > > s/the supported offloads/the TCP options/ > >> + <span class="since">Since 1.2.9 (QEMU only)</span> >> + </dd> >> + <dt>guest offloading options</dt> > > Similar in here... Does the host setting override the guest value > or can the host say "tso4=off" while the guest has "tso4=on"? Curious > mainly. It can say that, but I doubt it'll work. > >> + <dd> >> + The <code>csum</code>, <code>tso4</code>, >> + <code>tso6</code>, <code>ecn</code>, <code>ufo</code> > > same here with the "and" - although I suppose you could just > reference the <host> bits "above"... > > Another way to say it is guests can use the same options except gso > >> + attributes with possible values <code>on</code> >> + and <code>off</code> can be used to turn guest offloads off. > > s/turn guest offloads off/turn off guest TCP options/ How about 'turn off guest offload options'? > >> + By default, the supported offloads are enabled by QEMU. > > s/the supported offloads/the TCP options/ > > And of course the usage disclaimer! > > >> + <span class="since">Since 1.2.9 (QEMU only)</span> >> + </dd> >> </dl> >> >> <h5><a name="elementsBackendOptions">Setting network backend-specific options</a></h5> >> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >> index 3ccec1c..cdaafa6 100644 >> --- a/src/conf/domain_conf.c >> +++ b/src/conf/domain_conf.c >> @@ -6897,6 +6897,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, >> char *ioeventfd = NULL; >> char *event_idx = NULL; >> char *queues = NULL; >> + char *str = NULL; >> char *filter = NULL; >> char *internal = NULL; >> char *devaddr = NULL; >> @@ -7385,6 +7386,115 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, >> } >> def->driver.virtio.queues = q; >> } > > How about something like this? Not that you have anything wrong, but > it avoids the chance that some "change" in the future requires 11 similar > changes... I was worried it wouldn't be clear enough and since we use similar repetition all over domain_conf, it would be better handled separately. Jan
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list