Re: [PATCH bpf v3 1/2] bpf: fix wrong copied_seq calculation

[Jakub Sitnicki]

On Thu, Dec 19, 2024 at 05:30 PM +08, Jiayuan Chen wrote:
> Currently, not all modules using strparser have issues with
> copied_seq miscalculation. The issue exists mainly with
> bpf::sockmap + strparser because bpf::sockmap implements a
> proprietary read interface for user-land: tcp_bpf_recvmsg_parser().
>
> Both this and strp_recv->tcp_read_sock update copied_seq, leading
> to errors.
>
> This is why I rewrote the tcp_read_sock() interface specifically for
> bpf::sockmap.

All right. Looks like reusing read_skb is not going to pan out.

But I think we should not give up just yet. It's easy to add new code.

We can try to break up and parametrize tcp_read_sock - if other
maintainers are not against it. Does something like this work for you?

  https://github.com/jsitnicki/linux/commits/review/stp-copied_seq/idea-2/

Other minor feedback I have:

- The newly added code is optional and should depend on
  CONFIG_BPF_STREAM_PARSER being enabled. Please check that it builds
  with CONFIG_BPF_STREAM_PARSER=n as well.

- Let's not add complexity until it's really needed, and today we don't
  need seprate tcp_bpf_proto_ops for IPv4 and IPv6.

- There are style issues with the added test. Please run checkpatch.pl.