RE: removing bridge in vlan_filtering mode requests delete of attached ports main MAC address

2017-11-02 Thread Keller, Jacob E
> -Original Message-
> From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org]
> On Behalf Of Toshiaki Makita
> Sent: Thursday, November 02, 2017 2:23 AM
> To: Keller, Jacob E <jacob.e.kel...@intel.com>; netdev@vger.kernel.org
> Cc: vyase...@redhat.com; Malek, Patryk <patryk.ma...@intel.com>
> Subject: Re: removing bridge in vlan_filtering mode requests delete of 
> attached
> ports main MAC address
> 
> On 2017/11/02 7:25, Keller, Jacob E wrote:
> ...
> >> If we skip adding them, we cannot receive frames which should be
> >> received on the bridge device during non-promiscuous mode.
> >>
> >> --
> >> Toshiaki Makita
> >
> > This makes sense, but then what removes the addresses upon bridge deletion
> or exiting static mode?
> >
> > We want to make sure we remove the correct addresses but don't request a
> delete of the permanent MAC address? Or, do we just completely assume that a
> device will never actually delete it's own permanent address, and thus say 
> this is
> a driver's fault for allowing a delete request of its permanent address to do
> anything..?
> 
> We may be able to skip adding or deleting local address which is
> identical to dev_addr in bridge code.
> Having said that I feel like drivers should ensure not to remove their
> permanent address even when the same address is removed from the uc
> list, since currently it is not prohibited to do that kind of admin
> operation through bridge command (bridge fdb add|del self).
> Note that "bridge fdb ... self" is a command which modifies device's uc
> filter, not modify bridge's fdb entries.
> 
> --
> Toshiaki Makita   

Ok. I'll go ahead and cook a patch for preventing such a removal from deleting 
the permanent address from i40e. That sounds like the most reasonable approach 
given that from digging into other drivers, they don't store the permanent 
address in the regular UC table anyways.

Thanks,
Jake



Re: removing bridge in vlan_filtering mode requests delete of attached ports main MAC address

2017-11-02 Thread Toshiaki Makita
On 2017/11/02 7:25, Keller, Jacob E wrote:
...
>> If we skip adding them, we cannot receive frames which should be
>> received on the bridge device during non-promiscuous mode.
>>
>> --
>> Toshiaki Makita
> 
> This makes sense, but then what removes the addresses upon bridge deletion or 
> exiting static mode?
> 
> We want to make sure we remove the correct addresses but don't request a 
> delete of the permanent MAC address? Or, do we just completely assume that a 
> device will never actually delete it's own permanent address, and thus say 
> this is a driver's fault for allowing a delete request of its permanent 
> address to do anything..?

We may be able to skip adding or deleting local address which is
identical to dev_addr in bridge code.
Having said that I feel like drivers should ensure not to remove their
permanent address even when the same address is removed from the uc
list, since currently it is not prohibited to do that kind of admin
operation through bridge command (bridge fdb add|del self).
Note that "bridge fdb ... self" is a command which modifies device's uc
filter, not modify bridge's fdb entries.

-- 
Toshiaki Makita



RE: removing bridge in vlan_filtering mode requests delete of attached ports main MAC address

2017-11-01 Thread Keller, Jacob E
> -Original Message-
> From: Toshiaki Makita [mailto:makita.toshi...@lab.ntt.co.jp]
> Sent: Tuesday, October 31, 2017 5:58 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
> 
> On 2017/11/01 9:10, Keller, Jacob E wrote:
> >> -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
> >
> > There's definitely some weirdness going on, because I've been able to get 
> > the
> local port addresses added to the wrong device under some circumstances. It
> seems to be some sort of race condition, since I can't reliably re-create the
> scenario.
> >
> > Either way, some more insight on what the correct fix here would be nice.
> >
> > I'm thinking we want to skip adding or removing local addresses when 
> > switching
> into the static mode configuration.
> 
> If we skip adding them, we cannot receive frames which should be
> received on the bridge device during non-promiscuous mode.
> 
> --
> Toshiaki Makita

This makes sense, but then what removes the addresses upon bridge deletion or 
exiting static mode?

We want to make sure we remove the correct addresses but don't request a delete 
of the permanent MAC address? Or, do we just completely assume that a device 
will never actually delete it's own permanent address, and thus say this is a 
driver's fault for allowing a delete request of its permanent address to do 
anything..?

Thanks,
Jake
 


Re: removing bridge in vlan_filtering mode requests delete of attached ports main MAC address

2017-10-31 Thread Toshiaki Makita
On 2017/11/01 9:10, Keller, Jacob E wrote:
>> -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
> 
> There's definitely some weirdness going on, because I've been able to get the 
> local port addresses added to the wrong device under some circumstances. It 
> seems to be some sort of race condition, since I can't reliably re-create the 
> scenario.
> 
> Either way, some more insight on what the correct fix here would be nice.
> 
> I'm thinking we want to skip adding or removing local addresses when 
> switching into the static mode configuration.

If we skip adding them, we cannot receive frames which should be
received on the bridge device during non-promiscuous mode.

-- 
Toshiaki Makita



RE: removing bridge in vlan_filtering mode requests delete of attached ports main MAC address

2017-10-31 Thread Keller, Jacob E
> -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

There's definitely some weirdness going on, because I've been able to get the 
local port addresses added to the wrong device under some circumstances. It 
seems to be some sort of race condition, since I can't reliably re-create the 
scenario.

Either way, some more insight on what the correct fix here would be nice.

I'm thinking we want to skip adding or removing local addresses when switching 
into the static mode configuration.

Thanks,
Jake


RE: removing bridge in vlan_filtering mode requests delete of attached ports main MAC address

2017-10-26 Thread Keller, Jacob E
> -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


RE: removing bridge in vlan_filtering mode requests delete of attached ports main MAC address

2017-10-26 Thread Keller, Jacob E
> -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


RE: removing bridge in vlan_filtering mode requests delete of attached ports main MAC address

