The arm64 Guarded Control Stack (GCS) feature provides support for
hardware protected stacks of return addresses, intended to provide
hardening against return oriented programming (ROP) attacks and to make
it easier to gather call stacks for applications such as profiling.
When GCS is active a secondary stack called the Guarded Control Stack is
maintained, protected with a memory attribute which means that it can
only be written with specific GCS operations. The current GCS pointer
can not be directly written to by userspace. When a BL is executed the
value stored in LR is also pushed onto the GCS, and when a RET is
executed the top of the GCS is popped and compared to LR with a fault
being raised if the values do not match. GCS operations may only be
performed on GCS pages, a data abort is generated if they are not.
The combination of hardware enforcement and lack of extra instructions
in the function entry and exit paths should result in something which
has less overhead and is more difficult to attack than a purely software
implementation like clang's shadow stacks.
This series implements support for use of GCS by userspace, along with
support for use of GCS within KVM guests. It does not enable use of GCS
by either EL1 or EL2, this will be implemented separately. Executables
are started without GCS and must use a prctl() to enable it, it is
expected that this will be done very early in application execution by
the dynamic linker or other startup code. For dynamic linking this will
be done by checking that everything in the executable is marked as GCS
compatible.
x86 has an equivalent feature called shadow stacks, this series depends
on the x86 patches for generic memory management support for the new
guarded/shadow stack page type and shares APIs as much as possible. As
there has been extensive discussion with the wider community around the
ABI for shadow stacks I have as far as practical kept implementation
decisions close to those for x86, anticipating that review would lead to
similar conclusions in the absence of strong reasoning for divergence.
The main divergence I am concious of is that x86 allows shadow stack to
be enabled and disabled repeatedly, freeing the shadow stack for the
thread whenever disabled, while this implementation keeps the GCS
allocated after disable but refuses to reenable it. This is to avoid
races with things actively walking the GCS during a disable, we do
anticipate that some systems will wish to disable GCS at runtime but are
not aware of any demand for subsequently reenabling it.
x86 uses an arch_prctl() to manage enable and disable, since only x86
and S/390 use arch_prctl() a generic prctl() was proposed[1] as part of a
patch set for the equivalent RISC-V zisslpcfi feature which I initially
adopted fairly directly but following review feedback has been revised
quite a bit.
There is an open issue with support for CRIU, on x86 this required the
ability to set the GCS mode via ptrace. This series supports
configuring mode bits other than enable/disable via ptrace but it needs
to be confirmed if this is sufficient.
There's a few bits where I'm not convinced with where I've placed
things, in particular the GCS write operation is in the GCS header not
in uaccess.h, I wasn't sure what was clearest there and am probably too
close to the code to have a clear opinion. The reporting of GCS in
/proc/PID/smaps is also a bit awkward.
The series depends on the x86 shadow stack support:
https://lore.kernel.org/lkml/20230227222957.24501-1-rick.p.edgecombe@intel.…
I've rebased this onto v6.5-rc4 but not included it in the series in
order to avoid confusion with Rick's work and cut down the size of the
series, you can see the branch at:
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/misc.git arm64-gcs
[1] https://lore.kernel.org/lkml/20230213045351.3945824-1-debug@rivosinc.com/
Pending feedback from Catalin:
- Use clone3() paramaters to size/place the GCS.
- Switch copy_to_user_gcs() to be put_user_gcs().
Signed-off-by: Mark Brown <broonie(a)kernel.org>
---
Changes in v6:
- Rebase onto v6.6-rc3.
- Add some more gcsb_dsync() barriers following spec clarifications.
- Due to ongoing discussion around clone()/clone3() I've not updated
anything there, the behaviour is the same as on previous versions.
- Link to v5: https://lore.kernel.org/r/20230822-arm64-gcs-v5-0-9ef181dd6324@kernel.org
Changes in v5:
- Don't map any permissions for user GCSs, we always use EL0 accessors
or use a separate mapping of the page.
- Reduce the standard size of the GCS to RLIMIT_STACK/2.
- Enforce a PAGE_SIZE alignment requirement on map_shadow_stack().
- Clarifications and fixes to documentation.
- More tests.
- Link to v4: https://lore.kernel.org/r/20230807-arm64-gcs-v4-0-68cfa37f9069@kernel.org
Changes in v4:
- Implement flags for map_shadow_stack() allowing the cap and end of
stack marker to be enabled independently or not at all.
- Relax size and alignment requirements for map_shadow_stack().
- Add more blurb explaining the advantages of hardware enforcement.
- Link to v3: https://lore.kernel.org/r/20230731-arm64-gcs-v3-0-cddf9f980d98@kernel.org
Changes in v3:
- Rebase onto v6.5-rc4.
- Add a GCS barrier on context switch.
- Add a GCS stress test.
- Link to v2: https://lore.kernel.org/r/20230724-arm64-gcs-v2-0-dc2c1d44c2eb@kernel.org
Changes in v2:
- Rebase onto v6.5-rc3.
- Rework prctl() interface to allow each bit to be locked independently.
- map_shadow_stack() now places the cap token based on the size
requested by the caller not the actual space allocated.
- Mode changes other than enable via ptrace are now supported.
- Expand test coverage.
- Various smaller fixes and adjustments.
- Link to v1: https://lore.kernel.org/r/20230716-arm64-gcs-v1-0-bf567f93bba6@kernel.org
---
Mark Brown (38):
arm64/mm: Restructure arch_validate_flags() for extensibility
prctl: arch-agnostic prctl for shadow stack
mman: Add map_shadow_stack() flags
arm64: Document boot requirements for Guarded Control Stacks
arm64/gcs: Document the ABI for Guarded Control Stacks
arm64/sysreg: Add new system registers for GCS
arm64/sysreg: Add definitions for architected GCS caps
arm64/gcs: Add manual encodings of GCS instructions
arm64/gcs: Provide copy_to_user_gcs()
arm64/cpufeature: Runtime detection of Guarded Control Stack (GCS)
arm64/mm: Allocate PIE slots for EL0 guarded control stack
mm: Define VM_SHADOW_STACK for arm64 when we support GCS
arm64/mm: Map pages for guarded control stack
KVM: arm64: Manage GCS registers for guests
arm64/gcs: Allow GCS usage at EL0 and EL1
arm64/idreg: Add overrride for GCS
arm64/hwcap: Add hwcap for GCS
arm64/traps: Handle GCS exceptions
arm64/mm: Handle GCS data aborts
arm64/gcs: Context switch GCS state for EL0
arm64/gcs: Allocate a new GCS for threads with GCS enabled
arm64/gcs: Implement shadow stack prctl() interface
arm64/mm: Implement map_shadow_stack()
arm64/signal: Set up and restore the GCS context for signal handlers
arm64/signal: Expose GCS state in signal frames
arm64/ptrace: Expose GCS via ptrace and core files
arm64: Add Kconfig for Guarded Control Stack (GCS)
kselftest/arm64: Verify the GCS hwcap
kselftest/arm64: Add GCS as a detected feature in the signal tests
kselftest/arm64: Add framework support for GCS to signal handling tests
kselftest/arm64: Allow signals tests to specify an expected si_code
kselftest/arm64: Always run signals tests with GCS enabled
kselftest/arm64: Add very basic GCS test program
kselftest/arm64: Add a GCS test program built with the system libc
kselftest/arm64: Add test coverage for GCS mode locking
selftests/arm64: Add GCS signal tests
kselftest/arm64: Add a GCS stress test
kselftest/arm64: Enable GCS for the FP stress tests
Documentation/admin-guide/kernel-parameters.txt | 6 +
Documentation/arch/arm64/booting.rst | 22 +
Documentation/arch/arm64/elf_hwcaps.rst | 3 +
Documentation/arch/arm64/gcs.rst | 233 +++++++
Documentation/arch/arm64/index.rst | 1 +
Documentation/filesystems/proc.rst | 2 +-
arch/arm64/Kconfig | 19 +
arch/arm64/include/asm/cpufeature.h | 6 +
arch/arm64/include/asm/el2_setup.h | 17 +
arch/arm64/include/asm/esr.h | 28 +-
arch/arm64/include/asm/exception.h | 2 +
arch/arm64/include/asm/gcs.h | 106 +++
arch/arm64/include/asm/hwcap.h | 1 +
arch/arm64/include/asm/kvm_arm.h | 4 +-
arch/arm64/include/asm/kvm_host.h | 12 +
arch/arm64/include/asm/mman.h | 23 +-
arch/arm64/include/asm/pgtable-prot.h | 14 +-
arch/arm64/include/asm/processor.h | 7 +
arch/arm64/include/asm/sysreg.h | 20 +
arch/arm64/include/asm/uaccess.h | 42 ++
arch/arm64/include/uapi/asm/hwcap.h | 1 +
arch/arm64/include/uapi/asm/ptrace.h | 8 +
arch/arm64/include/uapi/asm/sigcontext.h | 9 +
arch/arm64/kernel/cpufeature.c | 19 +
arch/arm64/kernel/cpuinfo.c | 1 +
arch/arm64/kernel/entry-common.c | 23 +
arch/arm64/kernel/idreg-override.c | 2 +
arch/arm64/kernel/process.c | 92 +++
arch/arm64/kernel/ptrace.c | 59 ++
arch/arm64/kernel/signal.c | 237 ++++++-
arch/arm64/kernel/traps.c | 11 +
arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 17 +
arch/arm64/kvm/sys_regs.c | 22 +
arch/arm64/mm/Makefile | 1 +
arch/arm64/mm/fault.c | 79 ++-
arch/arm64/mm/gcs.c | 228 +++++++
arch/arm64/mm/mmap.c | 13 +-
arch/arm64/tools/cpucaps | 1 +
arch/arm64/tools/sysreg | 55 ++
arch/x86/include/uapi/asm/mman.h | 3 -
fs/proc/task_mmu.c | 3 +
include/linux/mm.h | 16 +-
include/uapi/asm-generic/mman.h | 4 +
include/uapi/asm-generic/unistd.h | 5 +-
include/uapi/linux/elf.h | 1 +
include/uapi/linux/prctl.h | 22 +
kernel/sys.c | 30 +
tools/testing/selftests/arm64/Makefile | 2 +-
tools/testing/selftests/arm64/abi/hwcap.c | 19 +
tools/testing/selftests/arm64/fp/assembler.h | 15 +
tools/testing/selftests/arm64/fp/fpsimd-test.S | 2 +
tools/testing/selftests/arm64/fp/sve-test.S | 2 +
tools/testing/selftests/arm64/fp/za-test.S | 2 +
tools/testing/selftests/arm64/fp/zt-test.S | 2 +
tools/testing/selftests/arm64/gcs/.gitignore | 5 +
tools/testing/selftests/arm64/gcs/Makefile | 24 +
tools/testing/selftests/arm64/gcs/asm-offsets.h | 0
tools/testing/selftests/arm64/gcs/basic-gcs.c | 356 ++++++++++
tools/testing/selftests/arm64/gcs/gcs-locking.c | 200 ++++++
.../selftests/arm64/gcs/gcs-stress-thread.S | 311 +++++++++
tools/testing/selftests/arm64/gcs/gcs-stress.c | 532 +++++++++++++++
tools/testing/selftests/arm64/gcs/gcs-util.h | 100 +++
tools/testing/selftests/arm64/gcs/libc-gcs.c | 742 +++++++++++++++++++++
tools/testing/selftests/arm64/signal/.gitignore | 1 +
.../testing/selftests/arm64/signal/test_signals.c | 17 +-
.../testing/selftests/arm64/signal/test_signals.h | 6 +
.../selftests/arm64/signal/test_signals_utils.c | 32 +-
.../selftests/arm64/signal/test_signals_utils.h | 39 ++
.../arm64/signal/testcases/gcs_exception_fault.c | 59 ++
.../selftests/arm64/signal/testcases/gcs_frame.c | 78 +++
.../arm64/signal/testcases/gcs_write_fault.c | 67 ++
.../selftests/arm64/signal/testcases/testcases.c | 7 +
.../selftests/arm64/signal/testcases/testcases.h | 1 +
73 files changed, 4110 insertions(+), 41 deletions(-)
---
base-commit: 6465e260f48790807eef06b583b38ca9789b6072
change-id: 20230303-arm64-gcs-e311ab0d8729
Best regards,
--
Mark Brown <broonie(a)kernel.org>
On Sun, Oct 8, 2023 at 7:22 AM Akihiko Odaki <akihiko.odaki(a)daynix.com> wrote:
>
> tun_vnet_hash can use this flag to indicate it stored virtio-net hash
> cache to cb.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki(a)daynix.com>
> ---
> include/linux/skbuff.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 4174c4b82d13..e638f157c13c 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -837,6 +837,7 @@ typedef unsigned char *sk_buff_data_t;
> * @truesize: Buffer size
> * @users: User count - see {datagram,tcp}.c
> * @extensions: allocated extensions, valid if active_extensions is nonzero
> + * @tun_vnet_hash: tun stored virtio-net hash cache to cb
> */
>
> struct sk_buff {
> @@ -989,6 +990,7 @@ struct sk_buff {
> #if IS_ENABLED(CONFIG_IP_SCTP)
> __u8 csum_not_inet:1;
> #endif
> + __u8 tun_vnet_hash:1;
sk_buff space is very limited.
No need to extend it, especially for code that stays within a single
subsystem (tun).
To a lesser extent the same point applies to the qdisc_skb_cb.
On Sun, Oct 8, 2023 at 7:21 AM Akihiko Odaki <akihiko.odaki(a)daynix.com> wrote:
>
> virtio-net have two usage of hashes: one is RSS and another is hash
> reporting. Conventionally the hash calculation was done by the VMM.
> However, computing the hash after the queue was chosen defeats the
> purpose of RSS.
>
> Another approach is to use eBPF steering program. This approach has
> another downside: it cannot report the calculated hash due to the
> restrictive nature of eBPF.
>
> Introduce the code to compute hashes to the kernel in order to overcome
> thse challenges.
>
> An alternative solution is to extend the eBPF steering program so that it
> will be able to report to the userspace, but it makes little sense to
> allow to implement different hashing algorithms with eBPF since the hash
> value reported by virtio-net is strictly defined by the specification.
But using the existing BPF steering may have the benefit of requiring
a lot less new code.
There is ample precedence for BPF programs that work this way. The
flow dissector comes to mind.
The test will not work for systems with pagesize != 4096 like aarch64
and some others.
Other testcases are already testing the same functionality:
* auxv_AT_UID tests getauxval() in general.
* test_getpagesize() tests pagesize() which directly calls
getauxval(AT_PAGESZ).
Fixes: 48967b73f8fe ("selftests/nolibc: add testcases for startup code")
Cc: stable(a)vger.kernel.org
Signed-off-by: Thomas Weißschuh <linux(a)weissschuh.net>
---
Note:
This should probably also make it into 6.6.
---
tools/testing/selftests/nolibc/nolibc-test.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index a3ee4496bf0a..7e3936c182dc 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -630,7 +630,6 @@ int run_startup(int min, int max)
CASE_TEST(environ_HOME); EXPECT_PTRNZ(1, getenv("HOME")); break;
CASE_TEST(auxv_addr); EXPECT_PTRGT(test_auxv != (void *)-1, test_auxv, brk); break;
CASE_TEST(auxv_AT_UID); EXPECT_EQ(1, getauxval(AT_UID), getuid()); break;
- CASE_TEST(auxv_AT_PAGESZ); EXPECT_GE(1, getauxval(AT_PAGESZ), 4096); break;
case __LINE__:
return ret; /* must be last */
/* note: do not set any defaults so as to permit holes above */
---
base-commit: ab663cc32912914258bc8a2fbd0e753f552ee9d8
change-id: 20231007-nolibc-auxval-pagesz-05f5ff79c4c4
Best regards,
--
Thomas Weißschuh <linux(a)weissschuh.net>
The indentation of parameterized tests messages is currently broken in kunit.
Try to fix that by introducing a test level attribute, that will be increased
during nested parameterized tests execution, and use it to generate correct
indent at the runtime when printing message or writing them to the log.
Also improve kunit by providing test plan for the parameterized tests.
Cc: David Gow <davidgow(a)google.com>
Cc: Rae Moar <rmoar(a)google.com>
Michal Wajdeczko (4):
kunit: Drop redundant text from suite init failure message
kunit: Fix indentation level of suite messages
kunit: Fix indentation of parameterized tests messages
kunit: Prepare test plan for parameterized subtests
include/kunit/test.h | 25 ++++++++++++--
lib/kunit/test.c | 79 +++++++++++++++++++++++---------------------
2 files changed, 65 insertions(+), 39 deletions(-)
--
2.25.1
Today we reset the suite counter as part of the suite cleanup,
called from the module exit callback, but it might not work that
well as one can try to collect results without unloading a previous
test (either unintentionally or due to dependencies).
For easy reproduction try to load the kunit-test.ko and then
collect and parse results from the kunit-example-test.ko load.
Parser will complain about mismatch of expected test number:
[ ] KTAP version 1
[ ] 1..1
[ ] # example: initializing suite
[ ] KTAP version 1
[ ] # Subtest: example
..
[ ] # example: pass:5 fail:0 skip:4 total:9
[ ] # Totals: pass:6 fail:0 skip:6 total:12
[ ] ok 7 example
[ ] [ERROR] Test: example: Expected test number 1 but found 7
[ ] ===================== [PASSED] example =====================
[ ] ============================================================
[ ] Testing complete. Ran 12 tests: passed: 6, skipped: 6, errors: 1
Since we are now printing suite test plan on every module load,
right before running suite tests, we should make sure that suite
counter will also start from 1. Easiest solution seems to be move
counter reset to the __kunit_test_suites_init() function.
Signed-off-by: Michal Wajdeczko <michal.wajdeczko(a)intel.com>
Cc: David Gow <davidgow(a)google.com>
Cc: Rae Moar <rmoar(a)google.com>
---
lib/kunit/test.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index f2eb71f1a66c..9325d309ed82 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -670,6 +670,8 @@ int __kunit_test_suites_init(struct kunit_suite * const * const suites, int num_
return 0;
}
+ kunit_suite_counter = 1;
+
static_branch_inc(&kunit_running);
for (i = 0; i < num_suites; i++) {
@@ -696,8 +698,6 @@ void __kunit_test_suites_exit(struct kunit_suite **suites, int num_suites)
for (i = 0; i < num_suites; i++)
kunit_exit_suite(suites[i]);
-
- kunit_suite_counter = 1;
}
EXPORT_SYMBOL_GPL(__kunit_test_suites_exit);
--
2.25.1
With the startup code moved to C, implementing support for
constructors and deconstructors is fairly easy to implement.
Examples for code size impact:
text data bss dec hex filename
21837 104 88 22029 560d nolibc-test.before
22135 120 88 22343 5747 nolibc-test.after
21970 104 88 22162 5692 nolibc-test.after-only-crt.h-changes
The sections are defined by [0].
[0] https://refspecs.linuxfoundation.org/elf/gabi4+/ch5.dynamic.html
Signed-off-by: Thomas Weißschuh <linux(a)weissschuh.net>
---
Note:
This is only an RFC as I'm not 100% sure it belong into nolibc.
But at least the code is visible as an example.
Also it is one prerequisite for full ksefltest_harness.h support in
nolibc, should we want that.
---
tools/include/nolibc/crt.h | 23 ++++++++++++++++++++++-
tools/testing/selftests/nolibc/nolibc-test.c | 16 ++++++++++++++++
2 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/tools/include/nolibc/crt.h b/tools/include/nolibc/crt.h
index a5f33fef1672..c1176611d9a9 100644
--- a/tools/include/nolibc/crt.h
+++ b/tools/include/nolibc/crt.h
@@ -13,11 +13,22 @@ const unsigned long *_auxv __attribute__((weak));
static void __stack_chk_init(void);
static void exit(int);
+extern void (*const __preinit_array_start[])(void) __attribute__((weak));
+extern void (*const __preinit_array_end[])(void) __attribute__((weak));
+
+extern void (*const __init_array_start[])(void) __attribute__((weak));
+extern void (*const __init_array_end[])(void) __attribute__((weak));
+
+extern void (*const __fini_array_start[])(void) __attribute__((weak));
+extern void (*const __fini_array_end[])(void) __attribute__((weak));
+
void _start_c(long *sp)
{
long argc;
char **argv;
char **envp;
+ int exitcode;
+ void (* const *func)(void);
const unsigned long *auxv;
/* silence potential warning: conflicting types for 'main' */
int _nolibc_main(int, char **, char **) __asm__ ("main");
@@ -54,8 +65,18 @@ void _start_c(long *sp)
;
_auxv = auxv;
+ for (func = __preinit_array_start; func < __preinit_array_end; func++)
+ (*func)();
+ for (func = __init_array_start; func < __init_array_end; func++)
+ (*func)();
+
/* go to application */
- exit(_nolibc_main(argc, argv, envp));
+ exitcode = _nolibc_main(argc, argv, envp);
+
+ for (func = __fini_array_end - 1; func >= __fini_array_start; func--)
+ (*func)();
+
+ exit(exitcode);
}
#endif /* _NOLIBC_CRT_H */
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index a3ee4496bf0a..f166b425613a 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -57,6 +57,9 @@ static int test_argc;
/* will be used by some test cases as readable file, please don't write it */
static const char *argv0;
+/* will be used by constructor tests */
+static int constructor_test_value;
+
/* definition of a series of tests */
struct test {
const char *name; /* test name */
@@ -594,6 +597,18 @@ int expect_strne(const char *expr, int llen, const char *cmp)
#define CASE_TEST(name) \
case __LINE__: llen += printf("%d %s", test, #name);
+__attribute__((constructor))
+static void constructor1(void)
+{
+ constructor_test_value = 1;
+}
+
+__attribute__((constructor))
+static void constructor2(void)
+{
+ constructor_test_value *= 2;
+}
+
int run_startup(int min, int max)
{
int test;
@@ -631,6 +646,7 @@ int run_startup(int min, int max)
CASE_TEST(auxv_addr); EXPECT_PTRGT(test_auxv != (void *)-1, test_auxv, brk); break;
CASE_TEST(auxv_AT_UID); EXPECT_EQ(1, getauxval(AT_UID), getuid()); break;
CASE_TEST(auxv_AT_PAGESZ); EXPECT_GE(1, getauxval(AT_PAGESZ), 4096); break;
+ CASE_TEST(constructor); EXPECT_EQ(1, constructor_test_value, 2); break;
case __LINE__:
return ret; /* must be last */
/* note: do not set any defaults so as to permit holes above */
---
base-commit: ab663cc32912914258bc8a2fbd0e753f552ee9d8
change-id: 20231005-nolibc-constructors-b2aebffe9b65
Best regards,
--
Thomas Weißschuh <linux(a)weissschuh.net>
The original order of cases in kunit_module_notify() is confusing and
misleading.
And the best practice is return the err code from
MODULE_STATE_COMING func.
And the test suits should be executed when notify MODULE_STATE_LIVE.
Jinjie Ruan (3):
kunit: Make the cases sequence more reasonable for
kunit_module_notify()
kunit: Return error from kunit_module_init()
kunit: Init and run test suites in the right state
lib/kunit/test.c | 37 +++++++++++++++++++++++--------------
1 file changed, 23 insertions(+), 14 deletions(-)
--
2.34.1
Attn: linux-kselftest(a)vger.kernel.org
Date: 03-10-2023
Subject: Letter of Intent (LOI) (03-10-2023)
This is a Letter of Intent concerning my Board of investors intent to fund some of your available investment projects.
If this is of any interest to you then kindly advise.
Yours Faithfully,
Ahmad R. Deeb
Head of Investment Team
This patch series introduces UFFDIO_REMAP feature to userfaultfd, which
has long been implemented and maintained by Andrea in his local tree [1],
but was not upstreamed due to lack of use cases where this approach would
be better than allocating a new page and copying the contents.
UFFDIO_COPY performs ~20% better than UFFDIO_REMAP when the application
needs pages to be allocated [2]. However, with UFFDIO_REMAP, if pages are
available (in userspace) for recycling, as is usually the case in heap
compaction algorithms, then we can avoid the page allocation and memcpy
(done by UFFDIO_COPY). Also, since the pages are recycled in the
userspace, we avoid the need to release (via madvise) the pages back to
the kernel [3].
We see over 40% reduction (on a Google pixel 6 device) in the compacting
thread’s completion time by using UFFDIO_REMAP vs. UFFDIO_COPY. This was
measured using a benchmark that emulates a heap compaction implementation
using userfaultfd (to allow concurrent accesses by application threads).
More details of the usecase are explained in [3].
Furthermore, UFFDIO_REMAP enables remapping swapped-out pages without
touching them within the same vma. Today, it can only be done by mremap,
however it forces splitting the vma.
Main changes since Andrea's last version [1]:
- Trivial translations from page to folio, mmap_sem to mmap_lock
- Replace pmd_trans_unstable() with pte_offset_map_nolock() and handle its
possible failure
- Move pte mapping into remap_pages_pte to allow for retries when source
page or anon_vma is contended. Since pte_offset_map_nolock() start RCU
read section, we can't block anymore after mapping a pte, so have to unmap
the ptesm do the locking and retry.
- Add and use anon_vma_trylock_write() to avoid blocking while in RCU
read section.
- Accommodate changes in mmu_notifier_range_init() API, switch to
mmu_notifier_invalidate_range_start_nonblock() to avoid blocking while in
RCU read section.
- Open-code now removed __swp_swapcount()
- Replace pmd_read_atomic() with pmdp_get_lockless()
- Add new selftest for UFFDIO_REMAP
Changes since v1 [4]:
- add mmget_not_zero in userfaultfd_remap, per Jann Horn
- removed extern from function definitions, per Matthew Wilcox
- converted to folios in remap_pages_huge_pmd, per Matthew Wilcox
- use PageAnonExclusive in remap_pages_huge_pmd, per David Hildenbrand
- handle pgtable transfers between MMs, per Jann Horn
- ignore concurrent A/D pte bit changes, per Jann Horn
- split functions into smaller units, per David Hildenbrand
- test for folio_test_large in remap_anon_pte, per Matthew Wilcox
- use pte_swp_exclusive for swapcount check, per David Hildenbrand
- eliminated use of mmu_notifier_invalidate_range_start_nonblock,
per Jann Horn
- simplified THP alignment checks, per Jann Horn
- refactored the loop inside remap_pages, per Jann Horn
- additional clarifying comments, per Jann Horn
[1] https://gitlab.com/aarcange/aa/-/commit/2aec7aea56b10438a3881a20a411aa4b1fc…
[2] https://lore.kernel.org/all/1425575884-2574-1-git-send-email-aarcange@redha…
[3] https://lore.kernel.org/linux-mm/CA+EESO4uO84SSnBhArH4HvLNhaUQ5nZKNKXqxRCyj…
[4] https://lore.kernel.org/all/20230914152620.2743033-1-surenb@google.com/
Andrea Arcangeli (2):
userfaultfd: UFFDIO_REMAP: rmap preparation
userfaultfd: UFFDIO_REMAP uABI
Suren Baghdasaryan (1):
selftests/mm: add UFFDIO_REMAP ioctl test
fs/userfaultfd.c | 63 ++
include/linux/rmap.h | 5 +
include/linux/userfaultfd_k.h | 12 +
include/uapi/linux/userfaultfd.h | 22 +
mm/huge_memory.c | 130 ++++
mm/khugepaged.c | 3 +
mm/rmap.c | 13 +
mm/userfaultfd.c | 590 +++++++++++++++++++
tools/testing/selftests/mm/uffd-common.c | 41 +-
tools/testing/selftests/mm/uffd-common.h | 1 +
tools/testing/selftests/mm/uffd-unit-tests.c | 62 ++
11 files changed, 940 insertions(+), 2 deletions(-)
--
2.42.0.515.g380fc7ccd1-goog
The abi_test currently uses a long sized test value for enablement
checks. On LE this works fine, however, on BE this results in inaccurate
assert checks due to a bit being used and assuming it's value is the
same on both LE and BE.
Use int type for 32-bit values and long type for 64-bit values to ensure
appropriate behavior on both LE and BE.
Fixes: 60b1af8de8c1 ("tracing/user_events: Add ABI self-test")
Signed-off-by: Beau Belgrave <beaub(a)linux.microsoft.com>
---
V2 Changes:
Rebase to linux-kselftest/fixes.
tools/testing/selftests/user_events/abi_test.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/user_events/abi_test.c b/tools/testing/selftests/user_events/abi_test.c
index 8202f1327c39..aa297d3ad95e 100644
--- a/tools/testing/selftests/user_events/abi_test.c
+++ b/tools/testing/selftests/user_events/abi_test.c
@@ -47,7 +47,7 @@ static int change_event(bool enable)
return ret;
}
-static int reg_enable(long *enable, int size, int bit)
+static int reg_enable(void *enable, int size, int bit)
{
struct user_reg reg = {0};
int fd = open(data_file, O_RDWR);
@@ -69,7 +69,7 @@ static int reg_enable(long *enable, int size, int bit)
return ret;
}
-static int reg_disable(long *enable, int bit)
+static int reg_disable(void *enable, int bit)
{
struct user_unreg reg = {0};
int fd = open(data_file, O_RDWR);
@@ -90,7 +90,8 @@ static int reg_disable(long *enable, int bit)
}
FIXTURE(user) {
- long check;
+ int check;
+ long check_long;
bool umount;
};
@@ -99,6 +100,7 @@ FIXTURE_SETUP(user) {
change_event(false);
self->check = 0;
+ self->check_long = 0;
}
FIXTURE_TEARDOWN(user) {
@@ -136,9 +138,9 @@ TEST_F(user, bit_sizes) {
#if BITS_PER_LONG == 8
/* Allow 0-64 bits for 64-bit */
- ASSERT_EQ(0, reg_enable(&self->check, sizeof(long), 63));
- ASSERT_NE(0, reg_enable(&self->check, sizeof(long), 64));
- ASSERT_EQ(0, reg_disable(&self->check, 63));
+ ASSERT_EQ(0, reg_enable(&self->check_long, sizeof(long), 63));
+ ASSERT_NE(0, reg_enable(&self->check_long, sizeof(long), 64));
+ ASSERT_EQ(0, reg_disable(&self->check_long, 63));
#endif
/* Disallowed sizes (everything beside 4 and 8) */
base-commit: 6f874fa021dfc7bf37f4f37da3a5aaa41fe9c39c
--
2.34.1
All architectures should use a long aligned address passed to set_bit().
User processes can pass either a 32-bit or 64-bit sized value to be
updated when tracing is enabled when on a 64-bit kernel. Both cases are
ensured to be naturally aligned, however, that is not enough. The
address must be long aligned without affecting checks on the value
within the user process which require different adjustments for the bit
for little and big endian CPUs.
32 bit on 64 bit, even when properly long aligned, still require a 32 bit
offset to be done for BE. Due to this, it cannot be easily put into a
generic method.
The abi_test also used a long, which broke the test on 64-bit BE machines.
The change simply uses an int for 32-bit value checks and a long when on
64-bit kernels for 64-bit specific checks.
I've run these changes and self tests for user_events on ppc64 BE, x86_64
LE, and aarch64 LE. It'd be great to test this also on RISC-V, but I do
not have one.
Clément Léger originally put a patch together for the alignment issue, but
we uncovered more issues as we went further into the problem. Clément felt
my version was better [1] so I am sending this series out that addresses
the selftest, BE bit offset, and the alignment issue.
1. https://lore.kernel.org/linux-trace-kernel/713f4916-00ff-4a24-82d1-72884500…
Beau Belgrave (2):
tracing/user_events: Align set_bit() address for all archs
selftests/user_events: Fix abi_test for BE archs
kernel/trace/trace_events_user.c | 58 ++++++++++++++++---
.../testing/selftests/user_events/abi_test.c | 16 ++---
2 files changed, 60 insertions(+), 14 deletions(-)
base-commit: fc1653abba0d554aad80224e51bcad42b09895ed
--
2.34.1
Fix three issues with resctrl selftests.
The signal handling fix became necessary after the mount/umount fixes.
The other two came up when I ran resctrl selftests across the server
fleet in our lab to validate the upcoming CAT test rewrite (the rewrite
is not part of this series).
These are developed and should apply cleanly at least on top the
benchmark cleanup series (might apply cleanly also w/o the benchmark
series, I didn't test).
v2:
- Include patch to move _GNU_SOURCE to Makefile to allow normal #include
placement
- Rework the signal register/unregister into patch to use helpers
- Fixed incorrect function parameter description
- Use return !!res to avoid confusing implicit boolean conversion
- Improve MBA/MBM success bound patch's changelog
- Tweak Cc: stable dependencies (make it a chain).
Ilpo Järvinen (6):
selftests/resctrl: Extend signal handler coverage to unmount on
receiving signal
selftests/resctrl: Remove duplicate feature check from CMT test
selftests/resctrl: Move _GNU_SOURCE define into Makefile
selftests/resctrl: Refactor feature check to use resource and feature
name
selftests/resctrl: Fix feature checks
selftests/resctrl: Reduce failures due to outliers in MBA/MBM tests
tools/testing/selftests/resctrl/Makefile | 2 +-
tools/testing/selftests/resctrl/cat_test.c | 8 --
tools/testing/selftests/resctrl/cmt_test.c | 3 -
tools/testing/selftests/resctrl/mba_test.c | 2 +-
tools/testing/selftests/resctrl/mbm_test.c | 2 +-
tools/testing/selftests/resctrl/resctrl.h | 7 +-
.../testing/selftests/resctrl/resctrl_tests.c | 78 +++++++++++--------
tools/testing/selftests/resctrl/resctrl_val.c | 22 +++---
tools/testing/selftests/resctrl/resctrlfs.c | 69 +++++++---------
9 files changed, 88 insertions(+), 105 deletions(-)
--
2.30.2
This series extends KVM RISC-V to allow Guest/VM discover and use
conditional operations related ISA extensions (namely XVentanaCondOps
and Zicond).
To try these patches, use KVMTOOL from riscv_zbx_zicntr_smstateen_condops_v1
branch at: https://github.com/avpatel/kvmtool.git
These patches are based upon the latest riscv_kvm_queue and can also be
found in the riscv_kvm_condops_v1 branch at:
https://github.com/avpatel/linux.git
Anup Patel (7):
RISC-V: Detect XVentanaCondOps from ISA string
RISC-V: Detect Zicond from ISA string
RISC-V: KVM: Allow XVentanaCondOps extension for Guest/VM
RISC-V: KVM: Allow Zicond extension for Guest/VM
KVM: riscv: selftests: Add senvcfg register to get-reg-list test
KVM: riscv: selftests: Add smstateen registers to get-reg-list test
KVM: riscv: selftests: Add condops extensions to get-reg-list test
.../devicetree/bindings/riscv/extensions.yaml | 13 ++++++
arch/riscv/include/asm/hwcap.h | 2 +
arch/riscv/include/uapi/asm/kvm.h | 2 +
arch/riscv/kernel/cpufeature.c | 2 +
arch/riscv/kvm/vcpu_onereg.c | 4 ++
.../selftests/kvm/riscv/get-reg-list.c | 41 +++++++++++++++++++
6 files changed, 64 insertions(+)
--
2.34.1
Add functionality to run built-in tests after boot by writing to a
debugfs file.
Add a new debugfs file labeled "run" for each test suite to use for
this purpose.
As an example, write to the file using the following:
echo "any string" > /sys/kernel/debugfs/kunit/<testsuite>/run
This will trigger the test suite to run and will print results to the
kernel log.
Note that what you "write" to the debugfs file will not be saved.
To guard against running tests concurrently with this feature, add a
mutex lock around running kunit. This supports the current practice of
not allowing tests to be run concurrently on the same kernel.
This functionality may not work for all tests.
This new functionality could be used to design a parameter
injection feature in the future.
Signed-off-by: Rae Moar <rmoar(a)google.com>
---
Changes since v1:
- Removed second patch as this problem has been fixed
- Added Documentation patch
- Made changes to work with new dynamically-extending log feature
Note that these patches now rely on (and are rebased on) the patch series:
https://lore.kernel.org/all/20230828104111.2394344-1-rf@opensource.cirrus.c…
lib/kunit/debugfs.c | 66 +++++++++++++++++++++++++++++++++++++++++++++
lib/kunit/test.c | 13 +++++++++
2 files changed, 79 insertions(+)
diff --git a/lib/kunit/debugfs.c b/lib/kunit/debugfs.c
index 270d185737e6..8c0a970321ce 100644
--- a/lib/kunit/debugfs.c
+++ b/lib/kunit/debugfs.c
@@ -8,12 +8,14 @@
#include <linux/module.h>
#include <kunit/test.h>
+#include <kunit/test-bug.h>
#include "string-stream.h"
#include "debugfs.h"
#define KUNIT_DEBUGFS_ROOT "kunit"
#define KUNIT_DEBUGFS_RESULTS "results"
+#define KUNIT_DEBUGFS_RUN "run"
/*
* Create a debugfs representation of test suites:
@@ -21,6 +23,8 @@
* Path Semantics
* /sys/kernel/debug/kunit/<testsuite>/results Show results of last run for
* testsuite
+ * /sys/kernel/debug/kunit/<testsuite>/run Write to this file to trigger
+ * testsuite to run
*
*/
@@ -99,6 +103,51 @@ static int debugfs_results_open(struct inode *inode, struct file *file)
return single_open(file, debugfs_print_results, suite);
}
+/*
+ * Print a usage message to the debugfs "run" file
+ * (/sys/kernel/debug/kunit/<testsuite>/run) if opened.
+ */
+static int debugfs_print_run(struct seq_file *seq, void *v)
+{
+ struct kunit_suite *suite = (struct kunit_suite *)seq->private;
+
+ seq_puts(seq, "Write to this file to trigger the test suite to run.\n");
+ seq_printf(seq, "usage: echo \"any string\" > /sys/kernel/debugfs/kunit/%s/run\n",
+ suite->name);
+ return 0;
+}
+
+/*
+ * The debugfs "run" file (/sys/kernel/debug/kunit/<testsuite>/run)
+ * contains no information. Write to the file to trigger the test suite
+ * to run.
+ */
+static int debugfs_run_open(struct inode *inode, struct file *file)
+{
+ struct kunit_suite *suite;
+
+ suite = (struct kunit_suite *)inode->i_private;
+
+ return single_open(file, debugfs_print_run, suite);
+}
+
+/*
+ * Trigger a test suite to run by writing to the suite's "run" debugfs
+ * file found at: /sys/kernel/debug/kunit/<testsuite>/run
+ *
+ * Note: what is written to this file will not be saved.
+ */
+static ssize_t debugfs_run(struct file *file,
+ const char __user *buf, size_t count, loff_t *ppos)
+{
+ struct inode *f_inode = file->f_inode;
+ struct kunit_suite *suite = (struct kunit_suite *) f_inode->i_private;
+
+ __kunit_test_suites_init(&suite, 1);
+
+ return count;
+}
+
static const struct file_operations debugfs_results_fops = {
.open = debugfs_results_open,
.read = seq_read,
@@ -106,10 +155,23 @@ static const struct file_operations debugfs_results_fops = {
.release = debugfs_release,
};
+static const struct file_operations debugfs_run_fops = {
+ .open = debugfs_run_open,
+ .read = seq_read,
+ .write = debugfs_run,
+ .llseek = seq_lseek,
+ .release = debugfs_release,
+};
+
void kunit_debugfs_create_suite(struct kunit_suite *suite)
{
struct kunit_case *test_case;
+ if (suite->log) {
+ /* Clear the suite log that's leftover from a previous run. */
+ string_stream_clear(suite->log);
+ return;
+ }
/* Allocate logs before creating debugfs representation. */
suite->log = alloc_string_stream(GFP_KERNEL);
string_stream_set_append_newlines(suite->log, true);
@@ -124,6 +186,10 @@ void kunit_debugfs_create_suite(struct kunit_suite *suite)
debugfs_create_file(KUNIT_DEBUGFS_RESULTS, S_IFREG | 0444,
suite->debugfs,
suite, &debugfs_results_fops);
+
+ debugfs_create_file(KUNIT_DEBUGFS_RUN, S_IFREG | 0644,
+ suite->debugfs,
+ suite, &debugfs_run_fops);
}
void kunit_debugfs_destroy_suite(struct kunit_suite *suite)
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index 651cbda9f250..d376b886d72d 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -13,6 +13,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
+#include <linux/mutex.h>
#include <linux/panic.h>
#include <linux/sched/debug.h>
#include <linux/sched.h>
@@ -22,6 +23,8 @@
#include "string-stream.h"
#include "try-catch-impl.h"
+static struct mutex kunit_run_lock;
+
/*
* Hook to fail the current test and print an error message to the log.
*/
@@ -668,6 +671,11 @@ int __kunit_test_suites_init(struct kunit_suite * const * const suites, int num_
return 0;
}
+ /* Use mutex lock to guard against running tests concurrently. */
+ if (mutex_lock_interruptible(&kunit_run_lock)) {
+ pr_err("kunit: test interrupted\n");
+ return -EINTR;
+ }
static_branch_inc(&kunit_running);
for (i = 0; i < num_suites; i++) {
@@ -676,6 +684,7 @@ int __kunit_test_suites_init(struct kunit_suite * const * const suites, int num_
}
static_branch_dec(&kunit_running);
+ mutex_unlock(&kunit_run_lock);
return 0;
}
EXPORT_SYMBOL_GPL(__kunit_test_suites_init);
@@ -836,6 +845,10 @@ static int __init kunit_init(void)
kunit_install_hooks();
kunit_debugfs_init();
+
+ /* Initialize lock to guard against running tests concurrently. */
+ mutex_init(&kunit_run_lock);
+
#ifdef CONFIG_MODULES
return register_module_notifier(&kunit_mod_nb);
#else
base-commit: b754593274e04fc840482a658b29791bc8f8b933
--
2.42.0.283.g2d96d420d3-goog
From: Björn Töpel <bjorn(a)rivosinc.com>
Yet another "more cross-building support for RISC-V" series.
An example how to invoke a gen_tar build:
| make ARCH=riscv CROSS_COMPILE=riscv64-linux-gnu- CC=riscv64-linux-gnu-gcc \
| HOSTCC=gcc O=/workspace/kbuild FORMAT= \
| SKIP_TARGETS="arm64 ia64 powerpc sparc64 x86 sgx" -j $(($(nproc)-1)) \
| -C tools/testing/selftests gen_tar
Björn
Björn Töpel (3):
selftests/bpf: Add cross-build support for urandom_read et al
selftests/bpf: Enable lld usage for RISC-V
selftests/bpf: Add uprobe_multi to gen_tar target
tools/testing/selftests/bpf/Makefile | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
base-commit: 2147c8d07e1abc8dfc3433ca18eed5295e230ede
--
2.39.2
From: Björn Töpel <bjorn(a)rivosinc.com>
Commit 08d0ce30e0e4 ("riscv: Implement syscall wrappers") introduced
some regressions in libbpf, and the kselftests BPF suite, which are
fixed with these three patches.
Note that there's an outstanding fix [1] for ftrace syscall tracing
which is also a fallout from the commit above.
Björn
[1] https://lore.kernel.org/linux-riscv/20231003182407.32198-1-alexghiti@rivosi…
Alexandre Ghiti (1):
libbpf: Fix syscall access arguments on riscv
Björn Töpel (2):
selftests/bpf: Define SYS_PREFIX for riscv
selftests/bpf: Define SYS_NANOSLEEP_KPROBE_NAME for riscv
tools/lib/bpf/bpf_tracing.h | 2 --
tools/testing/selftests/bpf/progs/bpf_misc.h | 3 +++
tools/testing/selftests/bpf/test_progs.h | 2 ++
3 files changed, 5 insertions(+), 2 deletions(-)
base-commit: 9077fc228f09c9f975c498c55f5d2e882cd0da59
--
2.39.2
Hi Linus,
Please pull the following Kselftest fixes update for Linux 6.6-rc5.
This kselftest fixes update for Linux 6.6-rc5 consists of one single
fix to Makefile to fix the incorrect TARGET name for uevent test.
diff is attached.
thanks.
-- Shuah
----------------------------------------------------------------
The following changes since commit 8ed99af4a266a3492d773b5d85c3f8e9f81254b6:
selftests/user_events: Fix to unmount tracefs when test created mount (2023-09-18 11:04:52 -0600)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest tags/linux-kselftest-fixes-6.6-rc5
for you to fetch changes up to 6f874fa021dfc7bf37f4f37da3a5aaa41fe9c39c:
selftests: Fix wrong TARGET in kselftest top level Makefile (2023-09-26 18:47:37 -0600)
----------------------------------------------------------------
linux-kselftest-fixes-6.6-rc5
This kselftest fixes update for Linux 6.6-rc5 consists of one single
fix to Makefile to fix the incorrect TARGET name for uevent test.
----------------------------------------------------------------
Juntong Deng (1):
selftests: Fix wrong TARGET in kselftest top level Makefile
tools/testing/selftests/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------
user_events, tdx and dmabuf-heaps build a series of binaries that can be
safely ignored by git as it is done by other selftests.
Signed-off-by: Javier Carrasco <javier.carrasco.cruz(a)gmail.com>
---
Javier Carrasco (3):
selftests/user_events: add gitignore file
selftests/tdx: add gitignore file
selftests/dmabuf-heaps: add gitignore file
tools/testing/selftests/dmabuf-heaps/.gitignore | 1 +
tools/testing/selftests/tdx/.gitignore | 1 +
tools/testing/selftests/user_events/.gitignore | 4 ++++
3 files changed, 6 insertions(+)
---
base-commit: cbf3a2cb156a2c911d8f38d8247814b4c07f49a2
change-id: 20231004-topic-selftest_gitignore-3e82f4341001
Best regards,
--
Javier Carrasco <javier.carrasco.cruz(a)gmail.com>
From: Björn Töpel <bjorn(a)rivosinc.com>
Two minor changes to the kselftest-merge target:
1. Let builtin have presedence over modules when merging configs
2. Merge per-arch configs, if available
Björn
Björn Töpel (2):
kbuild: Let builtin have precedence over modules for kselftest-merge
kbuild: Merge per-arch config for kselftest-merge target
Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
base-commit: cbf3a2cb156a2c911d8f38d8247814b4c07f49a2
--
2.39.2
Szanowni Państwo!
W ramach nowej edycji programu Mój Prąd mogą otrzymać Państwo dofinansowanie na zakup i montaż fotowoltaiki i/lub magazynu energii. Maksymalna kwota dofinansowania wynosi 58 tys. zł.
Jako firma wyspecjalizowana w tym zakresie zajmiemy się Państwa wnioskiem o dofinansowanie oraz instalacją i serwisem dopasowanych do Państwa budynku paneli słonecznych.
Będę wdzięczny za informację czy są Państwo zainteresowani.
Pozdrawiam,
Kamil Lasek
Hi, I am sending this series on behalf of myself and Benjamin Tissoires. There
existed an initial n=3 patch series which was later expanded to n=4 and
is now back to n=3 with some fixes added in and rebased against
mainline.
This patch series aims to ensure that the hid/bpf selftests can be built
without errors.
Here's Benjamin's initial cover letter for context:
| These fixes have been triggered by [0]:
| basically, if you do not recompile the kernel first, and are
| running on an old kernel, vmlinux.h doesn't have the required
| symbols and the compilation fails.
|
| The tests will fail if you run them on that very same machine,
| of course, but the binary should compile.
|
| And while I was sorting out why it was failing, I realized I
| could do a couple of improvements on the Makefile.
|
| [0] https://lore.kernel.org/linux-input/56ba8125-2c6f-a9c9-d498-0ca1c153dcb2@re…
Changes from v1 -> v2:
- roll Justin's fix into patch 1/3
- add __attribute__((preserve_access_index)) (thanks Eduard)
- rebased onto mainline (2dde18cd1d8fac735875f2e4987f11817cc0bc2c)
- Link to v1: https://lore.kernel.org/all/20230825-wip-selftests-v1-0-c862769020a8@kernel…
Link: https://github.com/ClangBuiltLinux/linux/issues/1698
Link: https://github.com/ClangBuiltLinux/continuous-integration2/issues/61
---
Benjamin Tissoires (3):
selftests/hid: ensure we can compile the tests on kernels pre-6.3
selftests/hid: do not manually call headers_install
selftests/hid: force using our compiled libbpf headers
tools/testing/selftests/hid/Makefile | 10 ++---
tools/testing/selftests/hid/progs/hid.c | 3 --
.../testing/selftests/hid/progs/hid_bpf_helpers.h | 49 ++++++++++++++++++++++
3 files changed, 53 insertions(+), 9 deletions(-)
---
base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
change-id: 20230908-kselftest-09-08-56d7f4a8d5c4
Best regards,
--
Justin Stitt <justinstitt(a)google.com>
Write_schemata() uses fprintf() to write a bitmask into a schemata file
inside resctrl FS. It checks fprintf() return value but it doesn't check
fclose() return value. Error codes from fprintf() such as write errors,
are buffered and flushed back to the user only after fclose() is executed
which means any invalid bitmask can be written into the schemata file.
Rewrite write_schemata() to use syscalls instead of stdio file
operations to avoid the buffering.
The resctrlfs.c defines functions that interact with the resctrl FS
while resctrl_val.c defines functions that perform measurements on
the cache. Run_benchmark() fits logically into the second file before
resctrl_val() that uses it.
Move run_benchmark() from resctrlfs.c to resctrl_val.c and remove
redundant part of the kernel-doc comment. Make run_benchmark() static
and remove it from the header file.
Patch series is based on [1] which is based on [2] which are based on
kselftest next branch.
Changelog v5:
- Add Ilpo's reviewed-by tag to Patch 1/2.
- Reword patch messages slightly.
- Add error check to schema_len variable.
Changelog v4:
- Change git signature from Wieczor-Retman Maciej to Maciej
Wieczor-Retman.
- Rebase onto [1] which is based on [2]. (Reinette)
- Add fcntl.h explicitly to provide glibc backward compatibility.
(Reinette)
Changelog v3:
- Use snprintf() return value instead of strlen() in write_schemata().
(Ilpo)
- Make run_benchmark() static and remove it from the header file.
(Reinette)
- Add Ilpo's reviewed-by tag to Patch 2/2.
- Patch messages and cover letter rewording.
Changelog v2:
- Change sprintf() to snprintf() in write_schemata().
- Redo write_schemata() with syscalls instead of stdio functions.
- Fix typos and missing dots in patch messages.
- Branch printf attribute patch to a separate series.
[v1] https://lore.kernel.org/all/cover.1692880423.git.maciej.wieczor-retman@inte…
[v2] https://lore.kernel.org/all/cover.1693213468.git.maciej.wieczor-retman@inte…
[v3] https://lore.kernel.org/all/cover.1693575451.git.maciej.wieczor-retman@inte…
[v4] https://lore.kernel.org/all/cover.1695369120.git.maciej.wieczor-retman@inte…
[1] https://lore.kernel.org/all/20230915154438.82931-1-ilpo.jarvinen@linux.inte…
[2] https://lore.kernel.org/all/20230904095339.11321-1-ilpo.jarvinen@linux.inte…
Maciej Wieczor-Retman (2):
selftests/resctrl: Fix schemata write error check
selftests/resctrl: Move run_benchmark() to a more fitting file
tools/testing/selftests/resctrl/resctrl.h | 1 -
tools/testing/selftests/resctrl/resctrl_val.c | 50 +++++++++++
tools/testing/selftests/resctrl/resctrlfs.c | 88 +++++--------------
3 files changed, 73 insertions(+), 66 deletions(-)
base-commit: 3b3e8a34b1d50c2c5c6b030dab7682b123162cb4
--
2.42.0
The existing macro KUNIT_ARRAY_PARAM can produce parameter
generator function but only when we fully know the definition
of the array. However, there might be cases where we would like
to generate test params based on externaly defined array, which
is defined as zero-terminated array, like pci_driver.id_table.
Add helper macro KUNIT_ZERO_ARRAY_PARAM that can work with zero
terminated arrays and provide example how to use it.
$ ./tools/testing/kunit/kunit.py run \
--kunitconfig ./lib/kunit/.kunitconfig *.example_params*
[ ] Starting KUnit Kernel (1/1)...
[ ] ============================================================
[ ] ========================= example =========================
[ ] =================== example_params_test ===================
[ ] [SKIPPED] example value 3
[ ] [PASSED] example value 2
[ ] [PASSED] example value 1
[ ] [SKIPPED] example value 0
[ ] =============== [PASSED] example_params_test ===============
[ ] =================== example_params_test ===================
[ ] [SKIPPED] example value 3
[ ] [PASSED] example value 2
[ ] [PASSED] example value 1
[ ] =============== [PASSED] example_params_test ===============
[ ] ===================== [PASSED] example =====================
[ ] ============================================================
[ ] Testing complete. Ran 7 tests: passed: 4, skipped: 3
Signed-off-by: Michal Wajdeczko <michal.wajdeczko(a)intel.com>
Cc: David Gow <davidgow(a)google.com>
Cc: Rae Moar <rmoar(a)google.com>
---
include/kunit/test.h | 22 ++++++++++++++++++++++
lib/kunit/kunit-example-test.c | 2 ++
2 files changed, 24 insertions(+)
diff --git a/include/kunit/test.h b/include/kunit/test.h
index 20ed9f9275c9..280113ceb6a6 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -1514,6 +1514,28 @@ do { \
return NULL; \
}
+/**
+ * KUNIT_ZERO_ARRAY_PARAM() - Define test parameter generator from a zero terminated array.
+ * @name: prefix for the test parameter generator function.
+ * @array: zero terminated array of test parameters.
+ * @get_desc: function to convert param to description; NULL to use default
+ *
+ * Define function @name_gen_params which uses zero terminated @array to generate parameters.
+ */
+#define KUNIT_ZERO_ARRAY_PARAM(name, array, get_desc) \
+ static const void *name##_gen_params(const void *prev, char *desc) \
+ { \
+ typeof((array)[0]) *__prev = prev; \
+ typeof(__prev) __next = __prev ? __prev + 1 : (array); \
+ void (*__get_desc)(typeof(__next), char *) = get_desc; \
+ for (; memchr_inv(__next, 0, sizeof(*__next)); __prev = __next++) { \
+ if (__get_desc) \
+ __get_desc(__next, desc); \
+ return __next; \
+ } \
+ return NULL; \
+ }
+
// TODO(dlatypov(a)google.com): consider eventually migrating users to explicitly
// include resource.h themselves if they need it.
#include <kunit/resource.h>
diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c
index 6bb5c2ef6696..ad9ebcfd513e 100644
--- a/lib/kunit/kunit-example-test.c
+++ b/lib/kunit/kunit-example-test.c
@@ -202,6 +202,7 @@ static void example_param_get_desc(const struct example_param *p, char *desc)
}
KUNIT_ARRAY_PARAM(example, example_params_array, example_param_get_desc);
+KUNIT_ZERO_ARRAY_PARAM(example_zero, example_params_array, example_param_get_desc);
/*
* This test shows the use of params.
@@ -246,6 +247,7 @@ static struct kunit_case example_test_cases[] = {
KUNIT_CASE(example_all_expect_macros_test),
KUNIT_CASE(example_static_stub_test),
KUNIT_CASE_PARAM(example_params_test, example_gen_params),
+ KUNIT_CASE_PARAM(example_params_test, example_zero_gen_params),
KUNIT_CASE_SLOW(example_slow_test),
{}
};
--
2.25.1
Fix four issues with resctrl selftests.
The signal handling fix became necessary after the mount/umount fixes
and the uninitialized member bug was discovered during the review.
The other two came up when I ran resctrl selftests across the server
fleet in our lab to validate the upcoming CAT test rewrite (the rewrite
is not part of this series).
These are developed and should apply cleanly at least on top the
benchmark cleanup series (might apply cleanly also w/o the benchmark
series, I didn't test).
v3:
- Add fix to uninitialized sa_flags
- Handle ksft_exit_fail_msg() in per test functions
- Make signal handler register fails to also exit
- Improve changelogs
v2:
- Include patch to move _GNU_SOURCE to Makefile to allow normal #include
placement
- Rework the signal register/unregister into patch to use helpers
- Fixed incorrect function parameter description
- Use return !!res to avoid confusing implicit boolean conversion
- Improve MBA/MBM success bound patch's changelog
- Tweak Cc: stable dependencies (make it a chain).
Ilpo Järvinen (7):
selftests/resctrl: Fix uninitialized .sa_flags
selftests/resctrl: Extend signal handler coverage to unmount on
receiving signal
selftests/resctrl: Remove duplicate feature check from CMT test
selftests/resctrl: Move _GNU_SOURCE define into Makefile
selftests/resctrl: Refactor feature check to use resource and feature
name
selftests/resctrl: Fix feature checks
selftests/resctrl: Reduce failures due to outliers in MBA/MBM tests
tools/testing/selftests/resctrl/Makefile | 2 +-
tools/testing/selftests/resctrl/cat_test.c | 8 --
tools/testing/selftests/resctrl/cmt_test.c | 3 -
tools/testing/selftests/resctrl/mba_test.c | 2 +-
tools/testing/selftests/resctrl/mbm_test.c | 2 +-
tools/testing/selftests/resctrl/resctrl.h | 7 +-
.../testing/selftests/resctrl/resctrl_tests.c | 82 ++++++++++++-------
tools/testing/selftests/resctrl/resctrl_val.c | 24 +++---
tools/testing/selftests/resctrl/resctrlfs.c | 69 ++++++----------
9 files changed, 96 insertions(+), 103 deletions(-)
--
2.30.2
Missing field KSM is added in g_smaps_rollup[] array as it
fixes assert in function test_proc_pid_smaps_rollup()
Without this patchset test fails for "proc-empty-vm" as can be seen below:
$make TARGETS="proc" kselftest
...
selftests: proc: proc-empty-vm
proc-empty-vm: proc-empty-vm.c:299: test_proc_pid_smaps_rollup:
Assertion `rv == sizeof(g_smaps_rollup) - 1' failed.
/usr/bin/timeout: the monitored command dumped core
Aborted
not ok 5 selftests: proc: proc-empty-vm # exit=134
...
With this patchset test passes for "proc-empty-vm" as can be seen below:
$make TARGETS="proc" kselftest
....
timeout set to 45
selftests: proc: proc-empty-vm
ok 5 selftests: proc: proc-empty-vm
....
Signed-off-by: Swarup Laxman Kotiaklapudi <swarupkotikalapudi(a)gmail.com>
---
tools/testing/selftests/proc/proc-empty-vm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/proc/proc-empty-vm.c b/tools/testing/selftests/proc/proc-empty-vm.c
index b16c13688b88..ee71ce52cb6a 100644
--- a/tools/testing/selftests/proc/proc-empty-vm.c
+++ b/tools/testing/selftests/proc/proc-empty-vm.c
@@ -267,6 +267,7 @@ static const char g_smaps_rollup[] =
"Private_Dirty: 0 kB\n"
"Referenced: 0 kB\n"
"Anonymous: 0 kB\n"
+"KSM: 0 kB\n"
"LazyFree: 0 kB\n"
"AnonHugePages: 0 kB\n"
"ShmemPmdMapped: 0 kB\n"
--
2.34.1
As a general rule, the name of the selftest is printed at the beginning
of every message.
Use "static_keys" (name of the test itself) consistently instead of mixing
"static_key" and "static_keys" at the beginning of the messages in the
test_static_keys script.
Signed-off-by: Javier Carrasco <javier.carrasco.cruz(a)gmail.com>
---
tools/testing/selftests/static_keys/test_static_keys.sh | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/static_keys/test_static_keys.sh b/tools/testing/selftests/static_keys/test_static_keys.sh
index fc9f8cde7d42..3b0f17b81ac2 100755
--- a/tools/testing/selftests/static_keys/test_static_keys.sh
+++ b/tools/testing/selftests/static_keys/test_static_keys.sh
@@ -6,18 +6,18 @@
ksft_skip=4
if ! /sbin/modprobe -q -n test_static_key_base; then
- echo "static_key: module test_static_key_base is not found [SKIP]"
+ echo "static_keys: module test_static_key_base is not found [SKIP]"
exit $ksft_skip
fi
if ! /sbin/modprobe -q -n test_static_keys; then
- echo "static_key: module test_static_keys is not found [SKIP]"
+ echo "static_keys: module test_static_keys is not found [SKIP]"
exit $ksft_skip
fi
if /sbin/modprobe -q test_static_key_base; then
if /sbin/modprobe -q test_static_keys; then
- echo "static_key: ok"
+ echo "static_keys: ok"
/sbin/modprobe -q -r test_static_keys
/sbin/modprobe -q -r test_static_key_base
else
@@ -25,6 +25,6 @@ if /sbin/modprobe -q test_static_key_base; then
/sbin/modprobe -q -r test_static_key_base
fi
else
- echo "static_key: [FAIL]"
+ echo "static_keys: [FAIL]"
exit 1
fi
---
base-commit: cefc06e4de1477dbdc3cb2a91d4b1873b7797a5c
change-id: 20231001-topic-static_keys_selftest_messages-099232047b79
Best regards,
--
Javier Carrasco <javier.carrasco.cruz(a)gmail.com>
This series converts the execveat test to generate KTAP output so it
plays a bit more nicely with automation, KTAP means that kselftest
runners can track the individual tests in the suite rather than just an
overall pass/fail for the suite as a whole.
The first patch adding a perror() equivalent for kselftest was
previously sent as part of a similar conversion for the timers tests:
https://lore.kernel.org/linux-kselftest/8734yyfx00.ffs@tglx/T
there's probably no harm in applying it twice or possibly these should
both go via the kselftest tree - I'm not sure who usually applies timers
test changes.
Signed-off-by: Mark Brown <broonie(a)kernel.org>
---
Mark Brown (2):
kselftest: Add a ksft_perror() helper
selftests/exec: Convert execveat test to generate KTAP output
tools/testing/selftests/exec/execveat.c | 87 ++++++++++++++++++++-------------
tools/testing/selftests/kselftest.h | 14 ++++++
2 files changed, 66 insertions(+), 35 deletions(-)
---
base-commit: 6465e260f48790807eef06b583b38ca9789b6072
change-id: 20230928-ktap-exec-45ea8d28309a
Best regards,
--
Mark Brown <broonie(a)kernel.org>
The ret variable is used to check function return values and assigning
values to it on error has no effect as it is an unused value.
The current implementation uses an additional variable (fret) to return
the error value, which in this case is unnecessary and lead to the above
described misuse. There is no restriction in the current implementation
to always return -1 on error and the actual negative error value can be
returned safely without storing -1 in a specific variable.
Simplify the error checking by using a single variable which always
holds the returned value.
Signed-off-by: Javier Carrasco <javier.carrasco.cruz(a)gmail.com>
---
Changes in v2:
- Remove fret and use a single variable to check errors.
- Link to v1: https://lore.kernel.org/r/20230916-topic-self_uevent_filtering-v1-1-26ede50…
---
tools/testing/selftests/uevent/uevent_filtering.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/uevent/uevent_filtering.c b/tools/testing/selftests/uevent/uevent_filtering.c
index 5cebfb356345..dbe55f3a66f4 100644
--- a/tools/testing/selftests/uevent/uevent_filtering.c
+++ b/tools/testing/selftests/uevent/uevent_filtering.c
@@ -78,7 +78,7 @@ static int uevent_listener(unsigned long post_flags, bool expect_uevent,
{
int sk_fd, ret;
socklen_t sk_addr_len;
- int fret = -1, rcv_buf_sz = __UEVENT_BUFFER_SIZE;
+ int rcv_buf_sz = __UEVENT_BUFFER_SIZE;
uint64_t sync_add = 1;
struct sockaddr_nl sk_addr = { 0 }, rcv_addr = { 0 };
char buf[__UEVENT_BUFFER_SIZE] = { 0 };
@@ -121,6 +121,7 @@ static int uevent_listener(unsigned long post_flags, bool expect_uevent,
if ((size_t)sk_addr_len != sizeof(sk_addr)) {
fprintf(stderr, "Invalid socket address size\n");
+ ret = -1;
goto on_error;
}
@@ -147,11 +148,12 @@ static int uevent_listener(unsigned long post_flags, bool expect_uevent,
ret = write_nointr(sync_fd, &sync_add, sizeof(sync_add));
close(sync_fd);
if (ret != sizeof(sync_add)) {
+ ret = -1;
fprintf(stderr, "Failed to synchronize with parent process\n");
goto on_error;
}
- fret = 0;
+ ret = 0;
for (;;) {
ssize_t r;
@@ -187,7 +189,7 @@ static int uevent_listener(unsigned long post_flags, bool expect_uevent,
on_error:
close(sk_fd);
- return fret;
+ return ret;
}
int trigger_uevent(unsigned int times)
---
base-commit: cefc06e4de1477dbdc3cb2a91d4b1873b7797a5c
change-id: 20230916-topic-self_uevent_filtering-17b53262bc46
Best regards,
--
Javier Carrasco <javier.carrasco.cruz(a)gmail.com>
Kselftest.h declares many variadic functions that can print some
formatted message while also executing selftest logic. These
declarations don't have any compiler mechanism to verify if passed
arguments are valid in comparison with format specifiers used in
printf() calls.
Attribute addition can make debugging easier, the code more consistent
and prevent mismatched or missing variables.
Add a __printf() macro that validates types of variables passed to the
format string. The macro is similarly used in other tools in the kernel.
Add __printf() attributes to function definitions inside kselftest.h that
use printing.
Adding the __printf() macro exposes some mismatches in format strings
across different selftests.
Fix the mismatched format specifiers in multiple tests.
Series is based on kselftests next branch.
Changelog v3:
- Change git signature from Wieczor-Retman Maciej to Maciej
Wieczor-Retman.
- Add one review tag.
- Rebase onto updated kselftests next branch and change base commit.
Changelog v2:
- Add review and fixes tags to patches.
- Add two patches with mismatch fixes.
- Fix missed attribute in selftests/kvm. (Andrew)
- Fix previously missed issues in selftests/mm (Ilpo)
[v2] https://lore.kernel.org/all/cover.1693829810.git.maciej.wieczor-retman@inte…
[v1] https://lore.kernel.org/all/cover.1693216959.git.maciej.wieczor-retman@inte…
Maciej Wieczor-Retman (8):
selftests: Add printf attribute to ksefltest prints
selftests/cachestat: Fix print_cachestat format
selftests/openat2: Fix wrong format specifier
selftests/pidfd: Fix ksft print formats
selftests/sigaltstack: Fix wrong format specifier
selftests/kvm: Replace attribute with macro
selftests/mm: Substitute attribute with a macro
selftests/resctrl: Fix wrong format specifier
.../selftests/cachestat/test_cachestat.c | 2 +-
tools/testing/selftests/kselftest.h | 18 ++++++++++--------
.../testing/selftests/kvm/include/test_util.h | 8 ++++----
tools/testing/selftests/mm/mremap_test.c | 2 +-
tools/testing/selftests/mm/pkey-helpers.h | 2 +-
tools/testing/selftests/openat2/openat2_test.c | 2 +-
.../selftests/pidfd/pidfd_fdinfo_test.c | 2 +-
tools/testing/selftests/pidfd/pidfd_test.c | 12 ++++++------
tools/testing/selftests/resctrl/cache.c | 2 +-
tools/testing/selftests/sigaltstack/sas.c | 2 +-
10 files changed, 27 insertions(+), 25 deletions(-)
base-commit: ce9ecca0238b140b88f43859b211c9fdfd8e5b70
--
2.42.0
SVE 2.1 introduced a new feature FEAT_SVE_B16B16 which adds instructions
supporting the BFloat16 floating point format. Report this to userspace
through the ID registers and hwcap.
Signed-off-by: Mark Brown <broonie(a)kernel.org>
---
Mark Brown (2):
arm64/sve: Report FEAT_SVE_B16B16 to userspace
kselftest/arm64: Verify HWCAP2_SVE_B16B16
Documentation/arch/arm64/cpu-feature-registers.rst | 2 ++
Documentation/arch/arm64/elf_hwcaps.rst | 3 +++
arch/arm64/include/asm/hwcap.h | 1 +
arch/arm64/include/uapi/asm/hwcap.h | 1 +
arch/arm64/kernel/cpufeature.c | 3 +++
arch/arm64/kernel/cpuinfo.c | 1 +
arch/arm64/tools/sysreg | 6 +++++-
tools/testing/selftests/arm64/abi/hwcap.c | 13 +++++++++++++
8 files changed, 29 insertions(+), 1 deletion(-)
---
base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d
change-id: 20230913-arm64-zfr-b16b16-el0-0811fc70f147
Best regards,
--
Mark Brown <broonie(a)kernel.org>
Write_schemata() uses fprintf() to write a bitmask into a schemata file
inside resctrl FS. It checks fprintf() return value but it doesn't check
fclose() return value. Error codes from fprintf() such as write errors,
are buffered and flushed back to the user only after fclose() is executed
which means any invalid bitmask can be written into the schemata file.
Rewrite write_schemata() to use syscalls instead of stdio file
operations to avoid the buffering.
The resctrlfs.c file defines functions that interact with the resctrl FS
while resctrl_val.c file defines functions that perform measurements on
the cache. Run_benchmark() fits logically into the second file before
resctrl_val() function that uses it.
Move run_benchmark() from resctrlfs.c to resctrl_val.c and remove
redundant part of the kernel-doc comment. Make run_benchmark() static
and remove it from the header file.
Patch series is based on [1] which is based on [2] which are based on
ksefltest next branch.
Changelog v4:
- Change git signature from Wieczor-Retman Maciej to Maciej
Wieczor-Retman.
- Rebase onto [1] which is based on [2]. (Reinette)
- Add fcntl.h explicitly to provide glibc backward compatibility.
(Reinette)
Changelog v3:
- Use snprintf() return value instead of strlen() in write_schemata().
(Ilpo)
- Make run_benchmark() static and remove it from the header file.
(Reinette)
- Added Ilpo's reviewed-by tag to Patch 2/2.
- Patch messages and cover letter rewording.
Changelog v2:
- Change sprintf() to snprintf() in write_schemata().
- Redo write_schemata() with syscalls instead of stdio functions.
- Fix typos and missing dots in patch messages.
- Branch printf attribute patch to a separate series.
[v1] https://lore.kernel.org/all/cover.1692880423.git.maciej.wieczor-retman@inte…
[v2] https://lore.kernel.org/all/cover.1693213468.git.maciej.wieczor-retman@inte…
[v3] https://lore.kernel.org/all/cover.1693575451.git.maciej.wieczor-retman@inte…
[1] https://lore.kernel.org/all/20230915154438.82931-1-ilpo.jarvinen@linux.inte…
[2] https://lore.kernel.org/all/20230904095339.11321-1-ilpo.jarvinen@linux.inte…
Maciej Wieczor-Retman (2):
selftests/resctrl: Fix schemata write error check
selftests/resctrl: Move run_benchmark() to a more fitting file
tools/testing/selftests/resctrl/resctrl.h | 1 -
tools/testing/selftests/resctrl/resctrl_val.c | 50 +++++++++++
tools/testing/selftests/resctrl/resctrlfs.c | 82 ++++---------------
3 files changed, 67 insertions(+), 66 deletions(-)
base-commit: 3b3e8a34b1d50c2c5c6b030dab7682b123162cb4
--
2.42.0
Diagnostic message for failed KUNIT_ASSERT|EXPECT_NOT_ERR_OR_NULL
shows only raw error code, like in this example:
[ ] # example_all_expect_macros_test: EXPECTATION FAILED at lib/kunit/kunit-example-test.c:126
[ ] Expected myptr is not error, but is: -12
but we can improve it by using more friendly error pointer format:
[ ] # example_all_expect_macros_test: EXPECTATION FAILED at lib/kunit/kunit-example-test.c:126
[ ] Expected myptr is not error, but is -ENOMEM
Signed-off-by: Michal Wajdeczko <michal.wajdeczko(a)intel.com>
Cc: David Gow <davidgow(a)google.com>
Cc: Rae Moar <rmoar(a)google.com>
---
lib/kunit/assert.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c
index dd1d633d0fe2..96ef236d3ca3 100644
--- a/lib/kunit/assert.c
+++ b/lib/kunit/assert.c
@@ -80,9 +80,9 @@ void kunit_ptr_not_err_assert_format(const struct kunit_assert *assert,
ptr_assert->text);
} else if (IS_ERR(ptr_assert->value)) {
string_stream_add(stream,
- KUNIT_SUBTEST_INDENT "Expected %s is not error, but is: %ld\n",
+ KUNIT_SUBTEST_INDENT "Expected %s is not error, but is %pe\n",
ptr_assert->text,
- PTR_ERR(ptr_assert->value));
+ ptr_assert->value);
}
kunit_assert_print_msg(message, stream);
}
--
2.25.1
Commit 804b854d374e ("net: bridge: disable bridge MTU auto tuning if it
was set manually") disabled auto-tuning of the bridge MTU when the MTU
was explicitly set by the user, however that would only happen when the
MTU was set after creation. This commit ensures auto-tuning is also
disabled when the MTU is set during bridge creation.
Currently when the br_netdev_ops br_change_mtu function is called, the
flag BROPT_MTU_SET_BY_USER is set. However this function is only called
when the MTU is changed after interface creation and is not called if
the MTU is specified during creation with IFLA_MTU (br_dev_newlink).
br_change_mtu also does not get called if the MTU is set to the same
value it currently has, which makes it difficult to work around this
issue (especially for the default MTU of 1500) as you have to first
change the MTU to some other value and then back to the desired value.
Add new selftests to ensure the bridge MTU is handled correctly:
- Bridge created with user-specified MTU (1500)
- Bridge created with user-specified MTU (2000)
- Bridge created without user-specified MTU
- Bridge created with user-specified MTU set after creation (2000)
Regression risk: Any workload which erroneously specified an MTU during
creation but accidentally relied upon auto-tuning to a different value
may be broken by this change.
Link: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2034099
Fixes: 804b854d374e ("net: bridge: disable bridge MTU auto tuning if it was set manually")
Signed-off-by: Trent Lloyd <trent.lloyd(a)canonical.com>
---
net/bridge/br_netlink.c | 3 +
.../selftests/drivers/net/bridge/Makefile | 10 ++
.../drivers/net/bridge/bridge-user-mtu.sh | 148 ++++++++++++++++++
.../drivers/net/bridge/net_forwarding_lib.sh | 1 +
4 files changed, 162 insertions(+)
create mode 100644 tools/testing/selftests/drivers/net/bridge/Makefile
create mode 100755 tools/testing/selftests/drivers/net/bridge/bridge-user-mtu.sh
create mode 120000 tools/testing/selftests/drivers/net/bridge/net_forwarding_lib.sh
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 10f0d33d8ccf..8aff7d077848 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -1559,6 +1559,9 @@ static int br_dev_newlink(struct net *src_net, struct net_device *dev,
spin_unlock_bh(&br->lock);
}
+ if (tb[IFLA_MTU])
+ br_opt_toggle(br, BROPT_MTU_SET_BY_USER, true);
+
err = br_changelink(dev, tb, data, extack);
if (err)
br_dev_delete(dev, NULL);
diff --git a/tools/testing/selftests/drivers/net/bridge/Makefile b/tools/testing/selftests/drivers/net/bridge/Makefile
new file mode 100644
index 000000000000..23e407c75a7f
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/bridge/Makefile
@@ -0,0 +1,10 @@
+# SPDX-License-Identifier: GPL-2.0
+# Makefile for net selftests
+
+TEST_PROGS := \
+ bridge-user-mtu.sh
+
+TEST_FILES := \
+ net_forwarding_lib.sh
+
+include ../../../lib.mk
diff --git a/tools/testing/selftests/drivers/net/bridge/bridge-user-mtu.sh b/tools/testing/selftests/drivers/net/bridge/bridge-user-mtu.sh
new file mode 100755
index 000000000000..07e0ac972b00
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/bridge/bridge-user-mtu.sh
@@ -0,0 +1,148 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Ensure a bridge MTU does not automatically change when it has been specified
+# by the user.
+#
+# To run independently:
+# make TARGETS=drivers/net/bridge kselftest
+
+ALL_TESTS="
+ bridge_created_with_user_specified_mtu
+ bridge_created_without_user_specified_mtu
+ bridge_with_late_user_specified_mtu
+"
+
+REQUIRE_MZ=no
+NUM_NETIFS=0
+lib_dir=$(dirname "$0")
+source "${lib_dir}"/net_forwarding_lib.sh
+
+setup_prepare()
+{
+ for i in 1 3 5; do
+ ip link add "vtest${i}" mtu 9000 type veth peer name "vtest${i}b" mtu 9000
+ done
+}
+
+cleanup()
+{
+ for interface in vtest1 vtest3 vtest5 br-test0 br-test1 br-test2; do
+ if [[ -d "/sys/class/net/${interface}" ]]; then
+ ip link del "${interface}" &> /dev/null
+ fi
+ done
+}
+
+check_mtu()
+{
+ cur_mtu=$(<"/sys/class/net/$1/mtu")
+ [[ ${cur_mtu} -eq $2 ]]
+ exit_status=$?
+ return "${exit_status}"
+}
+
+check_bridge_user_specified_mtu()
+{
+ if [[ -z $1 ]]
+ then
+ exit 1
+ fi
+ mtu=$1
+
+ RET=0
+
+ ip link add dev br-test0 mtu "${mtu}" type bridge
+ ip link set br-test0 up
+ check_mtu br-test0 "${mtu}"
+ check_err $? "Bridge was not created with the user-specified MTU"
+
+ check_mtu vtest1 9000
+ check_err $? "vtest1 does not have MTU 9000"
+
+ ip link set dev vtest1 master br-test0
+ check_mtu br-test0 "${mtu}"
+ check_err $? "Bridge user-specified MTU incorrectly changed after adding an interface"
+
+ log_test "Bridge created with user-specified MTU (${mtu})"
+
+ ip link del br-test0
+}
+
+bridge_created_with_user_specified_mtu() {
+ # Check two user-specified MTU values
+ # - 1500: To ensure the default MTU (1500) is not special-cased, you
+ # should be able to lock a bridge to the default MTU.
+ # - 2000: Ensure bridges are actually created with a user-specified MTU
+ check_bridge_user_specified_mtu 1500
+ check_bridge_user_specified_mtu 2000
+}
+
+bridge_created_without_user_specified_mtu()
+{
+ RET=0
+ ip link add dev br-test1 type bridge
+ ip link set br-test1 up
+ check_mtu br-test1 1500
+ check_err $? "Bridge was not created with the user-specified MTU"
+
+ ip link set dev vtest3 master br-test1
+ check_mtu br-test1 9000
+ check_err $? "Bridge without user-specified MTU did not change MTU"
+
+ log_test "Bridge created without user-specified MTU"
+
+ ip link del br-test1
+}
+
+check_bridge_late_user_specified_mtu()
+{
+ if [[ -z $1 ]]
+ then
+ exit 1
+ fi
+ mtu=$1
+
+ RET=0
+ ip link add dev br-test2 type bridge
+ ip link set br-test2 up
+ check_mtu br-test2 1500
+ check_err $? "Bridge was not created with default MTU (1500)"
+
+ ip link set br-test2 mtu "${mtu}"
+ check_mtu br-test2 "${mtu}"
+ check_err $? "User-specified MTU set after creation was not set"
+ check_mtu vtest5 9000
+ check_err $? "vtest5 does not have MTU 9000"
+
+ ip link set dev vtest5 master br-test2
+ check_mtu br-test2 "${mtu}"
+ check_err $? "Bridge late-specified MTU incorrectly changed after adding an interface"
+
+ log_test "Bridge created without user-specified MTU and changed after (${mtu})"
+
+ ip link del br-test2
+}
+
+bridge_with_late_user_specified_mtu()
+{
+ # Note: Unfortunately auto-tuning is not disabled when you set the MTU
+ # to it's current value, including the default of 1500. The reason is
+ # that dev_set_mtu_ext skips notifying any handlers if the MTU is set
+ # to the current value. Normally that makes sense, but is confusing
+ # since you might expect "ip link set br0 mtu 1500" to lock the MTU to
+ # 1500 but that will only happen if the MTU was not already 1500. So we
+ # only check a non-default value of 2000 here unlike the earlier
+ # bridge_created_with_user_specified_mtu test
+
+ # Check one user-specified MTU value
+ # - 2000: Ensure bridges actually change to a user-specified MTU
+ check_bridge_late_user_specified_mtu 2000
+}
+
+trap cleanup EXIT
+
+setup_prepare
+tests_run
+
+exit "${EXIT_STATUS}"
diff --git a/tools/testing/selftests/drivers/net/bridge/net_forwarding_lib.sh b/tools/testing/selftests/drivers/net/bridge/net_forwarding_lib.sh
new file mode 120000
index 000000000000..39c96828c5ef
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/bridge/net_forwarding_lib.sh
@@ -0,0 +1 @@
+../../../net/forwarding/lib.sh
\ No newline at end of file
--
2.34.1