<p>Hoernchen <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/25428">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
pespin: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">endp: do not cache cfg pointer<br><br>There is no obvious reason why we would want to complicate the code by<br>caching pointers, since pointer traversal is probably not a performance<br>bottleneck, and if it is we should rather take a look at our dozens of<br>linked lists first..<br><br>Change-Id: I2456ba63598f76200d53e00223abf60bb36a49c0<br>---<br>M include/osmocom/mgcp/mgcp_endp.h<br>M src/libosmo-mgcp/mgcp_conn.c<br>M src/libosmo-mgcp/mgcp_endp.c<br>M src/libosmo-mgcp/mgcp_network.c<br>M src/libosmo-mgcp/mgcp_osmux.c<br>M src/libosmo-mgcp/mgcp_protocol.c<br>M src/libosmo-mgcp/mgcp_sdp.c<br>M src/libosmo-mgcp/mgcp_stat.c<br>M src/libosmo-mgcp/mgcp_vty.c<br>M tests/mgcp/mgcp_test.c<br>10 files changed, 26 insertions(+), 33 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/mgcp/mgcp_endp.h b/include/osmocom/mgcp/mgcp_endp.h</span><br><span>index f687bae..b8796c1 100644</span><br><span>--- a/include/osmocom/mgcp/mgcp_endp.h</span><br><span>+++ b/include/osmocom/mgcp/mgcp_endp.h</span><br><span>@@ -97,9 +97,6 @@</span><br><span> /*! List of struct mgcp_conn, of the connections active on this endpoint */</span><br><span> struct llist_head conns;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /*! Backpointer to the MGW configuration */</span><br><span style="color: hsl(0, 100%, 40%);">- struct mgcp_config *cfg;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> /*! Backpointer to the trunk this endpoint belongs to */</span><br><span> struct mgcp_trunk *trunk;</span><br><span> </span><br><span>diff --git a/src/libosmo-mgcp/mgcp_conn.c b/src/libosmo-mgcp/mgcp_conn.c</span><br><span>index ca12347..4acf18c 100644</span><br><span>--- a/src/libosmo-mgcp/mgcp_conn.c</span><br><span>+++ b/src/libosmo-mgcp/mgcp_conn.c</span><br><span>@@ -143,7 +143,7 @@</span><br><span> </span><br><span> void mgcp_conn_watchdog_kick(struct mgcp_conn *conn)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- int timeout = conn->endp->cfg->conn_timeout;</span><br><span style="color: hsl(120, 100%, 40%);">+ int timeout = conn->endp->trunk->cfg->conn_timeout;</span><br><span> if (!timeout)</span><br><span> return;</span><br><span> </span><br><span>diff --git a/src/libosmo-mgcp/mgcp_endp.c b/src/libosmo-mgcp/mgcp_endp.c</span><br><span>index 4fcddb8..19446ce 100644</span><br><span>--- a/src/libosmo-mgcp/mgcp_endp.c</span><br><span>+++ b/src/libosmo-mgcp/mgcp_endp.c</span><br><span>@@ -89,7 +89,6 @@</span><br><span> return NULL;</span><br><span> </span><br><span> INIT_LLIST_HEAD(&endp->conns);</span><br><span style="color: hsl(0, 100%, 40%);">- endp->cfg = trunk->cfg;</span><br><span> endp->trunk = trunk;</span><br><span> </span><br><span> switch (trunk->trunk_type) {</span><br><span>diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c</span><br><span>index 5249fef..2d275ec 100644</span><br><span>--- a/src/libosmo-mgcp/mgcp_network.c</span><br><span>+++ b/src/libosmo-mgcp/mgcp_network.c</span><br><span>@@ -112,7 +112,7 @@</span><br><span> char *bind_addr;</span><br><span> </span><br><span> /* Try probing the local IP-Address */</span><br><span style="color: hsl(0, 100%, 40%);">- if (endp->cfg->net_ports.bind_addr_probe && rem_addr_set) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (endp->trunk->cfg->net_ports.bind_addr_probe && rem_addr_set) {</span><br><span> rc = osmo_sock_local_ip(addr, osmo_sockaddr_ntop(&conn->end.addr.u.sa, ipbuf));</span><br><span> if (rc < 0)</span><br><span> LOGPCONN(conn->conn, DRTP, LOGL_ERROR,</span><br><span>@@ -130,13 +130,13 @@</span><br><span> /* Check there is a bind IP for the RTP traffic configured,</span><br><span> * if so, use that IP-Address */</span><br><span> bind_addr = conn->end.addr.u.sa.sa_family == AF_INET6 ?</span><br><span style="color: hsl(0, 100%, 40%);">- endp->cfg->net_ports.bind_addr_v6 :</span><br><span style="color: hsl(0, 100%, 40%);">- endp->cfg->net_ports.bind_addr_v4;</span><br><span style="color: hsl(120, 100%, 40%);">+ endp->trunk->cfg->net_ports.bind_addr_v6 :</span><br><span style="color: hsl(120, 100%, 40%);">+ endp->trunk->cfg->net_ports.bind_addr_v4;</span><br><span> } else {</span><br><span> /* Choose any of the bind addresses, preferring v6 over v4 */</span><br><span style="color: hsl(0, 100%, 40%);">- bind_addr = endp->cfg->net_ports.bind_addr_v6;</span><br><span style="color: hsl(120, 100%, 40%);">+ bind_addr = endp->trunk->cfg->net_ports.bind_addr_v6;</span><br><span> if (!strlen(bind_addr))</span><br><span style="color: hsl(0, 100%, 40%);">- bind_addr = endp->cfg->net_ports.bind_addr_v4;</span><br><span style="color: hsl(120, 100%, 40%);">+ bind_addr = endp->trunk->cfg->net_ports.bind_addr_v4;</span><br><span> }</span><br><span> if (strlen(bind_addr)) {</span><br><span> LOGPCONN(conn->conn, DRTP, LOGL_DEBUG,</span><br><span>@@ -146,7 +146,7 @@</span><br><span> /* No specific bind IP is configured for the RTP traffic, so</span><br><span> * assume the IP where we listen for incoming MGCP messages</span><br><span> * as bind IP */</span><br><span style="color: hsl(0, 100%, 40%);">- bind_addr = endp->cfg->source_addr;</span><br><span style="color: hsl(120, 100%, 40%);">+ bind_addr = endp->trunk->cfg->source_addr;</span><br><span> LOGPCONN(conn->conn, DRTP, LOGL_DEBUG,</span><br><span> "using mgcp bind ip as local rtp bind ip: %s\n", bind_addr);</span><br><span> }</span><br><span>@@ -1177,9 +1177,7 @@</span><br><span> </span><br><span> do {</span><br><span> /* Run transcoder */</span><br><span style="color: hsl(0, 100%, 40%);">- cont = endp->cfg->rtp_processing_cb(endp, rtp_end,</span><br><span style="color: hsl(0, 100%, 40%);">- (char *)msgb_data(msg), &buflen,</span><br><span style="color: hsl(0, 100%, 40%);">- RTP_BUF_SIZE);</span><br><span style="color: hsl(120, 100%, 40%);">+ cont = endp->trunk->cfg->rtp_processing_cb(endp, rtp_end, (char *)msgb_data(msg), &buflen, RTP_BUF_SIZE);</span><br><span> if (cont < 0)</span><br><span> break;</span><br><span> </span><br><span>@@ -1657,7 +1655,7 @@</span><br><span> osmo_fd_setup(&end->rtp, -1, OSMO_FD_READ, rtp_data_net, conn, 0);</span><br><span> osmo_fd_setup(&end->rtcp, -1, OSMO_FD_READ, rtp_data_net, conn, 0);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- return bind_rtp(endp->cfg, conn->end.local_addr, end, endp);</span><br><span style="color: hsl(120, 100%, 40%);">+ return bind_rtp(endp->trunk->cfg, conn->end.local_addr, end, endp);</span><br><span> }</span><br><span> </span><br><span> /*! free allocated RTP and RTCP ports.</span><br><span>diff --git a/src/libosmo-mgcp/mgcp_osmux.c b/src/libosmo-mgcp/mgcp_osmux.c</span><br><span>index 8f0a906..de19042 100644</span><br><span>--- a/src/libosmo-mgcp/mgcp_osmux.c</span><br><span>+++ b/src/libosmo-mgcp/mgcp_osmux.c</span><br><span>@@ -116,7 +116,7 @@</span><br><span> osmux_handle_alloc(struct mgcp_conn_rtp *conn, struct in_addr *addr, int rem_port)</span><br><span> {</span><br><span> struct osmux_handle *h;</span><br><span style="color: hsl(0, 100%, 40%);">- struct mgcp_config *cfg = conn->conn->endp->cfg;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct mgcp_config *cfg = conn->conn->endp->trunk->cfg;</span><br><span> </span><br><span> h = talloc_zero(osmux, struct osmux_handle);</span><br><span> if (!h)</span><br><span>@@ -460,7 +460,7 @@</span><br><span> */</span><br><span> struct in6_addr addr_unset = {};</span><br><span> static const uint32_t rtp_ssrc_winlen = UINT32_MAX / (OSMUX_CID_MAX + 1);</span><br><span style="color: hsl(0, 100%, 40%);">- uint16_t osmux_dummy = endp->cfg->osmux_dummy;</span><br><span style="color: hsl(120, 100%, 40%);">+ uint16_t osmux_dummy = endp->trunk->cfg->osmux_dummy;</span><br><span> </span><br><span> /* Check if osmux is enabled for the specified connection */</span><br><span> if (conn->osmux.state != OSMUX_STATE_ACTIVATING) {</span><br><span>diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c</span><br><span>index 736b071..6341f07 100644</span><br><span>--- a/src/libosmo-mgcp/mgcp_protocol.c</span><br><span>+++ b/src/libosmo-mgcp/mgcp_protocol.c</span><br><span>@@ -144,7 +144,7 @@</span><br><span> static int setup_rtp_processing(struct mgcp_endpoint *endp,</span><br><span> struct mgcp_conn_rtp *conn)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct mgcp_config *cfg = endp->cfg;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct mgcp_config *cfg = endp->trunk->cfg;</span><br><span> struct mgcp_conn_rtp *conn_src = NULL;</span><br><span> struct mgcp_conn_rtp *conn_dst = conn;</span><br><span> struct mgcp_conn *_conn;</span><br><span>@@ -273,7 +273,7 @@</span><br><span> * us for OSMUX connections. Perhaps adding a new internal API to get it</span><br><span> * based on conn type.</span><br><span> */</span><br><span style="color: hsl(0, 100%, 40%);">- const char *addr = strlen(endp->cfg->local_ip) ? endp->cfg->local_ip : conn->end.local_addr;</span><br><span style="color: hsl(120, 100%, 40%);">+ const char *addr = strlen(endp->trunk->cfg->local_ip) ? endp->trunk->cfg->local_ip : conn->end.local_addr;</span><br><span> struct msgb *sdp;</span><br><span> int rc;</span><br><span> struct msgb *result;</span><br><span>@@ -477,7 +477,7 @@</span><br><span> </span><br><span> OSMO_ASSERT(conn);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- range = &endp->cfg->net_ports;</span><br><span style="color: hsl(120, 100%, 40%);">+ range = &endp->trunk->cfg->net_ports;</span><br><span> </span><br><span> pthread_mutex_lock(&range->lock);</span><br><span> /* attempt to find a port */</span><br><span>@@ -741,8 +741,8 @@</span><br><span> */</span><br><span> static int mgcp_osmux_setup(struct mgcp_endpoint *endp, const char *line)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- if (!endp->cfg->osmux_init) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (osmux_init(OSMUX_ROLE_BSC, endp->cfg) < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!endp->trunk->cfg->osmux_init) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (osmux_init(OSMUX_ROLE_BSC, endp->trunk->cfg) < 0) {</span><br><span> LOGPENDP(endp, DLMGCP, LOGL_ERROR, "Cannot init OSMUX\n");</span><br><span> return -3;</span><br><span> }</span><br><span>@@ -887,7 +887,7 @@</span><br><span> case 'X':</span><br><span> if (strncasecmp("Osmux: ", line + 2, strlen("Osmux: ")) == 0) {</span><br><span> /* If osmux is disabled, just skip setting it up */</span><br><span style="color: hsl(0, 100%, 40%);">- if (!rq->endp->cfg->osmux)</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!rq->endp->trunk->cfg->osmux)</span><br><span> break;</span><br><span> osmux_cid = mgcp_osmux_setup(endp, line);</span><br><span> break;</span><br><span>@@ -1001,7 +1001,7 @@</span><br><span> rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_NO_OSMUX));</span><br><span> goto error2;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- } else if (endp->cfg->osmux == OSMUX_USAGE_ONLY) {</span><br><span style="color: hsl(120, 100%, 40%);">+ } else if (endp->trunk->cfg->osmux == OSMUX_USAGE_ONLY) {</span><br><span> LOGPCONN(_conn, DLMGCP, LOGL_ERROR,</span><br><span> "CRCX: osmux only and no osmux offered\n");</span><br><span> rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_NO_OSMUX));</span><br><span>@@ -1165,7 +1165,7 @@</span><br><span> case 'X':</span><br><span> if (strncasecmp("Osmux: ", line + 2, strlen("Osmux: ")) == 0) {</span><br><span> /* If osmux is disabled, just skip setting it up */</span><br><span style="color: hsl(0, 100%, 40%);">- if (!endp->cfg->osmux)</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!endp->trunk->cfg->osmux)</span><br><span> break;</span><br><span> osmux_cid = mgcp_osmux_setup(endp, line);</span><br><span> break;</span><br><span>@@ -1680,7 +1680,7 @@</span><br><span> if (len < 0)</span><br><span> return -1;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- rc = send_agent(endp->cfg, buf, len);</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = send_agent(endp->trunk->cfg, buf, len);</span><br><span> if (rc <= 0)</span><br><span> return -1;</span><br><span> </span><br><span>diff --git a/src/libosmo-mgcp/mgcp_sdp.c b/src/libosmo-mgcp/mgcp_sdp.c</span><br><span>index 077ac96..a36c6d2 100644</span><br><span>--- a/src/libosmo-mgcp/mgcp_sdp.c</span><br><span>+++ b/src/libosmo-mgcp/mgcp_sdp.c</span><br><span>@@ -576,7 +576,7 @@</span><br><span> OSMO_ASSERT(addr);</span><br><span> </span><br><span> /* FIXME: constify endp and conn args in get_net_donwlink_format_cb() */</span><br><span style="color: hsl(0, 100%, 40%);">- endp->cfg->get_net_downlink_format_cb((struct mgcp_endpoint *)endp,</span><br><span style="color: hsl(120, 100%, 40%);">+ endp->trunk->cfg->get_net_downlink_format_cb((struct mgcp_endpoint *)endp,</span><br><span> &codec, &fmtp_extra,</span><br><span> (struct mgcp_conn_rtp *)conn);</span><br><span> </span><br><span>@@ -601,7 +601,7 @@</span><br><span> </span><br><span> payload_types[0] = payload_type;</span><br><span> if (mgcp_conn_rtp_is_osmux(conn))</span><br><span style="color: hsl(0, 100%, 40%);">- local_port = endp->cfg->osmux_port;</span><br><span style="color: hsl(120, 100%, 40%);">+ local_port = endp->trunk->cfg->osmux_port;</span><br><span> else</span><br><span> local_port = conn->end.local_port;</span><br><span> rc = add_audio(sdp, payload_types, 1, local_port);</span><br><span>diff --git a/src/libosmo-mgcp/mgcp_stat.c b/src/libosmo-mgcp/mgcp_stat.c</span><br><span>index 357b593..59ef917 100644</span><br><span>--- a/src/libosmo-mgcp/mgcp_stat.c</span><br><span>+++ b/src/libosmo-mgcp/mgcp_stat.c</span><br><span>@@ -28,6 +28,7 @@</span><br><span> #include <osmocom/mgcp/mgcp_conn.h></span><br><span> #include <osmocom/mgcp/mgcp_stat.h></span><br><span> #include <osmocom/mgcp/mgcp_endp.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmocom/mgcp/mgcp_trunk.h></span><br><span> </span><br><span> /* Helper function for mgcp_format_stats_rtp() to calculate packet loss */</span><br><span> #if defined(__has_attribute)</span><br><span>@@ -98,7 +99,7 @@</span><br><span> str += nchars;</span><br><span> str_len -= nchars;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (conn->conn->endp->cfg->osmux != OSMUX_USAGE_OFF) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (conn->conn->endp->trunk->cfg->osmux != OSMUX_USAGE_OFF) {</span><br><span> /* Error Counter */</span><br><span> nchars = snprintf(str, str_len,</span><br><span> "\r\nX-Osmo-CP: EC TI=%" PRIu64 ", TO=%" PRIu64,</span><br><span>diff --git a/src/libosmo-mgcp/mgcp_vty.c b/src/libosmo-mgcp/mgcp_vty.c</span><br><span>index 738bfcc..a05733f 100644</span><br><span>--- a/src/libosmo-mgcp/mgcp_vty.c</span><br><span>+++ b/src/libosmo-mgcp/mgcp_vty.c</span><br><span>@@ -218,7 +218,7 @@</span><br><span> vty_out(vty, " CONN: %s%s", mgcp_conn_dump(conn), VTY_NEWLINE);</span><br><span> </span><br><span> if (show_stats) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (endp->cfg->conn_timeout) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (endp->trunk->cfg->conn_timeout) {</span><br><span> struct timeval remaining;</span><br><span> osmo_timer_remaining(&conn->watchdog, NULL, &remaining);</span><br><span> vty_out(vty, " Currently remaining timeout (seconds): %d.%06d%s",</span><br><span>diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c</span><br><span>index 1c0d3cc..a8aad14 100644</span><br><span>--- a/tests/mgcp/mgcp_test.c</span><br><span>+++ b/tests/mgcp/mgcp_test.c</span><br><span>@@ -960,7 +960,7 @@</span><br><span> static int rqnt_cb(struct mgcp_endpoint *endp, char _tone)</span><br><span> {</span><br><span> ptrdiff_t tone = _tone;</span><br><span style="color: hsl(0, 100%, 40%);">- endp->cfg->data = (void *)tone;</span><br><span style="color: hsl(120, 100%, 40%);">+ endp->trunk->cfg->data = (void *)tone;</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span>@@ -1050,7 +1050,6 @@</span><br><span> memset(&endp, 0, sizeof(endp));</span><br><span> cfg = mgcp_config_alloc();</span><br><span> trunk = mgcp_trunk_alloc(cfg, MGCP_TRUNK_VIRTUAL, MGCP_VIRT_TRUNK_ID);</span><br><span style="color: hsl(0, 100%, 40%);">- endp.cfg = cfg;</span><br><span> endp.type = &ep_typeset.rtp;</span><br><span> trunk->v.vty_number_endpoints = 1;</span><br><span> trunk->endpoints = endpoints;</span><br><span>@@ -1307,7 +1306,6 @@</span><br><span> state.in_stream.err_ts_ctr = &test_ctr_in;</span><br><span> state.out_stream.err_ts_ctr = &test_ctr_out;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- endp.cfg = cfg;</span><br><span> endp.type = &ep_typeset.rtp;</span><br><span> </span><br><span> trunk->v.vty_number_endpoints = 1;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-mgw/+/25428">change 25428</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-mgw/+/25428"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-mgw </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I2456ba63598f76200d53e00223abf60bb36a49c0 </div>
<div style="display:none"> Gerrit-Change-Number: 25428 </div>
<div style="display:none"> Gerrit-PatchSet: 15 </div>
<div style="display:none"> Gerrit-Owner: Hoernchen <ewild@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Hoernchen <ewild@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>