New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

being able to choose arp implementation #750

Merged
merged 4 commits into from Dec 27, 2016

Conversation

Projects
None yet
2 participants
@hannesm
Member

hannesm commented Dec 25, 2016

PLEASE only look at the commit 74a751c (PR on top of #749)

This enables someone to use a slightly different ARP implementation (named arp') without any hashtables, and with other timeout and retry values. The source of this arp library is https://github.com/hannesm/arp

The common functions to construct a stack, dhcp_ipv4_stack, static_ipv4_stack, and qubes_ipv4_stack now have an ?arp:(?clock -> ?time -> ethernet -> arp) argument, and a new arp' value is constructed in mirage.ml.

This again shows to me how hard it is to use mirage in a modular way -- both tcpip.arpv4 and arp.mirage have the very same interface, but nevertheless we need to specify two devices, and in order to make it usable these devices need to live in the mirage cli. This means that the mirage cli has global (closed) knowledge over all devices, and using custom bits and pieces is hard (without modifying mirage.ml). Earlier I used to modify main.ml, but this is very painful if you run a handfull of unikernels.

Another issue, while being here, is optional arguments to the mirage functions: not all respect all, thus deciding globally on a specific arp/udp/time/clock is rather hard. I suspect we should follow the random approach: single key, and expose a default_random inside of mirage, used by all the implementations (or is there demand to use a different clock implementation for your different pieces of the stack (X.509 / TCP)? I'd suspect that not, well, at least I haven't come across any use case.

I currently use this branch for my unikernels (nqsb.io/ns.nqsb.io/btc-pinata/...) (was tired of way too many ARP timeout log messages).

hannesm added some commits Dec 23, 2016

syslog support
in config.ml, configure your syslog server, using your own hostname,
an ipv4 address, possibly a port and truncation.

choose the desired protocol (UDP, TCP, TLS)
@samoht

samoht requested changes Dec 26, 2016 edited

Please revert 11df4d9.

@@ -267,6 +267,9 @@ val arpv4 : arpv4 typ
val arp : ?clock:mclock impl -> ?time:time impl -> ethernet impl -> arpv4 impl
(** Default ARPv4 implementation provided by the tcpip library *)
val arp' : ?clock:mclock impl -> ?time:time impl -> ethernet impl -> arpv4 impl

This comment has been minimized.

@samoht

samoht Dec 26, 2016

Member

I am really not fond of '.

(** Build a stackv4 by looking up configuration information via QubesDB,
* building an ipv4, then building a stack on top of that. *)
val qubes_ipv4_stack : ?group:string -> ?qubesdb:qubesdb impl -> network impl -> stackv4 impl
val qubes_ipv4_stack : ?group:string -> ?qubesdb:qubesdb impl -> ?arp:arp_sig -> network impl -> stackv4 impl

This comment has been minimized.

@samoht

samoht Dec 26, 2016

Member

why this isn't ?arp:(ethernet impl -> arpv4 impl)? Does the stack really care about your arp device needing a clock/time device?

This comment has been minimized.

@hannesm

hannesm Dec 26, 2016

Member

this is related to #743 (comment) - I personally don't think there is much value in clock/time, but I'd want to use only a single time&clock in a unikernel. if we provide ?clock for some combinators, but use default_clock hardcoded in others, this does not allow to use a custom clock.

the other reason is that ocaml complained that the signatures don't match (I suspect I've to partially apply the functions with clock and time being None).

This comment has been minimized.

@samoht

samoht Dec 26, 2016

Member

you can let people build their own arp implementation (using the clock they want), I don't think we need to thread that through all the arguments.

This comment has been minimized.

@hannesm

hannesm Dec 26, 2016

Member

ok. done in a455761

@samoht

samoht approved these changes Dec 26, 2016

netmasks : Ipaddr.V6.Prefix.t list;
gateways : Ipaddr.V6.t list;
netmasks : Ipaddr.V6.Prefix.t list;
gateways : Ipaddr.V6.t list;

This comment has been minimized.

@samoht

samoht Dec 26, 2016

Member

not sure why these are needed

@samoht

This comment has been minimized.

Member

samoht commented Dec 26, 2016

LGTM, thanks for the changes.

@hannesm hannesm merged commit a6d678c into mirage:master Dec 27, 2016

1 of 6 checks passed

ci/datakit/1 Build Command "sh" "-c" "docker build --label com.docker.datakit.digest=a3b44a4b6211df67097a9e981e85f434 --label com.docker.datakit.builton=148278
Details
ci/datakit/2 Revdeps depfail
Details
ci/datakit/3 Compilers depfail
Details
ci/datakit/4 Common Distros depfail
Details
ci/datakit/5 All Distros depfail
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@hannesm hannesm deleted the hannesm:myarp branch Dec 27, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment