From: Alexei Starovoitov <ast@plumgrid.com>
To: Cong Wang <cwang@twopensource.com>,
Daniel Borkmann <daniel@iogearbox.net>
Cc: David Miller <davem@davemloft.net>,
netdev <netdev@vger.kernel.org>, Jiri Pirko <jiri@resnulli.us>,
Jamal Hadi Salim <jhs@mojatatu.com>
Subject: Re: [PATCH net] act_bpf: allow non-default TC_ACT opcodes as BPF exec outcome
Date: Tue, 17 Mar 2015 15:16:12 -0700 [thread overview]
Message-ID: <5508A7AC.1090903@plumgrid.com> (raw)
In-Reply-To: <CAHA+R7PhYvs2hPgQ1Rj+UjWdASsQq759_8o8125-mvmOKJzWKA@mail.gmail.com>
On 3/17/15 2:53 PM, Cong Wang wrote:
> On Tue, Mar 17, 2015 at 12:25 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
>> Revisiting commit d23b8ad8ab23 ("tc: add BPF based action") with regards
>> to eBPF support, I was thinking that it might be better to improve
>> return semantics from a BPF program invoked through BPF_PROG_RUN().
>>
>> Currently, in case filter_res is 0, we overwrite the default action
>> opcode with TC_ACT_SHOT. A default action opcode configured through tc's
>> m_bpf can be: TC_ACT_RECLASSIFY, TC_ACT_PIPE, TC_ACT_SHOT, TC_ACT_UNSPEC,
>> TC_ACT_OK.
>>
>> In cls_bpf, we have the possibility to overwrite the default class
>> associated with the classifier in case filter_res is _not_ 0xffffffff
>> (-1).
>>
>> That allows us to fold multiple [e]BPF programs into a single one, where
>> they would otherwise need to be defined as a separate classifier with
>> its own classid, needlessly redoing parsing work, etc.
>>
>> Similarly, we could do better in act_bpf: Since above TC_ACT* opcodes
>> are exported to UAPI anyway, we reuse them for return-code-to-tc-opcode
>> mapping, where we would allow above possibilities. Thus, like in cls_bpf,
>> a filter_res of 0xffffffff (-1) means that the configured _default_ action
>> is used. Any unkown return code from the BPF program would fail in
>> tcf_bpf() with TC_ACT_UNSPEC.
>>
>
> So you allow bpf bytecode to override the action code specified
> in cmdline, I doubt this is user-friendly since if I run different
> bytecode I would see different behaviors even if I specify the same
> action in cmdline. Allowing cmdline to override bytecode makes
> more sense to me.
>
> A even cleaner solution is not to override either of them, that is
> either bytecode or cmdline fully controls the action code.
>
There was ambiguity in priority. Whether program decides
the action or default/fallback action taken from command line...
This patch is trying to make both of them useful by saying
that program result has higher priority than 'default/fallback' action.
To avoid priorities we can drop command line action and just
let program decide. But that is less flexible. I think default
action is useful too. The user can have one program that returns
act_unspec and fine tune it via command line by changing default action.
next prev parent reply other threads:[~2015-03-17 22:16 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-17 19:25 [PATCH net] act_bpf: allow non-default TC_ACT opcodes as BPF exec outcome Daniel Borkmann
2015-03-17 21:53 ` Cong Wang
2015-03-17 22:16 ` Alexei Starovoitov [this message]
2015-03-17 22:27 ` Jiri Pirko
2015-03-18 2:15 ` David Miller
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=5508A7AC.1090903@plumgrid.com \
--to=ast@plumgrid.com \
--cc=cwang@twopensource.com \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=jhs@mojatatu.com \
--cc=jiri@resnulli.us \
--cc=netdev@vger.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 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).