Skip to content
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

Run without itimer support in alpine #143

Open
wants to merge 5 commits into
base: sockperf_v2
Choose a base branch
from

Conversation

salsal97
Copy link

Added changes to -

  1. Build on alpine
  2. Run without timer support in the kernel using sleep in a separate thread and a signal to callback.

@salsal97 salsal97 changed the title Notimers Run without itimer support in alpine Feb 11, 2021
@swx-jenkins2
Copy link

Can one of the admins verify this patch?

src/client.cpp Outdated Show resolved Hide resolved
src/client.cpp Outdated Show resolved Hide resolved
src/client.cpp Outdated Show resolved Hide resolved
src/client.cpp Outdated Show resolved Hide resolved
@@ -33,7 +33,8 @@
#include <unistd.h>
#include <stdlib.h>
#include <signal.h>
#include <sys/poll.h>
#include <stdio.h>
Copy link
Contributor

@ChrisCoe ChrisCoe Feb 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is stdio.h being included to vma-redirect.h when vma-redirect.cpp has not been touched? Not clear to me.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, this is another change required to build sockperf in alpine OS

@@ -103,6 +103,7 @@ with simple integral values. The following describes these calculations:
#endif

#include "ticks_os.h"
#include "os_abstract.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, I see that you use os_thread_t in file client.cpp. Why not include header os_abstract.h in client.h?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is one of the changes required to properly build in alpine OS, the program was not able to find this file

@igor-ivanov
Copy link
Collaborator

Hi @salsal97, could you provide a motivation not to use setitimer

@salsal97
Copy link
Author

@igor-ivanov , I am running this code inside an Intel SGX secure enclave that does not have support for the itimers syscall yet, hence these modifications in the upstream help us

Copy link
Collaborator

@igor-ivanov igor-ivanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Windows system does not have itimerval too.
Please consider changes in os_abstraction.h|cpp to support Alpine as it is done for WIN.
Probably it is possible to use something as

#ifdef ALPINE 

@salsal97
Copy link
Author

salsal97 commented Mar 5, 2021

@igor-ivanov, the changes made to client.cpp support the use of intel sgx enclaves, which might have itimer support as well in the future
the additions in terms of headers to the other two files were what were needed to build the project in alpine. How would one source the header file additions from os_abstract.cpp?

@igor-ivanov
Copy link
Collaborator

bot:retest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants