Replace C-calls by structs for better long term

[vanyingenzi]

On a GitHub issue with mptcp_net-next developer, @matttbe highlighted that it was better to define structs in python instead of having a C function for each attribute as it is currently implemented in the library. At least for the current Linux support. We might use C functions for other features later down the road.

[vanyingenzi]

Good news and bad news.

• The good news is that for the function socket_is_mptcp we can manage to do so in python.
• The bad news, is that it's not possible to pass a buffer to Python's getsockopt, which means that calls requiring a special initialization of the buffer passed to getsockopt aren't possible in Python. cf getsockopt implementation.

Therefore, I suggest making our own getsockopt C-extension function that can take bytes object from python.

[matttbe]

Are you sure you cannot retrieve bytes? https://stackoverflow.com/a/18189190
Or: https://github.com/search?l=Python&q=socket.TCP_INFO&type=Code

[vanyingenzi]

Hello @matttbe, first of all thanks for joining me here,

If you try to run the following code :

import socket
import struct 
sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM, socket.IPPROTO_MPTCP)
SOL_MPTCP = 284
MPTCP_TCPINFO = 2

returned = sock.getsockopt(SOL_MPTCP, MPTCP_TCPINFO, struct.calcsize("=IIII"))
print(struct.unpack("=IIII", returned))

You get the following error :

Traceback (most recent call last):
  File "/home/vany/Desktop/open_source/temp/test.py", line 7, in <module>
    returned = sock.getsockopt(SOL_MPTCP, MPTCP_TCPINFO, struct.calcsize("=IIII"))
OSError: [Errno 22] Invalid argument

I tried to reproduce the same thing in C by commenting out the memset of the struct mptcp_subflow_data :

#include <errno.h>
#include <string.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <stdbool.h>
#include <linux/tcp.h>
#include <linux/types.h>

#ifndef SOL_MPTCP
#define SOL_MPTCP 284
#endif

#define IPPROTO_MPTCP 262
#define MPTCPLIB_OPERATION_UNSUPPORTED -2
#define MPTCPLIB_SOCKET_FALLBACK_TCP -1
#define MPTCPLIB_ERROR_FLAG -3

#ifndef _UAPI_MPTCP_H

#define MPTCP_INFO 		1
#define MPTCP_TCPINFO	2

struct mptcp_subflow_data {
	__u32		size_subflow_data;		/* size of this structure in userspace */
	__u32		num_subflows;			/* must be 0, set by kernel */
	__u32		size_kernel;			/* must be 0, set by kernel */
	__u32		size_user;				/* size of one element in data[] */
} __attribute__((aligned(8)));

#endif

int main(int argc, char const *argv[])
{
    int fd = socket(AF_INET, SOCK_STREAM, IPPROTO_MPTCP);
    struct mptcp_subflow_data inf;
    // memset(&inf, 0, sizeof(inf));
    inf.size_subflow_data = sizeof(struct mptcp_subflow_data);
    socklen_t optlen = sizeof(inf);
	if (getsockopt(fd, SOL_MPTCP, 2, &inf, &optlen) < 0){
		perror("The error");
		return 1;
	}
	printf("The number of subflows used are %d\n", inf.num_subflows);
    return 0;
}

Therefore, I concluded that the reason I get the invalid argument is because we don't initialize the buff as the kernel expects it.
I'd like to have your insight about it ?

[matttbe]

@vanyingenzi oh OK, sorry, I see what you meant now.

For MPTCP_INFO, the kernel will not read anything from the buffer. So it is fine to do it with Python and return a dict for example. For the others (MPTCP_TCPINFO and MPTCP_SUBFLOW_ADDRS), that's an issue to do it in Python directly indeed. (maybe there are other libs that would help here, maybe it is needed to do it with C, I don't know)

[vanyingenzi]

Out of curiosity @matttbe, why does for (MPTCP_TCPINFO and MPTCP_SUBFLOW_ADDRS) the kernel need to read from the buffer, the optname and optlen weren't enough parameters ?

[matttbe]

Because the kernel needs to return X times another structure (X for the number of subflows).

Better with an example I guess: MPTCP_TCPINFO will returns 2 types of data:

struct mptcp_subflow_data  # num of subflows + what has been written
struct tcp_info[X]         # different types of info, this structure might grow on newer kernels

Depending on the kernel version, struct tcp_info might get more fields. If you ask to get TCP_INFO (without MPTCP) and when you wrote your app, the structure was taking X bytes, the kernel write up to X bytes: easy and anyway, the userspace don't care about the new fields so fine not to include them (no other choices anyway if we don't want to return an error). But here with MPTCP, different boundaries need to be shared.

So you need to tell the kernel:

• where is the end of struct mptcp_subflow_data: it can also grow in the future
• where is the end of one struct tcp_info: same
• where is the end of the buffer: maybe the userspace is only interested by info from the 2 first subflows

[vanyingenzi]

I'm glad I got the courage to ask for help on Python discuss forum, other people advised me to use ctypes. They convinced me that using ctypes in this case was the direction to take. I'll shortly make a pull request ending this issue.

TL;DR we will no longer need the C-extension for the current supported functionalities.