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

[L] Change in osmo-mgw[master]: mgw: CRCX: Split mgcp header pars parsing into a previous step

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39224?usp=email ) Change subject: mgw: CRCX: Split mgcp header pars parsing into a previous step ...................................................................... mgw: CRCX: Split mgcp header pars parsing into a previous step Do most of the initial parsing and verification in a prior step, filling in a "parsed" struct which simplifies logic coming after for different message types. This commit only modifies stuff enough to work for CRCX. Further work (commits) will follow for MDCX and DLCX. Change-Id: I3ee5158c254213203830fe9c38de11c15b4b19c1 --- M include/osmocom/mgcp/mgcp_msg.h M include/osmocom/mgcp/mgcp_protocol.h M src/libosmo-mgcp/mgcp_msg.c M src/libosmo-mgcp/mgcp_protocol.c 4 files changed, 168 insertions(+), 107 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/24/39224/1 diff --git a/include/osmocom/mgcp/mgcp_msg.h b/include/osmocom/mgcp/mgcp_msg.h index d11fb85..d56a36c 100644 --- a/include/osmocom/mgcp/mgcp_msg.h +++ b/include/osmocom/mgcp/mgcp_msg.h @@ -26,6 +26,7 @@ #include <stdint.h> #include <stdbool.h> +#include <ctype.h> #include <osmocom/mgcp/mgcp_common.h> @@ -39,10 +40,11 @@ enum mgcp_connection_mode mgcp_parse_conn_mode(const char *msg); int mgcp_parse_header(struct mgcp_parse_data *pdata, char *data); +int mgcp_parse_hdr_pars(struct mgcp_parse_data *pdata); int mgcp_parse_osmux_cid(const char *line); -bool mgcp_check_param(const struct mgcp_endpoint *endp, struct mgcp_trunk *trunk, const char *line); +bool mgcp_check_param(const char *line); int mgcp_verify_call_id(struct mgcp_endpoint *endp, const char *callid); diff --git a/include/osmocom/mgcp/mgcp_protocol.h b/include/osmocom/mgcp/mgcp_protocol.h index 77f17d5..dd6e17c 100644 --- a/include/osmocom/mgcp/mgcp_protocol.h +++ b/include/osmocom/mgcp/mgcp_protocol.h @@ -3,12 +3,43 @@ #include <osmocom/core/utils.h> #include <osmocom/mgcp/mgcp_common.h> +#define MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET (-2) +#define MGCP_PARSE_HDR_PARS_OSMUX_CID_WILDCARD (-1) + +struct mgcp_parse_hdr_pars { + const char *local_options; + const char *callid; + enum mgcp_connection_mode mode; + int remote_osmux_cid; + bool have_sdp; + /*! MGCP_X_OSMO_IGN_* flags from 'X-Osmo-IGN:' header */ + uint32_t x_osmo_ign; +}; + +static inline void mgcp_parse_hdr_pars_init(struct mgcp_parse_hdr_pars *hpars) +{ + *hpars = (struct mgcp_parse_hdr_pars){ + .local_options = NULL, + .callid = NULL, + .mode = MGCP_CONN_NONE, + .remote_osmux_cid = MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET, + .have_sdp = false, + .x_osmo_ign = 0, + }; +} + +//struct mgcp_parsed_body { +//}; + /* Internal structure while parsing a request */ struct mgcp_parse_data { struct mgcp_config *cfg; + char *save; + /* MGCP Header: */ char *epname; char *trans; - char *save; + /* MGCP Body: */ + struct mgcp_parse_hdr_pars hpars; }; /* Local connection options */ diff --git a/src/libosmo-mgcp/mgcp_msg.c b/src/libosmo-mgcp/mgcp_msg.c index f8ef50b..5d6a110 100644 --- a/src/libosmo-mgcp/mgcp_msg.c +++ b/src/libosmo-mgcp/mgcp_msg.c @@ -143,6 +143,80 @@ return 0; } +static bool parse_x_osmo_ign(struct mgcp_parse_hdr_pars *hpars, char *line) +{ + char *saveptr = NULL; + + if (strncasecmp(line, MGCP_X_OSMO_IGN_HEADER, strlen(MGCP_X_OSMO_IGN_HEADER))) + return false; + line += strlen(MGCP_X_OSMO_IGN_HEADER); + + while (1) { + char *token = strtok_r(line, " ", &saveptr); + line = NULL; + if (!token) + break; + + if (!strcasecmp(token, "C")) + hpars->x_osmo_ign |= MGCP_X_OSMO_IGN_CALLID; + else + LOGP(DLMGCP, LOGL_ERROR, "received unknown X-Osmo-IGN item '%s'\n", token); + } + + return true; +} + +/*! Analyze and parse the the hader of an MGCP messeage string. + * \param[inout] pdata caller provided memory to store the parsing results. + * \returns 0 when parsing was successful, negative (MGCP cause code) on error. */ +int mgcp_parse_hdr_pars(struct mgcp_parse_data *pdata) +{ + struct mgcp_parse_hdr_pars *hp = &pdata->hpars; + char *line; + + mgcp_parse_hdr_pars_init(hp); + /* parse CallID C: and LocalParameters L: */ + for_each_line(line, pdata->save) { + if (!mgcp_check_param(line)) + continue; + + switch (toupper(line[0])) { + case 'L': + hp->local_options = (const char *)line + 3; + break; + case 'C': + hp->callid = (const char *)line + 3; + break; + case 'I': + /* It is illegal to send a connection identifier + * together with a CRCX, the MGW will assign the + * connection identifier by itself on CRCX */ + return 523; + case 'M': + hp->mode = mgcp_parse_conn_mode((const char *)line + 3); + break; + case 'X': + if (strncasecmp("Osmux: ", line + 2, strlen("Osmux: ")) == 0) { + hp->remote_osmux_cid = mgcp_parse_osmux_cid(line); + break; + } + if (parse_x_osmo_ign(hp, line)) + break; + /* Ignore unknown X-headers */ + break; + case '\0': + hp->have_sdp = true; + goto mgcp_header_done; + default: + LOGP(DLMGCP, LOGL_NOTICE, "CRCX: unhandled option: '%c'/%d\n", *line, *line); + return 539; + } + } + +mgcp_header_done: + return 0; +} + /*! Extract OSMUX CID from an MGCP parameter line (string). * \param[in] line single parameter line from the MGCP message * \returns OSMUX CID, -1 wildcard, -2 on error */ @@ -153,19 +227,19 @@ if (strcasecmp(line + 2, "Osmux: *") == 0) { LOGP(DLMGCP, LOGL_DEBUG, "Parsed wilcard Osmux CID\n"); - return -1; + return MGCP_PARSE_HDR_PARS_OSMUX_CID_WILDCARD; } if (sscanf(line + 2 + 7, "%u", &osmux_cid) != 1) { LOGP(DLMGCP, LOGL_ERROR, "Failed parsing Osmux in MGCP msg line: %s\n", line); - return -2; + return MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET; } if (osmux_cid > OSMUX_CID_MAX) { LOGP(DLMGCP, LOGL_ERROR, "Osmux ID too large: %u > %u\n", osmux_cid, OSMUX_CID_MAX); - return -2; + return MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET; } LOGP(DLMGCP, LOGL_DEBUG, "MGCP client offered Osmux CID %u\n", osmux_cid); @@ -173,18 +247,13 @@ } /*! Check MGCP parameter line (string) for plausibility. - * \param[in] endp pointer to endpoint (only used for log output, may be NULL) - * \param[in] trunk pointer to trunk (only used for log output, may be NULL if endp is not NULL) * \param[in] line single parameter line from the MGCP message * \returns true when line seems plausible, false on error */ -bool mgcp_check_param(const struct mgcp_endpoint *endp, struct mgcp_trunk *trunk, const char *line) +bool mgcp_check_param(const char *line) { const size_t line_len = strlen(line); if (line[0] != '\0' && line_len < 2) { - if (endp) - LOGPENDP(endp, DLMGCP, LOGL_NOTICE, "wrong MGCP option format: '%s'\n", line); - else - LOGPTRUNK(trunk, DLMGCP, LOGL_NOTICE, "wrong MGCP option format: '%s'\n", line); + LOGP(DLMGCP, LOGL_NOTICE, "wrong MGCP option format: '%s'\n", line); return false; } diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index 32fbd43..d592cc9 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -702,6 +702,20 @@ codec->frame_duration_den; } +static int mgcp_trunk_osmux_init_if_needed(struct mgcp_trunk *trunk) +{ + if (trunk->cfg->osmux.initialized) + return 0; + + if (osmux_init(trunk) < 0) { + LOGPTRUNK(trunk, DOSMUX, LOGL_ERROR, "Cannot init OSMUX\n"); + return -3; + } + LOGPTRUNK(trunk, DOSMUX, LOGL_NOTICE, "OSMUX socket has been set up\n"); + + return 0; +} + /*! Initializes osmux socket if not yet initialized. Parses Osmux CID from MGCP line. * \param[in] endp Endpoint willing to initialize osmux * \param[in] line Line X-Osmux from MGCP header msg to parse @@ -709,13 +723,8 @@ */ static int mgcp_osmux_setup(struct mgcp_endpoint *endp, const char *line) { - if (!endp->trunk->cfg->osmux.initialized) { - if (osmux_init(endp->trunk) < 0) { - LOGPENDP(endp, DOSMUX, LOGL_ERROR, "Cannot init OSMUX\n"); - return -3; - } - LOGPENDP(endp, DOSMUX, LOGL_NOTICE, "OSMUX socket has been set up\n"); - } + if (mgcp_trunk_osmux_init_if_needed(endp->trunk) < 0) + return -3; return mgcp_parse_osmux_cid(line); } @@ -791,42 +800,15 @@ return 534; } -static bool parse_x_osmo_ign(struct mgcp_endpoint *endp, char *line) -{ - char *saveptr = NULL; - - if (strncasecmp(line, MGCP_X_OSMO_IGN_HEADER, strlen(MGCP_X_OSMO_IGN_HEADER))) - return false; - line += strlen(MGCP_X_OSMO_IGN_HEADER); - - while (1) { - char *token = strtok_r(line, " ", &saveptr); - line = NULL; - if (!token) - break; - - if (!strcasecmp(token, "C")) - endp->x_osmo_ign |= MGCP_X_OSMO_IGN_CALLID; - else - LOGPENDP(endp, DLMGCP, LOGL_ERROR, "received unknown X-Osmo-IGN item '%s'\n", token); - } - - return true; -} - /* CRCX command handler, processes the received command */ static struct msgb *handle_create_con(struct mgcp_request_data *rq) { struct mgcp_parse_data *pdata = rq->pdata; struct mgcp_trunk *trunk = rq->trunk; struct mgcp_endpoint *endp = rq->endp; + struct mgcp_parse_hdr_pars *hpars = &pdata->hpars; struct rate_ctr_group *rate_ctrs; int error_code = 400; - const char *local_options = NULL; - const char *callid = NULL; - enum mgcp_connection_mode mode = MGCP_CONN_NONE; - char *line; - int have_sdp = 0, remote_osmux_cid = -2; struct mgcp_conn *conn = NULL; struct mgcp_conn_rtp *conn_rtp = NULL; char conn_name[512]; @@ -857,69 +839,43 @@ } /* parse CallID C: and LocalParameters L: */ - for_each_line(line, pdata->save) { - if (!mgcp_check_param(endp, trunk, line)) - continue; - - switch (toupper(line[0])) { - case 'L': - local_options = (const char *)line + 3; - break; - case 'C': - callid = (const char *)line + 3; - break; - case 'I': - /* It is illegal to send a connection identifier - * together with a CRCX, the MGW will assign the - * connection identifier by itself on CRCX */ - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_BAD_ACTION)); - return create_err_response(rq->trunk, NULL, 523, "CRCX", pdata->trans); - break; - case 'M': - mode = mgcp_parse_conn_mode((const char *)line + 3); - break; - case 'X': - if (strncasecmp("Osmux: ", line + 2, strlen("Osmux: ")) == 0) { - /* If osmux is disabled, just skip setting it up */ - if (rq->endp->trunk->cfg->osmux.usage == OSMUX_USAGE_OFF) - break; - remote_osmux_cid = mgcp_osmux_setup(endp, line); - break; - } - - if (parse_x_osmo_ign(endp, line)) - break; - - /* Ignore unknown X-headers */ - break; - case '\0': - have_sdp = 1; - goto mgcp_header_done; - default: - LOGPENDP(endp, DLMGCP, LOGL_NOTICE, - "CRCX: unhandled option: '%c'/%d\n", *line, *line); - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_UNHANDLED_PARAM)); - return create_err_response(rq->trunk, NULL, 539, "CRCX", pdata->trans); - break; - } + rc = mgcp_parse_hdr_pars(pdata); + switch (rc) { + case 0: + break; /* all good, continue below */ + case 523: + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_BAD_ACTION)); + return create_err_response(rq->trunk, NULL, rc, "CRCX", pdata->trans); + case 539: + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_UNHANDLED_PARAM)); + return create_err_response(rq->trunk, NULL, 539, "CRCX", pdata->trans); + default: + return create_err_response(rq->trunk, NULL, rc, "CRCX", pdata->trans); } -mgcp_header_done: /* Check parameters */ - if (!callid) { + if (!hpars->callid) { LOGPENDP(endp, DLMGCP, LOGL_ERROR, "CRCX: insufficient parameters, missing callid\n"); rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_MISSING_CALLID)); return create_err_response(endp, endp, 516, "CRCX", pdata->trans); } - if (mode == MGCP_CONN_NONE) { + if (hpars->mode == MGCP_CONN_NONE) { LOGPENDP(endp, DLMGCP, LOGL_ERROR, "CRCX: insufficient parameters, invalid mode\n"); rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_INVALID_MODE)); return create_err_response(endp, endp, 517, "CRCX", pdata->trans); } + /* If osmux is disabled, just skip setting it up */ + if (trunk->cfg->osmux.usage == OSMUX_USAGE_OFF) + hpars->remote_osmux_cid = MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET; + + /* Make sure osmux is setup: */ + if (hpars->remote_osmux_cid != MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET) + mgcp_trunk_osmux_init_if_needed(trunk); + /* Check if we are able to accept the creation of another connection */ if (mgcp_endp_is_full(endp)) { LOGPENDP(endp, DLMGCP, LOGL_ERROR, @@ -939,9 +895,12 @@ } } + /* Update endp->x_osmo_ign: */ + endp->x_osmo_ign |= hpars->x_osmo_ign; + /* Check if this endpoint already serves a call, if so, check if the * callids match up so that we are sure that this is our call */ - if (endp->callid && mgcp_verify_call_id(endp, callid)) { + if (endp->callid && mgcp_verify_call_id(endp, hpars->callid)) { LOGPENDP(endp, DLMGCP, LOGL_ERROR, "CRCX: already seized by other call (%s)\n", endp->callid); @@ -961,14 +920,14 @@ /* Claim endpoint resources. This will also set the callid, * creating additional connections will only be possible if * the callid matches up (see above). */ - rc = mgcp_endp_claim(endp, callid); + rc = mgcp_endp_claim(endp, hpars->callid); if (rc != 0) { rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_CLAIM)); return create_err_response(endp, endp, 502, "CRCX", pdata->trans); } } - snprintf(conn_name, sizeof(conn_name), "%s", callid); + snprintf(conn_name, sizeof(conn_name), "%s", hpars->callid); conn = mgcp_conn_alloc(trunk->endpoints, endp, MGCP_CONN_TYPE_RTP, conn_name); if (!conn) { LOGPENDP(endp, DLMGCP, LOGL_ERROR, @@ -977,7 +936,7 @@ goto error2; } - if (mgcp_conn_set_mode(conn, mode) < 0) { + if (mgcp_conn_set_mode(conn, hpars->mode) < 0) { error_code = 517; rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_INVALID_MODE)); goto error2; @@ -987,14 +946,14 @@ OSMO_ASSERT(conn_rtp); /* If X-Osmux (remote CID) was received (-1 is wilcard), alloc next avail CID as local CID */ - if (remote_osmux_cid >= -1) { + if (hpars->remote_osmux_cid >= -1) { if (osmux_init_conn(conn_rtp) < 0) { rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_NO_OSMUX)); goto error2; } - if (remote_osmux_cid >= 0) { + if (hpars->remote_osmux_cid >= 0) { conn_rtp->osmux.remote_cid_present = true; - conn_rtp->osmux.remote_cid = remote_osmux_cid; + conn_rtp->osmux.remote_cid = hpars->remote_osmux_cid; } } else if (endp->trunk->cfg->osmux.usage == OSMUX_USAGE_ONLY) { LOGPCONN(conn, DLMGCP, LOGL_ERROR, @@ -1004,9 +963,9 @@ } /* Set local connection options, if present */ - if (local_options) { + if (hpars->local_options) { rc = set_local_cx_options(trunk->endpoints, - &endp->local_options, local_options); + &endp->local_options, hpars->local_options); if (rc != 0) { LOGPCONN(conn, DLMGCP, LOGL_ERROR, "CRCX: invalid local connection options!\n"); @@ -1017,7 +976,7 @@ } /* Handle codec information and decide for a suitable codec */ - rc = handle_codec_info(conn_rtp, rq, have_sdp, true); + rc = handle_codec_info(conn_rtp, rq, hpars->have_sdp, true); mgcp_codecset_summary(&conn_rtp->end.cset, mgcp_conn_dump(conn)); if (rc) { error_code = rc; @@ -1131,7 +1090,7 @@ } for_each_line(line, pdata->save) { - if (!mgcp_check_param(endp, trunk, line)) + if (!mgcp_check_param(line)) continue; switch (toupper(line[0])) { @@ -1357,7 +1316,7 @@ } for_each_line(line, pdata->save) { - if (!mgcp_check_param(endp, trunk, line)) + if (!mgcp_check_param(line)) continue; switch (toupper(line[0])) { -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39224?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I3ee5158c254213203830fe9c38de11c15b4b19c1 Gerrit-Change-Number: 39224 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 month, 2 weeks

