diff mbox series

[ovs-dev,1/3] Replace direct use of POLLXXX macros with OVS_POLLXXX

Message ID 20200207084033.23018-1-anton.ivanov@cambridgegreys.com
State Superseded
Headers show
Series [ovs-dev,1/3] Replace direct use of POLLXXX macros with OVS_POLLXXX | expand

Commit Message

Anton Ivanov Feb. 7, 2020, 8:40 a.m. UTC
From: Anton Ivanov <anton.ivanov@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.ivanov@cambridgegreys.com>
---
 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(-)

Comments

Dumitru Ceara Feb. 12, 2020, 1:41 p.m. UTC | #1
On 2/7/20 9:40 AM, anton.ivanov@cambridgegreys.com wrote:
> From: Anton Ivanov <anton.ivanov@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.ivanov@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();
>      }
>  }
>
Anton Ivanov Feb. 12, 2020, 2 p.m. UTC | #2
On 12/02/2020 13:41, Dumitru Ceara wrote:
> On 2/7/20 9:40 AM, anton.ivanov@cambridgegreys.com wrote:
>> From: Anton Ivanov <anton.ivanov@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.ivanov@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;

Thanks,

I will look into it.

I looked mostly into stream use for ovn/ovnsdb and I am not surprised that some stuff broke elsewhere.

I will be fixed in the next revision.

A.


>
> 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();
>>       }
>>   }
>>
>
Dumitru Ceara Feb. 12, 2020, 2:06 p.m. UTC | #3
On 2/12/20 3:00 PM, Anton Ivanov wrote:
> 
> On 12/02/2020 13:41, Dumitru Ceara wrote:
>> On 2/7/20 9:40 AM, anton.ivanov@cambridgegreys.com wrote:
>>> From: Anton Ivanov <anton.ivanov@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.ivanov@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;
> 
> Thanks,
> 
> I will look into it.
> 
> I looked mostly into stream use for ovn/ovnsdb and I am not surprised
> that some stuff broke elsewhere.
> 

I actually first saw the issue when trying to run an OVN scale test but
I simplified the report. So, indirectly the patches also break OVN.

> I will be fixed in the next revision.
> 
> A.
> 

Thanks,
Dumitru

> 
>>
>> 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();
>>>       }
>>>   }
>>>
>>
Anton Ivanov Feb. 12, 2020, 2:32 p.m. UTC | #4
On 12/02/2020 13:41, Dumitru Ceara wrote:
> On 2/7/20 9:40 AM, anton.ivanov@cambridgegreys.com wrote:
>> From: Anton Ivanov <anton.ivanov@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.ivanov@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)

This was dumb on my side - netlink runs its own epoll loop, its sockets should not go into the main loop.

Fixed in my tree, once I retest I will resend an amended version.

A.

> 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();
>>       }
>>   }
>>
> 
>
Anton Ivanov Feb. 12, 2020, 3:17 p.m. UTC | #5
On 12/02/2020 13:41, Dumitru Ceara wrote:
> On 2/7/20 9:40 AM, anton.ivanov@cambridgegreys.com wrote:
>> From: Anton Ivanov <anton.ivanov@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.ivanov@cambridgegreys.com>
> 
> Hi Anton,
> 
> Your patches break OVS. Take the following scenario which adds an OVS
> bridge and an internal port to it:

Can you send me your ovs-vswitchd  and ovsdb arguments lists to make sure I am testing under the same conditions.

Thanks in advance,

> 
> $ 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();
>>       }
>>   }
>>
> 
>
Dumitru Ceara Feb. 12, 2020, 3:30 p.m. UTC | #6
On 2/12/20 4:17 PM, Anton Ivanov wrote:
> 
> 
> On 12/02/2020 13:41, Dumitru Ceara wrote:
>> On 2/7/20 9:40 AM, anton.ivanov@cambridgegreys.com wrote:
>>> From: Anton Ivanov <anton.ivanov@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.ivanov@cambridgegreys.com>
>>
>> Hi Anton,
>>
>> Your patches break OVS. Take the following scenario which adds an OVS
>> bridge and an internal port to it:
> 
> Can you send me your ovs-vswitchd  and ovsdb arguments lists to make
> sure I am testing under the same conditions.

