From: Jesper Dangaard Brouer <brouer@redhat.com>
To: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: "Toke Høiland-Jørgensen" <toke@redhat.com>,
"Machulsky, Zorik" <zorik@amazon.com>,
"Jubran, Samih" <sameehj@amazon.com>,
"davem@davemloft.net" <davem@davemloft.net>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"Woodhouse, David" <dwmw@amazon.co.uk>,
"Matushevsky, Alexander" <matua@amazon.com>,
"Bshara, Saeed" <saeedb@amazon.com>,
"Wilson, Matt" <msw@amazon.com>,
"Liguori, Anthony" <aliguori@amazon.com>,
"Bshara, Nafea" <nafea@amazon.com>,
"Tzalik, Guy" <gtzalik@amazon.com>,
"Belgazal, Netanel" <netanel@amazon.com>,
"Saidi, Ali" <alisaidi@amazon.com>,
"Herrenschmidt, Benjamin" <benh@amazon.com>,
"Kiyanovski, Arthur" <akiyano@amazon.com>,
"Daniel Borkmann" <borkmann@iogearbox.net>,
"Ilias Apalodimas" <ilias.apalodimas@linaro.org>,
"Alexei Starovoitov" <alexei.starovoitov@gmail.com>,
"Jakub Kicinski" <jakub.kicinski@netronome.com>,
"xdp-newbies@vger.kernel.org" <xdp-newbies@vger.kernel.org>,
brouer@redhat.com
Subject: Re: XDP multi-buffer incl. jumbo-frames (Was: [RFC V1 net-next 1/1] net: ena: implement XDP drop support)
Date: Fri, 28 Jun 2019 10:46:23 +0200 [thread overview]
Message-ID: <20190628104557.1ffef3e5@carbon> (raw)
In-Reply-To: <CA+FuTSfKnhv9rr=cDa_4m7Dd9qkEm_oabDfyvH0T0sM+fQTU=w@mail.gmail.com>
On Wed, 26 Jun 2019 11:20:45 -0400 Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:
> On Wed, Jun 26, 2019 at 11:01 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > Jesper Dangaard Brouer <brouer@redhat.com> writes:
> > > On Wed, 26 Jun 2019 13:52:16 +0200
> > > Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > >
> > >> Jesper Dangaard Brouer <brouer@redhat.com> writes:
> > >>
[...]
> > >
> > > You touch upon some interesting complications already:
> > >
> > > 1. It is valuable for XDP bpf_prog to know "full" length?
> > > (if so, then we need to extend xdp ctx with info)
> >
> > Valuable, quite likely. A hard requirement, probably not (for all use
> > cases).
>
> Agreed.
>
> One common validation use would be to drop any packets whose header
> length disagrees with the actual packet length.
That is a good point.
Added a section "XDP access to full packet length?" to capture this:
- https://github.com/xdp-project/xdp-project/commit/da5b84264b85b0d
- https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp-multi-buffer01-design.org#xdp-access-to-full-packet-length
> > > But if we need to know the full length, when the first-buffer is
> > > processed. Then realize that this affect the drivers RX-loop, because
> > > then we need to "collect" all the buffers before we can know the
> > > length (although some HW provide this in first descriptor).
> > >
> > > We likely have to change drivers RX-loop anyhow, as XDP_TX and
> > > XDP_REDIRECT will also need to "collect" all buffers before the packet
> > > can be forwarded. (Although this could potentially happen later in
> > > driver loop when it meet/find the End-Of-Packet descriptor bit).
>
> Yes, this might be quite a bit of refactoring of device driver code.
>
> Should we move forward with some initial constraints, e.g., no
> XDP_REDIRECT, no "full" length and no bpf_xdp_adjust_tail?
I generally like this...
If not adding "full" length. Maybe we could add an indication to
XDP-developer, that his is a multi-buffer/multi-segment packet, such
that header length validation code against packet length (data_end-data)
is not possible. This is user visible, so we would have to keep it
forever... I'm leaning towards adding "full" length from beginning.
> That already allows many useful programs.
>
> As long as we don't arrive at a design that cannot be extended with
> those features later.
That is the important part...
> > > 2. Can we even allow helper bpf_xdp_adjust_tail() ?
[...]
> >
> > > Perhaps it is better to let bpf_xdp_adjust_tail() fail runtime?
> >
> > If we do disallow it, I think I'd lean towards failing the call at
> > runtime...
>
> Disagree. I'd rather have a program fail at load if it depends on
> multi-frag support while the (driver) implementation does not yet
> support it.
I usually agree that we should fail the program, early at load time.
For XDP we are unfortunately missing some knobs to do this, see[1].
Specifically for bpf_xdp_adjust_tail(), it might be better to fail
runtime. Because, the driver might have enabled TSO for TCP packets,
while your XDP use-case is for adjusting UDP-packets (and do XDP level
replies), which will never see multi-buffer packets. If we fail use of
bpf_xdp_adjust_tail(), then you would have to disable TSO to allow
loading your XDP-prog, hurting the other TSO-TCP use-case.
[1] http://vger.kernel.org/netconf2019_files/xdp-feature-detection.pdf
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
next prev parent reply other threads:[~2019-06-28 8:48 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-23 7:06 [RFC V1 net-next 0/1] Introduce xdp to ena sameehj
2019-06-23 7:06 ` [RFC V1 net-next 1/1] net: ena: implement XDP drop support sameehj
2019-06-23 14:09 ` Jesper Dangaard Brouer
2019-06-23 14:21 ` Jesper Dangaard Brouer
2019-06-25 3:19 ` Machulsky, Zorik
2019-06-26 8:38 ` XDP multi-buffer incl. jumbo-frames (Was: [RFC V1 net-next 1/1] net: ena: implement XDP drop support) Jesper Dangaard Brouer
2019-06-26 11:52 ` Toke Høiland-Jørgensen
2019-06-26 14:40 ` Jesper Dangaard Brouer
2019-06-26 15:01 ` Toke Høiland-Jørgensen
2019-06-26 15:20 ` Willem de Bruijn
2019-06-26 16:42 ` Jonathan Lemon
2019-06-26 20:00 ` Jesper Dangaard Brouer
2019-06-27 22:07 ` Jonathan Lemon
2019-06-28 8:46 ` Jesper Dangaard Brouer [this message]
2019-06-26 15:14 ` Toke Høiland-Jørgensen
2019-06-26 16:36 ` Jesper Dangaard Brouer
2019-06-28 7:14 ` Eelco Chaudron
2019-06-28 7:46 ` Toke Høiland-Jørgensen
2019-06-28 11:49 ` Eelco Chaudron
2019-06-28 8:22 ` Jesper Dangaard Brouer
2019-06-23 14:28 ` [RFC V1 net-next 1/1] net: ena: implement XDP drop support David Ahern
2019-06-23 14:51 ` Maciej Fijalkowski
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=20190628104557.1ffef3e5@carbon \
--to=brouer@redhat.com \
--cc=akiyano@amazon.com \
--cc=alexei.starovoitov@gmail.com \
--cc=aliguori@amazon.com \
--cc=alisaidi@amazon.com \
--cc=benh@amazon.com \
--cc=borkmann@iogearbox.net \
--cc=davem@davemloft.net \
--cc=dwmw@amazon.co.uk \
--cc=gtzalik@amazon.com \
--cc=ilias.apalodimas@linaro.org \
--cc=jakub.kicinski@netronome.com \
--cc=matua@amazon.com \
--cc=msw@amazon.com \
--cc=nafea@amazon.com \
--cc=netanel@amazon.com \
--cc=netdev@vger.kernel.org \
--cc=saeedb@amazon.com \
--cc=sameehj@amazon.com \
--cc=toke@redhat.com \
--cc=willemdebruijn.kernel@gmail.com \
--cc=xdp-newbies@vger.kernel.org \
--cc=zorik@amazon.com \
/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).