search

mirage / ocaml-dns

OCaml implementation of the DNS protocol
BSD 2-Clause "Simplified" License
106 stars 43 forks source link

`Transport.send_recv` out of order delivery #345

Closed RyanGibb closed 1 year ago

RyanGibb commented 1 year ago

Hi,

I noticed that Transport.send_recv is used in Dns_client, which seems to take the first received packet after sending a packet as the response, and I was wondering if you've considered how out of order delivery could effect this when making concurrent requests?

Thanks, -- Ryan

reynir commented 1 year ago

Dear Ryan,

DNS messages contain an ID to match replies with queries. In Dns_client_lwt this is used to match incoming replies with queries sent out. The reply is bubbled back through a Lwt_condition.t stored in a map:

https://github.com/mirage/ocaml-dns/blob/49b821f5faed6e7705a43f0f66468b42ceeb58b3/lwt/client/dns_client_lwt.ml#L296-L313

This line extracts the ID of the TCP DNS query message.

https://github.com/mirage/ocaml-dns/blob/49b821f5faed6e7705a43f0f66468b42ceeb58b3/lwt/client/dns_client_lwt.ml#L301

For Dns_client_unix a query is sent out and the call blocks until a reply is sent back. Concurrent requests are not expected, and I believe this is why send_recv in Dns_client_unix doesn't try to do any query ID / reply ID matching.

The documentation could be clearer about this - what do you think?

RyanGibb commented 1 year ago

Dear Reynir,

Ah yes, I missed that for the lwt and mirage clients. That makes sense! Thanks for pointing it out.

Perhaps the documentation could be a bit clearer. It's not completely apparent that the decision about what behavior this function exhibits differs between implementations.