All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stanislav Fomichev <sdf@fomichev.me>
To: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: Stanislav Fomichev <sdf@google.com>,
	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
Subject: Re: [PATCH v3 bpf-next 4/4] bpftool: support loading flow dissector
Date: Thu, 8 Nov 2018 13:29:40 -0800	[thread overview]
Message-ID: <20181108212940.wvfnkanxqs2owcy6@mini-arch> (raw)
In-Reply-To: <20181108114630.28907570@cakuba.hsd1.ca.comcast.net>

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?

WARNING: multiple messages have this Message-ID (diff)
From: sdf at fomichev.me (Stanislav Fomichev)
Subject: [PATCH v3 bpf-next 4/4] bpftool: support loading flow dissector
Date: Thu, 8 Nov 2018 13:29:40 -0800	[thread overview]
Message-ID: <20181108212940.wvfnkanxqs2owcy6@mini-arch> (raw)
In-Reply-To: <20181108114630.28907570@cakuba.hsd1.ca.comcast.net>

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?

WARNING: multiple messages have this Message-ID (diff)
From: sdf@fomichev.me (Stanislav Fomichev)
Subject: [PATCH v3 bpf-next 4/4] bpftool: support loading flow dissector
Date: Thu, 8 Nov 2018 13:29:40 -0800	[thread overview]
Message-ID: <20181108212940.wvfnkanxqs2owcy6@mini-arch> (raw)
Message-ID: <20181108212940.vR22PWav69peNsnBTOTe3tHucO8Y_55IEEqHI2IPRsk@z> (raw)
In-Reply-To: <20181108114630.28907570@cakuba.hsd1.ca.comcast.net>

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?

  reply	other threads:[~2018-11-09  7:07 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-08  5:39 [PATCH v3 bpf-next 0/4] bpftool: support loading flow dissector Stanislav Fomichev
2018-11-08  5:39 ` Stanislav Fomichev
2018-11-08  5:39 ` sdf
2018-11-08  5:39 ` [PATCH v3 bpf-next 1/4] selftests/bpf: rename flow dissector section to flow_dissector Stanislav Fomichev
2018-11-08  5:39   ` Stanislav Fomichev
2018-11-08  5:39   ` sdf
2018-11-08  5:39 ` [PATCH v3 bpf-next 2/4] libbpf: cleanup after partial failure in bpf_object__pin Stanislav Fomichev
2018-11-08  5:39   ` Stanislav Fomichev
2018-11-08  5:39   ` sdf
2018-11-08  5:39 ` [PATCH v3 bpf-next 3/4] libbpf: bpf_program__pin: add special case for instances.nr == 1 Stanislav Fomichev
2018-11-08  5:39   ` Stanislav Fomichev
2018-11-08  5:39   ` sdf
2018-11-08  5:39 ` [PATCH v3 bpf-next 4/4] bpftool: support loading flow dissector Stanislav Fomichev
2018-11-08  5:39   ` Stanislav Fomichev
2018-11-08  5:39   ` sdf
2018-11-08 11:16   ` Quentin Monnet
2018-11-08 11:16     ` Quentin Monnet
2018-11-08 11:16     ` quentin.monnet
2018-11-08 18:01     ` Stanislav Fomichev
2018-11-08 18:01       ` Stanislav Fomichev
2018-11-08 18:01       ` sdf
2018-11-08 18:21       ` Quentin Monnet
2018-11-08 18:21         ` Quentin Monnet
2018-11-08 18:21         ` quentin.monnet
2018-11-08 19:01         ` Stanislav Fomichev
2018-11-08 19:01           ` Stanislav Fomichev
2018-11-08 19:01           ` sdf
2018-11-08 19:35         ` Jakub Kicinski
2018-11-08 19:35           ` Jakub Kicinski
2018-11-08 19:35           ` jakub.kicinski
2018-11-08 21:20           ` Stanislav Fomichev
2018-11-08 21:20             ` Stanislav Fomichev
2018-11-08 21:20             ` sdf
2018-11-08 21:51             ` Jakub Kicinski
2018-11-08 21:51               ` Jakub Kicinski
2018-11-08 21:51               ` jakub.kicinski
2018-11-08 19:45     ` Jakub Kicinski
2018-11-08 19:45       ` Jakub Kicinski
2018-11-08 19:45       ` jakub.kicinski
2018-11-08 21:25       ` Stanislav Fomichev
2018-11-08 21:25         ` Stanislav Fomichev
2018-11-08 21:25         ` sdf
2018-11-08 21:52         ` Jakub Kicinski
2018-11-08 21:52           ` Jakub Kicinski
2018-11-08 21:52           ` jakub.kicinski
2018-11-08 19:46   ` Jakub Kicinski
2018-11-08 19:46     ` Jakub Kicinski
2018-11-08 19:46     ` jakub.kicinski
2018-11-08 21:29     ` Stanislav Fomichev [this message]
2018-11-08 21:29       ` Stanislav Fomichev
2018-11-08 21:29       ` sdf
2018-11-08 21:54       ` Jakub Kicinski
2018-11-08 21:54         ` Jakub Kicinski
2018-11-08 21:54         ` jakub.kicinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181108212940.wvfnkanxqs2owcy6@mini-arch \
    --to=sdf@fomichev.me \
    --cc=ast@kernel.org \
    --cc=bhole_prashant_q7@lab.ntt.co.jp \
    --cc=daniel@iogearbox.net \
    --cc=guro@fb.com \
    --cc=jakub.kicinski@netronome.com \
    --cc=jbenc@redhat.com \
    --cc=jiong.wang@netronome.com \
    --cc=john.fastabend@gmail.com \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=osk@fb.com \
    --cc=quentin.monnet@netronome.com \
    --cc=sandipan@linux.vnet.ibm.com \
    --cc=sdf@google.com \
    --cc=shuah@kernel.org \
    --cc=treeze.taeung@gmail.com \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.