[{"id":1772111,"web_url":"http://patchwork.ozlabs.org/comment/1772111/","msgid":"<1505929295.29839.103.camel@edumazet-glaptop3.roam.corp.google.com>","list_archive_url":null,"date":"2017-09-20T17:41:35","subject":"Re: [PATCH net-next 1/5] net: add support for noref skb->sk","submitter":{"id":2404,"url":"http://patchwork.ozlabs.org/api/people/2404/","name":"Eric Dumazet","email":"eric.dumazet@gmail.com"},"content":"On Wed, 2017-09-20 at 18:54 +0200, Paolo Abeni wrote:\n> Noref sk do not carry a socket refcount, are valid\n> only inside the current RCU section and must be\n> explicitly cleared before exiting such section.\n> \n> They will be used in a later patch to allow early demux\n> without sock refcounting.\n\n\n\n\n> +/* dummy destructor used by noref sockets */\n> +void sock_dummyfree(struct sk_buff *skb)\n> +{\n\nBUG();\n\n> +}\n> +EXPORT_SYMBOL(sock_dummyfree);\n> +\n\n\nI do not see how you ensure we do not leave RCU section with an skb\ndestructor pointing to this sock_dummyfree()\n\nThis patch series looks quite dangerous to me.\n\nDo we really have real applications using connected UDP sockets and\nwanting very high pps throughput ?\n\nI am pretty sure the bottleneck is the sender part.","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=\"Bb60eOy8\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xy6Vr6ftjz9s4q\n\tfor ;\n\tThu, 21 Sep 2017 03:41:40 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751465AbdITRli (ORCPT );\n\tWed, 20 Sep 2017 13:41:38 -0400","from mail-pf0-f193.google.com ([209.85.192.193]:37041 \"EHLO\n\tmail-pf0-f193.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751024AbdITRlh (ORCPT\n\t); Wed, 20 Sep 2017 13:41:37 -0400","by mail-pf0-f193.google.com with SMTP id e69so1450918pfg.4\n\tfor ; Wed, 20 Sep 2017 10:41:37 -0700 (PDT)","from ?IPv6:2620:15c:2c1:100:c941:3c52:e479:7d4f?\n\t([2620:15c:2c1:100:c941:3c52:e479:7d4f])\n\tby smtp.googlemail.com with ESMTPSA id\n\tz1sm8783002pge.45.2017.09.20.10.41.36\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tWed, 20 Sep 2017 10:41:36 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=message-id:subject:from:to:cc:date:in-reply-to:references\n\t:mime-version:content-transfer-encoding;\n\tbh=coHMVPGCdpo/6ekA5jsrtyyzPOOINzF26mkz5Gy8PJc=;\n\tb=Bb60eOy8zAv536ZLZ0XQW7DTzg++m1NQwp2+aXXBTzEgarFwi59XrToBDTvADzlZxG\n\tyYcv9wgT+TUd5p1xBPNH6t0AQuUDw0PGRQ0lhGCY7lc4qVMip/KVq44F0m1Mmgt8Uew+\n\tAvyueCd4eILIDeg6G8kPusADNImECq8okzeS0/a3G4LzDlsO6EnJQ6ZuZ5UtJQRTM2/r\n\tChOQdNuvkmigkF5txI3WvM/GA7MhpC1ZKjnYQBxHznmM3Li4fb+wnHw7oXLfoKqJAjrg\n\tzVyifwaHMHLmUlWfJhS62sSXrY4VXDWHe/IvJcYvuMp+0x5/Z69ZIL2IsfSL4KiAHyi1\n\trf9w==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to\n\t:references:mime-version:content-transfer-encoding;\n\tbh=coHMVPGCdpo/6ekA5jsrtyyzPOOINzF26mkz5Gy8PJc=;\n\tb=AC10rsBEAPfPSG34l6XJWf6/mHWPIykwnhNAH4PtPw4FD5yeAnJfM9SM86xDFba7Ts\n\tga/jPDVfohmfp3U9O1hnaPJF8eA98GgWIWFichNHDFBMxNVhZ2tVPug6F4hG0xdEFf3t\n\tFmPyJFp6OLFODICooLOuzK4WRcf31ozGicv60URdb8ef8MECqDHMCmZu36s114MmvNeL\n\tOKeIeWML4obK8PBB3XvdGScMZrr4fAU65fUyYnpVaryY/eEVrJ3vV7r+lWO9ha7xc3Nw\n\tKiuVEZcWBt4lv0ncR3uZWdPvUNnKJ7qSEMr5XzBG//Ic/c2jVl1XtRviUUricBdaiDdb\n\t+KlQ==","X-Gm-Message-State":"AHPjjUhgAt4yKgxViGN/sScMDXuDjKDNnWvrakERBiiXFD3+1i7tm1LX\n\tmQ+aUqSqaSsbHcMDpMdqLAg=","X-Google-Smtp-Source":"AOwi7QCDnUfJ8468kW+z3y11x04pJfuJlBlV4xkmgIm3kHVokRSKb+T0LlI0384/WTmB8xhD2yjJtw==","X-Received":"by 10.98.159.76 with SMTP id g73mr2893876pfe.293.1505929297303; \n\tWed, 20 Sep 2017 10:41:37 -0700 (PDT)","Message-ID":"<1505929295.29839.103.camel@edumazet-glaptop3.roam.corp.google.com>","Subject":"Re: [PATCH net-next 1/5] net: add support for noref skb->sk","From":"Eric Dumazet ","To":"Paolo Abeni ","Cc":"netdev@vger.kernel.org, \"David S. Miller\" ,\n\tPablo Neira Ayuso , Florian Westphal ,\n\tEric Dumazet ,\n\tHannes Frederic Sowa ","Date":"Wed, 20 Sep 2017 10:41:35 -0700","In-Reply-To":"<39b55ec9584df7dfd6eb498a3d354cd2e08e3eaa.1505926196.git.pabeni@redhat.com>","References":"\n\t<39b55ec9584df7dfd6eb498a3d354cd2e08e3eaa.1505926196.git.pabeni@redhat.com>","Content-Type":"text/plain; charset=\"UTF-8\"","X-Mailer":"Evolution 3.10.4-0ubuntu2 ","Mime-Version":"1.0","Content-Transfer-Encoding":"7bit","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1772597,"web_url":"http://patchwork.ozlabs.org/comment/1772597/","msgid":"<1505985297.2560.39.camel@redhat.com>","list_archive_url":null,"date":"2017-09-21T09:14:57","subject":"Re: [PATCH net-next 1/5] net: add support for noref skb->sk","submitter":{"id":67312,"url":"http://patchwork.ozlabs.org/api/people/67312/","name":"Paolo Abeni","email":"pabeni@redhat.com"},"content":"Hi,\n\nThank you for looking at it!\n\nOn Wed, 2017-09-20 at 10:41 -0700, Eric Dumazet wrote:\n> On Wed, 2017-09-20 at 18:54 +0200, Paolo Abeni wrote:\n> > Noref sk do not carry a socket refcount, are valid\n> > only inside the current RCU section and must be\n> > explicitly cleared before exiting such section.\n> > \n> > They will be used in a later patch to allow early demux\n> > without sock refcounting.\n> \n> \n> \n> \n> > +/* dummy destructor used by noref sockets */\n> > +void sock_dummyfree(struct sk_buff *skb)\n> > +{\n> \n> BUG();\n> \n> > +}\n> > +EXPORT_SYMBOL(sock_dummyfree);\n> > +\n\nWe can call sock_dummyfree() in legitimate paths, see below, but we can\nadd a:\n\nWARN_ON_ONCE(!rcu_read_lock_held());\n\nhere and in skb_clear_noref_sk(). That should help much to catch\npossible bugs.\n\n> I do not see how you ensure we do not leave RCU section with an skb\n> destructor pointing to this sock_dummyfree()\n> \n> This patch series looks quite dangerous to me.\n\nThe idea is to explicitly clear the sknoref references before leaving\nthe RCU section. Quite alike what we currently do for dst noref, but\nhere the only place where we get a noref socket is the socket early\ndemux, thus the scope of this change is more limited to what we have\nwith noref dst_entries.\n\nThe relevant code is in the next 2 patches; after the demux we preserve\nthe sknoref only if the skb has a local destination. The UDP socket\nwill then set the noref on early demux lookup, and the skb will either:\n\n* land on the corresponding UDP socket, the receive function will steal\nthe sknoref\n* be dropped by some nft/iptables target - the dummy destructor is\ncalled\n* forwarded by some nft/iptables target outside the input path; we\nclear the skref explicitly in such targets. \n\nCurrently there are an handful of places affected, and we can simplify\nthe code dropping the early demux result for locally terminated\nmulticast sockets on a host acting as a multicast router, please see\nthe comment on the next patch.\n\n> Do we really have real applications using connected UDP sockets and\n> wanting very high pps throughput ?\n\nThe ultimate goal is to improve the unconnected UDP sockets scenario,\nwe do actually have use cases for that - DNS servers and VoIP SBCs.\n\nThanks,\n\nPaolo","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=)","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=pabeni@redhat.com"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xyWCp393wz9t3w\n\tfor ;\n\tThu, 21 Sep 2017 19:15:02 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751787AbdIUJPA (ORCPT );\n\tThu, 21 Sep 2017 05:15:00 -0400","from mx1.redhat.com ([209.132.183.28]:37376 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751548AbdIUJO7 (ORCPT );\n\tThu, 21 Sep 2017 05:14:59 -0400","from smtp.corp.redhat.com\n\t(int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 915617F7D0;\n\tThu, 21 Sep 2017 09:14:59 +0000 (UTC)","from localhost.localdomain (unknown [10.32.181.195])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 1BC0C5EDEB;\n\tThu, 21 Sep 2017 09:14:57 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 915617F7D0","Message-ID":"<1505985297.2560.39.camel@redhat.com>","Subject":"Re: [PATCH net-next 1/5] net: add support for noref skb->sk","From":"Paolo Abeni ","To":"Eric Dumazet ","Cc":"netdev@vger.kernel.org, \"David S. Miller\" ,\n\tPablo Neira Ayuso , Florian Westphal ,\n\tEric Dumazet ,\n\tHannes Frederic Sowa ","Date":"Thu, 21 Sep 2017 11:14:57 +0200","In-Reply-To":"<1505929295.29839.103.camel@edumazet-glaptop3.roam.corp.google.com>","References":"\n\t<39b55ec9584df7dfd6eb498a3d354cd2e08e3eaa.1505926196.git.pabeni@redhat.com>\n\t<1505929295.29839.103.camel@edumazet-glaptop3.roam.corp.google.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Mime-Version":"1.0","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.15","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.28]);\n\tThu, 21 Sep 2017 09:14:59 +0000 (UTC)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1772660,"web_url":"http://patchwork.ozlabs.org/comment/1772660/","msgid":"<1505990108.29839.117.camel@edumazet-glaptop3.roam.corp.google.com>","list_archive_url":null,"date":"2017-09-21T10:35:08","subject":"Re: [PATCH net-next 1/5] net: add support for noref skb->sk","submitter":{"id":2404,"url":"http://patchwork.ozlabs.org/api/people/2404/","name":"Eric Dumazet","email":"eric.dumazet@gmail.com"},"content":"On Thu, 2017-09-21 at 11:14 +0200, Paolo Abeni wrote:\n> Hi,\n> \n> Thank you for looking at it!\n> \n> On Wed, 2017-09-20 at 10:41 -0700, Eric Dumazet wrote:\n> > On Wed, 2017-09-20 at 18:54 +0200, Paolo Abeni wrote:\n> > > Noref sk do not carry a socket refcount, are valid\n> > > only inside the current RCU section and must be\n> > > explicitly cleared before exiting such section.\n> > > \n> > > They will be used in a later patch to allow early demux\n> > > without sock refcounting.\n> > \n> > \n> > \n> > \n> > > +/* dummy destructor used by noref sockets */\n> > > +void sock_dummyfree(struct sk_buff *skb)\n> > > +{\n> > \n> > BUG();\n> > \n> > > +}\n> > > +EXPORT_SYMBOL(sock_dummyfree);\n> > > +\n> \n> We can call sock_dummyfree() in legitimate paths, see below, but we can\n> add a:\n> \n> WARN_ON_ONCE(!rcu_read_lock_held());\n\nThis wont be enough see below.\n\n> \n> here and in skb_clear_noref_sk(). That should help much to catch\n> possible bugs.\n> \n> > I do not see how you ensure we do not leave RCU section with an skb\n> > destructor pointing to this sock_dummyfree()\n> > \n> > This patch series looks quite dangerous to me.\n> \n> The idea is to explicitly clear the sknoref references before leaving\n> the RCU section. Quite alike what we currently do for dst noref, but\n> here the only place where we get a noref socket is the socket early\n> demux, thus the scope of this change is more limited to what we have\n> with noref dst_entries.\n> \n> The relevant code is in the next 2 patches; after the demux we preserve\n> the sknoref only if the skb has a local destination. The UDP socket\n> will then set the noref on early demux lookup, and the skb will either:\n> \n> * land on the corresponding UDP socket, the receive function will steal\n> the sknoref\n> * be dropped by some nft/iptables target - the dummy destructor is\n> called\n> * forwarded by some nft/iptables target outside the input path; we\n> clear the skref explicitly in such targets. \n> \n> Currently there are an handful of places affected, and we can simplify\n> the code dropping the early demux result for locally terminated\n> multicast sockets on a host acting as a multicast router, please see\n> the comment on the next patch.\n> \n> > Do we really have real applications using connected UDP sockets and\n> > wanting very high pps throughput ?\n> \n> The ultimate goal is to improve the unconnected UDP sockets scenario,\n> we do actually have use cases for that - DNS servers and VoIP SBCs.\n\nUnconnected UDP traffic does not use refcounting on sk _already_.\n\nAnd SO_REUSEPORT already allows us to handle all the traffic we want\n_already_.\n\n\nPlease take a look at 71563f3414e917c62acd8e0fb0edf8ed6af63e4b\n\nThis might tell you why I am so nervous about your changes.\n\nChecking WARN_ON_ONCE(!rcu_read_lock_held());\nis not enough.\n\nrcu_read_lock()\nskb->destructor = sock_dummyfree;\n\nqueue the packet into an intermediate queue.\nrcu_read_unlock();\n\n....\n\nrcu_read_lock()\n...\nif (skb->sk && skb->sk->state == ...) // crash\n\nAlso you covered IPv4, but really we need to forget about IPv4 and focus\non IPv6 only. And _then_ take care of IPv4 compat.","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=\"kjZn9RFW\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xyY0M51ySz9s0Z\n\tfor ;\n\tThu, 21 Sep 2017 20:35:15 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751646AbdIUKfN (ORCPT );\n\tThu, 21 Sep 2017 06:35:13 -0400","from mail-io0-f196.google.com ([209.85.223.196]:34724 \"EHLO\n\tmail-io0-f196.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751605AbdIUKfM (ORCPT\n\t); Thu, 21 Sep 2017 06:35:12 -0400","by mail-io0-f196.google.com with SMTP id g32so4852403ioj.1\n\tfor ; Thu, 21 Sep 2017 03:35:11 -0700 (PDT)","from [192.168.86.171] (c-67-180-167-114.hsd1.ca.comcast.net.\n\t[67.180.167.114]) by smtp.googlemail.com with ESMTPSA id\n\tn63sm835143itb.16.2017.09.21.03.35.09\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tThu, 21 Sep 2017 03:35:10 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=message-id:subject:from:to:cc:date:in-reply-to:references\n\t:mime-version:content-transfer-encoding;\n\tbh=N54UjhG+ysMHbCLH05qThMrzCfDpXhRl+MpulM2EcQw=;\n\tb=kjZn9RFW/6SK+W6BgNpUoZBVdC8MFJXiCL20e9H4FdltXP8NXAlDg7n8sXlWPFX2Yh\n\tGXAZzoRlxX3ftJRKn8DYVbGZOXrTK8xojlk3t7ZHNAIk0wXPqvhftf+2ftwAgnYjaf7g\n\tEfDHbvrq7x8uMZ3wY2elPh0OXMGyhuVyQnlvqUSjKnt2NMhk4AFMRuBMnJNPflFs/IFG\n\tM2lqa0GXz/sMxgHd+jPs5gMKtq0XtSZ35YyjyHqCQ1swJe86QeOVQ5CU9e8FAIOD4Zhn\n\tvt/u76oCYgwOGb4Owc3Rk8N2G75rxVyZMRgEb2aXgwQhhk96zIq6NgKUZmWvzqAUM35f\n\tLEvA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to\n\t:references:mime-version:content-transfer-encoding;\n\tbh=N54UjhG+ysMHbCLH05qThMrzCfDpXhRl+MpulM2EcQw=;\n\tb=CKJ4ngW7/5wAMG7WaGiEDoE7rSf/x1vq+AlvXTtjZIjbdyq4R5QSgANvg6u4O99xG7\n\tTyD8D1ayCEI8FjySiUUc888w1Y8VuR6lVmACb7cG69YJOIzqwoeLlgKyQVFJdUGjjQY6\n\t5qlvZaalHaJOhx6XMblwu4JfN2sV4bzZUb4+x/wPVX/zIU8/PQyqZZAbUgPPJYbbHdNd\n\thxTw24qtuyti883Pz4Y9+dyTjL3umHDWvfwwHDtlTUr4VwGBlN3WZyeTqKUlW7f9J/yi\n\thgXg7v5cyUbkjORxwsmxyf8pjaCk09NxTd9At3hYi7BmWwDX6e92MaP661ZR4SYRodXx\n\tpwOA==","X-Gm-Message-State":"AHPjjUheLm+D4hhw3QsejDPbmtNysSkBjho5loXqgoBr/J7QQUQwYCn4\n\tDy915tO9z66FUb5udLPz5hI=","X-Google-Smtp-Source":"AOwi7QAOOGcsHO11GYZPTmULmxO+c1V/n1iEoSzp1CW1QvMmFhD3Ins96qyjBHxBDC+SdTEW/MlZwg==","X-Received":"by 10.107.141.136 with SMTP id\n\tp130mr2071924iod.102.1505990111337; \n\tThu, 21 Sep 2017 03:35:11 -0700 (PDT)","Message-ID":"<1505990108.29839.117.camel@edumazet-glaptop3.roam.corp.google.com>","Subject":"Re: [PATCH net-next 1/5] net: add support for noref skb->sk","From":"Eric Dumazet ","To":"Paolo Abeni ","Cc":"netdev@vger.kernel.org, \"David S. Miller\" ,\n\tPablo Neira Ayuso , Florian Westphal ,\n\tEric Dumazet ,\n\tHannes Frederic Sowa ","Date":"Thu, 21 Sep 2017 03:35:08 -0700","In-Reply-To":"<1505985297.2560.39.camel@redhat.com>","References":"\n\t<39b55ec9584df7dfd6eb498a3d354cd2e08e3eaa.1505926196.git.pabeni@redhat.com>\n\t<1505929295.29839.103.camel@edumazet-glaptop3.roam.corp.google.com>\n\t<1505985297.2560.39.camel@redhat.com>","Content-Type":"text/plain; charset=\"UTF-8\"","X-Mailer":"Evolution 3.10.4-0ubuntu2 ","Mime-Version":"1.0","Content-Transfer-Encoding":"7bit","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}}]