[libvirt PATCH] Adds e1000e/vmxnet3 Vnet_hdr suuport

Patrick Magauran posted 1 patch 3 years, 7 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200807025332.31851-1-patmagauran.j@gmail.com
There is a newer version of this series
src/conf/domain_conf.c    | 8 +++++++-
src/conf/domain_conf.h    | 1 +
src/libvirt_private.syms  | 1 +
src/qemu/qemu_interface.c | 8 ++++----
4 files changed, 13 insertions(+), 5 deletions(-)
[libvirt PATCH] Adds e1000e/vmxnet3 Vnet_hdr suuport
Posted by Patrick Magauran 3 years, 7 months ago
Libvirt bases its decision about whether to apply the vnet_hdr flag to the tap interface on whether or not the selected model is VirtIO. Originally, VirtIO was the only model to support the vnet_hdr in QEMU; however, the e1000e & vmxnet3 adapters also support it(seemingly from introduction based on commits). This passes the whole packet to the host, reducing emulation overhead and improving performance.

Signed-off-by: Patrick Magauran <patmagauran.j@gmail.com>
---
 src/conf/domain_conf.c    | 8 +++++++-
 src/conf/domain_conf.h    | 1 +
 src/libvirt_private.syms  | 1 +
 src/qemu/qemu_interface.c | 8 ++++----
 4 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 69e0439e7e..cb184110f7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30981,7 +30981,13 @@ virDomainNetIsVirtioModel(const virDomainNetDef *net)
             net->model == VIR_DOMAIN_NET_MODEL_VIRTIO_TRANSITIONAL ||
             net->model == VIR_DOMAIN_NET_MODEL_VIRTIO_NON_TRANSITIONAL);
 }
-
+bool
+virDomainNetIsVnetCompatModel(const virDomainNetDef *net)
+{
+    return (virDomainNetIsVirtioModel(net) ||
+            net->model == VIR_DOMAIN_NET_MODEL_E1000E ||
+            net->model == VIR_DOMAIN_NET_MODEL_VMXNET3);
+}
 
 /* Return listens[i] from the appropriate union for the graphics
  * type, or NULL if this is an unsuitable type, or the index is out of
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 011bf66cb4..cbc46fdf78 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3371,6 +3371,7 @@ const char *virDomainNetGetModelString(const virDomainNetDef *net);
 int virDomainNetSetModelString(virDomainNetDefPtr et,
                                const char *model);
 bool virDomainNetIsVirtioModel(const virDomainNetDef *net);
+bool virDomainNetIsVnetCompatModel(const virDomainNetDef *net);
 int virDomainNetAppendIPAddress(virDomainNetDefPtr def,
                                 const char *address,
                                 int family,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 01c2e710cd..2b64042dd2 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -525,6 +525,7 @@ virDomainNetGetActualVlan;
 virDomainNetGetModelString;
 virDomainNetInsert;
 virDomainNetIsVirtioModel;
+virDomainNetIsVnetCompatModel;
 virDomainNetModelTypeFromString;
 virDomainNetModelTypeToString;
 virDomainNetNotifyActualDevice;
diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
index ffec992596..8397ed8645 100644
--- a/src/qemu/qemu_interface.c
+++ b/src/qemu/qemu_interface.c
@@ -255,7 +255,7 @@ qemuInterfaceDirectConnect(virDomainDefPtr def,
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
     unsigned int macvlan_create_flags = VIR_NETDEV_MACVLAN_CREATE_WITH_TAP;
 
-    if (virDomainNetIsVirtioModel(net))
+    if (virDomainNetIsVnetCompatModel(net))
         macvlan_create_flags |= VIR_NETDEV_MACVLAN_VNET_HDR;
 
     if (virNetDevMacVLanCreateWithVPortProfile(net->ifname,
@@ -417,7 +417,7 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def,
         }
     }
 
-    if (virDomainNetIsVirtioModel(net))
+    if (virDomainNetIsVnetCompatModel(net))
         tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR;
 
     if (net->managed_tap == VIR_TRISTATE_BOOL_NO) {
@@ -436,7 +436,7 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def,
             if (virNetDevMacVLanTapOpen(net->ifname, tapfd, tapfdSize) < 0)
                 goto cleanup;
             if (virNetDevMacVLanTapSetup(tapfd, tapfdSize,
-                                         virDomainNetIsVirtioModel(net)) < 0) {
+                                         virDomainNetIsVnetCompatModel(net)) < 0) {
                 goto cleanup;
             }
         } else {
@@ -559,7 +559,7 @@ qemuInterfaceBridgeConnect(virDomainDefPtr def,
         template_ifname = true;
     }
 
-    if (virDomainNetIsVirtioModel(net))
+    if (virDomainNetIsVnetCompatModel(net))
         tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR;
 
     if (driver->privileged) {
-- 
2.26.2

Re: [libvirt PATCH] Adds e1000e/vmxnet3 Vnet_hdr suuport
Posted by Daniel P. Berrangé 3 years, 7 months ago
On Thu, Aug 06, 2020 at 10:53:32PM -0400, Patrick Magauran wrote:
> Libvirt bases its decision about whether to apply the vnet_hdr flag to the tap interface on whether or not the selected model is VirtIO. Originally, VirtIO was the only model to support the vnet_hdr in QEMU; however, the e1000e & vmxnet3 adapters also support it(seemingly from introduction based on commits). This passes the whole packet to the host, reducing emulation overhead and improving performance.
> 
> Signed-off-by: Patrick Magauran <patmagauran.j@gmail.com>
> ---
>  src/conf/domain_conf.c    | 8 +++++++-
>  src/conf/domain_conf.h    | 1 +
>  src/libvirt_private.syms  | 1 +
>  src/qemu/qemu_interface.c | 8 ++++----
>  4 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 69e0439e7e..cb184110f7 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -30981,7 +30981,13 @@ virDomainNetIsVirtioModel(const virDomainNetDef *net)
>              net->model == VIR_DOMAIN_NET_MODEL_VIRTIO_TRANSITIONAL ||
>              net->model == VIR_DOMAIN_NET_MODEL_VIRTIO_NON_TRANSITIONAL);
>  }
> -
> +bool
> +virDomainNetIsVnetCompatModel(const virDomainNetDef *net)
> +{
> +    return (virDomainNetIsVirtioModel(net) ||
> +            net->model == VIR_DOMAIN_NET_MODEL_E1000E ||
> +            net->model == VIR_DOMAIN_NET_MODEL_VMXNET3);
> +}

Should really live in the qemu_inteface.c file,  with
a qemuOnterfaceIsVnetCompatModel name, since the decision
is inherantly tied to QEMU, and inappropriate for other
virt drivers.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [libvirt PATCH] Adds e1000e/vmxnet3 Vnet_hdr suuport
Posted by Patrick J. Magauran 3 years, 7 months ago
On Fri, 2020-08-07 at 17:16 +0100, Daniel P. Berrangé wrote:
> On Thu, Aug 06, 2020 at 10:53:32PM -0400, Patrick Magauran wrote:
> > Libvirt bases its decision about whether to apply the vnet_hdr flag
> > to the tap interface on whether or not the selected model is
> > VirtIO. Originally, VirtIO was the only model to support the
> > vnet_hdr in QEMU; however, the e1000e & vmxnet3 adapters also
> > support it(seemingly from introduction based on commits). This
> > passes the whole packet to the host, reducing emulation overhead
> > and improving performance.
> > 
> > Signed-off-by: Patrick Magauran <patmagauran.j@gmail.com>
> > ---
> >  src/conf/domain_conf.c    | 8 +++++++-
> >  src/conf/domain_conf.h    | 1 +
> >  src/libvirt_private.syms  | 1 +
> >  src/qemu/qemu_interface.c | 8 ++++----
> >  4 files changed, 13 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 69e0439e7e..cb184110f7 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -30981,7 +30981,13 @@ virDomainNetIsVirtioModel(const
> > virDomainNetDef *net)
> >              net->model == VIR_DOMAIN_NET_MODEL_VIRTIO_TRANSITIONAL
> > ||
> >              net->model ==
> > VIR_DOMAIN_NET_MODEL_VIRTIO_NON_TRANSITIONAL);
> >  }
> > -
> > +bool
> > +virDomainNetIsVnetCompatModel(const virDomainNetDef *net)
> > +{
> > +    return (virDomainNetIsVirtioModel(net) ||
> > +            net->model == VIR_DOMAIN_NET_MODEL_E1000E ||
> > +            net->model == VIR_DOMAIN_NET_MODEL_VMXNET3);
> > +}
> 
> Should really live in the qemu_inteface.c file,  with
> a qemuOnterfaceIsVnetCompatModel name, since the decision
> is inherantly tied to QEMU, and inappropriate for other
> virt drivers.
> 
> 
> Regards,
> Daniel

I would think the qemu_interface.c file is more for the actual setup
functionality rather than simple helper functions. Would the
qemu_domain.c file make for sense?

--Patrick

Re: [libvirt PATCH] Adds e1000e/vmxnet3 Vnet_hdr suuport
Posted by Laine Stump 3 years, 7 months ago
On 8/7/20 3:46 PM, Patrick J. Magauran wrote:
> On Fri, 2020-08-07 at 17:16 +0100, Daniel P. Berrangé wrote:
>> On Thu, Aug 06, 2020 at 10:53:32PM -0400, Patrick Magauran wrote:
>>> Libvirt bases its decision about whether to apply the vnet_hdr flag
>>> to the tap interface on whether or not the selected model is
>>> VirtIO. Originally, VirtIO was the only model to support the
>>> vnet_hdr in QEMU; however, the e1000e & vmxnet3 adapters also
>>> support it(seemingly from introduction based on commits). This
>>> passes the whole packet to the host, reducing emulation overhead
>>> and improving performance.
>>>
>>> Signed-off-by: Patrick Magauran <patmagauran.j@gmail.com>
>>> ---
>>>   src/conf/domain_conf.c    | 8 +++++++-
>>>   src/conf/domain_conf.h    | 1 +
>>>   src/libvirt_private.syms  | 1 +
>>>   src/qemu/qemu_interface.c | 8 ++++----
>>>   4 files changed, 13 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>>> index 69e0439e7e..cb184110f7 100644
>>> --- a/src/conf/domain_conf.c
>>> +++ b/src/conf/domain_conf.c
>>> @@ -30981,7 +30981,13 @@ virDomainNetIsVirtioModel(const
>>> virDomainNetDef *net)
>>>               net->model == VIR_DOMAIN_NET_MODEL_VIRTIO_TRANSITIONAL
>>> ||
>>>               net->model ==
>>> VIR_DOMAIN_NET_MODEL_VIRTIO_NON_TRANSITIONAL);
>>>   }
>>> -
>>> +bool
>>> +virDomainNetIsVnetCompatModel(const virDomainNetDef *net)
>>> +{
>>> +    return (virDomainNetIsVirtioModel(net) ||
>>> +            net->model == VIR_DOMAIN_NET_MODEL_E1000E ||
>>> +            net->model == VIR_DOMAIN_NET_MODEL_VMXNET3);
>>> +}
>> Should really live in the qemu_inteface.c file,  with
>> a qemuOnterfaceIsVnetCompatModel name, since the decision
>> is inherantly tied to QEMU, and inappropriate for other
>> virt drivers.
>>
>>
>> Regards,
>> Daniel
> I would think the qemu_interface.c file is more for the actual setup
> functionality rather than simple helper functions. Would the
> qemu_domain.c file make for sense?
>

If it's network interface specific and also qemu specific, then it 
should go into qemu_interface.c. That file didn't use to exist (all the 
functions there were just included in qemu_command.c) but was created as 
a place to split out all the interface-specific stuff (not just the 
toplevel "setup this kind of interface" functions).