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.
The changes in this commit affect the following scenario:
* Remote end sends octets 1..2, they are delivered and buffered
on local end;
* Remote end sends octets 3..4, they are lost;
* Remote end sends octets 5..6, they are delivered but cannot
be buffered on local end because we don't perform reassembly.
Before this commit, we would silently drop the segment with octets
5..6, relying on retransmission timer on the remote end. This works,
but can result in severe decrease in throughput. After this commit,
we send a duplicate ACK, which may trigger fast retransmit, if
implemented by the congestion control algorithm on the remote end.
This is a form of an uninitialized read bug; although safe it caused
panics. In short, transmit buffers received from the network stack
should be considered uninitialized (in practice they will often
contain previously transmitted packets or parts thereof). Wrapping
them with the only method we had (e.g. Ipv4Packet) treated the buffer
as if it contained a valid incoming packet, which can easily fail
with Error::Truncated.
This commit splits every `fn new(buffer: T) -> Result<Self, Error>`
method on a `Packet` into three smaller ones:
* `fn check_len(&self) -> Result<(), Error>`, purely a validator;
* `fn new(T) -> Self`, purely a wrapper;
* `fn new_checked(T) -> Result<Self, Error>`, a validating wrapper.
This makes it easy to process ingress packets (using `new_checked`),
egress packets (using `new`), and, if needed, maintain the invariants
at any point during packet construction (using `check_len`).
Fixes#17.
This is a conservative bound; if we don't have enough buffers
to receive more than four segments, clearly we shouldn't advertise
our ability to.
It however will only work reliably with exactly one TCP connection
continuously receiving; for two, another window adjustment mechanism
will be needed for reliable reception.
Previously, sockets could get stuck in the CLOSING state, after
the sequence described in the new test_mutual_close_with_data_2.
The root cause was that some state machine transitions got
folded into ACK handling.
Now, all transitions are handled in the same match statement,
and ACK handling is broken up around it.