On Fri, Dec 20, 2019 at 10:26 PM Martin KaFai Lau [off-list ref] wrote:

This patch makes "struct tcp_congestion_ops" to be the first user
of BPF STRUCT_OPS.  It allows implementing a tcp_congestion_ops
in bpf.

The BPF implemented tcp_congestion_ops can be used like
regular kernel tcp-cc through sysctl and setsockopt.  e.g.
[root@arch-fb-vm1 bpf]# sysctl -a | egrep congestion
net.ipv4.tcp_allowed_congestion_control = reno cubic bpf_cubic
net.ipv4.tcp_available_congestion_control = reno bic cubic bpf_cubic
net.ipv4.tcp_congestion_control = bpf_cubic

There has been attempt to move the TCP CC to the user space
(e.g. CCP in TCP).   The common arguments are faster turn around,
get away from long-tail kernel versions in production...etc,
which are legit points.

BPF has been the continuous effort to join both kernel and
userspace upsides together (e.g. XDP to gain the performance
advantage without bypassing the kernel).  The recent BPF
advancements (in particular BTF-aware verifier, BPF trampoline,
BPF CO-RE...) made implementing kernel struct ops (e.g. tcp cc)
possible in BPF.  It allows a faster turnaround for testing algorithm
in the production while leveraging the existing (and continue growing)
BPF feature/framework instead of building one specifically for
userspace TCP CC.

This patch allows write access to a few fields in tcp-sock
(in bpf_tcp_ca_btf_struct_access()).

The optional "get_info" is unsupported now.  It can be added
later.  One possible way is to output the info with a btf-id
to describe the content.

Signed-off-by: Martin KaFai Lau <redacted>
---
 include/linux/filter.h            |   2 +
 include/net/tcp.h                 |   1 +
 kernel/bpf/bpf_struct_ops_types.h |   7 +-
 net/core/filter.c                 |   2 +-
 net/ipv4/Makefile                 |   4 +
 net/ipv4/bpf_tcp_ca.c             | 226 ++++++++++++++++++++++++++++++
 net/ipv4/tcp_cong.c               |  14 +-
 net/ipv4/tcp_ipv4.c               |   6 +-
 net/ipv4/tcp_minisocks.c          |   4 +-
 net/ipv4/tcp_output.c             |   4 +-
 10 files changed, 255 insertions(+), 15 deletions(-)
 create mode 100644 net/ipv4/bpf_tcp_ca.c

Naming nits below. Other than that:

Acked-by: Andrii Nakryiko <redacted>

[...]

+static const struct btf_type *tcp_sock_type;
+static u32 tcp_sock_id, sock_id;
+
+static int bpf_tcp_ca_init(struct btf *_btf_vmlinux)
+{

there is no reason to pass anything but vmlinux's BTF to this
function, so I think just having "btf" as a name is OK.

+       s32 type_id;
+
+       type_id = btf_find_by_name_kind(_btf_vmlinux, "sock", BTF_KIND_STRUCT);
+       if (type_id < 0)
+               return -EINVAL;
+       sock_id = type_id;
+
+       type_id = btf_find_by_name_kind(_btf_vmlinux, "tcp_sock",
+                                       BTF_KIND_STRUCT);
+       if (type_id < 0)
+               return -EINVAL;
+       tcp_sock_id = type_id;
+       tcp_sock_type = btf_type_by_id(_btf_vmlinux, tcp_sock_id);
+
+       return 0;
+}
+
+static bool check_optional(u32 member_offset)

check_xxx is quite ambiguous, in general: is it a check that it is
optional or that it's not optional? How about using
is_optional/is_unsupported to make this clear?

+{
+       unsigned int i;
+
+       for (i = 0; i < ARRAY_SIZE(optional_ops); i++) {
+               if (member_offset == optional_ops[i])
+                       return true;
+       }
+
+       return false;
+}
+

[...]