27 Jul
2013
27 Jul
'13
10:20 p.m.
From: Holger Hans Peter Freyther <holger(a)moiji-mobile.com>
One of the issues with not properly re-setting everything is that
due the global state it is not clear which variables belong together
and how long it exists. Begin with creating a osmo_pcu and moving
things into this class.
Think of an organic cell, this commit is introducing the cell wall
around it... and defines what is inside and what is outside of it.
---
src/gprs_bssgp_pcu.cpp | 56 ++++++++++++++++++++++++++++----------------------
1 file changed, 32 insertions(+), 24 deletions(-)
diff --git a/src/gprs_bssgp_pcu.cpp b/src/gprs_bssgp_pcu.cpp
index 59185e3..e94b5eb 100644
--- a/src/gprs_bssgp_pcu.cpp
+++ b/src/gprs_bssgp_pcu.cpp
@@ -21,13 +21,21 @@
#include <gprs_bssgp_pcu.h>
#include <pcu_l1_if.h>
+struct osmo_pcu {
+ struct gprs_nsvc *nsvc;
+
+ int bvc_sig_reset;
+ int bvc_reset;
+ int bvc_unblocked;
+ int exit_on_destroy;
+};
+
+static struct osmo_pcu the_pcu = { 0, };
+
struct sgsn_instance *sgsn;
extern void *tall_pcu_ctx;
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;
@@ -353,10 +361,10 @@ int gprs_bssgp_pcu_rx_sign(struct msgb *msg, struct tlv_parsed *tp,
struct bssgp
break;
case BSSGP_PDUT_BVC_RESET_ACK:
LOGP(DBSSGP, LOGL_DEBUG, "rx BSSGP_PDUT_BVC_RESET_ACK\n");
- if (!bvc_sig_reset)
- bvc_sig_reset = 1;
+ if (!the_pcu.bvc_sig_reset)
+ the_pcu.bvc_sig_reset = 1;
else
- bvc_reset = 1;
+ the_pcu.bvc_reset = 1;
bvc_timeout(NULL);
break;
case BSSGP_PDUT_PAGING_PS:
@@ -380,7 +388,7 @@ int gprs_bssgp_pcu_rx_sign(struct msgb *msg, struct tlv_parsed *tp,
struct bssgp
break;
case BSSGP_PDUT_BVC_UNBLOCK_ACK:
LOGP(DBSSGP, LOGL_DEBUG, "rx BSSGP_PDUT_BVC_UNBLOCK_ACK\n");
- bvc_unblocked = 1;
+ the_pcu.bvc_unblocked = 1;
bvc_timeout(NULL);
break;
case BSSGP_PDUT_SGSN_INVOKE_TRACE:
@@ -496,7 +504,7 @@ static int nsvc_signal_cb(unsigned int subsys, unsigned int signal,
return -EINVAL;
nssd = (struct ns_signal_data *)signal_data;
- if (nssd->nsvc != nsvc) {
+ if (nssd->nsvc != the_pcu.nsvc) {
LOGP(DPCU, LOGL_ERROR, "Signal received of unknown NSVC\n");
return -EINVAL;
}
@@ -506,10 +514,10 @@ static int nsvc_signal_cb(unsigned int subsys, unsigned int signal,
if (!nsvc_unblocked) {
nsvc_unblocked = 1;
LOGP(DPCU, LOGL_NOTICE, "NS-VC %d is unblocked.\n",
- nsvc->nsvci);
- bvc_sig_reset = 0;
- bvc_reset = 0;
- bvc_unblocked = 0;
+ the_pcu.nsvc->nsvci);
+ the_pcu.bvc_sig_reset = 0;
+ the_pcu.bvc_reset = 0;
+ the_pcu.bvc_unblocked = 0;
bvc_timeout(NULL);
}
break;
@@ -517,9 +525,9 @@ static int nsvc_signal_cb(unsigned int subsys, unsigned int signal,
if (nsvc_unblocked) {
nsvc_unblocked = 0;
osmo_timer_del(&bvc_timer);
- bvc_sig_reset = 0;
- bvc_reset = 0;
- bvc_unblocked = 0;
+ the_pcu.bvc_sig_reset = 0;
+ the_pcu.bvc_reset = 0;
+ the_pcu.bvc_unblocked = 0;
LOGP(DPCU, LOGL_NOTICE, "NS-VC is blocked.\n");
}
break;
@@ -545,14 +553,14 @@ static void bvc_timeout(void *_priv)
{
struct gprs_rlcmac_bts *bts = gprs_rlcmac_bts;
- if (!bvc_sig_reset) {
+ if (!the_pcu.bvc_sig_reset) {
LOGP(DBSSGP, LOGL_INFO, "Sending reset on BVCI 0\n");
bssgp_tx_bvc_reset(bctx, 0, BSSGP_CAUSE_OML_INTERV);
osmo_timer_schedule(&bvc_timer, 1, 0);
return;
}
- if (!bvc_reset) {
+ if (!the_pcu.bvc_reset) {
LOGP(DBSSGP, LOGL_INFO, "Sending reset on BVCI %d\n",
bctx->bvci);
bssgp_tx_bvc_reset(bctx, bctx->bvci, BSSGP_CAUSE_OML_INTERV);
@@ -560,7 +568,7 @@ static void bvc_timeout(void *_priv)
return;
}
- if (!bvc_unblocked) {
+ if (!the_pcu.bvc_unblocked) {
LOGP(DBSSGP, LOGL_INFO, "Sending unblock on BVCI %d\n",
bctx->bvci);
bssgp_tx_bvc_unblock(bctx);
@@ -609,8 +617,8 @@ int gprs_bssgp_create(uint16_t local_port, uint32_t sgsn_ip,
dest.sin_port = htons(sgsn_port);
dest.sin_addr.s_addr = htonl(sgsn_ip);
- nsvc = gprs_ns_nsip_connect(bssgp_nsi, &dest, nsei, nsvci);
- if (!nsvc) {
+ the_pcu.nsvc = gprs_ns_nsip_connect(bssgp_nsi, &dest, nsei, nsvci);
+ if (!the_pcu.nsvc) {
LOGP(DBSSGP, LOGL_ERROR, "Failed to create NSVCt\n");
gprs_ns_destroy(bssgp_nsi);
bssgp_nsi = NULL;
@@ -620,7 +628,7 @@ int gprs_bssgp_create(uint16_t local_port, uint32_t sgsn_ip,
bctx = btsctx_alloc(bvci, nsei);
if (!bctx) {
LOGP(DBSSGP, LOGL_ERROR, "Failed to create BSSGP context\n");
- nsvc = NULL;
+ the_pcu.nsvc = NULL;
gprs_ns_destroy(bssgp_nsi);
bssgp_nsi = NULL;
return -EINVAL;
@@ -643,7 +651,7 @@ int gprs_bssgp_create(uint16_t local_port, uint32_t sgsn_ip,
void gprs_bssgp_destroy_or_exit(void)
{
- if (exit_on_destroy) {
+ if (the_pcu.exit_on_destroy) {
LOGP(DBSSGP, LOGL_NOTICE, "Exiting on BSSGP destruction.\n");
exit(0);
}
@@ -655,7 +663,7 @@ void gprs_bssgp_destroy_or_exit(void)
osmo_signal_unregister_handler(SS_L_NS, nsvc_signal_cb, NULL);
- nsvc = NULL;
+ the_pcu.nsvc = NULL;
/* FIXME: move this to libgb: btsctx_free() */
llist_del(&bctx->list);
@@ -671,5 +679,5 @@ void gprs_bssgp_destroy_or_exit(void)
void gprs_bssgp_exit_on_destroy(void)
{
LOGP(DBSSGP, LOGL_NOTICE, "Going to quit on BSSGP destruction\n");
- exit_on_destroy = 1;
+ the_pcu.exit_on_destroy = 1;
}
--
1.8.3.2
27 Jul
27 Jul
10:20 p.m.
New subject: [PATCH 2/8] misc: Move the struct bssgp_bvc_ctx into the struct osmo_pcu
From: Holger Hans Peter Freyther <holger(a)moiji-mobile.com>
---
src/gprs_bssgp_pcu.cpp | 47 ++++++++++++++++++++++++++---------------------
src/gprs_bssgp_pcu.h | 4 ++--
src/gprs_rlcmac.cpp | 1 +
src/gprs_rlcmac_data.cpp | 3 ++-
4 files changed, 31 insertions(+), 24 deletions(-)
diff --git a/src/gprs_bssgp_pcu.cpp b/src/gprs_bssgp_pcu.cpp
index e94b5eb..2b5068e 100644
--- a/src/gprs_bssgp_pcu.cpp
+++ b/src/gprs_bssgp_pcu.cpp
@@ -23,6 +23,7 @@
struct osmo_pcu {
struct gprs_nsvc *nsvc;
+ struct bssgp_bvc_ctx *bctx;
int bvc_sig_reset;
int bvc_reset;
@@ -34,7 +35,6 @@ static struct osmo_pcu the_pcu = { 0, };
struct sgsn_instance *sgsn;
extern void *tall_pcu_ctx;
-struct bssgp_bvc_ctx *bctx = NULL;
extern uint16_t spoof_mcc, spoof_mnc;
struct osmo_timer_list bvc_timer;
@@ -538,12 +538,12 @@ static int nsvc_signal_cb(unsigned int subsys, unsigned int signal,
int gprs_bssgp_tx_fc_bvc(void)
{
- if (!bctx) {
+ if (!the_pcu.bctx) {
LOGP(DBSSGP, LOGL_ERROR, "No bctx\n");
return -EIO;
}
/* FIXME: use real values */
- return bssgp_tx_fc_bvc(bctx, 1, 6553500, 819100, 50000, 50000,
+ return bssgp_tx_fc_bvc(the_pcu.bctx, 1, 6553500, 819100, 50000, 50000,
NULL, NULL);
// return bssgp_tx_fc_bvc(bctx, 1, 84000, 25000, 48000, 45000,
// NULL, NULL);
@@ -555,29 +555,29 @@ static void bvc_timeout(void *_priv)
if (!the_pcu.bvc_sig_reset) {
LOGP(DBSSGP, LOGL_INFO, "Sending reset on BVCI 0\n");
- bssgp_tx_bvc_reset(bctx, 0, BSSGP_CAUSE_OML_INTERV);
+ bssgp_tx_bvc_reset(the_pcu.bctx, 0, BSSGP_CAUSE_OML_INTERV);
osmo_timer_schedule(&bvc_timer, 1, 0);
return;
}
if (!the_pcu.bvc_reset) {
LOGP(DBSSGP, LOGL_INFO, "Sending reset on BVCI %d\n",
- bctx->bvci);
- bssgp_tx_bvc_reset(bctx, bctx->bvci, BSSGP_CAUSE_OML_INTERV);
+ the_pcu.bctx->bvci);
+ bssgp_tx_bvc_reset(the_pcu.bctx, the_pcu.bctx->bvci, BSSGP_CAUSE_OML_INTERV);
osmo_timer_schedule(&bvc_timer, 1, 0);
return;
}
if (!the_pcu.bvc_unblocked) {
LOGP(DBSSGP, LOGL_INFO, "Sending unblock on BVCI %d\n",
- bctx->bvci);
- bssgp_tx_bvc_unblock(bctx);
+ the_pcu.bctx->bvci);
+ bssgp_tx_bvc_unblock(the_pcu.bctx);
osmo_timer_schedule(&bvc_timer, 1, 0);
return;
}
LOGP(DBSSGP, LOGL_DEBUG, "Sending flow control info on BVCI %d\n",
- bctx->bvci);
+ the_pcu.bctx->bvci);
gprs_bssgp_tx_fc_bvc();
osmo_timer_schedule(&bvc_timer, bts->fc_interval, 0);
}
@@ -595,7 +595,7 @@ int gprs_bssgp_create(uint16_t local_port, uint32_t sgsn_ip,
mnc = ((mnc & 0xf00) >> 8) * 100 + ((mnc & 0x0f0) >> 4) * 10 + (mnc
& 0x00f);
cell_id = ntohs(cell_id);
- if (bctx)
+ if (the_pcu.bctx)
return 0; /* if already created, must return 0: no error */
bssgp_nsi = gprs_ns_instantiate(&sgsn_ns_cb, tall_pcu_ctx);
@@ -625,23 +625,23 @@ int gprs_bssgp_create(uint16_t local_port, uint32_t sgsn_ip,
return -EINVAL;
}
- bctx = btsctx_alloc(bvci, nsei);
- if (!bctx) {
+ the_pcu.bctx = btsctx_alloc(bvci, nsei);
+ if (!the_pcu.bctx) {
LOGP(DBSSGP, LOGL_ERROR, "Failed to create BSSGP context\n");
the_pcu.nsvc = NULL;
gprs_ns_destroy(bssgp_nsi);
bssgp_nsi = NULL;
return -EINVAL;
}
- bctx->ra_id.mcc = spoof_mcc ? : mcc;
- bctx->ra_id.mnc = spoof_mnc ? : mnc;
- bctx->ra_id.lac = lac;
- bctx->ra_id.rac = rac;
- bctx->cell_id = cell_id;
+ the_pcu.bctx->ra_id.mcc = spoof_mcc ? : mcc;
+ the_pcu.bctx->ra_id.mnc = spoof_mnc ? : mnc;
+ the_pcu.bctx->ra_id.lac = lac;
+ the_pcu.bctx->ra_id.rac = rac;
+ the_pcu.bctx->cell_id = cell_id;
osmo_signal_register_handler(SS_L_NS, nsvc_signal_cb, NULL);
-// bssgp_tx_bvc_reset(bctx, bctx->bvci, BSSGP_CAUSE_PROTO_ERR_UNSPEC);
+// bssgp_tx_bvc_reset(the_pcu.bctx, the_pcu.bctx->bvci,
BSSGP_CAUSE_PROTO_ERR_UNSPEC);
bvc_timer.cb = bvc_timeout;
@@ -666,9 +666,9 @@ void gprs_bssgp_destroy_or_exit(void)
the_pcu.nsvc = NULL;
/* FIXME: move this to libgb: btsctx_free() */
- llist_del(&bctx->list);
- talloc_free(bctx);
- bctx = NULL;
+ llist_del(&the_pcu.bctx->list);
+ talloc_free(the_pcu.bctx);
+ the_pcu.bctx = NULL;
/* FIXME: blocking... */
@@ -681,3 +681,8 @@ void gprs_bssgp_exit_on_destroy(void)
LOGP(DBSSGP, LOGL_NOTICE, "Going to quit on BSSGP destruction\n");
the_pcu.exit_on_destroy = 1;
}
+
+struct bssgp_bvc_ctx *gprs_bssgp_pcu_current_bctx(void)
+{
+ return the_pcu.bctx;
+}
diff --git a/src/gprs_bssgp_pcu.h b/src/gprs_bssgp_pcu.h
index dc57e4f..dff86a3 100644
--- a/src/gprs_bssgp_pcu.h
+++ b/src/gprs_bssgp_pcu.h
@@ -41,8 +41,6 @@ struct bssgp_bvc_ctx *btsctx_alloc(uint16_t bvci, uint16_t nsei);
#define NS_HDR_LEN 4
#define IE_LLC_PDU 14
-extern struct bssgp_bvc_ctx *bctx;
-
int gprs_bssgp_pcu_rx_dl_ud(struct msgb *msg, struct tlv_parsed *tp);
int gprs_bssgp_pcu_rx_ptp(struct msgb *msg, struct tlv_parsed *tp, struct bssgp_bvc_ctx
*bctx);
@@ -59,4 +57,6 @@ int gprs_bssgp_create(uint16_t local_port, uint32_t sgsn_ip, uint16_t
void gprs_bssgp_exit_on_destroy(void);
void gprs_bssgp_destroy_or_exit(void);
+struct bssgp_bvc_ctx *gprs_bssgp_pcu_current_bctx(void);
+
#endif // GPRS_BSSGP_PCU_H
diff --git a/src/gprs_rlcmac.cpp b/src/gprs_rlcmac.cpp
index 2ea0a61..f3a24f7 100644
--- a/src/gprs_rlcmac.cpp
+++ b/src/gprs_rlcmac.cpp
@@ -1789,6 +1789,7 @@ int gprs_rlcmac_tx_ul_ud(gprs_rlcmac_tbf *tbf)
uint8_t qos_profile[3];
struct msgb *llc_pdu;
unsigned msg_len = NS_HDR_LEN + BSSGP_HDR_LEN + tbf->llc_index;
+ struct bssgp_bvc_ctx *bctx = gprs_bssgp_pcu_current_bctx();
LOGP(DBSSGP, LOGL_INFO, "LLC [PCU -> SGSN] TFI: %u TLLI: 0x%08x len=%d\n",
tbf->tfi, tbf->tlli, tbf->llc_index);
if (!bctx) {
diff --git a/src/gprs_rlcmac_data.cpp b/src/gprs_rlcmac_data.cpp
index 4f2b649..99fac1b 100644
--- a/src/gprs_rlcmac_data.cpp
+++ b/src/gprs_rlcmac_data.cpp
@@ -1253,7 +1253,8 @@ static struct msgb *llc_dequeue(struct gprs_rlcmac_tbf *tbf)
frames = 0xff;
if (octets > 0xffffff)
octets = 0xffffff;
- bssgp_tx_llc_discarded(bctx, tbf->tlli, frames, octets);
+ bssgp_tx_llc_discarded(gprs_bssgp_pcu_current_bctx(),
+ tbf->tlli, frames, octets);
}
return msg;
--
1.8.3.2
10:20 p.m.
New subject: [PATCH 3/8] misc: Remove the unused sgsn pointer from the compilation unit
From: Holger Hans Peter Freyther <holger(a)moiji-mobile.com>
---
src/gprs_bssgp_pcu.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/gprs_bssgp_pcu.cpp b/src/gprs_bssgp_pcu.cpp
index 2b5068e..66c4cb7 100644
--- a/src/gprs_bssgp_pcu.cpp
+++ b/src/gprs_bssgp_pcu.cpp
@@ -33,7 +33,6 @@ struct osmo_pcu {
static struct osmo_pcu the_pcu = { 0, };
-struct sgsn_instance *sgsn;
extern void *tall_pcu_ctx;
extern uint16_t spoof_mcc, spoof_mnc;
--
1.8.3.2
10:20 p.m.
New subject: [PATCH 4/8] misc: Move the bvc_timer into the struct osmo_pcu
From: Holger Hans Peter Freyther <holger(a)moiji-mobile.com>
This continues with the previous changes to reduce the global state.
---
src/gprs_bssgp_pcu.cpp | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/gprs_bssgp_pcu.cpp b/src/gprs_bssgp_pcu.cpp
index 66c4cb7..8c68bf6 100644
--- a/src/gprs_bssgp_pcu.cpp
+++ b/src/gprs_bssgp_pcu.cpp
@@ -25,6 +25,8 @@ struct osmo_pcu {
struct gprs_nsvc *nsvc;
struct bssgp_bvc_ctx *bctx;
+ struct osmo_timer_list bvc_timer;
+
int bvc_sig_reset;
int bvc_reset;
int bvc_unblocked;
@@ -36,8 +38,6 @@ static struct osmo_pcu the_pcu = { 0, };
extern void *tall_pcu_ctx;
extern uint16_t spoof_mcc, spoof_mnc;
-struct osmo_timer_list bvc_timer;
-
static void bvc_timeout(void *_priv);
static int parse_imsi(struct tlv_parsed *tp, char *imsi)
@@ -523,7 +523,7 @@ static int nsvc_signal_cb(unsigned int subsys, unsigned int signal,
case S_NS_BLOCK:
if (nsvc_unblocked) {
nsvc_unblocked = 0;
- osmo_timer_del(&bvc_timer);
+ osmo_timer_del(&the_pcu.bvc_timer);
the_pcu.bvc_sig_reset = 0;
the_pcu.bvc_reset = 0;
the_pcu.bvc_unblocked = 0;
@@ -555,7 +555,7 @@ static void bvc_timeout(void *_priv)
if (!the_pcu.bvc_sig_reset) {
LOGP(DBSSGP, LOGL_INFO, "Sending reset on BVCI 0\n");
bssgp_tx_bvc_reset(the_pcu.bctx, 0, BSSGP_CAUSE_OML_INTERV);
- osmo_timer_schedule(&bvc_timer, 1, 0);
+ osmo_timer_schedule(&the_pcu.bvc_timer, 1, 0);
return;
}
@@ -563,7 +563,7 @@ static void bvc_timeout(void *_priv)
LOGP(DBSSGP, LOGL_INFO, "Sending reset on BVCI %d\n",
the_pcu.bctx->bvci);
bssgp_tx_bvc_reset(the_pcu.bctx, the_pcu.bctx->bvci, BSSGP_CAUSE_OML_INTERV);
- osmo_timer_schedule(&bvc_timer, 1, 0);
+ osmo_timer_schedule(&the_pcu.bvc_timer, 1, 0);
return;
}
@@ -571,14 +571,14 @@ static void bvc_timeout(void *_priv)
LOGP(DBSSGP, LOGL_INFO, "Sending unblock on BVCI %d\n",
the_pcu.bctx->bvci);
bssgp_tx_bvc_unblock(the_pcu.bctx);
- osmo_timer_schedule(&bvc_timer, 1, 0);
+ osmo_timer_schedule(&the_pcu.bvc_timer, 1, 0);
return;
}
LOGP(DBSSGP, LOGL_DEBUG, "Sending flow control info on BVCI %d\n",
the_pcu.bctx->bvci);
gprs_bssgp_tx_fc_bvc();
- osmo_timer_schedule(&bvc_timer, bts->fc_interval, 0);
+ osmo_timer_schedule(&the_pcu.bvc_timer, bts->fc_interval, 0);
}
/* create BSSGP/NS layer instances */
@@ -642,7 +642,7 @@ int gprs_bssgp_create(uint16_t local_port, uint32_t sgsn_ip,
// bssgp_tx_bvc_reset(the_pcu.bctx, the_pcu.bctx->bvci,
BSSGP_CAUSE_PROTO_ERR_UNSPEC);
- bvc_timer.cb = bvc_timeout;
+ the_pcu.bvc_timer.cb = bvc_timeout;
return 0;
@@ -658,7 +658,7 @@ void gprs_bssgp_destroy_or_exit(void)
if (!bssgp_nsi)
return;
- osmo_timer_del(&bvc_timer);
+ osmo_timer_del(&the_pcu.bvc_timer);
osmo_signal_unregister_handler(SS_L_NS, nsvc_signal_cb, NULL);
--
1.8.3.2
10:20 p.m.
New subject: [PATCH 5/8] misc: Move the nsvc_unblocked into the struct osmo_pcu
From: Holger Hans Peter Freyther <holger(a)moiji-mobile.com>
---
src/gprs_bssgp_pcu.cpp | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/gprs_bssgp_pcu.cpp b/src/gprs_bssgp_pcu.cpp
index 8c68bf6..edb088c 100644
--- a/src/gprs_bssgp_pcu.cpp
+++ b/src/gprs_bssgp_pcu.cpp
@@ -27,6 +27,8 @@ struct osmo_pcu {
struct osmo_timer_list bvc_timer;
+ int nsvc_unblocked;
+
int bvc_sig_reset;
int bvc_reset;
int bvc_unblocked;
@@ -492,7 +494,6 @@ static int sgsn_ns_cb(enum gprs_ns_evt event, struct gprs_nsvc *nsvc,
struct msg
return rc;
}
-static int nsvc_unblocked = 0;
static int nsvc_signal_cb(unsigned int subsys, unsigned int signal,
void *handler_data, void *signal_data)
@@ -510,8 +511,8 @@ static int nsvc_signal_cb(unsigned int subsys, unsigned int signal,
switch (signal) {
case S_NS_UNBLOCK:
- if (!nsvc_unblocked) {
- nsvc_unblocked = 1;
+ if (!the_pcu.nsvc_unblocked) {
+ the_pcu.nsvc_unblocked = 1;
LOGP(DPCU, LOGL_NOTICE, "NS-VC %d is unblocked.\n",
the_pcu.nsvc->nsvci);
the_pcu.bvc_sig_reset = 0;
@@ -521,8 +522,8 @@ static int nsvc_signal_cb(unsigned int subsys, unsigned int signal,
}
break;
case S_NS_BLOCK:
- if (nsvc_unblocked) {
- nsvc_unblocked = 0;
+ if (the_pcu.nsvc_unblocked) {
+ the_pcu.nsvc_unblocked = 0;
osmo_timer_del(&the_pcu.bvc_timer);
the_pcu.bvc_sig_reset = 0;
the_pcu.bvc_reset = 0;
--
1.8.3.2
10:20 p.m.
New subject: [PATCH 6/8] bssgp: Reset the BVC and NSVC state in the destroy routine
From: Holger Hans Peter Freyther <holger(a)moiji-mobile.com>
This might explain the issue of the BVCI > 1 not being unblocked
as the internal state has not been re-set when destroying the bssgp.
---
src/gprs_bssgp_pcu.cpp | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/gprs_bssgp_pcu.cpp b/src/gprs_bssgp_pcu.cpp
index edb088c..2e93a02 100644
--- a/src/gprs_bssgp_pcu.cpp
+++ b/src/gprs_bssgp_pcu.cpp
@@ -671,6 +671,10 @@ void gprs_bssgp_destroy_or_exit(void)
the_pcu.bctx = NULL;
/* FIXME: blocking... */
+ the_pcu.nsvc_unblocked = 0;
+ the_pcu.bvc_sig_reset = 0;
+ the_pcu.bvc_reset = 0;
+ the_pcu.bvc_unblocked = 0;
gprs_ns_destroy(bssgp_nsi);
bssgp_nsi = NULL;
--
1.8.3.2
10:20 p.m.
New subject: [PATCH 7/8] bssgp: Remove commented out code that is currently not used.
From: Holger Hans Peter Freyther <holger(a)moiji-mobile.com>
---
src/gprs_bssgp_pcu.cpp | 4 ----
1 file changed, 4 deletions(-)
diff --git a/src/gprs_bssgp_pcu.cpp b/src/gprs_bssgp_pcu.cpp
index 2e93a02..b90f2c5 100644
--- a/src/gprs_bssgp_pcu.cpp
+++ b/src/gprs_bssgp_pcu.cpp
@@ -545,8 +545,6 @@ int gprs_bssgp_tx_fc_bvc(void)
/* FIXME: use real values */
return bssgp_tx_fc_bvc(the_pcu.bctx, 1, 6553500, 819100, 50000, 50000,
NULL, NULL);
-// return bssgp_tx_fc_bvc(bctx, 1, 84000, 25000, 48000, 45000,
-// NULL, NULL);
}
static void bvc_timeout(void *_priv)
@@ -641,8 +639,6 @@ int gprs_bssgp_create(uint16_t local_port, uint32_t sgsn_ip,
osmo_signal_register_handler(SS_L_NS, nsvc_signal_cb, NULL);
-// bssgp_tx_bvc_reset(the_pcu.bctx, the_pcu.bctx->bvci,
BSSGP_CAUSE_PROTO_ERR_UNSPEC);
-
the_pcu.bvc_timer.cb = bvc_timeout;
--
1.8.3.2
10:20 p.m.
New subject: [PATCH 8/8] misc: Move the parsing of the ms_class from RA Capabilities to a method
From: Holger Hans Peter Freyther <holger(a)moiji-mobile.com>
Decrease the number of lines of a single method by splitting things up.
The fewer lines of code, branches and side-effects in a method, the easier
it will be to understand. The other benefit is that one can start creating
unit tests for the some parts of the code.
---
src/gprs_bssgp_pcu.cpp | 75 +++++++++++++++++++++++++++++---------------------
1 file changed, 43 insertions(+), 32 deletions(-)
diff --git a/src/gprs_bssgp_pcu.cpp b/src/gprs_bssgp_pcu.cpp
index b90f2c5..e994f9f 100644
--- a/src/gprs_bssgp_pcu.cpp
+++ b/src/gprs_bssgp_pcu.cpp
@@ -71,6 +71,47 @@ static int parse_imsi(struct tlv_parsed *tp, char *imsi)
return 0;
}
+static int parse_ra_cap_ms_class(struct tlv_parsed *tp)
+{
+ bitvec *block;
+ unsigned rp = 0;
+ uint8_t ms_class = 0;
+ uint8_t cap_len;
+ uint8_t *cap;
+
+ if (!TLVP_PRESENT(tp, BSSGP_IE_MS_RADIO_ACCESS_CAP))
+ return ms_class;
+
+ cap_len = TLVP_LEN(tp, BSSGP_IE_MS_RADIO_ACCESS_CAP);
+ cap = (uint8_t *) TLVP_VAL(tp, BSSGP_IE_MS_RADIO_ACCESS_CAP);
+
+ block = bitvec_alloc(cap_len);
+ bitvec_unpack(block, cap);
+ bitvec_read_field(block, rp, 4); // Access Technology Type
+ bitvec_read_field(block, rp, 7); // Length of Access Capabilities
+ bitvec_read_field(block, rp, 3); // RF Power Capability
+ if (bitvec_read_field(block, rp, 1)) // A5 Bits Present
+ bitvec_read_field(block, rp, 7); // A5 Bits
+ bitvec_read_field(block, rp, 1); // ES IND
+ bitvec_read_field(block, rp, 1); // PS
+ bitvec_read_field(block, rp, 1); // VGCS
+ bitvec_read_field(block, rp, 1); // VBS
+ if (bitvec_read_field(block, rp, 1)) { // Multislot Cap Present
+ if (bitvec_read_field(block, rp, 1)) // HSCSD Present
+ bitvec_read_field(block, rp, 5); // Class
+ if (bitvec_read_field(block, rp, 1)) { // GPRS Present
+ ms_class = bitvec_read_field(block, rp, 5); // Class
+ bitvec_read_field(block, rp, 1); // Ext.
+ }
+ if (bitvec_read_field(block, rp, 1)) // SMS Present
+ bitvec_read_field(block, rp, 4); // SMS Value
+ bitvec_read_field(block, rp, 4); // SMS Value
+ }
+
+ bitvec_free(block);
+ return ms_class;
+}
+
int gprs_bssgp_pcu_rx_dl_ud(struct msgb *msg, struct tlv_parsed *tp)
{
struct bssgp_ud_hdr *budh;
@@ -107,38 +148,8 @@ int gprs_bssgp_pcu_rx_dl_ud(struct msgb *msg, struct tlv_parsed *tp)
parse_imsi(tp, imsi);
/* parse ms radio access capability */
- uint8_t ms_class = 0;
- if (TLVP_PRESENT(tp, BSSGP_IE_MS_RADIO_ACCESS_CAP))
- {
- bitvec *block;
- uint8_t cap_len = TLVP_LEN(tp, BSSGP_IE_MS_RADIO_ACCESS_CAP);
- uint8_t *cap = (uint8_t *) TLVP_VAL(tp, BSSGP_IE_MS_RADIO_ACCESS_CAP);
- unsigned rp = 0;
-
- block = bitvec_alloc(cap_len);
- bitvec_unpack(block, cap);
- bitvec_read_field(block, rp, 4); // Access Technology Type
- bitvec_read_field(block, rp, 7); // Length of Access Capabilities
- bitvec_read_field(block, rp, 3); // RF Power Capability
- if (bitvec_read_field(block, rp, 1)) // A5 Bits Present
- bitvec_read_field(block, rp, 7); // A5 Bits
- bitvec_read_field(block, rp, 1); // ES IND
- bitvec_read_field(block, rp, 1); // PS
- bitvec_read_field(block, rp, 1); // VGCS
- bitvec_read_field(block, rp, 1); // VBS
- if (bitvec_read_field(block, rp, 1)) { // Multislot Cap Present
- if (bitvec_read_field(block, rp, 1)) // HSCSD Present
- bitvec_read_field(block, rp, 5); // Class
- if (bitvec_read_field(block, rp, 1)) { // GPRS Present
- ms_class = bitvec_read_field(block, rp, 5); // Class
- bitvec_read_field(block, rp, 1); // Ext.
- }
- if (bitvec_read_field(block, rp, 1)) // SMS Present
- bitvec_read_field(block, rp, 4); // SMS Value
- bitvec_read_field(block, rp, 4); // SMS Value
- }
- bitvec_free(block);
- }
+ uint8_t ms_class = parse_ra_cap_ms_class(tp);
+
/* get lifetime */
uint16_t delay_csec = 0xffff;
if (TLVP_PRESENT(tp, BSSGP_IE_PDU_LIFETIME))
--
1.8.3.2
29 Jul
29 Jul
1:25 p.m.
New subject: [PATCH 1/8] misc: Introduce a struct osmo_pcu and move things into it.
On Sat, Jul 27, 2013 at 10:20:47PM +0200, Holger Freyther wrote:
Dear Ivan,
I have 14 more patches after this patchset. Could you please indicate
how we can end up with a mode where my commit queue is draining within
a reasonable amount of time? I started with the PCU Emu to re-produce
some SGSN issue(s).
I really appreciate someone reviewing and arguing about the changes
I do and each individual one is fairly small and one topic at a time.
Any ideas?
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
30 Jul
30 Jul
10:56 a.m.
New subject: [PATCH 1/8] misc: Introduce a struct osmo_pcu and move things into it.
Hi Holger,
I looked through your patches and commited them to master, all of them are
reasonable.
I have only one question about the name of the this variable:
+static struct osmo_pcu the_pcu = { 0, };
Why do you use "the_pcu", not "pcu"?
Actually it is not the problem, just unusual for me. :)
Now I am ready for new patchset.
What is the reasonable amount of time for patchset review for you?
Sometimes I am overloaded by other work, but I will try to delay review no
longer than 1-2 days.
Another mode, you can commit patches to master by yourself and just notify
about it mailing list.
2013/7/29 Holger Hans Peter Freyther <hfreyther(a)sysmocom.de>
On Sat, Jul 27, 2013 at 10:20:47PM +0200, Holger
Freyther wrote:
Dear Ivan,
I have 14 more patches after this patchset. Could you please indicate
how we can end up with a mode where my commit queue is draining within
a reasonable amount of time? I started with the PCU Emu to re-produce
some SGSN issue(s).
I really appreciate someone reviewing and arguing about the changes
I do and each individual one is fairly small and one topic at a time.
Any ideas?
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
--
Regards,
Ivan Kluchnikov.
http://fairwaves.ru
11:01 a.m.
New subject: [PATCH 1/8] misc: Introduce a struct osmo_pcu and move things into it.
On Tue, Jul 30, 2013 at 12:56:59PM +0400, Ivan Kluchnikov wrote:
Hi Holger,
Dear Ivan,
thanks a lot.
+static struct osmo_pcu the_pcu = { 0, };
Why do you use "the_pcu", not "pcu"?
I wanted to differentiate that 'pcu' is not a local variable. I could
have picked g_pcu or something else. I used 'the' because there is only
one pcu right now. I will remove that limit though (and remove global
state from the PCU).
Now I am ready for new patchset.
Thanks. I will send things tomorrow.
What is the reasonable amount of time for patchset
review for you?
Sometimes I am overloaded by other work, but I will try to delay review no
longer than 1-2 days.
'reasonable' anything < 5 days. I just want to avoid ending with 40
patches and you as the maintainer not having time to look at them. E.g.
for OpenBSC I try to at least give a quick NACK/or ask for changes.
Another mode, you can commit patches to master by
yourself and just notify
about it mailing list.
I would love to have more eyes before the commit hits the repository.
Everybody is making mistakes and it would be nice if mine are catched
with review too.
cheers
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
11:17 a.m.
New subject: [PATCH 1/8] misc: Introduce a struct osmo_pcu and move things into it.
2013/7/30 Holger Hans Peter Freyther <hfreyther(a)sysmocom.de>
I think we should use this mode, because as you mentioned, reviews are the
good opportunity to catch mistakes.
--
Regards,
Ivan Kluchnikov.
http://fairwaves.ru
I wanted to differentiate that 'pcu' is not a
local variable. I could
have picked g_pcu or something else. I used 'the' because there is only
one pcu right now. I will remove that limit though (and remove global
state from the PCU).
Ok, I understand.
Now I am ready for new patchset.
Thanks. I will send things tomorrow.
What is the reasonable amount of time for
patchset review for you?
Sometimes I am overloaded by other work, but I will try to delay review
no
longer than 1-2 days.
'reasonable' anything < 5 days. I just want to avoid ending with 40
patches and you as the maintainer not having time to look at them. E.g.
for OpenBSC I try to at least give a quick NACK/or ask for changes.
3945
days inactive
3948
days old
osmocom-net-gprs@lists.osmocom.org
11 comments
3 participants
participants (3)
-
Holger Freyther -
Holger Hans Peter Freyther
-
Ivan Kluchnikov