From b966b8e63ff71159d55bc4510b4d9d96a01c19db Mon Sep 17 00:00:00 2001 From: Damjan Marion Date: Wed, 13 Sep 2017 21:30:31 +0200 Subject: memif: use clib_socket_t for socket connections This reverts commit 590acf8fa7af6a8604edd72a32f9f087be52c767. new version includes minor fix for the crash when the interface is deleted. Change-Id: I8fc56eb9145e4d8e1d410206f84e705045898608 Signed-off-by: Damjan Marion --- src/plugins/memif/cli.c | 5 +- src/plugins/memif/memif.c | 125 +++++++++++++---------------- src/plugins/memif/private.h | 14 +--- src/plugins/memif/socket.c | 188 ++++++++++++++------------------------------ 4 files changed, 122 insertions(+), 210 deletions(-) diff --git a/src/plugins/memif/cli.c b/src/plugins/memif/cli.c index 3d38550c1ba..29d13310776 100644 --- a/src/plugins/memif/cli.c +++ b/src/plugins/memif/cli.c @@ -309,8 +309,9 @@ memif_show_command_fn (vlib_main_t * vm, unformat_input_t * input, vlib_cli_output (vm, " id %d mode %U file %s", mif->id, format_memif_if_mode, mif, msf->filename); vlib_cli_output (vm, " flags%U", format_memif_if_flags, mif->flags); - vlib_cli_output (vm, " listener-fd %d conn-fd %d", msf->fd, - mif->conn_fd); + vlib_cli_output (vm, " listener-fd %d conn-fd %d", + msf->sock ? msf->sock->fd : 0, + mif->sock ? mif->sock->fd : 0); vlib_cli_output (vm, " num-s2m-rings %u num-m2s-rings %u buffer-size %u", mif->run.num_s2m_rings, mif->run.num_m2s_rings, diff --git a/src/plugins/memif/memif.c b/src/plugins/memif/memif.c index a3be49fa116..aa476be9a98 100644 --- a/src/plugins/memif/memif.c +++ b/src/plugins/memif/memif.c @@ -83,7 +83,7 @@ memif_disconnect (memif_if_t * mif, clib_error_t * err) { clib_error_t *e = 0; mif->local_disc_string = vec_dup (err->what); - if (mif->conn_fd > -1) + if (mif->sock && clib_socket_is_connected (mif->sock)) e = memif_msg_send_disconnect (mif, err); clib_error_free (e); } @@ -94,17 +94,21 @@ memif_disconnect (memif_if_t * mif, clib_error_t * err) vnet_hw_interface_set_flags (vnm, mif->hw_if_index, 0); /* close connection socket */ - if (mif->conn_clib_file_index != ~0) + if (mif->sock && mif->sock->fd) { memif_socket_file_t *msf = vec_elt_at_index (mm->socket_files, mif->socket_file_index); - hash_unset (msf->dev_instance_by_fd, mif->conn_fd); - memif_file_del_by_index (mif->conn_clib_file_index); - mif->conn_clib_file_index = ~0; + hash_unset (msf->dev_instance_by_fd, mif->sock->fd); + memif_socket_close (&mif->sock); + } + else if (mif->sock) + { + clib_error_t *err; + err = clib_socket_close (mif->sock); + if (err) + clib_error_report (err); + clib_mem_free (mif->sock); } - else if (mif->conn_fd > -1) - close (mif->conn_fd); - mif->conn_fd = -1; vec_foreach_index (i, mif->rx_queues) { @@ -137,8 +141,6 @@ memif_disconnect (memif_if_t * mif, clib_error_t * err) close (mr->fd); } vec_free (mif->regions); - - mif->remote_pid = 0; vec_free (mif->remote_name); vec_free (mif->remote_if_name); clib_fifo_free (mif->msg_queue); @@ -362,19 +364,14 @@ memif_process (vlib_main_t * vm, vlib_node_runtime_t * rt, vlib_frame_t * f) { memif_main_t *mm = &memif_main; memif_if_t *mif; - struct sockaddr_un sun; - int sockfd; + clib_socket_t *sock; uword *event_data = 0, event_type; u8 enabled = 0; f64 start_time, last_run_duration = 0, now; + clib_error_t *err; - sockfd = socket (AF_UNIX, SOCK_SEQPACKET, 0); - if (sockfd < 0) - { - DBG_UNIX_LOG ("socket AF_UNIX"); - return 0; - } - sun.sun_family = AF_UNIX; + sock = clib_mem_alloc (sizeof (clib_socket_t)); + memset (sock, 0, sizeof (clib_socket_t)); while (1) { @@ -425,33 +422,29 @@ memif_process (vlib_main_t * vm, vlib_node_runtime_t * rt, vlib_frame_t * f) if (mif->flags & MEMIF_IF_FLAG_IS_SLAVE) { - strncpy (sun.sun_path, (char *) msf->filename, - sizeof (sun.sun_path) - 1); - - if (connect - (sockfd, (struct sockaddr *) &sun, - sizeof (struct sockaddr_un)) == 0) + memset (sock, 0, sizeof(clib_socket_t)); + sock->config = (char *) msf->filename; + sock->flags = CLIB_SOCKET_F_IS_CLIENT| CLIB_SOCKET_F_SEQPACKET; + + if ((err = clib_socket_init (sock))) + { + clib_error_free (err); + } + else { clib_file_t t = { 0 }; - mif->conn_fd = sockfd; t.read_function = memif_slave_conn_fd_read_ready; t.write_function = memif_slave_conn_fd_write_ready; t.error_function = memif_slave_conn_fd_error; - t.file_descriptor = mif->conn_fd; + t.file_descriptor = sock->fd; t.private_data = mif->dev_instance; - memif_file_add (&mif->conn_clib_file_index, &t); - hash_set (msf->dev_instance_by_fd, mif->conn_fd, mif->dev_instance); + memif_file_add (&sock->private_data, &t); + hash_set (msf->dev_instance_by_fd, sock->fd, mif->dev_instance); mif->flags |= MEMIF_IF_FLAG_CONNECTING; - - /* grab another fd */ - sockfd = socket (AF_UNIX, SOCK_SEQPACKET, 0); - if (sockfd < 0) - { - DBG_UNIX_LOG ("socket AF_UNIX"); - return 0; - } + mif->sock = sock; + sock = clib_mem_alloc (sizeof(clib_socket_t)); } } })); @@ -506,18 +499,25 @@ memif_delete_if (vlib_main_t * vm, memif_if_t * mif) { if (msf->is_listener) { - uword *x; - memif_file_del_by_index (msf->clib_file_index); - vec_foreach (x, msf->pending_file_indices) + int i; + vec_foreach_index (i, msf->pending_clients) { - memif_file_del_by_index (*x); + memif_socket_close (msf->pending_clients + i); } - vec_free (msf->pending_file_indices); + memif_socket_close (&msf->sock); + vec_free (msf->pending_clients); } mhash_free (&msf->dev_instance_by_id); hash_free (msf->dev_instance_by_fd); mhash_unset (&mm->socket_file_index_by_filename, msf->filename, 0); vec_free (msf->filename); + if (msf->sock) + { + err = clib_socket_close (msf->sock); + if (err) + clib_error_report (err); + clib_mem_free (msf->sock); + } pool_put (mm->socket_files, msf); } @@ -625,7 +625,6 @@ memif_create_if (vlib_main_t * vm, memif_create_if_args_t * args) sizeof (memif_interface_id_t)); msf->dev_instance_by_fd = hash_create (0, sizeof (uword)); msf->filename = socket_filename; - msf->fd = -1; msf->is_listener = (args->is_master != 0); socket_filename = 0; mhash_set (&mm->socket_file_index_by_filename, msf->filename, @@ -639,8 +638,6 @@ memif_create_if (vlib_main_t * vm, memif_create_if_args_t * args) mif->socket_file_index = msf - mm->socket_files; mif->id = args->id; mif->sw_if_index = mif->hw_if_index = mif->per_interface_next_index = ~0; - mif->conn_clib_file_index = ~0; - mif->conn_fd = -1; mif->mode = args->mode; if (args->secret) mif->secret = vec_dup (args->secret); @@ -701,33 +698,22 @@ memif_create_if (vlib_main_t * vm, memif_create_if_args_t * args) /* If this is new one, start listening */ if (msf->is_listener && msf->ref_cnt == 0) { - struct sockaddr_un un = { 0 }; struct stat file_stat; - int on = 1; + clib_socket_t *s = clib_mem_alloc (sizeof (clib_socket_t)); - if ((msf->fd = socket (AF_UNIX, SOCK_SEQPACKET, 0)) < 0) - { - ret = VNET_API_ERROR_SYSCALL_ERROR_4; - goto error; - } + ASSERT (msf->sock == 0); + msf->sock = s; - un.sun_family = AF_UNIX; - strncpy ((char *) un.sun_path, (char *) msf->filename, - sizeof (un.sun_path) - 1); + memset (s, 0, sizeof (clib_socket_t)); + s->config = (char *) msf->filename; + s->flags = CLIB_SOCKET_F_IS_SERVER | + CLIB_SOCKET_F_ALLOW_GROUP_WRITE | + CLIB_SOCKET_F_SEQPACKET | CLIB_SOCKET_F_PASSCRED; - if (setsockopt (msf->fd, SOL_SOCKET, SO_PASSCRED, &on, sizeof (on)) < 0) + if ((error = clib_socket_init (s))) { - ret = VNET_API_ERROR_SYSCALL_ERROR_5; - goto error; - } - if (bind (msf->fd, (struct sockaddr *) &un, sizeof (un)) == -1) - { - ret = VNET_API_ERROR_SYSCALL_ERROR_6; - goto error; - } - if (listen (msf->fd, 1) == -1) - { - ret = VNET_API_ERROR_SYSCALL_ERROR_7; + clib_error_report (error); + ret = VNET_API_ERROR_SYSCALL_ERROR_4; goto error; } @@ -737,12 +723,11 @@ memif_create_if (vlib_main_t * vm, memif_create_if_args_t * args) goto error; } - msf->clib_file_index = ~0; clib_file_t template = { 0 }; template.read_function = memif_conn_fd_accept_ready; - template.file_descriptor = msf->fd; + template.file_descriptor = msf->sock->fd; template.private_data = mif->socket_file_index; - memif_file_add (&msf->clib_file_index, &template); + memif_file_add (&msf->sock->private_data, &template); } msf->ref_cnt++; diff --git a/src/plugins/memif/private.h b/src/plugins/memif/private.h index 912ec59abd2..49357ddc617 100644 --- a/src/plugins/memif/private.h +++ b/src/plugins/memif/private.h @@ -40,7 +40,6 @@ #if MEMIF_DEBUG == 1 #define memif_file_add(a, b) do { \ - ASSERT (*a == ~0); \ *a = clib_file_add (&file_main, b); \ clib_warning ("clib_file_add fd %d private_data %u idx %u", \ (b)->file_descriptor, (b)->private_data, *a); \ @@ -57,7 +56,6 @@ } while (0) #else #define memif_file_add(a, b) do { \ - ASSERT (*a == ~0); \ *a = clib_file_add (&file_main, b); \ } while (0) #define memif_file_del(a) clib_file_del(&file_main, a) @@ -67,9 +65,8 @@ typedef struct { u8 *filename; - int fd; - uword clib_file_index; - uword *pending_file_indices; + clib_socket_t *sock; + clib_socket_t **pending_clients; int ref_cnt; int is_listener; @@ -138,9 +135,8 @@ typedef struct u32 per_interface_next_index; /* socket connection */ + clib_socket_t *sock; uword socket_file_index; - int conn_fd; - uword conn_clib_file_index; memif_msg_fifo_elt_t *msg_queue; u8 *secret; @@ -150,9 +146,6 @@ typedef struct memif_queue_t *tx_queues; /* remote info */ - pid_t remote_pid; - uid_t remote_uid; - gid_t remote_gid; u8 *remote_name; u8 *remote_if_name; @@ -241,6 +234,7 @@ clib_error_t *memif_connect (memif_if_t * mif); void memif_disconnect (memif_if_t * mif, clib_error_t * err); /* socket.c */ +void memif_socket_close (clib_socket_t ** sock); clib_error_t *memif_conn_fd_accept_ready (clib_file_t * uf); clib_error_t *memif_master_conn_fd_read_ready (clib_file_t * uf); clib_error_t *memif_slave_conn_fd_read_ready (clib_file_t * uf); diff --git a/src/plugins/memif/socket.c b/src/plugins/memif/socket.c index 1abc0f119aa..23ea24902e3 100644 --- a/src/plugins/memif/socket.c +++ b/src/plugins/memif/socket.c @@ -39,6 +39,14 @@ #include #include +void +memif_socket_close (clib_socket_t ** s) +{ + memif_file_del_by_index ((*s)->private_data); + clib_mem_free (*s); + *s = 0; +} + static u8 * memif_str2vec (uint8_t * str, int len) { @@ -59,38 +67,6 @@ memif_str2vec (uint8_t * str, int len) return s; } -static clib_error_t * -memif_msg_send (int fd, memif_msg_t * msg, int afd) -{ - struct msghdr mh = { 0 }; - struct iovec iov[1]; - char ctl[CMSG_SPACE (sizeof (int))]; - int rv; - - iov[0].iov_base = (void *) msg; - iov[0].iov_len = sizeof (memif_msg_t); - mh.msg_iov = iov; - mh.msg_iovlen = 1; - - if (afd > 0) - { - struct cmsghdr *cmsg; - memset (&ctl, 0, sizeof (ctl)); - mh.msg_control = ctl; - mh.msg_controllen = sizeof (ctl); - cmsg = CMSG_FIRSTHDR (&mh); - cmsg->cmsg_len = CMSG_LEN (sizeof (int)); - cmsg->cmsg_level = SOL_SOCKET; - cmsg->cmsg_type = SCM_RIGHTS; - memcpy (CMSG_DATA (cmsg), &afd, sizeof (int)); - } - rv = sendmsg (fd, &mh, 0); - if (rv < 0) - return clib_error_return_unix (0, "sendmsg"); - DBG ("Message type %u sent (fd %d)", msg->type, afd); - return 0; -} - static void memif_msg_enq_ack (memif_if_t * mif) { @@ -102,7 +78,7 @@ memif_msg_enq_ack (memif_if_t * mif) } static clib_error_t * -memif_msg_enq_hello (int fd) +memif_msg_enq_hello (clib_socket_t * sock) { u8 *s; memif_msg_t msg = { 0 }; @@ -117,7 +93,7 @@ memif_msg_enq_hello (int fd) s = format (0, "VPP %s%c", VPP_BUILD_VER, 0); strncpy ((char *) h->name, (char *) s, sizeof (h->name) - 1); vec_free (s); - return memif_msg_send (fd, &msg, -1); + return clib_socket_sendmsg (sock, &msg, sizeof (memif_msg_t), 0, 0); } static void @@ -219,7 +195,7 @@ memif_msg_send_disconnect (memif_if_t * mif, clib_error_t * err) d->code = err->code; strncpy ((char *) d->string, (char *) err->what, sizeof (d->string) - 1); - return memif_msg_send (mif->conn_fd, &msg, -1); + return clib_socket_sendmsg (mif->sock, &msg, sizeof (memif_msg_t), 0, 0); } static clib_error_t * @@ -246,11 +222,11 @@ memif_msg_receive_hello (memif_if_t * mif, memif_msg_t * msg) static clib_error_t * memif_msg_receive_init (memif_if_t ** mifp, memif_msg_t * msg, - clib_file_t * uf) + clib_socket_t * sock, uword socket_file_index) { memif_main_t *mm = &memif_main; memif_socket_file_t *msf = - vec_elt_at_index (mm->socket_files, uf->private_data); + vec_elt_at_index (mm->socket_files, socket_file_index); memif_msg_init_t *i = &msg->init; memif_if_t *mif, tmp; clib_error_t *err; @@ -258,7 +234,7 @@ memif_msg_receive_init (memif_if_t ** mifp, memif_msg_t * msg, if (i->version != MEMIF_VERSION) { - memif_file_del_by_index (uf - file_main.file_pool); + memif_file_del_by_index (sock->private_data); return clib_error_return (0, "unsupported version"); } @@ -278,7 +254,7 @@ memif_msg_receive_init (memif_if_t ** mifp, memif_msg_t * msg, goto error; } - if (mif->conn_fd != -1) + if (mif->sock) { err = clib_error_return (0, "already connected"); goto error; @@ -290,9 +266,8 @@ memif_msg_receive_init (memif_if_t ** mifp, memif_msg_t * msg, goto error; } - mif->conn_fd = uf->file_descriptor; - mif->conn_clib_file_index = uf - file_main.file_pool; - hash_set (msf->dev_instance_by_fd, mif->conn_fd, mif->dev_instance); + mif->sock = sock; + hash_set (msf->dev_instance_by_fd, mif->sock->fd, mif->dev_instance); mif->remote_name = memif_str2vec (i->name, sizeof (i->name)); *mifp = mif; @@ -314,9 +289,7 @@ memif_msg_receive_init (memif_if_t ** mifp, memif_msg_t * msg, return 0; error: - tmp.conn_fd = uf->file_descriptor; memif_msg_send_disconnect (&tmp, err); - memif_file_del_by_index (uf - file_main.file_pool); return err; } @@ -422,64 +395,24 @@ memif_msg_receive_disconnect (memif_if_t * mif, memif_msg_t * msg) } static clib_error_t * -memif_msg_receive (memif_if_t ** mifp, clib_file_t * uf) +memif_msg_receive (memif_if_t ** mifp, clib_socket_t * sock, clib_file_t * uf) { - char ctl[CMSG_SPACE (sizeof (int)) + - CMSG_SPACE (sizeof (struct ucred))] = { 0 }; - struct msghdr mh = { 0 }; - struct iovec iov[1]; memif_msg_t msg = { 0 }; - ssize_t size; clib_error_t *err = 0; int fd = -1; int i; memif_if_t *mif = *mifp; - iov[0].iov_base = (void *) &msg; - iov[0].iov_len = sizeof (memif_msg_t); - mh.msg_iov = iov; - mh.msg_iovlen = 1; - mh.msg_control = ctl; - mh.msg_controllen = sizeof (ctl); - - /* receive the incoming message */ - size = recvmsg (uf->file_descriptor, &mh, 0); - if (size != sizeof (memif_msg_t)) - { - return (size == 0) ? clib_error_return (0, "disconnected") : - clib_error_return_unix (0, - "recvmsg: malformed message received on fd %d", - uf->file_descriptor); - } + err = clib_socket_recvmsg (sock, &msg, sizeof (memif_msg_t), &fd, 1); + if (err) + return err; if (mif == 0 && msg.type != MEMIF_MSG_TYPE_INIT) { - memif_file_del (uf); + memif_socket_close (&sock); return clib_error_return (0, "unexpected message received"); } - /* process anciliary data */ - struct ucred *cr = 0; - struct cmsghdr *cmsg; - - cmsg = CMSG_FIRSTHDR (&mh); - while (cmsg) - { - if (cmsg->cmsg_level == SOL_SOCKET) - { - if (cmsg->cmsg_type == SCM_CREDENTIALS) - { - cr = (struct ucred *) CMSG_DATA (cmsg); - } - else if (cmsg->cmsg_type == SCM_RIGHTS) - { - int *fdp = (int *) CMSG_DATA (cmsg); - fd = *fdp; - } - } - cmsg = CMSG_NXTHDR (&mh, cmsg); - } - DBG ("Message type %u received", msg.type); /* process the message based on its type */ switch (msg.type) @@ -502,12 +435,9 @@ memif_msg_receive (memif_if_t ** mifp, clib_file_t * uf) break; case MEMIF_MSG_TYPE_INIT: - if ((err = memif_msg_receive_init (mifp, &msg, uf))) + if ((err = memif_msg_receive_init (mifp, &msg, sock, uf->private_data))) return err; mif = *mifp; - mif->remote_pid = cr->pid; - mif->remote_uid = cr->uid; - mif->remote_gid = cr->gid; memif_msg_enq_ack (mif); break; @@ -544,9 +474,9 @@ memif_msg_receive (memif_if_t ** mifp, clib_file_t * uf) return err; } - if (clib_fifo_elts (mif->msg_queue) && mif->conn_clib_file_index != ~0) + if (clib_fifo_elts (mif->msg_queue)) clib_file_set_data_available_to_write (&file_main, - mif->conn_clib_file_index, 1); + mif->sock->private_data, 1); return 0; } @@ -558,28 +488,29 @@ memif_master_conn_fd_read_ready (clib_file_t * uf) pool_elt_at_index (mm->socket_files, uf->private_data); uword *p; memif_if_t *mif = 0; - uword conn_clib_file_index = ~0; + clib_socket_t *sock = 0; clib_error_t *err = 0; p = hash_get (msf->dev_instance_by_fd, uf->file_descriptor); if (p) { mif = vec_elt_at_index (mm->interfaces, p[0]); + sock = mif->sock; } else { /* This is new connection, remove index from pending vector */ int i; - vec_foreach_index (i, msf->pending_file_indices) - if (msf->pending_file_indices[i] == uf - file_main.file_pool) + vec_foreach_index (i, msf->pending_clients) + if (msf->pending_clients[i]->fd == uf->file_descriptor) { - conn_clib_file_index = msf->pending_file_indices[i]; - vec_del1 (msf->pending_file_indices, i); + sock = msf->pending_clients[i]; + vec_del1 (msf->pending_clients, i); break; } - ASSERT (conn_clib_file_index != ~0); + ASSERT (sock != 0); } - err = memif_msg_receive (&mif, uf); + err = memif_msg_receive (&mif, sock, uf); if (err) { memif_disconnect (mif, err); @@ -594,7 +525,7 @@ memif_slave_conn_fd_read_ready (clib_file_t * uf) memif_main_t *mm = &memif_main; clib_error_t *err; memif_if_t *mif = vec_elt_at_index (mm->interfaces, uf->private_data); - err = memif_msg_receive (&mif, uf); + err = memif_msg_receive (&mif, mif->sock, uf); if (err) { memif_disconnect (mif, err); @@ -609,9 +540,9 @@ memif_conn_fd_write_ready (clib_file_t * uf, memif_if_t * mif) memif_msg_fifo_elt_t *e; clib_fifo_sub2 (mif->msg_queue, e); clib_file_set_data_available_to_write (&file_main, - mif->conn_clib_file_index, 0); - memif_msg_send (mif->conn_fd, &e->msg, e->fd); - return 0; + mif->sock->private_data, 0); + return clib_socket_sendmsg (mif->sock, &e->msg, sizeof (memif_msg_t), + &e->fd, e->fd > -1 ? 1 : 0); } clib_error_t * @@ -675,11 +606,12 @@ memif_master_conn_fd_error (clib_file_t * uf) else { int i; - vec_foreach_index (i, msf->pending_file_indices) - if (msf->pending_file_indices[i] == uf - file_main.file_pool) + vec_foreach_index (i, msf->pending_clients) + if (msf->pending_clients[i]->fd == uf->file_descriptor) { - vec_del1 (msf->pending_file_indices, i); - memif_file_del (uf); + clib_socket_t *s = msf->pending_clients[i]; + memif_socket_close (&s); + vec_del1 (msf->pending_clients, i); return 0; } } @@ -696,39 +628,39 @@ memif_conn_fd_accept_ready (clib_file_t * uf) memif_main_t *mm = &memif_main; memif_socket_file_t *msf = pool_elt_at_index (mm->socket_files, uf->private_data); - int addr_len; - struct sockaddr_un client; - int conn_fd; clib_file_t template = { 0 }; - uword clib_file_index = ~0; clib_error_t *err; + clib_socket_t *client; - - addr_len = sizeof (client); - conn_fd = accept (uf->file_descriptor, - (struct sockaddr *) &client, (socklen_t *) & addr_len); - - if (conn_fd < 0) - return clib_error_return_unix (0, "accept fd %d", uf->file_descriptor); + client = clib_mem_alloc (sizeof (clib_socket_t)); + memset (client, 0, sizeof (clib_socket_t)); + err = clib_socket_accept (msf->sock, client); + if (err) + goto error; template.read_function = memif_master_conn_fd_read_ready; template.write_function = memif_master_conn_fd_write_ready; template.error_function = memif_master_conn_fd_error; - template.file_descriptor = conn_fd; + template.file_descriptor = client->fd; template.private_data = uf->private_data; - memif_file_add (&clib_file_index, &template); + memif_file_add (&client->private_data, &template); - err = memif_msg_enq_hello (conn_fd); + err = memif_msg_enq_hello (client); if (err) { - clib_error_report (err); - memif_file_del_by_index (clib_file_index); + clib_socket_close (client); + goto error; } - else - vec_add1 (msf->pending_file_indices, clib_file_index); + + vec_add1 (msf->pending_clients, client); return 0; + +error: + clib_error_report (err); + clib_mem_free (client); + return err; } /* -- cgit 1.2.3-korg