Attention is currently required from: pespin.
daniel has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39240?usp=email )
Change subject: mgw: DLCX: Split mgcp header pars parsing into a previous step
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39240?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I557a3a257ddefedc479a4aff974a74c4e4e2c85d
Gerrit-Change-Number: 39240
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 14 Jan 2025 16:13:36 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: laforge, pespin.
lynxis lazus has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-msc/+/38490?usp=email )
Change subject: vlr: add PS support
......................................................................
Patch Set 5:
(5 comments)
File src/libvlr/vlr_lu_fsm.c:
https://gerrit.osmocom.org/c/osmo-msc/+/38490/comment/e49a35ab_34d6e3f1?usp… :
PS5, Line 357: int N; /*! counter of timeouts */
> can we have something more descriptive like num_timeouts?
I would like to have it in osmo_fsm.
The N is a common concept in GMM/MM timeouts.
Same as T for the timer name
https://gerrit.osmocom.org/c/osmo-msc/+/38490/comment/742df379_4ba9ba2e?usp… :
PS5, Line 569: static void vlr_lu_compl_fsm_reset_n(struct osmo_fsm_inst *fi, uint32_t prev_state)
> I'd really prefer have proper function names, one for each state with its name plus "oenenter" appen […]
I'll copy the same code in here then.
Maybe we move this reset N feature into the fsm, because I need it a couple of times.
https://gerrit.osmocom.org/c/osmo-msc/+/38490/comment/626869fb_627b2679?usp… :
PS5, Line 1437: static void lu_fsm_reset_n(struct osmo_fsm_inst *fi, uint32_t prev_state)
> same, state name + "oneneter"
Acknowledged
https://gerrit.osmocom.org/c/osmo-msc/+/38490/comment/26ab7fc1_8e6f1dd1?usp… :
PS5, Line 1589: lfp->vlr->ops.tx_lu_rej(lfp->msc_conn_ref, gsm48_cause, lfp->lu_type);
> need to guard for null ptr func?
No, because the initialize checks for null.
OSMO_ASSERT(ops->tx_lu_rej); in vlr_alloc()
https://gerrit.osmocom.org/c/osmo-msc/+/38490/comment/74913d92_630bc33e?usp… :
PS5, Line 1607: vlr->ops.tx_id_req(lfp->msc_conn_ref, GSM_MI_TYPE_IMSI);
> need to guard for null ptr func?
No, because the initialize checks for null.
OSMO_ASSERT(ops->tx_lu_rej); in vlr_alloc()
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/38490?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Ie9ffeb140c9d354b3a0f4822e2619f623235add0
Gerrit-Change-Number: 38490
Gerrit-PatchSet: 5
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 14 Jan 2025 16:07:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Attention is currently required from: pespin.
daniel has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39239?usp=email )
Change subject: mgw: MDCX: Split mgcp header pars parsing into a previous step
......................................................................
Patch Set 4:
(1 comment)
File src/libosmo-mgcp/mgcp_protocol.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/39239/comment/51d6fdc4_eb73933a?usp… :
PS4, Line 1122: remote_osmux_cid = mgcp_osmux_setup(endp, line);
`handle_create_con()` contains the following code:
```
if (hpars->remote_osmux_cid != MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET)
mgcp_trunk_osmux_init_if_needed(trunk);
```
Should you add that to `handle_modify_con()` as well after removing `mgcp_osmux_setup()` here?
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39239?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I6ecb41fc2cc737c3a161b6bc98bd08ae01909655
Gerrit-Change-Number: 39239
Gerrit-PatchSet: 4
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: daniel <dwillmann(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 14 Jan 2025 16:06:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: daniel.
pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmo-netif/+/39316?usp=email )
Change subject: stream: Introduce osmo_stream_{cli,srv}_set_segmentation_cb2
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
I'll need this most probably when moving osmo-pcap-server to use osmo_stream_srv, in order to segment taking tls into account...
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/39316?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: I6e8dd6ece13397074075f05a1a0a8dbdd80d8848
Gerrit-Change-Number: 39316
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 14 Jan 2025 16:00:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: lynxis lazus.
daniel has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-msc/+/39232?usp=email )
Change subject: vlr: only set HLR number if field is present
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/39232?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I26dc4c00b81723cd7ead095a515269afd0f73f8e
Gerrit-Change-Number: 39232
Gerrit-PatchSet: 2
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Comment-Date: Tue, 14 Jan 2025 15:58:21 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39223?usp=email )
(
2 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: mgw: Rename and cleanup code allocating rtp/rtcp ports in trunk
......................................................................
mgw: Rename and cleanup code allocating rtp/rtcp ports in trunk
Clean up pointers passed. Rename function since it's clearly operating
on trunk related fields through mutexes.
Change-Id: Ib894afcb61609c247883d5ccdd7b8fbf29b2cbf8
---
M include/osmocom/mgcp/mgcp_trunk.h
M src/libosmo-mgcp/mgcp_protocol.c
M src/libosmo-mgcp/mgcp_trunk.c
3 files changed, 45 insertions(+), 43 deletions(-)
Approvals:
daniel: Looks good to me, but someone else must approve
laforge: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/include/osmocom/mgcp/mgcp_trunk.h b/include/osmocom/mgcp/mgcp_trunk.h
index 68d37dd..d36c004 100644
--- a/include/osmocom/mgcp/mgcp_trunk.h
+++ b/include/osmocom/mgcp/mgcp_trunk.h
@@ -2,9 +2,10 @@
#include <osmocom/gsm/i460_mux.h>
#include <osmocom/abis/e1_input.h>
+#include <osmocom/mgcp/mgcp_conn.h>
+#include <osmocom/mgcp/mgcp_network.h>
#include <osmocom/mgcp/mgcp_ratectr.h>
-
#define LOGPTRUNK(trunk, cat, level, fmt, args...) \
LOGP(cat, level, "trunk:%u " fmt, \
trunk ? trunk->trunk_nr : 0, \
@@ -78,6 +79,7 @@
struct mgcp_trunk *mgcp_trunk_by_name(const struct mgcp_config *cfg, const char *epname);
int e1_trunk_nr_from_epname(unsigned int *trunk_nr, const char *epname);
struct mgcp_trunk *mgcp_trunk_by_line_num(const struct mgcp_config *cfg, unsigned int num);
+int mgcp_trunk_allocate_conn_rtp_ports(struct mgcp_trunk *trunk, struct mgcp_conn_rtp *conn_rtp);
/* The virtual trunk is always created on trunk id 0 for historical reasons,
* use this define constant as ID when allocating a virtual trunk. Other
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index 3efa0e0..32fbd43 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -489,46 +489,6 @@
return create_ok_response(rq->trunk, rq->endp, 200, "AUEP", rq->pdata->trans);
}
-/* Try to find a free port by attempting to bind on it. Also handle the
- * counter that points on the next free port. Since we have a pointer
- * to the next free port, binding should in work on the first attempt in
- * general. In case of failure the next port is tried until the whole port
- * range is tried once. */
-static int allocate_port(struct mgcp_endpoint *endp, struct mgcp_conn_rtp *conn)
-{
- int i;
- struct mgcp_port_range *range;
- unsigned int tries;
-
- OSMO_ASSERT(conn);
-
- range = &endp->trunk->cfg->net_ports;
-
- pthread_mutex_lock(&range->lock);
- /* attempt to find a port */
- tries = (range->range_end - range->range_start) / 2;
- for (i = 0; i < tries; ++i) {
- int rc;
-
- if (range->last_port >= range->range_end)
- range->last_port = range->range_start;
-
- rc = mgcp_conn_rtp_bind_rtp_ports(conn, range->last_port);
-
- range->last_port += 2;
- if (rc == 0) {
- pthread_mutex_unlock(&range->lock);
- return 0;
- }
-
- }
- pthread_mutex_unlock(&range->lock);
- LOGPENDP(endp, DLMGCP, LOGL_ERROR,
- "Allocating a RTP/RTCP port failed %u times.\n",
- tries);
- return -1;
-}
-
/*! Helper function for check_local_cx_options() to get a pointer of the next
* lco option identifier
* \param[in] lco string
@@ -1077,7 +1037,7 @@
rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_BIND_PORT));
goto error2;
}
- if (allocate_port(endp, conn_rtp) != 0) {
+ if (mgcp_trunk_allocate_conn_rtp_ports(trunk, conn_rtp) != 0) {
rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_BIND_PORT));
goto error2;
}
@@ -1311,7 +1271,7 @@
if (strcmp(new_local_addr, conn_rtp->end.local_addr)) {
osmo_strlcpy(conn_rtp->end.local_addr, new_local_addr, sizeof(conn_rtp->end.local_addr));
mgcp_rtp_end_free_port(&conn_rtp->end);
- if (allocate_port(endp, conn_rtp) != 0) {
+ if (mgcp_trunk_allocate_conn_rtp_ports(trunk, conn_rtp) != 0) {
rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_BIND_PORT));
goto error3;
}
diff --git a/src/libosmo-mgcp/mgcp_trunk.c b/src/libosmo-mgcp/mgcp_trunk.c
index 69750f8..8359098 100644
--- a/src/libosmo-mgcp/mgcp_trunk.c
+++ b/src/libosmo-mgcp/mgcp_trunk.c
@@ -306,3 +306,43 @@
return NULL;
}
+
+/* Try to find a free port by attempting to bind on it. Also handle the
+ * counter that points on the next free port. Since we have a pointer
+ * to the next free port, binding should in work on the first attempt in
+ * general. In case of failure the next port is tried until the whole port
+ * range is tried once. */
+int mgcp_trunk_allocate_conn_rtp_ports(struct mgcp_trunk *trunk, struct mgcp_conn_rtp *conn_rtp)
+{
+ int i;
+ struct mgcp_port_range *range;
+ unsigned int tries;
+
+ OSMO_ASSERT(trunk);
+ OSMO_ASSERT(conn_rtp);
+
+ range = &trunk->cfg->net_ports;
+
+ pthread_mutex_lock(&range->lock);
+ /* attempt to find a port */
+ tries = (range->range_end - range->range_start) / 2;
+ for (i = 0; i < tries; ++i) {
+ int rc;
+
+ if (range->last_port >= range->range_end)
+ range->last_port = range->range_start;
+
+ rc = mgcp_conn_rtp_bind_rtp_ports(conn_rtp, range->last_port);
+
+ range->last_port += 2;
+ if (rc == 0) {
+ pthread_mutex_unlock(&range->lock);
+ return 0;
+ }
+
+ }
+ pthread_mutex_unlock(&range->lock);
+ LOGPCONN(conn_rtp->conn, DLMGCP, LOGL_ERROR,
+ "Allocating a RTP/RTCP port failed %u times.\n", tries);
+ return -1;
+}
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39223?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Ib894afcb61609c247883d5ccdd7b8fbf29b2cbf8
Gerrit-Change-Number: 39223
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
pespin has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/39233?usp=email )
Change subject: msc: add testenv.cfg
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS1:
> So far I tested, it looks good.
@osmith@sysmocom.de @lynxis@fe80.eu we need to remember to move ttcn3-msc-tests in jenkins to use testenv, plus drop the directory from docker-playground.git
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/39233?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: Ia93115e3a27ac43b6530f2669e210f59169d75b9
Gerrit-Change-Number: 39233
Gerrit-PatchSet: 2
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 14 Jan 2025 15:57:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: lynxis lazus <lynxis(a)fe80.eu>
Attention is currently required from: laforge, pespin.
daniel has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39222?usp=email )
Change subject: mgw: Clean up code allocating conn_rtp rtp/rtcp sockets
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39222?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I50a251b66e85ceeeccb30dcd1813863d7c754961
Gerrit-Change-Number: 39222
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 14 Jan 2025 15:57:38 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes