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

Refine select nodelay #211

Merged
merged 62 commits into from
Mar 20, 2024
Merged
Show file tree
Hide file tree
Changes from 61 commits
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
372784e
rewrite FdSet and it's uses
yizhuoliang Feb 23, 2024
53fd0b0
FdSet init from raw fd_set
yizhuoliang Feb 24, 2024
7f8f1a4
attempt to fix borrowing, kernel_select nonblocking
yizhuoliang Feb 24, 2024
3d3c720
attempt continued
yizhuoliang Feb 24, 2024
c28e122
fix is_set
yizhuoliang Feb 24, 2024
66434dd
attempt to fix borrow checking within select
yizhuoliang Feb 24, 2024
af7760a
fix in-select borrow, fix warnings
yizhuoliang Feb 24, 2024
f752848
cleanup
yizhuoliang Feb 24, 2024
dfe4f91
fix a warning
yizhuoliang Feb 24, 2024
e377010
fix serious typo
yizhuoliang Feb 24, 2024
12f2375
fix typo
yizhuoliang Feb 24, 2024
968604e
debugging
yizhuoliang Feb 24, 2024
15af3b6
debugging
yizhuoliang Feb 25, 2024
1b382b5
prints
yizhuoliang Feb 25, 2024
654abcc
prints!!
yizhuoliang Feb 25, 2024
c88e28e
debugging
yizhuoliang Feb 25, 2024
2b57dac
debugging
yizhuoliang Feb 25, 2024
6739514
debugging
yizhuoliang Feb 25, 2024
276be6a
debugging
yizhuoliang Feb 25, 2024
6886660
debug
yizhuoliang Feb 25, 2024
668a92d
debuging
yizhuoliang Feb 25, 2024
4b89ca3
update
yizhuoliang Feb 25, 2024
0706eb3
debug
yizhuoliang Feb 25, 2024
04b8c43
debug
yizhuoliang Feb 25, 2024
768e909
debug
yizhuoliang Feb 25, 2024
3c157e8
debug
yizhuoliang Feb 25, 2024
e618bc1
debug
yizhuoliang Feb 25, 2024
c6e23c3
cleanup and refactor
yizhuoliang Feb 25, 2024
6d13964
fixing the nfds parameter for kernel select
yizhuoliang Feb 25, 2024
f7d8695
rawfd in socketdesc
rennergade Feb 28, 2024
98c19c8
rawfd in socketdesc
rennergade Feb 28, 2024
5d44346
rawfd in socketdesc
rennergade Feb 29, 2024
c9042f6
rawfd in socketdesc
rennergade Feb 29, 2024
f4d4e71
rawfd in socketdesc
rennergade Feb 29, 2024
6a49aaf
rawfd in socketdesc
rennergade Feb 29, 2024
b308497
rawfd in socketdesc
rennergade Feb 29, 2024
71f8cd5
rawfd in socketdesc
rennergade Feb 29, 2024
18541b0
rawfd in socketdesc
rennergade Feb 29, 2024
f012a49
rawfd in socketdesc
rennergade Feb 29, 2024
1068027
rawfd in socketdesc
rennergade Mar 1, 2024
f8fcab7
rawfd in socketdesc
rennergade Mar 1, 2024
36edf84
increase timeout
rennergade Mar 1, 2024
3359aea
change gettimeofday log level
rennergade Mar 3, 2024
ab57640
migrate FdSet, nodelay getsockopt, accept rawfd
yizhuoliang Mar 4, 2024
3c49f1f
small fix
yizhuoliang Mar 5, 2024
fc52610
store rawfd in acpt/lst/cnct, fix INPROGRESS
yizhuoliang Mar 6, 2024
75050b5
fix ownership
yizhuoliang Mar 6, 2024
667b15a
of course domain sockets don't have raw_sys_fd
yizhuoliang Mar 6, 2024
fbc3f15
fix warnings
yizhuoliang Mar 6, 2024
4a1fa40
fix warnings
yizhuoliang Mar 6, 2024
f4ecd78
small coding style fixes
yizhuoliang Mar 7, 2024
5cb8ded
add blank line
yizhuoliang Mar 8, 2024
20d3176
add TCP level options
yizhuoliang Mar 8, 2024
a47f167
getsockopt tcp without kernel
yizhuoliang Mar 8, 2024
77d3004
add FdSet::clear back for tests
yizhuoliang Mar 8, 2024
cca05f9
fix warnings
yizhuoliang Mar 8, 2024
1642070
fix warning
yizhuoliang Mar 8, 2024
bc32d01
revert select sleep
rennergade Mar 9, 2024
720ece9
fix PR suggestions
yizhuoliang Mar 15, 2024
7e3764a
refactor kernel select logic
yizhuoliang Mar 16, 2024
eebbd66
reorganize
yizhuoliang Mar 20, 2024
6496168
update
yizhuoliang Mar 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 98 additions & 3 deletions src/interface/comm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@
// //
// //

