[PATCH] network: drop use of dummy tap device in bridges

[Daniel P. Berrangé]

On Thu, Sep 03, 2020 at 10:56:25AM -0400, Laine Stump wrote:
> On 9/3/20 9:34 AM, Daniel P. Berrangé wrote:
> > A long time ago we introduced a dummy tap device (e.g. virbr0-nic) that
> > we attached to the bridge device created for virtual networks:
> > 
> >    commit 5754dbd56d4738112a86776c09e810e32f7c3224
> >    Author: Laine Stump <laine at redhat.com>
> >    Date:   Wed Feb 9 03:28:12 2011 -0500
> > 
> >      Give each virtual network bridge its own fixed MAC address
> > 
> > This was a hack to workaround a Linux kernel bug where it would not
> > honour any attempt to set a MAC address on a bridge. Instead the
> > bridge would adopt the numerically lowest MAC address of all NICs
> > attached to the bridge. This lead to the MAC addrss of the bridge
> > changing over time as NICs were attached/detached.
> > 
> > The Linux bug was actually fixed 3 years before the libvirt
> > workaround was added in:
> > 
> >    commit 92c0574f11598c8036f81e27d2e8bdd6eed7d76d
> >    Author: Stephen Hemminger <shemminger at vyatta.com>
> >    Date:   Tue Jun 17 16:10:06 2008 -0700
> > 
> >      bridge: make bridge address settings sticky
> > 
> >      Normally, the bridge just chooses the smallest mac address as the
> >      bridge id and mac address of bridge device. But if the administrator
> >      has explictly set the interface address then don't change it.
> > 
> >      Signed-off-by: Stephen Hemminger <shemminger at vyatta.com>
> >      Signed-off-by: David S. Miller <davem at davemloft.net>
> > 
> > but libvirt needed to support RHEL-5 kernels at that time, so
> > none the less added the workaround.
> > 
> > We have long since dropped support for RHEL-5 vintage distros,
> > so there's no reason to keep the dummy tap device for the purpose
> > of setting the bridge MAC address.
> > 
> > Later the dummy TAP device was used for a second purpose related
> > to IPv6 DAD (Duplicate Address Detection) in:
> > 
> >    commit db488c79173b240459c7754f38c3c6af9b432970
> >    Author: Benjamin Cama <benoar at dolka.fr>
> >    Date:   Wed Sep 26 21:02:20 2012 +0200
> > 
> >      network: fix dnsmasq/radvd binding to IPv6 on recent kernels
> > 
> > This was again dealing with a regression in the Linux kernel, where
> > if there were no devices attached to the bridge in the UP state,
> > IPv6 DAD would not be performed. The virbr0-nic was attached but
> > in the DOWN state, so the above libvirt fix tenporarily brought
> > the NIC online. The Linux commit causing the problem was in v2.6.38
> > 
> >    commit 1faa4356a3bd89ea11fb92752d897cff3a20ec0e
> >    Author: stephen hemminger <shemminger at vyatta.com>
> >    Date:   Mon Mar 7 08:34:06 2011 +0000
> > 
> >      bridge: control carrier based on ports online
> > 
> > A short while later Linux was tweaked so that DAD would still occur
> > if the bridge had no attached devices at all in 3.1:
> > 
> >    commit b64b73d7d0c480f75684519c6134e79d50c1b341
> >    Author: stephen hemminger <shemminger at vyatta.com>
> >    Date:   Mon Oct 3 18:14:45 2011 +0000
> > 
> >      bridge: leave carrier on for empty bridge
> > 
> > IOW, the only reason we need the DAD hack of bringing virbr0-nic
> > online is because virbr0-nic exists. Once it doesn't exist, then
> > we hit the "empty bridge" case which works in Linux.
> 
> Ooh, cool! Having it used for DAD was the only reason I didn't remove
> virbrX-nic after I put in the patch to set the bridge MAC directly:
> 
> 
> commit 13ec827052fcd79a4350f499aab5f4aa20ea83fa
> Author: Laine Stump <laine at redhat.com>
> Date:   Thu Oct 17 21:12:30 2019 -0400
> 
>     util: set bridge device MAC address explicitly during
>     virNetDevBridgeCreate
> 
> (I even comment about this, but didn't dig down to learn that it was no
> longer required for DAD - I just thought that was the way bridges + DAD were
> designed to work).
> 
> Nice detective work! Did some problem lead you to this, or did you just get
> tired of always seeing that useless device hanging around?

The bug report from a user mentioned in the commit message prompted it

> 
> Reviewed-by: Laine Stump <laine at redhat.com>
> 
> (I give this assuming the veracity of all the info you've cited.)
> 
> One thing that comes to mind - do we still need to wait for DAD to finish in
> networkStartNetworkVirtual(). I think maybe we were waiting for that only so
> that we could set the dummy interface offline (there might be some other
> reason I'm not thinking of though).

I'm not sure, and I don't currently have an IPv6 deployment to test
this, so I'm loathe to touch it myself. Perhaps someone else can do the
change and validate it though...


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|