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

  From: Arne Schwabe <arne@rfc2549.org>

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: Arne Schwabe <arne@rfc2549.org>
Signed-off-by: Antonio Quartulli <a@unstable.cc>
---
 src/openvpn/event.h   | 37 +++++++++++++++++++++++++++++++++++++
 src/openvpn/forward.c | 14 ++++++++------
 src/openvpn/openvpn.h | 12 +-----------
 3 files changed, 46 insertions(+), 17 deletions(-)
  

Am 26.04.21 um 15:26 schrieb Antonio Quartulli:
> From: Arne Schwabe <arne@rfc2549.org>
> 
> 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: Arne Schwabe <arne@rfc2549.org>
> Signed-off-by: Antonio Quartulli <a@unstable.cc>

Thanks for picking it up and getting it in shape. It probably should now
say From: Antonio Quartulli <a@unstable.cc> as that is now most of the
code. That also looks better on the commit log as it doesn't look like I
acked my own patch.

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

Hi,

unfortunately something is not right.
Additional tests showed that something broke after applying this patch.

Will debug and provide a new version.

Regards,

On 26/04/2021 15:26, Antonio Quartulli wrote:
> From: Arne Schwabe <arne@rfc2549.org>
> 
> 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: Arne Schwabe <arne@rfc2549.org>
> Signed-off-by: Antonio Quartulli <a@unstable.cc>
> ---
>  src/openvpn/event.h   | 37 +++++++++++++++++++++++++++++++++++++
>  src/openvpn/forward.c | 14 ++++++++------
>  src/openvpn/openvpn.h | 12 +-----------
>  3 files changed, 46 insertions(+), 17 deletions(-)
> 
> diff --git a/src/openvpn/event.h b/src/openvpn/event.h
> index 4af6371e..67baea3e 100644
> --- a/src/openvpn/event.h
> +++ b/src/openvpn/event.h
> @@ -35,6 +35,43 @@
>  #define EVENT_UNDEF    4
>  #define EVENT_READ     (1<<0)
>  #define EVENT_WRITE    (1<<1)
> +
> +/* 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 EVENT_SHIFT(_name, _op)   (1 << (_name ## _SHIFT + EVENT_ ## _op))
> +
> +#define SOCKET_SHIFT        0
> +#define SOCKET_READ         EVENT_SHIFT(SOCKET, READ)
> +#define SOCKET_WRITE        EVENT_SHIFT(SOCKET, WRITE)
> +#define TUN_SHIFT           2
> +#define TUN_READ            EVENT_SHIFT(TUN, READ)
> +#define TUN_WRITE           EVENT_SHIFT(TUN, WRITE)
> +#define ERR_SHIFT           4
> +#define ES_ERROR            EVENT_SHIFT(ERR, READ)
> +#define ES_TIMEOUT          EVENT_SHIFT(ERR, WRITE)
> +#define MANAGEMENT_SHIFT    6
> +#define MANAGEMENT_READ     EVENT_SHIFT(MANAGEMENT, READ)
> +#define MANAGEMENT_WRITE    EVENT_SHIFT(MANAGEMENT, WRITE)
> +#define FILE_SHIFT          8
> +#define FILE_CLOSED         EVENT_SHIFT(FILE, READ)
> +
>  /*
>   * 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 */
>