History

bpf: tcp: Allow bpf-tcp-cc to call bpf_(get|set)sockopt

This patch allows the bpf-tcp-cc to call bpf_setsockopt. One use case is to allow a bpf-tcp-cc switching to another cc during init(). For example, when the tcp flow is not ecn ready, the bpf_dctcp can switch to another cc by calling setsockopt(TCP_CONGESTION). During setsockopt(TCP_CONGESTION), the new tcp-cc's init() will be called and this could cause a recursion but it is stopped by the current trampoline's logic (in the prog->active counter). While retiring a bpf-tcp-cc (e.g. in tcp_v[46]_destroy_sock()), the tcp stack calls bpf-tcp-cc's release(). To avoid the retiring bpf-tcp-cc making further changes to the sk, bpf_setsockopt is not available to the bpf-tcp-cc's release(). This will avoid release() making setsockopt() call that will potentially allocate new resources. Although the bpf-tcp-cc already has a more powerful way to read tcp_sock from the PTR_TO_BTF_ID, it is usually expected that bpf_getsockopt and bpf_setsockopt are available together. Thus, bpf_getsockopt() is also added to all tcp_congestion_ops except release(). When the old bpf-tcp-cc is calling setsockopt(TCP_CONGESTION) to switch to a new cc, the old bpf-tcp-cc will be released by bpf_struct_ops_put(). Thus, this patch also puts the bpf_struct_ops_map after a rcu grace period because the trampoline's image cannot be freed while the old bpf-tcp-cc is still running. bpf-tcp-cc can only access icsk_ca_priv as SCALAR. All kernel's tcp-cc is also accessing the icsk_ca_priv as SCALAR. The size of icsk_ca_priv has already been raised a few times to avoid extra kmalloc and memory referencing. The only exception is the kernel's tcp_cdg.c that stores a kmalloc()-ed pointer in icsk_ca_priv. To avoid the old bpf-tcp-cc accidentally overriding this tcp_cdg's pointer value stored in icsk_ca_priv after switching and without over-complicating the bpf's verifier for this one exception in tcp_cdg, this patch does not allow switching to tcp_cdg. If there is a need, bpf_tcp_cdg can be implemented and then use the bpf_sk_storage as the extended storage. bpf_sk_setsockopt proto has only been recently added and used in bpf-sockopt and bpf-iter-tcp, so impose the tcp_cdg limitation in the same proto instead of adding a new proto specifically for bpf-tcp-cc.

Martin KaFai Lau committed 5 years ago

Commit eb18b491 parent 7d789bd3 files · +65 −4Browse files

Unified · Split

3 files
  1. kernel/bpf/bpf_struct_ops.c +21 −1
  2. net/core/filter.c +6 −0
  3. net/ipv4/bpf_tcp_ca.c +38 −3
