From da5e6a6d1835c81713fd5cb7f7aa40977925d9ed Mon Sep 17 00:00:00 2001 From: Hannes Mehnert Date: Sun, 23 Aug 2020 14:44:07 +0200 Subject: [PATCH 01/10] Ndpv6: set IPv6 length in packets --- src/ipv6/ndpv6.ml | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ipv6/ndpv6.ml b/src/ipv6/ndpv6.ml index b5cfecee6..52f9d92b0 100644 --- a/src/ipv6/ndpv6.ml +++ b/src/ipv6/ndpv6.ml @@ -164,6 +164,7 @@ module Allocate = struct Ipv6_wire.set_ipv6_nhdr ipbuf (Ipv6_wire.protocol_to_int proto); let hdr, payload = Cstruct.split ipbuf Ipv6_wire.sizeof_ipv6 in let len' = fillf hdr payload in + Ipv6_wire.set_ipv6_len ipbuf len'; assert (len' <= size') ; len' + Ipv6_wire.sizeof_ipv6 in From 92f262de5476595c269b6c5e010a11bed9b2e67c Mon Sep 17 00:00:00 2001 From: Hannes Mehnert Date: Sun, 23 Aug 2020 20:08:18 +0200 Subject: [PATCH 02/10] Ipv6: update/preserve ctx returned from ndpv6.handle --- src/ipv6/ipv6.ml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ipv6/ipv6.ml b/src/ipv6/ipv6.ml index a57d51a95..800e864c5 100644 --- a/src/ipv6/ipv6.ml +++ b/src/ipv6/ipv6.ml @@ -90,7 +90,8 @@ module Make (E : Mirage_protocols.ETHERNET) let input t ~tcp ~udp ~default buf = let now = C.elapsed_ns () in - let _, outs, actions = Ndpv6.handle ~now ~random:R.generate t.ctx buf in + let ctx, outs, actions = Ndpv6.handle ~now ~random:R.generate t.ctx buf in + t.ctx <- ctx; Lwt_list.iter_s (function | `Tcp (src, dst, buf) -> tcp ~src ~dst buf | `Udp (src, dst, buf) -> udp ~src ~dst buf From 23c8685afdc56ebdec4dab25122a090c7e273f95 Mon Sep 17 00:00:00 2001 From: Hannes Mehnert Date: Sun, 23 Aug 2020 20:13:18 +0200 Subject: [PATCH 03/10] Ndpv6: set length early --- src/ipv6/ndpv6.ml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ipv6/ndpv6.ml b/src/ipv6/ndpv6.ml index 52f9d92b0..ac3ec30b9 100644 --- a/src/ipv6/ndpv6.ml +++ b/src/ipv6/ndpv6.ml @@ -158,14 +158,14 @@ module Allocate = struct let size' = size + Ipv6_wire.sizeof_ipv6 in let fill ipbuf = Ipv6_wire.set_ipv6_version_flow ipbuf 0x60000000l; (* IPv6 *) + Ipv6_wire.set_ipv6_len ipbuf size; ipaddr_to_cstruct_raw src (Ipv6_wire.get_ipv6_src ipbuf) 0; ipaddr_to_cstruct_raw dst (Ipv6_wire.get_ipv6_dst ipbuf) 0; Ipv6_wire.set_ipv6_hlim ipbuf hlim; Ipv6_wire.set_ipv6_nhdr ipbuf (Ipv6_wire.protocol_to_int proto); let hdr, payload = Cstruct.split ipbuf Ipv6_wire.sizeof_ipv6 in let len' = fillf hdr payload in - Ipv6_wire.set_ipv6_len ipbuf len'; - assert (len' <= size') ; + assert (len' = size) ; len' + Ipv6_wire.sizeof_ipv6 in (size', fill) From 9278c44b27f1726f28a13ea7b3e51793f0a04015 Mon Sep 17 00:00:00 2001 From: Hannes Mehnert Date: Sun, 23 Aug 2020 20:16:54 +0200 Subject: [PATCH 04/10] Ndpv6: fix ICMP pong checksum computation --- src/ipv6/ndpv6.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ipv6/ndpv6.ml b/src/ipv6/ndpv6.ml index ac3ec30b9..4c25a95c0 100644 --- a/src/ipv6/ndpv6.ml +++ b/src/ipv6/ndpv6.ml @@ -234,8 +234,8 @@ module Allocate = struct Ipv6_wire.set_pingv6_id icmpbuf id; Ipv6_wire.set_pingv6_seq icmpbuf seq; Ipv6_wire.set_pingv6_csum icmpbuf 0; - Ipv6_wire.set_pingv6_csum icmpbuf @@ checksum hdr (icmpbuf :: data :: []); Cstruct.blit data 0 icmpbuf Ipv6_wire.sizeof_pingv6 (Cstruct.len data); + Ipv6_wire.set_pingv6_csum icmpbuf @@ checksum hdr [ icmpbuf ]; size in hdr ~src ~dst ~hlim ~proto:`ICMP ~size fillf From c73d0795d970836193d55ec1c921f892d44bf220 Mon Sep 17 00:00:00 2001 From: Hannes Mehnert Date: Sun, 23 Aug 2020 21:40:16 +0200 Subject: [PATCH 05/10] test: disable connect and iperf test to check whether this fixes CI --- test/test.ml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test.ml b/test/test.ml index ede9b9b8b..e4acff326 100644 --- a/test/test.ml +++ b/test/test.ml @@ -25,9 +25,9 @@ let suite = [ "mtu+tcp" , Test_mtus.suite ; "rfc5961" , Test_rfc5961.suite ; "socket" , Test_socket.suite ; - "connect" , Test_connect.suite ; + (* "connect" , Test_connect.suite ;*) "deadlock" , Test_deadlock.suite ; - "iperf" , Test_iperf.suite ; + (* "iperf" , Test_iperf.suite ;*) "keepalive" , Test_keepalive.suite ; ] From c2d16b24ec7feccc19585652706a144defe39bdc Mon Sep 17 00:00:00 2001 From: Hannes Mehnert Date: Sun, 23 Aug 2020 21:45:04 +0200 Subject: [PATCH 06/10] Ndpv6: remove assertion regarding sizes --- src/ipv6/ndpv6.ml | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ipv6/ndpv6.ml b/src/ipv6/ndpv6.ml index 4c25a95c0..666b41713 100644 --- a/src/ipv6/ndpv6.ml +++ b/src/ipv6/ndpv6.ml @@ -165,7 +165,6 @@ module Allocate = struct Ipv6_wire.set_ipv6_nhdr ipbuf (Ipv6_wire.protocol_to_int proto); let hdr, payload = Cstruct.split ipbuf Ipv6_wire.sizeof_ipv6 in let len' = fillf hdr payload in - assert (len' = size) ; len' + Ipv6_wire.sizeof_ipv6 in (size', fill) From bfbb7fbf8dbbc6f751111fc7fba58e47f8e19ffd Mon Sep 17 00:00:00 2001 From: Magnus Skjegstad Date: Mon, 24 Aug 2020 19:43:35 +0200 Subject: [PATCH 07/10] Fix ipv6 test exception The ipv6 test disconnected from the vnetif backend while the stack was still running, resulting in a `Not_found` exception being returned from mirage-vnetif when Ethernet.write was called. This only became a problem in a recent commit that fixed an issue that prevented some of the background timers from running in the IPv6 stack. The V.disconnect call seems to have been added to work around another IPv6 issue where the UDP sender will queue packets with an empty source address until an IPv6 address is configured. This results in multiple packets with empty source address being received after the first correct packet, unless the receiver disconnected from the backend before this happens. This updates the test to not send UDP packets until an IP address is acquired and removes the call V.disconnect in the receiver. It does not change the queuing behaviour of the IPv6 stack, which may also be incorrect. --- test/test_ipv6.ml | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/test/test_ipv6.ml b/test/test_ipv6.ml index 57211068b..8e201e37c 100644 --- a/test/test_ipv6.ml +++ b/test/test_ipv6.ml @@ -47,24 +47,29 @@ let listen ?(tcp = noop) ?(udp = noop) ?(default = noop) stack = ~udp:udp ~default:(fun ~proto:_ -> default))) >>= fun _ -> Lwt.return_unit -let udp_message = Cstruct.of_string "hello on UDP over IPv6" +let udp_message = (Cstruct.of_string "hello on UDP over IPv6") -let check_for_one_udp_packet netif on_received_one ~src ~dst buf = - Alcotest.(check ip) "sender address" (Ipaddr.V6.of_string_exn "fc00::23") src; - Alcotest.(check ip) "receiver address" (Ipaddr.V6.of_string_exn "fc00::45") dst; +let check_for_one_udp_packet on_received_one ~src ~dst buf = (match Udp_packet.Unmarshal.of_cstruct buf with | Ok (_, payload) -> + Printf.fprintf stderr "Receiver got UDP from src=%s dst=%s payload='%s'\n%!" (Ipaddr.V6.to_string src) (Ipaddr.V6.to_string dst) (Cstruct.to_string payload); + Alcotest.(check ip) "sender address" (Ipaddr.V6.of_string_exn "fc00::23") src; + Alcotest.(check ip) "receiver address" (Ipaddr.V6.of_string_exn "fc00::45") dst; Alcotest.(check cstruct) "payload is correct" udp_message payload | Error m -> Alcotest.fail m); (try Lwt.wakeup_later on_received_one () with _ -> () (* the first succeeds, the rest raise *)); - (*after receiving 1 packet, disconnect stack so test can continue*) - V.disconnect netif + Lwt.return_unit let send_forever sender receiver_address udp_message = let rec loop () = - Printf.fprintf stderr "Udp.write\n%!"; - Udp.write sender.udp ~dst:receiver_address ~dst_port:1234 udp_message - >|= Rresult.R.get_ok >>= fun () -> + (* Check that we have an IP before sending *) + if (List.length (Ipv6.get_ip sender.ip)) >= 1 then + begin + Udp.write sender.udp ~dst:receiver_address ~dst_port:1234 udp_message + >|= Rresult.R.get_ok + end else + Lwt.return_unit + >>= fun () -> Time.sleep_ns (Duration.of_ms 50) >>= fun () -> loop () in loop () @@ -77,7 +82,7 @@ let pass_udp_traffic () = get_stack backend receiver_address >>= fun receiver -> let received_one, on_received_one = Lwt.task () in Lwt.pick [ - listen receiver ~udp:(check_for_one_udp_packet receiver.netif on_received_one); + listen receiver ~udp:(check_for_one_udp_packet on_received_one); listen sender; send_forever sender receiver_address udp_message; received_one; (* stop on the first packet *) From de101139bfdb22852c92534099573cd9fe23b105 Mon Sep 17 00:00:00 2001 From: Magnus Skjegstad Date: Mon, 24 Aug 2020 19:54:50 +0200 Subject: [PATCH 08/10] Reenable the ipv6 test, turn on exception backtraces --- test/test.ml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/test.ml b/test/test.ml index e4acff326..98221bdcb 100644 --- a/test/test.ml +++ b/test/test.ml @@ -25,7 +25,7 @@ let suite = [ "mtu+tcp" , Test_mtus.suite ; "rfc5961" , Test_rfc5961.suite ; "socket" , Test_socket.suite ; - (* "connect" , Test_connect.suite ;*) + "connect" , Test_connect.suite ; "deadlock" , Test_deadlock.suite ; (* "iperf" , Test_iperf.suite ;*) "keepalive" , Test_keepalive.suite ; @@ -35,6 +35,7 @@ let run test () = Lwt_main.run (test ()) let () = + Printexc.record_backtrace true; (* someone has to call Mirage_random_test.initialize () *) Mirage_random_test.initialize (); (* enable logging to stdout for all modules *) From 3b91e2e8d9af34f7dc43acf0369cbfca0346fadb Mon Sep 17 00:00:00 2001 From: Magnus Skjegstad Date: Tue, 25 Aug 2020 14:06:33 +0200 Subject: [PATCH 09/10] Fix MTU enforced test backend The previous backend contained a ref to the MTU in the module, so the MTU was remembered between tests. The iperf test would not succeed unless the mtu+tcp test had run previously, as it set the MTU to 9000 instead of the expected 1500. This renames the MTU enforced backend to Frame_size_enforced to make it clear that the size limit is on the Ethernet layer. The frame size is stored in the returned `t` and is now reset between tests. A helper function is also added to set the enforced frame size based on the IP layer MTU (MTU + size of ethernet header). This also re-enables the iperf test as it now succeeds. The previous failure was due to the enforced frame size being equal to the MTU, leaving no room for the Ethernet header. --- test/test.ml | 2 +- test/test_iperf.ml | 13 ++++++++----- test/test_ipv6.ml | 1 - test/test_mtus.ml | 4 ++-- test/vnetif_backends.ml | 42 +++++++++++++++++++++++++++++++++-------- 5 files changed, 45 insertions(+), 17 deletions(-) diff --git a/test/test.ml b/test/test.ml index 98221bdcb..64e7f4e3e 100644 --- a/test/test.ml +++ b/test/test.ml @@ -27,7 +27,7 @@ let suite = [ "socket" , Test_socket.suite ; "connect" , Test_connect.suite ; "deadlock" , Test_deadlock.suite ; - (* "iperf" , Test_iperf.suite ;*) + "iperf" , Test_iperf.suite ; "keepalive" , Test_keepalive.suite ; ] diff --git a/test/test_iperf.ml b/test/test_iperf.ml index 6fa29472e..ad49b9774 100644 --- a/test/test_iperf.ml +++ b/test/test_iperf.ml @@ -43,9 +43,9 @@ module Test_iperf (B : Vnetif_backends.Backend) = struct client : V.Stackv4.t; } - let default_network ?(backend = B.create ()) () = - V.create_stack ~cidr:client_cidr ~gateway backend >>= fun client -> - V.create_stack ~cidr:server_cidr ~gateway backend >>= fun server -> + let default_network ?mtu ?(backend = B.create ()) () = + V.create_stack ?mtu ~cidr:client_cidr ~gateway backend >>= fun client -> + V.create_stack ?mtu ~cidr:server_cidr ~gateway backend >>= fun server -> Lwt.return {backend; server; client} let msg = @@ -196,8 +196,11 @@ let test_tcp_iperf_two_stacks_basic amt timeout () = (Test.tcp_iperf ~server ~client amt timeout) let test_tcp_iperf_two_stacks_mtu amt timeout () = - let module Test = Test_iperf (Vnetif_backends.Mtu_enforced) in - Test.default_network () >>= fun { backend; Test.client; Test.server } -> + let mtu = 1500 in + let module Test = Test_iperf (Vnetif_backends.Frame_size_enforced) in + let backend = Vnetif_backends.Frame_size_enforced.create () in + Vnetif_backends.Frame_size_enforced.set_max_ip_mtu backend mtu; + Test.default_network ?mtu:(Some mtu) ?backend:(Some backend) () >>= fun { backend; Test.client; Test.server } -> Test.V.record_pcap backend (Printf.sprintf "tcp_iperf_two_stacks_mtu_%d.pcap" amt) (Test.tcp_iperf ~server ~client amt timeout) diff --git a/test/test_ipv6.ml b/test/test_ipv6.ml index 8e201e37c..042adff96 100644 --- a/test/test_ipv6.ml +++ b/test/test_ipv6.ml @@ -52,7 +52,6 @@ let udp_message = (Cstruct.of_string "hello on UDP over IPv6") let check_for_one_udp_packet on_received_one ~src ~dst buf = (match Udp_packet.Unmarshal.of_cstruct buf with | Ok (_, payload) -> - Printf.fprintf stderr "Receiver got UDP from src=%s dst=%s payload='%s'\n%!" (Ipaddr.V6.to_string src) (Ipaddr.V6.to_string dst) (Cstruct.to_string payload); Alcotest.(check ip) "sender address" (Ipaddr.V6.of_string_exn "fc00::23") src; Alcotest.(check ip) "receiver address" (Ipaddr.V6.of_string_exn "fc00::45") dst; Alcotest.(check cstruct) "payload is correct" udp_message payload diff --git a/test/test_mtus.ml b/test/test_mtus.ml index 18f8211ee..0d46e5817 100644 --- a/test/test_mtus.ml +++ b/test/test_mtus.ml @@ -5,7 +5,7 @@ let client_cidr = Ipaddr.V4.Prefix.of_string_exn "192.168.1.10/24" let server_port = 7 -module Backend = Vnetif_backends.Mtu_enforced +module Backend = Vnetif_backends.Frame_size_enforced module Stack = Vnetif_common.VNETIF_STACK(Backend) let default_mtu = 1500 @@ -36,7 +36,7 @@ let get_stacks ?client_mtu ?server_mtu backend = Stack.create_stack ~cidr:client_cidr ~mtu:client_mtu backend >>= fun client -> Stack.create_stack ~cidr:server_cidr ~mtu:server_mtu backend >>= fun server -> let max_mtu = max client_mtu server_mtu in - Backend.set_mtu max_mtu; + Backend.set_max_ip_mtu backend max_mtu; Lwt.return (server, client) let start_server ~f server = diff --git a/test/vnetif_backends.ml b/test/vnetif_backends.ml index 0a7262d53..73a07ff54 100644 --- a/test/vnetif_backends.ml +++ b/test/vnetif_backends.ml @@ -21,23 +21,49 @@ module type Backend = sig val create : unit -> t end -(** This backend enforces an MTU. *) -module Mtu_enforced = struct +(** This backend enforces an Ethernet frame size. *) +module Frame_size_enforced = struct module X = Basic_backend.Make - include X + type t = { + xt : X.t; + mutable frame_size : int; + } - let mtu = ref 1500 + type macaddr = X.macaddr + type 'a io = 'a X.io + type buffer = X.buffer + type id = X.id + + let register t = + X.register t.xt + + let unregister t id = + X.unregister t.xt id + + let mac t id = + X.mac t.xt id + + let set_listen_fn t id buf = + X.set_listen_fn t.xt id buf + + let unregister_and_flush t id = + X.unregister_and_flush t.xt id let write t id ~size fill = - if size > !mtu then + if size > t.frame_size then Lwt.return (Error `Invalid_length) else - X.write t id ~size fill + X.write t.xt id ~size fill + + let set_frame_size t m = t.frame_size <- m + let set_max_ip_mtu t m = t.frame_size <- m + Ethernet_wire.sizeof_ethernet - let set_mtu m = mtu := m + let create ~frame_size () = + let xt = X.create ~use_async_readers:true ~yield:(fun() -> Lwt_main.yield () ) () in + { xt ; frame_size } let create () = - X.create ~use_async_readers:true ~yield:(fun() -> Lwt_main.yield () ) () + create ~frame_size:(1500 + Ethernet_wire.sizeof_ethernet) () end From c9e1a04aed647a6a7b0d98fb85854e8faa16b696 Mon Sep 17 00:00:00 2001 From: Magnus Skjegstad Date: Wed, 26 Aug 2020 16:30:40 +0200 Subject: [PATCH 10/10] Remove unnecessary parentheses Co-authored-by: Hannes Mehnert --- test/test_ipv6.ml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_ipv6.ml b/test/test_ipv6.ml index 042adff96..bdcd862f1 100644 --- a/test/test_ipv6.ml +++ b/test/test_ipv6.ml @@ -47,7 +47,7 @@ let listen ?(tcp = noop) ?(udp = noop) ?(default = noop) stack = ~udp:udp ~default:(fun ~proto:_ -> default))) >>= fun _ -> Lwt.return_unit -let udp_message = (Cstruct.of_string "hello on UDP over IPv6") +let udp_message = Cstruct.of_string "hello on UDP over IPv6" let check_for_one_udp_packet on_received_one ~src ~dst buf = (match Udp_packet.Unmarshal.of_cstruct buf with @@ -62,7 +62,7 @@ let check_for_one_udp_packet on_received_one ~src ~dst buf = let send_forever sender receiver_address udp_message = let rec loop () = (* Check that we have an IP before sending *) - if (List.length (Ipv6.get_ip sender.ip)) >= 1 then + if List.length (Ipv6.get_ip sender.ip) >= 1 then begin Udp.write sender.udp ~dst:receiver_address ~dst_port:1234 udp_message >|= Rresult.R.get_ok