Skip to content

Commit

Permalink
Fix MTU enforced test backend
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
MagnusS committed Aug 25, 2020
1 parent de10113 commit 3b91e2e
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 17 deletions.
2 changes: 1 addition & 1 deletion test/test.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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 ;
]

Expand Down
13 changes: 8 additions & 5 deletions test/test_iperf.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion test/test_ipv6.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions test/test_mtus.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 =
Expand Down
42 changes: 34 additions & 8 deletions test/vnetif_backends.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 3b91e2e

Please sign in to comment.