Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/1.2/patches/831010/?format=api
{ "id": 831010, "url": "http://patchwork.ozlabs.org/api/1.2/patches/831010/?format=api", "web_url": "http://patchwork.ozlabs.org/project/netdev/patch/20171027012443.3306-2-xiyou.wangcong@gmail.com/", "project": { "id": 7, "url": "http://patchwork.ozlabs.org/api/1.2/projects/7/?format=api", "name": "Linux network development", "link_name": "netdev", "list_id": "netdev.vger.kernel.org", "list_email": "netdev@vger.kernel.org", "web_url": null, "scm_url": null, "webscm_url": null, "list_archive_url": "", "list_archive_url_format": "", "commit_url_format": "" }, "msgid": "<20171027012443.3306-2-xiyou.wangcong@gmail.com>", "list_archive_url": null, "date": "2017-10-27T01:24:28", "name": "[net,01/16] net_sched: introduce a workqueue for RCU callbacks of tc filter", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": true, "hash": "c5324c91702dcd7fa0428409cac5fb4b446e050c", "submitter": { "id": 211, "url": "http://patchwork.ozlabs.org/api/1.2/people/211/?format=api", "name": "Cong Wang", "email": "xiyou.wangcong@gmail.com" }, "delegate": { "id": 34, "url": "http://patchwork.ozlabs.org/api/1.2/users/34/?format=api", "username": "davem", "first_name": "David", "last_name": "Miller", "email": "davem@davemloft.net" }, "mbox": "http://patchwork.ozlabs.org/project/netdev/patch/20171027012443.3306-2-xiyou.wangcong@gmail.com/mbox/", "series": [ { "id": 10472, "url": "http://patchwork.ozlabs.org/api/1.2/series/10472/?format=api", "web_url": "http://patchwork.ozlabs.org/project/netdev/list/?series=10472", "date": "2017-10-27T01:24:27", "name": "net_sched: fix races with RCU callbacks", "version": 1, "mbox": "http://patchwork.ozlabs.org/series/10472/mbox/" } ], "comments": "http://patchwork.ozlabs.org/api/patches/831010/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/831010/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<netdev-owner@vger.kernel.org>", "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=<UNKNOWN>)", "ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"D3344UCs\"; dkim-atps=neutral" ], "Received": [ "from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yNR510TZlz9t2h\n\tfor <patchwork-incoming@ozlabs.org>;\n\tFri, 27 Oct 2017 12:25:09 +1100 (AEDT)", "(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752006AbdJ0BZG (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tThu, 26 Oct 2017 21:25:06 -0400", "from mail-pf0-f196.google.com ([209.85.192.196]:49809 \"EHLO\n\tmail-pf0-f196.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751947AbdJ0BY7 (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Thu, 26 Oct 2017 21:24:59 -0400", "by mail-pf0-f196.google.com with SMTP id i5so3774498pfe.6\n\tfor <netdev@vger.kernel.org>; Thu, 26 Oct 2017 18:24:59 -0700 (PDT)", "from tw-172-25-30-113.office.twttr.net ([8.25.197.25])\n\tby smtp.gmail.com with ESMTPSA id\n\tb16sm13211973pfe.58.2017.10.26.18.24.57\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tThu, 26 Oct 2017 18:24:57 -0700 (PDT)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=from:to:cc:subject:date:message-id:in-reply-to:references;\n\tbh=IQp++UM5XTDuuPTdh6Opk3Zl9phCAq4QmMOhxqbOtPM=;\n\tb=D3344UCsjfM2ma2UCPELqkXfCESisUT7cEZhpBMUo9zJPbqJQgAc4buIqg/RIT8002\n\tZtjutwXzGy+ez7bfgjwx4fu8rPLvYNSv1IRJ/6ceaN9d6yKA3Uzn38FNYli2yrdoLNjT\n\t8Sajk9QssI9hDz1M6PdubKjjK/Ab1ty0i++8KgbcO7C6j5Q+6U65fW1ZAoEeWz02/fUF\n\tMF86b0hkk/vcXrJqFIfu4UNrcGTOgj/8yaB+Q/Bks+DKDSbW4XelMI2n8oWSlIqlYxm+\n\tmBJY0WYnIUQh4bKBOq6vr5MLbedqSxKWseIoboOPbUD/gdTE31iZ34mLdMBRHs49aapv\n\tNLCw==", "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to\n\t:references;\n\tbh=IQp++UM5XTDuuPTdh6Opk3Zl9phCAq4QmMOhxqbOtPM=;\n\tb=VPWdHk1n8iDiFchUWmgSkFGl2kLJyaxknesi5eL3Rbw9OTQZXt1ABIwwX70eh/SA2N\n\tVlaBiAW9ColmIUBwai/pkNypLpnA/610C/1fyMgIaqfhYjLABwcW9I1CpoEGf2UCX11l\n\tn7wHzVnpjSu68bHijrtrfMmUpKP+KCRpWa3QvZzEgDG0bP/CcgDQpKWt5LeCxpt9is3T\n\tZB6yncV442LTtOaZGVH2VAmEQwZaMRMyNuFXMKZUG0hLgmeM2IWhf0WzcMyWgZ1jM8Df\n\tZDNyxy9mcW4P8xZqu9LLIhe7qiy4RpxcZ8QbUluhB/s9b6QxyMVh9uXgwEdvxRcF08JU\n\teH7w==", "X-Gm-Message-State": "AMCzsaXzHVxyzkL+1l66waoKr6g2327dzU2k+jsMdvFgwckiWDEfA2SR\n\tZrj5nA0UofEQCI2VFVCC2A9Iolrm", "X-Google-Smtp-Source": "ABhQp+ThqvxjvGby0PPkd9v2Tplgd9Hy1SmMx3E7XZ1gI0o0uND1geP+4IEXzMeA5cUfIalmY1+Bmg==", "X-Received": "by 10.84.252.141 with SMTP id y13mr5651894pll.295.1509067498718; \n\tThu, 26 Oct 2017 18:24:58 -0700 (PDT)", "From": "Cong Wang <xiyou.wangcong@gmail.com>", "To": "netdev@vger.kernel.org", "Cc": "Chris Mi <chrism@mellanox.com>, Cong Wang <xiyou.wangcong@gmail.com>,\n\tDaniel Borkmann <daniel@iogearbox.net>, Jiri Pirko <jiri@resnulli.us>,\n\tJohn Fastabend <john.fastabend@gmail.com>,\n\tJamal Hadi Salim <jhs@mojatatu.com>,\n\t\"Paul E. McKenney\" <paulmck@linux.vnet.ibm.com>", "Subject": "[Patch net 01/16] net_sched: introduce a workqueue for RCU\n\tcallbacks of tc filter", "Date": "Thu, 26 Oct 2017 18:24:28 -0700", "Message-Id": "<20171027012443.3306-2-xiyou.wangcong@gmail.com>", "X-Mailer": "git-send-email 2.9.4", "In-Reply-To": "<20171027012443.3306-1-xiyou.wangcong@gmail.com>", "References": "<20171027012443.3306-1-xiyou.wangcong@gmail.com>", "Sender": "netdev-owner@vger.kernel.org", "Precedence": "bulk", "List-ID": "<netdev.vger.kernel.org>", "X-Mailing-List": "netdev@vger.kernel.org" }, "content": "This patch introduces a dedicated workqueue for tc filters\nso that each tc filter's RCU callback could defer their\naction destroy work to this workqueue. The helper\ntcf_queue_work() is introduced for them to use.\n\nBecause we hold RTNL lock when calling tcf_block_put(), we\ncan not simply flush works inside it, therefore we have to\ndefer it again to this workqueue and make sure all flying RCU\ncallbacks have already queued their work before this one, in\nother words, to ensure this is the last one to execute to\nprevent any use-after-free.\n\nOn the other hand, this makes tcf_block_put() ugly and\nharder to understand. Since David and Eric strongly dislike\nadding synchronize_rcu(), this is probably the only\nsolution that could make everyone happy.\n\nPlease also see the code comments below.\n\nReported-by: Chris Mi <chrism@mellanox.com>\nCc: Daniel Borkmann <daniel@iogearbox.net>\nCc: Jiri Pirko <jiri@resnulli.us>\nCc: John Fastabend <john.fastabend@gmail.com>\nCc: Jamal Hadi Salim <jhs@mojatatu.com>\nCc: \"Paul E. McKenney\" <paulmck@linux.vnet.ibm.com>\nSigned-off-by: Cong Wang <xiyou.wangcong@gmail.com>\n---\n include/net/pkt_cls.h | 3 +++\n include/net/sch_generic.h | 2 ++\n net/sched/cls_api.c | 68 +++++++++++++++++++++++++++++++++++------------\n 3 files changed, 56 insertions(+), 17 deletions(-)", "diff": "diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h\nindex e80edd8879ef..3009547f3c66 100644\n--- a/include/net/pkt_cls.h\n+++ b/include/net/pkt_cls.h\n@@ -2,6 +2,7 @@\n #define __NET_PKT_CLS_H\n \n #include <linux/pkt_cls.h>\n+#include <linux/workqueue.h>\n #include <net/sch_generic.h>\n #include <net/act_api.h>\n \n@@ -17,6 +18,8 @@ struct tcf_walker {\n int register_tcf_proto_ops(struct tcf_proto_ops *ops);\n int unregister_tcf_proto_ops(struct tcf_proto_ops *ops);\n \n+bool tcf_queue_work(struct work_struct *work);\n+\n #ifdef CONFIG_NET_CLS\n struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index,\n \t\t\t\tbool create);\ndiff --git a/include/net/sch_generic.h b/include/net/sch_generic.h\nindex 135f5a2dd931..0dec8a23be57 100644\n--- a/include/net/sch_generic.h\n+++ b/include/net/sch_generic.h\n@@ -10,6 +10,7 @@\n #include <linux/dynamic_queue_limits.h>\n #include <linux/list.h>\n #include <linux/refcount.h>\n+#include <linux/workqueue.h>\n #include <net/gen_stats.h>\n #include <net/rtnetlink.h>\n \n@@ -271,6 +272,7 @@ struct tcf_chain {\n \n struct tcf_block {\n \tstruct list_head chain_list;\n+\tstruct work_struct work;\n };\n \n static inline void qdisc_cb_private_validate(const struct sk_buff *skb, int sz)\ndiff --git a/net/sched/cls_api.c b/net/sched/cls_api.c\nindex 0b2219adf520..045d13679ad6 100644\n--- a/net/sched/cls_api.c\n+++ b/net/sched/cls_api.c\n@@ -77,6 +77,8 @@ int register_tcf_proto_ops(struct tcf_proto_ops *ops)\n }\n EXPORT_SYMBOL(register_tcf_proto_ops);\n \n+static struct workqueue_struct *tc_filter_wq;\n+\n int unregister_tcf_proto_ops(struct tcf_proto_ops *ops)\n {\n \tstruct tcf_proto_ops *t;\n@@ -86,6 +88,7 @@ int unregister_tcf_proto_ops(struct tcf_proto_ops *ops)\n \t * tcf_proto_ops's destroy() handler.\n \t */\n \trcu_barrier();\n+\tflush_workqueue(tc_filter_wq);\n \n \twrite_lock(&cls_mod_lock);\n \tlist_for_each_entry(t, &tcf_proto_base, head) {\n@@ -100,6 +103,12 @@ int unregister_tcf_proto_ops(struct tcf_proto_ops *ops)\n }\n EXPORT_SYMBOL(unregister_tcf_proto_ops);\n \n+bool tcf_queue_work(struct work_struct *work)\n+{\n+\treturn queue_work(tc_filter_wq, work);\n+}\n+EXPORT_SYMBOL(tcf_queue_work);\n+\n /* Select new prio value from the range, managed by kernel. */\n \n static inline u32 tcf_auto_prio(struct tcf_proto *tp)\n@@ -266,23 +275,30 @@ int tcf_block_get(struct tcf_block **p_block,\n }\n EXPORT_SYMBOL(tcf_block_get);\n \n-void tcf_block_put(struct tcf_block *block)\n+static void tcf_block_put_final(struct work_struct *work)\n {\n+\tstruct tcf_block *block = container_of(work, struct tcf_block, work);\n \tstruct tcf_chain *chain, *tmp;\n \n-\tif (!block)\n-\t\treturn;\n-\n-\t/* XXX: 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 hold the chains\n-\t * first, otherwise we would always race with RCU callbacks on this list\n-\t * without proper locking.\n-\t */\n+\t/* At this point, all the chains should have refcnt == 1. */\n+\trtnl_lock();\n+\tlist_for_each_entry_safe(chain, tmp, &block->chain_list, list)\n+\t\ttcf_chain_put(chain);\n+\trtnl_unlock();\n+\tkfree(block);\n+}\n \n-\t/* Wait for existing RCU callbacks to cool down. */\n-\trcu_barrier();\n+/* XXX: Standalone actions are not allowed to jump to any chain, and bound\n+ * actions should be all removed after flushing. However, filters are destroyed\n+ * in RCU callbacks, we have to hold the chains first, otherwise we would\n+ * always race with RCU callbacks on this list without proper locking.\n+ */\n+static void tcf_block_put_deferred(struct work_struct *work)\n+{\n+\tstruct tcf_block *block = container_of(work, struct tcf_block, work);\n+\tstruct tcf_chain *chain;\n \n+\trtnl_lock();\n \t/* Hold a refcnt for all chains, except 0, in case they are gone. */\n \tlist_for_each_entry(chain, &block->chain_list, list)\n \t\tif (chain->index)\n@@ -292,13 +308,27 @@ void tcf_block_put(struct tcf_block *block)\n \tlist_for_each_entry(chain, &block->chain_list, list)\n \t\ttcf_chain_flush(chain);\n \n-\t/* Wait for RCU callbacks to release the reference count. */\n+\tINIT_WORK(&block->work, tcf_block_put_final);\n+\t/* Wait for RCU callbacks to release the reference count and make\n+\t * sure their works have been queued before this.\n+\t */\n \trcu_barrier();\n+\ttcf_queue_work(&block->work);\n+\trtnl_unlock();\n+}\n \n-\t/* At this point, all the chains should have refcnt == 1. */\n-\tlist_for_each_entry_safe(chain, tmp, &block->chain_list, list)\n-\t\ttcf_chain_put(chain);\n-\tkfree(block);\n+void tcf_block_put(struct tcf_block *block)\n+{\n+\tif (!block)\n+\t\treturn;\n+\n+\tINIT_WORK(&block->work, tcf_block_put_deferred);\n+\t/* Wait for existing RCU callbacks to cool down, make sure their works\n+\t * have been queued before this. We can not flush pending works here\n+\t * because we are holding the RTNL lock.\n+\t */\n+\trcu_barrier();\n+\ttcf_queue_work(&block->work);\n }\n EXPORT_SYMBOL(tcf_block_put);\n \n@@ -1030,6 +1060,10 @@ EXPORT_SYMBOL(tcf_exts_get_dev);\n \n static int __init tc_filter_init(void)\n {\n+\ttc_filter_wq = alloc_ordered_workqueue(\"tc_filter_workqueue\", 0);\n+\tif (!tc_filter_wq)\n+\t\treturn -ENOMEM;\n+\n \trtnl_register(PF_UNSPEC, RTM_NEWTFILTER, tc_ctl_tfilter, NULL, 0);\n \trtnl_register(PF_UNSPEC, RTM_DELTFILTER, tc_ctl_tfilter, NULL, 0);\n \trtnl_register(PF_UNSPEC, RTM_GETTFILTER, tc_ctl_tfilter,\n", "prefixes": [ "net", "01/16" ] }