Commit 0b17ad25 authored by John Fastabend's avatar John Fastabend Committed by Alexei Starovoitov

bpf, sockmap: Add memory accounting so skbs on ingress lists are visible

Move skb->sk assignment out of sk_psock_bpf_run() and into individual
callers. Then we can use proper skb_set_owner_r() call to assign a
sk to a skb. This improves things by also charging the truesize against
the sockets sk_rmem_alloc counter. With this done we get some accounting
in place to ensure the memory associated with skbs on the workqueue are
still being accounted for somewhere. Finally, by using skb_set_owner_r
the destructor is setup so we can just let the normal skb_kfree logic
recover the memory. Combined with previous patch dropping skb_orphan()
we now can recover from memory pressure and maintain accounting.

Note, we will charge the skbs against their originating socket even
if being redirected into another socket. Once the skb completes the
redirect op the kfree_skb will give the memory back. This is important
because if we charged the socket we are redirecting to (like it was
done before this series) the sock_writeable() test could fail because
of the skb trying to be sent is already charged against the socket.

Also TLS case is special. Here we wait until we have decided not to
simply PASS the packet up the stack. In the case where we PASS the
packet up the stack we already have an skb which is accounted for on
the TLS socket context.

For the parser case we continue to just set/clear skb->sk this is
because the skb being used here may be combined with other skbs or
turned into multiple skbs depending on the parser logic. For example
the parser could request a payload length greater than skb->len so
that the strparser needs to collect multiple skbs. At any rate
the final result will be handled in the strparser recv callback.

Fixes: 604326b4 ("bpf, sockmap: convert to generic sk_msg interface")
Signed-off-by: default avatarJohn Fastabend <john.fastabend@gmail.com>
Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
Link: https://lore.kernel.org/bpf/160226867513.5692.10579573214635925960.stgit@john-Precision-5820-Tower
parent 10d58d00
...@@ -684,20 +684,8 @@ EXPORT_SYMBOL_GPL(sk_psock_msg_verdict); ...@@ -684,20 +684,8 @@ EXPORT_SYMBOL_GPL(sk_psock_msg_verdict);
static int sk_psock_bpf_run(struct sk_psock *psock, struct bpf_prog *prog, static int sk_psock_bpf_run(struct sk_psock *psock, struct bpf_prog *prog,
struct sk_buff *skb) struct sk_buff *skb)
{ {
int ret;
/* strparser clones the skb before handing it to a upper layer,
* meaning we have the same data, but sk is NULL. We do want an
* sk pointer though when we run the BPF program. So we set it
* here and then NULL it to ensure we don't trigger a BUG_ON()
* in skb/sk operations later if kfree_skb is called with a
* valid skb->sk pointer and no destructor assigned.
*/
skb->sk = psock->sk;
bpf_compute_data_end_sk_skb(skb); bpf_compute_data_end_sk_skb(skb);
ret = bpf_prog_run_pin_on_cpu(prog, skb); return bpf_prog_run_pin_on_cpu(prog, skb);
skb->sk = NULL;
return ret;
} }
static struct sk_psock *sk_psock_from_strp(struct strparser *strp) static struct sk_psock *sk_psock_from_strp(struct strparser *strp)
...@@ -736,10 +724,11 @@ static void sk_psock_skb_redirect(struct sk_buff *skb) ...@@ -736,10 +724,11 @@ static void sk_psock_skb_redirect(struct sk_buff *skb)
schedule_work(&psock_other->work); schedule_work(&psock_other->work);
} }
static void sk_psock_tls_verdict_apply(struct sk_buff *skb, int verdict) static void sk_psock_tls_verdict_apply(struct sk_buff *skb, struct sock *sk, int verdict)
{ {
switch (verdict) { switch (verdict) {
case __SK_REDIRECT: case __SK_REDIRECT:
skb_set_owner_r(skb, sk);
sk_psock_skb_redirect(skb); sk_psock_skb_redirect(skb);
break; break;
case __SK_PASS: case __SK_PASS:
...@@ -757,11 +746,17 @@ int sk_psock_tls_strp_read(struct sk_psock *psock, struct sk_buff *skb) ...@@ -757,11 +746,17 @@ int sk_psock_tls_strp_read(struct sk_psock *psock, struct sk_buff *skb)
rcu_read_lock(); rcu_read_lock();
prog = READ_ONCE(psock->progs.skb_verdict); prog = READ_ONCE(psock->progs.skb_verdict);
if (likely(prog)) { if (likely(prog)) {
/* We skip full set_owner_r here because if we do a SK_PASS
* or SK_DROP we can skip skb memory accounting and use the
* TLS context.
*/
skb->sk = psock->sk;
tcp_skb_bpf_redirect_clear(skb); tcp_skb_bpf_redirect_clear(skb);
ret = sk_psock_bpf_run(psock, prog, skb); ret = sk_psock_bpf_run(psock, prog, skb);
ret = sk_psock_map_verd(ret, tcp_skb_bpf_redirect_fetch(skb)); ret = sk_psock_map_verd(ret, tcp_skb_bpf_redirect_fetch(skb));
skb->sk = NULL;
} }
sk_psock_tls_verdict_apply(skb, ret); sk_psock_tls_verdict_apply(skb, psock->sk, ret);
rcu_read_unlock(); rcu_read_unlock();
return ret; return ret;
} }
...@@ -823,6 +818,7 @@ static void sk_psock_strp_read(struct strparser *strp, struct sk_buff *skb) ...@@ -823,6 +818,7 @@ static void sk_psock_strp_read(struct strparser *strp, struct sk_buff *skb)
kfree_skb(skb); kfree_skb(skb);
goto out; goto out;
} }
skb_set_owner_r(skb, sk);
prog = READ_ONCE(psock->progs.skb_verdict); prog = READ_ONCE(psock->progs.skb_verdict);
if (likely(prog)) { if (likely(prog)) {
tcp_skb_bpf_redirect_clear(skb); tcp_skb_bpf_redirect_clear(skb);
...@@ -847,8 +843,11 @@ static int sk_psock_strp_parse(struct strparser *strp, struct sk_buff *skb) ...@@ -847,8 +843,11 @@ static int sk_psock_strp_parse(struct strparser *strp, struct sk_buff *skb)
rcu_read_lock(); rcu_read_lock();
prog = READ_ONCE(psock->progs.skb_parser); prog = READ_ONCE(psock->progs.skb_parser);
if (likely(prog)) if (likely(prog)) {
skb->sk = psock->sk;
ret = sk_psock_bpf_run(psock, prog, skb); ret = sk_psock_bpf_run(psock, prog, skb);
skb->sk = NULL;
}
rcu_read_unlock(); rcu_read_unlock();
return ret; return ret;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment