update and fix some outdated comments

forwarder/src/socket/icmp.rs

@@ -29,14 +29,15 @@ static OPEN_PORTS: [RwLock<BTreeSet<u16>>; 2] =
 /// `IcmpSocket` that is very similiar to `UdpSocket`
 #[derive(Debug)]
 pub struct IcmpSocket {
+    /// icmp socket
     socket: socket2::Socket,
-    /// is underline icmp socket blocking
     is_blocking: bool,
-    /// udp socket that is kept alive for avoiding duplicate port
+    /// udp socket that is kept alive for avoiding duplicate port and
+    /// receives packets from icmp receiver if the socket is non blocking
     udp_socket: std::net::UdpSocket,
-    /// address of udp socket
+    /// address of udp socket same as `udp_socket.local_addr()`
     udp_socket_addr: SocketAddr,
-    /// saves the socket that is connected to
+    /// contains the address that the socket is connected to
     connected_addr: Option<SocketAddr>,
 }
 
@@ -99,7 +100,7 @@ impl SocketTrait for IcmpSocket {
         // icmp receiver sends packets that it receives to udp socket of `IcmpSocket`
         let (size, from_addr) = self.udp_socket.recv_from(buffer)?;
         // make sure that the receiver sent the packet
-        // receiver is local so the packet ip is from loopback
+        // receiver is local so the packet ip must be from loopback
         if from_addr.ip().is_loopback() {
             Ok(size)
         } else {
@@ -111,7 +112,7 @@ impl SocketTrait for IcmpSocket {
         let dst_addr = self.connected_addr.unwrap();
         let packet = craft_icmp_packet(buffer, &self.local_addr()?, &dst_addr);
         let dst_addr: SocketAddr = if dst_addr.is_ipv6() {
-            // in linux `send_to` on icmpv6 socket requires dst address port to be zero
+            // in linux `send_to` on icmpv6 socket requires destination port to be zero
             let mut addr_without_port = dst_addr;
             addr_without_port.set_port(0);
             addr_without_port
@@ -131,7 +132,7 @@ impl SocketTrait for IcmpSocket {
     fn send_to(&self, buffer: &[u8], to: &SocketAddr) -> io::Result<usize> {
         let packet = craft_icmp_packet(buffer, &self.local_addr()?, to);
         let mut to_addr = *to;
-        // in linux `send_to` on icmpv6 socket requires dst address port to be zero
+        // in linux `send_to` on icmpv6 socket requires destination port to be zero
         to_addr.set_port(0);
         self.socket.send_to(&packet, &to_addr.into())
     }
@@ -199,7 +200,6 @@ fn craft_icmp_packet(payload: &[u8], source_addr: &SocketAddr, dst_addr: &Socket
         seq: source_addr.port(),
     };
 
-    // TODO: rewrite this part to use fewer allocations
     let icmp_header = if source_addr.is_ipv4() {
         let icmp_type = Icmpv4Type::EchoRequest(echo_header);
         Icmpv4Header::with_checksum(icmp_type, payload)

forwarder/src/socket/icmp/receiver.rs

@@ -4,9 +4,9 @@ use etherparse::Ipv4HeaderSlice;
 use std::{mem::MaybeUninit, net::SocketAddr};
 
 // each nonblocking `IcmpSocket` does not actually listen for new packets because
-// icmp protocol is on layer 2 and doesn't have any concept of ports
-// so each packet will wake up all `IcmpSocket`s, to fix that and remove
-// overheads of parsing each packet multiple times we listen to packets
+// icmp protocol is on layer 3 and doesn't have any concept of ports
+// so if each `IcmpSocket` called `recv` each packet that comes through icmp will wake up all `IcmpSocket`s
+// to fix that and remove overheads of parsing each packet multiple times we listen to packets
 // only on one socket on another thread and after parsing port and packet
 // we send it back to `IcmpSocket` via udp protocol
 pub fn run_icmp_receiver(addr: SocketAddr) -> anyhow::Result<()> {
@@ -44,14 +44,13 @@ pub struct IcmpPacket<'a> {
 }
 
 pub fn parse_icmp_packet(packet: &[u8], is_ipv6: bool) -> Option<IcmpPacket<'_>> {
-    // according to 'icmp6' man page on freebsd (seems like linux does this the same way):
+    // according to 'icmp6' man page on freebsd (seems like linux does this too):
     // 'Incoming packets on the socket are received with the IPv6 header and any extension headers removed'
     //
     // but on 'icmp' man page that is for icmpv4, it says:
     // 'Incoming packets are received with the IP header and options intact.'
     //
     // so we need to parse header in icmpv4 but not in icmpv6
-    // why tf??? i don't know, and don't ask me how i found this out
     let payload_start_index = if is_ipv6 {
         0
     } else {
@@ -61,7 +60,7 @@ pub fn parse_icmp_packet(packet: &[u8], is_ipv6: bool) -> Option<IcmpPacket<'_>>
     };
 
     let icmp = IcmpSlice::from_slice(is_ipv6, &packet[payload_start_index..])?;
-    // we only work with icmp echo request so if any other type of icmp
+    // we only work with icmp echo requests so if any other type of icmp
     // packet we receive we just ignore it
     let correct_icmp_type = if is_ipv6 {
         etherparse::icmpv6::TYPE_ECHO_REQUEST
@@ -74,9 +73,8 @@ pub fn parse_icmp_packet(packet: &[u8], is_ipv6: bool) -> Option<IcmpPacket<'_>>
 
     let bytes5to8 = icmp.bytes5to8();
     // icmp is on layer 3 so it has no idea about ports
-    // we use identification part of icmp packet that usually
-    // is the pid of ping program as destination port to identify
-    // packets that are really meant for us
+    // we use identification part of icmp packet as destination port
+    // to identify packets that are really meant for us
     let dst_port = u16::from_be_bytes([bytes5to8[0], bytes5to8[1]]);
 
     // we also use sequence part of icmp packet as source port