On 2/7/20 9:40 AM, anton.iva...@cambridgegreys.com wrote:
> From: Anton Ivanov <anton.iva...@cambridgegreys.com>
>
> In order to allow switching between poll and epoll we need to
> switch from the direct use of POLLXXX macros to OVS_POLLXXX which
> can be either POLLXXX or EPOLLXXX depending on the actual backend.
>
> Signed-off-by: Anton Ivanov <anton.iva...@cambridgegreys.com>
Hi Anton,
Your patches break OVS. Take the following scenario which adds an OVS
bridge and an internal port to it:
$ ovs-vsctl add-br br-test
$ ovs-vsctl add-port br-test vm1 -- set interface vm1 type=internal
$ ip netns add vm1
$ ip link set vm1 netns vm1
$ ip netns exec vm1 ip link set vm1 address 00:00:00:00:00:01
$ ip netns exec vm1 ip addr add 42.42.42.2/24 dev vm1
# Bring the port up which will trigger ICMPv6 neighbor/router solicitation
$ ip netns exec vm1 ip link set dev vm1 up
2020-02-12T13:28:20.026Z|00036|poll_loop|INFO|wakeup due to [OVS_POLLIN]
on fd 62 (NETLINK_GENERIC<->NETLINK_GENERIC) at lib/netlink-socket.c:234
(69% CPU usage)
2020-02-12T13:28:20.026Z|00037|poll_loop|INFO|wakeup due to [OVS_POLLIN]
on fd 32 (unknown anon_inode:[eventpoll]) at lib/dpif-netlink.c:2299
(69% CPU usage)
2020-02-12T13:28:20.026Z|00038|poll_loop|INFO|wakeup due to [OVS_POLLIN]
on fd 33 (unknown anon_inode:[eventpoll]) at lib/dpif-netlink.c:2299
(69% CPU usage)
2020-02-12T13:28:20.026Z|00039|poll_loop|INFO|wakeup due to [OVS_POLLIN]
on fd 34 (unknown anon_inode:[eventpoll]) at lib/dpif-netlink.c:2299
(69% CPU usage)
2020-02-12T13:28:20.026Z|00040|poll_loop|INFO|wakeup due to [OVS_POLLIN]
on fd 35 (unknown anon_inode:[eventpoll]) at lib/dpif-netlink.c:2299
(69% CPU usage)
2020-02-12T13:28:20.026Z|00041|poll_loop|INFO|wakeup due to [OVS_POLLIN]
on fd 36 (unknown anon_inode:[eventpoll]) at lib/dpif-netlink.c:2299
(69% CPU usage)
2020-02-12T13:28:20.026Z|00042|poll_loop|INFO|wakeup due to [OVS_POLLIN]
on fd 62 (NETLINK_GENERIC<->NETLINK_GENERIC) at lib/netlink-socket.c:234
(69% CPU usage)
2020-02-12T13:28:20.026Z|00043|poll_loop|INFO|wakeup due to [OVS_POLLIN]
on fd 32 (unknown anon_inode:[eventpoll]) at lib/dpif-netlink.c:2299
(69% CPU usage)
2020-02-12T13:28:20.026Z|00044|poll_loop|INFO|wakeup due to [OVS_POLLIN]
on fd 33 (unknown anon_inode:[eventpoll]) at lib/dpif-netlink.c:2299
(69% CPU usage)
2020-02-12T13:28:20.026Z|00045|poll_loop|INFO|wakeup due to [OVS_POLLIN]
on fd 34 (unknown anon_inode:[eventpoll]) at lib/dpif-netlink.c:2299
(69% CPU usage)
2020-02-12T13:28:26.026Z|00046|poll_loop|INFO|Dropped 1530469 log
messages in last 6 seconds (most recently, 0 seconds ago) due to
excessive rate
2020-02-12T13:28:26.026Z|00047|poll_loop|INFO|wakeup due to [OVS_POLLIN]
on fd 62 (NETLINK_GENERIC<->NETLINK_GENERIC) at lib/netlink-socket.c:234
(98% CPU usage)
2020-02-12T13:28:32.026Z|00048|poll_loop|INFO|Dropped 1548848 log
messages in last 6 seconds (most recently, 0 seconds ago) due to
excessive rate
# ovs-vswitchd stays at 100% CPU until the we bring the bridge down:
$ ovs-vsctl del-br br-test
Also, your patches generate build warnings:
lib/poll-loop.c: In function 'poll_block':
lib/poll-loop.c:383:9: error: unused variable 'counter'
[-Werror=unused-variable]
int counter;
Regards,
Dumitru
> ---
> include/openvswitch/poll-loop.h | 6 ++++++
> lib/fatal-signal.c | 2 +-
> lib/latch-unix.c | 4 ++--
> lib/netdev-afxdp.c | 2 +-
> lib/netdev-bsd.c | 4 ++--
> lib/netdev-linux.c | 4 ++--
> lib/netlink-notifier.c | 4 +++-
> lib/poll-loop.c | 30 +++++++++++++++---------------
> lib/process.c | 2 +-
> lib/route-table-bsd.c | 2 +-
> lib/rtbsd.c | 2 +-
> lib/socket-util.c | 8 ++++----
> lib/stream-fd.c | 6 +++---
> lib/stream-ssl.c | 8 ++++----
> manpages.mk | 3 ---
> tests/test-netflow.c | 2 +-
> tests/test-sflow.c | 2 +-
> utilities/nlmon.c | 2 +-
> 18 files changed, 49 insertions(+), 44 deletions(-)
>
> diff --git a/include/openvswitch/poll-loop.h b/include/openvswitch/poll-loop.h
> index 532640497..532d9caa6 100644
> --- a/include/openvswitch/poll-loop.h
> +++ b/include/openvswitch/poll-loop.h
> @@ -41,6 +41,12 @@
> #include <windows.h>
> #endif
>
> +#define OVS_POLLIN POLLIN
> +#define OVS_POLLOUT POLLOUT
> +#define OVS_POLLERR POLLERR
> +#define OVS_POLLNVAL POLLNVAL
> +#define OVS_POLLHUP POLLHUP
> +
> #ifdef __cplusplus
> extern "C" {
> #endif
> diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c
> index 09f7c6ecf..97d8d1dab 100644
> --- a/lib/fatal-signal.c
> +++ b/lib/fatal-signal.c
> @@ -271,7 +271,7 @@ fatal_signal_wait(void)
> #ifdef _WIN32
> poll_wevent_wait(wevent);
> #else
> - poll_fd_wait(signal_fds[0], POLLIN);
> + poll_fd_wait(signal_fds[0], OVS_POLLIN);
> #endif
> }
>
> diff --git a/lib/latch-unix.c b/lib/latch-unix.c
> index 2995076d6..fea61ab28 100644
> --- a/lib/latch-unix.c
> +++ b/lib/latch-unix.c
> @@ -67,7 +67,7 @@ latch_is_set(const struct latch *latch)
> int retval;
>
> pfd.fd = latch->fds[0];
> - pfd.events = POLLIN;
> + pfd.events = POLLIN; /* This is POLL specific, it should use POLL only
> macro */
> do {
> retval = poll(&pfd, 1, 0);
> } while (retval < 0 && errno == EINTR);
> @@ -83,5 +83,5 @@ latch_is_set(const struct latch *latch)
> void
> latch_wait_at(const struct latch *latch, const char *where)
> {
> - poll_fd_wait_at(latch->fds[0], POLLIN, where);
> + poll_fd_wait_at(latch->fds[0], OVS_POLLIN, where);
> }
> diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
> index 482400d8d..ef367e5ea 100644
> --- a/lib/netdev-afxdp.c
> +++ b/lib/netdev-afxdp.c
> @@ -184,7 +184,7 @@ xsk_rx_wakeup_if_needed(struct xsk_umem_info *umem,
>
> if (xsk_ring_prod__needs_wakeup(&umem->fq)) {
> pfd.fd = fd;
> - pfd.events = POLLIN;
> + pfd.events = OVS_POLLIN;
>
> ret = poll(&pfd, 1, 0);
> if (OVS_UNLIKELY(ret < 0)) {
> diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
> index 7875636cc..45385b187 100644
> --- a/lib/netdev-bsd.c
> +++ b/lib/netdev-bsd.c
> @@ -659,7 +659,7 @@ netdev_bsd_rxq_wait(struct netdev_rxq *rxq_)
> {
> struct netdev_rxq_bsd *rxq = netdev_rxq_bsd_cast(rxq_);
>
> - poll_fd_wait(rxq->fd, POLLIN);
> + poll_fd_wait(rxq->fd, OVS_POLLIN);
> }
>
> /* Discards all packets waiting to be received from 'rxq'. */
> @@ -752,7 +752,7 @@ netdev_bsd_send_wait(struct netdev *netdev_, int qid
> OVS_UNUSED)
> /* TAP device always accepts packets. */
> poll_immediate_wake();
> } else if (dev->pcap) {
> - poll_fd_wait(dev->fd, POLLOUT);
> + poll_fd_wait(dev->fd, OVS_POLLOUT);
> } else {
> /* We haven't even tried to send a packet yet. */
> poll_immediate_wake();
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index 6add3e2fc..d0dc00d97 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -808,7 +808,7 @@ netdev_linux_wait(const struct netdev_class *netdev_class
> OVS_UNUSED)
> }
> sock = netdev_linux_notify_sock();
> if (sock) {
> - nl_sock_wait(sock, POLLIN);
> + nl_sock_wait(sock, OVS_POLLIN);
> }
> }
>
> @@ -1465,7 +1465,7 @@ static void
> netdev_linux_rxq_wait(struct netdev_rxq *rxq_)
> {
> struct netdev_rxq_linux *rx = netdev_rxq_linux_cast(rxq_);
> - poll_fd_wait(rx->fd, POLLIN);
> + poll_fd_wait(rx->fd, OVS_POLLIN);
> }
>
> static int
> diff --git a/lib/netlink-notifier.c b/lib/netlink-notifier.c
> index dfecb9778..1747274de 100644
> --- a/lib/netlink-notifier.c
> +++ b/lib/netlink-notifier.c
> @@ -1,4 +1,5 @@
> /*
> +
> * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2016 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> @@ -27,6 +28,7 @@
> #include "netlink-socket.h"
> #include "openvswitch/ofpbuf.h"
> #include "openvswitch/vlog.h"
> +#include "openvswitch/poll-loop.h"
>
> VLOG_DEFINE_THIS_MODULE(netlink_notifier);
>
> @@ -219,7 +221,7 @@ nln_wait(struct nln *nln)
> {
> nln->has_run = false;
> if (nln->notify_sock) {
> - nl_sock_wait(nln->notify_sock, POLLIN);
> + nl_sock_wait(nln->notify_sock, OVS_POLLIN);
> }
> }
>
> diff --git a/lib/poll-loop.c b/lib/poll-loop.c
> index 4e751ff2c..3902d6c1f 100644
> --- a/lib/poll-loop.c
> +++ b/lib/poll-loop.c
> @@ -80,7 +80,7 @@ find_poll_node(struct poll_loop *loop, int fd, HANDLE
> wevent)
> /* On Unix based systems:
> *
> * Registers 'fd' as waiting for the specified 'events' (which should be
> - * POLLIN or POLLOUT or POLLIN | POLLOUT). The following call to
> + * OVS_POLLIN or OVS_POLLOUT or OVS_POLLIN | OVS_POLLOUT). The
> following call to
> * poll_block() will wake up when 'fd' becomes ready for one or more of
> the
> * requested events. The 'fd's are given to poll() function later.
> *
> @@ -130,8 +130,8 @@ poll_create_node(int fd, HANDLE wevent, short int events,
> const char *where)
> }
> }
>
> -/* Registers 'fd' as waiting for the specified 'events' (which should be
> POLLIN
> - * or POLLOUT or POLLIN | POLLOUT). The following call to poll_block() will
> +/* Registers 'fd' as waiting for the specified 'events' (which should be
> OVS_POLLIN
> + * or OVS_POLLOUT or OVS_POLLIN | OVS_POLLOUT). The following call to
> poll_block() will
> * wake up when 'fd' becomes ready for one or more of the requested events.
> *
> * On Windows, 'fd' must be a socket.
> @@ -265,20 +265,20 @@ log_wakeup(const char *where, const struct pollfd
> *pollfd, int timeout)
> ds_put_cstr(&s, "wakeup due to ");
> if (pollfd) {
> char *description = describe_fd(pollfd->fd);
> - if (pollfd->revents & POLLIN) {
> - ds_put_cstr(&s, "[POLLIN]");
> + if (pollfd->revents & OVS_POLLIN) {
> + ds_put_cstr(&s, "[OVS_POLLIN]");
> }
> - if (pollfd->revents & POLLOUT) {
> - ds_put_cstr(&s, "[POLLOUT]");
> + if (pollfd->revents & OVS_POLLOUT) {
> + ds_put_cstr(&s, "[OVS_POLLOUT]");
> }
> - if (pollfd->revents & POLLERR) {
> - ds_put_cstr(&s, "[POLLERR]");
> + if (pollfd->revents & OVS_POLLERR) {
> + ds_put_cstr(&s, "[OVS_POLLERR]");
> }
> - if (pollfd->revents & POLLHUP) {
> - ds_put_cstr(&s, "[POLLHUP]");
> + if (pollfd->revents & OVS_POLLHUP) {
> + ds_put_cstr(&s, "[OVS_POLLHUP]");
> }
> - if (pollfd->revents & POLLNVAL) {
> - ds_put_cstr(&s, "[POLLNVAL]");
> + if (pollfd->revents & OVS_POLLNVAL) {
> + ds_put_cstr(&s, "[OVS_POLLNVAL]");
> }
> ds_put_format(&s, " on fd %d (%s)", pollfd->fd, description);
> free(description);
> @@ -349,10 +349,10 @@ poll_block(void)
> wevents[i] = node->wevent;
> if (node->pollfd.fd && node->wevent) {
> short int wsa_events = 0;
> - if (node->pollfd.events & POLLIN) {
> + if (node->pollfd.events & OVS_POLLIN) {
> wsa_events |= FD_READ | FD_ACCEPT | FD_CLOSE;
> }
> - if (node->pollfd.events & POLLOUT) {
> + if (node->pollfd.events & OVS_POLLOUT) {
> wsa_events |= FD_WRITE | FD_CONNECT | FD_CLOSE;
> }
> WSAEventSelect(node->pollfd.fd, node->wevent, wsa_events);
> diff --git a/lib/process.c b/lib/process.c
> index 78de4b8df..7a7f182e1 100644
> --- a/lib/process.c
> +++ b/lib/process.c
> @@ -592,7 +592,7 @@ process_wait(struct process *p)
> if (p->exited) {
> poll_immediate_wake();
> } else {
> - poll_fd_wait(fds[0], POLLIN);
> + poll_fd_wait(fds[0], OVS_POLLIN);
> }
> #else
> OVS_NOT_REACHED();
> diff --git a/lib/route-table-bsd.c b/lib/route-table-bsd.c
> index 34d42cfab..3dfa80c7f 100644
> --- a/lib/route-table-bsd.c
> +++ b/lib/route-table-bsd.c
> @@ -113,7 +113,7 @@ retry:
>
> memset(&pfd, 0, sizeof(pfd));
> pfd.fd = rtsock;
> - pfd.events = POLLIN;
> + pfd.events = OVS_POLLIN;
> /*
> * The timeout value below is somehow arbitrary.
> * It's to detect the lost of routing messages due to
> diff --git a/lib/rtbsd.c b/lib/rtbsd.c
> index 564595c3a..aa0fa0860 100644
> --- a/lib/rtbsd.c
> +++ b/lib/rtbsd.c
> @@ -159,7 +159,7 @@ rtbsd_notifier_wait(void)
> {
> ovs_mutex_lock(&rtbsd_mutex);
> if (notify_sock >= 0) {
> - poll_fd_wait(notify_sock, POLLIN);
> + poll_fd_wait(notify_sock, OVS_POLLIN);
> }
> ovs_mutex_unlock(&rtbsd_mutex);
> }
> diff --git a/lib/socket-util.c b/lib/socket-util.c
> index 4f1ffecf5..d37867365 100644
> --- a/lib/socket-util.c
> +++ b/lib/socket-util.c
> @@ -259,7 +259,7 @@ check_connection_completion(int fd)
> int retval;
>
> pfd.fd = fd;
> - pfd.events = POLLOUT;
> + pfd.events = OVS_POLLOUT;
>
> #ifndef _WIN32
> do {
> @@ -280,17 +280,17 @@ check_connection_completion(int fd)
> pfd.revents |= pfd.events;
> }
> if (FD_ISSET(fd, &exset)) {
> - pfd.revents |= POLLERR;
> + pfd.revents |= OVS_POLLERR;
> }
> }
> #endif
> if (retval == 1) {
> - if (pfd.revents & (POLLERR | POLLHUP)) {
> + if (pfd.revents & (OVS_POLLERR | OVS_POLLHUP)) {
> ssize_t n = send(fd, "", 1, 0);
> if (n < 0) {
> return sock_errno();
> } else {
> - VLOG_ERR_RL(&rl, "poll return POLLERR but send succeeded");
> + VLOG_ERR_RL(&rl, "poll return OVS_POLLERR but send
> succeeded");
> return EPROTO;
> }
> }
> diff --git a/lib/stream-fd.c b/lib/stream-fd.c
> index 46ee7ae27..62f768d45 100644
> --- a/lib/stream-fd.c
> +++ b/lib/stream-fd.c
> @@ -150,11 +150,11 @@ fd_wait(struct stream *stream, enum stream_wait_type
> wait)
> switch (wait) {
> case STREAM_CONNECT:
> case STREAM_SEND:
> - poll_fd_wait(s->fd, POLLOUT);
> + poll_fd_wait(s->fd, OVS_POLLOUT);
> break;
>
> case STREAM_RECV:
> - poll_fd_wait(s->fd, POLLIN);
> + poll_fd_wait(s->fd, OVS_POLLIN);
> break;
>
> default:
> @@ -271,7 +271,7 @@ static void
> pfd_wait(struct pstream *pstream)
> {
> struct fd_pstream *ps = fd_pstream_cast(pstream);
> - poll_fd_wait(ps->fd, POLLIN);
> + poll_fd_wait(ps->fd, OVS_POLLIN);
> }
>
> static const struct pstream_class fd_pstream_class = {
> diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
> index 078fcbc3a..3b7f9865e 100644
> --- a/lib/stream-ssl.c
> +++ b/lib/stream-ssl.c
> @@ -210,10 +210,10 @@ want_to_poll_events(int want)
> OVS_NOT_REACHED();
>
> case SSL_READING:
> - return POLLIN;
> + return OVS_POLLIN;
>
> case SSL_WRITING:
> - return POLLOUT;
> + return OVS_POLLOUT;
>
> default:
> OVS_NOT_REACHED();
> @@ -811,7 +811,7 @@ ssl_wait(struct stream *stream, enum stream_wait_type
> wait)
> } else {
> switch (sslv->state) {
> case STATE_TCP_CONNECTING:
> - poll_fd_wait(sslv->fd, POLLOUT);
> + poll_fd_wait(sslv->fd, OVS_POLLOUT);
> break;
>
> case STATE_SSL_CONNECTING:
> @@ -965,7 +965,7 @@ static void
> pssl_wait(struct pstream *pstream)
> {
> struct pssl_pstream *pssl = pssl_pstream_cast(pstream);
> - poll_fd_wait(pssl->fd, POLLIN);
> + poll_fd_wait(pssl->fd, OVS_POLLIN);
> }
>
> const struct pstream_class pssl_pstream_class = {
> diff --git a/manpages.mk b/manpages.mk
> index dc201484c..54a3a82ad 100644
> --- a/manpages.mk
> +++ b/manpages.mk
> @@ -104,7 +104,6 @@ utilities/bugtool/ovs-bugtool.8: \
> utilities/bugtool/ovs-bugtool.8.in:
> lib/ovs.tmac:
>
> -
> utilities/ovs-dpctl-top.8: \
> utilities/ovs-dpctl-top.8.in \
> lib/ovs.tmac
> @@ -155,8 +154,6 @@ lib/common-syn.man:
> lib/common.man:
> lib/ovs.tmac:
>
> -lib/ovs.tmac:
> -
> utilities/ovs-testcontroller.8: \
> utilities/ovs-testcontroller.8.in \
> lib/common.man \
> diff --git a/tests/test-netflow.c b/tests/test-netflow.c
> index d2322d450..90516d274 100644
> --- a/tests/test-netflow.c
> +++ b/tests/test-netflow.c
> @@ -229,7 +229,7 @@ test_netflow_main(int argc, char *argv[])
> break;
> }
>
> - poll_fd_wait(sock, POLLIN);
> + poll_fd_wait(sock, OVS_POLLIN);
> unixctl_server_wait(server);
> poll_block();
> }
> diff --git a/tests/test-sflow.c b/tests/test-sflow.c
> index 460d4d6c5..ae169e726 100644
> --- a/tests/test-sflow.c
> +++ b/tests/test-sflow.c
> @@ -739,7 +739,7 @@ test_sflow_main(int argc, char *argv[])
> break;
> }
>
> - poll_fd_wait(sock, POLLIN);
> + poll_fd_wait(sock, OVS_POLLIN);
> unixctl_server_wait(server);
> poll_block();
> }
> diff --git a/utilities/nlmon.c b/utilities/nlmon.c
> index 32aa948c6..e8c6bfa24 100644
> --- a/utilities/nlmon.c
> +++ b/utilities/nlmon.c
> @@ -141,7 +141,7 @@ main(int argc OVS_UNUSED, char *argv[])
> }
> }
>
> - nl_sock_wait(sock, POLLIN);
> + nl_sock_wait(sock, OVS_POLLIN);
> poll_block();
> }
> }
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
