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=-7.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,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 8E718C43381 for ; Tue, 26 Feb 2019 15:09:00 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 4A1CD2184D for ; Tue, 26 Feb 2019 15:09:00 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=Mellanox.com header.i=@Mellanox.com header.b="JYLMDMbG" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726810AbfBZPI6 (ORCPT ); Tue, 26 Feb 2019 10:08:58 -0500 Received: from mail-eopbgr130070.outbound.protection.outlook.com ([40.107.13.70]:36069 "EHLO EUR01-HE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726245AbfBZPI6 (ORCPT ); Tue, 26 Feb 2019 10:08:58 -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=xAJ4wl08MhIrZnNv3R1ErT5ufg4bW24pUYGxyDf6bFc=; b=JYLMDMbGWPX/F5OppjySdIo8QyaSoMdielevD0+nMXykbk8iQ65R5QLqY+claAlM/UwA1kZf7ZXGx+IxYIrTgG5e/+JuvECYVHr6JpjP+GcDOSoKSQ8Ka6QUpcho2+/YVcTh81THjcYvBlYBwH41tMR22sTYC/zhQvGY1YFgZ3I= Received: from HE1PR0502MB3641.eurprd05.prod.outlook.com (10.167.127.11) by HE1PR0502MB3740.eurprd05.prod.outlook.com (10.167.127.142) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1643.14; Tue, 26 Feb 2019 15:08:45 +0000 Received: from HE1PR0502MB3641.eurprd05.prod.outlook.com ([fe80::b03d:8cd4:d259:f749]) by HE1PR0502MB3641.eurprd05.prod.outlook.com ([fe80::b03d:8cd4:d259:f749%5]) with mapi id 15.20.1643.019; Tue, 26 Feb 2019 15:08:45 +0000 From: Vlad Buslov To: Cong Wang CC: Linux Kernel Network Developers , Jamal Hadi Salim , Jiri Pirko , David Miller Subject: Re: [PATCH net-next] net: sched: set dedicated tcf_walker flag when tp is empty Thread-Topic: [PATCH net-next] net: sched: set dedicated tcf_walker flag when tp is empty Thread-Index: AQHUzSA84+Ovu25HxU6CQqb1GENP0KXxH2oAgAEQo4A= Date: Tue, 26 Feb 2019 15:08:45 +0000 Message-ID: References: <20190225153831.10037-1-vladbu@mellanox.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-clientproxiedby: LNXP123CA0016.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:d2::28) To HE1PR0502MB3641.eurprd05.prod.outlook.com (2603:10a6:7:85::11) x-ms-exchange-messagesentrepresentingtype: 1 x-originating-ip: [37.142.13.130] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 9e24222a-06b3-4795-7471-08d69bfc4d2c x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0;PCL:0;RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600127)(711020)(4605104)(4618075)(2017052603328)(7153060)(7193020);SRVR:HE1PR0502MB3740; x-ms-traffictypediagnostic: HE1PR0502MB3740: x-microsoft-exchange-diagnostics: =?iso-8859-1?Q?1;HE1PR0502MB3740;23:JqRnD9disZQV5voJTv5JOYFRhDIY1dNkWGlJB?= =?iso-8859-1?Q?yohYjhm36ijcUBLf7+4RBMfpEgxB+Csc6UQAKiH1iMrBJv87TFwi+WqaUA?= =?iso-8859-1?Q?NU6HiwlvXoDozIG0FTDtaEFLhU2+hXHeyWxqb0nMXouEOec90YxsxWrmN1?= =?iso-8859-1?Q?T9Pz1xMOutAoaqdRE93EV4kmkOljeANr8NszaV3+f7mMNmIdrVypwFkGgG?= =?iso-8859-1?Q?T4l/79lZHnGDC1N/jXwE01IB4tjP7K4bbgdql8YXil5VtszuL6mjPWmziC?= =?iso-8859-1?Q?kOxPACjVYSIokaXq5hoQQ6SOMzHQbP59KAkcjemSeH60Z3An/TnY0Sby31?= =?iso-8859-1?Q?kMBIB2JFcy59/ro0jFctzAoJigKuwyM3mVLvXew4C5wgkOYn9tk4xBNJDF?= =?iso-8859-1?Q?sqrpnCc+gw5Oa0LvflQn+rBUQYm0zFqjFMH7JqLRVoboxDk+XdafyCPYqY?= =?iso-8859-1?Q?4IXdjoKsbE3tMHFxPLfS8N1rI9wOVWsukcFHVfW8KnyRx23yfZGPQsH4Sr?= =?iso-8859-1?Q?RWvQuz1E/9gOBfk7OygBfFA6na++mz8Y6eNDBxxgzYIaC9ZAHECjgNEg3k?= =?iso-8859-1?Q?E3R2rFjXpChyABt4fSkbKTTPW5cIeNe/LexZmPyBJQoR9ie++sS+k2xIx1?= =?iso-8859-1?Q?ZKLCk7MX/Z7kxrJ7XLTmeMQMNLmqaKX96J/rmUZrNN6dOSfRqYOSYfczI8?= =?iso-8859-1?Q?LWkAFcMLBuzUNilF7JH9FKZO0uCFfDaywpUodXc4xlvTgemk1Xy9oAH4j3?= =?iso-8859-1?Q?7DlTK0k4E76rfAqQRF0qSAsrY2qH2EoJcVwxxeNNO5QKR+RqdsYfBUAGjf?= =?iso-8859-1?Q?naWRKQd3juNntqjKH4SBj1RnGMOC86IQJ5JkLm8VhN6xoYllV1zB8o88yI?= =?iso-8859-1?Q?ZlzMXeBBFr0uxGXbQ22PP5PUJtBJ1lOyeiJvyVRiDDrs+h4LwCgO+3wTWK?= =?iso-8859-1?Q?+6prqVdV9Xghiawn6+PW3AIkJjwl+/WgHUKiP3yC+homTZsKRvVlnpaDcV?= =?iso-8859-1?Q?yAjgXxcUnWMsmtJe88GI1mKXMpVRjUXWdpvm3MAskiQc5QpHanPK0+STzW?= =?iso-8859-1?Q?jkjso4pezmiMv4k3FPaSeUTbIUEvJTLItvqVIaXRgpeotccOykCHbnY4zJ?= =?iso-8859-1?Q?10YnxpkSgGjXNkvE9Q4iS+fpJrSXCoNTC/05UxPu3OHzpJaVqw3Y4QsgVi?= =?iso-8859-1?Q?DsZThTJlj8T9Y6WPMhXsyeensp1P0fSmtlpSOnmFiH4D6yrCZDfUO16LyS?= =?iso-8859-1?Q?PzHmzg5D1S3fAd0f6zohoTVVoIjPmB57zp2Uqs1AMkQR1GLkpSoDMlQ+14?= =?iso-8859-1?Q?AGrn+s6k5KvLdg7OjmPmH2RuA?= x-microsoft-antispam-prvs: x-forefront-prvs: 096029FF66 x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(366004)(39860400002)(136003)(396003)(376002)(346002)(189003)(199004)(5660300002)(8936002)(229853002)(486006)(6916009)(6436002)(52116002)(316002)(2906002)(478600001)(86362001)(71200400001)(305945005)(256004)(14444005)(6486002)(105586002)(106356001)(99286004)(54906003)(102836004)(476003)(11346002)(6116002)(71190400001)(3846002)(2616005)(26005)(186003)(6346003)(14454004)(446003)(386003)(6506007)(53546011)(6512007)(53936002)(7736002)(66066001)(25786009)(68736007)(76176011)(81156014)(36756003)(8676002)(81166006)(4326008)(6246003)(97736004);DIR:OUT;SFP:1101;SCL:1;SRVR:HE1PR0502MB3740;H:HE1PR0502MB3641.eurprd05.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;MX:1;A:1; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) authentication-results: spf=none (sender IP is ) smtp.mailfrom=vladbu@mellanox.com; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: T5q33WjDRfBs+yAR0zDs/xyRVR/2qGk+zDyPI4kGr8ZesFhc3OIYDErq1QJRCCWCtBjOh35sj4fcanCw/Y8omXOIfEqfhEKP77ruXgcMpQ9Qjzqp6jQbBDo+2PCMtmWkKHfIWmjve6K61kcs/3mn9I+7EizcyZpubePa7o2Fng/hEqAnfbKDA9PvVbZSsK8urLekiuFPRDjCGaob5pOXDgZOvnIijyFAXg/lnBY0lcEYxbOSp6BfDQETxku9VHmBDxx/4InnUcf+yT+a8NT6GAyTO4r01JK8We4aBwXcVQikpeCCaH3PisWW54WKvw2BlKr0DL+UOFdf+LSN5G5yfsOx2SSvhwONWgHdO275mZ7Rx6RZpWwvKW5M9xTe/nHnjCy3T8OpExqiBUkmAN/k+XWRWx4xKsqL+h8VnY4hNG8= 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: 9e24222a-06b3-4795-7471-08d69bfc4d2c X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Feb 2019 15:08:43.8920 (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: HE1PR0502MB3740 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Mon 25 Feb 2019 at 22:52, Cong Wang wrote: > On Mon, Feb 25, 2019 at 7:38 AM Vlad Buslov wrote: >> >> Using tcf_walker->stop flag to determine when tcf_walker->fn() was calle= d >> at least once is unreliable. Some classifiers set 'stop' flag on error >> before calling walker callback, other classifiers used to call it with N= ULL >> filter pointer when empty. In order to prevent further regressions, exte= nd >> tcf_walker structure with dedicated 'nonempty' flag. Set this flag in >> tcf_walker->fn() implementation that is used to check if classifier has >> filters configured. > > > So, after this patch commits like 31a998487641 ("net: sched: fw: don't > set arg->stop in fw_walk() when empty") can be reverted?? Yes, it is safe now to revert following commits: 3027ff41f67c ("net: sched: route: don't set arg->stop in route4_walk() when= empty") 31a998487641 ("net: sched: fw: don't set arg->stop in fw_walk() when empty"= ) > > >> >> Fixes: 8b64678e0af8 ("net: sched: refactor tp insert/delete for concurre= nt execution") >> Signed-off-by: Vlad Buslov >> Suggested-by: Cong Wang >> --- >> include/net/pkt_cls.h | 1 + >> net/sched/cls_api.c | 13 +++++++++---- >> 2 files changed, 10 insertions(+), 4 deletions(-) >> >> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h >> index 232f801f2a21..422dd8800478 100644 >> --- a/include/net/pkt_cls.h >> +++ b/include/net/pkt_cls.h >> @@ -17,6 +17,7 @@ struct tcf_walker { >> int stop; >> int skip; >> int count; >> + bool nonempty; >> unsigned long cookie; >> int (*fn)(struct tcf_proto *, void *node, struct tcf_walker = *); >> }; >> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c >> index e2c888961379..3543be31d400 100644 >> --- a/net/sched/cls_api.c >> +++ b/net/sched/cls_api.c >> @@ -238,18 +238,23 @@ static void tcf_proto_put(struct tcf_proto *tp, bo= ol rtnl_held, >> tcf_proto_destroy(tp, rtnl_held, extack); >> } >> >> -static int walker_noop(struct tcf_proto *tp, void *d, struct tcf_walker= *arg) >> +static int walker_check_empty(struct tcf_proto *tp, void *d, >> + struct tcf_walker *arg) >> { >> - return -1; >> + if (tp) { >> + arg->nonempty =3D true; >> + return -1; >> + } >> + return 0; > > How does this even work? If we can simply check tp!=3DNULL as > non-empty, why do we even need a walker?? > > For me, it must be pushed down to each implementation to > determine how it is empty. Sorry, this is a typo. Intention is to check the filter pointer (void *d). Sending the fix. Thanks for spotting this!