From patchwork Sat Jan 9 17:26:10 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vladimir Oltean X-Patchwork-Id: 360083 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B4C28C433E6 for ; Sat, 9 Jan 2021 17:28:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7D90123A63 for ; Sat, 9 Jan 2021 17:28:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726113AbhAIR2F (ORCPT ); Sat, 9 Jan 2021 12:28:05 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55126 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725926AbhAIR2F (ORCPT ); Sat, 9 Jan 2021 12:28:05 -0500 Received: from mail-ej1-x632.google.com (mail-ej1-x632.google.com [IPv6:2a00:1450:4864:20::632]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 857E8C06179F for ; Sat, 9 Jan 2021 09:27:24 -0800 (PST) Received: by mail-ej1-x632.google.com with SMTP id w1so18741509ejf.11 for ; Sat, 09 Jan 2021 09:27:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=+YiBRM9YeSjdZIuUHkMx1oBKC5O7YxTJrot0Nozlrpw=; b=S4Kq5mkR0TbQaVC6eVd/7Y/R1UP3HTA1B+CJeEaLzdrJZNWGYd41z7kbJjcXVkzLqJ EX87lp0UtYQQxnVWsRNnTNBFC1wN9KsreIDmiioVYK44ghH+kAXmwC/z4lN4iQoyT6n7 bb/K4Mz8h/iU0aXUoF7kDVaF9nE1CcSVLcx3SXyv9PfWSWHIMjB7eeqNkmom3M4dfzeL s7XKwoWpUTZmZgG3RIurDK8QJkgUUNNjlOP2LzSpVawnsbAEb3rJnYiRougC2Oq4NJRt 1KP1eMy40jz/zSNGjTUzE84Ny8sB7eK2KaK3fiMhM3w657TCjdld4/21b5RIekVdaVJP 1zdA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=+YiBRM9YeSjdZIuUHkMx1oBKC5O7YxTJrot0Nozlrpw=; b=LZH9533qdLimZtkS/cvuNob3+wUvJXmoRawh0t4VwK4Eth2lzyEtyn+hJw9rumo4Uf iFKRn+ZO+BQnjuEC8SeX8AnKcNua2hj1lDnNDu+YHcwnEVDuMRKmGZI87eka2wX9yIiF MakNdgya+kjX1vnYRcA7uSy7bZCuvM37jhsMvwSrVsyxg8u+QMIpyxR4vJmGET7T9/Qm m7Tabai/VGPOvSL+c6FJ+ymyAomDfSc1xt8Sv2KxLg94Gw3+5OTijmpOdsMaycQdItd5 QLYVIdkuNt8LuKnbsiz119KcZRVl2k1AWAWLS7j+H221zTl9F/pjFpF1PIPbHWvk+sgC vO/w== X-Gm-Message-State: AOAM530UdPyeANWwIrAqYU/WaRyVh1TmmioiB218WahWU4m4DMGs/spq LLPNtyaIoL9q7gnNmjfbk8E= X-Google-Smtp-Source: ABdhPJwIeaW6zpTURva3wkFRB8ZnFr+ZuhTVaa1I78P99zxofG7MCMyZhgARHPsUkfDt/fXEFgLp7w== X-Received: by 2002:a17:907:20a4:: with SMTP id pw4mr5907298ejb.499.1610213242151; Sat, 09 Jan 2021 09:27:22 -0800 (PST) Received: from localhost.localdomain (5-12-227-87.residential.rdsnet.ro. [5.12.227.87]) by smtp.gmail.com with ESMTPSA id h16sm4776714eji.110.2021.01.09.09.27.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 09 Jan 2021 09:27:21 -0800 (PST) From: Vladimir Oltean To: "David S . Miller" , Jakub Kicinski Cc: netdev@vger.kernel.org, Andrew Lunn , Florian Fainelli , Cong Wang , Stephen Hemminger , Eric Dumazet , George McCollister , Oleksij Rempel , Jay Vosburgh , Veaceslav Falico , Andy Gospodarek , Arnd Bergmann , Taehee Yoo , Jiri Pirko , Florian Westphal , Nikolay Aleksandrov , Pravin B Shelar , Sridhar Samudrala , Saeed Mahameed Subject: [PATCH v6 net-next 01/15] net: mark dev_base_lock for deprecation Date: Sat, 9 Jan 2021 19:26:10 +0200 Message-Id: <20210109172624.2028156-2-olteanv@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210109172624.2028156-1-olteanv@gmail.com> References: <20210109172624.2028156-1-olteanv@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Vladimir Oltean There is a movement to eliminate the usage of dev_base_lock, which exists since as far as I could track the kernel history down (the "7a2deb329241 Import changeset" commit from the bitkeeper branch). The dev_base_lock approach has multiple issues: - It is global and not per netns. - Its meaning has mutated over the years and the vast majority of current users is using it to protect against changes of network device state, as opposed to changes of the interface list. - It is overlapping with other protection mechanisms introduced in the meantime, which have higher performance for readers, like RCU introduced in 2009 by Eric Dumazet for kernel 2.6. The old comment that I just deleted made this distinction: writers protect only against readers by holding dev_base_lock, whereas they need to protect against other writers by holding the RTNL mutex. This is slightly confusing/incorrect, since a rwlock_t cannot have more than one concurrently running writer, so that explanation does not justify why the RTNL mutex would be necessary. Instead, Stephen Hemminger makes this clarification here: https://lore.kernel.org/netdev/20201129211230.4d704931@hermes.local/T/#u | There are really two different domains being covered by locks here. | | The first area is change of state of network devices. This has traditionally | been covered by RTNL because there are places that depend on coordinating | state between multiple devices. RTNL is too big and held too long but getting | rid of it is hard because there are corner cases (like state changes from userspace | for VPN devices). | | The other area is code that wants to do read access to look at list of devices. | These pure readers can/should be converted to RCU by now. Writers should hold RTNL. This patch edits the comment for dev_base_lock, minimizing its role in the protection of network interface lists, and clarifies that it is has other purposes as well. Signed-off-by: Vladimir Oltean --- Changes in v6: None. Changes in v5: None. Changes in v4: None. Changes in v3: None. Changes in v2: None. net/core/dev.c | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/net/core/dev.c b/net/core/dev.c index 7afbb642e203..8e02240bb11c 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -169,23 +169,22 @@ static int call_netdevice_notifiers_extack(unsigned long val, static struct napi_struct *napi_by_id(unsigned int napi_id); /* - * The @dev_base_head list is protected by @dev_base_lock and the rtnl - * semaphore. - * - * Pure readers hold dev_base_lock for reading, or rcu_read_lock() - * - * Writers must hold the rtnl semaphore while they loop through the - * dev_base_head list, and hold dev_base_lock for writing when they do the - * actual updates. This allows pure readers to access the list even - * while a writer is preparing to update it. - * - * To put it another way, dev_base_lock is held for writing only to - * protect against pure readers; the rtnl semaphore provides the - * protection against other writers. - * - * See, for example usages, register_netdevice() and - * unregister_netdevice(), which must be called with the rtnl - * semaphore held. + * The network interface list of a netns (@net->dev_base_head) and the hash + * tables per ifindex (@net->dev_index_head) and per name (@net->dev_name_head) + * are protected using the following rules: + * + * Pure readers should hold rcu_read_lock() which should protect them against + * concurrent changes to the interface lists made by the writers. Pure writers + * must serialize by holding the RTNL mutex while they loop through the list + * and make changes to it. + * + * It is also possible to hold the global rwlock_t @dev_base_lock for + * protection (holding its read side as an alternative to rcu_read_lock, and + * its write side as an alternative to the RTNL mutex), however this should not + * be done in new code, since it is deprecated and pending removal. + * + * One other role of @dev_base_lock is to protect against changes in the + * operational state of a network interface. */ DEFINE_RWLOCK(dev_base_lock); EXPORT_SYMBOL(dev_base_lock); From patchwork Sat Jan 9 17:26:12 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vladimir Oltean X-Patchwork-Id: 360082 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E4AB0C433E9 for ; Sat, 9 Jan 2021 17:28:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id ABC5623A82 for ; Sat, 9 Jan 2021 17:28:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726281AbhAIR2I (ORCPT ); Sat, 9 Jan 2021 12:28:08 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55138 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725926AbhAIR2H (ORCPT ); Sat, 9 Jan 2021 12:28:07 -0500 Received: from mail-ed1-x52f.google.com (mail-ed1-x52f.google.com [IPv6:2a00:1450:4864:20::52f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B17A9C0617A3 for ; Sat, 9 Jan 2021 09:27:26 -0800 (PST) Received: by mail-ed1-x52f.google.com with SMTP id j16so14528225edr.0 for ; Sat, 09 Jan 2021 09:27:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=9m3oKGvCSeWHdPInnGbCMaa6M732P3ZHcFpaYA/9tjc=; b=nZs1Sf5Ks5o97y3DH1rodjHLm8LbgfWAALJgwURcCeg1J1DTpZueNEmeJ6cnTy9Qcx YNDqN6ovAhvpeJrJt4XdNwD7XRib/RqwQIcfx4durYqNsjxJQ6R3HPwpU76EeA5n3JFi KT81ROsoVFL8jeDwPMKo29BDqblhwFP3HwkXP/VEOZA9m3h1SKyCZQ71FThlOM7weICy Qi8d6lIr025cn65F0+sRdVhsr/No+zg0mtlfRaYmbR1FCDQXbe7yQEAUk3/hhgS2G1ht vcv5+VGw9hCKEWEYSbuJ/E5Gpiu9oV22FIVISI7ezkNd6PN31yIg+550rjpJ19ZEeRSm oxHA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=9m3oKGvCSeWHdPInnGbCMaa6M732P3ZHcFpaYA/9tjc=; b=rS7gBLvO8yrALCAR/s5eC9KKc+sYdWA32BWf6s4Xl71pK2dFgCpuAx50zFViPRm5aS +A/k6HjUY3+OMe5zhPea6/eez9Pdz04GbqgFKKhTL8PDH7gXiLy9fnhogplfR0lICEWS W1fmYXt8dYFCXjKJJZiux35UO8wIUMzea7XPBlkNjPI8dD6olRaJmpqYGhReqyBd8bfS dlh9l787VII0yMGGGovE54lnn44MQFnN26boBfH58zJGsOjT1xRaa0PVYCpSdaYc/QU1 S20S+YeJ7FopUo6kbbetxzK62M71SMW7/e0geFZPUpCYJw043eFQ8lNRGFvY0gZF6UbE CnKg== X-Gm-Message-State: AOAM530f6gWRUco6EOsZjuu6jVcB3Vq/2MPQIOOfpYgVKROj1wrRoLpk 49QpPGa0VSeCQDEGA8Mh0a4= X-Google-Smtp-Source: ABdhPJzTCI4g2QX+ye8xRp4yRL7H6IN+mf90dRw2oXticPElMVgeLA1k4C8W8t80xT9gtRcyChAHiA== X-Received: by 2002:a50:9dc9:: with SMTP id l9mr9051970edk.377.1610213245490; Sat, 09 Jan 2021 09:27:25 -0800 (PST) Received: from localhost.localdomain (5-12-227-87.residential.rdsnet.ro. [5.12.227.87]) by smtp.gmail.com with ESMTPSA id h16sm4776714eji.110.2021.01.09.09.27.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 09 Jan 2021 09:27:24 -0800 (PST) From: Vladimir Oltean To: "David S . Miller" , Jakub Kicinski Cc: netdev@vger.kernel.org, Andrew Lunn , Florian Fainelli , Cong Wang , Stephen Hemminger , Eric Dumazet , George McCollister , Oleksij Rempel , Jay Vosburgh , Veaceslav Falico , Andy Gospodarek , Arnd Bergmann , Taehee Yoo , Jiri Pirko , Florian Westphal , Nikolay Aleksandrov , Pravin B Shelar , Sridhar Samudrala , Saeed Mahameed Subject: [PATCH v6 net-next 03/15] net: procfs: hold netif_lists_lock when retrieving device statistics Date: Sat, 9 Jan 2021 19:26:12 +0200 Message-Id: <20210109172624.2028156-4-olteanv@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210109172624.2028156-1-olteanv@gmail.com> References: <20210109172624.2028156-1-olteanv@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Vladimir Oltean In the effort of making .ndo_get_stats64 be able to sleep, we need to ensure the callers of dev_get_stats do not use atomic context. The /proc/net/dev file uses an RCU read-side critical section to ensure the integrity of the list of network interfaces, because it iterates through all net devices in the netns to show their statistics. To offer the equivalent protection against an interface registering or deregistering, while also remaining in sleepable context, we can use the netns mutex for the interface lists. Cc: Cong Wang Cc: Eric Dumazet Signed-off-by: Vladimir Oltean --- Changes in v6: None. Changes in v5: None. Changes in v4: None. Changes in v3: None. Changes in v2: None. net/core/net-procfs.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/net/core/net-procfs.c b/net/core/net-procfs.c index c714e6a9dad4..4784703c1e39 100644 --- a/net/core/net-procfs.c +++ b/net/core/net-procfs.c @@ -21,7 +21,7 @@ static inline struct net_device *dev_from_same_bucket(struct seq_file *seq, loff unsigned int count = 0, offset = get_offset(*pos); h = &net->dev_index_head[get_bucket(*pos)]; - hlist_for_each_entry_rcu(dev, h, index_hlist) { + hlist_for_each_entry(dev, h, index_hlist) { if (++count == offset) return dev; } @@ -51,9 +51,11 @@ static inline struct net_device *dev_from_bucket(struct seq_file *seq, loff_t *p * in detail. */ static void *dev_seq_start(struct seq_file *seq, loff_t *pos) - __acquires(RCU) { - rcu_read_lock(); + struct net *net = seq_file_net(seq); + + netif_lists_lock(net); + if (!*pos) return SEQ_START_TOKEN; @@ -70,9 +72,10 @@ static void *dev_seq_next(struct seq_file *seq, void *v, loff_t *pos) } static void dev_seq_stop(struct seq_file *seq, void *v) - __releases(RCU) { - rcu_read_unlock(); + struct net *net = seq_file_net(seq); + + netif_lists_unlock(net); } static void dev_seq_printf_stats(struct seq_file *seq, struct net_device *dev) From patchwork Sat Jan 9 17:26:13 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vladimir Oltean X-Patchwork-Id: 360081 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0D950C43381 for ; Sat, 9 Jan 2021 17:28:20 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D048F23884 for ; Sat, 9 Jan 2021 17:28:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726315AbhAIR2J (ORCPT ); Sat, 9 Jan 2021 12:28:09 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55146 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725926AbhAIR2I (ORCPT ); Sat, 9 Jan 2021 12:28:08 -0500 Received: from mail-ej1-x631.google.com (mail-ej1-x631.google.com [IPv6:2a00:1450:4864:20::631]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5F2D1C0617A4 for ; Sat, 9 Jan 2021 09:27:28 -0800 (PST) Received: by mail-ej1-x631.google.com with SMTP id qw4so18739992ejb.12 for ; Sat, 09 Jan 2021 09:27:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=ovfprG4fnZ7oxcJrrp2stelHr4YWI6qY04aWSEUZeCY=; b=k7R0PClCpvqFPWkzIr0mQz6jXrZGVG1yT9pwObwzXk2NJ7bRo/FdkaNlObF49jXdmM 3enJhxyot1Mhj1Lh8gUrZRVi8k4dYjhU9tvQyRW9TufaMwpsD8aFX2WbpGo3PEYmQCie jJPhwLIuh1PfQ6aTyRBqmRXBVZ39wvQ1g/YehVAy7qfJ6ojgW1lnvKomxK7HLJqSROT7 krgiROHC/4q3VAdlw2byvECQFfoynUnpUjXHGir9z4dwr7lmFwDa9UTeJEQQC5iIH1jQ a27Xlmy2Jf1cQKvhndfxIMOKJ6FQuxxc6Lpu6Yx9S9WEDa9kmizlj984Ht2/X/4AxWGO R14g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=ovfprG4fnZ7oxcJrrp2stelHr4YWI6qY04aWSEUZeCY=; b=r4j7g9i1WUwLadUDTqXWPCinmk4PJe8ulGZybLY27t+QnZpOvzznnMLZyDl3o/bHtO S70Dh9qaVaaPa3xZEXXApEqnOJS0NQojtxcen+SsDDBm9e1H7WjAw98W2n1NGk4S7aVP V5RG7NVytFzy+fzTc7mrpBh4tV1uMkNpUudREA6ZwlGsML03EN/samy+ydFBiFbbFgmA 5eTNgeF13RjXbQhzPn2OT2Ww34b57+UYGFvffHhmgdKuEZ4vynnZ+iQ/TEkVmaSbjXYr cl1T5C7TxKV0YyjeiN98vHBYibviZyhgLdqpfULWPkc493GWJ2WrZMXgtuRIWh39gGRL 4bdQ== X-Gm-Message-State: AOAM533kgkCaSExPWV1XaRioo/21Pm/w+9FC/ltLtCy8YKCbK5oJdb/R cNWnAJtGlh9GvwPXvXycLss= X-Google-Smtp-Source: ABdhPJy+bWhlZh8HI1Ff8AnfYsyHgQqu5vmdPtfj6GLZ9GzqjDjBVEK6Za7oJIfofcsbsruyqAal+g== X-Received: by 2002:a17:906:eb8d:: with SMTP id mh13mr5850514ejb.299.1610213247043; Sat, 09 Jan 2021 09:27:27 -0800 (PST) Received: from localhost.localdomain (5-12-227-87.residential.rdsnet.ro. [5.12.227.87]) by smtp.gmail.com with ESMTPSA id h16sm4776714eji.110.2021.01.09.09.27.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 09 Jan 2021 09:27:26 -0800 (PST) From: Vladimir Oltean To: "David S . Miller" , Jakub Kicinski Cc: netdev@vger.kernel.org, Andrew Lunn , Florian Fainelli , Cong Wang , Stephen Hemminger , Eric Dumazet , George McCollister , Oleksij Rempel , Jay Vosburgh , Veaceslav Falico , Andy Gospodarek , Arnd Bergmann , Taehee Yoo , Jiri Pirko , Florian Westphal , Nikolay Aleksandrov , Pravin B Shelar , Sridhar Samudrala , Saeed Mahameed Subject: [PATCH v6 net-next 04/15] net: sysfs: don't hold dev_base_lock while retrieving device statistics Date: Sat, 9 Jan 2021 19:26:13 +0200 Message-Id: <20210109172624.2028156-5-olteanv@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210109172624.2028156-1-olteanv@gmail.com> References: <20210109172624.2028156-1-olteanv@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Vladimir Oltean In the effort of making .ndo_get_stats64 be able to sleep, we need to ensure the callers of dev_get_stats do not use atomic context. I need to preface this by saying that I have no idea why netstat_show takes the dev_base_lock rwlock. Two things can be observed: (a) it does not appear to be due to dev_isalive requiring it for some reason, because broadcast_show() also calls dev_isalive() and has had no problem existing since the beginning of git. (b) the dev_get_stats function definitely does not need dev_base_lock protection either. In fact, holding the dev_base_lock is the entire problem here, because we want to make dev_get_stats sleepable, and holding a rwlock gives us atomic context. So since no protection seems to be necessary, just run unlocked while retrieving the /sys/class/net/eth0/statistics/* values. Cc: Christian Brauner Cc: Eric Dumazet Signed-off-by: Vladimir Oltean --- Changes in v6: None. Changes in v5: None. Changes in v4: None. Changes in v3: None. Changes in v2: None. net/core/net-sysfs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c index daf502c13d6d..8604183678fc 100644 --- a/net/core/net-sysfs.c +++ b/net/core/net-sysfs.c @@ -585,14 +585,13 @@ static ssize_t netstat_show(const struct device *d, WARN_ON(offset > sizeof(struct rtnl_link_stats64) || offset % sizeof(u64) != 0); - read_lock(&dev_base_lock); if (dev_isalive(dev)) { struct rtnl_link_stats64 temp; const struct rtnl_link_stats64 *stats = dev_get_stats(dev, &temp); ret = sprintf(buf, fmt_u64, *(u64 *)(((u8 *)stats) + offset)); } - read_unlock(&dev_base_lock); + return ret; } From patchwork Sat Jan 9 17:26:14 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vladimir Oltean X-Patchwork-Id: 360080 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BE222C433E0 for ; Sat, 9 Jan 2021 17:28:46 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8621323A23 for ; Sat, 9 Jan 2021 17:28:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726253AbhAIR2o (ORCPT ); Sat, 9 Jan 2021 12:28:44 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55268 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725872AbhAIR2n (ORCPT ); Sat, 9 Jan 2021 12:28:43 -0500 Received: from mail-ed1-x52d.google.com (mail-ed1-x52d.google.com [IPv6:2a00:1450:4864:20::52d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DF8C9C0617A5 for ; Sat, 9 Jan 2021 09:27:29 -0800 (PST) Received: by mail-ed1-x52d.google.com with SMTP id u19so14506900edx.2 for ; Sat, 09 Jan 2021 09:27:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=u84TeGJaXfTDMnhIuYMOBv+8WC6ikVA8oDqJpLmIyoM=; b=no3UtteUB2UmggiV2TaEXxv56kdtZJxCaAAM4C0lNj11Fu70+aWQd1fttDsYWZRdgq AGQgknke41Pi14UO36DqmxF2IRPgUb8sfEqbW353y0GlS8SkX1GMscqnJlbQ8YI4A1dg 1/aHvSIOM4oj86XIwOepdtL3VQtqt1igY7EeQoU+CboY0ZKZsCzRYx8fBWelSWzTVE3w hz2kwfefNVKnVNtaysSmyFETpaywf/IveWQJN9nOH7+NHBuO4ntRQfeYLtZSzt8xUTbz QfZbKNoBCIGaAJuIWb4FBS4aQbc44Qx0/zyUtQAuSL+3XNPRgzLTm/RhyuIy3PUNfd+o pFOA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=u84TeGJaXfTDMnhIuYMOBv+8WC6ikVA8oDqJpLmIyoM=; b=il775nDDhb+iZnlDHKDkMi/6MvnH96kR3XA23QYCcqPaaUihj4jkc2iQBO6+M8XTcL fo5LEwu3hNexzCoND8YnRugVeRe1kbKBcql0uTN008kWd6jTjAgpwY7FCQLS6rEsV/rx Z/WOM1b95xSvEjhtb6DbsbjzyuUtELYMuo1KT5kvC3VSgr7s3Sy7Zj6KCkfXnjAz9WCD cdD0AW4fmQL/4w5JBSR9vHtkXc2PocWekX2VSfR7Tuft24W2E7WTO6umHUbgMuD0aT4R gw1p4W6FbO3Zcx5py977TRBN+wT7zNMto2wYvFHQvPEF/MddUjtvwQ9gN9u0/yT66LgT QldA== X-Gm-Message-State: AOAM531LT5lXHlfJ8n7AvjH0CfUmZVzpMWGYDn8dS1BOI5xVmL2W83JT kK8YNbKUwiwGU4O3Mz1JoXs= X-Google-Smtp-Source: ABdhPJz5uPiqA7SGPE+urOaQpthPVBfsKMtSpkG1eGhBNZcPVGEwc1iwlMGrNcXPJxUnmCUU80exvg== X-Received: by 2002:aa7:cccf:: with SMTP id y15mr9116436edt.112.1610213248668; Sat, 09 Jan 2021 09:27:28 -0800 (PST) Received: from localhost.localdomain (5-12-227-87.residential.rdsnet.ro. [5.12.227.87]) by smtp.gmail.com with ESMTPSA id h16sm4776714eji.110.2021.01.09.09.27.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 09 Jan 2021 09:27:28 -0800 (PST) From: Vladimir Oltean To: "David S . Miller" , Jakub Kicinski Cc: netdev@vger.kernel.org, Andrew Lunn , Florian Fainelli , Cong Wang , Stephen Hemminger , Eric Dumazet , George McCollister , Oleksij Rempel , Jay Vosburgh , Veaceslav Falico , Andy Gospodarek , Arnd Bergmann , Taehee Yoo , Jiri Pirko , Florian Westphal , Nikolay Aleksandrov , Pravin B Shelar , Sridhar Samudrala , Saeed Mahameed Subject: [PATCH v6 net-next 05/15] s390/appldata_net_sum: hold the netdev lists lock when retrieving device statistics Date: Sat, 9 Jan 2021 19:26:14 +0200 Message-Id: <20210109172624.2028156-6-olteanv@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210109172624.2028156-1-olteanv@gmail.com> References: <20210109172624.2028156-1-olteanv@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Vladimir Oltean In the effort of making .ndo_get_stats64 be able to sleep, we need to ensure the callers of dev_get_stats do not use atomic context. In the case of the appldata driver, an RCU read-side critical section is used to ensure the integrity of the list of network interfaces, because the driver iterates through all net devices in the netns to aggregate statistics. We still need some protection against an interface registering or deregistering, and the writer-side lock, the netns's mutex, is fine for that, because it offers sleepable context. The ops->callback function is called from under appldata_ops_mutex protection, so this is proof that the context is sleepable and holding a mutex is therefore fine. Cc: Heiko Carstens Cc: Vasily Gorbik Cc: linux-s390@vger.kernel.org Signed-off-by: Vladimir Oltean --- Changes in v6: None. Changes in v5: None. Changes in v4: None. Changes in v3: None. Changes in v2: None. arch/s390/appldata/appldata_net_sum.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/arch/s390/appldata/appldata_net_sum.c b/arch/s390/appldata/appldata_net_sum.c index 59c282ca002f..4db886980cba 100644 --- a/arch/s390/appldata/appldata_net_sum.c +++ b/arch/s390/appldata/appldata_net_sum.c @@ -78,8 +78,9 @@ static void appldata_get_net_sum_data(void *data) tx_dropped = 0; collisions = 0; - rcu_read_lock(); - for_each_netdev_rcu(&init_net, dev) { + netif_lists_lock(&init_net); + + for_each_netdev(&init_net, dev) { const struct rtnl_link_stats64 *stats; struct rtnl_link_stats64 temp; @@ -95,7 +96,8 @@ static void appldata_get_net_sum_data(void *data) collisions += stats->collisions; i++; } - rcu_read_unlock(); + + netif_lists_unlock(&init_net); net_data->nr_interfaces = i; net_data->rx_packets = rx_packets; From patchwork Sat Jan 9 17:26:18 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vladimir Oltean X-Patchwork-Id: 360079 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9E69EC433E0 for ; Sat, 9 Jan 2021 17:28:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6E4A223884 for ; Sat, 9 Jan 2021 17:28:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726407AbhAIR2t (ORCPT ); Sat, 9 Jan 2021 12:28:49 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55286 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726364AbhAIR2q (ORCPT ); Sat, 9 Jan 2021 12:28:46 -0500 Received: from mail-ej1-x633.google.com (mail-ej1-x633.google.com [IPv6:2a00:1450:4864:20::633]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C3BB1C0617AA for ; Sat, 9 Jan 2021 09:27:36 -0800 (PST) Received: by mail-ej1-x633.google.com with SMTP id g20so18919391ejb.1 for ; Sat, 09 Jan 2021 09:27:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=6ApqBmYJPVuM89MlPQNToDynXEEzwj/6TydzQNxRN9s=; b=RIJnYyVZSGpbsoifJUFj8Ov2HInZn/2SlEWV6zVXjUIDAg6fvkv9eaEu9pYGCP6A5K xlOaqUwQsZj9i6yG+ONQt+36BY6Ln8BoGrFwI0bDDYXVBxb/V3tiflQhOu8iQH8zEb/F plUDKpf/ts8ucs21FBJl20U8ig3+hgLNAUhN1MYnGUflbLlIoLl5KzTyRMj/YkN3QB+C lntnSy7fvKhNWo3kq3+7c64SAym8XfAQcM9XUu+3ceNRU8WHUXM8n4LJIV1d7aLlwVwI csEEgscHCP7CHVKpggQcYXa++ptHLH3fUXaZomy3JRJ7D5unXdcl3tj0vXVEiINaO3aa zZJg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=6ApqBmYJPVuM89MlPQNToDynXEEzwj/6TydzQNxRN9s=; b=cCedFEA+/2qPyhhzAsg7mGd02YjUpLYCyMUgxGbWhSY2YmiNOoUvdTDne/3h5I4/ID RVbKf2e9RoDuL6O54b/LAr+9uQ+AI9hzFVRDgy6qnL1hkN/NxKFkFt0L4QLqUOMisEej QoEZVDe8/5GhZAYteM8nfkMYpnUK18QDWzix/N+hawOs9V9xAvTdpdCPNWkJq2Ex3ImS 4g2Py4H5sFxqErhg138SGzT+E2syxLKLqHIVmEPLceS7HG55XTXS17wdjR7Vr87Zelx2 kkL3mtihD/vs0NOZtLMUKtE+G/bx5sz3Eu0HyVfdC8P2VtN9H7/b4Hz/bd2Eg0so7ww0 mmgA== X-Gm-Message-State: AOAM533SZnUNDV/aiw/4JNLqgkmc9/ovqDdGa9q54LbIhG8c641sGPrB CorPeyS8pJ82arSjgjXG0Qk= X-Google-Smtp-Source: ABdhPJx/rIHYhKIpkHvsQAer9hoaKBd33G3utt5k/OChKjWaf6KO7sSkUun6FIhBs19QXLhxXX+pXQ== X-Received: by 2002:a17:906:3499:: with SMTP id g25mr6128050ejb.18.1610213255463; Sat, 09 Jan 2021 09:27:35 -0800 (PST) Received: from localhost.localdomain (5-12-227-87.residential.rdsnet.ro. [5.12.227.87]) by smtp.gmail.com with ESMTPSA id h16sm4776714eji.110.2021.01.09.09.27.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 09 Jan 2021 09:27:34 -0800 (PST) From: Vladimir Oltean To: "David S . Miller" , Jakub Kicinski Cc: netdev@vger.kernel.org, Andrew Lunn , Florian Fainelli , Cong Wang , Stephen Hemminger , Eric Dumazet , George McCollister , Oleksij Rempel , Jay Vosburgh , Veaceslav Falico , Andy Gospodarek , Arnd Bergmann , Taehee Yoo , Jiri Pirko , Florian Westphal , Nikolay Aleksandrov , Pravin B Shelar , Sridhar Samudrala , Saeed Mahameed Subject: [PATCH v6 net-next 09/15] scsi: fcoe: propagate errors from dev_get_stats Date: Sat, 9 Jan 2021 19:26:18 +0200 Message-Id: <20210109172624.2028156-10-olteanv@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210109172624.2028156-1-olteanv@gmail.com> References: <20210109172624.2028156-1-olteanv@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Vladimir Oltean The FCoE callback for the Link Error Status Block retrieves the FCS error count using dev_get_stats. This function can now return errors. Propagate these all the way to the sysfs device attributes. Signed-off-by: Vladimir Oltean --- Changes in v6: None. Changes in v5: None. Changes in v4: Patch is new (Eric's suggestion). drivers/scsi/fcoe/fcoe_sysfs.c | 9 +++++++-- drivers/scsi/fcoe/fcoe_transport.c | 24 +++++++++++++++--------- drivers/scsi/libfc/fc_rport.c | 5 ++++- include/scsi/fcoe_sysfs.h | 12 ++++++------ include/scsi/libfc.h | 2 +- include/scsi/libfcoe.h | 8 ++++---- 6 files changed, 37 insertions(+), 23 deletions(-) diff --git a/drivers/scsi/fcoe/fcoe_sysfs.c b/drivers/scsi/fcoe/fcoe_sysfs.c index af658aa38fed..a197e66ffa8a 100644 --- a/drivers/scsi/fcoe/fcoe_sysfs.c +++ b/drivers/scsi/fcoe/fcoe_sysfs.c @@ -139,8 +139,13 @@ static ssize_t show_fcoe_ctlr_device_##field(struct device *dev, \ char *buf) \ { \ struct fcoe_ctlr_device *ctlr = dev_to_ctlr(dev); \ - if (ctlr->f->get_fcoe_ctlr_##field) \ - ctlr->f->get_fcoe_ctlr_##field(ctlr); \ + int err; \ + \ + if (ctlr->f->get_fcoe_ctlr_##field) { \ + err = ctlr->f->get_fcoe_ctlr_##field(ctlr); \ + if (err) \ + return err; \ + } \ return snprintf(buf, sz, format_string, \ cast fcoe_ctlr_##field(ctlr)); \ } diff --git a/drivers/scsi/fcoe/fcoe_transport.c b/drivers/scsi/fcoe/fcoe_transport.c index 213ee9efb044..5d19650e9bc1 100644 --- a/drivers/scsi/fcoe/fcoe_transport.c +++ b/drivers/scsi/fcoe/fcoe_transport.c @@ -166,15 +166,16 @@ EXPORT_SYMBOL_GPL(fcoe_link_speed_update); * Note, the Link Error Status Block (LESB) for FCoE is defined in FC-BB-6 * Clause 7.11 in v1.04. */ -void __fcoe_get_lesb(struct fc_lport *lport, - struct fc_els_lesb *fc_lesb, - struct net_device *netdev) +int __fcoe_get_lesb(struct fc_lport *lport, + struct fc_els_lesb *fc_lesb, + struct net_device *netdev) { struct rtnl_link_stats64 dev_stats; unsigned int cpu; u32 lfc, vlfc, mdac; struct fc_stats *stats; struct fcoe_fc_els_lesb *lesb; + int err; lfc = 0; vlfc = 0; @@ -190,8 +191,14 @@ void __fcoe_get_lesb(struct fc_lport *lport, lesb->lesb_link_fail = htonl(lfc); lesb->lesb_vlink_fail = htonl(vlfc); lesb->lesb_miss_fka = htonl(mdac); - dev_get_stats(netdev, &dev_stats); + + err = dev_get_stats(netdev, &dev_stats); + if (err) + return err; + lesb->lesb_fcs_error = htonl(dev_stats.rx_crc_errors); + + return 0; } EXPORT_SYMBOL_GPL(__fcoe_get_lesb); @@ -200,12 +207,11 @@ EXPORT_SYMBOL_GPL(__fcoe_get_lesb); * @lport: the local port * @fc_lesb: the link error status block */ -void fcoe_get_lesb(struct fc_lport *lport, - struct fc_els_lesb *fc_lesb) +int fcoe_get_lesb(struct fc_lport *lport, struct fc_els_lesb *fc_lesb) { struct net_device *netdev = fcoe_get_netdev(lport); - __fcoe_get_lesb(lport, fc_lesb, netdev); + return __fcoe_get_lesb(lport, fc_lesb, netdev); } EXPORT_SYMBOL_GPL(fcoe_get_lesb); @@ -215,14 +221,14 @@ EXPORT_SYMBOL_GPL(fcoe_get_lesb); * @ctlr_dev: The given fcoe controller device * */ -void fcoe_ctlr_get_lesb(struct fcoe_ctlr_device *ctlr_dev) +int fcoe_ctlr_get_lesb(struct fcoe_ctlr_device *ctlr_dev) { struct fcoe_ctlr *fip = fcoe_ctlr_device_priv(ctlr_dev); struct net_device *netdev = fcoe_get_netdev(fip->lp); struct fc_els_lesb *fc_lesb; fc_lesb = (struct fc_els_lesb *)(&ctlr_dev->lesb); - __fcoe_get_lesb(fip->lp, fc_lesb, netdev); + return __fcoe_get_lesb(fip->lp, fc_lesb, netdev); } EXPORT_SYMBOL_GPL(fcoe_ctlr_get_lesb); diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c index 56003208d2e7..cb299fef7a78 100644 --- a/drivers/scsi/libfc/fc_rport.c +++ b/drivers/scsi/libfc/fc_rport.c @@ -1633,6 +1633,7 @@ static void fc_rport_recv_rls_req(struct fc_rport_priv *rdata, struct fc_els_lesb *lesb; struct fc_seq_els_data rjt_data; struct fc_host_statistics *hst; + int err; lockdep_assert_held(&rdata->rp_mutex); @@ -1659,7 +1660,9 @@ static void fc_rport_recv_rls_req(struct fc_rport_priv *rdata, lesb = &rsp->rls_lesb; if (lport->tt.get_lesb) { /* get LESB from LLD if it supports it */ - lport->tt.get_lesb(lport, lesb); + err = lport->tt.get_lesb(lport, lesb); + if (err) + goto out_rjt; } else { fc_get_host_stats(lport->host); hst = &lport->host_stats; diff --git a/include/scsi/fcoe_sysfs.h b/include/scsi/fcoe_sysfs.h index 4b1216de3f22..076b593f2625 100644 --- a/include/scsi/fcoe_sysfs.h +++ b/include/scsi/fcoe_sysfs.h @@ -16,12 +16,12 @@ struct fcoe_ctlr_device; struct fcoe_fcf_device; struct fcoe_sysfs_function_template { - void (*get_fcoe_ctlr_link_fail)(struct fcoe_ctlr_device *); - void (*get_fcoe_ctlr_vlink_fail)(struct fcoe_ctlr_device *); - void (*get_fcoe_ctlr_miss_fka)(struct fcoe_ctlr_device *); - void (*get_fcoe_ctlr_symb_err)(struct fcoe_ctlr_device *); - void (*get_fcoe_ctlr_err_block)(struct fcoe_ctlr_device *); - void (*get_fcoe_ctlr_fcs_error)(struct fcoe_ctlr_device *); + int (*get_fcoe_ctlr_link_fail)(struct fcoe_ctlr_device *); + int (*get_fcoe_ctlr_vlink_fail)(struct fcoe_ctlr_device *); + int (*get_fcoe_ctlr_miss_fka)(struct fcoe_ctlr_device *); + int (*get_fcoe_ctlr_symb_err)(struct fcoe_ctlr_device *); + int (*get_fcoe_ctlr_err_block)(struct fcoe_ctlr_device *); + int (*get_fcoe_ctlr_fcs_error)(struct fcoe_ctlr_device *); void (*set_fcoe_ctlr_mode)(struct fcoe_ctlr_device *); int (*set_fcoe_ctlr_enabled)(struct fcoe_ctlr_device *); void (*get_fcoe_fcf_selected)(struct fcoe_fcf_device *); diff --git a/include/scsi/libfc.h b/include/scsi/libfc.h index 9b87e1a1c646..510654796db5 100644 --- a/include/scsi/libfc.h +++ b/include/scsi/libfc.h @@ -501,7 +501,7 @@ struct libfc_function_template { * * STATUS: OPTIONAL */ - void (*get_lesb)(struct fc_lport *, struct fc_els_lesb *lesb); + int (*get_lesb)(struct fc_lport *, struct fc_els_lesb *lesb); /* * Reset an exchange manager, completing all sequences and exchanges. diff --git a/include/scsi/libfcoe.h b/include/scsi/libfcoe.h index 2568cb0627ec..a42cbe847ce6 100644 --- a/include/scsi/libfcoe.h +++ b/include/scsi/libfcoe.h @@ -255,13 +255,13 @@ int fcoe_libfc_config(struct fc_lport *, struct fcoe_ctlr *, u32 fcoe_fc_crc(struct fc_frame *fp); int fcoe_start_io(struct sk_buff *skb); int fcoe_get_wwn(struct net_device *netdev, u64 *wwn, int type); -void __fcoe_get_lesb(struct fc_lport *lport, struct fc_els_lesb *fc_lesb, - struct net_device *netdev); +int __fcoe_get_lesb(struct fc_lport *lport, struct fc_els_lesb *fc_lesb, + struct net_device *netdev); void fcoe_wwn_to_str(u64 wwn, char *buf, int len); int fcoe_validate_vport_create(struct fc_vport *vport); int fcoe_link_speed_update(struct fc_lport *); -void fcoe_get_lesb(struct fc_lport *, struct fc_els_lesb *); -void fcoe_ctlr_get_lesb(struct fcoe_ctlr_device *ctlr_dev); +int fcoe_get_lesb(struct fc_lport *, struct fc_els_lesb *); +int fcoe_ctlr_get_lesb(struct fcoe_ctlr_device *ctlr_dev); /** * is_fip_mode() - returns true if FIP mode selected. From patchwork Sat Jan 9 17:26:20 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vladimir Oltean X-Patchwork-Id: 360077 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 14170C433E0 for ; Sat, 9 Jan 2021 17:28:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D592D23A23 for ; Sat, 9 Jan 2021 17:28:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726463AbhAIR26 (ORCPT ); Sat, 9 Jan 2021 12:28:58 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55296 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725926AbhAIR2s (ORCPT ); Sat, 9 Jan 2021 12:28:48 -0500 Received: from mail-ej1-x62c.google.com (mail-ej1-x62c.google.com [IPv6:2a00:1450:4864:20::62c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2122FC0617B0 for ; Sat, 9 Jan 2021 09:27:40 -0800 (PST) Received: by mail-ej1-x62c.google.com with SMTP id d17so18801717ejy.9 for ; Sat, 09 Jan 2021 09:27:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=GFHFsO2k/SRW6fg93UiE4iJUmTDgEkwLLbxjP0lkX+E=; b=taJfwDtXxkgtlKZKQHhuHOxLX0uikBPP9iW8a4o1nL9T/mT1LDuf4uuTKF1Vn89ntc 4SoU+EP6xgFNlPTUrNO+E8eKwFNJKNqvW7hmvZSUDqLZlw+QzN2q2JBHkDdg091XabUF vGDFTHa5B1htRPW9pXtrfMYaJP060oNQSb6/59QrbDIOGTx9aS7vvygeDA5c8Fg45g9u LtVGBBN1gICemGTHLRN76hg0IhbLCtM2NAQlkstaivpE6FvgHuYCoRyopfwkD6rj9p0K NTJMhKq7ar5F6n68Drn0tRUkmHJuSYXq23XMZNysY0oXfWmuE5fl5N6aBOQ/D3VgEfpv pnvg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=GFHFsO2k/SRW6fg93UiE4iJUmTDgEkwLLbxjP0lkX+E=; b=c3O5nN/d9T8zA22tRJdJiUI4JWa0ACmaq1xTOYGIJZq1yR1s7H6ONyvuymsT1wA3sP cq1e8YGUdDyUV1V0iTDQJ4ZnE+lcACEfycTiCywDDK3v9Rg7rk81Bnr6ER53e8XGZgHp cztKBjPRse+HFoL4CzHllZHUy1ocjMdHAhQ3whBHTq1dnuOB4mnLOkcwubUadXz/bnW/ kJZ7IRza1O7+q5tWtRhGylW7b+4d5uJ5aWncSon7jnWBGFxAg8mpra9m1ZfjhSkyAOtm gYOiUFYtAXD+NIPMQ7T5AWcNfTbP3qf1SNp9gy2FKYgUcMM8hNWaCfqfKUaQiSL0IMB0 tLlw== X-Gm-Message-State: AOAM532oLtBhHbg5ocx9m5CwK9aANUAg+414S5EspKoPWH6L4+UsKNwn 4A32M0CM7DYqUREpDG7eJ3A= X-Google-Smtp-Source: ABdhPJyZcYMmjjfMhbdxXkOMcvsk6Ud/SuUvGyGvPvWJIhVaRkx1X5tfVLTH8xLkOhW/Hmm2zW4kGQ== X-Received: by 2002:a17:906:2b1a:: with SMTP id a26mr5880476ejg.23.1610213258705; Sat, 09 Jan 2021 09:27:38 -0800 (PST) Received: from localhost.localdomain (5-12-227-87.residential.rdsnet.ro. [5.12.227.87]) by smtp.gmail.com with ESMTPSA id h16sm4776714eji.110.2021.01.09.09.27.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 09 Jan 2021 09:27:38 -0800 (PST) From: Vladimir Oltean To: "David S . Miller" , Jakub Kicinski Cc: netdev@vger.kernel.org, Andrew Lunn , Florian Fainelli , Cong Wang , Stephen Hemminger , Eric Dumazet , George McCollister , Oleksij Rempel , Jay Vosburgh , Veaceslav Falico , Andy Gospodarek , Arnd Bergmann , Taehee Yoo , Jiri Pirko , Florian Westphal , Nikolay Aleksandrov , Pravin B Shelar , Sridhar Samudrala , Saeed Mahameed Subject: [PATCH v6 net-next 11/15] net: catch errors from dev_get_stats Date: Sat, 9 Jan 2021 19:26:20 +0200 Message-Id: <20210109172624.2028156-12-olteanv@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210109172624.2028156-1-olteanv@gmail.com> References: <20210109172624.2028156-1-olteanv@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Vladimir Oltean dev_get_stats can now return error codes. Convert all remaining call sites to look at that error code and stop processing. The effects of simulating a kernel error (returning -ENOMEM) upon existing programs or kernel interfaces: - ifconfig and "cat /proc/net/dev" print up until the interface that failed, and there they return: cat: read error: Cannot allocate memory - ifstat and "ip -s -s link show": RTNETLINK answers: Cannot allocate memory Dump terminated Some call sites are coming from a context that returns void (ethtool stats, workqueue context). So since we can't report to the upper layer, do the next best thing: print an error to the console. This patch wraps up the conversion of existing dev_get_stats callers, so we can add the __must_check attribute now to ensure that future callers keep doing this too. Signed-off-by: Vladimir Oltean --- Changes in v6: - Fixed rtnetlink incorrectly returning 0 in rtnl_fill_ifinfo on nla_put_failure and causing "ip a" to not show any interfaces. - Squashed the patch to propagate errors with the one to terminate them. Changes in v5: - Actually propagate errors from bonding and net_failover from within this patch. - Properly propagating the dev_get_stats() error code from rtnl_fill_stats now, and not -EMSGSIZE. Changes in v4: Patch is new (Eric's suggestion). arch/s390/appldata/appldata_net_sum.c | 10 ++++-- drivers/leds/trigger/ledtrig-netdev.c | 9 ++++- drivers/net/bonding/bond_main.c | 29 ++++++++++++---- .../ethernet/apm/xgene/xgene_enet_ethtool.c | 9 +++-- .../net/ethernet/hisilicon/hns/hns_ethtool.c | 7 +++- drivers/net/ethernet/intel/e1000e/ethtool.c | 9 +++-- .../net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 9 +++-- drivers/net/ethernet/intel/ixgbevf/ethtool.c | 9 +++-- drivers/net/net_failover.c | 33 ++++++++++++++----- drivers/parisc/led.c | 7 +++- drivers/usb/gadget/function/rndis.c | 4 ++- include/linux/netdevice.h | 3 +- net/8021q/vlanproc.c | 7 ++-- net/core/dev.c | 3 +- net/core/net-procfs.c | 16 ++++++--- net/core/net-sysfs.c | 4 ++- net/core/rtnetlink.c | 23 +++++++++---- 17 files changed, 147 insertions(+), 44 deletions(-) diff --git a/arch/s390/appldata/appldata_net_sum.c b/arch/s390/appldata/appldata_net_sum.c index 6146606ac9a3..72cb5344e488 100644 --- a/arch/s390/appldata/appldata_net_sum.c +++ b/arch/s390/appldata/appldata_net_sum.c @@ -58,11 +58,11 @@ struct appldata_net_sum_data { */ static void appldata_get_net_sum_data(void *data) { - int i; struct appldata_net_sum_data *net_data; struct net_device *dev; unsigned long rx_packets, tx_packets, rx_bytes, tx_bytes, rx_errors, tx_errors, rx_dropped, tx_dropped, collisions; + int ret, i; net_data = data; net_data->sync_count_1++; @@ -83,7 +83,13 @@ static void appldata_get_net_sum_data(void *data) for_each_netdev(&init_net, dev) { struct rtnl_link_stats64 stats; - dev_get_stats(dev, &stats); + ret = dev_get_stats(dev, &stats); + if (ret) { + netif_lists_unlock(&init_net); + netdev_err(dev, "dev_get_stats returned %d\n", ret); + return; + } + rx_packets += stats.rx_packets; tx_packets += stats.tx_packets; rx_bytes += stats.rx_bytes; diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c index 4382ee278309..c717b7e7dd81 100644 --- a/drivers/leds/trigger/ledtrig-netdev.c +++ b/drivers/leds/trigger/ledtrig-netdev.c @@ -351,6 +351,7 @@ static void netdev_trig_work(struct work_struct *work) unsigned int new_activity; unsigned long interval; int invert; + int err; /* If we dont have a device, insure we are off */ if (!trigger_data->net_dev) { @@ -363,7 +364,13 @@ static void netdev_trig_work(struct work_struct *work) !test_bit(NETDEV_LED_RX, &trigger_data->mode)) return; - dev_get_stats(trigger_data->net_dev, &dev_stats); + err = dev_get_stats(trigger_data->net_dev, &dev_stats); + if (err) { + netdev_err(trigger_data->net_dev, + "dev_get_stats returned %d\n", err); + return; + } + new_activity = (test_bit(NETDEV_LED_TX, &trigger_data->mode) ? dev_stats.tx_packets : 0) + diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 17faf431d85b..b70ca0e41142 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -1757,7 +1757,12 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev, slave_dev->priv_flags |= IFF_BONDING; /* initialize slave stats */ - dev_get_stats(new_slave->dev, &new_slave->slave_stats); + res = dev_get_stats(new_slave->dev, &new_slave->slave_stats); + if (res) { + slave_err(bond_dev, slave_dev, "dev_get_stats returned %d\n", + res); + goto err_close; + } if (bond_is_lb(bond)) { /* bond_alb_init_slave() must be called before all other stages since @@ -2070,6 +2075,7 @@ static int __bond_release_one(struct net_device *bond_dev, struct sockaddr_storage ss; int old_flags = bond_dev->flags; netdev_features_t old_features = bond_dev->features; + int err; /* slave is not a slave or master is not master of this slave */ if (!(slave_dev->flags & IFF_SLAVE) || @@ -2088,13 +2094,19 @@ static int __bond_release_one(struct net_device *bond_dev, return -EINVAL; } + /* recompute stats just before removing the slave */ + err = bond_get_stats(bond->dev, &bond->bond_stats); + if (err) { + slave_info(bond_dev, slave_dev, "dev_get_stats returned %d\n", + err); + unblock_netpoll_tx(); + return err; + } + bond_set_slave_inactive_flags(slave, BOND_SLAVE_NOTIFY_NOW); bond_sysfs_slave_del(slave); - /* recompute stats just before removing the slave */ - bond_get_stats(bond->dev, &bond->bond_stats); - bond_upper_dev_unlink(bond, slave); /* unregister rx_handler early so bond_handle_frame wouldn't be called * for this slave anymore. @@ -3742,7 +3754,7 @@ static int bond_get_stats(struct net_device *bond_dev, struct list_head *iter; struct slave *slave; int nest_level = 0; - + int res = 0; rcu_read_lock(); #ifdef CONFIG_LOCKDEP @@ -3753,7 +3765,9 @@ static int bond_get_stats(struct net_device *bond_dev, memcpy(stats, &bond->bond_stats, sizeof(*stats)); bond_for_each_slave_rcu(bond, slave, iter) { - dev_get_stats(slave->dev, &temp); + res = dev_get_stats(slave->dev, &temp); + if (res) + goto out; bond_fold_stats(stats, &temp, &slave->slave_stats); @@ -3762,10 +3776,11 @@ static int bond_get_stats(struct net_device *bond_dev, } memcpy(&bond->bond_stats, stats, sizeof(*stats)); +out: spin_unlock(&bond->stats_lock); rcu_read_unlock(); - return 0; + return res; } static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd) diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c b/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c index ada70425b48c..aab6a81f0438 100644 --- a/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c @@ -266,9 +266,14 @@ static void xgene_get_ethtool_stats(struct net_device *ndev, { struct xgene_enet_pdata *pdata = netdev_priv(ndev); struct rtnl_link_stats64 stats; - int i; + int err, i; + + err = dev_get_stats(ndev, &stats); + if (err) { + netdev_err(ndev, "dev_get_stats returned %d\n", err); + return; + } - dev_get_stats(ndev, &stats); for (i = 0; i < XGENE_STATS_LEN; i++) data[i] = *(u64 *)((char *)&stats + gstrings_stats[i].offset); diff --git a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c index ee2172011051..d05fa7b3f6e0 100644 --- a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c +++ b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c @@ -840,6 +840,7 @@ static void hns_get_ethtool_stats(struct net_device *netdev, struct hns_nic_priv *priv = netdev_priv(netdev); struct hnae_handle *h = priv->ae_handle; struct rtnl_link_stats64 net_stats; + int err; if (!h->dev->ops->get_stats || !h->dev->ops->update_stats) { netdev_err(netdev, "get_stats or update_stats is null!\n"); @@ -848,7 +849,11 @@ static void hns_get_ethtool_stats(struct net_device *netdev, h->dev->ops->update_stats(h, &netdev->stats); - dev_get_stats(netdev, &net_stats); + err = dev_get_stats(netdev, &net_stats); + if (err) { + netdev_err(netdev, "dev_get_stats returned %d\n", err); + return; + } /* get netdev statistics */ p[0] = net_stats.rx_packets; diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c index 06442e6bef73..41bd3e0598ce 100644 --- a/drivers/net/ethernet/intel/e1000e/ethtool.c +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c @@ -2060,15 +2060,20 @@ static void e1000_get_ethtool_stats(struct net_device *netdev, { struct e1000_adapter *adapter = netdev_priv(netdev); struct rtnl_link_stats64 net_stats; - int i; char *p = NULL; + int err, i; pm_runtime_get_sync(netdev->dev.parent); - dev_get_stats(netdev, &net_stats); + err = dev_get_stats(netdev, &net_stats); pm_runtime_put_sync(netdev->dev.parent); + if (err) { + netdev_err(netdev, "dev_get_stats returned %d\n", err); + return; + } + for (i = 0; i < E1000_GLOBAL_STATS_LEN; i++) { switch (e1000_gstrings_stats[i].type) { case NETDEV_STATS: diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c index 2b8084664403..a647e2774f76 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c @@ -1298,11 +1298,16 @@ static void ixgbe_get_ethtool_stats(struct net_device *netdev, struct rtnl_link_stats64 net_stats; unsigned int start; struct ixgbe_ring *ring; - int i, j; char *p = NULL; + int err, i, j; ixgbe_update_stats(adapter); - dev_get_stats(netdev, &net_stats); + err = dev_get_stats(netdev, &net_stats); + if (err) { + netdev_err(netdev, "dev_get_stats returned %d\n", err); + return; + } + for (i = 0; i < IXGBE_GLOBAL_STATS_LEN; i++) { switch (ixgbe_gstrings_stats[i].type) { case NETDEV_STATS: diff --git a/drivers/net/ethernet/intel/ixgbevf/ethtool.c b/drivers/net/ethernet/intel/ixgbevf/ethtool.c index 3b9b7e5c2998..665e39301092 100644 --- a/drivers/net/ethernet/intel/ixgbevf/ethtool.c +++ b/drivers/net/ethernet/intel/ixgbevf/ethtool.c @@ -423,11 +423,16 @@ static void ixgbevf_get_ethtool_stats(struct net_device *netdev, struct rtnl_link_stats64 net_stats; unsigned int start; struct ixgbevf_ring *ring; - int i, j; + int err, i, j; char *p; ixgbevf_update_stats(adapter); - dev_get_stats(netdev, &net_stats); + err = dev_get_stats(netdev, &net_stats); + if (err) { + netdev_err(netdev, "dev_get_stats returned %d\n", err); + return; + } + for (i = 0; i < IXGBEVF_GLOBAL_STATS_LEN; i++) { switch (ixgbevf_gstrings_stats[i].type) { case NETDEV_STATS: diff --git a/drivers/net/net_failover.c b/drivers/net/net_failover.c index e032ad1c5e22..2815228a34d5 100644 --- a/drivers/net/net_failover.c +++ b/drivers/net/net_failover.c @@ -185,6 +185,7 @@ static int net_failover_get_stats(struct net_device *dev, struct net_failover_info *nfo_info = netdev_priv(dev); struct rtnl_link_stats64 temp; struct net_device *slave_dev; + int err = 0; spin_lock(&nfo_info->stats_lock); memcpy(stats, &nfo_info->failover_stats, sizeof(*stats)); @@ -193,24 +194,29 @@ static int net_failover_get_stats(struct net_device *dev, slave_dev = rcu_dereference(nfo_info->primary_dev); if (slave_dev) { - dev_get_stats(slave_dev, &temp); + err = dev_get_stats(slave_dev, &temp); + if (err) + goto out; net_failover_fold_stats(stats, &temp, &nfo_info->primary_stats); memcpy(&nfo_info->primary_stats, &temp, sizeof(temp)); } slave_dev = rcu_dereference(nfo_info->standby_dev); if (slave_dev) { - dev_get_stats(slave_dev, &temp); + err = dev_get_stats(slave_dev, &temp); + if (err) + goto out; net_failover_fold_stats(stats, &temp, &nfo_info->standby_stats); memcpy(&nfo_info->standby_stats, &temp, sizeof(temp)); } +out: rcu_read_unlock(); memcpy(&nfo_info->failover_stats, stats, sizeof(*stats)); spin_unlock(&nfo_info->stats_lock); - return 0; + return err; } static int net_failover_change_mtu(struct net_device *dev, int new_mtu) @@ -545,11 +551,15 @@ static int net_failover_slave_register(struct net_device *slave_dev, if (slave_is_standby) { rcu_assign_pointer(nfo_info->standby_dev, slave_dev); standby_dev = slave_dev; - dev_get_stats(standby_dev, &nfo_info->standby_stats); + err = dev_get_stats(standby_dev, &nfo_info->standby_stats); + if (err) + goto err_stats_get; } else { rcu_assign_pointer(nfo_info->primary_dev, slave_dev); primary_dev = slave_dev; - dev_get_stats(primary_dev, &nfo_info->primary_stats); + err = dev_get_stats(primary_dev, &nfo_info->primary_stats); + if (err) + goto err_stats_get; failover_dev->min_mtu = slave_dev->min_mtu; failover_dev->max_mtu = slave_dev->max_mtu; } @@ -564,6 +574,8 @@ static int net_failover_slave_register(struct net_device *slave_dev, return 0; +err_stats_get: + vlan_vids_del_by_dev(slave_dev, failover_dev); err_vlan_add: dev_uc_unsync(slave_dev, failover_dev); dev_mc_unsync(slave_dev, failover_dev); @@ -597,6 +609,7 @@ static int net_failover_slave_unregister(struct net_device *slave_dev, struct net_device *standby_dev, *primary_dev; struct net_failover_info *nfo_info; bool slave_is_standby; + int err; nfo_info = netdev_priv(failover_dev); primary_dev = rtnl_dereference(nfo_info->primary_dev); @@ -611,7 +624,8 @@ static int net_failover_slave_unregister(struct net_device *slave_dev, dev_close(slave_dev); nfo_info = netdev_priv(failover_dev); - dev_get_stats(failover_dev, &nfo_info->failover_stats); + /* Proceed with the deregistration anyway */ + err = dev_get_stats(failover_dev, &nfo_info->failover_stats); slave_is_standby = slave_dev->dev.parent == failover_dev->dev.parent; if (slave_is_standby) { @@ -631,7 +645,7 @@ static int net_failover_slave_unregister(struct net_device *slave_dev, netdev_info(failover_dev, "failover %s slave:%s unregistered\n", slave_is_standby ? "standby" : "primary", slave_dev->name); - return 0; + return err; } static int net_failover_slave_link_change(struct net_device *slave_dev, @@ -639,6 +653,7 @@ static int net_failover_slave_link_change(struct net_device *slave_dev, { struct net_device *primary_dev, *standby_dev; struct net_failover_info *nfo_info; + int err; nfo_info = netdev_priv(failover_dev); @@ -653,7 +668,9 @@ static int net_failover_slave_link_change(struct net_device *slave_dev, netif_carrier_on(failover_dev); netif_tx_wake_all_queues(failover_dev); } else { - dev_get_stats(failover_dev, &nfo_info->failover_stats); + err = dev_get_stats(failover_dev, &nfo_info->failover_stats); + if (err) + return err; netif_carrier_off(failover_dev); netif_tx_stop_all_queues(failover_dev); } diff --git a/drivers/parisc/led.c b/drivers/parisc/led.c index cc6108785323..d17d0fbf878d 100644 --- a/drivers/parisc/led.c +++ b/drivers/parisc/led.c @@ -370,7 +370,12 @@ static __inline__ int led_get_net_activity(void) in_dev_put(in_dev); - dev_get_stats(dev, &stats); + retval = dev_get_stats(dev, &stats); + if (retval) { + netif_lists_unlock(&init_net); + return retval; + } + rx_total += stats.rx_packets; tx_total += stats.tx_packets; } diff --git a/drivers/usb/gadget/function/rndis.c b/drivers/usb/gadget/function/rndis.c index 7ec29e007ae9..bec474819c3d 100644 --- a/drivers/usb/gadget/function/rndis.c +++ b/drivers/usb/gadget/function/rndis.c @@ -198,7 +198,9 @@ static int gen_ndis_query_resp(struct rndis_params *params, u32 OID, u8 *buf, resp->InformationBufferOffset = cpu_to_le32(16); net = params->dev; - dev_get_stats(net, &stats); + retval = dev_get_stats(net, &stats); + if (retval) + return retval; switch (OID) { diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index bd471f1e1fa3..b1aebab916a8 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -4545,7 +4545,8 @@ void netdev_notify_peers(struct net_device *dev); void netdev_features_change(struct net_device *dev); /* Load a device via the kmod */ void dev_load(struct net *net, const char *name); -int dev_get_stats(struct net_device *dev, struct rtnl_link_stats64 *storage); +int __must_check dev_get_stats(struct net_device *dev, + struct rtnl_link_stats64 *storage); void netdev_stats_to_stats64(struct rtnl_link_stats64 *stats64, const struct net_device_stats *netdev_stats); void dev_fetch_sw_netstats(struct rtnl_link_stats64 *s, diff --git a/net/8021q/vlanproc.c b/net/8021q/vlanproc.c index 3a6682d79630..d89b75804834 100644 --- a/net/8021q/vlanproc.c +++ b/net/8021q/vlanproc.c @@ -244,12 +244,15 @@ static int vlandev_seq_show(struct seq_file *seq, void *offset) const struct vlan_dev_priv *vlan = vlan_dev_priv(vlandev); static const char fmt64[] = "%30s %12llu\n"; struct rtnl_link_stats64 stats; - int i; + int err, i; if (!is_vlan_dev(vlandev)) return 0; - dev_get_stats(vlandev, &stats); + err = dev_get_stats(vlandev, &stats); + if (err) + return err; + seq_printf(seq, "%s VID: %d REORDER_HDR: %i dev->priv_flags: %hx\n", vlandev->name, vlan->vlan_id, diff --git a/net/core/dev.c b/net/core/dev.c index dfbd66ba3cad..30facac95d5e 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -10403,7 +10403,8 @@ EXPORT_SYMBOL(netdev_stats_to_stats64); * dev->netdev_ops->get_stats64 or dev->netdev_ops->get_stats; * otherwise the internal statistics structure is used. */ -int dev_get_stats(struct net_device *dev, struct rtnl_link_stats64 *storage) +int __must_check dev_get_stats(struct net_device *dev, + struct rtnl_link_stats64 *storage) { const struct net_device_ops *ops = dev->netdev_ops; int err = 0; diff --git a/net/core/net-procfs.c b/net/core/net-procfs.c index 64666ba7ccab..ee19c35f6e00 100644 --- a/net/core/net-procfs.c +++ b/net/core/net-procfs.c @@ -78,11 +78,14 @@ static void dev_seq_stop(struct seq_file *seq, void *v) netif_lists_unlock(net); } -static void dev_seq_printf_stats(struct seq_file *seq, struct net_device *dev) +static int dev_seq_printf_stats(struct seq_file *seq, struct net_device *dev) { struct rtnl_link_stats64 stats; + int err; - dev_get_stats(dev, &stats); + err = dev_get_stats(dev, &stats); + if (err) + return err; seq_printf(seq, "%6s: %7llu %7llu %4llu %4llu %4llu %5llu %10llu %9llu " "%8llu %7llu %4llu %4llu %4llu %5llu %7llu %10llu\n", @@ -101,6 +104,8 @@ static void dev_seq_printf_stats(struct seq_file *seq, struct net_device *dev) stats.tx_window_errors + stats.tx_heartbeat_errors, stats.tx_compressed); + + return 0; } /* @@ -109,6 +114,8 @@ static void dev_seq_printf_stats(struct seq_file *seq, struct net_device *dev) */ static int dev_seq_show(struct seq_file *seq, void *v) { + int err = 0; + if (v == SEQ_START_TOKEN) seq_puts(seq, "Inter-| Receive " " | Transmit\n" @@ -116,8 +123,9 @@ static int dev_seq_show(struct seq_file *seq, void *v) "compressed multicast|bytes packets errs " "drop fifo colls carrier compressed\n"); else - dev_seq_printf_stats(seq, v); - return 0; + err = dev_seq_printf_stats(seq, v); + + return err; } static u32 softnet_backlog_len(struct softnet_data *sd) diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c index 5d89c85b42d4..6f789e178e92 100644 --- a/net/core/net-sysfs.c +++ b/net/core/net-sysfs.c @@ -588,7 +588,9 @@ static ssize_t netstat_show(const struct device *d, if (dev_isalive(dev)) { struct rtnl_link_stats64 stats; - dev_get_stats(dev, &stats); + ret = dev_get_stats(dev, &stats); + if (ret) + return ret; ret = sprintf(buf, fmt_u64, *(u64 *)(((u8 *)&stats) + offset)); } diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index bb0596c41b3e..6cc87094924d 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -1201,6 +1201,7 @@ static noinline_for_stack int rtnl_fill_stats(struct sk_buff *skb, { struct rtnl_link_stats64 *sp; struct nlattr *attr; + int err; attr = nla_reserve_64bit(skb, IFLA_STATS64, sizeof(struct rtnl_link_stats64), IFLA_PAD); @@ -1208,7 +1209,9 @@ static noinline_for_stack int rtnl_fill_stats(struct sk_buff *skb, return -EMSGSIZE; sp = nla_data(attr); - dev_get_stats(dev, sp); + err = dev_get_stats(dev, sp); + if (err) + return err; attr = nla_reserve(skb, IFLA_STATS, sizeof(struct rtnl_link_stats)); @@ -1707,6 +1710,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, { struct ifinfomsg *ifm; struct nlmsghdr *nlh; + int err; ASSERT_RTNL(); nlh = nlmsg_put(skb, pid, seq, type, sizeof(*ifm), flags); @@ -1780,8 +1784,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, if (rtnl_phys_switch_id_fill(skb, dev)) goto nla_put_failure; - if (rtnl_fill_stats(skb, dev)) - goto nla_put_failure; + err = rtnl_fill_stats(skb, dev); + if (err) + goto get_stats_failure; if (rtnl_fill_vf(skb, dev, ext_filter_mask)) goto nla_put_failure; @@ -1825,8 +1830,10 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, nla_put_failure_rcu: rcu_read_unlock(); nla_put_failure: + err = -EMSGSIZE; +get_stats_failure: nlmsg_cancel(skb, nlh); - return -EMSGSIZE; + return err; } static const struct nla_policy ifla_policy[IFLA_MAX+1] = { @@ -5135,7 +5142,9 @@ static int rtnl_fill_statsinfo(struct sk_buff *skb, struct net_device *dev, goto nla_put_failure; sp = nla_data(attr); - dev_get_stats(dev, sp); + err = dev_get_stats(dev, sp); + if (err) + goto get_stats_failure; } if (stats_attr_valid(filter_mask, IFLA_STATS_LINK_XSTATS, *idxattr)) { @@ -5242,13 +5251,15 @@ static int rtnl_fill_statsinfo(struct sk_buff *skb, struct net_device *dev, return 0; nla_put_failure: + err = -EMSGSIZE; +get_stats_failure: /* not a multi message or no progress mean a real error */ if (!(flags & NLM_F_MULTI) || s_prividx == *prividx) nlmsg_cancel(skb, nlh); else nlmsg_end(skb, nlh); - return -EMSGSIZE; + return err; } static size_t if_nlmsg_stats_size(const struct net_device *dev, From patchwork Sat Jan 9 17:26:21 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vladimir Oltean X-Patchwork-Id: 360076 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 39B26C433DB for ; Sat, 9 Jan 2021 17:29:04 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id EFF4F23A23 for ; Sat, 9 Jan 2021 17:29:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726490AbhAIR3D (ORCPT ); Sat, 9 Jan 2021 12:29:03 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55298 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726385AbhAIR2s (ORCPT ); Sat, 9 Jan 2021 12:28:48 -0500 Received: from mail-ed1-x52b.google.com (mail-ed1-x52b.google.com [IPv6:2a00:1450:4864:20::52b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CCA77C0617B1 for ; Sat, 9 Jan 2021 09:27:41 -0800 (PST) Received: by mail-ed1-x52b.google.com with SMTP id cm17so14483181edb.4 for ; Sat, 09 Jan 2021 09:27:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=0reHTgeA6uUXSGgmhIjkpBPzkJXmAfi9+BfDtKYO0M0=; b=T1l0K0tJXBAmg00fFgY/VbFQIxiu222QJYPwYx1oEQc3nYMBCGFKJDIShcgsxgDXyQ WA1mE/dV1bpOBv7EVh1OsgDmaXXpa9T+cTIzvyBlBplaWgwF0ucVlEJ/NBjLzTRfIiBZ iXwKaP564vyfPIn+fo1Dkgfx+DYQYnfIkZGBgsELjMe18laGltwld/u6CIWglyW0uNpE OiAgHlVodc9tRpu9tJ3rvEywgARybDtog2wGAWml8ZmRIgHOv6UoDJmiloRB/juu94yK NwS+c6v/vOyx5gyHD3f8csTepimoUHgcVm5h1jVjmQ8KxMKJw4/tBeYlxl5WLcL2YGj1 quIQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=0reHTgeA6uUXSGgmhIjkpBPzkJXmAfi9+BfDtKYO0M0=; b=apUHgeXQRn/N7TrToXHYZORCRAz4o+8Gfpzaqm5Wnw78T6oWN1UtGpg1XfYVpv4C3x o9HQJYkfmQgcOhpyN5O+CFhEwygnLW4zSxuhHPrau3LQ8A3XhMvwfqhQz5pXIJomGMrS ZfQref+GPFCSf+6VXNw+0qSAJQ0+YsFxnmMXSywMZH7BbcPUgM3vszw/yrFlfOEkvkic rtzitGxXxPaf8Hun9cg4TD5Mnt/gjN6lPJPryLKg5dHLVv0Nsh43DjMFadUL92Mk+MzZ dz1o3YsMq2Pueg4/dYmeCYWLdAZuTTQ2r/khp0SRDsc58H02YqZwUFNCBS6roRj4XaX6 Bgbw== X-Gm-Message-State: AOAM530YzUmRZvSqa+XnXLzrTsf/hjRcNfJY6iq2f3zOzesAmhbiOsdZ Qwu7UpwEOPNZNLWW5KCq3/M= X-Google-Smtp-Source: ABdhPJwnfzPOfr254LlsTqe0CwzKwJD4W7P44aKOu/bv7awPI5dn7L42zgiEn1rt9FGa4hYQjfZLcg== X-Received: by 2002:a50:b5c5:: with SMTP id a63mr9125638ede.227.1610213260387; Sat, 09 Jan 2021 09:27:40 -0800 (PST) Received: from localhost.localdomain (5-12-227-87.residential.rdsnet.ro. [5.12.227.87]) by smtp.gmail.com with ESMTPSA id h16sm4776714eji.110.2021.01.09.09.27.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 09 Jan 2021 09:27:39 -0800 (PST) From: Vladimir Oltean To: "David S . Miller" , Jakub Kicinski Cc: netdev@vger.kernel.org, Andrew Lunn , Florian Fainelli , Cong Wang , Stephen Hemminger , Eric Dumazet , George McCollister , Oleksij Rempel , Jay Vosburgh , Veaceslav Falico , Andy Gospodarek , Arnd Bergmann , Taehee Yoo , Jiri Pirko , Florian Westphal , Nikolay Aleksandrov , Pravin B Shelar , Sridhar Samudrala , Saeed Mahameed Subject: [PATCH v6 net-next 12/15] net: openvswitch: ensure dev_get_stats can sleep Date: Sat, 9 Jan 2021 19:26:21 +0200 Message-Id: <20210109172624.2028156-13-olteanv@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210109172624.2028156-1-olteanv@gmail.com> References: <20210109172624.2028156-1-olteanv@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Vladimir Oltean There is an effort to convert .ndo_get_stats64 to sleepable context, and for that to work, we need to prevent callers of dev_get_stats from using atomic locking. The OVS vport driver calls ovs_vport_get_stats from ovs_vport_cmd_fill_info, a function with 7 callers: 5 under ovs_lock() and 2 under rcu_read_lock(). The RCU-protected callers are the doit and dumpit callbacks of the OVS_VPORT_CMD_GET genetlink event. Things have been this way ever since the OVS introduction in commit ccb1352e76cf ("net: Add Open vSwitch kernel components."), probably so that OVS_PORT_CMD_GET doesn't have to serialize with all the others through ovs_mutex. Sadly, now they do have to, otherwise we don't have protection while accessing the datapath and vport structures. Convert all callers of ovs_vport_cmd_fill_info to assume ovs_mutex protection. This means that we can get rid of the gfp argument, since all callers are now sleepable, we can just use GFP_KERNEL for memory allocation. Signed-off-by: Vladimir Oltean --- Changes in v6: None. Changes in v5: None. Changes in v4: Patch is new. net/openvswitch/datapath.c | 38 ++++++++++++++++++-------------------- net/openvswitch/vport.c | 2 +- 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c index 160b8dc453da..318caa8f12c2 100644 --- a/net/openvswitch/datapath.c +++ b/net/openvswitch/datapath.c @@ -1957,10 +1957,10 @@ static struct genl_family dp_datapath_genl_family __ro_after_init = { .module = THIS_MODULE, }; -/* Called with ovs_mutex or RCU read lock. */ +/* Called with ovs_mutex */ static int ovs_vport_cmd_fill_info(struct vport *vport, struct sk_buff *skb, struct net *net, u32 portid, u32 seq, - u32 flags, u8 cmd, gfp_t gfp) + u32 flags, u8 cmd) { struct ovs_header *ovs_header; struct ovs_vport_stats vport_stats; @@ -1981,7 +1981,7 @@ static int ovs_vport_cmd_fill_info(struct vport *vport, struct sk_buff *skb, goto nla_put_failure; if (!net_eq(net, dev_net(vport->dev))) { - int id = peernet2id_alloc(net, dev_net(vport->dev), gfp); + int id = peernet2id_alloc(net, dev_net(vport->dev), GFP_KERNEL); if (nla_put_s32(skb, OVS_VPORT_ATTR_NETNSID, id)) goto nla_put_failure; @@ -2029,8 +2029,7 @@ struct sk_buff *ovs_vport_cmd_build_info(struct vport *vport, struct net *net, if (!skb) return ERR_PTR(-ENOMEM); - retval = ovs_vport_cmd_fill_info(vport, skb, net, portid, seq, 0, cmd, - GFP_KERNEL); + retval = ovs_vport_cmd_fill_info(vport, skb, net, portid, seq, 0, cmd); BUG_ON(retval == -EMSGSIZE); if (retval) return ERR_PTR(retval); @@ -2038,7 +2037,7 @@ struct sk_buff *ovs_vport_cmd_build_info(struct vport *vport, struct net *net, return skb; } -/* Called with ovs_mutex or RCU read lock. */ +/* Called with ovs_mutex */ static struct vport *lookup_vport(struct net *net, const struct ovs_header *ovs_header, struct nlattr *a[OVS_VPORT_ATTR_MAX + 1]) @@ -2177,7 +2176,7 @@ static int ovs_vport_cmd_new(struct sk_buff *skb, struct genl_info *info) err = ovs_vport_cmd_fill_info(vport, reply, genl_info_net(info), info->snd_portid, info->snd_seq, 0, - OVS_VPORT_CMD_NEW, GFP_KERNEL); + OVS_VPORT_CMD_NEW); BUG_ON(err == -EMSGSIZE); if (err) goto exit_unlock_free; @@ -2240,7 +2239,7 @@ static int ovs_vport_cmd_set(struct sk_buff *skb, struct genl_info *info) err = ovs_vport_cmd_fill_info(vport, reply, genl_info_net(info), info->snd_portid, info->snd_seq, 0, - OVS_VPORT_CMD_SET, GFP_KERNEL); + OVS_VPORT_CMD_SET); BUG_ON(err == -EMSGSIZE); if (err) goto exit_unlock_free; @@ -2282,7 +2281,7 @@ static int ovs_vport_cmd_del(struct sk_buff *skb, struct genl_info *info) err = ovs_vport_cmd_fill_info(vport, reply, genl_info_net(info), info->snd_portid, info->snd_seq, 0, - OVS_VPORT_CMD_DEL, GFP_KERNEL); + OVS_VPORT_CMD_DEL); BUG_ON(err == -EMSGSIZE); if (err) goto exit_unlock_free; @@ -2324,23 +2323,23 @@ static int ovs_vport_cmd_get(struct sk_buff *skb, struct genl_info *info) if (!reply) return -ENOMEM; - rcu_read_lock(); + ovs_lock(); vport = lookup_vport(sock_net(skb->sk), ovs_header, a); err = PTR_ERR(vport); if (IS_ERR(vport)) goto exit_unlock_free; err = ovs_vport_cmd_fill_info(vport, reply, genl_info_net(info), info->snd_portid, info->snd_seq, 0, - OVS_VPORT_CMD_GET, GFP_ATOMIC); + OVS_VPORT_CMD_GET); BUG_ON(err == -EMSGSIZE); if (err) goto exit_unlock_free; - rcu_read_unlock(); + ovs_unlock(); return genlmsg_reply(reply, info); exit_unlock_free: - rcu_read_unlock(); + ovs_unlock(); kfree_skb(reply); return err; } @@ -2352,25 +2351,24 @@ static int ovs_vport_cmd_dump(struct sk_buff *skb, struct netlink_callback *cb) int bucket = cb->args[0], skip = cb->args[1]; int i, j = 0; - rcu_read_lock(); - dp = get_dp_rcu(sock_net(skb->sk), ovs_header->dp_ifindex); + ovs_lock(); + dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex); if (!dp) { - rcu_read_unlock(); + ovs_unlock(); return -ENODEV; } for (i = bucket; i < DP_VPORT_HASH_BUCKETS; i++) { struct vport *vport; j = 0; - hlist_for_each_entry_rcu(vport, &dp->ports[i], dp_hash_node) { + hlist_for_each_entry(vport, &dp->ports[i], dp_hash_node) { if (j >= skip && ovs_vport_cmd_fill_info(vport, skb, sock_net(skb->sk), NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq, NLM_F_MULTI, - OVS_VPORT_CMD_GET, - GFP_ATOMIC) < 0) + OVS_VPORT_CMD_GET) < 0) goto out; j++; @@ -2378,7 +2376,7 @@ static int ovs_vport_cmd_dump(struct sk_buff *skb, struct netlink_callback *cb) skip = 0; } out: - rcu_read_unlock(); + ovs_unlock(); cb->args[0] = i; cb->args[1] = j; diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c index e66c949fd97a..ba1a52addff2 100644 --- a/net/openvswitch/vport.c +++ b/net/openvswitch/vport.c @@ -265,7 +265,7 @@ void ovs_vport_del(struct vport *vport) * * Retrieves transmit, receive, and error stats for the given device. * - * Must be called with ovs_mutex or rcu_read_lock. + * Must be called with ovs_mutex. */ int ovs_vport_get_stats(struct vport *vport, struct ovs_vport_stats *stats) { From patchwork Sat Jan 9 17:26:23 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vladimir Oltean X-Patchwork-Id: 360078 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6A9A9C433E6 for ; Sat, 9 Jan 2021 17:28:53 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 35A9B23A23 for ; Sat, 9 Jan 2021 17:28:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726433AbhAIR2w (ORCPT ); Sat, 9 Jan 2021 12:28:52 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55306 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726410AbhAIR2t (ORCPT ); Sat, 9 Jan 2021 12:28:49 -0500 Received: from mail-ed1-x531.google.com (mail-ed1-x531.google.com [IPv6:2a00:1450:4864:20::531]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EDB8AC0617BA for ; Sat, 9 Jan 2021 09:27:44 -0800 (PST) Received: by mail-ed1-x531.google.com with SMTP id p22so14382229edu.11 for ; Sat, 09 Jan 2021 09:27:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=uumYfeLD/WT/2ZsjqYAQDNsvJgxI+0mz0rY4cpEgYn0=; b=se0sxtYp0Azmk6d3K14TUKAL/KhsIi7wf2z7UlNH2gQcz3SQjQF9kQSnpnjVp0i8iz WLdgIM8TbjAwnPFWkitaVKoLOJlDMugMk+hoMn5tJUzU0rtz6YAgg7VHnmLPMSRhc+eo MC9sasB7AV7kZspAKhc43lrFKS/eQ7m6NYDak7cixAaFcFhwkQcxt55171H+XkwsxYnV VuBMlujbQcq0KsosdgTSjR9ITBvIri0F/nj4/emhoZ+cG0RnJ7GzojB9yBwXeA3CLWLK 0ZG9bQ+DUcmZARugUz6XGjfuMRjWHFxDmc4IdR5+m8BWt6JKYzgqaxr5k/TLFuLWCsMB eZ/g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=uumYfeLD/WT/2ZsjqYAQDNsvJgxI+0mz0rY4cpEgYn0=; b=s55+eDdPU94qS/tRQ0zty/+Yq/sRm8ct6TJYw1qa6jCS20cS3ZTZkaEwduF2DRh01L O5t89WsEzd1HcGH33LIE2+ZvQcvqb+Fa00ftgvsReaSp2ilsfZBu/2qMCrAXqgVrES/o IhXX6lErxtW/ID9rnCwcSBmBxISV3Qzb6YJKnYjyp0kg/Pcm1f4njLFKf6OXEoB+8iX5 vpGzHkK4qSd75olIl1osmj6P6Z1Bm//ePyOIW6lAW1AjCpeHlPnAzvfmXpkMHsYBU/qs 4r0bAHG79rqHqtNZXubmuZzmR1XcLggAbjD1y4GOdq89DvSTgPSqoCDsZyl30ud0qHzG D+8A== X-Gm-Message-State: AOAM531xZOyw4zwaMpvP5wgQ9H3FfH285gYUAjJPKVENnWYfY+NLXlp4 vtO8LxEFsGXdX9cSx6Qdbhw= X-Google-Smtp-Source: ABdhPJyCHIcV5u0P/3TSBFgUvRl9aILufdIdFp97r21ZEtU568X9K5oc0RYp4T4uovJAJuAoYsruEA== X-Received: by 2002:aa7:c7d8:: with SMTP id o24mr9325399eds.328.1610213263635; Sat, 09 Jan 2021 09:27:43 -0800 (PST) Received: from localhost.localdomain (5-12-227-87.residential.rdsnet.ro. [5.12.227.87]) by smtp.gmail.com with ESMTPSA id h16sm4776714eji.110.2021.01.09.09.27.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 09 Jan 2021 09:27:43 -0800 (PST) From: Vladimir Oltean To: "David S . Miller" , Jakub Kicinski Cc: netdev@vger.kernel.org, Andrew Lunn , Florian Fainelli , Cong Wang , Stephen Hemminger , Eric Dumazet , George McCollister , Oleksij Rempel , Jay Vosburgh , Veaceslav Falico , Andy Gospodarek , Arnd Bergmann , Taehee Yoo , Jiri Pirko , Florian Westphal , Nikolay Aleksandrov , Pravin B Shelar , Sridhar Samudrala , Saeed Mahameed Subject: [PATCH v6 net-next 14/15] net: bonding: ensure .ndo_get_stats64 can sleep Date: Sat, 9 Jan 2021 19:26:23 +0200 Message-Id: <20210109172624.2028156-15-olteanv@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210109172624.2028156-1-olteanv@gmail.com> References: <20210109172624.2028156-1-olteanv@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Vladimir Oltean There is an effort to convert .ndo_get_stats64 to sleepable context, and for that to work, we need to prevent callers of dev_get_stats from using atomic locking. The bonding driver retrieves its statistics recursively from its lower interfaces, with additional care to only count packets sent/received while those lowers were actually enslaved to the bond - see commit 5f0c5f73e5ef ("bonding: make global bonding stats more reliable"). Since commit 87163ef9cda7 ("bonding: remove last users of bond->lock and bond->lock itself"), the bonding driver uses the following protection for its array of slaves: RCU for readers and rtnl_mutex for updaters. The aforementioned commit removed an interesting comment: /* [...] we can't hold bond->lock [...] because we'll * deadlock. The only solution is to rely on the fact * that we're under rtnl_lock here, and the slaves * list won't change. This doesn't solve the problem * of setting the slave's MTU while it is * transmitting, but the assumption is that the base * driver can handle that. * * TODO: figure out a way to safely iterate the slaves * list, but without holding a lock around the actual * call to the base driver. */ The above summarizes pretty well the challenges we have with nested bonding interfaces (bond over bond over bond over...) and locking for their slaves. To solve the nesting problem, the simple way is to not hold any locks when recursing into the slave netdev operation. We can "cheat" and use dev_hold to take a reference on the slave net_device, which is enough to ensure that netdev_wait_allrefs() waits until we finish, and the kernel won't fault. However, the slave structure might no longer be valid, just its associated net_device. So we need to do some more work to ensure that the slave exists after we took the statistics, and if it still does, reapply the logic from Andy's commit 5f0c5f73e5ef. Tested using the following two scripts running in parallel: #!/bin/bash while :; do ip link del bond0 ip link del bond1 ip link add bond0 type bond mode 802.3ad ip link add bond1 type bond mode 802.3ad ip link set sw0p1 down && ip link set sw0p1 master bond0 && ip link set sw0p1 up ip link set sw0p2 down && ip link set sw0p2 master bond0 && ip link set sw0p2 up ip link set sw0p3 down && ip link set sw0p3 master bond0 && ip link set sw0p3 up ip link set bond0 down && ip link set bond0 master bond1 && ip link set bond0 up ip link set sw1p1 down && ip link set sw1p1 master bond1 && ip link set sw1p1 up ip link set bond1 up ip -s -s link show cat /sys/class/net/bond1/statistics/* done #!/bin/bash while :; do echo spi2.0 > /sys/bus/spi/drivers/sja1105/unbind echo spi2.0 > /sys/bus/spi/drivers/sja1105/bind sleep 30 done where the sja1105 driver was explicitly modified for the purpose of this test to have a msleep(500) in its .ndo_get_stats64 method, to catch some more potential races. Signed-off-by: Vladimir Oltean --- Changes in v6: s/break/return -ENOMEM/ in bond_get_slaves. Changes in v5: - Use rcu_read_lock() and do not change the locking architecture of the driver. - Gave some details on my testing procedure. Changes in v4: Now there is code to propagate errors. Changes in v3: - Added kfree(dev_stats); - Removed the now useless stats_lock (bond->bond_stats and slave->slave_stats are now protected by bond->slaves_lock) Changes in v2: Switched to the new scheme of holding just a refcnt to the slave interfaces while recursing with dev_get_stats. drivers/net/bonding/bond_main.c | 113 +++++++++++++++----------------- include/net/bonding.h | 53 +++++++++++++++ 2 files changed, 107 insertions(+), 59 deletions(-) diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index b70ca0e41142..84d4e97007cb 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -3705,80 +3705,75 @@ static void bond_fold_stats(struct rtnl_link_stats64 *_res, } } -#ifdef CONFIG_LOCKDEP -static int bond_get_lowest_level_rcu(struct net_device *dev) -{ - struct net_device *ldev, *next, *now, *dev_stack[MAX_NEST_DEV + 1]; - struct list_head *niter, *iter, *iter_stack[MAX_NEST_DEV + 1]; - int cur = 0, max = 0; - - now = dev; - iter = &dev->adj_list.lower; - - while (1) { - next = NULL; - while (1) { - ldev = netdev_next_lower_dev_rcu(now, &iter); - if (!ldev) - break; - - next = ldev; - niter = &ldev->adj_list.lower; - dev_stack[cur] = now; - iter_stack[cur++] = iter; - if (max <= cur) - max = cur; - break; - } - - if (!next) { - if (!cur) - return max; - next = dev_stack[--cur]; - niter = iter_stack[cur]; - } - - now = next; - iter = niter; - } - - return max; -} -#endif - static int bond_get_stats(struct net_device *bond_dev, struct rtnl_link_stats64 *stats) { struct bonding *bond = netdev_priv(bond_dev); - struct rtnl_link_stats64 temp; - struct list_head *iter; - struct slave *slave; - int nest_level = 0; - int res = 0; + struct rtnl_link_stats64 *dev_stats; + struct bonding_slave_dev *s; + struct list_head slaves; + int res, num_slaves; + int i = 0; - rcu_read_lock(); -#ifdef CONFIG_LOCKDEP - nest_level = bond_get_lowest_level_rcu(bond_dev); -#endif + res = bond_get_slaves(bond, &slaves, &num_slaves); + if (res) + return res; - spin_lock_nested(&bond->stats_lock, nest_level); - memcpy(stats, &bond->bond_stats, sizeof(*stats)); + dev_stats = kcalloc(num_slaves, sizeof(*dev_stats), GFP_KERNEL); + if (!dev_stats) { + bond_put_slaves(&slaves); + return -ENOMEM; + } - bond_for_each_slave_rcu(bond, slave, iter) { - res = dev_get_stats(slave->dev, &temp); + /* Recurse with no locks taken */ + list_for_each_entry(s, &slaves, list) { + res = dev_get_stats(s->ndev, &dev_stats[i]); if (res) goto out; + i++; + } + + spin_lock(&bond->stats_lock); + + memcpy(stats, &bond->bond_stats, sizeof(*stats)); + + /* Because we released the RCU lock in bond_get_slaves, the new slave + * array might be different from the original one, so we need to take + * it again and only update the stats of the slaves that still exist. + */ + rcu_read_lock(); + + i = 0; + + list_for_each_entry(s, &slaves, list) { + struct list_head *iter; + struct slave *slave; - bond_fold_stats(stats, &temp, &slave->slave_stats); + bond_for_each_slave_rcu(bond, slave, iter) { + if (slave->dev != s->ndev) + continue; - /* save off the slave stats for the next run */ - memcpy(&slave->slave_stats, &temp, sizeof(temp)); + bond_fold_stats(stats, &dev_stats[i], + &slave->slave_stats); + + /* save off the slave stats for the next run */ + memcpy(&slave->slave_stats, &dev_stats[i], + sizeof(dev_stats[i])); + break; + } + + i++; } + rcu_read_unlock(); + memcpy(&bond->bond_stats, stats, sizeof(*stats)); -out: + spin_unlock(&bond->stats_lock); - rcu_read_unlock(); + +out: + kfree(dev_stats); + bond_put_slaves(&slaves); return res; } diff --git a/include/net/bonding.h b/include/net/bonding.h index adc3da776970..2df1f7ea9036 100644 --- a/include/net/bonding.h +++ b/include/net/bonding.h @@ -449,6 +449,59 @@ static inline void bond_hw_addr_copy(u8 *dst, const u8 *src, unsigned int len) memcpy(dst, src, len); } +/* Helpers for reference counting the struct net_device behind the bond slaves. + * These can be used to propagate the net_device_ops from the bond to the + * slaves while not holding rcu_read_lock() or the rtnl_mutex. + */ +struct bonding_slave_dev { + struct net_device *ndev; + struct list_head list; +}; + +static inline void bond_put_slaves(struct list_head *slaves) +{ + struct bonding_slave_dev *s, *tmp; + + list_for_each_entry_safe(s, tmp, slaves, list) { + dev_put(s->ndev); + list_del(&s->list); + kfree(s); + } +} + +static inline int bond_get_slaves(struct bonding *bond, + struct list_head *slaves, + int *num_slaves) +{ + struct list_head *iter; + struct slave *slave; + + INIT_LIST_HEAD(slaves); + *num_slaves = 0; + + rcu_read_lock(); + + bond_for_each_slave_rcu(bond, slave, iter) { + struct bonding_slave_dev *s; + + s = kzalloc(sizeof(*s), GFP_ATOMIC); + if (!s) { + rcu_read_unlock(); + bond_put_slaves(slaves); + return -ENOMEM; + } + + s->ndev = slave->dev; + dev_hold(s->ndev); + list_add_tail(&s->list, slaves); + (*num_slaves)++; + } + + rcu_read_unlock(); + + return 0; +} + #define BOND_PRI_RESELECT_ALWAYS 0 #define BOND_PRI_RESELECT_BETTER 1 #define BOND_PRI_RESELECT_FAILURE 2