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@xxxxxx>
> ---
>
> 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@xxxxxxxxxx
> 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 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list