All of lore.kernel.org
 help / color / mirror / Atom feed
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.

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