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

unix: add Cstruct_unix.{read,write,writev,send,recv} #302

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions unix/dune
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,8 @@
(name cstruct_unix)
(wrapped false)
(public_name cstruct-unix)
(foreign_stubs
(language c)
(names read_stubs write_stubs writev_stubs send_stubs recv_stubs
recvfrom_stubs sendto_stubs))
(libraries cstruct unix))
70 changes: 70 additions & 0 deletions unix/read_stubs.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
#include <caml/mlvalues.h>
#include <caml/memory.h>
#include <caml/custom.h>
#include <caml/callback.h>
#include <caml/alloc.h>
#include <caml/unixsupport.h>
#include <caml/bigarray.h>
#include <caml/threads.h>

#include <stdio.h>
#include <errno.h>

CAMLprim value stub_cstruct_read(value val_fd, value val_c)
dinosaure marked this conversation as resolved.
Show resolved Hide resolved
{
CAMLparam2(val_fd, val_c);
CAMLlocal3(val_buf, val_ofs, val_len);

val_buf = Field(val_c, 0);
val_ofs = Field(val_c, 1);
val_len = Field(val_c, 2);

void *buf = (char *)Caml_ba_data_val(val_buf) + Long_val(val_ofs);
size_t len = Long_val(val_len);
int n = 0;

#ifdef _WIN32
int win32err = 0;
switch (Descr_kind_val(val_fd))
{
case KIND_SOCKET:
SOCKET s = Socket_val(val_fd);

caml_release_runtime_system();
n = recv(s, buf, len, 0);
win32err = WSAGetLastError();
caml_acquire_runtime_system();

if (n == SOCKET_ERROR)
{
win32_maperr(win32err);
unix_error(errno, "read", Nothing);
}
break;
case KIND_HANDLE:
HANDLE h = Handle_val(val_fd);
DWORD numread;
caml_release_runtime_system();
int ok = ReadFile(h, buf, len, &numread, NULL);
win32err = GetLastError();
n = numread;
caml_acquire_runtime_system();

if (!ok)
{
win32_maperr(win32err);
unix_error(errno, "read", Nothing);
}
break;
default:
caml_failwith("unknown Descr_kind_val");
}
#else
caml_release_runtime_system();
n = read(Int_val(val_fd), buf, len);
caml_acquire_runtime_system();
if (n < 0)
unix_error(errno, "read", Nothing);
#endif
CAMLreturn(Val_int(n));
}
63 changes: 63 additions & 0 deletions unix/recv_stubs.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
#include <caml/mlvalues.h>
#include <caml/memory.h>
#include <caml/custom.h>
#include <caml/callback.h>
#include <caml/alloc.h>
#include <caml/unixsupport.h>
#include <caml/bigarray.h>
#include <caml/threads.h>

#include <stdio.h>

#ifdef WIN32
#define WIN32_LEAN_AND_MEAN
#include <winsock2.h>
#include <ws2tcpip.h>
#include <NTSecAPI.h>
#else
#include <sys/socket.h>
#include <netinet/in.h>
#include <errno.h>
#endif

