From: Jesper Dangaard Brouer <brouer@redhat.com>
To: Daniel Borkmann <daniel@iogearbox.net>
Cc: 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, John Fastabend <john.fastabend@gmail.com>,
Jakub Kicinski <kuba@kernel.org>,
eyal.birger@gmail.com, colrack@gmail.com, brouer@redhat.com
Subject: Re: [PATCH bpf-next V12 4/7] bpf: add BPF-helper for MTU checking
Date: Tue, 26 Jan 2021 10:13:25 +0100 [thread overview]
Message-ID: <20210126101325.75097ddb@carbon> (raw)
In-Reply-To: <3c542e42-2033-aca6-ba0e-4854c24980c2@iogearbox.net>
On Mon, 25 Jan 2021 23:27:22 +0100
Daniel Borkmann <daniel@iogearbox.net> wrote:
> >>> + /* At this point, skb->len exceed MTU, but as it include length of all
> >>> + * segments, it can still be below MTU. The SKB can possibly get
> >>> + * re-segmented in transmit path (see validate_xmit_skb). Thus, user
> >>> + * must choose if segs are to be MTU checked.
> >>> + */
> >>> + if (skb_is_gso(skb)) {
> >>> + ret = BPF_MTU_CHK_RET_SUCCESS;
> >>> +
> >>> + if (flags & BPF_MTU_CHK_SEGS &&
> >>> + !skb_gso_validate_network_len(skb, mtu))
> >>> + ret = BPF_MTU_CHK_RET_SEGS_TOOBIG;
> >>
> >> I think that looks okay overall now. One thing that will easily slip through
> >> is that in the helper description you mentioned 'Check cannot use len_diff.'
> >> for BPF_MTU_CHK_SEGS flag. So right now for non-zero len_diff the user
> >> will still get BPF_MTU_CHK_RET_SUCCESS if the current length check via
> >> skb_gso_validate_network_len(skb, mtu) passes. If it cannot be checked,
> >> maybe enforce len_diff == 0 for gso skbs on BPF_MTU_CHK_SEGS?
> >
> > Ok. Do you want/think this can be enforced by the verifier or are you
> > simply requesting that the helper will return -EINVAL (or another errno)?
>
> Simple -EINVAL should be fine in this case. Generally, we can detect this from
> verifier side but I don't think the extra complexity is worth it especially given
> this is dependent on BPF_MTU_CHK_SEGS and otherwise can be non-zero.
Luckily this was also my choice in V13 that I've already send out.
https://lore.kernel.org/netdev/161159457239.321749.9067604476261493815.stgit@firesoul/
--
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:[~2021-01-26 18:46 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-18 16:54 [PATCH bpf-next V12 0/7] bpf: New approach for BPF MTU handling Jesper Dangaard Brouer
2021-01-18 16:54 ` [PATCH bpf-next V12 1/7] bpf: Remove MTU check in __bpf_skb_max_len Jesper Dangaard Brouer
2021-01-18 16:54 ` [PATCH bpf-next V12 2/7] bpf: fix bpf_fib_lookup helper MTU check for SKB ctx Jesper Dangaard Brouer
2021-01-23 1:47 ` Daniel Borkmann
2021-01-25 15:58 ` Jesper Dangaard Brouer
2021-01-18 16:54 ` [PATCH bpf-next V12 3/7] bpf: bpf_fib_lookup return MTU value as output when looked up Jesper Dangaard Brouer
2021-01-18 16:54 ` [PATCH bpf-next V12 4/7] bpf: add BPF-helper for MTU checking Jesper Dangaard Brouer
2021-01-23 1:35 ` Daniel Borkmann
2021-01-25 8:41 ` Jesper Dangaard Brouer
2021-01-25 22:27 ` Daniel Borkmann
2021-01-26 9:13 ` Jesper Dangaard Brouer [this message]
2021-01-18 16:54 ` [PATCH bpf-next V12 5/7] bpf: drop MTU check when doing TC-BPF redirect to ingress Jesper Dangaard Brouer
2021-01-18 16:54 ` [PATCH bpf-next V12 6/7] selftests/bpf: use bpf_check_mtu in selftest test_cls_redirect Jesper Dangaard Brouer
2021-01-18 16:54 ` [PATCH bpf-next V12 7/7] bpf/selftests: tests using bpf_check_mtu BPF-helper Jesper Dangaard Brouer
2021-01-23 1:49 ` Daniel Borkmann
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=20210126101325.75097ddb@carbon \
--to=brouer@redhat.com \
--cc=alexei.starovoitov@gmail.com \
--cc=borkmann@iogearbox.net \
--cc=bpf@vger.kernel.org \
--cc=colrack@gmail.com \
--cc=daniel@iogearbox.net \
--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 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.