diff mbox series

[bpf-next,7/7] selftests/bpf: Add a new test based on loop6.c

Message ID 20230330055636.93471-1-yhs@fb.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series bpf: Improve verifier for cond_op and spilled loop index variables | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 20 this patch: 20
netdev/cc_maintainers warning 15 maintainers not CCed: mykolal@fb.com llvm@lists.linux.dev deso@posteo.net song@kernel.org shuah@kernel.org sdf@google.com haoluo@google.com trix@redhat.com nathan@kernel.org john.fastabend@gmail.com ndesaulniers@google.com kpsingh@kernel.org jolsa@kernel.org martin.lau@linux.dev linux-kselftest@vger.kernel.org
netdev/build_clang success Errors and warnings before: 18 this patch: 18
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 20 this patch: 20
netdev/checkpatch fail CHECK: No space is necessary after a cast ERROR: Bad function definition - void test_verif_scale_loop7() should probably be void test_verif_scale_loop7(void) WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline fail Was 0 now: 2
bpf/vmtest-bpf-next-VM_Test-10 fail Logs for test_maps on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-15 success Logs for test_progs on s390x with gcc
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-2 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-3 success Logs for build for aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-5 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-7 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-4 success Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for test_maps on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-11 success Logs for test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-12 success Logs for test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-17 success Logs for test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-18 success Logs for test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-19 success Logs for test_progs_no_alu32 on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-21 success Logs for test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-22 success Logs for test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-23 success Logs for test_progs_no_alu32_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for test_progs_no_alu32_parallel on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-25 success Logs for test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-26 success Logs for test_progs_no_alu32_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-27 success Logs for test_progs_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-28 success Logs for test_progs_parallel on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-29 success Logs for test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-30 success Logs for test_progs_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-31 success Logs for test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-32 success Logs for test_verifier on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-34 success Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-35 success Logs for test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-13 success Logs for test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-14 success Logs for test_progs on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-16 success Logs for test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-33 success Logs for test_verifier on s390x with gcc

Commit Message

Yonghong Song March 30, 2023, 5:56 a.m. UTC 
With LLVM commit [1], loop6.c will fail verification without Commit 3c2611bac08a
("selftests/bpf: Fix trace_virtqueue_add_sgs test issue with LLVM 17.").
Also, there is an effort to fix LLVM since LLVM17 may be used by old kernels
for bpf development.

A new test is added by manually doing similar transformation in [1]
so it can be used to test related verifier changes.

  [1] https://reviews.llvm.org/D143726

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 .../bpf/prog_tests/bpf_verif_scale.c          |   5 +
 tools/testing/selftests/bpf/progs/loop7.c     | 102 ++++++++++++++++++
 2 files changed, 107 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/loop7.c

Comments

