- 24 Feb, 2020 5 commits
-
-
Jiri Pirko authored
Commit 0791051c ("mlxsw: core: Create a generic function to register / unregister traps") moved this field to struct mlxsw_listener, but forgot to remove it from struct mlxsw_rx_listener. Remove the unused field. Signed-off-by:
Jiri Pirko <jiri@mellanox.com> Signed-off-by:
Ido Schimmel <idosch@mellanox.com> Signed-off-by:
David S. Miller <davem@davemloft.net>
-
Jiri Pirko authored
No need to initialize a single policer multiple times for each group. So move the initialization to be done from mlxsw_sp_trap_init(), making the function much simpler. Also, rename it so it is with sync with spectrum.c policers initialization. Signed-off-by:
-
Jiri Pirko authored
The Trap / Discard Action action got renamed in PRM, so rename it in the code as well. Signed-off-by:
-
Jiri Pirko authored
No need to have forward declarations for mlxsw_sp_rx_drop_listener() and mlxsw_sp_rx_exception_listener(). Just move them up and avoid it. Signed-off-by:
-
Jiri Pirko authored
When calling mlxsw_sp_rx_listener(), use err variable instead of directly checking func return value. Signed-off-by:
-
- 23 Feb, 2020 14 commits
-
-
Heiner Kallweit authored
These constants are used in one place only, so we can remove them and use the values directly. Signed-off-by:
Heiner Kallweit <hkallweit1@gmail.com> Signed-off-by:
-
David S. Miller authored
Ido Schimmel says: ==================== mlxsw: Remove RTNL from route insertion path This patch set removes RTNL from the route insertion path in mlxsw in order to reduce the control plane latency: the time it takes to push routes from user space to the kernel and mlxsw. Up until now mlxsw did not have a lock to protect its shared router data structures and instead relied on RTNL. While this was simple and worked, it resulted in large control plane latencies as RTNL was heavily contended - by both the task receiving the netlink messages from user space and the mlxsw workqueue that programs the routes to the device. By removing RTNL and introducing a new router mutex, this patch set reduces the control plane latency by ~80%. A single mutex is added as inside mlxsw there is not a lot of concurrency. In addition, a more fine-grained locking scheme is much more error-prone. Patches #1-#6 are preparations. They add needed locking in NVE and multicast routing code instead of relying on RTNL Patch #7 introduces the new mutex Patches #8-#12 gradually take the lock in various entry points into the routing code Patch #13 removes RTNL in places where it is no longer required ==================== Signed-off-by: -
Ido Schimmel authored
After introducing the router lock in previous patches and making sure it protects internal router structures, we no longer need to rely on RTNL to serialize access to these structures. Remove RTNL from call sites that no longer require it. Two calls sites that keep taking the lock are mlxsw_sp_router_fibmr_event_work() and mlxsw_sp_inet6addr_event_work(). The first calls into ACL code that still assumes RTNL is taken. The second potentially calls into the FID code that also relies on RTNL. Removing RTNL from these two call sites is the subject of future work. Signed-off-by:
-
Ido Schimmel authored
The routing code exports some helper functions that can be called from other driver modules such as the bridge. These helpers are never called with the router lock already held and therefore need to take it in order to serialize access to shared router structures. Signed-off-by:
-
Ido Schimmel authored
Another entry point into the routing code is from inetaddr listeners. The driver registers listeners to IPv4 and IPv6 inetaddr notification chains in order to understand when a RIF needs to be created or destroyed. Serialize access to shared router structures from these listeners by taking the router lock when processing inetaddr events. Signed-off-by:
-
Ido Schimmel authored
One entry point into the routing code is from the netdev listener block. Some netdev events require access to internal router structures. For example, changing the MTU of a netdev requires looking-up the backing RIF and adjusting its MTU. In order to serialize access to shared router structures, take the router lock when processing netdev events that require access to it. Signed-off-by:
-
Ido Schimmel authored
The dpipe code traverses internal router structures such as neighbours and adjacency entries and dumps them to user space via netlink. Up until now the routing code did not have its own locks and relied on RTNL lock to serialize access. This is going to change with the introduction of the router lock. Take the router lock in the code paths where RTNL lock is currently taken so that the latter could be removed by subsequent patches. Signed-off-by:
-
Ido Schimmel authored
There are several work items in the routing code that currently rely on RTNL lock to guard against concurrent changes. Have these work items acquire the router lock in preparation for the removal for RTNL lock from the routing code. Signed-off-by:
-
Ido Schimmel authored
Introduce a mutex to protect the internal structure of the routing code. A single lock is added instead of a more fine-grained and complicated locking scheme because there is not a lot of concurrency in the routing code. The main motivation is remove the dependence on RTNL lock, which is currently used by both the process pushing routes to the kernel and the workqueue pushing the routes to the underlying device. Signed-off-by:
-
Ido Schimmel authored
When a host route is added, the driver checks if the route needs to be promoted to perform NVE decapsulation based on the current NVE configuration. If so, the index of the decapsulation entry is retrieved and associated with the route. Currently, this information is stored in the NVE module which the router module consults. Since the information is protected under RTNL and since route insertion happens with RTNL held, there is no problem to retrieve the information from the NVE module. However, this is going to change and route insertion will no longer happen under RTNL. Instead, a dedicated lock will be introduced for the router module. Therefore, store this information in the router module and change the router module to consult this copy. The validity of the information is set / cleared whenever an NVE tunnel is initialized / de-initialized. When this happens the NVE module calls into the router module to promote / demote the relevant host route. Signed-off-by:
-
Ido Schimmel authored
The dpipe code accesses internal router data structures and acquires RTNL to protect against their changes. Subsequent patches will remove reliance on RTNL and introduce a dedicated lock to protect router data structures. Publish the router struct to internal users such as the dpipe, so that they could acquire it instead of RTNL. Signed-off-by:
-
Ido Schimmel authored
Protect the per-table multicast route list with a lock and remove RTNL from the delayed work that periodically updates the kernel about packets and bytes statistics from each multicast route. Signed-off-by:
-
Ido Schimmel authored
The multicast table list is traversed from a delayed work that periodically updates the kernel about packets and bytes statistics from each multicast route. The list is currently protected by RTNL, but subsequent patches will remove the driver's dependence on this contended lock. In order to be able to remove dependence on RTNL in the next patch, guard this list with a dedicated mutex. Signed-off-by:
-
Ido Schimmel authored
The driver periodically traverses the linked list of multicast routes and updates the kernel about packets and bytes statistics from each multicast route. These statistics are read from a counter associated with the route when it is written to the device. Currently, multicast routes are published via this linked list before they are associated with a counter. Despite that, it is not possible for the driver to access an invalid counter because the delayed work that reads the statistics and multicast route addition / deletion are mutually exclusive using RTNL. In order to be able to remove RTNL, the list needs to be protected by a dedicated lock, but any route published via the list must have an associated counter, otherwise the driver will access an invalid counter. Solve this by re-ordering the operations during multicast route addition so that the route is only added to the linked list after it was written to the device. Similarly, during deletion the route is first removed from the linked list before its deletion from the device. Signed-off-by:
-
- 21 Feb, 2020 21 commits
-
-
David S. Miller authored
Saeed Mahameed says: ==================== mlxfw: Improve error reporting and FW reactivate support This patchset improves mlxfw error reporting to netlink and to kernel log. V2: - Use proper err codes, EBUSY/EIO instead of EALREADY/EREMOTEIO - Fix typo. From Eran and me. 1) patch #1, Make mlxfw/mlxsw fw flash devlink status notify generic, and enable it for mlx5. 2) patches #2..#5 are improving mlxfw flash error messages by reporting detailed mlxfw FSM error messages to netlink and kernel log. 3) patches #6,7 From Eran: Add FW reactivate flow to mlxfw and mlx5 ==================== Reviewed-by:
Jakub Kicinski <kuba@kernel.org> Signed-off-by:
-
Eran Ben Elisha authored
Add support for fsm reactivate via MIRC (Management Image Re-activation Control) set and query commands. For re-activation flow, driver shall first run MIRC set, and then wait until FW is done (via querying MIRC status). Signed-off-by:
Eran Ben Elisha <eranbe@mellanox.com> Signed-off-by:
Saeed Mahameed <saeedm@mellanox.com> Acked-by:
-
Eran Ben Elisha authored
Expose fsm_reactivate callback to the mlxfw_dev_ops struct. FSM reactivate is needed before flashing the new image in order to flush the old flashed but not running firmware image. In case mlxfw_dev do not support the reactivation, this step will be skipped. But if later image flash will fail, a hint will be provided by the extack to advise the user that the failure might be related to it. Signed-off-by:
-
Saeed Mahameed authored
Instead of always calling both mlxfw_err and NL_SET_ERR_MSG_MOD with the same message, use the dedicated macro instead. Signed-off-by:
-
Saeed Mahameed authored
Introduce mlxfw_{info, err, dbg} macros and make them call corresponding dev_* macros, then convert all instances of pr_* to mlxfw_*. This will allow printing the device name mlxfw is operating on. Signed-off-by: -
Saeed Mahameed authored
Make sure mlxfw_firmware_flash reports a detailed user readable error message in every possible error path, basically every time mlxfw_dev->ops->*() is called and an error is returned, or when image initialization is failed. Signed-off-by:
-
Saeed Mahameed authored
Report unique and standard error codes corresponding to the specific FW flash error. In addition, add a more detailed error messages to netlink. Before: $ devlink dev flash pci/0000:05:00.0 file ... Error: mlxfw: Firmware flash failed. devlink answers: Invalid argument After: $ devlink dev flash pci/0000:05:00.0 file ... Error: mlxfw: Firmware flash failed: pending reset. devlink answers: Device busy Signed-off-by:
-
Saeed Mahameed authored
FW flash status notify is currently implemented via a callback to the caller mlx module, and all it is doing is to call devlink_flash_update_status_notify with the specific module devlink instance. Instead of repeating the whole process for all mlx modules and re-implement the status_notify callback again and again. Just provide the devlink instance as part of mlxfw_dev when calling mlxfw_firmware_flash and let mlxfw do the devlink status updates directly. This will be very useful for adding status notify support to mlx5, as already done in this patch, with a simple one line of just providing the devlink instance to mlxfw_firmware_flash. mlxfw now depends on NET_DEVLINK as all other mlx modules. Signed-off-by:
-
git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-nextDavid S. Miller authored
Daniel Borkmann says: ==================== pull-request: bpf-next 2020-02-21 The following pull-request contains BPF updates for your *net-next* tree. We've added 25 non-merge commits during the last 4 day(s) which contain a total of 33 files changed, 2433 insertions(+), 161 deletions(-). The main changes are: 1) Allow for adding TCP listen sockets into sock_map/hash so they can be used with reuseport BPF programs, from Jakub Sitnicki. 2) Add a new bpf_program__set_attach_target() helper for adding libbpf support to specify the tracepoint/function dynamically, from Eelco Chaudron. 3) Add bpf_read_branch_records() BPF helper which helps use cases like profile guided optimizations, from Daniel Xu. 4) Enable bpf_perf_event_read_value() in all tracing programs, from Song Liu. 5) Relax BTF mandatory check if only used for libbpf itself e.g. to process BTF defined maps, from Andrii Nakryiko. 6) Move BPF selftests -mcpu compilation attribute from 'probe' to 'v3' as it has been observed that former fails in envs with low memlock, from Yonghong Song. ==================== Signed-off-by: -
git://git.kernel.org/pub/scm/linux/kernel/git/netdev/netDavid S. Miller authored
Conflict resolution of ice_virtchnl_pf.c based upon work by Stephen Rothwell. Signed-off-by: -
Daniel Borkmann authored
Jakub Sitnicki says: ==================== This patch set turns SOCK{MAP,HASH} into generic collections for TCP sockets, both listening and established. Adding support for listening sockets enables us to use these BPF map types with reuseport BPF programs. Why? SOCKMAP and SOCKHASH, in comparison to REUSEPORT_SOCKARRAY, allow the socket to be in more than one map at the same time. Having a BPF map type that can hold listening sockets, and gracefully co-exist with reuseport BPF is important if, in the future, we want BPF programs that run at socket lookup time [0]. Cover letter for v1 of this series tells the full story of how we got here [1]. Although SOCK{MAP,HASH} are not a drop-in replacement for SOCKARRAY just yet, because UDP support is lacking, it's a step in this direction. We're working with Lorenz on extending SOCK{MAP,HASH} to hold UDP sockets, and expect to post RFC series for sockmap + UDP in the near future. I've dropped Acks from all patches that have been touched since v6. The audit for missing READ_ONCE annotations for access to sk_prot is ongoing. Thus far I've found one location specific to TCP listening sockets that needed annotating. This got fixed it in this iteration. I wonder if sparse checker could be put to work to identify places where we have sk_prot access while not holding sk_lock... The patch series depends on another one, posted earlier [2], that has been split out of it. v6 -> v7: - Extended the series to cover SOCKHASH. (patches 4-8, 10-11) (John) - Rebased onto recent bpf-next. Resolved conflicts in recent fixes to sk_state checks on sockmap/sockhash update path. (patch 4) - Added missing READ_ONCE annotation in sock_copy. (patch 1) - Split out patches that simplify sk_psock_restore_proto [2]. v5 -> v6: - Added a fix-up for patch 1 which I forgot to commit in v5. Sigh. v4 -> v5: - Rebase onto recent bpf-next to resolve conflicts. (Daniel) v3 -> v4: - Make tcp_bpf_clone parameter names consistent across function declaration and definition. (Martin) - Use sock_map_redirect_okay helper everywhere we need to take a different action for listening sockets. (Lorenz) - Expand comment explaining the need for a callback from reuseport to sockarray code in reuseport_detach_sock. (Martin) - Mention the possibility of using a u64 counter for reuseport IDs in the future in the description for patch 10. (Martin) v2 -> v3: - Generate reuseport ID when group is created. Please see patch 10 description for details. (Martin) - Fix the build when CONFIG_NET_SOCK_MSG is not selected by either CONFIG_BPF_STREAM_PARSER or CONFIG_TLS. (kbuild bot & John) - Allow updating sockmap from BPF on BPF_SOCK_OPS_TCP_LISTEN_CB callback. An oversight in previous iterations. Users may want to populate the sockmap with listening sockets from BPF as well. - Removed RCU read lock assertion in sock_map_lookup_sys. (Martin) - Get rid of a warning when child socket was cloned with parent's psock state. (John) - Check for tcp_bpf_unhash rather than tcp_bpf_recvmsg when deciding if sk_proto needs restoring on clone. Check for recvmsg in the context of listening socket cloning was confusing. (Martin) - Consolidate sock_map_sk_is_suitable with sock_map_update_okay. This led to adding dedicated predicates for sockhash. Update self-tests accordingly. (John) - Annotate unlikely branch in bpf_{sk,msg}_redirect_map when socket isn't in a map, or isn't a valid redirect target. (John) - Document paired READ/WRITE_ONCE annotations and cover shared access in more detail in patch 2 description. (John) - Correct a couple of log messages in sockmap_listen self-tests so the message reflects the actual failure. - Rework reuseport tests from sockmap_listen suite so that ENOENT error from bpf_sk_select_reuseport handler does not happen on happy path. v1 -> v2: - af_ops->syn_recv_sock callback is no longer overridden and burdened with restoring sk_prot and clearing sk_user_data in the child socket. As child socket is already hashed when syn_recv_sock returns, it is too late to put it in the right state. Instead patches 3 & 4 address restoring sk_prot and clearing sk_user_data before we hash the child socket. (Pointed out by Martin Lau) - Annotate shared access to sk->sk_prot with READ_ONCE/WRITE_ONCE macros as we write to it from sk_msg while socket might be getting cloned on another CPU. (Suggested by John Fastabend) - Convert tests for SOCKMAP holding listening sockets to return-on-error style, and hook them up to test_progs. Also use BPF skeleton for setup. Add new tests to cover the race scenario discovered during v1 review. RFC -> v1: - Switch from overriding proto->accept to af_ops->syn_recv_sock, which happens earlier. Clearing the psock state after accept() does not work for child sockets that become orphaned (never got accepted). v4-mapped sockets need special care. - Return the socket cookie on SOCKMAP lookup from syscall to be on par with REUSEPORT_SOCKARRAY. Requires SOCKMAP to take u64 on lookup/update from syscall. - Make bpf_sk_redirect_map (ingress) and bpf_msg_redirect_map (egress) SOCKMAP helpers fail when target socket is a listening one. - Make bpf_sk_select_reuseport helper fail when target is a TCP established socket. - Teach libbpf to recognize SK_REUSEPORT program type from section name. - Add a dedicated set of tests for SOCKMAP holding listening sockets, covering map operations, overridden socket callbacks, and BPF helpers. [0] https://lore.kernel.org/bpf/20190828072250.29828-1-jakub@cloudflare.com/ [1] https://lore.kernel.org/bpf/20191123110751.6729-1-jakub@cloudflare.com/ [2] https://lore.kernel.org/bpf/20200217121530.754315-1-jakub@cloudflare.com/ ==================== Signed-off-by:Daniel Borkmann <daniel@iogearbox.net>
-
Jakub Sitnicki authored
Now that SOCKMAP and SOCKHASH map types can store listening sockets, user-space and BPF API is open to a new set of potential pitfalls. Exercise the map operations, with extra attention to code paths susceptible to races between map ops and socket cloning, and BPF helpers that work with SOCKMAP/SOCKHASH to gain confidence that all works as expected. Signed-off-by:
Jakub Sitnicki <jakub@cloudflare.com> Signed-off-by:
John Fastabend <john.fastabend@gmail.com> Link: https://lore.kernel.org/bpf/20200218171023.844439-12-jakub@cloudflare.com
-
Jakub Sitnicki authored
Parametrize the SK_REUSEPORT tests so that the map type for storing sockets is not hard-coded in the test setup routine. This, together with careful state cleaning after the tests, lets us run the test cases for REUSEPORT_ARRAY, SOCKMAP, and SOCKHASH to have test coverage for all supported map types. The last two support only TCP sockets at the moment. Signed-off-by:
-
Jakub Sitnicki authored
Commit 736b4602 ("net: Add ID (if needed) to sock_reuseport and expose reuseport_lock") has introduced lazy generation of reuseport group IDs that survive group resize. By comparing the identifier we check if BPF reuseport program is not trying to select a socket from a BPF map that belongs to a different reuseport group than the one the packet is for. Because SOCKARRAY used to be the only BPF map type that can be used with reuseport BPF, it was possible to delay the generation of reuseport group ID until a socket from the group was inserted into BPF map for the first time. Now that SOCK{MAP,HASH} can be used with reuseport BPF we have two options, either generate the reuseport ID on map update, like SOCKARRAY does, or allocate an ID from the start when reuseport group gets created. This patch takes the latter approach to keep sockmap free of calls into reuseport code. This streamlines the reuseport_id access as its lifetime now matches the longevity of reuseport object. The cost of this simplification, however, is that we allocate reuseport IDs for all SO_REUSEPORT users. Even those that don't use SOCKARRAY in their setups. With the way identifiers are currently generated, we can have at most S32_MAX reuseport groups, which hopefully is sufficient. If we ever get close to the limit, we can switch an u64 counter like sk_cookie. Another change is that we now always call into SOCKARRAY logic to unlink the socket from the map when unhashing or closing the socket. Previously we did it only when at least one socket from the group was in a BPF map. It is worth noting that this doesn't conflict with sockmap tear-down in case a socket is in a SOCK{MAP,HASH} and belongs to a reuseport group. sockmap tear-down happens first: prot->unhash `- tcp_bpf_unhash |- tcp_bpf_remove | `- while (sk_psock_link_pop(psock)) | `- sk_psock_unlink | `- sock_map_delete_from_link | `- __sock_map_delete | `- sock_map_unref | `- sk_psock_put | `- sk_psock_drop | `- rcu_assign_sk_user_data(sk, NULL) `- inet_unhash `- reuseport_detach_sock `- bpf_sk_reuseport_detach `- WRITE_ONCE(sk->sk_user_data, NULL) Suggested-by:
Martin Lau <kafai@fb.com> Signed-off-by:
-
Jakub Sitnicki authored
SOCKMAP & SOCKHASH now support storing references to listening sockets. Nothing keeps us from using these map types a collection of sockets to select from in BPF reuseport programs. Whitelist the map types with the bpf_sk_select_reuseport helper. The restriction that the socket has to be a member of a reuseport group still applies. Sockets in SOCKMAP/SOCKHASH that don't have sk_reuseport_cb set are not a valid target and we signal it with -EINVAL. The main benefit from this change is that, in contrast to REUSEPORT_SOCKARRAY, SOCK{MAP,HASH} don't impose a restriction that a listening socket can be just one BPF map at the same time. Signed-off-by: -
Jakub Sitnicki authored
Don't require the kernel code, like BPF helpers, that needs access to SOCK{MAP,HASH} map contents to live in net/core/sock_map.c. Expose the lookup operation to all kernel-land. Lookup from BPF context is not whitelisted yet. While syscalls have a dedicated lookup handler. Signed-off-by: -
Jakub Sitnicki authored
Tooling that populates the SOCK{MAP,HASH} with sockets from user-space needs a way to inspect its contents. Returning the struct sock * that the map holds to user-space is neither safe nor useful. An approach established by REUSEPORT_SOCKARRAY is to return a socket cookie (a unique identifier) instead. Since socket cookies are u64 values, SOCK{MAP,HASH} need to support such a value size for lookup to be possible. This requires special handling on update, though. Attempts to do a lookup on a map holding u32 values will be met with ENOSPC error. Signed-off-by: -
Jakub Sitnicki authored
Now that sockmap/sockhash can hold listening sockets, when setting up the psock we will (i) grab references to verdict/parser progs, and (2) override socket upcalls sk_data_ready and sk_write_space. However, since we cannot redirect to listening sockets so we don't need to link the socket to the BPF progs. And more importantly we don't want the listening socket to have overridden upcalls because they would get inherited by child sockets cloned from it. Introduce a separate initialization path for listening sockets that does not change the upcalls and ignores the BPF progs. Signed-off-by:
-
Jakub Sitnicki authored
In order for sockmap/sockhash types to become generic collections for storing TCP sockets we need to loosen the checks during map update, while tightening the checks in redirect helpers. Currently sock{map,hash} require the TCP socket to be in established state, which prevents inserting listening sockets. Change the update pre-checks so the socket can also be in listening state. Since it doesn't make sense to redirect with sock{map,hash} to listening sockets, add appropriate socket state checks to BPF redirect helpers too. Signed-off-by: -
Jakub Sitnicki authored
Prepare for cloning listening sockets that have their protocol callbacks overridden by sk_msg. Child sockets must not inherit parent callbacks that access state stored in sk_user_data owned by the parent. Restore the child socket protocol callbacks before it gets hashed and any of the callbacks can get invoked. Signed-off-by:
-
Jakub Sitnicki authored
sk_user_data can hold a pointer to an object that is not intended to be shared between the parent socket and the child that gets a pointer copy on clone. This is the case when sk_user_data points at reference-counted object, like struct sk_psock. One way to resolve it is to tag the pointer with a no-copy flag by repurposing its lowest bit. Based on the bit-flag value we clear the child sk_user_data pointer after cloning the parent socket. The no-copy flag is stored in the pointer itself as opposed to externally, say in socket flags, to guarantee that the pointer and the flag are copied from parent to child socket in an atomic fashion. Parent socket state is subject to change while copying, we don't hold any locks at that time. This approach relies on an assumption that sk_user_data holds a pointer to an object aligned at least 2 bytes. A manual audit of existing users of rcu_dereference_sk_user_data helper confirms our assumption. Also, an RCU-protected sk_user_data is not likely to hold a pointer to a char value or a pathological case of "struct { char c; }". To be safe, warn when the flag-bit is set when setting sk_user_data to catch any future misuses. It is worth considering why clearing sk_user_data unconditionally is not an option. There exist users, DRBD, NVMe, and Xen drivers being among them, that rely on the pointer being copied when cloning the listening socket. Potentially we could distinguish these users by checking if the listening socket has been created in kernel-space via sock_create_kern, and hence has sk_kern_sock flag set. However, this is not the case for NVMe and Xen drivers, which create sockets without marking them as belonging to the kernel. Signed-off-by:
-
