[PATCH] Add subscriber delete command

[Ruben Pollan]

---
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);

[Ruben Pollan]

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.

[Holger Hans Peter Freyther]

On Sun, Aug 24, 2014 at 02:02:16PM -0500, Ruben Pollan wrote:

Hi!

Post by Ruben Pollan
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

[Ruben Pollan]

Quoting Holger Hans Peter Freyther (2014-08-31 05:42:56)

Post by Holger Hans Peter Freyther
Hi!

Post by Ruben Pollan
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.

[Ruben Pollan]

---
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")

[Holger Hans Peter Freyther]

Post by Ruben Pollan
+ 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.

Post by Ruben Pollan
+ # Delte it

Delete

Post by Ruben Pollan
+ # Now it should not be there anymore
+ res = self.vty.command('show subscriber imsi '+imsi)
+ self.assert_(res != '% No subscriber found for imsi '+imsi)

good!

[Ruben Pollan]

Quoting Holger Hans Peter Freyther (2014-09-01 13:45:02)

Post by Holger Hans Peter Freyther

+ 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.

Post by Holger Hans Peter Freyther

+ # Delte it

Delete

Ok.

[Holger Hans Peter Freyther]

Post by Ruben Pollan
This code is basically copied from openbsc/src/libmsc/ctrl_commands.c where is

You are right! my bad!

Post by Ruben Pollan

Post by Ruben Pollan
Delete

Could you please rebase and send an updated patch? thanks
holger

[Ruben Pollan]

---
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")