All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Borkmann <daniel@iogearbox.net>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Alexei Starovoitov <ast@fb.com>,
	"David S . Miller" <davem@davemloft.net>,
	David Ahern <dsa@cumulusnetworks.com>, Tejun Heo <tj@kernel.org>,
	Andy Lutomirski <luto@amacapital.net>,
	"Eric W . Biederman" <ebiederm@xmission.com>,
	Thomas Graf <tgraf@suug.ch>,
	netdev@vger.kernel.org
Subject: Re: [PATCH net] bpf: expose netns inode to bpf programs
Date: Sat, 04 Feb 2017 00:42:31 +0100	[thread overview]
Message-ID: <58951567.1030401@iogearbox.net> (raw)
In-Reply-To: <20170203230627.GB26227@ast-mbp.thefacebook.com>

On 02/04/2017 12:06 AM, Alexei Starovoitov wrote:
> On Fri, Feb 03, 2017 at 10:56:43PM +0100, Daniel Borkmann wrote:
>> On 01/26/2017 04:27 AM, Alexei Starovoitov wrote:
>>> in cases where bpf programs are looking at sockets and packets
>>> that belong to different netns, it could be useful to read netns inode,
>>> so that programs can make intelligent decisions.
>>> For example to disallow raw sockets in all non-init netns the program can do:
>>> if (sk->type == SOCK_RAW && sk->netns_inum != 0xf0000075)
>>>    return 0;
>>> where 0xf0000075 inode comes from /proc/pid/ns/net
>>>
>>> Similarly TC cls_bpf/act_bpf and socket filters can do
>>> if (skb->netns_inum == expected_inode)
>>>
>>> The lack of netns awareness was a concern even for socket filters,
>>> since the application can attach the same bpf program to sockets
>>> in a different netns. Just like tc cls_bpf program can work in
>>> different netns as well, so it has to be addressed uniformly
>>> across all types of bpf programs.
>>
>> Sorry for jumping in late, but my question is, isn't this helper
>> really only relevant for BPF_PROG_TYPE_CGROUP_* typed programs?
>> Thus other prog types making use of bpf_convert_ctx_access()
>> should probably reject that in .is_valid_access() callback?
>>
>> Reason why I'm asking is that for sockets or tc progs, you
>> already have a netns context where you're attached to, and f.e.
>> skbs leaving that netns context will be orphaned. Thus, why
>> would tc or sock filter tailor a program with such a check,
>> if it can only match/mismatch its own netns inum eventually?
>
> Please see the example I provided earlier.

That example for both socket filter and tc progs specifically
wasn't quite clear to me, hence my question wrt why it's right
now a "concern" for these ones. (Again, clear to me for cgroups
progs.)

> We can have the same cls_bpf attached to all netns-es.
> Same for socket filters and everything else.

So use-case would be that someone wants to attach the very same
prog via tc to various netdevs sitting in different netns, and
that prog looks up a map, controlled by initns, with skb->netns_inum
as key and the resulting value could contain allowed feature bits
for that specific netns prog the skbs goes through? That would be
a feature, not "concern", no? At the same time, it's up to the
user or mgmt app what gets loaded so f.e. it might just as well
tailor/optimize the progs individually for the devs sitting in
netns-es to avoid such map lookup.

> All bpf programs are global.

True, but for socket filter and tc they are hooked/attached under
a given netns context.

> They can all share info via maps and so on.

>> When making this effort to lookup and hardcode the dev/inode
>> num into the prog, wouldn't it be easier for these types if
>
> we cannot hardcode dev/inode. They are dynamic and depends
> where program runs.

Was referring to the test from above provided example:

 >>> if (skb->netns_inum == expected_inode)

> I'll send a patch shortly that exposes both.

Thanks,
Daniel

  reply	other threads:[~2017-02-03 23:42 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-26  3:27 [PATCH net] bpf: expose netns inode to bpf programs Alexei Starovoitov
2017-01-26  5:46 ` Eric W. Biederman
2017-01-26  6:00   ` Ying Xue
2017-01-26  6:23   ` Alexei Starovoitov
2017-01-26 16:37     ` Andy Lutomirski
2017-01-26 17:46       ` Alexei Starovoitov
2017-01-26 18:12         ` Andy Lutomirski
2017-01-26 18:32           ` Alexei Starovoitov
2017-01-26 19:07             ` Andy Lutomirski
2017-01-26 19:25               ` Alexei Starovoitov
2017-02-03  4:33                 ` Eric W. Biederman
2017-02-03  6:05                   ` Alexei Starovoitov
2017-02-03 10:30                     ` Eric W. Biederman
2017-02-03 21:00                   ` Andy Lutomirski
2017-02-03 21:06                     ` Eric W. Biederman
2017-02-03 23:08                     ` Alexei Starovoitov
2017-02-04 17:07                       ` Andy Lutomirski
2017-02-05  3:10                         ` Alexei Starovoitov
2017-02-05  3:27                           ` Andy Lutomirski
2017-02-05  3:48                             ` Alexei Starovoitov
2017-02-05  3:54                               ` Andy Lutomirski
2017-02-05  4:37                                 ` Alexei Starovoitov
2017-02-05  5:05                                   ` Andy Lutomirski
2017-02-07  1:43                                     ` Alexei Starovoitov
2017-01-31 18:02 ` David Miller
2017-01-31 22:11 ` David Ahern
2017-02-03 21:56 ` Daniel Borkmann
2017-02-03 23:06   ` Alexei Starovoitov
2017-02-03 23:42     ` Daniel Borkmann [this message]
2017-02-04  1:25       ` Alexei Starovoitov
2017-02-04 17:08       ` Andy Lutomirski
2017-02-05  3:18         ` Alexei Starovoitov
2017-02-05  3:22           ` Andy Lutomirski
2017-02-05  3:35             ` Alexei Starovoitov
2017-02-05  3:49               ` Andy Lutomirski

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=58951567.1030401@iogearbox.net \
    --to=daniel@iogearbox.net \
    --cc=alexei.starovoitov@gmail.com \
    --cc=ast@fb.com \
    --cc=davem@davemloft.net \
    --cc=dsa@cumulusnetworks.com \
    --cc=ebiederm@xmission.com \
    --cc=luto@amacapital.net \
    --cc=netdev@vger.kernel.org \
    --cc=tgraf@suug.ch \
    --cc=tj@kernel.org \
    /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.