From: Jakub Sitnicki <jakub@cloudflare•com>
To: John Fastabend <john.fastabend@gmail•com>
Cc: daniel@iogearbox•net, lmb@isovalent•com, edumazet@google•com,
bpf@vger•kernel.org, netdev@vger•kernel.org, ast@kernel•org,
andrii@kernel•org, will@isovalent•com
Subject: Re: [PATCH bpf v7 11/13] bpf: sockmap, test shutdown() correctly exits epoll and recv()=0
Date: Tue, 16 May 2023 15:41:05 +0200 [thread overview]
Message-ID: <87h6scfl3w.fsf@cloudflare.com> (raw)
In-Reply-To: <6462e1986fb64_250b4208ac@john.notmuch>
On Mon, May 15, 2023 at 06:51 PM -07, John Fastabend wrote:
> Jakub Sitnicki wrote:
>> On Tue, May 02, 2023 at 08:51 AM -07, John Fastabend wrote:
>> > When session gracefully shutdowns epoll needs to wake up and any recv()
>> > readers should return 0 not the -EAGAIN they previously returned.
>> >
>> > Note we use epoll instead of select to test the epoll wake on shutdown
>> > event as well.
>> >
>> > Signed-off-by: John Fastabend <john.fastabend@gmail•com>
>> > ---
>> > .../selftests/bpf/prog_tests/sockmap_basic.c | 68 +++++++++++++++++++
>> > .../bpf/progs/test_sockmap_pass_prog.c | 32 +++++++++
>> > 2 files changed, 100 insertions(+)
>> > create mode 100644 tools/testing/selftests/bpf/progs/test_sockmap_pass_prog.c
>> >
>> > diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
>> > index 0ce25a967481..f9f611618e45 100644
>> > --- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
>> > +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
>> > @@ -2,6 +2,7 @@
>> > // Copyright (c) 2020 Cloudflare
>> > #include <error.h>
>> > #include <netinet/tcp.h>
>> > +#include <sys/epoll.h>
>> >
>> > #include "test_progs.h"
>> > #include "test_skmsg_load_helpers.skel.h"
>> > @@ -9,8 +10,11 @@
>> > #include "test_sockmap_invalid_update.skel.h"
>> > #include "test_sockmap_skb_verdict_attach.skel.h"
>> > #include "test_sockmap_progs_query.skel.h"
>> > +#include "test_sockmap_pass_prog.skel.h"
>> > #include "bpf_iter_sockmap.skel.h"
>> >
>> > +#include "sockmap_helpers.h"
>> > +
>> > #define TCP_REPAIR 19 /* TCP sock is under repair right now */
>> >
>> > #define TCP_REPAIR_ON 1
>> > @@ -350,6 +354,68 @@ static void test_sockmap_progs_query(enum bpf_attach_type attach_type)
>> > test_sockmap_progs_query__destroy(skel);
>> > }
>> >
>> > +#define MAX_EVENTS 10
>> > +static void test_sockmap_skb_verdict_shutdown(void)
>> > +{
>> > + int n, err, map, verdict, s, c0, c1, p0, p1;
>> > + struct epoll_event ev, events[MAX_EVENTS];
>> > + struct test_sockmap_pass_prog *skel;
>> > + int epollfd;
>> > + int zero = 0;
>> > + char b;
>> > +
>> > + skel = test_sockmap_pass_prog__open_and_load();
>> > + if (!ASSERT_OK_PTR(skel, "open_and_load"))
>> > + return;
>> > +
>> > + verdict = bpf_program__fd(skel->progs.prog_skb_verdict);
>> > + map = bpf_map__fd(skel->maps.sock_map_rx);
>> > +
>> > + err = bpf_prog_attach(verdict, map, BPF_SK_SKB_STREAM_VERDICT, 0);
>> > + if (!ASSERT_OK(err, "bpf_prog_attach"))
>> > + goto out;
>> > +
>> > + s = socket_loopback(AF_INET, SOCK_STREAM);
>> > + if (s < 0)
>> > + goto out;
>> > + err = create_socket_pairs(s, AF_INET, SOCK_STREAM, &c0, &c1, &p0, &p1);
>> > + if (err < 0)
>> > + goto out;
>> > +
>> > + err = bpf_map_update_elem(map, &zero, &c1, BPF_NOEXIST);
>> > + if (err < 0)
>> > + goto out_close;
>> > +
>> > + shutdown(c0, SHUT_RDWR);
>> > + shutdown(p1, SHUT_WR);
>> > +
>> > + ev.events = EPOLLIN;
>> > + ev.data.fd = c1;
>> > +
>> > + epollfd = epoll_create1(0);
>> > + if (!ASSERT_GT(epollfd, -1, "epoll_create(0)"))
>> > + goto out_close;
>> > + err = epoll_ctl(epollfd, EPOLL_CTL_ADD, c1, &ev);
>> > + if (!ASSERT_OK(err, "epoll_ctl(EPOLL_CTL_ADD)"))
>> > + goto out_close;
>> > + err = epoll_wait(epollfd, events, MAX_EVENTS, -1);
>> > + if (!ASSERT_EQ(err, 1, "epoll_wait(fd)"))
>> > + goto out_close;
>> > +
>> > + n = recv(c1, &b, 1, SOCK_NONBLOCK);
>> > + ASSERT_EQ(n, 0, "recv_timeout(fin)");
>> > + n = recv(p0, &b, 1, SOCK_NONBLOCK);
>> > + ASSERT_EQ(n, 0, "recv_timeout(fin)");
>> > +
>> > +out_close:
>> > + close(c0);
>> > + close(p0);
>> > + close(c1);
>> > + close(p1);
>> > +out:
>> > + test_sockmap_pass_prog__destroy(skel);
>> > +}
>> > +
>>
>> This test has me scratching my head. I don't grasp what we're testing
>> with (c0, p0) socket pair, since c0 is not in any sockmap?
>
> Yeah the test is on (c1,p1) I was just lazy and using the API as is
> I can fix the API to allow single set c1,p1.
It's not an issue that one pair is unused, IMO. I was just surprised
that you operated on (c0, p0) rather than closing them immediately.
next prev parent reply other threads:[~2023-05-16 13:43 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-02 15:51 [PATCH bpf v7 00/13] bpf sockmap fixes John Fastabend
2023-05-02 15:51 ` [PATCH bpf v7 01/13] bpf: sockmap, pass skb ownership through read_skb John Fastabend
2023-05-02 15:51 ` [PATCH bpf v7 02/13] bpf: sockmap, convert schedule_work into delayed_work John Fastabend
2023-05-02 15:51 ` [PATCH bpf v7 03/13] bpf: sockmap, reschedule is now done through backlog John Fastabend
2023-05-03 9:49 ` Jakub Sitnicki
2023-05-02 15:51 ` [PATCH bpf v7 04/13] bpf: sockmap, improved check for empty queue John Fastabend
2023-05-04 16:53 ` Jakub Sitnicki
2023-05-04 17:42 ` John Fastabend
2023-05-02 15:51 ` [PATCH bpf v7 05/13] bpf: sockmap, handle fin correctly John Fastabend
2023-05-02 15:51 ` [PATCH bpf v7 06/13] bpf: sockmap, TCP data stall on recv before accept John Fastabend
2023-05-02 15:51 ` [PATCH bpf v7 07/13] bpf: sockmap, wake up polling after data copy John Fastabend
2023-05-02 15:51 ` [PATCH bpf v7 08/13] bpf: sockmap, incorrectly handling copied_seq John Fastabend
2023-05-05 12:14 ` Jakub Sitnicki
2023-05-02 15:51 ` [PATCH bpf v7 09/13] bpf: sockmap, pull socket helpers out of listen test for general use John Fastabend
2023-05-05 17:38 ` Jakub Sitnicki
2023-05-02 15:51 ` [PATCH bpf v7 10/13] bpf: sockmap, build helper to create connected socket pair John Fastabend
2023-05-05 17:39 ` Jakub Sitnicki
2023-05-02 15:51 ` [PATCH bpf v7 11/13] bpf: sockmap, test shutdown() correctly exits epoll and recv()=0 John Fastabend
2023-05-08 11:04 ` Jakub Sitnicki
2023-05-16 1:51 ` John Fastabend
2023-05-16 13:41 ` Jakub Sitnicki [this message]
2023-05-02 15:51 ` [PATCH bpf v7 12/13] bpf: sockmap, test FIONREAD returns correct bytes in rx buffer John Fastabend
2023-05-08 11:19 ` Jakub Sitnicki
2023-05-02 15:51 ` [PATCH bpf v7 13/13] bpf: sockmap, test FIONREAD returns correct bytes in rx buffer with drops John Fastabend
2023-05-08 11:34 ` Jakub Sitnicki
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=87h6scfl3w.fsf@cloudflare.com \
--to=jakub@cloudflare$(echo .)com \
--cc=andrii@kernel$(echo .)org \
--cc=ast@kernel$(echo .)org \
--cc=bpf@vger$(echo .)kernel.org \
--cc=daniel@iogearbox$(echo .)net \
--cc=edumazet@google$(echo .)com \
--cc=john.fastabend@gmail$(echo .)com \
--cc=lmb@isovalent$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=will@isovalent$(echo .)com \
/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