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

Re: [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
>        &lt;model type='virtio'/&gt;
>        <b>&lt;driver name='vhost' txmode='iothread' ioeventfd='on' event_idx='off' queues='5'/&gt;</b>
>      &lt;/interface&gt;
> +    &lt;interface type='network'&gt;
> +      &lt;source network='default'/&gt;
> +      &lt;target dev='vnet2'/&gt;
> +      &lt;model type='virtio'/&gt;
> +      <b>&lt;driver csum='off' gso='off' guest_tso4='off' guest_tso6='off' guest_ecn='off'/&gt;</b>
> +    &lt;/interface&gt;

This doesn't require a driver name='' value?

>    &lt;/devices&gt;
>    ...</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");
> 


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