This lint is in stable Rust now: https://github.com/rust-lang/rust/issues/43302.
These changes are noted according to code comment from @whitequark.
Some test cases and functions are altered due to changes of #[must_use].
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
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)
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.
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
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
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.
- There are several warnings that are thrown when running `cargo doc`. Fix
these warnings.
- Convert all module documentation to use /*! for consistency.
Some of them already use Result, but the ones that can return
an empty slice (or a slice shorter than requested) also must have
their return value (at least) checked.
set_len() with a positive length change is better represented
with enqueue(), and set_len() with a negative length change
is not really a valid operation on its own.
This also makes TcpSocket::{send,recv}_slice slightly more efficient
in case when the slice wraps around the corresponding buffer,
halving the necessary amount of calls.
Before this commit, anything that touched RawSocket or TapInterface
worked partly by accident and partly because of a horrible crutch
that resulted in massive latencies as well as inevitable packet loss
every time an ARP request had to be issued. Also, there was no way
to use poll() other than by continuously calling it in a busy loop.
After this commit, poll() indicates when the earliest timer expires,
and so the caller can sleep until that moment (or until packets
arrive).
Note that there is a subtle problem remaining: every time poll()
is called, every socket with a pending outbound packet whose
IP address doesn't correspond to a MAC address will send a new
ARP request, resulting in potentially a whole lot of such requests.
ARP rate limiting is a separate topic though.
The use of this type has several drawbacks:
* It does not allow distinguishing between different error
conditions. In fact, we wrongly conflated some of them
before this commit.
* It does not allow propagation via ? and requires manual use
of map_err, which is especially tiresome for downstream code.
* It prevents us from expanding the set of error conditions
even if right now we have only one.
* It prevents us from blanket using Result<T> everywhere
(a nitpick at most).
Instead, use Result<T, Error> everywhere, and differentiate error
conditions where applicable.
luojia65
Thibaut Vandervelden
Dario Nieuwenhuis
Ryan Summers
Alex Crawford
Gary Guo
Scott Mabin
Kai Lüke
jhwgh1968
Dan Robertson
Astro
whitequark
Philipp Oppermann
Dan Robertson
Egor Karavaev