1
0
0 0

[M] Change in osmo-mgw[master]: Rename and cleanup code allocating rtp/rtcp ports in trunk

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39223?usp=email ) Change subject: Rename and cleanup code allocating rtp/rtcp ports in trunk ...................................................................... Rename and cleanup code allocating rtp/rtcp ports in trunk Clean up pointers passed. Rename function since it's clearly operating on trunk related fields through mutexes. Change-Id: Ib894afcb61609c247883d5ccdd7b8fbf29b2cbf8 --- M include/osmocom/mgcp/mgcp_trunk.h M src/libosmo-mgcp/mgcp_protocol.c M src/libosmo-mgcp/mgcp_trunk.c 3 files changed, 45 insertions(+), 43 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/23/39223/1 diff --git a/include/osmocom/mgcp/mgcp_trunk.h b/include/osmocom/mgcp/mgcp_trunk.h index 68d37dd..8d87954 100644 --- a/include/osmocom/mgcp/mgcp_trunk.h +++ b/include/osmocom/mgcp/mgcp_trunk.h @@ -2,9 +2,10 @@ #include <osmocom/gsm/i460_mux.h> #include <osmocom/abis/e1_input.h> +#include <osmocom/mgcp/mgcp_conn.h> +#include <osmocom/mgcp/mgcp_network.h> #include <osmocom/mgcp/mgcp_ratectr.h> - #define LOGPTRUNK(trunk, cat, level, fmt, args...) \ LOGP(cat, level, "trunk:%u " fmt, \ trunk ? trunk->trunk_nr : 0, \ @@ -78,6 +79,7 @@ struct mgcp_trunk *mgcp_trunk_by_name(const struct mgcp_config *cfg, const char *epname); int e1_trunk_nr_from_epname(unsigned int *trunk_nr, const char *epname); struct mgcp_trunk *mgcp_trunk_by_line_num(const struct mgcp_config *cfg, unsigned int num); +int mgcp_trunk_allocate_conn_rtp_ports(struct mgcp_trunk *trunk, struct mgcp_conn_rtp *conn); /* The virtual trunk is always created on trunk id 0 for historical reasons, * use this define constant as ID when allocating a virtual trunk. Other diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index 3efa0e0..32fbd43 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -489,46 +489,6 @@ return create_ok_response(rq->trunk, rq->endp, 200, "AUEP", rq->pdata->trans); } -/* Try to find a free port by attempting to bind on it. Also handle the - * counter that points on the next free port. Since we have a pointer - * to the next free port, binding should in work on the first attempt in - * general. In case of failure the next port is tried until the whole port - * range is tried once. */ -static int allocate_port(struct mgcp_endpoint *endp, struct mgcp_conn_rtp *conn) -{ - int i; - struct mgcp_port_range *range; - unsigned int tries; - - OSMO_ASSERT(conn); - - range = &endp->trunk->cfg->net_ports; - - pthread_mutex_lock(&range->lock); - /* attempt to find a port */ - tries = (range->range_end - range->range_start) / 2; - for (i = 0; i < tries; ++i) { - int rc; - - if (range->last_port >= range->range_end) - range->last_port = range->range_start; - - rc = mgcp_conn_rtp_bind_rtp_ports(conn, range->last_port); - - range->last_port += 2; - if (rc == 0) { - pthread_mutex_unlock(&range->lock); - return 0; - } - - } - pthread_mutex_unlock(&range->lock); - LOGPENDP(endp, DLMGCP, LOGL_ERROR, - "Allocating a RTP/RTCP port failed %u times.\n", - tries); - return -1; -} - /*! Helper function for check_local_cx_options() to get a pointer of the next * lco option identifier * \param[in] lco string @@ -1077,7 +1037,7 @@ rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_BIND_PORT)); goto error2; } - if (allocate_port(endp, conn_rtp) != 0) { + if (mgcp_trunk_allocate_conn_rtp_ports(trunk, conn_rtp) != 0) { rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_BIND_PORT)); goto error2; } @@ -1311,7 +1271,7 @@ if (strcmp(new_local_addr, conn_rtp->end.local_addr)) { osmo_strlcpy(conn_rtp->end.local_addr, new_local_addr, sizeof(conn_rtp->end.local_addr)); mgcp_rtp_end_free_port(&conn_rtp->end); - if (allocate_port(endp, conn_rtp) != 0) { + if (mgcp_trunk_allocate_conn_rtp_ports(trunk, conn_rtp) != 0) { rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_BIND_PORT)); goto error3; } diff --git a/src/libosmo-mgcp/mgcp_trunk.c b/src/libosmo-mgcp/mgcp_trunk.c index 69750f8..8d258e2 100644 --- a/src/libosmo-mgcp/mgcp_trunk.c +++ b/src/libosmo-mgcp/mgcp_trunk.c @@ -306,3 +306,43 @@ return NULL; } + +/* Try to find a free port by attempting to bind on it. Also handle the + * counter that points on the next free port. Since we have a pointer + * to the next free port, binding should in work on the first attempt in + * general. In case of failure the next port is tried until the whole port + * range is tried once. */ +int mgcp_trunk_allocate_conn_rtp_ports(struct mgcp_trunk *trunk, struct mgcp_conn_rtp *conn) +{ + int i; + struct mgcp_port_range *range; + unsigned int tries; + + OSMO_ASSERT(trunk); + OSMO_ASSERT(conn); + + range = &trunk->cfg->net_ports; + + pthread_mutex_lock(&range->lock); + /* attempt to find a port */ + tries = (range->range_end - range->range_start) / 2; + for (i = 0; i < tries; ++i) { + int rc; + + if (range->last_port >= range->range_end) + range->last_port = range->range_start; + + rc = mgcp_conn_rtp_bind_rtp_ports(conn, range->last_port); + + range->last_port += 2; + if (rc == 0) { + pthread_mutex_unlock(&range->lock); + return 0; + } + + } + pthread_mutex_unlock(&range->lock); + LOGPCONN(conn->conn, DLMGCP, LOGL_ERROR, + "Allocating a RTP/RTCP port failed %u times.\n", tries); + return -1; +} -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39223?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ib894afcb61609c247883d5ccdd7b8fbf29b2cbf8 Gerrit-Change-Number: 39223 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 month, 2 weeks

