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=-1.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,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 1AB0EC43381 for ; Fri, 15 Feb 2019 15:55:05 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D4F282190C for ; Fri, 15 Feb 2019 15:55:04 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=Mellanox.com header.i=@Mellanox.com header.b="Y7BVdZMI" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388572AbfBOPzD (ORCPT ); Fri, 15 Feb 2019 10:55:03 -0500 Received: from mail-eopbgr50071.outbound.protection.outlook.com ([40.107.5.71]:36736 "EHLO EUR03-VE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1729485AbfBOPzD (ORCPT ); Fri, 15 Feb 2019 10:55:03 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=FJOvu9eYn8rPtM0RD31RnSlxU+HWIkOVWsMlaGvq+3U=; b=Y7BVdZMIrLtskWUc35KHfGlbee9Aejub55roF42D9ZcBax/BydE6ypYp55YxywtcP4Fb8mcihROLIKlBobZucIGjnNPwrel7uy2r+QgDlciOGE4dVv0gKP5vQHPTfzE/57nft0+DSqLdp4rV+Sl3edhQV1gpwa52mffn1Ffmffs= Received: from HE1PR0502MB3641.eurprd05.prod.outlook.com (10.167.127.11) by HE1PR0502MB3897.eurprd05.prod.outlook.com (10.167.143.28) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1601.21; Fri, 15 Feb 2019 15:54:54 +0000 Received: from HE1PR0502MB3641.eurprd05.prod.outlook.com ([fe80::4041:bb68:2af3:eab8]) by HE1PR0502MB3641.eurprd05.prod.outlook.com ([fe80::4041:bb68:2af3:eab8%5]) with mapi id 15.20.1622.018; Fri, 15 Feb 2019 15:54:54 +0000 From: Vlad Buslov To: Stefano Brivio CC: "netdev@vger.kernel.org" , "jhs@mojatatu.com" , "xiyou.wangcong@gmail.com" , "jiri@resnulli.us" , "davem@davemloft.net" Subject: Re: [PATCH net-next 04/12] net: sched: flower: track filter deletion with flag Thread-Topic: [PATCH net-next 04/12] net: sched: flower: track filter deletion with flag Thread-Index: AQHUxDmffbwHRjP1qEqJek/exrQB6KXfxRgAgAFABQA= Date: Fri, 15 Feb 2019 15:54:54 +0000 Message-ID: References: <20190214074712.17846-1-vladbu@mellanox.com> <20190214074712.17846-5-vladbu@mellanox.com> <20190214214926.0cb839a7@redhat.com> In-Reply-To: <20190214214926.0cb839a7@redhat.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-clientproxiedby: LO2P265CA0426.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:a0::30) To HE1PR0502MB3641.eurprd05.prod.outlook.com (2603:10a6:7:85::11) authentication-results: spf=none (sender IP is ) smtp.mailfrom=vladbu@mellanox.com; x-ms-exchange-messagesentrepresentingtype: 1 x-originating-ip: [37.142.13.130] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 0f644840-b89d-435c-fc4b-08d6935ded46 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0;PCL:0;RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600110)(711020)(4605077)(4618075)(2017052603328)(7153060)(7193020);SRVR:HE1PR0502MB3897; x-ms-traffictypediagnostic: HE1PR0502MB3897: x-microsoft-exchange-diagnostics: =?iso-8859-1?Q?1;HE1PR0502MB3897;23:FTxiXrKUeUaL3CzAhhkdqKB2kbfI/TZ8Gu0g/?= =?iso-8859-1?Q?KbNEah8BhkccxQcVcI41VKZ6td5HcyJtnHdfuvKIH3AIDlg/SvgYN2CBHN?= =?iso-8859-1?Q?jgVlXiz9pXQexHzavl4iODd7KdgPaWbWOPA6xUEWkQ8NnigArx/tZgVWr+?= =?iso-8859-1?Q?J9pdsYkTKVs9Ee5z9y79qVWR5djBu1YVG6Ksg8Z1kfdKV0my68VaskqxQE?= =?iso-8859-1?Q?nO+KKUpl3sRuR86FjHSdGzK1XGuXnkjG5W3b6QjHrzDkqn05Vc03DcJHI+?= =?iso-8859-1?Q?d0uAf7qfUlNeGm6aRXI9y9fCG4AgfEY5Xm0EgnSRPtukqnloGsI74HoKlm?= =?iso-8859-1?Q?49Hxh2fs8Zx2ioEC+fxL5ySesQcqh+KnKb/n0Vig4yX+NOzjJ/NEJVzir6?= =?iso-8859-1?Q?4wYC7v7cWV7Xe/Yxf7Wj2WbV5qljjp91xU47NDPcoWUH7yn0hf7zOpfp9s?= =?iso-8859-1?Q?ZQvk6zFVMQguUrr/uT5rQQ9joAd7uBL5Hlnxifz759lV/tbmHCj+4dZowG?= =?iso-8859-1?Q?A+oCsXjuVO7P0t1gpw6D5VzjfDg2cpvhf6eA3ZISNHEJDTzhA7HiNWWr2c?= =?iso-8859-1?Q?5eWWatO0dHKIwfC1eIXugbrRvO2vkdAl35bdDJYn5VJS4Un5sYUXYzKTYH?= =?iso-8859-1?Q?W/aFkc/U+9+P4VyjAVH9zF2Sv1hLWU/HGIoAhUC2twRHx2q3dtiazBLQjs?= =?iso-8859-1?Q?vDMkZKj40YbQix/R7GumZZP6kDy41SvB+GIATf1HtFEFhTgRwtfCmVQZzq?= =?iso-8859-1?Q?pbgW17R5KI5X1bBzz95B50FZvk6olTg8qxS3gwo2BgxwHEJ4lQGtI57cBB?= =?iso-8859-1?Q?QjkjQHGyZ2GDDNvjM/sXa6FFDFJ3Y80LYvEYto1iL5JooN//OUqQmV5Egm?= =?iso-8859-1?Q?UAbAyl2F9IU9WhgniaVgYL4all0ISp9WEk48Tsf969Nd325D0EATsRdVMQ?= =?iso-8859-1?Q?WCJG/YTu5PGqiRWxDNCRb8GaTUp72cDgr+2GW3dqRh1ny0SRiLNdvDbFps?= =?iso-8859-1?Q?IfDWT+rgaXY3tQZvwjcXCjskAyR+LOLa+I9NbHOKtAh7gSzWUvwK3q/Kdz?= =?iso-8859-1?Q?0C47UN9UzhGZwi6PRMugq1GcLNuEgWmKLh9gP3m4tSFg9M7GgwHueyNy8O?= =?iso-8859-1?Q?/tKROvJNQZKcH4ONmkcZ2A2HHT0g1QrZ6oGkinFo2iYKKjjN6DdN85r9iy?= =?iso-8859-1?Q?XvgOUsgcWfPHtpJKn68BXboyc6SZI1g9rZS61J5KyQOJX3tzl56zkAJr8v?= =?iso-8859-1?Q?9VC0GPrnUwzFfFmmRmF?= x-microsoft-antispam-prvs: x-forefront-prvs: 09497C15EB x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(136003)(396003)(39860400002)(376002)(346002)(366004)(199004)(189003)(6486002)(6116002)(476003)(4326008)(446003)(316002)(2616005)(11346002)(6916009)(3846002)(486006)(54906003)(7736002)(99286004)(86362001)(8676002)(81166006)(81156014)(71190400001)(68736007)(71200400001)(8936002)(102836004)(76176011)(105586002)(66066001)(106356001)(14454004)(97736004)(6506007)(52116002)(6246003)(53936002)(386003)(186003)(6436002)(305945005)(26005)(36756003)(229853002)(2906002)(14444005)(256004)(6512007)(478600001)(25786009);DIR:OUT;SFP:1101;SCL:1;SRVR:HE1PR0502MB3897;H:HE1PR0502MB3641.eurprd05.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;A:1;MX:1; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: grtC2AzUbMb4nXOmFffATrZmJnE4rdNkDRVif4rQFH7r7AeOIjBGy8fevFvxKsnygsTkq0qbGdSc5c6xpg/L1cRIdMhs7YYEZaJVaVtm3OAiFH4RmZugs7IZ6TZv7eKexfPbArHE4O5lOsdM+tKXjmEp5dZ9uN45o+jVJxSlyvsW3/FePJshOaPd0k0sQzghkRkcVnv6RLTCXVfJ8Ff1Q/i1V+EsDv5+Tu82Ub9R+RiyYTtNejHLu4uNevYoAC4UAJZ/cCty5Wc4cAT5XbNMA9VD5vcbPQk9uRKhdUoo3vSUkKvKfIbgVqqY2sMwTwqsa1gXGSeF+cSAcwS+vWhUYBWVCVhXJm7r0YB6hWAuyo7xTGT2iW1/Gt53TN+x1o1UA8Q33wBCG8cxa9u1EggiZ78luW6USXoYeHhoZa7mWlQ= Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: 0f644840-b89d-435c-fc4b-08d6935ded46 X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Feb 2019 15:54:53.3101 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR0502MB3897 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Thu 14 Feb 2019 at 20:49, Stefano Brivio wrote: > On Thu, 14 Feb 2019 09:47:04 +0200 > Vlad Buslov wrote: > >> +static int __fl_delete(struct tcf_proto *tp, struct cls_fl_filter *f, >> + bool *last, struct netlink_ext_ack *extack) > > This would be easier to follow (at least for me): > >> { >> struct cls_fl_head *head =3D fl_head_dereference(tp); >> bool async =3D tcf_exts_get_net(&f->exts); >> - bool last; >> - >> - idr_remove(&head->handle_idr, f->handle); >> - list_del_rcu(&f->list); >> - last =3D fl_mask_put(head, f->mask, async); >> - if (!tc_skip_hw(f->flags)) >> - fl_hw_destroy_filter(tp, f, extack); >> - tcf_unbind_filter(tp, &f->res); >> - __fl_put(f); >> + int err =3D 0; > > without this > >> + >> + (*last) =3D false; > > with *last =3D false; > >> + >> + if (!f->deleted) { > > with: > if (f->deleted) > return -ENOENT; > >> + f->deleted =3D true; >> + rhashtable_remove_fast(&f->mask->ht, &f->ht_node, >> + f->mask->filter_ht_params); >> + idr_remove(&head->handle_idr, f->handle); >> + list_del_rcu(&f->list); >> + (*last) =3D fl_mask_put(head, f->mask, async); > > with: > *last =3D fl_mask_put(head, f->mask, async); > >> + if (!tc_skip_hw(f->flags)) >> + fl_hw_destroy_filter(tp, f, extack); >> + tcf_unbind_filter(tp, &f->res); >> + __fl_put(f); > > and a return 0; here Agree, this function looks better when structured in the way you suggest. Will change it in V2. > >> + } else { >> + err =3D -ENOENT; >> + } >> =20 >> - return last; >> + return err; >> } >> =20 >> [...] >> >> @@ -1520,14 +1541,14 @@ static int fl_delete(struct tcf_proto *tp, void = *arg, bool *last, >> { >> struct cls_fl_head *head =3D fl_head_dereference(tp); >> struct cls_fl_filter *f =3D arg; >> + bool last_on_mask; > > This is unused in this series, maybe change __fl_delete() to optionally > take NULL as 'bool *last' argument? It was implemented like that originally, but on internal review with Jiri we decided that having unused variable here is better than complicating __fl_delete() with support for "last" being NULL without good reason. > >> + int err =3D 0; > > Nit: no need to initialise this. Yes, but I always regret having uninitialized variables in my functions later on :( > =20 >> - rhashtable_remove_fast(&f->mask->ht, &f->ht_node, >> - f->mask->filter_ht_params); >> - __fl_delete(tp, f, extack); >> + err =3D __fl_delete(tp, f, &last_on_mask, extack); >> *last =3D list_empty(&head->masks); >> __fl_put(f); >> =20 >> - return 0; >> + return err; >> } >> =20 >> static void fl_walk(struct tcf_proto *tp, struct tcf_walker *arg,