[libvirt] [PATCH] Remove MAX_TAP_ID, take 2

[Daniel P. Berrange]

On Wed, Jul 29, 2009 at 12:27:58AM -0400, Aron Griffis wrote:
> (Sorry, my first posting included some gnulib droppings in the
> patch. This removes that, everything else is the same.)
> 
> As far as I can tell, there's no reason to format the device string in
> brAddTap().  Delegate the job to TUNSETIFF, thereby removing the loop
> and the MAX_TAP_ID artificial limit.  This patch allows me to get
> 421 guests running before hitting other limits.

Very bizarre, the kernel seems to interpret 'vnet%d' and auto
fillin the unique integer for us. So this patch is just removing
the equivalent logic from libvirt. Wonder why we had it there in
the first place ! So this looks basically sound to me.

Out of interest, what are the other limits you hit .. ?

This patch does not apply though - I think it has whitespace 
damage from your mail client - neither patch, or git apply
like it. Could you try re-sending it..

Regards,
Daniel

> 
> Signed-off-by: Aron Griffis <aron.griffis at hp.com>
> ---
> 
>   src/bridge.c |   94 +++++++++++++++++++++-------------------------------------
>   1 files changed, 34 insertions(+), 60 deletions(-)
> 
> diff --git a/src/bridge.c b/src/bridge.c
> index 0509afd..ec37192 100644
> --- a/src/bridge.c
> +++ b/src/bridge.c
> @@ -49,8 +49,6 @@
>   #include "util.h"
>   #include "logging.h"
>   
> -#define MAX_TAP_ID 256
> -
>   #define JIFFIES_TO_MS(j) (((j)*1000)/HZ)
>   #define MS_TO_JIFFIES(ms) (((ms)*HZ)/1000)
>   
> @@ -466,76 +464,52 @@ brAddTap(brControl *ctl,
>            int vnet_hdr,
>            int *tapfd)
>   {
> -    int id, subst, fd;
> +    int fd, len;
> +    struct ifreq ifr = {0};
>   
>       if (!ctl || !ctl->fd || !bridge || !ifname)
>           return EINVAL;
>   
> -    subst = id = 0;
> -
> -    if (strstr(*ifname, "%d"))
> -        subst = 1;
> -
>       if ((fd = open("/dev/net/tun", O_RDWR)) < 0)
>         return errno;
>   
> -    if (vnet_hdr)
> -        vnet_hdr = brProbeVnetHdr(fd);
> +    ifr.ifr_flags = IFF_TAP|IFF_NO_PI;
>   
> -    do {
> -        struct ifreq try;
> -        int len;
> +#ifdef IFF_VNET_HDR
> +    if (vnet_hdr && brProbeVnetHdr(fd))
> +        ifr.ifr_flags |= IFF_VNET_HDR;
> +#endif
>   
> -        memset(&try, 0, sizeof(struct ifreq));
> +    strncpy(ifr.ifr_name, *ifname, IFNAMSIZ-1);
>   
> -        try.ifr_flags = IFF_TAP|IFF_NO_PI;
> +    if (ioctl(fd, TUNSETIFF, &ifr) < 0)
> +        goto error;
>   
> -#ifdef IFF_VNET_HDR
> -        if (vnet_hdr)
> -            try.ifr_flags |= IFF_VNET_HDR;
> -#endif
> +    len = strlen(ifr.ifr_name);
> +    if (len >= BR_IFNAME_MAXLEN - 1) {
> +        errno = EINVAL;
> +        goto error;
> +    }
>   
> -        if (subst) {
> -            len = snprintf(try.ifr_name, BR_IFNAME_MAXLEN, *ifname, id);
> -            if (len >= BR_IFNAME_MAXLEN) {
> -                errno = EADDRINUSE;
> -                goto error;
> -            }
> -        } else {
> -            len = strlen(*ifname);
> -            if (len >= BR_IFNAME_MAXLEN - 1) {
> -                errno = EINVAL;
> -                goto error;
> -            }
> -
> -            strncpy(try.ifr_name, *ifname, len);
> -            try.ifr_name[len] = '\0';
> -        }
> -
> -        if (ioctl(fd, TUNSETIFF, &try) == 0) {
> -            /* We need to set the interface MTU before adding it
> -             * to the bridge, because the bridge will have its
> -             * MTU adjusted automatically when we add the new interface.
> -             */
> -            if ((errno = brSetInterfaceMtu(ctl, bridge, try.ifr_name)))
> -                goto error;
> -            if ((errno = brAddInterface(ctl, bridge, try.ifr_name)))
> -                goto error;
> -            if ((errno = brSetInterfaceUp(ctl, try.ifr_name, 1)))
> -                goto error;
> -            if (!tapfd &&
> -                (errno = ioctl(fd, TUNSETPERSIST, 1)))
> -                goto error;
> -            VIR_FREE(*ifname);
> -            if (!(*ifname = strdup(try.ifr_name)))
> -                goto error;
> -            if (tapfd)
> -                *tapfd = fd;
> -            return 0;
> -        }
> -
> -        id++;
> -    } while (subst && id <= MAX_TAP_ID);
> +    /* We need to set the interface MTU before adding it
> +     * to the bridge, because the bridge will have its
> +     * MTU adjusted automatically when we add the new interface.
> +     */
> +    if ((errno = brSetInterfaceMtu(ctl, bridge, ifr.ifr_name)))
> +        goto error;
> +    if ((errno = brAddInterface(ctl, bridge, ifr.ifr_name)))
> +        goto error;
> +    if ((errno = brSetInterfaceUp(ctl, ifr.ifr_name, 1)))
> +        goto error;
> +    if (!tapfd &&
> +        (errno = ioctl(fd, TUNSETPERSIST, 1)))
> +        goto error;
> +    VIR_FREE(*ifname);
> +    if (!(*ifname = strdup(ifr.ifr_name)))
> +        goto error;
> +    if (tapfd)
> +        *tapfd = fd;
> +    return 0;
>   
>    error:
>       close(fd);
> 
> --
> Libvir-list mailing list
> Libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|