13 Jul
2013
13 Jul
'13
3:34 p.m.
osmo_timer_del is an idempotent operation. There is no requirement
to check if it is running. If you don't want a timer to run, delete
it. Maybe one should have called the method _unschedule, _cancel to
make this more clear.
---
src/gprs_bssgp_pcu.cpp | 6 ++----
src/sysmo_sock.cpp | 3 +--
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/src/gprs_bssgp_pcu.cpp b/src/gprs_bssgp_pcu.cpp
index 54753b8..c791913 100644
--- a/src/gprs_bssgp_pcu.cpp
+++ b/src/gprs_bssgp_pcu.cpp
@@ -515,8 +515,7 @@ static int nsvc_signal_cb(unsigned int subsys, unsigned int signal,
case S_NS_BLOCK:
if (nsvc_unblocked) {
nsvc_unblocked = 0;
- if (osmo_timer_pending(&bvc_timer))
- osmo_timer_del(&bvc_timer);
+ osmo_timer_del(&bvc_timer);
bvc_sig_reset = 0;
bvc_reset = 0;
bvc_unblocked = 0;
@@ -646,8 +645,7 @@ void gprs_bssgp_destroy(void)
if (!bssgp_nsi)
return;
- if (osmo_timer_pending(&bvc_timer))
- osmo_timer_del(&bvc_timer);
+ osmo_timer_del(&bvc_timer);
osmo_signal_unregister_handler(SS_L_NS, nsvc_signal_cb, NULL);
diff --git a/src/sysmo_sock.cpp b/src/sysmo_sock.cpp
index a4cc6de..d4fb5a6 100644
--- a/src/sysmo_sock.cpp
+++ b/src/sysmo_sock.cpp
@@ -304,8 +304,7 @@ void pcu_l1if_close(void)
if (!state)
return;
- if (osmo_timer_pending(&state->timer))
- osmo_timer_del(&state->timer);
+ osmo_timer_del(&state->timer);
bfd = &state->conn_bfd;
if (bfd->fd > 0)
--
1.8.3.2
13 Jul
13 Jul
3:34 p.m.
New subject: [PATCH 2/2] misc: Add an option exit/quit when the BSSGP is supposed to be destroyed
The PCU does not properly re-set the state when the connection to the
BTS is lost (and the SGSN potentially is re-started during that). This
results in the BSSGP BVCI > 1 remaining blocked and no data will be
accepted by the SGSN.
Add the '-e' option and exit the PCU when the BSSGP/NS are getting
destroyed.
---
src/gprs_bssgp_pcu.cpp | 13 ++++++++++++-
src/gprs_bssgp_pcu.h | 3 ++-
src/openbts_sock.cpp | 2 +-
src/pcu_l1_if.cpp | 2 +-
src/pcu_main.cpp | 7 ++++++-
src/sysmo_sock.cpp | 2 +-
6 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/src/gprs_bssgp_pcu.cpp b/src/gprs_bssgp_pcu.cpp
index c791913..59185e3 100644
--- a/src/gprs_bssgp_pcu.cpp
+++ b/src/gprs_bssgp_pcu.cpp
@@ -27,6 +27,7 @@ struct bssgp_bvc_ctx *bctx = NULL;
struct gprs_nsvc *nsvc = NULL;
static int bvc_sig_reset = 0, bvc_reset = 0, bvc_unblocked = 0;
extern uint16_t spoof_mcc, spoof_mnc;
+static int exit_on_destroy = 0;
struct osmo_timer_list bvc_timer;
@@ -640,8 +641,13 @@ int gprs_bssgp_create(uint16_t local_port, uint32_t sgsn_ip,
return 0;
}
-void gprs_bssgp_destroy(void)
+void gprs_bssgp_destroy_or_exit(void)
{
+ if (exit_on_destroy) {
+ LOGP(DBSSGP, LOGL_NOTICE, "Exiting on BSSGP destruction.\n");
+ exit(0);
+ }
+
if (!bssgp_nsi)
return;
@@ -662,3 +668,8 @@ void gprs_bssgp_destroy(void)
bssgp_nsi = NULL;
}
+void gprs_bssgp_exit_on_destroy(void)
+{
+ LOGP(DBSSGP, LOGL_NOTICE, "Going to quit on BSSGP destruction\n");
+ exit_on_destroy = 1;
+}
diff --git a/src/gprs_bssgp_pcu.h b/src/gprs_bssgp_pcu.h
index d669c3a..dc57e4f 100644
--- a/src/gprs_bssgp_pcu.h
+++ b/src/gprs_bssgp_pcu.h
@@ -56,6 +56,7 @@ int gprs_bssgp_create(uint16_t local_port, uint32_t sgsn_ip, uint16_t
uint16_t mcc, uint16_t mnc, uint16_t lac, uint16_t rac,
uint16_t cell_id);
-void gprs_bssgp_destroy(void);
+void gprs_bssgp_exit_on_destroy(void);
+void gprs_bssgp_destroy_or_exit(void);
#endif // GPRS_BSSGP_PCU_H
diff --git a/src/openbts_sock.cpp b/src/openbts_sock.cpp
index 845aa77..a09f834 100644
--- a/src/openbts_sock.cpp
+++ b/src/openbts_sock.cpp
@@ -179,7 +179,7 @@ int pcu_l1if_open()
void pcu_l1if_close(void)
{
- gprs_bssgp_destroy();
+ gprs_bssgp_destroy_or_exit();
/* FIXME: cleanup l1if */
talloc_free(l1fh->fl1h);
diff --git a/src/pcu_l1_if.cpp b/src/pcu_l1_if.cpp
index 923070f..037cf60 100644
--- a/src/pcu_l1_if.cpp
+++ b/src/pcu_l1_if.cpp
@@ -369,7 +369,7 @@ bssgp_failed:
trx, ts);
}
}
- gprs_bssgp_destroy();
+ gprs_bssgp_destroy_or_exit();
return 0;
}
LOGP(DL1IF, LOGL_INFO, "BTS available\n");
diff --git a/src/pcu_main.cpp b/src/pcu_main.cpp
index 041831f6..754043f 100644
--- a/src/pcu_main.cpp
+++ b/src/pcu_main.cpp
@@ -60,6 +60,7 @@ static void print_help()
"provided by BTS\n"
" -r --realtime PRIO Use SCHED_RR with the specified "
"priority\n"
+ " -e --exit Exit the application on disconnect\n"
);
}
@@ -75,10 +76,11 @@ static void handle_options(int argc, char **argv)
{ "mnc", 1, 0, 'n' },
{ "version", 0, 0, 'V' },
{ "realtime", 1, 0, 'r' },
+ { "exit", 0, 0, 'e' },
{ 0, 0, 0, 0 }
};
- c = getopt_long(argc, argv, "hc:m:n:Vr:",
+ c = getopt_long(argc, argv, "hc:m:n:Vr:e",
long_options, &option_idx);
if (c == -1)
break;
@@ -105,6 +107,9 @@ static void handle_options(int argc, char **argv)
case 'r':
rt_prio = atoi(optarg);
break;
+ case 'e':
+ gprs_bssgp_exit_on_destroy();
+ break;
default:
fprintf(stderr, "Unknown option '%c'\n", c);
exit(0);
diff --git a/src/sysmo_sock.cpp b/src/sysmo_sock.cpp
index d4fb5a6..e116e5a 100644
--- a/src/sysmo_sock.cpp
+++ b/src/sysmo_sock.cpp
@@ -117,7 +117,7 @@ static void pcu_sock_close(struct pcu_sock_state *state, int lost)
}
}
- gprs_bssgp_destroy();
+ gprs_bssgp_destroy_or_exit();
if (lost) {
state->timer.cb = pcu_sock_timeout;
--
1.8.3.2
25 Jul
25 Jul
6:27 p.m.
New subject: [PATCH 2/2] misc: Add an option exit/quit when the BSSGP is supposed to be destroyed
Hi Holger,
Sorry for slow response.
Actually I think that we should implement more intellectual behavior
of PCUfor handling restarts and disconnections of
BTS and SGSN.
I should try to find the way to solve this problem, but I will commit this
patch, because it fixes current problems.
2013/7/13 Holger Hans Peter Freyther <holger(a)moiji-mobile.com>
The PCU does not properly re-set the state when the
connection to the
BTS is lost (and the SGSN potentially is re-started during that). This
results in the BSSGP BVCI > 1 remaining blocked and no data will be
accepted by the SGSN.
Add the '-e' option and exit the PCU when the BSSGP/NS are getting
destroyed.
---
src/gprs_bssgp_pcu.cpp | 13 ++++++++++++-
src/gprs_bssgp_pcu.h | 3 ++-
src/openbts_sock.cpp | 2 +-
src/pcu_l1_if.cpp | 2 +-
src/pcu_main.cpp | 7 ++++++-
src/sysmo_sock.cpp | 2 +-
6 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/src/gprs_bssgp_pcu.cpp b/src/gprs_bssgp_pcu.cpp
index c791913..59185e3 100644
--- a/src/gprs_bssgp_pcu.cpp
+++ b/src/gprs_bssgp_pcu.cpp
@@ -27,6 +27,7 @@ struct bssgp_bvc_ctx *bctx = NULL;
struct gprs_nsvc *nsvc = NULL;
static int bvc_sig_reset = 0, bvc_reset = 0, bvc_unblocked = 0;
extern uint16_t spoof_mcc, spoof_mnc;
+static int exit_on_destroy = 0;
struct osmo_timer_list bvc_timer;
@@ -640,8 +641,13 @@ int gprs_bssgp_create(uint16_t local_port, uint32_t
sgsn_ip,
return 0;
}
-void gprs_bssgp_destroy(void)
+void gprs_bssgp_destroy_or_exit(void)
{
+ if (exit_on_destroy) {
+ LOGP(DBSSGP, LOGL_NOTICE, "Exiting on BSSGP
destruction.\n");
+ exit(0);
+ }
+
if (!bssgp_nsi)
return;
@@ -662,3 +668,8 @@ void gprs_bssgp_destroy(void)
bssgp_nsi = NULL;
}
+void gprs_bssgp_exit_on_destroy(void)
+{
+ LOGP(DBSSGP, LOGL_NOTICE, "Going to quit on BSSGP destruction\n");
+ exit_on_destroy = 1;
+}
diff --git a/src/gprs_bssgp_pcu.h b/src/gprs_bssgp_pcu.h
index d669c3a..dc57e4f 100644
--- a/src/gprs_bssgp_pcu.h
+++ b/src/gprs_bssgp_pcu.h
@@ -56,6 +56,7 @@ int gprs_bssgp_create(uint16_t local_port, uint32_t
sgsn_ip, uint16_t
uint16_t mcc, uint16_t mnc, uint16_t lac, uint16_t rac,
uint16_t cell_id);
-void gprs_bssgp_destroy(void);
+void gprs_bssgp_exit_on_destroy(void);
+void gprs_bssgp_destroy_or_exit(void);
#endif // GPRS_BSSGP_PCU_H
diff --git a/src/openbts_sock.cpp b/src/openbts_sock.cpp
index 845aa77..a09f834 100644
--- a/src/openbts_sock.cpp
+++ b/src/openbts_sock.cpp
@@ -179,7 +179,7 @@ int pcu_l1if_open()
void pcu_l1if_close(void)
{
- gprs_bssgp_destroy();
+ gprs_bssgp_destroy_or_exit();
/* FIXME: cleanup l1if */
talloc_free(l1fh->fl1h);
diff --git a/src/pcu_l1_if.cpp b/src/pcu_l1_if.cpp
index 923070f..037cf60 100644
--- a/src/pcu_l1_if.cpp
+++ b/src/pcu_l1_if.cpp
@@ -369,7 +369,7 @@ bssgp_failed:
trx, ts);
}
}
- gprs_bssgp_destroy();
+ gprs_bssgp_destroy_or_exit();
return 0;
}
LOGP(DL1IF, LOGL_INFO, "BTS available\n");
diff --git a/src/pcu_main.cpp b/src/pcu_main.cpp
index 041831f6..754043f 100644
--- a/src/pcu_main.cpp
+++ b/src/pcu_main.cpp
@@ -60,6 +60,7 @@ static void print_help()
"provided by BTS\n"
" -r --realtime PRIO Use SCHED_RR with the specified "
"priority\n"
+ " -e --exit Exit the application on
disconnect\n"
);
}
@@ -75,10 +76,11 @@ static void handle_options(int argc, char **argv)
{ "mnc", 1, 0, 'n' },
{ "version", 0, 0, 'V' },
{ "realtime", 1, 0, 'r' },
+ { "exit", 0, 0, 'e' },
{ 0, 0, 0, 0 }
};
- c = getopt_long(argc, argv, "hc:m:n:Vr:",
+ c = getopt_long(argc, argv, "hc:m:n:Vr:e",
long_options, &option_idx);
if (c == -1)
break;
@@ -105,6 +107,9 @@ static void handle_options(int argc, char **argv)
case 'r':
rt_prio = atoi(optarg);
break;
+ case 'e':
+ gprs_bssgp_exit_on_destroy();
+ break;
default:
fprintf(stderr, "Unknown option '%c'\n", c);
exit(0);
diff --git a/src/sysmo_sock.cpp b/src/sysmo_sock.cpp
index d4fb5a6..e116e5a 100644
--- a/src/sysmo_sock.cpp
+++ b/src/sysmo_sock.cpp
@@ -117,7 +117,7 @@ static void pcu_sock_close(struct pcu_sock_state
*state, int lost)
}
}
- gprs_bssgp_destroy();
+ gprs_bssgp_destroy_or_exit();
if (lost) {
state->timer.cb = pcu_sock_timeout;
--
1.8.3.2
--
Regards,
Ivan Kluchnikov.
http://fairwaves.ru
26 Jul
26 Jul
6:31 a.m.
New subject: [PATCH 2/2] misc: Add an option exit/quit when the BSSGP is supposed to be destroyed
On Thu, Jul 25, 2013 at 08:27:04PM +0400, Ivan Kluchnikov wrote:
Dear Ivan,
I should try to find the way to solve this problem,
but I will commit this
patch, because it fixes current problems.
Well, I have more commits pending for review but I would like to get
the first bits in first instead of sending you a patch bomb. The
'root' cause appears to be that the state is in a couple of static
fields that are throughout the file. I started to move them into a
struct (an organic cell/entity). At that point it became clear of why
NS/BSSGP is behaving like it does behave.
Once you merge the two changes, I will rebase and sent my cleanup
patchset. And then we can look into properly re-setting the state.
kind regards
holger
--
- Holger Freyther <hfreyther(a)sysmocom.de> http://www.sysmocom.de/
=======================================================================
* sysmocom - systems for mobile communications GmbH
* Schivelbeiner Str. 5
* 10439 Berlin, Germany
* Sitz / Registered office: Berlin, HRB 134158 B
* Geschaeftsfuehrer / Managing Directors: Holger Freyther, Harald Welte
29 Jul
29 Jul
12:51 p.m.
New subject: [PATCH 2/2] misc: Add an option exit/quit when the BSSGP is supposed to be destroyed
Hi Ivan,
On Thu, Jul 25, 2013 at 08:27:04PM +0400, Ivan Kluchnikov wrote:
Actually I think that we should implement more
intellectual behavior
of PCUfor handling restarts and disconnections of BTS and SGSN.
I think there is nothing wrong with a 'fail fast' approach. the SGSN
disconnecting should be a rare event. The PCU is a small process.
Exiting and re-spawning just to make 100% sure that no bogus state
remains is a valid approach from my point of view. osmo-bts also does
it the same way.
It's not like we're talking about a 200MByte sized complex process where
re-spawning is an expensive operation.
Regards,
Harald
--
- Harald Welte <laforge(a)gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
3947
days inactive
3963
days old
osmocom-net-gprs@lists.osmocom.org
4 comments
4 participants
participants (4)
-
Harald Welte -
Holger Hans Peter Freyther -
Holger Hans Peter Freyther -
Ivan Kluchnikov