netdev - Re: [PATCH net-next v2 00/13] Change DSA's FDB API and perform switchdev cleanup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 20 Jul 2017 12:04:46 +0300
From:   Arkadi Sharshevsky <arkadis@...lanox.com>
To:     Vivien Didelot <vivien.didelot@...oirfairelinux.com>,
        netdev@...r.kernel.org
Cc:     davem@...emloft.net, jiri@...nulli.us, ivecera@...hat.com,
        f.fainelli@...il.com, andrew@...n.ch, Woojung.Huh@...rochip.com,
        stephen@...workplumber.org, mlxsw@...lanox.com
Subject: Re: [PATCH net-next v2 00/13] Change DSA's FDB API and perform
 switchdev cleanup



On 07/19/2017 11:17 PM, Vivien Didelot wrote:
> Hi Arkadi,
> 
> I am testing your patch series the behavior changes suspiciously:
> 
>     # brctl show br0
>     bridge name	bridge id		STP enabled	interfaces
>     br0		8000.f6d5ef06ccdd	no		lan0
>                                                         lan1
>                                                         lan2
>                                                         lan3
>                                                         lan4
>                                                         lan5
>                                                         lan6
>                                                         lan7
>                                                         lan8
>                                                         optical3
>                                                         optical4
> 
> Without the patchset I have this behavior:
> 
>     # bridge fdb add 00:11:22:33:44:55 dev lan4
>     # bridge fdb add 22:33:44:55:66:77 dev lan2 self
>     # bridge fdb show                               
>     01:00:5e:00:00:01 dev eth0 self permanent
>     01:00:5e:00:00:01 dev eth1 self permanent
>     0a:3f:f6:06:a2:ee dev lan0 master br0 permanent
>     22:33:44:55:66:77 dev lan2 self static
>     00:11:22:33:44:55 dev lan4 self static
>     01:00:5e:00:00:01 dev br0 self permanent
> 
> And now with the patchset applied I have:
> 
> # bridge fdb add 00:11:22:33:44:55 dev lan4
> # bridge fdb add 22:33:44:55:66:77 dev lan2 self
> # bridge fdb show
> 01:00:5e:00:00:01 dev eth0 self permanent
> 00:11:22:33:44:55 dev eth1 self permanent
> 22:33:44:55:66:77 dev eth1 self permanent
> 01:00:5e:00:00:01 dev eth1 self permanent
> 0a:ca:c8:6b:05:65 dev lan0 master br0 permanent
> 01:00:5e:00:00:01 dev br0 self permanent
> 
> 
> It looks like the FDB entries are reported to be associated with the
> master net device (eth1). Is the dump broken or is it the whole add?
> 
> Thanks,
> 
>         Vivien
> 

Hi, thanks for the test. If the fdb is marked as self its not in the
bridge at all. So before my patch it was OK because you supported the
self thing.

Please notice that both fdbs you added are marked the same because the
default is self: vim bridge/fdb.c +499 (I think this is a bug because
the man page states that master is the default). So in order to put it
in the bridge you should specify "master":

$bridge fdb add e4:1d:2d:a5:f0:4a dev sw1p7 master
$bridge fdb show brport sw1p7
e4:1d:2d:a5:f0:4a vlan 1 offload master br0 permanent <---also should
e4:1d:2d:46:13:f1 vlan 1 master br0 permanent             be offloaded*
e4:1d:2d:46:13:f1 master br0 permanent
e4:1d:2d:a5:f0:4a offload master br0 permanent
33:33:00:00:00:01 self permanent
01:00:5e:00:00:01 self permanent
33:33:ff:46:13:f1 self permanent

*you should take the latest iproute.

Also it seems strange that I removed the self support from the driver
but you still managed to configure it. The reason is the default
self implementation:

vim net/core/rtnetlink.c +3112

I think it is relevant for NICs mostly, so we can ignore it.