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

[M] Change in pysim[master]: [2/7] personalization: refactor ConfigurableParameter, Iccid, Imsi

by neels

Attention is currently required from: fixeria. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/pysim/+/39742?usp=email to look at the new patch set (#5). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder Change subject: [2/7] personalization: refactor ConfigurableParameter, Iccid, Imsi ...................................................................... [2/7] personalization: refactor ConfigurableParameter, Iccid, Imsi Main points/rationales of the refactoring, details below: 1) common validation implementation 2) offer classmethods The new features are optional, and will be heavily used by batch personalization patches coming soon. Implement Iccid and Imsi to use the new way, with a common abstract DecimalParam implementation. So far leave the other parameter classes working as they always did, to follow suit in subsequent commits. Details: 1) common validation implementation: There are very common validation steps in the various parameter implementations. It is more convenient and much more readable to implement those once and set simple validation parameters per subclass. So there now is a validate_val() classmethod, which subclasses can use as-is to apply the validation parameters -- or subclasses can override their cls.validate_val() for specialized validation. (Those subclasses that this patch doesn't touch still override the self.validate() instance method. Hence they still work as before this patch, but don't use the new common features yet.) 2) offer stateless classmethods: It is useful for... - batch processing of multiple profiles (in upcoming patches) and - user input validation to be able to have classmethods that do what self.validate() and self.apply() do, but do not modify any self.* members. So far the paradigm was to create a class instance to keep state about the value. This remains available, but in addition we make available the paradigm of a singleton that is stateless (the classmethods). Using self.validate() and self.apply() still work the same as before this patch, i.e. via self.input_value and self.value -- but in addition, there are now classmethods that don't touch self.* members. Related: SYS#6768 Change-Id: I6522be4c463e34897ca9bff2309b3706a88b3ce8 --- M pySim/esim/saip/personalization.py 1 file changed, 180 insertions(+), 34 deletions(-) git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/42/39742/5 -- To view, visit https://gerrit.osmocom.org/c/pysim/+/39742?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I6522be4c463e34897ca9bff2309b3706a88b3ce8 Gerrit-Change-Number: 39742 Gerrit-PatchSet: 5 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>

4 months

1
0
0 0

[M] Change in pysim[master]: [2/7] personalization: refactor ConfigurableParameter, Iccid, Imsi

by neels

Attention is currently required from: fixeria. neels has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/pysim/+/39742?usp=email ) Change subject: [2/7] personalization: refactor ConfigurableParameter, Iccid, Imsi ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: if there are only cosmetic comments that refer to changes from the previous patch, then i would appreciate a +1, ouch -- or let's have some comments on the actual functional changes in this patch, thanks -- To view, visit https://gerrit.osmocom.org/c/pysim/+/39742?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I6522be4c463e34897ca9bff2309b3706a88b3ce8 Gerrit-Change-Number: 39742 Gerrit-PatchSet: 4 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Mon, 10 Mar 2025 12:33:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

4 months

1
0
0 0

