March 2024 - gerrit-log - lists.osmocom.org

[S] Change in libosmocore[master]: msgb: Also copy msgb {dst, lchan, cb} during msgb_copy_*

by laforge

Attention is currently required from: dexter. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/36324?usp=email ) Change subject: msgb: Also copy msgb {dst, lchan, cb} during msgb_copy_* ...................................................................... Patch Set 2: (1 comment) File src/core/msgb.c: https://gerrit.osmocom.org/c/libosmocore/+/36324/comment/8d80ac34_4e0e9f4a PS1, Line 320: * and adjusts the pointers (incl l1h-l4h) accordingly. The cb part is not copied. > mh, ok. so the old function was *specified* not to copy the cb. That's weird. […] I did a rather exhaustive study and could not find any osmocoom code that would rely on the existing behaviour of not copying the cb. It should therefore be safe to merge this patch, but of course the comment should be updated to reflect reality. I still don't feel 100% allright with changing the semantics. Even if I believe the cb should always have been copied, we had different behaviour since the function was introduced in 2015... -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36324?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I68328adb952ca8833ba047cb3b49ccc6f8a1f1b5 Gerrit-Change-Number: 36324 Gerrit-PatchSet: 2 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 19 Mar 2024 19:46:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: comment

1 year, 5 months

1
0
0 0

[S] Change in libosmocore[master]: WIP: migrate gsmtap logging over to osmo_io

by laforge

laforge has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/36362?usp=email ) Change subject: WIP: migrate gsmtap logging over to osmo_io ...................................................................... WIP: migrate gsmtap logging over to osmo_io This is working in principle; The problem is just that osmo_io itself contains LOGP calls from within osmo_iofd_write_msgb() and downstream code, so we might easily get up in an infinite recursion bug. This needs further study; possibly we need a "safe" logging function which will never use osmo_io. Or something like a per-thread global boolean variable which the code can check to know if it's re-entering? Change-Id: I69d7eca7d50666b4fcfe73a1dc03f8e8491b32fc --- M include/osmocom/core/gsmtap_util.h M src/core/gsmtap_util.c M src/core/libosmocore.map M src/vty/logging_vty.c 4 files changed, 41 insertions(+), 5 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/62/36362/1 diff --git a/include/osmocom/core/gsmtap_util.h b/include/osmocom/core/gsmtap_util.h index d24ee95..e755931 100644 --- a/include/osmocom/core/gsmtap_util.h +++ b/include/osmocom/core/gsmtap_util.h @@ -1,5 +1,6 @@ #pragma once +#include <stdbool.h> #include <stdint.h> #include <osmocom/core/write_queue.h> #include <osmocom/core/select.h> @@ -31,6 +32,8 @@ int gsmtap_inst_fd2(const struct gsmtap_inst *gti); +bool gsmtap_inst_get_osmo_io_mode(const struct gsmtap_inst *gti); + int gsmtap_source_init_fd(const char *host, uint16_t port); int gsmtap_source_init_fd2(const char *local_host, uint16_t local_port, const char *rem_host, uint16_t rem_port); diff --git a/src/core/gsmtap_util.c b/src/core/gsmtap_util.c index b64c7b0..6c36b28 100644 --- a/src/core/gsmtap_util.c +++ b/src/core/gsmtap_util.c @@ -38,6 +38,7 @@ #include <stdio.h> #include <unistd.h> #include <stdint.h> +#include <stdbool.h> #include <string.h> #include <errno.h> @@ -85,6 +86,12 @@ return gti->wq.bfd.fd; } +/*! determine whether the gsmtap_inst is in osmo_io mode or not. */ +bool gsmtap_inst_get_osmo_io_mode(const struct gsmtap_inst *gti) +{ + return gti->osmo_io_mode; +} + /*! convert RSL channel number to GSMTAP channel type * \param[in] rsl_chantype RSL channel type * \param[in] link_id RSL link identifier diff --git a/src/core/libosmocore.map b/src/core/libosmocore.map index 072efee..bdbfe07 100644 --- a/src/core/libosmocore.map +++ b/src/core/libosmocore.map @@ -41,6 +41,7 @@ gsmtap_gsm_channel_names; gsmtap_inst_fd; gsmtap_inst_fd2; +gsmtap_inst_get_osmo_io_mode; gsmtap_makemsg; gsmtap_makemsg_ex; gsmtap_send; diff --git a/src/vty/logging_vty.c b/src/vty/logging_vty.c index 2a07422..bfe73bb 100644 --- a/src/vty/logging_vty.c +++ b/src/vty/logging_vty.c @@ -29,6 +29,7 @@ #include <osmocom/core/strrb.h> #include <osmocom/core/loggingrb.h> #include <osmocom/core/gsmtap.h> +#include <osmocom/core/gsmtap_util.h> #include <osmocom/core/application.h> #include <osmocom/vty/command.h> @@ -803,18 +804,24 @@ } DEFUN(cfg_log_gsmtap, cfg_log_gsmtap_cmd, - "log gsmtap [HOSTNAME]", + "log gsmtap [HOSTNAME] [sync-io]", LOG_STR "Logging via GSMTAP\n" "Host name to send the GSMTAP logging to (UDP port 4729)\n") { - const char *hostname = argc ? argv[0] : "127.0.0.1"; + const char *hostname = "127.0.0.1"; + bool async_io = true; struct log_target *tgt; + if (argc >= 1) + hostname = argv[0]; + if (argc >= 2 && !strcmp(argv[1], "sync-io")) + async_io = false; + log_tgt_mutex_lock(); tgt = log_target_find(LOG_TGT_TYPE_GSMTAP, hostname); if (!tgt) { tgt = log_target_create_gsmtap(hostname, GSMTAP_UDP_PORT, - host.app_info->name, false, + host.app_info->name, async_io, true); if (!tgt) { vty_out(vty, "%% Unable to create GSMTAP log for %s%s", @@ -1032,8 +1039,9 @@ log_target_rb_avail_size(tgt), VTY_NEWLINE); break; case LOG_TGT_TYPE_GSMTAP: - vty_out(vty, "log gsmtap %s%s", - tgt->tgt_gsmtap.hostname, VTY_NEWLINE); + vty_out(vty, "log gsmtap %s%s%s", tgt->tgt_gsmtap.hostname, + gsmtap_inst_get_osmo_io_mode(tgt->tgt_gsmtap.gsmtap_inst) ? "" : " sync-io", + VTY_NEWLINE); break; case LOG_TGT_TYPE_SYSTEMD: vty_out(vty, "log systemd-journal%s%s", -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/36362?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I69d7eca7d50666b4fcfe73a1dc03f8e8491b32fc Gerrit-Change-Number: 36362 Gerrit-PatchSet: 1 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-MessageType: newchange

