[PATCH] ipaccess: more robust error handling in the send path

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/OpenBSC@lists.osmocom.org/.

Pablo Neira Ayuso pablo at gnumonks.org
Wed Aug 22 12:41:28 UTC 2012


In case of problems to transmit message, spot an error and
close the socket to force a re-establishment.

I have also modified ipa_client_read function. Now it closes
the socket if the callback returns an error. I think this is
good to force a connection re-establishment and start from
scratch.

Thanks to Holger Freyther for feedback.
---
 src/input/ipa.c      |   10 ++++--
 src/input/ipaccess.c |   82 ++++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 78 insertions(+), 14 deletions(-)

diff --git a/src/input/ipa.c b/src/input/ipa.c
index 3c6a507..ceb271f 100644
--- a/src/input/ipa.c
+++ b/src/input/ipa.c
@@ -131,8 +131,14 @@ static void ipa_client_read(struct ipa_client_conn *link)
 		ipa_client_retry(link);
 		return;
 	}
-	if (link->read_cb)
-		link->read_cb(link, msg);
+	if (link->read_cb) {
+		ret = link->read_cb(link, msg);
+		if (ret < 0) {
+			LOGP(DLINP, LOGL_ERROR, "Connection closed with "
+						"server.\n");
+			ipa_client_retry(link);
+		}
+	}
 }
 
 static void ipa_client_write(struct ipa_client_conn *link)
diff --git a/src/input/ipaccess.c b/src/input/ipaccess.c
index ea21de7..079ed61 100644
--- a/src/input/ipaccess.c
+++ b/src/input/ipaccess.c
@@ -189,26 +189,39 @@ int ipaccess_send_id_req(int fd)
 /* base handling of the ip.access protocol */
 int ipaccess_rcvmsg_base(struct msgb *msg, struct osmo_fd *bfd)
 {
-	int ipa_ccm = 0;
 	uint8_t msg_type = *(msg->l2h);
-	int ret = 0;
+	int ret;
 
 	switch (msg_type) {
 	case IPAC_MSGT_PING:
-		ipa_ccm = 1;
 		ret = ipaccess_send_pong(bfd->fd);
+		if (ret < 0) {
+			LOGP(DLINP, LOGL_ERROR, "Cannot send PING "
+			     "message. Reason: %s\n", strerror(errno));
+			break;
+		}
+		ret = 1;
 		break;
 	case IPAC_MSGT_PONG:
 		DEBUGP(DLMI, "PONG!\n");
-		ipa_ccm = 1;
+		ret = 1;
 		break;
 	case IPAC_MSGT_ID_ACK:
 		DEBUGP(DLMI, "ID_ACK? -> ACK!\n");
-		ipa_ccm = 1;
 		ret = ipaccess_send_id_ack(bfd->fd);
+		if (ret < 0) {
+			LOGP(DLINP, LOGL_ERROR, "Cannot send ID_ACK "
+			     "message. Reason: %s\n", strerror(errno));
+			break;
+		}
+		ret = 1;
+		break;
+	default:
+		/* This is not an IPA PING, PONG or ID_ACK message */
+		ret = 0;
 		break;
 	}
-	return ipa_ccm;
+	return ret;
 }
 
 /* base handling of the ip.access protocol */
