On 8/2/24 00:25, Praveen K Paladugu wrote:
> From: Praveen K Paladugu <praveenkpaladugu@xxxxxxxxx>
>
> From: Praveen K Paladugu <prapal@xxxxxxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxx>
> Signed-off-by: Praveen K Paladugu <prapal@xxxxxxxxxxxxxxxxxxx>
> ---
> 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
