Design for sendmsg/recvmsg

[infiniteregrets]

Through this discussion, I propose adding hooks for sendmsg and recvmsg system calls.

The main reason why we need to hook these system calls is because of the use case highlighted in #969. The error encountered looked like so:

sh-3.2$ dig py-serv
/AppleInternal/Library/BuildRoots/66382bca-8bca-11ec-aade-6613bcf0e2ee/Library/Caches/com.apple.xbs/Sources/bind9/bind9/lib/isc/unix/net.c:601: probing sendmsg() with IPV6_TCLASS=b8 failed: No route to host

And on further exploration it was found that sendmsg was called, which we don't hook.
https://github.com/apple-open-source/macos/blob/2494a13fb49fdc15996d7297069bf84d42b30d3c/bind9/bind9/lib/isc/unix/net.c#L575

In terms of the design of how two hooks should be implemented, I have the following suggestion:

Considerations

• when sendmsg is called, there is an implicit bind on the socket for the given address
  consider the case of a udp client:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>

int main() {
    int udp_socket = socket(AF_INET, SOCK_DGRAM, 0);
    if (udp_socket < 0) {
        printf("Error creating socket\n");
        return 1;
    }

    struct sockaddr_in server_addr;
    memset(&server_addr, 0, sizeof(server_addr));
    server_addr.sin_family = AF_INET;
    server_addr.sin_port = htons(8090);
    inet_pton(AF_INET, "127.0.0.1", &server_addr.sin_addr);

    char message[] = "Hello World!";
    
    struct msghdr msg;
    memset(&msg, 0, sizeof(msg));
    msg.msg_name = &server_addr;
    msg.msg_namelen = sizeof(server_addr);

    struct iovec iov;
    iov.iov_base = message;
    iov.iov_len = sizeof(message);
    msg.msg_iov = &iov;
    msg.msg_iovlen = 1;

    int bytes_sent = sendmsg(udp_socket, &msg, 0);
    if (bytes_sent < 0) {
        printf("Error sending message\n");
        return 1;
    }
    
    char buffer[1024];

    memset(buffer, 0, sizeof(buffer));

    struct sockaddr_in client_addr;
    socklen_t client_addr_len = sizeof(client_addr);
    struct msghdr recv_msg;
    memset(&recv_msg, 0, sizeof(recv_msg));
    recv_msg.msg_name = &client_addr;
    recv_msg.msg_namelen = sizeof(client_addr);

    struct iovec recv_iov;
    recv_iov.iov_base = buffer;
    recv_iov.iov_len = sizeof(buffer);
    recv_msg.msg_iov = &recv_iov;
    recv_msg.msg_iovlen = 1;

    int bytes_received = recvmsg(udp_socket, &recv_msg, 0);
    if (bytes_received < 0) {
        printf("Error receiving message\n");
        return 1;
    }

    printf("Received message: %s\n", buffer);
    
    close(udp_socket);

    return 0;
}

the address is provided to the msg_name is sendmsg but when recvmsg is called, it is implicitly bound to the address on which sendmsg was called.

• consider the case of an explicit bind in case of a udp server

#include <stdio.h>
#include <string.h>
#include <sys/socket.h>
#include <arpa/inet.h>
#include <unistd.h>

int main(int argc, char *argv[])
{
    int sockfd;
    struct sockaddr_in serv_addr, cli_addr;
    struct iovec iov[1];
    struct msghdr message;
    struct msghdr message1;
    char buffer[1024];
    int bytes_received;
    socklen_t cli_len;
    
    sockfd = socket(AF_INET, SOCK_DGRAM, 0);
    if (sockfd < 0) {
        perror("Error creating socket");
        return 1;
    }

    memset(&serv_addr, 0, sizeof(serv_addr));
    serv_addr.sin_family = AF_INET;
    serv_addr.sin_port = htons(8090);
    serv_addr.sin_addr.s_addr = htonl(INADDR_ANY);
    
    if (bind(sockfd, (struct sockaddr *) &serv_addr, sizeof(serv_addr)) < 0) {
        perror("Error binding socket to address");
        return 1;
    }

    for (;;) {        
        cli_len = sizeof(cli_addr);
        iov[0].iov_base = buffer;
        iov[0].iov_len = sizeof(buffer);
        message.msg_name = &cli_addr;
        message.msg_namelen = cli_len;
        message.msg_iov = iov;
        message.msg_iovlen = 1;
        message.msg_control = NULL;
        message.msg_controllen = 0;
        bytes_received = recvmsg(sockfd, &message, 0);
        if (bytes_received < 0) {
            perror("Error receiving message");
            return 1;
        }
        printf("Received %d bytes from %s: %.*s\n", bytes_received,
               inet_ntoa(cli_addr.sin_addr), bytes_received, buffer);
               

        if (sendmsg(sockfd, &message, 0) < 0) {
            perror("Error sending response");
            return 1;
        }
    }
    
    close(sockfd);

    return 0;
}

