netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexei Starovoitov <ast@fb.com>
To: David Ahern <dsa@cumulusnetworks.com>,
	Thomas Graf <tgraf@suug.ch>, "Daniel Mack" <daniel@zonque.org>
Cc: David Miller <davem@davemloft.net>, <netdev@vger.kernel.org>,
	<daniel@iogearbox.net>, <maheshb@google.com>
Subject: Re: [PATCH v2 net-next 1/5] bpf: Refactor cgroups code in prep for new type
Date: Mon, 14 Nov 2016 18:23:17 -0800	[thread overview]
Message-ID: <582A7195.60101@fb.com> (raw)
In-Reply-To: <65b46edc-02bd-415c-4b1d-af36928d22da@cumulusnetworks.com>

On 11/13/16 7:51 PM, David Ahern wrote:
> On 10/31/16 11:49 AM, Thomas Graf wrote:
>> On 10/31/16 at 06:16pm, Daniel Mack wrote:
>>> On 10/31/2016 06:05 PM, David Ahern wrote:
>>>> On 10/31/16 11:00 AM, Daniel Mack wrote:
>>>>> Yeah, I'm confused too. I changed that name in my v7 from
>>>>> BPF_PROG_TYPE_CGROUP_SOCK to BPF_PROG_TYPE_CGROUP_SKB on David's
>>>>> (Ahern) request. Why is it now renamed again?
>>>>
>>>> Thomas pushed back on adding another program type in favor of using
>>>> subtypes. So this makes the program type generic to CGROUP and patch
>>>> 2 in this v2 set added Mickaël's subtype patch with the socket
>>>> mangling done that way in patch 3.
>>>>
>>>
>>> Fine for me. I can change it around again.
>>
>> I would like to hear from Daniel B and Alexei as well. We need to
>> decide whether to use subtypes consistently and treat prog types as
>> something more high level or whether to bluntly introduce a new prog
>> type for every distinct set of verifier limits. I will change lwt_bpf
>> as well accordingly.
>>
>
> Alexei / Daniel - any comments/preferences on subtypes vs program types?

looks like in this particular case it's better to use different program
types, since they all serve different purpose and context is different.
Daniel Mack's programs can stay BPF_PROG_TYPE_CGROUP_SKB and operate on 
skb, whereas your ifindex changing programs can be 
BPF_PROG_TYPE_CGROUP_SOCK.

regarding DanielM's patches.. Dave's comment regarding skb->sk vs sk
made us think hard about it. We looked into tunnels and at the end
realized that skb->sk won't fly, since the program will be called
multiple times on tx for every ip_output. And since we were not sure
that using 'sk' will fix it, we had to do a bunch of tests with
different tunnels and analyze the code to make sure it's all good.
We'll repost soon.

  reply	other threads:[~2016-11-15  2:23 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-27  0:58 [PATCH v2 net-next 0/5] Add bpf support to set sk_bound_dev_if David Ahern
2016-10-27  0:58 ` [PATCH v2 net-next 1/5] bpf: Refactor cgroups code in prep for new type David Ahern
2016-10-31 16:58   ` David Miller
2016-10-31 17:00     ` Daniel Mack
2016-10-31 17:05       ` David Ahern
2016-10-31 17:16         ` Daniel Mack
2016-10-31 17:49           ` Thomas Graf
2016-11-14  3:51             ` David Ahern
2016-11-15  2:23               ` Alexei Starovoitov [this message]
2016-10-27  0:58 ` [PATCH net-next 2/5] bpf: Add eBPF program subtype and is_valid_subtype() verifier David Ahern
2016-10-27  0:58 ` [PATCH v2 net-next 3/5] bpf: Add new cgroup attach type to enable sock modifications David Ahern
2016-10-27  0:58 ` [PATCH net-next 4/5] samples: bpf: Add prog_subtype to bpf_prog_load David Ahern
2016-10-27  0:58 ` [PATCH v2 net-next 5/5] samples: bpf: add userspace example for modifying sk_bound_dev_if David Ahern
2016-10-31 17:01 ` [PATCH v2 net-next 0/5] Add bpf support to set sk_bound_dev_if David Miller
2016-10-31 17:16   ` David Ahern
2016-10-31 17:46     ` Thomas Graf

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=582A7195.60101@fb.com \
    --to=ast@fb.com \
    --cc=daniel@iogearbox.net \
    --cc=daniel@zonque.org \
    --cc=davem@davemloft.net \
    --cc=dsa@cumulusnetworks.com \
    --cc=maheshb@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=tgraf@suug.ch \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).