Attention is currently required from: daniel, laforge, lynxis lazus. pespin has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309?usp=email )
Change subject: Add TCAP based loadsharing/routing ...................................................................... Patch Set 9: Code-Review-1 (11 comments) Patchset: PS8: > @pespin: I like to keep most of the #ifdef / #endif out of the code. […] I don't want to be looking at functions which turn to be noops 99% of the setups while reading at the code. And the amount of places where the tcap functionaltities need to be hooked is so small I don't see a problem with having some extra ifdefs in the few places where the calls are done. that also helps by a simple grep ENABLE_TCAP figuring out all the places where tcap related features are hooked. File examples/sccp_demo_user.c: https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/8ec02608_e29c8725?usp=email : PS8, Line 30: void *talloc_asn1_ctx; > Will check it again, if it is still required. The asn1 library changed. If it is required, do you mind explaining why? Doesn't seem obvious to me. File src/ipa.c: https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/f08d589b_c596409e?usp=email : PS8, Line 316: ss7_asp_tcap_rx_sccp(as, asp, opc, dpc, msg); > I want to keep the code changes to the other files minimal. […] To start with, I'd prefix it with tcap_* tbh. I can also have a look whether this is the best place to lookup the content. File src/ss7_as_loadshare_tcap.c: https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/02c34788_99b16e16?usp=email : PS8, Line 138: ssn ^= ((pc >> 24) & 0xff); > Wikipedia says it's 14 bits for ITU. […] 3+8+3=14 indeed, I counted wrong from the top of my head :D https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/349aace2_4a47ab9d?usp=email : PS8, Line 273: /** Traffic from the TCAP ASP -> AS -> osmo-stp, only used to update transaction tracking > Yes it is. If TCAP is enabled on the ASP, it must track both, Rx and Tx. Done https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/f5e0cd0c_b7bd1fba?usp=email : PS8, Line 380: static int sent_back_utds(struct osmo_ss7_as *as, > It sends back an SCCP/UTDS back to the origin I need to investigate about this: * The meanding of sending that UTDS thing I don't know about * Whether ROUTE-FAIL.ind would be enough * Whether the sending back up the stack would require some sort of async queue like ROUTE-FAIL.ind to avoid sending a primtiive to the user while it's busy sending the previous request primitive. Also, some spec reference here would be nice. https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/a1dbbab8_4ca39f3a?usp=email : PS8, Line 487: /* decode SCCP and convert to a SUA/xUA representation */ > I don't really see the gain from move the code back into an own function with > a lot of pointers. […] Done https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/0b32957c_ff879ff1?usp=email : PS8, Line 560: sent_back_utds(as, xua, sua, SCCP_RETURN_CAUSE_SUBSYSTEM_FAILURE); > An UTDS was requested. I don't know enough of SCCP. […] I need to investigate about this: * The meanding of sending that UTDS thing I don't know about * Whether ROUTE-FAIL.ind would be enough * Whether the sending back up the stack would require some sort of async queue like ROUTE-FAIL.ind to avoid sending a primtiive to the user while it's busy sending the previous request primitive. Also, some spec reference here would be nice. https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/bff75e23_5ec772bc?usp=email : PS8, Line 607: * @return 0 on success > There is a difference between (rc = EPROTONOSUPPORT) and (rc = 0 && asp == > NULL.) […] afaiu this should only return the asp, no need to return an rc? File src/ss7_asp.h: https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/9b30ad68_34e55cba?usp=email : PS8, Line 96: bool enabled; > Not all ASP of an AS has it enabled. […] Does that make sense? why would somebody want to have TCAP routing enabled on some and not all of the ASPs serving an AS? File src/ss7_asp.c: https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/04268b90_8a906d6d?usp=email : PS8, Line 1344: tcap_asp_down(asp); > Where do you think it belongs exactly? I'm not used to this code base. I can have a look. -- To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309?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: Ibcb48aa0e515ad346f59ddd84b24c6e2c026144d Gerrit-Change-Number: 41309 Gerrit-PatchSet: 9 Gerrit-Owner: lynxis lazus <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <[email protected]> Gerrit-CC: daniel <[email protected]> Gerrit-CC: laforge <[email protected]> Gerrit-Attention: laforge <[email protected]> Gerrit-Attention: daniel <[email protected]> Gerrit-Attention: lynxis lazus <[email protected]> Gerrit-Comment-Date: Mon, 10 Nov 2025 18:32:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: pespin <[email protected]> Comment-In-Reply-To: lynxis lazus <[email protected]>
