- gerrit-log - lists.osmocom.org

Change in libosmo-sccp[master]: SIGTRAN: cache Optional Data for SCCP CR/CC/RLSD
by neels 01 Sep '22

01 Sep '22

Attention is currently required from: laforge, pespin, fixeria, msuraev. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-sccp/+/29084 ) Change subject: SIGTRAN: cache Optional Data for SCCP CR/CC/RLSD ...................................................................... Patch Set 16: (9 comments) File src/sccp_scoc.c: https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/cef92837_7aa7730a PS14, Line 1110: xua_opt_data_send_cache(conn, SUA_CO_CORE, xua->hdr.msg_class); > I think I'm missing a point in here. […] maybe i was misreading this code. It looks to me like below scu_gen_encode_and_send(N_CONNECT, CONFIRM) *sends* a Conn Conf; that made me assume this here is the side that received the Conn Req and is responding with a CC. Then again above case "CC_IND" looks like this *receives* a Conn Conf, didn't see that before.. now i'm confused between the prims, which side is which. File src/sccp_scoc.c: https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/b76d4a42_2892dd9f PS15, Line 642: /* optional: importance */ > The RLC does not have optional importance field. like i said before, a patch should ideally do one thing. To fix other bits along with it, just put it in a separate commit. it is an act of courtesy to code reviewers; it is harder to read a complex patch when unrelated fixes go with it. it also makes it possible to work with patches as "atoms", though we hardly ever need that part of it. https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/e9d63318_a58c7883 PS15, Line 635: LOGP(DLSCCP, LOGL_ERROR, "replacing unsent %u bytes of optional data cache with %s optional data\n", > AFAIU that's something coming from outside, from a peer, so not really an error of the program itsel […] I understand, this case is where we want to cache optional data, but there already is other data in the cache. Is this even possible to happen? ... maybe if we send a CR to the peer, but receive no response, and try again with the same CR. Then we already have data in the cache that could not be sent -- so seeing this error means that it is a software bug, did not clean up unsent cached data after a failure?? IMHO a comment and the log message could clarify this, like "ERROR: caching optional data for N-CONNECT, but there already is optional data occupying the cache. Dropping previous data." ERROR category is good. File src/sccp_scoc.c: https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/264d5726_895dcaeb PS16, Line 592: nt exp_type could you explain a situation where there might be a mismatch from the expected type? Isn't it always DT1 to be sent at the right time? https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/24694b57_c30425dc PS16, Line 638: msgb_trim(conn->opt_data_cache, 0); (i'd prefer sanitation: msgb_free() here, and always alloc a new one.) https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/d76a2fe0_c812277d PS16, Line 667: see Figure C.3 / Q.714 (sheet 2 of 6) */ (osmocom asks to put '*' on every line of comment) https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/d0fb57e1_5cc3e68f PS16, Line 673: if (xua_drop_data_check_drop(prim, SCCP_MAX_DATA, "cache overrun")) the optional data is larger than the upper limit of an SCCP DATA section -- this is not related to cache at all, the conn is still open and no cache is ever involved. it is a protocol error caused by the caller. Is it even possible / likely? Also not an overrun. There is plenty of space in a msgb, there is no previous data in the cache... https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/2949a077_4679ee42 PS16, Line 675: /* There's no need to cache the optional data since the connection is still active at this point */ /* Send the Optional Data in a DT1 ahead of the RLSD, because it is too large to be sent in one message. */ https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/c84d1859_9f367b02 PS16, Line 712: ount (again modifying unrelated comments) -- To view, visit https://gerrit.osmocom.org/c/libosmo-sccp/+/29084 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-sccp Gerrit-Branch: master Gerrit-Change-Id: I0033faf9da393418930252233ce74d62cd1cef8a Gerrit-Change-Number: 29084 Gerrit-PatchSet: 16 Gerrit-Owner: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Thu, 01 Sep 2022 11:27:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de> Gerrit-MessageType: comment

1 0

Change in osmocom-bb[master]: trxcon: Fix printf type
by pespin 01 Sep '22

01 Sep '22

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmocom-bb/+/29261 ) Change subject: trxcon: Fix printf type ...................................................................... trxcon: Fix printf type data_len is a size_t, and gcc warns about it. Change-Id: Ib0c1bfefc0371b8cfb3b7a2e74a6980dd3987931 --- M src/host/trxcon/src/trxcon_fsm.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/61/29261/1 diff --git a/src/host/trxcon/src/trxcon_fsm.c b/src/host/trxcon/src/trxcon_fsm.c index 40d5253..aa7193a 100644 --- a/src/host/trxcon/src/trxcon_fsm.c +++ b/src/host/trxcon/src/trxcon_fsm.c @@ -80,7 +80,7 @@ break; } if (req->data_len != GSM_MACBLOCK_LEN) { - LOGPFSML(fi, LOGL_ERROR, "Unexpected data length=%u\n", req->data_len); + LOGPFSML(fi, LOGL_ERROR, "Unexpected data length=%zu\n", req->data_len); break; } -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/29261 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Ib0c1bfefc0371b8cfb3b7a2e74a6980dd3987931 Gerrit-Change-Number: 29261 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

Change in libosmo-netif[master]: osmux: Allow specifying extra headroom & tailroom in generated rtp msgb
by pespin 31 Aug '22

31 Aug '22

pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29260 ) Change subject: osmux: Allow specifying extra headroom & tailroom in generated rtp msgb ...................................................................... Patch Set 1: Code-Review-2 (1 comment) Patchset: PS1: Don't merge this one yet. I may need to add a msgb_alloc_cb instead of specifying headroom & tailroom, since otherwise after msgb_push()ing the l1sap header i get a lot of misaligned ptr runtime errors. -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29260 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I632654221826340423e1e97b0f8ed9a2baf6c6c3 Gerrit-Change-Number: 29260 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 31 Aug 2022 18:59:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in libosmo-netif[master]: osmux: Drop long time deprecated APIs
by pespin 31 Aug '22

31 Aug '22

Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-netif/+/29256 to look at the new patch set (#2). Change subject: osmux: Drop long time deprecated APIs ...................................................................... osmux: Drop long time deprecated APIs Those APIs where deprecated 4 years ago (end of Aug 2018), and they have been used since around that time. Hence it can be considered safe to drop them, since they only make the whole code more complex to understand. API osmux_xfrm_output_init() is left since openbsc.git is still using it. Related: OS#5987 Change-Id: Icbdd364a8161a8113dbf1406716946f684d0a853 --- M include/osmocom/netif/osmux.h M src/osmux.c 2 files changed, 0 insertions(+), 122 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/56/29256/2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29256 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: Icbdd364a8161a8113dbf1406716946f684d0a853 Gerrit-Change-Number: 29256 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-MessageType: newpatchset

1 0

Change in libosmo-netif[master]: osmux: Allow specifying extra headroom & tailroom in generated rtp msgb
by pespin 31 Aug '22

31 Aug '22

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29260 ) Change subject: osmux: Allow specifying extra headroom & tailroom in generated rtp msgb ...................................................................... osmux: Allow specifying extra headroom & tailroom in generated rtp msgb This is useful for users of the API whic need to keep forwarding the msgb to lower layers which may need prepending a new header to the msgb, like osmo-bts with l1sap. Related: OS#5987 Change-Id: I632654221826340423e1e97b0f8ed9a2baf6c6c3 --- M include/osmocom/netif/osmux.h M src/osmux.c 2 files changed, 16 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/60/29260/1 diff --git a/include/osmocom/netif/osmux.h b/include/osmocom/netif/osmux.h index 8742797..7b561fe 100644 --- a/include/osmocom/netif/osmux.h +++ b/include/osmocom/netif/osmux.h @@ -86,6 +86,8 @@ struct llist_head list; void (*tx_cb)(struct msgb *msg, void *data); /* Used defined rtp tx callback */ void *data; /* User defined opaque data structure */ + unsigned int rtp_msgb_headroom; + unsigned int rtp_msgb_tailroom; }; static inline uint8_t *osmux_get_payload(struct osmux_hdr *osmuxh) @@ -112,6 +114,7 @@ void osmux_xfrm_output_init2(struct osmux_out_handle *h, uint32_t rtp_ssrc, uint8_t rtp_payload_type) OSMO_DEPRECATED("Use osmux_xfrm_output_alloc() and osmux_xfrm_output_set_rtp_*() instead"); void osmux_xfrm_output_set_rtp_ssrc(struct osmux_out_handle *h, uint32_t rtp_ssrc); void osmux_xfrm_output_set_rtp_pl_type(struct osmux_out_handle *h, uint32_t rtp_payload_type); +void osmux_xfrm_output_set_rtp_msgb_extra_room(struct osmux_out_handle *h, unsigned int headroom, unsigned int tailroom); void osmux_xfrm_output_set_tx_cb(struct osmux_out_handle *h, void (*tx_cb)(struct msgb *msg, void *data), void *data); int osmux_xfrm_output_sched(struct osmux_out_handle *h, struct osmux_hdr *osmuxh); void osmux_xfrm_output_flush(struct osmux_out_handle *h); diff --git a/src/osmux.c b/src/osmux.c index 49cc368..7f0bca3 100644 --- a/src/osmux.c +++ b/src/osmux.c @@ -128,10 +128,13 @@ struct amr_hdr *amrh; struct timespec delta = { .tv_sec = 0, .tv_nsec = DELTA_RTP_MSG*1000 }; - out_msg = msgb_alloc(sizeof(struct rtp_hdr) + - sizeof(struct amr_hdr) + - osmo_amr_bytes(osmuxh->amr_ft), - "OSMUX test"); + out_msg = msgb_alloc_headroom(h->rtp_msgb_headroom + + sizeof(struct rtp_hdr) + + sizeof(struct amr_hdr) + + osmo_amr_bytes(osmuxh->amr_ft) + + h->rtp_msgb_tailroom, + h->rtp_msgb_headroom, + "osmux-rtp"); if (out_msg == NULL) return NULL; @@ -934,6 +937,12 @@ h->rtp_payload_type = rtp_payload_type; } +void osmux_xfrm_output_set_rtp_msgb_extra_room(struct osmux_out_handle *h, unsigned int headroom, unsigned int tailroom) +{ + h->rtp_msgb_headroom = headroom; + h->rtp_msgb_tailroom = tailroom; +} + #define SNPRINTF_BUFFER_SIZE(ret, remain, offset) \ if (ret < 0) \ ret = 0; \ -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29260 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I632654221826340423e1e97b0f8ed9a2baf6c6c3 Gerrit-Change-Number: 29260 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