@@ -221,6 +234,10 @@ int ipaccess_rcvmsg_bts_base(struct msgb *msg,
 	switch (msg_type) {
 	case IPAC_MSGT_PING:
 		ret = ipaccess_send_pong(bfd->fd);
+		if (ret < 0) {
+			LOGP(DLINP, LOGL_ERROR, "Cannot send PONG "
+			     "message. Reason: %s\n", strerror(errno));
+		}
 		break;
 	case IPAC_MSGT_PONG:
 		DEBUGP(DLMI, "PONG!\n");
@@ -229,7 +246,7 @@ int ipaccess_rcvmsg_bts_base(struct msgb *msg,
 		DEBUGP(DLMI, "ID_ACK\n");
 		break;
 	}
-	return 0;
+	return ret;
 }
 
 static int ipaccess_drop(struct osmo_fd *bfd)
@@ -264,8 +281,23 @@ static int ipaccess_rcvmsg(struct e1inp_line *line, struct msgb *msg,
 	int len, ret;
 
 	/* Handle IPA PING, PONG and ID_ACK messages. */
-	if (ipaccess_rcvmsg_base(msg, bfd))
+	ret = ipaccess_rcvmsg_base(msg, bfd);
+	switch(ret) {
+	case -1:
+		/* error in IPA control message handling */
+		goto err;
+	case 1:
+		/* this is an IPA control message, skip further processing */
 		return 0;
+	case 0:
+		/* this is not an IPA control message, continue */
+		break;
+	default:
+		LOGP(DLINP, LOGL_ERROR, "Unexpected return from "
+					"ipaccess_rcvmsg_base "
+					"(ret=%d)\n", ret);
+		goto err;
+	}
 
 	switch (msg_type) {
 	case IPAC_MSGT_ID_RESP:
@@ -512,6 +544,16 @@ static int __handle_ts1_write(struct osmo_fd *bfd, struct e1inp_line *line)
 	DEBUGP(DLMI, "TX %u: %s\n", ts_nr, osmo_hexdump(msg->l2h, msgb_l2len(msg)));
 
 	ret = send(bfd->fd, msg->data, msg->len, 0);
+	if (ret != msg->len) {
+		LOGP(DLINP, LOGL_ERROR, "failed to send A-bis IPA signalling "
+			"message. Reason: %s\n", strerror(errno));
+		if (ipaccess_drop(bfd) >= 0) {
+			LOGP(DLINP, LOGL_NOTICE, "Sign link problems, "
+			"closing socket. Reason: %s\n", strerror(errno));
+			return ret;
+		}
+	}
+
 	msgb_free(msg);
 
 	/* set tx delay timer for next event */
@@ -754,13 +796,16 @@ static int ipaccess_bts_cb(struct ipa_client_conn *link, struct msgb *msg)
 	struct ipaccess_head *hh = (struct ipaccess_head *) msg->data;
 	struct e1inp_ts *e1i_ts = NULL;
 	struct e1inp_sign_link *sign_link;
+	int ret = 0;
 
 	/* special handling for IPA CCM. */
 	if (hh->proto == IPAC_PROTO_IPACCESS) {
 		uint8_t msg_type = *(msg->l2h);
 
 		/* ping, pong and acknowledgment cases. */
-		ipaccess_rcvmsg_bts_base(msg, link->ofd);
+		ret = ipaccess_rcvmsg_bts_base(msg, link->ofd);
+		if (ret < 0)
+			return ret;
 
 		/* this is a request for identification from the BSC. */
 		if (msg_type == IPAC_MSGT_ID_GET) {
@@ -793,15 +838,28 @@ static int ipaccess_bts_cb(struct ipa_client_conn *link, struct msgb *msg)
 			}
 			rmsg = ipa_bts_id_resp(link->line->ops->cfg.ipa.dev,
 						data + 1, len - 1);
-			ipaccess_send(link->ofd->fd, rmsg->data, rmsg->len);
+			ret = ipaccess_send(link->ofd->fd, rmsg->data,
+						rmsg->len);
 			msgb_free(rmsg);
 
+			if (ret != rmsg->len) {
+				LOGP(DLINP, LOGL_ERROR, "cannot send ID_RESP "
+				     "message. Reason: %s\n", strerror(errno));
+				return ret;
+			}
+
 			/* send ID_ACK. */
 			rmsg = ipa_bts_id_ack();
-			ipaccess_send(link->ofd->fd, rmsg->data, rmsg->len);
+			ret = ipaccess_send(link->ofd->fd, rmsg->data,
+						rmsg->len);
 			msgb_free(rmsg);
+
+			if (ret != rmsg->len) {
+				LOGP(DLINP, LOGL_ERROR, "cannot send ID_ACK "
+				     "message. Reason: %s\n", strerror(errno));
+			}
 		}
-		return 0;
+		return ret;
 	} else if (link->port == IPA_TCP_PORT_OML)
 		e1i_ts = &link->line->ts[0];
 	else if (link->port == IPA_TCP_PORT_RSL)
-- 
1.7.10.4


--9jxsPFA5p3P2qPhR--




More information about the OpenBSC mailing list