[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT/LWIP PATCH] sockets: Adopt to updated vfscore



Hello Simon,

Please find the review comment inline.

Thanks & Regards
Sharan

On 2/18/19 3:32 PM, Simon Kuenzer wrote:
Adopts the sockets glue implementation to the updated vfscore library.

Signed-off-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
---
  include/lwipopts.h |  10 ++++
  sockets.c          | 132 +++++++++++++++++++++++++++++++++++++----------------
  2 files changed, 102 insertions(+), 40 deletions(-)

diff --git a/include/lwipopts.h b/include/lwipopts.h
index c94c23d..c72c161 100644
--- a/include/lwipopts.h
+++ b/include/lwipopts.h
@@ -194,6 +194,16 @@ void sys_free(void *ptr);
  #define FD_SET
  #endif
+/* Stop lwip to provide icovec */
+#include <sys/uio.h>
+
+/*
+ * We need to stop lwip introducing `struct iovec` because it is provided by
+ * our libc already. The only way lwip allows us to do this is to define iovec
+ * as iovec. This keeps depending definitions of lwip still working.
+ */
+#define iovec iovec
+
  /*
   * disable BSD-style socket interface
   * We will provide the layer with socket.c|unikraft
diff --git a/sockets.c b/sockets.c
In functions: accept, bind, shutdown, getpeername, getsockname, getsockopt, setsockopt, connect, listen, recv, recvfrom, send, sendmsg,
sendto

we use the function call the function sock_net_file_get which uses vfscore_get_file to obtain the vfscore_file. The vfscore_get_file uses a reference counting to increment the usage of the vfscore file. So we will have to release the reference counting corresponding to our vfs file descriptor at the end of all these functions. Probably we need to add an fdrop call to these functions. This is probably causing some memory leaks.


index bdf21ad..fd6d61a 100644
--- a/sockets.c
+++ b/sockets.c
@@ -36,6 +36,8 @@
  /* network stub calls */
  #include <sys/time.h>
  #include <vfscore/file.h>
+#include <vfscore/fs.h>
+#include <vfscore/vnode.h>
  #include <uk/alloc.h>
  #include <uk/essentials.h>
  #include <uk/errptr.h>
@@ -68,99 +70,149 @@ EXIT:
        return file;
  }
-static int sock_fd_alloc(struct vfscore_fops *fops, int sock_fd)
+static int sock_fd_alloc(struct vnops *v_op, int sock_fd)
  {
        int ret = 0;
        int vfs_fd;
        struct sock_net_file *file = NULL;
+       struct dentry *s_dentry;
+       struct vnode *s_vnode;
- /* Allocate file descriptor */
+       /* Reserve file descriptor number */
        vfs_fd = vfscore_alloc_fd();
        if (vfs_fd < 0) {
                ret = -ENFILE;
-               LWIP_DEBUGF(SOCKETS_DEBUG, ("failed to allocate socket fd\n"));
-               goto EXIT;
+               LWIP_DEBUGF(SOCKETS_DEBUG,
+                           ("Failed to allocate file descriptor number\n"));
+               goto ERR_EXIT;
        }
- file = uk_malloc(uk_alloc_get_default(), sizeof(*file));
+       /* Allocate file, dentry, and vnode */
+       file = uk_calloc(uk_alloc_get_default(), 1, sizeof(*file));
        if (!file) {
                ret = -ENOMEM;
                LWIP_DEBUGF(SOCKETS_DEBUG,
-                           ("failed to allocate socket fd: no mem\n"));
-               goto UK_MEM_ALLOC_ERR;
+                           ("Failed to allocate socket file: Out of 
memory\n"));
+               goto ERR_MALLOC_FILE;
        }
-       file->vfscore_file.fops = fops;
+       s_dentry = uk_calloc(uk_alloc_get_default(), 1, sizeof(*s_dentry));
+       if (!s_dentry) {
+               ret = -ENOMEM;
+               LWIP_DEBUGF(SOCKETS_DEBUG,
+                           ("Failed to allocate socket dentry: Out of 
memory\n"));
+               goto ERR_MALLOC_DENTRY;
+       }
+       s_vnode = uk_calloc(uk_alloc_get_default(), 1, sizeof(*s_vnode));
+       if (!s_vnode) {
+               ret = -ENOMEM;
+               LWIP_DEBUGF(SOCKETS_DEBUG,
+                           ("Failed to allocate socket vnode: Out of 
memory\n"));
+               goto ERR_MALLOC_VNODE;
+       }
+
+       /* Put things together, and fill out necessary fields */
+       file->vfscore_file.fd = vfs_fd;
+       file->vfscore_file.f_flags = FWRITE | FREAD;
+       file->vfscore_file.f_count = 1;
+       file->vfscore_file.f_dentry = s_dentry;
+
+       s_dentry->d_vnode = s_vnode;
+
+       s_vnode->v_op = v_op;
+       uk_mutex_init(&s_vnode->v_lock);
+       s_vnode->v_refcnt = 1;
+       s_vnode->v_data = file;
+
        file->sock_fd = sock_fd;
-       LWIP_DEBUGF(SOCKETS_DEBUG, ("allocated socket %d (%x)\n",
+       LWIP_DEBUGF(SOCKETS_DEBUG, ("Allocated socket %d (%x)\n",
                                    file->vfscore_file.fd,
                                    file->sock_fd));
+
        /* Storing the information within the vfs structure */
-       vfscore_install_fd(vfs_fd, &file->vfscore_file);
-       ret = vfs_fd;
-EXIT:
-       return ret;
+       ret = vfscore_install_fd(vfs_fd, &file->vfscore_file);
+       if (ret) {
+               LWIP_DEBUGF(SOCKETS_DEBUG,
+                           ("Failed to install socket fd\n"));
+               goto ERR_VFS_INSTALL;
+       }
+
+       /* Return file descriptor of our socket */
+       return vfs_fd;
-UK_MEM_ALLOC_ERR:
+ERR_VFS_INSTALL:
+       uk_free(uk_alloc_get_default(), s_vnode);
+ERR_MALLOC_VNODE:
+       uk_free(uk_alloc_get_default(), s_dentry);
+ERR_MALLOC_DENTRY:
+       uk_free(uk_alloc_get_default(), file);
+ERR_MALLOC_FILE:
        vfscore_put_fd(vfs_fd);
-       goto EXIT;
+ERR_EXIT:
+       UK_ASSERT(ret < 0);
+       return ret;
  }
-static int sock_net_close(struct vfscore_file *vfscore_file)
+static int sock_net_close(struct vnode *s_vnode,
+                         struct vfscore_file *vfscore_file __maybe_unused)
  {
        int ret = 0;
        struct sock_net_file *file = NULL;
- file = __containerof(vfscore_file, struct sock_net_file, vfscore_file);
-
+       file = s_vnode->v_data;
        LWIP_DEBUGF(SOCKETS_DEBUG, ("close %d (%x)\n",
                                    file->vfscore_file.fd,
                                    file->sock_fd));
+ UK_ASSERT(vfscore_file->f_dentry->d_vnode == s_vnode);
+
        /* Close and release the lwip socket */
        ret = lwip_close(file->sock_fd);
-       /* Release the file descriptor */
+       /* Release the file descriptor number */
        vfscore_put_fd(file->vfscore_file.fd);
-       /* Free the sock net structure */
+       /* Free socket vnode */
+       uk_free(uk_alloc_get_default(), file->vfscore_file.f_dentry->d_vnode);
+       /* Free socket dentry */
+       uk_free(uk_alloc_get_default(), file->vfscore_file.f_dentry);
+       /* Free socket file */
        uk_free(uk_alloc_get_default(), file);
        return ret;
  }
-static ssize_t sock_net_write(struct vfscore_file *vfscore_file,
-                             const void *buf, size_t count)
+static ssize_t sock_net_write(struct vnode *s_vnode,
+                             struct uio *buf, int ioflag __unused)
  {
        int ret = 0;
        struct sock_net_file *file = NULL;
- file = __containerof(vfscore_file, struct sock_net_file,
-                            vfscore_file);
-       LWIP_DEBUGF(SOCKETS_DEBUG, ("write %d (%x):%s\n",
+       file = s_vnode->v_data;
+       LWIP_DEBUGF(SOCKETS_DEBUG, ("%s fd:%d lwip_fd:%d\n",
+                                   __func__,
                                    file->vfscore_file.fd,
-                                   file->sock_fd,
-                                   (char *) buf));
-       ret = lwip_write(file->sock_fd, buf, count);
+                                   file->sock_fd));
+       ret = lwip_writev(file->sock_fd, buf->uio_iov, buf->uio_iovcnt);
        return ret;
  }
-static ssize_t sock_net_read(struct vfscore_file *vfscore_file, void *buf,
-                           size_t count)
+static ssize_t sock_net_read(struct vnode *s_vnode,
+                            struct vfscore_file *vfscore_file __unused,
+                            struct uio *buf, int ioflag __unused)
  {
        int ret = 0;
        struct sock_net_file *file = NULL;
- file = __containerof(vfscore_file, struct sock_net_file,
-                            vfscore_file);
-       LWIP_DEBUGF(SOCKETS_DEBUG, ("write %d (%x):%s\n",
+       file = s_vnode->v_data;
+       LWIP_DEBUGF(SOCKETS_DEBUG, ("%s fd:%d lwip_fd:%d\n",
+                                   __func__,
                                    file->vfscore_file.fd,
-                                   file->sock_fd,
-                                   (char *) buf));
-       ret = lwip_read(file->sock_fd, buf, count);
+                                   file->sock_fd));
+       ret = lwip_readv(file->sock_fd, buf->uio_iov, buf->uio_iovcnt);
        return ret;
  }
-static struct vfscore_fops sock_net_fops = {
-       .close = sock_net_close,
-       .write = sock_net_write,
-       .read  = sock_net_read,
+static struct vnops sock_net_fops = {
+       .vop_close = sock_net_close,
+       .vop_write = sock_net_write,
+       .vop_read  = sock_net_read,
  };
int socket(int domain, int type, int protocol)


_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.