Re: [ovs-dev] 答复: [PATCH] socket-util: Introduce emulation and wrapper for recvmmsg().

2019-12-20 Thread Ben Pfaff
On Fri, Dec 20, 2019 at 01:25:29AM +, Yi Yang (杨�D)-云服务集团 wrote:
> Current ovs matser has included sendmmsg declaration in
> include/sparse/sys/socket.h

I believe you are mistaken.

> int sendmmsg(int, struct mmsghdr *, unsigned int, unsigned int);
> 
> I saw  "+^L" in your patch.

Yes, OVS uses page breaks to separate logical sections of code.  The
coding style document mentions this.

> --- a/lib/socket-util.c
> +++ b/lib/socket-util.c
> @@ -1283,3 +1283,59 @@ wrap_sendmmsg(int fd, struct mmsghdr *msgs, unsigned
> int n, unsigned int flags)
>  }
>  #endif
>  #endif
> +^L
> +#ifndef _WIN32 /* Avoid using recvmsg on Windows entirely. */
> 
> +#undef recvmmsg
> +int
> +wrap_recvmmsg(int fd, struct mmsghdr *msgs, unsigned int n,
> +  int flags, struct timespec *timeout)
> +{
> +ovs_assert(!timeout);   /* XXX not emulated */
> +
> +static bool recvmmsg_broken = false;
> +if (!recvmmsg_broken) {
> +int save_errno = errno;
> +int retval = recvmmsg(fd, msgs, n, flags, timeout);
> +if (retval >= 0 || errno != ENOSYS) {
> +return retval;
> +}
> +recvmmsg_broken = true;
> +errno = save_errno;
> +}
> +return emulate_recvmmsg(fd, msgs, n, flags, timeout);
> +}
> +#endif
> 
> I don't understand why call recvmmsg here although we have known recvmmsg
> isn't defined,

Can you explain that comment?  I don't believe that the code tries to
call recvmmsg() when it is not defined.  The code inside #ifndef
HAVE_SENDMMSG only uses emulate_recvmmsg(), which itself only calls
recvmsg(), not recvmmsg().

> I don't think "static bool recvmmsg_broken" is thread-safe. 

It is thread-safe enough, because it is merely an optimization: if the
value is wrong, then at most the code gets a little bit slower.

> I think we can completely remove the below part if we do know recvmmsg
> isn't defined (I think autoconf can detect it very precisely, we
> needn't to do runtime check for this)
> +static bool recvmmsg_broken = false;
> +if (!recvmmsg_broken) {
> +int save_errno = errno;
> +int retval = recvmmsg(fd, msgs, n, flags, timeout);
> +if (retval >= 0 || errno != ENOSYS) {
> +return retval;
> +}
> +recvmmsg_broken = true;
> +errno = save_errno;
> +}

There are three cases:

1. The C library does not have recvmmsg().  Then we cannot call it at
   all.  In this case, HAVE_SENDMMSG is false and the "#ifndef
   HAVE_SENDMMSG" fork will use emulate_recvmmsg().

2. The C library has recvmmsg() but the kernel does not, because it is
   too old.  Then wrap_recvmmsg() will receive an ENOSYS error from the
   kernel, call emulate_recvmmsg(), and set recvmmsg_broken so that
   future calls don't have to bother going into the kernel at all.

3. The C library and the kernel both have recvmmsg().  Then
   wrap_recvmmsg() will call recvmmsg() and either succeed or get back
   some error other than ENOSYS.  recvmmsg_broken will remain false, and
   all future calls to recvmmsg() will also take the kernel fast path.

Autoconf cannot distinguish cases 2 and 3, nor can anything that runs at
build time, because there is no way to guess whether the runtime kernel
matches the build time kernel.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] 答复: [PATCH] socket-util: Introduce emulation and wrapper for recvmmsg().

2019-12-19 Thread 杨�D
Current ovs matser has included sendmmsg declaration in
include/sparse/sys/socket.h

int sendmmsg(int, struct mmsghdr *, unsigned int, unsigned int);

I saw  "+^L" in your patch.

--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -1283,3 +1283,59 @@ wrap_sendmmsg(int fd, struct mmsghdr *msgs, unsigned
int n, unsigned int flags)
 }
 #endif
 #endif
+^L
+#ifndef _WIN32 /* Avoid using recvmsg on Windows entirely. */

+#undef recvmmsg
+int
+wrap_recvmmsg(int fd, struct mmsghdr *msgs, unsigned int n,
+  int flags, struct timespec *timeout)
+{
+ovs_assert(!timeout);   /* XXX not emulated */
+
+static bool recvmmsg_broken = false;
+if (!recvmmsg_broken) {
+int save_errno = errno;
+int retval = recvmmsg(fd, msgs, n, flags, timeout);
+if (retval >= 0 || errno != ENOSYS) {
+return retval;
+}
+recvmmsg_broken = true;
+errno = save_errno;
+}
+return emulate_recvmmsg(fd, msgs, n, flags, timeout);
+}
+#endif