1
0
0 0

[M] Change in osmo-mgw[master]: mgw: Clean up code allocating conn_rtp rtp/rtcp sockets

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39222?usp=email ) Change subject: mgw: Clean up code allocating conn_rtp rtp/rtcp sockets ...................................................................... mgw: Clean up code allocating conn_rtp rtp/rtcp sockets Clean up pointers, join 2 functions only making stuff more confusing. Rename function to make it clear it operates on a conn_rtp object. Change-Id: I50a251b66e85ceeeccb30dcd1813863d7c754961 --- M include/osmocom/mgcp/mgcp_network.h M src/libosmo-mgcp/mgcp_network.c M src/libosmo-mgcp/mgcp_protocol.c 3 files changed, 56 insertions(+), 75 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/22/39222/1 diff --git a/include/osmocom/mgcp/mgcp_network.h b/include/osmocom/mgcp/mgcp_network.h index fe35e78..8a1fc47 100644 --- a/include/osmocom/mgcp/mgcp_network.h +++ b/include/osmocom/mgcp/mgcp_network.h @@ -91,8 +91,7 @@ void mgcp_cleanup_rtp_bridge_cb(struct mgcp_endpoint *endp, struct mgcp_conn *conn); int mgcp_dispatch_e1_bridge_cb(struct msgb *msg); void mgcp_cleanup_e1_bridge_cb(struct mgcp_endpoint *endp, struct mgcp_conn *conn); -int mgcp_bind_net_rtp_port(struct mgcp_endpoint *endp, int rtp_port, - struct mgcp_conn_rtp *conn); +int mgcp_conn_rtp_bind_rtp_ports(struct mgcp_conn_rtp *conn, int rtp_port); void mgcp_patch_and_count(const struct mgcp_endpoint *endp, struct mgcp_rtp_state *state, struct mgcp_rtp_end *rtp_end, diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c index 894ca2a..cd988dc 100644 --- a/src/libosmo-mgcp/mgcp_network.c +++ b/src/libosmo-mgcp/mgcp_network.c @@ -1625,78 +1625,23 @@ 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, rtp_fd, rtcp_fd; - - /* NOTE: The port that is used for RTCP is the RTP port incremented by one - * (e.g. RTP-Port = 16000 ==> RTCP-Port = 16001) */ - - 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_fd = rc; - - 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; - } - rtcp_fd = rc; - - if (osmo_iofd_register(rtp_end->rtp, rtp_fd) < 0) { - LOGPENDP(endp, DRTP, LOGL_ERROR, - "failed to register RTP port %d\n", - rtp_end->local_port); - goto cleanup2; - } - - if (osmo_iofd_register(rtp_end->rtcp, rtcp_fd) != 0) { - LOGPENDP(endp, DRTP, LOGL_ERROR, - "failed to register RTCP port %d\n", - rtp_end->local_port + 1); - goto cleanup3; - } - - return 0; - -cleanup3: - osmo_iofd_unregister(rtp_end->rtp); -cleanup2: - close(rtcp_fd); -cleanup1: - close(rtp_fd); -cleanup0: - return -1; -} - /*! bind RTP port to endpoint/connection. - * \param[in] endp endpoint that holds the RTP connection. - * \param[in] rtp_port port number to bind on. * \param[in] conn associated RTP connection. + * \param[in] rtp_port port number to bind on. * \returns 0 on success, -1 on ERROR. */ -int mgcp_bind_net_rtp_port(struct mgcp_endpoint *endp, int rtp_port, - struct mgcp_conn_rtp *conn) +int mgcp_conn_rtp_bind_rtp_ports(struct mgcp_conn_rtp *conn, int rtp_port) { char name[512]; - struct mgcp_rtp_end *end; + struct mgcp_conn *_conn = conn->conn; + struct mgcp_config *cfg = _conn->endp->trunk->cfg; + struct mgcp_rtp_end *end = &conn->end; + int rc, rtp_fd, rtcp_fd; - snprintf(name, sizeof(name), "%s-%s", conn->conn->name, conn->conn->id); - end = &conn->end; + snprintf(name, sizeof(name), "%s-%s", _conn->name, _conn->id); if ((end->rtp && osmo_iofd_get_fd(end->rtp) != -1) || (end->rtcp && osmo_iofd_get_fd(end->rtcp) != -1)) { - LOGPENDP(endp, DRTP, LOGL_ERROR, "%u was already bound on conn:%s\n", - rtp_port, mgcp_conn_dump(conn->conn)); - + LOG_CONN_RTP(conn, LOGL_ERROR, "%u was already bound\n", rtp_port); /* Double bindings should never occour! Since we always allocate * connections dynamically and free them when they are not * needed anymore, there must be no previous binding leftover. @@ -1706,18 +1651,55 @@ } end->local_port = rtp_port; - end->rtp = osmo_iofd_setup(conn->conn, -1, name, OSMO_IO_FD_MODE_RECVFROM_SENDTO, &rtp_ioops, conn); + end->rtp = osmo_iofd_setup(_conn, -1, name, OSMO_IO_FD_MODE_RECVFROM_SENDTO, &rtp_ioops, conn); if (!end->rtp) - return -EIO; + goto free_iofd_ret; osmo_iofd_set_alloc_info(end->rtp, RTP_BUF_SIZE, 0); - end->rtcp = osmo_iofd_setup(conn->conn, -1, name, OSMO_IO_FD_MODE_RECVFROM_SENDTO, &rtp_ioops, conn); - if (!end->rtcp) { - osmo_iofd_free(end->rtp); - end->rtp = NULL; - return -EIO; - } + end->rtcp = osmo_iofd_setup(_conn, -1, name, OSMO_IO_FD_MODE_RECVFROM_SENDTO, &rtp_ioops, conn); + if (!end->rtcp) + goto free_iofd_ret; osmo_iofd_set_alloc_info(end->rtcp, RTP_BUF_SIZE, 0); osmo_iofd_set_priv_nr(end->rtcp, 1); /* we use priv_nr as identifier for RTCP */ - return bind_rtp(endp->trunk->cfg, conn->end.local_addr, end, endp); + /* NOTE: The port that is used for RTCP is the RTP port incremented by one + * (e.g. RTP-Port = 16000 ==> RTCP-Port = 16001) */ + + rc = mgcp_create_bind(end->local_addr, end->local_port, cfg->endp_dscp, cfg->endp_priority); + if (rc < 0) { + LOG_CONN_RTP(conn, LOGL_ERROR, "failed to create RTP port: %s:%d\n", end->local_addr, end->local_port); + goto free_iofd_ret; + } + rtp_fd = rc; + + rc = mgcp_create_bind(end->local_addr, end->local_port + 1, cfg->endp_dscp, cfg->endp_priority); + if (rc < 0) { + LOG_CONN_RTP(conn, LOGL_ERROR, "failed to create RTCP port: %s:%d\n", end->local_addr, end->local_port + 1); + goto cleanup1; + } + rtcp_fd = rc; + + if (osmo_iofd_register(end->rtp, rtp_fd) < 0) { + LOG_CONN_RTP(conn, LOGL_ERROR, "failed to register RTP port %d\n", end->local_port); + goto cleanup2; + } + + if (osmo_iofd_register(end->rtcp, rtcp_fd) != 0) { + LOG_CONN_RTP(conn, LOGL_ERROR, "failed to register RTCP port %d\n", end->local_port + 1); + goto cleanup3; + } + + return 0; + +cleanup3: + osmo_iofd_unregister(end->rtp); +cleanup2: + close(rtcp_fd); +cleanup1: + close(rtp_fd); +free_iofd_ret: + osmo_iofd_free(end->rtcp); + end->rtcp = NULL; + osmo_iofd_free(end->rtp); + end->rtp = NULL; + return -EIO; } diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index bbd38da..3efa0e0 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -513,7 +513,7 @@ if (range->last_port >= range->range_end) range->last_port = range->range_start; - rc = mgcp_bind_net_rtp_port(endp, range->last_port, conn); + rc = mgcp_conn_rtp_bind_rtp_ports(conn, range->last_port); range->last_port += 2; if (rc == 0) { -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39222?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I50a251b66e85ceeeccb30dcd1813863d7c754961 Gerrit-Change-Number: 39222 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 month, 2 weeks

1
0
0 0

[M] Change in osmo-pcap[master]: pcap-server: Introduce VTY cmd '[no] completed-path'

by pespin

pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-pcap/+/39201?usp=email ) Change subject: pcap-server: Introduce VTY cmd '[no] completed-path' ...................................................................... Patch Set 3: (1 comment) File doc/manuals/chapters/server.adoc: https://gerrit.osmocom.org/c/osmo-pcap/+/39201/comment/21ca606e_0148a674?us… : PS2, Line 138: wants > users ... […] Done -- To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/39201?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-pcap Gerrit-Branch: master Gerrit-Change-Id: I5166d1c5d9eb45358c87c2e1c5fbf7969d1d5294 Gerrit-Change-Number: 39201 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Tue, 24 Dec 2024 14:45:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>

1 month, 2 weeks

1
0
0 0

[S] Change in osmo-pcap[master]: pcap-server: Resolve real path of pcap file before opening

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcap/+/39200?usp=email ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: pcap-server: Resolve real path of pcap file before opening ...................................................................... pcap-server: Resolve real path of pcap file before opening This means we always end up with a canonicalized absolute path. While at it, also valdiate that we can indeed resolve the path (eg. it exists and is reachable with current process permissions) during startup. It is preferrable to resolve it everytime a file is opened, this allows the user to eg. change the base-path to a different symlink it they wish to change the base-path for new pcaps without restarting osmo-pcap. Change-Id: I8d161010dc8b480dd4cf90e19ca28a77914a50ad --- M include/osmo-pcap/osmo_pcap_server.h M src/osmo_server_network.c M src/osmo_server_vty.c 3 files changed, 19 insertions(+), 4 deletions(-) Approvals: fixeria: Looks good to me, approved Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve diff --git a/include/osmo-pcap/osmo_pcap_server.h b/include/osmo-pcap/osmo_pcap_server.h index 0a43a69..50bb121 100644 --- a/include/osmo-pcap/osmo_pcap_server.h +++ b/include/osmo-pcap/osmo_pcap_server.h @@ -87,6 +87,7 @@ /* Remote connection */ struct osmo_wqueue rem_wq; int local_fd; + /* canonicalized absolute pathname of pcap file we write to */ char *curr_filename; /* pcap stuff */ diff --git a/src/osmo_server_network.c b/src/osmo_server_network.c index 0b0073f..8747091 100644 --- a/src/osmo_server_network.c +++ b/src/osmo_server_network.c @@ -166,6 +166,7 @@ time_t now = time(NULL); struct tm tm; int rc; + char *real_base_path; osmo_pcap_server_close_trace(conn); @@ -178,11 +179,17 @@ } localtime_r(&now, &tm); + real_base_path = realpath(conn->server->base_path, NULL); + if (!real_base_path) { + LOGP(DSERVER, LOGL_ERROR, "Failed to resolve real path '%s': %s\n", + conn->server->base_path, strerror(errno)); + return; + } conn->curr_filename = talloc_asprintf(conn, "%s/trace-%s-%d%.2d%.2d_%.2d%.2d%.2d.pcap", - conn->server->base_path, conn->name, - tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday, - tm.tm_hour, tm.tm_min, tm.tm_sec); - + real_base_path, conn->name, + tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday, + tm.tm_hour, tm.tm_min, tm.tm_sec); + free(real_base_path); if (!conn->curr_filename) { LOGP(DSERVER, LOGL_ERROR, "Failed to assemble filename for %s.\n", conn->name); return; diff --git a/src/osmo_server_vty.c b/src/osmo_server_vty.c index c3014c9..ee3f995 100644 --- a/src/osmo_server_vty.c +++ b/src/osmo_server_vty.c @@ -148,6 +148,13 @@ "base-path PATH", "Base path for log files\n" "Path\n") { + /* Validate we can resolve path: */ + char *tmp = realpath(argv[0], NULL); + if (!tmp) { + vty_out(vty, "%% Failed to resolve path '%s': %s%s", argv[0], strerror(errno), VTY_NEWLINE); + return CMD_WARNING; + } + free(tmp); osmo_talloc_replace_string(pcap_server, &pcap_server->base_path, argv[0]); return CMD_SUCCESS; } -- To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/39200?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-pcap Gerrit-Branch: master Gerrit-Change-Id: I8d161010dc8b480dd4cf90e19ca28a77914a50ad Gerrit-Change-Number: 39200 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

