Change in osmo-msc[master]: a_reset: cleanup + remove dead code

[Harald Welte]

Harald Welte has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/8054 )

Change subject: a_reset: cleanup + remove dead code
..

a_reset: cleanup + remove dead code

a_reset.c/h was originally developed to be used in both, bsc and
msc without changes. Unfortunately no suitable library has been
found for a_reset.c/h so the file ended up as duplicated code in
both split brances. Eventually we decided to specialize the
generalized code again, which means some of the functions needed
only by osmo-bsc are removed.

- Remove dead code
- Fix timer identification number (T16)
- use fi->priv to hold context info
- Minor cosmetic fixes

Change-Id: I8e489eb494d358d130e51cb2167929edeaa12e92
Depends: libosmocore I36d221c973d3890721ef1d376fb9be82c4311378
Related: OS#3103
---
M include/osmocom/msc/a_iface.h
M include/osmocom/msc/a_reset.h
M src/libmsc/a_iface.c
M src/libmsc/a_iface_bssap.c
M src/libmsc/a_reset.c
5 files changed, 59 insertions(+), 159 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/include/osmocom/msc/a_iface.h b/include/osmocom/msc/a_iface.h
index 3098b51..217011d 100644
--- a/include/osmocom/msc/a_iface.h
+++ b/include/osmocom/msc/a_iface.h
@@ -51,7 +51,7 @@
/* A pointer to the reset handler FSM, the
 * state machine is allocated when the BSC
 * is registerd. */
-   struct a_reset_ctx *reset;
+   struct osmo_fsm_inst *reset_fsm;

/* A pointer to the sccp_user that is associated
 * with the A interface. We need this information
diff --git a/include/osmocom/msc/a_reset.h b/include/osmocom/msc/a_reset.h
index cdb17c2..8eb3bbf 100644
--- a/include/osmocom/msc/a_reset.h
+++ b/include/osmocom/msc/a_reset.h
@@ -20,45 +20,12 @@

 #pragma once

-
-
-/* Reset context data (callbacks, state machine etc...) */
-struct a_reset_ctx {
-
-   /* FSM instance, which handles the reset procedure */
-   struct osmo_fsm_inst *fsm;
-
-   /* Connection failure counter. When this counter
-* reaches a certain threshold, the reset procedure
-* will be triggered */
-   int conn_loss_counter;
-
-   /* A human readable name to display in the logs */
-   char name[256];
-
-   /* Callback function to be called when a connection
-* failure is detected and a rest must occur */
-   void (*cb)(void *priv);
-
-   /* Privated data for the callback function */
-   void *priv;
-};
-
 /* Create and start state machine which handles the reset/reset-ack procedure 
*/
-struct a_reset_ctx *a_reset_alloc(const void *ctx, const char *name, void *cb, 
void *priv,
- bool already_connected);
-
-/* Tear down state machine */
-void a_reset_free(struct a_reset_ctx *reset);
+struct osmo_fsm_inst *a_reset_alloc(void *ctx, const char *name, void *cb,
+   void *priv, bool already_connected);

 /* Confirm that we sucessfully received a reset acknowlege message */
-void a_reset_ack_confirm(struct a_reset_ctx *reset);
-
-/* Report a failed connection */
-void a_reset_conn_fail(struct a_reset_ctx *reset);
-
-/* Report a successful connection */
-void a_reset_conn_success(struct a_reset_ctx *reset);
+void a_reset_ack_confirm(struct osmo_fsm_inst *reset_fsm);

 /* Check if we have a connection to a specified msc */
-bool a_reset_conn_ready(struct a_reset_ctx *reset);
+bool a_reset_conn_ready(struct osmo_fsm_inst *reset_fsm);
diff --git a/src/libmsc/a_iface.c b/src/libmsc/a_iface.c
index 6f2000e..75fa438 100644
--- a/src/libmsc/a_iface.c
+++ b/src/libmsc/a_iface.c
@@ -209,7 +209,7 @@

