ipa: Use enhanced ipa_msg_recv_buffered() to cope with partioned IPA messages
diff mbox

Message ID 1396266131-31083-1-git-send-email-jerlbeck@sysmocom.de
State Accepted
Headers show

Commit Message

Jacob Erlbeck March 31, 2014, 11:42 a.m. UTC
The old ipa_msg_recv() implementation didn't support partial receive,
so IPA connections got disconnected when this happened.

This patch adds the handling of the temporary message buffers and uses
ipa_msg_recv_buffered().

It has been successfully tested by jerlbeck with osmo-nitb and
osmo-bsc.

Ticket: OW#768
Sponsored-by: On-Waves ehf
---
 openbsc/include/openbsc/bsc_msc.h     |    2 ++
 openbsc/include/openbsc/bsc_nat.h     |    5 +++++
 openbsc/include/openbsc/control_cmd.h |    3 +++
 openbsc/src/libbsc/bsc_msc.c          |   10 ++++++++++
 openbsc/src/libctrl/control_if.c      |    5 ++++-
 openbsc/src/osmo-bsc/osmo_bsc_msc.c   |    6 ++++--
 openbsc/src/osmo-bsc_nat/bsc_nat.c    |   19 +++++++++++++++----
 openbsc/src/osmo-bsc_nat/bsc_ussd.c   |    8 ++++++--
 8 files changed, 49 insertions(+), 9 deletions(-)

Comments

Holger Freyther March 31, 2014, 1:20 p.m. UTC | #1
On Mon, Mar 31, 2014 at 01:42:11PM +0200, Jacob Erlbeck wrote:

Dear Jacob,

I think there is a typo in the subject. Can you confirm?

> It has been successfully tested by jerlbeck with osmo-nitb and
> osmo-bsc.

My usual question is: What has been tested? You ran the two
processes and did LUs and placed calls? I think it is good to
capture this information in the commit message.

The patch appears fine! Great to finally have these changes.

holger

Patch
diff mbox

diff --git a/openbsc/include/openbsc/bsc_msc.h b/openbsc/include/openbsc/bsc_msc.h
index 647f47e..0adbd26 100644
--- a/openbsc/include/openbsc/bsc_msc.h
+++ b/openbsc/include/openbsc/bsc_msc.h
@@ -48,6 +48,8 @@  struct bsc_msc_connection {
 	void (*connected) (struct bsc_msc_connection *);
 	struct osmo_timer_list reconnect_timer;
 	struct osmo_timer_list timeout_timer;
+
+	struct msgb *pending_msg;
 };
 
 struct bsc_msc_connection *bsc_msc_create(void *ctx, struct llist_head *dest);
