lib/*: Add syscall interfaces for dotnet runtime

[i-Pear]

Prerequisite checklist

• Read the contribution guidelines regarding submitting new changes to the project;
• Tested your changes against relevant architectures and platforms;
• Ran the checkpatch.uk on your commit series before opening this PR;
• Updated relevant documentation.

Description of changes

Stub the following syscall interfaces for dotnet runtime 7.0:

• posix-mmap: SYS_mlock and SYS_msync
• uksched: SYS_sched_getaffinity and SYS_sched_setaffinity

[StefanJum]

Thank you for this @i-Pear.
Please add a UK_WARN_STUBBED() in the stubbed syscalls (see an example here) and wrap the commit messages to ~50 characters per line.

Also, does the dotnet require the syscalls to return success? If not I would do an errno = ENOTSUP; return -1 where it is the case.

[i-Pear]

@StefanJum Thanks for your review. msync doesn't need to return success, so I changed it to -ENOTSUP. But others must return 0 to prevent the dotnet runtime from crashing.

[mschlumpp]

msync doesn't need to return success, so I changed it to -ENOTSUP.

I think we should return zero from msync. It's unlikely that it breaks anything and we don't run into issues where software only checks for the documented error codes.

[StefanJum]

I think we should return zero from msync. It's unlikely that it breaks anything and we don't run into issues where software only checks for the documented error codes.

Agree. I was a bit worried about the mknodat returning 0, the others I doubt will matter that much.

[i-Pear]

Changed msync to return zero.
The mknodat is for the dotnet runtime to create /tmp/clr-debug-pipe, seems to be pipes for connecting to the debugger.

[i-Pear]

I found that adding env COMPlus_EnableDiagnostics=0 can prevent creating these debug pipes. So just assert users will add this env, and remove mknodat from this PR? @StefanJum

However, I realize that the design of debugging through pipes is quite interesting. This means that we might be able to debug dotnet programs in unikernels, which is challenging to achieve with other languages.

[StefanJum]

I found that adding env COMPlus_EnableDiagnostics=0 can prevent creating these debug pipes. So just assert users will add this env, and remove mknodat from this PR? @StefanJum

Yes, I think that would be safer than stubbing mknodat, just have the users set the env variable in the elfloader config.
Any thoughts @mschlumpp @razvand?

@i-Pear if this is selectable (via an env variable), could there be a way to completely disable it in the dotnet configuration step?

However, I realize that the design of debugging through pipes is quite interesting. This means that we might be able to debug dotnet programs in unikernels, which is challenging to achieve with other languages.

That would be nice indeed, it would require a proper implementation of mknodat though.

[i-Pear]

if this is selectable (via an env variable), could there be a way to completely disable it in the dotnet configuration step?

Do you mean build our own dotnet framework? I think it will be easy to modify the default value in the source code, if you think this is worth. As the number of applications grows, we might find ourselves in need of developing our own package manager to effectively manage them.

&& update: removed mknodat from this PR.

[StefanJum]

Do you mean build our own dotnet framework? I think it will be easy to modify the default value in the source code, if you think this is worth. As the number of applications grows, we might find ourselves in need of developing our own package manager to effectively manage them.

We do have (or soon will have, I don't think there are any merged yet) dynamic apps that are build from source (this pr for example).
I think we could do the same here, provide a script that does everything, and in the end have 2 versions in the dynamic-apps repo, the one you opened a pr for, already built, where you will need to set the env variable, and the one that will be build from source.

In the meantime, we'll just state in the dynamic-apps/lang/c-sharp/README.md file that COMPlus_EnableDiagnostics=0 needs to be set.

[StefanJum]

Looks fine @i-Pear, just one more comment below.

[i-Pear]

Looks fine @i-Pear, just one more comment below.

Fixed. Besides, while porting ASP.NET, I found it uses sched_getaffinity to get ncpus, and it must > 0, which cost me a lot of time. I think it would be better to return a positive value, while not having imported the affinity related functions and headers.

[i-Pear]

I think we could do the same here, provide a script that does everything, and in the end have 2 versions in the dynamic-apps repo, the one you opened a pr for, already built, where you will need to set the env variable, and the one that will be build from source.

It works. For backup:

• Change EXTERNAL_EnableDiagnostics 's default value to 0.
• And overwrite build/runtime/artifacts/source-build/self/src/artifacts/bin/coreclr/Linux.x64.Release/libcoreclr.so to /usr/share/dotnet/shared/Microsoft.NETCore.App/7.0.7/libcoreclr.so

[StefanJum]

It works. For backup:

* Change [EXTERNAL_EnableDiagnostics](https://github.com/dotnet/runtime/blob/ef3fd8523ddc69e4f508df2d1875b088e2ee3833/src/coreclr/inc/clrconfigvalues.h#L167) 's default value to `0`.

* And overwrite `build/runtime/artifacts/source-build/self/src/artifacts/bin/coreclr/Linux.x64.Release/libcoreclr.so` to `/usr/share/dotnet/shared/Microsoft.NETCore.App/7.0.7/libcoreclr.so`

Perfect, if it's just the libcoreclr.so we need to modify I saw we just overwrite that in the pr you opened on dynamic-apps.
We can still document the change in a README file.

[i-Pear]

Perfect, if it's just the libcoreclr.so we need to modify I saw we just overwrite that in the pr you opened on dynamic-apps. We can still document the change in a README file.

Done, the readme is here: https://github.com/unikraft/dynamic-apps/pull/60/files

Provided patched libcoreclr.so and origin libcoreclr.so.bak

[i-Pear]

I realized that unikraft doesn't support swap memory, so mlock doesn't make sense. Moreover, according to the description in [1], unikraft also doesn't support saving mmap data to file, causing msync also to be meaningless, it can simply return 0.

[1] https://github.com/unikraft/unikraft/blob/staging/lib/ukvmem/vma_file.c#L230

[i-Pear]

It also seems that unikraft only supports running on a single cpu. I tried to run qemu with -smp 4 but only one core is used. If so, sched_getaffinity and sched_setaffinity also seems meaningless -- there's only one cpu available.

[unikraft-bot]

⚠️ Checkpatch passed with warnings

Beep boop! I ran Unikraft's checkpatch.pl support script on your pull request but it encountered warnings:

SHA	commit	checkpatch
783152c	lib/uksched: Stub `sched_getaffinity` & `sched_setaffinity`	⚠️
34cb1ee	lib/posix-mmap: Stub `mlock` & `msync`	✅

Truncated logs starting from first warning 783152c:

WARNING:LONG_LINE: line over 80 characters
#46: FILE: lib/uksched/sched.c:396:
+UK_SYSCALL_R_DEFINE(int, sched_getaffinity, int, pid, long, cpusetsize, unsigned long*, mask)

WARNING:LONG_LINE: line over 80 characters
#63: FILE: lib/uksched/sched.c:413:
+UK_SYSCALL_R_DEFINE(int, sched_setaffinity, int, pid, long, cpusetsize, unsigned long*, mask)

total: 0 errors, 2 warnings, 39 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/tmp/build/a53d4c5b/patches/0001-lib-uksched-Stub-sched_getaffinity-sched_setaffinity.patch has style problems, please review.

NOTE: Ignored message types: ASSIGN_IN_IF FILE_PATH_CHANGES NEW_TYPEDEFS OBSOLETE

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.

---

View complete logs | Learn more about Unikraft's coding style and contribution guidelines.

[StefanJum]

It also seems that unikraft only supports running on a single cpu. I tried to run qemu with -smp 4 but only one core is used. If so, sched_getaffinity and sched_setaffinity also seems meaningless -- there's only one cpu available.

Yes, that's being worked on, but we can go for the implementation you provided for now.

[StefanJum]

Few more comments @i-Pear, it looks fine besides those.

[i-Pear]

Few more comments @i-Pear, it looks fine besides those.

Thanks, these have been fixed.

[StefanJum]

All good now, thank you @i-Pear.
Reviewed-by: Stefan Jumarea stefanjumarea02@gmail.com

[RaduNichita]

All good with me.

Reviewed-by: Radu Nichita radunichita99@gmail.com

[razvand]

Approved-by: Razvan Deaconescu razvand@unikraft.io