Change in libosmo-netif[master]: stream: Re-arrange cli states to fix 100% cpu usage bug

[laforge]

laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/17030 )

Change subject: stream: Re-arrange cli states to fix 100% cpu usage bug
......................................................................

stream: Re-arrange cli states to fix 100% cpu usage bug

With previous state, osmo_stream_cli_close() could be called from
osmo_stream_cli_open()(), and in that case state was kept as NONE while
ending up with an associated fd being registered in the select loop.
As a result, osmo_stream_cli_fd_cb() could be called while in state
NONE, which was not expected and would simply return without modifying
fd state flags, causing it to be called again and again.

Related: OS#4378
Change-Id: Ie3342f882893a71ad1538c17ad9ee9fa4433eaa4
---
M src/stream.c
1 file changed, 11 insertions(+), 16 deletions(-)

Approvals:
  Jenkins Builder: Verified
  fixeria: Looks good to me, but someone else must approve; Verified
  laforge: Looks good to me, approved



diff --git a/src/stream.c b/src/stream.c
index 1a38e77..c47ae3e 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -129,9 +129,9 @@
  */
 
 enum osmo_stream_cli_state {
-        STREAM_CLI_STATE_NONE         = 0,
-        STREAM_CLI_STATE_CONNECTING   = 1,
-        STREAM_CLI_STATE_CONNECTED    = 2,
+        STREAM_CLI_STATE_NONE         = 0, /* No fd associated, may have timer active to try to connect again */
+        STREAM_CLI_STATE_CONNECTING   = 1, /* Fd associated, but connection not yet confirmed by peer or lower layers */
+        STREAM_CLI_STATE_CONNECTED    = 2, /* Fd associated and connection is established */
         STREAM_CLI_STATE_MAX
 };
 
@@ -190,7 +190,6 @@
 	LOGSCLI(cli, LOGL_INFO, "retrying in %d seconds...\n",
 		cli->reconnect_timeout);
 	osmo_timer_schedule(&cli->timer, cli->reconnect_timeout, 0);
-	cli->state = STREAM_CLI_STATE_CONNECTING;
 }
 
 /*! \brief Check if Osmocom Stream Client is in connected state
@@ -324,7 +323,8 @@
 		}
 		break;
 	default:
-		break;
+		/* Only CONNECTING and CONNECTED states are expected, since they are the only states where FD exists: */
+		osmo_panic("osmo_stream_cli_fd_cb called with unexpected state %d\n", cli->state);
 	}
         return 0;
 }
@@ -345,11 +345,10 @@
 
 	cli->proto = IPPROTO_TCP;
 	cli->ofd.fd = -1;
-	cli->ofd.when |= BSC_FD_READ | BSC_FD_WRITE;
 	cli->ofd.priv_nr = 0;	/* XXX */
 	cli->ofd.cb = osmo_stream_cli_fd_cb;
 	cli->ofd.data = cli;
-	cli->state = STREAM_CLI_STATE_CONNECTING;
+	cli->state = STREAM_CLI_STATE_NONE;
 	osmo_timer_setup(&cli->timer, cli_timer_cb, cli);
 	cli->reconnect_timeout = 5;	/* default is 5 seconds. */
 	INIT_LLIST_HEAD(&cli->tx_queue);
@@ -583,6 +582,7 @@
 		return ret;
 	}
 	cli->ofd.fd = ret;
+	cli->ofd.when = BSC_FD_READ | BSC_FD_WRITE;
 
 	if (cli->flags & OSMO_STREAM_CLI_F_NODELAY) {
 		ret = setsockopt_nodelay(cli->ofd.fd, cli->proto, 1);
@@ -593,6 +593,7 @@
 	if (osmo_fd_register(&cli->ofd) < 0)
 		goto error_close_socket;
 
+	cli->state = STREAM_CLI_STATE_CONNECTING;
 	return 0;
 
 error_close_socket:
@@ -653,6 +654,7 @@
 		return ret;
 	}
 	cli->ofd.fd = ret;
+	cli->ofd.when = BSC_FD_READ | BSC_FD_WRITE;
 
 	if (cli->flags & OSMO_STREAM_CLI_F_NODELAY) {
 		ret = setsockopt_nodelay(cli->ofd.fd, cli->proto, 1);
@@ -663,6 +665,7 @@
 	if (osmo_fd_register(&cli->ofd) < 0)
 		goto error_close_socket;
 
+	cli->state = STREAM_CLI_STATE_CONNECTING;
 	return 0;
 
 error_close_socket:
@@ -676,15 +679,7 @@
 	struct osmo_stream_cli *cli = data;
 
 	LOGSCLI(cli, LOGL_DEBUG, "reconnecting.\n");
-
-	switch(cli->state) {
-	case STREAM_CLI_STATE_CONNECTING:
-		cli->ofd.when |= BSC_FD_READ | BSC_FD_WRITE;
-		osmo_stream_cli_open(cli);
-	        break;
-	default:
-		break;
-	}
+	osmo_stream_cli_open(cli);
 }
 
 /*! \brief Enqueue data to be sent via an Osmocom stream client

-- 
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/17030
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: Ie3342f882893a71ad1538c17ad9ee9fa4433eaa4
Gerrit-Change-Number: 17030
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <axilirator at gmail.com>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200128/e7f9bff5/attachment.htm>