use crate::interface;
use std::mem::size_of;
use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
use std::fs::read_to_string;
use std::str::from_utf8;
use std::os::unix::io::{AsRawFd, RawFd};
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we don't actually need to import this. Can't we just use ints?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice catch

use std::mem;
Copy link
Contributor

Choose a reason for hiding this comment

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

also we import mem:size_of above, maybe combine and just import what you need?


extern crate libc;

Expand Down Expand Up @@ -206,7 +209,7 @@ pub struct SockaddrV6 {
#[derive(Debug)]
pub struct Socket {
pub refcnt: i32,
raw_sys_fd: i32
pub raw_sys_fd: i32
}

impl Socket {
Expand Down Expand Up @@ -340,8 +343,8 @@ impl Socket {

pub fn setsockopt(&self, level: i32, optname: i32, optval: i32) -> i32 {
let valbuf = optval;
let sor = unsafe{libc::setsockopt(self.raw_sys_fd, level, optname, (&valbuf as *const i32).cast::<libc::c_void>(), size_of::<i32>() as u32)};
sor
let ret = unsafe{libc::setsockopt(self.raw_sys_fd, level, optname, (&valbuf as *const i32).cast::<libc::c_void>(), size_of::<i32>() as u32)};
ret
}

pub fn shutdown(&self, how: i32) -> i32 {
Expand All @@ -366,3 +369,95 @@ impl Drop for Socket {
pub fn getifaddrs_from_file() -> String {
read_to_string(NET_DEV_FILENAME).expect("No net_devices file present!").to_owned()
}

// Implementations of select related FD_SET structure
pub struct FdSet(libc::fd_set);

impl FdSet {
pub fn new() -> FdSet {
unsafe {
let mut raw_fd_set = std::mem::MaybeUninit::<libc::fd_set>::uninit();
libc::FD_ZERO(raw_fd_set.as_mut_ptr());
FdSet(raw_fd_set.assume_init())
}
}

pub fn new_from_ptr(raw_fdset_ptr: *const libc::fd_set) -> &'static mut FdSet {
unsafe {
&mut *(raw_fdset_ptr as *mut FdSet)
}
}

// copy the src FdSet into self
pub fn copy_from(&mut self, src_fds: &FdSet) {
unsafe {
std::ptr::copy_nonoverlapping(&src_fds.0 as *const libc::fd_set, &mut self.0 as *mut libc::fd_set, 1);
}
}

// turn off the fd bit in fd_set (currently only used by the tests)
#[allow(dead_code)]
pub fn clear(&mut self, fd: RawFd) {
unsafe { libc::FD_CLR(fd, &mut self.0) }
}

// turn on the fd bit in fd_set
pub fn set(&mut self, fd: RawFd) {
unsafe { libc::FD_SET(fd, &mut self.0) }
}

// return true if the bit for fd is set, false otherwise
pub fn is_set(&self, fd: RawFd) -> bool {
unsafe { libc::FD_ISSET(fd, &self.0) }
}

pub fn is_empty(&self) -> bool {
let fd_array: &[u8] = unsafe {
std::slice::from_raw_parts(&self.0 as *const _ as *const u8, mem::size_of::<libc::fd_set>())
};
fd_array.iter().all(|&byte| byte == 0)
}

// for each fd, if kernel_fds turned it on, then self will turn the corresponding tranlated fd on
pub fn set_from_kernelfds_and_translate(&mut self, kernel_fds: &FdSet, nfds: i32, rawfd_lindfd_tuples: &Vec<(i32, i32)>) {
Copy link
Contributor

Choose a reason for hiding this comment

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

theres a lot of nesting here, maybe condense some of this to one liners for readability

Choose a reason for hiding this comment

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

Following Nick's suggestion, the nested loop can be modified as something like this:

use std::collections::HashMap;

pub fn set_from_kernelfds_and_translate(&mut self, kernel_fds: &FdSet, nfds: i32, rawfd_lindfd_tuples: &Vec<(i32, i32)>) {
    // Create a HashMap from the rawfd_lindfd_tuples for O(1) look-up
    let fd_map: HashMap<i32, i32> = rawfd_lindfd_tuples.iter().cloned().collect();
    for fd in 0..nfds {
        if kernel_fds.is_set(fd) {
            if let Some(lindfd) = fd_map.get(&fd) {
                self.set(*lindfd);
            }
        }
    }
}

Copy link
Contributor

Choose a reason for hiding this comment

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

this is probably not a great idea because I believe creating a hashmap is slow, so even though the algorithmic complexity may be better N should never be large enough where it surpasses that allocation cost.

for fd in 0..nfds {
if !kernel_fds.is_set(fd) {
continue;
}
// translate and set
if let Some((_, lindfd)) = rawfd_lindfd_tuples.iter().find(|(rawfd, _)| *rawfd == fd) {
self.set(*lindfd);
}
}
}
}

// for unwrapping in kernel_select
fn to_fdset_ptr(opt: Option<&mut FdSet>) -> *mut libc::fd_set {
match opt {
None => std::ptr::null_mut(),
Some(&mut FdSet(ref mut raw_fd_set)) => raw_fd_set,
}
}

pub fn kernel_select(nfds: libc::c_int, readfds: Option<&mut FdSet>, writefds: Option<&mut FdSet>, errorfds: Option<&mut FdSet>) -> i32 {
// Call libc::select and store the result
let result = unsafe {
// Create a timeval struct with zero timeout

let mut kselect_timeout = libc::timeval {
tv_sec: 0, // 0 seconds
tv_usec: 0, // 0 microseconds
};

libc::select(
nfds,
to_fdset_ptr(readfds),
to_fdset_ptr(writefds),
to_fdset_ptr(errorfds),
&mut kselect_timeout as *mut libc::timeval,
)
};

return result;
}
53 changes: 10 additions & 43 deletions src/interface/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ pub union Arg {
pub dispatch_constsigsett: *const SigsetType,
pub dispatch_structitimerval: *mut ITimerVal,
pub dispatch_conststructitimerval: *const ITimerVal,
pub dispatch_fdset: *mut libc::fd_set,
}


Expand Down Expand Up @@ -271,6 +272,15 @@ pub fn get_mutcbuf_null(union_argument: Arg) -> Result<Option<*mut u8>, i32> {
return Ok(None);
}

pub fn get_fdset(union_argument: Arg) -> Result<Option<&'static mut interface::FdSet>, i32> {
let data: *mut libc::fd_set = unsafe{union_argument.dispatch_fdset};
if !data.is_null() {
let internal_fds: &mut interface::FdSet = interface::FdSet::new_from_ptr(data);
return Ok(Some(internal_fds));
}
return Ok(None);
}

pub fn get_cstr<'a>(union_argument: Arg) -> Result<&'a str, i32> {

//first we check that the pointer is not null
Expand Down Expand Up @@ -412,49 +422,6 @@ pub fn get_sockpair<'a>(union_argument: Arg) -> Result<&'a mut SockPair, i32> {
return Err(syscall_error(Errno::EFAULT, "dispatcher", "input data not valid"));
}

// turn on the fd bit in fd_set
pub fn fd_set_insert(fd_set: *mut u8, fd: i32) {
let byte_offset = fd / 8;
let byte_ptr = fd_set.wrapping_offset(byte_offset as isize);
let bit_offset = fd & 0b111;
unsafe{*byte_ptr |= 1 << bit_offset;}
}

// turn off the fd bit in fd_set
pub fn fd_set_remove(fd_set: *mut u8, fd: i32) {
let byte_offset = fd / 8;
let byte_ptr = fd_set.wrapping_offset(byte_offset as isize);
let bit_offset = fd & 0b111;
unsafe{*byte_ptr &= !(1 << bit_offset);}
}

// return true if the bit for fd is set in fd_set, false otherwise
pub fn fd_set_check_fd(fd_set: *const u8, fd: i32) -> bool {
let byte_offset = fd / 8;
let byte_ptr = fd_set.wrapping_offset(byte_offset as isize);
let bit_offset = fd & 0b111;
return (unsafe{*byte_ptr}) & (1 << bit_offset) != 0;
}

pub fn fd_set_copy(src_set: *const u8, dst_set: *mut u8, nfds: i32) {
for fd in 0..nfds {
if interface::fd_set_check_fd(src_set, fd) {
interface::fd_set_insert(dst_set, fd);
} else {
interface::fd_set_remove(dst_set, fd);
}
}
}

pub fn fd_set_is_empty(fd_set: *const u8, highest_fd: i32) -> bool {
for fd in 0..highest_fd + 1 {
if fd_set_check_fd(fd_set, fd) {
return false;
}
}
return true;
}

pub fn get_sockaddr(union_argument: Arg, addrlen: u32) -> Result<interface::GenSockaddr, i32> {
let pointer = unsafe{union_argument.dispatch_constsockaddrstruct};
if !pointer.is_null() {
Expand Down
2 changes: 2 additions & 0 deletions src/safeposix/cage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ pub struct StreamDesc {
#[derive(Debug, Clone)]
pub struct SocketDesc {
pub flags: i32,
pub domain: i32,
pub rawfd: i32,
pub handle: interface::RustRfc<interface::RustLock<SocketHandle>>,
pub advlock: interface::RustRfc<interface::AdvisoryLock>,
}
Expand Down
2 changes: 1 addition & 1 deletion src/safeposix/dispatcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ pub extern "C" fn dispatcher(cageid: u64, callnum: i32, arg1: Arg, arg2: Arg, ar
if nfds < 0 { //RLIMIT_NOFILE check as well?
return syscall_error(Errno::EINVAL, "select", "The number of fds passed was invalid");
}
check_and_dispatch!(cage.select_syscall, Ok::<i32, i32>(nfds), interface::get_mutcbuf_null(arg2), interface::get_mutcbuf_null(arg3), interface::get_mutcbuf_null(arg4), interface::duration_fromtimeval(arg5))
check_and_dispatch!(cage.select_syscall, Ok::<i32, i32>(nfds), interface::get_fdset(arg2), interface::get_fdset(arg3), interface::get_fdset(arg4), interface::duration_fromtimeval(arg5))
}
POLL_SYSCALL => {
let nfds = get_onearg!(interface::get_usize(arg2));
Expand Down
33 changes: 32 additions & 1 deletion src/safeposix/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ pub struct UnixSocketInfo {
#[derive(Debug)]
pub struct SocketHandle {
pub innersocket: Option<interface::Socket>,
pub options: i32,
pub socket_options: i32,
pub tcp_options: i32,
pub state: ConnState,
pub protocol: i32,
pub domain: i32,
Expand Down Expand Up @@ -387,3 +388,33 @@ impl NetMetadata {
domainsock_paths
}
}

pub struct SelectInetInfo {
pub rawfd_lindfd_tuples: Vec<(i32, i32)>,
pub kernel_fds: interface::FdSet,
pub highest_raw_fd: i32,
}

impl SelectInetInfo {
pub fn new() -> Self {
SelectInetInfo {
rawfd_lindfd_tuples: Vec::new(),
kernel_fds: interface::FdSet::new(),
highest_raw_fd: 0,
}
}
}

pub fn update_readfds_from_kernel_select(readfds: &mut interface::FdSet, inet_info: &mut SelectInetInfo, retval: &mut i32) -> i32 {
let kernel_ret;
// note that this select call always have timeout = 0, so it doesn't block

kernel_ret = interface::kernel_select(inet_info.highest_raw_fd + 1, Some(&mut inet_info.kernel_fds), None, None);
if kernel_ret > 0 {
// increment retval of our select
*retval += kernel_ret;
// translate the kernel checked fds to lindfds, and add to our new_writefds
readfds.set_from_kernelfds_and_translate(&mut inet_info.kernel_fds, inet_info.highest_raw_fd + 1, &inet_info.rawfd_lindfd_tuples);
}
return kernel_ret;
}
Loading
Loading