rust-netlink / netlink-sys

netlink sockets, with optional integration with tokio
Other
15 stars 10 forks source link

Add setsockopt method to the 'Socket' interface #18

Closed qpmr closed 5 months ago

qpmr commented 5 months ago

Hi, I am using a crate that depends on yours (thanks for the implementation, by the way :) ) and I need to set some socket options (for example, SO_RCVBUF). The current Socket interface doesn't provide an access to this method and you use it internally only (why?). Maybe I can add it to the Socket interface?

impl Socket {
    ...
    pub fn bind(&mut self, addr: &SocketAddr) -> Result<()> {
    ...
    }
    ...
    pub fn connect(&self, remote_addr: &SocketAddr) -> Result<()> {
    ...
    }
    ...
    pub fn setsockopt<T>(
        fd: RawFd,
        level: libc::c_int,
        option: libc::c_int,
        payload: T,
    ) -> Result<()> {
        let payload = &payload as *const T as *const libc::c_void;
        let payload_len = mem::size_of::<T>() as libc::socklen_t;

        let res =
            unsafe { libc::setsockopt(fd, level, option, payload, payload_len) };
        if res < 0 {
            return Err(Error::last_os_error());
        }
        Ok(())
    }
}
cathay4t commented 5 months ago

I am currently using stuff like:

    let (mut connection, handle, _) = new_connection()?;
    if unsafe {
        libc::setsockopt(
            connection.socket_mut().as_raw_fd(),
            libc::SOL_NETLINK,
            NETLINK_GET_STRICT_CHK,
            1u32.to_ne_bytes().as_ptr() as *const _,
            4,
        )
    } != 0
    {
        let e = NisporError::bug(format!(
            "Failed to set socket option NETLINK_GET_STRICT_CHK: error {}",
            std::io::Error::last_os_error()
        ));
        return Err(e);
    }

    tokio::spawn(connection);

PR will be welcome for wrapping above into API would be great!.

Meanwhile, nix::sys::ioctl might be better than libc::ioctl.

qpmr commented 5 months ago

Sure, I will create a PR later in a few days

qpmr commented 5 months ago

@cathay4t Hi, I decided do not expose the setsockopt(), but only option that I tested (get/set SO_RCVBUF). And it is also save backward compatibility with existed projects.

qpmr commented 5 months ago

@cathay4t Hi again, maybe you have some plans about the next release? : ) Some other issue waiting for these tiny changes Listener for the exiting tasks And in my project I use this linux-taskstats-rs crate. These changes are optional (useful for some corner cases), but I have added them to be compliant with the documentation taskstats: 'Flow control for taskstats'

cathay4t commented 5 months ago

Exposing setsockopt() as API in this crate might be wrong, I was expecting PR like https://github.com/rust-netlink/netlink-sys/pull/20

In other world, please wrap the setsockopt() in a API function with actual use case. For example, my above lines on NETLINK_GET_STRICT_CHK should be wrapped into something like Socket::enable_get_strict_chk().

cathay4t commented 5 months ago

Are you expecting me to do new release to include PR #20 ?

qpmr commented 5 months ago

Sorry for misleading: at the first place I wanted exposing setsockopt() BUT then I realized, that it is not a good idea. So I created this #20 PR . I am not going to create other PR at this crate ) I just want to create other PR in the linux-taskstats-rs crate which uses your crate

qpmr commented 5 months ago

Are you expecting me to do new release to include PR #20 ?

I would really appreciate it if you could include these changes in the next build release so that I can use them officially

cathay4t commented 5 months ago

The new 0.8.6 release uploaded to crates.io. cargo update could do the magic on refresh your local cache.

qpmr commented 5 months ago

Thank's!