[libvirt] [PATCH v2] Fix dnsmasq/radvd on bridges not being able to be bound to IPv6 address on "recent" kernels
Benjamin Cama
benjamin.cama at telecom-bretagne.eu
Fri Sep 21 17:04:31 UTC 2012
Ping?
I just stumbled over this again today: vanillia Ubuntu 12.04, set up a
network bridge with an IPv6 address, and it stays “tentative” and can't
be used (it doesn't block when starting the network, though, contrary to
my Debian experience). On my Debian squeeze, it's still blocking when
doing net-start, with the latest backport (0.9.12) with kernel 3.2.0.
Le mercredi 20 juin 2012 à 14:27 +0100, Daniel P. Berrange a écrit :
> On Wed, Jun 20, 2012 at 01:49:30PM +0200, Benjamin Cama wrote:
> > Le mercredi 20 juin 2012 à 11:05 +0100, Daniel P. Berrange a écrit :
> > > I think you also need to have a VIR_FOFRCE_CLOSE(tapfd) in the
> > > very last of the cleanup jump labels. Since between the time
> > > we open the tapfd & close it, we might have done a 'goto err1'
> > > or similar.
> >
> > Yes, I forgot that. New patch following.
> >
> > > Looks reasonable apart from the cleanup bug.
> >
> > Thanks. BTW, if this patch is commited, I'm already in AUTHORS under
> > another email address, which is OK.
> >
> > ---
> > src/network/bridge_driver.c | 22 ++++++++++++++++++++--
> > src/uml/uml_conf.c | 2 +-
> > src/util/virnetdevtap.c | 24 ++++++++++++++----------
> > src/util/virnetdevtap.h | 2 ++
> > 4 files changed, 37 insertions(+), 13 deletions(-)
> >
> > diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> > index 79d3010..e9f3c90 100644
> > --- a/src/network/bridge_driver.c
> > +++ b/src/network/bridge_driver.c
> > @@ -62,6 +62,7 @@
> > #include "virnetdev.h"
> > #include "virnetdevbridge.h"
> > #include "virnetdevtap.h"
> > +#include "virfile.h"
> >
> > #define NETWORK_PID_DIR LOCALSTATEDIR "/run/libvirt/network"
> > #define NETWORK_STATE_DIR LOCALSTATEDIR "/lib/libvirt/network"
> > @@ -840,6 +841,7 @@ networkStartRadvd(virNetworkObjPtr network)
> > " AdvSendAdvert on;\n"
> > " AdvManagedFlag off;\n"
> > " AdvOtherConfigFlag off;\n"
> > + " IgnoreIfMissing on;\n"
> > "\n",
> > network->def->bridge);
> > for (ii = 0;
> > @@ -1744,6 +1746,7 @@ networkStartNetworkVirtual(struct network_driver *driver,
> > virErrorPtr save_err = NULL;
> > virNetworkIpDefPtr ipdef;
> > char *macTapIfName = NULL;
> > + int tapfd = -1;
> >
> > /* Check to see if any network IP collides with an existing route */
> > if (networkCheckRouteCollision(network) < 0)
> > @@ -1765,10 +1768,13 @@ networkStartNetworkVirtual(struct network_driver *driver,
> > virReportOOMError();
> > goto err0;
> > }
> > + /* Keep tun fd open and interface up to allow for IPv6 DAD to happen */
> > if (virNetDevTapCreateInBridgePort(network->def->bridge,
> > &macTapIfName, network->def->mac,
> > - NULL, NULL, NULL,
> > - VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE) < 0) {
> > + NULL, &tapfd, NULL,
> > + VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE
> > + |VIR_NETDEV_TAP_CREATE_IFUP
> > + |VIR_NETDEV_TAP_CREATE_PERSIST) < 0) {
> > VIR_FREE(macTapIfName);
> > goto err0;
> > }
> > @@ -1828,6 +1834,16 @@ networkStartNetworkVirtual(struct network_driver *driver,
> > if (v6present && networkStartRadvd(network) < 0)
> > goto err4;
> >
> > + /* DAD has happened (dnsmasq waits for it), dnsmasq is now bound to the
> > + * bridge's IPv6 address, and radvd to the interface, so we can now set the
> > + * dummy tun down.
> > + */
> > + if (tapfd >= 0) {
> > + if (virNetDevSetOnline(macTapIfName, false) < 0)
> > + goto err4;
> > + VIR_FORCE_CLOSE(tapfd);
> > + }
> > +
> > if (virNetDevBandwidthSet(network->def->bridge, network->def->bandwidth) < 0) {
> > networkReportError(VIR_ERR_INTERNAL_ERROR,
> > _("cannot set bandwidth limits on %s"),
> > @@ -1866,6 +1882,8 @@ networkStartNetworkVirtual(struct network_driver *driver,
> > save_err = virSaveLastError();
> >
> > if (macTapIfName) {
> > + if (tapfd >= 0)
> > + VIR_FORCE_CLOSE(tapfd);
> > ignore_value(virNetDevTapDelete(macTapIfName));
> > VIR_FREE(macTapIfName);
> > }
> > diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c
> > index 79b249d..c9b5044 100644
> > --- a/src/uml/uml_conf.c
> > +++ b/src/uml/uml_conf.c
> > @@ -141,7 +141,7 @@ umlConnectTapDevice(virConnectPtr conn,
> > if (virNetDevTapCreateInBridgePort(bridge, &net->ifname, net->mac,
> > vm->uuid, NULL,
> > virDomainNetGetActualVirtPortProfile(net),
> > - VIR_NETDEV_TAP_CREATE_IFUP) < 0) {
> > + VIR_NETDEV_TAP_CREATE_IFUP|VIR_NETDEV_TAP_CREATE_PERSIST) < 0) {
> > if (template_ifname)
> > VIR_FREE(net->ifname);
> > goto error;
> > diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
> > index 5d21164..45ff20f 100644
> > --- a/src/util/virnetdevtap.c
> > +++ b/src/util/virnetdevtap.c
> > @@ -118,18 +118,20 @@ virNetDevProbeVnetHdr(int tapfd)
> > *
> > * VIR_NETDEV_TAP_CREATE_VNET_HDR
> > * - Enable IFF_VNET_HDR on the tap device
> > + * 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 made
> > - * persistent and closed. The caller must use virNetDevTapDelete to
> > - * remove a persistent TAP devices when it is no longer needed.
> > + * 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.
> > *
> > * Returns 0 in case of success or an errno code in case of failure.
> > */
> > int virNetDevTapCreate(char **ifname,
> > int *tapfd,
> > - unsigned int flags ATTRIBUTE_UNUSED)
> > + unsigned int flags)
> > {
> > int fd;
> > struct ifreq ifr;
> > @@ -166,7 +168,7 @@ int virNetDevTapCreate(char **ifname,
> > goto cleanup;
> > }
> >
> > - if (!tapfd &&
> > + if ((flags & VIR_NETDEV_TAP_CREATE_PERSIST) &&
> > (errno = ioctl(fd, TUNSETPERSIST, 1))) {
> > virReportSystemError(errno,
> > _("Unable to set tap device %s to persistent"),
> > @@ -267,14 +269,16 @@ int virNetDevTapDelete(const char *ifname ATTRIBUTE_UNUSED)
> > * - Enable IFF_VNET_HDR on the tap device
> > * VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE
> > * - Set this interface's MAC as the bridge's MAC address
> > + * VIR_NETDEV_TAP_CREATE_PERSIST
> > + * - The device will persist after the file descriptor is closed
> > *
> > * This function creates a new tap device on a bridge. @ifname can be either
> > * a fixed name or a name template with '%d' for dynamic name allocation.
> > * in either case the final name for the bridge will be stored in @ifname.
> > - * If the @tapfd parameter is supplied, the open tap device file
> > - * descriptor will be returned, otherwise the TAP device will be made
> > - * persistent and closed. The caller must use virNetDevTapDelete to remove
> > - * a persistent TAP devices when it is no longer needed.
> > + * 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.
> > *
> > * Returns 0 in case of success or -1 on failure
> > */
> > diff --git a/src/util/virnetdevtap.h b/src/util/virnetdevtap.h
> > index d9a3593..f1355ba 100644
> > --- a/src/util/virnetdevtap.h
> > +++ b/src/util/virnetdevtap.h
> > @@ -42,6 +42,8 @@ typedef enum {
> > VIR_NETDEV_TAP_CREATE_VNET_HDR = 1 << 1,
> > /* Set this interface's MAC as the bridge's MAC address */
> > VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE = 1 << 2,
> > + /* The device will persist after the file descriptor is closed */
> > + VIR_NETDEV_TAP_CREATE_PERSIST = 1 << 3,
> > } virNetDevTapCreateFlags;
> >
> > int virNetDevTapCreateInBridgePort(const char *brname,
> >
>
> ACK
>
> Daniel
> --
> |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org -o- http://virt-manager.org :|
> |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
>
Regards,
--
Benjamin Cama <benjamin.cama at telecom-bretagne.eu>
More information about the libvir-list
mailing list