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

Change in osmo-mgw[master]: osmux: Rotate over available Osmux CID when allocating a new one

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/30157 ) Change subject: osmux: Rotate over available Osmux CID when allocating a new one ...................................................................... osmux: Rotate over available Osmux CID when allocating a new one Before this patch, the free CID with the smallest number was always selected to be used. This caused more or less the same subset of CIDs to be used all the time, while the CIDs with bigger numbers were mostly unused. Let's distribute the use so that all CIDs are used roughly the same. This has the advantage, among others, that the same CID will not be re-used immediatelly after being freed if a new call is established. It is useful to leave the CIDs unused for some time since the other end peer may know of the call being tear down with some delay. Hence if a new call is established immediately after the CID was released, the same CID would be allocated and passed at the peer, which would then detect that the old call (in its view still active) would already make use of that remote CID. Change-Id: I9dfbcc5e4b4c61ce217020e533d68fbcfa6b9f56 Related: SYS#6161 --- M src/libosmo-mgcp/mgcp_osmux.c M tests/mgcp/mgcp_test.c 2 files changed, 30 insertions(+), 9 deletions(-) Approvals: Jenkins Builder: Verified daniel: Looks good to me, but someone else must approve pespin: Looks good to me, approved diff --git a/src/libosmo-mgcp/mgcp_osmux.c b/src/libosmo-mgcp/mgcp_osmux.c index 5df5446..fcff841 100644 --- a/src/libosmo-mgcp/mgcp_osmux.c +++ b/src/libosmo-mgcp/mgcp_osmux.c @@ -734,26 +734,46 @@ return used; } -/*! take a free OSMUX cid. +/*! Find and reserve a free OSMUX cid. Keep state of last allocated CID to + * rotate allocated CIDs over time. This helps in letting CIDs unused for some + * time after last use. * \returns OSMUX cid */ int osmux_cid_pool_get_next(void) { - int i, j; + static uint8_t next_free_osmux_cid_lookup = 0; + uint8_t start_i, start_j; + uint8_t i, j, cid; - for (i = 0; i < sizeof(osmux_cid_bitmap); i++) { - for (j = 0; j < 8; j++) { + /* i = octet index, j = bit index inside ith octet */ + start_i = next_free_osmux_cid_lookup >> 3; + start_j = next_free_osmux_cid_lookup & 0x07; + + for (i = start_i; i < sizeof(osmux_cid_bitmap); i++) { + for (j = start_j; j < 8; j++) { if (osmux_cid_bitmap[i] & (1 << j)) continue; + goto found; + } + } - osmux_cid_bitmap[i] |= (1 << j); - LOGP(DOSMUX, LOGL_DEBUG, - "Allocating Osmux CID %u from pool\n", (i * 8) + j); - return (i * 8) + j; + for (i = 0; i <= start_i; i++) { + for (j = 0; j < start_j; j++) { + if (osmux_cid_bitmap[i] & (1 << j)) + continue; + goto found; } } LOGP(DOSMUX, LOGL_ERROR, "All Osmux circuits are in use!\n"); return -1; + +found: + osmux_cid_bitmap[i] |= (1 << j); + cid = (i << 3) | j; + next_free_osmux_cid_lookup = (cid + 1) & 0xff; + LOGP(DOSMUX, LOGL_DEBUG, + "Allocating Osmux CID %u from pool\n", cid); + return cid; } /*! take a specific OSMUX cid. diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c index 2d8dfec..4aa655d 100644 --- a/tests/mgcp/mgcp_test.c +++ b/tests/mgcp/mgcp_test.c @@ -1632,7 +1632,8 @@ for (i = 0; i < 256; ++i) { id = osmux_cid_pool_get_next(); - OSMO_ASSERT(id == i); + /* We called osmux_cid_pool_get_next() above, so next CID is i+1. */ + OSMO_ASSERT(id == ((i + 1) & 0xff)); OSMO_ASSERT(osmux_cid_pool_count_used() == i + 1); } -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/30157 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I9dfbcc5e4b4c61ce217020e533d68fbcfa6b9f56 Gerrit-Change-Number: 30157 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 7 months

1
0
0 0

