[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [MirageOS-devel] Update on capnp-rpc



On 24 July 2017 at 11:22, Thomas Gazagnaire <thomas@xxxxxxxxxxxxxx> wrote:
>> 1. The fuzz tests found a race, where messages may get delivered out
>> of order in some (convoluted) cases.
>> I asked on the capnp list, and it seems that the C++ reference
>> implementation doesn't handle this quite right either.
>> We're investigating a fix, but it's unlikely you'll hit the problem.
>> See: https://github.com/mirage/capnp-rpc/issues/59
>
> Being able to run fuzz-testing on a complex RPC protocol is really great! The 
> bug that it founds look also quite involved :-)

Yep, I'm pretty happy with the fuzzing, though there are lots more
things it could be testing:

  https://github.com/mirage/capnp-rpc#fuzzing

I particularly like the way that when it hits a failing test, it
automatically writes out the code for an OCaml unit-test to reproduce
it (though the code usually needs some manual simplification and
comments).

>> 2. The API for building requests and responses has a lot of
>> boilerplate. I'd like to simplify this by modifying capnp-ocaml to
>> allow attaching capabilities to messages without requiring a separate
>> type.
>
> I have been using the library quite a lot recently and indeed there is a lot 
> of boiler-plate. Generating more code seems indeed sensible. 95% of the 
> client/code is type-driven so I am pretty sure we could auto-generate almost 
> everything with a few annotation to select the right calling convention for 
> instance (to switch between call_for_value and pipeline calls).
>
> My latest experiment is to generate client/server bindings for some MirageOS 
> signatures:
>
> https://github.com/linuxkit/linuxkit/blob/master/projects/miragesdk/src/sdk/proto.capnp

Very interesting!

struct Result {
  union {
    ok @0: Void;
    disconnected @1: Void;
    unimplemented @2: Void;
    error @3: Text;
  }
}

Note: Cap'n Proto's own exception type has tags for unimplemented
(unknown method) and generic errors (fail). We might want to reuse
that instead, e.g.

  Service.fail ~ty:`Unimplemented "Operation not supported"

Note that the RPC library will do this automatically for methods that
aren't in the schema. This is needed anyway because new methods might
get added later and old servers won't know about them.

You can also do "~ty:`Disconnected". That's intended to mean that a
call failed due to a network connection failing somewhere. It seems
reasonable to reuse that for some uses of the Mirage value. The other
Mirage meaning (you can't use this because you already called the
"disconnect" method) should perhaps be considered a bug, and therefore
handled via a "fail" exception rather than a result type.

write @1 (buffer: Data) -> Result;
writev @2 (buffers: List(Data)) -> Result;

I suspect the performance benefit of writev goes away when sending
things over the network. There might be a benefit to streaming the
Cap'n Proto message from multiple local buffers (internally it
represents a message as a list of segments anyway), but I don't see
any reason to represent it as a list object on the wire.

> and see the (verbose) client/server code for Mirage_flow_lwt:
>
> https://github.com/linuxkit/linuxkit/blob/master/projects/miragesdk/src/sdk/flow.ml

let connect ~switch ?tags f =
  let ep = Capnp_rpc_lwt.Endpoint.of_flow ~switch (module F) f in
  let client = Capnp_rpc_lwt.CapTP.connect ~switch ?tags ep in
  Capnp_rpc_lwt.CapTP.bootstrap client |> Lwt.return

We should probably keep the initial connection logic out of the
individual service types. A user application may want to use many
services, but will only want a single network connection to the
server. I've made some utility functions to make setting up the
network simpler. e.g. instead of calling this connect function, the
user can get a flow instance with:

  Capnp_rpc_unix.connect addr

You can replace:

  let resp, _ = Service.Response.create R.init_pointer in Ok resp

with

 Service.return_empty ()

Also, you can now remove:

let listen ~switch ?tags service fd =
  let endpoint = Capnp_rpc_lwt.Endpoint.of_flow ~switch (module F) fd in
  Capnp_rpc_lwt.CapTP.connect ~switch ?tags ~offer:service endpoint
  |> ignore

The main application can listen with the new:

  Capnp_rpc_unix.serve ~offer:service listen_addr

You'll only want to listen once, even when offering multiple services
(offer a bootstrap service that provides access to the others).

>> 3. There are some (minor) Unix dependencies in the library and in
>> capnp-ocaml, which need moving out so we can use it in unikernels.
>>
>> 4. Start on level 2 support, which will require the library to be able
>> to establish secure connections. Currently, it doesn't do any crypto
>> and so the helpers only allow connections over Unix-domain sockets for
>> now.
>>
>> I'd be interested in feedback from anyone who wants to try it!
>
> Thanks for doing that work, I really like 1st class support for passing 
> around functions. It fits very well with turning a random MirageOS signature 
> into an RPC boundary. I really would like to use this to define MirageOS 
> groups of unikernels coordinating via secure RPC over the MirageOS signatures 
> -- I have plans to extend the mirage tool to do this (or at least try) at one 
> point :-)
>
> Best,
> Thomas


-- 
talex5 (GitHub/Twitter)        http://roscidus.com/blog/
GPG: 5DD5 8D70 899C 454A 966D  6A51 7513 3C8F 94F6 E0CC

_______________________________________________
MirageOS-devel mailing list
MirageOS-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/cgi-bin/mailman/listinfo/mirageos-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.