1 month, 2 weeks

1
0
0 0

[XS] Change in osmo-pcap[master]: pcap-server: remove unneeded check for null base_path

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcap/+/39192?usp=email ) Change subject: pcap-server: remove unneeded check for null base_path ...................................................................... pcap-server: remove unneeded check for null base_path It cannot be ever NULL. It is set to a non-null path during alloc(), and it can only be replaced with another string through VTY. Change-Id: I2f503f6f9af35d6ae6fbd03e85cade4f0eb93df2 --- M src/osmo_server_vty.c 1 file changed, 1 insertion(+), 2 deletions(-) Approvals: pespin: Looks good to me, approved fixeria: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/osmo_server_vty.c b/src/osmo_server_vty.c index 6a44da6..c3014c9 100644 --- a/src/osmo_server_vty.c +++ b/src/osmo_server_vty.c @@ -96,8 +96,7 @@ vty_out(vty, "server%s", VTY_NEWLINE); - if (pcap_server->base_path) - vty_out(vty, " base-path %s%s", pcap_server->base_path, VTY_NEWLINE); + vty_out(vty, " base-path %s%s", pcap_server->base_path, VTY_NEWLINE); vty_out(vty, " file-permission-mask 0%o%s", pcap_server->permission_mask, VTY_NEWLINE); if (pcap_server->addr) vty_out(vty, " server ip %s%s", pcap_server->addr, VTY_NEWLINE); -- To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/39192?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-pcap Gerrit-Branch: master Gerrit-Change-Id: I2f503f6f9af35d6ae6fbd03e85cade4f0eb93df2 Gerrit-Change-Number: 39192 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org>