Change in osmo-mgw[master]: osmux: Rotate over available Osmux CID when allocating a new one

by pespin

pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/30157 ) Change subject: osmux: Rotate over available Osmux CID when allocating a new one ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Patchset: PS2: Readding osmith's +1 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/30157 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I9dfbcc5e4b4c61ce217020e533d68fbcfa6b9f56 Gerrit-Change-Number: 30157 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 15 Nov 2022 17:23:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 7 months

1
0
0 0

Change in libosmo-netif[master]: osmux: dup in RTP pkt: Replace potentially internally forged pkt with...

by Jenkins Builder

Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/30168 ) Change subject: osmux: dup in RTP pkt: Replace potentially internally forged pkt with incoming one ...................................................................... Patch Set 1: (2 comments) File src/osmux_input.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-1032): https://gerrit.osmocom.org/c/libosmo-netif/+/30168/comment/5b8d4c1f_b36a0abc PS1, Line 473: } else { else is not generally useful after a break or return File tests/osmux/osmux_input_test.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-1032): https://gerrit.osmocom.org/c/libosmo-netif/+/30168/comment/ca8412b5_59cd1f12 PS1, Line 430: osmux_pl = (uint8_t*)osmuxh + sizeof(*osmuxh); "(foo*)" should be "(foo *)" -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/30168 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I82e11ef3dcd20ffea33c94ed83abcedf0f186871 Gerrit-Change-Number: 30168 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-CC: Jenkins Builder Gerrit-Comment-Date: Tue, 15 Nov 2022 17:21:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

2 years, 7 months

1
0
0 0

