netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Borkmann <dborkman@redhat.com>
To: Chema Gonzalez <chema@google.com>
Cc: Alexei Starovoitov <ast@plumgrid.com>,
	David Miller <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Network Development <netdev@vger.kernel.org>
Subject: Re: [PATCH v5 net-next 1/3] net: flow_dissector: avoid multiple calls in BPF
Date: Tue, 20 May 2014 11:58:59 +0200	[thread overview]
Message-ID: <537B2763.5090402@redhat.com> (raw)
In-Reply-To: <CA+ZOOTPmSPeBrSHAsJtyxkAUnZDpgdhhw2PkrCJ3LhmG+ZSRYA@mail.gmail.com>

On 05/20/2014 12:23 AM, Chema Gonzalez wrote:
> On Fri, May 16, 2014 at 3:00 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
...
>> Other than design issue there is technical problem too.
>> You're not updating eBPF JIT and context->flow_initted will have junk,
>> so ld_poff will be returning junk too.
>> JITs and interpreter must match. That was always the case even in
>> old days of classic BPF. Interpreter initialized A and X to zero,
>> JITs had to do the same. If you add stuff like this to interpreter,
>> JITs would have to follow. Fast forward few years and this is not scalable.
>> You have several JITs around and adding custom stuff to interpreter
>> may not even be possible to do in JITs.
> This is a fair point. In fact my patch is breaking "ld poff."
>
> I think the previous mechanism where a new insn added to BPF would
> cause the BPF JIT engine to refuse to run was a more scalable: New
> stuff always went in the non-jitted runner, and people interested in
> the jitted version will port when they needed it. We definitely not
> want to require people adding new features to provide 6 versions of
> the code.

Absolutely, "we definitely not want to require people adding new features
to provide 6 versions of the code". I agree with this statement and that
is exactly one of the goals of the new BPF mechanism. You might have seen
how the internal conversion function performs this task for all current
BPF extensions, and that is exactly so that JITs, once they are in place,
do not need to be upgraded 6 times anymore (in contrast to your previous
statement -- as people who want to use one of the extensions, needed to
port them to each of their arch-specific JITs before).

Best,

Daniel

  reply	other threads:[~2014-05-20  9:59 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-30 18:29 [PATCH] net: filter: add insn for loading internal transport header offset Chema Gonzalez
2014-04-30 22:21 ` Alexei Starovoitov
2014-05-01 10:53 ` Daniel Borkmann
2014-05-01 10:55   ` Daniel Borkmann
2014-05-01 18:44     ` Chema Gonzalez
2014-05-01 18:44 ` [PATCH net-next v2] " Chema Gonzalez
2014-05-02 16:21   ` Ben Hutchings
2014-05-02 21:49   ` David Miller
2014-05-03  0:53     ` Chema Gonzalez
2014-05-03  2:52       ` David Miller
2014-05-05 18:42         ` Chema Gonzalez
2014-05-05 19:12           ` David Miller
2014-05-14 18:42             ` Chema Gonzalez
2014-05-14 18:51               ` Chema Gonzalez
2014-05-14 18:42 ` [PATCH v2 net-next 1/3] net: flow_dissector: avoid multiple calls in BPF Chema Gonzalez
2014-05-14 18:42   ` [PATCH v2 net-next 2/3] net: filter: add insn for loading internal transport header offset Chema Gonzalez
2014-05-14 18:42   ` [PATCH v2 net-next 3/3] net: filter: add insn for loading internal transport header proto Chema Gonzalez
2014-05-14 18:51 ` [PATCH v3 net-next 1/3] net: flow_dissector: avoid multiple calls in BPF Chema Gonzalez
2014-05-14 20:05   ` Alexei Starovoitov
2014-05-14 21:51     ` Chema Gonzalez
2014-05-14 22:44       ` Alexei Starovoitov
2014-05-16 18:41         ` Chema Gonzalez
2014-05-14 18:51 ` [PATCH v3 net-next 2/3] net: filter: add insn for loading internal transport header offset Chema Gonzalez
2014-05-14 18:51 ` [PATCH v3 net-next 3/3] net: filter: add insn for loading internal transport header proto Chema Gonzalez
2014-05-16 18:41 ` [PATCH v5 net-next 1/3] net: flow_dissector: avoid multiple calls in BPF Chema Gonzalez
2014-05-16 22:00   ` Alexei Starovoitov
2014-05-19 22:23     ` Chema Gonzalez
2014-05-20  9:58       ` Daniel Borkmann [this message]
2014-05-16 18:41 ` [PATCH v5 net-next 2/3] net: filter: add insn for loading internal transport header offset Chema Gonzalez
2014-05-16 18:41 ` [PATCH v5 net-next 3/3] net: filter: add insn for loading internal transport header proto Chema Gonzalez
2014-05-29 18:55 ` [PATCH v6 net-next 1/4] net: flow_dissector: avoid multiple calls in eBPF Chema Gonzalez
2014-05-29 23:54   ` Daniel Borkmann
2014-05-30 17:12     ` Chema Gonzalez
2014-06-02 12:36       ` Daniel Borkmann
2014-06-02 16:48         ` Alexei Starovoitov
2014-06-03  8:33           ` Daniel Borkmann
2014-06-03 20:15             ` Alexei Starovoitov
2014-06-03 21:12               ` Chema Gonzalez
2014-06-04  8:51                 ` Daniel Borkmann
2014-06-05  6:55                   ` David Miller
2014-06-20 21:56                   ` Chema Gonzalez
2014-06-24  8:14                     ` Daniel Borkmann
2014-06-25 22:00                       ` Chema Gonzalez
2014-06-27 10:19                         ` Daniel Borkmann
2014-05-29 18:56 ` [PATCH v6 net-next 2/4] net: filter: add insn for loading internal transport header offset Chema Gonzalez
2014-05-29 18:56 ` [PATCH v6 net-next 3/4] net: filter: add insn for loading internal transport header proto Chema Gonzalez
2014-05-29 18:56 ` [PATCH v6 net-next 4/4] net: filter: minor BPF cleanups Chema Gonzalez

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=537B2763.5090402@redhat.com \
    --to=dborkman@redhat.com \
    --cc=ast@plumgrid.com \
    --cc=chema@google.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --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).