/* Deliver paging request to all known BSCs */
llist_for_each_entry(bsc_ctx, _network->a.bscs, list) {
-   if (a_reset_conn_ready(bsc_ctx->reset)) {
+   if (a_reset_conn_ready(bsc_ctx->reset_fsm)) {
LOGP(DBSSAP, LOGL_DEBUG,
 "Tx BSSMAP paging message from MSC %s to BSC %s 
(imsi=%s, tmsi=0x%08x, lac=%u)\n",
 osmo_sccp_addr_name(ss7, _ctx->msc_addr),
@@ -471,10 +471,10 @@
 void a_start_reset(struct bsc_context *bsc_ctx, bool already_connected)
 {
char bsc_name[32];
-   OSMO_ASSERT(bsc_ctx->reset == NULL);
+   OSMO_ASSERT(bsc_ctx->reset_fsm == NULL);
/* Start reset procedure to make the new connection active */
snprintf(bsc_name, sizeof(bsc_name), "bsc-%i", bsc_ctx->bsc_addr.pc);
-   bsc_ctx->reset = a_reset_alloc(bsc_ctx, bsc_name, a_reset_cb, bsc_ctx, 
already_connected);
+   bsc_ctx->reset_fsm = a_reset_alloc(bsc_ctx, bsc_name, a_reset_cb, 
bsc_ctx, already_connected);
 }

 /* determine if given msg is BSSMAP RESET related (true) or not (false) */
@@ -521,7 +521,7 @@
a_start_reset(a_conn_info.bsc, false);
} else {
/* This BSC is already 

Change in osmo-msc[master]: a_reset: cleanup + remove dead code

[Harald Welte]

Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/8054 )

Change subject: a_reset: cleanup + remove dead code
..


Patch Set 3: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/8054
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e489eb494d358d130e51cb2167929edeaa12e92
Gerrit-Change-Number: 8054
Gerrit-PatchSet: 3
Gerrit-Owner: dexter 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Comment-Date: Thu, 17 May 2018 12:09:52 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes

Change in osmo-msc[master]: a_reset: cleanup + remove dead code

[dexter]

Hello Neels Hofmeyr, Jenkins Builder,

I'd like you to reexamine a change. Please visit

https://gerrit.osmocom.org/8054

to look at the new patch set (#3).

Change subject: a_reset: cleanup + remove dead code
..

a_reset: cleanup + remove dead code

a_reset.c/h was originally developed to be used in both, bsc and
msc without changes. Unfortunately no suitable library has been
found for a_reset.c/h so the file ended up as duplicated code in
both split brances. Eventually we decided to specialize the
generalized code again, which means some of the functions needed
only by osmo-bsc are removed.

- Remove dead code
- Fix timer identification number (T16)
- use fi->priv to hold context info
- Minor cosmetic fixes

Change-Id: I8e489eb494d358d130e51cb2167929edeaa12e92
Depends: libosmocore I36d221c973d3890721ef1d376fb9be82c4311378
Related: OS#3103
---
M include/osmocom/msc/a_iface.h
M include/osmocom/msc/a_reset.h
M src/libmsc/a_iface.c
M src/libmsc/a_iface_bssap.c
M src/libmsc/a_reset.c
5 files changed, 59 insertions(+), 159 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/54/8054/3
--
To view, visit https://gerrit.osmocom.org/8054
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8e489eb494d358d130e51cb2167929edeaa12e92
Gerrit-Change-Number: 8054
Gerrit-PatchSet: 3
Gerrit-Owner: dexter 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 

Change in osmo-msc[master]: a_reset: cleanup + remove dead code

[Neels Hofmeyr]

Neels Hofmeyr has posted comments on this change. ( 
https://gerrit.osmocom.org/8054 )

Change subject: a_reset: cleanup + remove dead code
..


Patch Set 2:

(4 comments)

https://gerrit.osmocom.org/#/c/8054/2/src/libmsc/a_reset.c
File src/libmsc/a_reset.c:

https://gerrit.osmocom.org/#/c/8054/2/src/libmsc/a_reset.c@34
PS2, Line 34: enum fsm_states {
(name is too general, has always been, but not that important; same for fsm_evt)


https://gerrit.osmocom.org/#/c/8054/2/src/libmsc/a_reset.c@90
PS2, Line 90:.in_event_mask = (1 << EV_CONN_ACK),
As I asked before: we allow repeated Conn ACKs? Is that a real thing?

And even if yes, AFAICT this state doesn't need an action cb, because the 
osmo_fsm_* code already logs the event, and fsm_conn_cb() does nothing but 
trivial logging.


https://gerrit.osmocom.org/#/c/8054/2/src/libmsc/a_reset.c@91
PS2, Line 91:.out_state_mask = 0,
('.member = 0' can be omitted, happens implicitly)


https://gerrit.osmocom.org/#/c/8054/2/src/libmsc/a_reset.c@125
PS2, Line 125: reset_fsm = osmo_fsm_inst_alloc(, NULL, NULL, 
LOGL_DEBUG, name);
use priv arg of osmo_fsm_inst_alloc() (unless you want to use reset_fsm as 
talloc parent for reset_ctx)



--
To view, visit https://gerrit.osmocom.org/8054
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e489eb494d358d130e51cb2167929edeaa12e92
Gerrit-Change-Number: 8054
Gerrit-PatchSet: 2
Gerrit-Owner: dexter 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Comment-Date: Mon, 14 May 2018 17:59:03 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: No

Change in osmo-msc[master]: a_reset: cleanup + remove dead code

[dexter]

Hello Neels Hofmeyr, Jenkins Builder,

I'd like you to reexamine a change. Please visit

https://gerrit.osmocom.org/8054

to look at the new patch set (#2).

Change subject: a_reset: cleanup + remove dead code
..

a_reset: cleanup + remove dead code

a_reset.c/h was originally developed to be used in both, bsc and
msc without changes. Unfortunately no suitable library has been
found for a_reset.c/h so the file ended up as duplicated code in
both split brances. Eventually we decided to specialize the
generalized code again, which means some of the functions needed
only by osmo-bsc are removed.

- Remove dead code
- Fix counter number
- use fi->priv to hold context info
- Minor cosmetic fixes

Change-Id: I8e489eb494d358d130e51cb2167929edeaa12e92
Related: OS#3103
---
M include/osmocom/msc/a_iface.h
M include/osmocom/msc/a_reset.h
M src/libmsc/a_iface.c
M src/libmsc/a_iface_bssap.c
M src/libmsc/a_reset.c
5 files changed, 58 insertions(+), 148 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/54/8054/2
--
To view, visit https://gerrit.osmocom.org/8054
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8e489eb494d358d130e51cb2167929edeaa12e92
Gerrit-Change-Number: 8054
Gerrit-PatchSet: 2
Gerrit-Owner: dexter 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr