Skip to content

IPv6 stack support#431

Merged
hannesm merged 9 commits into
mirage:masterfrom
MagnusS:v6-sig
Sep 2, 2020
Merged

IPv6 stack support#431
hannesm merged 9 commits into
mirage:masterfrom
MagnusS:v6-sig

Conversation

@MagnusS
Copy link
Copy Markdown
Member

@MagnusS MagnusS commented Aug 27, 2020

This adds support for configuring a V6 stack in tcpip_stack_direct and adds a TCPv6 connect test that succeeds. It depends on unmerged changes in mirage-stack so shouldn't be merged yet (mirage-stack is pinned to my branch).

I had to disable the test that adds trailing bytes to the ethernet frames, as this results in a lot of packets being dropped due to checksum errors in our stack (checksum in pcap is fine). This seems to indicate that the implementation calculates the checksum of the full buffer - not just the IP packet length, but I haven't investigated this further. (fixed)

The stack will also queue packets without a source IP before the IPv6 address has been negotiated. These packets will be sent when the IP is available and confuse the other end. I've added a temporary fix to wait for the IPv6 address to be configured before the test starts.

Comment thread test/vnetif_common.ml
@hannesm
Copy link
Copy Markdown
Member

hannesm commented Aug 30, 2020

thanks @MagnusS

I had to disable the test that adds trailing bytes to the ethernet frames, as this results in a lot of packets being dropped due to checksum errors in our stack (checksum in pcap is fine). This seems to indicate that the implementation calculates the checksum of the full buffer - not just the IP packet length, but I haven't investigated this further.

I fixed the code in 6d68284, which includes some more safety (i.e. avoiding Cstruct exceptions in get_yyy).

@MagnusS
Copy link
Copy Markdown
Member Author

MagnusS commented Aug 31, 2020

Thanks for the fixes @hannesm !

Now that ipv6 generally seems to work I also enabled the iperf test to test TCP connections over IPv6. I've tested with ~100mb transfers over vnetif and it seems to work well, also with packet loss (at least as long as both ends are mirage unikernels...).

The iperf test now also reports the results every second with Logs.info, so it's possible to run an ipv4/ipv6 comparison by executing test.exe directly: ALCOTEST_VERBOSE=1 dune exec test/test.exe test iperf 6 (a bug had been introduced at some point which made the output kbit/ns and very noisy)

On my machine it looks like the ipv4 test is a little bit faster with an avg rate of ~657 Mbit/s, while IPv6 is ~627 Mbit/s. This is measured over 10 x test/test.exe test iperf 6, which will transfer around 100mb of data. This seems very reasonable as we just started using this part of the stack.

@hannesm
Copy link
Copy Markdown
Member

hannesm commented Sep 1, 2020

I'm fine with these changes, but would wait until ocaml/opam-repository#17090 hits opam-repository, and then remove the mirage-stack pin (and instead put a lower bound on "mirage-stack" {>= "2.1.0"} into tcpip.opam) before merging this PR.

MagnusS and others added 9 commits September 1, 2020 20:26
use buffer only up to advertised length
check version to be 6

--> enable the trailing bytes connect test for IPv6
This updates the iperf test to report speed in mbit/s once every second
and when the test ends. The output can be seen when alcotest is executed
in verbose mode.

Also increases the length of the slow tests to around ~100MB per
transfer. These are not run in CI by default.
This duplicates the iperf test for ipv6. As the V4 stack signatures
contain a reference to the IP version (e.g. listen_tcpv4 etc) it was
difficult to reuse the test without major changes. We should clean this
up when we unify the signatures to avoid duplicated tests for IPv4/IPv6.
@MagnusS
Copy link
Copy Markdown
Member Author

MagnusS commented Sep 1, 2020

mirage-stack 2.1.0 is merged -- I rebased and updated tcpip.opam. I think this is ready to be merged now when CI is green

@MagnusS MagnusS changed the title Experimental V6 stack support [do not merge] IPv6 stack support Sep 1, 2020
@hannesm hannesm merged commit 2c7fa32 into mirage:master Sep 2, 2020
hannesm added a commit to hannesm/opam-repository that referenced this pull request Sep 22, 2020
CHANGES:

* Assorted IPv6 improvements (mirage/mirage-tcpip#428 mirage/mirage-tcpip#431 mirage/mirage-tcpip#432 @MagnusS @hannesm)
  - set length in packets to be sent
  - preserve updated ctx from Ndv6.handle
  - fix ICMP checksum computation
  - implement Mirage_stack.V6 signature
  - add connect, mtu, iperf tests
  - fix DAD protocol implementation (and test it)
  - avoid out of bounds accesses of IPv6 packets (check length before accessing)
* Fix 32 bit issues (@MagnusS)
* Implement stack-direct and tcp disconnect: tear down existing connections (mirage/mirage-tcpip#429 @hannesm)
* Treat broadcast address of network as broadcast as well (mirage/mirage-tcpip#430 @hannesm, reported in mirage/mirage-tcpip#427)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants