MIME-Version: 1.0
References: <20230203031742.1730761-1-imagedong@tencent.com>
 <20230203031742.1730761-3-imagedong@tencent.com> <CAEf4BzYh90NyyYvfTT=M=-KLspydMX4PZK8jCwNDydAP=kFgYw@mail.gmail.com>
 <CADxym3a6_wBHW_c_ZYtZ5QXbbunhKxau6k-fn4TNrn+6qzW6fw@mail.gmail.com>
In-Reply-To: <CADxym3a6_wBHW_c_ZYtZ5QXbbunhKxau6k-fn4TNrn+6qzW6fw@mail.gmail.com>
From:   Andrii Nakryiko <andrii.nakryiko@gmail.com>
Date:   Tue, 7 Feb 2023 14:50:26 -0800
Message-ID: <CAEf4BzZAo6Bfio3pbY3j5yUDArCbdiWPC-r=XhFM9Bwq+4VVMg@mail.gmail.com>
Subject: Re: [PATCH bpf-next 2/2] selftests/bpf: add test for legacy/perf
 kprobe/uprobe attach mode
To:     Menglong Dong <menglong8.dong@gmail.com>
Cc:     alan.maguire@oracle.com, ast@kernel.org, daniel@iogearbox.net,
        andrii@kernel.org, martin.lau@linux.dev, song@kernel.org,
        yhs@fb.com, john.fastabend@gmail.com, kpsingh@kernel.org,
        sdf@google.com, haoluo@google.com, jolsa@kernel.org,
        bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
        Menglong Dong <imagedong@tencent.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Feb 6, 2023 at 6:39 PM Menglong Dong <menglong8.dong@gmail.com> wrote:
>
> On Tue, Feb 7, 2023 at 4:05 AM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Thu, Feb 2, 2023 at 7:18 PM <menglong8.dong@gmail.com> wrote:
> > >
> > > From: Menglong Dong <imagedong@tencent.com>
> > >
> > > Add the testing for kprobe/uprobe attaching in legacy and perf mode.
> > > And the testing passed:
> > >
> > > ./test_progs -t attach_probe
> > > $5       attach_probe:OK
> > > Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
> > >
> > > Signed-off-by: Menglong Dong <imagedong@tencent.com>
> > > ---
> >
> > Do you mind refactoring attach_probe test into multiple subtests,
> > where each subtest will only test one of the attach mode and type. The
> > reason is that libbpf CI runs tests with latest selftests and libbpf
> > against old kernels (4.9 and 5.5, currently). Due to attach_probe
> > testing all these uprobe/kprobe attach modes with extra features (like
> > cookie, ref count, etc), we had to disable attach_probe test in libbpf
> > CI on old kernels.
> >
> > If we can split each individual uprobe/kprobe mode, that will give us
> > flexibility to selectively allowlist those tests that don't force
> > libbpf to use newer features (like cookies, LINK or PERF mode, etc).
> >
> > It would be a great improvement and highly appreciated! If you don't
> > mind doing this, let's do the split of existing use cases into subtest
> > in a separate patch, and then add PERF/LEGACY/LINK mode tests on top
> > of that patch.
> >
>
> Of course, with pleasure. For the existing use cases, we split it into
> subtests, such as:
>
>   kprobe/kretprobe auto attach
>   kprobe/kretprobe manual attach
>   uprobe/uretprobe ref_ctr test
>   uprobe/uretprobe auto attach
>   sleepable kprobe/uprobe
>   ......
>
> Am I right?

I haven't analysed all the different cases, but roughly it makes
sense. With more granular subtests we can also drop `legacy` flag and
rely on subtest allowlisting in CI.

>
> Thanks!
> Dongmeng Long
>
> >
> > >  .../selftests/bpf/prog_tests/attach_probe.c   | 61 ++++++++++++++++++-
> > >  .../selftests/bpf/progs/test_attach_probe.c   | 32 ++++++++++
> > >  2 files changed, 92 insertions(+), 1 deletion(-)
> > >
> >
> > [...]