August 2022 - gerrit-log - lists.osmocom.org

Change in osmo-bts[master]: Avoid counting lchan->dl_tch_queue length every time a msg is enqueued

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/29034 ) Change subject: Avoid counting lchan->dl_tch_queue length every time a msg is enqueued ...................................................................... Avoid counting lchan->dl_tch_queue length every time a msg is enqueued The queue_limit_to method iterates the entire list of messages every time a new message is added. Let's use msgb_{enqueue,dequeue}_count() APIs to do that in constant time. It is true that since the queue is limited to 1, there's usually at most 1 item in the queue so it's not a real problem. However, when we add Osmux in the future, we may need to tweak the amount of messages which can be in the list, due to Osmux batching mechansim which may be more bursty sometimes. In any case, this change doesn't make things worse for sure. The patch also takes the chance to group the queue_limit_to + enqueue into one function to avoid having the code spread several times. Change-Id: I61818a3bb521c27bd21a8b6fa70581d27638ec9b --- M include/osmo-bts/lchan.h M src/common/l1sap.c M src/common/lchan.c 3 files changed, 16 insertions(+), 19 deletions(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, approved diff --git a/include/osmo-bts/lchan.h b/include/osmo-bts/lchan.h index c86acb0..64b7efa 100644 --- a/include/osmo-bts/lchan.h +++ b/include/osmo-bts/lchan.h @@ -191,6 +191,7 @@ uint8_t sapis_ul[23]; struct lapdm_channel lapdm_ch; struct llist_head dl_tch_queue; + unsigned int dl_tch_queue_len; struct { /* bitmask of all SI that are present/valid in si_buf */ uint32_t valid; diff --git a/src/common/l1sap.c b/src/common/l1sap.c index b74fd5a..640ff57 100644 --- a/src/common/l1sap.c +++ b/src/common/l1sap.c @@ -153,17 +153,16 @@ } /*! limit number of queue entries to %u; drops any surplus messages */ -static void queue_limit_to(const char *prefix, struct llist_head *queue, unsigned int limit) +static void lchan_dl_tch_queue_enqueue(struct gsm_lchan *lchan, struct msgb *msg, unsigned int limit) { - unsigned int count = llist_count(queue); - - if (count > limit) - LOGP(DL1P, LOGL_NOTICE, "%s: freeing %d queued frames\n", prefix, count-limit); - while (count > limit) { - struct msgb *tmp = msgb_dequeue(queue); + if (lchan->dl_tch_queue_len > limit) + LOGPLCHAN(lchan, DL1P, LOGL_NOTICE, "freeing %d queued frames\n", + lchan->dl_tch_queue_len - limit); + while (lchan->dl_tch_queue_len > limit) { + struct msgb *tmp = msgb_dequeue_count(&lchan->dl_tch_queue, &lchan->dl_tch_queue_len); msgb_free(tmp); - count--; } + msgb_enqueue_count(&lchan->dl_tch_queue, msg, &lchan->dl_tch_queue_len); } /* allocate a msgb containing a osmo_phsap_prim + optional l2 data @@ -915,7 +914,7 @@ uint8_t *p; /* de-queue response message (loopback) */ - loop_msg = msgb_dequeue(&lchan->dl_tch_queue); + loop_msg = msgb_dequeue_count(&lchan->dl_tch_queue, &lchan->dl_tch_queue_len); if (!loop_msg) { LOGPGT(DL1P, LOGL_NOTICE, tm, "%s: no looped PDTCH message, sending empty\n", gsm_lchan_name(lchan)); @@ -1305,7 +1304,7 @@ lchan->abis_ip.rtp_socket->rx_user_ts += GSM_RTP_DURATION; } /* get a msgb from the dl_tx_queue */ - resp_msg = msgb_dequeue(&lchan->dl_tch_queue); + resp_msg = msgb_dequeue_count(&lchan->dl_tch_queue, &lchan->dl_tch_queue_len); if (!resp_msg) { DEBUGPGT(DL1P, &g_time, "%s DL TCH Tx queue underrun\n", gsm_lchan_name(lchan)); resp_l1sap = &empty_l1sap; @@ -1500,8 +1499,7 @@ /* we are in loopback mode (for BER testing) * mode and need to enqeue the frame to be * returned in downlink */ - queue_limit_to(gsm_lchan_name(lchan), &lchan->dl_tch_queue, 1); - msgb_enqueue(&lchan->dl_tch_queue, msg); + lchan_dl_tch_queue_enqueue(lchan, msg, 1); /* Return 1 to signal that we're still using msg * and it should not be freed */ @@ -1622,10 +1620,8 @@ msg->data, msg->len, fn_ms_adj(fn, lchan), lchan->rtp_tx_marker); /* if loopback is enabled, also queue received RTP data */ if (lchan->loopback) { - /* make sure the queue doesn't get too long */ - queue_limit_to(gsm_lchan_name(lchan), &lchan->dl_tch_queue, 1); - /* add new frame to queue */ - msgb_enqueue(&lchan->dl_tch_queue, msg); + /* add new frame to queue, make sure the queue doesn't get too long */ + lchan_dl_tch_queue_enqueue(lchan, msg, 1); /* Return 1 to signal that we're still using msg and it should not be freed */ return 1; } @@ -1914,9 +1910,7 @@ rtpmsg_ts(msg) = timestamp; /* make sure the queue doesn't get too long */ - queue_limit_to(gsm_lchan_name(lchan), &lchan->dl_tch_queue, 1); - - msgb_enqueue(&lchan->dl_tch_queue, msg); + lchan_dl_tch_queue_enqueue(lchan, msg, 1); } static int l1sap_chan_act_dact_modify(struct gsm_bts_trx *trx, uint8_t chan_nr, diff --git a/src/common/lchan.c b/src/common/lchan.c index 6f344e2..83d20da 100644 --- a/src/common/lchan.c +++ b/src/common/lchan.c @@ -132,6 +132,7 @@ INIT_LLIST_HEAD(&lchan->sapi_cmds); INIT_LLIST_HEAD(&lchan->dl_tch_queue); + lchan->dl_tch_queue_len = 0; } void gsm_lchan_name_update(struct gsm_lchan *lchan) @@ -638,4 +639,5 @@ osmo_rtp_socket_free(lchan->abis_ip.rtp_socket); lchan->abis_ip.rtp_socket = NULL; msgb_queue_flush(&lchan->dl_tch_queue); + lchan->dl_tch_queue_len = 0; } -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/29034 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I61818a3bb521c27bd21a8b6fa70581d27638ec9b Gerrit-Change-Number: 29034 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 10 months

1
0
0 0

Change in osmo-bts[master]: Use libosmocore API msgb_queue_free() to free lists

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/29035 ) Change subject: Use libosmocore API msgb_queue_free() to free lists ...................................................................... Use libosmocore API msgb_queue_free() to free lists There's no need to maintain a duplicate msgb_queue_flush(), which returns the amount of freed messages (feature not used at all by the callers). Change-Id: I9841e18ca0b7b852130bbb02a510e43a3b3fd93f --- M include/osmo-bts/rsl.h M src/common/lchan.c M src/common/rsl.c M src/common/scheduler.c 4 files changed, 2 insertions(+), 17 deletions(-) Approvals: Jenkins Builder: Verified fixeria: Looks good to me, but someone else must approve laforge: Looks good to me, approved diff --git a/include/osmo-bts/rsl.h b/include/osmo-bts/rsl.h index dcd476c..a530734 100644 --- a/include/osmo-bts/rsl.h +++ b/include/osmo-bts/rsl.h @@ -4,8 +4,6 @@ #define LCHAN_FN_DUMMY 0xFFFFFFFF #define LCHAN_FN_WAIT 0xFFFFFFFE -int msgb_queue_flush(struct llist_head *list); - int down_rsl(struct gsm_bts_trx *trx, struct msgb *msg); int rsl_tx_rf_res(struct gsm_bts_trx *trx); int rsl_tx_chan_rqd(struct gsm_bts_trx *trx, struct gsm_time *gtime, diff --git a/src/common/lchan.c b/src/common/lchan.c index 83d20da..c7d8d45 100644 --- a/src/common/lchan.c +++ b/src/common/lchan.c @@ -638,6 +638,6 @@ { osmo_rtp_socket_free(lchan->abis_ip.rtp_socket); lchan->abis_ip.rtp_socket = NULL; - msgb_queue_flush(&lchan->dl_tch_queue); + msgb_queue_free(&lchan->dl_tch_queue); lchan->dl_tch_queue_len = 0; } diff --git a/src/common/rsl.c b/src/common/rsl.c index bcbe27c..8cdb662 100644 --- a/src/common/rsl.c +++ b/src/common/rsl.c @@ -88,19 +88,6 @@ } #define OSMO_IN_ARRAY(search, arr) osmo_in_array(search, arr, ARRAY_SIZE(arr)) -int msgb_queue_flush(struct llist_head *list) -{ - struct msgb *msg, *msg2; - int count = 0; - - llist_for_each_entry_safe(msg, msg2, list, list) { - msgb_free(msg); - count++; - } - - return count; -} - /* FIXME: move this to libosmocore */ void gsm48_gen_starting_time(uint8_t *out, struct gsm_time *gtime) { diff --git a/src/common/scheduler.c b/src/common/scheduler.c index 6564e5a..14aff34 100644 --- a/src/common/scheduler.c +++ b/src/common/scheduler.c @@ -659,7 +659,7 @@ struct l1sched_ts *l1ts = ts->priv; unsigned int i; - msgb_queue_flush(&l1ts->dl_prims); + msgb_queue_free(&l1ts->dl_prims); rate_ctr_group_free(l1ts->ctrs); l1ts->ctrs = NULL; 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/29035 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I9841e18ca0b7b852130bbb02a510e43a3b3fd93f Gerrit-Change-Number: 29035 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 10 months

1
0
0 0

Change in osmo-bts[master]: Avoid counting lchan->dl_tch_queue length every time a msg is enqueued

by pespin

pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/29034 ) Change subject: Avoid counting lchan->dl_tch_queue length every time a msg is enqueued ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/29034 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I61818a3bb521c27bd21a8b6fa70581d27638ec9b Gerrit-Change-Number: 29034 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 11 Aug 2022 21:21:31 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 10 months

1
0
0 0

Change in osmo-bts[master]: Avoid counting lchan->dl_tch_queue length every time a msg is enqueued

by pespin

Attention is currently required from: pespin. Hello Jenkins Builder, laforge, fixeria, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bts/+/29034 to look at the new patch set (#3). Change subject: Avoid counting lchan->dl_tch_queue length every time a msg is enqueued ...................................................................... Avoid counting lchan->dl_tch_queue length every time a msg is enqueued The queue_limit_to method iterates the entire list of messages every time a new message is added. Let's use msgb_{enqueue,dequeue}_count() APIs to do that in constant time. It is true that since the queue is limited to 1, there's usually at most 1 item in the queue so it's not a real problem. However, when we add Osmux in the future, we may need to tweak the amount of messages which can be in the list, due to Osmux batching mechansim which may be more bursty sometimes. In any case, this change doesn't make things worse for sure. The patch also takes the chance to group the queue_limit_to + enqueue into one function to avoid having the code spread several times. Change-Id: I61818a3bb521c27bd21a8b6fa70581d27638ec9b --- M include/osmo-bts/lchan.h M src/common/l1sap.c M src/common/lchan.c 3 files changed, 16 insertions(+), 19 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/34/29034/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/29034 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I61818a3bb521c27bd21a8b6fa70581d27638ec9b Gerrit-Change-Number: 29034 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newpatchset

2 years, 10 months

1
0
0 0

Change in osmo-ttcn3-hacks[master]: deps: use titan.TestPorts.UDPasp from ECLIPSEGITLAB_REPOS

by fixeria

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/29046 ) Change subject: deps: use titan.TestPorts.UDPasp from ECLIPSEGITLAB_REPOS ...................................................................... deps: use titan.TestPorts.UDPasp from ECLIPSEGITLAB_REPOS This change basically reverts [1]. Oliver's patch allowing to set SO_REUSEADDR, which is needed for D-GSM mslookup mDNS testing, has been merged upstream. No need to depend on our own fork anymore. Change-Id: Idf96a64f3d5f7928ed0fb81f4a91e469df3a9adc Related: [1] Ie7e1c3dbd67dba9079a5768e442faffc936fd7fa Related: SYS#4618 --- M deps/Makefile 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/46/29046/1 diff --git a/deps/Makefile b/deps/Makefile index 2a906eb..2367f97 100644 --- a/deps/Makefile +++ b/deps/Makefile @@ -57,11 +57,11 @@ titan.TestPorts.SIPmsg \ titan.TestPorts.TELNETasp \ titan.TestPorts.TCPasp \ + titan.TestPorts.UDPasp \ titan.TestPorts.UNIX_DOMAIN_SOCKETasp OSMOGITHUB_REPOS= titan.TestPorts.SCTPasp \ titan.TestPorts.MTP3asp \ - titan.TestPorts.UDPasp \ titan.ProtocolEmulations.SCCP \ titan.ProtocolModules.BSSGP_v13.0.0 @@ -124,7 +124,7 @@ titan.TestPorts.SIPmsg_commit= R.15.A-1-ge4f9dd0 titan.TestPorts.TCPasp_commit= R.9.A-5-g2c91bc6 titan.TestPorts.TELNETasp_commit= 873fe539642542cd9a901c208f1ec11c6d2f5387 -titan.TestPorts.UDPasp_commit= c20d77a34f288dd70dd4aaa30e520778876e9336 +titan.TestPorts.UDPasp_commit= 54176e95850654e5e8b0ffa2f1b5f35c412b949c titan.TestPorts.UNIX_DOMAIN_SOCKETasp_commit= R.2.A-13-gd34ab71 titan.TestPorts.USB_commit= a9470fdc99c82fbf988f4508c3161118c9106e28 osmo-uecups_commit= 8362efef7c6fa341eb947a75786878e0685767b7 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/29046 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: Idf96a64f3d5f7928ed0fb81f4a91e469df3a9adc Gerrit-Change-Number: 29046 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 10 months