diff --git a/openbsc/include/openbsc/bsc_nat.h b/openbsc/include/openbsc/bsc_nat.h
index fe8e521..7bd582c 100644
--- a/openbsc/include/openbsc/bsc_nat.h
+++ b/openbsc/include/openbsc/bsc_nat.h
@@ -97,6 +97,9 @@  struct bsc_connection {
 	/* the fd we use to communicate */
 	struct osmo_wqueue write_queue;
 
+	/* incoming message buffer */
+	struct msgb *pending_msg;
+
 	/* the BSS associated */
 	struct bsc_config *cfg;
 
@@ -343,6 +346,8 @@  struct bsc_nat_ussd_con {
 	struct bsc_nat *nat;
 	int authorized;
 
+	struct msgb *pending_msg;
+
 	struct osmo_timer_list auth_timeout;
 };
 
diff --git a/openbsc/include/openbsc/control_cmd.h b/openbsc/include/openbsc/control_cmd.h
index 725dce0..8aede15 100644
--- a/openbsc/include/openbsc/control_cmd.h
+++ b/openbsc/include/openbsc/control_cmd.h
@@ -39,6 +39,9 @@  struct ctrl_connection {
 	/* The queue for sending data back */
 	struct osmo_wqueue write_queue;
 
+	/* Buffer for partial input data */
+	struct msgb *pending_msg;
+
 	/* Callback if the connection was closed */
 	void (*closed_cb)(struct ctrl_connection *conn);
 
diff --git a/openbsc/src/libbsc/bsc_msc.c b/openbsc/src/libbsc/bsc_msc.c
index 1a0f78a..a24efab 100644
--- a/openbsc/src/libbsc/bsc_msc.c
+++ b/openbsc/src/libbsc/bsc_msc.c
@@ -42,6 +42,13 @@  static void connection_loss(struct bsc_msc_connection *con)
 
 	fd = &con->write_queue.bfd;
 
+	if (con->pending_msg) {
+		LOGP(DMSC, LOGL_ERROR,
+		     "MSC(%s) dropping incomplete message.\n", con->name);
+		msgb_free(con->pending_msg);
+		con->pending_msg = NULL;
+	}
+
 	close(fd->fd);
 	fd->fd = -1;
 	fd->cb = osmo_wqueue_bfd_cb;
@@ -162,6 +169,9 @@  int bsc_msc_connect(struct bsc_msc_connection *con)
 
 	con->is_connected = 0;
 
+	msgb_free(con->pending_msg);
+	con->pending_msg = NULL;
+
 	fd = &con->write_queue.bfd;
 	fd->fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
 	fd->priv_nr = 1;
diff --git a/openbsc/src/libctrl/control_if.c b/openbsc/src/libctrl/control_if.c
index 2727d0d..ca59d8c 100644
--- a/openbsc/src/libctrl/control_if.c
+++ b/openbsc/src/libctrl/control_if.c
@@ -123,6 +123,7 @@  static void control_close_conn(struct ctrl_connection *ccon)
 	llist_del(&ccon->list_entry);
 	if (ccon->closed_cb)
 		ccon->closed_cb(ccon);
+	msgb_free(ccon->pending_msg);
 	talloc_free(ccon);
 }
 
@@ -140,8 +141,10 @@  static int handle_control_read(struct osmo_fd * bfd)
 	queue = container_of(bfd, struct osmo_wqueue, bfd);
 	ccon = container_of(queue, struct ctrl_connection, write_queue);
 
-	ret = ipa_msg_recv(bfd->fd, &msg);
+	ret = ipa_msg_recv_buffered(bfd->fd, &msg, &ccon->pending_msg);
 	if (ret <= 0) {
+		if (ret == -EAGAIN)
+			return 0;
 		if (ret == 0)
 			LOGP(DCTRL, LOGL_INFO, "The control connection was closed\n");
 		else
diff --git a/openbsc/src/osmo-bsc/osmo_bsc_msc.c b/openbsc/src/osmo-bsc/osmo_bsc_msc.c
index 6033985..04e9cf3 100644
--- a/openbsc/src/osmo-bsc/osmo_bsc_msc.c
+++ b/openbsc/src/osmo-bsc/osmo_bsc_msc.c
@@ -247,13 +247,15 @@  static void osmo_ext_handle(struct osmo_msc_data *msc, struct msgb *msg)
 
 static int ipaccess_a_fd_cb(struct osmo_fd *bfd)
 {
-	struct msgb *msg;
+	struct msgb *msg = NULL;
 	struct ipaccess_head *hh;
 	struct osmo_msc_data *data = (struct osmo_msc_data *) bfd->data;
 	int ret;
 
-	ret = ipa_msg_recv(bfd->fd, &msg);
+	ret = ipa_msg_recv_buffered(bfd->fd, &msg, &data->msc_con->pending_msg);
 	if (ret <= 0) {
+		if (ret == -EAGAIN)
+			return 0;
 		if (ret == 0) {
 			LOGP(DMSC, LOGL_ERROR, "The connection to the MSC was lost.\n");
 			bsc_msc_lost(data->msc_con);
diff --git a/openbsc/src/osmo-bsc_nat/bsc_nat.c b/openbsc/src/osmo-bsc_nat/bsc_nat.c
index d9fc0ca..524186a 100644
--- a/openbsc/src/osmo-bsc_nat/bsc_nat.c
+++ b/openbsc/src/osmo-bsc_nat/bsc_nat.c
@@ -796,14 +796,16 @@  static void msc_send_reset(struct bsc_msc_connection *msc_con)
 static int ipaccess_msc_read_cb(struct osmo_fd *bfd)
 {
 	struct bsc_msc_connection *msc_con;
-	struct msgb *msg;
+	struct msgb *msg = NULL;
 	struct ipaccess_head *hh;
 	int ret;
 
 	msc_con = (struct bsc_msc_connection *) bfd->data;
 
-	ret = ipa_msg_recv(bfd->fd, &msg);
+	ret = ipa_msg_recv_buffered(bfd->fd, &msg, &msc_con->pending_msg);
 	if (ret <= 0) {
+		if (ret == -EAGAIN)
+			return 0;
 		if (ret == 0)
 			LOGP(DNAT, LOGL_FATAL,
 				"The connection the MSC(%s) was lost, exiting\n",
@@ -912,6 +914,13 @@  void bsc_close_connection(struct bsc_connection *connection)
 	osmo_wqueue_clear(&connection->write_queue);
 	llist_del(&connection->list_entry);
 
+	if (connection->pending_msg) {
+		LOGP(DNAT, LOGL_ERROR, "Dropping partial message on connection %d.\n",
+		     connection->cfg->nr);
+		msgb_free(connection->pending_msg);
+		connection->pending_msg = NULL;
+	}
+
 	talloc_free(connection);
 }
 
@@ -1206,13 +1215,15 @@  exit3:
 static int ipaccess_bsc_read_cb(struct osmo_fd *bfd)
 {
 	struct bsc_connection *bsc = bfd->data;
-	struct msgb *msg;
+	struct msgb *msg = NULL;
 	struct ipaccess_head *hh;
 	struct ipaccess_head_ext *hh_ext;
 	int ret;
 
-	ret = ipa_msg_recv(bfd->fd, &msg);
+	ret = ipa_msg_recv_buffered(bfd->fd, &msg, &bsc->pending_msg);
 	if (ret <= 0) {
+		if (ret == -EAGAIN)
+			return 0;
 		if (ret == 0)
 			LOGP(DNAT, LOGL_ERROR,
 			     "The connection to the BSC Nr: %d was lost. Cleaning it\n",
diff --git a/openbsc/src/osmo-bsc_nat/bsc_ussd.c b/openbsc/src/osmo-bsc_nat/bsc_ussd.c
index 8da8181..5f073bf 100644
--- a/openbsc/src/osmo-bsc_nat/bsc_ussd.c
+++ b/openbsc/src/osmo-bsc_nat/bsc_ussd.c
@@ -66,6 +66,8 @@  static void bsc_nat_ussd_destroy(struct bsc_nat_ussd_con *con)
 	osmo_fd_unregister(&con->queue.bfd);
 	osmo_timer_del(&con->auth_timeout);
 	osmo_wqueue_clear(&con->queue);
+
+	msgb_free(con->pending_msg);
 	talloc_free(con);
 }
 
@@ -117,12 +119,14 @@  static int forward_sccp(struct bsc_nat *nat, struct msgb *msg)
 static int ussd_read_cb(struct osmo_fd *bfd)
 {
 	struct bsc_nat_ussd_con *conn = bfd->data;
-	struct msgb *msg;
+	struct msgb *msg = NULL;
 	struct ipaccess_head *hh;
 	int ret;
 
-	ret = ipa_msg_recv(bfd->fd, &msg);
+	ret = ipa_msg_recv_buffered(bfd->fd, &msg, &conn->pending_msg);
 	if (ret <= 0) {
+		if (ret == -EAGAIN)
+			return 0;
 		LOGP(DNAT, LOGL_ERROR, "USSD Connection was lost.\n");
 		bsc_nat_ussd_destroy(conn);
 		return -1;