arehbein has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-bts/+/31533 )
Change subject: common: Have PCU socket connection use osmo_wqueue
......................................................................
common: Have PCU socket connection use osmo_wqueue
Fixes memleak in case of connected PCU process being suspended without proper
close on socket
Related: OS#5774
Change-Id: Ia6e61dda4b3cd4bba76e6acb7771d70335062fe1
---
M include/osmo-bts/bts.h
M include/osmo-bts/pcuif_proto.h
M src/common/pcu_sock.c
3 files changed, 57 insertions(+), 68 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/33/31533/1
diff --git a/include/osmo-bts/bts.h b/include/osmo-bts/bts.h
index 0ecec1e..2c36c55 100644
--- a/include/osmo-bts/bts.h
+++ b/include/osmo-bts/bts.h
@@ -360,6 +360,7 @@
struct {
char *sock_path;
+ int sock_qlength_max;
} pcu;
/* GSMTAP Um logging (disabled by default) */
diff --git a/include/osmo-bts/pcuif_proto.h b/include/osmo-bts/pcuif_proto.h
index 0fece48..7e854ce 100644
--- a/include/osmo-bts/pcuif_proto.h
+++ b/include/osmo-bts/pcuif_proto.h
@@ -4,7 +4,8 @@
#include <osmocom/gsm/l1sap.h>
#include <arpa/inet.h>
-#define PCU_SOCK_DEFAULT "/tmp/pcu_bts"
+#define PCU_SOCK_DEFAULT "/tmp/pcu_bts"
+#define PCU_SOCK_QLENGTH_MAX_DEFAULT 10
#define PCU_IF_VERSION 0x0a
#define TXT_MAX_LEN 128
diff --git a/src/common/pcu_sock.c b/src/common/pcu_sock.c
index 8f8b3af..1d9fd2e 100644
--- a/src/common/pcu_sock.c
+++ b/src/common/pcu_sock.c
@@ -30,6 +30,7 @@
#include <osmocom/core/talloc.h>
#include <osmocom/core/select.h>
#include <osmocom/core/socket.h>
+#include <osmocom/core/write_queue.h>
#include <osmocom/gsm/gsm23003.h>
#include <osmocom/gsm/abis_nm.h>
#include <osmo-bts/logging.h>
@@ -956,10 +957,11 @@
struct pcu_sock_state {
struct gsm_network *net;
struct osmo_fd listen_bfd; /* fd for listen socket */
- struct osmo_fd conn_bfd; /* fd for connection to lcr */
- struct llist_head upqueue; /* queue for sending messages */
+ struct osmo_wqueue upqueue; /* For sending messages; has fd for
conn. to PCU */
};
+static void pcu_sock_close(struct pcu_sock_state *state);
+
int pcu_sock_send(struct gsm_network *net, struct msgb *msg)
{
struct pcu_sock_state *state = net->pcu_state;
@@ -974,7 +976,7 @@
msgb_free(msg);
return -EINVAL;
}
- conn_bfd = &state->conn_bfd;
+ conn_bfd = &state->upqueue.bfd;
if (conn_bfd->fd <= 0) {
if (pcu_prim->msg_type != PCU_IF_MSG_TIME_IND &&
pcu_prim->msg_type != PCU_IF_MSG_INTERF_IND)
@@ -983,15 +985,19 @@
msgb_free(msg);
return -EIO;
}
- msgb_enqueue(&state->upqueue, msg);
- osmo_fd_write_enable(conn_bfd);
-
+ if (osmo_wqueue_enqueue(&state->upqueue, msg) == -ENOSPC) {
+ LOGP(DLGLOBAL, LOGL_NOTICE, "PCU not reacting (more thatn %d
messages waiting). Closing connection\n",
+ state->upqueue.max_length);
+ pcu_sock_close(state);
+ } else {
+ osmo_fd_write_enable(conn_bfd);
+ }
return 0;
}
static void pcu_sock_close(struct pcu_sock_state *state)
{
- struct osmo_fd *bfd = &state->conn_bfd;
+ struct osmo_fd *bfd = &state->upqueue.bfd;
struct gsm_bts *bts;
struct gsm_bts_trx *trx;
unsigned int tn;
@@ -1037,11 +1043,7 @@
}
}
- /* flush the queue */
- while (!llist_empty(&state->upqueue)) {
- struct msgb *msg = msgb_dequeue(&state->upqueue);
- msgb_free(msg);
- }
+ osmo_wqueue_clear(&state->upqueue);
}
static int pcu_sock_read(struct osmo_fd *bfd)
@@ -1090,45 +1092,30 @@
return -1;
}
-static int pcu_sock_write(struct osmo_fd *bfd)
+static int pcu_sock_write(struct osmo_fd *bfd, struct msgb *m)
{
struct pcu_sock_state *state = bfd->data;
int rc;
+ struct gsm_pcu_if *pcu_prim;
- while (!llist_empty(&state->upqueue)) {
- struct msgb *msg, *msg2;
- struct gsm_pcu_if *pcu_prim;
-
- /* peek at the beginning of the queue */
- msg = llist_entry(state->upqueue.next, struct msgb, list);
- pcu_prim = (struct gsm_pcu_if *)msg->data;
-
- osmo_fd_write_disable(bfd);
-
- /* bug hunter 8-): maybe someone forgot msgb_put(...) ? */
- if (!msgb_length(msg)) {
- LOGP(DPCU, LOGL_ERROR, "message type (%d) with ZERO "
- "bytes!\n", pcu_prim->msg_type);
- goto dontsend;
+ pcu_prim = (struct gsm_pcu_if *)m->data;
+ osmo_fd_write_disable(&state->upqueue.bfd);
+ /* bug hunter 8-): maybe someone forgot msgb_put(...) ? */
+ if (!msgb_length(m)) {
+ LOGP(DPCU, LOGL_ERROR, "message type (%d) with ZERO "
+ "bytes!\n", pcu_prim->msg_type);
+ return 0;
+ }
+ /* try to send it over the socket */
+ rc = write(bfd->fd, msgb_data(m), msgb_length(m));
+ if (rc == 0)
+ goto close;
+ if (rc < 0) {
+ if (errno == EAGAIN) {
+ osmo_fd_write_enable(bfd);
+ return -EAGAIN;
}
-
- /* try to send it over the socket */
- rc = write(bfd->fd, msgb_data(msg), msgb_length(msg));
- if (rc == 0)
- goto close;
- if (rc < 0) {
- if (errno == EAGAIN) {
- osmo_fd_write_enable(bfd);
- break;
- }
- goto close;
- }
-
-dontsend:
- /* _after_ we send it, we can deueue */
- msg2 = msgb_dequeue(&state->upqueue);
- assert(msg == msg2);
- msgb_free(msg);
+ goto close;
}
return 0;
@@ -1138,26 +1125,11 @@
return -1;
}
-static int pcu_sock_cb(struct osmo_fd *bfd, unsigned int flags)
-{
- int rc = 0;
-
- if (flags & OSMO_FD_READ)
- rc = pcu_sock_read(bfd);
- if (rc < 0)
- return rc;
-
- if (flags & OSMO_FD_WRITE)
- rc = pcu_sock_write(bfd);
-
- return rc;
-}
-
/* accept connection coming from PCU */
static int pcu_sock_accept(struct osmo_fd *bfd, unsigned int flags)
{
struct pcu_sock_state *state = (struct pcu_sock_state *)bfd->data;
- struct osmo_fd *conn_bfd = &state->conn_bfd;
+ struct osmo_fd *conn_bfd = &state->upqueue.bfd;
struct sockaddr_un un_addr;
socklen_t len;
int rc;
@@ -1178,7 +1150,7 @@
return 0;
}
- osmo_fd_setup(conn_bfd, rc, OSMO_FD_READ, pcu_sock_cb, state, 0);
+ osmo_fd_setup(conn_bfd, rc, OSMO_FD_READ, osmo_wqueue_bfd_cb, state, 0);
if (osmo_fd_register(conn_bfd) != 0) {
LOGP(DPCU, LOGL_ERROR, "Failed to register new connection "
@@ -1206,9 +1178,12 @@
if (!state)
return -ENOMEM;
- INIT_LLIST_HEAD(&state->upqueue);
+ osmo_wqueue_init(&state->upqueue, PCU_SOCK_QLENGTH_MAX_DEFAULT);
+ state->upqueue.read_cb = pcu_sock_read;
+ state->upqueue.write_cb = pcu_sock_write;
+ state->upqueue.bfd.fd = -1;
+
state->net = &bts_gsmnet;
- state->conn_bfd.fd = -1;
bfd = &state->listen_bfd;
@@ -1249,7 +1224,7 @@
return;
osmo_signal_unregister_handler(SS_GLOBAL, pcu_if_signal_cb, NULL);
- conn_bfd = &state->conn_bfd;
+ conn_bfd = &state->upqueue.bfd;
if (conn_bfd->fd > 0)
pcu_sock_close(state);
bfd = &state->listen_bfd;
@@ -1265,7 +1240,7 @@
if (!state)
return false;
- if (state->conn_bfd.fd <= 0)
+ if (state->upqueue.bfd.fd <= 0)
return false;
return true;
}
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31533
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Ia6e61dda4b3cd4bba76e6acb7771d70335062fe1
Gerrit-Change-Number: 31533
Gerrit-PatchSet: 1
Gerrit-Owner: arehbein <[email protected]>
Gerrit-MessageType: newchange
