laforge has submitted this change. (
https://gerrit.osmocom.org/c/osmo-bsc/+/34243 )
Change subject: meas_feed: Refactor fd/wqueue lifecycle
......................................................................
meas_feed: Refactor fd/wqueue lifecycle
The previous checks had several rough edges which may end up in
unexpected behaviors, specially with fd=0 vs fd=-1.
The new code is much more robust.
Change-Id: I96b0b5c4654970ba1c3e2aecfa896e310063ab6f
---
M src/osmo-bsc/meas_feed.c
1 file changed, 47 insertions(+), 31 deletions(-)
Approvals:
Jenkins Builder: Verified
daniel: Looks good to me, but someone else must approve
laforge: Looks good to me, approved
diff --git a/src/osmo-bsc/meas_feed.c b/src/osmo-bsc/meas_feed.c
index afd96e1..fbfd553 100644
--- a/src/osmo-bsc/meas_feed.c
+++ b/src/osmo-bsc/meas_feed.c
@@ -29,7 +29,9 @@
uint16_t dst_port;
};
-static struct meas_feed_state g_mfs = {};
+static struct meas_feed_state g_mfs = {
+ .wqueue.bfd.fd = -1,
+};
static int process_meas_rep(struct gsm_meas_rep *mr)
{
@@ -37,6 +39,8 @@
struct meas_feed_meas *mfm;
struct bsc_subscr *bsub;
+ OSMO_ASSERT(g_mfs.wqueue.bfd.fd != -1);
+
/* ignore measurements as long as we don't know who it is */
if (!mr->lchan) {
LOGP(DMEAS, LOGL_DEBUG, "meas_feed: no lchan, not sending
report\n");
@@ -49,7 +53,7 @@
bsub = mr->lchan->conn->bsub;
- msg = msgb_alloc(sizeof(struct meas_feed_meas), "Meas. Feed");
+ msg = msgb_alloc(sizeof(struct meas_feed_meas), "meas_feed_msg");
if (!msg)
return 0;
@@ -125,48 +129,47 @@
return rc;
}
+static void meas_feed_close(void)
+{
+ if (g_mfs.wqueue.bfd.fd == -1)
+ return;
+ osmo_signal_unregister_handler(SS_LCHAN, meas_feed_sig_cb, NULL);
+ osmo_wqueue_clear(&g_mfs.wqueue);
+ osmo_fd_unregister(&g_mfs.wqueue.bfd);
+ close(g_mfs.wqueue.bfd.fd);
+ g_mfs.wqueue.bfd.fd = -1;
+}
+
int meas_feed_cfg_set(const char *dst_host, uint16_t dst_port)
{
int rc;
- int already_initialized = 0;
- if (g_mfs.wqueue.bfd.fd)
- already_initialized = 1;
-
-
- if (already_initialized &&
- !strcmp(dst_host, g_mfs.dst_host) &&
- dst_port == g_mfs.dst_port)
- return 0;
-
- if (!already_initialized) {
- osmo_wqueue_init(&g_mfs.wqueue, 10);
- g_mfs.wqueue.write_cb = feed_write_cb;
- g_mfs.wqueue.read_cb = feed_read_cb;
- osmo_signal_register_handler(SS_LCHAN, meas_feed_sig_cb, NULL);
- LOGP(DMEAS, LOGL_DEBUG, "meas_feed: registered signal
callback\n");
+ /* Already initialized */
+ if (g_mfs.wqueue.bfd.fd > 0) {
+ /* No change needed, do nothing */
+ if (!strcmp(dst_host, g_mfs.dst_host) && dst_port ==
g_mfs.dst_port)
+ return 0;
+ meas_feed_close();
}
- if (already_initialized) {
- osmo_wqueue_clear(&g_mfs.wqueue);
- osmo_fd_unregister(&g_mfs.wqueue.bfd);
- close(g_mfs.wqueue.bfd.fd);
- /* don't set to zero, as that would mean 'not yet initialized'
*/
- g_mfs.wqueue.bfd.fd = -1;
- }
+ osmo_wqueue_init(&g_mfs.wqueue, 10);
+ g_mfs.wqueue.write_cb = feed_write_cb;
+ g_mfs.wqueue.read_cb = feed_read_cb;
+
rc = osmo_sock_init_ofd(&g_mfs.wqueue.bfd, AF_UNSPEC, SOCK_DGRAM,
IPPROTO_UDP, dst_host, dst_port,
OSMO_SOCK_F_CONNECT);
- if (rc < 0)
+ if (rc < 0) {
+ g_mfs.wqueue.bfd.fd = -1;
return rc;
+ }
osmo_fd_read_disable(&g_mfs.wqueue.bfd);
-
- if (g_mfs.dst_host)
- talloc_free(g_mfs.dst_host);
- g_mfs.dst_host = talloc_strdup(NULL, dst_host);
+ osmo_talloc_replace_string(NULL, &g_mfs.dst_host, dst_host);
g_mfs.dst_port = dst_port;
-
+ osmo_signal_register_handler(SS_LCHAN, meas_feed_sig_cb, NULL);
+ LOGP(DMEAS, LOGL_DEBUG, "meas_feed: started %s\n",
+ osmo_sock_get_name2(g_mfs.wqueue.bfd.fd));
return 0;
}
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34243
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I96b0b5c4654970ba1c3e2aecfa896e310063ab6f
Gerrit-Change-Number: 34243
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-MessageType: merged