[M] Change in libosmo-sigtran[master]: ASP loadsharing: vty: Add cmd 'show cs7 instance <0-15> as binding-ta...

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/39711?usp=email ) Change subject: ASP loadsharing: vty: Add cmd 'show cs7 instance <0-15> as binding-table name AS_NAME' ...................................................................... ASP loadsharing: vty: Add cmd 'show cs7 instance <0-15> as binding-table name AS_NAME' Similar to the one available in Cisco ITP, but output is organized differently, printed indexes by eSLS to fit our implementation. Change-Id: Ia2e16b541b229805be23971957f5f7edecf61ed1 --- M src/osmo_ss7_vty.c M tests/vty/osmo_stp_test.vty 2 files changed, 184 insertions(+), 5 deletions(-) Approvals: daniel: Looks good to me, but someone else must approve Jenkins Builder: Verified pespin: Looks good to me, approved osmith: Looks good to me, but someone else must approve diff --git a/src/osmo_ss7_vty.c b/src/osmo_ss7_vty.c index 63b127c..4344ea2 100644 --- a/src/osmo_ss7_vty.c +++ b/src/osmo_ss7_vty.c @@ -2433,6 +2433,49 @@ return show_as(vty, id, as_name, NULL); } +DEFUN(show_cs7_as_bindingtable_name, show_cs7_as_bindingtable_name_cmd, + "show cs7 instance <0-15> as binding-table name AS_NAME", + SHOW_STR CS7_STR INST_STR INST_STR "Application Server (AS)\n" + "Display binding table\n" + "Look up AS with a given name\n" + "Name of the Application Server (AS)\n") +{ + int id = atoi(argv[0]); + const char *as_name = argv[1]; + struct osmo_ss7_instance *inst; + struct osmo_ss7_as *as = NULL; + + inst = osmo_ss7_instance_find(id); + if (!inst) { + vty_out(vty, "No SS7 instance %d found%s", id, VTY_NEWLINE); + return CMD_WARNING; + } + + if (as_name) { + as = osmo_ss7_as_find_by_name(inst, as_name); + if (!as) { + vty_out(vty, "No AS %s found%s", as_name, VTY_NEWLINE); + return CMD_WARNING; + } + } + + vty_out(vty, "Loadshare Seed Normal ASP Active Alternative ASP Active%s", VTY_NEWLINE); + vty_out(vty, "-------------- --------------- ------ --------------- ------%s", VTY_NEWLINE); + + for (unsigned int i = 0; i < ARRAY_SIZE(as->aesls_table); i++) { + struct osmo_ss7_as_esls_entry *e = &as->aesls_table[i]; + vty_out(vty, "%-15u %-16s %-7s %-16s %-7s%s", + i, + e->normal_asp ? e->normal_asp->cfg.name : "-", + e->normal_asp ? (osmo_ss7_asp_active(e->normal_asp) ? "Yes" : "No") : "-", + e->alt_asp ? e->alt_asp->cfg.name : "-", + e->alt_asp ? (osmo_ss7_asp_active(e->alt_asp) ? "Yes" : "No") : "-", + VTY_NEWLINE); + } + + return CMD_SUCCESS; +} + /*********************************************************************** * SCCP addressbook handling ***********************************************************************/ @@ -3306,6 +3349,7 @@ install_node(&as_node, NULL); install_lib_element_ve(&show_cs7_as_cmd); install_lib_element_ve(&show_cs7_as_name_cmd); + install_lib_element_ve(&show_cs7_as_bindingtable_name_cmd); install_lib_element(L_CS7_NODE, &cs7_as_cmd); install_lib_element(L_CS7_NODE, &no_cs7_as_cmd); install_lib_element(L_CS7_AS_NODE, &cfg_description_cmd); diff --git a/tests/vty/osmo_stp_test.vty b/tests/vty/osmo_stp_test.vty index 81515c1..75f3eae 100644 --- a/tests/vty/osmo_stp_test.vty +++ b/tests/vty/osmo_stp_test.vty @@ -12,6 +12,7 @@ show cs7 instance <0-15> asp-assoc-status name ASP_NAME show cs7 instance <0-15> as (active|all|m3ua|sua) show cs7 instance <0-15> as name AS_NAME + show cs7 instance <0-15> as binding-table name AS_NAME show cs7 instance <0-15> route [POINT_CODE] show cs7 instance <0-15> route-lookup POINT_CODE from POINT_CODE sls <0-15> [list-asps] show cs7 instance <0-15> sccp addressbook @@ -37,6 +38,7 @@ show cs7 instance <0-15> asp-assoc-status name ASP_NAME show cs7 instance <0-15> as (active|all|m3ua|sua) show cs7 instance <0-15> as name AS_NAME + show cs7 instance <0-15> as binding-table name AS_NAME show cs7 instance <0-15> route [POINT_CODE] show cs7 instance <0-15> route-lookup POINT_CODE from POINT_CODE sls <0-15> [list-asps] show cs7 instance <0-15> sccp addressbook @@ -86,11 +88,12 @@ sccp Signalling Connection Control Part OsmoSTP# show cs7 instance 0 as ? - active Display all active ASs - all Display all ASs (default) - m3ua Display all m3ua ASs - sua Display all SUA ASs - name Look up AS with a given name + active Display all active ASs + all Display all ASs (default) + m3ua Display all m3ua ASs + sua Display all SUA ASs + name Look up AS with a given name + binding-table Display binding table OsmoSTP# show cs7 instance 0 sccp ? addressbook List all SCCP addressbook entries @@ -414,6 +417,138 @@ OsmoSTP(config-cs7-as)# do show cs7 instance 0 as name my-ass-not-exists % No AS 'my-ass-not-exists' found +OsmoSTP(config-cs7-as)# do show cs7 instance 0 as binding-table name my-ass +Loadshare Seed Normal ASP Active Alternative ASP Active +-------------- --------------- ------ --------------- ------ +0 - - - - +1 - - - - +2 - - - - +3 - - - - +4 - - - - +5 - - - - +6 - - - - +7 - - - - +8 - - - - +9 - - - - +10 - - - - +11 - - - - +12 - - - - +13 - - - - +14 - - - - +15 - - - - +16 - - - - +17 - - - - +18 - - - - +19 - - - - +20 - - - - +21 - - - - +22 - - - - +23 - - - - +24 - - - - +25 - - - - +26 - - - - +27 - - - - +28 - - - - +29 - - - - +30 - - - - +31 - - - - +32 - - - - +33 - - - - +34 - - - - +35 - - - - +36 - - - - +37 - - - - +38 - - - - +39 - - - - +40 - - - - +41 - - - - +42 - - - - +43 - - - - +44 - - - - +45 - - - - +46 - - - - +47 - - - - +48 - - - - +49 - - - - +50 - - - - +51 - - - - +52 - - - - +53 - - - - +54 - - - - +55 - - - - +56 - - - - +57 - - - - +58 - - - - +59 - - - - +60 - - - - +61 - - - - +62 - - - - +63 - - - - +64 - - - - +65 - - - - +66 - - - - +67 - - - - +68 - - - - +69 - - - - +70 - - - - +71 - - - - +72 - - - - +73 - - - - +74 - - - - +75 - - - - +76 - - - - +77 - - - - +78 - - - - +79 - - - - +80 - - - - +81 - - - - +82 - - - - +83 - - - - +84 - - - - +85 - - - - +86 - - - - +87 - - - - +88 - - - - +89 - - - - +90 - - - - +91 - - - - +92 - - - - +93 - - - - +94 - - - - +95 - - - - +96 - - - - +97 - - - - +98 - - - - +99 - - - - +100 - - - - +101 - - - - +102 - - - - +103 - - - - +104 - - - - +105 - - - - +106 - - - - +107 - - - - +108 - - - - +109 - - - - +110 - - - - +111 - - - - +112 - - - - +113 - - - - +114 - - - - +115 - - - - +116 - - - - +117 - - - - +118 - - - - +119 - - - - +120 - - - - +121 - - - - +122 - - - - +123 - - - - +124 - - - - +125 - - - - +126 - - - - +127 - - - - + OsmoSTP(config-cs7-as)# asp my-asp OsmoSTP(config-cs7-as)# routing-key 0 3.2.1 -- To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/39711?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: libosmo-sigtran Gerrit-Branch: master Gerrit-Change-Id: Ia2e16b541b229805be23971957f5f7edecf61ed1 Gerrit-Change-Number: 39711 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