2017-10-26 Thread Keller, Jacob E
> -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
> 
> On 10/20/2017 08:06 PM, Keller, Jacob E wrote:
> >> -Original Message-
> >> From: Keller, Jacob E
> >> Sent: Friday, October 20, 2017 10:23 AM
> >> To: netdev@vger.kernel.org
> >> Cc: Malek, Patryk <patryk.ma...@intel.com>; 'Vlad Yasevich'
> >> <vyase...@redhat.com>
> >> Subject: removing bridge in vlan_filtering mode requests delete of attached
> >> ports main MAC address
> >>
> >> Hi,
> >>
> >> We've run into an issue with bridges set in vlan_filtering mode. 
> >> Basically, if we
> >> attach a device to a bridge which has enabled vlan_filtering, and then 
> >> remove
> the
> >> bridge, we end up requesting the driver of the attached device to remove 
> >> its
> >> own MAC HW address.
> >>
> >> In i40e, at least, this causes the driver to actually delete such an 
> >> address and
> then
> >> it will no longer receive any traffic.
> >>
> >> To reproduce this:
> >>
> >> a) brctl addbr br0
> >> b) brctl addif br0 enp
> >> # enable vlan filtering
> >> c) echo 1 >/sys/class/net/br0/bridge/vlan_filtering
> >> d) brctl delbr br0
> >>
> >> Specifically this appears to happen because of how we automatically enter
> static
> >> configuration for routes when vlan_filtering is enabled, and we call
> >> br_fdb_unsync_static which will clear all the routes from the fdb table 
> >> for the
> >> device. See commit 2796d0c648c9 ("bridge: Automatically manage port
> >> promiscuous mode.", 2014-05-16) for more details.
> >>
> >> This happens to include the devices own default address, which results in 
> >> the
> >> bug.
> >>
> >> I'm not sure if this is a driver bug, or if it's a bug in the bridging 
> >> code.
> >>
> >> Who would know more about this and what to do about this?
> >>
> >> One obvious solution is to hard code the i40e device driver so that it 
> >> does not
> >> actually delete the HW address from the unicast filter list. This could 
> >> work, but
> >> seems to me like its papering over the problem. Is this just a known thing 
> >> that
> >> drivers should be aware of? I don't really know...
> >>
> >> An alternative solution would be to possibly ignore any fdb addresses which
> >> specifically target that port?
> >>
> >> Any ideas?
> >
> > For the record, adding a check to prevent unsync_static from removing
> addresses which are targetting the specific port does work to resolve this 
> specific
> issue, but I'm sure it's not the correct solution as I expect that would 
> cause other
> problems.
> >
> 
> 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

> > Thanks,
> > Jake
> >
> >>
> >> Regards,
> >> Jake



RE: removing bridge in vlan_filtering mode requests delete of attached ports main MAC address

2017-10-26 Thread Keller, Jacob E
> -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
> 
> On 10/20/2017 08:06 PM, Keller, Jacob E wrote:
> >> -Original Message-
> >> From: Keller, Jacob E
> >> Sent: Friday, October 20, 2017 10:23 AM
> >> To: netdev@vger.kernel.org
> >> Cc: Malek, Patryk <patryk.ma...@intel.com>; 'Vlad Yasevich'
> >> <vyase...@redhat.com>
> >> Subject: removing bridge in vlan_filtering mode requests delete of attached
> >> ports main MAC address
> >>
> >> Hi,
> >>
> >> We've run into an issue with bridges set in vlan_filtering mode. 
> >> Basically, if we
> >> attach a device to a bridge which has enabled vlan_filtering, and then 
> >> remove
> the
> >> bridge, we end up requesting the driver of the attached device to remove 
> >> its
> >> own MAC HW address.
> >>
> >> In i40e, at least, this causes the driver to actually delete such an 
> >> address and
> then
> >> it will no longer receive any traffic.
> >>
> >> To reproduce this:
> >>
> >> a) brctl addbr br0
> >> b) brctl addif br0 enp
> >> # enable vlan filtering
> >> c) echo 1 >/sys/class/net/br0/bridge/vlan_filtering
> >> d) brctl delbr br0
> >>
> >> Specifically this appears to happen because of how we automatically enter
> static
> >> configuration for routes when vlan_filtering is enabled, and we call
> >> br_fdb_unsync_static which will clear all the routes from the fdb table 
> >> for the
> >> device. See commit 2796d0c648c9 ("bridge: Automatically manage port
> >> promiscuous mode.", 2014-05-16) for more details.
> >>
> >> This happens to include the devices own default address, which results in 
> >> the
> >> bug.
> >>
> >> I'm not sure if this is a driver bug, or if it's a bug in the bridging 
> >> code.
> >>
> >> Who would know more about this and what to do about this?
> >>
> >> One obvious solution is to hard code the i40e device driver so that it 
> >> does not
> >> actually delete the HW address from the unicast filter list. This could 
> >> work, but
> >> seems to me like its papering over the problem. Is this just a known thing 
> >> that
> >> drivers should be aware of? I don't really know...
> >>
> >> An alternative solution would be to possibly ignore any fdb addresses which
> >> specifically target that port?
> >>
> >> Any ideas?
> >
> > For the record, adding a check to prevent unsync_static from removing
> addresses which are targetting the specific port does work to resolve this 
> specific
> issue, but I'm sure it's not the correct solution as I expect that would 
> cause other
> problems.
> >
> 
> 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
> 

I'll give this a shot, and if so, cook up a patch.

Thanks,
Jake

> > Thanks,
> > Jake
> >
> >>
> >> Regards,
> >> Jake



Re: removing bridge in vlan_filtering mode requests delete of attached ports main MAC address

2017-10-26 Thread Vlad Yasevich
On 10/20/2017 08:06 PM, Keller, Jacob E wrote:
>> -Original Message-
>> From: Keller, Jacob E
>> Sent: Friday, October 20, 2017 10:23 AM
>> To: netdev@vger.kernel.org
>> Cc: Malek, Patryk <patryk.ma...@intel.com>; 'Vlad Yasevich'
>> <vyase...@redhat.com>
>> Subject: removing bridge in vlan_filtering mode requests delete of attached
>> ports main MAC address
>>
>> Hi,
>>
>> We've run into an issue with bridges set in vlan_filtering mode. Basically, 
>> if we
>> attach a device to a bridge which has enabled vlan_filtering, and then 
>> remove the
>> bridge, we end up requesting the driver of the attached device to remove its
>> own MAC HW address.
>>
>> In i40e, at least, this causes the driver to actually delete such an address 
>> and then
>> it will no longer receive any traffic.
>>
>> To reproduce this:
>>
>> a) brctl addbr br0
>> b) brctl addif br0 enp
>> # enable vlan filtering
>> c) echo 1 >/sys/class/net/br0/bridge/vlan_filtering
>> d) brctl delbr br0
>>
>> Specifically this appears to happen because of how we automatically enter 
>> static
>> configuration for routes when vlan_filtering is enabled, and we call
>> br_fdb_unsync_static which will clear all the routes from the fdb table for 
>> the
>> device. See commit 2796d0c648c9 ("bridge: Automatically manage port
>> promiscuous mode.", 2014-05-16) for more details.
>>
>> This happens to include the devices own default address, which results in the
>> bug.
>>
>> I'm not sure if this is a driver bug, or if it's a bug in the bridging code.
>>
>> Who would know more about this and what to do about this?
>>
>> One obvious solution is to hard code the i40e device driver so that it does 
>> not
>> actually delete the HW address from the unicast filter list. This could 
>> work, but
>> seems to me like its papering over the problem. Is this just a known thing 
>> that
>> drivers should be aware of? I don't really know...
>>
>> An alternative solution would be to possibly ignore any fdb addresses which
>> specifically target that port?
>>
>> Any ideas?
> 
> For the record, adding a check to prevent unsync_static from removing 
> addresses which are targetting the specific port does work to resolve this 
> specific issue, but I'm sure it's not the correct solution as I expect that 
> would cause other problems.
> 

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

> Thanks,
> Jake
> 
>>
>> Regards,
>> Jake



RE: removing bridge in vlan_filtering mode requests delete of attached ports main MAC address

2017-10-20 Thread Keller, Jacob E
> -Original Message-
> From: Keller, Jacob E
> Sent: Friday, October 20, 2017 10:23 AM
> To: netdev@vger.kernel.org
> Cc: Malek, Patryk <patryk.ma...@intel.com>; 'Vlad Yasevich'
> <vyase...@redhat.com>
> Subject: removing bridge in vlan_filtering mode requests delete of attached
> ports main MAC address
> 
> Hi,
> 
> We've run into an issue with bridges set in vlan_filtering mode. Basically, 
> if we
> attach a device to a bridge which has enabled vlan_filtering, and then remove 
> the
> bridge, we end up requesting the driver of the attached device to remove its
> own MAC HW address.
> 
> In i40e, at least, this causes the driver to actually delete such an address 
> and then
> it will no longer receive any traffic.
> 
> To reproduce this:
> 
> a) brctl addbr br0
> b) brctl addif br0 enp
> # enable vlan filtering
> c) echo 1 >/sys/class/net/br0/bridge/vlan_filtering
> d) brctl delbr br0
> 
> Specifically this appears to happen because of how we automatically enter 
> static
> configuration for routes when vlan_filtering is enabled, and we call
> br_fdb_unsync_static which will clear all the routes from the fdb table for 
> the
> device. See commit 2796d0c648c9 ("bridge: Automatically manage port
> promiscuous mode.", 2014-05-16) for more details.
> 
> This happens to include the devices own default address, which results in the
> bug.
> 
> I'm not sure if this is a driver bug, or if it's a bug in the bridging code.
> 
> Who would know more about this and what to do about this?
> 
> One obvious solution is to hard code the i40e device driver so that it does 
> not
> actually delete the HW address from the unicast filter list. This could work, 
> but
> seems to me like its papering over the problem. Is this just a known thing 
> that
> drivers should be aware of? I don't really know...
> 
> An alternative solution would be to possibly ignore any fdb addresses which
> specifically target that port?
> 
> Any ideas?

For the record, adding a check to prevent unsync_static from removing addresses 
which are targetting the specific port does work to resolve this specific 
issue, but I'm sure it's not the correct solution as I expect that would cause 
other problems.

Thanks,
Jake

> 
> Regards,
> Jake


removing bridge in vlan_filtering mode requests delete of attached ports main MAC address

2017-10-20 Thread Keller, Jacob E
Hi,

We've run into an issue with bridges set in vlan_filtering mode. Basically, if 
we attach a device to a bridge which has enabled vlan_filtering, and then 
remove the bridge, we end up requesting the driver of the attached device to 
remove its own MAC HW address.

In i40e, at least, this causes the driver to actually delete such an address 
and then it will no longer receive any traffic.

To reproduce this:

a) brctl addbr br0
b) brctl addif br0 enp
# enable vlan filtering
c) echo 1 >/sys/class/net/br0/bridge/vlan_filtering
d) brctl delbr br0

Specifically this appears to happen because of how we automatically enter 
static configuration for routes when vlan_filtering is enabled, and we call 
br_fdb_unsync_static which will clear all the routes from the fdb table for the 
device. See commit 2796d0c648c9 ("bridge: Automatically manage port promiscuous 
mode.", 2014-05-16) for more details.

This happens to include the devices own default address, which results in the 
bug.

I'm not sure if this is a driver bug, or if it's a bug in the bridging code.

Who would know more about this and what to do about this?

One obvious solution is to hard code the i40e device driver so that it does not 
actually delete the HW address from the unicast filter list. This could work, 
but seems to me like its papering over the problem. Is this just a known thing 
that drivers should be aware of? I don't really know...

An alternative solution would be to possibly ignore any fdb addresses which 
specifically target that port?

Any ideas?

Regards,
Jake