Change in osmo-mgw[master]: mgcp_client: add MGW name as logging context

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/gerrit-log@lists.osmocom.org/.

dexter gerrit-no-reply at lists.osmocom.org
Fri Sep 3 16:04:11 UTC 2021


dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/25335 )


Change subject: mgcp_client: add MGW name as logging context
......................................................................

mgcp_client: add MGW name as logging context

Usually only one MGCP client per application is present. Then the log
lines from mgcp_client.c will be distinguishable without additional
information. When the application is using a pool of MGWs, then the
various MGCP Client instances become hard to distinguish.

- Add a possibility to set a description (name) for each MGW pool
  member. When no description is set, use the domain name.

- Output the pool member name on each log line in mgcp_client.c
  and mgcp_client_pool.c

Change-Id: I53ff5445c8e5faffa4ef908ffb1fdb1f47ea2904
Related: SYS#5091
---
M include/osmocom/mgcp_client/mgcp_client.h
M include/osmocom/mgcp_client/mgcp_client_pool_internal.h
M src/libosmo-mgcp-client/mgcp_client.c
M src/libosmo-mgcp-client/mgcp_client_pool.c
M src/libosmo-mgcp-client/mgcp_client_vty.c
5 files changed, 165 insertions(+), 96 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/35/25335/1

diff --git a/include/osmocom/mgcp_client/mgcp_client.h b/include/osmocom/mgcp_client/mgcp_client.h
index 4d162d0..0405175 100644
--- a/include/osmocom/mgcp_client/mgcp_client.h
+++ b/include/osmocom/mgcp_client/mgcp_client.h
@@ -32,6 +32,9 @@
 	 * 'rtpbridge/(wildcard)' or a number of specific E1 like e.g.
 	 * 'ds/e1-0/s-3/su16-4' */
 	struct llist_head reset_epnames;
+
+	/* human readable name / description */
+	char *description;
 };
 
 typedef unsigned int mgcp_trans_id_t;
@@ -173,3 +176,5 @@
 			     enum mgcp_codecs codec);
 enum mgcp_codecs map_pt_to_codec(struct ptmap *ptmap, unsigned int ptmap_len,
 				 unsigned int pt);
+
+const char *mgcp_client_name(const struct mgcp_client *mgcp);
diff --git a/include/osmocom/mgcp_client/mgcp_client_pool_internal.h b/include/osmocom/mgcp_client/mgcp_client_pool_internal.h
index c58ec02..95f5525 100644
--- a/include/osmocom/mgcp_client/mgcp_client_pool_internal.h
+++ b/include/osmocom/mgcp_client/mgcp_client_pool_internal.h
@@ -41,3 +41,5 @@
 	/* VTY node specification used with this pool. This field is populated by mgcp_client_pool_vty_init() */
 	struct cmd_node *vty_node;
 };
+
+const char *mgcp_client_pool_member_name(const struct mgcp_client_pool_member *pool_member);
diff --git a/src/libosmo-mgcp-client/mgcp_client.c b/src/libosmo-mgcp-client/mgcp_client.c
index 2bed90f..5945459 100644
--- a/src/libosmo-mgcp-client/mgcp_client.c
+++ b/src/libosmo-mgcp-client/mgcp_client.c
@@ -46,6 +46,9 @@
 #define OSMUX_CID_MAX 255 /* FIXME: use OSMUX_CID_MAX from libosmo-netif? */
 #endif
 