1
0
0 0

Change in osmo-bts[master]: Move lchan_dl_tch_queue_enqueue to lchan.c and make it public

by fixeria

Attention is currently required from: pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/29045 ) Change subject: Move lchan_dl_tch_queue_enqueue to lchan.c and make it public ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/29045 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ie7fa57bb04db9ad9b03971467e12ee7b8e4c190a Gerrit-Change-Number: 29045 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: Jenkins Builder Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 11 Aug 2022 19:18:32 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 10 months

1
0
0 0

Change in osmo-bts[master]: Use libosmocore API msgb_queue_free() to free lists

by fixeria

Attention is currently required from: pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/29035 ) Change subject: Use libosmocore API msgb_queue_free() to free lists ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/29035 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I9841e18ca0b7b852130bbb02a510e43a3b3fd93f Gerrit-Change-Number: 29035 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 11 Aug 2022 19:16:10 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 10 months

1
0
0 0

Change in osmo-bts[master]: Avoid counting lchan->dl_tch_queue length every time a msg is enqueued

by fixeria

Attention is currently required from: pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/29034 ) Change subject: Avoid counting lchan->dl_tch_queue length every time a msg is enqueued ...................................................................... Patch Set 2: Code-Review+1 (1 comment) File src/common/l1sap.c: https://gerrit.osmocom.org/c/osmo-bts/+/29034/comment/bcc8b064_d4951524 PS2, Line 1502: remove this space -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/29034 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I61818a3bb521c27bd21a8b6fa70581d27638ec9b Gerrit-Change-Number: 29034 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 11 Aug 2022 19:15:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 10 months

1
0
0 0

Change in osmo-upf[master]: GTP mockup: list active GTP endecaps actions

by laforge

Attention is currently required from: neels, pespin, fixeria. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-upf/+/28986 ) Change subject: GTP mockup: list active GTP endecaps actions ...................................................................... Patch Set 3: (1 comment) Patchset: PS2: > I personally see no reason for using the mockup stuff in the ttcn3 tests, and hence no reason for th […] I tend to agree with pespin's assessment. We shouldn't have development/debug only features [enabled] in normal production builds. Particularlly not if they make it look like some objects existed, while in reality they don't. Looks like a huge source of potential user confusion. Also, for the TTCN-3 tests, I think in general the goal should be to run the unmodified program just like in production, with all of its interfaces etc. If for some reason we cannot do that in our existing normal jenkins slaves or docker containers, we can always use some kind of special slaves (we already do for example for testing the E1 / frame relay bits. So I think if some testing/mocking is happening, it should happen on the tester (application, OS, VM, ...) side, and not inside the implementation-under-test. VTY tests is a different special case, and we're not arguing about that here. -- To view, visit https://gerrit.osmocom.org/c/osmo-upf/+/28986 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-upf Gerrit-Branch: master Gerrit-Change-Id: Ic09a5ccea24086eb04f46e6af669668e5fade752 Gerrit-Change-Number: 28986 Gerrit-PatchSet: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 11 Aug 2022 19:07:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 10 months

1
0
0 0

Change in osmo-bts[master]: Split out lchan rtp socket creation from rsl handling code

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/29032 ) Change subject: Split out lchan rtp socket creation from rsl handling code ...................................................................... Split out lchan rtp socket creation from rsl handling code This makes the code more clear, as well as allows adding Osmux in the future. Change-Id: Iade43fde7f6113f9d68539f7d6cc9843783c2b3f --- M include/osmo-bts/lchan.h M src/common/lchan.c M src/common/rsl.c 3 files changed, 143 insertions(+), 113 deletions(-) Approvals: Jenkins Builder: Verified fixeria: Looks good to me, but someone else must approve dexter: Looks good to me, but someone else must approve laforge: Looks good to me, approved diff --git a/include/osmo-bts/lchan.h b/include/osmo-bts/lchan.h index 90c28d9..c86acb0 100644 --- a/include/osmo-bts/lchan.h +++ b/include/osmo-bts/lchan.h @@ -2,6 +2,7 @@ #include <stdbool.h> #include <stdint.h> +#include <netinet/in.h> #include <osmocom/core/timer.h> #include <osmocom/core/linuxlist.h> @@ -354,6 +355,10 @@ void lchan_set_state(struct gsm_lchan *lchan, enum gsm_lchan_state state); +int lchan_rtp_socket_create(struct gsm_lchan *lchan, const char *bind_ip); +int lchan_rtp_socket_connect(struct gsm_lchan *lchan, const struct in_addr *ia, uint16_t connect_port); +void lchan_rtp_socket_free(struct gsm_lchan *lchan); + static inline bool lchan_is_dcch(const struct gsm_lchan *lchan) { switch (lchan->type) { diff --git a/src/common/lchan.c b/src/common/lchan.c index fe5efd5..6f344e2 100644 --- a/src/common/lchan.c +++ b/src/common/lchan.c @@ -19,6 +19,8 @@ * */ +#include "btsconfig.h" /* for PACKAGE_VERSION */ + #include <osmocom/core/logging.h> #include <osmocom/trau/osmo_ortp.h> @@ -200,9 +202,7 @@ rsl_tx_ipac_dlcx_ind(lchan, RSL_ERR_NORMAL_UNSPEC); osmo_rtp_socket_log_stats(lchan->abis_ip.rtp_socket, DRTP, LOGL_INFO, "Closing RTP socket on Channel Release "); - osmo_rtp_socket_free(lchan->abis_ip.rtp_socket); - lchan->abis_ip.rtp_socket = NULL; - msgb_queue_flush(&lchan->dl_tch_queue); + lchan_rtp_socket_free(lchan); } /* FIXME: right now we allow creating the rtp_socket even if chan is not @@ -510,3 +510,132 @@ return -1; } } + +static int bind_rtp(struct gsm_bts *bts, struct osmo_rtp_socket *rs, const char *ip) +{ + int rc; + unsigned int i; + unsigned int tries; + + tries = (bts->rtp_port_range_end - bts->rtp_port_range_start) / 2; + for (i = 0; i < tries; i++) { + + if (bts->rtp_port_range_next >= bts->rtp_port_range_end) + bts->rtp_port_range_next = bts->rtp_port_range_start; + + rc = osmo_rtp_socket_bind(rs, ip, bts->rtp_port_range_next); + + bts->rtp_port_range_next += 2; + + if (rc != 0) + continue; + + if (bts->rtp_ip_dscp != -1) { + if (osmo_rtp_socket_set_dscp(rs, bts->rtp_ip_dscp)) + LOGP(DRSL, LOGL_ERROR, "failed to set DSCP=%d: %s\n", + bts->rtp_ip_dscp, strerror(errno)); + } + if (bts->rtp_priority != -1) { + if (osmo_rtp_socket_set_priority(rs, bts->rtp_priority)) + LOGP(DRSL, LOGL_ERROR, "failed to set socket priority %d: %s\n", + bts->rtp_priority, strerror(errno)); + } + return 0; + } + + return -1; +} + +int lchan_rtp_socket_create(struct gsm_lchan *lchan, const char *bind_ip) +{ + struct gsm_bts *bts = lchan->ts->trx->bts; + char cname[256+4]; + int rc; + + if (lchan->abis_ip.rtp_socket) { + LOGPLCHAN(lchan, DRSL, LOGL_ERROR, "Rx RSL IPAC CRCX, " + "but we already have socket!\n"); + return -EALREADY; + } + + /* FIXME: select default value depending on speech_mode */ + //if (!payload_type) + lchan->tch.last_fn = LCHAN_FN_DUMMY; + lchan->abis_ip.rtp_socket = osmo_rtp_socket_create(lchan->ts->trx, + OSMO_RTP_F_POLL); + + if (!lchan->abis_ip.rtp_socket) { + LOGPLCHAN(lchan, DRTP, LOGL_ERROR, "IPAC Failed to create RTP/RTCP sockets\n"); + oml_tx_failure_event_rep(&lchan->ts->trx->mo, + NM_SEVER_MINOR, OSMO_EVT_CRIT_RTP_TOUT, + "%s IPAC Failed to create RTP/RTCP sockets", + gsm_lchan_name(lchan)); + return -ENOTCONN; + } + + rc = osmo_rtp_socket_set_param(lchan->abis_ip.rtp_socket, + bts->rtp_jitter_adaptive ? + OSMO_RTP_P_JIT_ADAP : + OSMO_RTP_P_JITBUF, + bts->rtp_jitter_buf_ms); + if (rc < 0) + LOGPLCHAN(lchan, DRTP, LOGL_ERROR, + "IPAC Failed to set RTP socket parameters: %s\n", strerror(-rc)); + else + LOGPLCHAN(lchan, DRTP, LOGL_INFO, "IPAC set RTP socket parameters: %d\n", rc); + + lchan->abis_ip.rtp_socket->priv = lchan; + lchan->abis_ip.rtp_socket->rx_cb = &l1sap_rtp_rx_cb; + + rc = bind_rtp(bts, lchan->abis_ip.rtp_socket, bind_ip); + if (rc < 0) { + LOGPLCHAN(lchan, DRTP, LOGL_ERROR, "IPAC Failed to bind RTP/RTCP sockets\n"); + oml_tx_failure_event_rep(&lchan->ts->trx->mo, + NM_SEVER_MINOR, OSMO_EVT_CRIT_RTP_TOUT, + "%s IPAC Failed to bind RTP/RTCP sockets", + gsm_lchan_name(lchan)); + lchan_rtp_socket_free(lchan); + return -EBADFD; + } + + /* Ensure RTCP SDES contains some useful information */ + snprintf(cname, sizeof(cname), "bts@%s", bind_ip); + osmo_rtp_set_source_desc(lchan->abis_ip.rtp_socket, cname, + gsm_lchan_name(lchan), NULL, NULL, + gsm_trx_unit_id(lchan->ts->trx), + "OsmoBTS-" PACKAGE_VERSION, NULL); + /* FIXME: multiplex connection, BSC proxy */ + return 0; +} + + +int lchan_rtp_socket_connect(struct gsm_lchan *lchan, const struct in_addr *ia, uint16_t connect_port) +{ + int bound_port = 0; + int rc; + + rc = osmo_rtp_socket_connect(lchan->abis_ip.rtp_socket, + inet_ntoa(*ia), connect_port); + if (rc < 0) { + LOGPLCHAN(lchan, DRTP, LOGL_ERROR, "Failed to connect RTP/RTCP sockets\n"); + return -ECONNREFUSED; + } + /* save IP address and port number */ + lchan->abis_ip.connect_ip = ntohl(ia->s_addr); + lchan->abis_ip.connect_port = connect_port; + + rc = osmo_rtp_get_bound_ip_port(lchan->abis_ip.rtp_socket, + &lchan->abis_ip.bound_ip, + &bound_port); + if (rc < 0) + LOGPLCHAN(lchan, DRTP, LOGL_ERROR, "IPAC cannot obtain locally bound IP/port: %d\n", rc); + lchan->abis_ip.bound_port = bound_port; + return 0; +} + +void lchan_rtp_socket_free(struct gsm_lchan *lchan) +{ + osmo_rtp_socket_free(lchan->abis_ip.rtp_socket); + lchan->abis_ip.rtp_socket = NULL; + msgb_queue_flush(&lchan->dl_tch_queue); +} diff --git a/src/common/rsl.c b/src/common/rsl.c index e8765a7..bcbe27c 100644 --- a/src/common/rsl.c +++ b/src/common/rsl.c @@ -21,8 +21,6 @@ * */ -#include "btsconfig.h" /* for PACKAGE_VERSION */ - #include <stdio.h> #include <errno.h> #include <netdb.h> @@ -2706,51 +2704,15 @@ return hostbuf; } -static int bind_rtp(struct gsm_bts *bts, struct osmo_rtp_socket *rs, const char *ip) -{ - int rc; - unsigned int i; - unsigned int tries; - - tries = (bts->rtp_port_range_end - bts->rtp_port_range_start) / 2; - for (i = 0; i < tries; i++) { - - if (bts->rtp_port_range_next >= bts->rtp_port_range_end) - bts->rtp_port_range_next = bts->rtp_port_range_start; - - rc = osmo_rtp_socket_bind(rs, ip, bts->rtp_port_range_next); - - bts->rtp_port_range_next += 2; - - if (rc != 0) - continue; - - if (bts->rtp_ip_dscp != -1) { - if (osmo_rtp_socket_set_dscp(rs, bts->rtp_ip_dscp)) - LOGP(DRSL, LOGL_ERROR, "failed to set DSCP=%i: %s\n", - bts->rtp_ip_dscp, strerror(errno)); - } - if (bts->rtp_priority != -1) { - if (osmo_rtp_socket_set_priority(rs, bts->rtp_priority)) - LOGP(DRSL, LOGL_ERROR, "failed to set socket priority %d: %s\n", - bts->rtp_priority, strerror(errno)); - } - return 0; - } - - return -1; -} - static int rsl_rx_ipac_XXcx(struct msgb *msg) { struct abis_rsl_dchan_hdr *dch = msgb_l2(msg); struct tlv_parsed tp; struct gsm_lchan *lchan = msg->lchan; - struct gsm_bts *bts = lchan->ts->trx->bts; const uint8_t *payload_type, *speech_mode, *payload_type2; uint32_t connect_ip = 0; uint16_t connect_port = 0; - int rc, inc_ip_port = 0, port; + int rc, inc_ip_port = 0; char *name; struct in_addr ia; struct in_addr addr; @@ -2807,41 +2769,7 @@ } if (dch->c.msg_type == RSL_MT_IPAC_CRCX) { - char cname[256+4]; char *ipstr = NULL; - if (lchan->abis_ip.rtp_socket) { - LOGPLCHAN(lchan, DRSL, LOGL_ERROR, "Rx RSL IPAC CRCX, " - "but we already have socket!\n"); - return tx_ipac_XXcx_nack(lchan, RSL_ERR_RES_UNAVAIL, - inc_ip_port, dch->c.msg_type); - } - /* FIXME: select default value depending on speech_mode */ - //if (!payload_type) - lchan->tch.last_fn = LCHAN_FN_DUMMY; - lchan->abis_ip.rtp_socket = osmo_rtp_socket_create(lchan->ts->trx, - OSMO_RTP_F_POLL); - if (!lchan->abis_ip.rtp_socket) { - LOGPLCHAN(lchan, DRTP, LOGL_ERROR, "IPAC Failed to create RTP/RTCP sockets\n"); - oml_tx_failure_event_rep(&lchan->ts->trx->mo, - NM_SEVER_MINOR, OSMO_EVT_CRIT_RTP_TOUT, - "%s IPAC Failed to create RTP/RTCP sockets", - gsm_lchan_name(lchan)); - return tx_ipac_XXcx_nack(lchan, RSL_ERR_RES_UNAVAIL, - inc_ip_port, dch->c.msg_type); - } - rc = osmo_rtp_socket_set_param(lchan->abis_ip.rtp_socket, - bts->rtp_jitter_adaptive ? - OSMO_RTP_P_JIT_ADAP : - OSMO_RTP_P_JITBUF, - bts->rtp_jitter_buf_ms); - if (rc < 0) - LOGPLCHAN(lchan, DRTP, LOGL_ERROR, - "IPAC Failed to set RTP socket parameters: %s\n", strerror(-rc)); - else - LOGPLCHAN(lchan, DRTP, LOGL_INFO, "IPAC set RTP socket parameters: %d\n", rc); - lchan->abis_ip.rtp_socket->priv = lchan; - lchan->abis_ip.rtp_socket->rx_cb = &l1sap_rtp_rx_cb; - if (connect_ip && connect_port) { /* if CRCX specifies a remote IP, we can bind() * here to 0.0.0.0 and wait for the connect() @@ -2856,26 +2784,10 @@ * back to the BSC in the CRCX_ACK */ ipstr = get_rsl_local_ip(lchan->ts->trx); } - rc = bind_rtp(bts, lchan->abis_ip.rtp_socket, ipstr); - if (rc < 0) { - LOGPLCHAN(lchan, DRTP, LOGL_ERROR, "IPAC Failed to bind RTP/RTCP sockets\n"); - oml_tx_failure_event_rep(&lchan->ts->trx->mo, - NM_SEVER_MINOR, OSMO_EVT_CRIT_RTP_TOUT, - "%s IPAC Failed to bind RTP/RTCP sockets", - gsm_lchan_name(lchan)); - osmo_rtp_socket_free(lchan->abis_ip.rtp_socket); - lchan->abis_ip.rtp_socket = NULL; - msgb_queue_flush(&lchan->dl_tch_queue); + rc = lchan_rtp_socket_create(lchan, ipstr); + if (rc < 0) return tx_ipac_XXcx_nack(lchan, RSL_ERR_RES_UNAVAIL, inc_ip_port, dch->c.msg_type); - } - /* Ensure RTCP SDES contains some useful information */ - snprintf(cname, sizeof(cname), "bts@%s", ipstr); - osmo_rtp_set_source_desc(lchan->abis_ip.rtp_socket, cname, - gsm_lchan_name(lchan), NULL, NULL, - gsm_trx_unit_id(lchan->ts->trx), - "OsmoBTS-" PACKAGE_VERSION, NULL); - /* FIXME: multiplex connection, BSC proxy */ } else { /* MDCX */ if (!lchan->abis_ip.rtp_socket) { @@ -2896,26 +2808,12 @@ ia.s_addr = htonl(get_signlink_remote_ip(sign_link)); } else ia.s_addr = connect_ip; - rc = osmo_rtp_socket_connect(lchan->abis_ip.rtp_socket, - inet_ntoa(ia), connect_port); + rc = lchan_rtp_socket_connect(lchan, &ia, connect_port); if (rc < 0) { - LOGPLCHAN(lchan, DRTP, LOGL_ERROR, "Failed to connect RTP/RTCP sockets\n"); - osmo_rtp_socket_free(lchan->abis_ip.rtp_socket); - lchan->abis_ip.rtp_socket = NULL; - msgb_queue_flush(&lchan->dl_tch_queue); + lchan_rtp_socket_free(lchan); return tx_ipac_XXcx_nack(lchan, RSL_ERR_RES_UNAVAIL, inc_ip_port, dch->c.msg_type); } - /* save IP address and port number */ - lchan->abis_ip.connect_ip = ntohl(ia.s_addr); - lchan->abis_ip.connect_port = connect_port; - - rc = osmo_rtp_get_bound_ip_port(lchan->abis_ip.rtp_socket, - &lchan->abis_ip.bound_ip, - &port); - if (rc < 0) - LOGPLCHAN(lchan, DRTP, LOGL_ERROR, "IPAC cannot obtain locally bound IP/port: %d\n", rc); - lchan->abis_ip.bound_port = port; /* Everything has succeeded, we can store new values in lchan */ if (payload_type) { @@ -2957,9 +2855,7 @@ if (lchan->abis_ip.rtp_socket) { osmo_rtp_socket_log_stats(lchan->abis_ip.rtp_socket, DRTP, LOGL_INFO, "Closing RTP socket on DLCX "); - osmo_rtp_socket_free(lchan->abis_ip.rtp_socket); - lchan->abis_ip.rtp_socket = NULL; - msgb_queue_flush(&lchan->dl_tch_queue); + lchan_rtp_socket_free(lchan); } return rc; } 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/29032 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Iade43fde7f6113f9d68539f7d6cc9843783c2b3f Gerrit-Change-Number: 29032 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 10 months

1
0
0 0

2025

2024

2023

2022

gerrit-log August 2022