4 months

1
0
0 0

[M] Change in libosmo-sigtran[master]: ASP loadsharing: vty: Add cmd 'show cs7 instance <0-15> as binding-ta...

by pespin

Attention is currently required from: laforge. pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/39711?usp=email ) Change subject: ASP loadsharing: vty: Add cmd 'show cs7 instance <0-15> as binding-table name AS_NAME' ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/39711?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmo-sigtran Gerrit-Branch: master Gerrit-Change-Id: Ia2e16b541b229805be23971957f5f7edecf61ed1 Gerrit-Change-Number: 39711 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Mon, 10 Mar 2025 12:23:21 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

4 months

1
0
0 0

[M] Change in pysim[master]: [1/7] personalization: refactor: drop ClassVarMeta use

by neels

Attention is currently required from: fixeria. neels has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/pysim/+/39741?usp=email ) Change subject: [1/7] personalization: refactor: drop ClassVarMeta use ...................................................................... Patch Set 4: (2 comments) File pySim/esim/saip/personalization.py: https://gerrit.osmocom.org/c/pysim/+/39741/comment/98345a6c_03268920?usp=em… : PS4, Line 61: @abc.abstractmethod > Why are you removing this? The COMMIT_MSG says you're dropping `ClassVarMeta`, but you're also remov […] i commented on this on the subsequent commit, TL;DR: no noticeable difference and no need to have it. If you insist i can put back the @abc.abstractmethod decorator. Can you explain why is @abstractmethod not a python built-in!? (The ClassVarMeta is not something I am willing to bring back, see below.) https://gerrit.osmocom.org/c/pysim/+/39741/comment/9830d14d_d5d00c50?usp=em… : PS4, Line 147: class SdKeyScp80_01(SdKey, kvn=0x01, key_type=0x88, permitted_len=[16,24,32]): # AES key type > IMO, the old approach is much more compact and thus more readable. IMHO absolutely not. All members in one line? With future extensions in upcoming patches, it will look like this class SdKeyScp81_01Psk(SdKeyScp81_01, is_abstract = False, name = 'SCP81.01-PSK', key_id = 0x01, key_type = KeyType.tls_psk # 0x85, key_usage_qual = 0x3C): pass you cannot seriously prefer that. So let's make it more readable class SdKeyScp81_01Psk(SdKeyScp81_01, is_abstract = False, name = 'SCP81.01-PSK', key_id = 0x01, key_type = KeyType.tls_psk, # 0x85 key_usage_qual = 0x3C): pass Congratulations, that is almost standard python, but messed up. I prefer a million times: class SdKeyScp81_01Psk(SdKeyScp81_01): is_abstract = False name = 'SCP81.01-PSK' key_id = 0x01 key_type = KeyType.tls_psk # 0x85 key_usage_qual = 0x3C It's standard, shorter, doesn't need messing with the builtin __new__() method. Complete no-brainer to me. -- To view, visit https://gerrit.osmocom.org/c/pysim/+/39741?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I60ea8fd11fb438ec90ddb08b17b658cbb789c051 Gerrit-Change-Number: 39741 Gerrit-PatchSet: 4 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Mon, 10 Mar 2025 12:21:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>

4 months

1
0
0 0