+#define LOGPMGW(mgcp, level, fmt, args...) \
+LOGP(DLMGCP, level, "MGW(%s) " fmt, mgcp_client_name(mgcp), ## args)
+
 /* Codec descripton for dynamic payload types (SDP) */
 const struct value_string osmo_mgcpc_codec_names[] = {
 	{ CODEC_PCMU_8000_1, "PCMU/8000/1" },
@@ -209,14 +212,13 @@
 					struct mgcp_response *response)
 {
 	if (!pending) {
-		LOGP(DLMGCP, LOGL_ERROR,
-		     "Cannot handle NULL response\n");
+		LOGPMGW(mgcp, LOGL_ERROR, "Cannot handle NULL response\n");
 		return;
 	}
 	if (pending->response_cb)
 		pending->response_cb(response, pending->priv);
 	else
-		LOGP(DLMGCP, LOGL_DEBUG, "MGCP response ignored (NULL cb)\n");
+		LOGPMGW(mgcp, LOGL_DEBUG, "MGCP response ignored (NULL cb)\n");
 	talloc_free(pending);
 }
 
@@ -673,26 +675,25 @@
 
 	rc = mgcp_response_parse_head(r, msg);
 	if (rc) {
-		LOGP(DLMGCP, LOGL_ERROR, "Cannot parse MGCP response (head)\n");
+		LOGPMGW(mgcp, LOGL_ERROR, "Cannot parse MGCP response (head)\n");
 		rc = 1;
 		goto error;
 	}
 
-	LOGP(DLMGCP, LOGL_DEBUG, "MGCP client: Rx %d %u %s\n",
+	LOGPMGW(mgcp, LOGL_DEBUG, "MGCP client: Rx %d %u %s\n",
 	     r->head.response_code, r->head.trans_id, r->head.comment);
 
 	rc = parse_head_params(r);
 	if (rc) {
-		LOGP(DLMGCP, LOGL_ERROR, "Cannot parse MGCP response (head parameters)\n");
+		LOGPMGW(mgcp, LOGL_ERROR, "Cannot parse MGCP response (head parameters)\n");
 		rc = 1;
 		goto error;
 	}
 
 	pending = mgcp_client_response_pending_get(mgcp, r->head.trans_id);
 	if (!pending) {
-		LOGP(DLMGCP, LOGL_ERROR,
-		     "Cannot find matching MGCP transaction for trans_id %d\n",
-		     r->head.trans_id);
+		LOGPMGW(mgcp, LOGL_ERROR, "Cannot find matching MGCP transaction for trans_id %d\n",
+			r->head.trans_id);
 		rc = -ENOENT;
 		goto error;
 	}
@@ -713,19 +714,19 @@
 
 	msg = msgb_alloc_headroom(4096, 128, "mgcp_from_gw");
 	if (!msg) {
-		LOGP(DLMGCP, LOGL_ERROR, "Failed to allocate MGCP message.\n");
+		LOGPMGW(mgcp, LOGL_ERROR, "Failed to allocate MGCP message.\n");
 		return -1;
 	}
 
 	ret = read(fd->fd, msg->data, 4096 - 128);
 	if (ret <= 0) {
-		LOGP(DLMGCP, LOGL_ERROR, "Failed to read: %s: %d='%s'\n", osmo_sock_get_name2(fd->fd),
-		     errno, strerror(errno));
+		LOGPMGW(mgcp, LOGL_ERROR, "Failed to read: %s: %d='%s'\n",
+		     osmo_sock_get_name2(fd->fd), errno, strerror(errno));
 
 		msgb_free(msg);
 		return -1;
 	} else if (ret > 4096 - 128) {
-		LOGP(DLMGCP, LOGL_ERROR, "Too much data: %s: %d\n", osmo_sock_get_name2(fd->fd), ret);
+		LOGPMGW(mgcp, LOGL_ERROR, "Too much data: %s: %d\n", osmo_sock_get_name2(fd->fd), ret);
 		msgb_free(msg);
 		return -1;
 	}
@@ -739,15 +740,17 @@
 static int mgcp_do_write(struct osmo_fd *fd, struct msgb *msg)
 {
 	int ret;
+	struct mgcp_client *mgcp = fd->data;
 
-	LOGP(DLMGCP, LOGL_DEBUG, "Tx MGCP: %s: len=%u '%s'...\n",
-	     osmo_sock_get_name2(fd->fd), msg->len, osmo_escape_str((const char*)msg->data, OSMO_MIN(42, msg->len)));
+	LOGPMGW(mgcp, LOGL_DEBUG, "Tx MGCP: %s: len=%u '%s'...\n",
+		osmo_sock_get_name2(fd->fd), msg->len,
+		osmo_escape_str((const char*)msg->data,OSMO_MIN(42, msg->len)));
 
 	ret = write(fd->fd, msg->data, msg->len);
 	if (ret != msg->len)
-		LOGP(DLMGCP, LOGL_ERROR, "Failed to Tx MGCP: %s: %d='%s'; msg: len=%u '%s'...\n",
-		     osmo_sock_get_name2(fd->fd), errno, strerror(errno),
-		     msg->len, osmo_escape_str((const char*)msg->data, OSMO_MIN(42, msg->len)));
+		LOGPMGW(mgcp, LOGL_ERROR, "Failed to Tx MGCP: %s: %d='%s'; msg: len=%u '%s'...\n",
+			osmo_sock_get_name2(fd->fd), errno, strerror(errno),
+			msg->len, osmo_escape_str((const char*)msg->data, OSMO_MIN(42, msg->len)));
 	return ret;
 }
 
@@ -780,12 +783,12 @@
 	if (osmo_strlcpy(mgcp->actual.endpoint_domain_name, conf->endpoint_domain_name,
 			 sizeof(mgcp->actual.endpoint_domain_name))
 	    >= sizeof(mgcp->actual.endpoint_domain_name)) {
-		LOGP(DLMGCP, LOGL_ERROR, "MGCP client: endpoint domain name is too long, max length is %zu: '%s'\n",
-		     sizeof(mgcp->actual.endpoint_domain_name) - 1, conf->endpoint_domain_name);
+		LOGPMGW(mgcp, LOGL_ERROR, "MGCP client: endpoint domain name is too long, max length is %zu: '%s'\n",
+			sizeof(mgcp->actual.endpoint_domain_name) - 1, conf->endpoint_domain_name);
 		talloc_free(mgcp);
 		return NULL;
 	}
-	LOGP(DLMGCP, LOGL_NOTICE, "MGCP client: using endpoint domain '@%s'\n", mgcp_client_endpoint_domain(mgcp));
+	LOGPMGW(mgcp, LOGL_NOTICE, "MGCP client: using endpoint domain '@%s'\n", mgcp_client_endpoint_domain(mgcp));
 
 	INIT_LLIST_HEAD(&mgcp->actual.reset_epnames);
 	llist_for_each_entry(reset_ep, &conf->reset_epnames, list) {
@@ -793,6 +796,9 @@
 		llist_add_tail(&actual_reset_ep->list, &mgcp->actual.reset_epnames);
 	}
 
+	if (conf->description)
+		mgcp->actual.description = talloc_strdup(mgcp, conf->description);
+
 	return mgcp;
 }
 
@@ -821,21 +827,21 @@
 
 		if (i == retry_n_ports) {
 			/* Last try failed */
-			LOGP(DLMGCP, LOGL_NOTICE, "MGCPGW failed to bind to %s:%d -- check configuration!\n",
+			LOGPMGW(mgcp, LOGL_NOTICE, "Failed to bind to %s:%d -- check configuration!\n",
 			     mgcp->actual.local_addr ? mgcp->actual.local_addr : "(any)", mgcp->actual.local_port);
 			if (retry_n_ports == 0)
 				return -EINVAL;
 		} else {
 			/* Choose a new port number to try next */
-			LOGP(DLMGCP, LOGL_NOTICE,
-			     "MGCPGW failed to bind to %s:%d, retrying with port %d -- check configuration!\n",
+			LOGPMGW(mgcp, LOGL_NOTICE,
+			     "Failed to bind to %s:%d, retrying with port %d -- check configuration!\n",
 			     mgcp->actual.local_addr ? mgcp->actual.local_addr : "(any)", mgcp->actual.local_port,
 			     mgcp->actual.local_port + 1);
 			mgcp->actual.local_port++;
 		}
 	}
 
-	LOGP(DLMGCP, LOGL_FATAL, "MGCPGW failed to find a port to bind on %u times -- check configuration!\n", i);
+	LOGPMGW(mgcp, LOGL_FATAL, "Failed to find a port to bind on %u times -- check configuration!\n", i);
 	return -EINVAL;
 }
 
@@ -862,12 +868,12 @@
 
 	rc = snprintf(endpoint, sizeof(endpoint), "%s@%s", name, mgcp_client_endpoint_domain(mgcp));
 	if (rc > sizeof(endpoint) - 1) {
-		LOGP(DLMGCP, LOGL_ERROR, "MGCP endpoint exceeds maximum length of %zu: '%s@%s'\n",
-		     sizeof(endpoint) - 1, name, mgcp_client_endpoint_domain(mgcp));
+		LOGPMGW(mgcp, LOGL_ERROR, "MGCP endpoint exceeds maximum length of %zu: '%s@%s'\n",
+			sizeof(endpoint) - 1, name, mgcp_client_endpoint_domain(mgcp));
 		return NULL;
 	}
 	if (rc < 1) {
-		LOGP(DLMGCP, LOGL_ERROR, "Cannot compose MGCP endpoint name\n");
+		LOGPMGW(mgcp, LOGL_ERROR, "Cannot compose MGCP endpoint name\n");
 		return NULL;
 	}
 	return endpoint;
@@ -885,7 +891,7 @@
 	const char *epname;
 
 	if (!mgcp) {
-		LOGP(DLMGCP, LOGL_FATAL, "MGCPGW client not initialized properly\n");
+		LOGPMGW(mgcp, LOGL_FATAL, "Client not initialized properly\n");
 		return -EINVAL;
 	}
 
@@ -898,21 +904,21 @@
 
 	rc = init_socket(mgcp, retry_n_ports);
 	if (rc < 0) {
-		LOGP(DLMGCP, LOGL_FATAL,
-		     "Failed to initialize socket %s:%u -> %s:%u for MGCP GW: %s\n",
+		LOGPMGW(mgcp, LOGL_FATAL,
+		     "Failed to initialize socket %s:%u -> %s:%u for MGW: %s\n",
 		     mgcp->actual.local_addr ? mgcp->actual.local_addr : "(any)", mgcp->actual.local_port,
 		     mgcp->actual.remote_addr ? mgcp->actual.local_addr : "(any)", mgcp->actual.remote_port,
 		     strerror(errno));
 		goto error_close_fd;
 	}
 
-	LOGP(DLMGCP, LOGL_INFO, "MGCP GW connection: %s\n", osmo_sock_get_name2(wq->bfd.fd));
+	LOGPMGW(mgcp, LOGL_INFO, "MGW connection: %s\n", osmo_sock_get_name2(wq->bfd.fd));
 
 	/* If configured, send a DLCX message to the endpoints that are configured to
 	 * be reset on startup. Usually this is a wildcarded endpoint. */
 	llist_for_each_entry(reset_ep, &mgcp->actual.reset_epnames, list) {
 		epname = _mgcp_client_name_append_domain(mgcp, reset_ep->name);
-		LOGP(DLMGCP, LOGL_INFO, "MGCP GW sending DLCX to: %s\n", epname);
+		LOGPMGW(mgcp, LOGL_INFO, "Sending DLCX to: %s\n", epname);
 		_mgcp_client_send_dlcx(mgcp, epname);
 	}
 	return 0;
@@ -938,13 +944,13 @@
 	struct osmo_wqueue *wq;
 
 	if (!mgcp) {
-		LOGP(DLMGCP, LOGL_FATAL, "MGCPGW client not initialized properly\n");
+		LOGP(DLMGCP, LOGL_FATAL, "MGCP client not initialized properly\n");
 		return;
 	}
 
 	wq = &mgcp->wq;
 	osmo_wqueue_clear(wq);
-	LOGP(DLMGCP, LOGL_INFO, "MGCP GW connection: %s -- closed!\n", osmo_sock_get_name2(wq->bfd.fd));
+	LOGPMGW(mgcp, LOGL_INFO, "MGCP association: %s -- closed!\n", osmo_sock_get_name2(wq->bfd.fd));
 	close(wq->bfd.fd);
 	wq->bfd.fd = -1;
 	if (osmo_fd_is_registered(&wq->bfd))
@@ -1014,7 +1020,7 @@
 	    talloc_asprintf(ctx, "ds/e1-%u/s-%u/su%u-%u@%s", trunk_id, ts, rate, offset,
 			    mgcp_client_endpoint_domain(mgcp));
 	if (!epname) {
-		LOGP(DLMGCP, LOGL_ERROR, "Cannot compose MGCP e1-endpoint name!\n");
+		LOGPMGW(mgcp, LOGL_ERROR, "Cannot compose MGCP e1-endpoint name!\n");
 		return NULL;
 	}
 
@@ -1024,9 +1030,9 @@
 			rate_offs_valid = true;
 	}
 	if (!rate_offs_valid) {
-		LOGP(DLMGCP, LOGL_ERROR,
-		     "Cannot compose MGCP e1-endpoint name (%s), rate(%u)/offset(%u) combination is invalid!\n", epname,
-		     rate, offset);
+		LOGPMGW(mgcp, LOGL_ERROR,
+			"Cannot compose MGCP e1-endpoint name (%s), rate(%u)/offset(%u) combination is invalid!\n",
+			epname, rate, offset);
 		talloc_free(epname);
 		return NULL;
 	}
@@ -1034,8 +1040,9 @@
 	/* An E1 line has a maximum of 32 timeslots, while the first (ts=0) is
 	 * reserverd for framing and alignment, so we can not use it here. */
 	if (ts == 0 || ts > NUM_E1_TS-1) {
-		LOGP(DLMGCP, LOGL_ERROR,
-		     "Cannot compose MGCP e1-endpoint name (%s), E1-timeslot number (%u) is invalid!\n", epname, ts);
+		LOGPMGW(mgcp, LOGL_ERROR,
+			"Cannot compose MGCP e1-endpoint name (%s), E1-timeslot number (%u) is invalid!\n",
+			epname, ts);
 		talloc_free(epname);
 		return NULL;
 	}
@@ -1063,7 +1070,7 @@
 	return pending;
 }
 
