Re: [libvirt PATCH v2] Adds e1000e/vmxnet3 Vnet_hdr suuport

[Daniel Henrique Barboza <danielhb413@xxxxxxxxx>]

Be mindful of the typo in the commit title: suuport -> support

Just a nit below:

On 8/8/20 10:59 AM, Patrick Magauran wrote:
> Changes from Original:
> Moved Comparison to qemuInterfaceIsVnetCompatModel in qemu_interface.c per the recommendation of Daniel Berrangé
>
> -----
> 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@xxxxxxxxx>
> ---
>   src/qemu/qemu_interface.c | 15 +++++++++++----
>   src/qemu/qemu_interface.h |  1 +
>   2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
> index ffec992596..229bb299aa 100644
> --- a/src/qemu/qemu_interface.c
> +++ b/src/qemu/qemu_interface.c
> @@ -230,6 +230,13 @@ qemuInterfaceStopDevices(virDomainDefPtr def)
>       return 0;
>   }
>   
> +bool qemuInterfaceIsVnetCompatModel(const virDomainNetDef *net)
> +{
> +    return (virDomainNetIsVirtioModel(net) ||
> +           net->model == VIR_DOMAIN_NET_MODEL_E1000E ||
> +           net->model == VIR_DOMAIN_NET_MODEL_VMXNET3);
> +}


^ indentation here is misaligned by a few spaces:

    return (virDomainNetIsVirtioModel(net) ||
            net->model == VIR_DOMAIN_NET_MODEL_E1000E ||
            net->model == VIR_DOMAIN_NET_MODEL_VMXNET3);


Rest of the code LGTM. I'm CC'ing Laine because I don't have authority to
decide if what you're doing here is enough or more changes are required,
and network is Laine's turf. As far as I understand:


Reviewed-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>


> +
>   
>   /**
>    * qemuInterfaceDirectConnect:
> @@ -255,7 +262,7 @@ qemuInterfaceDirectConnect(virDomainDefPtr def,
>       virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>       unsigned int macvlan_create_flags = VIR_NETDEV_MACVLAN_CREATE_WITH_TAP;
>   
> -    if (virDomainNetIsVirtioModel(net))
> +    if (qemuInterfaceIsVnetCompatModel(net))
>           macvlan_create_flags |= VIR_NETDEV_MACVLAN_VNET_HDR;
>   
>       if (virNetDevMacVLanCreateWithVPortProfile(net->ifname,
> @@ -417,7 +424,7 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def,
>           }
>       }
>   
> -    if (virDomainNetIsVirtioModel(net))
> +    if (qemuInterfaceIsVnetCompatModel(net))
>           tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR;
>   
>       if (net->managed_tap == VIR_TRISTATE_BOOL_NO) {
> @@ -436,7 +443,7 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def,
>               if (virNetDevMacVLanTapOpen(net->ifname, tapfd, tapfdSize) < 0)
>                   goto cleanup;
>               if (virNetDevMacVLanTapSetup(tapfd, tapfdSize,
> -                                         virDomainNetIsVirtioModel(net)) < 0) {
> +                                         qemuInterfaceIsVnetCompatModel(net)) < 0) {
>                   goto cleanup;
>               }
>           } else {
> @@ -559,7 +566,7 @@ qemuInterfaceBridgeConnect(virDomainDefPtr def,
>           template_ifname = true;
>       }
>   
> -    if (virDomainNetIsVirtioModel(net))
> +    if (qemuInterfaceIsVnetCompatModel(net))
>           tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR;
>   
>       if (driver->privileged) {
> diff --git a/src/qemu/qemu_interface.h b/src/qemu/qemu_interface.h
> index 0464b903d7..9e3f61e8e0 100644
> --- a/src/qemu/qemu_interface.h
> +++ b/src/qemu/qemu_interface.h
> @@ -58,3 +58,4 @@ int qemuInterfaceOpenVhostNet(virDomainDefPtr def,
>   
>   qemuSlirpPtr qemuInterfacePrepareSlirp(virQEMUDriverPtr driver,
>                                          virDomainNetDefPtr net);
> +bool qemuInterfaceIsVnetCompatModel(const virDomainNetDef *net);