pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-cbc/+/28712 ) Change subject: Refactor {cbsp,sbcap}_cbc_accept_cb ...................................................................... Refactor {cbsp,sbcap}_cbc_accept_cb Delay allocating the structures until really needed. First do checks agains the opened connection, and if an issue is detected simply close the fd. Next, fetch or allocate the related peer. Finally, create the link from the fd. This makes it easier to add early extra checks later when peers will contain info on link_mode (client|server|disabled) later on. Change-Id: Id4f83ec6b0b14e556b1caa9c80e7f68d062fec57 --- M src/cbsp_link.c M src/sbcap_link.c 2 files changed, 110 insertions(+), 74 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-cbc refs/changes/12/28712/1 diff --git a/src/cbsp_link.c b/src/cbsp_link.c index bc421f2..cc7f703 100644 --- a/src/cbsp_link.c +++ b/src/cbsp_link.c @@ -19,6 +19,7 @@ */ #include <errno.h> +#include <unistd.h> #include <sys/types.h> #include <sys/socket.h> @@ -37,6 +38,26 @@ #include <osmocom/cbc/cbsp_link_fsm.h> #include <osmocom/cbc/cbc_peer.h> +static struct cbc_cbsp_link *cbc_cbsp_link_alloc(struct cbc_cbsp_mgr *cbc, struct cbc_peer *peer) +{ + struct cbc_cbsp_link *link; + + link = talloc_zero(cbc, struct cbc_cbsp_link); + OSMO_ASSERT(link); + + link->peer = peer; + + link->fi = osmo_fsm_inst_alloc(&cbsp_link_fsm, link, link, LOGL_DEBUG, NULL); + if (!link->fi) { + LOGPCC(link, LOGL_ERROR, "Unable to allocate FSM\n"); + talloc_free(link); + return NULL; + } + + llist_add_tail(&link->list, &cbc->links); + return link; +} + const char *cbc_cbsp_link_name(const struct cbc_cbsp_link *link) { OSMO_ASSERT(link); @@ -112,10 +133,10 @@ static int cbsp_cbc_accept_cb(struct osmo_stream_srv_link *srv_link, int fd) { struct cbc_cbsp_mgr *cbc = osmo_stream_srv_link_get_data(srv_link); - struct cbc_cbsp_link *link = talloc_zero(cbc, struct cbc_cbsp_link); + struct cbc_peer *peer; + struct cbc_cbsp_link *link; char remote_ip[INET6_ADDRSTRLEN], portbuf[6]; int remote_port; - OSMO_ASSERT(link); remote_ip[0] = '\0'; portbuf[0] = '\0'; @@ -124,44 +145,41 @@ LOGP(DCBSP, LOGL_NOTICE, "New CBSP link connection from %s:%u\n", remote_ip, remote_port); - link->conn = osmo_stream_srv_create(srv_link, srv_link, fd, cbsp_cbc_read_cb, cbsp_cbc_closed_cb, link); - if (!link->conn) { - LOGP(DCBSP, LOGL_ERROR, "Unable to create stream server for %s:%d\n", - remote_ip, remote_port); - talloc_free(link); - return -1; - } - link->fi = osmo_fsm_inst_alloc(&cbsp_link_fsm, link, link, LOGL_DEBUG, NULL); - if (!link->fi) { - LOGPCC(link, LOGL_ERROR, "Unable to allocate FSM\n"); - cbc_cbsp_link_close(link); - talloc_free(link); - return -1; - } - llist_add_tail(&link->list, &cbc->links); - /* Match link to peer */ - link->peer = cbc_peer_by_addr_proto(remote_ip, remote_port, CBC_PEER_PROTO_CBSP); - if (!link->peer) { - if (g_cbc->config.permit_unknown_peers) { - LOGPCC(link, LOGL_NOTICE, "Accepting unknown CBSP peer %s:%d\n", - remote_ip, remote_port); - link->peer = cbc_peer_create(NULL, CBC_PEER_PROTO_CBSP); - OSMO_ASSERT(link->peer); - link->peer->unknown_dynamic_peer = true; - } else { - LOGPCC(link, LOGL_NOTICE, "Rejecting unknown CBSP peer %s:%d (not permitted)\n", - remote_ip, remote_port); - cbc_cbsp_link_close(link); + peer = cbc_peer_by_addr_proto(remote_ip, remote_port, CBC_PEER_PROTO_CBSP); + if (!peer) { + if (!g_cbc->config.permit_unknown_peers) { + LOGP(DCBSP, LOGL_NOTICE, + "Rejecting unknown CBSP peer %s:%d (not permitted)\n", + remote_ip, remote_port); + close(fd); return -1; } - } else { - if (link->peer->link.cbsp) { - LOGPCC(link, LOGL_ERROR, "We already have a connection for peer %s\n", - link->peer->name); - /* FIXME */ - } - link->peer->link.cbsp = link; + LOGP(DCBSP, LOGL_NOTICE, "Accepting unknown CBSP peer %s:%d\n", + remote_ip, remote_port); + peer = cbc_peer_create(NULL, CBC_PEER_PROTO_CBSP); + OSMO_ASSERT(peer); + peer->unknown_dynamic_peer = true; + } + if (peer->link.cbsp) { + LOGPCC(peer->link.cbsp, LOGL_ERROR, + "We already have a connection for peer %s, closing it\n", + peer->name); + cbc_cbsp_link_close(peer->link.cbsp); + } + link = cbc_cbsp_link_alloc(cbc, peer); + OSMO_ASSERT(link); + peer->link.cbsp = link; + + link->conn = osmo_stream_srv_create(srv_link, srv_link, fd, + cbsp_cbc_read_cb, cbsp_cbc_closed_cb, + link); + if (!link->conn) { + LOGPCC(link, LOGL_ERROR, + "Unable to create stream server for %s:%u\n", + remote_ip, remote_port); + talloc_free(link); + return -1; } osmo_fsm_inst_dispatch(link->fi, CBSP_LINK_E_CMD_RESET, NULL); diff --git a/src/sbcap_link.c b/src/sbcap_link.c index 4656f6d..208b806 100644 --- a/src/sbcap_link.c +++ b/src/sbcap_link.c @@ -19,6 +19,7 @@ */ #include <errno.h> +#include <unistd.h> #include <sys/types.h> #include <sys/socket.h> #include <netdb.h> @@ -41,6 +42,26 @@ #include <osmocom/cbc/cbc_peer.h> #include <osmocom/cbc/debug.h> +static struct cbc_sbcap_link *cbc_sbcap_link_alloc(struct cbc_sbcap_mgr *cbc, struct cbc_peer *peer) +{ + struct cbc_sbcap_link *link; + + link = talloc_zero(cbc, struct cbc_sbcap_link); + OSMO_ASSERT(link); + + link->peer = peer; + + link->fi = osmo_fsm_inst_alloc(&sbcap_link_fsm, link, link, LOGL_DEBUG, NULL); + if (!link->fi) { + LOGPSBCAPC(link, LOGL_ERROR, "Unable to allocate FSM\n"); + talloc_free(link); + return NULL; + } + + llist_add_tail(&link->list, &cbc->links); + return link; +} + const char *cbc_sbcap_link_name(const struct cbc_sbcap_link *link) { struct osmo_fd *ofd; @@ -140,10 +161,10 @@ static int sbcap_cbc_accept_cb(struct osmo_stream_srv_link *srv_link, int fd) { struct cbc_sbcap_mgr *cbc = osmo_stream_srv_link_get_data(srv_link); - struct cbc_sbcap_link *link = talloc_zero(cbc, struct cbc_sbcap_link); + struct cbc_peer *peer; + struct cbc_sbcap_link *link; char remote_ip[INET6_ADDRSTRLEN], portbuf[6]; int remote_port; - OSMO_ASSERT(link); remote_ip[0] = '\0'; portbuf[0] = '\0'; @@ -152,44 +173,41 @@ LOGP(DSBcAP, LOGL_NOTICE, "New SBc-AP link connection from %s:%u\n", remote_ip, remote_port); - link->conn = osmo_stream_srv_create(srv_link, srv_link, fd, sbcap_cbc_read_cb, sbcap_cbc_closed_cb, link); - if (!link->conn) { - LOGP(DSBcAP, LOGL_ERROR, "Unable to create stream server for %s:%d\n", - remote_ip, remote_port); - talloc_free(link); - return -1; - } - link->fi = osmo_fsm_inst_alloc(&sbcap_link_fsm, link, link, LOGL_DEBUG, NULL); - if (!link->fi) { - LOGPSBCAPC(link, LOGL_ERROR, "Unable to allocate FSM\n"); - cbc_sbcap_link_close(link); - talloc_free(link); - return -1; - } - llist_add_tail(&link->list, &cbc->links); - /* Match link to peer */ - link->peer = cbc_peer_by_addr_proto(remote_ip, remote_port, CBC_PEER_PROTO_SBcAP); - if (!link->peer) { - if (g_cbc->config.permit_unknown_peers) { - LOGPSBCAPC(link, LOGL_NOTICE, "Accepting unknown SBc-AP peer %s:%d\n", - remote_ip, remote_port); - link->peer = cbc_peer_create(NULL, CBC_PEER_PROTO_SBcAP); - OSMO_ASSERT(link->peer); - link->peer->unknown_dynamic_peer = true; - } else { - LOGPSBCAPC(link, LOGL_NOTICE, "Rejecting unknown SBc-AP peer %s:%d (not permitted)\n", - remote_ip, remote_port); - cbc_sbcap_link_close(link); + peer = cbc_peer_by_addr_proto(remote_ip, remote_port, CBC_PEER_PROTO_SBcAP); + if (!peer) { + if (!g_cbc->config.permit_unknown_peers) { + LOGP(DSBcAP, LOGL_NOTICE, + "Rejecting unknown SBc-AP peer %s:%d (not permitted)\n", + remote_ip, remote_port); + close(fd); return -1; } - } else { - if (link->peer->link.sbcap) { - LOGPSBCAPC(link, LOGL_ERROR, "We already have a connection for peer %s\n", - link->peer->name); - /* FIXME */ - } - link->peer->link.sbcap = link; + LOGP(DSBcAP, LOGL_NOTICE, "Accepting unknown SBc-AP peer %s:%d\n", + remote_ip, remote_port); + peer = cbc_peer_create(NULL, CBC_PEER_PROTO_SBcAP); + OSMO_ASSERT(peer); + peer->unknown_dynamic_peer = true; + } + if (peer->link.sbcap) { + LOGPSBCAPC(peer->link.sbcap, LOGL_ERROR, + "We already have a connection for peer %s, closing it\n", + peer->name); + cbc_sbcap_link_close(peer->link.sbcap); + } + link = cbc_sbcap_link_alloc(cbc, peer); + OSMO_ASSERT(link); + peer->link.sbcap = link; + + link->conn = osmo_stream_srv_create(srv_link, srv_link, fd, + sbcap_cbc_read_cb, sbcap_cbc_closed_cb, + link); + if (!link->conn) { + LOGPSBCAPC(link, LOGL_ERROR, + "Unable to create stream server for %s:%u\n", + remote_ip, remote_port); + talloc_free(link); + return -1; } osmo_fsm_inst_dispatch(link->fi, SBcAP_LINK_E_CMD_RESET, NULL); -- To view, visit https://gerrit.osmocom.org/c/osmo-cbc/+/28712 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-cbc Gerrit-Branch: master Gerrit-Change-Id: Id4f83ec6b0b14e556b1caa9c80e7f68d062fec57 Gerrit-Change-Number: 28712 Gerrit-PatchSet: 1 Gerrit-Owner: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-MessageType: newchange