Attention is currently required from: laforge, neels, osmith. pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-msc/+/40630?usp=email ) Change subject: msc: Initial implementation of N-PCSTATE.ind ...................................................................... Patch Set 3: (10 comments) Patchset: PS3:
some optional nitpicks.. Done
File include/osmocom/msc/ran_peer.h: https://gerrit.osmocom.org/c/osmo-msc/+/40630/comment/5dc978db_d565762c?usp=... : PS3, Line 94: struct ran_peer *ran_peer_find_by_addr(const struct sccp_ran_inst *sri, const struct osmo_sccp_addr *peer_addr);
please consider placing orthogonal cosmetic fixes of prior code in a separate patch, because you sho […] Done
File src/libmsc/ran_peer.c: https://gerrit.osmocom.org/c/osmo-msc/+/40630/comment/a51992a2_2c86a264?usp=... : PS3, Line 131: /* N-PCSTATE.ind informs us the peer went down and is no longer reachable: */
in this comment i expect to read what the *function* does Done
File src/libmsc/sccp_ran.c: https://gerrit.osmocom.org/c/osmo-msc/+/40630/comment/96c603d3_97769765?usp=... : PS3, Line 63: n r
typo Done
https://gerrit.osmocom.org/c/osmo-msc/+/40630/comment/6c391fe4_c39670cd?usp=... : PS3, Line 63: address
you mean PC? PC is a subset of possible SCCP addresses It doesn't really matter, since the SSN is known/hardcoded in this case.
https://gerrit.osmocom.org/c/osmo-msc/+/40630/comment/42ca709f_41e228dd?usp=... : PS3, Line 64: get_ran_
below you call function ran_peer_find_by_addr(). […] Done
https://gerrit.osmocom.org/c/osmo-msc/+/40630/comment/85d4da41_6a02a2b0?usp=... : PS3, Line 74: LOGP(DMSC, LOGL_DEBUG, "No ran_peer found under remote address: %s\n", osmo_sccp_addr_name(cs7, &rem_addr));
(if this function becomes public API: error logging should be done at the caller, because these help […] Done
https://gerrit.osmocom.org/c/osmo-msc/+/40630/comment/6b0d894d_0ae1e076?usp=... : PS3, Line 85: LOGP(DMSC, LOGL_DEBUG, "N-PCSTATE ind: affected_pc=%u=%s sp_status=%s remote_sccp_status=%s\n",
(not sure if we should log *all* N-PCSTATE events) It's in debug, so sure we should.
https://gerrit.osmocom.org/c/osmo-msc/+/40630/comment/facd68d8_674f01aa?usp=... : PS3, Line 95: /* See if this marks the point code to have become available, or to have been lost.
hehe i know this comment from somewhere =) high five Yeah I'm reusing stuff from osmo-bsc which had better support already,
https://gerrit.osmocom.org/c/osmo-msc/+/40630/comment/050e2f92_9270ddc7?usp=... : PS3, Line 146: }
(... this logic exists also in osmo-bsc. […] No, I believe this is really logic of apps should take care of, specially since we'll be improving it at different pace in different apps in the future.
-- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/40630?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Ice1b2c163b1b0d134fcaa1c8bf543038a35fabdf Gerrit-Change-Number: 40630 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr@sysmocom.de> Gerrit-Reviewer: osmith <osmith@sysmocom.de> Gerrit-Reviewer: pespin <pespin@sysmocom.de> Gerrit-CC: laforge <laforge@osmocom.org> Gerrit-Attention: osmith <osmith@sysmocom.de> Gerrit-Attention: neels <nhofmeyr@sysmocom.de> Gerrit-Attention: laforge <laforge@osmocom.org> Gerrit-Comment-Date: Mon, 14 Jul 2025 15:34:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr@sysmocom.de>