Re: [PATCH bpf v5 1/3] bpf: fix wrong copied_seq calculation

[Jiayuan Chen]

On Mon, Jan 13, 2025 at 04:04:04PM +0800, Jakub Kicinski wrote:
> On Thu,  9 Jan 2025 17:43:59 +0800 Jiayuan Chen wrote:
> > 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 update 'sk->copied_seq', leading to duplicated
> > updates.
> 
> To state the obvious feels like the abstraction between TCP and psock
> has broken down pretty severely at this stage. You're modifying TCP
> and straight up calling TCP functions from skmsg.c :(
>
You are right!

How about we construct code like this:

sk_psock_strp_read_sock(strp)    skmsg.c
    tcp_bpf_strp_read_sock(sk)   tcp_bpf.c
        tcp_read_sock_noack(sk)  tcp.c

In skmsg.c we just register read_sock handler for strparser, then move
core code into tcp_bpf.c. I believe it makes more sense than before as
there already exist some psock with tcp operation(especially ops handler)
implemented in tcp_bpf.c.
 
> > +int tcp_read_sock_noack(struct sock *sk, read_descriptor_t *desc,
> > +			sk_read_actor_t recv_actor, u32 noack,
> > +			u32 *copied_seq)
> > +{
> > +	return __tcp_read_sock(sk, desc, recv_actor,
> > +			       noack, copied_seq);
> > +}
> > +EXPORT_SYMBOL(tcp_read_sock_noack);
> 
> Pretty sure you don't have to export this. skmsg can't be a module.
ok, i will remove it.