Ben Pfaff <[email protected]> writes:
> 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 <[email protected]>
> Signed-off-by: Ben Pfaff <[email protected]>
> ---
> I haven't actually tested this!
I have trouble understanding the motivation for this patch. Will
there be an attempt to rewrite netlink-socket or netdev-linux to use
recvmmsg? Simply adding this function with no new callers seems like
introducing dead code. I'd expect the next version to include some
in-tree user.
> 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 /* <sys/socket.h> 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 */
Why not? A really simple way of implementing it (albeit not a terribly
resource friendly one) would be to call recvmsg() with
(flags | MSG_DONTWAIT)
Then decrement time, and if there's still time left in the timeout,
retry. Alternately, setup an alarm. There may be other methods to
achieve it. The point being that we probably can accommodate this
timeout.
Otherwise, it should at least be documented that the behavior isn't the
same. In that case, I'd prefer calling it 'ovs_recvmmsg' rather than
simply masquerading the function away. That way, reading the code (and
authoring new code) the difference is obvious (and we could even
just... eliminate timeout as a parameter).
> + bool waitforone = flags & MSG_WAITFORONE;
> + flags &= ~MSG_WAITFORONE;
> +
> + for (unsigned int i = 0; i < n; i++) {
> + ssize_t retval = recvmsg(fd, &msgs[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
Shouldn't that read lib/socket-util.h ? That also applies to the
sendmmsg comment ;)
> + * 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 */
This doesn't make sense. recvmmsg(..., NULL); is perfectly valid, and
this is supposed to be a wrapper? On systems where it is valid to call,
why can't we use it?
> + 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) {
It's strange, no matter what retval is, we would return here. The only
case we really care about is errno, yes? I'm not asking for any change
at this line, it's just an observation.
> + return retval;
> + }
> + recvmmsg_broken = true;
> + errno = save_errno;
> + }
> + return emulate_recvmmsg(fd, msgs, n, flags, timeout);
> +}
> +#endif
> +#endif
> diff --git a/lib/socket-util.h b/lib/socket-util.h
> index a65433d90738..71bd68926aaa 100644
> --- a/lib/socket-util.h
> +++ b/lib/socket-util.h
> @@ -104,19 +104,20 @@ int make_unix_socket(int style, bool nonblock,
> const char *bind_path, const char *connect_path);
> int get_unix_name_len(const struct sockaddr_un *sun, socklen_t sun_len);
>
> -/* Universal sendmmsg support.
> +/* Universal sendmmsg and recvmmsg support.
> *
> - * Some platforms, such as new enough Linux and FreeBSD, support sendmmsg,
> but
> - * other platforms (or older ones) do not. We add the following
> infrastructure
> - * to allow all code to use sendmmsg, regardless of platform support:
> + * Some platforms, such as new enough Linux and FreeBSD, support sendmmsg and
> + * recvmmsg, but other platforms (or older ones) do not. We add the
> following
> + * infrastructure to allow all code to use sendmmsg and recvmmsg, regardless
> of
> + * platform support:
> *
> - * - For platforms that lack sendmmsg entirely, we emulate it.
> + * - For platforms that lack these functions entirely, we emulate them.
> *
> - * - Some platforms have sendmmsg() in the C library but not in the kernel.
> - * For example, this is true if a Linux system has a newer glibc with an
> - * old kernel. To compensate, even if sendmmsg() appears to be
> available,
> - * we still wrap it with a handler that uses our emulation if sendmmsg()
> - * returns ENOSYS.
> + * - Some platforms have sendmmsg() and recvmmsg() in the C library but not
> in
> + * the kernel. For example, this is true if a Linux system has a newer
> glibc
> + * with an old kernel. To compensate, even if these functions appear to be
> + * available, we still wrap them with handlers that uses our emulation if
> the
> + * underlying function returns ENOSYS.
> */
> #ifndef HAVE_STRUCT_MMSGHDR_MSG_LEN
> struct mmsghdr {
> @@ -126,9 +127,12 @@ struct mmsghdr {
> #endif
> #ifndef HAVE_SENDMMSG
> int sendmmsg(int, struct mmsghdr *, unsigned int, unsigned int);
> +int recvmmsg(int, struct mmsghdr *, unsigned int, int, struct timespec *);
> #else
> #define sendmmsg wrap_sendmmsg
> int wrap_sendmmsg(int, struct mmsghdr *, unsigned int, unsigned int);
> +#define recvmmsg wrap_recvmmsg
> +int wrap_recvmmsg(int, struct mmsghdr *, unsigned int, int, struct timespec
> *);
> #endif
>
> /* Helpers for calling ioctl() on an AF_INET socket. */
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
