From: Jesper Dangaard Brouer <brouer@redhat.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: John Fastabend <john.fastabend@gmail.com>,
bpf@vger.kernel.org, netdev@vger.kernel.org,
Daniel Borkmann <borkmann@iogearbox.net>,
Alexei Starovoitov <alexei.starovoitov@gmail.com>,
maze@google.com, lmb@cloudflare.com, shaun@tigera.io,
Lorenzo Bianconi <lorenzo@kernel.org>,
marek@cloudflare.com, eyal.birger@gmail.com, brouer@redhat.com
Subject: Re: [PATCH bpf-next V3 0/6] bpf: New approach for BPF MTU handling
Date: Tue, 13 Oct 2020 22:40:09 +0200 [thread overview]
Message-ID: <20201013224009.77d6f746@carbon> (raw)
In-Reply-To: <20201010093212.374d1e68@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
On Sat, 10 Oct 2020 09:32:12 -0700
Jakub Kicinski <kuba@kernel.org> wrote:
> On Sat, 10 Oct 2020 12:44:02 +0200 Jesper Dangaard Brouer wrote:
> > > > > We will not be sprinkling validation checks across the drivers because
> > > > > some reconfiguration path may occasionally yield a bad packet, or it's
> > > > > hard to do something right with BPF.
> > > >
> > > > This is a driver bug then. As it stands today drivers may get hit with
> > > > skb with MTU greater than set MTU as best I can tell.
> > >
> > > You're talking about taking it from "maybe this can happen, but will
> > > still be at most jumbo" to "it's going to be very easy to trigger and
> > > length may be > MAX_U16".
> >
> > It is interesting that a misbehaving BPF program can easily trigger this.
> > Next week, I will looking writing such a BPF-prog and then test it on
> > the hardware I have avail in my testlab.
I've tested sending different packet sizes that exceed the MTU on
different hardware. They all silently drop the transmitted packet. mlx5
and i40e configured to (L3) MTU 1500, will lets through upto 1504, while
ixgbe will drop size 1504.
Packets can be observed locally with tcpdump, but the other end doesn't
receive the packet. I didn't find any counters (including ethtool -S)
indicating these packets were dropped at hardware/firmware level, which
were a little concerning for later troubleshooting.
Another observation is that size increases (with bpf_skb_adjust_room)
above 4096 + e.g 128 will likely fail, even-though I have the 64K limit in
this kernel.
> FWIW I took a quick swing at testing it with the HW I have and it did
> exactly what hardware should do. The TX unit entered an error state
> and then the driver detected that and reset it a few seconds later.
The drivers (i40e, mlx5, ixgbe) I tested with didn't entered an error
state, when getting packets exceeding the MTU. I didn't go much above
4K, so maybe I didn't trigger those cases.
> Hardware is almost always designed to behave like that. If some NIC
> actually cleanly drops over sized TX frames, I'd bet it's done in FW,
> or some other software piece.
--
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:[~2020-10-13 20:40 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-08 14:08 [PATCH bpf-next V3 0/6] bpf: New approach for BPF MTU handling Jesper Dangaard Brouer
2020-10-08 14:09 ` [PATCH bpf-next V3 1/6] bpf: Remove MTU check in __bpf_skb_max_len Jesper Dangaard Brouer
2020-10-09 16:12 ` Daniel Borkmann
2020-10-09 18:26 ` Maciej Żenczykowski
2020-10-10 10:25 ` Jesper Dangaard Brouer
2020-10-08 14:09 ` [PATCH bpf-next V3 2/6] bpf: bpf_fib_lookup return MTU value as output when looked up Jesper Dangaard Brouer
2020-10-09 4:05 ` David Ahern
2020-10-08 14:09 ` [PATCH bpf-next V3 3/6] bpf: add BPF-helper for MTU checking Jesper Dangaard Brouer
2020-10-09 23:29 ` Maciej Żenczykowski
2020-10-21 11:32 ` Jesper Dangaard Brouer
2020-10-12 15:54 ` Lorenz Bauer
2020-10-08 14:09 ` [PATCH bpf-next V3 4/6] bpf: make it possible to identify BPF redirected SKBs Jesper Dangaard Brouer
2020-10-09 16:47 ` Daniel Borkmann
2020-10-09 18:33 ` Maciej Żenczykowski
2020-10-10 11:09 ` Jesper Dangaard Brouer
2020-10-12 21:04 ` Maciej Żenczykowski
2020-10-08 14:09 ` [PATCH bpf-next V3 5/6] bpf: drop MTU check when doing TC-BPF redirect to ingress Jesper Dangaard Brouer
2020-10-09 23:17 ` Maciej Żenczykowski
2020-10-08 14:09 ` [PATCH bpf-next V3 6/6] net: inline and splitup is_skb_forwardable Jesper Dangaard Brouer
2020-10-09 16:33 ` [PATCH bpf-next V3 0/6] bpf: New approach for BPF MTU handling Jakub Kicinski
2020-10-09 20:49 ` John Fastabend
2020-10-09 21:07 ` Alexei Starovoitov
2020-10-09 21:57 ` Maciej Żenczykowski
2020-10-09 23:00 ` Jakub Kicinski
2020-10-10 10:44 ` Jesper Dangaard Brouer
2020-10-10 16:32 ` Jakub Kicinski
2020-10-10 23:52 ` John Fastabend
2020-10-11 23:30 ` Jakub Kicinski
2020-10-13 20:40 ` Jesper Dangaard Brouer [this message]
2020-10-13 23:07 ` Jakub Kicinski
2020-10-13 23:37 ` Alexei Starovoitov
2020-10-13 23:54 ` Maciej Żenczykowski
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=20201013224009.77d6f746@carbon \
--to=brouer@redhat.com \
--cc=alexei.starovoitov@gmail.com \
--cc=borkmann@iogearbox.net \
--cc=bpf@vger.kernel.org \
--cc=eyal.birger@gmail.com \
--cc=john.fastabend@gmail.com \
--cc=kuba@kernel.org \
--cc=lmb@cloudflare.com \
--cc=lorenzo@kernel.org \
--cc=marek@cloudflare.com \
--cc=maze@google.com \
--cc=netdev@vger.kernel.org \
--cc=shaun@tigera.io \
/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).