[M] Change in osmo-mgw[master]: mgw: Move struct mgcp_request_data definition to header file

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39718?usp=email ) Change subject: mgw: Move struct mgcp_request_data definition to header file ...................................................................... mgw: Move struct mgcp_request_data definition to header file All the related handling structs are placed in the header file, let's move them there. This will also allow accessing the rq object information in other files. Change-Id: I1b2831874c1aaad054ad19bf87646062ba9d4da4 --- M include/osmocom/mgcp/mgcp_protocol.h M src/libosmo-mgcp/mgcp_protocol.c 2 files changed, 25 insertions(+), 25 deletions(-) Approvals: osmith: Looks good to me, but someone else must approve Jenkins Builder: Verified daniel: Looks good to me, approved diff --git a/include/osmocom/mgcp/mgcp_protocol.h b/include/osmocom/mgcp/mgcp_protocol.h index 4fed248..dda0fb3 100644 --- a/include/osmocom/mgcp/mgcp_protocol.h +++ b/include/osmocom/mgcp/mgcp_protocol.h @@ -82,6 +82,31 @@ struct mgcp_parse_sdp sdp; }; +/* Request data passed to the request handler */ +struct mgcp_request_data { + enum mgcp_verb verb; + /* Verb string (e.g. "MDCX") */ + char name[4+1]; + + /* parsing results from the MGCP header (trans id, endpoint name ...) */ + struct mgcp_parse_data *pdata; + + /* pointer to endpoint resource (may be NULL for wildcarded requests) */ + struct mgcp_endpoint *endp; + + /* pointer to trunk resource */ + struct mgcp_trunk *trunk; + + /* set to true when the request has been classified as wildcarded */ + bool wildcarded; + + /* Set to true when the request is targeted at the "null" endpoint */ + bool null_endp; + + /* contains cause code in case of problems during endp/trunk resolution */ + int mgcp_cause; +}; + /* Local connection options */ struct mgcp_lco { char *string; diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index 36d9ebf..74ed14e 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -89,31 +89,6 @@ LOGPTRUNK(trunk, cat, level, fmt, ## args); \ } while (0) -/* Request data passed to the request handler */ -struct mgcp_request_data { - enum mgcp_verb verb; - /* Verb string (e.g. "MDCX") */ - char name[4+1]; - - /* parsing results from the MGCP header (trans id, endpoint name ...) */ - struct mgcp_parse_data *pdata; - - /* pointer to endpoint resource (may be NULL for wildcarded requests) */ - struct mgcp_endpoint *endp; - - /* pointer to trunk resource */ - struct mgcp_trunk *trunk; - - /* set to true when the request has been classified as wildcarded */ - bool wildcarded; - - /* Set to true when the request is targeted at the "null" endpoint */ - bool null_endp; - - /* contains cause code in case of problems during endp/trunk resolution */ - int mgcp_cause; -}; - static struct msgb *handle_audit_endpoint(struct mgcp_request_data *data); static struct msgb *handle_create_con(struct mgcp_request_data *data); static struct msgb *handle_delete_con(struct mgcp_request_data *data); -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39718?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I1b2831874c1aaad054ad19bf87646062ba9d4da4 Gerrit-Change-Number: 39718 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

4 months

1
0
0 0

