bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: John Fastabend <john.fastabend@gmail.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Edward Cree <ecree@solarflare.com>
Cc: "Toke Høiland-Jørgensen" <toke@redhat.com>,
	"John Fastabend" <john.fastabend@gmail.com>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Martin KaFai Lau" <kafai@fb.com>,
	"Song Liu" <songliubraving@fb.com>, "Yonghong Song" <yhs@fb.com>,
	"Marek Majkowski" <marek@cloudflare.com>,
	"Lorenz Bauer" <lmb@cloudflare.com>,
	"Alan Maguire" <alan.maguire@oracle.com>,
	"Jesper Dangaard Brouer" <brouer@redhat.com>,
	"David Miller" <davem@davemloft.net>,
	netdev@vger.kernel.org, bpf@vger.kernel.org
Subject: Re: static and dynamic linking. Was: [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF
Date: Fri, 15 Nov 2019 08:56:27 -0800	[thread overview]
Message-ID: <5dced8bbe72ed_5f462b1f489665bc31@john-XPS-13-9370.notmuch> (raw)
In-Reply-To: <20191115021318.sj5zfokruljugcno@ast-mbp.dhcp.thefacebook.com>

Alexei Starovoitov wrote:
> On Wed, Nov 13, 2019 at 06:30:04PM +0000, Edward Cree wrote:
> > > There is also
> > > no way to place extern into a section. Currently SEC("..") is a standard way to
> > > annotate bpf programs.
> > While the symbol itself doesn't have a section, each _use_ of the symbol has a
> >  reloc, and the SHT_REL[A] in which that reloc resides has a sh_info specifying
> >  "the section header index of the section to which the relocation applies."  So
> >  can't that be used if symbol visibility needs to depend on section?  Tbh I
> >  can't exactly see why externs need placing in a section in the first place.
> 
> I think section for extern can give a scope of search and make libbpf decisions
> more predictable? May be we can live without it for now, but we need BTF of
> extern symbols. See my example in reply to John.
> 
> > > I think we need to be able to specify something like section to
> > > extern variables and functions.
> > It seems unnecessary to have the user code specify this.  Another a bad
> >  analogy: in userland C code you don't have to annotate the function protos in
> >  your header files to say whether they come from another .o file, a random
> >  library or the libc.  You just declare "a function called this exists somewhere
> >  and we'll find it at link time".
> 
> yeah. good analogy.
> 
> > > I was imagining that the verifier will do per-function verification
> > > of program with sub-programs instead of analyzing from root.
> > Ah I see.  Yes, that's a very attractive design.
> > 
> > If we make it from a sufficiently generic idea of pre/postconditions, then it
> >  could also be useful for e.g. loop bodies (user-supplied annotations that allow
> >  us to walk the body only once instead of N times); then a function call just
> >  gets standard pre/postconditions generated from its argument types if the user
> >  didn't specify something else.
> 
> regarding pre/post conditions.
> I think we have them already. These conditions are the function prototypes.
> Instead of making the verifier figuring the conditions it's simpler to use
> function prototypes instead. If program author is saying that argument to the
> function is 'struct xpd_md *' the verifier will check that the function is safe
> when such pointer is passed into it. Then to verify the callsite the verifier
> only need to check that what is passed into such function matches the type. I
> think it's easy to see when such type is context. Like 'struct __sk_buff *'.
> But the idea applies to pointer to int too. I believe you were arguing that
> instead of tnum_unknown there could be cases with tnum_range(0-2) as
> pre-condition is useful. May be. I think it will simplify the verifier logic
> quite a bit if we avoid going fine grain.

I was thinking we may want tnum_range(0-2) conditions and also min/max lengths
of arrays, buffers etc. otherwise it might be tricky to convince the verifier
its safe to write into an array. I at least am already hitting some limits here
with helper calls that I would like to resolve at some point. We had to use
some inline 'asm goto' logic to convince clang to generate code that the
verifier would accept. Perhaps some of this can be resolved on LLVM backend
side to insert checks as needed. Also adding compiler barriers and if/else
bounds everywhere is a bit natural.  I expect post conditions will
be important for same reason, returning a pointer into a buffer without
bounds is going to make it difficult to use in the caller program. 

But(!) lets walk first implementing BTF based conditions and linking is
a huge step and doesn't preclude doing more advance things next.

> Say we have a function:
> int foo(struct __sk_buff *skb, int arg)
> {
>    if (arg > 2)
>       return 0;
>    // do safe stuff with skb depending whether arg is 0, 1, or 2.
> }
> That first 'if' is enough to turn pre-conditions into 'any skb' and 'any arg'.
> That is exactly what BTF says about this function.
> 

But this would probably break,

 char *foo(struct __Sk_buff *skb, char *buffer)
 {
      int i;

      for i = 0; i < BUFFER_LENGTH; i++) {
             buffer[i] = blah
      }
      return buffer[i]
 }

