[Openvpn-devel,v5] Add documentation on EVENT_READ/EVENT_WRITE constants

Message ID 20210427191314.21317-1-a@unstable.cc
State Accepted
Headers show
Series
  • [Openvpn-devel,v5] Add documentation on EVENT_READ/EVENT_WRITE constants
Related show

Commit Message

Antonio Quartulli April 27, 2021, 7:13 p.m.
From: Antonio Quartulli <antonio@openvpn.net>

Changes from v4:
- get rid of the overly complex EVENT_SHIFT() macro

Changes from v3:
- re-introduce READ/WRITE_SHIFT because they are different from EVENT_READ/WRITE
- define also EVENT_READ/WRITE using READ/WRITE_SHIFT

Changes from v2:
- moved event definitions to event.h
- removed READ/WRITE_SHIFT and use EVENT_READ/WRITE
- removed ifdefs around *_SHIFTS definitions in event.h

Changes from v1:
- fixed typ0s
- extended comment
- moved *_SHIFT definition to openvpn.h
- made READ/WRITE events dependant on _SHIFT definition with a macro

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 src/openvpn/event.h   | 44 ++++++++++++++++++++++++++++++++++++++++---
 src/openvpn/forward.c | 14 ++++++++------
 src/openvpn/openvpn.h | 12 +-----------
 3 files changed, 50 insertions(+), 20 deletions(-)

Comments

Arne Schwabe May 2, 2021, 12:28 p.m. | #1
Am 27.04.21 um 21:13 schrieb Antonio Quartulli:
> From: Antonio Quartulli <antonio@openvpn.net>
> 
> Changes from v4:
> - get rid of the overly complex EVENT_SHIFT() macro
> 
> Changes from v3:
> - re-introduce READ/WRITE_SHIFT because they are different from EVENT_READ/WRITE
> - define also EVENT_READ/WRITE using READ/WRITE_SHIFT
> 
> Changes from v2:
> - moved event definitions to event.h
> - removed READ/WRITE_SHIFT and use EVENT_READ/WRITE
> - removed ifdefs around *_SHIFTS definitions in event.h
> 
> Changes from v1:
> - fixed typ0s
> - extended comment
> - moved *_SHIFT definition to openvpn.h
> - made READ/WRITE events dependant on _SHIFT definition with a macro

Acked-By: Arne Schwabe <arne@rfc2549.org>

Arne
Gert Doering May 2, 2021, 12:42 p.m. | #2
I've done a quick stare-on-code and ran the client side tests on FreeBSD,
and all worked.

Your patch has been applied to the master branch.

commit c2912912047fdd228faedb6466672288e14dbba6
Author: Antonio Quartulli
Date:   Tue Apr 27 21:13:14 2021 +0200

     Add documentation on EVENT_READ/EVENT_WRITE constants

     Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
     Acked-by: Arne Schwabe <arne@rfc2549.org>
     Message-Id: <20210427191314.21317-1-a@unstable.cc>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22247.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/event.h b/src/openvpn/event.h
index 4af6371e..ce9c31a2 100644
--- a/src/openvpn/event.h
+++ b/src/openvpn/event.h
@@ -32,9 +32,47 @@ 
  * rwflags passed to event_ctl and returned by
  * struct event_set_return.
  */
