* Re: [PATCH v3 bpf-next 4/4] selftests/bpf: add auto-detach test [not found] ` <20190523194532.2376233-5-guro@fb.com> @ 2019-05-23 23:09 ` Yonghong Song 2019-05-23 23:43 ` Roman Gushchin 2019-05-23 23:49 ` Roman Gushchin 0 siblings, 2 replies; 5+ messages in thread From: Yonghong Song @ 2019-05-23 23:09 UTC (permalink / raw) To: Roman Gushchin, Alexei Starovoitov, bpf@vger.kernel.org Cc: Daniel Borkmann, netdev@vger.kernel.org, Tejun Heo, Kernel Team, cgroups@vger.kernel.org, Stanislav Fomichev, linux-kernel@vger.kernel.org On 5/23/19 12:45 PM, Roman Gushchin wrote: > Add a kselftest to cover bpf auto-detachment functionality. > The test creates a cgroup, associates some resources with it, > attaches a couple of bpf programs and deletes the cgroup. > > Then it checks that bpf programs are going away in 5 seconds. > > Expected output: > $ ./test_cgroup_attach > #override:PASS > #multi:PASS > #autodetach:PASS > test_cgroup_attach:PASS > > On a kernel without auto-detaching: > $ ./test_cgroup_attach > #override:PASS > #multi:PASS > #autodetach:FAIL > test_cgroup_attach:FAIL > > Signed-off-by: Roman Gushchin <guro@fb.com> Looks good to me. It will be good if you can add test_cgroup_attach to .gitignore to avoid it shows up in `git status`. With that, Acked-by: Yonghong Song <yhs@fb.com> > --- > .../selftests/bpf/test_cgroup_attach.c | 98 ++++++++++++++++++- > 1 file changed, 97 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/bpf/test_cgroup_attach.c b/tools/testing/selftests/bpf/test_cgroup_attach.c > index 2d6d57f50e10..7671909ee1cb 100644 > --- a/tools/testing/selftests/bpf/test_cgroup_attach.c > +++ b/tools/testing/selftests/bpf/test_cgroup_attach.c > @@ -456,9 +456,105 @@ static int test_multiprog(void) > return rc; > } > > +static int test_autodetach(void) > +{ > + __u32 prog_cnt = 4, attach_flags; > + int allow_prog[2] = {0}; > + __u32 prog_ids[2] = {0}; > + int cg = 0, i, rc = -1; > + void *ptr = NULL; > + int attempts; > + > + for (i = 0; i < ARRAY_SIZE(allow_prog); i++) { > + allow_prog[i] = prog_load_cnt(1, 1 << i); > + if (!allow_prog[i]) > + goto err; > + } > + > + if (setup_cgroup_environment()) > + goto err; > + > + /* create a cgroup, attach two programs and remember their ids */ > + cg = create_and_get_cgroup("/cg_autodetach"); > + if (cg < 0) > + goto err; > + > + if (join_cgroup("/cg_autodetach")) > + goto err; > + > + for (i = 0; i < ARRAY_SIZE(allow_prog); i++) { > + if (bpf_prog_attach(allow_prog[i], cg, BPF_CGROUP_INET_EGRESS, > + BPF_F_ALLOW_MULTI)) { > + log_err("Attaching prog[%d] to cg:egress", i); > + goto err; > + } > + } > + > + /* make sure that programs are attached and run some traffic */ > + assert(bpf_prog_query(cg, BPF_CGROUP_INET_EGRESS, 0, &attach_flags, > + prog_ids, &prog_cnt) == 0); > + assert(system(PING_CMD) == 0); > + > + /* allocate some memory (4Mb) to pin the original cgroup */ > + ptr = malloc(4 * (1 << 20)); > + if (!ptr) > + goto err; > + > + /* close programs and cgroup fd */ > + for (i = 0; i < ARRAY_SIZE(allow_prog); i++) { > + close(allow_prog[i]); > + allow_prog[i] = 0; > + } > + > + close(cg); > + cg = 0; > + > + /* leave the cgroup and remove it. don't detach programs */ > + cleanup_cgroup_environment(); > + > + /* wait for the asynchronous auto-detachment. > + * wait for no more than 5 sec and give up. > + */ > + for (i = 0; i < ARRAY_SIZE(prog_ids); i++) { > + for (attempts = 5; attempts >= 0; attempts--) { > + int fd = bpf_prog_get_fd_by_id(prog_ids[i]); > + > + if (fd < 0) > + break; > + > + /* don't leave the fd open */ > + close(fd); > + > + if (!attempts) > + goto err; > + > + sleep(1); > + } > + } > + > + rc = 0; > +err: > + for (i = 0; i < ARRAY_SIZE(allow_prog); i++) > + if (allow_prog[i] > 0) > + close(allow_prog[i]); > + if (cg) > + close(cg); > + free(ptr); > + cleanup_cgroup_environment(); > + if (!rc) > + printf("#autodetach:PASS\n"); > + else > + printf("#autodetach:FAIL\n"); > + return rc; > +} > + > int main(void) > { > - int (*tests[])(void) = {test_foo_bar, test_multiprog}; > + int (*tests[])(void) = { > + test_foo_bar, > + test_multiprog, > + test_autodetach, > + }; > int errors = 0; > int i; > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 bpf-next 4/4] selftests/bpf: add auto-detach test 2019-05-23 23:09 ` [PATCH v3 bpf-next 4/4] selftests/bpf: add auto-detach test Yonghong Song @ 2019-05-23 23:43 ` Roman Gushchin 2019-05-23 23:49 ` Roman Gushchin 1 sibling, 0 replies; 5+ messages in thread From: Roman Gushchin @ 2019-05-23 23:43 UTC (permalink / raw) To: Yonghong Song Cc: Alexei Starovoitov, bpf@vger.kernel.org, Daniel Borkmann, netdev@vger.kernel.org, Tejun Heo, Kernel Team, cgroups@vger.kernel.org, Stanislav Fomichev, linux-kernel@vger.kernel.org On Thu, May 23, 2019 at 04:09:30PM -0700, Yonghong Song wrote: > > > On 5/23/19 12:45 PM, Roman Gushchin wrote: > > Add a kselftest to cover bpf auto-detachment functionality. > > The test creates a cgroup, associates some resources with it, > > attaches a couple of bpf programs and deletes the cgroup. > > > > Then it checks that bpf programs are going away in 5 seconds. > > > > Expected output: > > $ ./test_cgroup_attach > > #override:PASS > > #multi:PASS > > #autodetach:PASS > > test_cgroup_attach:PASS > > > > On a kernel without auto-detaching: > > $ ./test_cgroup_attach > > #override:PASS > > #multi:PASS > > #autodetach:FAIL > > test_cgroup_attach:FAIL > > > > Signed-off-by: Roman Gushchin <guro@fb.com> > > Looks good to me. It will be good if you can add test_cgroup_attach > to .gitignore to avoid it shows up in `git status`. With that, I don't think it deserves a new version, I'll prepare a separate patch for it. > > Acked-by: Yonghong Song <yhs@fb.com> Thank you for the review! ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 bpf-next 4/4] selftests/bpf: add auto-detach test 2019-05-23 23:09 ` [PATCH v3 bpf-next 4/4] selftests/bpf: add auto-detach test Yonghong Song 2019-05-23 23:43 ` Roman Gushchin @ 2019-05-23 23:49 ` Roman Gushchin 1 sibling, 0 replies; 5+ messages in thread From: Roman Gushchin @ 2019-05-23 23:49 UTC (permalink / raw) To: Yonghong Song Cc: Alexei Starovoitov, bpf@vger.kernel.org, Daniel Borkmann, netdev@vger.kernel.org, Tejun Heo, Kernel Team, cgroups@vger.kernel.org, Stanislav Fomichev, linux-kernel@vger.kernel.org On Thu, May 23, 2019 at 04:09:30PM -0700, Yonghong Song wrote: > > > On 5/23/19 12:45 PM, Roman Gushchin wrote: > > Add a kselftest to cover bpf auto-detachment functionality. > > The test creates a cgroup, associates some resources with it, > > attaches a couple of bpf programs and deletes the cgroup. > > > > Then it checks that bpf programs are going away in 5 seconds. > > > > Expected output: > > $ ./test_cgroup_attach > > #override:PASS > > #multi:PASS > > #autodetach:PASS > > test_cgroup_attach:PASS > > > > On a kernel without auto-detaching: > > $ ./test_cgroup_attach > > #override:PASS > > #multi:PASS > > #autodetach:FAIL > > test_cgroup_attach:FAIL > > > > Signed-off-by: Roman Gushchin <guro@fb.com> > > Looks good to me. It will be good if you can add test_cgroup_attach > to .gitignore to avoid it shows up in `git status`. With that, > Here it is! Thanks! --- From 150fb4db7ab37f0e8b6482386e8830f3d11b64e1 Mon Sep 17 00:00:00 2001 From: Roman Gushchin <guro@fb.com> Date: Thu, 23 May 2019 16:46:58 -0700 Subject: [PATCH bpf-next] selftests/bpf: add test_cgroup_attach to .gitignore Add test_cgroup_attach binary to .gitignore. Signed-off-by: Roman Gushchin <guro@fb.com> --- tools/testing/selftests/bpf/.gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore index dd5d69529382..86a546e5e4db 100644 --- a/tools/testing/selftests/bpf/.gitignore +++ b/tools/testing/selftests/bpf/.gitignore @@ -22,6 +22,7 @@ test_lirc_mode2_user get_cgroup_id_user test_skb_cgroup_id_user test_socket_cookie +test_cgroup_attach test_cgroup_storage test_select_reuseport test_flow_dissector -- 2.20.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 bpf-next 0/4] cgroup bpf auto-detachment [not found] <20190523194532.2376233-1-guro@fb.com> [not found] ` <20190523194532.2376233-5-guro@fb.com> @ 2019-05-24 21:03 ` Alexei Starovoitov 2019-05-24 21:40 ` Roman Gushchin 1 sibling, 1 reply; 5+ messages in thread From: Alexei Starovoitov @ 2019-05-24 21:03 UTC (permalink / raw) To: Roman Gushchin Cc: Alexei Starovoitov, bpf, Daniel Borkmann, netdev, Tejun Heo, kernel-team, cgroups, Stanislav Fomichev, Yonghong Song, linux-kernel On Thu, May 23, 2019 at 12:45:28PM -0700, Roman Gushchin wrote: > This patchset implements a cgroup bpf auto-detachment functionality: > bpf programs are detached as soon as possible after removal of the > cgroup, without waiting for the release of all associated resources. The idea looks great, but doesn't quite work: $ ./test_cgroup_attach #override:PASS [ 66.475219] BUG: sleeping function called from invalid context at ../include/linux/percpu-rwsem.h:34 [ 66.476095] in_atomic(): 1, irqs_disabled(): 0, pid: 21, name: ksoftirqd/2 [ 66.476706] CPU: 2 PID: 21 Comm: ksoftirqd/2 Not tainted 5.2.0-rc1-00211-g1861420d0162 #1564 [ 66.477595] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.el7 04/01/2014 [ 66.478360] Call Trace: [ 66.478591] dump_stack+0x5b/0x8b [ 66.478892] ___might_sleep+0x22f/0x290 [ 66.479230] cpus_read_lock+0x18/0x50 [ 66.479550] static_key_slow_dec+0x41/0x70 [ 66.479914] cgroup_bpf_release+0x1a6/0x400 [ 66.480285] percpu_ref_switch_to_atomic_rcu+0x203/0x330 [ 66.480754] rcu_core+0x475/0xcc0 [ 66.481047] ? switch_mm_irqs_off+0x684/0xa40 [ 66.481422] ? rcu_note_context_switch+0x260/0x260 [ 66.481842] __do_softirq+0x1cf/0x5ff [ 66.482174] ? takeover_tasklets+0x5f0/0x5f0 [ 66.482542] ? smpboot_thread_fn+0xab/0x780 [ 66.482911] run_ksoftirqd+0x1a/0x40 [ 66.483225] smpboot_thread_fn+0x3ad/0x780 [ 66.483583] ? sort_range+0x20/0x20 [ 66.483894] ? __kthread_parkme+0xb0/0x190 [ 66.484253] ? sort_range+0x20/0x20 [ 66.484562] ? sort_range+0x20/0x20 [ 66.484878] kthread+0x2e2/0x3e0 [ 66.485166] ? kthread_create_worker_on_cpu+0xb0/0xb0 [ 66.485620] ret_from_fork+0x1f/0x30 Same test runs fine before the patches. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 bpf-next 0/4] cgroup bpf auto-detachment 2019-05-24 21:03 ` [PATCH v3 bpf-next 0/4] cgroup bpf auto-detachment Alexei Starovoitov @ 2019-05-24 21:40 ` Roman Gushchin 0 siblings, 0 replies; 5+ messages in thread From: Roman Gushchin @ 2019-05-24 21:40 UTC (permalink / raw) To: Alexei Starovoitov Cc: Alexei Starovoitov, bpf@vger.kernel.org, Daniel Borkmann, netdev@vger.kernel.org, Tejun Heo, Kernel Team, cgroups@vger.kernel.org, Stanislav Fomichev, Yonghong Song, linux-kernel@vger.kernel.org On Fri, May 24, 2019 at 02:03:23PM -0700, Alexei Starovoitov wrote: > On Thu, May 23, 2019 at 12:45:28PM -0700, Roman Gushchin wrote: > > This patchset implements a cgroup bpf auto-detachment functionality: > > bpf programs are detached as soon as possible after removal of the > > cgroup, without waiting for the release of all associated resources. > > The idea looks great, but doesn't quite work: > > $ ./test_cgroup_attach > #override:PASS > [ 66.475219] BUG: sleeping function called from invalid context at ../include/linux/percpu-rwsem.h:34 > [ 66.476095] in_atomic(): 1, irqs_disabled(): 0, pid: 21, name: ksoftirqd/2 > [ 66.476706] CPU: 2 PID: 21 Comm: ksoftirqd/2 Not tainted 5.2.0-rc1-00211-g1861420d0162 #1564 > [ 66.477595] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.el7 04/01/2014 > [ 66.478360] Call Trace: > [ 66.478591] dump_stack+0x5b/0x8b > [ 66.478892] ___might_sleep+0x22f/0x290 > [ 66.479230] cpus_read_lock+0x18/0x50 > [ 66.479550] static_key_slow_dec+0x41/0x70 > [ 66.479914] cgroup_bpf_release+0x1a6/0x400 > [ 66.480285] percpu_ref_switch_to_atomic_rcu+0x203/0x330 > [ 66.480754] rcu_core+0x475/0xcc0 > [ 66.481047] ? switch_mm_irqs_off+0x684/0xa40 > [ 66.481422] ? rcu_note_context_switch+0x260/0x260 > [ 66.481842] __do_softirq+0x1cf/0x5ff > [ 66.482174] ? takeover_tasklets+0x5f0/0x5f0 > [ 66.482542] ? smpboot_thread_fn+0xab/0x780 > [ 66.482911] run_ksoftirqd+0x1a/0x40 > [ 66.483225] smpboot_thread_fn+0x3ad/0x780 > [ 66.483583] ? sort_range+0x20/0x20 > [ 66.483894] ? __kthread_parkme+0xb0/0x190 > [ 66.484253] ? sort_range+0x20/0x20 > [ 66.484562] ? sort_range+0x20/0x20 > [ 66.484878] kthread+0x2e2/0x3e0 > [ 66.485166] ? kthread_create_worker_on_cpu+0xb0/0xb0 > [ 66.485620] ret_from_fork+0x1f/0x30 > > Same test runs fine before the patches. > Ouch, static_branch_dec() might block, so it's not possible to call it from percpu ref counter release callback. It's not what I expected, tbh. Good catch, thanks! ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-05-24 21:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20190523194532.2376233-1-guro@fb.com>
[not found] ` <20190523194532.2376233-5-guro@fb.com>
2019-05-23 23:09 ` [PATCH v3 bpf-next 4/4] selftests/bpf: add auto-detach test Yonghong Song
2019-05-23 23:43 ` Roman Gushchin
2019-05-23 23:49 ` Roman Gushchin
2019-05-24 21:03 ` [PATCH v3 bpf-next 0/4] cgroup bpf auto-detachment Alexei Starovoitov
2019-05-24 21:40 ` Roman Gushchin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).