From: Jakub Sitnicki <jakub@cloudflare.com>
To: Jiayuan Chen <mrpre@163.com>
Cc: bpf@vger.kernel.org, john.fastabend@gmail.com,
netdev@vger.kernel.org, martin.lau@linux.dev, ast@kernel.org,
edumazet@google.com, davem@davemloft.net, dsahern@kernel.org,
kuba@kernel.org, pabeni@redhat.com,
linux-kernel@vger.kernel.org, song@kernel.org,
andrii@kernel.org, mhal@rbox.co, yonghong.song@linux.dev,
daniel@iogearbox.net, xiyou.wangcong@gmail.com,
horms@kernel.org, corbet@lwn.net, eddyz87@gmail.com,
cong.wang@bytedance.com, shuah@kernel.org, mykolal@fb.com,
jolsa@kernel.org, haoluo@google.com, sdf@fomichev.me,
kpsingh@kernel.org, linux-doc@vger.kernel.org,
linux-kselftest@vger.kernel.org
Subject: Re: [PATCH bpf v9 0/5] bpf: fix wrong copied_seq calculation and add tests
Date: Sun, 26 Jan 2025 15:16:47 +0100 [thread overview]
Message-ID: <877c6hd5io.fsf@cloudflare.com> (raw)
In-Reply-To: <20250122100917.49845-1-mrpre@163.com> (Jiayuan Chen's message of "Wed, 22 Jan 2025 18:09:12 +0800")
On Wed, Jan 22, 2025 at 06:09 PM +08, Jiayuan Chen wrote:
> A previous commit described in this topic
> http://lore.kernel.org/bpf/20230523025618.113937-9-john.fastabend@gmail.com
> directly updated 'sk->copied_seq' in the tcp_eat_skb() function when the
> action of a BPF program was SK_REDIRECT. For other actions, like SK_PASS,
> the update logic for 'sk->copied_seq' was moved to
> tcp_bpf_recvmsg_parser() to ensure the accuracy of the 'fionread' feature.
>
> That commit works for a single stream_verdict scenario, as it also
> modified 'sk_data_ready->sk_psock_verdict_data_ready->tcp_read_skb'
> to remove updating 'sk->copied_seq'.
>
> However, for programs where both stream_parser and stream_verdict are
> active (strparser purpose), tcp_read_sock() was used instead of
> tcp_read_skb() (sk_data_ready->strp_data_ready->tcp_read_sock).
> tcp_read_sock() now still updates 'sk->copied_seq', leading to duplicated
> updates.
>
> In summary, for strparser + SK_PASS, copied_seq is redundantly calculated
> in both tcp_read_sock() and tcp_bpf_recvmsg_parser().
>
> The issue causes incorrect copied_seq calculations, which prevent
> correct data reads from the recv() interface in user-land.
>
> Also we added test cases for bpf + strparser and separated them from
> sockmap_basic, as strparser has more encapsulation and parsing
> capabilities compared to sockmap.
>
> ---
> V8 -> v9
> https://lore.kernel.org/bpf/20250121050707.55523-1-mrpre@163.com/
> Fixed some issues suggested by Jakub Sitnicki.
>
> V7 -> V8
> https://lore.kernel.org/bpf/20250116140531.108636-1-mrpre@163.com/
> Avoid using add read_sock to psock. (Jakub Sitnicki)
> Avoid using warpper function to check whether strparser is supported.
>
> V3 -> V7:
> https://lore.kernel.org/bpf/20250109094402.50838-1-mrpre@163.com/
> https://lore.kernel.org/bpf/20241218053408.437295-1-mrpre@163.com/
> Avoid introducing new proto_ops. (Jakub Sitnicki).
> Add more edge test cases for strparser + bpf.
> Fix patchwork fail of test cases code.
> Fix psock fetch without rcu lock.
> Move code of modifying to tcp_bpf.c.
>
> V1 -> V3:
> https://lore.kernel.org/bpf/20241209152740.281125-1-mrpre@163.com/
> Fix patchwork fail by adding Fixes tag.
> Save skb data offset for ENOMEM. (John Fastabend)
> ---
Thanks for addressing all feedback, Jiayuan. Series LGTM.
Feel free to carry my tags if there is another iteration.
-jkbs
next prev parent reply other threads:[~2025-01-26 14:16 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-22 10:09 [PATCH bpf v9 0/5] bpf: fix wrong copied_seq calculation and add tests Jiayuan Chen
2025-01-22 10:09 ` [PATCH bpf v9 1/5] strparser: add read_sock callback Jiayuan Chen
2025-01-26 14:03 ` Jakub Sitnicki
2025-01-22 10:09 ` [PATCH bpf v9 2/5] bpf: fix wrong copied_seq calculation Jiayuan Chen
2025-01-26 14:10 ` Jakub Sitnicki
2025-01-22 10:09 ` [PATCH bpf v9 3/5] bpf: disable non stream socket for strparser Jiayuan Chen
2025-01-26 14:11 ` Jakub Sitnicki
2025-01-22 10:09 ` [PATCH bpf v9 4/5] selftests/bpf: fix invalid flag of recv() Jiayuan Chen
2025-01-26 14:12 ` Jakub Sitnicki
2025-01-22 10:09 ` [PATCH bpf v9 5/5] selftests/bpf: add strparser test for bpf Jiayuan Chen
2025-01-26 14:15 ` Jakub Sitnicki
2025-01-26 14:16 ` Jakub Sitnicki [this message]
2025-01-27 16:04 ` [PATCH bpf v9 0/5] bpf: fix wrong copied_seq calculation and add tests John Fastabend
2025-01-29 22:00 ` patchwork-bot+netdevbpf
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=877c6hd5io.fsf@cloudflare.com \
--to=jakub@cloudflare.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=cong.wang@bytedance.com \
--cc=corbet@lwn.net \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=dsahern@kernel.org \
--cc=eddyz87@gmail.com \
--cc=edumazet@google.com \
--cc=haoluo@google.com \
--cc=horms@kernel.org \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kpsingh@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=martin.lau@linux.dev \
--cc=mhal@rbox.co \
--cc=mrpre@163.com \
--cc=mykolal@fb.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=sdf@fomichev.me \
--cc=shuah@kernel.org \
--cc=song@kernel.org \
--cc=xiyou.wangcong@gmail.com \
--cc=yonghong.song@linux.dev \
/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.