Change in libosmo-netif[master]: osmux: dup in RTP pkt: Replace potentially internally forged pkt with...

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/30168 ) Change subject: osmux: dup in RTP pkt: Replace potentially internally forged pkt with incoming one ...................................................................... osmux: dup in RTP pkt: Replace potentially internally forged pkt with incoming one When RTP packet provided by user to osmux layer, it may contain a seq gap, and osmux will refill the packets to avoid losing that information on the other side when it receives the batch. If out of order UDP packets are received, it can happen that we first detect a gap and later on we receive the previous RTP packet, which we is then detected as duplicate because it was previously forged to fill the hole. In that case, let's better keep the incoming packet instead of the potentially internall forged one which doesn't contain the real AMR payload. Related: SYS#6161 Change-Id: I82e11ef3dcd20ffea33c94ed83abcedf0f186871 --- M src/osmux_input.c M tests/osmux/osmux_input_test.c M tests/osmux/osmux_input_test.ok 3 files changed, 107 insertions(+), 6 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/68/30168/1 diff --git a/src/osmux_input.c b/src/osmux_input.c index 7387ab0..21a137a 100644 --- a/src/osmux_input.c +++ b/src/osmux_input.c @@ -443,7 +443,7 @@ static int osmux_link_add(struct osmux_link *link, struct osmux_in_req *req) { - struct msgb *cur; + struct msgb *cur, *next; int rc; /* We've seen the first RTP message, disable dummy padding */ @@ -455,15 +455,28 @@ /* Extra validation: check if this message already exists, should not * happen but make sure we don't propagate duplicated messages. */ - llist_for_each_entry(cur, &req->circuit->msg_list, list) { + llist_for_each_entry_safe(cur, next, &req->circuit->msg_list, list) { struct rtp_hdr *rtph2 = osmo_rtp_get_hdr(cur); OSMO_ASSERT(rtph2); - /* Already exists message with this sequence, skip */ + /* Already exists message with this sequence. Let's copy over + * the new RTP, since there's the chance that the existing one may + * be a forged copy we did when we detected a hole. */ if (rtph2->sequence == req->rtph->sequence) { - LOGP(DLMUX, LOGL_ERROR, "RTP pkt with seq=%u already exists, skip it\n", - ntohs(req->rtph->sequence)); - return -1; + if (msgb_length(cur) == msgb_length(req->msg)) { + LOGP(DLMUX, LOGL_INFO, "RTP pkt with seq=%u already exists, replace it\n", + ntohs(req->rtph->sequence)); + __llist_add(&req->msg->list, &cur->list, cur->list.next); + llist_del(&cur->list); + msgb_free(cur); + return 0; + } else { + /* Different packet size, AMR FT may have changed. Let's avoid changing it to + * break accounted size to be written (would need new osmux_hdr, etc.) */ + LOGP(DLMUX, LOGL_NOTICE, "RTP pkt with seq=%u and different len %u != %u already exists, skip it\n", + ntohs(req->rtph->sequence), msgb_length(cur), msgb_length(req->msg)); + return -1; + } } } diff --git a/tests/osmux/osmux_input_test.c b/tests/osmux/osmux_input_test.c index 8645929..e37857c 100644 --- a/tests/osmux/osmux_input_test.c +++ b/tests/osmux/osmux_input_test.c @@ -413,6 +413,84 @@ talloc_free(h_input); } +static void test_rtp_dup_osmux_deliver_cb(struct msgb *batch_msg, void *data) +{ + struct osmux_hdr *osmuxh; + char buf[2048]; + uint8_t *osmux_pl; + bool *osmux_transmitted = (bool *)data; + + osmux_snprintf(buf, sizeof(buf), batch_msg); + clock_debug("OSMUX message (len=%d): %s\n", batch_msg->len, buf); + + /* We expect 1 batch: */ + osmuxh = osmux_xfrm_output_pull(batch_msg); + /* Check seqnum is the one configured beforehand: */ + OSMO_ASSERT(osmuxh->seq == 123); + osmux_pl = (uint8_t*)osmuxh + sizeof(*osmuxh); + OSMO_ASSERT(osmux_pl[0] == 0x12); + + osmuxh = osmux_xfrm_output_pull(batch_msg); + OSMO_ASSERT(osmuxh == NULL); + + msgb_free(batch_msg); + + *osmux_transmitted = true; +} +/* Test user pushes duplicated RTP (dup seqnum) to osmux: */ +static void test_rtp_dup(void) +{ + struct msgb *msg, *msg_dup; + struct amr_hdr *amrh; + int rc; + const uint8_t cid = 33; + bool osmux_transmitted = false; + struct osmux_in_handle *h_input; + + printf("===%s===\n", __func__); + + + + clock_override_enable(true); + clock_override_set(0, 0); + rtp_init(0, 0); + + h_input = osmux_xfrm_input_alloc(tall_ctx); + osmux_xfrm_input_set_initial_seqnum(h_input, 123); + osmux_xfrm_input_set_batch_factor(h_input, 2); + osmux_xfrm_input_set_deliver_cb(h_input, + test_rtp_dup_osmux_deliver_cb, + &osmux_transmitted); + osmux_xfrm_input_open_circuit(h_input, cid, false); + + /* First RTP frame at t=0 */ + msg = rtp_next(); + msg_dup = msgb_copy(msg, "dup"); + rtp_append_amr(msg, AMR_FT_2); + rc = osmux_xfrm_input(h_input, msg, cid); + OSMO_ASSERT(rc == 0); + + clock_debug("Submit 2nd RTP packet, seqnum dup"); + clock_override_add(0, TIME_RTP_PKT_MS*1000); + amrh = rtp_append_amr(msg_dup, AMR_FT_2); + amrh->data[0] = 0x12; /* Change AMR payload to check it is updated. */ + rc = osmux_xfrm_input(h_input, msg_dup, cid); + OSMO_ASSERT(rc == 0); + OSMO_ASSERT(osmux_transmitted == false); + + /* t=60, osmux batch is scheduled to be transmitted: */ + clock_debug("Submit 3rd RTP packet, triggers osmux batch"); + clock_override_add(0, TIME_RTP_PKT_MS*1000); + msg = rtp_next(); + rtp_append_amr(msg, AMR_FT_2); + osmo_select_main(0); + OSMO_ASSERT(osmux_transmitted == true); + + clock_debug("Closing circuit"); + osmux_xfrm_input_close_circuit(h_input, cid); + talloc_free(h_input); +} + int main(int argc, char **argv) { @@ -433,6 +511,7 @@ test_amr_ft_change_middle_batch(); test_last_amr_cmr_f_q_used(); test_initial_osmux_seqnum(); + test_rtp_dup(); fprintf(stdout, "OK: Test passed\n"); return EXIT_SUCCESS; diff --git a/tests/osmux/osmux_input_test.ok b/tests/osmux/osmux_input_test.ok index 60daf26..b5b2f00 100644 --- a/tests/osmux/osmux_input_test.ok +++ b/tests/osmux/osmux_input_test.ok @@ -28,4 +28,13 @@ sys={0.020000}, mono={0.020000}: OSMUX message (len=19): OSMUX seq=123 ccid=033 ft=1 rtp_m=0 ctr=0 amr_f=1 amr_q=1 amr_ft=02 amr_cmr=00 [ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ] sys={0.020000}, mono={0.020000}: Closing circuit +===test_rtp_dup=== +sys={0.000000}, mono={0.000000}: clock_override_set +sys={0.000000}, mono={0.000000}: Submit 2nd RTP packet, seqnum dup +sys={0.020000}, mono={0.020000}: clock_override_add +sys={0.020000}, mono={0.020000}: Submit 3rd RTP packet, triggers osmux batch +sys={0.040000}, mono={0.040000}: clock_override_add +sys={0.040000}, mono={0.040000}: OSMUX message (len=19): OSMUX seq=123 ccid=033 ft=1 rtp_m=0 ctr=0 amr_f=0 amr_q=1 amr_ft=02 amr_cmr=00 [ 12 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ] + +sys={0.040000}, mono={0.040000}: Closing circuit OK: Test passed -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/30168 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I82e11ef3dcd20ffea33c94ed83abcedf0f186871 Gerrit-Change-Number: 30168 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 7 months

1
0
0 0

Change in libosmo-netif[master]: osmux: dup in RTP pkt: check before applying queue flush due to Marke...

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/30167 ) Change subject: osmux: dup in RTP pkt: check before applying queue flush due to Marker bit ...................................................................... osmux: dup in RTP pkt: check before applying queue flush due to Marker bit We need to filter duplicate messages before checking the Mark bit and returning 1 to tell the user to deliver the current batchset content. Otherwise, a repeated RTP packet with an M bit would trigger delivery, which is wrong. Change-Id: I4a01b0935f112d650d8fc161b3389eda6c8e75ec --- M src/osmux_input.c 1 file changed, 6 insertions(+), 6 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/67/30167/1 diff --git a/src/osmux_input.c b/src/osmux_input.c index c2a96af..7387ab0 100644 --- a/src/osmux_input.c +++ b/src/osmux_input.c @@ -452,12 +452,6 @@ link->ndummy--; } - /* Init of talkspurt (RTP M marker bit) needs to be in the first AMR slot - * of the OSMUX packet, enforce sending previous batch if required: - */ - if (req->rtph->marker && req->circuit->nmsgs != 0) - return 1; - /* Extra validation: check if this message already exists, should not * happen but make sure we don't propagate duplicated messages. */ @@ -473,6 +467,12 @@ } } + /* Init of talkspurt (RTP M marker bit) needs to be in the first AMR slot + * of the OSMUX packet, enforce sending previous batch if required: + */ + if (req->rtph->marker && req->circuit->nmsgs != 0) + return 1; + /* First check if there is room for this message in the batch */ /* First in batch comes after the batch header: */ if (req->circuit->nmsgs == 0) -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/30167 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I4a01b0935f112d650d8fc161b3389eda6c8e75ec Gerrit-Change-Number: 30167 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 7 months

1
0
0 0

Change in libosmo-netif[master]: osmux: Use internal struct to cache parsing state of rtp pkt from user

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/30166 ) Change subject: osmux: Use internal struct to cache parsing state of rtp pkt from user ...................................................................... osmux: Use internal struct to cache parsing state of rtp pkt from user This allows incrementally gathering all the information once and only once. While doing so, this patch tends to move the decoding/parsing of the incoming RTP packet further towards the start of the code, hence detecing errors in the input data early in the process and avoiding touching internal state in this case. Change-Id: I55e0d2772e7054c0d734f5918c6938a5c8a6e781 --- M src/osmux_input.c 1 file changed, 64 insertions(+), 56 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/66/30166/1 diff --git a/src/osmux_input.c b/src/osmux_input.c index fb246a5..c2a96af 100644 --- a/src/osmux_input.c +++ b/src/osmux_input.c @@ -87,6 +87,17 @@ uint8_t seq; }; +/* Used internally to temporarily cache all parsed content of an RTP pkt from user to be transmitted as Osmux */ +struct osmux_in_req { + struct osmux_circuit *circuit; + struct msgb *msg; + struct rtp_hdr *rtph; + uint32_t rtp_payload_len; + struct amr_hdr *amrh; + int amr_payload_len; + unsigned int needed_bytes; +}; + /* returns: 1 if batch is full, 0 if batch still not full, negative on error. */ static int osmux_circuit_enqueue(struct osmux_link *link, struct osmux_circuit *circuit, struct msgb *msg) { @@ -344,8 +355,7 @@ } /* returns: 1 if batch is full, 0 if batch still not full, negative on error. */ -static int osmux_replay_lost_packets(struct osmux_link *link, struct osmux_circuit *circuit, - struct rtp_hdr *cur_rtph) +static int osmux_replay_lost_packets(struct osmux_link *link, struct osmux_in_req *req) { int16_t diff; struct msgb *last; @@ -354,16 +364,16 @@ int i, rc; /* Have we seen any RTP packet in this batch before? */ - if (llist_empty(&circuit->msg_list)) + if (llist_empty(&req->circuit->msg_list)) return 0; /* Get last RTP packet seen in this batch */ - last = llist_entry(circuit->msg_list.prev, struct msgb, list); + last = llist_entry(req->circuit->msg_list.prev, struct msgb, list); last_rtph = osmo_rtp_get_hdr(last); if (last_rtph == NULL) return -1; last_seq = ntohs(last_rtph->sequence); - cur_seq = ntohs(cur_rtph->sequence); + cur_seq = ntohs(req->rtph->sequence); diff = cur_seq - last_seq; /* If diff between last RTP packet seen and this one is > 1, @@ -399,7 +409,7 @@ DELTA_RTP_TIMESTAMP); /* No more room in this batch, skip padding with more clones */ - rc = osmux_circuit_enqueue(link, circuit, clone); + rc = osmux_circuit_enqueue(link, req->circuit, clone); if (rc != 0) { msgb_free(clone); return rc; @@ -431,78 +441,59 @@ /* returns: 1 if batch is full, 0 if batch still not full, negative on error. */ static int -osmux_link_add(struct osmux_link *link, struct msgb *msg, - struct rtp_hdr *rtph, int ccid) +osmux_link_add(struct osmux_link *link, struct osmux_in_req *req) { - int bytes = 0, amr_payload_len; - struct osmux_circuit *circuit; struct msgb *cur; int rc; - struct amr_hdr *amrh; - uint32_t amr_len; - - circuit = osmux_link_find_circuit(link, ccid); - if (!circuit) - return -1; /* We've seen the first RTP message, disable dummy padding */ - if (circuit->dummy) { - circuit->dummy = 0; + if (req->circuit->dummy) { + req->circuit->dummy = 0; link->ndummy--; } - amrh = osmo_rtp_get_payload(rtph, msg, &amr_len); - if (amrh == NULL) - return -1; - amr_payload_len = osmux_rtp_amr_payload_len(amrh, amr_len); - if (amr_payload_len < 0) { - LOGP(DLMUX, LOGL_NOTICE, "AMR payload invalid\n"); - return -1; - } - /* Init of talkspurt (RTP M marker bit) needs to be in the first AMR slot * of the OSMUX packet, enforce sending previous batch if required: */ - if (rtph->marker && circuit->nmsgs != 0) + if (req->rtph->marker && req->circuit->nmsgs != 0) return 1; /* Extra validation: check if this message already exists, should not * happen but make sure we don't propagate duplicated messages. */ - llist_for_each_entry(cur, &circuit->msg_list, list) { + llist_for_each_entry(cur, &req->circuit->msg_list, list) { struct rtp_hdr *rtph2 = osmo_rtp_get_hdr(cur); - if (rtph2 == NULL) - return -1; + OSMO_ASSERT(rtph2); /* Already exists message with this sequence, skip */ - if (rtph2->sequence == rtph->sequence) { + if (rtph2->sequence == req->rtph->sequence) { LOGP(DLMUX, LOGL_ERROR, "RTP pkt with seq=%u already exists, skip it\n", - ntohs(rtph->sequence)); + ntohs(req->rtph->sequence)); return -1; } } /* First check if there is room for this message in the batch */ /* First in batch comes after the batch header: */ - if (circuit->nmsgs == 0) - bytes += sizeof(struct osmux_hdr); + if (req->circuit->nmsgs == 0) + req->needed_bytes += sizeof(struct osmux_hdr); /* If AMR FT changes in the middle of current batch a new header is * required to adapt to size change: */ - else if (osmux_circuit_get_last_stored_amr_ft(circuit) != amrh->ft) - bytes += sizeof(struct osmux_hdr); - bytes += amr_payload_len; + else if (osmux_circuit_get_last_stored_amr_ft(req->circuit) != req->amrh->ft) + req->needed_bytes += sizeof(struct osmux_hdr); + req->needed_bytes += req->amr_payload_len; /* No room, sorry. You'll have to retry */ - if (bytes > link->remaining_bytes) + if (req->needed_bytes > link->remaining_bytes) return 1; /* Handle RTP packet loss scenario */ - rc = osmux_replay_lost_packets(link, circuit, rtph); + rc = osmux_replay_lost_packets(link, req); if (rc != 0) return rc; /* This batch is full, force batch delivery */ - rc = osmux_circuit_enqueue(link, circuit, msg); + rc = osmux_circuit_enqueue(link, req->circuit, req->msg); if (rc != 0) return rc; @@ -512,7 +503,7 @@ #endif /* Update remaining room in this batch */ - link->remaining_bytes -= bytes; + link->remaining_bytes -= req->needed_bytes; if (link->nmsgs == 0) { #ifdef DEBUG_MSG @@ -542,8 +533,22 @@ int osmux_xfrm_input(struct osmux_in_handle *h, struct msgb *msg, int ccid) { int ret; - struct rtp_hdr *rtph; struct osmux_link *link = (struct osmux_link *)h->internal_data; + struct osmux_in_req req = { + .msg = msg, + .rtph = osmo_rtp_get_hdr(msg), + .circuit = osmux_link_find_circuit(link, ccid), + }; + + if (!req.circuit) { + LOGP(DLMUX, LOGL_INFO, "Couldn't find circuit CID=%u\n", ccid); + goto err_free; + } + + if (!req.rtph) { + LOGP(DLMUX, LOGL_NOTICE, "msg not containing an RTP header\n"); + goto err_free; + } /* Ignore too big RTP/RTCP messages, most likely forged. Sanity check * to avoid a possible forever loop in the caller. @@ -551,18 +556,10 @@ if (msg->len > h->batch_size - sizeof(struct osmux_hdr)) { LOGP(DLMUX, LOGL_NOTICE, "RTP payload too big (%u) for configured batch size (%u)\n", msg->len, h->batch_size); - msgb_free(msg); - return -1; + goto err_free; } - rtph = osmo_rtp_get_hdr(msg); - if (rtph == NULL) { - LOGP(DLMUX, LOGL_NOTICE, "msg not containing an RTP header\n"); - msgb_free(msg); - return -1; - } - - switch (rtph->payload_type) { + switch (req.rtph->payload_type) { case RTP_PT_RTCP: LOGP(DLMUX, LOGL_INFO, "Dropping RTCP packet\n"); msgb_free(msg); @@ -572,17 +569,24 @@ * although we use static ones. Assume that we always * receive AMR traffic. */ + req.amrh = osmo_rtp_get_payload(req.rtph, req.msg, &req.rtp_payload_len); + if (req.amrh == NULL) + goto err_free; + req.amr_payload_len = osmux_rtp_amr_payload_len(req.amrh, req.rtp_payload_len); + if (req.amr_payload_len < 0) { + LOGP(DLMUX, LOGL_NOTICE, "AMR payload invalid\n"); + goto err_free; + } /* Add this RTP to the OSMUX batch */ - ret = osmux_link_add(link, msg, rtph, ccid); + ret = osmux_link_add(link, &req); if (ret < 0) { /* Cannot put this message into the batch. * Malformed, duplicated, OOM. Drop it and tell * the upper layer that we have digest it. */ LOGP(DLMUX, LOGL_DEBUG, "Dropping RTP packet instead of adding to batch\n"); - msgb_free(msg); - return ret; + goto err_free; } h->stats.input_rtp_msgs++; @@ -590,6 +594,10 @@ break; } return ret; + +err_free: + msgb_free(msg); + return -1; } static int osmux_xfrm_input_talloc_destructor(struct osmux_in_handle *h) -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/30166 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I55e0d2772e7054c0d734f5918c6938a5c8a6e781 Gerrit-Change-Number: 30166 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 7 months

1
0
0 0

Change in osmo-bts[master]: osmux: Rotate over available Osmux CID when allocating a new one

by daniel

Attention is currently required from: pespin. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/30158 ) Change subject: osmux: Rotate over available Osmux CID when allocating a new one ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/30158 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I72803fb172accbabfc81923572890f8ecb06cefd Gerrit-Change-Number: 30158 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 15 Nov 2022 17:13:19 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 7 months

1
0
0 0

Change in osmo-mgw[master]: osmux: Rotate over available Osmux CID when allocating a new one

by daniel

Attention is currently required from: pespin. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/30157 ) Change subject: osmux: Rotate over available Osmux CID when allocating a new one ...................................................................... Patch Set 2: Code-Review+1 (1 comment) File src/libosmo-mgcp/mgcp_osmux.c: https://gerrit.osmocom.org/c/osmo-mgw/+/30157/comment/8410b68f_1ed33017 PS1, Line 771: (i * 8) > <<Yes>> Done -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/30157 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I9dfbcc5e4b4c61ce217020e533d68fbcfa6b9f56 Gerrit-Change-Number: 30157 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 15 Nov 2022 17:12:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 7 months

1
0
0 0

Change in osmo-mgw[master]: osmux: Rotate over available Osmux CID when allocating a new one

by pespin

Attention is currently required from: pespin. Hello osmith, Jenkins Builder, daniel, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-mgw/+/30157 to look at the new patch set (#2). Change subject: osmux: Rotate over available Osmux CID when allocating a new one ...................................................................... osmux: Rotate over available Osmux CID when allocating a new one Before this patch, the free CID with the smallest number was always selected to be used. This caused more or less the same subset of CIDs to be used all the time, while the CIDs with bigger numbers were mostly unused. Let's distribute the use so that all CIDs are used roughly the same. This has the advantage, among others, that the same CID will not be re-used immediatelly after being freed if a new call is established. It is useful to leave the CIDs unused for some time since the other end peer may know of the call being tear down with some delay. Hence if a new call is established immediately after the CID was released, the same CID would be allocated and passed at the peer, which would then detect that the old call (in its view still active) would already make use of that remote CID. Change-Id: I9dfbcc5e4b4c61ce217020e533d68fbcfa6b9f56 Related: SYS#6161 --- M src/libosmo-mgcp/mgcp_osmux.c M tests/mgcp/mgcp_test.c 2 files changed, 30 insertions(+), 9 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/57/30157/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/30157 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I9dfbcc5e4b4c61ce217020e533d68fbcfa6b9f56 Gerrit-Change-Number: 30157 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newpatchset

2 years, 7 months

1
0
0 0

Change in osmo-mgw[master]: osmux: Rotate over available Osmux CID when allocating a new one

by daniel

Attention is currently required from: pespin. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/30157 ) Change subject: osmux: Rotate over available Osmux CID when allocating a new one ...................................................................... Patch Set 1: (1 comment) File src/libosmo-mgcp/mgcp_osmux.c: https://gerrit.osmocom.org/c/osmo-mgw/+/30157/comment/5304450b_034f4f03 PS1, Line 771: (i * 8) > you mean << not >> here :) <<Yes>> -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/30157 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I9dfbcc5e4b4c61ce217020e533d68fbcfa6b9f56 Gerrit-Change-Number: 30157 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 15 Nov 2022 17:10:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 7 months

1
0
0 0

2025

2024

2023

2022

gerrit-log November 2022