-/* Send the MGCP message in msg to the MGCP GW and handle a response with
+/* Send the MGCP message in msg to the MGW and handle a response with
  * response_cb. NOTE: the response_cb still needs to call
  * mgcp_response_parse_params(response) to get the parsed parameters -- to
  * potentially save some CPU cycles, only the head line has been parsed when
@@ -1080,8 +1087,8 @@
 
 	trans_id = msg->cb[MSGB_CB_MGCP_TRANS_ID];
 	if (!trans_id) {
-		LOGP(DLMGCP, LOGL_ERROR,
-		     "Unset transaction id in mgcp send request\n");
+		LOGPMGW(mgcp, LOGL_ERROR,
+			"Unset transaction id in mgcp send request\n");
 		talloc_free(msg);
 		return -EINVAL;
 	}
@@ -1096,9 +1103,9 @@
 	}
 
 	if (msgb_l2len(msg) > 4096) {
-		LOGP(DLMGCP, LOGL_ERROR,
-		     "Cannot send, MGCP message too large: %u\n",
-		     msgb_l2len(msg));
+		LOGPMGW(mgcp, LOGL_ERROR,
+			"Cannot send, MGCP message too large: %u\n",
+			msgb_l2len(msg));
 		msgb_free(msg);
 		rc = -EINVAL;
 		goto mgcp_tx_error;
@@ -1106,12 +1113,12 @@
 
 	rc = osmo_wqueue_enqueue(&mgcp->wq, msg);
 	if (rc) {
-		LOGP(DLMGCP, LOGL_FATAL, "Could not queue message to MGCP GW\n");
+		LOGPMGW(mgcp, LOGL_FATAL, "Could not queue message to MGW\n");
 		msgb_free(msg);
 		goto mgcp_tx_error;
 	} else
-		LOGP(DLMGCP, LOGL_DEBUG, "Queued %u bytes for MGCP GW\n",
-		     msgb_l2len(msg));
+		LOGPMGW(mgcp, LOGL_DEBUG, "Queued %u bytes for MGW\n",
+			msgb_l2len(msg));
 	return 0;
 
 mgcp_tx_error:
@@ -1138,10 +1145,10 @@
 	struct mgcp_response_pending *pending = mgcp_client_response_pending_get(mgcp, trans_id);
 	if (!pending) {
 		/*! Note: it is not harmful to cancel a transaction twice. */
