[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv3 06/11] util: Learn virNetDevTapCreate multiqueue network



On 05/16/2013 08:49 AM, Michal Privoznik wrote:
> Currently, the function knows how to open a TAP device for a single
> time. However, in case of multiqueue network we need to open it multiple
> times. Moreover, when doing TUNSETIFF ioctl, the IFF_MULTI_QUEUE flag
> shall be requested. This commit changes a behaviour slightly as well.
> Till now it was possible to pass NULL as an @fd address by which caller
> didn't care about returned FD. While this was used only in UML driver,
> the appropriate UML code snippets has been changed as well.
> ---
>  src/network/bridge_driver.c |   2 +-
>  src/qemu/qemu_command.c     |   2 +-
>  src/uml/uml_conf.c          |   5 +-
>  src/util/virnetdevtap.c     | 113 ++++++++++++++++++++++++--------------------
>  src/util/virnetdevtap.h     |   2 +
>  5 files changed, 71 insertions(+), 53 deletions(-)
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 6c6ce6d..ad4ab00 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -2495,7 +2495,7 @@ networkStartNetworkVirtual(struct network_driver *driver,
>          /* Keep tun fd open and interface up to allow for IPv6 DAD to happen */
>          if (virNetDevTapCreateInBridgePort(network->def->bridge,
>                                             &macTapIfName, &network->def->mac,
> -                                           NULL, &tapfd, NULL, NULL,
> +                                           NULL, &tapfd, 1, NULL, NULL,
>                                             VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE |
>                                             VIR_NETDEV_TAP_CREATE_IFUP |
>                                             VIR_NETDEV_TAP_CREATE_PERSIST) < 0) {
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 67baad3..a053d49 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -355,7 +355,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
>  
>      if (cfg->privileged)
>          err = virNetDevTapCreateInBridgePort(brname, &net->ifname, &net->mac,
> -                                             def->uuid, &tapfd,
> +                                             def->uuid, &tapfd, 1,
>                                               virDomainNetGetActualVirtPortProfile(net),
>                                               virDomainNetGetActualVlan(net),
>                                               tap_create_flags);
> diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c
> index 52b705c..3fda7e4 100644
> --- a/src/uml/uml_conf.c
> +++ b/src/uml/uml_conf.c
> @@ -109,6 +109,7 @@ umlConnectTapDevice(virConnectPtr conn,
>                      const char *bridge)
>  {
>      bool template_ifname = false;
> +    int tapfd;
>  
>      if (!net->ifname ||
>          STRPREFIX(net->ifname, VIR_NET_GENERATED_PREFIX) ||
> @@ -121,7 +122,7 @@ umlConnectTapDevice(virConnectPtr conn,
>      }
>  
>      if (virNetDevTapCreateInBridgePort(bridge, &net->ifname, &net->mac,
> -                                       vm->uuid, NULL,
> +                                       vm->uuid, &tapfd, 1,
>                                         virDomainNetGetActualVirtPortProfile(net),
>                                         virDomainNetGetActualVlan(net),
>                                         VIR_NETDEV_TAP_CREATE_IFUP |
> @@ -139,9 +140,11 @@ umlConnectTapDevice(virConnectPtr conn,
>          }
>      }
>  
> +    VIR_FORCE_CLOSE(tapfd);
>      return 0;
>  
>  error:
> +    VIR_FORCE_CLOSE(tapfd);
>      return -1;
>  }
>  
> diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
> index 75599db..62a1ba2 100644
> --- a/src/util/virnetdevtap.c
> +++ b/src/util/virnetdevtap.c
> @@ -140,7 +140,8 @@ virNetDevProbeVnetHdr(int tapfd)
>  /**
>   * virNetDevTapCreate:
>   * @ifname: the interface name
> - * @tapfd: file descriptor return value for the new tap device
> + * @tapfds: file descriptor return value for the new tap device

"array of file descriptors ..."


> + * @tapfdSize: size of @tapfd


"number of file descriptors in @tapfd"


>   * @flags: OR of virNetDevTapCreateFlags. Only one flag is recognized:
>   *
>   *   VIR_NETDEV_TAP_CREATE_VNET_HDR
> @@ -148,76 +149,85 @@ virNetDevProbeVnetHdr(int tapfd)
>   *   VIR_NETDEV_TAP_CREATE_PERSIST
>   *     - The device will persist after the file descriptor is closed
>   *
> - * Creates a tap interface.
> - * If the @tapfd parameter is supplied, the open tap device file descriptor
> - * will be returned, otherwise the TAP device will be closed. The caller must
> - * use virNetDevTapDelete to remove a persistent TAP device when it is no
> - * longer needed.
> + * Creates a tap interface. The caller must use virNetDevTapDelete to
> + * remove a persistent TAP device when it is no longer needed. In case
> + * @tapfdSize is greater than one, multiqueue extension is requested
> + * from kernel.
>   *
>   * Returns 0 in case of success or -1 on failure.
>   */
>  int virNetDevTapCreate(char **ifname,
>                         int *tapfd,
> +                       int tapfdSize,
>                         unsigned int flags)
>  {
> -    int fd;
> +    int i;
>      struct ifreq ifr;
>      int ret = -1;
> -
> -    if ((fd = open("/dev/net/tun", O_RDWR)) < 0) {
> -        virReportSystemError(errno, "%s",
> -                             _("Unable to open /dev/net/tun, is tun module loaded?"));
> -        return -1;
> -    }
> +    int fd;
>  
>      memset(&ifr, 0, sizeof(ifr));
> +    for (i = 0; i < tapfdSize; i++) {
> +        if ((fd = open("/dev/net/tun", O_RDWR)) < 0) {
> +            virReportSystemError(errno, "%s",
> +                                 _("Unable to open /dev/net/tun, is tun module loaded?"));
> +            goto cleanup;
> +        }
>  
> -    ifr.ifr_flags = IFF_TAP|IFF_NO_PI;
> +        memset(&ifr, 0, sizeof(ifr));
> +
> +        ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
> +        /* If tapfdSize is greater than one, request multiqueue */
> +        if (tapfdSize > 1)
> +            ifr.ifr_flags |= IFF_MULTI_QUEUE;
>  
>  # ifdef IFF_VNET_HDR
> -    if ((flags &  VIR_NETDEV_TAP_CREATE_VNET_HDR) &&
> -        virNetDevProbeVnetHdr(fd))
> -        ifr.ifr_flags |= IFF_VNET_HDR;
> +        if ((flags &  VIR_NETDEV_TAP_CREATE_VNET_HDR) &&
> +            virNetDevProbeVnetHdr(fd))
> +            ifr.ifr_flags |= IFF_VNET_HDR;
>  # endif
>  
> -    if (virStrcpyStatic(ifr.ifr_name, *ifname) == NULL) {
> -        virReportSystemError(ERANGE,
> -                             _("Network interface name '%s' is too long"),
> -                             *ifname);
> -        goto cleanup;
> +        if (virStrcpyStatic(ifr.ifr_name, *ifname) == NULL) {
> +            virReportSystemError(ERANGE,
> +                                 _("Network interface name '%s' is too long"),
> +                                 *ifname);
> +            goto cleanup;
>  
> -    }
> +        }
>  
> -    if (ioctl(fd, TUNSETIFF, &ifr) < 0) {
> -        virReportSystemError(errno,
> -                             _("Unable to create tap device %s"),
> -                             NULLSTR(*ifname));
> -        goto cleanup;
> -    }
> +        if (ioctl(fd, TUNSETIFF, &ifr) < 0) {
> +            virReportSystemError(errno,
> +                                 _("Unable to create tap device %s"),
> +                                 NULLSTR(*ifname));
> +            goto cleanup;
> +        }
>  
> -    if ((flags & VIR_NETDEV_TAP_CREATE_PERSIST) &&
> -        (errno = ioctl(fd, TUNSETPERSIST, 1))) {
> -        virReportSystemError(errno,
> -                             _("Unable to set tap device %s to persistent"),
> -                             NULLSTR(*ifname));
> -        goto cleanup;
> -    }
> +        if (i == 0) {
> +            /* In case we are looping more than once, set other
> +             * TAPs to have the same name */
> +            VIR_FREE(*ifname);
> +            if (ifr.ifr_name && VIR_STRDUP(*ifname, ifr.ifr_name) < 0)
> +                goto cleanup;
> +        }
>  
> -    VIR_FREE(*ifname);
> -    if (!(*ifname = strdup(ifr.ifr_name))) {
> -        virReportOOMError();
> -        goto cleanup;
> +        if ((flags & VIR_NETDEV_TAP_CREATE_PERSIST) &&
> +            (errno = ioctl(fd, TUNSETPERSIST, 1))) {
> +            virReportSystemError(errno,
> +                                 _("Unable to set tap device %s to persistent"),
> +                                 NULLSTR(*ifname));
> +            goto cleanup;
> +        }
> +        tapfd[i] = fd;
>      }
> -    if (tapfd)
> -        *tapfd = fd;
> -    else
> -        VIR_FORCE_CLOSE(fd);
>  
>      ret = 0;
>  
>  cleanup:
> -    if (ret < 0)
> +    if (ret < 0) {
>          VIR_FORCE_CLOSE(fd);
> +        while (i--)
> +            VIR_FORCE_CLOSE(tapfd[i]);
> +    }
>  
>      return ret;
>  }
> @@ -266,6 +276,7 @@ cleanup:
>  #else /* ! TUNSETIFF */
>  int virNetDevTapCreate(char **ifname ATTRIBUTE_UNUSED,
>                         int *tapfd ATTRIBUTE_UNUSED,
> +                       int tapfdSize ATTRIBUTE_UNUSED,
>                         unsigned int flags ATTRIBUTE_UNUSED)
>  {
>      virReportSystemError(ENOSYS, "%s",
> @@ -286,7 +297,8 @@ int virNetDevTapDelete(const char *ifname ATTRIBUTE_UNUSED)
>   * @brname: the bridge name
>   * @ifname: the interface name (or name template)
>   * @macaddr: desired MAC address
> - * @tapfd: file descriptor return value for the new tap device
> + * @tapfd: array of file descriptor return value for the new tap device
> + * @tapfdSize: size of @tapfd

"number of file descriptors in @tapfd"

>   * @virtPortProfile: bridge/port specific configuration
>   * @flags: OR of virNetDevTapCreateFlags:
>  
> @@ -314,6 +326,7 @@ int virNetDevTapCreateInBridgePort(const char *brname,
>                                     const virMacAddrPtr macaddr,
>                                     const unsigned char *vmuuid,
>                                     int *tapfd,
> +                                   int tapfdSize,
>                                     virNetDevVPortProfilePtr virtPortProfile,
>                                     virNetDevVlanPtr virtVlan,
>                                     unsigned int flags)
> @@ -321,7 +334,7 @@ int virNetDevTapCreateInBridgePort(const char *brname,
>      virMacAddr tapmac;
>      char macaddrstr[VIR_MAC_STRING_BUFLEN];
>  
> -    if (virNetDevTapCreate(ifname, tapfd, flags) < 0)
> +    if (virNetDevTapCreate(ifname, tapfd, tapfdSize, flags) < 0)
>          return -1;
>  
>      /* We need to set the interface MAC before adding it
> @@ -372,9 +385,9 @@ int virNetDevTapCreateInBridgePort(const char *brname,
>  
>      return 0;
>  
> - error:
> -    if (tapfd)
> -        VIR_FORCE_CLOSE(*tapfd);
> +error:
> +    while (tapfdSize)
> +        VIR_FORCE_CLOSE(tapfd[--tapfdSize]);
>  
>      return -1;
>  }
> diff --git a/src/util/virnetdevtap.h b/src/util/virnetdevtap.h
> index 6bfc80c..cb6c284 100644
> --- a/src/util/virnetdevtap.h
> +++ b/src/util/virnetdevtap.h
> @@ -29,6 +29,7 @@
>  
>  int virNetDevTapCreate(char **ifname,
>                         int *tapfd,
> +                       int tapfdSize,
>                         unsigned int flags)
>      ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
>  
> @@ -55,6 +56,7 @@ int virNetDevTapCreateInBridgePort(const char *brname,
>                                     const virMacAddrPtr macaddr,
>                                     const unsigned char *vmuuid,
>                                     int *tapfd,
> +                                   int tapfdSize,
>                                     virNetDevVPortProfilePtr virtPortProfile,
>                                     virNetDevVlanPtr virtVlan,
>                                     unsigned int flags)


ACK with comment fixes.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]