On Thu, Jun 09, 2022 at 10:47:02PM +0200, Jörn-Thorben Hinz wrote:
> Remove the check for required and optional functions in a struct
> tcp_congestion_ops from bpf_tcp_ca.c. Rely on
> tcp_register_congestion_control() to reject a BPF CC that does not
> implement all required functions, as it will do for a non-BPF CC.
>
> When a CC implements tcp_congestion_ops.cong_control(), the alternate
> cong_avoid() is not in use in the TCP stack. Previously, a BPF CC was
> still forced to implement cong_avoid() as a no-op since it was
> non-optional in bpf_tcp_ca.c.
>
> Signed-off-by: Jörn-Thorben Hinz <jthinz@xxxxxxxxxxxxxxxxxxxx>
> ---
> net/ipv4/bpf_tcp_ca.c | 32 --------------------------------
> 1 file changed, 32 deletions(-)
>
> diff --git a/net/ipv4/bpf_tcp_ca.c b/net/ipv4/bpf_tcp_ca.c
> index 1f5c53ede4e5..39b64f317499 100644
> --- a/net/ipv4/bpf_tcp_ca.c
> +++ b/net/ipv4/bpf_tcp_ca.c
> @@ -14,18 +14,6 @@
> /* "extern" is to avoid sparse warning. It is only used in bpf_struct_ops.c. */
> 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, set_state),
> - offsetof(struct tcp_congestion_ops, cwnd_event),
> - offsetof(struct tcp_congestion_ops, in_ack_event),
> - offsetof(struct tcp_congestion_ops, pkts_acked),
> - offsetof(struct tcp_congestion_ops, min_tso_segs),
> - offsetof(struct tcp_congestion_ops, sndbuf_expand),
> - offsetof(struct tcp_congestion_ops, cong_control),
> -};
> -
> static u32 unsupported_ops[] = {
> offsetof(struct tcp_congestion_ops, get_info),
> };
> @@ -51,18 +39,6 @@ static int bpf_tcp_ca_init(struct btf *btf)
> return 0;
> }
>
> -static bool is_optional(u32 member_offset)
> -{
> - unsigned int i;
> -
> - for (i = 0; i < ARRAY_SIZE(optional_ops); i++) {
> - if (member_offset == optional_ops[i])
> - return true;
> - }
> -
> - return false;
> -}
> -
> static bool is_unsupported(u32 member_offset)
> {
> unsigned int i;
> @@ -268,14 +244,6 @@ static int bpf_tcp_ca_init_member(const struct btf_type *t,
> return 1;
> }
>
> - if (!btf_type_resolve_func_ptr(btf_vmlinux, member->type, NULL))
> - return 0;
> -
> - /* Ensure bpf_prog is provided for compulsory func ptr */
> - prog_fd = (int)(*(unsigned long *)(udata + moff));
> - if (!prog_fd && !is_optional(moff) && !is_unsupported(moff))
!is_unsupported() is still needed.
and remove 'int prog_fd' as reported by the test bot.
Test is still needed. You can copy the simpler "bpf_dctcp"
to another tcp_congestion_ops. Write+read the sk_packing
and also use .cong_control instead of .cong_avoid. I think rs->acked_sacked
is the 'delivered' and the 'ack' is not used.
