bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: John Fastabend <john.fastabend@gmail.com>
To: "Toke Høiland-Jørgensen" <toke@redhat.com>,
	"John Fastabend" <john.fastabend@gmail.com>,
	"Alan Maguire" <alan.maguire@oracle.com>
Cc: 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>,
	David Miller <davem@davemloft.net>,
	Jesper Dangaard Brouer <brouer@redhat.com>,
	netdev@vger.kernel.org, bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next 0/9] xdp: Support multiple programs on a single interface through chain calls
Date: Wed, 02 Oct 2019 13:34:38 -0700	[thread overview]
Message-ID: <5d9509de4acb6_32c02ab4bb3b05c052@john-XPS-13-9370.notmuch> (raw)
In-Reply-To: <8736gbro8x.fsf@toke.dk>

Toke Høiland-Jørgensen wrote:
> John Fastabend <john.fastabend@gmail.com> writes:
> 
> > Toke Høiland-Jørgensen wrote:
> >> Alan Maguire <alan.maguire@oracle.com> writes:
> >> 
> >> > On Wed, 2 Oct 2019, Toke Høiland-Jørgensen wrote:
> >> >
> >> >> This series adds support for executing multiple XDP programs on a single
> >> >> interface in sequence, through the use of chain calls, as discussed at the Linux
> >> >> Plumbers Conference last month:
> >> >> 
> >> >> https://linuxplumbersconf.org/event/4/contributions/460/
> >> >> 
> >> >> # HIGH-LEVEL IDEA
> >> >> 
> >> >> The basic idea is to express the chain call sequence through a special map type,
> >> >> which contains a mapping from a (program, return code) tuple to another program
> >> >> to run in next in the sequence. Userspace can populate this map to express
> >> >> arbitrary call sequences, and update the sequence by updating or replacing the
> >> >> map.
> >> >> 
> >> >> The actual execution of the program sequence is done in bpf_prog_run_xdp(),
> >> >> which will lookup the chain sequence map, and if found, will loop through calls
> >> >> to BPF_PROG_RUN, looking up the next XDP program in the sequence based on the
> >> >> previous program ID and return code.
> >> >> 
> >> >> An XDP chain call map can be installed on an interface by means of a new netlink
> >> >> attribute containing an fd pointing to a chain call map. This can be supplied
> >> >> along with the XDP prog fd, so that a chain map is always installed together
> >> >> with an XDP program.
> >> >> 
> >> >
> >> > This is great stuff Toke!
> >> 
> >> Thanks! :)
> >> 
> >> > One thing that wasn't immediately clear to me - and this may be just
> >> > me - is the relationship between program behaviour for the XDP_DROP
> >> > case and chain call execution. My initial thought was that a program
> >> > in the chain XDP_DROP'ping the packet would terminate the call chain,
> >> > but on looking at patch #4 it seems that the only way the call chain
> >> > execution is terminated is if
> >> >
> >> > - XDP_ABORTED is returned from a program in the call chain; or
> >> 
> >> Yes. Not actually sure about this one...
> >> 
> >> > - the map entry for the next program (determined by the return value
> >> >   of the current program) is empty; or
> >> 
> >> This will be the common exit condition, I expect
> >> 
> >> > - we run out of entries in the map
> >> 
> >> You mean if we run the iteration counter to zero, right?
> >> 
> >> > The return value of the last-executed program in the chain seems to be
> >> > what determines packet processing behaviour after executing the chain
> >> > (_DROP, _TX, _PASS, etc). So there's no way to both XDP_PASS and
> >> > XDP_TX a packet from the same chain, right? Just want to make sure
> >> > I've got the semantics correct. Thanks!
> >> 
> >> Yeah, you've got all this right. The chain call mechanism itself doesn't
> >> change any of the underlying fundamentals of XDP. I.e., each packet gets
> >> exactly one verdict.
> >> 
> >> For chaining actual XDP programs that do different things to the packet,
> >> I expect that the most common use case will be to only run the next
> >> program if the previous one returns XDP_PASS. That will make the most
> >> semantic sense I think.
> >> 
> >> But there are also use cases where one would want to match on the other
> >> return codes; such as packet capture, for instance, where one might
> >> install a capture program that would carry forward the previous return
> >> code, but do something to the packet (throw it out to userspace) first.
> >> 
> >> For the latter use case, the question is if we need to expose the
> >> previous return code to the program when it runs. You can do things
> >> without it (by just using a different program per return code), but it
> >> may simplify things if we just expose the return code. However, since
> >> this will also change the semantics for running programs, I decided to
> >> leave that off for now.
> >> 
> >> -Toke
> >
> > In other cases where programs (e.g. cgroups) are run in an array the
> > return codes are 'AND'ed together so that we get
> >
> >    result1 & result2 & ... & resultN
> 
> How would that work with multiple programs, though? PASS -> DROP seems
> obvious, but what if the first program returns TX? Also, programs may
> want to be able to actually override return codes (e.g., say you want to
> turn DROPs into REDIRECTs, to get all your dropped packets mirrored to
> your IDS or something).

In general I think either you hard code a precedence that will have to
be overly conservative because if one program (your firewall) tells XDP
to drop the packet and some other program redirects it, passes, etc. that
seems incorrect to me. Or you get creative with the precedence rules and
they become complex and difficult to manage, where a drop will drop a packet
unless a previous/preceding program redirects it, etc. I think any hard
coded precedence you come up with will make some one happy and some other
user annoyed. Defeating the programability of BPF.

Better if its programmable. I would prefer to pass the context into the
next program then programs can build their own semantics. Then leave
the & of return codes so any program can if needed really drop a packet.
The context could be pushed into a shared memory region and then it
doesn't even need to be part of the program signature.

.John

  reply	other threads:[~2019-10-02 20:34 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-02 13:30 [PATCH bpf-next 0/9] xdp: Support multiple programs on a single interface through chain calls Toke Høiland-Jørgensen
2019-10-02 13:30 ` [PATCH bpf-next 1/9] hashtab: Add new bpf_map_fd_put_value op Toke Høiland-Jørgensen
2019-10-02 13:30 ` [PATCH bpf-next 2/9] xdp: Add new xdp_chain_map type for specifying XDP call sequences Toke Høiland-Jørgensen
2019-10-02 15:50   ` Lorenz Bauer
2019-10-02 18:25     ` Toke Høiland-Jørgensen
2019-10-02 13:30 ` [PATCH bpf-next 3/9] xdp: Support setting and getting device chain map Toke Høiland-Jørgensen
2019-10-02 15:50   ` Lorenz Bauer
2019-10-02 18:32     ` Toke Høiland-Jørgensen
2019-10-02 18:07   ` kbuild test robot
2019-10-02 18:29   ` kbuild test robot
2019-10-02 13:30 ` [PATCH bpf-next 4/9] xdp: Implement chain call logic to support multiple programs on one interface Toke Høiland-Jørgensen
2019-10-02 17:33   ` kbuild test robot
2019-10-02 17:53   ` kbuild test robot
2019-10-02 13:30 ` [PATCH bpf-next 5/9] tools/include/uapi: Add XDP chain map definitions Toke Høiland-Jørgensen
2019-10-02 13:30 ` [PATCH bpf-next 6/9] tools/libbpf_probes: Add support for xdp_chain map type Toke Høiland-Jørgensen
2019-10-02 13:30 ` [PATCH bpf-next 7/9] bpftool: Add definitions " Toke Høiland-Jørgensen
2019-10-02 13:30 ` [PATCH bpf-next 8/9] libbpf: Add support for setting and getting XDP chain maps Toke Høiland-Jørgensen
2019-10-02 13:30 ` [PATCH bpf-next 9/9] selftests: Add tests for XDP chain calls Toke Høiland-Jørgensen
2019-10-02 15:10 ` [PATCH bpf-next 0/9] xdp: Support multiple programs on a single interface through " Alan Maguire
2019-10-02 15:33   ` Toke Høiland-Jørgensen
2019-10-02 16:34     ` John Fastabend
2019-10-02 18:33       ` Toke Høiland-Jørgensen
2019-10-02 20:34         ` John Fastabend [this message]
2019-10-03  7:48           ` Toke Høiland-Jørgensen
2019-10-03 10:09             ` Jesper Dangaard Brouer
2019-10-03 19:45               ` John Fastabend
2019-10-02 16:35 ` Lorenz Bauer
2019-10-02 18:54   ` Toke Høiland-Jørgensen
2019-10-02 16:43 ` John Fastabend
2019-10-02 19:09   ` Toke Høiland-Jørgensen
2019-10-02 19:15   ` Daniel Borkmann
2019-10-02 19:29     ` Toke Høiland-Jørgensen
2019-10-02 19:46     ` Alexei Starovoitov
2019-10-03  7:58       ` Toke Høiland-Jørgensen
2019-10-02 18:38 ` Song Liu
2019-10-02 18:54   ` Song Liu
2019-10-02 19:25     ` Toke Høiland-Jørgensen
2019-10-03  8:53       ` Jesper Dangaard Brouer
2019-10-03 14:03         ` Alexei Starovoitov
2019-10-03 14:33           ` Toke Høiland-Jørgensen
2019-10-03 14:53             ` Edward Cree
2019-10-03 18:49               ` Jesper Dangaard Brouer
2019-10-03 19:35               ` John Fastabend
2019-10-04  8:09                 ` Toke Høiland-Jørgensen
2019-10-04 10:34                   ` Edward Cree
2019-10-04 15:58                     ` Lorenz Bauer
2019-10-07 16:43                       ` Edward Cree
2019-10-07 17:12                         ` Lorenz Bauer
2019-10-07 19:21                           ` Edward Cree
2019-10-07 21:01                         ` Alexei Starovoitov
2019-10-02 19:23   ` Toke Høiland-Jørgensen
2019-10-02 19:49     ` Song Liu
2019-10-03  7:59       ` 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=5d9509de4acb6_32c02ab4bb3b05c052@john-XPS-13-9370.notmuch \
    --to=john.fastabend@gmail.com \
    --cc=alan.maguire@oracle.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=brouer@redhat.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --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 \
    /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).