April 2023 - gerrit-log - lists.osmocom.org

[S] Change in osmo-mgw[master]: mgcp_find_section_end(): skip spaces at start of SDP
by neels 14 Apr '23

14 Apr '23

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/32161 ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: mgcp_find_section_end(): skip spaces at start of SDP ...................................................................... mgcp_find_section_end(): skip spaces at start of SDP Change-Id: I015e0347a268b61c0ca45d40754942f87b461c09 --- M src/libosmo-mgcp-client/mgcp_client.c 1 file changed, 12 insertions(+), 3 deletions(-) Approvals: pespin: Looks good to me, but someone else must approve fixeria: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/libosmo-mgcp-client/mgcp_client.c b/src/libosmo-mgcp-client/mgcp_client.c index 4dd278f..670910b 100644 --- a/src/libosmo-mgcp-client/mgcp_client.c +++ b/src/libosmo-mgcp-client/mgcp_client.c @@ -456,15 +456,15 @@ rc = strstr(string, "\n\n"); if (rc) - return rc; + return rc + 2; rc = strstr(string, "\n\r\n\r"); if (rc) - return rc; + return rc + 4; rc = strstr(string, "\r\n\r\n"); if (rc) - return rc; + return rc + 4; return NULL; } -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/32161 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I015e0347a268b61c0ca45d40754942f87b461c09 Gerrit-Change-Number: 32161 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[M] Change in osmo-mgw[master]: mgcp_client: simpler error handling
by neels 14 Apr '23