1 month, 2 weeks

1
0
0 0

[M] Change in osmo-pcap[master]: pcap-server: Introduce VTY cmd '[no] completed-path'

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcap/+/39201?usp=email ) Change subject: pcap-server: Introduce VTY cmd '[no] completed-path' ...................................................................... pcap-server: Introduce VTY cmd '[no] completed-path' This VTY allows enabling feature to instruct osmo-pcap-server to move the closed files (due to ration) into a separate directory. This is useful for users who want to monitor and act on closed pcap files to eg. generate statistics, etc. Important: completed-path must be in the same filesystem mountpoint as base-path, since osmo-pcap-server is actually not copying the files, but atomically renaming the paths through rename() syscall. Related: SYS#7248 Change-Id: I5166d1c5d9eb45358c87c2e1c5fbf7969d1d5294 --- M doc/manuals/chapters/server.adoc M include/osmo-pcap/osmo_pcap_server.h M src/osmo_server_network.c M src/osmo_server_vty.c 4 files changed, 141 insertions(+), 0 deletions(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, approved diff --git a/doc/manuals/chapters/server.adoc b/doc/manuals/chapters/server.adoc index c6404ab..e0d9ff4 100644 --- a/doc/manuals/chapters/server.adoc +++ b/doc/manuals/chapters/server.adoc @@ -125,3 +125,31 @@ Saving procedure), osmo-pcap-server may end up recreating (and truncating) a previous pcap file if it is generated with the same localtime timestamp, for instance because connection from osmo-pcap-client was re-established. + +=== Completed pcap directory + +`osmo-pcap-server` creates and writes data to opened pcap files in the +`base-path` directory configured through VTY. Furthermore, if user supplies a +`completed-path` directory through the VTY, `osmo-pcap-server` will move the +file from `base-path` to `completed-path` directory once it closes the current +file due to rotation or because `osmo-pcap-client` became disconnected. The file +name is kept when moving from one directory to the other. + +This feature is useful for users willing to have an external tool to monitor a +directory for new closed pcap files and then acting on them, eg. to gather +statistics of packets. + +.Example: Move files from /tmp/recording to /tmp/done +---- +server + ... + base-path /tmp/recording <1> + completed-path /tmp/done <2> +---- +<1> Files are opened and recorded under /tmp/recording +<2> Once closed files are moved under /tmp/done + +NOTE:: `osmo-pcap-server` uses a rename() call to move the file atomically. +However, this has the drawback that both `base-path` and `completed-path` must +be placed in the same filesystem mounting point. Using directories in different +filesystem mount points will fail. diff --git a/include/osmo-pcap/osmo_pcap_server.h b/include/osmo-pcap/osmo_pcap_server.h index 50bb121..614ceca 100644 --- a/include/osmo-pcap/osmo_pcap_server.h +++ b/include/osmo-pcap/osmo_pcap_server.h @@ -140,6 +140,7 @@ bool dh_params_allocated; char *base_path; + char *completed_path; mode_t permission_mask; off_t max_size; bool max_size_enabled; diff --git a/src/osmo_server_network.c b/src/osmo_server_network.c index 8747091..9750170 100644 --- a/src/osmo_server_network.c +++ b/src/osmo_server_network.c @@ -39,6 +39,9 @@ #include <errno.h> #include <string.h> #include <unistd.h> +#include <limits.h> +#include <stdlib.h> +#include <libgen.h> static void pcap_zmq_send(void *publ, const void *data, size_t len, int flags) { @@ -112,6 +115,84 @@ 0); } +/* Move pcap file from base_path to completed_path, and updates + * conn->curr_filename to point to new location. */ +void move_completed_trace_if_needed(struct osmo_pcap_conn *conn) +{ + struct osmo_pcap_server *server = conn->server; + char *curr_filename_cpy_bname = NULL; + char *curr_filename_cpy_dname = NULL; + char *bname = NULL; + char *curr_dirname = NULL; + char *new_dirname = NULL; + char *new_filename = NULL; + size_t new_filename_len; + int rc; + + if (!conn->curr_filename) + return; + + if (!server->completed_path) + return; + + /* Assumption: curr_filename is anonicalized absolute pathname. */ + + /* basename and dirname may modify input param, and return a string + * which shall not be freed, potentially pointing to the input param. */ + curr_filename_cpy_dname = talloc_strdup(conn, conn->curr_filename); + curr_filename_cpy_bname = talloc_strdup(conn, conn->curr_filename); + if (!curr_filename_cpy_dname || !curr_filename_cpy_bname) + goto ret_free1; + + curr_dirname = dirname(curr_filename_cpy_dname); + bname = basename(curr_filename_cpy_bname); + if (!curr_dirname || !bname) { + LOGP(DSERVER, LOGL_ERROR, "Failed to resolve dirname and basename for '%s'\n", + conn->curr_filename); + goto ret_free1; + } + + new_dirname = realpath(server->completed_path, NULL); + if (!new_dirname) { + LOGP(DSERVER, LOGL_ERROR, "Failed to resolve path '%s': %s\n", + server->completed_path, strerror(errno)); + goto ret_free1; + } + + new_filename_len = strlen(new_dirname) + 1 /* '/' */ + strlen(bname) + 1 /* '\0' */; + new_filename = talloc_size(conn, new_filename_len); + if (!new_filename) + goto ret_free1; + rc = snprintf(new_filename, new_filename_len, "%s/%s", new_dirname, bname); + if (rc != new_filename_len - 1) + goto ret_free2; + + LOGP(DSERVER, LOGL_INFO, "Moving completed pcap file '%s' -> '%s'\n", conn->curr_filename, new_filename); + rc = rename(conn->curr_filename, new_filename); + if (rc == -1) { + int err = errno; + LOGP(DSERVER, LOGL_ERROR, "Failed moving completed pcap file '%s' -> '%s': %s\n", + conn->curr_filename, new_filename, strerror(err)); + if (err == EXDEV) + LOGP(DSERVER, LOGL_ERROR, "Fix your config! %s and %s shall not be in different filesystems!\n", + curr_dirname, new_dirname); + goto ret_free2; + } + + /* Now replace conn->curr_filename with new path: */ + talloc_free(conn->curr_filename); + conn->curr_filename = new_filename; + /* new_filename has been assigned, so we don't want to free it, hence move to ret_free1: */ + goto ret_free1; + +ret_free2: + talloc_free(new_filename); +ret_free1: + free(new_dirname); + talloc_free(curr_filename_cpy_bname); + talloc_free(curr_filename_cpy_dname); +} + void osmo_pcap_server_close_trace(struct osmo_pcap_conn *conn) { if (conn->local_fd >= 0) { @@ -119,6 +200,8 @@ conn->local_fd = -1; } + move_completed_trace_if_needed(conn); + if (conn->curr_filename) { client_event(conn, "closingtracefile", conn->curr_filename); rate_ctr_inc2(conn->ctrg, PEER_CTR_PROTATE); diff --git a/src/osmo_server_vty.c b/src/osmo_server_vty.c index ee3f995..1e8e05c 100644 --- a/src/osmo_server_vty.c +++ b/src/osmo_server_vty.c @@ -97,6 +97,8 @@ vty_out(vty, "server%s", VTY_NEWLINE); vty_out(vty, " base-path %s%s", pcap_server->base_path, VTY_NEWLINE); + if (pcap_server->completed_path) + vty_out(vty, " completed-path %s%s", pcap_server->completed_path, VTY_NEWLINE); vty_out(vty, " file-permission-mask 0%o%s", pcap_server->permission_mask, VTY_NEWLINE); if (pcap_server->addr) vty_out(vty, " server ip %s%s", pcap_server->addr, VTY_NEWLINE); @@ -159,6 +161,31 @@ return CMD_SUCCESS; } +DEFUN(cfg_server_no_completed_path, + cfg_server_no_completed_path_cmd, + "no completed-path", + NO_STR "Base path for completed (already closed, rotated) log files. Completed files won't be moved.\n") +{ + TALLOC_FREE(pcap_server->completed_path); + return CMD_SUCCESS; +} + +DEFUN(cfg_server_completed_path, + cfg_server_completed_path_cmd, + "completed-path PATH", + "Base path for completed (already closed, rotated) log files\n" "Path\n") +{ + /* Validate we can resolve path: */ + char *tmp = realpath(argv[0], NULL); + if (!tmp) { + vty_out(vty, "%% Failed to resolve path '%s': %s%s", argv[0], strerror(errno), VTY_NEWLINE); + return CMD_WARNING; + } + free(tmp); + osmo_talloc_replace_string(pcap_server, &pcap_server->completed_path, argv[0]); + return CMD_SUCCESS; +} + DEFUN(cfg_server_file_permission_mask, cfg_server_file_permission_mask_cmd, "file-permission-mask MODE", @@ -671,6 +698,8 @@ install_node(&server_node, config_write_server); install_element(SERVER_NODE, &cfg_server_base_cmd); + install_element(SERVER_NODE, &cfg_server_no_completed_path_cmd); + install_element(SERVER_NODE, &cfg_server_completed_path_cmd); install_element(SERVER_NODE, &cfg_server_file_permission_mask_cmd); install_element(SERVER_NODE, &cfg_server_ip_cmd); install_element(SERVER_NODE, &cfg_server_port_cmd); -- To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/39201?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-pcap Gerrit-Branch: master Gerrit-Change-Id: I5166d1c5d9eb45358c87c2e1c5fbf7969d1d5294 Gerrit-Change-Number: 39201 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

