Attention is currently required from: laforge, lynxis lazus, pespin. daniel 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 10: (20 comments) File examples/sccp_demo_user.c: https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/ac650df0_151b4759?usp=email : PS8, Line 30: void *talloc_asn1_ctx; > If it is required, do you mind explaining why? Doesn't seem obvious to me. It's not required, will remove it File src/ipa.c: https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/f2b47446_acc5796d?usp=email : PS8, Line 360: case IPAC_PROTO_OSMO: > I could move it further upwards. Done https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/d30c31ae_973fcefc?usp=email : PS8, Line 368: if (hh_ext->proto == IPAC_PROTO_EXT_TCAP_ROUTING) { > yes, I can use switch/case there. Should be ok with the ifdef encompassing the whole case. File src/m3ua.c: https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/7238899a_81ed7dfd?usp=email : PS8, Line 50: #include "ss7_as_loadshare_tcap.h" > I prefer the style how it is currently. It reduces the usage of #ifdef in the > common code. Not needed here, removed https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/5a5b3d3f_9a5975e6?usp=email : PS8, Line 343: if (!xua) > Ack. Done File src/ss7_as.h: https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/e34133a1_15cd0a33?usp=email : PS8, Line 97: /* optimisation: true if tid_ranges contains PCs (not only wildcards) */ > #ifdef WITH_TCAP_LOADSHARING Done https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/feae522b_d41b7103?usp=email : PS8, Line 145: /* Should we do load-sharing based on tcap ids? */ > #ifdef WITH_TCAP_LOADSHARING Done File src/ss7_as.c: https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/a6382c8a_30e8989e?usp=email : PS8, Line 139: hash_init(as->tcap.tid_ranges); > #ifdef WITH_TCAP_LOADSHARING Done https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/757b4052_aea76d93?usp=email : PS8, Line 545: case OSMO_SS7_AS_TMOD_LOADSHARE: > This is fine. Done https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/779e6bed_26960371?usp=email : PS8, Line 634: > Acknowledged Done File src/ss7_as_loadshare_tcap.h: https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/ee27e2c7_e8d9d1cc?usp=email : PS8, Line 3: #include <complex.h> > I wonder why do we need this here... Acknowledged File src/ss7_as_loadshare_tcap.c: https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/40b30e96_916a3644?usp=email : PS8, Line 61: return osmo_ntohl(*(uint32_t *)src->buf); > is this always properly aligned? maybe use osmo_load_32be or however it's > called? Done https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/e4fc72e4_d93f9bd9?usp=email : PS8, Line 72: tcapmsg = talloc_zero(as, struct TCAP_TCMessage); > Can we avoid talloc-allocating a new chunk for every message? Simply "struct > TCAP_TCMessage tcapmsg […] Done https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/fcd4dd97_98e33a3e?usp=email : PS8, Line 159: static struct osmo_ss7_asp *tcap_hlist_get(struct osmo_ss7_as *as, uint32_t pc, uint8_t ssn, uint32_t tid) > const as? Acknowledged File src/ss7_as_vty.c: https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/6d99e087_6b46ceda?usp=email : PS8, Line 165: "traffic-mode (loadshare|loadshare-tcap) [bindings] [sls] [opc-sls] [opc-shift] [<0-2>]", > traffic-mode loadshare-tcap is an extension to the general loadshare. […] I'm not 100% familiar with the tcap_enable/disable hooks, but wouldn't you need to call tcap_disable in other traffic modes even with the separate command? It's always possible to ``` traffic-mode loadshare tcap-loadshare [...later] traffic-mode roundrobin ``` and then you'd end up in a bad state (if calling tcap_disable is required). File src/ss7_asp.c: https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/87fc4c21_6f04743f?usp=email : PS8, Line 1435: /*! Received an unknown packet on asp > Will look into it. Done File src/ss7_internal.h: https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/90db1725_095055d5?usp=email : PS8, Line 9: #include <osmocom/sigtran/mtp_sap.h> > why is this now needed here? if needed, please submit another commit. Done File src/tcap_transaction_tracking.c: https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/87ba06d1_706ae06b?usp=email : PS8, Line 122: LOGPASP(entry->asp, DLSS7, LOGL_DEBUG, "Creating tcap cache, entry (own) pc/ssn/tid %u/%u/%u -> %u/%u/%u\n", > Ack Done File src/xua_as_fsm.c: https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/41902dc8_f40ba430?usp=email : PS8, Line 204: asp = ss7_as_select_asp(as, xua); > This could actually go in a preparatory patch to simplify this one a bit. Done File stp/stp_main.c: https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/1cba7d0b_e3f6b92d?usp=email : PS8, Line 52: void *talloc_asn1_ctx; > Need to check it again, if it is still required. It's not needed, removed -- 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: 10 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: pespin <[email protected]> Gerrit-Attention: lynxis lazus <[email protected]> Gerrit-Comment-Date: Thu, 13 Nov 2025 16:47:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <[email protected]> Comment-In-Reply-To: lynxis lazus <[email protected]>
