- 08 Jul, 2018 16 commits
-
-
David S. Miller authored
The kbuild test robot reports: >> net/sched/act_api.c:71:15: sparse: incorrect type in initializer (different address spaces) @@ expected struct tc_cookie [noderef] <asn:4>*__ret @@ got [noderef] <asn:4>*__ret @@ net/sched/act_api.c:71:15: expected struct tc_cookie [noderef] <asn:4>*__ret net/sched/act_api.c:71:15: got struct tc_cookie *new_cookie >> net/sched/act_api.c:71:13: sparse: incorrect type in assignment (different address spaces) @@ expected struct tc_cookie *old @@ got struct tc_cookie [noderef] <struct tc_cookie *old @@ net/sched/act_api.c:71:13: expected struct tc_cookie *old net/sched/act_api.c:71:13: got struct tc_cookie [noderef] <asn:4>*[assigned] __ret >> net/sched/act_api.c:132:48: sparse: dereference of noderef expression Handle this in the usual way by force casting away the __rcu annotation when we are using xchg() on it. Fixes: eec94fdb ("net: sched: use rcu for action cookie update") Reported-by:
kbuild test robot <lkp@intel.com> Signed-off-by:
David S. Miller <davem@davemloft.net>
-
David S. Miller authored
Vlad Buslov says: ==================== Modify action API for implementing lockless actions Currently, all netlink protocol handlers for updating rules, actions and qdiscs are protected with single global rtnl lock which removes any possibility for parallelism. This patch set is a first step to remove rtnl lock dependency from TC rules update path. Recently, new rtnl registration flag RTNL_FLAG_DOIT_UNLOCKED was added. Handlers registered with this flag are called without RTNL taken. End goal is to have rule update handlers(RTM_NEWTFILTER, RTM_DELTFILTER, etc.) to be registered with UNLOCKED flag to allow parallel execution. However, there is no intention to completely remove or split rtnl lock itself. This patch set addresses specific problems in action API that prevents it from being executed concurrently. This patch set does not completely unlock rules or actions update path. Additional patch sets are required to refactor individual actions and filters update for parallel execution. As a preparation for executing TC rules update handlers without rtnl lock, action API code was audited to determine areas that assume external synchronization with rtnl lock and must be changed to allow safe concurrent access with following results: 1. Action idr is already protected with spinlock. However, some code paths assume that idr state is not changes between several consecutive tcf_idr_* function calls. 2. tc_action reference and bind counters are implemented as plain integers. They purpose was to allow single actions to be shared between multiple filters, not to provide means for concurrent modification. 3. tc_action 'cookie' pointer field is not protected against modification. 4. Action API functions, that work with set of actions, use intrusive linked list, which cannot be used concurrently without additional synchronization. 5. Action API functions don't take reference to actions while using them, assuming external synchronization with rtnl lock. Following solutions to these problems are implemented: 1. To remove assumption that idr state doesn't change between tcf_idr_* calls, implement new functions that atomically perform several operations on idr without releasing idr spinlock. (function to atomically lookup and delete action by index, function to atomically check if action exists and allocate new one if necessary, etc.) 2. Use atomic operations on counters to make them suitable for concurrent get/put operations. 3. Data that 'cookie' points to is never modified, so it enough to refactor it to rcu pointer to prevent concurrent de-allocation. 4. Action API doesn't actually use any linked list specific operations on actions intrusive linked list, so it can be refactored to array in straightforward manner. 5. Always take reference to action while accessing it in action API. tcf_idr_search function modified to take reference to action before returning it, so there is no way to lookup an action without incrementing its reference counter. All users of this function are modified to release the reference, after they done using action. With all users using reference counting, it is now safe to concurrently delete actions. Additionally, actions init function signature was expanded with 'rtnl_held' argument, that allows actions that have internal dependency on rtnl lock to take/release it when necessary. Since only shared state in action API module are actions themselves and action idr, these changes are sufficient to not to rely on global rtnl lock for protection of internal action API data structures. Changes from V5 to V6: - Rebase on current net-next - When action is deleted, set pointer in actions array to NULL to prevent double freeing. Changes from V4 to V5: - Change action delete API to track actions that were deleted, to prevent releasing them on error. Changes from V3 to V4: - Expand cover letter. - Reduce actions array size in tcf_action_init_1. - Rebase on latest net-next. Changes from V2 to V3: - Re-send with changelog copied to individual patches. Changes from V1 to V2: - Removed redundant actions ops lookup during delete. - Merge action ops delete definition and implementation. - Assume all actions have delete implemented and don't check for it explicitly. - Resplit action lookup/release code to prevent memory leaks in individual patches. - Make __tcf_idr_check function static - Remove unique idr insertion function. Change original idr insert to do the same thing. - Merge changes that take reference to action when performing lookup and changes that account for this additional reference when dumping action to user space into single patch. - Change convoluted commit message. - Rename "unlocked" to "rtnl_held" for clarity. - Remove estimator lock add patch. - Refactor action check-alloc code into standalone function. - Rename tcf_idr_find_delete to tcf_idr_delete_index. - Rearrange variable definitions in tc_action_delete. - Add patch that refactors action API code to use array of pointers to actions instead of intrusive linked list. - Expand cover letter. ==================== Signed-off-by: -
Vlad Buslov authored
Act API used linked list to pass set of actions to functions. It is intrusive data structure that stores list nodes inside action structure itself, which means it is not safe to modify such list concurrently. However, action API doesn't use any linked list specific operations on this set of actions, so it can be safely refactored into plain pointer array. Refactor action API to use array of pointers to tc_actions instead of linked list. Change argument 'actions' type of exported action init, destroy and dump functions. Acked-by:
Jiri Pirko <jiri@mellanox.com> Signed-off-by:
Vlad Buslov <vladbu@mellanox.com> Signed-off-by:
-
Vlad Buslov authored
Implement function that atomically checks if action exists and either takes reference to it, or allocates idr slot for action index to prevent concurrent allocations of actions with same index. Use EBUSY error pointer to indicate that idr slot is reserved. Implement cleanup helper function that removes temporary error pointer from idr. (in case of error between idr allocation and insertion of newly created action to specified index) Refactor all action init functions to insert new action to idr using this API. Reviewed-by:
Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Signed-off-by:
-
Vlad Buslov authored
Change action API to assume that action init function always takes reference to action, even when overwriting existing action. This is necessary because action API continues to use action pointer after init function is done. At this point action becomes accessible for concurrent modifications, so user must always hold reference to it. Implement helper put list function to atomically release list of actions after action API init code is done using them. Reviewed-by:
-
Vlad Buslov authored
Return from action init function with reference to action taken, even when overwriting existing action. Action init API initializes its fourth argument (pointer to pointer to tc action) to either existing action with same index or newly created action. In case of existing index(and bind argument is zero), init function returns without incrementing action reference counter. Caller of action init then proceeds working with action, without actually holding reference to it. This means that action could be deleted concurrently. Change action init behavior to always take reference to action before returning successfully, in order to protect from concurrent deletion. Reviewed-by:
-
Vlad Buslov authored
Implement helper delete function that uses new action ops 'delete', instead of destroying action directly. This is required so act API could delete actions by index, without holding any references to action that is being deleted. Implement function __tcf_action_put() that releases reference to action and frees it, if necessary. Refactor action deletion code to use new put function and not to rely on rtnl lock. Remove rtnl lock assertions that are no longer needed. Reviewed-by:
-
Vlad Buslov authored
Extend action ops with 'delete' function. Each action type to implements its own delete function that doesn't depend on rtnl lock. Implement delete function that is required to delete actions without holding rtnl lock. Use action API function that atomically deletes action only if it is still in action idr. This implementation prevents concurrent threads from deleting same action twice. Reviewed-by:
-
Vlad Buslov authored
Implement new action API function that atomically finds and deletes action from idr by index. Intended to be used by lockless actions that do not rely on rtnl lock. Reviewed-by:
-
Vlad Buslov authored
Without rtnl lock protection it is no longer safe to use pointer to tc action without holding reference to it. (it can be destroyed concurrently) Remove unsafe action idr lookup function. Instead of it, implement safe tcf idr check function that atomically looks up action in idr and increments its reference and bind counters. Implement both action search and check using new safe function Reference taken by idr check is temporal and should not be accounted by userspace clients (both logically and to preserver current API behavior). Subtract temporal reference when dumping action to userspace using existing tca_get_fill function arguments. Reviewed-by:
-
Vlad Buslov authored
Add additional 'rtnl_held' argument to act API init functions. It is required to implement actions that need to release rtnl lock before loading kernel module and reacquire if afterwards. Reviewed-by:
-
Vlad Buslov authored
Change type of action reference counter to refcount_t. Change type of action bind counter to atomic_t. This type is used to allow decrementing bind counter without testing for 0 result. Reviewed-by:
-
Vlad Buslov authored
Implement functions to atomically update and free action cookie using rcu mechanism. Reviewed-by:
-
Yifeng Sun authored
Add 'clone' action to kernel datapath by using existing functions. When actions within clone don't modify the current flow, the flow key is not cloned before executing clone actions. This is a follow up patch for this incomplete work: https://patchwork.ozlabs.org/patch/722096/ v1 -> v2: Refactor as advised by reviewer. Signed-off-by:
Yifeng Sun <pkusunyifeng@gmail.com> Signed-off-by:
Andy Zhou <azhou@ovn.org> Acked-by:
Pravin B Shelar <pshelar@ovn.org> Signed-off-by:
-
Randy Dunlap authored
Fix build warnings in drivers/isdn/capi/ when CONFIG_PROC_FS is not enabled by marking the unused functions as __maybe_unused. ../drivers/isdn/capi/capi.c:1324:12: warning: 'capi20_proc_show' defined but not used [-Wunused-function] ../drivers/isdn/capi/capi.c:1347:12: warning: 'capi20ncci_proc_show' defined but not used [-Wunused-function] ../drivers/isdn/capi/capidrv.c:2454:12: warning: 'capidrv_proc_show' defined but not used [-Wunused-function] Signed-off-by:
Randy Dunlap <rdunlap@infradead.org> Cc: Karsten Keil <isdn@linux-pingi.de> Cc: isdn4linux@listserv.isdn4linux.de (subscribers-only) Cc: netdev@vger.kernel.org Signed-off-by:
-
Randy Dunlap authored
Fix a build warning in connector.c when CONFIG_PROC_FS is not enabled by marking the unused function as __maybe_unused. ../drivers/connector/connector.c:242:12: warning: 'cn_proc_show' defined but not used [-Wunused-function] Signed-off-by:
-
- 07 Jul, 2018 24 commits
-
-
Colin Ian King authored
Variable next_rx is being assigned but is never used hence it is redundant and can be removed. Cleans up clang warning: warning: variable 'next_rx' set but not used [-Wunused-but-set-variable] Signed-off-by:
Colin Ian King <colin.king@canonical.com> Signed-off-by:
-
David S. Miller authored
Ivan Khoronzhuk says: ==================== allow PTP 224.0.0.107 to be timestamped Allows packets with 224.0.0.107 mcas address for PTP packets to be timestamped. Only for cpsw version > v1.15. ==================== Signed-off-by: -
Ivan Khoronzhuk authored
Tested on AM572x with cpsw v1.15 for PTP sync and delay_req messages. It doesn't work on cpsw v1.12, so added only for cpsw v > 1.15. Command for testing: ptp4l -P -4 -H -i eth0 -l 6 -m -q -p /dev/ptp0 -f ptp.cfg where ptp.cfg: [global] tx_timestamp_timeout 20 Signed-off-by:
Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> Signed-off-by:
-
Ivan Khoronzhuk authored
It's needed to avoid checkpatch warnings for farther changes. Signed-off-by:
-
Arnd Bergmann authored
I link error on 32-bit ARM points to yet another arithmetic bug: drivers/net/ethernet/stmicro/stmmac/stmmac_tc.o: In function `tc_setup_cbs': stmmac_tc.c:(.text+0x148): undefined reference to `__aeabi_uldivmod' stmmac_tc.c:(.text+0x1fc): undefined reference to `__aeabi_uldivmod' stmmac_tc.c:(.text+0x308): undefined reference to `__aeabi_uldivmod' stmmac_tc.c:(.text+0x320): undefined reference to `__aeabi_uldivmod' stmmac_tc.c:(.text+0x33c): undefined reference to `__aeabi_uldivmod' drivers/net/ethernet/stmicro/stmmac/stmmac_tc.o:stmmac_tc.c:(.text+0x3a4): more undefined references to `__aeabi_uldivmod' follow I observe that the last change to add the 'ul' prefix was incorrect, as it did not turn the result of the multiplication into a 64-bit expression on 32-bit architectures. Further, it seems that the do_div() macro gets confused by the fact that we pass a signed variable rather than unsigned into it. This changes the code to instead use the div_s64() helper that is meant for signed division, along with changing the constant suffix to 'll' to actually make it a 64-bit argument everywhere, fixing both of the issues I pointed out. I'm not completely convinced that this makes the code correct, but I'm fairly sure that we have two problems less than before. Fixes: 1f705bc6 ("net: stmmac: Add support for CBS QDISC") Fixes: c18a9c09 ("net: stmmac_tc: use 64-bit arithmetic instead of 32-bit") Signed-off-by:
Arnd Bergmann <arnd@arndb.de> Acked-by:
Gustavo A. R. Silva <gustavo@embeddedor.com> Signed-off-by:
-
Jon Maloy authored
Currently a link is declared stale and reset if there has been 100 repeated attempts to retransmit the same packet. However, in certain infrastructures we see that packet (NACK) duplicates and delays may cause such retransmit attempts to occur at a high rate, so that the peer doesn't have a reasonable chance to acknowledge the reception before the 100-limit is hit. This may take much less than the stipulated link tolerance time, and despite that probe/probe replies otherwise go through as normal. We now extend the criteria for link reset to also being time based. I.e., we don't reset the link until the link tolerance time is passed AND we have made 100 retransmissions attempts. Acked-by:
Ying Xue <ying.xue@windriver.com> Signed-off-by:
Jon Maloy <jon.maloy@ericsson.com> Signed-off-by:
-
David S. Miller authored
Jianbo Liu says: ==================== Introduce matching on double vlan/QinQ headers for TC flower Currently TC flower supports only one vlan tag, it doesn't match on both outer and inner vlan headers for QinQ. To do this, we add support to get both outer and inner vlan headers for flow dissector, and then TC flower do matching on those information. We also plan to extend TC command to support this feature. We add new cvlan_id/cvlan_prio/cvlan_ethtype keywords for inner vlan header. The existing vlan_id/vlan_prio/vlan_ethtype are for outer vlan header, and vlan_ethtype must be 802.1q or 802.1ad. The examples for command and output are as the following. flower vlan_id 1000 vlan_ethtype 802.1q \ cvlan_id 100 cvlan_ethtype ipv4 \ action vlan pop \ action vlan pop \ action mirred egress redirect dev ens1f1_0 filter protocol 802.1ad pref 33 flower chain 0 filter protocol 802.1ad pref 33 flower chain 0 handle 0x1 vlan_id 1000 vlan_ethtype 802.1Q cvlan_id 100 cvlan_ethtype ip eth_type ipv4 in_hw ... v2: fix sparse warning. ==================== Signed-off-by: -
Jianbo Liu authored
As support dissecting of QinQ inner and outer vlan headers, user can add rules to match on QinQ vlan headers. Signed-off-by:
Jianbo Liu <jianbol@mellanox.com> Acked-by:
-
Jianbo Liu authored
Currently the encapsulated ethertype is not dumped as it's the same as TCA_FLOWER_KEY_ETH_TYPE keyvalue. But the dumping result is inconsistent with input, we add dumping it with TCA_FLOWER_KEY_VLAN_ETH_TYPE. Signed-off-by:
-
Jianbo Liu authored
Dissect the QinQ packets to get both outer and inner vlan information, then store to the extended flow keys. Signed-off-by:
-
Jianbo Liu authored
As flow dissector stores vlan ethertype, tc flower now can match on that. It is to make preparation for supporting QinQ. Signed-off-by:
-
Jianbo Liu authored
Change vlan dissector key to save vlan tpid to support both 802.1Q and 802.1AD ethertype. Signed-off-by:
-
Colin Ian King authored
Variable txq_length is being assigned but is never used hence it is redundant and can be removed. Cleans up clang warning: warning: variable 'txq_length' set but not used [-Wunused-but-set-variable] Signed-off-by:
-
David S. Miller authored
Petr Machata says: ==================== Add a mlxsw-specific test for mirror-to-gretap Some configurations of mirror-to-gretap are impossible for mlxsw to offload. Add a test that checks that these out-of-domain conditions are handled properly by mlxsw. In patch #1, fix mirror_gre_lib.sh and mirror_gre_topo_lib.sh so that they can be imported from directories other than forwarding/. In patch #2, add a test to check handling of several scenarios that mlxsw is expected to fail to offload. ==================== Signed-off-by: -
Petr Machata authored
Test several aspects of offloading mirror to gretap and ip6gretap netdevices that are specific to mlxsw, such as requirements for TTL and TOS values. Signed-off-by:
Petr Machata <petrm@mellanox.com> Reviewed-by:
-
Petr Machata authored
The next patch introduces a new mlxsw-specific test that uses mirror_gre_lib.sh and mirror_gre_topo_lib.sh. However when sourcing their own deps, these libraries assume that the test that's running is in the same directory. That's not the case for driver-specific tests. So change the libraries to source their deps through $relative_path. That variable is set up by lib.sh, which should be imported by the test in question in any case. Signed-off-by:
-
David S. Miller authored
Roopa Prabhu says: ==================== vxlan: fix default fdb entry user-space notify ordering/race Problem: In vxlan_newlink, a default fdb entry is added before register_netdev. The default fdb creation function notifies user-space of the fdb entry on the vxlan device which user-space does not know about yet. (RTM_NEWNEIGH goes before RTM_NEWLINK for the same ifindex). This series fixes the user-space netlink notification ordering issue with the following changes: - decouple fdb notify from fdb create. - Move fdb notify after register_netdev. - modify rtnl_configure_link to allow configuring a link early. - Call rtnl_configure_link in vxlan newlink handler to notify userspace about the newlink before fdb notify and hence avoiding the user-space race. ==================== Fixes: afbd8bae ("vxlan: add implicit fdb entry for default destination") Signed-off-by:
Roopa Prabhu <roopa@cumulusnetworks.com>
-
Roopa Prabhu authored
Problem: In vxlan_newlink, a default fdb entry is added before register_netdev. The default fdb creation function also notifies user-space of the fdb entry on the vxlan device which user-space does not know about yet. (RTM_NEWNEIGH goes before RTM_NEWLINK for the same ifindex). This patch fixes the user-space netlink notification ordering issue with the following changes: - decouple fdb notify from fdb create. - Move fdb notify after register_netdev. - Call rtnl_configure_link in vxlan newlink handler to notify userspace about the newlink before fdb notify and hence avoiding the user-space race. Fixes: afbd8bae ("vxlan: add implicit fdb entry for default destination") Signed-off-by:
-
Roopa Prabhu authored
Add a new option do_notify to vxlan_fdb_destroy to make sending netlink notify optional. Used by a later patch. Signed-off-by:
-
Roopa Prabhu authored
- Add new vxlan_fdb_alloc helper - rename existing vxlan_fdb_create into vxlan_fdb_update: because it really creates or updates an existing fdb entry - move new fdb creation into a separate vxlan_fdb_create Main motivation for this change is to introduce the ability to decouple vxlan fdb creation and notify, used in a later patch. Signed-off-by: -
Roopa Prabhu authored
rtnl_configure_link sets dev->rtnl_link_state to RTNL_LINK_INITIALIZED and unconditionally calls __dev_notify_flags to notify user-space of dev flags. current call sequence for rtnl_configure_link rtnetlink_newlink rtnl_link_ops->newlink rtnl_configure_link (unconditionally notifies userspace of default and new dev flags) If a newlink handler wants to call rtnl_configure_link early, we will end up with duplicate notifications to user-space. This patch fixes rtnl_configure_link to check rtnl_link_state and call __dev_notify_flags with gchanges = 0 if already RTNL_LINK_INITIALIZED. Later in the series, this patch will help the following sequence where a driver implementing newlink can call rtnl_configure_link to initialize the link early. makes the following call sequence work: rtnetlink_newlink rtnl_link_ops->newlink (vxlan) -> rtnl_configure_link (initializes link and notifies user-space of default dev flags) rtnl_configure_link (updates dev flags if requested by user ifm and notifies user-space of new dev flags) Signed-off-by: -
Arnd Bergmann authored
A newly added dummy helper function tries to return a failure from a "void" function: In file included from include/net/dsa.h:24, from arch/arm/plat-orion/common.c:21: include/net/devlink.h: In function 'devlink_param_value_changed': include/net/devlink.h:771:9: error: 'return' with a value, in function returning void [-Werror] return -EOPNOTSUPP; This fixes it by removing the bogus statement. Fixes: ea601e17 ("devlink: Add devlink notifications support for params") Signed-off-by: -
Arnd Bergmann authored
When CONFIG_BRIDGE_VLAN_FILTERING is disabled, gcc correctly points out that the 'vid' variable is uninitialized whenever br_vlan_get_pvid returns an error: drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c: In function 'mlxsw_sp_rif_vlan_fid_get': drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c:6881:6: error: 'vid' may be used uninitialized in this function [-Werror=maybe-uninitialized] This changes the condition check to always return -EINVAL here, which I guess is what the author intended here. Fixes: e6f1960a ("mlxsw: spectrum_router: Allocate FID according to PVID") Signed-off-by:
Ido Schimmel <idosch@mellanox.com> Signed-off-by:
-
David S. Miller authored
Salil Mehta says: ==================== Misc. bug fixes & cleanups for HNS3 driver This patch-set presents some miscellaneous bug fixes and cleanups for the HNS3 driver. ==================== Signed-off-by:
-