-		LOGP(DLMGCP, LOGL_ERROR, "Cannot cancel, no such transaction: %u\n", trans_id);
+		LOGPMGW(mgcp, LOGL_ERROR, "Cannot cancel, no such transaction: %u\n", trans_id);
 		return -ENOENT;
 	}
-	LOGP(DLMGCP, LOGL_DEBUG, "Canceled transaction %u\n", trans_id);
+	LOGPMGW(mgcp, LOGL_DEBUG, "Canceled transaction %u\n", trans_id);
 	talloc_free(pending);
 	return 0;
 	/*! We don't really need to clean up the wqueue: In all sane cases, the msgb has already been sent
@@ -1233,8 +1240,8 @@
 
 	/* Determine local IP-Address */
 	if (osmo_sock_local_ip(local_ip, mgcp->actual.remote_addr) < 0) {
-		LOGP(DLMGCP, LOGL_ERROR,
-		     "Could not determine local IP-Address!\n");
+		LOGPMGW(mgcp, LOGL_ERROR,
+			"Could not determine local IP-Address!\n");
 		msgb_free(msg);
 		return -EINVAL;
 	}
@@ -1259,14 +1266,14 @@
 
 	/* Add RTP address and port */
 	if (mgcp_msg->audio_port == 0) {
-		LOGP(DLMGCP, LOGL_ERROR,
-		     "Invalid port number, can not generate MGCP message\n");
+		LOGPMGW(mgcp, LOGL_ERROR,
+			"Invalid port number, can not generate MGCP message\n");
 		msgb_free(msg);
 		return -EINVAL;
 	}
 	if (strlen(mgcp_msg->audio_ip) <= 0) {
-		LOGP(DLMGCP, LOGL_ERROR,
-		     "Empty ip address, can not generate MGCP message\n");
+		LOGPMGW(mgcp, LOGL_ERROR,
+			"Empty ip address, can not generate MGCP message\n");
 		msgb_free(msg);
 		return -EINVAL;
 	}
@@ -1323,8 +1330,7 @@
 		rc |= msgb_printf(msg, "a=ptime:%u\r\n", mgcp_msg->ptime);
 
 	if (rc != 0) {
-		LOGP(DLMGCP, LOGL_ERROR,
-		     "message buffer to small, can not generate MGCP message (SDP)\n");
+		LOGPMGW(mgcp, LOGL_ERROR, "Message buffer to small, can not generate MGCP message (SDP)\n");
 		msgb_free(msg);
 		return -ENOBUFS;
 	}
@@ -1371,16 +1377,15 @@
 		rc |= msgb_printf(msg, "RSIP %u", trans_id);
 		break;
 	default:
-		LOGP(DLMGCP, LOGL_ERROR,
-		     "Invalid command verb, can not generate MGCP message\n");
+		LOGPMGW(mgcp, LOGL_ERROR, "Invalid command verb, can not generate MGCP message\n");
 		msgb_free(msg);
 		return NULL;
 	}
 
 	/* Check if mandatory fields are missing */
 	if (!((mgcp_msg->presence & mandatory_mask) == mandatory_mask)) {
-		LOGP(DLMGCP, LOGL_ERROR,
-		     "One or more missing mandatory fields, can not generate MGCP message\n");
+		LOGPMGW(mgcp, LOGL_ERROR,
+			"One or more missing mandatory fields, can not generate MGCP message\n");
 		msgb_free(msg);
 		return NULL;
 	}
