From mboxrd@z Thu Jan 1 00:00:00 1970 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=-16.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 5AC2AC433DB for ; Mon, 11 Jan 2021 22:26:05 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2897622D03 for ; Mon, 11 Jan 2021 22:26:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390193AbhAKWZt (ORCPT ); Mon, 11 Jan 2021 17:25:49 -0500 Received: from mail.kernel.org ([198.145.29.99]:55594 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2389320AbhAKWZt (ORCPT ); Mon, 11 Jan 2021 17:25:49 -0500 Received: by mail.kernel.org (Postfix) with ESMTPSA id 8B66922D0B; Mon, 11 Jan 2021 22:24:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1610403894; bh=/mBulmBYWYLmzUDnOrABFLE0aRMTrHDtmWt3NavFSI4=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=aSIB6m8VU5UB9RmVATFeVp+cRPeEr8MDW8ZGXGmM5tNNeQKnpfctjiqYgVPFisO0s iquPTfNqgnVac1ip7q7dlYyXreJ7J953cP3ccITuDv5Jj0P/8zwsk174ria8ewQDuA a/w3cjaojZH/Xy0ukuc5NdnuJilcwopG8b8I0cYoShUgYA3hfAD/CkYm+y2fZf++EY 1ECFi4c9d8lWIJloF/tvGsVon9tPRJl9HzfxYfxMkFgGYVrPYSOBqSOXVp8ANYN0CA snjVM86TSlSSbN/id5kM/XpZE6JpGiC/wxveJowGB2Ichclt40yxNT2ag+2M8Ort+w Il2fzU2OtbJng== Message-ID: <3e065920586e87e58a365eac94b69aabb3b244cb.camel@kernel.org> Subject: Re: [PATCH v6 net-next 08/15] net: allow ndo_get_stats64 to return an int error code From: Saeed Mahameed To: Vladimir Oltean , "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 Date: Mon, 11 Jan 2021 14:24:52 -0800 In-Reply-To: <20210109172624.2028156-9-olteanv@gmail.com> References: <20210109172624.2028156-1-olteanv@gmail.com> <20210109172624.2028156-9-olteanv@gmail.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.36.5 (3.36.5-2.fc32) MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Sat, 2021-01-09 at 19:26 +0200, Vladimir Oltean wrote: > From: Vladimir Oltean > > Some drivers need to do special tricks to comply with the new policy > of > ndo_get_stats64 being sleepable. For example, the bonding driver, > which > derives its stats from its lower interfaces, must recurse with > dev_get_stats into its lowers with no locks held. But for that to > work, > it needs to dynamically allocate some memory for a refcounted copy of > its array of slave interfaces (because recursing unlocked means that > the > original one is subject to disappearing). And since memory allocation > can fail under pressure, we should not let it go unnoticed, but > instead > propagate the error code. > > This patch converts all implementations of .ndo_get_stats64 to return > int, and propagates that to the dev_get_stats calling site. Error > checking will be done in further patches. > > Signed-off-by: Vladimir Oltean > --- > Changes in v6: > Remove the unused "int err" from __bond_release_one and add it in the > patch it belongs to. > > Changes in v5: > None. > > Changes in v4: > Patch is new (Eric's suggestion). > Just Give Eric the proper credit and add: Suggested-by: Eric .. [...] > @@ -354,4 +356,4 @@ int rmnet_vnd_update_dev_mtu(struct rmnet_port > *port, > } > > return 0; > -} > \ No newline at end of file > +} Not related? [...] > > -void dev_get_stats(struct net_device *dev, struct rtnl_link_stats64 > *storage) > +int dev_get_stats(struct net_device *dev, struct rtnl_link_stats64 > *storage) > { > const struct net_device_ops *ops = dev->netdev_ops; > + int err = 0; > > if (ops->ndo_get_stats64) { > memset(storage, 0, sizeof(*storage)); > - ops->ndo_get_stats64(dev, storage); > + err = ops->ndo_get_stats64(dev, storage); > } else if (ops->ndo_get_stats) { > netdev_stats_to_stats64(storage, ops- > >ndo_get_stats(dev)); > } else { > @@ -10418,6 +10419,8 @@ void dev_get_stats(struct net_device *dev, > struct rtnl_link_stats64 *storage) > storage->rx_dropped += (unsigned long)atomic_long_read(&dev- > >rx_dropped); > storage->tx_dropped += (unsigned long)atomic_long_read(&dev- > >tx_dropped); > storage->rx_nohandler += (unsigned long)atomic_long_read(&dev- > >rx_nohandler); Must be consistent here, if there was an error you should abort without touching the caller provided storage, even if you can for some stats. > + > + return err; > } > EXPORT_SYMBOL(dev_get_stats); >