- Add `medium` in `DeviceCapabilities`.
- Rename EthernetInterface to Interface.
- Add support to Interface for both Ethernet and IP mediums. The medium to use is detected from `device.capabilities().medium`.
- Ethernet-only features are gated behind the "ethernet" feature, as before.
- IP features are always enabled for now.
The actual header length may be larger than the bpf_hdr struct due to aligning:
37ecb4d066/sys/net/bpf.c (L1649)8f02f2a044/bsd/net/bpf.c (L3580)
Tests are only valid for 32 and 64 bit architectures. I did not bother
guarding them with additional cfg flags.
Adds `is_subnet_broadcast` to the ethernet interface which checks for
subnet broadcasts, which are discussed on page 8 in
https://tools.ietf.org/html/rfc917. The subnet broadcast addresses are
derived from the interfaces ipv4 addresses.
This de-duplicates and (hopefully) simplifies a few if-else blocks. The
others were given an exception because I thought they were more readable
as is. I've verified that these changes don't result in larger binaries.
This was flagged by `cargo clippy`:
warning: field assignment outside of initializer for an instance
created with Default::default()
This changes the visibility of the dummy field to be public, but only to
the crate.
These were flagged by `cargo clippy`:
warning: using `clone` on a `Copy` type
warning: passing a unit value to a function
warning: redundant closure found
warning: called `iter().cloned().collect()` on a slice to create a
`Vec`. Calling `to_vec()` is both faster and more readable
This was flagged by `cargo clippy`:
warning: called `map(f)` on an `Option` value where `f` is a closure
that returns the unit type `()`
I decided to disable this because it was suggesting changes like the
following and I prefer the original:
@@ -1 +1 @@
-self.waker.take().map(|w| w.wake());
+if let Some(w) = self.waker.take() { w.wake() }
These were flagged by `cargo clippy`:
warning: the operation is ineffective. Consider reducing it to `number`
warning: this function has too many arguments (8/7)
warning: you should consider adding a `Default` implementation for
`phy::loopback::Loopback`
I like the code better as it is.
These were flagged by `cargo clippy`:
warning: the loop variable is used to index
I've verified that this doesn't increase the size of consuming binaries.
Pretty impressive. I tested this with a project that uses the following
features: ethernet, proto-dhcpv4, socket-tcp.
These were flagged by `cargo clippy`:
warning: redundant field names in struct initialization
There are plenty more redundant field names, but I only changed the ones
where the initialization was a single line of code. I still prefer the
redundant style for multi-line initializations (and I'm under the
impression that others agree), so I've also disabled the warning.
These were flagged by `cargo clippy`:
warning: called `is_some()` after searching an `Iterator` with find.
This is more succinctly expressed by calling `any()`.
warning: this `.into_iter()` call is equivalent to `.iter_mut()` and
will not consume the `BTreeMap`
warning: called `skip_while(p).next()` on an `Iterator`
The skip_while conversion is a little tricky. Clippy notes that:
warning: called `skip_while(p).next()` on an `Iterator`
help: this is more succinctly expressed by calling `.find(!p)` instead
So the condition of the skip_while is inverted and then simplified using
De Morgan's laws.
These were flagged by `cargo clippy`:
warning: explicit lifetimes given in parameter types where they
could be elided (or replaced with `'_` if needed by type
declaration)
These were flagged by `cargo clippy`:
warning: this seems like a manual implementation of the
non-exhaustive pattern
The non_exhaustive attribute isn't available until we increase the
minimum Rust version to 1.40, so we should just silence these warnings
in the meantime.
These were flagged by `cargo clippy`:
warning: you don't need to add `&` to all patterns
This should have happened in ac830e8b, but I guess I missed it.
These were flagged by `cargo clippy`:
warning: you seem to be trying to use match for destructuring a
single pattern. Consider using `if let`
This also silences a few cases where the match couldn't be replaced with
an if because of the following error:
error: attributes are not yet allowed on `if` expressions
Once we increase the minimum Rust version to 1.43, these can be updated
as well.
Functions that only deal with IP packets take/return IpPacket's. IpPacket's
are wrapped into EthernetPacket's as late as possible.
This will later allow generalizing Interface to handle both Ethernet
and pure-IP mediums.
This commit fixes a small bug, where the TcpSocket computed the maximum
size per payload in a tx_buffer without taking into account the MTU
defined by the underlying network device.
Prior to this change, the neighbor cache would get limited even when the
device failed to process our arp packet, which could put the socket in
the waiting state in the next poll.
This change only limits the cache rate if the device successfully
consumed our packet.
Previously, the window update was sent anytime it changed. This is
not desirable for 2 reasons:
- It would send window updates in TimeWait or Closed states, which is incorrect.
- It would send window updates in states where we've already received the
remote side's FIN, such as CloseWait. This is not necessarily incorrect, but
is useless, since the remote side is not going to send us more data, therefore
it's not interested in our window size anymore.
This allows applications to distinguish whether the remote end has
gracefully closed the connection with a FIN (in which case it has
received all the data intact), or the connection has failed due to e.g. a
RST or a timeout (in which case the received data may be truncated).
Previously, `ack_reply()` was sending the ack number present in `remote_last_ack`,
which is only updated by `dispatch`. This means the ack replies could contain
a wrong, old ack number.
Most of the time this simply caused wasting an extra roundtrip until the socket state
settles down, but in extreme cases it could lead to an infinite loop of packets, such
as in https://github.com/smoltcp-rs/smoltcp/issues/338Fixes#338
This fixes build on latest nightly. The cause of the breakage
is this commit:
7c4ca59f4b
which added a #[must_use = "if you don't need the old value, you
can just assign the new value directly"] hint to mem::replace.
When packet buffer's payload buffer does not have enough contiguous
window left, the ring buffer roll over uses an incorrect size
causing the ring buffer pointer not resetting to the head.
When the payload enqueued is larger than 1/2 of the payload ring
buffer, this bug will cause the slice returned by
`PacketBuffer::enqueue` to not match the requested size, and
trigger `debug_assert` in debug profile or size mismatch panic in
`copy_from_slice` when compiled in release profile.
Although ordering is not specified in the spec, some implementations expect
some ordering (unforntunately I have found one that does, and blocks outbound
packets due to 'invalid tcp options').
Instead of configuring the ifaces ip address when a dhcp server offers
and ip, we now use the requested_ip option, based on the offered ip
(yiaddr). The user now only has to configure the iface once the dhcp
transaction has completed (Ack'd).
Disable ethernet-related features and framing based on a feature gate.
Add a no-ethernet option to Travis.
notes:
- allow(unused) is added when not using ethernet feature
- Don't run pretty-print doctest, ethernet is optional.
Closes: #308
Approved by: whitequark
The OPT_DOMAIN_NAME_SERVER DHCP option field can advertise an unbounded
DNS servers, but we currently use a fixed length array to store these
servers. This commit ensures only the first 3 (highest priority) servers
are used.
Fixes#305.
`TCPSocket` buffers are type `RingBuffer<'a, _>` whilst other
socket types have buffers with type `PacketBuffer<'a, 'b,
_>`.
There is an enum over socket types `pub enum Socket<'a, 'b: 'a>`,
which is in turn used by `set::Item` and `SocketSet`. The lifetime
`'b` is not required for `TcpSocket`, but the `Into<Socket>`
implementation for `TCPSocket` specifies the `'static` lifetime for
`'b`.
Therefore using `TCPSocket` in a `SocketSet` currently requires the
lifetime `'b` for other sockets to be `'static` which is unnecessarily
restrictive. This patch fixes this.
The lifetime signature of socket specifies `'b: 'a`, that is `'b`
outlives `'a`. Instead of `'static`, this is also satisfied by
`impl<'a> Into<Socket<'a, 'a>> for TcpSocket<'a>` as `'a` "outlives"
`'a` by definition.
The example
[here](https://gist.github.com/richardeoin/b562e79d0d499e00a8bf4944f00594d9)
fails to compile without this patch.
Closes: #304
Approved by: whitequark
Using move semantics allows an Option to keep track of the
initialization state while satisfying the borrow checker. This replaces
the functionality that the 'consumed' flag had before. It also retains
the smaller object size since the Option of a reference can use the
representation of the null pointer, which is an invalid reference, as a
niche for the None variant.
Closes: #301
Approved by: whitequark
Since 0.4.4, log uses #[macro_export(local_inner_macros)], which is
the right thing to do, but it breaks our CI because of a now-unused
\#[macro_use(log)].
This didn't ever actually cause UB because the drop implementation
should have compiled to a no-op under all conditions; `finish` is
a part of a patchset that never got in. (Even if it didn't, the code
should have been dynamically sound, under all but the most strict
interpretations of Rust's aliasing requirements.)
But it's still a problem, at least because there is a warning for
this potential unsoundness that will become an error in the future.
So everything else aside, we can't use this pattern going forward.
For now, remove the Drop impl, exploiting the fact that `finish`
is a no-op. Going forward this will likely require unsafe code, or
some fundamental change in the way SocketRef is implemented, but
I could not come up with any way to do it. :(
Actually, this is not a new requirement at all; because we do not
check the minimum version on CI, some dependencies on 1.28 have
already sneaked in. In particular, our required version of the crate
managed only works on 1.28+.
This allows us to use:
(1.28)
- ops::RangeBounds
- num::NonZero
Some trait bounds were added to make sure everything builds on 1.28.
This doesn't affect downstream code because log 0.3.9 is a facade
crate implemented in terms of log 0.4, and so log 0.3 and log 0.4
APIs can be used together.
This allows us to use:
(1.26)
- impl Trait
- autoderef in pattern matching
- fixed slice patterns
- inclusive ranges
(1.27)
- dyn Trait
- #[must_use] on functions
To prepare for edition change, dyn is added where applicable. Other
edition changes would require bumping the requirement even higher,
and so they are not applied for now.
Adds accessor functions to the device implementing the phy::Device trait
within an ethernet interface. The intent is to allow access to read-only
methods such as gathering of device statistics or other diagnostics
while the interface is running. The interface does not hold any internal
invariants on the device, so that a mutable accessor can be added as
well.
Closes: #289
Approved by: whitequark
Allow the control flow to continue when udp parsing encounters an
invalid checksum within an ipv4 packet, instead of returning with an
error. Additionally adds a test to ensure the correct behaviour.
Closes: #280Closes: #290
Approved by: whitequark
Fixes a denied lint uncovered on a recent nightly compiler version,
apparently through improved analysis. The code in question is only
compiled with the proto-ipv6 feature and older compiler versions did not
detect the unused qualifier, sometime around 2019-04-23.
Closes: #291
Approved by: whitequark
A regression meant broadcast IPv4 packets were ignored. Restore
handling those packets and also reply to broadcast ICMPv4 echo requests.
Closes#287.
Closes: #288
Approved by: whitequark
Converting from `::std::net::*` types to the ip related library structs
previously required both ip-version features to be enabled. This
introduced dedicated converters from the ip-version specific standard
address and endpoint representations (`IpV4Addr`, `IpV6Addr`,
`SocketAddrV4`, and `SocketAddrV6`) that are enabled without requiring
the other ip-version feature to be selected.
Closes: #286
Approved by: whitequark
This allows any type that can be converted into an `i64` to be used when
creating an Instant. Because this is no longer a concrete type, this
change may break existing code like the following:
error: attempt to multiply with overflow
--> src/time.rs:282:37
|
282 | epoc = Instant::from_millis(2085955200 * 1000).into();
| ^^^^^^^^^^^^^^^^^
|
= note: #[deny(const_err)] on by default
Closes: #272
Approved by: whitequark
Only allowing four missing packets is hurting
performance for large receive windows.
The new value was picked by observing good
performance with 500 kB socket buffers.
Closes: #264
Approved by: whitequark
The current hole was always shrinked even if the
packet assembler rejected adding a packet due to
not being able to insert a new hole.
Shrink only after a new hole was added.
Shrinking before or after does not make a difference
because offset + size < hole_size and thus
contigs[index] is not empty, passing the
debug_assert in add_contig_at.
https://github.com/m-labs/smoltcp/issues/261Closes: #265
Approved by: whitequark
This cargo feature only exists because (a) ARTIQ uses a fork of Rust,
(b) Rust has some ridiculous renaming going on in the alloc crate,
(c) smoltcp exists because of ARTIQ.
Such features will not be ordinarily provided by smoltcp.
The presence of TCP options will only increase
the header length to muliples of 4. The added
padding consists of zeros.
This also fixes an emit panic when the Window
Scale and MSS options result in a length of 27
which then gets floored to 24 when applied to
the header.
Closes: #251
Approved by: whitequark
The size of the contiguous window should be prioritized over the size of
the contiguous window if padding were added.
For example, given the following packet buffer:
1 2 3
|---|----|------|
If 2 is used and 1 and 3 are empty, enqueing a buffer of size 4 should
be placed in contiguous window 3 and the size of 1 should not cause
any issues in enqueue.
Closes: #247
Approved by: whitequark
Using Routes<'static> used to make these functions impossible because it created a reference that needed to be valid for the static lifetime
Closes: #243
Approved by: dlrobertson
Improve detection of duplicate ACKs by checking that the segment does
not contain data.
Move implementation from the 'reject unacceptable acknowledgements'
to later in the process function, because a duplicate ACK is not an
'unacceptable' acknowledgement but rather a condition to update state.
Implement more tests to verify the operation of the fast retransmit
implementation.
Related issue: #224Closes: #225
Approved by: whitequark
- Add a helper function for returning an error in the implementation of
next
- Add an assert in Ipv6OptionsIterator::new that ensures that an
iterator is not created with a length field larger than that of the
data buffer we're iterating over.
Closes: #230
Approved by: dlrobertson
The IPv6 Option representation layer structure includes the type value
as the raw u8. This should be stored as the Type enum.
Closes: #229
Approved by: whitequark
- Do not require the packet structure of the header have the same
lifetime as the underlying bytes or the parsed representation.
- Add the bytes of the contained options to the parsed representation
structure.
- Add the IPv6OptionsIterator structure to make iterating over the
contained options easier.
Closes: #212
Approved by: whitequark
This optimization makes sure that enqueueing to an empty ring buffer
would never wrap around, in turn ensuring that enqueueing to an empty
packet buffer would never use more than one metadata entry.
Right now, pushing buffer-sized packets into a packet buffer requires
at least two metadata entries, which is surprising and wasteful.
Implement a simple way to discover and respond to three duplicate ACKs
from the reciver of the data. This leads to a fast retransmit of the
last unacknowledged seqment. This is feature is described in [RFC 5681].
Closes: #104Closes: #214
Approved by: whitequark
Add the FailureType enumeration that can be acquired from a IPv6
extension header option which determines the required action if the
extension header option type is not known or is not supported.
Closes: #204
Approved by: whitequark
Only ACKs generated by `dispatch` were updating
`remote_last_win` but not the ACKs generated by
`process`, failing to keep track of the last
announced receive window. This left
`window_to_update` to return false in `dispatch`
after the RX buffer was emptied, thus not announcing
that the receive window is not zero anymore.
The change updates `remote_last_win` for immediate
ACKs generated by `process`.
Closes: #200
Approved by: whitequark
Dario Nieuwenhuis
Ryan Summers
whitequark
David Nadlinger
Nathan K. Zhinn
Nathan K. Zhinn
Ben Stabler
Vlad Krasnov
Scott Mabin
Alex Crawford
Niclas Hoyer
JOE1994
mustermeiszer
Sam Jones
Ryan Summers
Thales Fragoso
YOUNGSUK KIM
Lucas Zanela
Robin Lambertz
Zhaofeng Li
fdb-hiroshima
nabijaczleweli
Gary Guo
Pierre Krieger
ficapy
Scott Mabin
Ruben De Smet
Chris Ballance
Richard Meadows
Andreas Molzer
whitequark
David Wood
jhwgh1968
Astro
Adam Greig
Chris Branch
Kai Lüke
Derk Bell
Luka Atanasovski
M@ Dunlap
Dan Robertson
Sidharth Shanker
Dan Robertson
jD91mZM2
Valentin Lorentz
Ole Martin Ruud
Herman J. Radtke III
whitequark