From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stanislav Fomichev Subject: Re: [PATCH v3 bpf-next 4/4] bpftool: support loading flow dissector Date: Thu, 8 Nov 2018 13:29:40 -0800 Message-ID: <20181108212940.wvfnkanxqs2owcy6@mini-arch> References: <20181108053957.205681-1-sdf@google.com> <20181108053957.205681-5-sdf@google.com> <20181108114630.28907570@cakuba.hsd1.ca.comcast.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Stanislav Fomichev , netdev@vger.kernel.org, linux-kselftest@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net, shuah@kernel.org, quentin.monnet@netronome.com, guro@fb.com, jiong.wang@netronome.com, bhole_prashant_q7@lab.ntt.co.jp, john.fastabend@gmail.com, jbenc@redhat.com, treeze.taeung@gmail.com, yhs@fb.com, osk@fb.com, sandipan@linux.vnet.ibm.com To: Jakub Kicinski Return-path: Received: from mail-pg1-f196.google.com ([209.85.215.196]:45434 "EHLO mail-pg1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725882AbeKIHHE (ORCPT ); Fri, 9 Nov 2018 02:07:04 -0500 Received: by mail-pg1-f196.google.com with SMTP id y4so8960672pgc.12 for ; Thu, 08 Nov 2018 13:29:42 -0800 (PST) Content-Disposition: inline In-Reply-To: <20181108114630.28907570@cakuba.hsd1.ca.comcast.net> Sender: netdev-owner@vger.kernel.org List-ID: On 11/08, Jakub Kicinski wrote: > On Wed, 7 Nov 2018 21:39:57 -0800, Stanislav Fomichev wrote: > > This commit adds support for loading/attaching/detaching flow > > dissector program. The structure of the flow dissector program is > > assumed to be the same as in the selftests: > > > > * flow_dissector section with the main entry point > > * a bunch of tail call progs > > * a jmp_table map that is populated with the tail call progs > > Could you split the loadall changes and the flow_dissector changes into > two separate patches? Sure, will do, but let's first agree on the semantical differences of load vs loadall. So far *load* actually loads _all_ progs (via bpf_object__load), but pins only the first program. Is that what we want? I wonder whether the assumption there was that there is only single program in the object. Should we load only the first program in *load*? If we add *loadall*, then the difference would be: *load*: * loads all maps and only the first program, pins only the first program *loadall*: * loads all maps and all programs, pins everything (maps and programs) Is this the expected behavior? From mboxrd@z Thu Jan 1 00:00:00 1970 From: sdf at fomichev.me (Stanislav Fomichev) Date: Thu, 8 Nov 2018 13:29:40 -0800 Subject: [PATCH v3 bpf-next 4/4] bpftool: support loading flow dissector In-Reply-To: <20181108114630.28907570@cakuba.hsd1.ca.comcast.net> References: <20181108053957.205681-1-sdf@google.com> <20181108053957.205681-5-sdf@google.com> <20181108114630.28907570@cakuba.hsd1.ca.comcast.net> Message-ID: <20181108212940.wvfnkanxqs2owcy6@mini-arch> On 11/08, Jakub Kicinski wrote: > On Wed, 7 Nov 2018 21:39:57 -0800, Stanislav Fomichev wrote: > > This commit adds support for loading/attaching/detaching flow > > dissector program. The structure of the flow dissector program is > > assumed to be the same as in the selftests: > > > > * flow_dissector section with the main entry point > > * a bunch of tail call progs > > * a jmp_table map that is populated with the tail call progs > > Could you split the loadall changes and the flow_dissector changes into > two separate patches? Sure, will do, but let's first agree on the semantical differences of load vs loadall. So far *load* actually loads _all_ progs (via bpf_object__load), but pins only the first program. Is that what we want? I wonder whether the assumption there was that there is only single program in the object. Should we load only the first program in *load*? If we add *loadall*, then the difference would be: *load*: * loads all maps and only the first program, pins only the first program *loadall*: * loads all maps and all programs, pins everything (maps and programs) Is this the expected behavior? From mboxrd@z Thu Jan 1 00:00:00 1970 From: sdf@fomichev.me (Stanislav Fomichev) Date: Thu, 8 Nov 2018 13:29:40 -0800 Subject: [PATCH v3 bpf-next 4/4] bpftool: support loading flow dissector In-Reply-To: <20181108114630.28907570@cakuba.hsd1.ca.comcast.net> References: <20181108053957.205681-1-sdf@google.com> <20181108053957.205681-5-sdf@google.com> <20181108114630.28907570@cakuba.hsd1.ca.comcast.net> Message-ID: <20181108212940.wvfnkanxqs2owcy6@mini-arch> Content-Type: text/plain; charset="UTF-8" Message-ID: <20181108212940.vR22PWav69peNsnBTOTe3tHucO8Y_55IEEqHI2IPRsk@z> On 11/08, Jakub Kicinski wrote: > On Wed, 7 Nov 2018 21:39:57 -0800, Stanislav Fomichev wrote: > > This commit adds support for loading/attaching/detaching flow > > dissector program. The structure of the flow dissector program is > > assumed to be the same as in the selftests: > > > > * flow_dissector section with the main entry point > > * a bunch of tail call progs > > * a jmp_table map that is populated with the tail call progs > > Could you split the loadall changes and the flow_dissector changes into > two separate patches? Sure, will do, but let's first agree on the semantical differences of load vs loadall. So far *load* actually loads _all_ progs (via bpf_object__load), but pins only the first program. Is that what we want? I wonder whether the assumption there was that there is only single program in the object. Should we load only the first program in *load*? If we add *loadall*, then the difference would be: *load*: * loads all maps and only the first program, pins only the first program *loadall*: * loads all maps and all programs, pins everything (maps and programs) Is this the expected behavior?