Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/12354


Change subject: mgcp_client: drop a bunch of dead code
......................................................................

mgcp_client: drop a bunch of dead code

Remove public API that makes no sense anymore and is dead code.

I see the dropped API as a dead-end initial misconception of the early mgcp
client, and it doesn't really make sense to drag this stuff along. It has not
been used by osmo-msc,-bsc for a long time now, and just confuses the reader.

It is public API, yes, and older versions of osmo-msc / osmo-bsc will not be
able to compile against this, but even if it did, the resulting MGCP client
would not work with the current osmo-mgw: this API is still based on the
premise that the MGCP client dictates the MGW endpoint numbers, a concept that
cannot be used with the current osmo-mgw. Instead, osmo-mgw expects a
wildcarded endpoint upon CRCX and assigns its own endpoint names.

Also, the bts-base configuration is unused and a legacy of when osmo-bsc_mgcp
had explicit BTS and CN sides.

Change-Id: I98a9f1f17a1c4ab20cea3b08c7d21663592134d6
---
M include/osmocom/mgcp_client/mgcp_client.h
M src/libosmo-mgcp-client/mgcp_client.c
M src/libosmo-mgcp-client/mgcp_client_vty.c
M tests/mgcp_client/mgcp_client_test.c
M tests/mgcp_client/mgcp_client_test.err
M tests/mgcp_client/mgcp_client_test.ok
6 files changed, 11 insertions(+), 415 deletions(-)



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

diff --git a/include/osmocom/mgcp_client/mgcp_client.h 
b/include/osmocom/mgcp_client/mgcp_client.h
index 79f2078..c1fd1b0 100644
--- a/include/osmocom/mgcp_client/mgcp_client.h
+++ b/include/osmocom/mgcp_client/mgcp_client.h
@@ -22,9 +22,6 @@
        int local_port;
        const char *remote_addr;
        int remote_port;
-       uint16_t first_endpoint;
-       uint16_t last_endpoint;
-       uint16_t bts_base;
 };

 typedef unsigned int mgcp_trans_id_t;
@@ -123,9 +120,6 @@
 uint16_t mgcp_client_remote_port(struct mgcp_client *mgcp);
 uint32_t mgcp_client_remote_addr_n(struct mgcp_client *mgcp);

-int mgcp_client_next_endpoint(struct mgcp_client *client);
-void mgcp_client_release_endpoint(uint16_t id, struct mgcp_client *client);
-
 /* Invoked when an MGCP response is received or sending failed.  When the
  * response is passed as NULL, this indicates failure during transmission. */
 typedef void (* mgcp_response_cb_t )(struct mgcp_response *response, void 
*priv);
@@ -137,20 +131,6 @@

 enum mgcp_connection_mode;

-struct msgb *mgcp_msg_crcx(struct mgcp_client *mgcp,
-                          uint16_t rtp_endpoint, unsigned int call_id,
-                          enum mgcp_connection_mode mode)
-OSMO_DEPRECATED("Use mgcp_msg_gen() instead");
-
-struct msgb *mgcp_msg_mdcx(struct mgcp_client *mgcp,
-                          uint16_t rtp_endpoint, const char *rtp_conn_addr,
-                          uint16_t rtp_port, enum mgcp_connection_mode mode)
-OSMO_DEPRECATED("Use mgcp_msg_gen() instead");
-
-struct msgb *mgcp_msg_dlcx(struct mgcp_client *mgcp, uint16_t rtp_endpoint,
-                          unsigned int call_id)
-OSMO_DEPRECATED("Use mgcp_msg_gen() instead");
-
 struct msgb *mgcp_msg_gen(struct mgcp_client *mgcp, struct mgcp_msg *mgcp_msg);
 mgcp_trans_id_t mgcp_msg_trans_id(struct msgb *msg);

diff --git a/src/libosmo-mgcp-client/mgcp_client.c 
b/src/libosmo-mgcp-client/mgcp_client.c
index fc9c5d3..2ceab3c 100644
--- a/src/libosmo-mgcp-client/mgcp_client.c
+++ b/src/libosmo-mgcp-client/mgcp_client.c
@@ -190,75 +190,9 @@
                .local_port = -1,
                .remote_addr = NULL,
                .remote_port = -1,
-               .first_endpoint = 0,
-               .last_endpoint = 0,
-               .bts_base = 0,
        };
 }

-/* Test if a given endpoint id is currently in use */
-static bool endpoint_in_use(uint16_t id, struct mgcp_client *client)
-{
-       struct mgcp_inuse_endpoint *endpoint;
-       llist_for_each_entry(endpoint, &client->inuse_endpoints, entry) {
-               if (endpoint->id == id)
-                       return true;
-       }
-
-       return false;
-}
-
-/*! Pick next free endpoint ID.
- *  \param[in,out] client MGCP client descriptor.
- *  \returns 0 on success, -EINVAL on error. */
-int mgcp_client_next_endpoint(struct mgcp_client *client)
-{
-       int i;
-       uint16_t first_endpoint = client->actual.first_endpoint;
-       uint16_t last_endpoint = client->actual.last_endpoint;
-       struct mgcp_inuse_endpoint *endpoint;
-
-       /* Use the maximum permitted range if the VTY
-        * configuration does not specify a range */
-       if (client->actual.last_endpoint == 0) {
-               first_endpoint = 1;
-               last_endpoint = 65534;
-       }
-
-       /* Test the permitted endpoint range for an endpoint
-        * number that is not in use. When a suitable endpoint
-        * number can be found, seize it by adding it to the
-        * inuse list. */
-       for (i=first_endpoint;i<last_endpoint;i++)
-       {
-               if (endpoint_in_use(i,client) == false) {
-                       endpoint = talloc_zero(client, struct 
mgcp_inuse_endpoint);
-                       endpoint->id = i;
-                       llist_add_tail(&endpoint->entry, 
&client->inuse_endpoints);
-                       return endpoint->id;
-               }
-       }
-
-       /* All endpoints are busy! */
-       return -EINVAL;
-}
-
-/*! Release a seized endpoint ID to make it available again for other calls.
- *  \param[in] id Endpoint ID
- *  \param[in,out] client MGCP client descriptor. */
-/* Release a seized endpoint id to make it available again for other calls */
-void mgcp_client_release_endpoint(uint16_t id, struct mgcp_client *client)
-{
-       struct mgcp_inuse_endpoint *endpoint;
-       struct mgcp_inuse_endpoint *endpoint_tmp;
-       llist_for_each_entry_safe(endpoint, endpoint_tmp, 
&client->inuse_endpoints, entry) {
-               if (endpoint->id == id) {
-                       llist_del(&endpoint->entry);
-                       talloc_free(endpoint);
-               }
-       }
-}
-
 static void mgcp_client_handle_response(struct mgcp_client *mgcp,
                                        struct mgcp_response_pending *pending,
                                        struct mgcp_response *response)
@@ -769,10 +703,6 @@
        mgcp->actual.remote_port = conf->remote_port >= 0 ? 
(uint16_t)conf->remote_port :
                MGCP_CLIENT_REMOTE_PORT_DEFAULT;

-       mgcp->actual.first_endpoint = conf->first_endpoint > 0 ? 
(uint16_t)conf->first_endpoint : 0;
-       mgcp->actual.last_endpoint = conf->last_endpoint > 0 ? 
(uint16_t)conf->last_endpoint : 0;
-       mgcp->actual.bts_base = conf->bts_base > 0 ? (uint16_t)conf->bts_base : 
4000;
-
        return mgcp;
 }

@@ -977,54 +907,6 @@
         */
 }

-static struct msgb *mgcp_msg_from_buf(mgcp_trans_id_t trans_id,
-                                     const char *buf, int len)
-{
-       struct msgb *msg;
-
-       if (len > (4096 - 128)) {
-               LOGP(DLMGCP, LOGL_ERROR, "Cannot send to MGCP GW:"
-                    " message too large: %d\n", len);
-               return NULL;
-       }
-
-       msg = msgb_alloc_headroom(4096, 128, "MGCP tx");
-       OSMO_ASSERT(msg);
-
-       char *dst = (char*)msgb_put(msg, len);
-       memcpy(dst, buf, len);
-       msg->l2h = msg->data;
-       msg->cb[MSGB_CB_MGCP_TRANS_ID] = trans_id;
-
-       return msg;
-}
-
-static struct msgb *mgcp_msg_from_str(mgcp_trans_id_t trans_id,
-                                     const char *fmt, ...)
-{
-       static char compose[4096 - 128];
-       va_list ap;
-       int len;
-       OSMO_ASSERT(fmt);
-
-       va_start(ap, fmt);
-       len = vsnprintf(compose, sizeof(compose), fmt, ap);
-       va_end(ap);
-       if (len >= sizeof(compose)) {
-               LOGP(DLMGCP, LOGL_ERROR,
-                    "Message too large: trans_id=%u len=%d\n",
-                    trans_id, len);
-               return NULL;
-       }
-       if (len < 1) {
-               LOGP(DLMGCP, LOGL_ERROR,
-                    "Failed to compose message: trans_id=%u len=%d\n",
-                    trans_id, len);
-               return NULL;
-       }
-       return mgcp_msg_from_buf(trans_id, compose, len);
-}
-
 static mgcp_trans_id_t mgcp_client_next_trans_id(struct mgcp_client *mgcp)
 {
        /* avoid zero trans_id to distinguish from unset trans_id */
@@ -1033,52 +915,6 @@
        return mgcp->next_trans_id ++;
 }

-struct msgb *mgcp_msg_crcx(struct mgcp_client *mgcp,
-                          uint16_t rtp_endpoint, unsigned int call_id,
-                          enum mgcp_connection_mode mode)
-{
-       mgcp_trans_id_t trans_id = mgcp_client_next_trans_id(mgcp);
-       return mgcp_msg_from_str(trans_id,
-                "CRCX %u %x@mgw MGCP 1.0\r\n"
-                "C: %x\r\n"
-                "L: p:20, a:AMR, nt:IN\r\n"
-                "M: %s\r\n"
-                ,
-                trans_id,
-                rtp_endpoint,
-                call_id,
-                mgcp_client_cmode_name(mode));
-}
-
-struct msgb *mgcp_msg_mdcx(struct mgcp_client *mgcp,
-                          uint16_t rtp_endpoint, const char *rtp_conn_addr,
-                          uint16_t rtp_port, enum mgcp_connection_mode mode)
-
-{
-       mgcp_trans_id_t trans_id = mgcp_client_next_trans_id(mgcp);
-       return mgcp_msg_from_str(trans_id,
-                "MDCX %u %x@mgw MGCP 1.0\r\n"
-                "M: %s\r\n"
-                "\r\n"
-                "c=IN IP4 %s\r\n"
-                "m=audio %u RTP/AVP 255\r\n"
-                ,
-                trans_id,
-                rtp_endpoint,
-                mgcp_client_cmode_name(mode),
-                rtp_conn_addr,
-                rtp_port);
-}
-
-struct msgb *mgcp_msg_dlcx(struct mgcp_client *mgcp, uint16_t rtp_endpoint,
-                          unsigned int call_id)
-{
-       mgcp_trans_id_t trans_id = mgcp_client_next_trans_id(mgcp);
-       return mgcp_msg_from_str(trans_id,
-                                "DLCX %u %x@mgw MGCP 1.0\r\n"
-                                "C: %x\r\n", trans_id, rtp_endpoint, call_id);
-}
-
 #define MGCP_CRCX_MANDATORY (MGCP_MSG_PRESENCE_ENDPOINT | \
                             MGCP_MSG_PRESENCE_CALL_ID | \
                             MGCP_MSG_PRESENCE_CONN_MODE)
diff --git a/src/libosmo-mgcp-client/mgcp_client_vty.c 
b/src/libosmo-mgcp-client/mgcp_client_vty.c
index 48fcd70..e19dbee 100644
--- a/src/libosmo-mgcp-client/mgcp_client_vty.c
+++ b/src/libosmo-mgcp-client/mgcp_client_vty.c
@@ -99,23 +99,14 @@
                 MGW_STR "remote bind to connect to MGCP gateway with\n"
                 "remote bind port\n")

-DEFUN(cfg_mgw_endpoint_range, cfg_mgw_endpoint_range_cmd,
+DEFUN_DEPRECATED(cfg_mgw_endpoint_range, cfg_mgw_endpoint_range_cmd,
       "mgw endpoint-range <1-65534> <1-65534>",
-      MGW_STR "usable range of endpoint identifiers\n"
-      "set first usable endpoint identifier\n"
-      "set last usable endpoint identifier\n")
+      MGW_STR "DEPRECATED: the endpoint range cannot be defined by the 
client\n"
+      "-\n" "-\n")
 {
-       uint16_t first_endpoint = atoi(argv[0]);
-       uint16_t last_endpoint = atoi(argv[1]);
-
-       if (last_endpoint < first_endpoint) {
-               vty_out(vty, "last endpoint must be greater than first 
endpoint!%s",
-                       VTY_NEWLINE);
-               return CMD_SUCCESS;
-       }
-
-       global_mgcp_client_conf->first_endpoint = first_endpoint;
-       global_mgcp_client_conf->last_endpoint = last_endpoint;
+       vty_out(vty, "Please do not use legacy config 'mgw endpoint-range'"
+               " (the range can no longer be defined by the MGCP client)%s",
+               VTY_NEWLINE);
        return CMD_SUCCESS;
 }
 ALIAS_DEPRECATED(cfg_mgw_endpoint_range, cfg_mgcpgw_endpoint_range_cmd,
@@ -126,14 +117,15 @@

 #define BTS_START_STR "First UDP port allocated for the BTS side\n"
 #define UDP_PORT_STR "UDP Port number\n"
-DEFUN(cfg_mgw_rtp_bts_base_port,
+DEFUN_DEPRECATED(cfg_mgw_rtp_bts_base_port,
       cfg_mgw_rtp_bts_base_port_cmd,
       "mgw bts-base <0-65534>",
       MGW_STR
-      BTS_START_STR
-      UDP_PORT_STR)
+      "DEPRECATED: there is no explicit BTS side in current osmo-mgw\n" "-\n")
 {
-       global_mgcp_client_conf->bts_base = atoi(argv[0]);
+       vty_out(vty, "Please do not use legacy config 'mgw bts-base'"
+               " (there is no explicit BTS side in an MGW anymore)%s",
+               VTY_NEWLINE);
        return CMD_SUCCESS;
 }
 ALIAS_DEPRECATED(cfg_mgw_rtp_bts_base_port,
@@ -147,9 +139,6 @@
 {
        const char *addr;
        int port;
-       uint16_t first_endpoint;
-       uint16_t last_endpoint;
-       uint16_t bts_base;

        addr = global_mgcp_client_conf->local_addr;
        if (addr)
@@ -169,19 +158,6 @@
                vty_out(vty, "%smgw remote-port %u%s", indent,
                        (uint16_t)port, VTY_NEWLINE);

-       first_endpoint = global_mgcp_client_conf->first_endpoint;
-       last_endpoint = global_mgcp_client_conf->last_endpoint;
-       if (last_endpoint != 0) {
-               vty_out(vty, "%smgw endpoint-range %u %u%s", indent,
-                       first_endpoint, last_endpoint, VTY_NEWLINE);
-       }
-
-       bts_base = global_mgcp_client_conf->bts_base;
-       if (bts_base) {
-               vty_out(vty, "%smgw bts-base %u%s", indent,
-                       bts_base, VTY_NEWLINE);
-       }
-
        return CMD_SUCCESS;
 }

diff --git a/tests/mgcp_client/mgcp_client_test.c 
b/tests/mgcp_client/mgcp_client_test.c
index e6982e4..1db70cf 100644
--- a/tests/mgcp_client/mgcp_client_test.c
+++ b/tests/mgcp_client/mgcp_client_test.c
@@ -135,87 +135,6 @@
        return trans_id;
 }

-void test_crcx(void)
-{
-       struct msgb *msg;
-       mgcp_trans_id_t trans_id;
-
-       printf("\n===== %s =====\n", __func__);
-
-       if (mgcp)
-               talloc_free(mgcp);
-       mgcp = mgcp_client_init(ctx, &conf);
-
-       msg = mgcp_msg_crcx(mgcp, 23, 42, MGCP_CONN_LOOPBACK);
-       trans_id = dummy_mgcp_send(msg);
-
-       reply_to(trans_id, 200, "OK",
-               "I: 1\r\n\r\n"
-               "v=0\r\n"
-               "o=- 1 23 IN IP4 10.9.1.120\r\n"
-               "s=-\r\n"
-               "c=IN IP4 10.9.1.120\r\n"
-               "t=0 0\r\n"
-               "m=audio 16002 RTP/AVP 110 96\r\n"
-               "a=rtpmap:110 AMR/8000\r\n"
-               "a=rtpmap:96 GSM-EFR/8000\r\n"
-               "a=ptime:20\r\n");
-}
-
-void test_crcx_long_conn_id(void)
-{
-       struct msgb *msg;
-       mgcp_trans_id_t trans_id;
-
-       printf("\n===== %s =====\n", __func__);
-
-       if (mgcp)
-               talloc_free(mgcp);
-       mgcp = mgcp_client_init(ctx, &conf);
-
-       msg = mgcp_msg_crcx(mgcp, 23, 42, MGCP_CONN_LOOPBACK);
-       trans_id = dummy_mgcp_send(msg);
-
-       reply_to(trans_id, 200, "OK",
-               "I: 123456789abcdef0123456789ABCDEF0\r\n\r\n"
-               "v=0\r\n"
-               "o=- 1 23 IN IP4 10.9.1.120\r\n"
-               "s=-\r\n"
-               "c=IN IP4 10.9.1.120\r\n"
-               "t=0 0\r\n"
-               "m=audio 16002 RTP/AVP 110 96\r\n"
-               "a=rtpmap:110 AMR/8000\r\n"
-               "a=rtpmap:96 GSM-EFR/8000\r\n"
-               "a=ptime:20\r\n");
-}
-
-void test_crcx_too_long_conn_id(void)
-{
-       struct msgb *msg;
-       mgcp_trans_id_t trans_id;
-
-       printf("\n===== %s =====\n", __func__);
-
-       if (mgcp)
-               talloc_free(mgcp);
-       mgcp = mgcp_client_init(ctx, &conf);
-
-       msg = mgcp_msg_crcx(mgcp, 23, 42, MGCP_CONN_LOOPBACK);
-       trans_id = dummy_mgcp_send(msg);
-
-       reply_to(trans_id, 200, "OK",
-               "I: 123456789abcdef0123456789ABCDEF01001029\r\n\r\n"
-               "v=0\r\n"
-               "o=- 1 23 IN IP4 10.9.1.120\r\n"
-               "s=-\r\n"
-               "c=IN IP4 10.9.1.120\r\n"
-               "t=0 0\r\n"
-               "m=audio 16002 RTP/AVP 110 96\r\n"
-               "a=rtpmap:110 AMR/8000\r\n"
-               "a=rtpmap:96 GSM-EFR/8000\r\n"
-               "a=ptime:20\r\n");
-}
-
 void test_mgcp_msg(void)
 {
        struct msgb *msg;
@@ -618,14 +537,11 @@

        mgcp_client_conf_init(&conf);

-       test_crcx();
        test_mgcp_msg();
        test_mgcp_client_cancel();
        test_sdp_section_start();
        test_map_codec_to_pt_and_map_pt_to_codec();
        test_map_pt_to_codec();
-       test_crcx_long_conn_id();
-       test_crcx_too_long_conn_id();

        printf("Done\n");
        fprintf(stderr, "Done\n");
diff --git a/tests/mgcp_client/mgcp_client_test.err 
b/tests/mgcp_client/mgcp_client_test.err
index b20f93f..1d5a1a0 100644
--- a/tests/mgcp_client/mgcp_client_test.err
+++ b/tests/mgcp_client/mgcp_client_test.err
@@ -66,6 +66,4 @@
 DLMGCP ptmap contains illegal mapping: codec=0 maps to pt=100
 DLMGCP ptmap contains illegal mapping: codec=113 maps to pt=2
 DLMGCP ptmap contains illegal mapping: codec=0 maps to pt=100
-DLMGCP Failed to parse MGCP response (parameter label: I): the received conn 
ID is too long: 39, maximum is 32 characters
-DLMGCP Cannot parse MGCP response (head parameters)
 Done
diff --git a/tests/mgcp_client/mgcp_client_test.ok 
b/tests/mgcp_client/mgcp_client_test.ok
index 40e32ba..65b5298 100644
--- a/tests/mgcp_client/mgcp_client_test.ok
+++ b/tests/mgcp_client/mgcp_client_test.ok
@@ -1,46 +1,4 @@
 
-===== test_crcx =====
-composed:
------
-CRCX 1 17@mgw MGCP 1.0
-C: 2a
-L: p:20, a:AMR, nt:IN
-M: loopback
-
------
-composed response:
------
-200 1 OK
-I: 1
-
-v=0
-o=- 1 23 IN IP4 10.9.1.120
-s=-
-c=IN IP4 10.9.1.120
-t=0 0
-m=audio 16002 RTP/AVP 110 96
-a=rtpmap:110 AMR/8000
-a=rtpmap:96 GSM-EFR/8000
-a=ptime:20
-
------
-response cb received:
-  head.response_code = 200
-  head.trans_id = 1
-  head.conn_id = 1
-  head.comment = OK
-  audio_port = 16002
-  audio_ip = 10.9.1.120
-  ptime = 20
-  codecs_len = 2
-  codecs[0] = 112
-  codecs[1] = 110
-  ptmap_len = 2
-  ptmap[0].codec = 112
-  ptmap[0].pt = 110
-  ptmap[1].codec = 110
-  ptmap[1].pt = 96
-
 Generated CRCX message:
 CRCX 1 23@mgw MGCP 1.0
 C: 2f
@@ -196,72 +154,4 @@
  2 <= 2
  100 <= 100

-
-===== test_crcx_long_conn_id =====
-composed:
------
-CRCX 1 17@mgw MGCP 1.0
-C: 2a
-L: p:20, a:AMR, nt:IN
-M: loopback
-
------
-composed response:
------
-200 1 OK
-I: 123456789abcdef0123456789ABCDEF0
-
-v=0
-o=- 1 23 IN IP4 10.9.1.120
-s=-
-c=IN IP4 10.9.1.120
-t=0 0
-m=audio 16002 RTP/AVP 110 96
-a=rtpmap:110 AMR/8000
-a=rtpmap:96 GSM-EFR/8000
-a=ptime:20
-
------
-response cb received:
-  head.response_code = 200
-  head.trans_id = 1
-  head.conn_id = 123456789abcdef0123456789ABCDEF0
-  head.comment = OK
-  audio_port = 16002
-  audio_ip = 10.9.1.120
-  ptime = 20
-  codecs_len = 2
-  codecs[0] = 112
-  codecs[1] = 110
-  ptmap_len = 2
-  ptmap[0].codec = 112
-  ptmap[0].pt = 110
-  ptmap[1].codec = 110
-  ptmap[1].pt = 96
-
-===== test_crcx_too_long_conn_id =====
-composed:
------
-CRCX 1 17@mgw MGCP 1.0
-C: 2a
-L: p:20, a:AMR, nt:IN
-M: loopback
-
------
-composed response:
------
-200 1 OK
-I: 123456789abcdef0123456789ABCDEF01001029
-
-v=0
-o=- 1 23 IN IP4 10.9.1.120
-s=-
-c=IN IP4 10.9.1.120
-t=0 0
-m=audio 16002 RTP/AVP 110 96
-a=rtpmap:110 AMR/8000
-a=rtpmap:96 GSM-EFR/8000
-a=ptime:20
-
------
 Done

--
To view, visit https://gerrit.osmocom.org/12354
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I98a9f1f17a1c4ab20cea3b08c7d21663592134d6
Gerrit-Change-Number: 12354
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr <[email protected]>

Reply via email to