1 year, 5 months

1
0
0 0

[M] Change in osmo-mgw[master]: Change msgb ownership in processing of received msgb

by Jenkins Builder

Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/36361?usp=email ) Change subject: Change msgb ownership in processing of received msgb ...................................................................... Patch Set 1: (1 comment) File src/libosmo-mgcp/mgcp_network.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-15248): https://gerrit.osmocom.org/c/osmo-mgw/+/36361/comment/892eaba7_ce3f0f8c PS1, Line 1285: else else is not generally useful after a break or return -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/36361?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I6a331f3c6b2eb51ea312ac6ef8c357185ddb79cf Gerrit-Change-Number: 36361 Gerrit-PatchSet: 1 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-CC: Jenkins Builder Gerrit-Comment-Date: Tue, 19 Mar 2024 18:03:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 year, 5 months

1
0
0 0

[S] Change in osmo-mgw[master]: remove osmo_fd from mgcp_create_bind()

by laforge

laforge has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/36360?usp=email ) Change subject: remove osmo_fd from mgcp_create_bind() ...................................................................... remove osmo_fd from mgcp_create_bind() preparation for osmo_io Change-Id: I4a3b66a14fdfbc867daca0f0a05f694d5e0d7b66 --- M include/osmocom/mgcp/mgcp.h M src/libosmo-mgcp/mgcp_network.c M src/libosmo-mgcp/mgcp_osmux.c 3 files changed, 26 insertions(+), 13 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/60/36360/1 diff --git a/include/osmocom/mgcp/mgcp.h b/include/osmocom/mgcp/mgcp.h index bfb412b..e61ba89 100644 --- a/include/osmocom/mgcp/mgcp.h +++ b/include/osmocom/mgcp/mgcp.h @@ -204,6 +204,5 @@ int mgcp_send_reset_all(struct mgcp_config *cfg); -int mgcp_create_bind(const char *source_addr, struct osmo_fd *fd, int port, uint8_t dscp, - uint8_t prio); +int mgcp_create_bind(const char *source_addr, int port, uint8_t dscp, uint8_t prio); int mgcp_udp_send(int fd, const struct osmo_sockaddr *addr, const char *buf, int len); diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c index ee25f74..0a1bc11 100644 --- a/src/libosmo-mgcp/mgcp_network.c +++ b/src/libosmo-mgcp/mgcp_network.c @@ -1517,13 +1517,11 @@ /*! bind RTP port to osmo_fd. * \param[in] source_addr source (local) address to bind on. - * \param[in] fd associated file descriptor. * \param[in] port to bind on. * \param[in] dscp IP DSCP value to use. * \param[in] prio socket priority to use. * \returns 0 on success, -1 on ERROR. */ -int mgcp_create_bind(const char *source_addr, struct osmo_fd *fd, int port, uint8_t dscp, - uint8_t prio) +int mgcp_create_bind(const char *source_addr, int port, uint8_t dscp, uint8_t prio) { int rc; @@ -1535,34 +1533,37 @@ source_addr, port); return -1; } - fd->fd = rc; LOGP(DRTP, LOGL_DEBUG, "created socket + bound UDP port (%s:%i).\n", source_addr, port); - return 0; + return rc; } /* Bind RTP and RTCP port (helper function for mgcp_bind_net_rtp_port()) */ static int bind_rtp(struct mgcp_config *cfg, const char *source_addr, struct mgcp_rtp_end *rtp_end, struct mgcp_endpoint *endp) { + int rc; + /* NOTE: The port that is used for RTCP is the RTP port incremented by one * (e.g. RTP-Port = 16000 ==> RTCP-Port = 16001) */ - if (mgcp_create_bind(source_addr, &rtp_end->rtp, rtp_end->local_port, - cfg->endp_dscp, cfg->endp_priority) != 0) { + rc = mgcp_create_bind(source_addr, rtp_end->local_port, cfg->endp_dscp, cfg->endp_priority); + if (rc < 0) { LOGPENDP(endp, DRTP, LOGL_ERROR, "failed to create RTP port: %s:%d\n", source_addr, rtp_end->local_port); goto cleanup0; } + rtp_end->rtp.fd = rc; - if (mgcp_create_bind(source_addr, &rtp_end->rtcp, rtp_end->local_port + 1, - cfg->endp_dscp, cfg->endp_priority) != 0) { + rc = mgcp_create_bind(source_addr, rtp_end->local_port + 1, cfg->endp_dscp, cfg->endp_priority); + if (rc < 0) { LOGPENDP(endp, DRTP, LOGL_ERROR, "failed to create RTCP port: %s:%d\n", source_addr, rtp_end->local_port + 1); goto cleanup1; } + rtp_end->rtcp.fd = rc; if (osmo_fd_register(&rtp_end->rtp) != 0) { LOGPENDP(endp, DRTP, LOGL_ERROR, diff --git a/src/libosmo-mgcp/mgcp_osmux.c b/src/libosmo-mgcp/mgcp_osmux.c index 185aecd..997b07b 100644 --- a/src/libosmo-mgcp/mgcp_osmux.c +++ b/src/libosmo-mgcp/mgcp_osmux.c @@ -514,13 +514,14 @@ osmo_fd_setup(&osmux_fd_v6, -1, OSMO_FD_READ, osmux_read_fd_cb, trunk, 0); if (cfg->osmux.local_addr_v4) { - ret = mgcp_create_bind(cfg->osmux.local_addr_v4, &osmux_fd_v4, cfg->osmux.local_port, + ret = mgcp_create_bind(cfg->osmux.local_addr_v4, cfg->osmux.local_port, cfg->endp_dscp, cfg->endp_priority); if (ret < 0) { LOGP(DOSMUX, LOGL_ERROR, "Cannot bind OSMUX IPv4 socket to %s:%u\n", cfg->osmux.local_addr_v4, cfg->osmux.local_port); return ret; } + osmux_fd_v4.fd = ret; ret = osmo_fd_register(&osmux_fd_v4); if (ret < 0) { @@ -532,13 +533,14 @@ osmo_sock_get_name2(osmux_fd_v4.fd)); } if (cfg->osmux.local_addr_v6) { - ret = mgcp_create_bind(cfg->osmux.local_addr_v6, &osmux_fd_v6, cfg->osmux.local_port, + ret = mgcp_create_bind(cfg->osmux.local_addr_v6, cfg->osmux.local_port, cfg->endp_dscp, cfg->endp_priority); if (ret < 0) { LOGP(DOSMUX, LOGL_ERROR, "Cannot bind OSMUX IPv6 socket to [%s]:%u\n", cfg->osmux.local_addr_v6, cfg->osmux.local_port); return ret; } + osmux_fd_v6.fd = ret; ret = osmo_fd_register(&osmux_fd_v6); if (ret < 0) { -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/36360?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I4a3b66a14fdfbc867daca0f0a05f694d5e0d7b66 Gerrit-Change-Number: 36360 Gerrit-PatchSet: 1 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-MessageType: newchange

1 year, 5 months

1
0
0 0

[M] Change in osmo-mgw[master]: Change msgb ownership in processing of received msgb

by laforge

laforge has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/36361?usp=email ) Change subject: Change msgb ownership in processing of received msgb ...................................................................... Change msgb ownership in processing of received msgb The old approach was: rtp_data_net() reads a msgb from the incomging socket, calls through whatever function chain and in the end free's it. So none of the intermediate functions was permitted to take msgb ownership. This was a good choice as all processing would happen synchronously, up to the point where that msgb was written on the output RTP socket. Let's change this from passing msgb ownership throug the whole call chain, through rx_rtp() to the various *_dispatch_rtp() functions. This is required for upcoming migration to osmo_io, as in that case the write (sendto) calls are asynchronous and hence msgb ownership needs to be transferred. Change-Id: I6a331f3c6b2eb51ea312ac6ef8c357185ddb79cf --- M TODO-RELEASE M src/libosmo-mgcp/mgcp_e1.c M src/libosmo-mgcp/mgcp_iuup.c M src/libosmo-mgcp/mgcp_network.c M src/libosmo-mgcp/mgcp_osmux.c 5 files changed, 131 insertions(+), 40 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/61/36361/1 diff --git a/TODO-RELEASE b/TODO-RELEASE index ea29b6a..59da45b 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -39,3 +39,5 @@ is backwards compat code that moves codecs[] entries, if any, over to ptmap[], so callers may migrate at own leisure. osmo-mgw remove cfg Remove VTY config item 'sdp audio fmtp-extra' (see OS#6313) +libosmocore bump_dep; workaround Bump libosmocore version dependency after I68328adb952ca8833ba047cb3b49ccc6f8a1f1b5 + has been merged to libosmocore.git; then remove my_msgb_copy_c wrapper function. diff --git a/src/libosmo-mgcp/mgcp_e1.c b/src/libosmo-mgcp/mgcp_e1.c index 40f976b..20aff95 100644 --- a/src/libosmo-mgcp/mgcp_e1.c +++ b/src/libosmo-mgcp/mgcp_e1.c @@ -301,7 +301,6 @@ mgcp_send(endp, 1, NULL, msg, &conn_dst->u.rtp, &conn_dst->u.rtp); - msgb_free(msg); return; skip: rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, E1_I460_TRAU_RX_FAIL_CTR)); diff --git a/src/libosmo-mgcp/mgcp_iuup.c b/src/libosmo-mgcp/mgcp_iuup.c index 6ad62c8..3818d3e 100644 --- a/src/libosmo-mgcp/mgcp_iuup.c +++ b/src/libosmo-mgcp/mgcp_iuup.c @@ -311,7 +311,6 @@ }; rc = mgcp_send(conn_rtp_dst->conn->endp, true, NULL, msg, conn_rtp_src, conn_rtp_dst); - msgb_free(msg); return rc; } @@ -640,7 +639,7 @@ } /* Build IuUP RNL Data primitive from msg containing an incoming RTP pkt from - * peer and send it down the IuUP layer towards the destination as IuUP/RTP: */ + * peer and send it down the IuUP layer towards the destination as IuUP/RTP. Takes ownership of msg. */ int mgcp_conn_iuup_send_rtp(struct mgcp_conn_rtp *conn_src_rtp, struct mgcp_conn_rtp *conn_dest_rtp, struct msgb *msg) { struct osmo_iuup_rnl_prim *irp; diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c index 0a1bc11..3ed4814 100644 --- a/src/libosmo-mgcp/mgcp_network.c +++ b/src/libosmo-mgcp/mgcp_network.c @@ -70,6 +70,18 @@ rtpconn_rate_ctr_add(conn_rtp, endp, id, 1); } +/* wrapper around libosmocore msgb_copy_c, which [at least before libosmocore.git Change-Id + * I68328adb952ca8833ba047cb3b49ccc6f8a1f1b5] doesn't copy the cb */ +static inline struct msgb *mgw_msgb_copy_c(void *ctx, struct msgb *msg, const char *name) +{ + struct msgb *msg2 = msgb_copy_c(ctx, msg, name); + if (OSMO_UNLIKELY(!msg2)) + return NULL; + + memcpy(msg2->cb, msg->cb, sizeof(msg2->cb)); + return msg2; +} + static int rx_rtp(struct msgb *msg); bool mgcp_rtp_end_remote_addr_available(const struct mgcp_rtp_end *rtp_end) @@ -963,7 +975,7 @@ return 0; } -/*! Dispatch msg bridged from the sister conn in the endpoint. +/*! Dispatch msg bridged from the sister conn in the endpoint. Takes ownership of msgb. * \param[in] conn_dst The destination conn that should handle and transmit the content to * its peer outside MGW. * \param[in] msg msgb containing an RTP pkt received by the sister conn in the endpoint, @@ -985,8 +997,10 @@ /* Before we try to deliver the packet, we check if the destination * port and IP-Address make sense at all. If not, we will be unable * to deliver the packet. */ - if (check_rtp_destin(conn_dst) != 0) + if (check_rtp_destin(conn_dst) != 0) { + msgb_free(msg); return -1; + } /* Depending on the RTP connection type, deliver the RTP packet to the * destination connection. */ @@ -1021,7 +1035,7 @@ * be discarded, this should not happen, normally the MGCP type * should be properly set */ LOGPENDP(endp, DRTP, LOGL_ERROR, "bad MGCP type -- data discarded!\n"); - + msgb_free(msg); return -1; } @@ -1101,7 +1115,7 @@ return -1; } -/*! Send RTP/RTCP data to a specified destination connection. +/*! Send RTP/RTCP data to a specified destination connection. Takes ownership of msg. * \param[in] endp associated endpoint (for configuration, logging). * \param[in] is_rtp flag to specify if the packet is of type RTP or RTCP. * \param[in] addr spoofed source address (set to NULL to disable). @@ -1140,6 +1154,7 @@ if (is_rtp && !mgcp_conn_rtp_is_iuup(conn_src)) { if (mgcp_patch_pt(conn_dst, msg) < 0) { LOGPENDP(endp, DRTP, LOGL_NOTICE, "unable to patch payload type RTP packet, discarding...\n"); + msgb_free(msg); return -EINVAL; } } @@ -1212,14 +1227,16 @@ len = mgcp_udp_send(rtp_end->rtp.fd, &rtp_end->addr, (char *)msgb_data(msg), msgb_length(msg)); - - if (len <= 0) + if (len <= 0) { + msgb_free(msg); return len; + } rtpconn_rate_ctr_inc(conn_dst, endp, RTP_PACKETS_TX_CTR); rtpconn_rate_ctr_add(conn_dst, endp, RTP_OCTETS_TX_CTR, len); rtp_state->alt_rtp_tx_sequence++; + msgb_free(msg); return len; } else if (!trunk->omit_rtcp) { struct osmo_sockaddr rtcp_addr = rtp_end->addr; @@ -1238,12 +1255,44 @@ rtpconn_rate_ctr_add(conn_dst, endp, RTP_OCTETS_TX_CTR, len); rtp_state->alt_rtp_tx_sequence++; + msgb_free(msg); return len; } + msgb_free(msg); return 0; } +/*! determine if there's only a single recipient in endp for data received via conn_src. + * The function returns NULL in case there is no recipient, or in case there are multiple recipients. + * \param endp The MGCP endpoint whose connections to analyze + * \param conn_src The source MGCP connection [which shall not count in results] + * \returns recipient donnection if there is only one; NULL in case there are multiple */ +static struct mgcp_conn *rtpbridge_get_only_recipient(struct mgcp_endpoint *endp, struct mgcp_conn *conn_src) +{ + struct mgcp_conn *conn_ret = NULL; + struct mgcp_conn *conn_dst; + + llist_for_each_entry(conn_dst, &endp->conns, entry) { + if (conn_dst == conn_src) + continue; + switch (conn_dst->mode) { + case MGCP_CONN_SEND_ONLY: + case MGCP_CONN_RECV_SEND: + case MGCP_CONN_CONFECHO: + if (conn_ret) + return NULL; + else + conn_ret = conn_dst; + break; + default: + break; + } + } + + return conn_ret; +} + /*! Dispatch incoming RTP packet to opposite RTP connection. * \param[in] msg Message buffer to bridge, coming from source connection. * msg shall contain "struct osmo_rtp_msg_ctx *" attached in @@ -1301,23 +1350,44 @@ return rc; } - /* If the mode is "confecho", send RTP back to the sender. */ - if (conn->mode == MGCP_CONN_CONFECHO) - rc = mgcp_conn_rtp_dispatch_rtp(conn_src, msg); + /* All the use cases above are 1:1 where we have one source msgb and we're sending that to one + * destination. msgb ownership had been passed to the respective _*dospatch_rtp() function. + * In the cases below, we actually [can] have multiple recipients, so we copy the original msgb + * for each of the recipients. */ - /* Dispatch RTP packet to all other connection(s) that send audio. */ - llist_for_each_entry(conn_dst, &endp->conns, entry) { - if (conn_dst == conn) - continue; - switch (conn_dst->mode) { - case MGCP_CONN_SEND_ONLY: - case MGCP_CONN_RECV_SEND: - case MGCP_CONN_CONFECHO: - rc = mgcp_conn_rtp_dispatch_rtp(&conn_dst->u.rtp, msg); - break; - default: - break; + /* If the mode is "confecho", send RTP back to the sender. */ + if (conn->mode == MGCP_CONN_CONFECHO) { + struct msgb *msg2 = mgw_msgb_copy_c(conn, msg, "RTP confecho"); + if (OSMO_LIKELY(msg2)) + rc = mgcp_conn_rtp_dispatch_rtp(conn_src, msg2); + } + + conn_dst = rtpbridge_get_only_recipient(endp, conn); + if (OSMO_LIKELY(conn_dst)) { + /* we only have a single recipient and cann hence send the original msgb without copying */ + rc = mgcp_conn_rtp_dispatch_rtp(&conn_dst->u.rtp, msg); + } else { + /* Dispatch RTP packet to all other connection(s) that send audio. */ + llist_for_each_entry(conn_dst, &endp->conns, entry) { + struct msgb *msg2; + if (conn_dst == conn) + continue; + switch (conn_dst->mode) { + case MGCP_CONN_SEND_ONLY: + case MGCP_CONN_RECV_SEND: + case MGCP_CONN_CONFECHO: + /* we have multiple recipients and must make copies for each recipient */ + msg2 = mgw_msgb_copy_c(conn_dst, msg, "RTP Tx copy"); + if (OSMO_LIKELY(msg2)) + rc = mgcp_conn_rtp_dispatch_rtp(&conn_dst->u.rtp, msg2); + break; + default: + break; + } } + /* as we only sent copies in the previous llist_for_each_entry() loop, we must free the + * original one */ + msgb_free(msg); } return rc; } @@ -1471,11 +1541,11 @@ rc = rx_rtp(msg); out: - msgb_free(msg); return rc; } -/* Note: This function is able to handle RTP and RTCP */ +/* Note: This function is able to handle RTP and RTCP. msgb ownership is transferred, so this function or its + * downstream consumers must make sure to [eventually] free the msgb. */ static int rx_rtp(struct msgb *msg) { struct osmo_rtp_msg_ctx *mc = OSMO_RTP_MSG_CTX(msg); @@ -1488,7 +1558,7 @@ /* Check if the origin of the RTP packet seems plausible */ if (!trunk->rtp_accept_all && check_rtp_origin(conn_src, from_addr)) - return -1; + goto out_free; /* Handle AMR frame format conversion (octet-aligned vs. bandwith-efficient) */ if (mc->proto == MGCP_PROTO_RTP @@ -1498,13 +1568,13 @@ * communicated via SDP when the connection was created/modfied. */ int oa = amr_oa_check((char*)msgb_data(msg), msgb_length(msg)); if (oa < 0) - return -1; + goto out_free; if (((bool)oa) != conn_src->end.codec->param.amr_octet_aligned) { LOG_CONN_RTP(conn_src, LOGL_NOTICE, "rx_rtp(%u bytes): Expected RTP AMR octet-aligned=%u but got octet-aligned=%u." " check the config of your call-agent!\n", msgb_length(msg), conn_src->end.codec->param.amr_octet_aligned, oa); - return -1; + goto out_free; } } @@ -1513,6 +1583,9 @@ /* Execute endpoint specific implementation that handles the * dispatching of the RTP data */ return conn->endp->type->dispatch_rtp_cb(msg); +out_free: + msgb_free(msg); + return -1; } /*! bind RTP port to osmo_fd. diff --git a/src/libosmo-mgcp/mgcp_osmux.c b/src/libosmo-mgcp/mgcp_osmux.c index 997b07b..70798eb 100644 --- a/src/libosmo-mgcp/mgcp_osmux.c +++ b/src/libosmo-mgcp/mgcp_osmux.c @@ -204,14 +204,13 @@ return h->in; } -/*! send RTP packet through OSMUX connection. +/*! send RTP packet through OSMUX connection. Takes ownership of msg. * \param[in] conn associated RTP connection * \param[in] msg msgb containing an RTP AMR packet * \returns 0 on success, -1 on ERROR */ int conn_osmux_send_rtp(struct mgcp_conn_rtp *conn, struct msgb *msg) { int ret; - struct msgb *msg2; if (!conn->end.output_enabled) { rtpconn_osmux_rate_ctr_inc(conn, OSMUX_RTP_PACKETS_TX_DROPPED_CTR); @@ -225,19 +224,14 @@ return -1; } - /* msg is not owned by us and will be freed by the caller stack upon return: */ - msg2 = msgb_copy_c(conn->conn, msg, "osmux-rtp-send"); - if (!msg2) - return -1; - /* Osmux implementation works with AMR OA only, make sure we convert to it if needed: */ - if (amr_oa_bwe_convert(conn->conn->endp, msg2, true) < 0) { + if (amr_oa_bwe_convert(conn->conn->endp, msg, true) < 0) { LOGPCONN(conn->conn, DOSMUX, LOGL_ERROR, "Error converting to AMR octet-aligned mode\n"); return -1; } - while ((ret = osmux_xfrm_input(conn->osmux.in, msg2, conn->osmux.remote_cid)) > 0) { + while ((ret = osmux_xfrm_input(conn->osmux.in, msg, conn->osmux.remote_cid)) > 0) { /* batch full, build and deliver it */ osmux_xfrm_input_deliver(conn->osmux.in); } @@ -245,7 +239,7 @@ rtpconn_osmux_rate_ctr_inc(conn, OSMUX_RTP_PACKETS_TX_DROPPED_CTR); } else { rtpconn_osmux_rate_ctr_inc(conn, OSMUX_RTP_PACKETS_TX_CTR); - rtpconn_osmux_rate_ctr_add(conn, OSMUX_AMR_OCTETS_TX_CTR, msgb_length(msg2) - sizeof(struct rtp_hdr)); + rtpconn_osmux_rate_ctr_add(conn, OSMUX_AMR_OCTETS_TX_CTR, msgb_length(msg) - sizeof(struct rtp_hdr)); } return 0; } @@ -325,7 +319,7 @@ }; endp->type->dispatch_rtp_cb(msg); - msgb_free(msg); + /* dispatch_rtp_cb() has taken ownership of the msgb */ } static struct msgb *osmux_recv(struct osmo_fd *ofd, struct osmo_sockaddr *addr) -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/36361?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I6a331f3c6b2eb51ea312ac6ef8c357185ddb79cf Gerrit-Change-Number: 36361 Gerrit-PatchSet: 1 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-MessageType: newchange

1 year, 5 months

1
0
0 0

[S] Change in osmo-remsim[master]: debian/rules: don't compress pdfs

by osmith

osmith has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-remsim/+/36359?usp=email ) Change subject: debian/rules: don't compress pdfs ...................................................................... debian/rules: don't compress pdfs Adjust debian packaging as in other Osmocom projects to not compress the pdf files. Related: SYS#6855 Change-Id: I4372c2d93b317a9deaed3650941c7438accf9211 --- M debian/rules 1 file changed, 17 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-remsim refs/changes/59/36359/1 diff --git a/debian/rules b/debian/rules index fddb043..779d944 100755 --- a/debian/rules +++ b/debian/rules @@ -25,3 +25,7 @@ # Print test results in case of a failure #override_dh_auto_test: # dh_auto_test || (find . -name testsuite.log -exec cat {} \; ; false) + +# Don't create .pdf.gz files (barely saves space and they can't be opened directly by most pdf readers) +override_dh_compress: + dh_compress -X.pdf -- To view, visit https://gerrit.osmocom.org/c/osmo-remsim/+/36359?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-remsim Gerrit-Branch: master Gerrit-Change-Id: I4372c2d93b317a9deaed3650941c7438accf9211 Gerrit-Change-Number: 36359 Gerrit-PatchSet: 1 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-MessageType: newchange

1 year, 5 months

1
0
0 0

[S] Change in osmo-mgw[master]: simplify unused transcoding/processing call-back

by laforge

laforge has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/36357?usp=email ) Change subject: simplify unused transcoding/processing call-back ...................................................................... simplify unused transcoding/processing call-back the processing call-back is working with a raw buffer + length, while we actually work with struct msgb. Let's simply pass the msgb into the call-back, and the call-back can then do what they want with the contents of that msgb. Change-Id: I002624f9008726e3d754d48aa2282c38e3b42953 --- M include/osmocom/mgcp/mgcp.h M include/osmocom/mgcp/mgcp_network.h M src/libosmo-mgcp/mgcp_network.c 3 files changed, 20 insertions(+), 13 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/57/36357/1 diff --git a/include/osmocom/mgcp/mgcp.h b/include/osmocom/mgcp/mgcp.h index 42d67cf..bfb412b 100644 --- a/include/osmocom/mgcp/mgcp.h +++ b/include/osmocom/mgcp/mgcp.h @@ -70,12 +70,10 @@ /** * Return: * < 0 in case no audio was processed - * >= 0 in case audio was processed. The remaining payload - * length will be returned. + * >= 0 in case audio was processed. */ typedef int (*mgcp_processing)(struct mgcp_endpoint *endp, - struct mgcp_rtp_end *dst_end, - char *data, int *len, int buf_size); + struct mgcp_rtp_end *dst_end, struct msgb *msg); struct mgcp_conn_rtp; diff --git a/include/osmocom/mgcp/mgcp_network.h b/include/osmocom/mgcp/mgcp_network.h index a7bd333..1ec8979 100644 --- a/include/osmocom/mgcp/mgcp_network.h +++ b/include/osmocom/mgcp/mgcp_network.h @@ -159,8 +159,7 @@ int mgcp_get_local_addr(char *addr, struct mgcp_conn_rtp *conn); /* payload processing default functions */ -int mgcp_rtp_processing_default(struct mgcp_endpoint *endp, struct mgcp_rtp_end *dst_end, - char *data, int *len, int buf_size); +int mgcp_rtp_processing_default(struct mgcp_endpoint *endp, struct mgcp_rtp_end *dst_end, struct msgb *msg); int mgcp_setup_rtp_processing_default(struct mgcp_endpoint *endp, struct mgcp_conn_rtp *conn_dst, diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c index 31c02d2..4e5b307 100644 --- a/src/libosmo-mgcp/mgcp_network.c +++ b/src/libosmo-mgcp/mgcp_network.c @@ -404,13 +404,11 @@ /*! dummy callback to disable transcoding (see also cfg->rtp_processing_cb). * \param[in] associated endpoint. * \param[in] destination RTP end. - * \param[in,out] pointer to buffer with voice data. - * \param[in] voice data length. - * \param[in] maximum size of caller provided voice data buffer. + * \param[in,out] msg message bufffer containing data. Function might change length. * \returns ignores input parameters, return always 0. */ int mgcp_rtp_processing_default(struct mgcp_endpoint *endp, struct mgcp_rtp_end *dst_end, - char *data, int *len, int buf_size) + struct msgb *msg) { LOGPENDP(endp, DRTP, LOGL_DEBUG, "transcoding disabled\n"); return 0; @@ -1168,14 +1166,12 @@ osmo_sockaddr_port(&rtp_end->addr.u.sa), ntohs(rtp_end->rtcp_port) ); } else if (is_rtp) { - int buflen = msgb_length(msg); - /* Make sure we have a valid RTP header, in cases where no RTP * header is present, we will generate one. */ gen_rtp_header(msg, rtp_end, rtp_state); /* Run transcoder */ - rc = endp->trunk->cfg->rtp_processing_cb(endp, rtp_end, (char *)msgb_data(msg), &buflen, RTP_BUF_SIZE); + rc = endp->trunk->cfg->rtp_processing_cb(endp, rtp_end, msg); if (rc < 0) { LOGPENDP(endp, DRTP, LOGL_ERROR, "Error %d during transcoding\n", rc); return rc; -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/36357?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I002624f9008726e3d754d48aa2282c38e3b42953 Gerrit-Change-Number: 36357 Gerrit-PatchSet: 1 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-MessageType: newchange

1 year, 5 months

1
0
0 0

[M] Change in osmo-mgw[master]: remove strange loop for non-existant transcoding support

by laforge

laforge has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/36356?usp=email ) Change subject: remove strange loop for non-existant transcoding support ...................................................................... remove strange loop for non-existant transcoding support The existing support preparing the mgw for transcoding (which doesn't exist) has some kind of method where the transcoding function might be called multiple times in a row. However, as it is not used, it is not entirely clear how it was intended to work. Let's remove this unused looping feature which makes it hard to understand how upcoming osmo_io should deal with it. Change-Id: Ie1a629fd31c5ab806fc929d1e6b279c4be5b8246 --- M src/libosmo-mgcp/mgcp_network.c 1 file changed, 59 insertions(+), 49 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/56/36356/1 diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c index 111c5cf..31c02d2 100644 --- a/src/libosmo-mgcp/mgcp_network.c +++ b/src/libosmo-mgcp/mgcp_network.c @@ -1168,70 +1168,64 @@ osmo_sockaddr_port(&rtp_end->addr.u.sa), ntohs(rtp_end->rtcp_port) ); } else if (is_rtp) { - int cont; - int nbytes = 0; int buflen = msgb_length(msg); /* Make sure we have a valid RTP header, in cases where no RTP * header is present, we will generate one. */ gen_rtp_header(msg, rtp_end, rtp_state); - do { - /* Run transcoder */ - cont = endp->trunk->cfg->rtp_processing_cb(endp, rtp_end, (char *)msgb_data(msg), &buflen, RTP_BUF_SIZE); - if (cont < 0) - break; + /* Run transcoder */ + rc = endp->trunk->cfg->rtp_processing_cb(endp, rtp_end, (char *)msgb_data(msg), &buflen, RTP_BUF_SIZE); + if (rc < 0) { + LOGPENDP(endp, DRTP, LOGL_ERROR, "Error %d during transcoding\n", rc); + return rc; + } - if (addr) - mgcp_patch_and_count(endp, rtp_state, rtp_end, - addr, msg); + if (addr) + mgcp_patch_and_count(endp, rtp_state, rtp_end, addr, msg); - if (mgcp_conn_rtp_is_iuup(conn_dst) || mgcp_conn_rtp_is_iuup(conn_src)) { - /* the iuup code will correctly transform to the correct AMR mode */ - } else if (mgcp_codec_amr_align_mode_is_indicated(conn_dst->end.codec)) { - rc = amr_oa_bwe_convert(endp, msg, - conn_dst->end.codec->param.amr_octet_aligned); - if (rc < 0) { - LOGPENDP(endp, DRTP, LOGL_ERROR, - "Error in AMR octet-aligned <-> bandwidth-efficient mode conversion (target=%s)\n", - conn_dst->end.codec->param.amr_octet_aligned ? "octet-aligned" : "bandwidth-efficient"); - break; - } - } else if (rtp_end->rfc5993_hr_convert && - strcmp(conn_src->end.codec->subtype_name, "GSM-HR-08") == 0) { - rc = rfc5993_hr_convert(endp, msg); - if (rc < 0) { - LOGPENDP(endp, DRTP, LOGL_ERROR, "Error while converting to GSM-HR-08\n"); - break; - } + if (mgcp_conn_rtp_is_iuup(conn_dst) || mgcp_conn_rtp_is_iuup(conn_src)) { + /* the iuup code will correctly transform to the correct AMR mode */ + } else if (mgcp_codec_amr_align_mode_is_indicated(conn_dst->end.codec)) { + rc = amr_oa_bwe_convert(endp, msg, conn_dst->end.codec->param.amr_octet_aligned); + if (rc < 0) { + LOGPENDP(endp, DRTP, LOGL_ERROR, + "Error in AMR octet-aligned <-> bandwidth-efficient mode conversion (target=%s)\n", + conn_dst->end.codec->param.amr_octet_aligned ? "octet-aligned" : "bandwidth-efficient"); + return rc; } + } else if (rtp_end->rfc5993_hr_convert && + strcmp(conn_src->end.codec->subtype_name, "GSM-HR-08") == 0) { + rc = rfc5993_hr_convert(endp, msg); + if (rc < 0) { + LOGPENDP(endp, DRTP, LOGL_ERROR, "Error while converting to GSM-HR-08\n"); + return rc; + } + } - LOGPENDP(endp, DRTP, LOGL_DEBUG, - "process/send to %s %s " - "rtp_port:%u rtcp_port:%u\n", - dest_name, - osmo_sockaddr_ntop(&rtp_end->addr.u.sa, ipbuf), - osmo_sockaddr_port(&rtp_end->addr.u.sa), ntohs(rtp_end->rtcp_port) - ); + LOGPENDP(endp, DRTP, LOGL_DEBUG, + "process/send to %s %s " + "rtp_port:%u rtcp_port:%u\n", + dest_name, + osmo_sockaddr_ntop(&rtp_end->addr.u.sa, ipbuf), + osmo_sockaddr_port(&rtp_end->addr.u.sa), ntohs(rtp_end->rtcp_port) + ); - /* Forward a copy of the RTP data to a debug ip/port */ - forward_data_tap(rtp_end->rtp.fd, &conn_src->tap_out, - msg); + /* Forward a copy of the RTP data to a debug ip/port */ + forward_data_tap(rtp_end->rtp.fd, &conn_src->tap_out, + msg); - len = mgcp_udp_send(rtp_end->rtp.fd, &rtp_end->addr, - (char *)msgb_data(msg), msgb_length(msg)); + len = mgcp_udp_send(rtp_end->rtp.fd, &rtp_end->addr, + (char *)msgb_data(msg), msgb_length(msg)); - if (len <= 0) - return len; + if (len <= 0) + return len; - rtpconn_rate_ctr_inc(conn_dst, endp, RTP_PACKETS_TX_CTR); - rtpconn_rate_ctr_add(conn_dst, endp, RTP_OCTETS_TX_CTR, len); - rtp_state->alt_rtp_tx_sequence++; + rtpconn_rate_ctr_inc(conn_dst, endp, RTP_PACKETS_TX_CTR); + rtpconn_rate_ctr_add(conn_dst, endp, RTP_OCTETS_TX_CTR, len); + rtp_state->alt_rtp_tx_sequence++; - nbytes += len; - buflen = cont; - } while (buflen > 0); - return nbytes; + return len; } else if (!trunk->omit_rtcp) { struct osmo_sockaddr rtcp_addr = rtp_end->addr; osmo_sockaddr_set_port(&rtcp_addr.u.sa, rtp_end->rtcp_port); -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/36356?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ie1a629fd31c5ab806fc929d1e6b279c4be5b8246 Gerrit-Change-Number: 36356 Gerrit-PatchSet: 1 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-MessageType: newchange

1 year, 5 months

1
0
0 0

[S] Change in osmo-mgw[master]: don't log useless "transcoding disabled" message

by laforge

laforge has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/36358?usp=email ) Change subject: don't log useless "transcoding disabled" message ...................................................................... don't log useless "transcoding disabled" message The entire mgw has no transcoding support. So printing that message is useless to begin with. And printing it for *every RTP packet* is even more useless. Let's remove it. Change-Id: If0ee2607404afc3a00665a5cf22a9e0eb62eb476 --- M src/libosmo-mgcp/mgcp_network.c 1 file changed, 13 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/58/36358/1 diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c index 4e5b307..ee25f74 100644 --- a/src/libosmo-mgcp/mgcp_network.c +++ b/src/libosmo-mgcp/mgcp_network.c @@ -410,7 +410,6 @@ struct mgcp_rtp_end *dst_end, struct msgb *msg) { - LOGPENDP(endp, DRTP, LOGL_DEBUG, "transcoding disabled\n"); return 0; } -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/36358?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: If0ee2607404afc3a00665a5cf22a9e0eb62eb476 Gerrit-Change-Number: 36358 Gerrit-PatchSet: 1 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-MessageType: newchange

1 year, 5 months

1
0
0 0

[S] Change in osmo-mgw[master]: mgw: Add our usual SIGABRT, SIGUSR1 signal handlers

by laforge

Attention is currently required from: pespin. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/36355?usp=email ) Change subject: mgw: Add our usual SIGABRT, SIGUSR1 signal handlers ...................................................................... Patch Set 1: (1 comment) File src/osmo-mgw/mgw_main.c: https://gerrit.osmocom.org/c/osmo-mgw/+/36355/comment/dad0e26a_c7779208 PS1, Line 358: signal(SIGUSR2, &signal_handler); > SIGUSR2 is not used, so probably leave the default one? yes, likely a copy+paste oversight from osmo-bsc, which does it the same way. thanks! -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/36355?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Idc35444d2b8a0bc52c267b468dfa3c1b59f9187a Gerrit-Change-Number: 36355 Gerrit-PatchSet: 1 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 19 Mar 2024 17:17:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 year, 5 months

1
0
0 0

2025

2024

2023

2022

gerrit-log March 2024