--- openbsc/include/openbsc/gsm_subscriber.h | 1 + openbsc/src/libbsc/gsm_subscriber_base.c | 2 +- openbsc/src/libmsc/vty_interface_layer3.c | 24 +++++++++++++++++++++++- 3 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/openbsc/include/openbsc/gsm_subscriber.h b/openbsc/include/openbsc/gsm_subscriber.h index 7aae4c3..120111b 100644 --- a/openbsc/include/openbsc/gsm_subscriber.h +++ b/openbsc/include/openbsc/gsm_subscriber.h @@ -101,6 +101,7 @@ int subscr_pending_kick(struct gsm_subscriber *subscr);
char *subscr_name(struct gsm_subscriber *subscr);
+void subscr_free(struct gsm_subscriber *subscr); int subscr_purge_inactive(struct gsm_network *net); void subscr_update_from_db(struct gsm_subscriber *subscr); void subscr_expire(struct gsm_network *net); diff --git a/openbsc/src/libbsc/gsm_subscriber_base.c b/openbsc/src/libbsc/gsm_subscriber_base.c index 5e00443..2b31e95 100644 --- a/openbsc/src/libbsc/gsm_subscriber_base.c +++ b/openbsc/src/libbsc/gsm_subscriber_base.c @@ -66,7 +66,7 @@ struct gsm_subscriber *subscr_alloc(void) return s; }
-static void subscr_free(struct gsm_subscriber *subscr) +void subscr_free(struct gsm_subscriber *subscr) { llist_del(&subscr->entry); talloc_free(subscr); diff --git a/openbsc/src/libmsc/vty_interface_layer3.c b/openbsc/src/libmsc/vty_interface_layer3.c index 064eca9..6b53f65 100644 --- a/openbsc/src/libmsc/vty_interface_layer3.c +++ b/openbsc/src/libmsc/vty_interface_layer3.c @@ -465,7 +465,28 @@ DEFUN(subscriber_ussd_notify, return CMD_SUCCESS; }
-DEFUN(ena_subscr_authorizde, +DEFUN(ena_subscr_delete, + ena_subscr_delete_cmd, + "subscriber " SUBSCR_TYPES " ID delete", + SUBSCR_HELP "Delete subscriber in HLR\n") +{ + struct gsm_network *gsmnet = gsmnet_from_vty(vty); + struct gsm_subscriber *subscr = + get_subscr_by_argv(gsmnet, argv[0], argv[1]); + + if (!subscr) { + vty_out(vty, "%% No subscriber found for %s %s%s", + argv[0], argv[1], VTY_NEWLINE); + return CMD_WARNING; + } + + db_subscriber_delete(subscr); + subscr_free(subscr); + + return CMD_SUCCESS; +} + +DEFUN(ena_subscr_authorized, ena_subscr_authorized_cmd, "subscriber " SUBSCR_TYPES " ID authorized (0|1)", SUBSCR_HELP "(De-)Authorize subscriber in HLR\n" @@ -982,6 +1003,7 @@ int bsc_vty_init_extra(void) install_element_ve(&show_stats_cmd); install_element_ve(&show_smsqueue_cmd);
+ install_element(ENABLE_NODE, &ena_subscr_delete_cmd); install_element(ENABLE_NODE, &ena_subscr_name_cmd); install_element(ENABLE_NODE, &ena_subscr_extension_cmd); install_element(ENABLE_NODE, &ena_subscr_authorized_cmd);
This patch comes from some needs of rhizmoatica. It looks good in our tests, but today it's my first time looking at openbsc code and I might miss something.
Cheers.
On Sun, Aug 24, 2014 at 02:02:16PM -0500, Ruben Pollan wrote:
Hi!
This patch comes from some needs of rhizmoatica. It looks good in our tests, but today it's my first time looking at openbsc code and I might miss something.
sorry, I intendted to reply right-away and then had to rush and dropped the ball. To make sure your feature continue to work you should definately add an end-to-end test.
There is one technical issue. The subscriber record is reference counted. So if you free the memory while someobody else is using it you will crash.
Third, for machine to machine interaction we have the control interface which already has a command to delete the subscriber. Please have a look at
src/libmsc/ctrl_commands.c:set_subscriber_delete
cheers holger
Quoting Holger Hans Peter Freyther (2014-08-31 05:42:56)
On Sun, Aug 24, 2014 at 02:02:16PM -0500, Ruben Pollan wrote:
Hi!
This patch comes from some needs of rhizmoatica. It looks good in our tests, but today it's my first time looking at openbsc code and I might miss something.
sorry, I intendted to reply right-away and then had to rush and dropped the ball. To make sure your feature continue to work you should definately add an end-to-end test.
There is one technical issue. The subscriber record is reference counted. So if you free the memory while someobody else is using it you will crash.
Third, for machine to machine interaction we have the control interface which already has a command to delete the subscriber. Please have a look at
src/libmsc/ctrl_commands.c:set_subscriber_delete
It makes sense, I just fixed. The new patch comes in a following email.
Thanks for the review.
--- openbsc/src/libmsc/vty_interface_layer3.c | 34 ++++++++++++++++++++++++++++++- openbsc/tests/vty_test_runner.py | 10 ++++++++- 2 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/openbsc/src/libmsc/vty_interface_layer3.c b/openbsc/src/libmsc/vty_interface_layer3.c index 064eca9..8890099 100644 --- a/openbsc/src/libmsc/vty_interface_layer3.c +++ b/openbsc/src/libmsc/vty_interface_layer3.c @@ -465,7 +465,38 @@ DEFUN(subscriber_ussd_notify, return CMD_SUCCESS; }
-DEFUN(ena_subscr_authorizde, +DEFUN(ena_subscr_delete, + ena_subscr_delete_cmd, + "subscriber " SUBSCR_TYPES " ID delete", + SUBSCR_HELP "Delete subscriber in HLR\n") +{ + int rc; + struct gsm_network *gsmnet = gsmnet_from_vty(vty); + struct gsm_subscriber *subscr = + get_subscr_by_argv(gsmnet, argv[0], argv[1]); + + if (!subscr) { + vty_out(vty, "%% No subscriber found for %s %s%s", + argv[0], argv[1], VTY_NEWLINE); + return CMD_WARNING; + } + + if (subscr->use_count != 1) { + vty_out(vty, "Removing active subscriber%s", VTY_NEWLINE); + } + + rc = db_subscriber_delete(subscr); + subscr_put(subscr); + + if (rc != 0) { + vty_out(vty, "Failed to remove subscriber%s", VTY_NEWLINE); + return CMD_WARNING; + } + + return CMD_SUCCESS; +} + +DEFUN(ena_subscr_authorized, ena_subscr_authorized_cmd, "subscriber " SUBSCR_TYPES " ID authorized (0|1)", SUBSCR_HELP "(De-)Authorize subscriber in HLR\n" @@ -982,6 +1013,7 @@ int bsc_vty_init_extra(void) install_element_ve(&show_stats_cmd); install_element_ve(&show_smsqueue_cmd);
+ install_element(ENABLE_NODE, &ena_subscr_delete_cmd); install_element(ENABLE_NODE, &ena_subscr_name_cmd); install_element(ENABLE_NODE, &ena_subscr_extension_cmd); install_element(ENABLE_NODE, &ena_subscr_authorized_cmd); diff --git a/openbsc/tests/vty_test_runner.py b/openbsc/tests/vty_test_runner.py index db8294d..c12121b 100644 --- a/openbsc/tests/vty_test_runner.py +++ b/openbsc/tests/vty_test_runner.py @@ -247,7 +247,7 @@ class TestVTYNITB(TestVTYGenericBSC): if classNum != 10: self.assertEquals(res.find("rach access-control-class " + str(classNum) + " barred"), -1)
- def testSubscriberCreate(self): + def testSubscriberCreateDelete(self): self.vty.enable()
imsi = "204300854013739" @@ -263,6 +263,14 @@ class TestVTYNITB(TestVTYGenericBSC): res = self.vty.command('show subscriber imsi '+imsi) self.assert_(res.find(" IMSI: "+imsi) > 0)
+ # Delte it + res = self.vty.command('subscriber delete imsi '+imsi) + self.assert_(res != "") + + # Now it should not be there anymore + res = self.vty.command('show subscriber imsi '+imsi) + self.assert_(res != '% No subscriber found for imsi '+imsi) + def testShowPagingGroup(self): res = self.vty.command("show paging-group 255 1234567") self.assertEqual(res, "% can't find BTS 255")
On Sun, Aug 31, 2014 at 05:13:15PM -0500, Ruben Pollan wrote:
- if (subscr->use_count != 1) {
vty_out(vty, "Removing active subscriber%s", VTY_NEWLINE);- }
You could write abort() as well. You should return CMD_ERROR or such as you can't remove an active subscriber.
# Delte it
Delete
# Now it should not be there anymoreres = self.vty.command('show subscriber imsi '+imsi)self.assert_(res != '% No subscriber found for imsi '+imsi)
good!
Quoting Holger Hans Peter Freyther (2014-09-01 13:45:02)
On Sun, Aug 31, 2014 at 05:13:15PM -0500, Ruben Pollan wrote:
if (subscr->use_count != 1) {vty_out(vty, "Removing active subscriber%s", VTY_NEWLINE);}You could write abort() as well. You should return CMD_ERROR or such as you can't remove an active subscriber.
This code is basically copied from openbsc/src/libmsc/ctrl_commands.c where is doing:
if (subscr->use_count != 1) { LOGP(DCTRL, LOGL_NOTICE, "Going to remove active subscriber.\n"); was_used = 1; }
rc = db_subscriber_delete(subscr);
In the ctrl commands interface even if is in use it keeps trying to delete it. Maybe I'm missing something and the two interfaces should behave differently, or it's a bug on the ctrl interface.
# Delte itDelete
Ok.
--- openbsc/src/libmsc/vty_interface_layer3.c | 34 ++++++++++++++++++++++++++++++- openbsc/tests/vty_test_runner.py | 10 ++++++++- 2 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/openbsc/src/libmsc/vty_interface_layer3.c b/openbsc/src/libmsc/vty_interface_layer3.c index 064eca9..8890099 100644 --- a/openbsc/src/libmsc/vty_interface_layer3.c +++ b/openbsc/src/libmsc/vty_interface_layer3.c @@ -465,7 +465,38 @@ DEFUN(subscriber_ussd_notify, return CMD_SUCCESS; }
-DEFUN(ena_subscr_authorizde, +DEFUN(ena_subscr_delete, + ena_subscr_delete_cmd, + "subscriber " SUBSCR_TYPES " ID delete", + SUBSCR_HELP "Delete subscriber in HLR\n") +{ + int rc; + struct gsm_network *gsmnet = gsmnet_from_vty(vty); + struct gsm_subscriber *subscr = + get_subscr_by_argv(gsmnet, argv[0], argv[1]); + + if (!subscr) { + vty_out(vty, "%% No subscriber found for %s %s%s", + argv[0], argv[1], VTY_NEWLINE); + return CMD_WARNING; + } + + if (subscr->use_count != 1) { + vty_out(vty, "Removing active subscriber%s", VTY_NEWLINE); + } + + rc = db_subscriber_delete(subscr); + subscr_put(subscr); + + if (rc != 0) { + vty_out(vty, "Failed to remove subscriber%s", VTY_NEWLINE); + return CMD_WARNING; + } + + return CMD_SUCCESS; +} + +DEFUN(ena_subscr_authorized, ena_subscr_authorized_cmd, "subscriber " SUBSCR_TYPES " ID authorized (0|1)", SUBSCR_HELP "(De-)Authorize subscriber in HLR\n" @@ -982,6 +1013,7 @@ int bsc_vty_init_extra(void) install_element_ve(&show_stats_cmd); install_element_ve(&show_smsqueue_cmd);
+ install_element(ENABLE_NODE, &ena_subscr_delete_cmd); install_element(ENABLE_NODE, &ena_subscr_name_cmd); install_element(ENABLE_NODE, &ena_subscr_extension_cmd); install_element(ENABLE_NODE, &ena_subscr_authorized_cmd); diff --git a/openbsc/tests/vty_test_runner.py b/openbsc/tests/vty_test_runner.py index db8294d..ece9ac5 100644 --- a/openbsc/tests/vty_test_runner.py +++ b/openbsc/tests/vty_test_runner.py @@ -247,7 +247,7 @@ class TestVTYNITB(TestVTYGenericBSC): if classNum != 10: self.assertEquals(res.find("rach access-control-class " + str(classNum) + " barred"), -1)
- def testSubscriberCreate(self): + def testSubscriberCreateDelete(self): self.vty.enable()
imsi = "204300854013739" @@ -263,6 +263,14 @@ class TestVTYNITB(TestVTYGenericBSC): res = self.vty.command('show subscriber imsi '+imsi) self.assert_(res.find(" IMSI: "+imsi) > 0)
+ # Delete it + res = self.vty.command('subscriber delete imsi '+imsi) + self.assert_(res != "") + + # Now it should not be there anymore + res = self.vty.command('show subscriber imsi '+imsi) + self.assert_(res != '% No subscriber found for imsi '+imsi) + def testShowPagingGroup(self): res = self.vty.command("show paging-group 255 1234567") self.assertEqual(res, "% can't find BTS 255")
-
Holger Hans Peter Freyther -
Ruben Pollan