> -----Original Message-----
> From: Keller, Jacob E
> Sent: Thursday, October 26, 2017 1:33 PM
> To: Keller, Jacob E <jacob.e.kel...@intel.com>; vyase...@redhat.com;
> netdev@vger.kernel.org
> Cc: Malek, Patryk <patryk.ma...@intel.com>
> Subject: RE: removing bridge in vlan_filtering mode requests delete of 
> attached
> ports main MAC address
> 
> > -----Original Message-----
> > From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org]
> > On Behalf Of Keller, Jacob E
> > Sent: Thursday, October 26, 2017 1:27 PM
> > To: vyase...@redhat.com; netdev@vger.kernel.org
> > Cc: Malek, Patryk <patryk.ma...@intel.com>
> > Subject: RE: removing bridge in vlan_filtering mode requests delete of 
> > attached
> > ports main MAC address
> >
> > > -----Original Message-----
> > > From: Vlad Yasevich [mailto:vyase...@redhat.com]
> > > Sent: Thursday, October 26, 2017 3:22 AM
> > > To: Keller, Jacob E <jacob.e.kel...@intel.com>; netdev@vger.kernel.org
> > > Cc: Malek, Patryk <patryk.ma...@intel.com>
> > > Subject: Re: removing bridge in vlan_filtering mode requests delete of
> attached
> > > ports main MAC address
> > >
> > > Hi Jake
> > >
> > > I think adding a !fdb->local should work.  local fdb contain the address 
> > > of
> > assigned
> > > to
> > > the ports of the bridge and those shouldn't be directly removed.
> > >
> > > If that works,  that looks like the right solution.
> > >
> > > -vlad
> > >
> >
> > So this does prevent us from removing the port's address. However, if I add
> two
> > devices to the bridge, then after removing the bridge, each device now keeps
> > both permanent addresses in their list, which isn't what we want is it?
> >
> > Do we even want to assign the local fdb addresses to every port?
> >
> > Obviously, I don't fully understand this code, so I think I'm missing 
> > something
> > here.
> >
> > Regards,
> > Jake
> >
> 
> Ok, I tried this again, and it didn't end up crossing the local device 
> addresses to
> each port. I'm not sure how that happened the first time yet, so maybe it is
> correct to skip removing local addresses... but if we skip removing them, 
> wouldn't
> we want to skip adding them too?
> 
> Thanks,
> Jake

I'm still digging into this. It turns out adding two devices, enabling vlan 
filtering, and deleting the bridge sometimes (but not always, not sure what 
condition triggers it) causes the hw address of one of the devices to be 
assigned to the other device.

I'm still unsure whether sync_static should be assigning local addresses to 
each device, but it appears like it should. In this case, I'm really unsure how 
to handle this case properly.

If we add local addresses, we need to delete the ones that aren't specific to 
that device so that after removing the bridge we end up in the original 
configuration.. but I'm not really sure how best to do this.

Using !fdb->is_local in unsync_static works to resolve my issue, but I believe 
it papers over other issues, since it means that we'll never delete static 
addresses when deleting the ports or exiting promiscuous mode.

I think checking fdb->dst might work, but that would break if we manually add a 
new address and tag is as permanent, see line 806 of br_fdb.c... In this case, 
we'd never delete this address even though it was not originally on the device.

I checked other drivers, and it turns out that at least one (ixgbe) doesn't 
have this problem because the hw address is special and isn't actually stored 
in a hardware MAC filter list. In i40e we keep the hardware address in the same 
list as all the other MAC filters.

We could "fix" this in i40e by treating the hw permanent address separately and 
essentially ignoring it from the dev_uc_del() calls.. but I still feel like 
this papers over the issues in the bridge code.

Any thoughts or suggestions? I haven't checked other drivers to see how they 
handle addresses in the unicast table (whether they treat the hw address as 
special or not, like ixgbe ultimately does).

Thanks,
Jake

Reply via email to