[libvirt PATCH v2] Adds e1000e/vmxnet3 Vnet_hdr suuport
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@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); +} + /** * 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); -- 2.26.2
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@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); +}
^ 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@gmail.com>
+
/** * 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);
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 <patmagauran.j@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 <laine@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!
participants (3)
-
Daniel Henrique Barboza -
Laine Stump -
Patrick Magauran