{"id":811831,"url":"http://patchwork.ozlabs.org/api/patches/811831/?format=json","web_url":"http://patchwork.ozlabs.org/project/netdev/patch/150490443011.11590.2847947557652786219.stgit@john-XPS-13-9360/","project":{"id":7,"url":"http://patchwork.ozlabs.org/api/projects/7/?format=json","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":"<150490443011.11590.2847947557652786219.stgit@john-XPS-13-9360>","list_archive_url":null,"date":"2017-09-08T21:00:30","name":"[net,1/3] net: rcu lock and preempt disable missing around generic xdp","commit_ref":null,"pull_url":null,"state":"accepted","archived":true,"hash":"db4fb26ab986e4b86ce14759d49b1395d7cb43a2","submitter":{"id":20028,"url":"http://patchwork.ozlabs.org/api/people/20028/?format=json","name":"John Fastabend","email":"john.fastabend@gmail.com"},"delegate":{"id":34,"url":"http://patchwork.ozlabs.org/api/users/34/?format=json","username":"davem","first_name":"David","last_name":"Miller","email":"davem@davemloft.net"},"mbox":"http://patchwork.ozlabs.org/project/netdev/patch/150490443011.11590.2847947557652786219.stgit@john-XPS-13-9360/mbox/","series":[{"id":2291,"url":"http://patchwork.ozlabs.org/api/series/2291/?format=json","web_url":"http://patchwork.ozlabs.org/project/netdev/list/?series=2291","date":"2017-09-08T21:00:05","name":"Fixes for XDP/BPF","version":1,"mbox":"http://patchwork.ozlabs.org/series/2291/mbox/"}],"comments":"http://patchwork.ozlabs.org/api/patches/811831/comments/","check":"pending","checks":"http://patchwork.ozlabs.org/api/patches/811831/checks/","tags":{},"related":[],"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=\"FIqnLTVJ\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xpqV72WMjz9s75\n\tfor ;\n\tSat, 9 Sep 2017 07:00:47 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S932521AbdIHVAp (ORCPT );\n\tFri, 8 Sep 2017 17:00:45 -0400","from mail-pg0-f66.google.com ([74.125.83.66]:38300 \"EHLO\n\tmail-pg0-f66.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S932400AbdIHVAo (ORCPT\n\t); Fri, 8 Sep 2017 17:00:44 -0400","by mail-pg0-f66.google.com with SMTP id t3so1778507pgt.5\n\tfor ; Fri, 08 Sep 2017 14:00:43 -0700 (PDT)","from [127.0.1.1] ([72.168.144.71])\n\tby smtp.gmail.com with ESMTPSA id\n\ty4sm4329079pgs.19.2017.09.08.14.00.36\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tFri, 08 Sep 2017 14:00:42 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=subject:from:to:cc:date:message-id:in-reply-to:references\n\t:user-agent:mime-version:content-transfer-encoding;\n\tbh=WkP4HqI6MyXumtjWVFRlWhtc5RCOhO4D3VCNFTcJGTo=;\n\tb=FIqnLTVJqkbwKdEkEb9k4FUodpj95K6uT/SsPIwDJG8JN8jJZv1ciJmK/XLKu71+Sk\n\tEmubGAb4yQE3BH43qRRji1/Qb17MMfU/7bvAQrnR5z19vkyeaHESuCBzN9DHZ+/MJfPq\n\tgCjBCJ9u5HQs0WLevoQ7Coe0yGxYMJJ5n4f8QwoGy5ZJFB/DGWDLNNEpuCX2sjX87vse\n\tDRgUkwrsKdqMA2NoauxuuwxHYg4Zhsuf5WeM5rkAb2pMOkhd5FVDATwOXwJdKaov17bW\n\tNglGWt7Om8vEsEAww5c3Y49PmqIp1eM1Xkuh2Tc9ULa61LBfpt09FmfGQSo9nDxQm9il\n\tiLGw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:subject:from:to:cc:date:message-id:in-reply-to\n\t:references:user-agent:mime-version:content-transfer-encoding;\n\tbh=WkP4HqI6MyXumtjWVFRlWhtc5RCOhO4D3VCNFTcJGTo=;\n\tb=YHGmoi/DjFixc+cTyzL4vtR7skrp3gcXoETUauysDb7fJozNOGCBCUrxJO0jn8JWpm\n\t3ZPRvrpqSzS1T63MfFLcPHZBJNAyErS/uT24LBEisxw8xrjTzYKQnF83w3uIjhxboJBr\n\ti1kfZeMQ7ZEiQ0mghZa/NOltrznyDz62/llGZUv3fre0IlpYY9FNPF6nOTkG31LdEAQ7\n\tvSWHtFG5kNUjYQ6VImeni8wWyfI+H/gSjMTpRDawOXbPChH52CVFJMyzLjdwEFAsIkIN\n\tkVCLFRuam6jL0P8BQ7gIvJ7nDlKIYLQYgKjA+b1b+A68U8IGHJBZNjqYMhA4Ak0As9lR\n\tx2BQ==","X-Gm-Message-State":"AHPjjUhfE+QRIJOJrkf6a7CtnaUrPyRnTMdKp1OoYBfnLleqE+KRtmiZ\n\t7Rnz2LnZ2yhrSFL2","X-Google-Smtp-Source":"ADKCNb5sa5ippAzqeUn71OqL7WWcrQCBMHN3mE55kvhAetW199+ZxNCU250nRQOYEkKl67zVof0bWA==","X-Received":"by 10.84.238.131 with SMTP id v3mr4884416plk.342.1504904443352; \n\tFri, 08 Sep 2017 14:00:43 -0700 (PDT)","Subject":"[net PATCH 1/3] net: rcu lock and preempt disable missing around\n\tgeneric xdp","From":"John Fastabend ","To":"davem@davemloft.net","Cc":"netdev@vger.kernel.org, john.fastabend@gmail.com,\n\tdaniel@iogearbox.net, ast@fb.com","Date":"Fri, 08 Sep 2017 14:00:30 -0700","Message-ID":"<150490443011.11590.2847947557652786219.stgit@john-XPS-13-9360>","In-Reply-To":"<150490397545.11590.1409723973253492363.stgit@john-XPS-13-9360>","References":"<150490397545.11590.1409723973253492363.stgit@john-XPS-13-9360>","User-Agent":"StGit/0.17.1-dirty","MIME-Version":"1.0","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"7bit","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"},"content":"do_xdp_generic must be called inside rcu critical section with preempt\ndisabled to ensure BPF programs are valid and per-cpu variables used\nfor redirect operations are consistent. This patch ensures this is true\nand fixes the splat below.\n\nThe netif_receive_skb_internal() code path is now broken into two rcu\ncritical sections. I decided it was better to limit the preempt_enable/disable\nblock to just the xdp static key portion and the fallout is more\nrcu_read_lock/unlock calls. Seems like the best option to me.\n\n[ 607.596901] =============================\n[ 607.596906] WARNING: suspicious RCU usage\n[ 607.596912] 4.13.0-rc4+ #570 Not tainted\n[ 607.596917] -----------------------------\n[ 607.596923] net/core/dev.c:3948 suspicious rcu_dereference_check() usage!\n[ 607.596927]\n[ 607.596927] other info that might help us debug this:\n[ 607.596927]\n[ 607.596933]\n[ 607.596933] rcu_scheduler_active = 2, debug_locks = 1\n[ 607.596938] 2 locks held by pool/14624:\n[ 607.596943] #0: (rcu_read_lock_bh){......}, at: [] ip_finish_output2+0x14d/0x890\n[ 607.596973] #1: (rcu_read_lock_bh){......}, at: [] __dev_queue_xmit+0x14a/0xfd0\n[ 607.597000]\n[ 607.597000] stack backtrace:\n[ 607.597006] CPU: 5 PID: 14624 Comm: pool Not tainted 4.13.0-rc4+ #570\n[ 607.597011] Hardware name: Dell Inc. Precision Tower 5810/0HHV7N, BIOS A17 03/01/2017\n[ 607.597016] Call Trace:\n[ 607.597027] dump_stack+0x67/0x92\n[ 607.597040] lockdep_rcu_suspicious+0xdd/0x110\n[ 607.597054] do_xdp_generic+0x313/0xa50\n[ 607.597068] ? time_hardirqs_on+0x5b/0x150\n[ 607.597076] ? mark_held_locks+0x6b/0xc0\n[ 607.597088] ? netdev_pick_tx+0x150/0x150\n[ 607.597117] netif_rx_internal+0x205/0x3f0\n[ 607.597127] ? do_xdp_generic+0xa50/0xa50\n[ 607.597144] ? lock_downgrade+0x2b0/0x2b0\n[ 607.597158] ? __lock_is_held+0x93/0x100\n[ 607.597187] netif_rx+0x119/0x190\n[ 607.597202] loopback_xmit+0xfd/0x1b0\n[ 607.597214] dev_hard_start_xmit+0x127/0x4e0\n\nFixes: d445516966dc (\"net: xdp: support xdp generic on virtual devices\")\nFixes: b5cdae3291f7 (\"net: Generic XDP\")\nAcked-by: Daniel Borkmann \nSigned-off-by: John Fastabend \n---\n net/core/dev.c | 25 ++++++++++++++++---------\n 1 file changed, 16 insertions(+), 9 deletions(-)","diff":"diff --git a/net/core/dev.c b/net/core/dev.c\nindex 6f845e4..fb766d9 100644\n--- a/net/core/dev.c\n+++ b/net/core/dev.c\n@@ -3981,8 +3981,13 @@ static int netif_rx_internal(struct sk_buff *skb)\n \ttrace_netif_rx(skb);\n \n \tif (static_key_false(&generic_xdp_needed)) {\n-\t\tint ret = do_xdp_generic(rcu_dereference(skb->dev->xdp_prog),\n-\t\t\t\t\t skb);\n+\t\tint ret;\n+\n+\t\tpreempt_disable();\n+\t\trcu_read_lock();\n+\t\tret = do_xdp_generic(rcu_dereference(skb->dev->xdp_prog), skb);\n+\t\trcu_read_unlock();\n+\t\tpreempt_enable();\n \n \t\t/* Consider XDP consuming the packet a success from\n \t\t * the netdev point of view we do not want to count\n@@ -4500,18 +4505,20 @@ static int netif_receive_skb_internal(struct sk_buff *skb)\n \tif (skb_defer_rx_timestamp(skb))\n \t\treturn NET_RX_SUCCESS;\n \n-\trcu_read_lock();\n-\n \tif (static_key_false(&generic_xdp_needed)) {\n-\t\tint ret = do_xdp_generic(rcu_dereference(skb->dev->xdp_prog),\n-\t\t\t\t\t skb);\n+\t\tint ret;\n \n-\t\tif (ret != XDP_PASS) {\n-\t\t\trcu_read_unlock();\n+\t\tpreempt_disable();\n+\t\trcu_read_lock();\n+\t\tret = do_xdp_generic(rcu_dereference(skb->dev->xdp_prog), skb);\n+\t\trcu_read_unlock();\n+\t\tpreempt_enable();\n+\n+\t\tif (ret != XDP_PASS)\n \t\t\treturn NET_RX_DROP;\n-\t\t}\n \t}\n \n+\trcu_read_lock();\n #ifdef CONFIG_RPS\n \tif (static_key_false(&rps_needed)) {\n \t\tstruct rps_dev_flow voidflow, *rflow = &voidflow;\n","prefixes":["net","1/3"]}