Alexei Starovoitov April 3, 2023, 1:39 a.m. UTC | #1 
On Wed, Mar 29, 2023 at 10:56:36PM -0700, Yonghong Song wrote:
> With LLVM commit [1], loop6.c will fail verification without Commit 3c2611bac08a
> ("selftests/bpf: Fix trace_virtqueue_add_sgs test issue with LLVM 17.").
> Also, there is an effort to fix LLVM since LLVM17 may be used by old kernels
> for bpf development.
> 
> A new test is added by manually doing similar transformation in [1]
> so it can be used to test related verifier changes.
> 
>   [1] https://reviews.llvm.org/D143726
> 
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
>  .../bpf/prog_tests/bpf_verif_scale.c          |   5 +
>  tools/testing/selftests/bpf/progs/loop7.c     | 102 ++++++++++++++++++
>  2 files changed, 107 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/progs/loop7.c
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
> index 731c343897d8..cb708235e654 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
> @@ -180,6 +180,11 @@ void test_verif_scale_loop6()
>  	scale_test("loop6.bpf.o", BPF_PROG_TYPE_KPROBE, false);
>  }
>  
> +void test_verif_scale_loop7()
> +{
> +	scale_test("loop7.bpf.o", BPF_PROG_TYPE_KPROBE, false);
> +}
> +
>  void test_verif_scale_strobemeta()
>  {
>  	/* partial unroll. 19k insn in a loop.
> diff --git a/tools/testing/selftests/bpf/progs/loop7.c b/tools/testing/selftests/bpf/progs/loop7.c
> new file mode 100644
> index 000000000000..b234ed6f0038
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/loop7.c
> @@ -0,0 +1,102 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/ptrace.h>
> +#include <stddef.h>
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +#include "bpf_misc.h"
> +
> +char _license[] SEC("license") = "GPL";
> +
> +/* typically virtio scsi has max SGs of 6 */
> +#define VIRTIO_MAX_SGS	6
> +
> +/* Verifier will fail with SG_MAX = 128. The failure can be
> + * workarounded with a smaller SG_MAX, e.g. 10.
> + */
> +#define WORKAROUND
> +#ifdef WORKAROUND
> +#define SG_MAX		10
> +#else
> +/* typically virtio blk has max SEG of 128 */
> +#define SG_MAX		128
> +#endif
> +
> +#define SG_CHAIN	0x01UL
> +#define SG_END		0x02UL
> +
> +struct scatterlist {
> +	unsigned long   page_link;
> +	unsigned int    offset;
> +	unsigned int    length;
> +};
> +
> +#define sg_is_chain(sg)		((sg)->page_link & SG_CHAIN)
> +#define sg_is_last(sg)		((sg)->page_link & SG_END)
> +#define sg_chain_ptr(sg)	\
> +	((struct scatterlist *) ((sg)->page_link & ~(SG_CHAIN | SG_END)))
> +
> +static inline struct scatterlist *__sg_next(struct scatterlist *sgp)
> +{
> +	struct scatterlist sg;
> +
> +	bpf_probe_read_kernel(&sg, sizeof(sg), sgp);
> +	if (sg_is_last(&sg))
> +		return NULL;
> +
> +	sgp++;
> +
> +	bpf_probe_read_kernel(&sg, sizeof(sg), sgp);
> +	if (sg_is_chain(&sg))
> +		sgp = sg_chain_ptr(&sg);
> +
> +	return sgp;
> +}
> +
> +static inline struct scatterlist *get_sgp(struct scatterlist **sgs, int i)
> +{
> +	struct scatterlist *sgp;
> +
> +	bpf_probe_read_kernel(&sgp, sizeof(sgp), sgs + i);
> +	return sgp;
> +}
> +
> +int config = 0;
> +int result = 0;
> +
> +SEC("kprobe/virtqueue_add_sgs")
> +int BPF_KPROBE(trace_virtqueue_add_sgs, void *unused, struct scatterlist **sgs,
> +	       unsigned int out_sgs, unsigned int in_sgs)
> +{
> +	struct scatterlist *sgp = NULL;
> +	__u64 length1 = 0, length2 = 0;
> +	unsigned int i, n, len, upper;
> +
> +	if (config != 0)
> +		return 0;
> +
> +	upper = out_sgs < VIRTIO_MAX_SGS ? out_sgs : VIRTIO_MAX_SGS;
> +	for (i = 0; i < upper; i++) {

since this test is doing manual hoistMinMax, let's keep __sink() in test 6,
so we guaranteed to have both flavors regardless of compiler choices?
Yonghong Song April 3, 2023, 3:59 a.m. UTC | #2 
On 4/2/23 6:39 PM, Alexei Starovoitov wrote:
> On Wed, Mar 29, 2023 at 10:56:36PM -0700, Yonghong Song wrote:
>> With LLVM commit [1], loop6.c will fail verification without Commit 3c2611bac08a
>> ("selftests/bpf: Fix trace_virtqueue_add_sgs test issue with LLVM 17.").
>> Also, there is an effort to fix LLVM since LLVM17 may be used by old kernels
>> for bpf development.
>>
>> A new test is added by manually doing similar transformation in [1]
>> so it can be used to test related verifier changes.
>>
>>    [1] https://reviews.llvm.org/D143726
>>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>>   .../bpf/prog_tests/bpf_verif_scale.c          |   5 +
>>   tools/testing/selftests/bpf/progs/loop7.c     | 102 ++++++++++++++++++
>>   2 files changed, 107 insertions(+)
>>   create mode 100644 tools/testing/selftests/bpf/progs/loop7.c
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
>> index 731c343897d8..cb708235e654 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
>> @@ -180,6 +180,11 @@ void test_verif_scale_loop6()
>>   	scale_test("loop6.bpf.o", BPF_PROG_TYPE_KPROBE, false);
>>   }
>>   
>> +void test_verif_scale_loop7()
>> +{
>> +	scale_test("loop7.bpf.o", BPF_PROG_TYPE_KPROBE, false);
>> +}
>> +
>>   void test_verif_scale_strobemeta()
>>   {
>>   	/* partial unroll. 19k insn in a loop.
>> diff --git a/tools/testing/selftests/bpf/progs/loop7.c b/tools/testing/selftests/bpf/progs/loop7.c
>> new file mode 100644
>> index 000000000000..b234ed6f0038
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/progs/loop7.c
>> @@ -0,0 +1,102 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#include <linux/ptrace.h>
>> +#include <stddef.h>
>> +#include <linux/bpf.h>
>> +#include <bpf/bpf_helpers.h>
>> +#include <bpf/bpf_tracing.h>
>> +#include "bpf_misc.h"
>> +
>> +char _license[] SEC("license") = "GPL";
>> +
>> +/* typically virtio scsi has max SGs of 6 */
>> +#define VIRTIO_MAX_SGS	6
>> +
>> +/* Verifier will fail with SG_MAX = 128. The failure can be
>> + * workarounded with a smaller SG_MAX, e.g. 10.
>> + */
>> +#define WORKAROUND
>> +#ifdef WORKAROUND
>> +#define SG_MAX		10
>> +#else
>> +/* typically virtio blk has max SEG of 128 */
>> +#define SG_MAX		128
>> +#endif
>> +
>> +#define SG_CHAIN	0x01UL
>> +#define SG_END		0x02UL
>> +
>> +struct scatterlist {
>> +	unsigned long   page_link;
>> +	unsigned int    offset;
>> +	unsigned int    length;
>> +};
>> +
>> +#define sg_is_chain(sg)		((sg)->page_link & SG_CHAIN)
>> +#define sg_is_last(sg)		((sg)->page_link & SG_END)
>> +#define sg_chain_ptr(sg)	\
>> +	((struct scatterlist *) ((sg)->page_link & ~(SG_CHAIN | SG_END)))
>> +
>> +static inline struct scatterlist *__sg_next(struct scatterlist *sgp)
>> +{
>> +	struct scatterlist sg;
>> +
>> +	bpf_probe_read_kernel(&sg, sizeof(sg), sgp);
>> +	if (sg_is_last(&sg))
>> +		return NULL;
>> +
>> +	sgp++;
>> +
>> +	bpf_probe_read_kernel(&sg, sizeof(sg), sgp);
>> +	if (sg_is_chain(&sg))
>> +		sgp = sg_chain_ptr(&sg);
>> +
>> +	return sgp;
>> +}
>> +
>> +static inline struct scatterlist *get_sgp(struct scatterlist **sgs, int i)
>> +{
>> +	struct scatterlist *sgp;
>> +
>> +	bpf_probe_read_kernel(&sgp, sizeof(sgp), sgs + i);
>> +	return sgp;
>> +}
>> +
>> +int config = 0;
>> +int result = 0;
>> +
>> +SEC("kprobe/virtqueue_add_sgs")
>> +int BPF_KPROBE(trace_virtqueue_add_sgs, void *unused, struct scatterlist **sgs,
>> +	       unsigned int out_sgs, unsigned int in_sgs)
>> +{
>> +	struct scatterlist *sgp = NULL;
>> +	__u64 length1 = 0, length2 = 0;
>> +	unsigned int i, n, len, upper;
>> +
>> +	if (config != 0)
>> +		return 0;
>> +
>> +	upper = out_sgs < VIRTIO_MAX_SGS ? out_sgs : VIRTIO_MAX_SGS;
>> +	for (i = 0; i < upper; i++) {
> 
> since this test is doing manual hoistMinMax, let's keep __sink() in test 6,
> so we guaranteed to have both flavors regardless of compiler choices?

Sounds good to me. Will drop patch 6.
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
index 731c343897d8..cb708235e654 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
@@ -180,6 +180,11 @@  void test_verif_scale_loop6()
 	scale_test("loop6.bpf.o", BPF_PROG_TYPE_KPROBE, false);
 }
 
+void test_verif_scale_loop7()
+{
+	scale_test("loop7.bpf.o", BPF_PROG_TYPE_KPROBE, false);
+}
+
 void test_verif_scale_strobemeta()
 {
 	/* partial unroll. 19k insn in a loop.
diff --git a/tools/testing/selftests/bpf/progs/loop7.c b/tools/testing/selftests/bpf/progs/loop7.c
new file mode 100644
index 000000000000..b234ed6f0038
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/loop7.c
@@ -0,0 +1,102 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/ptrace.h>
+#include <stddef.h>
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+
+char _license[] SEC("license") = "GPL";
+
+/* typically virtio scsi has max SGs of 6 */
+#define VIRTIO_MAX_SGS	6
+
+/* Verifier will fail with SG_MAX = 128. The failure can be
+ * workarounded with a smaller SG_MAX, e.g. 10.
+ */
+#define WORKAROUND
+#ifdef WORKAROUND
+#define SG_MAX		10
+#else
+/* typically virtio blk has max SEG of 128 */
+#define SG_MAX		128
+#endif
+
+#define SG_CHAIN	0x01UL
+#define SG_END		0x02UL
+
+struct scatterlist {
+	unsigned long   page_link;
+	unsigned int    offset;
+	unsigned int    length;
+};
+
+#define sg_is_chain(sg)		((sg)->page_link & SG_CHAIN)
+#define sg_is_last(sg)		((sg)->page_link & SG_END)
+#define sg_chain_ptr(sg)	\
+	((struct scatterlist *) ((sg)->page_link & ~(SG_CHAIN | SG_END)))
+
+static inline struct scatterlist *__sg_next(struct scatterlist *sgp)
+{
+	struct scatterlist sg;
+
+	bpf_probe_read_kernel(&sg, sizeof(sg), sgp);
+	if (sg_is_last(&sg))
+		return NULL;
+
+	sgp++;
+
+	bpf_probe_read_kernel(&sg, sizeof(sg), sgp);
+	if (sg_is_chain(&sg))
+		sgp = sg_chain_ptr(&sg);
+
+	return sgp;
+}
+
+static inline struct scatterlist *get_sgp(struct scatterlist **sgs, int i)
+{
+	struct scatterlist *sgp;
+
+	bpf_probe_read_kernel(&sgp, sizeof(sgp), sgs + i);
+	return sgp;
+}
+
+int config = 0;
+int result = 0;
+
+SEC("kprobe/virtqueue_add_sgs")
+int BPF_KPROBE(trace_virtqueue_add_sgs, void *unused, struct scatterlist **sgs,
+	       unsigned int out_sgs, unsigned int in_sgs)
+{
+	struct scatterlist *sgp = NULL;
+	__u64 length1 = 0, length2 = 0;
+	unsigned int i, n, len, upper;
+
+	if (config != 0)
+		return 0;
+
+	upper = out_sgs < VIRTIO_MAX_SGS ? out_sgs : VIRTIO_MAX_SGS;
+	for (i = 0; i < upper; i++) {
+		for (n = 0, sgp = get_sgp(sgs, i); sgp && (n < SG_MAX);
+		     sgp = __sg_next(sgp)) {
+			bpf_probe_read_kernel(&len, sizeof(len), &sgp->length);
+			length1 += len;
+			n++;
+		}
+	}
+
+	upper = in_sgs < VIRTIO_MAX_SGS ? in_sgs : VIRTIO_MAX_SGS;
+	for (i = 0; i < upper; i++) {
+		for (n = 0, sgp = get_sgp(sgs, i); sgp && (n < SG_MAX);
+		     sgp = __sg_next(sgp)) {
+			bpf_probe_read_kernel(&len, sizeof(len), &sgp->length);
+			length2 += len;
+			n++;
+		}
+	}
+
+	config = 1;
+	result = length2 - length1;
+	return 0;
+}