unikraft / lib-libsodium

Unikraft port of libsodium, a crypto library
Other
0 stars 5 forks source link

Migrate libsodium tests to use uktest #3

Closed michpappas closed 2 years ago

michpappas commented 2 years ago

Migrate libsodium tests to use uktest and introduce the libsodium_minimal testuite.

Tests are enabled either by selecting either of the CONFIG_LIBSODIUM_TESTS or CONFIG_LIBUKTEST_ALL options.

Signed-off-by: Michalis Pappas Michalis.Pappas@opensynergy.com

razvand commented 2 years ago

@nderjung , do you know of any issues with uktest on ARM or Xen? Should it work seamlessly on those as well?

michpappas commented 2 years ago

@maniatro111 , I only tested on KVM and linuxu so I'm not sure about Xen. Are you able for example to run self-tests on Xen without lib-libsodium?

flpostolache commented 2 years ago

@michpappas I don't really understand what do you mean with your question. Can you please explain to me a little bit more about self-tests without lib-libsodium? Or if you could guide me to a page where I could read about this, it would be awesome. Also, if it helps, I could explain to you how I managed to get those errors.

nderjung commented 2 years ago

@maniatro111, can you try running a fresh clone of the helloworld application on xen and arm with the self-tests enabled (i.e. LIBUKTEST_TEST_MYSELF=y actually, enable everything in uktest and also LIBUKDEBUG_PRINTK_INFO=y and LIBUKDEBUG_PRINTD=y etc. to get a full verbose output)?

michpappas commented 2 years ago

@nderjung, @maniatro111 just found out that self-tests fail on KVM. I opened #393.

michpappas commented 2 years ago

Updated the PR to comply with the recommendations of uk_test:

 * 1. A single test suite should be organised into its own file, prefixed with
 *    `test_`, e.g. `test_feature.c`. All tests suites of some library within
 *    Unikraft should be stored within a new folder located at the root of the
 *    library named `tests/`.
 * 2. All tests suites should have a corresponding KConfig option, prefixed with
 *    the library name and then the word "TEST", e.g. `LIBNAME_TEST_`.
 * 3. Every library implementing one or more suite of tests must have a new
 *    menuconfig housing all test suite options under the name `LIBNAME_TEST`.
 *    This menuconfig option must invoke all the suites if `LIBUKTEST_ALL` is
 *    set to true.

@nderjung I set LIBSODIUM_TEST as depends on LIBUKTEST. Alternatively it would be selects LIBUKTEST. Is there any preference? It would be useful if there was a policy on how this should across different libraries, for consistency.

@maniatro111 does https://github.com/unikraft/unikraft/pull/397 fix the issue with Xen?

flpostolache commented 2 years ago

@michpappas, @nderjung It seems that there is a problem when running self-tests on xen. I did just like @nderjung said. I downloaded the helloworld application, enabled everything in uktest and what was needed in ukdebug and the app also crashed. On arm-kvm everything seems to be all right now.

Output from running helloworld with self-tests on xen: sudo xl create -c app-helloworld.cfg Parsing config from app-helloworld.cfg [182470.845670] Info: [libxenplat] <setup.c @ 195> Entering from Xen (x86, PV)... [999820.068501] Info: [libxenplat] <setup.c @ 205> start_info: 0x63000 [999820.068502] Info: [libxenplat] <setup.c @ 206> shared_info: 0x1000 [999820.068503] Info: [libxenplat] <setup.c @ 207> hypercall_page: 0x2000 [999820.068503] Info: [libxenplat] <setup.c @ 138> start_pfn: 6b [999820.068504] Info: [libxenplat] <setup.c @ 139> max_pfn: 400 [999820.068505] Info: [libxenplat] <mm.c @ 163> Mapping memory range 0x6b000 - 0x400000 [999820.068509] dbg: [libxenplat] <mm.c @ 673> Clear bootstrapping memory: 0 [999820.068512] dbg: [libxenplat] <mm.c @ 589> Set 0-0x1b000 readonly [999820.068512] dbg: [libxenplat] <mm.c @ 628> skipped 1000 [999820.068520] Info: [libxenplat] <mm.c @ 780> Demand map pfns at 100000000000-108000000000. [999820.068521] dbg: [libxenplat] <hv_console.c @ 241> hv_console @ 0x65000 (evtchn: 2) [999820.068524] Info: [libukboot] <boot.c @ 200> Unikraft constructor table at 0x16000 - 0x16010 [999820.068536] dbg: [libukboot] <boot.c @ 203> Call constructor: 0x145e0())... test: uktest_myself_testsuite->uktest_test_sanity : expected ((void *) 0) to be 0 and was 0 ....................................... [ PASSED ] [999820.068564] CRIT: [libxenplat] <traps.c @ 123> Page fault at linear address 1a770, rip 14795, regs 0x3fcc0, sp 3fd70, our_sp 0x3fc98, code 3 [999820.068590] CRIT: [libxenplat] <trace.c @ 41> RIP: 0000000000014795 CS: e030 [999820.068617] CRIT: [libxenplat] <trace.c @ 43> RSP: 000000000003fd70 SS: e02b EFLAGS: 00010093 [999820.068628] CRIT: [libxenplat] <trace.c @ 45> RAX: 000000000001a770 RBX: 000000000000002a RCX: 000000000001a76c [999820.068640] CRIT: [libxenplat] <trace.c @ 47> RDX: 0000000000000001 RSI: 000000000003f84c RDI: 0000000000000004 [999820.068653] CRIT: [libxenplat] <trace.c @ 49> RBP: 000000000003feb0 R08: 0000000000000067 R09: 0000000000000067 [999820.068678] CRIT: [libxenplat] <trace.c @ 51> R10: 000000000003f870 R11: 0000000000000004 R12: 000000000000002a [999820.068691] CRIT: [libxenplat] <trace.c @ 53> R13: 000000000001a744 R14: 000000000003fd90 R15: 0000000000000001 [999820.068704] CRIT: [libxenplat] <trace.c @ 86> base is 0x3feb0 caller is 0x14816 [999820.068732] CRIT: [libxenplat] <trace.c @ 86> base is 0x3fef0 caller is 0x145a6 [999820.068747] CRIT: [libxenplat] <trace.c @ 86> base is 0x3ff30 caller is 0x113ad [999820.068773] CRIT: [libxenplat] <trace.c @ 86> base is 0x3ffa0 caller is 0x11624 [999820.068789] CRIT: [libxenplat] <trace.c @ 86> base is 0x3ffc0 caller is 0x44e9 [999820.068806] CRIT: [libxenplat] <trace.c @ 86> base is 0x3fff0 caller is 0x59 [999820.068822] CRIT: [libxenplat] <trace.c @ 66> [999820.068833] CRIT: [libxenplat] <trace.c @ 66> 3fd60: 70 fd 03 00 00 00 00 00 2b e0 00 00 00 00 00 00 [999820.068897] CRIT: [libxenplat] <trace.c @ 66> 3fd70: 00 00 00 00 00 00 00 00 30 00 00 00 30 00 00 00 [999820.068960] CRIT: [libxenplat] <trace.c @ 66> 3fd80: c8 fe 03 00 00 00 00 00 e0 fd 03 00 00 00 00 00 [999820.069022] CRIT: [libxenplat] <trace.c @ 66> 3fd90: 65 78 70 65 63 74 65 64 20 60 28 28 76 6f 69 64 [999820.069077] CRIT: [libxenplat] <trace.c @ 66> [999820.069090] CRIT: [libxenplat] <trace.c @ 66> 3fea0: ab a7 01 00 00 00 00 00 00 0b 06 00 00 00 00 00 [999820.069154] CRIT: [libxenplat] <trace.c @ 66> 3feb0: f0 fe 03 00 00 00 00 00 16 48 01 00 00 00 00 00 [999820.069219] CRIT: [libxenplat] <trace.c @ 66> 3fec0: 00 00 00 00 00 00 00 00 a6 1d 01 00 00 00 00 00 [999820.069280] CRIT: [libxenplat] <trace.c @ 66> 3fed0: b0 ff 03 00 00 00 00 00 30 00 00 00 b0 b0 ad de [999820.069336] CRIT: [libxenplat] <trace.c @ 66> [999820.069349] CRIT: [libxenplat] <trace.c @ 66> 14780: 0f b7 10 66 83 fa ff 75 e7 eb 15 0f 1f 44 00 00 [999820.069424] CRIT: [libxenplat] <trace.c @ 66> 14790: ba 01 00 00 00 66 89 10 eb 2c 66 0f 1f 44 00 00 [999820.069488] CRIT: [libxenplat] <trace.c @ 66> 147a0: 41 b9 3c 8a 01 00 41 b8 9c 74 01 00 b9 8d 02 00 [999820.069564] CRIT: [libxenplat] <trace.c @ 66> 147b0: 00 e9 62 ff ff ff 66 2e 0f 1f 84 00 00 00 00 00 [999820.069617] CRIT: [libxenplat] <traps.c @ 133> Crashing

