[ovs-dev,v2,1/4] Replace direct use of POLLXXX macros with OVS_POLLXXX
diff mbox series

Message ID 20200214175429.26111-1-anton.ivanov@cambridgegreys.com
State New
Headers show
Series
  • [ovs-dev,v2,1/4] Replace direct use of POLLXXX macros with OVS_POLLXXX
Related show

Commit Message

Anton Ivanov Feb. 14, 2020, 5:54 p.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

0-day Robot Feb. 14, 2020, 7 p.m. UTC | #1
Bleep bloop.  Greetings Anton Ivanov, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 83 characters long (recommended limit is 79)
#71 FILE: lib/latch-unix.c:70:
    pfd.events = POLLIN; /* This is POLL specific, it should use POLL only macro */

WARNING: Line is 85 characters long (recommended limit is 79)
#175 FILE: lib/poll-loop.c:83:
 *     OVS_POLLIN or OVS_POLLOUT or OVS_POLLIN | OVS_POLLOUT).  The following call to

WARNING: Line is 83 characters long (recommended limit is 79)
#185 FILE: lib/poll-loop.c:133:
/* Registers 'fd' as waiting for the specified 'events' (which should be OVS_POLLIN

WARNING: Line is 88 characters long (recommended limit is 79)
#186 FILE: lib/poll-loop.c:134:
 * or OVS_POLLOUT or OVS_POLLIN | OVS_POLLOUT).  The following call to poll_block() will

Lines checked: 431, Warnings: 4, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot
Ben Pfaff Feb. 14, 2020, 8:43 p.m. UTC | #2
On Fri, Feb 14, 2020 at 05:54:26PM +0000, 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>

I'm not sure that this level of abstraction is useful or needed,
especially since POLL* and EPOLL* appear to actually have the same
values.  It might be easier to just add some BUILD_ASSERT checks that
this is the case.
Anton Ivanov Feb. 14, 2020, 9:19 p.m. UTC | #3
On 14/02/2020 20:43, Ben Pfaff wrote:
> On Fri, Feb 14, 2020 at 05:54:26PM +0000, 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>
> I'm not sure that this level of abstraction is useful or needed,
> especially since POLL* and EPOLL* appear to actually have the same
> values.  It might be easier to just add some BUILD_ASSERT checks that
> this is the case.

Possible. I will give it a thought.

This was a first approximation -  I was not 100% sure that the "common" 
part will stay within the bits which are common and will not stray into 
the differences (NVAL, RDHUP, etc).

Patch
diff mbox series

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