Change in osmo-hlr[master]: fix and re-enable osmo_hlr_subscriber_update_notify()
Harald Welte has posted comments on this change. ( https://gerrit.osmocom.org/11233 ) Change subject: fix and re-enable osmo_hlr_subscriber_update_notify() .. Patch Set 6: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/11233 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c317de8329d9a115d072fc61ddb9abc21b7e8d8 Gerrit-Change-Number: 11233 Gerrit-PatchSet: 6 Gerrit-Owner: Neels Hofmeyr Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Neels Hofmeyr Gerrit-Comment-Date: Sun, 21 Oct 2018 12:28:40 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in osmo-hlr[master]: fix and re-enable osmo_hlr_subscriber_update_notify()
Harald Welte has submitted this change and it was merged. (
https://gerrit.osmocom.org/11233 )
Change subject: fix and re-enable osmo_hlr_subscriber_update_notify()
..
fix and re-enable osmo_hlr_subscriber_update_notify()
Send updated subscriber data out to exactly those GSUP clients that match the
last LU operations (depending on each client sending distinct identification).
As this adds logging on DLGSUP, also change adjacent GSUP related logging from
DMAIN to DLGSUP.
Related: OS#2785
Change-Id: I7c317de8329d9a115d072fc61ddb9abc21b7e8d8
---
M src/hlr.c
1 file changed, 42 insertions(+), 23 deletions(-)
Approvals:
Jenkins Builder: Verified
Harald Welte: Looks good to me, approved
diff --git a/src/hlr.c b/src/hlr.c
index 26be8d5..78d6c91 100644
--- a/src/hlr.c
+++ b/src/hlr.c
@@ -54,16 +54,15 @@
void
osmo_hlr_subscriber_update_notify(struct hlr_subscriber *subscr)
{
- /* FIXME: the below code can only be re-enabled after we make sure that
an ISD
-* is only sent tot the currently serving VLR and/or SGSN (if there are
any).
-* We cannot blindly flood the entire PLMN with this, as it would
create subscriber
-* state in every VLR/SGSN out there, even those that have never seen
the subscriber.
-* See https://osmocom.org/issues/3154 for details. */
-#if 0
struct osmo_gsup_conn *co;
- if (g_hlr->gs == NULL)
+ if (g_hlr->gs == NULL) {
+ LOGP(DLGSUP, LOGL_DEBUG,
+"IMSI %s: NOT Notifying peers of subscriber data change,"
+" there is no GSUP server\n",
+subscr->imsi);
return;
+ }
llist_for_each_entry(co, _hlr->gs->clients, list) {
struct osmo_gsup_message gsup = { };
@@ -72,20 +71,50 @@
struct msgb *msg_out;
uint8_t *peer;
int peer_len;
+ size_t peer_strlen;
+ const char *peer_compare;
enum osmo_gsup_cn_domain cn_domain;
- if (co->supports_ps)
+ if (co->supports_ps) {
cn_domain = OSMO_GSUP_CN_DOMAIN_PS;
- else if (co->supports_cs)
+ peer_compare = subscr->sgsn_number;
+ } else if (co->supports_cs) {
cn_domain = OSMO_GSUP_CN_DOMAIN_CS;
- else {
- /* We have not yet received a location update from this
subscriber .*/
+ peer_compare = subscr->vlr_number;
+ } else {
+ /* We have not yet received a location update from this
GSUP client.*/
continue;
}
+ peer_len = osmo_gsup_conn_ccm_get(co, , IPAC_IDTAG_SERNR);
+ if (peer_len < 0) {
+ LOGP(DLGSUP, LOGL_ERROR,
+ "IMSI='%s': cannot get peer name for connection
%s:%u\n", subscr->imsi,
+ co && co->conn && co->conn->server?
co->conn->server->addr : "unset",
+ co && co->conn && co->conn->server?
co->conn->server->port : 0);
+ continue;
+ }
+
+ peer_strlen = strnlen((const char*)peer, peer_len);
+ if (strlen(peer_compare) != peer_strlen ||
strncmp(peer_compare, (const char *)peer, peer_len)) {
+ /* Mismatch. The subscriber is not subscribed with this
GSUP client. */
+ /* I hope peer is always nul terminated... */
+ if (peer_strlen < peer_len)
+ LOGP(DLGSUP, LOGL_DEBUG,
+"IMSI %s: subscriber change: skipping %s
peer %s\n",
+subscr->imsi, cn_domain ==
OSMO_GSUP_CN_DOMAIN_PS ? "PS" : "CS",
+osmo_quote_str((char*)peer, -1));
+ continue;
+ }
+
+ LOGP(DLGSUP, LOGL_DEBUG,
+"IMSI %s: subscriber change: notifying %s peer %s\n",
+subscr->imsi, cn_domain == OSMO_GSUP_CN_DOMAIN_PS ? "PS" :
"CS",
+osmo_quote_str(peer_compare, -1));
+
if (osmo_gsup_create_insert_subscriber_data_msg(,
subscr->imsi, subscr->msisdn, msisdn_enc,
sizeof(msisdn_enc), apn, sizeof(apn), cn_domain) != 0) {
- LOGP(DMAIN, LOGL_ERROR,
+ LOGP(DLGSUP, LOGL_ERROR,
"IMSI='%s': Cannot notify GSUP client; could not
create gsup message "
"for %s:%u\n", subscr->imsi,
co && co->conn && co->conn->server?
co->conn->server->addr : "unset",
@@ -96,7 +125,7 @@
/* Send ISD to
Change in osmo-hlr[master]: fix and re-enable osmo_hlr_subscriber_update_notify()
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/11233 to look at the new patch set (#5). Change subject: fix and re-enable osmo_hlr_subscriber_update_notify() .. fix and re-enable osmo_hlr_subscriber_update_notify() Send updated subscriber data out to exactly those GSUP clients that match the last LU operations (depending on each client sending distinct identification). As this adds logging on DLGSUP, also change adjacent GSUP related logging from DMAIN to DLGSUP. Related: OS#2785 Change-Id: I7c317de8329d9a115d072fc61ddb9abc21b7e8d8 --- M src/hlr.c 1 file changed, 42 insertions(+), 23 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/33/11233/5 -- To view, visit https://gerrit.osmocom.org/11233 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7c317de8329d9a115d072fc61ddb9abc21b7e8d8 Gerrit-Change-Number: 11233 Gerrit-PatchSet: 5 Gerrit-Owner: Neels Hofmeyr Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Neels Hofmeyr
Change in osmo-hlr[master]: fix and re-enable osmo_hlr_subscriber_update_notify()
Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/11233 ) Change subject: fix and re-enable osmo_hlr_subscriber_update_notify() .. Patch Set 4: (1 comment) https://gerrit.osmocom.org/#/c/11233/4/src/hlr.c File src/hlr.c: https://gerrit.osmocom.org/#/c/11233/4/src/hlr.c@71 PS4, Line 71:osmo_quote_str(subscr->sgsn_number, -1)); actually, tow osmo_quote_str() on the same log won't work. Introducing another name buffer just to log this doesn't seem worth it, particularly since the loop logs each one again. dropping. -- To view, visit https://gerrit.osmocom.org/11233 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c317de8329d9a115d072fc61ddb9abc21b7e8d8 Gerrit-Change-Number: 11233 Gerrit-PatchSet: 4 Gerrit-Owner: Neels Hofmeyr Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Neels Hofmeyr Gerrit-Comment-Date: Thu, 11 Oct 2018 12:09:20 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in osmo-hlr[master]: fix and re-enable osmo_hlr_subscriber_update_notify()
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/11233 to look at the new patch set (#4). Change subject: fix and re-enable osmo_hlr_subscriber_update_notify() .. fix and re-enable osmo_hlr_subscriber_update_notify() Send updated subscriber data out to exactly those GSUP clients that match the last LU operations (depending on each client sending distinct identification). As this adds logging on DLGSUP, also change adjacent GSUP related logging from DMAIN to DLGSUP. Related: OS#2785 Change-Id: I7c317de8329d9a115d072fc61ddb9abc21b7e8d8 --- M src/hlr.c 1 file changed, 48 insertions(+), 23 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/33/11233/4 -- To view, visit https://gerrit.osmocom.org/11233 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7c317de8329d9a115d072fc61ddb9abc21b7e8d8 Gerrit-Change-Number: 11233 Gerrit-PatchSet: 4 Gerrit-Owner: Neels Hofmeyr Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Neels Hofmeyr
Change in osmo-hlr[master]: fix and re-enable osmo_hlr_subscriber_update_notify()
Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/11233 ) Change subject: fix and re-enable osmo_hlr_subscriber_update_notify() .. Patch Set 3: (1 comment) https://gerrit.osmocom.org/#/c/11233/3/src/hlr.c File src/hlr.c: https://gerrit.osmocom.org/#/c/11233/3/src/hlr.c@70 PS3, Line 70:subscr->imsi, subscr->vlr_number, subscr->sgsn_number); needs string escaping like in the preceding patch -- To view, visit https://gerrit.osmocom.org/11233 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c317de8329d9a115d072fc61ddb9abc21b7e8d8 Gerrit-Change-Number: 11233 Gerrit-PatchSet: 3 Gerrit-Owner: Neels Hofmeyr Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Neels Hofmeyr Gerrit-Comment-Date: Thu, 11 Oct 2018 12:04:46 + Gerrit-HasComments: Yes Gerrit-HasLabels: No
Change in osmo-hlr[master]: fix and re-enable osmo_hlr_subscriber_update_notify()
Neels Hofmeyr has uploaded this change for review. (
https://gerrit.osmocom.org/11233
Change subject: fix and re-enable osmo_hlr_subscriber_update_notify()
..
fix and re-enable osmo_hlr_subscriber_update_notify()
Send updated subscriber data out to exactly those GSUP clients that match the
last LU operations (depending on each client sending distinct identification).
As this adds logging on DLGSUP, also change adjacent GSUP related logging from
DMAIN to DLGSUP.
Related: OS#2785
Change-Id: I7c317de8329d9a115d072fc61ddb9abc21b7e8d8
---
M src/hlr.c
1 file changed, 45 insertions(+), 23 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/33/11233/1
diff --git a/src/hlr.c b/src/hlr.c
index cf4871e..4ed8a7c 100644
--- a/src/hlr.c
+++ b/src/hlr.c
@@ -54,16 +54,20 @@
void
osmo_hlr_subscriber_update_notify(struct hlr_subscriber *subscr)
{
- /* FIXME: the below code can only be re-enabled after we make sure that
an ISD
-* is only sent tot the currently serving VLR and/or SGSN (if there are
any).
-* We cannot blindly flood the entire PLMN with this, as it would
create subscriber
-* state in every VLR/SGSN out there, even those that have never seen
the subscriber.
-* See https://osmocom.org/issues/3154 for details. */
-#if 0
struct osmo_gsup_conn *co;
- if (g_hlr->gs == NULL)
+ if (g_hlr->gs == NULL) {
+ LOGP(DLGSUP, LOGL_DEBUG,
+"IMSI %s: NOT Notifying peers of subscriber data change,"
+" there is no GSUP server\n",
+subscr->imsi);
return;
+ }
+
+ LOGP(DLGSUP, LOGL_DEBUG,
+"IMSI %s: Notifying peers of subscriber data change"
+" (VLR number: '%s', SGSN number: '%s')\n",
+subscr->imsi, subscr->vlr_number, subscr->sgsn_number);
llist_for_each_entry(co, _hlr->gs->clients, list) {
struct osmo_gsup_message gsup = { };
@@ -72,20 +76,48 @@
struct msgb *msg_out;
uint8_t *peer;
int peer_len;
+ size_t peer_strlen;
+ const char *peer_compare;
enum osmo_gsup_cn_domain cn_domain;
- if (co->supports_ps)
+ if (co->supports_ps) {
cn_domain = OSMO_GSUP_CN_DOMAIN_PS;
- else if (co->supports_cs)
+ peer_compare = subscr->sgsn_number;
+ } else if (co->supports_cs) {
cn_domain = OSMO_GSUP_CN_DOMAIN_CS;
- else {
- /* We have not yet received a location update from this
subscriber .*/
+ peer_compare = subscr->vlr_number;
+ } else {
+ /* We have not yet received a location update from this
GSUP client.*/
continue;
}
+ peer_len = osmo_gsup_conn_ccm_get(co, , IPAC_IDTAG_SERNR);
+ if (peer_len < 0) {
+ LOGP(DLGSUP, LOGL_ERROR,
+ "IMSI='%s': cannot get peer name for connection
%s:%u\n", subscr->imsi,
+ co && co->conn && co->conn->server?
co->conn->server->addr : "unset",
+ co && co->conn && co->conn->server?
co->conn->server->port : 0);
+ continue;
+ }
+
+ peer_strlen = strnlen((const char*)peer, peer_len);
+ if (strlen(peer_compare) != peer_strlen ||
strncmp(peer_compare, (const char *)peer, peer_len)) {
+ /* Mismatch. The subscriber is not subscribed with this
GSUP client. */
+ /* I hope peer is always nul terminated... */
+ if (peer_strlen < peer_len)
+ LOGP(DLGSUP, LOGL_DEBUG,
+"IMSI %s: subscriber change: skipping %s
peer %s\n",
+subscr->imsi, cn_domain ==
OSMO_GSUP_CN_DOMAIN_PS ? "PS" : "CS", peer);
+ continue;
+ }
+
+ LOGP(DLGSUP, LOGL_DEBUG,
+"IMSI %s: subscriber change: notifying %s peer %s\n",
+subscr->imsi, cn_domain == OSMO_GSUP_CN_DOMAIN_PS ? "PS" :
"CS", peer_compare);
+
if (osmo_gsup_create_insert_subscriber_data_msg(,
subscr->imsi, subscr->msisdn, msisdn_enc,
sizeof(msisdn_enc), apn, sizeof(apn), cn_domain) != 0) {
- LOGP(DMAIN, LOGL_ERROR,
+ LOGP(DLGSUP, LOGL_ERROR,
"IMSI='%s': Cannot notify GSUP client; could not
create gsup message "
"for %s:%u\n", subscr->imsi,
co && co->conn
