lkml.org 
[lkml]   [2022]   [Jan]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.16 0373/1039] bpf, sockmap: Fix return codes from tcp_bpf_recvmsg_parser()
    Date
    From: John Fastabend <john.fastabend@gmail.com>

    [ Upstream commit 5b2c5540b8110eea0d67a78fb0ddb9654c58daeb ]

    Applications can be confused slightly because we do not always return the
    same error code as expected, e.g. what the TCP stack normally returns. For
    example on a sock err sk->sk_err instead of returning the sock_error we
    return EAGAIN. This usually means the application will 'try again'
    instead of aborting immediately. Another example, when a shutdown event
    is received we should immediately abort instead of waiting for data when
    the user provides a timeout.

    These tend to not be fatal, applications usually recover, but introduces
    bogus errors to the user or introduces unexpected latency. Before
    'c5d2177a72a16' we fell back to the TCP stack when no data was available
    so we managed to catch many of the cases here, although with the extra
    latency cost of calling tcp_msg_wait_data() first.

    To fix lets duplicate the error handling in TCP stack into tcp_bpf so
    that we get the same error codes.

    These were found in our CI tests that run applications against sockmap
    and do longer lived testing, at least compared to test_sockmap that
    does short-lived ping/pong tests, and in some of our test clusters
    we deploy.

    Its non-trivial to do these in a shorter form CI tests that would be
    appropriate for BPF selftests, but we are looking into it so we can
    ensure this keeps working going forward. As a preview one idea is to
    pull in the packetdrill testing which catches some of this.

    Fixes: c5d2177a72a16 ("bpf, sockmap: Fix race in ingress receive verdict with redirect to self")
    Signed-off-by: John Fastabend <john.fastabend@gmail.com>
    Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
    Link: https://lore.kernel.org/bpf/20220104205918.286416-1-john.fastabend@gmail.com
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    net/ipv4/tcp_bpf.c | 27 +++++++++++++++++++++++++++
    1 file changed, 27 insertions(+)

    diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
    index f70aa0932bd6c..9b9b02052fd36 100644
    --- a/net/ipv4/tcp_bpf.c
    +++ b/net/ipv4/tcp_bpf.c
    @@ -196,12 +196,39 @@ msg_bytes_ready:
    long timeo;
    int data;

    + if (sock_flag(sk, SOCK_DONE))
    + goto out;
    +
    + if (sk->sk_err) {
    + copied = sock_error(sk);
    + goto out;
    + }
    +
    + if (sk->sk_shutdown & RCV_SHUTDOWN)
    + goto out;
    +
    + if (sk->sk_state == TCP_CLOSE) {
    + copied = -ENOTCONN;
    + goto out;
    + }
    +
    timeo = sock_rcvtimeo(sk, nonblock);
    + if (!timeo) {
    + copied = -EAGAIN;
    + goto out;
    + }
    +
    + if (signal_pending(current)) {
    + copied = sock_intr_errno(timeo);
    + goto out;
    + }
    +
    data = tcp_msg_wait_data(sk, psock, timeo);
    if (data && !sk_psock_queue_empty(psock))
    goto msg_bytes_ready;
    copied = -EAGAIN;
    }
    +out:
    release_sock(sk);
    sk_psock_put(sk, psock);
    return copied;
    --
    2.34.1


    \
     
     \ /
      Last update: 2022-01-24 23:09    [W:2.202 / U:0.212 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site