Skip to contentSkip to conversation

Feedback wanted: proof-of-concept recvmmsg() support

PATCH In review Steffan Karger 2 participants 3 messages Dec 24, 2016 – Feb 17, 2017

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Feedback wanted: proof-of-concept recvmmsg() support

This patch is not done, but I would like to get some early feedback because I'm not very familiar with this part of the code, nor with the APIs involved. So I expect to have made some rooky mistakes.

A while back, I experimented a bit with recvmmsg(), to see if using it would yield some performance gain. Simple tests (I don't have a good stress test setup handy) indicate that this improves throughput by a few percent. I would expect more gain on the server side for P2MP servers though, where multiple client can be spamming the server at once.

What definitely still needs to be done:
* Improve error messages
* Improve documentation (doxygen, openvpn.8, Changes.rst)
* Figure out good default values

So, please, let me know what you think of the code. And if you do have a good setup for performance testing, I'd love to hear what this change does for you.

XXX step towards dynamic allocation (still working)

further move towards dynamic alloc

init more-or-less-properly (no alloc yet)

dynamic alloc

refactor1

refactor2

refactor3

Finish prototype code

---
 configure.ac          |   2 +-
 src/openvpn/forward.c |   3 +-
 src/openvpn/init.c    |   7 +++
 src/openvpn/integer.h |   7 +++
 src/openvpn/options.c |   6 ++
 src/openvpn/options.h |   1 +
 src/openvpn/socket.c  | 152 +++++++++++++++++++++++++++++++++++++++++++++++---
 src/openvpn/socket.h  |  43 ++++++++++++++
 8 files changed, 210 insertions(+), 11 deletions(-)
configure.ac +1 -1
index 43487b0..6bfb600 100644
@@ -672,7 +672,7 @@ AC_SUBST([SOCKETS_LIBS])
 
 old_LIBS="${LIBS}"
 LIBS="${LIBS} ${SOCKETS_LIBS}"
-AC_CHECK_FUNCS([sendmsg recvmsg])
+AC_CHECK_FUNCS([sendmsg recvmsg sendmmsg recvmmsg])
 # Windows use stdcall for winsock so we cannot auto detect these
 m4_define(
 	[SOCKET_FUNCS],
src/openvpn/forward.c +2 -1
index 8102e94..0c6af66 100644
@@ -1754,7 +1754,8 @@ io_wait_dowork(struct context *c, const unsigned int flags)
 
     if (!c->sig->signal_received)
     {
-        if (!(flags & IOW_CHECK_RESIDUAL) || !socket_read_residual(c->c2.link_socket))
+        if ((!(flags & IOW_CHECK_RESIDUAL) || !socket_read_residual(c->c2.link_socket))
+            && !openvpn_mmsg_ctx_available(&c->c2.link_socket->recvmmsg_ctx))
         {
             int status;
 
src/openvpn/init.c +7 -0
index 4ff7725..1996d4d 100644
@@ -3157,6 +3157,13 @@ do_init_socket_2(struct context *c)
 {
     link_socket_init_phase2(c->c2.link_socket, &c->c2.frame,
                             c->sig);
+
+    if (c->options.sockflags | SF_USE_IP_PKTINFO)
+    {
+        openvpn_mmsg_ctx_init(&c->c2.link_socket->recvmmsg_ctx,
+                              BUF_SIZE(&c->c2.frame),
+                              c->options.recvmmsg_buf_count);
+    }
 }
 
 /*
src/openvpn/integer.h +7 -0
index 5ea32c4..8ea6708 100644
@@ -31,6 +31,13 @@
  * min/max functions
  */
 
+#ifndef MIN
+#define MIN(a,b) (((a)<(b)) ? (a) : (b))
+#endif
+#ifndef MAX
+#define MAX(a,b) (((a)>(b)) ? (a) : (b))
+#endif
+
 static inline int
 max_int(int x, int y)
 {
src/openvpn/options.c +6 -0
index 953e376..cf08186 100644
@@ -811,6 +811,7 @@ init_options(struct options *o, const bool init_gc)
     o->resolve_retry_seconds = RESOLV_RETRY_INFINITE;
     o->resolve_in_advance = false;
     o->proto_force = -1;
+    o->recvmmsg_buf_count = 16;
 #ifdef ENABLE_OCC
     o->occ = true;
 #endif
@@ -5655,6 +5656,11 @@ add_option(struct options *options,
         VERIFY_PERMISSION(OPT_P_GENERAL);
         options->sockflags |= SF_USE_IP_PKTINFO;
     }
+    else if (streq(p[0], "recvmmsg-buf-count") && p[1] && !p[2])
+    {
+        VERIFY_PERMISSION(OPT_P_GENERAL);
+        options->recvmmsg_buf_count = strtoul(p[1], NULL, 10);
+    }
 #endif
     else if (streq(p[0], "verb") && p[1] && !p[2])
     {
src/openvpn/options.h +1 -0
index b3ab029..9b3897e 100644
@@ -325,6 +325,7 @@ struct options
 
     /* socket flags */
     unsigned int sockflags;
+    size_t recvmmsg_buf_count;
 
     /* route management */
     const char *route_script;
src/openvpn/socket.c +143 -9

203 lines not on this page · raw message

src/openvpn/socket.h +43 -0

61 lines not on this page · raw message

Signature · 1 line
2.7.4
#1 lore raw
Quoted 10 lines · Steffan Karger

This patch is not done, but I would like to get some early feedback because I'm not very familiar with this part of the code, nor with the APIs involved. So I expect to have made some rooky mistakes.

A while back, I experimented a bit with recvmmsg(), to see if using it would yield some performance gain. Simple tests (I don't have a good stress test setup handy) indicate that this improves throughput by a few percent. I would expect more gain on the server side for P2MP servers though, where multiple client can be spamming the server at once.

Quoted 8 lines

What definitely still needs to be done:
* Improve error messages
* Improve documentation (doxygen, openvpn.8, Changes.rst)
* Figure out good default values

So, please, let me know what you think of the code. And if you do have a good setup for performance testing, I'd love to hear what this change does for you.

Too bad I have changed employer ... as I have some vague memories doing some recvmmsg() send even sendmmsg() testing on kernel-rt, when these features were added there. I'll meet some of my ex-colleagues later this week and weekend (devconf.cz happens now) ... so I'll see if I can get some help retrieving some reasonable test scenarios.

So to the patch ...

First of all, not all kernels carry these system calls, I believe they were added in some of the 3.x kernels - but, IIRC, it has been backported to at least the RHEL6 2.6.32 kernels. My memory is scarce about the 2.6.18 kernel base on RHEL5, but I believe only recvmmsg() is supported there.

But kernel support isn't enough. It needs to be supported in glibc as well, where recvmmsg() arrived in glibc-2.12 and sendmmsg() arrived in glibc-2.14. And if glibc support is missing ... there's needed to so some tweaks here, having a wrapper which does the proper syscall() with SYS_recvmmsg and/or SYS_sendmmsg as the syscall number reference. (see /usr/include/bits/syscall.h and man syscall(2) for more info) ... but step carefully with such wrappers - as we need to consider cross-platforms (not saying we should support this feature on all platforms).

So all this considered, configure.ac will need to do some probing if recvmmsg() and sendmmsg() are supported.

I know this patch covers recvmmsg() only. But I think you can gain even more performance if looking into sendmmsg() as well.

I'll try to play a bit with this patch and see how things go on my side though.

Signature · 4 lines
kind regards,

David Sommerseth
OpenVPN Technologies, Inc
#2 lore raw

Hi David,

Thanks for the comments.

Quoted 16 lines · David Sommerseth

First of all, not all kernels carry these system calls, I believe they were added in some of the 3.x kernels - but, IIRC, it has been backported to at least the RHEL6 2.6.32 kernels. My memory is scarce about the 2.6.18 kernel base on RHEL5, but I believe only recvmmsg() is supported there.

But kernel support isn't enough. It needs to be supported in glibc as well, where recvmmsg() arrived in glibc-2.12 and sendmmsg() arrived in glibc-2.14. And if glibc support is missing ... there's needed to so some tweaks here, having a wrapper which does the proper syscall() with SYS_recvmmsg and/or SYS_sendmmsg as the syscall number reference. (see /usr/include/bits/syscall.h and man syscall(2) for more info) ... but step carefully with such wrappers - as we need to consider cross-platforms (not saying we should support this feature on all platforms).

Quoted 2 lines

So all this considered, configure.ac will need to do some probing if recvmmsg() and sendmmsg() are supported.

configure.ac has:

-AC_CHECK_FUNCS([sendmsg recvmsg]) +AC_CHECK_FUNCS([sendmsg recvmsg sendmmsg recvmmsg])

and the calls are inside

+#ifdef HAVE_RECVMMSG

Shouldn't that be enough?

Quoted 2 lines

I know this patch covers recvmmsg() only. But I think you can gain even more performance if looking into sendmmsg() as well.

Yes, but that's a little trickier, because it also need changing the tun/tap side (we need to read multiple messages there to have some gain from sendmmsg()). I decided to do this one thing at a time :)

Quoted 2 lines

I'll try to play a bit with this patch and see how things go on my side though.

Great, I am curious about your results.

-Steffan