I don't understand why call recvmmsg here although we have known recvmmsg
isn't defined, I don't think "static bool recvmmsg_broken" is thread-safe. I
think we can completely remove the below part if we do know recvmmsg isn't
defined (I think autoconf can detect it very precisely, we needn't to do
runtime check for this)
+static bool recvmmsg_broken = false;
+if (!recvmmsg_broken) {
+int save_errno = errno;
+int retval = recvmmsg(fd, msgs, n, flags, timeout);
+if (retval >= 0 || errno != ENOSYS) {
+return retval;
+}
+recvmmsg_broken = true;
+errno = save_errno;
+}


-邮件原件-
发件人: Ben Pfaff [mailto:b...@ovn.org] 
发送时间: 2019年12月18日 4:39
收件人: d...@openvswitch.org
抄送: Ben Pfaff ; Yi Yang (杨�D)-云服务集团

主题: [PATCH] socket-util: Introduce emulation and wrapper for recvmmsg().

Not every system will have recvmmsg(), so introduce compatibility code that
will allow it to be used blindly from the rest of the tree.

This assumes that recvmmsg() and sendmmsg() are either both present or both
absent in system libraries and headers.

CC: Yi Yang 
Signed-off-by: Ben Pfaff 
---
I haven't actually tested this!

 include/sparse/sys/socket.h |  7 -
 lib/socket-util.c   | 56 +
 lib/socket-util.h   | 24 +---
 3 files changed, 76 insertions(+), 11 deletions(-)

diff --git a/include/sparse/sys/socket.h b/include/sparse/sys/socket.h index
4178f57e2bda..6ff245ae939b 100644
--- a/include/sparse/sys/socket.h
+++ b/include/sparse/sys/socket.h
@@ -27,6 +27,7 @@
 
 typedef unsigned short int sa_family_t;  typedef __socklen_t socklen_t;
+struct timespec;
 
 struct sockaddr {
 sa_family_t sa_family;
@@ -126,7 +127,8 @@ enum {
 MSG_PEEK,
 MSG_TRUNC,
 MSG_WAITALL,
-MSG_DONTWAIT
+MSG_DONTWAIT,
+MSG_WAITFORONE
 };
 
 enum {
@@ -171,4 +173,7 @@ int sockatmark(int);  int socket(int, int, int);  int
socketpair(int, int, int, int[2]);
 
+int sendmmsg(int, struct mmsghdr *, unsigned int, int); int 
+recvmmsg(int, struct mmsghdr *, unsigned int, int, struct timespec *);
+
 #endif /*  for sparse */
diff --git a/lib/socket-util.c b/lib/socket-util.c index
6b7378de934b..f6f6f3b0a33f 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -1283,3 +1283,59 @@ wrap_sendmmsg(int fd, struct mmsghdr *msgs, unsigned
int n, unsigned int flags)  }  #endif  #endif
+

+#ifndef _WIN32 /* Avoid using recvmsg on Windows entirely. */ static 
+int emulate_recvmmsg(int fd, struct mmsghdr *msgs, unsigned int n,
+ int flags, struct timespec *timeout OVS_UNUSED) {
+ovs_assert(!timeout);   /* XXX not emulated */
+
+bool waitforone = flags & MSG_WAITFORONE;
+flags &= ~MSG_WAITFORONE;
+
+for (unsigned int i = 0; i < n; i++) {
+ssize_t retval = recvmsg(fd, [i].msg_hdr, flags);
+if (retval < 0) {
+return i ? i : retval;
+}
+msgs[i].msg_len = retval;
+
+if (waitforone) {
+flags |= MSG_DONTWAIT;
+}
+}
+return n;
+}
+
+#ifndef HAVE_SENDMMSG
+int
+recvmmsg(int fd, struct mmsghdr *msgs, unsigned int n,
+ int flags, struct timespec *timeout) {
+return emulate_recvmmsg(fd, msgs, n, flags, timeout); } #else
+/* recvmmsg was redefined in lib/socket-util.c, should undef recvmmsg 
+here
+ * to avoid recursion */
+#undef recvmmsg
+int
+wrap_recvmmsg(int fd, struct mmsghdr *msgs, unsigned int n,
+  int flags, struct timespec *timeout) {
+ovs_assert(!timeout);   /* XXX not emulated */
+
+static bool recvmmsg_broken = false;
+if (!recvmmsg_broken) {
+int save_errno = errno;
+int retval = recvmmsg(fd, msgs, n, flags, timeout);
+if (retval >= 0 || errno != ENOSYS) {
+return retval;
+}
+recvmmsg_broken = true;
+errno = save_errno;
+}
+return emulate_recvmmsg(fd, msgs, n, flags, timeout); } #endif 
+#endif
diff --git