Re: [Openvpn-devel] [PATCH v3] tun: remove tun_finalize()

2022-01-20 Thread Selva Nair
Hi,

On Mon, Jan 17, 2022 at 4:51 AM Lev Stipakov  wrote:

> From: Lev Stipakov 
>
> tun_finalize() is essentially subset of socket_finalize() apart from:
>
>  - using WSAFoo() functions instead of Foo()
>
>  - "from" address is not returned
>
> There is no clear official statement that one can use non-WSA
> API on handles, so let's be on a safe side and use both.
>
> Introduce sockethandle_t abstraction, which represents
> socket and handle. Add SocketHandle* routines which call
> proper API depends on underlying type in abstraction.
>
> Rename socket_finalize() to sockethandle_finalize(), take
> sockethandle_t and new routines into use and kick tun_finalize().
>
> Signed-off-by: Lev Stipakov 
> ---
>  v3: replace macros with inline functions
>
>  v2: explicitly initialize .is_handle to false for readablity
>
>
>  src/openvpn/forward.c |  3 +-
>  src/openvpn/socket.c  | 37 -
>  src/openvpn/socket.h  | 49 ++
>  src/openvpn/tun.c | 94 +--
>  src/openvpn/tun.h | 34 +---
>  5 files changed, 80 insertions(+), 137 deletions(-)
>
> diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
> index f82386a1..a905f993 100644
> --- a/src/openvpn/forward.c
> +++ b/src/openvpn/forward.c
> @@ -1115,7 +1115,8 @@ read_incoming_tun(struct context *c)
>  }
>  else
>  {
> -read_tun_buffered(c->c1.tuntap, >c2.buf);
> +sockethandle_t sh = { .is_handle = true, .h = c->c1.tuntap->hand
> };
> +sockethandle_finalize(sh, >c1.tuntap->reads, >c2.buf, NULL);
>  }
>  #else  /* ifdef _WIN32 */
>  ASSERT(buf_init(>c2.buf, FRAME_HEADROOM(>c2.frame)));
> diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
> index df736746..780c5cb3 100644
> --- a/src/openvpn/socket.c
> +++ b/src/openvpn/socket.c
> @@ -3198,7 +3198,8 @@ link_socket_read_tcp(struct link_socket *sock,
>  if (!sock->stream_buf.residual_fully_formed)
>  {
>  #ifdef _WIN32
> -len = socket_finalize(sock->sd, >reads, buf, NULL);
> +sockethandle_t sh = { .s = sock->sd };
> +len = sockethandle_finalize(sh, >reads, buf, NULL);
>  #else
>  struct buffer frag;
>  stream_buf_get_next(>stream_buf, );
> @@ -3664,10 +3665,10 @@ socket_send_queue(struct link_socket *sock, struct
> buffer *buf, const struct lin
>  }
>
>  int
> -socket_finalize(SOCKET s,
> -struct overlapped_io *io,
> -struct buffer *buf,
> -struct link_socket_actual *from)
> +sockethandle_finalize(sockethandle_t sh,
> +  struct overlapped_io *io,
> +  struct buffer *buf,
> +  struct link_socket_actual *from)
>  {
>  int ret = -1;
>  BOOL status;
> @@ -3675,13 +3676,7 @@ socket_finalize(SOCKET s,
>  switch (io->iostate)
>  {
>  case IOSTATE_QUEUED:
> -status = WSAGetOverlappedResult(
> -s,
> ->overlapped,
> ->size,
> -FALSE,
> ->flags
> -);
> +status = SocketHandleGetOverlappedResult(sh, io);
>  if (status)
>  {
>  /* successful return for a queued operation */
> @@ -3693,18 +3688,18 @@ socket_finalize(SOCKET s,
>  io->iostate = IOSTATE_INITIAL;
>  ASSERT(ResetEvent(io->overlapped.hEvent));
>
> -dmsg(D_WIN32_IO, "WIN32 I/O: Socket Completion success
> [%d]", ret);
> +dmsg(D_WIN32_IO, "WIN32 I/O: Completion success [%d]",
> ret);
>  }
>  else
>  {
>  /* error during a queued operation */
>  ret = -1;
> -if (WSAGetLastError() != WSA_IO_INCOMPLETE)
> +if (SocketHandleGetLastError(sh) != ERROR_IO_INCOMPLETE)
>

WSA_IO_INCOMPLETE is the same as ERROR_IO_INCOMPLETE, so using them
interchangeably looks ok.

 {
>  /* if no error (i.e. just not finished yet), then
> DON'T execute this code */
>  io->iostate = IOSTATE_INITIAL;
>  ASSERT(ResetEvent(io->overlapped.hEvent));
> -msg(D_WIN32_IO | M_ERRNO, "WIN32 I/O: Socket
> Completion error");
> +msg(D_WIN32_IO | M_ERRNO, "WIN32 I/O: Completion
> error");
>  }
>  }
>  break;
> @@ -3715,9 +3710,9 @@ socket_finalize(SOCKET s,
>  if (io->status)
>  {
>  /* error return for a non-queued operation */
> -WSASetLastError(io->status);
> +SocketHandleSetLastError(sh, io->status);
>  ret = -1;
> -msg(D_WIN32_IO | M_ERRNO, "WIN32 I/O: Socket Completion
> non-queued error");
> +msg(D_WIN32_IO | M_ERRNO, "WIN32 I/O: Completion
> non-queued error");
>  }
>  else
>  

Re: [Openvpn-devel] [PATCH v3] tun: remove tun_finalize()

2022-01-19 Thread Lev Stipakov
Gentle nudge.

ma 17. tammik. 2022 klo 13.42 Gert Doering (g...@greenie.muc.de) kirjoitti:
>
> Hi,
>
> On Mon, Jan 17, 2022 at 11:56:51AM +0200, Lev Stipakov wrote:
> > This is probably something committer (looks at cron2) could fix on the
> > fly, unless there are more issues which would require v4?
>
> If that's the only thing, I can fix that on the fly.
>
> gert
> --
> "If was one thing all people took for granted, was conviction that if you
>  feed honest figures into a computer, honest figures come out. Never doubted
>  it myself till I met a computer with a sense of humor."
>  Robert A. Heinlein, The Moon is a Harsh Mistress
>
> Gert Doering - Munich, Germany g...@greenie.muc.de



-- 
-Lev


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v3] tun: remove tun_finalize()

2022-01-17 Thread Gert Doering
Hi,

On Mon, Jan 17, 2022 at 11:56:51AM +0200, Lev Stipakov wrote:
> This is probably something committer (looks at cron2) could fix on the
> fly, unless there are more issues which would require v4?

If that's the only thing, I can fix that on the fly.

gert
-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v3] tun: remove tun_finalize()

2022-01-17 Thread Lev Stipakov
This is probably something committer (looks at cron2) could fix on the
fly, unless there are more issues which would require v4?

ma 17. tammik. 2022 klo 11.53 Antonio Quartulli (a...@unstable.cc) kirjoitti:
>
> Hi,
>
> On 17/01/2022 10:49, Lev Stipakov wrote:
> [cut]
> > -
> > -static inline int
> > -read_tun_buffered(struct tuntap *tt, struct buffer *buf)
> > -{
> > -return tun_finalize(tt->hand, >reads, buf);
> > -}
> > +int
> > +tun_write_win32(struct tuntap *tt, struct buffer *buf);
>
> for prototypes we put the return type on the same line as the function
> name (unless we have line length issues, but not in this case).
>
> Cheers,
>
>
> --
> Antonio Quartulli



-- 
-Lev


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v3] tun: remove tun_finalize()

2022-01-17 Thread Antonio Quartulli

Hi,

On 17/01/2022 10:49, Lev Stipakov wrote:
[cut]

-
-static inline int
-read_tun_buffered(struct tuntap *tt, struct buffer *buf)
-{
-return tun_finalize(tt->hand, >reads, buf);
-}
+int
+tun_write_win32(struct tuntap *tt, struct buffer *buf);


for prototypes we put the return type on the same line as the function 
name (unless we have line length issues, but not in this case).


Cheers,


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel