[{"id":1803250,"web_url":"http://patchwork.ozlabs.org/comment/1803250/","msgid":"<20171112090041.GG1993@nanopsycho>","list_archive_url":null,"date":"2017-11-12T09:00:41","subject":"Re: [PATCH net-next v2 05/15] xdp: allow attaching programs loaded\n\tfor specific device","submitter":{"id":15321,"url":"http://patchwork.ozlabs.org/api/people/15321/","name":"Jiri Pirko","email":"jiri@resnulli.us"},"content":"Fri, Nov 03, 2017 at 09:56:20PM CET, jakub.kicinski@netronome.com wrote:\n>Pass the netdev pointer to bpf_prog_get_type(). This way\n>BPF code can decide whether the device matches what the\n>code was loaded/translated for.\n>\n\n[...]\n\n\n>diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c\n>index 3217c20ea91b..68f9123acd39 100644\n>--- a/kernel/bpf/syscall.c\n>+++ b/kernel/bpf/syscall.c\n>@@ -1057,7 +1057,22 @@ struct bpf_prog *bpf_prog_inc_not_zero(struct bpf_prog *prog)\n> }\n> EXPORT_SYMBOL_GPL(bpf_prog_inc_not_zero);\n> \n>-static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *attach_type)\n>+static bool bpf_prog_can_attach(struct bpf_prog *prog,\n>+\t\t\t\tenum bpf_prog_type *attach_type,\n>+\t\t\t\tstruct net_device *netdev)\n>+{\n>+\tstruct bpf_dev_offload *offload = prog->aux->offload;\n>+\n>+\tif (prog->type != *attach_type)\n>+\t\treturn false;\n>+\tif (offload && offload->netdev != netdev)\n\nThis means you return false in case netdev function arg is NULL. Is that\nintentional?\n\nSeems wrong to me because for example in cls_bpf_prog_from_efd, you\nwould get an error in case TCA_CLS_FLAGS_SKIP_SW is not set.\n\n\n\n\n>+\t\treturn false;\n>+\n>+\treturn true;\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=\"ND1yeXQO\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yZSRP4PtTz9t3Z\n\tfor ;\n\tSun, 12 Nov 2017 20:00:49 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753352AbdKLJAo (ORCPT );\n\tSun, 12 Nov 2017 04:00:44 -0500","from mail-wm0-f46.google.com ([74.125.82.46]:55004 \"EHLO\n\tmail-wm0-f46.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1753050AbdKLJAn (ORCPT\n\t); Sun, 12 Nov 2017 04:00:43 -0500","by mail-wm0-f46.google.com with SMTP id r68so9714271wmr.3\n\tfor ; Sun, 12 Nov 2017 01:00:43 -0800 (PST)","from localhost (ip-94-113-127-32.net.upcbroadband.cz.\n\t[94.113.127.32])\n\tby smtp.gmail.com with ESMTPSA id 4sm9941036wrh.7.2017.11.12.01.00.41\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tSun, 12 Nov 2017 01:00:41 -0800 (PST)"],"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=TGGo/OI/NzImoCSOfPSMOf2y2ajquVVJJsV1V0efe4E=;\n\tb=ND1yeXQOq48f3H5fpUBxi9DSW/sXaRqElQJUAyfSz3J847BEa/prsAKyGhBOScg7fn\n\tDKbsVZH055k72U6XfTdnoIAZsF2EpdrS8vuCnYAyAotmayBGEkeQniPXXz6mbhoRm5Wx\n\ty+VgSgeoEirCyLXXy1in2S6cKaOdq8F8rh4xiMT+QGco6ULMevilYVbZveVwyjAZ/FqP\n\tJ+BpsGEdfVeiAkLEk6lsYTcASbu3gXW3cWbdV/npXyiWoX1V4OqT4FfTaoxajN5jDqUC\n\tUzJf9vDqdSYi8Zeq3NS7PJh/onxuL73UQX4Auo8CDyiGpToHVahDmXYWzm8R0JraJGoK\n\tmnmQ==","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=TGGo/OI/NzImoCSOfPSMOf2y2ajquVVJJsV1V0efe4E=;\n\tb=rdiVY9x5G0MZ/LMxbtnAhpaLED5ipibsD2mBDnd5fCj78seJ1lWAKfUotedayS6dX0\n\t7xnB7pnbsbU6aUCoZqHgK6PdRbA4ka/HdQf+2o3iZFsXDOTzWLVQ/ZKgc0lyRu4NnDkS\n\tDl1Ox5Ka6CrG9YhXytoL41eJTVgp1V69iIoVMiOxnlo+84ZnSU09Q/EZ7CFZvOpQQ0wf\n\tW2bFlpiwb/+ln6TCB5mTBNL2RWZmKliVErdmy0xPLX5oVynjY0tpaV4mv1nNJSYz322m\n\thdW8PGLnvfDyARxQw76Mf6lCN0td2HIsjOa13LU/uotFbK1cBGS1rwYYxW6jFCfXmjIa\n\tMDyg==","X-Gm-Message-State":"AJaThX7MUIKyV7UjmEG0XWs+8uMcAo5UvY2t+rNRhWCZaGFMTSaw+Kqs\n\tf0qitUG3u7nowl1NXHPjeLhYVA==","X-Google-Smtp-Source":"AGs4zMZz52rCWcvyBAtW6/3spcMHZorSue9XSR9fg3XCHNPQWI5c1psnOIo0uYvMmvh4maWiRkQ4kA==","X-Received":"by 10.28.24.70 with SMTP id 67mr3559047wmy.7.1510477242404;\n\tSun, 12 Nov 2017 01:00:42 -0800 (PST)","Date":"Sun, 12 Nov 2017 10:00:41 +0100","From":"Jiri Pirko ","To":"Jakub Kicinski ","Cc":"netdev@vger.kernel.org, oss-drivers@netronome.com,\n\talexei.starovoitov@gmail.com, daniel@iogearbox.net","Subject":"Re: [PATCH net-next v2 05/15] xdp: allow attaching programs loaded\n\tfor specific device","Message-ID":"<20171112090041.GG1993@nanopsycho>","References":"<20171103205630.1083-1-jakub.kicinski@netronome.com>\n\t<20171103205630.1083-6-jakub.kicinski@netronome.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20171103205630.1083-6-jakub.kicinski@netronome.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":1803334,"web_url":"http://patchwork.ozlabs.org/comment/1803334/","msgid":"<935db219-863a-3164-6b60-b203eeafa5ae@iogearbox.net>","list_archive_url":null,"date":"2017-11-12T19:33:07","subject":"Re: [PATCH net-next v2 05/15] xdp: allow attaching programs loaded\n\tfor specific device","submitter":{"id":65705,"url":"http://patchwork.ozlabs.org/api/people/65705/","name":"Daniel Borkmann","email":"daniel@iogearbox.net"},"content":"On 11/12/2017 10:00 AM, Jiri Pirko wrote:\n> Fri, Nov 03, 2017 at 09:56:20PM CET, jakub.kicinski@netronome.com wrote:\n>> Pass the netdev pointer to bpf_prog_get_type(). This way\n>> BPF code can decide whether the device matches what the\n>> code was loaded/translated for.\n> \n> [...]\n> \n>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c\n>> index 3217c20ea91b..68f9123acd39 100644\n>> --- a/kernel/bpf/syscall.c\n>> +++ b/kernel/bpf/syscall.c\n>> @@ -1057,7 +1057,22 @@ struct bpf_prog *bpf_prog_inc_not_zero(struct bpf_prog *prog)\n>> }\n>> EXPORT_SYMBOL_GPL(bpf_prog_inc_not_zero);\n>>\n>> -static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *attach_type)\n>> +static bool bpf_prog_can_attach(struct bpf_prog *prog,\n>> +\t\t\t\tenum bpf_prog_type *attach_type,\n>> +\t\t\t\tstruct net_device *netdev)\n>> +{\n>> +\tstruct bpf_dev_offload *offload = prog->aux->offload;\n>> +\n>> +\tif (prog->type != *attach_type)\n>> +\t\treturn false;\n>> +\tif (offload && offload->netdev != netdev)\n> \n> This means you return false in case netdev function arg is NULL. Is that\n> intentional?\n> \n> Seems wrong to me because for example in cls_bpf_prog_from_efd, you\n> would get an error in case TCA_CLS_FLAGS_SKIP_SW is not set.\n\nI think it's fine, the prog was originally loaded in the verifier pass\nto be JITed via nfp JIT, so you'll have a valid prog->aux->offload, and\nyou're specifying TCA_CLS_FLAGS_SKIP_SW for fetching the qdisc dev where\nwe match devs above. So if that dev is not set (NULL) e.g. due to intention\nof running the specified prog in sw, then you cannot use that bpf_prog\nwhich was loaded as offloaded one instead. Looks correct to me.","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=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yZkT50CQxz9sCZ\n\tfor ;\n\tMon, 13 Nov 2017 06:33:13 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751174AbdKLTdL (ORCPT );\n\tSun, 12 Nov 2017 14:33:11 -0500","from www62.your-server.de ([213.133.104.62]:43871 \"EHLO\n\twww62.your-server.de\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750910AbdKLTdK (ORCPT\n\t); Sun, 12 Nov 2017 14:33:10 -0500","from [194.230.159.195] (helo=localhost.localdomain)\n\tby www62.your-server.de with esmtpsa (TLSv1.2:DHE-RSA-AES256-SHA:256)\n\t(Exim 4.85_2) (envelope-from )\n\tid 1eDy0C-00025R-0W; Sun, 12 Nov 2017 20:33:08 +0100"],"Subject":"Re: [PATCH net-next v2 05/15] xdp: allow attaching programs loaded\n\tfor specific device","To":"Jiri Pirko ,\n\tJakub Kicinski ","Cc":"netdev@vger.kernel.org, oss-drivers@netronome.com,\n\talexei.starovoitov@gmail.com","References":"<20171103205630.1083-1-jakub.kicinski@netronome.com>\n\t<20171103205630.1083-6-jakub.kicinski@netronome.com>\n\t<20171112090041.GG1993@nanopsycho>","From":"Daniel Borkmann ","Message-ID":"<935db219-863a-3164-6b60-b203eeafa5ae@iogearbox.net>","Date":"Sun, 12 Nov 2017 20:33:07 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20171112090041.GG1993@nanopsycho>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Authenticated-Sender":"daniel@iogearbox.net","X-Virus-Scanned":"Clear (ClamAV 0.99.2/24037/Sun Nov 12 14:06:06 2017)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}}]