On Tue, Dec 10, 2024 at 10:01:38AM -0800, Alexei Starovoitov wrote:
> On Mon, Dec 9, 2024 at 8:04 PM D. Wythe <alibuda@xxxxxxxxxxxxxxxxx> wrote:
> >
> > +SEC("struct_ops/bpf_smc_set_tcp_option_cond")
> > +int BPF_PROG(bpf_smc_set_tcp_option_cond, const struct tcp_sock *tp, struct inet_request_sock *ireq)
> > +{
> > + return 0;
> > +}
> > +
> > +SEC("struct_ops/bpf_smc_set_tcp_option")
> > +int BPF_PROG(bpf_smc_set_tcp_option, struct tcp_sock *tp)
> > +{
> > + return 1;
> > +}
> > +
> > +SEC(".struct_ops.link")
> > +struct smc_ops sample_smc_ops = {
> > + .name = "sample",
> > + .set_option = (void *) bpf_smc_set_tcp_option,
> > + .set_option_cond = (void *) bpf_smc_set_tcp_option_cond,
> > +};
>
> These stubs don't inspire confidence that smc_ops api
> will be sufficient.
> Please implement a real bpf prog that demonstrates the actual use case.
>
> See how bpf_cubic was done. On the day one it was implemented
> as a parity to builtin cubic cong control.
> And over years we didn't need to touch tcp_congestion_ops.
> To be fair that api was already solid due to in-kernel cc modules,
> but bpf comes with its own limitations, so it wasn't a guarantee
> that tcp_congestion_ops would be enough.
> Here you're proposing a brand new smc_ops api while bpf progs
> are nothing but stubs. That's not sufficient to prove that api
> is viable long term.
Hi Alexei,
Thanks a lot for your advices. I will add actual cases in the
next version to prove why we need it.
>
> In terms of look and feel the smc_ops look ok.
> The change from v1 to v2 was a good step.
I'm glad that you feel it looks okay. If you have any questions,
please let me know.
Thanks,
D. Wythe
>
> pw-bot: cr
