summaryrefslogtreecommitdiffstats
path: root/net/bridge/br_fdb.c
Commit message (Collapse)AuthorAgeFilesLines
* treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 152Thomas Gleixner2019-05-301-5/+1Star
| | | | | | | | | | | | | | | | | | | | | Based on 1 normalized pattern(s): this program is free software you can redistribute it and or modify it under the terms of the gnu general public license as published by the free software foundation either version 2 of the license or at your option any later version extracted by the scancode license scanner the SPDX license identifier GPL-2.0-or-later has been chosen to replace the boilerplate/reference in 3029 file(s). Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Reviewed-by: Allison Randal <allison@lohutok.net> Cc: linux-spdx@vger.kernel.org Link: https://lkml.kernel.org/r/20190527070032.746973796@linutronix.de Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* rhashtable: use bit_spin_locks to protect hash bucket.NeilBrown2019-04-081-1/+0Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch changes rhashtables to use a bit_spin_lock on BIT(1) of the bucket pointer to lock the hash chain for that bucket. The benefits of a bit spin_lock are: - no need to allocate a separate array of locks. - no need to have a configuration option to guide the choice of the size of this array - locking cost is often a single test-and-set in a cache line that will have to be loaded anyway. When inserting at, or removing from, the head of the chain, the unlock is free - writing the new address in the bucket head implicitly clears the lock bit. For __rhashtable_insert_fast() we ensure this always happens when adding a new key. - even when lockings costs 2 updates (lock and unlock), they are in a cacheline that needs to be read anyway. The cost of using a bit spin_lock is a little bit of code complexity, which I think is quite manageable. Bit spin_locks are sometimes inappropriate because they are not fair - if multiple CPUs repeatedly contend of the same lock, one CPU can easily be starved. This is not a credible situation with rhashtable. Multiple CPUs may want to repeatedly add or remove objects, but they will typically do so at different buckets, so they will attempt to acquire different locks. As we have more bit-locks than we previously had spinlocks (by at least a factor of two) we can expect slightly less contention to go with the slightly better cache behavior and reduced memory consumption. To enhance type checking, a new struct is introduced to represent the pointer plus lock-bit that is stored in the bucket-table. This is "struct rhash_lock_head" and is empty. A pointer to this needs to be cast to either an unsigned lock, or a "struct rhash_head *" to be useful. Variables of this type are most often called "bkt". Previously "pprev" would sometimes point to a bucket, and sometimes a ->next pointer in an rhash_head. As these are now different types, pprev is NULL when it would have pointed to the bucket. In that case, 'blk' is used, together with correct locking protocol. Signed-off-by: NeilBrown <neilb@suse.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/netDavid S. Miller2019-01-211-0/+5
|\ | | | | | | | | | | Completely minor snmp doc conflict. Signed-off-by: David S. Miller <davem@davemloft.net>
| * net: bridge: Mark FDB entries that were added by user as suchIdo Schimmel2019-01-191-0/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Externally learned entries can be added by a user or by a switch driver that is notifying the bridge driver about entries that were learned in hardware. In the first case, the entries are not marked with the 'added_by_user' flag, which causes switch drivers to ignore them and not offload them. The 'added_by_user' flag can be set on externally learned FDB entries based on the 'swdev_notify' parameter in br_fdb_external_learn_add(), which effectively means if the created / updated FDB entry was added by a user or not. Fixes: 816a3bed9549 ("switchdev: Add fdb.added_by_user to switchdev notifications") Signed-off-by: Ido Schimmel <idosch@mellanox.com> Reported-by: Alexander Petrovskiy <alexpe@mellanox.com> Reviewed-by: Petr Machata <petrm@mellanox.com> Cc: Roopa Prabhu <roopa@cumulusnetworks.com> Cc: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Cc: bridge@lists.linux-foundation.org Signed-off-by: David S. Miller <davem@davemloft.net>
* | net: Add extack argument to ndo_fdb_add()Petr Machata2019-01-181-1/+2
|/ | | | | | | | | | | | | Drivers may not be able to support certain FDB entries, and an error code is insufficient to give clear hints as to the reasons of rejection. In order to make it possible to communicate the rejection reason, extend ndo_fdb_add() with an extack argument. Adapt the existing implementations of ndo_fdb_add() to take the parameter (and ignore it). Pass the extack parameter when invoking ndo_fdb_add() from rtnl_fdb_add(). Signed-off-by: Petr Machata <petrm@mellanox.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* bridge: support for ndo_fdb_getRoopa Prabhu2018-12-161-0/+26
| | | | | | | | | | This patch implements ndo_fdb_get for the bridge fdb. Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Reviewed-by: David Ahern <dsa@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* bridge: Add br_fdb_clear_offload()Petr Machata2018-12-071-0/+20
| | | | | | | | | | | When a driver unoffloads all FDB entries en bloc, it's inefficient to send the switchdev notification one by one. Add a helper that unsets the offload flag on FDB entries on a given bridge port and VLAN. Signed-off-by: Petr Machata <petrm@mellanox.com> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: Ido Schimmel <idosch@mellanox.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* bridge: switchdev: Allow clearing FDB entry offload indicationIdo Schimmel2018-10-181-2/+2
| | | | | | | | | | | | | | | | | | | Currently, an FDB entry only ceases being offloaded when it is deleted. This changes with VxLAN encapsulation. Devices capable of performing VxLAN encapsulation usually have only one FDB table, unlike the software data path which has two - one in the bridge driver and another in the VxLAN driver. Therefore, bridge FDB entries pointing to a VxLAN device are only offloaded if there is a corresponding entry in the VxLAN FDB. Allow clearing the offload indication in case the corresponding entry was deleted from the VxLAN FDB. Signed-off-by: Ido Schimmel <idosch@mellanox.com> Reviewed-by: Petr Machata <petrm@mellanox.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* net: bridge: explicitly zero is_sticky in fdb_createNikolay Aleksandrov2018-09-281-0/+1
| | | | | | | | | We need to explicitly zero is_sticky when creating a new fdb, otherwise we might get a stale value for a new entry. Fixes: 435f2e7cc0b7 ("net: bridge: add support for sticky fdb entries") Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* net: bridge: add support for sticky fdb entriesNikolay Aleksandrov2018-09-131-3/+16
| | | | | | | | | Add support for entries which are "sticky", i.e. will not change their port if they show up from a different one. A new ndm flag is introduced for that purpose - NTF_STICKY. We allow to set it only to non-local entries. Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* net: bridge: Fix locking in br_fdb_find_port()Petr Machata2018-06-091-1/+3
| | | | | | | | | | | | | | | | Callers of br_fdb_find() need to hold the hash lock, which br_fdb_find_port() doesn't do. However, since br_fdb_find_port() is not doing any actual FDB manipulation, the hash lock is not really needed at all. So convert to br_fdb_find_rcu(), surrounded by rcu_read_lock() / _unlock() pair. The device pointer copied from inside the FDB entry is then kept alive by the RTNL lock, which br_fdb_find_port() asserts. Fixes: 4d4fd36126d6 ("net: bridge: Publish bridge accessor functions") Signed-off-by: Petr Machata <petrm@mellanox.com> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* net: bridge: Notify about !added_by_user FDB entriesPetr Machata2018-05-031-21/+26
| | | | | | | | | | | | Do not automatically bail out on sending notifications about activity on non-user-added FDB entries. Instead, notify about this activity except for cases where the activity itself originates in a notification, to avoid sending duplicate notifications. Signed-off-by: Petr Machata <petrm@mellanox.com> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Acked-by: Ivan Vecera <ivecera@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* net: bridge: Publish bridge accessor functionsPetr Machata2018-04-301-0/+22
| | | | | | | | | | Add a couple new functions to allow querying FDB and vlan settings of a bridge. Signed-off-by: Petr Machata <petrm@mellanox.com> Signed-off-by: Ido Schimmel <idosch@mellanox.com> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* net: bridge: Fix uninitialized error in br_fdb_sync_static()Geert Uytterhoeven2018-02-011-1/+1
| | | | | | | | | | | | | | | | | With gcc-4.1.2.: net/bridge/br_fdb.c: In function ‘br_fdb_sync_static’: net/bridge/br_fdb.c:996: warning: ‘err’ may be used uninitialized in this function Indeed, if the list is empty, err will be uninitialized, and will be propagated up as the function return value. Fix this by preinitializing err to zero. Fixes: eb7935830d00b9e0 ("net: bridge: use rhashtable for fdbs") Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* net: bridge: use rhashtable for fdbsNikolay Aleksandrov2017-12-131-209/+183Star
| | | | | | | | | | | | | | | | | | | | | | | | Before this patch the bridge used a fixed 256 element hash table which was fine for small use cases (in my tests it starts to degrade above 1000 entries), but it wasn't enough for medium or large scale deployments. Modern setups have thousands of participants in a single bridge, even only enabling vlans and adding a few thousand vlan entries will cause a few thousand fdbs to be automatically inserted per participating port. So we need to scale the fdb table considerably to cope with modern workloads, and this patch converts it to use a rhashtable for its operations thus improving the bridge scalability. Tests show the following results (10 runs each), at up to 1000 entries rhashtable is ~3% slower, at 2000 rhashtable is 30% faster, at 3000 it is 2 times faster and at 30000 it is 50 times faster. Obviously this happens because of the properties of the two constructs and is expected, rhashtable keeps pretty much a constant time even with 10000000 entries (tested), while the fixed hash table struggles considerably even above 10000. As a side effect this also reduces the net_bridge struct size from 3248 bytes to 1344 bytes. Also note that the key struct is 8 bytes. Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* bridge: add tracepoint in br_fdb_updateRoopa Prabhu2017-08-311-1/+4
| | | | | | | | | | | | | | This extends bridge fdb table tracepoints to also cover learned fdb entries in the br_fdb_update path. Note that unlike other tracepoints I have moved this to when the fdb is modified because this is in the datapath and can generate a lot of noise in the trace output. br_fdb_update is also called from added_by_user context in the NTF_USE case which is already traced ..hence the !added_by_user check. Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* bridge: fdb add and delete tracepointsRoopa Prabhu2017-08-291-0/+7
| | | | | | | | | | A few useful tracepoints to trace bridge forwarding database updates. Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* net: bridge: Remove FDB deletion through switchdev objectArkadi Sharshevsky2017-08-071-18/+0Star
| | | | | | | | | | | | At this point no driver supports FDB add/del through switchdev object but rather via notification chain, thus, it is removed. Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com> Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com> Reviewed-by: Ivan Vecera <ivecera@redhat.com> Acked-by: Jiri Pirko <jiri@mellanox.com> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* bridge: allow ext learned entries to change portsNikolay Aleksandrov2017-07-041-8/+20
| | | | | | | | | | | current code silently ignores change of port in the request message. This patch makes sure the port is modified and notification is sent to userspace. Fixes: cf6b8e1eedff ("bridge: add API to notify bridge driver of learned FBD on offloaded device") Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* net: bridge: Receive notification about successful FDB offloadArkadi Sharshevsky2017-06-081-1/+21
| | | | | | | | | | | | | | | | | When a new static FDB is added to the bridge a notification is sent to the driver for offload. In case of successful offload the driver should notify the bridge back, which in turn should mark the FDB as offloaded. Currently, externally learned is equivalent for being offloaded which is not correct due to the fact that FDBs which are added from user-space are also marked as externally learned. In order to specify if an FDB was successfully offloaded a new flag is introduced. Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com> Reviewed-by: Ido Schimmel <idosch@mellanox.com> Reviewed-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: Jiri Pirko <jiri@mellanox.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* net: bridge: Add support for notifying devices about FDB add/delArkadi Sharshevsky2017-06-081-0/+2
| | | | | | | | | | | | | | | | | | | | | | | Currently the bridge doesn't notify the underlying devices about new FDBs learned. The FDB sync is placed on the switchdev notifier chain because devices may potentially learn FDB that are not directly related to their ports, for example: 1. Mixed SW/HW bridge - FDBs that point to the ASICs external devices should be offloaded as CPU traps in order to perform forwarding in slow path. 2. EVPN - Externally learned FDBs for the vtep device. Notification is sent only about static FDB add/del. This is done due to fact that currently this is the only scenario supported by switch drivers. Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com> Reviewed-by: Ido Schimmel <idosch@mellanox.com> Signed-off-by: Jiri Pirko <jiri@mellanox.com> Reviewed-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Reviewed-by: Ivan Vecera <ivecera@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* net: bridge: Add support for calling FDB external learning under rcuArkadi Sharshevsky2017-06-081-2/+0Star
| | | | | | | | | | | | This is done as a preparation to moving the switchdev notifier chain to be atomic. The FDB external learning should be called under rtnl or rcu. Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com> Reviewed-by: Ido Schimmel <idosch@mellanox.com> Reviewed-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: Jiri Pirko <jiri@mellanox.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* net: bridge: Fix improper taking over HW learned FDBArkadi Sharshevsky2017-05-011-5/+3Star
| | | | | | | | | | | | | | | | | | | Commit 7e26bf45e4cb ("net: bridge: allow SW learn to take over HW fdb entries") added the ability to "take over an entry which was previously learned via HW when it shows up from a SW port". However, if an entry was learned via HW and then a control packet (e.g., ARP request) was trapped to the CPU, the bridge driver will update the entry and remove the externally learned flag, although the entry is still present in HW. Instead, only clear the externally learned flag in case of roaming. Fixes: 7e26bf45e4cb ("net: bridge: allow SW learn to take over HW fdb entries") Signed-off-by: Ido Schimmel <idosch@mellanox.com> Signed-off-by: Arkadi Sharashevsky <arkadis@mellanox.com> Cc: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* net: bridge: notify on hw fdb takeoverNikolay Aleksandrov2017-04-171-1/+3
| | | | | | | | | Recently we added support for SW fdbs to take over HW ones, but that results in changing a user-visible fdb flag thus we need to send a notification, also it's consistent with how HW takes over SW entries. Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* net: bridge: allow to add externally learned entries from user-spaceNikolay Aleksandrov2017-03-241-0/+2
| | | | | | | | | | | | The NTF_EXT_LEARNED flag was added for switchdev and externally learned entries, but it can also be used for entries learned via a software in user-space which requires dynamic entries that do not expire. One such case that we have is with quagga and evpn which need dynamic entries but also require to age them themselves. Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Reviewed-by: Ido Schimmel <idosch@mellanox.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* net: bridge: allow SW learn to take over HW fdb entriesNikolay Aleksandrov2017-03-241-0/+3
| | | | | | | | | | | Allow to take over an entry which was previously learned via HW when it shows up from a SW port. This is analogous to how HW takes over SW learned entries already. Suggested-by: Roopa Prabhu <roopa@cumulusnetworks.com> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Reviewed-by: Ido Schimmel <idosch@mellanox.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* bridge: resolve a false alarm of lockdepWANG Cong2017-03-171-1/+1
| | | | | | | | | | | | | | Andrei reported a false alarm of lockdep at net/bridge/br_fdb.c:109, this is because in Andrei's case, a spin_bug() was already triggered before this, therefore the debug_locks is turned off, lockdep_is_held() is no longer accurate after that. We should use lockdep_assert_held_once() instead of lockdep_is_held() to respect debug_locks. Fixes: 410b3d48f5111 ("bridge: fdb: add proper lock checks in searching functions") Reported-by: Andrei Vagin <avagin@gmail.com> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* bridge: don't indicate expiry on NTF_EXT_LEARNED fdb entriesRoopa Prabhu2017-02-171-1/+1
| | | | | | | | | | | | | | | | added_by_external_learn fdb entries are added and expired by external entities like switchdev driver or external controllers. ageing is already disabled for such entries. Hence, don't indicate expiry for such fdb entries. CC: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> CC: Jiri Pirko <jiri@resnulli.us> CC: Ido Schimmel <idosch@mellanox.com> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com> Reviewed-by: Ido Schimmel <idosch@mellanox.com> Tested-by: Ido Schimmel <idosch@mellanox.com> Reviewed-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* bridge: fdb: converge fdb_delete_by functions into oneNikolay Aleksandrov2017-02-141-47/+15Star
| | | | | | | | | | We can simplify the logic of entries pointing to the bridge by converging the fdb_delete_by functions, this would allow us to use the same function for both cases since the fdb's dst is set to NULL if it is pointing to the bridge thus we can always check for a port match. Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* bridge: fdb: add proper lock checks in searching functionsNikolay Aleksandrov2017-02-141-0/+4
| | | | | | | | In order to avoid new errors add checks to br_fdb_find and fdb_find_rcu functions. The first requires hash_lock, the second obviously RCU. Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* bridge: fdb: converge fdb searching functions into oneNikolay Aleksandrov2017-02-141-65/+48Star
| | | | | | | | | | | | Before this patch we had 3 different fdb searching functions which was confusing. This patch reduces all of them to one - fdb_find_rcu(), and two flavors: br_fdb_find() which requires hash_lock and br_fdb_find_rcu which requires RCU. This makes it clear what needs to be used, we also remove two abusers of __br_fdb_get which called it under hash_lock and replace them with br_fdb_find(). Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* bridge: avoid unnecessary read of jiffiesstephen hemminger2017-02-071-2/+4
| | | | | | | | Jiffies is volatile so read it once. Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* bridge: fdb: write to used and updated at most once per jiffyNikolay Aleksandrov2017-02-071-1/+2
| | | | | | | | | Writing once per jiffy is enough to limit the bridge's false sharing. After this change the bridge doesn't show up in the local load HitM stats. Suggested-by: David S. Miller <davem@davemloft.net> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* bridge: move to workqueue gcNikolay Aleksandrov2017-02-071-12/+19
| | | | | | | | | | | | | Move the fdb garbage collector to a workqueue which fires at least 10 milliseconds apart and cleans chain by chain allowing for other tasks to run in the meantime. When having thousands of fdbs the system is much more responsive. Most importantly remove the need to check if the matched entry has expired in __br_fdb_get that causes false-sharing and is completely unnecessary if we cleanup entries, at worst we'll get 10ms of traffic for that entry before it gets deleted. Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* bridge: add address and vlan to fdb warning messagesRoopa Prabhu2016-10-131-6/+4Star
| | | | | | | | This patch adds vlan and address to warning messages printed in the bridge fdb code for debuggability. Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* rtnetlink: fdb dump: optimize by saving last interface markersRoopa Prabhu2016-09-021-11/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | fdb dumps spanning multiple skb's currently restart from the first interface again for every skb. This results in unnecessary iterations on the already visited interfaces and their fdb entries. In large scale setups, we have seen this to slow down fdb dumps considerably. On a system with 30k macs we see fdb dumps spanning across more than 300 skbs. To fix the problem, this patch replaces the existing single fdb marker with three markers: netdev hash entries, netdevs and fdb index to continue where we left off instead of restarting from the first netdev. This is consistent with link dumps. In the process of fixing the performance issue, this patch also re-implements fix done by commit 472681d57a5d ("net: ndo_fdb_dump should report -EMSGSIZE to rtnl_fdb_dump") (with an internal fix from Wilson Kok) in the following ways: - change ndo_fdb_dump handlers to return error code instead of the last fdb index - use cb->args strictly for dump frag markers and not error codes. This is consistent with other dump functions. Below results were taken on a system with 1000 netdevs and 35085 fdb entries: before patch: $time bridge fdb show | wc -l 15065 real 1m11.791s user 0m0.070s sys 1m8.395s (existing code does not return all macs) after patch: $time bridge fdb show | wc -l 35085 real 0m2.017s user 0m0.113s sys 0m1.942s Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com> Signed-off-by: Wilson Kok <wkok@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* bridge: Fix problems around fdb entries pointing to the bridge deviceToshiaki Makita2016-08-101-25/+27
| | | | | | | | | | | | | | | | | | | | Adding fdb entries pointing to the bridge device uses fdb_insert(), which lacks various checks and does not respect added_by_user flag. As a result, some inconsistent behavior can happen: * Adding temporary entries succeeds but results in permanent entries. * Same goes for "dynamic" and "use". * Changing mac address of the bridge device causes deletion of user-added entries. * Replacing existing entries looks successful from userspace but actually not, regardless of NLM_F_EXCL flag. Use the same logic as other entries and fix them. Fixes: 3741873b4f73 ("bridge: allow adding of fdb entries pointing to the bridge device") Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* bridge: Don't insert unnecessary local fdb entry on changing mac addressToshiaki Makita2016-06-081-0/+2
| | | | | | | | | | | | The missing br_vlan_should_use() test caused creation of an unneeded local fdb entry on changing mac address of a bridge device when there is a vlan which is configured on a bridge port but not on the bridge device. Fixes: 2594e9064a57 ("bridge: vlan: add per-vlan struct and move to rhashtables") Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* net: ndo_fdb_dump should report -EMSGSIZE to rtnl_fdb_dump.MINOURA Makoto / 箕浦 真2016-02-261-5/+10
| | | | | | | | | | | | When the send skbuff reaches the end, nlmsg_put and friends returns -EMSGSIZE but it is silently thrown away in ndo_fdb_dump. It is called within a for_each_netdev loop and the first fdb entry of a following netdev could fit in the remaining skbuff. This breaks the mechanism of cb->args[0] and idx to keep track of the entries that are already dumped, which results missing entries in bridge fdb show command. Signed-off-by: Minoura Makoto <minoura@valinux.co.jp> Signed-off-by: David S. Miller <davem@davemloft.net>
* switchdev: Pass original device to port netdev driverIdo Schimmel2015-12-151-0/+1
| | | | | | | | | | | | | | | | | | | | switchdev drivers need to know the netdev on which the switchdev op was invoked. For example, the STP state of a VLAN interface configured on top of a port can change while being member in a bridge. In this case, the underlying driver should only change the STP state of that particular VLAN and not of all the VLANs configured on the port. However, current switchdev infrastructure only passes the port netdev down to the driver. Solve that by passing the original device down to the driver as part of the required switchdev object / attribute. This doesn't entail any change in current switchdev drivers. It simply enables those supporting stacked devices to know the originating device and act accordingly. Signed-off-by: Ido Schimmel <idosch@mellanox.com> Signed-off-by: Jiri Pirko <jiri@mellanox.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* bridge: set is_local and is_static before fdb entry is added to the fdb ↵Roopa Prabhu2015-10-301-8/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | hashtable Problem Description: We can add fdbs pointing to the bridge with NULL ->dst but that has a few race conditions because br_fdb_insert() is used which first creates the fdb and then, after the fdb has been published/linked, sets "is_local" to 1 and in that time frame if a packet arrives for that fdb it may see it as non-local and either do a NULL ptr dereference in br_forward() or attach the fdb to the port where it arrived, and later br_fdb_insert() will make it local thus getting a wrong fdb entry. Call chain br_handle_frame_finish() -> br_forward(): But in br_handle_frame_finish() in order to call br_forward() the dst should not be local i.e. skb != NULL, whenever the dst is found to be local skb is set to NULL so we can't forward it, and here comes the problem since it's running only with RCU when forwarding packets it can see the entry before "is_local" is set to 1 and actually try to dereference NULL. The main issue is that if someone sends a packet to the switch while it's adding the entry which points to the bridge device, it may dereference NULL ptr. This is needed now after we can add fdbs pointing to the bridge. This poses a problem for br_fdb_update() as well, while someone's adding a bridge fdb, but before it has is_local == 1, it might get moved to a port if it comes as a source mac and then it may get its "is_local" set to 1 This patch changes fdb_create to take is_local and is_static as arguments to set these values in the fdb entry before it is added to the hash. Also adds null check for port in br_forward. Fixes: 3741873b4f73 ("bridge: allow adding of fdb entries pointing to the bridge device") Reported-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com> Reviewed-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Acked-by: Stephen Hemminger <stephen@networkplumber.org> Signed-off-by: David S. Miller <davem@davemloft.net>
* bridge: defer switchdev fdb del call in fdb_del_external_learnJiri Pirko2015-10-151-1/+4
| | | | | | | | | Since spinlock is held here, defer the switchdev operation. Also, ensure that defered switchdev ops are processed before port master device is unlinked. Signed-off-by: Jiri Pirko <jiri@mellanox.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* switchdev: remove pointers from switchdev objectsJiri Pirko2015-10-151-1/+1
| | | | | | | | | | | When object is used in deferred work, we cannot use pointers in switchdev object structures because the memory they point at may be already used by someone else. So rather do local copy of the value. Signed-off-by: Jiri Pirko <jiri@mellanox.com> Acked-by: Scott Feldman <sfeldma@gmail.com> Reviewed-by: John Fastabend <john.r.fastabend@intel.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* bridge: allow adding of fdb entries pointing to the bridge deviceRoopa Prabhu2015-10-121-30/+92
| | | | | | | | | | | | | | | | | This patch enables adding of fdb entries pointing to the bridge device. This can be used to propagate mac address of vlan interfaces configured on top of the vlan filtering bridge. Before: $bridge fdb add 44:38:39:00:27:9f dev bridge RTNETLINK answers: Invalid argument After: $bridge fdb add 44:38:39:00:27:9f dev bridge Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com> Reviewed-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* switchdev: push object ID back to object structureJiri Pirko2015-10-031-2/+2
| | | | | | | | Suggested-by: Scott Feldman <sfeldma@gmail.com> Signed-off-by: Jiri Pirko <jiri@mellanox.com> Acked-by: Scott Feldman <sfeldma@gmail.com> Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* switchdev: bring back switchdev_obj and use it as a generic object paramJiri Pirko2015-10-031-1/+2
| | | | | | | | | | Replace "void *obj" with a generic structure. Introduce couple of helpers along that. Signed-off-by: Jiri Pirko <jiri@mellanox.com> Acked-by: Scott Feldman <sfeldma@gmail.com> Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* switchdev: rename switchdev_obj_fdb to switchdev_obj_port_fdbJiri Pirko2015-10-031-1/+1
| | | | | | | | | | Make the struct name in sync with object id name. Suggested-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com> Signed-off-by: Jiri Pirko <jiri@mellanox.com> Acked-by: Scott Feldman <sfeldma@gmail.com> Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* switchdev: rename SWITCHDEV_OBJ_* enum values to SWITCHDEV_OBJ_ID_*Jiri Pirko2015-10-031-1/+1
| | | | | | | | Suggested-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com> Signed-off-by: Jiri Pirko <jiri@mellanox.com> Acked-by: Scott Feldman <sfeldma@gmail.com> Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* net: switchdev: abstract object in add/del opsVivien Didelot2015-09-301-7/+4Star
| | | | | | | | | | | | | | | | | Similar to the notifier_call callback of a notifier_block, change the function signature of switchdev add and del operations to: int switchdev_port_obj_add/del(struct net_device *dev, enum switchdev_obj_id id, void *obj); This allows the caller to pass a specific switchdev_obj_* structure instead of the generic switchdev_obj one. Drivers implementation of these operations and switchdev have been changed accordingly. Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* bridge: vlan: add per-vlan struct and move to rhashtablesNikolay Aleksandrov2015-09-291-37/+39
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch changes the bridge vlan implementation to use rhashtables instead of bitmaps. The main motivation behind this change is that we need extensible per-vlan structures (both per-port and global) so more advanced features can be introduced and the vlan support can be extended. I've tried to break this up but the moment net_port_vlans is changed and the whole API goes away, thus this is a larger patch. A few short goals of this patch are: - Extensible per-vlan structs stored in rhashtables and a sorted list - Keep user-visible behaviour (compressed vlans etc) - Keep fastpath ingress/egress logic the same (optimizations to come later) Here's a brief list of some of the new features we'd like to introduce: - per-vlan counters - vlan ingress/egress mapping - per-vlan igmp configuration - vlan priorities - avoid fdb entries replication (e.g. local fdb scaling issues) The structure is kept single for both global and per-port entries so to avoid code duplication where possible and also because we'll soon introduce "port0 / aka bridge as port" which should simplify things further (thanks to Vlad for the suggestion!). Now we have per-vlan global rhashtable (bridge-wide) and per-vlan port rhashtable, if an entry is added to a port it'll get a pointer to its global context so it can be quickly accessed later. There's also a sorted vlan list which is used for stable walks and some user-visible behaviour such as the vlan ranges, also for error paths. VLANs are stored in a "vlan group" which currently contains the rhashtable, sorted vlan list and the number of "real" vlan entries. A good side-effect of this change is that it resembles how hw keeps per-vlan data. One important note after this change is that if a VLAN is being looked up in the bridge's rhashtable for filtering purposes (or to check if it's an existing usable entry, not just a global context) then the new helper br_vlan_should_use() needs to be used if the vlan is found. In case the lookup is done only with a port's vlan group, then this check can be skipped. Things tested so far: - basic vlan ingress/egress - pvids - untagged vlans - undef CONFIG_BRIDGE_VLAN_FILTERING - adding/deleting vlans in different scenarios (with/without global ctx, while transmitting traffic, in ranges etc) - loading/removing the module while having/adding/deleting vlans - extracting bridge vlan information (user ABI), compressed requests - adding/deleting fdbs on vlans - bridge mac change, promisc mode - default pvid change - kmemleak ON during the whole time Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>