smoltcp-rs / smoltcp Goto Github PK

We don't have a clock with the 4 us resolution as RFC 793 wants, but it's likely fine to use the 1 ms resolution clock too.

Hello,

I think change #57 requires the server example to be updated or something else is not correct?

RUST_BACKTRACE=1 cargo +stable run --example server tap0 
    Finished dev [unoptimized + debuginfo] target(s) in 0.0 secs
     Running `target/debug/examples/server tap0`
[     0.001s] (socket::set): [0]: adding
[     0.001s] (socket::set): [1]: adding
[     0.001s] (socket::set): [2]: adding
[     0.001s] (socket::set): [3]: adding
[     0.001s] (socket::set): [4]: adding
[     0.001s] (socket::tcp): #1:*:6969: state=CLOSED=>LISTEN
[     0.001s] (socket::tcp): #2:*:6970: state=CLOSED=>LISTEN
[     0.001s] (socket::tcp): #3:*:6971: state=CLOSED=>LISTEN
[     0.001s] (socket::tcp): #4:*:6972: state=CLOSED=>LISTEN
[     0.001s] (iface::ethernet): cannot dispatch response packet: buffer space exhausted
thread 'main' panicked at 'poll error: Exhausted', /checkout/src/libcore/result.rs:906:4
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
             at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at /checkout/src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at /checkout/src/libstd/sys_common/backtrace.rs:60
             at /checkout/src/libstd/panicking.rs:381
   3: std::panicking::default_hook
             at /checkout/src/libstd/panicking.rs:397
   4: std::panicking::rust_panic_with_hook
             at /checkout/src/libstd/panicking.rs:611
   5: std::panicking::begin_panic
             at /checkout/src/libstd/panicking.rs:572
   6: std::panicking::begin_panic_fmt
             at /checkout/src/libstd/panicking.rs:522
   7: rust_begin_unwind
             at /checkout/src/libstd/panicking.rs:498
   8: core::panicking::panic_fmt
             at /checkout/src/libcore/panicking.rs:71
   9: core::result::unwrap_failed
             at /checkout/src/libcore/macros.rs:41
  10: <core::result::Result<T, E>>::expect
             at /checkout/src/libcore/result.rs:799
  11: server::main
             at examples/server.rs:188
  12: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:99
  13: std::rt::lang_start
             at /checkout/src/libstd/panicking.rs:459
             at /checkout/src/libstd/panic.rs:361
             at /checkout/src/libstd/rt.rs:61
  14: main
  15: __libc_start_main
  16: _start

From RFC 793:

If the connection is in a synchronized state (ESTABLISHED,
FIN-WAIT-1, FIN-WAIT-2, CLOSE-WAIT, CLOSING, LAST-ACK, TIME-WAIT),
any unacceptable segment (out of window sequence number or
unacceptible acknowledgment number) must elicit only an empty
acknowledgment segment containing the current send-sequence number
and an acknowledgment indicating the next sequence number expected
to be received, and the connection remains in the same state.

Hi,

I'm trying to wrap smoltcp::iface::EthernetInterface like so:

extern crate smoltcp;
use std::fs::File;
use std::io::{self, Read, Write};

struct Device {
    network: File,
}

struct TxBuffer {
    buffer: Vec<u8>,
    network: File,
}

impl AsRef<[u8]> for TxBuffer {
    fn as_ref(&self) -> &[u8] { self.buffer.as_ref() }
}

impl AsMut<[u8]> for TxBuffer {
    fn as_mut(&mut self) -> &mut [u8] { self.buffer.as_mut() }
}

impl Drop for TxBuffer {
    fn drop(&mut self) { self.network.write(&mut self.buffer[..]).unwrap(); }
}

impl smoltcp::phy::Device for Device {
    type RxBuffer = Vec<u8>;
    type TxBuffer = TxBuffer;

    fn mtu(&self) -> usize { 1536 }

    fn receive(&mut self) -> Result<Self::RxBuffer, smoltcp::Error> {
        let mut buffer = vec![0; self.mtu()];
        let size = self.network.read(&mut buffer[..])?;
        buffer.resize(size, 0);
        Ok(buffer)
    }

    fn transmit(&mut self, length: usize) -> Result<Self::TxBuffer, smoltcp::Error> {
        Ok(TxBuffer {
            network:  self.network.try_clone()?,
            buffer: vec![0; length]
        })
    }
}

pub struct EthernetDevice<'a, 'b, 'c>(smoltcp::iface::EthernetInterface<'a, 'b, 'c, Device>);