14 Apr '23

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/32162 ) Change subject: mgcp_client: simpler error handling ...................................................................... mgcp_client: simpler error handling add_sdp(), add_lco(): - do not msgb_free() within these functions. Just return error, and move the msgb_free() to the caller. - when failing to write to the msgb, directly return error. Change-Id: I904d56f56053952c2ebbbe5dca744fafa32b333e --- M src/libosmo-mgcp-client/mgcp_client.c M tests/mgcp_client/mgcp_client_test.err 2 files changed, 92 insertions(+), 88 deletions(-) Approvals: neels: Looks good to me, approved; Verified diff --git a/src/libosmo-mgcp-client/mgcp_client.c b/src/libosmo-mgcp-client/mgcp_client.c index 670910b..3df9ccf 100644 --- a/src/libosmo-mgcp-client/mgcp_client.c +++ b/src/libosmo-mgcp-client/mgcp_client.c @@ -1135,87 +1135,87 @@ static int add_lco(struct msgb *msg, struct mgcp_msg *mgcp_msg) { unsigned int i; - int rc = 0; const char *codec; unsigned int pt; - rc |= msgb_printf(msg, "L:"); +#define MSGB_PRINTF_OR_RET(FMT, ARGS...) do { \ + if (msgb_printf(msg, FMT, ##ARGS) != 0) { \ + LOGP(DLMGCP, LOGL_ERROR, "Message buffer too small, can not generate MGCP/SDP message\n"); \ + return -ENOBUFS; \ + } \ + } while (0) + + MSGB_PRINTF_OR_RET("L:"); if (mgcp_msg->ptime) - rc |= msgb_printf(msg, " p:%u,", mgcp_msg->ptime); + MSGB_PRINTF_OR_RET(" p:%u,", mgcp_msg->ptime); if (mgcp_msg->codecs_len) { - rc |= msgb_printf(msg, " a:"); + MSGB_PRINTF_OR_RET(" a:"); for (i = 0; i < mgcp_msg->codecs_len; i++) { pt = mgcp_msg->codecs[i]; codec = get_value_string_or_null(osmo_mgcpc_codec_names, pt); /* Note: Use codec descriptors from enum mgcp_codecs * in mgcp_client only! */ - if (!codec) { - msgb_free(msg); + if (!codec) return -EINVAL; - } - rc |= msgb_printf(msg, "%s", extract_codec_name(codec)); + MSGB_PRINTF_OR_RET("%s", extract_codec_name(codec)); if (i < mgcp_msg->codecs_len - 1) - rc |= msgb_printf(msg, ";"); + MSGB_PRINTF_OR_RET(";"); } - rc |= msgb_printf(msg, ","); + MSGB_PRINTF_OR_RET(","); } - rc |= msgb_printf(msg, " nt:IN\r\n"); - - if (rc != 0) { - LOGP(DLMGCP, LOGL_ERROR, - "message buffer to small, can not generate MGCP message (LCO)\n"); - msgb_free(msg); - return -ENOBUFS; - } + MSGB_PRINTF_OR_RET(" nt:IN\r\n"); return 0; + +#undef MSGB_PRINTF_OR_RET } /* Helper function for mgcp_msg_gen(): Add SDP information to MGCP message */ static int add_sdp(struct msgb *msg, struct mgcp_msg *mgcp_msg, struct mgcp_client *mgcp) { unsigned int i; - int rc = 0; char local_ip[INET6_ADDRSTRLEN]; int local_ip_family, audio_ip_family; const char *codec; unsigned int pt; +#define MSGB_PRINTF_OR_RET(FMT, ARGS...) do { \ + if (msgb_printf(msg, FMT, ##ARGS) != 0) { \ + LOGPMGW(mgcp, LOGL_ERROR, "Message buffer too small, can not generate MGCP message (SDP)\n"); \ + return -ENOBUFS; \ + } \ + } while (0) + /* Add separator to mark the beginning of the SDP block */ - rc |= msgb_printf(msg, "\r\n"); + MSGB_PRINTF_OR_RET("\r\n"); /* Add SDP protocol version */ - rc |= msgb_printf(msg, "v=0\r\n"); + MSGB_PRINTF_OR_RET("v=0\r\n"); /* Determine local IP-Address */ if (osmo_sock_local_ip(local_ip, mgcp->actual.remote_addr) < 0) { LOGPMGW(mgcp, LOGL_ERROR, "Could not determine local IP-Address!\n"); - msgb_free(msg); return -EINVAL; } local_ip_family = osmo_ip_str_type(local_ip); - if (local_ip_family == AF_UNSPEC) { - msgb_free(msg); + if (local_ip_family == AF_UNSPEC) return -EINVAL; - } audio_ip_family = osmo_ip_str_type(mgcp_msg->audio_ip); - if (audio_ip_family == AF_UNSPEC) { - msgb_free(msg); + if (audio_ip_family == AF_UNSPEC) return -EINVAL; - } /* Add owner/creator (SDP) */ - rc |= msgb_printf(msg, "o=- %x 23 IN IP%c %s\r\n", mgcp_msg->call_id, + MSGB_PRINTF_OR_RET("o=- %x 23 IN IP%c %s\r\n", mgcp_msg->call_id, local_ip_family == AF_INET6 ? '6' : '4', local_ip); /* Add session name (none) */ - rc |= msgb_printf(msg, "s=-\r\n"); + MSGB_PRINTF_OR_RET("s=-\r\n"); /* Add RTP address and port */ if (mgcp_msg->audio_port == 0) { @@ -1230,20 +1230,20 @@ msgb_free(msg); return -EINVAL; } - rc |= msgb_printf(msg, "c=IN IP%c %s\r\n", + MSGB_PRINTF_OR_RET("c=IN IP%c %s\r\n", audio_ip_family == AF_INET6 ? '6' : '4', mgcp_msg->audio_ip); /* Add time description, active time (SDP) */ - rc |= msgb_printf(msg, "t=0 0\r\n"); + MSGB_PRINTF_OR_RET("t=0 0\r\n"); - rc |= msgb_printf(msg, "m=audio %u RTP/AVP", mgcp_msg->audio_port); + MSGB_PRINTF_OR_RET("m=audio %u RTP/AVP", mgcp_msg->audio_port); for (i = 0; i < mgcp_msg->codecs_len; i++) { pt = map_codec_to_pt(mgcp_msg->ptmap, mgcp_msg->ptmap_len, mgcp_msg->codecs[i]); - rc |= msgb_printf(msg, " %u", pt); + MSGB_PRINTF_OR_RET(" %u", pt); } - rc |= msgb_printf(msg, "\r\n"); + MSGB_PRINTF_OR_RET("\r\n"); /* Add optional codec parameters (fmtp) */ if (mgcp_msg->param_present) { @@ -1253,9 +1253,9 @@ continue; pt = map_codec_to_pt(mgcp_msg->ptmap, mgcp_msg->ptmap_len, mgcp_msg->codecs[i]); if (mgcp_msg->param.amr_octet_aligned_present && mgcp_msg->param.amr_octet_aligned) - rc |= msgb_printf(msg, "a=fmtp:%u octet-align=1\r\n", pt); + MSGB_PRINTF_OR_RET("a=fmtp:%u octet-align=1\r\n", pt); else if (mgcp_msg->param.amr_octet_aligned_present && !mgcp_msg->param.amr_octet_aligned) - rc |= msgb_printf(msg, "a=fmtp:%u octet-align=0\r\n", pt); + MSGB_PRINTF_OR_RET("a=fmtp:%u octet-align=0\r\n", pt); } } @@ -1270,25 +1270,18 @@ /* Note: Use codec descriptors from enum mgcp_codecs * in mgcp_client only! */ - if (!codec) { - msgb_free(msg); + if (!codec) return -EINVAL; - } - rc |= msgb_printf(msg, "a=rtpmap:%u %s\r\n", pt, codec); + MSGB_PRINTF_OR_RET("a=rtpmap:%u %s\r\n", pt, codec); } } if (mgcp_msg->ptime) - rc |= msgb_printf(msg, "a=ptime:%u\r\n", mgcp_msg->ptime); - - if (rc != 0) { - LOGPMGW(mgcp, LOGL_ERROR, "Message buffer to small, can not generate MGCP message (SDP)\n"); - msgb_free(msg); - return -ENOBUFS; - } + MSGB_PRINTF_OR_RET("a=ptime:%u\r\n", mgcp_msg->ptime); return 0; +#undef MSGB_PRINTF_OR_RET } /*! Generate an MGCP message @@ -1300,10 +1293,16 @@ mgcp_trans_id_t trans_id = mgcp_client_next_trans_id(mgcp); uint32_t mandatory_mask; struct msgb *msg = msgb_alloc_headroom(4096, 128, "MGCP tx"); - int rc = 0; bool use_sdp = false; char buf[32]; +#define MSGB_PRINTF_OR_RET(FMT, ARGS...) do { \ + if (msgb_printf(msg, FMT, ##ARGS) != 0) { \ + LOGPMGW(mgcp, LOGL_ERROR, "Message buffer too small, can not generate MGCP/SDP message\n"); \ + goto exit_error; \ + } \ + } while (0) + msg->l2h = msg->data; msg->cb[MSGB_CB_MGCP_TRANS_ID] = trans_id; @@ -1311,72 +1310,67 @@ switch (mgcp_msg->verb) { case MGCP_VERB_CRCX: mandatory_mask = MGCP_CRCX_MANDATORY; - rc |= msgb_printf(msg, "CRCX %u", trans_id); + MSGB_PRINTF_OR_RET("CRCX %u", trans_id); break; case MGCP_VERB_MDCX: mandatory_mask = MGCP_MDCX_MANDATORY; - rc |= msgb_printf(msg, "MDCX %u", trans_id); + MSGB_PRINTF_OR_RET("MDCX %u", trans_id); break; case MGCP_VERB_DLCX: mandatory_mask = MGCP_DLCX_MANDATORY; - rc |= msgb_printf(msg, "DLCX %u", trans_id); + MSGB_PRINTF_OR_RET("DLCX %u", trans_id); break; case MGCP_VERB_AUEP: mandatory_mask = MGCP_AUEP_MANDATORY; - rc |= msgb_printf(msg, "AUEP %u", trans_id); + MSGB_PRINTF_OR_RET("AUEP %u", trans_id); break; case MGCP_VERB_RSIP: mandatory_mask = MGCP_RSIP_MANDATORY; - rc |= msgb_printf(msg, "RSIP %u", trans_id); + MSGB_PRINTF_OR_RET("RSIP %u", trans_id); break; default: LOGPMGW(mgcp, LOGL_ERROR, "Invalid command verb, can not generate MGCP message\n"); - msgb_free(msg); - return NULL; + goto exit_error; } /* Check if mandatory fields are missing */ if (!((mgcp_msg->presence & mandatory_mask) == mandatory_mask)) { LOGPMGW(mgcp, LOGL_ERROR, "One or more missing mandatory fields, can not generate MGCP message\n"); - msgb_free(msg); - return NULL; + goto exit_error; } /* Add endpoint name */ if (mgcp_msg->presence & MGCP_MSG_PRESENCE_ENDPOINT) { if (strlen(mgcp_msg->endpoint) <= 0) { LOGPMGW(mgcp, LOGL_ERROR, "Empty endpoint name, can not generate MGCP message\n"); - msgb_free(msg); - return NULL; + goto exit_error; } if (strstr(mgcp_msg->endpoint, "@") == NULL) { LOGPMGW(mgcp, LOGL_ERROR, "Endpoint name (%s) lacks separator (@), can not generate MGCP message\n", mgcp_msg->endpoint); - msgb_free(msg); - return NULL; + goto exit_error; } - rc |= msgb_printf(msg, " %s", mgcp_msg->endpoint); + MSGB_PRINTF_OR_RET(" %s", mgcp_msg->endpoint); } /* Add protocol version */ - rc |= msgb_printf(msg, " MGCP 1.0\r\n"); + MSGB_PRINTF_OR_RET(" MGCP 1.0\r\n"); /* Add call id */ if (mgcp_msg->presence & MGCP_MSG_PRESENCE_CALL_ID) - rc |= msgb_printf(msg, "C: %x\r\n", mgcp_msg->call_id); + MSGB_PRINTF_OR_RET("C: %x\r\n", mgcp_msg->call_id); /* Add connection id */ if (mgcp_msg->presence & MGCP_MSG_PRESENCE_CONN_ID) { if (strlen(mgcp_msg->conn_id) <= 0) { LOGPMGW(mgcp, LOGL_ERROR, "Empty connection id, can not generate MGCP message\n"); - msgb_free(msg); - return NULL; + goto exit_error; } - rc |= msgb_printf(msg, "I: %s\r\n", mgcp_msg->conn_id); + MSGB_PRINTF_OR_RET("I: %s\r\n", mgcp_msg->conn_id); } /* Using SDP makes sense when a valid IP/Port combination is specifiec, @@ -1391,35 +1385,30 @@ || mgcp_msg->verb == MGCP_VERB_MDCX)) { if (add_lco(msg, mgcp_msg) < 0) { LOGPMGW(mgcp, LOGL_ERROR, "Failed to add LCO, can not generate MGCP message\n"); - return NULL; + goto exit_error; } } /* Add mode */ if (mgcp_msg->presence & MGCP_MSG_PRESENCE_CONN_MODE) - rc |= - msgb_printf(msg, "M: %s\r\n", - mgcp_client_cmode_name(mgcp_msg->conn_mode)); + MSGB_PRINTF_OR_RET("M: %s\r\n", mgcp_client_cmode_name(mgcp_msg->conn_mode)); /* Add X-Osmo-IGN */ if ((mgcp_msg->presence & MGCP_MSG_PRESENCE_X_OSMO_IGN) && (mgcp_msg->x_osmo_ign != 0)) - rc |= - msgb_printf(msg, MGCP_X_OSMO_IGN_HEADER "%s\r\n", - mgcp_msg->x_osmo_ign & MGCP_X_OSMO_IGN_CALLID ? " C": ""); + MSGB_PRINTF_OR_RET(MGCP_X_OSMO_IGN_HEADER "%s\r\n", + mgcp_msg->x_osmo_ign & MGCP_X_OSMO_IGN_CALLID ? " C" : ""); /* Add X-Osmo-Osmux */ if ((mgcp_msg->presence & MGCP_MSG_PRESENCE_X_OSMO_OSMUX_CID)) { if (mgcp_msg->x_osmo_osmux_cid < -1 || mgcp_msg->x_osmo_osmux_cid > OSMUX_CID_MAX) { LOGPMGW(mgcp, LOGL_ERROR, "Wrong Osmux CID %d, can not generate MGCP message\n", mgcp_msg->x_osmo_osmux_cid); - msgb_free(msg); - return NULL; + goto exit_error; } snprintf(buf, sizeof(buf), " %d", mgcp_msg->x_osmo_osmux_cid); - rc |= - msgb_printf(msg, MGCP_X_OSMO_OSMUX_HEADER "%s\r\n", - mgcp_msg->x_osmo_osmux_cid == -1 ? " *": buf); + MSGB_PRINTF_OR_RET(MGCP_X_OSMO_OSMUX_HEADER "%s\r\n", + mgcp_msg->x_osmo_osmux_cid == -1 ? " *" : buf); } @@ -1429,17 +1418,16 @@ || mgcp_msg->verb == MGCP_VERB_MDCX)) { if (add_sdp(msg, mgcp_msg, mgcp) < 0) { LOGPMGW(mgcp, LOGL_ERROR, "Failed to add SDP, can not generate MGCP message\n"); - return NULL; + goto exit_error; } } - if (rc != 0) { - LOGPMGW(mgcp, LOGL_ERROR, "Message buffer to small, can not generate MGCP message\n"); - msgb_free(msg); - msg = NULL; - } - return msg; + +exit_error: + msgb_free(msg); + return NULL; +#undef MSGB_PRINTF_OR_RET } /*! Retrieve the MGCP transaction ID from a msgb generated by mgcp_msg_gen() diff --git a/tests/mgcp_client/mgcp_client_test.err b/tests/mgcp_client/mgcp_client_test.err index d666cba..0bf6d8f 100644 --- a/tests/mgcp_client/mgcp_client_test.err +++ b/tests/mgcp_client/mgcp_client_test.err @@ -1,5 +1,5 @@ DLMGCP MGW(mgw) MGCP client: using endpoint domain '@mgw' -DLMGCP MGW(mgw) Message buffer to small, can not generate MGCP message (SDP) +DLMGCP MGW(mgw) Message buffer too small, can not generate MGCP message (SDP) DLMGCP MGW(mgw) Failed to add SDP, can not generate MGCP message test_mgcp_client_cancel(): -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/32162 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I904d56f56053952c2ebbbe5dca744fafa32b333e Gerrit-Change-Number: 32162 Gerrit-PatchSet: 5 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[M] Change in osmo-mgw[master]: mgcp_client: simpler error handling
by neels 14 Apr '23

14 Apr '23

Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/32162 ) Change subject: mgcp_client: simpler error handling ...................................................................... Patch Set 5: Verified+1 Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/32162 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I904d56f56053952c2ebbbe5dca744fafa32b333e Gerrit-Change-Number: 32162 Gerrit-PatchSet: 5 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 14 Apr 2023 23:00:51 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in osmo-mgw[master]: mgcp_client: simpler error handling
by neels 14 Apr '23

14 Apr '23

Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/32162 ) Change subject: mgcp_client: simpler error handling ...................................................................... Patch Set 5: (1 comment) Commit Message: https://gerrit.osmocom.org/c/osmo-mgw/+/32162/comment/47e4e538_23e049d5 PS1, Line 12: the msgb_free() to the caler. > caller Done -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/32162 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I904d56f56053952c2ebbbe5dca744fafa32b333e Gerrit-Change-Number: 32162 Gerrit-PatchSet: 5 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 14 Apr 2023 22:56:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmo-mgw[master]: mgcp_client: simpler error handling
by neels 14 Apr '23

14 Apr '23

Attention is currently required from: neels. Hello Jenkins Builder, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-mgw/+/32162 to look at the new patch set (#5). Change subject: mgcp_client: simpler error handling ...................................................................... mgcp_client: simpler error handling add_sdp(), add_lco(): - do not msgb_free() within these functions. Just return error, and move the msgb_free() to the caller. - when failing to write to the msgb, directly return error. Change-Id: I904d56f56053952c2ebbbe5dca744fafa32b333e --- M src/libosmo-mgcp-client/mgcp_client.c M tests/mgcp_client/mgcp_client_test.err 2 files changed, 92 insertions(+), 88 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/62/32162/5 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/32162 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I904d56f56053952c2ebbbe5dca744fafa32b333e Gerrit-Change-Number: 32162 Gerrit-PatchSet: 5 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[M] Change in osmo-mgw[master]: mgcp_client: simpler error handling
by neels 14 Apr '23

14 Apr '23

Attention is currently required from: neels. Hello Jenkins Builder, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-mgw/+/32162 to look at the new patch set (#4). Change subject: mgcp_client: simpler error handling ...................................................................... mgcp_client: simpler error handling add_sdp(), add_lco(): - do not msgb_free() within these functions. Just return error, and move the msgb_free() to the caler. - when failing to write to the msgb, directly return error. Change-Id: I904d56f56053952c2ebbbe5dca744fafa32b333e --- M src/libosmo-mgcp-client/mgcp_client.c M tests/mgcp_client/mgcp_client_test.err 2 files changed, 92 insertions(+), 88 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/62/32162/4 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/32162 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I904d56f56053952c2ebbbe5dca744fafa32b333e Gerrit-Change-Number: 32162 Gerrit-PatchSet: 4 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[M] Change in libosmocore[master]: logging vty: probe 'print' and 'logging timestamp' cmds
by neels 14 Apr '23

14 Apr '23

neels has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32042 ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: logging vty: probe 'print' and 'logging timestamp' cmds ...................................................................... logging vty: probe 'print' and 'logging timestamp' cmds Upcoming patch I58c792dda3cbcf8618648ba4429c27fa398a9e15 aims to change the timestamp configuration. Show current state before these changes. Change-Id: I8e0a373496130004e453a2044c1091665fe02a05 --- M tests/logging/logging_vty_test.vty 1 file changed, 108 insertions(+), 0 deletions(-) Approvals: laforge: Looks good to me, but someone else must approve fixeria: Looks good to me, but someone else must approve pespin: Looks good to me, approved Jenkins Builder: Verified diff --git a/tests/logging/logging_vty_test.vty b/tests/logging/logging_vty_test.vty index 2cff62c..f9b4e01 100644 --- a/tests/logging/logging_vty_test.vty +++ b/tests/logging/logging_vty_test.vty @@ -70,6 +70,10 @@ set-log-mask Set the logmask of this logging target level Set the log level for a specified category +logging_vty_test# logging timestamp ? + 0 Don't prefix each log message + 1 Prefix each log message with current timestamp + logging_vty_test# logging level ? ... ! all aa Antropomorphic Armadillos (AA) @@ -109,6 +113,98 @@ error Log error messages and higher levels fatal Log only fatal messages +logging_vty_test# logging print ? + extended-timestamp Configure log message timestamping + thread-id Configure log message logging Thread ID + category Configure log message + category-hex Configure log message + level Configure log message + file Configure log message + +logging_vty_test# logging print extended-timestamp ? + 0 Don't prefix each log message + 1 Prefix each log message with current timestamp with YYYYMMDDhhmmssnnn + +logging_vty_test# logging print thread-id ? + 0 Don't prefix each log message + 1 Prefix each log message with current Thread ID + +logging_vty_test# logging print category ? + 0 Don't prefix each log message + 1 Prefix each log message with category/subsystem name + +logging_vty_test# logging print category-hex ? + 0 Don't prefix each log message + 1 Prefix each log message with category/subsystem nr in hex ('<000b>') + +logging_vty_test# logging print level ? + 0 Don't prefix each log message + 1 Prefix each log message with the log level name + +logging_vty_test# logging print file ? + 0 Don't prefix each log message + 1 Prefix each log message with the source file and line + basename Prefix each log message with the source file's basename (strip leading paths) and line + +logging_vty_test# logging print file basename ? + [last] Log source file info at the end of a log line. If omitted, log source file info just before the log text. + +logging_vty_test# configure terminal +logging_vty_test(config)# log stderr +logging_vty_test(config-log)# show running-config +... +log stderr +... !timestamp + logging timestamp 0 +... !timestamp + +logging_vty_test(config-log)# logging timestamp 1 +logging_vty_test(config-log)# show running-config +... +log stderr +... !timestamp + logging timestamp 1 +... !timestamp + +logging_vty_test(config-log)# ### with 'extended-timestamp 1', 'logging timestamp' is not shown +logging_vty_test(config-log)# logging print extended-timestamp 1 +logging_vty_test(config-log)# show running-config +... +log stderr +... !timestamp + logging print extended-timestamp 1 +... !timestamp + +logging_vty_test(config-log)# ### 'logging timestamp 0' effect not shown while 'extended-timestamp' == 1 +logging_vty_test(config-log)# logging timestamp 0 +logging_vty_test(config-log)# show running-config +... +log stderr +... !timestamp + logging print extended-timestamp 1 +... !timestamp + +logging_vty_test(config-log)# ### 'logging timestamp 1' remains set upon 'extended-timestamp 0' +logging_vty_test(config-log)# logging timestamp 1 +logging_vty_test(config-log)# logging print extended-timestamp 0 +logging_vty_test(config-log)# show running-config +... +log stderr +... !timestamp + logging timestamp 1 +... !timestamp + +logging_vty_test(config-log)# logging timestamp 0 +logging_vty_test(config-log)# show running-config +... +log stderr +... !timestamp + logging timestamp 0 +... !timestamp + +logging_vty_test(config-log)# exit +logging_vty_test(config)# no log stderr +logging_vty_test(config)# exit logging_vty_test# log-sweep DAA DEBUG Log message for DAA on level LOGL_DEBUG -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32042 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I8e0a373496130004e453a2044c1091665fe02a05 Gerrit-Change-Number: 32042 Gerrit-PatchSet: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[M] Change in libosmocore[master]: logging: add 'logging timezone (localtime|utc)'
by neels 14 Apr '23

14 Apr '23

Attention is currently required from: laforge, pespin, fixeria. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32043 ) Change subject: logging: add 'logging timezone (localtime|utc)' ...................................................................... Patch Set 2: (1 comment) File src/core/logging.c: https://gerrit.osmocom.org/c/libosmocore/+/32043/comment/bfc8215b_bd62f6de PS1, Line 874: target->timezone = timezone; > > But we do not *need* to care whether a given timezone function is available. […] Re: it adds non-conforming API: Ok granted, there could be rc in some functions. Here it's simply my taste, I'd prefer to keep the look&feel of log_set_foo() identical to the rest. Re: it makes code more complex: a limitation hidden in logging.c is duplicated to the function that sets the value. That is bad for code maintenance, if something can stay in one place, it should. So far I could work with your choice I guess, with a bit of hateful stare at the "submit" button but ok =) Re: harder to launch: YES, harder to launch. This is not something that the user can fix, really. To have gmtime_r() support, you need to move to a different operating system. So if you want to share a config file between machines, you may have to take explicit care to disable the irritating little completely unimportant timezone option for log output, just to be able to launch the program. I don't want to add that. If we're talking critical component, yes sure. Talking log output config: i don't want to make this a deal breaking configuration choice. Since I am firm on the last point, the rest above is tipped over: there would be no callers using the rc. So I remain that this patch is fine as it is. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32043 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I7f868b47bf8f8dfcf85e735f490ae69b18111af4 Gerrit-Change-Number: 32043 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Fri, 14 Apr 2023 22:50:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmo-mgw[master]: mgcp_client: simpler error handling
by neels 14 Apr '23

14 Apr '23

Attention is currently required from: neels. Hello Jenkins Builder, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-mgw/+/32162 to look at the new patch set (#3). Change subject: mgcp_client: simpler error handling ...................................................................... mgcp_client: simpler error handling add_sdp(), add_lco(): - do not msgb_free() within these functions. Just return error, and move the msgb_free() to the caler. - when failing to write to the msgb, directly return error. Change-Id: I904d56f56053952c2ebbbe5dca744fafa32b333e --- M src/libosmo-mgcp-client/mgcp_client.c 1 file changed, 91 insertions(+), 87 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/62/32162/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/32162 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I904d56f56053952c2ebbbe5dca744fafa32b333e Gerrit-Change-Number: 32162 Gerrit-PatchSet: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[M] Change in osmo-ttcn3-hacks[master]: bsc: add TC_mscpool_sccp_n_pcstate_detaches_msc
by neels 14 Apr '23

14 Apr '23

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32216 ) Change subject: bsc: add TC_mscpool_sccp_n_pcstate_detaches_msc ...................................................................... bsc: add TC_mscpool_sccp_n_pcstate_detaches_msc Related: SYS#6319 OS#5917 Related: I3a0869598b8395601a16d78dbc46eec400c0ea84 (osmo-bsc) Change-Id: Ia1aea4e33230d6a685b72ea5ba20dd9c7d265d44 --- M bsc/BSC_Tests.ttcn 1 file changed, 57 insertions(+), 0 deletions(-) Approvals: laforge: Looks good to me, but someone else must approve neels: Looks good to me, approved fixeria: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/bsc/BSC_Tests.ttcn b/bsc/BSC_Tests.ttcn index bfab0fa..39c82ab 100644 --- a/bsc/BSC_Tests.ttcn +++ b/bsc/BSC_Tests.ttcn @@ -9161,6 +9161,50 @@ f_shutdown_helper(); } +/* When an MSC point-code gets an SCCP N-PCSTATE saying it is unreachable, immediately mark the MSC as unusable. */ +testcase TC_mscpool_sccp_n_pcstate_detaches_msc() runs on test_CT { + + f_init(nr_bts := 3, handler_mode := true, nr_msc := 2); + f_sleep(1.0); + + /* Control which MSC gets chosen next by the round-robin, otherwise + * would be randomly affected by which other tests ran before this. */ + f_vty_transceive(BSCVTY, "mscpool roundrobin next 0"); + + f_ctrs_msc_init(); + + var MSC_ConnHdlr vc_conn1; + var TestHdlrParams pars1 := f_gen_test_hdlr_pars(bssap_idx := 0); + pars1.mscpool.rsl_idx := 0; + pars1.mscpool.l3_info := valueof(ts_LU_REQ(LU_Type_IMSI_Attach, valueof(ts_MI_IMSI_LV('001010000000001'H)), '00F110'O)); + vc_conn1 := f_start_handler(refers(f_tc_mscpool_compl_l3), pars1); + vc_conn1.done; + f_ctrs_msc_expect(0, "mscpool:subscr:new"); + + var MSC_ConnHdlr vc_conn2; + var TestHdlrParams pars2 := f_gen_test_hdlr_pars(bssap_idx := 1); + pars2.mscpool.rsl_idx := 1; + pars2.mscpool.l3_info := valueof(ts_CM_SERV_REQ(CM_TYPE_MO_CALL, valueof(ts_MI_IMSI_LV('001010000000002'H)))); + vc_conn2 := f_start_handler(refers(f_tc_mscpool_compl_l3), pars2); + vc_conn2.done; + f_ctrs_msc_expect(1, "mscpool:subscr:new"); + + f_logp(BSCVTY, "disconnecting msc0"); + /* Disconnect the first MSC */ + f_ran_adapter_cleanup(g_bssap[0]); + + /* Now round-robin would wrap to the first MSC, but since the first MSC is disconnected, it wraps around to msc + * 2 again. */ + var MSC_ConnHdlr vc_conn3; + var TestHdlrParams pars3 := f_gen_test_hdlr_pars(bssap_idx := 1); + pars3.mscpool.rsl_idx := 2; + pars3.mscpool.l3_info := valueof(ts_PAG_RESP(valueof(ts_MI_IMSI_LV('001010000000003'H)))); + vc_conn3 := f_start_handler(refers(f_tc_mscpool_compl_l3), pars3); + vc_conn3.done; + f_ctrs_msc_expect(1, "mscpool:subscr:new"); + f_shutdown_helper(); +} + /* Allow/Deny emergency calls globally via VTY */ private function f_vty_allow_emerg_msc(boolean allow) runs on test_CT { f_vty_enter_cfg_msc(BSCVTY, 0); @@ -12463,6 +12507,8 @@ execute( TC_mscpool_paging_and_response_tmsi() ); execute( TC_mscpool_no_allow_attach_round_robin() ); execute( TC_mscpool_no_allow_attach_valid_nri() ); + + execute( TC_mscpool_sccp_n_pcstate_detaches_msc() ); } execute( TC_early_conn_fail() ); -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32216 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: Ia1aea4e33230d6a685b72ea5ba20dd9c7d265d44 Gerrit-Change-Number: 32216 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0