From: Jakub Sitnicki <jakub@cloudflare•com>
To: John Fastabend <john.fastabend@gmail•com>
Cc: ast@kernel•org, daniel@iogearbox•net, bpf@vger•kernel.org,
netdev@vger•kernel.org
Subject: Re: [bpf PATCH v3 5/6] bpf, sockmap: Handle memory acct if skb_verdict prog redirects to self
Date: Tue, 17 Nov 2020 09:21:15 +0100 [thread overview]
Message-ID: <878sb0wg04.fsf@cloudflare.com> (raw)
In-Reply-To: <160556574804.73229.11328201020039674147.stgit@john-XPS-13-9370>
On Mon, Nov 16, 2020 at 11:29 PM CET, John Fastabend wrote:
> If the skb_verdict_prog redirects an skb knowingly to itself, fix your
> BPF program this is not optimal and an abuse of the API please use
> SK_PASS. That said there may be cases, such as socket load balancing,
> where picking the socket is hashed based or otherwise picks the same
> socket it was received on in some rare cases. If this happens we don't
> want to confuse userspace giving them an EAGAIN error if we can avoid
> it.
>
> To avoid double accounting in these cases. At the moment even if the
> skb has already been charged against the sockets rcvbuf and forward
> alloc we check it again and do set_owner_r() causing it to be orphaned
> and recharged. For one this is useless work, but more importantly we
> can have a case where the skb could be put on the ingress queue, but
> because we are under memory pressure we return EAGAIN. The trouble
> here is the skb has already been accounted for so any rcvbuf checks
> include the memory associated with the packet already. This rolls
> up and can result in unecessary EAGAIN errors in userspace read()
> calls.
>
> Fix by doing an unlikely check and skipping checks if skb->sk == sk.
>
> Fixes: 51199405f9672 ("bpf: skb_verdict, support SK_PASS on RX BPF path")
> Signed-off-by: John Fastabend <john.fastabend@gmail•com>
> ---
> net/core/skmsg.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/net/core/skmsg.c b/net/core/skmsg.c
> index 9aed5a2c7c5b..514bc9f6f8ae 100644
> --- a/net/core/skmsg.c
> +++ b/net/core/skmsg.c
> @@ -442,11 +442,19 @@ static int sk_psock_skb_ingress_enqueue(struct sk_buff *skb,
> return copied;
> }
>
> +static int sk_psock_skb_ingress_self(struct sk_psock *psock, struct sk_buff *skb);
> +
> static int sk_psock_skb_ingress(struct sk_psock *psock, struct sk_buff *skb)
> {
> struct sock *sk = psock->sk;
> struct sk_msg *msg;
>
> + /* If we are receiving on the same sock skb->sk is already assigned,
> + * skip memory accounting and owner transition seeing it already set
> + * correctly.
> + */
> + if (unlikely(skb->sk == sk))
> + return sk_psock_skb_ingress_self(psock, skb);
> msg = sk_psock_create_ingress_msg(sk, skb);
> if (!msg)
> return -EAGAIN;
Reviewed-by: Jakub Sitnicki <jakub@cloudflare•com>
next prev parent reply other threads:[~2020-11-17 8:21 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-16 22:27 [bpf PATCH v3 0/6] sockmap fixes John Fastabend
2020-11-16 22:27 ` [bpf PATCH v3 1/6] bpf, sockmap: fix partial copy_page_to_iter so progress can still be made John Fastabend
2020-11-16 22:28 ` [bpf PATCH v3 2/6] bpf, sockmap: Ensure SO_RCVBUF memory is observed on ingress redirect John Fastabend
2020-11-16 22:28 ` [bpf PATCH v3 3/6] bpf, sockmap: Use truesize with sk_rmem_schedule() John Fastabend
2020-11-16 22:28 ` [bpf PATCH v3 4/6] bpf, sockmap: Avoid returning unneeded EAGAIN when redirecting to self John Fastabend
2020-11-16 22:29 ` [bpf PATCH v3 5/6] bpf, sockmap: Handle memory acct if skb_verdict prog redirects " John Fastabend
2020-11-17 8:21 ` Jakub Sitnicki [this message]
2020-11-16 22:29 ` [bpf PATCH v3 6/6] bpf, sockmap: Avoid failures from skb_to_sgvec when skb has frag_list John Fastabend
2020-11-17 23:20 ` [bpf PATCH v3 0/6] sockmap fixes patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=878sb0wg04.fsf@cloudflare.com \
--to=jakub@cloudflare$(echo .)com \
--cc=ast@kernel$(echo .)org \
--cc=bpf@vger$(echo .)kernel.org \
--cc=daniel@iogearbox$(echo .)net \
--cc=john.fastabend@gmail$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox