Conversation
haesbaert
left a comment
haesbaert
left a comment
There was a problem hiding this comment.
I like this, thank you :)
I'm not keen on maintaining async stuff myself, so if you're ok with maintaining this async bits in the future I'd appreciate, otherwise it will end up rotting.
| let send_packet t buf = | ||
| let len = Cstruct.length buf in | ||
| let writer = Writer.create t.fd in | ||
| Writer.write_bytes ~pos:0 ~len writer (Cstruct.to_bytes ~len buf); |
There was a problem hiding this comment.
I don't know async at all, but is this the only way to do a write ? does it guarantee this is one system call ? These are all datagram-like file descriptors, meaning one write = one packet.
I guess this is ok but maybe there is a single_write and whatnot.
There was a problem hiding this comment.
I've looked it over and it does look like I need to use Writer.write_gen_whole to ensure we don't break up the write. Unfortunately that interface seems needlessly obtuse. I'll switch it to that function shortly.
There was a problem hiding this comment.
schedule_write does the correct thing, and now much more closely matches what lwt and eio are doing.
georgyo
force-pushed
the
async
branch
2 times, most recently
from
0e54b9c to
230ee73
Compare
|
That's great, I'll have a proper review in this and all the other PRs tomorrow, I'll test this during the weekend in my network home to see if nothing breaks thanks again for your work |
2 participants
This adds Async support, it has been tested by using these modules in the Churrua DHCP server, and works well.
Since Async depends on
Coreandppx_janealready, I used that in the rawcap_async test binary. It could just as easily useAsync.Deferred.run_in_async_waitinstead of the Command module to start the process. If there is desire to keeprawcap_asyncas minimal as possible.