On 8/2/24 00:25, Praveen K Paladugu wrote:
From: Praveen K Paladugu <praveenkpaladugu@gmail.com>
From: Praveen K Paladugu <prapal@linux.microsoft.com>
Move methods to connect domain interfaces to host bridges to hypervisor. This is to allow reuse between qemu and ch drivers.
Signed-off-by: Praveen K Paladugu <praveenkpaladugu@gmail.com> Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com> --- src/hypervisor/domain_interface.c | 228 ++++++++++++++++++++++++++++++ src/hypervisor/domain_interface.h | 10 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 9 +- src/qemu/qemu_interface.c | 219 +--------------------------- 5 files changed, 247 insertions(+), 220 deletions(-)
diff --git a/src/hypervisor/domain_interface.c b/src/hypervisor/domain_interface.c index 756abb08e9..0c19ff859a 100644 --- a/src/hypervisor/domain_interface.c +++ b/src/hypervisor/domain_interface.c @@ -39,6 +39,7 @@ #include "virnetdevmidonet.h" #include "virnetdevopenvswitch.h" #include "virnetdevtap.h" +#include "vircommand.h"
#define VIR_FROM_THIS VIR_FROM_DOMAIN @@ -514,3 +515,230 @@ virDomainClearNetBandwidth(virDomainDef *def) virDomainInterfaceClearQoS(def, def->nets[i]); } } + +/** + * virDomainCreateInBridgePortWithHelper: + * @bridgeHelperName: name of the bridge helper program + * @brname: the bridge name + * @ifname: the returned interface name + * @tapfd: file descriptor return value for the new tap device + * @flags: OR of virNetDevTapCreateFlags: + + * VIR_NETDEV_TAP_CREATE_VNET_HDR + * - Enable IFF_VNET_HDR on the tap device + * + * This function creates a new tap device on a bridge using an external + * helper. The final name for the bridge will be stored in @ifname. + * + * Returns 0 in case of success or -1 on failure + */ +static int +virDomainCreateInBridgePortWithHelper(const char *bridgeHelperName, + const char *brname, + char **ifname, + int *tapfd, + unsigned int flags) +{ + const char *const bridgeHelperDirs[] = { + "/usr/libexec", + "/usr/lib/qemu", + "/usr/lib", + NULL, + }; + g_autoptr(virCommand) cmd = NULL; + g_autofree char *bridgeHelperPath = NULL; + char *errbuf = NULL, *cmdstr = NULL; + int pair[2] = { -1, -1 }; + + if (!bridgeHelperName) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing bridge helper name")); + return -1; + } + + if ((flags & ~VIR_NETDEV_TAP_CREATE_VNET_HDR) != VIR_NETDEV_TAP_CREATE_IFUP) + return -1; + + if (socketpair(AF_UNIX, SOCK_STREAM, 0, pair) < 0) { + virReportSystemError(errno, "%s", _("failed to create socket")); + return -1; + } + + bridgeHelperPath = virFindFileInPathFull(bridgeHelperName, bridgeHelperDirs); + + if (!bridgeHelperPath) { + virReportSystemError(errno, _("'%1$s' is not a suitable bridge helper"), + bridgeHelperName); + return -1; + } + + VIR_DEBUG("Using qemu-bridge-helper: %s", bridgeHelperPath); + cmd = virCommandNew(bridgeHelperPath); + if (flags & VIR_NETDEV_TAP_CREATE_VNET_HDR) + virCommandAddArgFormat(cmd, "--use-vnet"); + virCommandAddArgFormat(cmd, "--br=%s", brname); + virCommandAddArgFormat(cmd, "--fd=%d", pair[1]); + virCommandSetErrorBuffer(cmd, &errbuf); + virCommandDoAsyncIO(cmd); + virCommandPassFD(cmd, pair[1], + VIR_COMMAND_PASS_FD_CLOSE_PARENT); + virCommandClearCaps(cmd); +#ifdef CAP_NET_ADMIN + virCommandAllowCap(cmd, CAP_NET_ADMIN); +#endif + if (virCommandRunAsync(cmd, NULL) < 0) { + *tapfd = -1; + goto cleanup; + } + + do { + *tapfd = virSocketRecvFD(pair[0], 0); + } while (*tapfd < 0 && errno == EINTR); + + if (*tapfd < 0) { + char *errstr = NULL; + + if (!(cmdstr = virCommandToString(cmd, false))) + goto cleanup; + virCommandAbort(cmd); + + if (errbuf && *errbuf) + errstr = g_strdup_printf("stderr=%s", errbuf); + + virReportSystemError(errno, + _("%1$s: failed to communicate with bridge helper: %2$s"), + cmdstr, + NULLSTR_EMPTY(errstr)); + VIR_FREE(errstr); + goto cleanup; + } + + if (virNetDevTapGetName(*tapfd, ifname) < 0 || + virCommandWait(cmd, NULL) < 0) { + VIR_FORCE_CLOSE(*tapfd); + *tapfd = -1; + } + + cleanup: + VIR_FREE(cmdstr); + VIR_FREE(errbuf); + VIR_FORCE_CLOSE(pair[0]); + return *tapfd < 0 ? -1 : 0; +} + + +/* virDomainInterfaceBridgeConnect: + * @def: the definition of the VM + * @net: pointer to the VM's interface description + * @tapfd: array of file descriptor return value for the new device + * @tapfdsize: number of file descriptors in @tapfd + * @privileged: whether running as privileged user + * @ebtables: ebtales context + * @macFilter: whether driver support mac Filtering + * @bridgeHelperName:name of the bridge helper program to run in non-privileged mode + * + * Called *only* called if actualType is VIR_DOMAIN_NET_TYPE_NETWORK or + * VIR_DOMAIN_NET_TYPE_BRIDGE (i.e. if the connection is made with a tap + * device connecting to a bridge device) + */ +int +virDomainInterfaceBridgeConnect(virDomainDef *def, + virDomainNetDef *net, + int *tapfd, + size_t tapfdSize,
Since this ^^ is not passed as a pointer, then ...
+ bool privileged, + ebtablesContext *ebtables, + bool macFilter, + const char *bridgeHelperName) +{ + const char *brname; + int ret = -1; + unsigned int tap_create_flags = VIR_NETDEV_TAP_CREATE_IFUP; + bool template_ifname = false; + const char *tunpath = "/dev/net/tun"; + + if (net->backend.tap) { + tunpath = net->backend.tap; + if (!privileged) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("cannot use custom tap device in session mode")); + goto cleanup; + } + } + + if (!(brname = virDomainNetGetActualBridgeName(net))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing bridge name")); + goto cleanup; + } + + if (!net->ifname) + template_ifname = true; + + if (virDomainInterfaceIsVnetCompatModel(net)) + tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR; + + if (privileged) { + if (virNetDevTapCreateInBridgePort(brname, &net->ifname, &net->mac, + def->uuid, tunpath, tapfd, tapfdSize, + virDomainNetGetActualVirtPortProfile(net), + virDomainNetGetActualVlan(net), + virDomainNetGetActualPortOptionsIsolated(net), + net->coalesce, 0, NULL, + tap_create_flags) < 0) { + virDomainAuditNetDevice(def, net, tunpath, false); + goto cleanup; + } + if (virDomainNetGetActualBridgeMACTableManager(net) + == VIR_NETWORK_BRIDGE_MAC_TABLE_MANAGER_LIBVIRT) { + /* libvirt is managing the FDB of the bridge this device + * is attaching to, so we need to turn off learning and + * unicast_flood on the device to prevent the kernel from + * adding any FDB entries for it. We will add an fdb + * entry ourselves (during virDomainInterfaceStartDevices(), + * using the MAC address from the interface config. + */ + if (virNetDevBridgePortSetLearning(brname, net->ifname, false) < 0) + goto cleanup; + if (virNetDevBridgePortSetUnicastFlood(brname, net->ifname, false) < 0) + goto cleanup; + } + } else { + if (virDomainCreateInBridgePortWithHelper(bridgeHelperName, brname, + &net->ifname, + tapfd, + tap_create_flags) < 0) { + virDomainAuditNetDevice(def, net, tunpath, false); + goto cleanup; + } + /* virDomainCreateInBridgePortWithHelper can only create a single FD */ + if (tapfdSize > 1) { + VIR_WARN("Ignoring multiqueue network request"); + tapfdSize = 1;
... this has not the desired effect.
+ } + } + + virDomainAuditNetDevice(def, net, tunpath, true); + + if (macFilter && + ebtablesAddForwardAllowIn(ebtables, + net->ifname, + &net->mac) < 0) + goto cleanup; + + if (net->filter && + virDomainConfNWFilterInstantiate(def->name, def->uuid, net, false) < 0) { + goto cleanup; + } + + ret = 0; + + cleanup: + if (ret < 0) { + size_t i; + for (i = 0; i < tapfdSize && tapfd[i] >= 0; i++) + VIR_FORCE_CLOSE(tapfd[i]); + if (template_ifname) + VIR_FREE(net->ifname); + } + + return ret; +} diff --git a/src/hypervisor/domain_interface.h b/src/hypervisor/domain_interface.h index 572b4dd8c5..6d06fe2499 100644 --- a/src/hypervisor/domain_interface.h +++ b/src/hypervisor/domain_interface.h @@ -48,3 +48,13 @@ int virDomainInterfaceClearQoS(virDomainDef *def, virDomainNetDef *net); void virDomainClearNetBandwidth(virDomainDef *def) ATTRIBUTE_NONNULL(1); + +int virDomainInterfaceBridgeConnect(virDomainDef *def, + virDomainNetDef *net, + int *tapfd, + size_t tapfdSize, + bool privileged, + ebtablesContext *ebtables, + bool macFilter, + const char *bridgeHelperName) + ATTRIBUTE_NONNULL(2) G_NO_INLINE; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d15d6a6a9d..8132a17c28 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1646,6 +1646,7 @@ virDomainDriverSetupPersistentDefBlkioParams;
# hypervisor/domain_interface.h virDomainClearNetBandwidth; +virDomainInterfaceBridgeConnect; virDomainInterfaceClearQoS; virDomainInterfaceDeleteDevice; virDomainInterfaceEthernetConnect; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f15e6bda1e..0d22bebe89 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8626,6 +8626,7 @@ qemuBuildInterfaceConnect(virDomainObj *vm, bool vhostfd = false; /* also used to signal processing of tapfds */ size_t tapfdSize = net->driver.virtio.queues; g_autofree int *tapfd = g_new0(int, tapfdSize + 1); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver);
memset(tapfd, -1, (tapfdSize + 1) * sizeof(*tapfd));
@@ -8636,8 +8637,12 @@ qemuBuildInterfaceConnect(virDomainObj *vm, case VIR_DOMAIN_NET_TYPE_NETWORK: case VIR_DOMAIN_NET_TYPE_BRIDGE: vhostfd = true; - if (qemuInterfaceBridgeConnect(vm->def, priv->driver, net, - tapfd, &tapfdSize) < 0)
There are two more occurances of qemuInterfaceBridgeConnect(): one in qemu_interface.h (which needs to be removed) and more crucially the other in qemuxml2argvmock.c because otherwise qemuxmlconftest fails.
+ if (virDomainInterfaceBridgeConnect(vm->def, net, + tapfd, tapfdSize, + priv->driver->privileged, + priv->driver->ebtables, + priv->driver->config->macFilter, + cfg->bridgeHelperName) < 0) return -1; break;
Michal