All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tom Herbert <tom@herbertland.com>
To: Thomas Graf <tgraf@suug.ch>
Cc: "David S. Miller" <davem@davemloft.net>,
	Linux Kernel Network Developers <netdev@vger.kernel.org>,
	Kernel Team <kernel-team@fb.com>,
	Tariq Toukan <tariqt@mellanox.com>,
	Brenden Blanco <bblanco@plumgrid.com>,
	Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Eric Dumazet <eric.dumazet@gmail.com>,
	Jesper Dangaard Brouer <brouer@redhat.com>
Subject: Re: [PATCH RFC 1/3] xdp: Infrastructure to generalize XDP
Date: Wed, 21 Sep 2016 07:19:52 -0700	[thread overview]
Message-ID: <CALx6S35Xctd-aA8DF1_vypMDejiGXcud6=UO33dqgvO60W0DZQ@mail.gmail.com> (raw)
In-Reply-To: <20160921115545.GA12789@pox.localdomain>

On Wed, Sep 21, 2016 at 4:55 AM, Thomas Graf <tgraf@suug.ch> wrote:
> On 09/20/16 at 04:59pm, Tom Herbert wrote:
>> Well, need to measure to ascertain the cost. As for complexity, this
>> actually reduces complexity needed for XDP in the drivers which is a
>> good thing because that's where most of the support and development
>> pain will be.
>
> I'm not objecting to anything that simplifies the process of adding
> XDP capability to drivers. You have my full support here.
>
>> I am looking at using this for ILA router. The problem I am hitting is
>> that not all packets that we need to translate go through the XDP
>> path. Some would go through the kernel path, some through XDP path but
>
> When you say kernel path, what do you mean specifically? One aspect of
> XDP I love is that XDP can act as an acceleration option for existing
> BPF programs attached to cls_bpf. Support for direct packet read and
> write at clsact level have made it straight forward to write programs
> which are compatible or at minimum share a lot of common code. They
> can share data structures, lookup functionality, etc.
>
>> We can optimize for allowing only one hook, or maybe limit to only
>> allowing one hook to be set. In any case this obviously requires a lot
>> of performance evaluation, I am hoping to feedback on the design
>> first. My question about using a linear list for this was real, do you
>> know a better method off hand to implement a call list?
>
> My main concern is that we overload the XDP hook. Instead of making use
> of the programmable glue, we put a linked list in front where everybody
> can attach a program to.
>
> A possible alternative:
>  1. The XDP hook always has single consumer controlled by the user
>     through Netlink, BPF is one of them. If a user wants to hardcode
>     the ILA router to that hook, he can do that.
>
>  2. BPF for XDP is extended to allow returning a verdict which results
>     in something else to be invoked. If user wants to invoke the ILA
>     router for just some packets, he can do that.
>
> That said, I see so much value in a BPF implementation of ILA at XDP
> level with all of the parsing logic and exact semantics remain
> flexible without the cost of translating some configuration to a set
> of actions.

Thomas,

As I mentioned though, not all packets we need to translate are going
to go through XDP. ILA already integrates with the routing table via
LWT and that solution works incredibly well especially when we are
sourcing connections (ILA is by far the fastest most efficient
technique of network virtualization). We already have a lookup table
and the translation process coded in the kernel and configuration is
already implemented by netlink and iproute. So I'm not yet sure I want
to redesign all of this around BPF, maybe that is the right answer
maybe it isn't; but, my point is I don't want to be _forced_ into a
certain design that because of constraints on one kernel interface. As
a kernel developer I want flexibility on how we design and implement
things!

I think there are two questions that this patch set poses for the
community wrt XDP:

#1: Should we allow alternate code to run in XDP other than BPF?
#2: If #1 is true what is the best way to implement that?

If the answer to #1 is "no" then the answer to #2 is irrelevant. So
with this RFC I'm hoping we can come the agreement on questions #1.

IMO, there are at least three factors that would motivate the answer
to #1 be yes

1) There is precedence in nfhooks in creating generic hooks in the
critical data path that can have other uses than those originally
intended by the authors. I would take it as general principle that
hooks in the data path should be as generic as possible to get the
most utility.
2) The changes to make XDP work in drivers are quite invasive and as
we've seen in at least one attempt to modify a driver to support XDP
it is easy to break other mechanisms. If we're going to be modifying a
lot of drivers we really want to get the most value for that work.
3) The XDP interface is well designed and really has no dependencies
on BPF. Writing code to directly parse packets is not rocket science.
So it seems reasonable to me that a subsystem may want to use XDP
interface to accelerate existing functionality without introducing
additional dependencies on BPF or other non-kernel mechanisms

In any case, I ask everyone to be open minded. If there is agreement
that BPF is sufficient for all XDP use cases then I wont' pursue these
patches. But given the ramifications of XDP, especially the
considering the changes required across drivers to support it, I
really would like to get input from the community on this.

Thanks,
Tom

  reply	other threads:[~2016-09-21 14:19 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-20 22:00 [PATCH RFC 0/3] xdp: Generalize XDP Tom Herbert
2016-09-20 22:00 ` [PATCH RFC 1/3] xdp: Infrastructure to generalize XDP Tom Herbert
2016-09-20 22:37   ` Eric Dumazet
2016-09-20 22:40     ` Tom Herbert
2016-09-20 22:44   ` Thomas Graf
2016-09-20 22:49     ` Tom Herbert
2016-09-20 23:09       ` Thomas Graf
2016-09-20 23:18         ` Tom Herbert
2016-09-20 23:43           ` Thomas Graf
2016-09-20 23:59             ` Tom Herbert
2016-09-21  0:13               ` Alexei Starovoitov
2016-09-21 11:55               ` Thomas Graf
2016-09-21 14:19                 ` Tom Herbert [this message]
2016-09-21 14:48                   ` Thomas Graf
2016-09-21 15:08                     ` Tom Herbert
2016-09-21 19:56                       ` Jesper Dangaard Brouer
2016-09-22 13:14                         ` Jesper Dangaard Brouer
2016-09-22 14:46                           ` Eric Dumazet
2016-09-21 15:39                   ` Alexei Starovoitov
2016-09-21 17:26                 ` Jakub Kicinski
2016-09-20 23:22         ` Daniel Borkmann
2016-09-21  0:01   ` Alexei Starovoitov
2016-09-21  6:39     ` Jesper Dangaard Brouer
2016-09-21  8:42       ` Daniel Borkmann
2016-09-21 15:44       ` Alexei Starovoitov
2016-09-21 17:26     ` Jakub Kicinski
2016-09-21 17:39       ` Tom Herbert
2016-09-21 18:45         ` Jakub Kicinski
2016-09-21 18:50           ` Tom Herbert
2016-09-21 18:54             ` Jakub Kicinski
2016-09-21 18:58             ` Thomas Graf
2016-09-23 11:13   ` Jamal Hadi Salim
2016-09-23 13:00     ` Jesper Dangaard Brouer
2016-09-23 14:26       ` Alexei Starovoitov
2016-09-25 11:32       ` Jamal Hadi Salim
2016-09-23 14:14     ` Tom Herbert
2016-09-25 12:29       ` Jamal Hadi Salim
2016-09-20 22:00 ` [PATCH RFC 2/3] mlx4: Change XDP/BPF to use generic XDP infrastructure Tom Herbert
2016-09-20 22:00 ` [PATCH RFC 3/3] netdevice: Remove obsolete xdp_netdev_command Tom Herbert

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='CALx6S35Xctd-aA8DF1_vypMDejiGXcud6=UO33dqgvO60W0DZQ@mail.gmail.com' \
    --to=tom@herbertland.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=bblanco@plumgrid.com \
    --cc=brouer@redhat.com \
    --cc=davem@davemloft.net \
    --cc=eric.dumazet@gmail.com \
    --cc=kernel-team@fb.com \
    --cc=netdev@vger.kernel.org \
    --cc=tariqt@mellanox.com \
    --cc=tgraf@suug.ch \
    /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.