ARP module type; build ipv4 on top of arpv4 explicitly#400
ARP module type; build ipv4 on top of arpv4 explicitly#400yomimono wants to merge 8 commits intomirage:masterfrom yomimono:separate_arp
Conversation
In this design, ipv4 doesn't keep a record of the underlying ethif, clock, or time at all; this is stuffed into the arpv4 record (and impl). With this design, Arpv4 has to provide pass-through read and write so the Ipv4 implementation can access the underlying Ethif for its own reads and writes (since apparently there are network use cases besides resolving IPs to MACs??).
|
Shouldn't be merged without mirage/mirage-tcpip#134 or something like it. |
lib/mirage.ml
Outdated
There was a problem hiding this comment.
to answer the question in the comment: what you are defining here is how to configure an ARP implementation, ie. what parameters needs to be given to (i) the functor and (ii) the constructor function (ie, create or connect or ...). So the type-checker cannot really infer anything here.
There was a problem hiding this comment.
writing notes to yourself in potentially public places: cool, until it isn't.
There was a problem hiding this comment.
no worries, that's a legitimate question :-)
There was a problem hiding this comment.
there's a good opportunity here to improve the current situation by using string_error (or whatever here the name of the error printing function in V1.DEVICE) to pretty-print the error in case the configuration failed.
There was a problem hiding this comment.
Good point; I'll put in a commit for that shortly.
There was a problem hiding this comment.
...actually it doesn't look like we require any such thing in V1.DEVICE at the moment. V1.FLOW has such a val error_message: error -> string, but I don't see a similar requirement in any of the other module types.
There was a problem hiding this comment.
ha yes, you are right. sorry for the noise!
|
I'm still curious why/if we need to support multiple ip addresses within arp. shouldn't there be a single ip -- mac address mapping? also, I'd prefer a more pure interface without |
|
@hannesm - I'd like to keep the scope of this PR small, since it's apparently been enough of a challenge for me just to not break the build! To address your point, I think a nicer way to support multiple IP addresses on a given interface would be to call Arpv4.connect multiple times on the same Ethif.t, then call Ip.connect on the respective Arpv4.t's. It seems sensible to me for there to be separate caches per IP, even if they're on the same Ethif.t -- relatedly, I think a sensible ARP cache would disregard updates from unexpected networks. Ipv4 has the same issue with its IP-binding interface, and I think it would be nice to remove the claimed multiple network support there as well, in favour of the expectation that users will construct separate Ipv4.t's, possibly with the same Ethif.t. (As far as I know, there are currently no users of Ipv4/Arpv4's facility for multiple IP addresses in a single Ipv4.t or Arpv4.t .) |
That's my understanding as well. |
|
why why why? :-) |
|
ugh, sorry On 06/16/2015 05:22 PM, Thomas Gazagnaire wrote:
|
|
I'd like to clean up both this and the related PR in mirage-tcpip and resubmit soon, although I'll confess that the mess I just made in here was completely the result of carelessness :( |
mirage-types: Instead of hiding the ARP implementation within IPv4, split out a separate module type for ARP for modules (e.g., the ARP module in
mirage-tcpip) to implement.mirage: When building stacks, correctly build ipv4 on top of an ethernet impl and arpv4 impl.