mirage / mirage-vnetif

Virtual network interface and software bridge for Mirage
ISC License
16 stars 14 forks source link

Outdated examples and documentation #31

Open lattice0 opened 4 years ago

lattice0 commented 4 years ago

Take a look into the dune file of the ipv4 part of the code in mirage-tcpip:

(library
 (name tcpip_ipv4)
 (public_name tcpip.ipv4)
 (libraries logs mirage-protocols ipaddr cstruct rresult tcpip
   tcpip.udp mirage-random mirage-clock randomconv lru)
 (preprocess (pps ppx_cstruct))
 (wrapped false))

Its name is tcpip_ipv4, but since wrapped = false, the modules should be available by the filenames in the same folder as this dune file, rigth?

I see no ipv4.ml file. However, this code uses it like this:

module I = Ipv4.Make(E)(Clock)(OS.Time)

where did Ipv4 come from?

This and the answer gave for me in https://discuss.ocaml.org/t/understanding-how-to-import-a-dune-module/5660 led me to believe that there was an ipv4.ml file in the past and therefore this repo is outdated.

Another thing that led me believe this is this line:

    let ip, port = Stack.TCPV4.get_dest flow in

I can't find get_dest anywhere in Mirage_protocols, Mirage-net, neither in github code search, neither in google!

MagnusS commented 4 years ago

You are right - the library should be up to date, but the examples and README in this repo have not been updated to the latest version of MirageOS unfortunately. So the IP-stack now needs to be constructed differently, which mirage-tcpip is doing.

You can see how it is set up in https://github.com/mirage/mirage-tcpip/blob/master/test/vnetif_common.ml#L65 and used in https://github.com/mirage/mirage-tcpip/blob/master/test/test_connect.ml#L47. The test_connect test in mirage-tcpip should be very similar to the connect unikernel example here.

I have pushed an early updated version of the example in a branch here, which should at least fix some of the issues: https://github.com/MagnusS/mirage-vnetif/tree/fix-connect-example/examples/connect

I'm in the process of cleaning up the documentation and examples, but it may take some time to update everything. Happy to merge PRs with fixes or updates :-) I'll leave this issue open to track it.