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=-5.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no 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 49900C4363D for ; Thu, 24 Sep 2020 21:24:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0109B235FD for ; Thu, 24 Sep 2020 21:24:48 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="BNMWfS1u" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726280AbgIXVYs (ORCPT ); Thu, 24 Sep 2020 17:24:48 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:32798 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726185AbgIXVYs (ORCPT ); Thu, 24 Sep 2020 17:24:48 -0400 Dkim-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1600982686; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=/24kjoQ/Bjx4xRwwYbCaHv77a28OhUHxxuJfybtFHzo=; b=BNMWfS1uiGGiqR+9t9qvbloUUGetInSxDLsRWlY+umw92sWWS84vXbCZNug0OHwh1nmnkz gnKH5pbHYah0zL+CHFRI61ApU5xUxU/S7v3BEGjYbRbJkx8Xp1O4SDF7yHRTuoCWY4MWCk oyEFt/YKPvyBajxFtn+hirAqYe5cBZw= Received: from mail-pj1-f70.google.com (mail-pj1-f70.google.com [209.85.216.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-211-J8t3pzymOHCShcRE5MdfSQ-1; Thu, 24 Sep 2020 17:24:43 -0400 X-MC-Unique: J8t3pzymOHCShcRE5MdfSQ-1 Received: by mail-pj1-f70.google.com with SMTP id d21so271775pjw.0 for ; Thu, 24 Sep 2020 14:24:43 -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=/24kjoQ/Bjx4xRwwYbCaHv77a28OhUHxxuJfybtFHzo=; b=a0he8TUhzrtb4rBWCpWVCbqcjKQArerS6ZFWfB4vnQ7w6MVPBwNQsX7tmrWjQdu3RO vhEt3iEbQ81WnyQZWlqKRW4GazsOT9X4poSnyYWy2zzRjNaj9K3K/k0zbEH3chTYZhvP 2DbntbANMaVcRz1IAOOhf8iY896FHN+YewFluWBGykh6m//ODevC3KQcZWcxPNsUXVWL OkNJ2foPnnuu3C5Qd30Ja9rqnMoxAOzHq7U5njZ1KjPX0/2QC2+8N4XRfiDA1COgbPsx xaHvj+2k4nr1xKJykWD3tfM+6ByrDREJ0MlUQedlsXBmsIgFTABmZwNKuGbZnIic83jM LxIQ== X-Gm-Message-State: AOAM532T2ZexxYpiG+gpQSKm1OOFKLh0OlDFFDGx0d0plQWEtYVc334t MODl4l2tAKFr5dDrqWhOJgC3yu9ahDWdmfK2tsnE6QoJyj5S7kK0EC28Aj6vpBMSmmtDIC9+Gfy Pe/qnjhpVBly9 X-Received: by 2002:a62:3547:0:b029:142:2501:35d4 with SMTP id c68-20020a6235470000b0290142250135d4mr984642pfa.52.1600982682355; Thu, 24 Sep 2020 14:24:42 -0700 (PDT) X-Google-Smtp-Source: ABdhPJw34M63oCQrylVcA36C+3yOWw4i+mf8TdGkoGu7DYjwCeD70FD1KoJvBhFS6J86py6KDv4q2g== X-Received: by 2002:a62:3547:0:b029:142:2501:35d4 with SMTP id c68-20020a6235470000b0290142250135d4mr984614pfa.52.1600982681981; Thu, 24 Sep 2020 14:24:41 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([45.145.92.2]) by smtp.gmail.com with ESMTPSA id 134sm389036pfa.93.2020.09.24.14.24.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 24 Sep 2020 14:24:40 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id 04038183A90; Thu, 24 Sep 2020 23:24:35 +0200 (CEST) From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= To: Andrii Nakryiko Cc: Alexei Starovoitov , Alexei Starovoitov , Daniel Borkmann , Martin KaFai Lau , Song Liu , Yonghong Song , Andrii Nakryiko , John Fastabend , Jiri Olsa , Eelco Chaudron , KP Singh , Networking , bpf Subject: Re: [PATCH bpf-next v8 04/11] bpf: move prog->aux->linked_prog and trampoline into bpf_link on attach In-Reply-To: References: <160079991372.8301.10648588027560707258.stgit@toke.dk> <160079991808.8301.6462172487971110332.stgit@toke.dk> <20200924001439.qitbu5tmzz55ck4z@ast-mbp.dhcp.thefacebook.com> <874knn1bw4.fsf@toke.dk> X-Clacks-Overhead: GNU Terry Pratchett Date: Thu, 24 Sep 2020 23:24:35 +0200 Message-ID: <87zh5ec1gs.fsf@toke.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org Andrii Nakryiko writes: > On Thu, Sep 24, 2020 at 7:36 AM Toke H=C3=B8iland-J=C3=B8rgensen wrote: >> >> Alexei Starovoitov writes: >> >> > On Tue, Sep 22, 2020 at 08:38:38PM +0200, Toke H=C3=83=C6=92=C3=82=C2= =B8iland-J=C3=83=C6=92=C3=82=C2=B8rgensen wrote: >> >> @@ -746,7 +748,9 @@ struct bpf_prog_aux { >> >> u32 max_rdonly_access; >> >> u32 max_rdwr_access; >> >> const struct bpf_ctx_arg_aux *ctx_arg_info; >> >> - struct bpf_prog *linked_prog; >> > >> > This change breaks bpf_preload and selftests test_bpffs. >> > There is really no excuse not to run the selftests. >> >> I did run the tests, and saw no more breakages after applying my patches >> than before. Which didn't catch this, because this is the current state >> of bpf-next selftests: >> >> # ./test_progs | grep FAIL >> test_lookup_update:FAIL:map1_leak inner_map1 leaked! >> #10/1 lookup_update:FAIL >> #10 btf_map_in_map:FAIL > > this failure suggests you are not running the latest kernel, btw I did see that discussion (about the reverted patch), and figured that was the case. So I did a 'git pull' just before testing, and still got this. $ git describe HEAD v5.9-rc3-2681-g182bf3f3ddb6 so any other ideas? :) >> configure_stack:FAIL:BPF load failed; run with -vv for more info >> #72 sk_assign:FAIL (and what about this one, now that I'm asking?) >> test_test_bpffs:FAIL:bpffs test failed 255 >> #96 test_bpffs:FAIL >> Summary: 113/844 PASSED, 14 SKIPPED, 4 FAILED >> >> The test_bpffs failure happens because the umh is missing from the >> .config; and when I tried to fix this I ended up with: > > yeah, seems like selftests/bpf/config needs to be updated to mention > UMH-related config values: > > CONFIG_BPF_PRELOAD=3Dy > CONFIG_BPF_PRELOAD_UMD=3Dm|y > > with that test_bpffs shouldn't fail on master Yup, did get that far, and got the below... >> >> [..] >> CC [M] kernel/bpf/preload/bpf_preload_kern.o >> >> Auto-detecting system features: >> ... libelf: [ OFF ] >> ... zlib: [ OFF ] >> ... bpf: [ OFF ] >> >> No libelf found > > might be worthwhile to look into why detection fails, might be > something with Makefiles or your environment I think it's actually another instance of the bug I fixed with this commit: 1eb832ac2dee ("tools/bpf: build: Make sure resolve_btfids cleans up after i= tself") which I finally remembered after being tickled by the error message seeming familiar. And indeed, manually removing the 'feature' directory in kernel/bpf/preload seems to fix the issue, so I'm planning to go fix that Makefile as well... >> ...which I just put down to random breakage, turned off the umh and >> continued on my way (ignoring the failed test). Until you wrote this I >> did not suspect this would be something I needed to pay attention to. >> Now that you did mention it, I'll obviously go investigate some more, my >> point is just that in this instance it's not accurate to assume I just >> didn't run the tests... :) > > Don't just assume some tests are always broken. Either ask or > investigate on your own. Such cases do happen from time to time while > we wait for a fix in bpf to get merged into bpf-next or vice versa, > but it's rare. We now have two different CI systems running selftests > all the time, in addition to running them locally as well, so any > permanent test failure is very apparent and annoying, so we fix them > quickly. So, when in doubt - ask or fix. That's good to know; and I do think the situation has improved immensely. There was a time when the selftests broke every other week (or so it felt, at least), and I guess I'm still a bit scarred from that. One thing that would be really useful would be to have a 'reference config' or something like that. Missing config options are a common reason for test failures (as we have just seen above), and it's not always obvious which option is missing for each test. Even something like grepping .config for BPF doesn't catch everything. If you already have a CI running, just pointing to that config would be a good start (especially if it has history). In an ideal world I think it would be great if each test could detect whether the kernel has the right config set for its features and abort with a clear error message if it isn't... >> > I think I will just start marking patches as changes-requested when I = see that >> > they break tests without replying and without reviewing. >> > Please respect reviewer's time. >> >> That is completely fine if the tests are working in the first place. And > > They are and hopefully moving forward that would be your assumption. Sure, with the exception of the two tests still failing that I mentioned above. Which I'm hoping you can help figure out the reason for :) -Toke