When running the helloworld application without self-tests, everything seems to work ok.

Output without self-tests:

Parsing config from app-helloworld.cfg Powered by

o.   .o       _ _               __ _
Oo   Oo  ___ (_) | __ __  __ _ ' _) :_
oO   oO ' _ `| | |/ /  _)' _` | |_|  _)
oOo oOO| | | | |   (| | | (_) |  _) :_
 OoOoO ._, ._:_:_,\_._,  .__,_:_, \___)
             Mimas 0.7.0~da0f1ee-custom

Hello world! Arguments: "app-helloworld"

Also, the self-tests for libsodium still crash on xen. I opened #417.

michpappas commented 2 years ago

Hi @maniatro111, thanks for that. In that case we can consider libsodium tests to be okay?

flpostolache commented 2 years ago

Yes.

michpappas commented 2 years ago

@nderjung I set LIBSODIUM_TEST as depends on LIBUKTEST. Alternatively it would be selects LIBUKTEST. Is there any preference? It would be useful if there was a policy on how this should across different libraries, for consistency.

@nderjung any thoughts on this? Unless there is a policy, I now lean towards changing it to selects, as I generally find it counterintuitive when certain features in KConfig magically appear only if you know the secret combination of the right options that need to be enabled first...

michpappas commented 2 years ago

Updated Config.uk and Makefile.uk to use select based on the outcome of the relevant discussion in https://github.com/unikraft/unikraft/pull/431

@maniatro111, @razvand maybe we can merge this now?

PS: In case you would like to test it, plase make sure you cherry-pick the fix for ukargparse in https://github.com/unikraft/unikraft/pull/439 and use at least 32MiB of memory in QEMU.

flpostolache commented 2 years ago

@michpappas In my opinion, it is ok to merge it. I looked at it again and tested it again and everything is ok.

michpappas commented 2 years ago

@maniatro111 thanks, can you please add your RB so that the bot can add it to the commit message?

michpappas commented 2 years ago

The include/sodium/uk_sodium_test.h should be removed, right? It was used for the initial (non-uktest) implementation.

Thanks, fixed 👍🏼

Also, any reason for the 0003... and 0005... patches? Why not simply add the files in the repository?

This is in the scope of the previous PR that introduced the tests 😛 So the reason is that these are changes in libsodium's test suite, so they need to be in the library's source tree. The problem is that their test-suite is invoked at the host by make test, we want instead to run them on the target. I am planning to have a look at some point to see if I can merge a fix upstream that would allow running tests on the target too. Then we will be able to get rid of those patches altogether.