[XS] Change in osmo-mgw[master]: mgw: Add backpointer from pdata to req to have context available

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39720?usp=email ) Change subject: mgw: Add backpointer from pdata to req to have context available ...................................................................... mgw: Add backpointer from pdata to req to have context available This will also allow eg. parsing/validating differently based on the command being parsed, or incrementing rate counters per cmd group. Change-Id: I464258ca1a8817d58ae5c5426dfc3b7cee6763d3 --- M include/osmocom/mgcp/mgcp_protocol.h M src/libosmo-mgcp/mgcp_msg.c M src/libosmo-mgcp/mgcp_protocol.c 3 files changed, 5 insertions(+), 2 deletions(-) Approvals: osmith: Looks good to me, but someone else must approve pespin: Looks good to me, approved Jenkins Builder: Verified daniel: Looks good to me, but someone else must approve diff --git a/include/osmocom/mgcp/mgcp_protocol.h b/include/osmocom/mgcp/mgcp_protocol.h index e61669c..2ef4c3e 100644 --- a/include/osmocom/mgcp/mgcp_protocol.h +++ b/include/osmocom/mgcp/mgcp_protocol.h @@ -71,7 +71,9 @@ } /* Internal structure while parsing a request */ +struct mgcp_request_data; struct mgcp_parse_data { + struct mgcp_request_data *rq; /* backpointer to request context */ char *save; /* MGCP Header: */ char *epname; diff --git a/src/libosmo-mgcp/mgcp_msg.c b/src/libosmo-mgcp/mgcp_msg.c index 2269a66..af4f9d0 100644 --- a/src/libosmo-mgcp/mgcp_msg.c +++ b/src/libosmo-mgcp/mgcp_msg.c @@ -36,7 +36,7 @@ /* (same fmt as LOGPENDP()) */ #define LOG_MGCP_PDATA(PDATA, LEVEL, FMT, ARGS...) \ - LOGP(DLMGCP, LEVEL, "endpoint:%s " FMT, (PDATA) ? ((PDATA)->epname ? : "null-epname") : "null-pdata", ##ARGS) + LOGP(DLMGCP, LEVEL, "%s: endpoint(%s) " FMT, (PDATA)->rq->name, (PDATA)->epname ? : "null-epname", ##ARGS) /*! Display an mgcp message on the log output. * \param[in] message mgcp message string @@ -212,7 +212,7 @@ hp->have_sdp = true; goto mgcp_header_done; default: - LOG_MGCP_PDATA(pdata, LOGL_NOTICE, "CRCX: unhandled option: '%c'/%d\n", *line, *line); + LOG_MGCP_PDATA(pdata, LOGL_NOTICE, "unhandled option: '%c'/%d\n", *line, *line); return -539; } } diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index 1215b95..1eb241b 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -332,6 +332,7 @@ memset(&rq, 0, sizeof(rq)); rq.cfg = cfg; memset(&pdata, 0, sizeof(pdata)); + pdata.rq = &rq; /* Parse command name: */ memcpy(rq.name, (const char *)&msg->l2h[0], sizeof(rq.name)-1); -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39720?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I464258ca1a8817d58ae5c5426dfc3b7cee6763d3 Gerrit-Change-Number: 39720 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

4 months

1
0
0 0

[S] Change in osmo-mgw[master]: mgw: Move cfg pointer from pdata to rq

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39719?usp=email ) Change subject: mgw: Move cfg pointer from pdata to rq ...................................................................... mgw: Move cfg pointer from pdata to rq The externa object context is held in rq object, not in pdata object, which holds parsed msg state only. Change-Id: Ie64fb6250cc84f1e7737b5c1cf5cca314c7020ed --- M include/osmocom/mgcp/mgcp_protocol.h M src/libosmo-mgcp/mgcp_protocol.c 2 files changed, 17 insertions(+), 15 deletions(-) Approvals: daniel: Looks good to me, approved osmith: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/include/osmocom/mgcp/mgcp_protocol.h b/include/osmocom/mgcp/mgcp_protocol.h index dda0fb3..e61669c 100644 --- a/include/osmocom/mgcp/mgcp_protocol.h +++ b/include/osmocom/mgcp/mgcp_protocol.h @@ -72,7 +72,6 @@ /* Internal structure while parsing a request */ struct mgcp_parse_data { - struct mgcp_config *cfg; char *save; /* MGCP Header: */ char *epname; @@ -88,6 +87,9 @@ /* Verb string (e.g. "MDCX") */ char name[4+1]; + /* Global MGW config */ + struct mgcp_config *cfg; + /* parsing results from the MGCP header (trans id, endpoint name ...) */ struct mgcp_parse_data *pdata; diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index 74ed14e..1215b95 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -329,9 +329,9 @@ } /* Initialize parsing data. */ - memset(&pdata, 0, sizeof(pdata)); memset(&rq, 0, sizeof(rq)); - pdata.cfg = cfg; + rq.cfg = cfg; + memset(&pdata, 0, sizeof(pdata)); /* Parse command name: */ memcpy(rq.name, (const char *)&msg->l2h[0], sizeof(rq.name)-1); @@ -360,7 +360,7 @@ if (!rq.wildcarded) rq.null_endp = mgcp_endp_is_null(pdata.epname); if (!rq.null_endp) - rq.endp = mgcp_endp_by_name(&rc, pdata.epname, pdata.cfg); + rq.endp = mgcp_endp_by_name(&rc, pdata.epname, rq.cfg); rq.mgcp_cause = rc; if (!rq.endp) { rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_GENERAL_RX_FAIL_NO_ENDPOINT)); @@ -371,7 +371,7 @@ LOGP(DLMGCP, LOGL_NOTICE, "%s: cannot find endpoint \"%s\", cause=%d -- trying to identify trunk...\n", rq.name, pdata.epname, -rq.mgcp_cause); - rq.trunk = mgcp_trunk_by_name(pdata.cfg, pdata.epname); + rq.trunk = mgcp_trunk_by_name(rq.cfg, pdata.epname); if (!rq.trunk) { LOGP(DLMGCP, LOGL_ERROR, "%s: failed to identify trunk for endpoint \"%s\" -- abort\n", rq.name, pdata.epname); @@ -438,7 +438,7 @@ /* Auditing "null" endpoint is allowed for keepalive purposes. There's no rq->endp nor rq->trunk in this case. */ if (rq->null_endp) - return create_ok_response(rq->pdata->cfg, NULL, 200, "AUEP", rq->pdata->trans); + return create_ok_response(rq->cfg, NULL, 200, "AUEP", rq->pdata->trans); if (!rq->endp || !mgcp_endp_avail(rq->endp)) { LOGPENDP(rq->endp, DLMGCP, LOGL_ERROR, "AUEP: selected endpoint not available!\n"); @@ -772,7 +772,7 @@ if (rq->null_endp) { /* trunk not available so rate_ctr aren't available either. */ LOGP(DLMGCP, LOGL_ERROR, "CRCX: Not allowed in 'null' endpoint!\n"); - return create_err_response(pdata->cfg, NULL, 502, "CRCX", pdata->trans); + return create_err_response(rq->cfg, NULL, 502, "CRCX", pdata->trans); } rate_ctrs = trunk->ratectr.mgcp_crcx_ctr_group; @@ -1042,7 +1042,7 @@ if (rq->null_endp) { /* trunk not available so rate_ctr aren't available either. */ LOGP(DLMGCP, LOGL_ERROR, "MDCX: Not allowed in 'null' endpoint!\n"); - return create_err_response(pdata->cfg, NULL, 502, "MDCX", pdata->trans); + return create_err_response(rq->cfg, NULL, 502, "MDCX", pdata->trans); } rate_ctrs = trunk->ratectr.mgcp_mdcx_ctr_group; @@ -1238,7 +1238,7 @@ if (rq->null_endp) { /* trunk not available so rate_ctr aren't available either. */ LOGP(DLMGCP, LOGL_ERROR, "DLCX: Not allowed in 'null' endpoint!\n"); - return create_err_response(pdata->cfg, NULL, 502, "DLCX", pdata->trans); + return create_err_response(rq->cfg, NULL, 502, "DLCX", pdata->trans); } rate_ctrs = trunk->ratectr.mgcp_dlcx_ctr_group; @@ -1377,11 +1377,11 @@ if (rq->null_endp) { /* trunk not available so rate_ctr aren't available either. */ LOGP(DLMGCP, LOGL_ERROR, "RSIP: Not allowed in 'null' endpoint!\n"); - return create_err_response(rq->pdata->cfg, NULL, 502, "RSIP", rq->pdata->trans); + return create_err_response(rq->cfg, NULL, 502, "RSIP", rq->pdata->trans); } - if (rq->pdata->cfg->reset_cb) - rq->pdata->cfg->reset_cb(rq->endp->trunk); + if (rq->cfg->reset_cb) + rq->cfg->reset_cb(rq->endp->trunk); return NULL; } @@ -1408,7 +1408,7 @@ if (rq->null_endp) { /* trunk not available so rate_ctr aren't available either. */ LOGP(DLMGCP, LOGL_ERROR, "RQNT: Not allowed in 'null' endpoint!\n"); - return create_err_response(rq->pdata->cfg, NULL, 502, "RQNT", rq->pdata->trans); + return create_err_response(rq->cfg, NULL, 502, "RQNT", rq->pdata->trans); } for_each_line(line, rq->pdata->save) { @@ -1423,8 +1423,8 @@ if (tone == CHAR_MAX) return create_ok_response(rq->endp, rq->endp, 200, "RQNT", rq->pdata->trans); - if (rq->pdata->cfg->rqnt_cb) - res = rq->pdata->cfg->rqnt_cb(rq->endp, tone); + if (rq->cfg->rqnt_cb) + res = rq->cfg->rqnt_cb(rq->endp, tone); return res == 0 ? create_ok_response(rq->endp, rq->endp, 200, "RQNT", rq->pdata->trans) : create_err_response(rq->endp, rq->endp, res, "RQNT", rq->pdata->trans); -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39719?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ie64fb6250cc84f1e7737b5c1cf5cca314c7020ed Gerrit-Change-Number: 39719 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

4 months

1
0
0 0

[XS] Change in osmo-mgw[master]: mgw: Use tall_mgw_ctx to allocate struct mgcp_config

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39721?usp=email ) Change subject: mgw: Use tall_mgw_ctx to allocate struct mgcp_config ...................................................................... mgw: Use tall_mgw_ctx to allocate struct mgcp_config Change-Id: Ib51f1066bb73da4472bd8967cc1b1c44e964e9e5 --- M include/osmocom/mgcp/mgcp.h M src/libosmo-mgcp/mgcp_protocol.c M tests/mgcp/mgcp_test.c 3 files changed, 6 insertions(+), 2 deletions(-) Approvals: Jenkins Builder: Verified osmith: Looks good to me, approved diff --git a/include/osmocom/mgcp/mgcp.h b/include/osmocom/mgcp/mgcp.h index 41eee42..f72807e 100644 --- a/include/osmocom/mgcp/mgcp.h +++ b/include/osmocom/mgcp/mgcp.h @@ -40,6 +40,8 @@ #include "mgcp_ratectr.h" +extern void *tall_mgw_ctx; + #define RTP_PORT_DEFAULT_RANGE_START 16002 #define RTP_PORT_DEFAULT_RANGE_END RTP_PORT_DEFAULT_RANGE_START + 64 diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index 1eb241b..163b816 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -1505,7 +1505,7 @@ * appropiate place! */ struct mgcp_config *cfg; - cfg = talloc_zero(NULL, struct mgcp_config); + cfg = talloc_zero(tall_mgw_ctx, struct mgcp_config); if (!cfg) { LOGP(DLMGCP, LOGL_FATAL, "Failed to allocate config.\n"); return NULL; diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c index 1c1a41e..b9224d9 100644 --- a/tests/mgcp/mgcp_test.c +++ b/tests/mgcp/mgcp_test.c @@ -43,6 +43,8 @@ #include <math.h> #include <ctype.h> +void *tall_mgw_ctx; + char *strline_r(char *str, char **saveptr); const char *strline_test_data = @@ -2323,7 +2325,7 @@ INIT_LLIST_HEAD(&endp.conns); - conn = talloc_zero(NULL, struct mgcp_conn); + conn = talloc_zero(tall_mgw_ctx, struct mgcp_conn); OSMO_ASSERT(conn); osmo_strlcpy(conn->id, conn_id_generated, sizeof(conn->id)); llist_add(&conn->entry, &endp.conns); -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39721?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ib51f1066bb73da4472bd8967cc1b1c44e964e9e5 Gerrit-Change-Number: 39721 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

4 months

1
0
0 0

[M] Change in osmo-mgw[master]: mgw: Allocate req and pdata with talloc

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39722?usp=email ) Change subject: mgw: Allocate req and pdata with talloc ...................................................................... mgw: Allocate req and pdata with talloc This will allow allocating parsed objects as child of the request and make sure they are freed after handling the request finishes. Change-Id: I2b971d0c8268f4c0a30b84b54a2e5f16ada4ecdb --- M src/libosmo-mgcp/mgcp_protocol.c 1 file changed, 56 insertions(+), 50 deletions(-) Approvals: Jenkins Builder: Verified osmith: Looks good to me, but someone else must approve pespin: Looks good to me, approved daniel: Looks good to me, but someone else must approve diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index 163b816..06da91a 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -297,8 +297,8 @@ struct msgb *mgcp_handle_message(struct mgcp_config *cfg, struct msgb *msg) { struct rate_ctr_group *rate_ctrs = cfg->ratectr.mgcp_general_ctr_group; - struct mgcp_parse_data pdata; - struct mgcp_request_data rq; + struct mgcp_parse_data *pdata = NULL; + struct mgcp_request_data *rq = NULL; int rc, code; struct msgb *resp = NULL; char *data; @@ -329,106 +329,112 @@ } /* Initialize parsing data. */ - memset(&rq, 0, sizeof(rq)); - rq.cfg = cfg; - memset(&pdata, 0, sizeof(pdata)); - pdata.rq = &rq; + rq = talloc_zero(tall_mgw_ctx, struct mgcp_request_data); + rq->cfg = cfg; + rq->pdata = pdata = talloc_zero(rq, struct mgcp_parse_data); + pdata->rq = rq; /* Parse command name: */ - memcpy(rq.name, (const char *)&msg->l2h[0], sizeof(rq.name)-1); - rc = get_string_value(mgcp_verb_names, rq.name); + memcpy(rq->name, (const char *)&msg->l2h[0], sizeof(rq->name)-1); + rc = get_string_value(mgcp_verb_names, rq->name); if (rc < 0) { - LOGP(DLMGCP, LOGL_ERROR, "%s: failed to parse command name in MCGP message\n", rq.name); + LOGP(DLMGCP, LOGL_ERROR, "%s: failed to parse command name in MCGP message\n", rq->name); rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_GENERAL_RX_FAIL_MSG_PARSE)); rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_GENERAL_RX_MSGS_UNHANDLED)); - return create_err_response(cfg, NULL, 504, rq.name, "000000"); + resp = create_err_response(cfg, NULL, 504, rq->name, "000000"); + goto ret_free; } - rq.verb = rc; + rq->verb = rc; /* Parse message, extract endpoint name and transaction identifier and request name etc. */ msg->l3h = &msg->l2h[4]; - data = mgcp_strline((char *)msg->l3h, &pdata.save); - rc = mgcp_parse_header(&pdata, data); + data = mgcp_strline((char *)msg->l3h, &pdata->save); + rc = mgcp_parse_header(pdata, data); if (rc < 0) { - LOGP(DLMGCP, LOGL_ERROR, "%s: failed to parse MCGP message\n", rq.name); + LOGP(DLMGCP, LOGL_ERROR, "%s: failed to parse MCGP message\n", rq->name); rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_GENERAL_RX_FAIL_MSG_PARSE)); - return create_err_response(cfg, NULL, -rc, rq.name, "000000"); + resp = create_err_response(cfg, NULL, -rc, rq->name, "000000"); + goto ret_free; } /* Locate endpoint and trunk, if no endpoint can be located try at least to identify the trunk. */ - rq.pdata = &pdata; - rq.wildcarded = mgcp_endp_is_wildcarded(pdata.epname); - if (!rq.wildcarded) - rq.null_endp = mgcp_endp_is_null(pdata.epname); - if (!rq.null_endp) - rq.endp = mgcp_endp_by_name(&rc, pdata.epname, rq.cfg); - rq.mgcp_cause = rc; - if (!rq.endp) { + rq->wildcarded = mgcp_endp_is_wildcarded(pdata->epname); + if (!rq->wildcarded) + rq->null_endp = mgcp_endp_is_null(pdata->epname); + if (!rq->null_endp) + rq->endp = mgcp_endp_by_name(&rc, pdata->epname, rq->cfg); + rq->mgcp_cause = rc; + if (!rq->endp) { rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_GENERAL_RX_FAIL_NO_ENDPOINT)); - if (rq.wildcarded) { + if (rq->wildcarded) { /* If we are unable to find the endpoint we still may be able to identify the trunk. Some * request handlers will still be able to perform a useful action if the request refers to * the whole trunk (wildcarded request). */ LOGP(DLMGCP, LOGL_NOTICE, - "%s: cannot find endpoint \"%s\", cause=%d -- trying to identify trunk...\n", rq.name, - pdata.epname, -rq.mgcp_cause); - rq.trunk = mgcp_trunk_by_name(rq.cfg, pdata.epname); - if (!rq.trunk) { + "%s: cannot find endpoint \"%s\", cause=%d -- trying to identify trunk...\n", rq->name, + pdata->epname, -rq->mgcp_cause); + rq->trunk = mgcp_trunk_by_name(rq->cfg, pdata->epname); + if (!rq->trunk) { LOGP(DLMGCP, LOGL_ERROR, "%s: failed to identify trunk for endpoint \"%s\" -- abort\n", - rq.name, pdata.epname); - return create_err_response(cfg, NULL, -rq.mgcp_cause, rq.name, pdata.trans); + rq->name, pdata->epname); + resp = create_err_response(cfg, NULL, -rq->mgcp_cause, rq->name, pdata->trans); + goto ret_free; } - } else if (!rq.null_endp) { + } else if (!rq->null_endp) { /* If the endpoint name suggests that the request refers to a specific endpoint, then the * request cannot be handled and we must stop early. */ LOGP(DLMGCP, LOGL_NOTICE, - "%s: cannot find endpoint \"%s\", cause=%d -- abort\n", rq.name, - pdata.epname, -rq.mgcp_cause); - return create_err_response(cfg, NULL, -rq.mgcp_cause, rq.name, pdata.trans); - } /* else: Handle special "null" endpoint below (with rq.endp=NULL, rq.trunk=NULL) */ + "%s: cannot find endpoint \"%s\", cause=%d -- abort\n", rq->name, + pdata->epname, -rq->mgcp_cause); + resp = create_err_response(cfg, NULL, -rq->mgcp_cause, rq->name, pdata->trans); + goto ret_free; + } /* else: Handle special "null" endpoint below (with rq->endp=NULL, rq->trunk=NULL) */ } else { - osmo_strlcpy(debug_last_endpoint_name, rq.endp->name, sizeof(debug_last_endpoint_name)); - rq.trunk = rq.endp->trunk; - rq.mgcp_cause = 0; + osmo_strlcpy(debug_last_endpoint_name, rq->endp->name, sizeof(debug_last_endpoint_name)); + rq->trunk = rq->endp->trunk; + rq->mgcp_cause = 0; /* Check if we have to retransmit a response from a previous transaction */ - if (pdata.trans && rq.endp->last_trans && strcmp(rq.endp->last_trans, pdata.trans) == 0) { + if (pdata->trans && rq->endp->last_trans && strcmp(rq->endp->last_trans, pdata->trans) == 0) { rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_GENERAL_RX_MSGS_RETRANSMITTED)); - return create_retransmission_response(rq.endp); + resp = create_retransmission_response(rq->endp); + goto ret_free; } } /* Execute request handler */ - if (rq.endp) + if (rq->endp) LOGP(DLMGCP, LOGL_INFO, "%s: executing request handler for endpoint resource \"%s\"\n", - rq.name, rq.endp->name); + rq->name, rq->endp->name); else LOGP(DLMGCP, LOGL_INFO, "%s: executing request handler for trunk resource of endpoint \"%s\"\n", - rq.name, pdata.epname); - switch (rq.verb) { + rq->name, pdata->epname); + switch (rq->verb) { case MGCP_VERB_AUEP: - resp = handle_audit_endpoint(&rq); + resp = handle_audit_endpoint(rq); break; case MGCP_VERB_CRCX: - resp = handle_create_con(&rq); + resp = handle_create_con(rq); break; case MGCP_VERB_DLCX: - resp = handle_delete_con(&rq); + resp = handle_delete_con(rq); break; case MGCP_VERB_MDCX: - resp = handle_modify_con(&rq); + resp = handle_modify_con(rq); break; case MGCP_VERB_RQNT: - resp = handle_noti_req(&rq); + resp = handle_noti_req(rq); break; case MGCP_VERB_RSIP: - resp = handle_rsip(&rq); + resp = handle_rsip(rq); break; default: OSMO_ASSERT(0); } rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_GENERAL_RX_MSGS_HANDLED)); +ret_free: + talloc_free(rq); return resp; } -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39722?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I2b971d0c8268f4c0a30b84b54a2e5f16ada4ecdb Gerrit-Change-Number: 39722 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

4 months

1
0
0 0

2025

2024

2023

2022

gerrit-log March 2025