All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: John Fastabend <john.fastabend@gmail.com>,
	netdev@vger.kernel.org, davem@davemloft.net, jiri@resnulli.us,
	horms@verge.net.au
Subject: Re: [PATCH RFC 0/3] intermediate representation for jit and cls_u32 conversion
Date: Fri, 26 Feb 2016 10:53:00 -0800	[thread overview]
Message-ID: <20160226185258.GA51957@ast-mbp.thefacebook.com> (raw)
In-Reply-To: <20160226161948.GA4667@salvia>

On Fri, Feb 26, 2016 at 05:19:48PM +0100, Pablo Neira Ayuso wrote:
> 
> Good, I'm all for reaching those numbers, we can optimize the generic
> IR if this ever becomes the bottleneck.

The 'generic IR' got mentioned hundred times in this thread,
but what was proposed is not generic. It doesn't even
fully fit u32. Here is why:

> This structure contains a protocol description (defined by struct
> net_ir_proto_desc) that is the initial node of the protocol graph that
> describes the protocol translation. This initial node starts from lower
> supported layer as base (eg. link-layer) then describing the upper
> protocols up to the transport protocols through the following structure:
> 
>  struct net_ir_proto_desc {
>        enum net_ir_payload_bases               base;
>        u32                                     protonum;
>        int                                     (*jit)(struct net_ir_jit_ctx *ctx,
>                                                       const struct net_ir_expr *expr,
>                                                       void *data);
>        const struct net_ir_proto_desc          *protocols[];
>  };

The above representation has builtin concept of protocols, whereas
u32 is protocol agnostic and fits this particular intel nic better.

>  struct net_ir_jit_desc {
>        enum net_ir_payload_bases               base;
>        const struct net_ir_proto_desc          *proto_desc;
>        int                                     (*verdict)(struct net_ir_jit_ctx *ctx,
>                                                           enum net_ir_stmt_verdict verdict,
>                                                           void *data);
>  };

imo the above is a misuse of JIT abbreviation.
Typically JIT means compiling to machine code that can be executed
directly. Converting one representation to another is not really JIT.
Also IR stands for _intermediate_ representation. It is a transitional
state when compiler converts high level language into machine code.
In this case the proposed format is protocol specific syntax tree,
so probably should be called as such.

imo the HW guys should be free to pick whatever representation
we have today and offload it. If u32 is convenient and applies
to HW architecture better, the driver should take u32 tree and
map it to HW (which is what was done already). When/If another
HW comes along with similar HW architecture we can generalize
and reuse u32->ixgbe code. And it should be done by developers
who actually have the HW and can test on it. Trying to 'generalize'
u32->ixgbe code without 2nd HW is not going to be successful.

  parent reply	other threads:[~2016-02-26 18:53 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-25 17:37 [PATCH RFC 0/3] intermediate representation for jit and cls_u32 conversion Pablo Neira Ayuso
2016-02-25 17:37 ` [PATCH RFC 1/3] net: ixgbe: add struct igxbe_filter Pablo Neira Ayuso
2016-02-25 17:37 ` [PATCH RFC 2/3] net: intermediate representation for jit translation Pablo Neira Ayuso
2016-02-25 17:37 ` [PATCH RFC 3/3] net: convert tc_u32 to use the intermediate representation Pablo Neira Ayuso
2016-02-25 20:37   ` John Fastabend
2016-02-26 14:24     ` Pablo Neira Ayuso
2016-02-26 14:53       ` John Fastabend
2016-02-26 16:02         ` Pablo Neira Ayuso
2016-02-26 16:34           ` John Fastabend
2016-02-26 17:38           ` David Miller
2016-02-26 17:34         ` David Miller
2016-02-25 18:11 ` [PATCH RFC 0/3] intermediate representation for jit and cls_u32 conversion John Fastabend
2016-02-26 11:47   ` Pablo Neira Ayuso
2016-02-26 15:42     ` John Fastabend
2016-02-26 16:19       ` Pablo Neira Ayuso
2016-02-26 16:46         ` John Fastabend
2016-02-26 17:40         ` David Miller
2016-02-26 18:53         ` Alexei Starovoitov [this message]
2016-02-26 17:26     ` 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=20160226185258.GA51957@ast-mbp.thefacebook.com \
    --to=alexei.starovoitov@gmail.com \
    --cc=davem@davemloft.net \
    --cc=horms@verge.net.au \
    --cc=jiri@resnulli.us \
    --cc=john.fastabend@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pablo@netfilter.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.