-#define EVENT_UNDEF    4
-#define EVENT_READ     (1<<0)
-#define EVENT_WRITE    (1<<1)
+#define READ_SHIFT      0
+#define WRITE_SHIFT     1
+
+#define EVENT_UNDEF     4
+#define EVENT_READ      (1 << READ_SHIFT)
+#define EVENT_WRITE     (1 << WRITE_SHIFT)
+
+/* event flags returned by io_wait.
+ *
+ * All these events are defined as bits in a bitfield.
+ * Each event 'type' owns two bits in the bitfield: one for the READ
+ * event and one for the WRITE event.
+ *
+ * For this reason, the specific event bit is calculated by adding
+ * the event type identifier (always a multiple of 2, as defined
+ * below) to 0 for READ and 1 for WRITE.
+ *
+ * E.g.
+ * MANAGEMENT_SHIFT = 6;  <---- event type identifier
+ * MANAGEMENT_READ = (1 << (6 + 0)),  <---- READ event
+ * MANAGEMENT_WRITE = (1 << (6 + 1))  <---- WRITE event
+ *
+ * 'error' and 'file_close' are special and use read/write for different
+ * signals.
+ */
+
+#define SOCKET_SHIFT        0
+#define SOCKET_READ         (1 << (SOCKET_SHIFT + READ_SHIFT))
+#define SOCKET_WRITE        (1 << (SOCKET_SHIFT + WRITE_SHIFT))
+#define TUN_SHIFT           2
+#define TUN_READ            (1 << (TUN_SHIFT + READ_SHIFT))
+#define TUN_WRITE           (1 << (TUN_SHIFT + WRITE_SHIFT))
+#define ERR_SHIFT           4
+#define ES_ERROR            (1 << (ERR_SHIFT + READ_SHIFT))
+#define ES_TIMEOUT          (1 << (ERR_SHIFT + WRITE_SHIFT))
+#define MANAGEMENT_SHIFT    6
+#define MANAGEMENT_READ     (1 << (MANAGEMENT_SHIFT + READ_SHIFT))
+#define MANAGEMENT_WRITE    (1 << (MANAGEMENT_SHIFT + WRITE_SHIFT))
+#define FILE_SHIFT          8
+#define FILE_CLOSED         (1 << (FILE_SHIFT + READ_SHIFT))
+
 /*
  * Initialization flags passed to event_set_init
  */
diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index df96d048..e302d8e0 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -1867,15 +1867,17 @@  io_wait_dowork(struct context *c, const unsigned int flags)
     unsigned int tuntap = 0;
     struct event_set_return esr[4];
 
-    /* These shifts all depend on EVENT_READ and EVENT_WRITE */
-    static int socket_shift = 0;   /* depends on SOCKET_READ and SOCKET_WRITE */
-    static int tun_shift = 2;      /* depends on TUN_READ and TUN_WRITE */
-    static int err_shift = 4;      /* depends on ES_ERROR */
+    /* These shifts all depend on EVENT_READ (=1) and EVENT_WRITE (=2)
+     * and are added to the shift. Check openvpn.h for more details.
+     */
+    static int socket_shift = SOCKET_SHIFT;
+    static int tun_shift = TUN_SHIFT;
+    static int err_shift = ERR_SHIFT;
 #ifdef ENABLE_MANAGEMENT
-    static int management_shift = 6; /* depends on MANAGEMENT_READ and MANAGEMENT_WRITE */
+    static int management_shift = MANAGEMENT_SHIFT;
 #endif
 #ifdef ENABLE_ASYNC_PUSH
-    static int file_shift = 8;     /* listening inotify events */
+    static int file_shift = FILE_SHIFT;
 #endif
 
     /*
diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h
index a15ff08d..6e4651cf 100644
--- a/src/openvpn/openvpn.h
+++ b/src/openvpn/openvpn.h
@@ -232,17 +232,7 @@  struct context_2
     int event_set_max;
     bool event_set_owned;
 
-    /* event flags returned by io_wait */
-#define SOCKET_READ       (1<<0)
-#define SOCKET_WRITE      (1<<1)
-#define TUN_READ          (1<<2)
-#define TUN_WRITE         (1<<3)
-#define ES_ERROR          (1<<4)
-#define ES_TIMEOUT        (1<<5)
-#define MANAGEMENT_READ  (1<<6)
-#define MANAGEMENT_WRITE (1<<7)
-#define FILE_CLOSED       (1<<8)
-
+    /* bitmask for event status. Check event.h for possible values */
     unsigned int event_set_status;
 
     struct link_socket *link_socket;     /* socket used for TCP/UDP connection to remote */