1 month, 2 weeks

1
0
0 0

[XS] Change in osmo-pcap[master]: pcap-server: remove unneeded check for null base_path

by pespin

Attention is currently required from: laforge. pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-pcap/+/39192?usp=email ) Change subject: pcap-server: remove unneeded check for null base_path ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/39192?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-pcap Gerrit-Branch: master Gerrit-Change-Id: I2f503f6f9af35d6ae6fbd03e85cade4f0eb93df2 Gerrit-Change-Number: 39192 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Tue, 24 Dec 2024 14:45:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 month, 2 weeks

1
0
0 0

[M] Change in osmo-pcap[master]: pcap-server: Introduce VTY cmd '[no] completed-path'

by pespin

Attention is currently required from: fixeria, laforge. pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-pcap/+/39201?usp=email ) Change subject: pcap-server: Introduce VTY cmd '[no] completed-path' ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/39201?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-pcap Gerrit-Branch: master Gerrit-Change-Id: I5166d1c5d9eb45358c87c2e1c5fbf7969d1d5294 Gerrit-Change-Number: 39201 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Tue, 24 Dec 2024 14:44:37 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 month, 2 weeks

1
0
0 0

[S] Change in osmo-pcap[master]: pcap-client,pcap-server: Use rate_ctr_inc2 and rate_ctr_add2 APIs

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcap/+/39174?usp=email ) ( 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: pcap-client,pcap-server: Use rate_ctr_inc2 and rate_ctr_add2 APIs ...................................................................... pcap-client,pcap-server: Use rate_ctr_inc2 and rate_ctr_add2 APIs Change-Id: Idcc20646d252ad7bf61a740d900b31ca37ada280 --- M src/osmo_client_core.c M src/osmo_client_network.c M src/osmo_server_network.c 3 files changed, 19 insertions(+), 19 deletions(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve fixeria: Looks good to me, approved diff --git a/src/osmo_client_core.c b/src/osmo_client_core.c index d111fca..7338622 100644 --- a/src/osmo_client_core.c +++ b/src/osmo_client_core.c @@ -163,7 +163,7 @@ data = pcap_next(ph->handle, &hdr); if (!data) { - rate_ctr_inc(rate_ctr_group_get_ctr(client->ctrg, CLIENT_CTR_PERR)); + rate_ctr_inc2(client->ctrg, CLIENT_CTR_PERR); return -1; } @@ -205,7 +205,7 @@ uint64_t inc; inc = get_psbl_wrapped_ctr(*old_val, new_val); - rate_ctr_add(rate_ctr_group_get_ctr(client->ctrg, ctr), inc); + rate_ctr_add2(client->ctrg, ctr, inc); *old_val = new_val; } @@ -223,7 +223,7 @@ rc = pcap_stats(ph->handle, &stat); if (rc != 0) { LOGPH(ph, LOGL_ERROR, "Failed to query pcap stats: %s\n", pcap_geterr(ph->handle)); - rate_ctr_inc(rate_ctr_group_get_ctr(client->ctrg, CLIENT_CTR_PERR)); + rate_ctr_inc2(client->ctrg, CLIENT_CTR_PERR); return; } diff --git a/src/osmo_client_network.c b/src/osmo_client_network.c index bc99087..70fe740 100644 --- a/src/osmo_client_network.c +++ b/src/osmo_client_network.c @@ -61,7 +61,7 @@ if (osmo_wqueue_enqueue_quiet(&conn->wqueue, msg) != 0) { LOGCONN(conn, LOGL_ERROR, "Failed to enqueue msg (capacity: %u/%u)\n", conn->wqueue.current_length, conn->wqueue.max_length); - rate_ctr_inc(rate_ctr_group_get_ctr(conn->client->ctrg, CLIENT_CTR_QERR)); + rate_ctr_inc2(conn->client->ctrg, CLIENT_CTR_QERR); msgb_free(msg); return; } @@ -91,7 +91,7 @@ if (rc < 0) { struct osmo_pcap_client_conn *conn = fd->data; LOGCONN(conn, LOGL_ERROR, "Lost connection on write\n"); - rate_ctr_inc(rate_ctr_group_get_ctr(conn->client->ctrg, CLIENT_CTR_WERR)); + rate_ctr_inc2(conn->client->ctrg, CLIENT_CTR_WERR); lost_connection(conn); return -1; } @@ -172,13 +172,13 @@ if (in_hdr->len > in_hdr->caplen) { LOGCONN(conn, LOGL_ERROR, "Recording truncated packet, len %zu > snaplen %zu\n", (size_t) in_hdr->len, (size_t) in_hdr->caplen); - rate_ctr_inc(rate_ctr_group_get_ctr(conn->client->ctrg, CLIENT_CTR_2BIG)); + rate_ctr_inc2(conn->client->ctrg, CLIENT_CTR_2BIG); } msg = msgb_alloc(in_hdr->caplen + sizeof(*om_hdr) + sizeof(*hdr), "data-data"); if (!msg) { LOGCONN(conn, LOGL_ERROR, "Failed to allocate\n"); - rate_ctr_inc(rate_ctr_group_get_ctr(conn->client->ctrg, CLIENT_CTR_NOMEM)); + rate_ctr_inc2(conn->client->ctrg, CLIENT_CTR_NOMEM); return; } @@ -198,8 +198,8 @@ memcpy(msg->l3h, data, in_hdr->caplen); om_hdr->len = htons(msgb_l2len(msg)); - rate_ctr_add(rate_ctr_group_get_ctr(conn->client->ctrg, CLIENT_CTR_BYTES), hdr->caplen); - rate_ctr_inc(rate_ctr_group_get_ctr(conn->client->ctrg, CLIENT_CTR_PKTS)); + rate_ctr_add2(conn->client->ctrg, CLIENT_CTR_BYTES, hdr->caplen); + rate_ctr_inc2(conn->client->ctrg, CLIENT_CTR_PKTS); break; case PROTOCOL_IPIP: /* TODO: support capturing from multiple interfaces here: */ @@ -310,7 +310,7 @@ osmo_fd_setup(&conn->wqueue.bfd, rc, when, conn_cb, conn, 0); osmo_fd_register(&conn->wqueue.bfd); - rate_ctr_inc(rate_ctr_group_get_ctr(conn->client->ctrg, CLIENT_CTR_CONNECT)); + rate_ctr_inc2(conn->client->ctrg, CLIENT_CTR_CONNECT); } void osmo_client_conn_reconnect(struct osmo_pcap_client_conn *conn) diff --git a/src/osmo_server_network.c b/src/osmo_server_network.c index 1c8d442..0b0073f 100644 --- a/src/osmo_server_network.c +++ b/src/osmo_server_network.c @@ -121,8 +121,8 @@ if (conn->curr_filename) { client_event(conn, "closingtracefile", conn->curr_filename); - rate_ctr_inc(rate_ctr_group_get_ctr(conn->ctrg, PEER_CTR_PROTATE)); - rate_ctr_inc(rate_ctr_group_get_ctr(conn->server->ctrg, SERVER_CTR_PROTATE)); + rate_ctr_inc2(conn->ctrg, PEER_CTR_PROTATE); + rate_ctr_inc2(conn->server->ctrg, SERVER_CTR_PROTATE); talloc_free(conn->curr_filename); conn->curr_filename = NULL; } @@ -568,12 +568,12 @@ conn->pend = sizeof(*conn->data); /* count the full packet we got */ - rate_ctr_inc(rate_ctr_group_get_ctr(conn->ctrg, PEER_CTR_PKTS)); - rate_ctr_inc(rate_ctr_group_get_ctr(conn->server->ctrg, SERVER_CTR_PKTS)); + rate_ctr_inc2(conn->ctrg, PEER_CTR_PKTS); + rate_ctr_inc2(conn->server->ctrg, SERVER_CTR_PKTS); /* count the bytes of it */ - rate_ctr_add(rate_ctr_group_get_ctr(conn->ctrg, PEER_CTR_BYTES), conn->data->len); - rate_ctr_add(rate_ctr_group_get_ctr(conn->server->ctrg, SERVER_CTR_BYTES), conn->data->len); + rate_ctr_add2(conn->ctrg, PEER_CTR_BYTES, conn->data->len); + rate_ctr_add2(conn->server->ctrg, SERVER_CTR_BYTES, conn->data->len); switch (conn->data->type) { case PKT_LINK_HDR: @@ -666,7 +666,7 @@ return; } - rate_ctr_inc(rate_ctr_group_get_ctr(client->ctrg, PEER_CTR_CONNECT)); + rate_ctr_inc2(client->ctrg, PEER_CTR_CONNECT); client->state = STATE_INITIAL; client->pend = sizeof(*client->data); @@ -711,7 +711,7 @@ server = fd->data; /* count any accept to see no clients */ - rate_ctr_inc(rate_ctr_group_get_ctr(server->ctrg, SERVER_CTR_CONNECT)); + rate_ctr_inc2(server->ctrg, SERVER_CTR_CONNECT); llist_for_each_entry(conn, &server->conn, entry) { if (conn->remote_addr.s_addr == addr.sin_addr.s_addr) { @@ -723,7 +723,7 @@ } } - rate_ctr_inc(rate_ctr_group_get_ctr(server->ctrg, SERVER_CTR_NOCLIENT)); + rate_ctr_inc2(server->ctrg, SERVER_CTR_NOCLIENT); /* * TODO: In the future start with a tls handshake and see if we know -- To view, visit https://gerrit.osmocom.org/c/osmo-pcap/+/39174?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-pcap Gerrit-Branch: master Gerrit-Change-Id: Idcc20646d252ad7bf61a740d900b31ca37ada280 Gerrit-Change-Number: 39174 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

1 month, 2 weeks

1
0
0 0

2025

2024

2023

2022

gerrit-log December 2024