[{"id":1764530,"web_url":"http://patchwork.ozlabs.org/comment/1764530/","msgid":"<20170907063216.GB1967@nanopsycho>","list_archive_url":null,"date":"2017-09-07T06:32:16","subject":"Re: [Patch net v2 2/2] net_sched: fix all the madness of tc filter\n\tchain","submitter":{"id":15321,"url":"http://patchwork.ozlabs.org/api/people/15321/","name":"Jiri Pirko","email":"jiri@resnulli.us"},"content":"Thu, Sep 07, 2017 at 06:26:07AM CEST, xiyou.wangcong@gmail.com wrote:\n>This patch fixes the following madness of tc filter chain:\n\nCould you avoid expressive words like \"madness\" and such?\nPlease be technical.\n\n\n>\n>1) tcf_chain_destroy() is called by both tcf_block_put() and\n> tcf_chain_put(). tcf_chain_put() is correctly refcnt'ed and paired\n> with tcf_chain_get(), but tcf_block_put() is not, it should be paired\n> with tcf_block_get() which means we still need to decrease the refcnt.\n> Think it in another way: if we call tcf_bock_put() immediately after\n> tcf_block_get(), could we get effectively a nop? This causes a memory\n> leak as reported by Jakub.\n>\n>2) tp proto should hold a refcnt to the chain too. This significantly\n> simplifies the logic:\n>\n>2a) Chain 0 is no longer special, it is created and refcnted by tp\n> like any other chains. All the ugliness in tcf_chain_put() can be\n> gone!\n>\n>2b) No need to handle the flushing oddly, because block still holds\n> chain 0, it can not be released, this guarantees block is the last\n> user.\n>\n>2c) The race condition with RCU callbacks is easier to handle with just\n> a rcu_barrier()! Much easier to understand, nothing to hide! Thanks\n> to the previous patch. Please see also the comments in code.\n>\n>2d) Make the code understandable by humans, much less error-prone.\n>\n>Fixes: 744a4cf63e52 (\"net: sched: fix use after free when tcf_chain_destroy is called multiple times\")\n>Fixes: 5bc1701881e3 (\"net: sched: introduce multichain support for filters\")\n>Reported-by: Jakub Kicinski \n>Cc: Jiri Pirko \n>Signed-off-by: Cong Wang \n>---\n> net/sched/cls_api.c | 38 ++++++++++++++++++++++----------------\n> 1 file changed, 22 insertions(+), 16 deletions(-)\n>\n>diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c\n>index 6c5ea84d2682..e9060dc36519 100644\n>--- a/net/sched/cls_api.c\n>+++ b/net/sched/cls_api.c\n>@@ -209,21 +209,20 @@ static void tcf_chain_flush(struct tcf_chain *chain)\n> \t\tRCU_INIT_POINTER(*chain->p_filter_chain, NULL);\n> \twhile ((tp = rtnl_dereference(chain->filter_chain)) != NULL) {\n> \t\tRCU_INIT_POINTER(chain->filter_chain, tp->next);\n>+\t\ttcf_chain_put(chain);\n> \t\ttcf_proto_destroy(tp);\n> \t}\n> }\n> \n> static void tcf_chain_destroy(struct tcf_chain *chain)\n> {\n>-\t/* May be already removed from the list by the previous call. */\n>-\tif (!list_empty(&chain->list))\n>-\t\tlist_del_init(&chain->list);\n>+\tlist_del(&chain->list);\n>+\tkfree(chain);\n>+}\n> \n>-\t/* There might still be a reference held when we got here from\n>-\t * tcf_block_put. Wait for the user to drop reference before free.\n>-\t */\n>-\tif (!chain->refcnt)\n>-\t\tkfree(chain);\n>+static void tcf_chain_hold(struct tcf_chain *chain)\n>+{\n>+\t++chain->refcnt;\n> }\n> \n> struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index,\n>@@ -233,7 +232,7 @@ struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index,\n> \n> \tlist_for_each_entry(chain, &block->chain_list, list) {\n> \t\tif (chain->index == chain_index) {\n>-\t\t\tchain->refcnt++;\n>+\t\t\ttcf_chain_hold(chain);\n> \t\t\treturn chain;\n> \t\t}\n> \t}\n>@@ -246,10 +245,7 @@ EXPORT_SYMBOL(tcf_chain_get);\n> \n> void tcf_chain_put(struct tcf_chain *chain)\n> {\n>-\t/* Destroy unused chain, with exception of chain 0, which is the\n>-\t * default one and has to be always present.\n>-\t */\n>-\tif (--chain->refcnt == 0 && !chain->filter_chain && chain->index != 0)\n>+\tif (--chain->refcnt == 0)\n\nOkay, so you take the reference for every goto_chain action and every\ntp, right? Note that for chain 0, you hold one more reference (due to\nthe creation). That is probably ok as we need chain 0 not to go away\neven if all tps and goto_chain actions are gone.\n\n\n> \t\ttcf_chain_destroy(chain);\n> }\n> EXPORT_SYMBOL(tcf_chain_put);\n>@@ -294,10 +290,18 @@ void tcf_block_put(struct tcf_block *block)\n> \tif (!block)\n> \t\treturn;\n> \n>-\tlist_for_each_entry_safe(chain, tmp, &block->chain_list, list) {\n>+\t/* Standalone actions are not allowed to jump to any chain, and\n>+\t * bound actions should be all removed after flushing. However,\n>+\t * filters are destroyed in RCU callbacks, we have to flush and wait\n>+\t * for them before releasing this refcnt, otherwise we race with RCU\n>+\t * callbacks!!!\n\nWhy the \"!!!\"? Please avoid that. Not necessary at all.\n\n\n>+\t */\n>+\tlist_for_each_entry(chain, &block->chain_list, list)\n> \t\ttcf_chain_flush(chain);\n>-\t\ttcf_chain_destroy(chain);\n>-\t}\n>+\trcu_barrier();\n\nThis actually tries to fix another bug I discovered yesterday. Good.\n\n\n>+\n>+\tlist_for_each_entry_safe(chain, tmp, &block->chain_list, list)\n>+\t\ttcf_chain_put(chain);\n\nWhich reference are you putting here? For chain 0, that is the original\nreference due to creation from block_get. But how about the other\nchains? If you do flush all in the previous list iteration, they are\nremoved there. Also note that they are removed from the list while\niterating it. \n\nI believe that you need to add tcf_chain_hold(chain) to the start of the\nprevious list iteration to ensure all existing chains will stay, then\nyou can put them here.\n\nDid you test this? I believe we need some simple test script.\n\n\n> \tkfree(block);\n> }\n> EXPORT_SYMBOL(tcf_block_put);\n>@@ -375,6 +379,7 @@ static void tcf_chain_tp_insert(struct tcf_chain *chain,\n> \t\trcu_assign_pointer(*chain->p_filter_chain, tp);\n> \tRCU_INIT_POINTER(tp->next, tcf_chain_tp_prev(chain_info));\n> \trcu_assign_pointer(*chain_info->pprev, tp);\n>+\ttcf_chain_hold(chain);\n> }\n> \n> static void tcf_chain_tp_remove(struct tcf_chain *chain,\n>@@ -386,6 +391,7 @@ static void tcf_chain_tp_remove(struct tcf_chain *chain,\n> \tif (chain->p_filter_chain && tp == chain->filter_chain)\n> \t\tRCU_INIT_POINTER(*chain->p_filter_chain, next);\n> \tRCU_INIT_POINTER(*chain_info->pprev, next);\n>+\ttcf_chain_put(chain);\n> }\n> \n> static struct tcf_proto *tcf_chain_tp_find(struct tcf_chain *chain,\n>-- \n>2.13.0\n>","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=resnulli-us.20150623.gappssmtp.com\n\theader.i=@resnulli-us.20150623.gappssmtp.com\n\theader.b=\"ai/kQYJA\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xnrGc0Zd2z9s82\n\tfor ;\n\tThu, 7 Sep 2017 16:32:24 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752854AbdIGGcV (ORCPT );\n\tThu, 7 Sep 2017 02:32:21 -0400","from mail-wm0-f65.google.com ([74.125.82.65]:35599 \"EHLO\n\tmail-wm0-f65.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751780AbdIGGcU (ORCPT\n\t); Thu, 7 Sep 2017 02:32:20 -0400","by mail-wm0-f65.google.com with SMTP id e64so684316wmi.2\n\tfor ; Wed, 06 Sep 2017 23:32:19 -0700 (PDT)","from localhost (ip-89-177-125-82.net.upcbroadband.cz.\n\t[89.177.125.82]) by smtp.gmail.com with ESMTPSA id\n\ta197sm299441wmd.26.2017.09.06.23.32.17\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tWed, 06 Sep 2017 23:32:17 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=resnulli-us.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=dOKRLf4uYMgl4t8x6GQRxNg/hej/h614lurQsge6Cns=;\n\tb=ai/kQYJAhxwmc8tdbsOeo+l4qE2urYNBGzwdp37Z2v4gz9mQizsv/OOdFMQCrnvMwE\n\tr0386xqAfnkzhUqAt8BszSVhsRiCS19vZmzSL+ANJsYSJIQA86X9eaDI1+EQQf2FsI4J\n\tdjXpJQ/xMEUuuotkyUvZLKqyxJxIY3ixuTVmND5Y3xjiJF3e5P2CbcTadyb/xDjUgbgB\n\tdnHZUwrA5/7j7cPYZad682XBqMAbRsI36yQPU5enXVo7jFY78mQo9YDysnuQNrIlNTEF\n\tclzCbMcAp2pmlkEf4L+PTCOtBYAJonOZSj1X9CdVQGnF4srdCTwg3RJCDqmiux6N8zbj\n\tV1Vg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=dOKRLf4uYMgl4t8x6GQRxNg/hej/h614lurQsge6Cns=;\n\tb=DtyFrzNzzGVuwc+dJokhyIGhTQykRoOxVhKhAzRmTBypRRvshdWF3fSsPPxnl8IDT+\n\t2wfxCpxfv4dbSsPkLQ6noFLVzqVqGY0OK8c/jCc7cKgC/ZGQDp1RP8kJWkxyUDj7kFmD\n\tfRBO8z0tj8pO4xE6koyuwaGSVCXLznYaE5ezeO44gqc0+lJvhCmMQ6BL0evrv5/qdRh9\n\ttT594WCskTAVRO3/2jloLqJj4r8T267uMzXmaFUKKN6Or5EAo4nQF31iaQEc9kvNyRLU\n\tXROBYgPIHUJVSFaSCotTUF1Rw2DBFKioziuraUnegnObxAwTDVrSDmOhM9Ncn43rgYmU\n\tMY7w==","X-Gm-Message-State":"AHPjjUjTGxLTEe9h9yGOVgfUjsxaKR1GP2XSh5CZYuovRj/24t0Gbpwe\n\tIbdYgCoR8G4Ztyev","X-Google-Smtp-Source":"ADKCNb7hhmoK8TrE33Oqc+8bfxFNT5DHqGtKvRwIuIpxpDMhxlS32kF5tl3cExjwihfTfTfAtg/reA==","X-Received":"by 10.28.38.69 with SMTP id m66mr1385089wmm.34.1504765938737;\n\tWed, 06 Sep 2017 23:32:18 -0700 (PDT)","Date":"Thu, 7 Sep 2017 08:32:16 +0200","From":"Jiri Pirko ","To":"Cong Wang ","Cc":"netdev@vger.kernel.org, jakub.kicinski@netronome.com,\n\tJiri Pirko ","Subject":"Re: [Patch net v2 2/2] net_sched: fix all the madness of tc filter\n\tchain","Message-ID":"<20170907063216.GB1967@nanopsycho>","References":"<20170907042607.24413-1-xiyou.wangcong@gmail.com>\n\t<20170907042607.24413-2-xiyou.wangcong@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170907042607.24413-2-xiyou.wangcong@gmail.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1764859,"web_url":"http://patchwork.ozlabs.org/comment/1764859/","msgid":"","list_archive_url":null,"date":"2017-09-07T17:45:49","subject":"Re: [Patch net v2 2/2] net_sched: fix all the madness of tc filter\n\tchain","submitter":{"id":211,"url":"http://patchwork.ozlabs.org/api/people/211/","name":"Cong Wang","email":"xiyou.wangcong@gmail.com"},"content":"On Wed, Sep 6, 2017 at 11:32 PM, Jiri Pirko wrote:\n> Thu, Sep 07, 2017 at 06:26:07AM CEST, xiyou.wangcong@gmail.com wrote:\n>>This patch fixes the following madness of tc filter chain:\n>\n> Could you avoid expressive words like \"madness\" and such?\n> Please be technical.\n>\n\nIf the following 2a) 2b) 2c) 2d) are not enough to show the madness,\nI don't know any other to show it. Madness is for code, not for you\nor any other person, so 100% technical.\n\n>\n>>\n>>1) tcf_chain_destroy() is called by both tcf_block_put() and\n>> tcf_chain_put(). tcf_chain_put() is correctly refcnt'ed and paired\n>> with tcf_chain_get(), but tcf_block_put() is not, it should be paired\n>> with tcf_block_get() which means we still need to decrease the refcnt.\n>> Think it in another way: if we call tcf_bock_put() immediately after\n>> tcf_block_get(), could we get effectively a nop? This causes a memory\n>> leak as reported by Jakub.\n>>\n>>2) tp proto should hold a refcnt to the chain too. This significantly\n>> simplifies the logic:\n>>\n>>2a) Chain 0 is no longer special, it is created and refcnted by tp\n>> like any other chains. All the ugliness in tcf_chain_put() can be\n>> gone!\n>>\n>>2b) No need to handle the flushing oddly, because block still holds\n>> chain 0, it can not be released, this guarantees block is the last\n>> user.\n>>\n>>2c) The race condition with RCU callbacks is easier to handle with just\n>> a rcu_barrier()! Much easier to understand, nothing to hide! Thanks\n>> to the previous patch. Please see also the comments in code.\n>>\n>>2d) Make the code understandable by humans, much less error-prone.\n>>\n>>Fixes: 744a4cf63e52 (\"net: sched: fix use after free when tcf_chain_destroy is called multiple times\")\n>>Fixes: 5bc1701881e3 (\"net: sched: introduce multichain support for filters\")\n>>Reported-by: Jakub Kicinski \n>>Cc: Jiri Pirko \n>>Signed-off-by: Cong Wang \n>>---\n>> net/sched/cls_api.c | 38 ++++++++++++++++++++++----------------\n>> 1 file changed, 22 insertions(+), 16 deletions(-)\n>>\n>>diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c\n>>index 6c5ea84d2682..e9060dc36519 100644\n>>--- a/net/sched/cls_api.c\n>>+++ b/net/sched/cls_api.c\n>>@@ -209,21 +209,20 @@ static void tcf_chain_flush(struct tcf_chain *chain)\n>> RCU_INIT_POINTER(*chain->p_filter_chain, NULL);\n>> while ((tp = rtnl_dereference(chain->filter_chain)) != NULL) {\n>> RCU_INIT_POINTER(chain->filter_chain, tp->next);\n>>+ tcf_chain_put(chain);\n>> tcf_proto_destroy(tp);\n>> }\n>> }\n>>\n>> static void tcf_chain_destroy(struct tcf_chain *chain)\n>> {\n>>- /* May be already removed from the list by the previous call. */\n>>- if (!list_empty(&chain->list))\n>>- list_del_init(&chain->list);\n>>+ list_del(&chain->list);\n>>+ kfree(chain);\n>>+}\n>>\n>>- /* There might still be a reference held when we got here from\n>>- * tcf_block_put. Wait for the user to drop reference before free.\n>>- */\n>>- if (!chain->refcnt)\n>>- kfree(chain);\n>>+static void tcf_chain_hold(struct tcf_chain *chain)\n>>+{\n>>+ ++chain->refcnt;\n>> }\n>>\n>> struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index,\n>>@@ -233,7 +232,7 @@ struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index,\n>>\n>> list_for_each_entry(chain, &block->chain_list, list) {\n>> if (chain->index == chain_index) {\n>>- chain->refcnt++;\n>>+ tcf_chain_hold(chain);\n>> return chain;\n>> }\n>> }\n>>@@ -246,10 +245,7 @@ EXPORT_SYMBOL(tcf_chain_get);\n>>\n>> void tcf_chain_put(struct tcf_chain *chain)\n>> {\n>>- /* Destroy unused chain, with exception of chain 0, which is the\n>>- * default one and has to be always present.\n>>- */\n>>- if (--chain->refcnt == 0 && !chain->filter_chain && chain->index != 0)\n>>+ if (--chain->refcnt == 0)\n>\n> Okay, so you take the reference for every goto_chain action and every\n> tp, right? Note that for chain 0, you hold one more reference (due to\n> the creation). That is probably ok as we need chain 0 not to go away\n> even if all tps and goto_chain actions are gone.\n\nYeah, this is the core of the patch.\n\n>\n>\n>> tcf_chain_destroy(chain);\n>> }\n>> EXPORT_SYMBOL(tcf_chain_put);\n>>@@ -294,10 +290,18 @@ void tcf_block_put(struct tcf_block *block)\n>> if (!block)\n>> return;\n>>\n>>- list_for_each_entry_safe(chain, tmp, &block->chain_list, list) {\n>>+ /* Standalone actions are not allowed to jump to any chain, and\n>>+ * bound actions should be all removed after flushing. However,\n>>+ * filters are destroyed in RCU callbacks, we have to flush and wait\n>>+ * for them before releasing this refcnt, otherwise we race with RCU\n>>+ * callbacks!!!\n>\n> Why the \"!!!\"? Please avoid that. Not necessary at all.\n\n\nBecause we don't have lock here, we have to pay more extra\nattention to it. Playing list without lock is like playing fire. I use \"!!!\" to\ndraw people's attention when they touch it, perhaps \"XXX\" is better?\n\n\n>\n>\n>>+ */\n>>+ list_for_each_entry(chain, &block->chain_list, list)\n>> tcf_chain_flush(chain);\n>>- tcf_chain_destroy(chain);\n>>- }\n>>+ rcu_barrier();\n>\n> This actually tries to fix another bug I discovered yesterday. Good.\n>\n\nThis on the other hand proves we should make the code clean\nand understandable asap, not just wait for net-next.\n\n\n\n>\n>>+\n>>+ list_for_each_entry_safe(chain, tmp, &block->chain_list, list)\n>>+ tcf_chain_put(chain);\n>\n> Which reference are you putting here? For chain 0, that is the original\n> reference due to creation from block_get. But how about the other\n> chains? If you do flush all in the previous list iteration, they are\n> removed there. Also note that they are removed from the list while\n> iterating it.\n\n\nYes it is for chain 0, because block holds a reference to chain 0 during\ncreation. Non-0 chains are created with refcnt==1 too but paired with\ntcf_chain_put() rather than tcf_block_put(). This is what makes chain 0\nnot special w.r.t. refcnt.\n\n\n>\n> I believe that you need to add tcf_chain_hold(chain) to the start of the\n> previous list iteration to ensure all existing chains will stay, then\n> you can put them here.\n\nWe don't need it, it is already perfectly paired with tcf_block_get().\nThink about the following:\n\n1)\ntcf_block_get(...); // create chain 0 with refcnt=1\ntcf_block_put(); // no tp, only chain 0 in chain_list\n// chain 0 is put, refcnt=0, it is gone\n\n2)\ntcf_block_get(); // create chain 0 with refcnt=1\n...\ntcf_chain_get(11); // create chain 11 with refcnt=1\n// one tp is inserted to chain 11, now refcnt==2\n// in tc_ctl_tfilter()\ntcf_chain_put(11); // paired with above get, refcnt==1\n...\ntcf_block_put(); // flush chain 11, tp removed, refcnt==0\n// put chain 0 too, paired with block get, refcnt == 0\n// both chain 0 and chain11 are gone\n\n\n\n>\n> Did you test this? I believe we need some simple test script.\n\nOf course I did. I verified memleak is gone and tested basic\nfilters and gact actions with chain 0 and chain 11, everything\nworks as expected, I also added a printk() to verify the chain\nis really gone as soon as all references are gone.\n\nI will contribute my test cases back after I figure out how.\nAlso net-next is already closed.","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"krydOwoa\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xp7D53jZWz9s83\n\tfor ;\n\tFri, 8 Sep 2017 03:46:13 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1755524AbdIGRqL (ORCPT );\n\tThu, 7 Sep 2017 13:46:11 -0400","from mail-pg0-f66.google.com ([74.125.83.66]:35471 \"EHLO\n\tmail-pg0-f66.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1755484AbdIGRqK (ORCPT\n\t); Thu, 7 Sep 2017 13:46:10 -0400","by mail-pg0-f66.google.com with SMTP id q68so192721pgq.2\n\tfor ; Thu, 07 Sep 2017 10:46:09 -0700 (PDT)","by 10.100.140.134 with HTTP; Thu, 7 Sep 2017 10:45:49 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=lvNGmqljunOj6jypvdyLoTimQPWqHPIseeQgXZ+x0JQ=;\n\tb=krydOwoaPhwgQSoP2KRqNTzcDE+ONDIckfaoG/qf151Es+eOPDmNM8uq+d9URbOA3J\n\tA7Slw9b5ifflQxNgnfGqli+FSwh3wnhMaazI3dde2Beb5isSayJJNSfrZEaPxaI+AXZd\n\tUSSULjNU5FJm+3IYZp0stjO6q5ha0SDzTuiT8403EyjRF/a49d1r5NBd+8grpSSr9ib/\n\tuJKk2hiBZfTlwA/Hzk2EO0DIx75hoZFV6xAgxtzoVyz+UIX4EYhrsrT3OnV1ND5teQ2k\n\tiaGtsX479ICTaTcSzKnaY1ex6v15o3SUev9G800uN1EwlRBHtEFlQ366FaHfJ1kxC2bA\n\t4o9g==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=lvNGmqljunOj6jypvdyLoTimQPWqHPIseeQgXZ+x0JQ=;\n\tb=PWs5MfYzx54lG2fzmDkMW23ikzv8OoR3uqGvXAHwQR8QKJYCqMsE9qkJP61t5K2tLZ\n\txJ0BPKstFyiUjvtU80BDpzAvUHnP6XilhPaUWwR2Hp2Vv+rq/axQyJP6j7eIIEOpfSnB\n\tkOfFGwuywAWW4rgwgcBfjsi9i89GXAVgjj97cZprseNSHpq1c8DC9P9P9qYrdpbK45Z0\n\tpCUecvqL9oGMqxpy7/iJiafqvVSX7eTvt0PP4Ftrh8RYhk6+i3eNjXE+e9k+1P5n7CRs\n\t7GPPEmcbxpMqpxH2r9R5utGWQoR/WvoK/kfWc9zazxd7dsYxy6WZGc/0WWOkUPA2LbUB\n\tDsnw==","X-Gm-Message-State":"AHPjjUiQt9ww5k9ihCAGjZw6tOH8tWtObaceEoRUG5MYmlqMbwds6n5Z\n\tkSAtG+ge/bnqy+wF8X8HNDwFOteVuA==","X-Google-Smtp-Source":"ADKCNb6qq+SyLRVOj6iC0pd740FkDc4hNynO157RApjgNZMOJhvl3bRvcboKmwtfRtsoiQEPoeZTmervrZFUCh8hlxA=","X-Received":"by 10.159.198.2 with SMTP id f2mr157837plo.10.1504806369626; Thu,\n\t07 Sep 2017 10:46:09 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<20170907063216.GB1967@nanopsycho>","References":"<20170907042607.24413-1-xiyou.wangcong@gmail.com>\n\t<20170907042607.24413-2-xiyou.wangcong@gmail.com>\n\t<20170907063216.GB1967@nanopsycho>","From":"Cong Wang ","Date":"Thu, 7 Sep 2017 10:45:49 -0700","Message-ID":"","Subject":"Re: [Patch net v2 2/2] net_sched: fix all the madness of tc filter\n\tchain","To":"Jiri Pirko ","Cc":"Linux Kernel Network Developers ,\n\tJakub Kicinski ,\n\tJiri Pirko ","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1764937,"web_url":"http://patchwork.ozlabs.org/comment/1764937/","msgid":"<20170907205239.GF1967@nanopsycho>","list_archive_url":null,"date":"2017-09-07T20:52:39","subject":"Re: [Patch net v2 2/2] net_sched: fix all the madness of tc filter\n\tchain","submitter":{"id":15321,"url":"http://patchwork.ozlabs.org/api/people/15321/","name":"Jiri Pirko","email":"jiri@resnulli.us"},"content":"Thu, Sep 07, 2017 at 07:45:49PM CEST, xiyou.wangcong@gmail.com wrote:\n>On Wed, Sep 6, 2017 at 11:32 PM, Jiri Pirko wrote:\n>> Thu, Sep 07, 2017 at 06:26:07AM CEST, xiyou.wangcong@gmail.com wrote:\n>>>This patch fixes the following madness of tc filter chain:\n>>\n>> Could you avoid expressive words like \"madness\" and such?\n>> Please be technical.\n>>\n>\n>If the following 2a) 2b) 2c) 2d) are not enough to show the madness,\n>I don't know any other to show it. Madness is for code, not for you\n>or any other person, so 100% technical.\n\nWe hav eto agree to disagree. You want to be expressive, apparently\nthere is no way to talk you out of that. Sigh...\n\n\n>\n>>\n>>>\n>>>1) tcf_chain_destroy() is called by both tcf_block_put() and\n>>> tcf_chain_put(). tcf_chain_put() is correctly refcnt'ed and paired\n>>> with tcf_chain_get(), but tcf_block_put() is not, it should be paired\n>>> with tcf_block_get() which means we still need to decrease the refcnt.\n>>> Think it in another way: if we call tcf_bock_put() immediately after\n>>> tcf_block_get(), could we get effectively a nop? This causes a memory\n>>> leak as reported by Jakub.\n>>>\n>>>2) tp proto should hold a refcnt to the chain too. This significantly\n>>> simplifies the logic:\n>>>\n>>>2a) Chain 0 is no longer special, it is created and refcnted by tp\n>>> like any other chains. All the ugliness in tcf_chain_put() can be\n>>> gone!\n>>>\n>>>2b) No need to handle the flushing oddly, because block still holds\n>>> chain 0, it can not be released, this guarantees block is the last\n>>> user.\n>>>\n>>>2c) The race condition with RCU callbacks is easier to handle with just\n>>> a rcu_barrier()! Much easier to understand, nothing to hide! Thanks\n>>> to the previous patch. Please see also the comments in code.\n>>>\n>>>2d) Make the code understandable by humans, much less error-prone.\n>>>\n>>>Fixes: 744a4cf63e52 (\"net: sched: fix use after free when tcf_chain_destroy is called multiple times\")\n>>>Fixes: 5bc1701881e3 (\"net: sched: introduce multichain support for filters\")\n>>>Reported-by: Jakub Kicinski \n>>>Cc: Jiri Pirko \n>>>Signed-off-by: Cong Wang \n>>>---\n>>> net/sched/cls_api.c | 38 ++++++++++++++++++++++----------------\n>>> 1 file changed, 22 insertions(+), 16 deletions(-)\n>>>\n>>>diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c\n>>>index 6c5ea84d2682..e9060dc36519 100644\n>>>--- a/net/sched/cls_api.c\n>>>+++ b/net/sched/cls_api.c\n>>>@@ -209,21 +209,20 @@ static void tcf_chain_flush(struct tcf_chain *chain)\n>>> RCU_INIT_POINTER(*chain->p_filter_chain, NULL);\n>>> while ((tp = rtnl_dereference(chain->filter_chain)) != NULL) {\n>>> RCU_INIT_POINTER(chain->filter_chain, tp->next);\n>>>+ tcf_chain_put(chain);\n>>> tcf_proto_destroy(tp);\n>>> }\n>>> }\n>>>\n>>> static void tcf_chain_destroy(struct tcf_chain *chain)\n>>> {\n>>>- /* May be already removed from the list by the previous call. */\n>>>- if (!list_empty(&chain->list))\n>>>- list_del_init(&chain->list);\n>>>+ list_del(&chain->list);\n>>>+ kfree(chain);\n>>>+}\n>>>\n>>>- /* There might still be a reference held when we got here from\n>>>- * tcf_block_put. Wait for the user to drop reference before free.\n>>>- */\n>>>- if (!chain->refcnt)\n>>>- kfree(chain);\n>>>+static void tcf_chain_hold(struct tcf_chain *chain)\n>>>+{\n>>>+ ++chain->refcnt;\n>>> }\n>>>\n>>> struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index,\n>>>@@ -233,7 +232,7 @@ struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index,\n>>>\n>>> list_for_each_entry(chain, &block->chain_list, list) {\n>>> if (chain->index == chain_index) {\n>>>- chain->refcnt++;\n>>>+ tcf_chain_hold(chain);\n>>> return chain;\n>>> }\n>>> }\n>>>@@ -246,10 +245,7 @@ EXPORT_SYMBOL(tcf_chain_get);\n>>>\n>>> void tcf_chain_put(struct tcf_chain *chain)\n>>> {\n>>>- /* Destroy unused chain, with exception of chain 0, which is the\n>>>- * default one and has to be always present.\n>>>- */\n>>>- if (--chain->refcnt == 0 && !chain->filter_chain && chain->index != 0)\n>>>+ if (--chain->refcnt == 0)\n>>\n>> Okay, so you take the reference for every goto_chain action and every\n>> tp, right? Note that for chain 0, you hold one more reference (due to\n>> the creation). That is probably ok as we need chain 0 not to go away\n>> even if all tps and goto_chain actions are gone.\n>\n>Yeah, this is the core of the patch.\n>\n>>\n>>\n>>> tcf_chain_destroy(chain);\n>>> }\n>>> EXPORT_SYMBOL(tcf_chain_put);\n>>>@@ -294,10 +290,18 @@ void tcf_block_put(struct tcf_block *block)\n>>> if (!block)\n>>> return;\n>>>\n>>>- list_for_each_entry_safe(chain, tmp, &block->chain_list, list) {\n>>>+ /* Standalone actions are not allowed to jump to any chain, and\n>>>+ * bound actions should be all removed after flushing. However,\n>>>+ * filters are destroyed in RCU callbacks, we have to flush and wait\n>>>+ * for them before releasing this refcnt, otherwise we race with RCU\n>>>+ * callbacks!!!\n>>\n>> Why the \"!!!\"? Please avoid that. Not necessary at all.\n>\n>\n>Because we don't have lock here, we have to pay more extra\n>attention to it. Playing list without lock is like playing fire. I use \"!!!\" to\n>draw people's attention when they touch it, perhaps \"XXX\" is better?\n>\n>\n>>\n>>\n>>>+ */\n>>>+ list_for_each_entry(chain, &block->chain_list, list)\n>>> tcf_chain_flush(chain);\n>>>- tcf_chain_destroy(chain);\n>>>- }\n>>>+ rcu_barrier();\n>>\n>> This actually tries to fix another bug I discovered yesterday. Good.\n>>\n>\n>This on the other hand proves we should make the code clean\n>and understandable asap, not just wait for net-next.\n>\n>\n>\n>>\n>>>+\n>>>+ list_for_each_entry_safe(chain, tmp, &block->chain_list, list)\n>>>+ tcf_chain_put(chain);\n>>\n>> Which reference are you putting here? For chain 0, that is the original\n>> reference due to creation from block_get. But how about the other\n>> chains? If you do flush all in the previous list iteration, they are\n>> removed there. Also note that they are removed from the list while\n>> iterating it.\n>\n>\n>Yes it is for chain 0, because block holds a reference to chain 0 during\n>creation. Non-0 chains are created with refcnt==1 too but paired with\n>tcf_chain_put() rather than tcf_block_put(). This is what makes chain 0\n>not special w.r.t. refcnt.\n\nSo you need to tcf_chain_put only chain 0 here, right? The rest of the\nchains get destroyed by the previous list_for_each_entry iteration when\nflush happens and actions destroy happens what decrements recnt to 0\nthere.\n\nWhat do I miss, who would still hold reference for non-0 chains when all\ntps and all goto_chain actions are gone?\n\n\n>\n>\n>>\n>> I believe that you need to add tcf_chain_hold(chain) to the start of the\n>> previous list iteration to ensure all existing chains will stay, then\n>> you can put them here.\n>\n>We don't need it, it is already perfectly paired with tcf_block_get().\n>Think about the following:\n>\n>1)\n>tcf_block_get(...); // create chain 0 with refcnt=1\n>tcf_block_put(); // no tp, only chain 0 in chain_list\n>// chain 0 is put, refcnt=0, it is gone\n>\n>2)\n>tcf_block_get(); // create chain 0 with refcnt=1\n>...\n>tcf_chain_get(11); // create chain 11 with refcnt=1\n>// one tp is inserted to chain 11, now refcnt==2\n>// in tc_ctl_tfilter()\n>tcf_chain_put(11); // paired with above get, refcnt==1\n>...\n>tcf_block_put(); // flush chain 11, tp removed, refcnt==0\n>// put chain 0 too, paired with block get, refcnt == 0\n>// both chain 0 and chain11 are gone\n>\n>\n>\n>>\n>> Did you test this? I believe we need some simple test script.\n>\n>Of course I did. I verified memleak is gone and tested basic\n>filters and gact actions with chain 0 and chain 11, everything\n>works as expected, I also added a printk() to verify the chain\n>is really gone as soon as all references are gone.\n>\n>I will contribute my test cases back after I figure out how.\n>Also net-next is already closed.","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=resnulli-us.20150623.gappssmtp.com\n\theader.i=@resnulli-us.20150623.gappssmtp.com\n\theader.b=\"YO9HCJ2s\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xpCMM6bBBz9sBW\n\tfor ;\n\tFri, 8 Sep 2017 06:52:47 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1755930AbdIGUwp (ORCPT );\n\tThu, 7 Sep 2017 16:52:45 -0400","from mail-wr0-f194.google.com ([209.85.128.194]:34831 \"EHLO\n\tmail-wr0-f194.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752375AbdIGUwn (ORCPT\n\t); Thu, 7 Sep 2017 16:52:43 -0400","by mail-wr0-f194.google.com with SMTP id n64so372853wrb.2\n\tfor ; Thu, 07 Sep 2017 13:52:43 -0700 (PDT)","from localhost (ip-89-177-125-82.net.upcbroadband.cz.\n\t[89.177.125.82]) by smtp.gmail.com with ESMTPSA id\n\tc46sm293636wrg.92.2017.09.07.13.52.40\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tThu, 07 Sep 2017 13:52:40 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=resnulli-us.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=hqlC6n2NlMngAeMjSU7E77Odqr8hYwqx2SA+tePwOZk=;\n\tb=YO9HCJ2sh8ER3hI2lShKyalXcJ5uReLgNC8JtNTrxw5zVR8R51KIOWdqtpKNDst8Pv\n\tBa5DC4e9N0Nm0eFN/f1WGAlpm+iUv93NeOVYg1OdLGJYFm+RRHBp5BvZSya7YyL29EDz\n\tKgo230oVBlJNrkEui219ejlJxHbDmCKL0LmjLOmL8p4yFHc+9+xjtVNH+hWdYNuz6U53\n\tMHDJTtg8p7s+v/V8O8DdbWh+Fq7hZlutJ26AO+ZdeE7V5eDSrLUBBG6R0bZbJz5Bq1FZ\n\t+xJt6Sz39l3C38EQ9RLmtnx+8zNP7pHodx31Cxhl4AHPHm2eqCk//l0tMkd9Px1AnCHh\n\taUxA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=hqlC6n2NlMngAeMjSU7E77Odqr8hYwqx2SA+tePwOZk=;\n\tb=fD5Kapt+Z78cyTsHttdrzM9QSyO1yvAGvsFm18sZdBwJWEI+zXTEuIbUJpOUqxtNml\n\tMxdg6wIJ5HnZwkEeGL5bM7hSdRaOctkPuATQl77OHY0Mvd4xfClzRpmPbrpmqKo2ShVF\n\tn8rU5Xlj6JeWZQcu7LNpRimH8mATH8roamDByFVMC2z2Ws3p9Zosny1iMEc9APbt6yRp\n\tUS+UxnKRWOP9UE/rR6sm2+Cb8bMQVIysm/I4paBTNbVEwgK9xtbePxdTTiXLC58l4f2s\n\tn4c9B/QhcLnGHN6/YKhgUma3ylN8h9Dn14hcMGIIsQsjCQYfdIRVV6v36ExxdFaexFnD\n\txrmw==","X-Gm-Message-State":"AHPjjUgUKhHncp82fcuBXw06LPd5hUyX7wGrBJsiHpa0Kfx1IHmVzp8Z\n\tOcnPNo2FSwsFlRb/","X-Google-Smtp-Source":"ADKCNb5NnhzGGU3DS3FJadqw6JmbBqCY/J/bC0kKFsf/3DPJ/hkJgBqCKIXK+Vewhpep3YG3NjD0IQ==","X-Received":"by 10.223.195.87 with SMTP id e23mr434406wrg.77.1504817562362;\n\tThu, 07 Sep 2017 13:52:42 -0700 (PDT)","Date":"Thu, 7 Sep 2017 22:52:39 +0200","From":"Jiri Pirko ","To":"Cong Wang ","Cc":"Linux Kernel Network Developers ,\n\tJakub Kicinski ,\n\tJiri Pirko ","Subject":"Re: [Patch net v2 2/2] net_sched: fix all the madness of tc filter\n\tchain","Message-ID":"<20170907205239.GF1967@nanopsycho>","References":"<20170907042607.24413-1-xiyou.wangcong@gmail.com>\n\t<20170907042607.24413-2-xiyou.wangcong@gmail.com>\n\t<20170907063216.GB1967@nanopsycho>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"Mutt/1.8.3 (2017-05-23)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1765502,"web_url":"http://patchwork.ozlabs.org/comment/1765502/","msgid":"","list_archive_url":null,"date":"2017-09-08T16:37:55","subject":"Re: [Patch net v2 2/2] net_sched: fix all the madness of tc filter\n\tchain","submitter":{"id":211,"url":"http://patchwork.ozlabs.org/api/people/211/","name":"Cong Wang","email":"xiyou.wangcong@gmail.com"},"content":"On Thu, Sep 7, 2017 at 1:52 PM, Jiri Pirko wrote:\n> Thu, Sep 07, 2017 at 07:45:49PM CEST, xiyou.wangcong@gmail.com wrote:\n>>Yes it is for chain 0, because block holds a reference to chain 0 during\n>>creation. Non-0 chains are created with refcnt==1 too but paired with\n>>tcf_chain_put() rather than tcf_block_put(). This is what makes chain 0\n>>not special w.r.t. refcnt.\n>\n> So you need to tcf_chain_put only chain 0 here, right? The rest of the\n> chains get destroyed by the previous list_for_each_entry iteration when\n> flush happens and actions destroy happens what decrements recnt to 0\n> there.\n\n\nThis is correct. And it should be only chain 0 after flush.\n\n>\n> What do I miss, who would still hold reference for non-0 chains when all\n> tps and all goto_chain actions are gone?\n\nNo one. This is totally correct and is exactly what this patch intends to do.\n\nLook, this is why we never need an object with refcnt==0 to exist. ;)","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"HHr/Osp/\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xpjgK07lcz9s76\n\tfor ;\n\tSat, 9 Sep 2017 02:38:21 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1756438AbdIHQiS (ORCPT );\n\tFri, 8 Sep 2017 12:38:18 -0400","from mail-pg0-f42.google.com ([74.125.83.42]:35409 \"EHLO\n\tmail-pg0-f42.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1756435AbdIHQiR (ORCPT\n\t); Fri, 8 Sep 2017 12:38:17 -0400","by mail-pg0-f42.google.com with SMTP id 188so5681074pgb.2\n\tfor ; Fri, 08 Sep 2017 09:38:16 -0700 (PDT)","by 10.100.140.134 with HTTP; Fri, 8 Sep 2017 09:37:55 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=UiAaBLDIO0vWNAJENbkk07wQKEg6EohG57Qd7hF9PCA=;\n\tb=HHr/Osp/0ffSLp9lLziq9/TixkBx8iBznxMuzvdpmZoZI4qsAdc4SdGGj84Uj0RxLL\n\t75MpmqLOrDIdkUqWXTXaw0eSwjGbGU/uzjth04cjzCg8u3x07ouPRe4P3SsAdtUpboH2\n\tI9MQliyWS4osgSGGqiuquUrtHPm9cpcaL1KbA5gBkbL85zcKgnk/5Srutm+DL4PKt809\n\tgXQaOeW192TeCfXRkYXuz2vO1fg6G5ZgC7y0oBV/OozlORuzBgP017r1jnYIIA83sc6Y\n\t3ZBpjNv70MW6/K4t354jBdGkH7cnU3IAZva97hafK/ta1M/MfUKlh54t8YY2umzZyX6K\n\tOIPA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=UiAaBLDIO0vWNAJENbkk07wQKEg6EohG57Qd7hF9PCA=;\n\tb=X1wIcDQGzdNniFG0bFl5vhLmNW4KIrXwnx4KP2b57vtN7HT3EDUOiyNPdt+nFzf9An\n\tSS0pZi2APWaVTnFI1f8wgQedhNRPmrgqnb0IqIlXu3A3qVtYnTr8eJ1YjPAGhvJiAyZS\n\t3cX0gFGpo+LbXyrr89AokZ7KMMa05AGJMEUY/aGYzsnSWHTGlmgcwGt+xBhCMyQ3+vd6\n\tl/c123yG2Q5DmC32hHpYkxhYxE8T0+eQbVGbBE4Gl2DWlRM57rlNmaZUOCB9CqyDrz4g\n\t7SLjnfoUAp6sbaAiReCKQbIYm4dF2+vJ13DYc84THW+qZSRgKUm/b4zZbydmCwGuMlDu\n\tUUzg==","X-Gm-Message-State":"AHPjjUj0+yjb7deWNoTsK14DyCMcm1sechccKHIIkSOvjphQ+RXiEn9L\n\tlnJFfub36zG9D3eMJqw3EhSirc0pXPkV","X-Google-Smtp-Source":"ADKCNb6VuSaqPNBwoU4tIm/XgrgmiPaEHYUeLeWTzREEZCB/YJDIyuJFlQctdR/dNr7i2+RkwCEo6cFIuF2hmHF8iXQ=","X-Received":"by 10.84.216.69 with SMTP id f5mr4259877plj.118.1504888696423;\n\tFri, 08 Sep 2017 09:38:16 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<20170907205239.GF1967@nanopsycho>","References":"<20170907042607.24413-1-xiyou.wangcong@gmail.com>\n\t<20170907042607.24413-2-xiyou.wangcong@gmail.com>\n\t<20170907063216.GB1967@nanopsycho>\n\t\n\t<20170907205239.GF1967@nanopsycho>","From":"Cong Wang ","Date":"Fri, 8 Sep 2017 09:37:55 -0700","Message-ID":"","Subject":"Re: [Patch net v2 2/2] net_sched: fix all the madness of tc filter\n\tchain","To":"Jiri Pirko ","Cc":"Linux Kernel Network Developers ,\n\tJakub Kicinski ,\n\tJiri Pirko ","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1765948,"web_url":"http://patchwork.ozlabs.org/comment/1765948/","msgid":"<20170910143300.GB1860@nanopsycho>","list_archive_url":null,"date":"2017-09-10T14:33:00","subject":"Re: [Patch net v2 2/2] net_sched: fix all the madness of tc filter\n\tchain","submitter":{"id":15321,"url":"http://patchwork.ozlabs.org/api/people/15321/","name":"Jiri Pirko","email":"jiri@resnulli.us"},"content":"Fri, Sep 08, 2017 at 06:37:55PM CEST, xiyou.wangcong@gmail.com wrote:\n>On Thu, Sep 7, 2017 at 1:52 PM, Jiri Pirko wrote:\n>> Thu, Sep 07, 2017 at 07:45:49PM CEST, xiyou.wangcong@gmail.com wrote:\n>>>Yes it is for chain 0, because block holds a reference to chain 0 during\n>>>creation. Non-0 chains are created with refcnt==1 too but paired with\n>>>tcf_chain_put() rather than tcf_block_put(). This is what makes chain 0\n>>>not special w.r.t. refcnt.\n>>\n>> So you need to tcf_chain_put only chain 0 here, right? The rest of the\n>> chains get destroyed by the previous list_for_each_entry iteration when\n>> flush happens and actions destroy happens what decrements recnt to 0\n>> there.\n>\n>\n>This is correct. And it should be only chain 0 after flush.\n>\n>>\n>> What do I miss, who would still hold reference for non-0 chains when all\n>> tps and all goto_chain actions are gone?\n>\n>No one. This is totally correct and is exactly what this patch intends to do.\n>\n>Look, this is why we never need an object with refcnt==0 to exist. ;)\n\nSo, I understand that correctly, good. But this is a problem.\n\nWhen you do:\n list_for_each_entry(chain, &block->chain_list, list)\n tcf_chain_flush(chain);\n\nThe reference may get dropped for chains to 0 (for those that does not\nhave a goto_chain action holding a ref), and therefore they get freed\nwithin the loop. That is problematic when you do the traversing of the\nlist. You may use list_for_each_entry_safe, but there is another issue:\n\nAs a part of tcf_chain_flush destruction, act goto_chain destruction may\nget scheduled by call_rcu. That may be the last reference held for the\nchain. So you race between this loop and rcu callback.\n\nConsider following example:\n\nchain0 - has only one rule with goto_chain 22 action\nchain22 - no rule (refcnt 1 because of the action mentioned above)\n\n CPU0 CPU1\n\n tcf_chain_flush(0)\n -> call_rcu(free_tcf)\n free_tcf\n ->tcf_chain_put(22)\n ->tcf_chain_destroy(22)\n ->kfree(22)\t\n tcf_chain_flush(22)...use-after-free\n\n\nSo what I suggest in order to prevent this is to change your code to\nsomething like:\n\n\t/* To avoid race between getting reference in the next loop and\n\t * rcu callbacks from deleleted actions freeing the chain.\n\t */\n\trcu_barrier();\n\n\tlist_for_each_entry(chain, &block->chain_list, list)\n\t\tif (chain->index) /* we already hold ref to chain 0 */\n\t\t\ttcf_chain_hold(chain);\n\n\tlist_for_each_entry(chain, &block->chain_list, list)\n\t\ttcf_chain_flush(chain);\n\t\n\t/* Wait for rcu callbacks from deleleted actions that were\n\t * sheduled as a result of tcf_chain_flush in the previous loop.\n\t * This is not absolutelly necessary, as the chain may live after\n\t * the tcf_chain_put is called in the next iteration and would\n\t * get freed on tcf_chain_put call from rcu callback later on.\n\t */\n\trcu_barrier();\n\n\t/* Now we are sure that we are the only one holding a reference\n\t * to all chains, drop it and let them go.\n\t */\n\tlist_for_each_entry_safe(chain, tmp, &block->chain_list, list)\n\t\ttcf_chain_put(chain);\n\tkfree(block);\n\nDoes this make sense?\n\nThanks!\n\nJiri","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=resnulli-us.20150623.gappssmtp.com\n\theader.i=@resnulli-us.20150623.gappssmtp.com\n\theader.b=\"CgvrYGzH\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xqtnx6PtXz9s71\n\tfor ;\n\tMon, 11 Sep 2017 00:33:09 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751822AbdIJOdH (ORCPT );\n\tSun, 10 Sep 2017 10:33:07 -0400","from mail-wr0-f176.google.com ([209.85.128.176]:35181 \"EHLO\n\tmail-wr0-f176.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751792AbdIJOdE (ORCPT\n\t); Sun, 10 Sep 2017 10:33:04 -0400","by mail-wr0-f176.google.com with SMTP id m18so10456501wrm.2\n\tfor ; Sun, 10 Sep 2017 07:33:03 -0700 (PDT)","from localhost (jirka.pirko.cz. [84.16.102.26])\n\tby smtp.gmail.com with ESMTPSA id\n\to21sm4291170wrf.31.2017.09.10.07.33.01\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tSun, 10 Sep 2017 07:33:01 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=resnulli-us.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=ad4vYGhNz/G7S3lrdDfvQQVziMtU4vVytB/r/LBn/Uc=;\n\tb=CgvrYGzH2ai5ePGnbBW/TTmfpDXLRpTjVljPGMlmPaI+1c72lc4wRlW5G2x1YK1i5S\n\tR2S3wXGnqgnMvju5PMfi7/N16nRqr4bJZxbYxVLQdjwimFQV3jBiDoxI0ynem4sZMXhK\n\tvQGZNd3PkzIpPHK9WkerWeyx+vUBrjxAZ/z31qk8G+UiR3gTnyqs0ITAi5ArNV5BG1ig\n\tNc2CmxKBTny5siTQo96wO7q4SzNqu4uiHsPFA6CB1ia4T4jioMTc3LSI3jInLUfHyCaC\n\tzUek9UNVbn2EyoT4vfvDHX3oXWUhcbDwH16DL7M9JcChQcRYl34iNQENw6QzHwtV7hDY\n\tIOXw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=ad4vYGhNz/G7S3lrdDfvQQVziMtU4vVytB/r/LBn/Uc=;\n\tb=MXJ3YCYJzFnEbW8INXlCxWIT7knK1/6jrPGSMbvNUWpxYzDeOeF+kp6F2W24ieVFyR\n\tyrS/ZY8bdFn2ciCeTSl9h2ng0cpqike94Qpp/Z1pbNo6cEtX9XaOuMAXsjFRToXb1TDt\n\tlArN1mDggWAtLPmtILhkTyL+6x8iUPdGK9bk5NYY8qaIdR/93/9ROqhtMR6pmRQwBjRd\n\tJcMozwruY/oFkaMrKQmpKq2SssY6n9e7eFjOGmbPBwUj4B9cPesx6kKwi3TLifzwFMXJ\n\tzI+D/xVVBJXhqqYU4CRnhHbE8mvFr3hsM69mpcXixtATpIeBhKzP8Nz7FQnVsNUQiju6\n\tdBfg==","X-Gm-Message-State":"AHPjjUjfdQfqHW1BACm/47Z6pL/dAWDPZKgxmZbv2BK/skd7JM2Wq/Zy\n\twruRvrj1wNd2R55/wNw=","X-Google-Smtp-Source":"ADKCNb5a2WFhbexU7AxwH3yJm43CoLSICDx5Q4Ni6XrmYjYAvOfwxTZ4azITMYrVMX3rey9tRDR6+Q==","X-Received":"by 10.223.130.116 with SMTP id 107mr6817600wrb.240.1505053982690;\n\tSun, 10 Sep 2017 07:33:02 -0700 (PDT)","Date":"Sun, 10 Sep 2017 16:33:00 +0200","From":"Jiri Pirko ","To":"Cong Wang ","Cc":"Linux Kernel Network Developers ,\n\tJakub Kicinski ,\n\tJiri Pirko ","Subject":"Re: [Patch net v2 2/2] net_sched: fix all the madness of tc filter\n\tchain","Message-ID":"<20170910143300.GB1860@nanopsycho>","References":"<20170907042607.24413-1-xiyou.wangcong@gmail.com>\n\t<20170907042607.24413-2-xiyou.wangcong@gmail.com>\n\t<20170907063216.GB1967@nanopsycho>\n\t\n\t<20170907205239.GF1967@nanopsycho>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"Mutt/1.8.3 (2017-05-23)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1766560,"web_url":"http://patchwork.ozlabs.org/comment/1766560/","msgid":"","list_archive_url":null,"date":"2017-09-11T21:14:54","subject":"Re: [Patch net v2 2/2] net_sched: fix all the madness of tc filter\n\tchain","submitter":{"id":211,"url":"http://patchwork.ozlabs.org/api/people/211/","name":"Cong Wang","email":"xiyou.wangcong@gmail.com"},"content":"On Sun, Sep 10, 2017 at 7:33 AM, Jiri Pirko wrote:\n> Fri, Sep 08, 2017 at 06:37:55PM CEST, xiyou.wangcong@gmail.com wrote:\n>>On Thu, Sep 7, 2017 at 1:52 PM, Jiri Pirko wrote:\n>>> Thu, Sep 07, 2017 at 07:45:49PM CEST, xiyou.wangcong@gmail.com wrote:\n>>>>Yes it is for chain 0, because block holds a reference to chain 0 during\n>>>>creation. Non-0 chains are created with refcnt==1 too but paired with\n>>>>tcf_chain_put() rather than tcf_block_put(). This is what makes chain 0\n>>>>not special w.r.t. refcnt.\n>>>\n>>> So you need to tcf_chain_put only chain 0 here, right? The rest of the\n>>> chains get destroyed by the previous list_for_each_entry iteration when\n>>> flush happens and actions destroy happens what decrements recnt to 0\n>>> there.\n>>\n>>\n>>This is correct. And it should be only chain 0 after flush.\n>>\n>>>\n>>> What do I miss, who would still hold reference for non-0 chains when all\n>>> tps and all goto_chain actions are gone?\n>>\n>>No one. This is totally correct and is exactly what this patch intends to do.\n>>\n>>Look, this is why we never need an object with refcnt==0 to exist. ;)\n>\n> So, I understand that correctly, good. But this is a problem.\n>\n> When you do:\n> list_for_each_entry(chain, &block->chain_list, list)\n> tcf_chain_flush(chain);\n>\n> The reference may get dropped for chains to 0 (for those that does not\n> have a goto_chain action holding a ref), and therefore they get freed\n> within the loop. That is problematic when you do the traversing of the\n> list. You may use list_for_each_entry_safe, but there is another issue:\n>\n> As a part of tcf_chain_flush destruction, act goto_chain destruction may\n> get scheduled by call_rcu. That may be the last reference held for the\n> chain. So you race between this loop and rcu callback.\n>\n> Consider following example:\n>\n> chain0 - has only one rule with goto_chain 22 action\n> chain22 - no rule (refcnt 1 because of the action mentioned above)\n>\n> CPU0 CPU1\n>\n> tcf_chain_flush(0)\n> -> call_rcu(free_tcf)\n> free_tcf\n> ->tcf_chain_put(22)\n> ->tcf_chain_destroy(22)\n> ->kfree(22)\n\n\nLooks like all due to the lack of locking on block->chain_list.\nI thought the rcu_barrier() could properly handle this,\nbut seems still not, probably I need to move it in the loop,\nI am still not 100% sure if it is totally safe with\nlist_for_each_safe():\n\n\n- list_for_each_entry(chain, &block->chain_list, list)\n+ list_for_each_entry_safe(chain, tmp, &block->chain_list, list) {\n tcf_chain_flush(chain);\n- rcu_barrier();\n+ rcu_barrier(); // are we safe now???\n+ }\n\n\n> tcf_chain_flush(22)...use-after-free\n>\n\nSame race could happen with your code too, right?\nchain 22 still has refcnt==1 so chain_put() will destroy\nit in flush() too. So this is not a regression.\n\nI know you have list_for_each_safe() but you lack of\nrcu_barrier(). This is why I said the lack of locking is\nthe cause, not your code or my code.\n\n\n>\n> So what I suggest in order to prevent this is to change your code to\n> something like:\n>\n> /* To avoid race between getting reference in the next loop and\n> * rcu callbacks from deleleted actions freeing the chain.\n> */\n> rcu_barrier();\n>\n> list_for_each_entry(chain, &block->chain_list, list)\n> if (chain->index) /* we already hold ref to chain 0 */\n> tcf_chain_hold(chain);\n>\n> list_for_each_entry(chain, &block->chain_list, list)\n> tcf_chain_flush(chain);\n>\n> /* Wait for rcu callbacks from deleleted actions that were\n> * sheduled as a result of tcf_chain_flush in the previous loop.\n> * This is not absolutelly necessary, as the chain may live after\n> * the tcf_chain_put is called in the next iteration and would\n> * get freed on tcf_chain_put call from rcu callback later on.\n> */\n> rcu_barrier();\n>\n> /* Now we are sure that we are the only one holding a reference\n> * to all chains, drop it and let them go.\n> */\n> list_for_each_entry_safe(chain, tmp, &block->chain_list, list)\n> tcf_chain_put(chain);\n> kfree(block);\n>\n> Does this make sense?\n\nYeah, but again this is all due to lack of locking. Do we really\nhave to to be such complex or just use either a) proper sync\nwith rcu_barrier() or b) proper locking on chain_list (with RCU\nof course)?\n\nThanks.","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"rAN+vsSa\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xrggX20hyz9s8J\n\tfor ;\n\tTue, 12 Sep 2017 07:15:20 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751491AbdIKVPR (ORCPT );\n\tMon, 11 Sep 2017 17:15:17 -0400","from mail-pf0-f181.google.com ([209.85.192.181]:36746 \"EHLO\n\tmail-pf0-f181.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751483AbdIKVPP (ORCPT\n\t); Mon, 11 Sep 2017 17:15:15 -0400","by mail-pf0-f181.google.com with SMTP id e199so15707330pfh.3\n\tfor ; Mon, 11 Sep 2017 14:15:15 -0700 (PDT)","by 10.100.160.131 with HTTP; Mon, 11 Sep 2017 14:14:54 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=0RBismIvD6i7wYQq7xgffQwCFCfHX1+MAhRdMxck6ow=;\n\tb=rAN+vsSaC1z1tagf+3gNSt2wKGev3qRVP8x39nqhJsMWNrmR70YeJNHvDRIhwZV7lc\n\t/9tOZ/x+QpkGe5tAGQdYKaX5/aQPiZ/hTWFh+SlXKQnxsVyaND75YlD+qHmsvdH5w3sK\n\tXs/FSyf2eT21BRWm58w5i67LcBgGrvdwuP/6zBu8QF2KIamn8UwoxX760Sx5QztudSRr\n\tHg8XMTdgdRP1n+3Yr+d8S3FtDafvGTQ/GeNAq1RZTqjrWssq3iVSDByAtXhUuC0w4S76\n\tj1+cDEYmkYL5ycBvFjWKHXFCgbUGZ/N0x7A0emRoZYkRA+VvaHgl5UHLK7XQyPc6Crm0\n\tT9mg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=0RBismIvD6i7wYQq7xgffQwCFCfHX1+MAhRdMxck6ow=;\n\tb=YC0osmhxHLdStFH4/fzg8HxNjAMV94kWAiCNSuEjbvz70ddrY1+UIyZbzAWodnFihD\n\tnBU3L2li/ewqHm6EguV294XwdXUjhheMSblccpIQYxyoZBEMYbpjIbUBN3L9P4HsnJJQ\n\ta7hKxUdPiUjwcqxWGKqleLN34DOAxPyCp6U2qhQKGBAe1q7MU8cDa3rDtCidAXRyPqus\n\trBA9CsCJwsNePArL7lm/XkFUfB22yE9oSTE1G4pyZ5QK7Ohl1BroMP1XBtrIY5/uwP2Z\n\t+10vxzmr11a9wXMyrXzpvD21ZVAqy48NPjET5Jx6c1GOyQuRmIjO2o0t7lUgo8iOglO0\n\t8XTA==","X-Gm-Message-State":"AHPjjUjQve9O7R6AsL9wqqyoz9UpvYD+RzYoXvGH/vzjVZFbFXt85Hi3\n\tq0fuuo0aMUQ3lfXOSTlyM4EreYIVLQ==","X-Google-Smtp-Source":"ADKCNb5pK6MUue6PktJxffEgkLm5GaSJkD01+AuTs606HSDvf4xPeUyKNBWTgXhM1P5Bt1PZ0z6Oxlk2s1HAYZApYxY=","X-Received":"by 10.84.129.193 with SMTP id b59mr15081392plb.33.1505164514659; \n\tMon, 11 Sep 2017 14:15:14 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<20170910143300.GB1860@nanopsycho>","References":"<20170907042607.24413-1-xiyou.wangcong@gmail.com>\n\t<20170907042607.24413-2-xiyou.wangcong@gmail.com>\n\t<20170907063216.GB1967@nanopsycho>\n\t\n\t<20170907205239.GF1967@nanopsycho>\n\t\n\t<20170910143300.GB1860@nanopsycho>","From":"Cong Wang ","Date":"Mon, 11 Sep 2017 14:14:54 -0700","Message-ID":"","Subject":"Re: [Patch net v2 2/2] net_sched: fix all the madness of tc filter\n\tchain","To":"Jiri Pirko ","Cc":"Linux Kernel Network Developers ,\n\tJakub Kicinski ,\n\tJiri Pirko ","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1766594,"web_url":"http://patchwork.ozlabs.org/comment/1766594/","msgid":"","list_archive_url":null,"date":"2017-09-11T21:58:16","subject":"Re: [Patch net v2 2/2] net_sched: fix all the madness of tc filter\n\tchain","submitter":{"id":211,"url":"http://patchwork.ozlabs.org/api/people/211/","name":"Cong Wang","email":"xiyou.wangcong@gmail.com"},"content":"On Mon, Sep 11, 2017 at 2:14 PM, Cong Wang wrote:\n>\n> Looks like all due to the lack of locking on block->chain_list.\n> I thought the rcu_barrier() could properly handle this,\n> but seems still not, probably I need to move it in the loop,\n> I am still not 100% sure if it is totally safe with\n> list_for_each_safe():\n>\n>\n> - list_for_each_entry(chain, &block->chain_list, list)\n> + list_for_each_entry_safe(chain, tmp, &block->chain_list, list) {\n> tcf_chain_flush(chain);\n> - rcu_barrier();\n> + rcu_barrier(); // are we safe now???\n> + }\n>\n\nAnswer myself: No, this is not safe either, because we may\nlist_del() the next node, and apparently _safe() can't guarantee\nthat...\n\nSo either we have to use locking or use the trick you suggested.","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"GMUcTXMy\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xrhdb1Gm5z9s7F\n\tfor ;\n\tTue, 12 Sep 2017 07:58:43 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751023AbdIKV6i (ORCPT );\n\tMon, 11 Sep 2017 17:58:38 -0400","from mail-pg0-f52.google.com ([74.125.83.52]:37019 \"EHLO\n\tmail-pg0-f52.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750911AbdIKV6h (ORCPT\n\t); Mon, 11 Sep 2017 17:58:37 -0400","by mail-pg0-f52.google.com with SMTP id d8so17707404pgt.4\n\tfor ; Mon, 11 Sep 2017 14:58:37 -0700 (PDT)","by 10.100.160.131 with HTTP; Mon, 11 Sep 2017 14:58:16 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=wOO0J0PO2iwtmTGSNR7JcJc3YKeqrBwi34Nf+jRrOtk=;\n\tb=GMUcTXMywRs1r2C7D9THnTiruNsTfq8NeU2teByTbKL203yD1HfPg/8IuKwGHr6yu/\n\tIwgRM/FgKbhG5Eh/QqrlTMYOu3SloitCPbGnUG3JLMvHqj6/0FxmcD8vPArxMbHuUNBE\n\thVovAwbao51jf70sYAh15G5EdhG+SenHt8uG+MobRC+5cdiNqElY3iRm0PG5EYBCC5l1\n\tI/Z39MFTdHe+By+ujjc20Z+0V6VBy3os26QOfYw3PO7Y+AdjaZ0rEPXlqXaVv9KZL6i9\n\taHpSUs0qEWsvTitScb3clamJMiQk9e0byDtKUjbBCCWYg6jgWQH/PVPuJX2hml02KXQv\n\tkLMw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=wOO0J0PO2iwtmTGSNR7JcJc3YKeqrBwi34Nf+jRrOtk=;\n\tb=d2Rzv83GqJTUADD5pfdH8D634vfkJTVVJTzt803UhuwzVW7qLAhNgm8qlaImQO3WpR\n\t71Wz+YS7+dZ5MX8Z/D+u6k07E1qG9tqIZbbZTnTcwfIu28pgePKKOcS1FlnkXeu29JpD\n\t8Ge9Cra4tJaGXIeLEbP7mV4tNYqgu/63oInf9Bg23d7szHIbZfqH3UMKBTNjLaK01oG+\n\t7saDUjQX3Z3dj031+cPJ4z4vN40W2ZF5rrZ9yCVEHBVHTXmbUBnOF2obGiEhDRaQUUyO\n\tYEoEKKAxsMOEapZL5Kbc/Ec6MEvJ9xpQq9w09uHw4cNX2B72OKO5TZc6JlY37AsOQK8x\n\tRjow==","X-Gm-Message-State":"AHPjjUitAqPVX/Ma/Quuh1fLguiVFfQQAH6Q0PyVSJVqBiflRtowVfYp\n\tv63fGFXiR4wi8JjHqQ/aQ+Tmxd/2Pg==","X-Google-Smtp-Source":"ADKCNb5WNzhO/Q97OLaonyr8skQn9BdbtJlOVAitKJhDBkiUq0Y/dZvA/VQTWkAMPCJGLtdHaIv3MEMtLs0d7Er/q6o=","X-Received":"by 10.84.132.98 with SMTP id 89mr15229090ple.19.1505167116998;\n\tMon, 11 Sep 2017 14:58:36 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"","References":"<20170907042607.24413-1-xiyou.wangcong@gmail.com>\n\t<20170907042607.24413-2-xiyou.wangcong@gmail.com>\n\t<20170907063216.GB1967@nanopsycho>\n\t\n\t<20170907205239.GF1967@nanopsycho>\n\t\n\t<20170910143300.GB1860@nanopsycho>\n\t","From":"Cong Wang ","Date":"Mon, 11 Sep 2017 14:58:16 -0700","Message-ID":"","Subject":"Re: [Patch net v2 2/2] net_sched: fix all the madness of tc filter\n\tchain","To":"Jiri Pirko ","Cc":"Linux Kernel Network Developers ,\n\tJakub Kicinski ,\n\tJiri Pirko ","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}}]