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 11:
(13 comments)
File src/ipa.c:
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/7e879692_e8bb0d... : PS8, Line 51: #include "ss7_as_loadshare_tcap.h"
#ifdef WITH_TCAP_LOADSHARING
Done
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/e017bfa1_635690... : PS8, Line 315: /* update TCAP Transaction Tracking (Rx) */
no, not needed.
Done
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/b8024c49_a656ac... : PS8, Line 368: if (hh_ext->proto == IPAC_PROTO_EXT_TCAP_ROUTING) {
Should be ok with the ifdef encompassing the whole case.
Done
File src/ss7_as.h:
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/f931cda8_bfcb61... : PS8, Line 6: #include <osmocom/core/hashtable.h>
Does the include hurt? I would try to prevent cluttering #ifdefs everywhere.
Here I would agree.
File src/ss7_as.c:
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/75f526fe_33bc5f... : PS8, Line 246: tcap_disable(as);
#ifdef WITH_TCAP_LOADSHARING
Done
File src/ss7_as_loadshare_tcap.h:
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/9e30b12d_26a6be... : PS8, Line 14: #ifdef WITH_TCAP_LOADSHARING
No, I want to keep it here because of the #else branch.
Done
File src/ss7_as_loadshare_tcap.c:
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/5e99493a_92a6d0... : PS8, Line 379: /* FIXME: use UTDS */
This needs to be fixed?
It's used in asp_loadshare_tcap_sccp. @lynxis@fe80.eu this comment can be removed?
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/0b931d51_b4796e... : PS8, Line 752: LOGPASP(asp, DLSS7, LOGL_ERROR, "TCAP routing message is too small\n");
I think we really want to add a new log category "TCAP" here.
ACK
File src/ss7_as_vty.c:
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/8f450e8c_ad0e55... : PS8, Line 39: #include "ss7_as_loadshare_tcap.h"
No
Done
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/c6f40028_8ae8b7... : PS8, Line 158: tcap_disable(as);
No, not needed.
Done
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/dd2d7ab1_6a5628... : PS8, Line 165: "traffic-mode (loadshare|loadshare-tcap) [bindings] [sls] [opc-sls] [opc-shift] [<0-2>]",
I'm not 100% familiar with the tcap_enable/disable hooks, but wouldn't you need to call tcap_disable […]
Done
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/fef869f3_7afc22... : PS8, Line 186: vty_out(vty, "tcap loadsharing is not supported in this build.\n");
This is wrong, will fix it in v2
Done
File src/ss7_asp.c:
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/ea3225b4_fabd57... : PS8, Line 53: #include "ss7_as_loadshare_tcap.h"
#ifdef WITH_TCAP_LOADSHARING
Done
-
daniel