From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 28A1FCA9ECB for ; Thu, 31 Oct 2019 18:18:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 003F7208E3 for ; Thu, 31 Oct 2019 18:18:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726602AbfJaSSP convert rfc822-to-8bit (ORCPT ); Thu, 31 Oct 2019 14:18:15 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57796 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729027AbfJaSSP (ORCPT ); Thu, 31 Oct 2019 14:18:15 -0400 Received: from mail-lf1-f72.google.com (mail-lf1-f72.google.com [209.85.167.72]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7A047C056808 for ; Thu, 31 Oct 2019 18:18:14 +0000 (UTC) Received: by mail-lf1-f72.google.com with SMTP id y188so1611766lfc.6 for ; Thu, 31 Oct 2019 11:18:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references:date :message-id:mime-version:content-transfer-encoding; bh=hAO8gdMBwEsb3+ZUjKPzDGZ8KzTY+WTufgu7EjXj1ak=; b=gsnVKyjy/JBwO1ChBXSKpqjktVzVjUmxSB5dd052fYhZKQ1KPbrp+AfaNutkfPk/ni 2Aw0aiRPnVTy0A/O41ipoy3jeT3Yd9ZXLTVGQsUxzqZI0bic6rglA8NhcE20aw1Y6ep5 W6O7EayiIit/m+gqzlx0JQ9pzZ9n4utU9WeUzoaPPJYc7sSs9yE6WYYK0t+s+7QIzvyA AxgejScqr6WIXMMnx7KbiJz18s0dnZMikDazhCDAJfCmD4GarqYEhERh0dt6pd6LO2oA KCrbOAae6uPlVgPQlUTGOwuefqFfmfUB6lb5o1NbgpSKkf8BGXbDkjIuWGFuvdk05SdT ISSw== X-Gm-Message-State: APjAAAXpEX0SkAAIqIjZvTuqko9LQA1EgxWLfT15/Y4dg5ozSRWUI0Lw uSWVke70thxvZXmc+5Pe7hdBCndUlY8S1t0Fmt5xnlHE/W2qM275EM3zNy+92R6zgdLDG9ZdfzM erQWf//WOTeFO X-Received: by 2002:a2e:93d7:: with SMTP id p23mr5134804ljh.251.1572545892337; Thu, 31 Oct 2019 11:18:12 -0700 (PDT) X-Google-Smtp-Source: APXvYqywIUuJCljaOckz21Hz3YoX+FPI0NftFuyHwoDO6aGwGg0dt2LL/+dQbjNswvIG65jLu3YFsg== X-Received: by 2002:a2e:93d7:: with SMTP id p23mr5134793ljh.251.1572545892132; Thu, 31 Oct 2019 11:18:12 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([2a00:7660:6da:443::2]) by smtp.gmail.com with ESMTPSA id o26sm1676354lfi.57.2019.10.31.11.18.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 31 Oct 2019 11:18:11 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id B34941818B5; Thu, 31 Oct 2019 19:18:10 +0100 (CET) From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= To: Andrii Nakryiko Cc: Daniel Borkmann , Alexei Starovoitov , Martin KaFai Lau , Song Liu , Yonghong Song , Jesper Dangaard Brouer , David Miller , Networking , bpf Subject: Re: [PATCH bpf-next v4 5/5] selftests: Add tests for automatic map pinning In-Reply-To: References: <157237796219.169521.2129132883251452764.stgit@toke.dk> <157237796779.169521.16760790702202542513.stgit@toke.dk> X-Clacks-Overhead: GNU Terry Pratchett Date: Thu, 31 Oct 2019 19:18:10 +0100 Message-ID: <87wockn5i5.fsf@toke.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Sender: bpf-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org Andrii Nakryiko writes: > On Tue, Oct 29, 2019 at 12:39 PM Toke Høiland-Jørgensen wrote: >> >> From: Toke Høiland-Jørgensen >> >> This adds a new BPF selftest to exercise the new automatic map pinning >> code. >> >> Signed-off-by: Toke Høiland-Jørgensen >> --- >> tools/testing/selftests/bpf/prog_tests/pinning.c | 157 ++++++++++++++++++++++ >> tools/testing/selftests/bpf/progs/test_pinning.c | 29 ++++ >> 2 files changed, 186 insertions(+) >> create mode 100644 tools/testing/selftests/bpf/prog_tests/pinning.c >> create mode 100644 tools/testing/selftests/bpf/progs/test_pinning.c >> >> diff --git a/tools/testing/selftests/bpf/prog_tests/pinning.c b/tools/testing/selftests/bpf/prog_tests/pinning.c >> new file mode 100644 >> index 000000000000..71f7dc51edc7 >> --- /dev/null >> +++ b/tools/testing/selftests/bpf/prog_tests/pinning.c >> @@ -0,0 +1,157 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> + >> +#include >> +#include >> +#include >> +#include >> + >> +__u32 get_map_id(struct bpf_object *obj, const char *name) >> +{ >> + __u32 map_info_len, duration, retval; >> + struct bpf_map_info map_info = {}; >> + struct bpf_map *map; >> + int err; >> + >> + map_info_len = sizeof(map_info); >> + >> + map = bpf_object__find_map_by_name(obj, name); >> + if (CHECK(!map, "find map", "NULL map")) >> + return 0; >> + >> + err = bpf_obj_get_info_by_fd(bpf_map__fd(map), >> + &map_info, &map_info_len); >> + CHECK(err, "get map info", "err %d errno %d", err, errno); >> + return map_info.id; >> +} >> + >> +void test_pinning(void) >> +{ >> + __u32 duration, retval, size, map_id, map_id2; >> + const char *custpinpath = "/sys/fs/bpf/custom/pinmap"; >> + const char *nopinpath = "/sys/fs/bpf/nopinmap"; >> + const char *custpath = "/sys/fs/bpf/custom"; > > Should this test mount/unmount (if necessary) /sys/fs/bpf? They will > all fail if BPF FS is not mounted, right? Yeah; I was kinda expecting that the test harness takes care of this. Is it really a good idea for a selftest to mess with mount()? >> + const char *pinpath = "/sys/fs/bpf/pinmap"; >> + const char *file = "./test_pinning.o"; >> + struct stat statbuf = {}; >> + struct bpf_object *obj; >> + struct bpf_map *map; >> + DECLARE_LIBBPF_OPTS(bpf_object_open_opts, opts, >> + .pin_root_path = custpath, >> + ); >> + >> + int err; >> + obj = bpf_object__open_file(file, NULL); >> + if (CHECK_FAIL(libbpf_get_error(obj))) >> + return; >> + >> + err = bpf_object__load(obj); >> + if (CHECK(err, "default load", "err %d errno %d\n", err, errno)) >> + goto out; >> + >> + /* check that pinmap was pinned */ >> + err = stat(pinpath, &statbuf); >> + if (CHECK(err, "stat pinpath", "err %d errno %d\n", err, errno)) >> + goto out; >> + >> + /* check that nopinmap was *not* pinned */ >> + err = stat(nopinpath, &statbuf); >> + if (CHECK(!err || errno != ENOENT, "stat nopinpath", >> + "err %d errno %d\n", err, errno)) >> + goto out; >> + >> + map_id = get_map_id(obj, "pinmap"); > > something wrong with whitespaces here? can you please run > scripts/checkpatch.pl to double-check? Yup, some space got in where a tab should be >> + if (!map_id) >> + goto out; >> + >> + bpf_object__close(obj); >> + >> + obj = bpf_object__open_file(file, NULL); >> + if (CHECK_FAIL(libbpf_get_error(obj))) > > obj = NULL here before you go to out Yup > >> + goto out; >> + >> + err = bpf_object__load(obj); >> + if (CHECK(err, "default load", "err %d errno %d\n", err, errno)) >> + goto out; >> + > > [...] > >> + err = rmdir(custpath); >> + if (CHECK(err, "rmdir custpindir", "err %d errno %d\n", err, errno)) >> + goto out; >> + >> + bpf_object__close(obj); >> + >> + /* test auto-pinning at custom path with open opt */ >> + obj = bpf_object__open_file(file, &opts); >> + if (CHECK_FAIL(libbpf_get_error(obj))) >> + return; > > obj = NULL; goto out; to ensure pinpath is unlinked? Yeah. >> + >> + err = bpf_object__load(obj); >> + if (CHECK(err, "custom load", "err %d errno %d\n", err, errno)) >> + goto out; >> + >> + /* check that pinmap was pinned at the custom path */ >> + err = stat(custpinpath, &statbuf); >> + if (CHECK(err, "stat custpinpath", "err %d errno %d\n", err, errno)) >> + goto out; >> + >> +out: >> + unlink(pinpath); >> + unlink(nopinpath); >> + unlink(custpinpath); >> + rmdir(custpath); >> + if (obj) >> + bpf_object__close(obj); >> +} >> diff --git a/tools/testing/selftests/bpf/progs/test_pinning.c b/tools/testing/selftests/bpf/progs/test_pinning.c >> new file mode 100644 >> index 000000000000..ff2d7447777e >> --- /dev/null >> +++ b/tools/testing/selftests/bpf/progs/test_pinning.c >> @@ -0,0 +1,29 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> + >> +#include >> +#include "bpf_helpers.h" >> + >> +int _version SEC("version") = 1; >> + >> +struct { >> + __uint(type, BPF_MAP_TYPE_ARRAY); >> + __uint(max_entries, 1); >> + __type(key, __u32); >> + __type(value, __u64); >> + __uint(pinning, LIBBPF_PIN_BY_NAME); >> +} pinmap SEC(".maps"); >> + >> +struct { >> + __uint(type, BPF_MAP_TYPE_ARRAY); >> + __uint(max_entries, 1); >> + __type(key, __u32); >> + __type(value, __u64); >> +} nopinmap SEC(".maps"); > > > would be nice to ensure that __uint(pinning, LIBBPF_PIN_NONE) also > works as expected, do you mind adding one extra map? Sure, can do... >> + >> +SEC("xdp_prog") >> +int _xdp_prog(struct xdp_md *xdp) >> +{ >> + return XDP_PASS; >> +} >> + >> +char _license[] SEC("license") = "GPL"; >>