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

Support specififying non-default batch size #9

Merged
merged 1 commit into from
Aug 1, 2024

Conversation

shutton
Copy link

@shutton shutton commented Jul 30, 2024

Provide new interfaces to allow calling send_mmsg and recv_mmsg with a non-default batch size, specified by API clients as a generic constant during compile time. This is presently 1024 on Linux and FreeBSD. The default batch size is increased to 128 on these platforms.

Also update to minimally support FreeBSD. Not all Linux features are supported yet.

Provide new interfaces to allow calling `sendmmsg` and `recvmmsg` with a
non-default batch size, specified by API clients as a generic constant during
compile time. This is presently 1024 on Linux and FreeBSD. The default batch
size is increased to 128 on these platforms.

Also update to minimally support FreeBSD. Not all Linux features are supported
yet.
@shutton shutton force-pushed the variable-batch-size branch from e41ea03 to faa071d Compare July 30, 2024 17:27
@@ -1084,6 +1224,8 @@ fn prepare_msg<B: AsPtr<u8>>(
};
encoder.push(libc::IPPROTO_IP, libc::IP_PKTINFO, pktinfo);
}
#[cfg(target_os = "freebsd")]
Source::Interface(_) => (), // Not yet supported on FreeBSD
Copy link
Owner

Choose a reason for hiding this comment

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

should we just disallow the variant itself on freebsd? if not, maybe some comment on Interface to indicate it can't be used with freebsd

Copy link
Author

@shutton shutton Jul 30, 2024

Choose a reason for hiding this comment

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

There's a way to do it, but I just didn't have time to investigate/verify. I don't have a FreeBSD development system, so I was mainly checking under cross (just to make sure it at least compiles).

See: rust-lang/libc#1583

Copy link
Author

Choose a reason for hiding this comment

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

It might make sense to switch this to unimplemented!() (which is a bit stronger way to disallow it). Would need some corresponding documentation.

@leshow
Copy link
Owner

leshow commented Jul 30, 2024

thanks! quinn mentions the default batch size was "arbitrary", any reason behind the increase to 128 for the default?

@shutton
Copy link
Author

shutton commented Jul 30, 2024

My guess was that they just picked a reasonable default for the situation.

In our use case, we easily exceed even the system cap (1024), so we're willing (and able) to forfeit some stack space to reduce the calls. However, small device applications might prefer a much smaller default to minimize the risk of stack overflows.

@leshow leshow merged commit 3d6d364 into leshow:master Aug 1, 2024
8 checks passed
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.

2 participants