servalproject/serval-dna — pull request #39 by rom1v (2012-11-07 01:40:29+00:00)
Title: MDP sockets support

It is not possible to use several "MDP sockets" with the API: there are no "sockets" exposed (in fact, there is only one per app, as `mdp_client_socket` is an external variable in `mdp_client.c`).

This is a problem, because the `overlay_mdp_recv()` function just retrieve the "first" packet, whatever it is. If it is not the right port, it is ignored (but the thread which listened to that port will never receive it, it is dropped).

Therefore, there is no way to "multiplex" services. If a service wants to listen on port *x* and another on port *y*, some packets from *x* will be "received" by the recv function of *y* (which will just drop it, but in that case this packet is lost for no reason and the recv() function considers it has received a packet which is invalid).

Moreover, functions like `overlay_mdp_getmyaddr()` or `overlay_mdp_bind()` do call `overlay_mdp_send()`, which can `call overlay_mdp_recv()`. The received response could be one packet from another device, which has nothing to do with the request, so it will be dropped. The easiest way to see the problem is to send a packet to yourself (to your own SID), and follow what happens in debug mode.

To fix it, I changed a bit the MDP API and implementation for providing "MDP sockets" support (commit f55e2b7):

int overlay_mdp_client_socket(void);
int overlay_mdp_client_close(int mdp_sockfd);
int overlay_mdp_client_poll(int mdp_sockfd, time_ms_t timeout_ms);
int overlay_mdp_recv(int mdp_sockfd, overlay_mdp_frame *mdp, int port, int *ttl);
int overlay_mdp_send(int mdp_sockfd, overlay_mdp_frame *mdp,int flags,int timeout_ms);

`overlay_mdp_client_init()` is replaced by `overlay_mdp_client_socket(void)`. Instead of returning 0 or -1, it returns the socket descriptor or -1.
`overlay_mdp_client_done()` is replaced by `overlay_mdp_client_close(int mdp_sockfd)`.

Each function now takes a parameter `mdp_sockfd`. This `mdp_sockfd` is created by `overlay_mdp_client_socket()`. There is no global `mdp_client_socket` anymore in `mdp_client.c`: instead, each service creates and uses its own MDP socket.

Then, I adapted usage in `commandline.c`.

Before that, I needed to make a tiny change: make `overlay_mdp_client_socket_path` local to `overlay_mdp_client_init()` (now renamed `overlay_mdp_client_socket()` for consistency with socket API). It was easy, because the unlink in `overlay_mdp_client_done()` was useless (it was already done in `overlay_mdp_client_init()`). (commit 263232b)

Quite independently, for making it work, I also needed to make `overlay_mdp_recv()` blocking. (commit 9d44386)

One remark: In `commandline.c`, in function:

int overlay_mdp_client_poll(time_ms_t timeout_ms)

There was:

ret=select(mdp_client_socket+1,&r,NULL,&r,&tv);
ret=select(mdp_client_socket+1,&r,NULL,&r,NULL);

What is this `+1` ?

I let it, but in my opinion this is an error.

What I tested after these changes:
- send data from one device to another (worked before with blocking recv);
- send data to myself (my own sid) (didn't work before);
- mdp ping in commandline (worked before).

What I did *not* tested:
- other command lines functions in `commandline.c`, because I don't know them (but I applied the same pattern for using `mdp_sockfd`).

Please give me any feedback.

----
Tip: for some functions, you should use `git diff -w` for ignoring whitespace differences (for example if I removed a `if() {}` condition, the indentation has changed).