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@xxxxxxxxxx> > > 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@xxxxxxxxxx> > > 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@xxxxxxxxxx> > > Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx> > > > > 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@xxxxxxxx> > > 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@xxxxxxxxxx> > > 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@xxxxxxxxxx> > > 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@xxxxxxxxxx> > 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@xxxxxxxxxx> > > (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 :|
