[ovs-dev] [PATCH] Add stream-fd implementation for windows. Need to use send/recv instead of write/read operations. Use the event handle for polling. check windows return errors.

Linda Sun lsun at vmware.com
Thu Feb 6 22:30:25 UTC 2014



----- Original Message -----
From: "Gurucharan Shetty" <shettyg at nicira.com>
To: "Linda Sun" <lsun at vmware.com>
Cc: "dev" <dev at openvswitch.org>
Sent: Tuesday, February 4, 2014 2:14:13 PM
Subject: Re: [ovs-dev] [PATCH] Add stream-fd implementation for windows. Need to use send/recv instead of write/read operations. Use the event handle for polling. check windows return errors.

On Mon, Jan 27, 2014 at 11:07 AM, Linda Sun <lsun at vmware.com> wrote:
> ---
>  lib/automake.mk         |    9 +-
>  lib/stream-fd-windows.c |  278 +++++++++++++++++++++++++++++++++++++++++++++++
>  lib/stream.c            |    4 +
>  3 files changed, 288 insertions(+), 3 deletions(-)
>  create mode 100644 lib/stream-fd-windows.c
>
> diff --git a/lib/automake.mk b/lib/automake.mk
> index 94ba060..899a479 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -189,7 +189,6 @@ lib_libopenvswitch_la_SOURCES = \
>         lib/sset.h \
>         lib/stp.c \
>         lib/stp.h \
> -       lib/stream-fd.c \
>         lib/stream-fd.h \
>         lib/stream-provider.h \
>         lib/stream-ssl.h \
> @@ -237,9 +236,13 @@ lib_libopenvswitch_la_SOURCES = \
>         lib/vtep-idl.c \
>         lib/vtep-idl.h
>  if WIN32
> -lib_libopenvswitch_la_SOURCES += lib/latch-windows.c
> +lib_libopenvswitch_la_SOURCES += \
> +       lib/latch-windows.c \
> +       lib/stream-fd-windows.c
>  else
> -lib_libopenvswitch_la_SOURCES += lib/latch.c
> +lib_libopenvswitch_la_SOURCES += \
> +       lib/latch.c \
> +       lib/stream-fd.c
>  endif
>
>  EXTRA_DIST += \
> diff --git a/lib/stream-fd-windows.c b/lib/stream-fd-windows.c
> new file mode 100644
> index 0000000..472d491
> --- /dev/null
> +++ b/lib/stream-fd-windows.c
> @@ -0,0 +1,278 @@
> +/*
> + * Copyright (c) 2013 Nicira, Inc.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     https://urldefense.proofpoint.com/v1/url?u=http://www.apache.org/licenses/LICENSE-2.0&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=0mp263sOpVS3FERoXOmmtg%3D%3D%0A&m=E8%2BwcUEFEBhqcn9tK%2FeNEOjyrsnuOiyA6R12hxgdcuA%3D%0A&s=027e67542af0b209541411bdaf8b6ced94aa9b7c50b8486c9cc3c9ffd5ea5040
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <config.h>
> +#include "stream-fd.h"
> +#include <errno.h>
> +#include <poll.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <winsock2.h>
> +
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include "fatal-signal.h"
> +#include "poll-loop.h"
> +#include "socket-util.h"
> +#include "util.h"
> +#include "stream-provider.h"
> +#include "stream.h"
> +#include "vlog.h"
> +
> +VLOG_DEFINE_THIS_MODULE(stream_fd_windows);
> +
> +/* Active file descriptor stream. */
> +
> +struct stream_fd
> +{
> +    struct stream stream;
> +    int fd;
> +    HANDLE wevent;
> +};
> +
> +static const struct stream_class stream_fd_class;
> +
> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(10, 25);
> +
> +/* Creates a new stream named 'name' that will send and receive data on 'fd'
> + * and stores a pointer to the stream in '*streamp'.  Initial connection status
> + * 'connect_status' is interpreted as described for stream_init().
> + *
> + * Returns 0 if successful, otherwise a positive errno value.  (The current
> + * implementation never fails.) */
> +int
> +new_fd_stream(const char *name, int fd, int connect_status,
> +              struct stream **streamp)
> +{
> +    struct stream_fd *s;
> +
> +    s = xmalloc(sizeof *s);
> +    stream_init(&s->stream, &stream_fd_class, connect_status, name);
> +    s->fd = fd;
> +    s->wevent = CreateEvent(NULL, FALSE, FALSE, NULL);
> +    WSAEventSelect(s->fd, s->wevent, FD_ALL_EVENTS);
I do not see FD_ALL_EVENTS defined in Microsoft documentation here:
https://urldefense.proofpoint.com/v1/url?u=http://msdn.microsoft.com/en-us/library/windows/desktop/ms741576%28v%3Dvs.85%29.aspx&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=0mp263sOpVS3FERoXOmmtg%3D%3D%0A&m=E8%2BwcUEFEBhqcn9tK%2FeNEOjyrsnuOiyA6R12hxgdcuA%3D%0A&s=b5fa77097f6e52e97b056ea08d241a047c95bfed2a6f2f631a38b108cd3682be

Is it defined somewhere?

<LS> I found it under WinSock2.h:

#define FD_ALL_EVENTS    ((1 << FD_MAX_EVENTS) - 1)


> +    *streamp = &s->stream;
> +    return 0;
> +}
> +
> +static struct stream_fd *
> +stream_fd_cast(struct stream *stream)
> +{
> +    stream_assert_class(stream, &stream_fd_class);
> +    return CONTAINER_OF(stream, struct stream_fd, stream);
> +}
> +
> +static void
> +fd_close(struct stream *stream)
> +{
> +    struct stream_fd *s = stream_fd_cast(stream);
> +    WSAEventSelect(s->fd, NULL, 0);
> +    CloseHandle(s->wevent);
> +    closesocket(s->fd);
> +    free(s);
> +}
> +
> +static int
> +fd_connect(struct stream *stream)
> +{
> +    struct stream_fd *s = stream_fd_cast(stream);
> +    return check_connection_completion(s->fd);
> +}
> +
> +static ssize_t
> +fd_recv(struct stream *stream, void *buffer, size_t n)
> +{
> +    struct stream_fd *s = stream_fd_cast(stream);
> +    ssize_t retval;
> +
> +    retval = recv(s->fd, buffer, n, 0);
> +    if (retval < 0) {
> +        retval = -WSAGetLastError();
> +    }
> +    if (retval == - WSAEWOULDBLOCK) {
> +        return -EAGAIN;
> +    }
> +    return retval;
> +}
> +
> +static ssize_t
> +fd_send(struct stream *stream, const void *buffer, size_t n)
> +{
> +    struct stream_fd *s = stream_fd_cast(stream);
> +    ssize_t retval;
> +
> +    retval = send(s->fd, buffer, n, 0);
> +    if (retval < 0) {
> +        retval = -WSAGetLastError();
> +    }
> +    if (retval == -WSAEWOULDBLOCK) {
> +       return -EAGAIN;
> +    }
> +
> +    return retval;
> +}
> +
> +static void
> +fd_wait(struct stream *stream, enum stream_wait_type wait)
> +{
> +    struct stream_fd *s = stream_fd_cast(stream);
> +    switch (wait) {
> +    case STREAM_CONNECT:
> +    case STREAM_SEND:
> +        poll_fd_wait_event(s->fd, s->wevent, POLLOUT);
I see that POLLOUT. POLLIN etc are not actually used in
WaitForMultipleObjects() call. So, I presume, you will be woken up for
any event? I guess, that is bad?
<LS> I tried a combination of FD_READ/FD_WRITE/FD_ACCEPT/FD_CONNECT and I'm not waken up.  For now I'll just wake up on any socket events.  We can improve this later.

> +        break;
> +
> +    case STREAM_RECV:
> +        poll_fd_wait_event(s->fd, s->wevent, POLLIN);
> +        break;
> +
> +    default:
> +        NOT_REACHED();
> +    }
> +}
> +
> +static const struct stream_class stream_fd_class = {
> +    "fd",                       /* name */
> +    false,                      /* needs_probes */
> +    NULL,                       /* open */
> +    fd_close,                   /* close */
> +    fd_connect,                 /* connect */
> +    fd_recv,                    /* recv */
> +    fd_send,                    /* send */
> +    NULL,                       /* run */
> +    NULL,                       /* run_wait */
> +    fd_wait,                    /* wait */
> +};
> +
> +/* Passive file descriptor stream. */
> +
> +struct fd_pstream
> +{
> +    struct pstream pstream;
> +    int fd;
> +    HANDLE wevent;
> +    int (*accept_cb)(int fd, const struct sockaddr *, size_t sa_len,
> +                     struct stream **);
> +    int (*set_dscp_cb)(int fd, uint8_t dscp);
> +    char *unlink_path;
> +};
> +
> +static const struct pstream_class fd_pstream_class;
> +
> +static struct fd_pstream *
> +fd_pstream_cast(struct pstream *pstream)
> +{
> +    pstream_assert_class(pstream, &fd_pstream_class);
> +    return CONTAINER_OF(pstream, struct fd_pstream, pstream);
> +}
> +
> +/* Creates a new pstream named 'name' that will accept new socket connections
> + * on 'fd' and stores a pointer to the stream in '*pstreamp'.
> + *
> + * When a connection has been accepted, 'accept_cb' will be called with the new
> + * socket fd 'fd' and the remote address of the connection 'sa' and 'sa_len'.
> + * accept_cb must return 0 if the connection is successful, in which case it
> + * must initialize '*streamp' to the new stream, or a positive errno value on
> + * error.  In either case accept_cb takes ownership of the 'fd' passed in.
> + *
> + * When '*pstreamp' is closed, then 'unlink_path' (if nonnull) will be passed
> + * to fatal_signal_unlink_file_now() and freed with free().
> + *
> + * Returns 0 if successful, otherwise a positive errno value.  (The current
> + * implementation never fails.) */
> +int
> +new_fd_pstream(const char *name, int fd,
> +               int (*accept_cb)(int fd, const struct sockaddr *sa,
> +                                size_t sa_len, struct stream **streamp),
> +               int (*set_dscp_cb)(int fd, uint8_t dscp),
> +               char *unlink_path, struct pstream **pstreamp)
> +{
> +    struct fd_pstream *ps = xmalloc(sizeof *ps);
> +    pstream_init(&ps->pstream, &fd_pstream_class, name);
> +    ps->fd = fd;
> +    ps->wevent = CreateEvent(NULL, FALSE, FALSE, NULL);
> +    WSAEventSelect(ps->fd, ps->wevent, FD_ALL_EVENTS);
> +    ps->accept_cb = accept_cb;
> +    ps->set_dscp_cb = set_dscp_cb;
> +    ps->unlink_path = unlink_path;
> +    *pstreamp = &ps->pstream;
> +    return 0;
> +}
> +
> +static void
> +pfd_close(struct pstream *pstream)
> +{
> +    struct fd_pstream *ps = fd_pstream_cast(pstream);
> +    close(ps->fd);
Should this be closesocket() ?
<LS> fixed.
> +    free(ps);
> +}
> +
> +static int
> +pfd_accept(struct pstream *pstream, struct stream **new_streamp)
> +{
> +    struct fd_pstream *ps = fd_pstream_cast(pstream);
> +    struct sockaddr_storage ss;
> +    socklen_t ss_len = sizeof ss;
> +    int new_fd;
> +    int retval;
> +
> +
> +       new_fd = accept(ps->fd, (struct sockaddr *) &ss, &ss_len);
> +    if (new_fd < 0) {
> +        retval = WSAGetLastError();
> +        if (retval == WSAEWOULDBLOCK) {
> +            return EAGAIN;
> +        }
> +        return retval;
> +    }
> +
> +    retval = set_nonblocking(new_fd);
> +    if (retval) {
> +        close(new_fd);
> +        return retval;
> +    }
> +
> +    return ps->accept_cb(new_fd, (const struct sockaddr *) &ss, ss_len,
> +                         new_streamp);
> +}
> +
> +static void
> +pfd_wait(struct pstream *pstream)
> +{
> +    struct fd_pstream *ps = fd_pstream_cast(pstream);
> +    poll_fd_wait_event(ps->fd, ps->wevent, POLLIN);
> +}
> +
> +static int
> +pfd_set_dscp(struct pstream *pstream, uint8_t dscp)
> +{
> +    struct fd_pstream *ps = fd_pstream_cast(pstream);
> +    if (ps->set_dscp_cb) {
> +        return ps->set_dscp_cb(ps->fd, dscp);
> +    }
> +    return 0;
> +}
> +
> +static const struct pstream_class fd_pstream_class = {
> +    "pstream",
> +    false,
> +    NULL,
> +    pfd_close,
> +    pfd_accept,
> +    pfd_wait,
> +    pfd_set_dscp,
> +};
> +
> diff --git a/lib/stream.c b/lib/stream.c
> index 3542455..a45ed4f 100644
> --- a/lib/stream.c
> +++ b/lib/stream.c
> @@ -51,7 +51,9 @@ enum stream_state {
>
>  static const struct stream_class *stream_classes[] = {
>      &tcp_stream_class,
> +#ifndef _WIN32
>      &unix_stream_class,
> +#endif
>  #ifdef HAVE_OPENSSL
>      &ssl_stream_class,
>  #endif
> @@ -59,7 +61,9 @@ static const struct stream_class *stream_classes[] = {
>
>  static const struct pstream_class *pstream_classes[] = {
>      &ptcp_pstream_class,
> +#ifndef _WIN32
>      &punix_pstream_class,
> +#endif
>  #ifdef HAVE_OPENSSL
>      &pssl_pstream_class,
>  #endif
> --
> 1.7.9.5
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=0mp263sOpVS3FERoXOmmtg%3D%3D%0A&m=E8%2BwcUEFEBhqcn9tK%2FeNEOjyrsnuOiyA6R12hxgdcuA%3D%0A&s=a78db0b2caf9ec1e63b4dbe48e4c6b8c32f833cbdb09846d3e428097d2fd282c



More information about the dev mailing list