Change in osmo-mgw[master]: osmux: Use new osmux APIs to let libosmo-netif alloc struct osmux_out...
by pespin 31 Aug '22

31 Aug '22

Attention is currently required from: pespin. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-mgw/+/29259 to look at the new patch set (#2). Change subject: osmux: Use new osmux APIs to let libosmo-netif alloc struct osmux_out_handle ...................................................................... osmux: Use new osmux APIs to let libosmo-netif alloc struct osmux_out_handle This way we don't need to worry about implementation details (struct size) in the future in case they change (they will). Related: OS#5987 Requires: libosmo-netif.git Change-Id Ie8df581f375c9a183a7af60b431561bda82f6e34 Change-Id: I2d0d8c152b8f1234ddfcd74d6cb04b1818b41510 --- M TODO-RELEASE M include/osmocom/mgcp/mgcp_conn.h M src/libosmo-mgcp/mgcp_osmux.c 3 files changed, 13 insertions(+), 13 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/59/29259/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/29259 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I2d0d8c152b8f1234ddfcd74d6cb04b1818b41510 Gerrit-Change-Number: 29259 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

Change in libosmo-netif[master]: osmux: Allocate struct osmux_out_handle through API
by pespin 31 Aug '22

31 Aug '22

Attention is currently required from: pespin. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-netif/+/29258 to look at the new patch set (#3). Change subject: osmux: Allocate struct osmux_out_handle through API ...................................................................... osmux: Allocate struct osmux_out_handle through API Until now, the osmux_out_handle was allocated by the client, and passed to the API to initialize it. This makes it really hard to improve the implementation without breaking the ABI. Let's break the ABI now one last time (hopefully) by allocating the struct through an API. With only this change, the already built users (osmo-mgw, openbsc) can still work fine, since there's no change on the struct osmux_out_handle. However, they will somehow break next time the struct is changed until they are ported to the same API (easy to do). Related: OS#5987 Change-Id: Ie8df581f375c9a183a7af60b431561bda82f6e34 --- M TODO-RELEASE M examples/osmux-test-output.c M include/osmocom/netif/osmux.h M src/osmux.c M tests/jibuf/jibuf_tool.c M tests/osmux/osmux_test.c M tests/osmux/osmux_test2.c 7 files changed, 145 insertions(+), 72 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/58/29258/3 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29258 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: Ie8df581f375c9a183a7af60b431561bda82f6e34 Gerrit-Change-Number: 29258 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

Change in libosmo-netif[master]: osmux: Allocate struct osmux_out_handle through API
by pespin 31 Aug '22

31 Aug '22

Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-netif/+/29258 to look at the new patch set (#2). Change subject: osmux: Allocate struct osmux_out_handle through API ...................................................................... osmux: Allocate struct osmux_out_handle through API Until now, the osmux_out_handle was allocated by the client, and passed to the API to initialize it. This makes it really hard to improve the implementation without breaking the ABI. Let's break the ABI now one last time (hopefully) by allocating the struct through an API. With only this change, the already built users (osmo-mgw, openbsc) can still work fine, since there's no change on the struct osmux_out_handle. However, they will somehow break next time the struct is changed until they are ported to the same API (easy to do). Related: OS#5987 Change-Id: Ie8df581f375c9a183a7af60b431561bda82f6e34 --- M TODO-RELEASE M examples/osmux-test-output.c M include/osmocom/netif/osmux.h M src/osmux.c M tests/jibuf/jibuf_tool.c M tests/osmux/osmux_test.c M tests/osmux/osmux_test2.c 7 files changed, 145 insertions(+), 72 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/58/29258/2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29258 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: Ie8df581f375c9a183a7af60b431561bda82f6e34 Gerrit-Change-Number: 29258 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-MessageType: newpatchset

1 0

Change in osmo-mgw[master]: osmux: Use new osmux APIs to let libosmo-netif alloc struct osmux_out...
by pespin 31 Aug '22

31 Aug '22

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/29259 ) Change subject: osmux: Use new osmux APIs to let libosmo-netif alloc struct osmux_out_handle ...................................................................... osmux: Use new osmux APIs to let libosmo-netif alloc struct osmux_out_handle This way we don't need to worry about implementation details (struct size) in the future in case they change (they will). Related: OS#5987 Requires: libosmo-netif.git Change-Id Ie8df581f375c9a183a7af60b431561bda82f6e34 Change-Id: I2d0d8c152b8f1234ddfcd74d6cb04b1818b41510 --- M include/osmocom/mgcp/mgcp_conn.h M src/libosmo-mgcp/mgcp_osmux.c 2 files changed, 12 insertions(+), 12 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/59/29259/1 diff --git a/include/osmocom/mgcp/mgcp_conn.h b/include/osmocom/mgcp/mgcp_conn.h index 7338027..a08c089 100644 --- a/include/osmocom/mgcp/mgcp_conn.h +++ b/include/osmocom/mgcp/mgcp_conn.h @@ -84,10 +84,10 @@ bool cid_allocated; /* Allocated Osmux circuit ID for this conn */ uint8_t cid; - /* handle to batch messages */ + /* handle to batch messages, shared (refcounted) among several conns */ struct osmux_in_handle *in; - /* handle to unbatch messages */ - struct osmux_out_handle out; + /* handle to unbatch messages, one allocated and owned per conn */ + struct osmux_out_handle *out; /* statistics */ struct { uint32_t chunks; diff --git a/src/libosmo-mgcp/mgcp_osmux.c b/src/libosmo-mgcp/mgcp_osmux.c index abc3f0e..5ce850d 100644 --- a/src/libosmo-mgcp/mgcp_osmux.c +++ b/src/libosmo-mgcp/mgcp_osmux.c @@ -403,7 +403,7 @@ if (endp_osmux_state_check(conn_src->conn->endp, conn_src, false) == 0) { conn_src->osmux.stats.octets += osmux_chunk_length(msg, rem); conn_src->osmux.stats.chunks++; - osmux_xfrm_output_sched(&conn_src->osmux.out, osmuxh); + osmux_xfrm_output_sched(conn_src->osmux.out, osmuxh); } rem = msg->len; } @@ -494,12 +494,12 @@ return -1; } - osmux_xfrm_output_init2(&conn->osmux.out, - (conn->osmux.cid * rtp_ssrc_winlen) + - (random() % rtp_ssrc_winlen), - conn->end.codec->payload_type); - - osmux_xfrm_output_set_tx_cb(&conn->osmux.out, + conn->osmux.out = osmux_xfrm_output_alloc(osmux); + osmux_xfrm_output_set_rtp_ssrc(conn->osmux.out, + (conn->osmux.cid * rtp_ssrc_winlen) + + (random() % rtp_ssrc_winlen)); + osmux_xfrm_output_set_rtp_pl_type(conn->osmux.out, conn->end.codec->payload_type); + osmux_xfrm_output_set_tx_cb(conn->osmux.out, scheduled_from_osmux_tx_rtp_cb, conn); conn->osmux.state = OSMUX_STATE_ENABLED; @@ -519,8 +519,8 @@ if (conn->osmux.state == OSMUX_STATE_ENABLED) { /* We are closing, we don't need pending RTP packets to be transmitted */ - osmux_xfrm_output_set_tx_cb(&conn->osmux.out, NULL, NULL); - osmux_xfrm_output_flush(&conn->osmux.out); + osmux_xfrm_output_set_tx_cb(conn->osmux.out, NULL, NULL); + TALLOC_FREE(conn->osmux.out); osmux_xfrm_input_close_circuit(conn->osmux.in, conn->osmux.cid); conn->osmux.state = OSMUX_STATE_DISABLED; -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/29259 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I2d0d8c152b8f1234ddfcd74d6cb04b1818b41510 Gerrit-Change-Number: 29259 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

Change in libosmo-netif[master]: osmux: Allocate struct osmux_out_handle through API
by pespin 31 Aug '22

31 Aug '22

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29258 ) Change subject: osmux: Allocate struct osmux_out_handle through API ...................................................................... osmux: Allocate struct osmux_out_handle through API Until now, the osmux_out_handle was allocated by the client, and passed to the API to initialize it. This makes it really hard to improve the implementation without breaking the ABI. Let's break the ABI now one last time (hopefully) by allocating the struct through an API. With only this change, the already built users (osmo-mgw, openbsc) can still work fine, since there's no change on the struct osmux_out_handle. However, they will somehow break next time the struct is changed until they are ported to the same API (easy to do). Related: OS#5987 Change-Id: Ie8df581f375c9a183a7af60b431561bda82f6e34 --- M TODO-RELEASE M examples/osmux-test-output.c M include/osmocom/netif/osmux.h M src/osmux.c 4 files changed, 50 insertions(+), 7 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/58/29258/1 diff --git a/TODO-RELEASE b/TODO-RELEASE index d0852fc..52a8c49 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -7,3 +7,4 @@ # If any interfaces have been added since the last public release: c:r:a + 1. # If any interfaces have been removed or changed since the last public release: c:r:0. #library what description / commit summary line +libosmo-netif osmux new osmux_xfrm_output_* APIs diff --git a/examples/osmux-test-output.c b/examples/osmux-test-output.c index d39277b..3f0e5a2 100644 --- a/examples/osmux-test-output.c +++ b/examples/osmux-test-output.c @@ -42,7 +42,7 @@ * This is the output handle for osmux, it stores last RTP sequence and * timestamp that has been used. There should be one per circuit ID. */ -static struct osmux_out_handle h_output; +static struct osmux_out_handle *h_output; static int fd; @@ -107,7 +107,7 @@ msg->len, buf); while((osmuxh = osmux_xfrm_output_pull(msg)) != NULL) - osmux_xfrm_output_sched(&h_output, osmuxh); + osmux_xfrm_output_sched(h_output, osmuxh); return 0; } @@ -117,7 +117,7 @@ LOGP(DOSMUX_TEST, LOGL_NOTICE, "closing OSMUX.\n"); osmo_dgram_close(conn); osmo_dgram_destroy(conn); - osmux_xfrm_output_flush(&h_output); + talloc_free(h_output); osmo_rtp_handle_free(rtp); amr_close(); exit(EXIT_SUCCESS); @@ -155,8 +155,10 @@ /* * initialize OSMUX handlers. */ - osmux_xfrm_output_init2(&h_output, random(), 98); - osmux_xfrm_output_set_tx_cb(&h_output, tx_cb, NULL); + h_output = osmux_xfrm_output_alloc(NULL); + osmux_xfrm_output_set_rtp_ssrc(h_output, random()); + osmux_xfrm_output_set_rtp_pl_type(h_output, 98); + osmux_xfrm_output_set_tx_cb(h_output, tx_cb, NULL); /* * initialize datagram server. */ diff --git a/include/osmocom/netif/osmux.h b/include/osmocom/netif/osmux.h index 8d39344..8742797 100644 --- a/include/osmocom/netif/osmux.h +++ b/include/osmocom/netif/osmux.h @@ -107,8 +107,11 @@ int osmux_xfrm_input(struct osmux_in_handle *h, struct msgb *msg, int ccid); void osmux_xfrm_input_deliver(struct osmux_in_handle *h); -void osmux_xfrm_output_init(struct osmux_out_handle *h, uint32_t rtp_ssrc) OSMO_DEPRECATED("Use osmux_xfrm_output_init2() instead"); -void osmux_xfrm_output_init2(struct osmux_out_handle *h, uint32_t rtp_ssrc, uint8_t rtp_payload_type); +struct osmux_out_handle *osmux_xfrm_output_alloc(void *ctx); +void osmux_xfrm_output_init(struct osmux_out_handle *h, uint32_t rtp_ssrc) OSMO_DEPRECATED("Use osmux_xfrm_output_alloc() and osmux_xfrm_output_set_rtp_*() instead"); +void osmux_xfrm_output_init2(struct osmux_out_handle *h, uint32_t rtp_ssrc, uint8_t rtp_payload_type) OSMO_DEPRECATED("Use osmux_xfrm_output_alloc() and osmux_xfrm_output_set_rtp_*() instead"); +void osmux_xfrm_output_set_rtp_ssrc(struct osmux_out_handle *h, uint32_t rtp_ssrc); +void osmux_xfrm_output_set_rtp_pl_type(struct osmux_out_handle *h, uint32_t rtp_payload_type); void osmux_xfrm_output_set_tx_cb(struct osmux_out_handle *h, void (*tx_cb)(struct msgb *msg, void *data), void *data); int osmux_xfrm_output_sched(struct osmux_out_handle *h, struct osmux_hdr *osmuxh); void osmux_xfrm_output_flush(struct osmux_out_handle *h); diff --git a/src/osmux.c b/src/osmux.c index 7d26fe8..c9b20aa 100644 --- a/src/osmux.c +++ b/src/osmux.c @@ -870,6 +870,33 @@ void *data; }; +static int osmux_xfrm_output_talloc_destructor(struct osmux_out_handle *h) +{ + osmux_xfrm_output_flush(h); + return 0; +} + +/* Returned pointer can be freed with regular talloc_free, queue will be flushed + * and all internal data will be freed. */ +struct osmux_out_handle *osmux_xfrm_output_alloc(void *ctx) +{ + struct osmux_out_handle *h; + + h = talloc_zero(ctx, struct osmux_out_handle); + OSMO_ASSERT(h); + + h->rtp_seq = (uint16_t)random(); + h->rtp_timestamp = (uint32_t)random(); + h->rtp_ssrc = (uint32_t)random(); + h->rtp_payload_type = 98; + INIT_LLIST_HEAD(&h->list); + osmo_timer_setup(&h->timer, osmux_xfrm_output_trigger, h); + + talloc_set_destructor(h, osmux_xfrm_output_talloc_destructor); + return h; +} + +/* DEPRECATED: Use osmux_xfrm_output_alloc() and osmux_xfrm_output_set_rtp_*() instead */ void osmux_xfrm_output_init2(struct osmux_out_handle *h, uint32_t rtp_ssrc, uint8_t rtp_payload_type) { memset(h, 0, sizeof(*h)); @@ -881,6 +908,7 @@ osmo_timer_setup(&h->timer, osmux_xfrm_output_trigger, h); } +/* DEPRECATED: Use osmux_xfrm_output_alloc() and osmux_xfrm_output_set_rtp_*() instead */ void osmux_xfrm_output_init(struct osmux_out_handle *h, uint32_t rtp_ssrc) { /* backward compatibility with old users, where 98 was harcoded in osmux_rebuild_rtp() */ @@ -903,6 +931,15 @@ h->data = data; } +void osmux_xfrm_output_set_rtp_ssrc(struct osmux_out_handle *h, uint32_t rtp_ssrc) +{ + h->rtp_ssrc = rtp_ssrc; +} +void osmux_xfrm_output_set_rtp_pl_type(struct osmux_out_handle *h, uint32_t rtp_payload_type) +{ + h->rtp_payload_type = rtp_payload_type; +} + #define SNPRINTF_BUFFER_SIZE(ret, remain, offset) \ if (ret < 0) \ ret = 0; \ -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29258 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: Ie8df581f375c9a183a7af60b431561bda82f6e34 Gerrit-Change-Number: 29258 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

2026

2025

2024

2023

2022

gerrit-log