On 2025-01-26 15:16:47, Jakub Sitnicki wrote: > 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@xxxxxxxxx > > 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@xxxxxxx/ > > Fixed some issues suggested by Jakub Sitnicki. > > > > V7 -> V8 > > https://lore.kernel.org/bpf/20250116140531.108636-1-mrpre@xxxxxxx/ > > 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@xxxxxxx/ > > https://lore.kernel.org/bpf/20241218053408.437295-1-mrpre@xxxxxxx/ > > 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@xxxxxxx/ > > 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. +1 Thanks Jiayuan for sticking with this. I've reviewed this a couple times. I had one nit on the if/else branch for a read call, but I haven't come up with anything better on my end and this fixes a real bug. So lets take it. For the series. Acked-by: John Fastabend <john.fastabend@xxxxxxxxx> > > -jkbs