@@ -1388,16 +1393,15 @@
 	/* Add endpoint name */
 	if (mgcp_msg->presence & MGCP_MSG_PRESENCE_ENDPOINT) {
 		if (strlen(mgcp_msg->endpoint) <= 0) {
-			LOGP(DLMGCP, LOGL_ERROR,
-			     "Empty endpoint name, can not generate MGCP message\n");
+			LOGPMGW(mgcp, LOGL_ERROR, "Empty endpoint name, can not generate MGCP message\n");
 			msgb_free(msg);
 			return NULL;
 		}
 
 		if (strstr(mgcp_msg->endpoint, "@") == NULL) {
-			LOGP(DLMGCP, LOGL_ERROR,
-			     "Endpoint name (%s) lacks separator (@), can not generate MGCP message\n",
-			     mgcp_msg->endpoint);
+			LOGPMGW(mgcp, LOGL_ERROR,
+				"Endpoint name (%s) lacks separator (@), can not generate MGCP message\n",
+				mgcp_msg->endpoint);
 			msgb_free(msg);
 			return NULL;
 		}
@@ -1415,8 +1419,7 @@
 	/* Add connection id */
 	if (mgcp_msg->presence & MGCP_MSG_PRESENCE_CONN_ID) {
 		if (strlen(mgcp_msg->conn_id) <= 0) {
-			LOGP(DLMGCP, LOGL_ERROR,
-			     "Empty connection id, can not generate MGCP message\n");
+			LOGPMGW(mgcp, LOGL_ERROR, "Empty connection id, can not generate MGCP message\n");
 			msgb_free(msg);
 			return NULL;
 		}
@@ -1453,9 +1456,8 @@
 	/* Add X-Osmo-Osmux */
 	if ((mgcp_msg->presence & MGCP_MSG_PRESENCE_X_OSMO_OSMUX_CID)) {
 		if (mgcp_msg->x_osmo_osmux_cid < -1 || mgcp_msg->x_osmo_osmux_cid > OSMUX_CID_MAX) {
-			LOGP(DLMGCP, LOGL_ERROR,
-			     "Wrong Osmux CID %d, can not generate MGCP message\n",
-			     mgcp_msg->x_osmo_osmux_cid);
+			LOGPMGW(mgcp, LOGL_ERROR, "Wrong Osmux CID %d, can not generate MGCP message\n",
+				mgcp_msg->x_osmo_osmux_cid);
 			msgb_free(msg);
 			return NULL;
 		}
@@ -1475,8 +1477,7 @@
 	}
 
 	if (rc != 0) {
-		LOGP(DLMGCP, LOGL_ERROR,
-		     "message buffer to small, can not generate MGCP message\n");
+		LOGPMGW(mgcp, LOGL_ERROR, "Message buffer to small, can not generate MGCP message\n");
 		msgb_free(msg);
 		msg = NULL;
 	}
@@ -1508,3 +1509,22 @@
 	{ MGCP_CONN_LOOPBACK, "loopback" },
 	{ 0, NULL }
 };
+
+/*! Get MGCP client instance mame (VTY).
+ *  \param[in] mgcp MGCP client descriptor.
+ *  \returns MGCP client name.
+ * 
+ *  The user can only modify the name of an MGCP client instance when it is
+ *  part of a pool. For single MGCP client instances and MGCP client instance
+ *  where no description is set via the VTY, the MGW domain name will be used
+ *  as name. */
+const char *mgcp_client_name(const struct mgcp_client *mgcp)
+{
+	if (!mgcp)
+		return "(null)";
+
+	if (mgcp->actual.description)
+		return mgcp->actual.description;
+	else
+		return mgcp_client_endpoint_domain(mgcp);
+}
diff --git a/src/libosmo-mgcp-client/mgcp_client_pool.c b/src/libosmo-mgcp-client/mgcp_client_pool.c
index 9311ac9..b61319d 100644
--- a/src/libosmo-mgcp-client/mgcp_client_pool.c
+++ b/src/libosmo-mgcp-client/mgcp_client_pool.c
@@ -24,6 +24,38 @@
 #include <osmocom/mgcp_client/mgcp_client_pool.h>
 #include <stddef.h>
 
+#define LOGPPMGW(pool_member, level, fmt, args...) \
+LOGP(DLMGCP, level, "MGW-pool(%s) " fmt, mgcp_client_pool_member_name(pool_member), ## args)
+
+/* Get a human readable name for a given pool member. */
+const char *mgcp_client_pool_member_name(const struct mgcp_client_pool_member *pool_member)
+{
+	const struct mgcp_client *mpcp_client;
+	struct mgcp_client mpcp_client_dummy;
+	static char name[512];
+	const char *description;
+
+	if (!pool_member)
+		return "(null)";
+
+	/* It is not guranteed that a pool_member has an MGCP client. The client may not yet be initialzed or the
+	 * initalization may have been failed. In this case we will generate a dummy MGCP client to work with. */
+	if (!pool_member->client) {
+		memcpy(&mpcp_client_dummy.actual, &pool_member->conf, sizeof(mpcp_client_dummy.actual));
+		mpcp_client = &mpcp_client_dummy;
+	} else {
+		mpcp_client = pool_member->client;
+	}
+
+	if (mpcp_client->actual.description)
+		description = mpcp_client->actual.description;
+	else
+		description = mgcp_client_endpoint_domain(mpcp_client);
+	snprintf(name, sizeof(name), "%i:%s", pool_member->nr, description);
+
+	return name;
+}
+
 /*! Allocate MGCP client pool. This is called once on startup and before the pool is used with
  *  mgcp_client_pool_vty_init(). Since the pool is linked with the VTY it must exist througout the entire runtime.
  *  \param[in] talloc_ctx talloc context. */
@@ -53,7 +85,7 @@
 		/* Initialize client */
 		pool_member->client = mgcp_client_init(pool_member, &pool_member->conf);
 		if (!pool_member->client) {
-			LOGP(DLMGCP, LOGL_ERROR, "MGW %u initialization failed\n", pool_member->nr);
+			LOGPPMGW(pool_member, LOGL_ERROR, "MGCP client initialization failed\n");
 			continue;
 		}
 
@@ -63,8 +95,8 @@
 
 		/* Connect client */
 		if (mgcp_client_connect2(pool_member->client, 0)) {
-			LOGP(DLMGCP, LOGL_ERROR, "MGW %u connect failed at (%s:%u)\n",
-			     pool_member->nr, pool_member->conf.remote_addr, pool_member->conf.remote_port);
+			LOGPPMGW(pool_member, LOGL_ERROR, "MGCP client connect failed at (%s:%u)\n",
+				 pool_member->conf.remote_addr, pool_member->conf.remote_port);
 			talloc_free(pool_member->client);
 			pool_member->client = NULL;
 			continue;
@@ -104,14 +136,14 @@
 			else if (pool_member_picked->refcount > pool_member->refcount)
 				pool_member_picked = pool_member;
 		} else {
-			LOGP(DLMGCP, LOGL_DEBUG, "MGW pool has %u members -- MGW %u is unusable\n", n_pool_members,
-			     pool_member->nr);
+			LOGPPMGW(pool_member, LOGL_DEBUG, "MGW pool has %u members -- MGW %u is unusable\n", n_pool_members,
+				 pool_member->nr);
 		}
 	}
 
 	if (pool_member_picked) {
-		LOGP(DLMGCP, LOGL_DEBUG, "MGW pool has %u members -- using MGW %u (active calls: %u)\n",
-		     n_pool_members, pool_member_picked->nr, pool_member_picked->refcount);
+		LOGPPMGW(pool_member_picked, LOGL_DEBUG, "MGW pool has %u members -- using MGW %u (active calls: %u)\n",
+			 n_pool_members, pool_member_picked->nr, pool_member_picked->refcount);
 		return pool_member_picked;
 	}
 
@@ -135,7 +167,8 @@
 
 	/* When the pool is empty, return a single MGCP client if it is registered. */
 	if (llist_empty(&pool->pool) && pool->mgcp_client_single) {
-		LOGP(DLMGCP, LOGL_DEBUG, "MGW pool is empty -- using (single) MGW\n");
+		LOGP(DLMGCP, LOGL_DEBUG, "MGW pool is empty -- using (single) MGW %s\n",
+		     mgcp_client_name(pool->mgcp_client_single));
 		return pool->mgcp_client_single;
 	}
 
@@ -177,7 +210,7 @@
 	llist_for_each_entry(pool_member, &pool->pool, list) {
 		if (pool_member->client == mgcp_client) {
 			if (pool_member->refcount == 0) {
-				LOGP(DLMGCP, LOGL_ERROR, "MGW %u has invalid refcount\n", pool_member->nr);
+				LOGPPMGW(pool_member, LOGL_ERROR, "MGW pool member has invalid refcount\n");
 				return;
 			}
 			pool_member->refcount--;
diff --git a/src/libosmo-mgcp-client/mgcp_client_vty.c b/src/libosmo-mgcp-client/mgcp_client_vty.c
index 8719e3c..4bf6cd0 100644
--- a/src/libosmo-mgcp-client/mgcp_client_vty.c
+++ b/src/libosmo-mgcp-client/mgcp_client_vty.c
@@ -242,6 +242,9 @@
 	int port;
 	struct reset_ep *reset_ep;
 
+	if (conf->description)
+		vty_out(vty, "%sdescription %s%s", indent, conf->description, VTY_NEWLINE);
+
 	addr = conf->local_addr;
 	if (addr)
 		vty_out(vty, "%smgw local-ip %s%s", indent, addr,
@@ -366,7 +369,7 @@
 	}
 
 	vty->index = &pool_member->conf;
-	vty->index_sub = NULL;
+	vty->index_sub = &pool_member->conf.description;
 	vty->node = global_mgcp_client_pool->vty_node->node;
 
 	return CMD_SUCCESS;
@@ -387,8 +390,8 @@
 
 	/* Make sure that there are no ongoing calls */
 	if (pool_member->refcount > 0) {
-		vty_out(vty, "%% MGCP client (MGW %u) is still serving ongoing calls -- can't remove it now!%s",
-			pool_member->nr, VTY_NEWLINE);
+		vty_out(vty, "%% MGCP client (MGW %s) is still serving ongoing calls -- can't remove it now!%s",
+			mgcp_client_pool_member_name(pool_member), VTY_NEWLINE);
 		return CMD_WARNING;
 	}
 
@@ -417,8 +420,8 @@
 
 	/* Make sure that there are no ongoing calls */
 	if (pool_member->refcount > 0) {
-		vty_out(vty, "%% MGCP client (MGW %u) is still serving ongoing calls -- can't reconnect it now!%s",
-			pool_member->nr, VTY_NEWLINE);
+		vty_out(vty, "%% MGCP client (MGW %s) is still serving ongoing calls -- can't reconnect it now!%s",
+			mgcp_client_pool_member_name(pool_member), VTY_NEWLINE);
 		return CMD_WARNING;
 	}
 
@@ -431,8 +434,10 @@
 	/* Create a new MGCP client instance with the current config */
 	pool_member->client = mgcp_client_init(pool_member, &pool_member->conf);
 	if (!pool_member->client) {
-		LOGP(DLMGCP, LOGL_ERROR, "(manual) MGW %u initalization failed\n", pool_member->nr);
-		vty_out(vty, "%% MGCP client initalization failed ('%s')%s", argv[0], VTY_NEWLINE);
+		LOGP(DLMGCP, LOGL_ERROR, "(manual) MGW %s initalization failed\n",
+		     mgcp_client_pool_member_name(pool_member));
+		vty_out(vty, "%% MGCP client (MGW %s) initalization failed ('%s')%s",
+			mgcp_client_pool_member_name(pool_member), argv[0], VTY_NEWLINE);
 		return CMD_WARNING;
 	}
 
@@ -441,11 +446,13 @@
 
 	/* Connect client */
 	if (mgcp_client_connect(pool_member->client)) {
-		LOGP(DLMGCP, LOGL_ERROR, "(manual) MGW %u connect failed at (%s:%u)\n",
-		     pool_member->nr, pool_member->conf.remote_addr, pool_member->conf.remote_port);
+		LOGP(DLMGCP, LOGL_ERROR, "(manual) MGW %s connect failed at (%s:%u)\n",
+		     mgcp_client_pool_member_name(pool_member), pool_member->conf.remote_addr,
+		     pool_member->conf.remote_port);
 		talloc_free(pool_member->client);
 		pool_member->client = NULL;
-		vty_out(vty, "%% MGCP client initalization failed ('%s')%s", argv[0], VTY_NEWLINE);
+		vty_out(vty, "%% MGCP client (MGW %s) initalization failed ('%s')%s",
+			mgcp_client_pool_member_name(pool_member), argv[0], VTY_NEWLINE);
 		return CMD_WARNING;
 	}
 
@@ -500,7 +507,7 @@
 	}
 
 	llist_for_each_entry(pool_member, &global_mgcp_client_pool->pool, list) {
-		vty_out(vty, "%%  MGW %u%s", pool_member->nr, VTY_NEWLINE);
+		vty_out(vty, "%%  MGW %s%s", mgcp_client_pool_member_name(pool_member), VTY_NEWLINE);
 		vty_out(vty, "%%   mgcp-client:   %s%s", pool_member->client ? "connected" : "disconnected",
 			VTY_NEWLINE);
 		vty_out(vty, "%%   service:       %s%s", pool_member->blocked ? "blocked" : "unblocked", VTY_NEWLINE);
@@ -538,6 +545,8 @@
 	install_node(pool->vty_node, config_write_pool);
 	vty_init_common(pool, mgw_node);
 
+	install_element(mgw_node, &cfg_description_cmd);
+
 	install_lib_element(ENABLE_NODE, &mgw_reconnect_cmd);
 	install_lib_element(ENABLE_NODE, &mgw_block_cmd);
 	install_lib_element(ENABLE_NODE, &mgw_unblock_cmd);

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/25335
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I53ff5445c8e5faffa4ef908ffb1fdb1f47ea2904
Gerrit-Change-Number: 25335
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210903/f5c67d19/attachment.htm>


More information about the gerrit-log mailing list