CAMLprim value stub_cstruct_recv(value val_fd, value val_c)
{
CAMLparam2(val_fd, val_c);
CAMLlocal3(val_buf, val_ofs, val_len);

val_buf = Field(val_c, 0);
val_ofs = Field(val_c, 1);
val_len = Field(val_c, 2);

void *buf = (void *)Caml_ba_data_val(val_buf) + Long_val(val_ofs);
Copy link
Member

@haesbaert haesbaert Oct 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor bikeshedding, would move buf to the beginning of the block, declarations after statement is not valid C89 (not sure if anyone cares these days)
Also void * arithmetic is technically undefined behaviour (also not sure if anyone cares).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think void * is used here to be in-sync with what recv expects:

ssize_t recv(int sockfd, void *buf, size_t len, int flags);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack but passing pointer of any type to something that expects a void pointer is ok, my only bit was that we're doing (void *)++, which is undefined behavior (but every compiler implicitly assumes char *) so it's ok.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://stackoverflow.com/questions/3922958/void-arithmetic says it's a GCC extension, and attempting this should produce warnings.

size_t len = (size_t)Long_val(val_len);
int n = 0;
#ifdef WIN32
int win32err = 0;
if (Descr_kind_val(val_fd) != KIND_SOCKET)
unix_error(EINVAL, "stub_cstruct_recv", Nothing);

SOCKET s = Socket_val(val_fd);

caml_release_runtime_system();
n = recv(s, buf, len, 0);
win32err = WSAGetLastError();
caml_acquire_runtime_system();

if (n == SOCKET_ERROR)
{
win32_maperr(win32err);
unix_error(errno, "recv", Nothing);
}
#else
int fd = Int_val(val_fd);

caml_release_runtime_system();
n = recv(fd, buf, len, 0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure a recv with null flags is very useful, it becomes just read(2), (don't know about windows though).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I’ve copied the flags code from your stubs :)

caml_acquire_runtime_system();

if (n < 0)
unix_error(errno, "recv", Nothing);
#endif
CAMLreturn(Val_int(n));
}
50 changes: 50 additions & 0 deletions unix/recvfrom_stubs.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
#include <caml/mlvalues.h>
#include <caml/memory.h>
#include <caml/custom.h>
#include <caml/callback.h>
#include <caml/alloc.h>
#include <caml/unixsupport.h>
#include <caml/bigarray.h>
#include <caml/threads.h>
#include <caml/socketaddr.h>

#include <sys/socket.h>

static int msg_flag_table[] = {
MSG_OOB, MSG_DONTROUTE, MSG_PEEK /* XXX */
};

CAMLprim value stub_cstruct_recvfrom(value val_fd, value val_c, value val_flags)
{
CAMLparam3(val_fd, val_c, val_flags);
CAMLlocal5(val_buf, val_ofs, val_len, val_addr, val_res);
uint8_t *buf;
size_t len;
ssize_t n;
int cv_flags;
union sock_addr_union addr;
socklen_param_type addr_len;

val_buf = Field(val_c, 0);
val_ofs = Field(val_c, 1);
val_len = Field(val_c, 2);
cv_flags = caml_convert_flag_list(val_flags, msg_flag_table);

buf = (uint8_t *)Caml_ba_data_val(val_buf) + Long_val(val_ofs);
len = Long_val(val_len);
addr_len = sizeof(addr);

caml_release_runtime_system();
n = recvfrom(Int_val(val_fd), buf, len, cv_flags, &addr.s_gen, &addr_len);
caml_acquire_runtime_system();

if (n == -1)
caml_uerror("recvfrom", Nothing);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
caml_uerror("recvfrom", Nothing);
caml_uerror("cstruct_recvfrom", Nothing);

I do find these Unix_errors are marginally more useful if you can grep for the function they came from


val_addr = caml_unix_alloc_sockaddr(&addr, addr_len, -1);
val_res = caml_alloc_small(2, 0);
Field(val_res, 0) = Val_int(n);
Field(val_res, 1) = val_addr;

CAMLreturn (val_res);
}
62 changes: 62 additions & 0 deletions unix/send_stubs.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
#include <caml/mlvalues.h>
#include <caml/memory.h>
#include <caml/custom.h>
#include <caml/callback.h>
#include <caml/alloc.h>
#include <caml/unixsupport.h>
#include <caml/bigarray.h>
#include <caml/threads.h>

#include <stdio.h>

#ifdef WIN32
#define WIN32_LEAN_AND_MEAN
#include <winsock2.h>
#include <ws2tcpip.h>
#include <NTSecAPI.h>
#else
#include <sys/socket.h>
#include <netinet/in.h>
#include <errno.h>
#endif

CAMLprim value stub_cstruct_send(value val_fd, value val_c)
{
CAMLparam2(val_fd, val_c);
CAMLlocal3(val_buf, val_ofs, val_len);

val_buf = Field(val_c, 0);
val_ofs = Field(val_c, 1);
val_len = Field(val_c, 2);

const char *buf = (char *)Caml_ba_data_val(val_buf) + Long_val(val_ofs);
size_t len = (size_t)Long_val(val_len);
int n = 0;

#ifdef WIN32
int win32err = 0;
if (Descr_kind_val(val_fd) != KIND_SOCKET)
unix_error(EINVAL, "send", Nothing);

SOCKET s = Socket_val(val_fd);
caml_release_runtime_system();
n = send(s, buf, len, 0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as recv

win32err = WSAGetLastError();
caml_acquire_runtime_system();

if (n == SOCKET_ERROR)
{
win32_maperr(win32err);
unix_error(errno, "send", Nothing);
}
#else
int fd = Int_val(val_fd);

caml_release_runtime_system();
n = send(fd, buf, len, 0);
caml_acquire_runtime_system();
if (n < 0)
unix_error(errno, "send", Nothing);
#endif
CAMLreturn(Val_int(n));
}
45 changes: 45 additions & 0 deletions unix/sendto_stubs.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
#include <caml/mlvalues.h>
#include <caml/memory.h>
#include <caml/custom.h>
#include <caml/callback.h>
#include <caml/alloc.h>
#include <caml/unixsupport.h>
#include <caml/bigarray.h>
#include <caml/threads.h>
#include <caml/socketaddr.h>

#include <sys/socket.h>

static int msg_flag_table[] = { /* XXX */
MSG_OOB, MSG_DONTROUTE, MSG_PEEK
};

CAMLprim value stub_cstruct_sendto(value val_fd, value val_c, value val_flags, value val_daddr)
{
CAMLparam4(val_fd, val_c, val_flags, val_daddr);
CAMLlocal5(val_buf, val_ofs, val_len, val_addr, val_res);
union sock_addr_union addr;
socklen_param_type addr_len;
uint8_t *buf;
size_t len;
ssize_t n;
int cv_flags;

val_buf = Field(val_c, 0);
val_ofs = Field(val_c, 1);
val_len = Field(val_c, 2);

buf = (uint8_t *)Caml_ba_data_val(val_buf) + Long_val(val_ofs);
len = Long_val(val_len);
caml_unix_get_sockaddr(val_daddr, &addr, &addr_len);
cv_flags = caml_convert_flag_list(val_flags, msg_flag_table);

caml_release_runtime_system();
n = sendto(Int_val(val_fd), buf, len, cv_flags, &addr.s_gen, addr_len);
caml_acquire_runtime_system();

if (n == -1)
caml_uerror("sendto", Nothing);

CAMLreturn (Val_int(n));
}
65 changes: 65 additions & 0 deletions unix/unix_cstruct.ml
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,68 @@
let of_fd fd =
let buffer = Bigarray.(array1_of_genarray (Unix.map_file fd char c_layout false [|-1|])) in
Cstruct.of_bigarray buffer

type buffer = (char, Bigarray.int8_unsigned_elt, Bigarray.c_layout) Bigarray.Array1.t
dinosaure marked this conversation as resolved.
Show resolved Hide resolved

(* Returns 0 if there is no writev *)
external stub_iov_max: unit -> int = "stub_cstruct_iov_max"

external stub_write: Unix.file_descr -> (buffer * int * int) -> int = "stub_cstruct_write"

external stub_writev: Unix.file_descr -> (buffer * int * int) list -> int = "stub_cstruct_writev"

let iov_max = stub_iov_max ()

(* return the first n fragments, suitable for writev *)
let rec first n rev_acc = function
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mildly dislike having to do these potentially O(n) list operations for a writev, and mandate that in the external interface. In your vpnkit usecase, would you be able to pass in a Cstruct.t array instead for writev, and then just call writev with simple O(1) time slices instead?

It affects the interface we expose in the mli, hence my asking now (as opposed to optimising later)

| [] -> List.rev rev_acc
| _ when n = 0 -> List.rev rev_acc
| x :: xs -> first (n - 1) (x :: rev_acc) xs

(* shift a list of fragments along by n bytes *)
let rec shift t x =
if x = 0 then t else match t with
| [] -> invalid_arg "foo"
| y :: ys ->
let y' = Cstruct.length y in
if y' > x
then Cstruct.shift y x :: ys
else shift ys (x - y')

let rec write fd buf =
if Cstruct.length buf > 0 then begin
let n = stub_write fd (buf.Cstruct.buffer, buf.Cstruct.off, buf.Cstruct.len) in
write fd @@ Cstruct.shift buf n
end

let writev fd bufs =
let rec use_writev = function
| [] -> ()
| remaining ->
(* write at most iov_max at a time *)
let to_send = first iov_max [] remaining in
let n = stub_writev fd (List.map (fun x -> x.Cstruct.buffer, x.Cstruct.off, x.Cstruct.len) to_send) in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you expand to a tuple here because the Cstruct.t record type might change? Given this is in the same repository, it's probably safe enough to just pass the Cstruct record in directly to the FFI and save some allocations

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that was it -- I was using these stubs in another repo. I'll drop the tuple since we're here.

let rest = shift remaining n in
use_writev rest in
let use_write_fallback = List.iter (write fd) in
(if iov_max = 0 then use_write_fallback else use_writev) bufs

external stub_send: Unix.file_descr -> (buffer * int * int) -> int = "stub_cstruct_send"

external stub_recv: Unix.file_descr -> (buffer * int * int) -> int = "stub_cstruct_recv"

let send fd x = stub_send fd (x.Cstruct.buffer, x.Cstruct.off, x.Cstruct.len)

let recv fd x = stub_recv fd (x.Cstruct.buffer, x.Cstruct.off, x.Cstruct.len)

external stub_read: Unix.file_descr -> (buffer * int * int) -> int = "stub_cstruct_read"

let read fd x = stub_read fd (x.Cstruct.buffer, x.Cstruct.off, x.Cstruct.len)

external stub_recvfrom : Unix.file_descr -> Cstruct.t -> Unix.msg_flag list -> int * Unix.sockaddr = "stub_cstruct_recvfrom"

let recvfrom fd x fl = stub_recvfrom fd x fl

external stub_sendto : Unix.file_descr -> Cstruct.t -> Unix.msg_flag list -> Unix.sockaddr -> int = "stub_cstruct_sendto"

let sendto fd x fl a = stub_sendto fd x fl a
Loading