On Wed, Jun 08, 2022 at 07:48:43PM +0200, Jörn-Thorben Hinz wrote:
> When a CC implements tcp_congestion_ops.cong_control(), the alternate
> cong_avoid() is not in use in the TCP stack. Do not force a BPF CC to
> implement cong_avoid() as a no-op by always requiring it.
>
> An incomplete BPF CC implementing neither cong_avoid() nor
> cong_control() will still get rejected by
> tcp_register_congestion_control().
>
> Signed-off-by: Jörn-Thorben Hinz <jthinz@xxxxxxxxxxxxxxxxxxxx>
> ---
> net/ipv4/bpf_tcp_ca.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/ipv4/bpf_tcp_ca.c b/net/ipv4/bpf_tcp_ca.c
> index 1f5c53ede4e5..37290d0bf134 100644
> --- a/net/ipv4/bpf_tcp_ca.c
> +++ b/net/ipv4/bpf_tcp_ca.c
> @@ -17,6 +17,7 @@ extern struct bpf_struct_ops bpf_tcp_congestion_ops;
> static u32 optional_ops[] = {
> offsetof(struct tcp_congestion_ops, init),
> offsetof(struct tcp_congestion_ops, release),
> + offsetof(struct tcp_congestion_ops, cong_avoid),
At least one of the cong_avoid() or cong_control() is needed.
It is better to remove is_optional(moff) check and its optional_ops[]
here. Only depends on the tcp_register_congestion_control() which
does a similar check at the beginning.
Patch 1 looks good. tcp_bbr.c also needs the sk_pacing fields.
A selftest is needed. Can you share your bpf tcp-cc and
use it as a selftest to exercise the change in this patch
set ?
> offsetof(struct tcp_congestion_ops, set_state),
> offsetof(struct tcp_congestion_ops, cwnd_event),
> offsetof(struct tcp_congestion_ops, in_ack_event),
> --
> 2.30.2
>
