On Sat, Dec 14, 2024 at 05:18 PM -08, Cong Wang wrote: > On Sat, Dec 14, 2024 at 07:50:37PM +0100, Jakub Sitnicki wrote: >> On Mon, Dec 09, 2024 at 11:27 PM +08, Jiayuan Chen wrote: >> >> [...] >> >> > We added test cases for bpf + strparser and separated them from >> > sockmap_basic. This is because we need to add more test cases for >> > strparser in the future. >> > >> > Fixes: e5c6de5fa025 ("bpf, sockmap: Incorrectly handling copied_seq") >> > >> > --- >> >> I have a question unrelated to the fix itself - >> >> Are you an active strparser+verdict sockmap user? >> >> I was wondering if we can deprecate strparser if/when KCM time comes > > I am afraid not. > > strparser is very different from skb verdict, upper layer (e.g. HTTP) > protocol messages may be splitted accross sendmsg() call's, strparser > is the only place where we can assemble the messages and parse them as a > whole. > > And I don't think we have to use KCM together with strparser. Therefore, > even _if_ KCM can be deprecated, strparse still can't. Thanks for the context. Good to know we have strparser users. I also wanna ask - did you guys consider migrating strp_data_ready->strp_read_sock->...->strp_recv to read_skb / tcp_read_skb to prevent the duplicate copied_seq update? tcp_bpf_read_sock looks awfully lot like tcp_read_skb. I realize it is easier said than done because there is an interface mismatch - desc.count used to stop reading, and desc.error to signal OOM / need to requeue is missing. And then there is the SW kTLS read_sock callback that would need adapting as well. Definitely more work, but maybe less code duplication in the long run?