impl<'a, 'b, 'c> EthernetDevice <'a, 'b, 'c> {
    pub fn new(network: File) -> Self {
        let device = Box::new(Device { network: network });
        let arp_cache = Box::new(smoltcp::iface::SliceArpCache::new(vec![Default::default(); 8])) as Box<smoltcp::iface::ArpCache>;
        let hardware_addr = smoltcp::wire::EthernetAddress([0x0, 0x0, 0x0, 0x0, 0x0, 0x0]);
        let ip_addresses = [smoltcp::wire::IpAddress::v4(192, 168, 0, 2)];
        EthernetDevice(smoltcp::iface::EthernetInterface::new(device, arp_cache, hardware_addr, ip_addresses))
    }
    // more stuff
}

This does not work:

error[E0495]: cannot infer an appropriate lifetime for lifetime parameter `'b` due to conflicting requirements
  --> new.rs:56:9
   |
56 |         EthernetDevice(smoltcp::iface::EthernetInterface::new(device, arp_cache, hardware_addr, ip_addresses))
   |         ^^^^^^^^^^^^^^
   |
note: first, the lifetime cannot outlive the lifetime 'b as defined on the body at 51:38...
  --> new.rs:51:39
   |
51 |       pub fn new(network: File) -> Self {
   |  _______________________________________^ starting here...
52 | |         let device = Box::new(Device { network: network });
53 | |         let arp_cache = Box::new(smoltcp::iface::SliceArpCache::new(vec![Default::default(); 8])) as Box<smoltcp::iface::ArpCache>;
54 | |         let hardware_addr = smoltcp::wire::EthernetAddress([0x0, 0x0, 0x0, 0x0, 0x0, 0x0]);
55 | |         let ip_addresses = [smoltcp::wire::IpAddress::v4(192, 168, 0, 2)];
56 | |         EthernetDevice(smoltcp::iface::EthernetInterface::new(device, arp_cache, hardware_addr, ip_addresses))
57 | |     }
   | |_____^ ...ending here
note: ...so that expression is assignable (expected EthernetDevice<'a, 'b, 'c>, found EthernetDevice<'_, '_, '_>)
  --> new.rs:56:9
   |
56 |         EthernetDevice(smoltcp::iface::EthernetInterface::new(device, arp_cache, hardware_addr, ip_addresses))
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   = note: but, the lifetime must be valid for the static lifetime...
note: ...so that the type `smoltcp::iface::Cache` will meet its required lifetime bounds
  --> new.rs:56:24
   |
56 |         EthernetDevice(smoltcp::iface::EthernetInterface::new(device, arp_cache, hardware_addr, ip_addresses))
   |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I know this is not a smoltcp issue, but I have really don't know how to make the compiler happy here, so I was hoping you would be able to suggest something? Or am I trying to do something fundamentally wrong?

When smoltcp receives an ICMPv4 Destination Unreachable Port Unreachable, it tries to parse the payload as an IPv4 packet, but this often fails with Truncated because the original packet was much longer than whatever initial portion of it was included with the ICMP response. This error (from https://github.com/m-labs/smoltcp/blob/master/src/wire/icmpv4.rs#L384) then gets bubbled up to the top poll() call where it looks like a network error.

As far as I can tell it shouldn't actually be an error though, since the RFC says you just include the first 64 bytes (in practice apparently Linux does a few hundred?) of the original packet, but you don't expect to see all of it. smoltcp parsing an IPv4 packet errors out if the packet buffer isn't long enough to contain the length the IPv4 header promises, so the whole thing stops. Not sure what the neatest solution is, perhaps having a way to parse the IPv4 header without caring that the payload likely won't be long enough?

I've been chasing down intermittent panics "undersized payload: Truncated" while sending UDP packets and I think I've gotten to the bottom of it.

The gist is that in socket/udp.rs:264 when we try to make a new UdpPacket from payload, the payload is a TX buffer obtained from Device, with the Ethernet and IPv4 headers set up, but without any UDP header set yet. UdpPacket::new then makes a UdpPacket from this buffer and checks the buffer length is at least packet.len(), a method which reads the UDP header length field. However, this hasn't been set yet!

So, at startup this is often 0 and works fine. During most of my operations, this is set to the previous packet length (and they're all the same) and works fine. Once I have to respond to an ARP request, this is most recently set to 0xC0A8, the first two octets of my desktop's IP address. Since the buffer being transmitted is not that long, we get a panic.

Slightly longer explanation of where this comes from:

1. User calls Interface::poll()
2. poll calls Interface::emit() to send new packets
3. Interface::emit() calls Socket::dispatch for each socket
4. In socket/udp.rs:238 Socket::dispatch() we dequeue a PBuf and put it into a UdpRepr called repr
5. Socket::dispatch() calls the emit closure from Interface::emit() with payload set to UdpRepr as an IpPayload
6. Back in interface/ethernet.rs:383 Interface::emit(), we obtain a TX buffer from Device and turn it into a Frame
7. The frame has its ethernet and IPv4 headers set and an Ipv4Packet made
8. payload: IpPayload (our UdpRepr from before with the PBuf to send) has its emit() called with the Ipv4Packet payload buffer (inside the TX buffer from Device), called payload
9. Now socket/udp.rs:264 impl IpPayload for UdpRepr emit() makes a new UdpPacket with buffer set to payload, which is the Ipv4Packet from 8 and has not had any UDP fields initialised
10. Now wire/udp.rs:37 new() makes a Packet (UdpPacket) from buffer
11. And finally calls packet.len(), which reads the LENGTH field from buffer

I think UdpPacket::new() shouldn't check packet.len() in this case, but I'm not sure how else it can find out what length UDP packet is meant to go into it. The IpPayload emit() in socket/udp.rs, on success, then calls UdpPacket::emit which will do a copy_from_slice to copy the PBuf into the TX buffer.

I also haven't checked to see if the same problem exists for TCP but I'm yet to hit any panics from it...

The recommended 1.18 is too old.

When sending an UDP broadcast packet via an UdpSocket,
I noticed that smoltcp attempted to check the ArpCache for 255.255.255.255
and therefore send an ARP request for that IP, which I believe to be unusual.

I think insteadof calling ArpCache::lookup a general lookup-function
should be called, which handles this (255.255.255.255 to MAC [0xff; 6] ) aswell
as other possible exceptions in the future.

Does that sound like the right approach?
I'd be fine with implementing it, but wanted some feedback first.

Scenario:

• RUST_BACKTRACE=1 cargo run --example server -- tap0
• nc 192.168.69.1 6972 > nc_out

Result:

[     3.605s] <- EthernetII src=2a-d3-30-70-6c-b6 dst=02-00-00-00-00-01 type=IPv4
                \ IPv4 src=192.168.69.100 dst=192.168.69.1 proto=TCP
                 \ TCP src=56036 dst=6972 seq=2966042689 ack=1329360159 win=65535 len=0
[     3.605s] (socket::tcp): [4]192.168.69.1:6972:192.168.69.100:56036: tx buffer: dequeueing 65535 octets (now 0)
thread 'main' panicked at 'assertion failed: acked.len() == ack_len', src/socket/tcp.rs:999:12
stack backtrace:
[...]
   5: std::panicking::begin_panic_new
             at /checkout/src/libstd/panicking.rs:553
   6: smoltcp::socket::tcp::TcpSocket::process
             at src/socket/tcp.rs:999
   7: <smoltcp::iface::ethernet::Interface<'a, 'b, 'c, DeviceT>>::process_tcp
             at ./src/iface/ethernet.rs:417
   8: <smoltcp::iface::ethernet::Interface<'a, 'b, 'c, DeviceT>>::process_ipv4
             at ./src/iface/ethernet.rs:315
   9: <smoltcp::iface::ethernet::Interface<'a, 'b, 'c, DeviceT>>::process_ethernet
             at ./src/iface/ethernet.rs:220
  10: <smoltcp::iface::ethernet::Interface<'a, 'b, 'c, DeviceT>>::socket_ingress
             at ./src/iface/ethernet.rs:144
  11: <smoltcp::iface::ethernet::Interface<'a, 'b, 'c, DeviceT>>::poll
             at ./src/iface/ethernet.rs:126
  12: server::main
             at examples/server.rs:184
[...]

ls -lh nc_out
-rw-r--r-- 1  9.7M Sep 12 21:52 nc_out

I noticed that you re-implement crc. Would you be interested in using a library instead (or contributing to it if it doesn't work for your needs)?

I just opened a PR for no_std support in mrhooray/crc-rs#13

Right now socket::set::Item (used in SocketSet::new) is a public type inside the set module, but it is not publicly used in socket, so it's impossible to refer to this type.

I think this is basically the issue discussed here.

It means you can do this:

let mut sockets_storage = [None];
let mut sockets = SocketSet::new(&mut sockets_storage[..]);
sockets_storage[0] = Some(1);

and the compiler infers the type of sockets_storage OK, and gives this error:

expected type `smoltcp::socket::set::Item<'_, '_>`
found type `{integer}`

but if you try and use that type:

let mut sockets_storage: [Option<smoltcp::socket::set::Item>; 1] = [None; 1];

you get

error: module `set` is private

This makes it really hard to statically allocate all the storage for smoltcp, as far as I can tell. Is there a nice way to do that? I'm running into a bunch of various pain points trying to allocate the storage statically, but I can't have it live on the stack (since it needs to live beyond the function call setting it up) and I don't have a heap available. There's a lot of static mut X: Option<SliceArpCache> = None; and then setting it to Some(SliceArpCache::new(...)) during setup.

Hi,

The wire module provides abstractions for packet decoding, which could also be useful outside of smoltcp. For example, I am considering implementing the OpenFlow protocol and I'd like to re-use what you already did for packet parsing and add support OpenFlow headers.

In case I'm unclear, I'd like to see something similar to Golang's gopacket/layers library.

Git revision a1910ba. Commenting out the UDP part of the server.rs example makes the rest of the code work.

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Illegal', /checkout/src/libcore/result.rs:860:4
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
             at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at /checkout/src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at /checkout/src/libstd/sys_common/backtrace.rs:60
             at /checkout/src/libstd/panicking.rs:380
   3: std::panicking::default_hook
             at /checkout/src/libstd/panicking.rs:396
   4: std::panicking::rust_panic_with_hook
             at /checkout/src/libstd/panicking.rs:610
   5: std::panicking::begin_panic
             at /checkout/src/libstd/panicking.rs:571
   6: std::panicking::begin_panic_fmt
             at /checkout/src/libstd/panicking.rs:521
   7: rust_begin_unwind
             at /checkout/src/libstd/panicking.rs:497
   8: core::panicking::panic_fmt
             at /checkout/src/libcore/panicking.rs:71
   9: core::result::unwrap_failed
             at /checkout/src/libcore/macros.rs:31
  10: <core::result::Result<T, E>>::unwrap
             at /checkout/src/libcore/result.rs:738
  11: server::main
             at examples/server.rs:67
  12: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:98
  13: std::rt::lang_start
             at /checkout/src/libstd/panicking.rs:458
             at /checkout/src/libstd/panic.rs:361
             at /checkout/src/libstd/rt.rs:61
  14: main
  15: __libc_start_main
  16: _start

Currently, smoltcp does not check the destination MAC address.

Here is an example (it reuses examples/utils.rs), where we create a device with hardware address 00:00:00:00:00:01 and ip 10.0.0.1:

#[macro_use]
extern crate log;
extern crate env_logger;
extern crate getopts;
extern crate smoltcp;

mod utils;

use std::str;
use smoltcp::phy::RawSocket;
use std::time::Instant;
use std::env;
use smoltcp::Error;
use smoltcp::wire::{EthernetAddress, IpAddress};
use smoltcp::iface::{ArpCache, SliceArpCache, EthernetInterface};
use smoltcp::socket::{SocketSet};

fn main() {
    utils::setup_logging();

    let opts = getopts::Options::new();
    let matches = opts.parse(env::args().skip(1)).unwrap();
    let device = RawSocket::new(&matches.free[0]).unwrap();
    let startup_time = Instant::now();


    let arp_cache = SliceArpCache::new(vec![Default::default(); 8]);
    let mut iface = EthernetInterface::new(
        Box::new(device), Box::new(arp_cache) as Box<ArpCache>,
        EthernetAddress([0x00, 0x00, 0x00, 0x00, 0x00, 0x01]),
        [IpAddress::v4(10, 0, 0, 1)]);

    let mut sockets = SocketSet::new(vec![]);

    loop {
        let timestamp = Instant::now().duration_since(startup_time);
        let timestamp_ms = (timestamp.as_secs() * 1000) + (timestamp.subsec_nanos() / 1000000) as u64;
        match iface.poll(&mut sockets, timestamp_ms) {
            Ok(()) | Err(Error::Exhausted) => (),
            Err(e) => debug!("poll error: {}", e)
        }
    }
}

Now we can create a veth pair:

ip link add veth0 type veth peer name veth1
ip link set dev veth0 up
ip link set dev veth1 up

We run the client:

target/debug/examples/arp veth0

Let's send some ping with a wrong destination mac:

# we add a dummy ip in the 10.0.0.0/24 subnet on veth1 to force linux to send ping through it
ip addr add 10.0.0.10/24 dev veth1
# we add a static arp entry to force echo request to be sent with a wrong mac address
ip neigh change 10.0.0.1 dev veth1 lladdr aa:bb:cc:dd:ee:ff nud permanent

Now we can ping:

~ ❯❯❯ ping 10.0.0.1
PING 10.0.0.1 (10.0.0.1) 56(84) bytes of data.
64 bytes from 10.0.0.1: icmp_seq=1 ttl=64 time=0.282 ms
64 bytes from 10.0.0.1: icmp_seq=2 ttl=64 time=0.349 ms
^C
--- 10.0.0.1 ping statistics ---
2 packets transmitted, 2 received, 0% packet loss, time 1023ms
rtt min/avg/max/mdev = 0.282/0.315/0.349/0.037 ms

Here is the packet capture, just change the extension to .pcap to open it.
out.txt

I'm not sure this is really an issue, and I could not find an RFC that explicitely states that a host should reject ethernet frames with a wrong destination address. Still, it seems to me that the correct behavior would be to ignore such packets.

Take 15ce667, apply the patch:

diff --git a/examples/loopback.rs b/examples/loopback.rs
index fa3806c..3305729 100644
--- a/examples/loopback.rs
+++ b/examples/loopback.rs
@@ -72 +72 @@ fn main() {
-    while !done && timestamp_ms < 500 {
+    while timestamp_ms < 5000 {
@@ -100,0 +101,3 @@ fn main() {
+            }
+
+            if socket.is_active() && done {

• Make IPv4 optional by adding a feature gating it. #98
• Add Ipv6Address/Ipv6Cidr. #72
• Add Ipv6Packet/Ipv6Repr. #89

Metabug from #72 (comment).

Related to #3

Code Lines: https://github.com/m-labs/smoltcp/blob/master/src/wire/ethernet.rs#L88-L91

The wiki page: https://en.wikipedia.org/wiki/Ethernet_frame#Structure

as you can see, wikipedia said ethernet frame after payload filed, have one Frame check sequence (32‑bit CRC) field , but i have not see in we ethernet frame struct .

I've recently started to look into adding the ability to set the TTL value for a socket.

This should be a trivial change. The following is my plan of attack.

• Add a Option<u8> to Ipv4Repr and the Unspecified Repr
  • Corresponds to TTL for ipv4 and the hop limit for ipv6 (when ipv6 is implemented)
  • On lower ensure that the ttl is not lost
  • Add getters and setters.
• Add a Option<u8> member to TcpSocket
  • Add getters and setters.
  • Initialize the IpRepr with the optional value.
• Use the configured value On emiting the ipv4 packet
  • set the TTL to the recommended value of 64 if the optional value is None
  • use the configured value if Some

Due to the simplicity of this change there are minimal open questions.

• Is hop limit (ipv6) or ttl (ipv4) a better/more readable name? At the moment I'm thinking I'll go with TTL.

Comments and critiques are welcomed!

Related to: #50

The type signature of new is getting really unwieldy, and the breaking changes for every new features are undesirable.

Right now that's not the case.

Problem

Currently, sloltcp uses iteration to dispatch ingress packets to sockets and to poll sockets for egress packets. While acceptable on the platforms without libstd and memory allocation, this approach is very inefficient on systems where growable containers are available.

Discussion

1. The problem is two-fold: ingress packets should be dispatched by lookup tables based on their destination, while only "dirty" sockets should be polled for egress packets.
2. After #18 is merged, there will be two kinds of sockets: l3 (raw) sockets should be dispatched to based on packet's IP version and IP protocol number, and l4 sockets (tcp and udp) should be dispatched to based on the dst port.
3. Sockets can be bound to a specific IP address. As a result, a solution should support several sockets of the same type listening on the same port but bound to different IP addresses.
4. A solution should keep the existing behavior when compiled without the std feature.

Proposal

1. Dispatch packets to sockets based on the port/protocol number only, leaving the question of sockets being bound to IPs to the Socket::proccess method.
2. Expand SocketSet by (conditionally) adding three std::HashMap's to it, for raw, tcp and udp sockets, and a Vec to keep dirty sockets' handlers.
3. Hash map for raw sockets should have a type of std::HashMap<(RawSocketType, IpProtocol), Vec<Handle>>, hash maps for udp and tcp should have a type of std::HashMap<u16, Vec<Handle>> . Vec is required to support the possibility of several sockets listening to the same port/protocol on different addresses.
4. Extend SocketSet::add to use socket's endpoint and ip_protocol methods to construct an index to insert socket's handler into the corresponding lookup table.
5. Extend SocketSet::remove to remove socket from a lookup table based on socket's endpoint.
6. Extend Set::IterMut to support construction from a mutable reference to ManagedSlice and an iterator of Handles, producing an iterator of Socket.
7. Replace SocketSet::iter_mut with iter_raw(&mut self, ip_repr: &IpRepr) -> IterMut and iter_l4(&mut self, ip_repr: &IpRepr) -> IterMut, which should look up sockets based on ip_repr and return an iterator over the sockets found. With the std feature disabled, the functions should return iterators over the entire SocketSet.
8. Add iter_dirty to SocketSet to produce an iterator of Socket by draining dirty_sockets vector, or an iterator over the entire SocketSet if std is disabled.
9. Add mark_drity to SocketSet to mark a socket as dirty.
10. Replace iter_mut in EthernetInterface::poll with iter_raw and iter_l4 and in EthernetInterface::emit with iter_dirty.

Questions

1. The only breakage of the current public API would be the requirement to explicitly mark written-to sockets as dirty. This can be avoided by having SocketSet::get_mut mark each requested socket as dirty (which kinda defeats the purpose), by having a specialized method SocketSet::get_mut_for_write to do so, or by devising some smart handler type which would mark a socket as dirty on send.
2. I'm not sure if the reference counting in Set::Item is relevant here.
3. I think EthernetInterface::poll should be refactored into several methods to increase readability.

IpEndpoint does not currently have a FromStr implementation or Parser::accept_ip_endpoint implementation.

Per the suggestion by @batonius.

... it's trivial to split an IPv4 endpoint at ':', IPv6 endpoints are quite tricky: https://tools.ietf.org/html/rfc5952#section-6 . I think the recommended style would be enough, it's the only one std::net::SocketAddrV6 supports: https://doc.rust-lang.org/nightly/src/std/net/parser.rs.html#280

Original comment: #72 (comment)

Is IPv6 something this library will support, may support, or will never support?

E.g. [0xff; 6] should be EthernetAddress::BROADCAST. I think they're stable since 1.20.

I think the possibility to move checksum calculation to the hardware when possible,
would be neat.

E.g. With LWIP you can do

#define CHECKSUM_BY_HARDWARE
#define CHECKSUM_GEN_IP 0
#define CHECKSUM_GEN_UDP 0
#define CHECKSUM_GEN_TCP 0
#define CHECKSUM_GEN_ICMP 0
#define CHECKSUM_CHECK_IP 0
#define CHECKSUM_CHECK_UDP 0
#define CHECKSUM_CHECK_TCP 0
#define CHECKSUM_CHECK_ICMP 0

and use Checksum insertion control of STM32 to let the hardware handle those checksums.
You'd simply not calculate the checksum in the software &
the SoC will insert it before transmitting/receiving the frame.

Would be possible to add a rustfmt.toml file (see https://github.com/rust-lang-nursery/rustfmt/blob/master/Configurations.md ) specifying the preferred formatting for smoltcp?

That way it would be easy to keep consistent formatting.

For example I noticed that the default rustfmt settings are different than what smoltcp is using, so it is hard to automatically format existing files.

smoltcp seems to be generating a bit of an ARP storm, at least when interacting with some security scan appliance.

Might be related to #25

@whitequark has the dump.

In a similar spirit, I would also like to implement support for IP fragmentation. The logic behind fragmentation is pretty straightforward, but the way to implement it less so.

So far it looks like it would need:

• a queue for the fragmented IP packets, where they can be stored before reassembling and passing them to the higher layers
• timers associated with each fragmented packet (similar to #25 ) so they can be dropped after a timeout

I think this means adding a queue to the https://github.com/m-labs/smoltcp/blob/master/src/iface/ethernet.rs

Thoughts?

Context: I've been working on reimplementing Redox's network stack with smoltcp, and it looks good so far. In the process, I've come up with a number of issues/ideas/proposals I want to discuss.

New features:

1. smoltcp doesn't support loopback interface, an attempt to connect to 127.0.0.1 results in a flood of ARP requests asking for the MAC address of 127.0.0.1. The loopback example solves this by using a loopback device, which is not an option for smolnetd. I think smoltcp should support several devices, each with several subnets assigned to them. This way we would be able to add loopback interface by adding a loopback device with 127.0.0.1/8 assigned to it.
2. smoltcp looks like a good place to implement DHCP since it already handles ARP, but I'm not sure how it should look like. Currently, Redox uses dhcpd daemon for this.
3. ICMP sockets aka IPPROTO_ICMP would be quite handy for implementing a version of ping able to work without suid, but I'm not sure if they're useful for anything besides ping. Right now Redox has icmpd daemon.
4. smoltcp doesn't check uniqueness of local ports when binding sockets and doesn't generate them when bind is called with port == 0, so smolnetd implements its own ports set. I think some form of it should be integrated into smoltcp itself, probably as part of the packet dispatch effort.

Improvements:

1. It would be nice to be able to manipulate TTL of a socket since it's supported by TcpStream and UdpSocket.
2. The current TCP accept/listen mechanism, while working, seems too limited. Namely, since smolnetd has to reopen a socket every time it accepts a connection, it's impossible to connect to the host during this period. I have no concrete proposal for this yet.
3. I think ARP requests rate should be limited, it's too easy to get smoltcp flooding the network with ARP packets.

Optimizations:

1. I still want to see #19 implemented.
2. Currently smolnetd iterates over all the sockets after each poll to see if it should send an event/resume a blocked call. It would be handy if EthernetInterface::poll produced some kind of list of the sockets which state changed during the call.
3. Right now smolnetd has to call SocketSet::prune after each SocketSet::release to free sockets, which means iteration over all the sockets in the set. I understand we can't free sockets in release because TCP sockets require shutdown, but maybe we should initiate the shutdown in release and then have SocketSet threat closed TCP sockets with refs == 0 as empty?

@whitequark Of all the issues, I think loopback interface is the most pressing right now, so I'd like to know what you think would be the best way to approach it.

cc @little-dude , @jackpot51

Implement ICMP sockets that are somewhat equivalent to linux IPPROTO_ICMP sockets. Much of the implementation may be similar to the other socket types, but there will likely be a few crucial differences.

• Icmpv4Socket::accepts is determined only by the source IP and destination IP. This means that if two separate icmp sockets sent from the same source to the same destination, things would get mixed up. Without access to the IP id, I don't see a way around this. If this direction is taken, a should_fail test with a note and an issue to make sure we don't lose track of this would be nice.
• The send and send_slice methods should be somewhat similar to the current udp implementation where the destination address is an arg.
• It might be more readable to use a recv_from method that takes the address to receive packets from as an arg instead of the typical recv function. If we really want to stick to recv instead of recv_from we could keep the destination that will be used by Icmpv4Socket::accepts from the previous send.

Related to: #50

To summarize:

• We should never return Err(Error::Malformed) for errors that depend on socket state (since it might simply be that we crashed and lost the state); only return that for packets that are invalid on their own, i.e. would not be accepted no matter what state the socket has.
• Returning Err(Error::Rejected) from TcpSocket::process() is only really appropriate while we're determining which endpoint they're addressed to. Once we know it's addressed to us (having two open sockets with the same local and remote endpoints, and the remote endpoint is not unspecified) we should not go on iterating through the sockets and send RST immediately.

In detail:

• Protocol check should be a debug_assert!() now.
• If state is CLOSED or the destination does not match, we return Err(Error::Rejected); it might be a half-open connection but it also might simply be addressed to another socket.
• If state is LISTEN and we get an ACK then we're probably a socket in backlog and it's actually destined to someone else, so return Err(Error::Rejected).
• If state is SYN-SENT and we get an RST with an unacceptable ACK we return Err(Error::Dropped); it's probably a half-open connection or other anomaly, and if the remote end doesn't want to accept our packets we'll get a proper RST when our SYN is retransmitted.
• RSTs in any other state need only have an acceptable SEQ.
• Any packet except SYNs is malformed if it's not also an ACK, so return Err(Error::Malformed).
• Unacceptable SEQs (so long as we're synchronized) should always trigger a challenge ACK.
• Unacceptable ACKs should always trigger a challenge ACK.
• Consider returning an Option<TcpRepr> from TcpSocket::process() so that we can bring reset generation together with the rest of the TCP code (and test it there too). This will still need to be exported so that EthernetInterface can respond with a reset e.g. even if we have no TCP sockets at all. It can also be used for generating challenge ACKs immediately and not on the next iteration of emission.

Line: https://github.com/m-labs/smoltcp/blob/master/src/phy/sys/raw_socket.rs#L21

hi, the libc::AF_PACKET constant only work on linux kernel.

After looking at the doc and at the code, I could not find out how to actively open a TCP connection. Is this implemented? If so, how can it be done?

But right now this returns Err.

From RFC 793:

When the receiving TCP has a zero window and a segment arrives it must
still send an acknowledgment showing its next expected sequence number
and current window (zero).

RFC1122 mandates in very strong terms that the following should be implemented:

• Cache entry invalidation timer, 60 seconds
• Flood prevention timer, 1 second

As mentioned in #50, I've been looking into support for multiple interfaces/devices. My overall plan is to leave the current EthernetInterface structure to represent a network interface with ARP cache, MAC/IP addresses, and a Device assigned to it, add InterfaceSet with a ManagedSlice of EthernetInterfaces and ipv4_gateway in it, and move all the packet processing logic from EthernetInterface to InterfaceSet.

I don't see any point in trying to do it statically, that means we should rely on trait objects a lot and have to accept the inevitable performance hit. The problem is that the current Device trait doesn't work well as a trait object because it uses the associated types by value. Since we need to call destructors on Rx/TxBuffer, and Box is the only way to own a trait object, refactoring Device::transmit to return something like Result<Box<AsMut[u8]>> looks promising, but using Box requires alloc, something we can't afford here.

All that means the Device trait should be redesigned into something more suitable for using as a trait object, maybe something like #49 (comment), but with the functions passed as function pointers directly to transmit/receive, if it makes any sense. Maybe we could have Device to own the latest received packet and have a method to return a slice to it, and use a functional argument for transmit.

I'm not sure if I should go ahead with this approach, so I wonder if you have any ideas.

Right now the way lifetimes are handled requires some very unobvious contortions. Here's the ionpak code: https://github.com/m-labs/ionpak/blob/4347ddc5379594b299e1fa6c294449e01cc27b78/firmware/src/main.rs#L158-L192

Is DHCP support planned? And is there a way to implement DHCP on top of smoltcp, i.e. without writing own UDP packet construction code?

Right now the ping example panics on iface.poll(&mut sockets, timestamp).expect("poll error") because of Error::Unaddressable "thrown" in EthernetInterface::lookup_hardware_addr, which "unrolls" all the way up to main.

The problem was introduced in 1ece71a when socket_egress stopped ignoring the error.

It's getting embarassing.

The phy::DeviceLimits struct has a private dummy field, which makes meaningful construction impossible. The only way is through the Default implementation, but this yields a max_transmission_unit of 0…

A DeviceLimits struct is required for implementing the limits method of the phy::Device trait.

Hi! First of all, thanks a lot for creating this library. It seems like a great fit for embedded and OS development.

Unfortunately, it no longer compiles with default-features=false, since
ManagedSlice::Owned is only available with use_std or use_collections.

error: no associated item named `Owned` found for type `managed::ManagedSlice<'_, _>` in the current scope
  --> /…/smoltcp-ebf9e93b1271bd34/874d192/src/socket/set.rs:64:13
   |
64 |             ManagedSlice::Owned(ref mut sockets) => {
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

(I think it would be a good idea to add an additional travis build config for default-features=false to prevent similar issues in the future.)

Hello,
actually I wanted to debug it, but need some help because it is disappears e.g. if writing to pcap or extensive logging is used.

The smoltcp server thread on tap0 will send in 64 byte steps, a Linux socket client thread will read only to ~292800 bytes instead of reading all 10000000 bytes. Smoltcp is handing out keep-alives from this point on. The minimal example is here:

https://github.com/pothos/taptest

cargo run --release should reproduce the behavior.

RUST_LOG=trace cargo run --release should let it sometimes complete (same as adding print statements in the sending server).
Uncommenting the pcap writer will let it complete reliably.

I have experienced it with other values as well, but hope that this combination also reproduces it on other machines.

Best regards,
Kai

Currently, when trying to send a packet to a destination that is not in the ARP cache, an unaddressable destination error is returned, but I think an ARP request should be made instead.

In process():

// Every acknowledgement must be for transmitted but unacknowledged data

This is incorrect. In RFC 793 the ACK sequence number being out of that range would have little effect; the payload would still be processed. It's just that the window or SND.UNA would not be updated based on the ACK sequence number.

This was then made more strict in RFC 5961 (which is the version worth implementing).

The ACK value is considered acceptable only if it is in the range of
((SND.UNA - MAX.SND.WND) <= SEG.ACK <= SND.NXT). All incoming segments
whose ACK value doesn't satisfy the above condition MUST be discarded and
an ACK sent back. 

So the changes to the code would be: a) expanding the range of acceptable ACKs to cover up to one window's worth of already ACKed data; b) sending a challenge ACK instead of just dropping the packet.

This is not just a theoretical concern. The too-strict check will cause trouble for bidirectional traffic in networks with reordering.

I think there is a bug in this line in wire/tcp.rs:

length = *buffer.get(1).ok_or(Error::Truncated)? as usize;
let data = buffer.get(2..length).ok_or(Error::Truncated)?;

I think it should be buffer.get(2..(2+length)) instead, because otherwise we try to take the range 2..1 for an option with length 1.

Hello, for our application I need smoltcp to handle multicast packets (the application communicates over multicast). I am happy to do that and implement this functionality to smoltcp, but I would like to get some pointers and guidance first.

1. IGMP protocol will have to be implemented - I would add another branch on the match expression here https://github.com/m-labs/smoltcp/blob/master/src/iface/ethernet.rs#L249
2. Implement EthernetProtocol::IGMP with underlying logic
3. when IGMP (and multicast) is enabled we check dst_addr() against the list of multicast MAC addresses we are subscribed to (here: https://github.com/m-labs/smoltcp/blob/master/src/iface/ethernet.rs#L244 )
4. appropriately handle timeouts (see https://en.wikipedia.org/wiki/Internet_Group_Management_Protocol ) - not sure if we need a separate timer or we should use the time supplied to iface.poll()

Does it sound like a good approach to you?

From RFC 793:

If the incoming segment has an ACK field, the reset takes its
sequence number from the ACK field of the segment, otherwise the
reset has sequence number zero and the ACK field is set to the sum
of the sequence number and segment length of the incoming segment.

We take small liberties with ACK handling but we shouldn't.