Lets deal with that later though.

  reply	other threads:[~2019-11-15 16:56 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-07 17:20 [PATCH bpf-next v3 0/5] xdp: Support multiple programs on a single interface through chain calls Toke Høiland-Jørgensen
2019-10-07 17:20 ` [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other Toke Høiland-Jørgensen
2019-10-07 20:42   ` Alexei Starovoitov
2019-10-08  8:07     ` Toke Høiland-Jørgensen
2019-10-09  1:51       ` Alexei Starovoitov
2019-10-09  8:03         ` Toke Høiland-Jørgensen
2019-10-10  4:41           ` Alexei Starovoitov
2019-10-14 12:35             ` Toke Høiland-Jørgensen
2019-10-14 17:08               ` John Fastabend
2019-10-14 18:48                 ` Toke Høiland-Jørgensen
2019-10-15 16:30                   ` Edward Cree
2019-10-15 16:42                     ` Toke Høiland-Jørgensen
2019-10-15 18:33                       ` Edward Cree
2019-10-17 12:11                         ` Toke Høiland-Jørgensen
2019-10-22 17:27                           ` Edward Cree
2019-10-22 18:07                             ` Toke Høiland-Jørgensen
2019-11-12  2:51                               ` static and dynamic linking. Was: [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF Alexei Starovoitov
2019-11-12 16:20                                 ` Toke Høiland-Jørgensen
2019-11-12 19:52                                   ` Alexei Starovoitov
2019-11-12 21:25                                     ` Edward Cree
2019-11-12 23:18                                       ` Alexei Starovoitov
2019-11-13 18:30                                         ` Edward Cree
2019-11-13 18:51                                           ` Andrii Nakryiko
2019-11-15  2:13                                           ` Alexei Starovoitov
2019-11-15 16:56                                             ` John Fastabend [this message]
2019-11-12 23:25                                     ` John Fastabend
2019-11-13  0:21                                       ` Alexei Starovoitov
2019-11-13  5:33                                         ` John Fastabend
2019-11-15  1:50                                           ` Alexei Starovoitov
2019-11-15 16:39                                             ` John Fastabend
2019-11-14 15:41                                     ` Toke Høiland-Jørgensen
2019-11-12 16:32                                 ` Edward Cree
2019-11-15 11:48                                 ` Lorenz Bauer
2019-11-15 23:02                                   ` Alexei Starovoitov
2019-11-18 13:29                                     ` Lorenz Bauer
2019-10-21 23:51                         ` [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other Edward Cree
2019-10-16  2:28               ` Alexei Starovoitov
2019-10-16  8:27                 ` Jesper Dangaard Brouer
2019-10-16 10:35                   ` Daniel Borkmann
2019-10-16 11:16                     ` Toke Høiland-Jørgensen
2019-10-16 13:51                 ` Toke Høiland-Jørgensen
2019-10-19 20:09                   ` bpf indirect calls Alexei Starovoitov
2019-10-20 10:58                     ` Toke Høiland-Jørgensen
2019-10-25 16:30                       ` Alexei Starovoitov
2019-10-27 12:15                         ` Toke Høiland-Jørgensen
2019-10-09 10:19         ` [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF programs after each other Jesper Dangaard Brouer
2019-10-09 17:57           ` Alexei Starovoitov
2019-10-07 17:20 ` [PATCH bpf-next v3 2/5] bpf: Add support for setting chain call sequence for programs Toke Høiland-Jørgensen
2019-10-07 20:38   ` Daniel Borkmann
2019-10-08  8:09     ` Toke Høiland-Jørgensen
2019-10-07 17:20 ` [PATCH bpf-next v3 3/5] tools: Update bpf.h header for program chain calls Toke Høiland-Jørgensen
2019-10-07 17:20 ` [PATCH bpf-next v3 4/5] libbpf: Add syscall wrappers for BPF_PROG_CHAIN_* commands Toke Høiland-Jørgensen
2019-10-07 17:20 ` [PATCH bpf-next v3 5/5] selftests: Add tests for XDP chain calls Toke Høiland-Jørgensen
2019-10-07 18:58 ` [PATCH bpf-next v3 0/5] xdp: Support multiple programs on a single interface through " John Fastabend
2019-10-08  8:42   ` Toke Høiland-Jørgensen

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=5dced8bbe72ed_5f462b1f489665bc31@john-XPS-13-9370.notmuch \
    --to=john.fastabend@gmail.com \
    --cc=alan.maguire@oracle.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=brouer@redhat.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=ecree@solarflare.com \
    --cc=kafai@fb.com \
    --cc=lmb@cloudflare.com \
    --cc=marek@cloudflare.com \
    --cc=netdev@vger.kernel.org \
    --cc=songliubraving@fb.com \
    --cc=toke@redhat.com \
    --cc=yhs@fb.com \
    --subject='Re: static and dynamic linking. Was: [PATCH bpf-next v3 1/5] bpf: Support chain calling multiple BPF' \
    /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

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).