kernel/bpf/bpf_struct_ops.c+21−1
@@ -28,6 +28,7 @@ struct bpf_struct_ops_value { · 27 unmodified lines
2828
2929 struct bpf_struct_ops_map {
3030 struct bpf_map map;
31+ struct rcu_head rcu;
3132 const struct bpf_struct_ops *st_ops;
3233 /* protect map_update */
3334 struct mutex lock;
@@ -622,6 +623,14 @@ bool bpf_struct_ops_get(const void *kdata) · 588 unmodified lines
622623 return refcount_inc_not_zero(&kvalue->refcnt);
623624 }
624625
626+static void bpf_struct_ops_put_rcu(struct rcu_head *head)
627+{
628+ struct bpf_struct_ops_map *st_map;
629+
630+ st_map = container_of(head, struct bpf_struct_ops_map, rcu);
631+ bpf_map_put(&st_map->map);
632+}
633+
625634 void bpf_struct_ops_put(const void *kdata)
626635 {
627636 struct bpf_struct_ops_value *kvalue;
@@ -632,6 +641,17 @@ void bpf_struct_ops_put(const void *kdata) · 4 unmodified lines
632641
633642 st_map = container_of(kvalue, struct bpf_struct_ops_map,
634643 kvalue);
635− bpf_map_put(&st_map->map);
644+ /* The struct_ops's function may switch to another struct_ops.
645+ *
646+ * For example, bpf_tcp_cc_x->init() may switch to
647+ * another tcp_cc_y by calling
648+ * setsockopt(TCP_CONGESTION, "tcp_cc_y").
649+ * During the switch, bpf_struct_ops_put(tcp_cc_x) is called
650+ * and its map->refcnt may reach 0 which then free its
651+ * trampoline image while tcp_cc_x is still running.
652+ *
653+ * Thus, a rcu grace period is needed here.
654+ */
655+ call_rcu(&st_map->rcu, bpf_struct_ops_put_rcu);
636656 }
637657 }
net/core/filter.c+6−0
@@ -5051,6 +5051,12 @@ err_clear: · 5050 unmodified lines
50515051 BPF_CALL_5(bpf_sk_setsockopt, struct sock *, sk, int, level,
50525052 int, optname, char *, optval, int, optlen)
50535053 {
5054+ if (level == SOL_TCP && optname == TCP_CONGESTION) {
5055+ if (optlen >= sizeof("cdg") - 1 &&
5056+ !strncmp("cdg", optval, optlen))
5057+ return -ENOTSUPP;
5058+ }
5059+
50545060 return _bpf_setsockopt(sk, level, optname, optval, optlen);
50555061 }
50565062
net/ipv4/bpf_tcp_ca.c+38−3
@@ -10,6 +10,9 @@ · 9 unmodified lines
1010 #include <net/tcp.h>
1111 #include <net/bpf_sk_storage.h>
1212
13+/* "extern" is to avoid sparse warning. It is only used in bpf_struct_ops.c. */
14+extern struct bpf_struct_ops bpf_tcp_congestion_ops;
15+
1316 static u32 optional_ops[] = {
1417 offsetof(struct tcp_congestion_ops, init),
1518 offsetof(struct tcp_congestion_ops, release),
@@ -163,6 +166,19 @@ static const struct bpf_func_proto bpf_tcp_send_ack_proto = { · 147 unmodified lines
163166 .arg2_type = ARG_ANYTHING,
164167 };
165168
169+static u32 prog_ops_moff(const struct bpf_prog *prog)
170+{
171+ const struct btf_member *m;
172+ const struct btf_type *t;
173+ u32 midx;
174+
175+ midx = prog->expected_attach_type;
176+ t = bpf_tcp_congestion_ops.type;
177+ m = &btf_type_member(t)[midx];
178+
179+ return btf_member_bit_offset(t, m) / 8;
180+}
181+
166182 static const struct bpf_func_proto *
167183 bpf_tcp_ca_get_func_proto(enum bpf_func_id func_id,
168184 const struct bpf_prog *prog)
@@ -174,6 +190,28 @@ bpf_tcp_ca_get_func_proto(enum bpf_func_id func_id, · 5 unmodified lines
174190 return &bpf_sk_storage_get_proto;
175191 case BPF_FUNC_sk_storage_delete:
176192 return &bpf_sk_storage_delete_proto;
193+ case BPF_FUNC_setsockopt:
194+ /* Does not allow release() to call setsockopt.
195+ * release() is called when the current bpf-tcp-cc
196+ * is retiring. It is not allowed to call
197+ * setsockopt() to make further changes which
198+ * may potentially allocate new resources.
199+ */
200+ if (prog_ops_moff(prog) !=
201+ offsetof(struct tcp_congestion_ops, release))
202+ return &bpf_sk_setsockopt_proto;
203+ return NULL;
204+ case BPF_FUNC_getsockopt:
205+ /* Since get/setsockopt is usually expected to
206+ * be available together, disable getsockopt for
207+ * release also to avoid usage surprise.
208+ * The bpf-tcp-cc already has a more powerful way
209+ * to read tcp_sock from the PTR_TO_BTF_ID.
210+ */
211+ if (prog_ops_moff(prog) !=
212+ offsetof(struct tcp_congestion_ops, release))
213+ return &bpf_sk_getsockopt_proto;
214+ return NULL;
177215 default:
178216 return bpf_base_func_proto(func_id);
179217 }
@@ -286,9 +324,6 @@ static void bpf_tcp_ca_unreg(void *kdata) · 106 unmodified lines
286324 tcp_unregister_congestion_control(kdata);
287325 }
288326
289−/* Avoid sparse warning. It is only used in bpf_struct_ops.c. */
290−extern struct bpf_struct_ops bpf_tcp_congestion_ops;
291−
292327 struct bpf_struct_ops bpf_tcp_congestion_ops = {
293328 .verifier_ops = &bpf_tcp_ca_verifier_ops,
294329 .reg = bpf_tcp_ca_reg,

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t