On a "clean" system, without any pre-existing OVS conf.db I manually
started OVS:

ovs-ctl start --system-id=local

> 
> Thanks in advance,
> 
>>
>> $ 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

Then I ran the commands above which create a single bridge with "normal"
action and 1 internal port that gets brought up.

That's it.

Regards,
Dumitru

>>
>> 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();
>>>       }
>>>   }
>>>
>>
>>
>
Anton Ivanov Feb. 12, 2020, 7:07 p.m. UTC | #7
On 12/02/2020 15:30, Dumitru Ceara wrote:
> On 2/12/20 4:17 PM, Anton Ivanov wrote:
>>
>> On 12/02/2020 13:41, Dumitru Ceara wrote:
>>> On 2/7/20 9:40 AM, anton.ivanov@cambridgegreys.com wrote:
>>>> From: Anton Ivanov <anton.ivanov@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.ivanov@cambridgegreys.com>
>>> Hi Anton,
>>>
>>> Your patches break OVS. Take the following scenario which adds an OVS
>>> bridge and an internal port to it:
>> Can you send me your ovs-vswitchd  and ovsdb arguments lists to make
>> sure I am testing under the same conditions.
> On a "clean" system, without any pre-existing OVS conf.db I manually
> started OVS:
>
> ovs-ctl start --system-id=local

In addition to me mistakenly adding the netlink fds to the main set which I have fixed in my tree, it appears that the code does not actually handle all epoll events.

There is an epoll event left on one of the existing netlink epolls after the handler runs.

This is presently masked by poll ignoring it until it is asked to wait on it again.

While I can add "one-shot" semantics similar to the ones used for write polling, that is suboptimal as it does not fix the root cause.

I will trace where is the "hanging" netlink epoll event is coming from and see what is the root cause before I send out the next revision.

One comment - AFAIK EPOLL_EXCLUSIVE does not actually guarantee exclusivity. It does a best effort to make it visible on just one, but it may still be visible on multiple epoll fds.

Brgds,

A.


>> Thanks in advance,
>>
>>> $ 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
> Then I ran the commands above which create a single bridge with "normal"
> action and 1 internal port that gets brought up.
>
> That's it.
>
> Regards,
> Dumitru
>
>>> 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();
>>>>        }
>>>>    }
>>>>
>>>
>
Anton Ivanov Feb. 13, 2020, 5:26 p.m. UTC | #8
On 12/02/2020 15:30, Dumitru Ceara wrote:
> On 2/12/20 4:17 PM, Anton Ivanov wrote:
>>
>>
>> On 12/02/2020 13:41, Dumitru Ceara wrote:
>>> On 2/7/20 9:40 AM, anton.ivanov@cambridgegreys.com wrote:
>>>> From: Anton Ivanov <anton.ivanov@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.ivanov@cambridgegreys.com>
>>>
>>> Hi Anton,
>>>
>>> Your patches break OVS. Take the following scenario which adds an OVS
>>> bridge and an internal port to it:
>>
>> Can you send me your ovs-vswitchd  and ovsdb arguments lists to make
>> sure I am testing under the same conditions.
> 
> On a "clean" system, without any pre-existing OVS conf.db I manually
> started OVS:
> 
> ovs-ctl start --system-id=local
> 
>>
>> Thanks in advance,
>>
>>>
>>> $ 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
> 
> Then I ran the commands above which create a single bridge with "normal"
> action and 1 internal port that gets brought up.
> 
> That's it.

Thanks, I have it diagnozed.

The epoll fds which are used to build the dpif-netlink handlers are created in one thread and used in another. As a result, they cannot be registered for persistence upon creation - they end up in the wrong poll loop.

I will sleep on it and fix it tomorrow in a way which does not compromise the performance for the other fds which can be registered at creation.

Brgds,

A.

> 
> Regards,
> Dumitru
> 
>>>
>>> 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();
>>>>        }
>>>>    }
>>>>
>>>
>>>
>>
> 
>
diff mbox series

Patch

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();
     }
 }