Here messages are received only on the bound address, providing any address to cli_addr does not make a difference and is ignored.
It seems that the address is filled by the call, as can be seen by adding the following line

printf("Received message from addr: %s, port: %d", inet_ntoa(cli_addr.sin_addr), ntohs(cli_addr.sin_port));

Design

sendmsg

• From our open sockets find the socket with which sendmsg is called
• Check the state of the socket, if it is bound let the agent know.
• The message to the agent should look like this:

SendMsgHook {
                        message,
                        addr,
                        bound: Some(BoundAddress {
                            requested_port: bound.requested_port,
                            address: bound.address,
                        }),
                        hook_channel_tx,
                    }

• In the agent, if BoundAddress is Some, bind the udp socket to that address, meaning an explicit bind, otherwise it is implicit bind, bind it to the addr
• call tokio's send_to and return back number of bytes sent

DISCUSS: how to keep some kind of mapping must be maintained between the layer and the agent for these sockets just like how we do for files

recvmsg

• let the agent which socket is being referred to
• in the agent, through our mapping call recv_from and return back the address and the len of bytes written
• In the layer fill the message header struct

Im maintaining a miro design diagram for this, ill post a design once there is final verdict on design from this

[meowjesty]

So, the main issue here is that, as we don't create a socket in the agent when the layer does a libc::socket call, the agent won't know about this socket until an actual networking operation is executed (sendmsg, connect, etc...).

Your design looks good to me, we're going to end up with something like the agent's FileManager, but for sockets, right?

Some general ideas, but again, what you have looks like it would work just fine.

• sendmsg

bind it to the addr

You mean create an UdpSocket with UdpSocket::bind(UNSPECIFIED)?

• recvmsg

This one is starting to tell us that maybe it would be nice to start mimicking libc calls more, I mean, when a user calls socket(...) we should tell the agent that a socket was created, and feed an agent generated fd to the user.

But you can get away right now with just creating a new socket in the agent when recvmsg hook is triggered (like we don for connect, etc).

• Non-blocking sockets:

How are you going to deal with non-blocking sockets?

If you use the "send ClientMessage to agent, and await for a response in the sendmsg hook" approach (similar to how we do in file ops), then you're going to be blocking until the agent-side operation completes. So maybe you need to use try_send_to instead? (same applies for the receiving side)

This depends on how reliable we want to have this be (and also means we need to handle select).

I would go for your proposal, of using send_to (and recv_from).

[aviramha]

I didn't have enough time to write all of my thoughts; but:

Your design looks good to me, we're going to end up with something like the agent's FileManager, but for sockets, right?

I think that it should leverage the existing socket mechanism in the agent, though for layer it would look different.
We can see a first recvmsg/sendto as the entrypoint for where we create the socket then hold it from there, just having different messages coming back

[aviramha]

P.S The select point might be really painful.

[meowjesty]

Yeah, if/when we ever hook select we would need to make some pretty big changes (or maybe skip our hooked select for some fds).

[aviramha]

Here's my proposal (which builds on top of the great research and great suggestions):

1. Use the existing SOCKETS hashmap.
2. if first operation on socket is recvmsg -> bind using the given address of that recvmsg
   if it's sendmsg -> bind using random address, and send data
3. on agent, once socket is bound, create a task that just reads from that socket using recvfrom. Each read should be sent to the layer asynchronously.
4. once layer receives that message, it sends the data itself to the local bound socket (like we do with bound udp/tcp).
5. on recvmsg, call the original one (if already bound and "listening" on the remote). once data is retrieved, change the source address to match the one we have in deque for that socket and message - maybe we can do a better data structure mechanism here - open for thoughts. the idea is that recvmsg uses the real socket, but we only alter the "recv address". this is so select/epoll/etc will work out of the box.

[infiniteregrets]

I found that the address was being ignored when provided to recvmsg, i.e. when not bound and whenrecvmsg is provided an address nothing happened on sending data via the udp client to the address provided to recvmsg

yes, the server calls recvmsg,

• i put a different addr for the header passed to recvmsg than the bound address, traffic was only received on the bound addr.
• i did not bind the addr, but passed the addr to the header for recvmsg, nothing happened, both client and server stuck

The address provided in the header for sendmsg is the destination address.

if sendmsg is called first, then I believe there is an implicit bind since when recvmsg is called it receives on the addr the sendmsg was called with

[aviramha]

i did not bind the addr, but passed the addr to the header for recvmsg, nothing happened, both client and server stuck

Can you provide code example and how you conducted that test?

[infiniteregrets]

i did not bind the addr, but passed the addr to the header for recvmsg, nothing happened, both client and server stuck

Can you provide code example and how you conducted that test?

server: https://gist.github.com/infiniteregrets/19305d2c6e258a25ddc91792cc5e558a

client: https://gist.github.com/infiniteregrets/7ec354e92254e630c30bc4d77379e6fe

ps, original example is in the post

[aviramha]

interesting, so we can drop the recvmsg causing a bind - it's interesting that it doesn't error..

[aviramha]

ah my bad, recvmsg msghdr is out parameter! goti t.