Patch Set 1: (5 comments)
https://gerrit.osmocom.org/#/c/3187/1/include/osmocom/ranap/iu_client.h File include/osmocom/ranap/iu_client.h: Line 26: /* TODO: It's not needed to store the full SCCP address for each > I took a brief look and the causality is as follows: It probably makes sense to create/lookup a RNC data structure when receiving N-CONNECT.ind. After all, the source SCCP address is known at this time, and we probably need some kind of context structure to identify the peer to whom we're talking to. This is similar to the AoIP case, where we need to look up the BSC that's sending us the N-CONNECT.ind, andwe still have an ongoing discussion if and when to auto-create BSC structures or whether they must always be manually defined in the VTY. Conceptually it's exactly the same in the Iu case. The only difference is that it's happening in libiu, as it is the same code for both SGSN and MSC. I'll leave it up to you to decide on the details, but I think we should avoid introducing something that we know is wrong (and needs to be changed) now. Line 43: * ranap_ue_conn_ctx should no longer be used, for whatever reason. */ > same here. I'll drop the FIXME because there is no need. Whichever user wan fine with me. Line 54: /* TODO "gprs_" in generic CS+PS domain ^ */ > more appropriate would be struct osmo_routing_area_id, or could replace wit definitely use a structure. I don't mind whcih ones, but having a handful of function arguments definitely seems less appealing. https://gerrit.osmocom.org/#/c/3187/1/src/iu_client_vty.c File src/iu_client_vty.c: Line 35: "logging asn1-debug (1|0)", > I think I added these commands back in the days upon you requesting VTY com well, those are useful commands to have. If they don't relate to our existing logging though, they should simply be called differently until we (might eventually) have new true "logging" commands. This is a bit similar to e.g. configuring sysmobts trace flags on osmo-bts-sysmo: We allow to confiugure them (persistently under a config node), but we don't call it "logging" to avoid confusion with the actual libosmocore logging Line 80: DEFUN(cfg_iu_local_addr_pc, cfg_iu_local_addr_pc_cmd, > you introduced this in the port-to-new-sigtran commit; the address book cam difficult choice. Having a global namespace where then the indidivual address book record points to the sccp instance makes probably sense. But I think there were some technical reasons preventing it? Philipp? -- To view, visit https://gerrit.osmocom.org/3187 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6a3f7ad15be03fb94689b4af6ccfa828c25f45c0 Gerrit-PatchSet: 1 Gerrit-Project: osmo-iuh Gerrit-Branch: master Gerrit-Owner: Neels Hofmeyr <[email protected]> Gerrit-Reviewer: Harald Welte <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <[email protected]> Gerrit-HasComments: Yes
