osmo-hlr[master]: notify GSUP clients when HLR subscriber information changes
Patch Set 2: > (1 comment) > > about that FIXME, I think there should also be an issue about that, > lest we forget the FIXME. https://osmocom.org/issues/3154 -- To view, visit https://gerrit.osmocom.org/7685 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iffe1d7afb9fc7dbae542f70bbf5391ddc08a14b4 Gerrit-PatchSet: 2 Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Owner: Stefan Sperling Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Stefan Sperling Gerrit-HasComments: No
[MERGED] osmo-hlr[master]: notify GSUP clients when HLR subscriber information changes
Stefan Sperling has submitted this change and it was merged. Change subject: notify GSUP clients when HLR subscriber information changes .. notify GSUP clients when HLR subscriber information changes Add a function which triggers subscriber update notifications to all connected GSUP clients, and invoke it when the MSISDN of a subscriber is changed via VTY. This makes the TTCN3 HLR test TC_vty_msisdn_isd pass. Note that the new function currently relies on implementation details of the Location Update Operation (luop) code. Because of this we currently log a slightly misleading message when the updated Insert Subscriber Data message is sent: "luop.c:161 LU OP state change: LU RECEIVED -> ISD SENT" This message is misleading because, in fact, no location update message was received from a GSUP client at that moment. So while this change fixes the externally visible behaviour, we may want to follow this up with some refactoring to avoid relying on luop internals. It seems acceptable to do that in a separate step since such a change will be more involved and harder to review. We may want to trigger such notifications in other situations as well. This is left for future work, too. There are no TTCN3 test cases for other situations yet, as far as I can see. Related: OS#2785 Change-Id: Iffe1d7afb9fc7dbae542f70bbf5391ddc08a14b4 --- M src/hlr.c M src/hlr.h M src/hlr_vty_subscr.c 3 files changed, 40 insertions(+), 0 deletions(-) Approvals: Neels Hofmeyr: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/hlr.c b/src/hlr.c index 838b1bc..4fbc268 100644 --- a/src/hlr.c +++ b/src/hlr.c @@ -46,6 +46,38 @@ static struct hlr *g_hlr; +/* Trigger 'Insert Subscriber Data' messages to all connected GSUP clients. + * + * FIXME: In order to support large-scale networks this function should skip + * VLRs/SGSNs which do not currently serve the subscriber. + * + * \param[in] subscr A subscriber we have new data to send for. + */ +void +osmo_hlr_subscriber_update_notify(struct hlr_subscriber *subscr) +{ +struct osmo_gsup_conn *co; + + if (g_hlr->gs == NULL) + return; + + llist_for_each_entry(co, &g_hlr->gs->clients, list) { + struct lu_operation *luop = lu_op_alloc_conn(co); + if (!luop) { + LOGP(DMAIN, LOGL_ERROR, + "IMSI='%s': Cannot notify GSUP client, cannot allocate lu_operation," + " for %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; + } + luop->subscr = *subscr; + luop->state = LU_S_LU_RECEIVED; /* Pretend we received a location update. */ + lu_op_tx_insert_subscr_data(luop); + lu_op_free(luop); + } +} + /*** * Send Auth Info handling ***/ diff --git a/src/hlr.h b/src/hlr.h index f63bc2b..368a052 100644 --- a/src/hlr.h +++ b/src/hlr.h @@ -38,3 +38,7 @@ /* Local bind addr */ char *gsup_bind_addr; }; + +struct hlr_subscriber; + +void osmo_hlr_subscriber_update_notify(struct hlr_subscriber *subscr); diff --git a/src/hlr_vty_subscr.c b/src/hlr_vty_subscr.c index 7191a1c..4092a8f 100644 --- a/src/hlr_vty_subscr.c +++ b/src/hlr_vty_subscr.c @@ -257,6 +257,10 @@ vty_out(vty, "%% Updated subscriber IMSI='%s' to MSISDN='%s'%s", subscr.imsi, msisdn, VTY_NEWLINE); + + if (db_subscr_get_by_msisdn(g_hlr->dbc, msisdn, &subscr) == 0) + osmo_hlr_subscriber_update_notify(&subscr); + return CMD_SUCCESS; } -- To view, visit https://gerrit.osmocom.org/7685 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: Iffe1d7afb9fc7dbae542f70bbf5391ddc08a14b4 Gerrit-PatchSet: 2 Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Owner: Stefan Sperling Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Stefan Sperling
osmo-hlr[master]: notify GSUP clients when HLR subscriber information changes
Patch Set 2: Code-Review+2 (1 comment) about that FIXME, I think there should also be an issue about that, lest we forget the FIXME. https://gerrit.osmocom.org/#/c/7685/1/src/hlr.h File src/hlr.h: Line 42: struct hlr_subscriber; (we usually place opaque declarations like this after the #include block) -- To view, visit https://gerrit.osmocom.org/7685 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iffe1d7afb9fc7dbae542f70bbf5391ddc08a14b4 Gerrit-PatchSet: 2 Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Owner: Stefan Sperling Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr Gerrit-Reviewer: Stefan Sperling Gerrit-HasComments: Yes
[PATCH] osmo-hlr[master]: notify GSUP clients when HLR subscriber information changes
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/7685 to look at the new patch set (#2). notify GSUP clients when HLR subscriber information changes Add a function which triggers subscriber update notifications to all connected GSUP clients, and invoke it when the MSISDN of a subscriber is changed via VTY. This makes the TTCN3 HLR test TC_vty_msisdn_isd pass. Note that the new function currently relies on implementation details of the Location Update Operation (luop) code. Because of this we currently log a slightly misleading message when the updated Insert Subscriber Data message is sent: "luop.c:161 LU OP state change: LU RECEIVED -> ISD SENT" This message is misleading because, in fact, no location update message was received from a GSUP client at that moment. So while this change fixes the externally visible behaviour, we may want to follow this up with some refactoring to avoid relying on luop internals. It seems acceptable to do that in a separate step since such a change will be more involved and harder to review. We may want to trigger such notifications in other situations as well. This is left for future work, too. There are no TTCN3 test cases for other situations yet, as far as I can see. Related: OS#2785 Change-Id: Iffe1d7afb9fc7dbae542f70bbf5391ddc08a14b4 --- M src/hlr.c M src/hlr.h M src/hlr_vty_subscr.c 3 files changed, 40 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/85/7685/2 diff --git a/src/hlr.c b/src/hlr.c index 838b1bc..4fbc268 100644 --- a/src/hlr.c +++ b/src/hlr.c @@ -46,6 +46,38 @@ static struct hlr *g_hlr; +/* Trigger 'Insert Subscriber Data' messages to all connected GSUP clients. + * + * FIXME: In order to support large-scale networks this function should skip + * VLRs/SGSNs which do not currently serve the subscriber. + * + * \param[in] subscr A subscriber we have new data to send for. + */ +void +osmo_hlr_subscriber_update_notify(struct hlr_subscriber *subscr) +{ +struct osmo_gsup_conn *co; + + if (g_hlr->gs == NULL) + return; + + llist_for_each_entry(co, &g_hlr->gs->clients, list) { + struct lu_operation *luop = lu_op_alloc_conn(co); + if (!luop) { + LOGP(DMAIN, LOGL_ERROR, + "IMSI='%s': Cannot notify GSUP client, cannot allocate lu_operation," + " for %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; + } + luop->subscr = *subscr; + luop->state = LU_S_LU_RECEIVED; /* Pretend we received a location update. */ + lu_op_tx_insert_subscr_data(luop); + lu_op_free(luop); + } +} + /*** * Send Auth Info handling ***/ diff --git a/src/hlr.h b/src/hlr.h index f63bc2b..368a052 100644 --- a/src/hlr.h +++ b/src/hlr.h @@ -38,3 +38,7 @@ /* Local bind addr */ char *gsup_bind_addr; }; + +struct hlr_subscriber; + +void osmo_hlr_subscriber_update_notify(struct hlr_subscriber *subscr); diff --git a/src/hlr_vty_subscr.c b/src/hlr_vty_subscr.c index 7191a1c..4092a8f 100644 --- a/src/hlr_vty_subscr.c +++ b/src/hlr_vty_subscr.c @@ -257,6 +257,10 @@ vty_out(vty, "%% Updated subscriber IMSI='%s' to MSISDN='%s'%s", subscr.imsi, msisdn, VTY_NEWLINE); + + if (db_subscr_get_by_msisdn(g_hlr->dbc, msisdn, &subscr) == 0) + osmo_hlr_subscriber_update_notify(&subscr); + return CMD_SUCCESS; } -- To view, visit https://gerrit.osmocom.org/7685 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iffe1d7afb9fc7dbae542f70bbf5391ddc08a14b4 Gerrit-PatchSet: 2 Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Owner: Stefan Sperling Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Stefan Sperling
osmo-hlr[master]: notify GSUP clients when HLR subscriber information changes
Patch Set 1: > (1 comment) Subscriber entries in the DB contain a vlr_number and sgsn_number. It seems those are only set, I cannot find code which reads them back. I'll add a comment as you suggest. -- To view, visit https://gerrit.osmocom.org/7685 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iffe1d7afb9fc7dbae542f70bbf5391ddc08a14b4 Gerrit-PatchSet: 1 Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Owner: Stefan Sperling Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Stefan Sperling Gerrit-HasComments: No
osmo-hlr[master]: notify GSUP clients when HLR subscriber information changes
Patch Set 1: (1 comment) https://gerrit.osmocom.org/#/c/7685/1/src/hlr.c File src/hlr.c: Line 49: /* Trigger 'Insert Subscriber Data' messages to all connected GSUP clients. to all GSUP clients is ok for now, as we only expect a single MSC+SGSN to be connected. However, in larger scale networks we should check to update only those MSC/SGSN which are listed in the subscriber data as the "current serving VLR/SGSN" for that subscriber. I believe we already have those fields in our database model, not sure if they're already populated though. In any case, we should at least have a a FIXME here to keep remembering something must be done in the future to avoid ISD into VLRs/SGSNs that are not even serving the subscriber. At that point, the respective "vlr_subscriber" would be allocated/created in them, despite the subscriber not existing. -- To view, visit https://gerrit.osmocom.org/7685 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iffe1d7afb9fc7dbae542f70bbf5391ddc08a14b4 Gerrit-PatchSet: 1 Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Owner: Stefan Sperling Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder Gerrit-HasComments: Yes
[PATCH] osmo-hlr[master]: notify GSUP clients when HLR subscriber information changes
Review at https://gerrit.osmocom.org/7685 notify GSUP clients when HLR subscriber information changes Add a function which triggers subscriber update notifications to all connected GSUP clients, and invoke it when the MSISDN of a subscriber is changed via VTY. This makes the TTCN3 HLR test TC_vty_msisdn_isd pass. Note that the new function currently relies on implementation details of the Location Update Operation (luop) code. Because of this we currently log a slightly misleading message when the updated Insert Subscriber Data message is sent: "luop.c:161 LU OP state change: LU RECEIVED -> ISD SENT" This message is misleading because, in fact, no location update message was received from a GSUP client at that moment. So while this change fixes the externally visible behaviour, we may want to follow this up with some refactoring to avoid relying on luop internals. It seems acceptable to do that in a separate step since such a change will be more involved and harder to review. We may want to trigger such notifications in other situations as well. This is left for future work, too. There are no TTCN3 test cases for other situations yet, as far as I can see. Related: OS#2785 Change-Id: Iffe1d7afb9fc7dbae542f70bbf5391ddc08a14b4 --- M src/hlr.c M src/hlr.h M src/hlr_vty_subscr.c 3 files changed, 37 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/85/7685/1 diff --git a/src/hlr.c b/src/hlr.c index 838b1bc..2ae4285 100644 --- a/src/hlr.c +++ b/src/hlr.c @@ -46,6 +46,35 @@ static struct hlr *g_hlr; +/* Trigger 'Insert Subscriber Data' messages to all connected GSUP clients. + * + * \param[in] subscr A subscriber we have new data to send for. + */ +void +osmo_hlr_subscriber_update_notify(struct hlr_subscriber *subscr) +{ +struct osmo_gsup_conn *co; + + if (g_hlr->gs == NULL) + return; + + llist_for_each_entry(co, &g_hlr->gs->clients, list) { + struct lu_operation *luop = lu_op_alloc_conn(co); + if (!luop) { + LOGP(DMAIN, LOGL_ERROR, + "IMSI='%s': Cannot notify GSUP client, cannot allocate lu_operation," + " for %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; + } + luop->subscr = *subscr; + luop->state = LU_S_LU_RECEIVED; /* Pretend we received a location update. */ + lu_op_tx_insert_subscr_data(luop); + lu_op_free(luop); + } +} + /*** * Send Auth Info handling ***/ diff --git a/src/hlr.h b/src/hlr.h index f63bc2b..368a052 100644 --- a/src/hlr.h +++ b/src/hlr.h @@ -38,3 +38,7 @@ /* Local bind addr */ char *gsup_bind_addr; }; + +struct hlr_subscriber; + +void osmo_hlr_subscriber_update_notify(struct hlr_subscriber *subscr); diff --git a/src/hlr_vty_subscr.c b/src/hlr_vty_subscr.c index 7191a1c..4092a8f 100644 --- a/src/hlr_vty_subscr.c +++ b/src/hlr_vty_subscr.c @@ -257,6 +257,10 @@ vty_out(vty, "%% Updated subscriber IMSI='%s' to MSISDN='%s'%s", subscr.imsi, msisdn, VTY_NEWLINE); + + if (db_subscr_get_by_msisdn(g_hlr->dbc, msisdn, &subscr) == 0) + osmo_hlr_subscriber_update_notify(&subscr); + return CMD_SUCCESS; } -- To view, visit https://gerrit.osmocom.org/7685 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iffe1d7afb9fc7dbae542f70bbf5391ddc08a14b4 Gerrit-PatchSet: 1 Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Owner: Stefan Sperling