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

Add shim for sendmmsg when sendmmsg does not exist #1896

Merged
merged 5 commits into from
Aug 11, 2021

Conversation

thhous-msft
Copy link
Contributor

Some users have asked to build with glibc versions that do not have sendmmsg. This change checks for the existance of sendmmsg, and if its missing implements a shim version that has identical functionality.

Closes #1884

Some users have asked to build with glibc versions that do not have sendmmsg. This change checks for the existance of sendmmsg, and if its missing implements a shim version that has identical functionality
@thhous-msft thhous-msft requested a review from a team as a code owner August 10, 2021 18:45
while (SuccessCount < MessageLen) {
int Result = sendmsg(fd, &Messages[SuccessCount].msg_hdr, Flags);
if (Result < 0) {
return SuccessCount == 0 ? Result : (ssize_t)SuccessCount;
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't returning Result incorrectly set SuccessfullySentMessages in line 2592?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. The contract of sendmmsg is if the first message errors that return value is returned. Otherwise it returns the number of successful sends.

Copy link
Member

@nibanks nibanks left a comment

Choose a reason for hiding this comment

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

How are you testing this?

@thhous-msft
Copy link
Contributor Author

thhous-msft commented Aug 11, 2021

I manually ran the test suite and enabled this flag. I also at the same time disabled the USO flag as well, which checks this even more.

@thhous-msft thhous-msft merged commit d9bf2d0 into main Aug 11, 2021
@thhous-msft thhous-msft deleted the thadhouse/sendmmsgshim branch August 11, 2021 17:37
@hros
Copy link
Contributor

hros commented Aug 11, 2021

Are there benchmarking data regarding the performance degradation when sendmmsg is not available?

@thhous-msft
Copy link
Contributor Author

We don't have current benchmarking numbers, because none of our systems support any off the offloading. However, we do have some older performance data. The red jump was adding sendmmsg. The black jump here is adding GSO, which with an older libc you also won't have. So I would expect performance to be similar to where things were before the red jump.

image

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

Successfully merging this pull request may close these issues.

Support epoll Without Using sendmmsg
4 participants