Attention is currently required from: pespin.
lynxis lazus 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 8:
(10 comments)
Patchset:
PS8: @pespin: I like to keep most of the #ifdef / #endif out of the code. The header file already takes care if the feature isn't enabled. Why do you want to have the #ifdef everywhere in the code? The header file transform it into no-ops.
File examples/sccp_demo_user.c:
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/b57d649e_3fcade... : PS8, Line 30: void *talloc_asn1_ctx;
why is this added?
Because the library needs it.
File src/ipa.c:
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/cc96cf5f_86b020... : PS8, Line 315: /* update TCAP Transaction Tracking (Rx) */
#ifdef WITH_TCAP_LOADSHARING
no, not needed.
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/ccabd2eb_642d91... : PS8, Line 316: ss7_asp_tcap_rx_sccp(as, asp, opc, dpc, msg);
This function naming is a bit misleading since it's not even known whether there's TCAP in the msg.. […]
I want to keep the code changes to the other files minimal. What would you suggest instead?
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/140bbf84_334c50... : PS8, Line 360: case IPAC_PROTO_OSMO:
#ifdef WITH_TCAP_LOADSHARING
I could move it further upwards.
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/349533d8_c8e320... : PS8, Line 368: if (hh_ext->proto == IPAC_PROTO_EXT_TCAP_ROUTING) {
(probably a switch case here will turn out cleaner together with the ifdef. […]
yes, I can use switch/case there.
File src/m3ua.c:
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/763c4907_f6e3e6... : PS8, Line 50: #include "ss7_as_loadshare_tcap.h"
#ifdef WITH_TCAP_LOADSHARINGActually, I don't see this header being used in this file. […]
I prefer the style how it is currently. It reduces the usage of #ifdef in the common code.
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/125b9129_316483... : PS8, Line 343: if (!xua)
This looks unrelated to this commit? Seems you want to add another commit describing this.
Ack.
File tests/tcap/tcap_transaction_tracking_test.c:
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/6c74d55e_52a733... : PS6, Line 19: typedef void (* tcap_trxn_track_test_func_t)(void);
space prohibited after that '*' (ctx:BxW)
fixed
https://gerrit.osmocom.org/c/libosmo-sigtran/+/41309/comment/7aa05f39_9a42b3... : PS6, Line 19: typedef void (* tcap_trxn_track_test_func_t)(void);
do not add new typedefs
Won't fix. I think it's fine here.