3
\$\begingroup\$

I have written code related to network communication interface which basically has functionalities of create socket and send & receive data. Currently this supports only IPV4 but facing lot of difficulties when making it to work with both IPV4 and IPV6.

use std::net::{UdpSocket, SocketAddrV4, Ipv4Addr};
use std::net::SocketAddr::V4;
use std::str::FromStr;

#[derive(Default)]
pub struct NetworkInterface {
    socket: Option<UdpSocket>,

    recv_sock_addr: Option<SocketAddrV4>,
    send_sock_addr: Option<SocketAddrV4>,
}

impl NetworkInterface {
    pub fn new() -> NetworkInterface {
        NetworkInterface {
            socket: None,
            recv_sock_addr: None,
            send_sock_addr: None,
        }
    }

    pub fn bind_socket(&mut self, ip_addr: &str, port: u16) {
        let pos = ip_addr.find('.').unwrap();
        let first_octet: u8 = ip_addr[..pos].parse().unwrap();

        if first_octet > 223 && first_octet < 240 {
            self.recv_sock_addr = Some(SocketAddrV4::new(Ipv4Addr::UNSPECIFIED, port));
        } else {
            self.recv_sock_addr = Some(SocketAddrV4::new(Ipv4Addr::from_str(ip_addr).unwrap(), port));
        }

        self.socket = Some(UdpSocket::bind(self.recv_sock_addr.as_ref().unwrap()).unwrap());
        if first_octet > 223 && first_octet < 240 {
            let _ = self.socket.as_ref().unwrap().join_multicast_v4(&Ipv4Addr::from_str(ip_addr).unwrap(), &Ipv4Addr::UNSPECIFIED);
        }
    }

    pub fn set_send_sock_addr(&mut self, ip_addr: &str, port: u16) {
        self.send_sock_addr = Some(SocketAddrV4::new(Ipv4Addr::from_str(ip_addr).unwrap(), port));

        let pos = ip_addr.find('.').unwrap();
        let first_octet: u8 = ip_addr[..pos].parse().unwrap();
        if first_octet > 223 && first_octet < 240 {
            self.socket.as_ref().unwrap().set_multicast_loop_v4(false).expect("set_multicast_loop_v4 call failed");
        }
    }

    pub fn recv_data(&self, buff: &mut[u8]) -> usize {
        let(recvd_bytes_len, _) = self.socket.as_ref().unwrap().recv_from(buff).expect("error while receiving data");

        recvd_bytes_len
    }

    pub fn send_data(&self, buff: &[u8]) {
        self.socket.as_ref().unwrap().send_to(buff, self.send_sock_addr.as_ref().unwrap()).expect("error while sending data");
    }

    pub fn get_host_bound_port(&self) -> u16 {
        self.socket.as_ref().unwrap().local_addr().unwrap().port()
    }

    pub fn update_send_sock_addr(&mut self) {
        let mut buff: [u8; 2048] = [0; 2048];
        let (_, send_sock_addr) = self.socket.as_ref().unwrap().recv_from(&mut buff).expect("error while receiving data during sender IP address updating");

        match send_sock_addr {
            V4(ip_addr) => self.send_sock_addr = Some(ip_addr),
            _ => {}
        }
    }
}
\$\endgroup\$
3
  • 1
    \$\begingroup\$ You .unwrap() a lot. Consider handling the errors instead of risking panics. \$\endgroup\$ Commented Aug 31, 2023 at 16:43
  • \$\begingroup\$ Also what was your reasoning that let you come up with the magic buffer size of 2048? \$\endgroup\$ Commented Aug 31, 2023 at 16:47
  • \$\begingroup\$ to be honest no logical reasoning. Do you think I need to keep it 2^16? \$\endgroup\$
    – Harry
    Commented Sep 1, 2023 at 16:59

1 Answer 1

3
\$\begingroup\$

Adapting your code to supporting both IpV4 and IpV6 should be straightforward. Just use the IpAddr and SocketAddr types instead of the IpV4Addr and SocketAddrV4 types.

pub fn new() -> NetworkInterface {
pub fn bind_socket(&mut self, ip_addr: &str, port: u16) {

Is there a reason to separate new from bind_socket? It seems that the only practical use would be to call new() and then immediately bind_socket. But in that case, it would be better to simply have one function which returns a bound NetworkInterface. This would simplify your implementation as the UdpSocket would not have to be in an Option

    let pos = ip_addr.find('.').unwrap();
    let first_octet: u8 = ip_addr[..pos].parse().unwrap();

You're parsing the first octet out of the ip_address. But you're going to need to parse the ip address anyways, so use that.

let address = IpV4Addr::from_str(ip_addr);
let first_octet = address.octets()[0];

That way you only parse it once and it's just easier to follow.

    if first_octet > 223 && first_octet < 240 {

If you parse it as suggested, you can use the is_multicast method instead.

        self.recv_sock_addr = Some(SocketAddrV4::new(Ipv4Addr::UNSPECIFIED, port));
    } else {
        self.recv_sock_addr = Some(SocketAddrV4::new(Ipv4Addr::from_str(ip_addr).unwrap(), port));
    }


    self.socket = Some(UdpSocket::bind(self.recv_sock_addr.as_ref().unwrap()).unwrap());

You don't really need to store the recv_sock_addr. You don't use it again after this function, and its available via self.socket.local_addr() anyways.

    if first_octet > 223 && first_octet < 240 {
        let _ = self.socket.as_ref().unwrap().join_multicast_v4(&Ipv4Addr::from_str(ip_addr).unwrap(), &Ipv4Addr::UNSPECIFIED);
    }
}

This code deliberately ignores the result returned from join_multicast. DO NOT DO THIS. DO NOT SILENTLY IGNORE ERRORS. Either return the error, or unwrap it, or at the very least leave it as warning produced by compiler.

pub fn set_send_sock_addr(&mut self, ip_addr: &str, port: u16) {

UdpSocket has a connect method which seems like it might do what you want already.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.