All of lore.kernel.org
 help / color / mirror / Atom feed
From: Martin KaFai Lau <kafai@fb.com>
To: Daniel Borkmann <daniel@iogearbox.net>
Cc: "Joanne Koong" <joannekoong@fb.com>,
	"Toke Høiland-Jørgensen" <toke@redhat.com>,
	bpf@vger.kernel.org, netdev@vger.kernel.org, Kernel-team@fb.com
Subject: Re: [PATCH bpf-next v2 0/3] Add XDP support for bpf_load_hdr_opt
Date: Thu, 7 Oct 2021 16:52:03 -0700	[thread overview]
Message-ID: <20211007235203.uksujks57djohg3p@kafai-mbp> (raw)
In-Reply-To: <43bfb0fe-5476-c62c-51f2-a83da9fef659@iogearbox.net>

On Thu, Oct 07, 2021 at 11:25:29PM +0200, Daniel Borkmann wrote:
> I tend to agree with Toke here that this is not generic. What has been tried
> to improve the verifier instead before submitting the series? It would be much
> more preferable to improve the developer experience with regards to a generic
> solution, so that other/similar problems can be tackled in one go as well such
> as IP options, extension headers, etc.
It would be nice to improve verifier to recognize it more smoothly.  Would
love to hear idea how to do it.

When adding the tcp header options for bpf_sockops, a bpf_store_hdr_opt()
is needed to ensure the header option is sane.  When writing test to parse
variable length header option, I also pulled in tricks (e.g. "#pragma unroll"
is easier to get it work.  Tried bounded loop but then hits max insns and
then moved some cases into subprog...etc).  Most (if not all) TCP headers
has some options (e.g. tstamp), so it will be useful to have an easy way
to search a particular option and bpf_load_hdr_opt() was also added to
bpf_sockops.

When bpf-tcp-hdr-opt was added, the initial use case was only useful in the
TCP stack because it wants to change the behavior of a tcp_sock.  When
bpf allows to add tcp-hdr-opt easily at tcp stack, it opens up other use
cases and one of them is to load-balance by the tcp-hdr-opt "server_id" in
XDP [1].  The same user that writes the bpf_sockops prog to add/parse
tcp-opt needs to do extra tricks to get this work in xdp prog.  The user
had repeated a similar try-and-error exercise (e.g. while the logical thinking
is to somehow bounded by the max tcp header size but that does not work
in unroll.  With the current cases in the loop, 15 is the max
magic number that works and hoping it will continue to work).

[1]: https://linuxplumbersconf.org/event/11/contributions/950/

  reply	other threads:[~2021-10-07 23:52 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-06 23:05 [PATCH bpf-next v2 0/3] Add XDP support for bpf_load_hdr_opt Joanne Koong
2021-10-06 23:05 ` [PATCH bpf-next v2 1/3] bpf/xdp: Add bpf_load_hdr_opt support for xdp Joanne Koong
2021-10-06 23:50   ` Song Liu
2021-10-06 23:05 ` [PATCH bpf-next v2 2/3] bpf/selftests: Rename test_tcp_hdr_options to test_sockops_tcp_hdr_options Joanne Koong
2021-10-06 23:47   ` Song Liu
2021-10-06 23:05 ` [PATCH bpf-next v2 3/3] bpf/selftests: Add xdp bpf_load_tcp_hdr_options tests Joanne Koong
2021-10-06 23:52   ` Song Liu
2021-10-07 14:41 ` [PATCH bpf-next v2 0/3] Add XDP support for bpf_load_hdr_opt Toke Høiland-Jørgensen
2021-10-07 20:57   ` Joanne Koong
2021-10-07 21:25     ` Daniel Borkmann
2021-10-07 23:52       ` Martin KaFai Lau [this message]
2021-10-08 22:20         ` Toke Høiland-Jørgensen
2021-10-11 18:43           ` Martin KaFai Lau
2021-10-12 14:11             ` Toke Høiland-Jørgensen
2021-10-12 20:51               ` Joanne Koong
2021-10-13 10:19                 ` Toke Høiland-Jørgensen
2021-10-19  0:00           ` Alexei Starovoitov
2021-10-19 16:02             ` Yonghong Song
2021-10-19 16:10             ` Toke Høiland-Jørgensen

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=20211007235203.uksujks57djohg3p@kafai-mbp \
    --to=kafai@fb.com \
    --cc=Kernel-team@fb.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=joannekoong@fb.com \
    --cc=netdev@vger.kernel.org \
    --cc=toke@redhat.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 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.