Skip to content
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

question about retiring Random / PCLOCK / MCLOCK / Time interfaces and functors #1513

Open
hannesm opened this issue Mar 27, 2024 · 10 comments

Comments

@hannesm
Copy link
Member

hannesm commented Mar 27, 2024

Dear Madam or Sir,

I encountered the "mirage-random" opam package, and its usage. I am curious what you think about it?

My feeling is that the interface can just be removed from the dependency cone, and unikernels can use Mirage_crypto_rng directly. I don't see much value in the abstraction (the mirage_impl_random only provide mirage-crypto-rng anyways).

I even think that "functorizing over the RNG" is maybe not what we desire to do -- unless there's actual use of a different RNG (or plans to do so)? I guess that "RNG initialization" is still something the mirage utility should do (and here I've no clue whether there's another path than functorizing stuff over the RNG).

From my perspective, this originates from when we introduced nocrypto and entropy harvesting, but with the current state of mirage-crypto-rng (being independent of gmp/zarith), there's no need to abstract/functorize over it (when I repeat for myself that functors are great if and only if you have the requirement to have the same thing using two different modules in the same application). But from where I stand, there's really only ever a single RNG being used, and to reach the CSPRNG properties, our fortuna is the current choice (obviously, happy to get something different if there's new research in that area).

WDYT @mirage/core?

@hannesm
Copy link
Member Author

hannesm commented Mar 27, 2024

Maybe there's a wider question, and we should address this in the next major release. There are certain things a unikernel is functorized over that I'm rather sad about:

  • pclock (for a posix timestamp) (Mirage_clock.PCLOCK)
  • mclock (for a monotonic clock) (Mirage_clock.MCLOCK)
  • time (for "sleep_ns") (Mirage_time.S)
  • random (for generate) (Mirage_random.S)

These all don't have an internal state that you act on (something like TCP.t for creating connections) - but usually you use (R : Mirage_random.S) to use R.generate in your unikernel.

Now, what are alternatives? Do we actually care which modules use these functionality? Could the dune virtual library help us, and we simply use Mirage_clock.now () and get a Ptime.t (and mirage takes care which platform we're on?)? Would that work in a testing environment (see TCP tests) as well (i.e. provide a testing clock that runs faster?)? I'm still unsure about this whole testing, I personally think that unit tests (of the pure core) and end-to-end tests are worth, but these intermediate level tests lead to embedding lots of code into the Lwt monad and testing it -- which I'm not too convinced of.

I also remember years ago someone said "we'll just use first-class modules for these functors" (so far I haven't seen anything like that). Resource-usage-wise they aren't too interesting either. Using objects and some "environment" (eio-style) does not (from my perspective) improve the situation at all.

Certainly, there's need for initialization (esp. for the rng to start the entropy collection). There's as well potential use for pclock-synced-with-ntp (or whatever network time protocol you prefer - we've been discussing this since a decade without any NTP implementation...). But using the linking trick should allow an application (unikernel) to select the implementation.

So, below the line: what is your take on these interfaces and implementations? Is there any real reason to retain this complexity? Or is there desire from your point to move on (and make life easier for 99%)?

@hannesm hannesm changed the title question about retiring mirage-random question about retiring Random / PCLOCK / MCLOCK / Time interfaces and functors Mar 27, 2024
@leostera
Copy link

+1 to not relying on the functorized capabilities interface. This gets in the way of using Riot in a unikernel.

I'd rather see a configuration (or even runtime) flag for enabling capabilities and replacing/injecting capability modules.

@hannesm
Copy link
Member Author

hannesm commented Apr 23, 2024

Back on this topic, I'd be happy to hear your opinion:

Now, with this change, and a modified mirage-skeleton/device-usage/clock unikernel:

config.ml:

open Mirage

let main = main "Unikernel.Main" (time @-> job)

let () =
  register "speaking_clock"
    [ main $ default_time ]

unikernel.ml:

open Lwt.Infix

let log = Logs.Src.create "speaking clock" ~doc:"At the third stroke..."

module Log = (val Logs.src_log log : Logs.LOG)

module Main
    (Time : Mirage_time.S) =
struct
  let str_of_time (posix_time, timezone) =
    Format.asprintf "%a" (Ptime.pp_human ?tz_offset_s:timezone ()) posix_time

  let start _time =
    let rec speak () =
      let current_time = Mirage_clock.Pclock.now_d_ps () |> Ptime.v in
      let tz = Mirage_clock.Pclock.current_tz_offset_s () in
      let str =
        Printf.sprintf
          "%Lu nanoseconds have elapsed. \n\
          \ At the stroke, the time will be %s \x07 *BEEP*"
          (Mirage_clock.Mclock.elapsed_ns ())
        @@ str_of_time (current_time, tz)
      in
      Log.info (fun f -> f "%s" str);
      Time.sleep_ns 1_000_000_000L >>= fun () -> speak ()
    in
    speak ()
end

After manual modification of mirage-generated files (dune.build refer to mirage-clock.unix / mirage-clock.solo5), also main.ml removing the functor instantiation of Mirage_logs.Make, I get both unix and hvt unikernels that are nice.

It is not clear to me whether it is possible in mirage-clock to define the default_implementation depending on the "context". But we can just emit the mirage-clock.solo5 / mirage-clock.unix inside of mirage!?

Is this a valuable path forward? Since quite some packages depend on mclock/pclock, it would be great to first hear input before revising all these libraries and cutting releases.

@hannesm
Copy link
Member Author

hannesm commented Apr 23, 2024

After discussing with @leostera, maybe for clock we can use conditional compilation instead. I'll give it a try.

@hannesm
Copy link
Member Author

hannesm commented Apr 23, 2024

in a parallel universe, we are able to use "conditional compilation" (using some dune magic) instead of "virtual libraries"... pick your poison -- mine is the conditional compilation since I think it is the most straightforward.

so, have a look at mirage/mirage-clock#59 and mirage/mirage-logs#28 - unikernel code same as in above comment. I really like that solution since it doesn't use the variant magic, but is a mostly normal dune file.

I tested it again with the mirage-skeleton device-usage/clock unikernel. let me know (cc @mirage/core) about your opinion (and/or I'll push through and adapt these changes)... likely good to bundle with the time and random changes as discussed at the top.

@hannesm
Copy link
Member Author

hannesm commented Apr 23, 2024

so, the mirage related changes for the clock unikernel are #1521 -- which only modifies the mirage-logs (removing functor, and clock requirement).

@samoht
Copy link
Member

samoht commented Apr 24, 2024

A quick reply: I think I'm generally in favour of simplifying this part of the stack. Ideally, it would be great if we could find a solution where:

  • by default, nobody has to care about clocks (but the tool)
  • it's possible for advanced usages to continue to functorise over the clock signatures (so they need to be defined somewhere)
  • It's possible for advanced usages to refer directly to the relevant clock implementation (I don't think we ever want to have multiple implementations of those clocks in the same program - so it's probably OK if they have the same name)
  • maybe it's not relevant for clocks, but in general, we want to let users access target-specific "extensions" to the clock signature
  • not super important but valuable in some cases: it should be possible to mock the clock (either by linking with mirage-clock.mock or by exposing a set_current_clock or something like this) (the case I'm thinking about are outputs of mdx tests as seen in mirage-www for instance - right now those outputs are not deterministic at all so hard to test with mdx).

I think using select solves most of these requirements but would be nice to double check.

@dinosaure
Copy link
Member

so, have a look at mirage/mirage-clock#59 and mirage/mirage-logs#28 - unikernel code same as in above comment. I really like that solution since it doesn't use the variant magic, but is a mostly normal dune file.

I like the idea but I think the most problematic point, from Mirage's point of view, is the addition of a new target where a change has to be made to several projects. Wouldn't it be possible to use select in that the mirage tool generates in order to keep the centrality of the "driver" according to the target on the mirage tool?

@hannesm
Copy link
Member Author

hannesm commented Apr 24, 2024

User code is at mirage/mirage-skeleton#390 -- still not really happy with it (that we need to define the ~extra_deps which are then resulting in a unit argument to start).

The remaining PRs include:

I guess the mirage "extra_deps" (and some "connect" adjustments - why is in the default_connect even any code generated?) would be useful to specify "I depend on this thingy, but don't bother about values".

I also think it is useful to go ahead and take a look at mirage-net (and mirage-block), figuring further things out..

WDYT?

@palainp
Copy link
Member

palainp commented Apr 25, 2024

Thank you a lot for these PRs @hannesm. To me, although I really like the code of the skeleton/clock example, I think the 3 unit args for start could be saw as strange artifacts by a new user as they are no longer present in the config.ml file :(

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

No branches or pull requests

5 participants