daniel has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/30937 )
Change subject: control_if: Use osmo_io instead of osmo_fd ......................................................................
control_if: Use osmo_io instead of osmo_fd
* uses the ipa support for osmo_io introduced to libosmogsm
Change-Id: I2977e6f90a2f5a74910ddf3bafb8865dda081b5a --- M include/osmocom/ctrl/control_cmd.h M include/osmocom/ctrl/control_if.h M src/ctrl/control_cmd.c M src/ctrl/control_if.c 4 files changed, 96 insertions(+), 89 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/37/30937/1
diff --git a/include/osmocom/ctrl/control_cmd.h b/include/osmocom/ctrl/control_cmd.h index 276a7de..0b0f6c7 100644 --- a/include/osmocom/ctrl/control_cmd.h +++ b/include/osmocom/ctrl/control_cmd.h @@ -45,11 +45,8 @@ struct ctrl_connection { struct llist_head list_entry;
- /*! The queue for sending data back */ - struct osmo_wqueue write_queue; - - /*! Buffer for partial input data */ - struct msgb *pending_msg; + /*! The io_fd of the connection */ + struct osmo_io_fd *iofd;
/*! Callback if the connection was closed */ void (*closed_cb)(struct ctrl_connection *conn); @@ -59,6 +56,9 @@
/*! Pending deferred command responses for this connection */ struct llist_head def_cmds; + + /*! User data pointer to be passed through*/ + void *data; };
struct ctrl_cmd_def; @@ -123,7 +123,7 @@
int ctrl_cmd_exec(vector vline, struct ctrl_cmd *command, vector node, void *data); int ctrl_cmd_install(enum ctrl_node_type node, struct ctrl_cmd_element *cmd); -int ctrl_cmd_send(struct osmo_wqueue *queue, struct ctrl_cmd *cmd); +int ctrl_cmd_send(struct osmo_io_fd *iofd, struct ctrl_cmd *cmd); int ctrl_cmd_send_to_all(struct ctrl_handle *ctrl, struct ctrl_cmd *cmd); struct ctrl_cmd *ctrl_cmd_parse3(void *ctx, struct msgb *msg, bool *parse_failed); struct ctrl_cmd *ctrl_cmd_parse2(void *ctx, struct msgb *msg); diff --git a/include/osmocom/ctrl/control_if.h b/include/osmocom/ctrl/control_if.h index 98cd100..f35a3e0 100644 --- a/include/osmocom/ctrl/control_if.h +++ b/include/osmocom/ctrl/control_if.h @@ -28,7 +28,7 @@ };
-int ctrl_cmd_send(struct osmo_wqueue *queue, struct ctrl_cmd *cmd); +int ctrl_cmd_send(struct osmo_io_fd *iofd, struct ctrl_cmd *cmd); int ctrl_cmd_send_trap(struct ctrl_handle *ctrl, const char *name, char *value); struct ctrl_handle *ctrl_handle_alloc(void *ctx, void *data, ctrl_cmd_lookup lookup); struct ctrl_handle *ctrl_handle_alloc2(void *ctx, void *data, @@ -47,7 +47,7 @@ uint16_t port, ctrl_cmd_lookup lookup, unsigned int node_count) OSMO_DEPRECATED_OUTSIDE_LIBOSMOCORE; -struct ctrl_connection *osmo_ctrl_conn_alloc(void *ctx, void *data); +struct ctrl_connection *osmo_ctrl_conn_alloc(void *ctx, void *data, int fd); int ctrl_cmd_handle(struct ctrl_handle *ctrl, struct ctrl_cmd *cmd, void *data); struct ctrl_cmd *ctrl_cmd_exec_from_string(struct ctrl_handle *ch, const char *cmdstr);
diff --git a/src/ctrl/control_cmd.c b/src/ctrl/control_cmd.c index b069ca4..e18f091 100644 --- a/src/ctrl/control_cmd.c +++ b/src/ctrl/control_cmd.c @@ -637,7 +637,7 @@ cmd->type = CTRL_TYPE_ERROR; }
- rc = ctrl_cmd_send(&cmd->ccon->write_queue, cmd); + rc = ctrl_cmd_send(cmd->ccon->iofd, cmd);
talloc_free(cmd); llist_del(&cd->list); diff --git a/src/ctrl/control_if.c b/src/ctrl/control_if.c index 9438f7e..b1f6756 100644 --- a/src/ctrl/control_if.c +++ b/src/ctrl/control_if.c @@ -23,6 +23,7 @@ */
#include "config.h" +#include <osmocom/core/osmo_io.h>
#include <errno.h> #include <inttypes.h> @@ -106,7 +107,7 @@ llist_for_each_entry(ccon, &ctrl->ccon_list, list_entry) { if (ccon == cmd->ccon) continue; - if (ctrl_cmd_send(&ccon->write_queue, cmd)) + if (ctrl_cmd_send(ccon->iofd, cmd)) ret++; } return ret; @@ -116,7 +117,7 @@ * \param[inout] queue write queue to which encoded \a cmd shall be appended * \param[in] cmd decoded command representation * \returns 0 in case of success; negative on error */ -int ctrl_cmd_send(struct osmo_wqueue *queue, struct ctrl_cmd *cmd) +int ctrl_cmd_send(struct osmo_io_fd *iofd, struct ctrl_cmd *cmd) { int ret; struct msgb *msg; @@ -130,7 +131,7 @@ ipa_prepend_header_ext(msg, IPAC_PROTO_EXT_CTRL); ipa_prepend_header(msg, IPAC_PROTO_OSMO);
- ret = osmo_wqueue_enqueue(queue, msg); + ret = osmo_iofd_write_msgb(iofd, msg); if (ret != 0) { LOGP(DLCTRL, LOGL_ERROR, "Failed to enqueue the command.\n"); msgb_free(msg); @@ -178,18 +179,17 @@ static void control_close_conn(struct ctrl_connection *ccon) { struct ctrl_cmd_def *cd, *cd2; - char *name = osmo_sock_get_name(ccon, ccon->write_queue.bfd.fd); + struct osmo_io_fd *iofd = ccon->iofd; + char *name = osmo_sock_get_name(ccon, osmo_iofd_get_fd(iofd));
LOGP(DLCTRL, LOGL_INFO, "close()d CTRL connection %s\n", name); talloc_free(name);
- osmo_wqueue_clear(&ccon->write_queue); - close(ccon->write_queue.bfd.fd); - osmo_fd_unregister(&ccon->write_queue.bfd); + osmo_iofd_close(iofd); + llist_del(&ccon->list_entry); if (ccon->closed_cb) ccon->closed_cb(ccon); - msgb_free(ccon->pending_msg);
/* clean up deferred commands */ llist_for_each_entry_safe(cd, cd2, &ccon->def_cmds, list) { @@ -339,41 +339,41 @@ }
-static int handle_control_read(struct osmo_fd * bfd) -{ - int ret; - struct osmo_wqueue *queue; - struct ctrl_connection *ccon; - struct msgb *msg = NULL; - struct ctrl_handle *ctrl = bfd->data; - - queue = container_of(bfd, struct osmo_wqueue, bfd); - ccon = container_of(queue, struct ctrl_connection, write_queue); - - ret = ipa_msg_recv_buffered(bfd->fd, &msg, &ccon->pending_msg); - if (ret == 0) { - /* msg was already discarded. */ - goto close_fd; - } else if (ret == -EAGAIN) { - /* received part of a message, it is stored in ccon->pending_msg and there's - * nothing left to do now. */ - return 0; - } else if (ret < 0) { - LOGP(DLCTRL, LOGL_ERROR, "Failed to parse ip access message: %d (%s)\n", ret, strerror(-ret)); - return 0; - } - - ret = ctrl_handle_msg(ctrl, ccon, msg); - msgb_free(msg); - if (ret) - goto close_fd; - - return 0; - -close_fd: - control_close_conn(ccon); - return -EBADF; -} +//static int handle_control_read(struct osmo_fd * bfd) +//{ +// int ret; +// struct osmo_wqueue *queue; +// struct ctrl_connection *ccon; +// struct msgb *msg = NULL; +// struct ctrl_handle *ctrl = bfd->data; +// +// queue = container_of(bfd, struct osmo_wqueue, bfd); +// ccon = container_of(queue, struct ctrl_connection, write_queue); +// +// ret = ipa_msg_recv_buffered(bfd->fd, &msg, &ccon->pending_msg); +// if (ret == 0) { +// /* msg was already discarded. */ +// goto close_fd; +// } else if (ret == -EAGAIN) { +// /* received part of a message, it is stored in ccon->pending_msg and there's +// * nothing left to do now. */ +// return 0; +// } else if (ret < 0) { +// LOGP(DLCTRL, LOGL_ERROR, "Failed to parse ip access message: %d (%s)\n", ret, strerror(-ret)); +// return 0; +// } +// +// ret = ctrl_handle_msg(ctrl, ccon, msg); +// msgb_free(msg); +// if (ret) +// goto close_fd; +// +// return 0; +// +//close_fd: +// control_close_conn(ccon); +// return -EBADF; +//}
/*! Handle a received CTRL command contained in a \ref msgb. * \param[in] ctrl CTRL interface handle @@ -382,6 +382,7 @@ * \returns 0 on success; negative on error */ int ctrl_handle_msg(struct ctrl_handle *ctrl, struct ctrl_connection *ccon, struct msgb *msg) { + struct osmo_io_fd *iofd = ccon->iofd; struct ctrl_cmd *cmd; bool parse_failed; struct ipaccess_head *iph; @@ -397,13 +398,13 @@ uint8_t msg_type = *(msg->l2h); switch (msg_type) { case IPAC_MSGT_PING: - if (ipa_ccm_send_pong(ccon->write_queue.bfd.fd) < 0) + if (ipa_ccm_send_pong_iofd(iofd) < 0) LOGP(DLINP, LOGL_ERROR, "Cannot send PONG message. Reason: %s\n", strerror(errno)); break; case IPAC_MSGT_PONG: break; case IPAC_MSGT_ID_ACK: - if (ipa_ccm_send_id_ack(ccon->write_queue.bfd.fd) < 0) + if (ipa_ccm_send_id_ack_iofd(iofd) < 0) LOGP(DLINP, LOGL_ERROR, "Cannot send ID_ACK message. Reason: %s\n", strerror(errno)); break; default: @@ -464,60 +465,74 @@
send_reply: /* There is a reply or error that should be reported back to the sender. */ - ctrl_cmd_send(&ccon->write_queue, cmd); + ctrl_cmd_send(iofd, cmd); just_free: talloc_free(cmd); return 0; }
-static int control_write_cb(struct osmo_fd *bfd, struct msgb *msg) +void ctrl_ioops_read_cb(struct osmo_io_fd *iofd, int res, struct msgb *msg) { - struct osmo_wqueue *queue; - struct ctrl_connection *ccon; - int rc; + int rc = -EINVAL; + struct ctrl_connection *ccon = osmo_iofd_get_data(iofd); + struct ctrl_handle *ctrl = ccon->data;
- queue = container_of(bfd, struct osmo_wqueue, bfd); - ccon = container_of(queue, struct ctrl_connection, write_queue); + if (res > 0) + rc = ctrl_handle_msg(ctrl, ccon, msg);
- rc = write(bfd->fd, msg->data, msg->len); - if (rc == 0) { + msgb_free(msg); + if (rc) control_close_conn(ccon); - return -EBADF; - } - if (rc < 0) { - LOGP(DLCTRL, LOGL_ERROR, "Failed to write message to the CTRL connection.\n"); - return 0; - } - if (rc < msg->len) { - msgb_pull(msg, rc); - return -EAGAIN; + + return; +} + +void ctrl_ioops_write_cb(struct osmo_io_fd *iofd, int res, struct msgb *msg) +{ + struct ctrl_connection *ccon = osmo_iofd_get_data(iofd); + + if (res == 0) { + control_close_conn(ccon); + return; }
- return 0; + if (res < 0) { + LOGP(DLCTRL, LOGL_ERROR, "Failed to write message to the CTRL connection.\n"); + return; + } }
+int ipa_iofd_segmentation_cb(struct msgb *msg, int read); + +static struct osmo_io_ops ctrl_ioops = { + .read_cb = ctrl_ioops_read_cb, + .write_cb = ctrl_ioops_write_cb, + .segmentation_cb = ipa_iofd_segmentation_cb, +}; + /*! Allocate CTRL connection * \param[in] ctx Context from which talloc should allocate it * \param[in] data caller's private data parameter which should assigned to write queue's file descriptor data parameter. * \return Allocated CTRL connection structure or NULL in case of errors */ -struct ctrl_connection *osmo_ctrl_conn_alloc(void *ctx, void *data) +struct ctrl_connection *osmo_ctrl_conn_alloc(void *ctx, void *data, int fd) { struct ctrl_connection *ccon = talloc_zero(ctx, struct ctrl_connection); if (!ccon) return NULL;
- osmo_wqueue_init(&ccon->write_queue, 100); /* Error handling here? */
INIT_LLIST_HEAD(&ccon->cmds); INIT_LLIST_HEAD(&ccon->def_cmds);
- ccon->write_queue.bfd.data = data; - ccon->write_queue.bfd.fd = -1; - ccon->write_queue.write_cb = control_write_cb; - ccon->write_queue.read_cb = handle_control_read; + ccon->data = data; + ccon->iofd = osmo_iofd_setup(ctx, 1024, 0, fd, NULL, OSMO_IO_FD_MODE_READ_WRITE, &ctrl_ioops, ccon, 0); + if (!ccon->iofd) { + talloc_free(ccon); + return NULL; + }
return ccon; } @@ -549,7 +564,7 @@ } #endif ctrl = listen_bfd->data; - ccon = osmo_ctrl_conn_alloc(listen_bfd->data, ctrl); + ccon = osmo_ctrl_conn_alloc(listen_bfd->data, ctrl, fd); if (!ccon) { LOGP(DLCTRL, LOGL_ERROR, "Failed to allocate.\n"); close(fd); @@ -559,15 +574,7 @@ name = osmo_sock_get_name(ccon, fd); LOGP(DLCTRL, LOGL_INFO, "accept()ed new CTRL connection from %s\n", name);
- ccon->write_queue.bfd.fd = fd; - ccon->write_queue.bfd.when = OSMO_FD_READ; - - ret = osmo_fd_register(&ccon->write_queue.bfd); - if (ret < 0) { - LOGP(DLCTRL, LOGL_ERROR, "Could not register FD.\n"); - close(ccon->write_queue.bfd.fd); - talloc_free(ccon); - } + osmo_iofd_read_enable(ccon->iofd);
llist_add(&ccon->list_entry, &ctrl->ccon_list);
-
daniel