On 8/8/20 9: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).
The above paragraph is a single line (probably not noticed because your
editor "flows" it into multiple lines?). Our formatting guidelines say
that it should be in multiple lines with the margin set at 72 (makes
displaying "git log" in an 80 char. wide terminal window more palatable).
And keeping in line with the obsessive-compulsive urge to locate exact
commits: The QEMU upstream commit first adding vmxnet3 support was
786fd2b0f87, and I verified it does mention vnet_hdr. The QEMU upstream
commit for e1000e was 6f3fbe4ed06a, and it also checks for vnet_hdr. I'm
going to add a mention to these in the commit log, for benefit of others
who suffer from OCD :-)
This passes the whole packet to the host, reducing emulation overhead and
improving performance.
Signed-off-by: Patrick Magauran <patmagaura...@gmail.com>
---
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);
+}
+
Since this function is never called from anywhere else, it should just
be made static, and not mentioned in qemu_interface.h.
(also, speaking of OCD... Newly added functions should have 2 blank
lines before and after the function, and the return type should be on a
separate line from the function name. Don't ask me why, I'm just the
enforcer :-))
/**
* 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);
Reviewed-by: Laine Stump <la...@redhat.com>
I made the cosmetic changes I noted above (plus those noticed by
Daniel), and pushed the patch. Thanks for your contribution, and
congratulations on your first libvirt patch!
