Some el10 fixes for #284 #287

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merged

cns-ci-onload-xilinx merged 1 commit into Xilinx-CNS:v9_0 from lukester1975:fix-el10

Jun 10, 2025

Merged

Some el10 fixes for #284 #287

cns-ci-onload-xilinx merged 1 commit into Xilinx-CNS:v9_0 from lukester1975:fix-el10

Jun 10, 2025

Conversation

@lukester1975

Copy link

Contributor

@lukester1975 lukester1975 commented Jun 9, 2025

No description provided.

@lukester1975


 Ensure egg-info is created with python >=3.12.

d71b6c3

@lukester1975 lukester1975 requested a review from a team as a code owner

June 9, 2025 12:11

@lukester1975 lukester1975 changed the base branch from master to v9_0

June 9, 2025 12:12

@ivatet-amd

Copy link

Contributor

ivatet-amd commented Jun 9, 2025

Hi @lukester1975, thank you for all your extremely useful contributions over the past few days!

While we're verifying your Python patch, would you be happy to reconsider the LTO patch from this PR?

AFAIU, the LTO error (error: inlining failed in call to ‘always_inline’ ‘open’: function body can be overwritten at link time) is legitimate because startup.c, that triggers the LTO error, belongs to the library where Onload intercepts syscalls, and genuinely overrides the open(), so that occurrence cannot be hardneded with the "fortify" feature. Instead, in startup.c we should use the unambiguous ci_sys_open(), also to avoid problems outlined in citp_basic_syscall_init().

Internally, we have a PR with two patches:

$ git diff HEAD~2
diff --git a/src/lib/transport/unix/startup.c b/src/lib/transport/unix/startup.c
index 7a12c4cf8d..7bcf94dfa9 100644
--- a/src/lib/transport/unix/startup.c
+++ b/src/lib/transport/unix/startup.c
@@ -213,7 +213,7 @@ static void citp_log_to_file(const char *s)
 {
 int fd;
 ci_assert(!CITP_OPTS.log_via_ioctl);
- fd = open(s, O_WRONLY | O_CREAT | O_TRUNC, S_IREAD | S_IWRITE);
+ fd = ci_sys_open(s, O_WRONLY | O_CREAT | O_TRUNC, S_IREAD | S_IWRITE);
 if( fd >= 0 ) {
 if( citp.log_fd >= 0 )
 ci_sys_close(citp.log_fd);
diff --git a/src/tests/onload/cplane_sysunit/cplane_lib.c b/src/tests/onload/cplane_sysunit/cplane_lib.c
index a352e7b350..92289dfe47 100644
--- a/src/tests/onload/cplane_sysunit/cplane_lib.c
+++ b/src/tests/onload/cplane_sysunit/cplane_lib.c
@@ -4,7 +4,7 @@
 #include <cplane/cplane.h>
 #include <cplane/server.h>
-extern int cplane_ioctl(int, long unsigned int, void* );
+extern int cplane_ioctl(int, long unsigned int, ...);
 int (* ci_sys_ioctl)(int, long unsigned int, ...) = (void*) cplane_ioctl;

Would you be able to give it (enabled-by-default-LTO + two patches above) a quick spin to see if it addresses the LTO problem in your setup (and EF_LOG_FILE still works OK)? If we keep LTO enabled, it might help us avoid/find more defects in the future.