osmocom-net-gprs

osmocom-net-gprs@lists.osmocom.org

5 participants
507 discussions

[PATCH 1/8] tbf: Begin to add some structure to the tbf code
by Holger Freyther 16 Oct '13

16 Oct '13

From: Holger Hans Peter Freyther <holger(a)moiji-mobile.com> The TBF should use the IMSI to identify a block flow but all handling is spread across the entire code. Start to clean this up by moving relevant code into the tbf file. Afterwards one can clean up and add more internal structure. --- src/Makefile.am | 6 +- src/gprs_bssgp_pcu.cpp | 121 +------------------------------------ src/tbf.cpp | 160 +++++++++++++++++++++++++++++++++++++++++++++++++ src/tbf.h | 27 +++++++++ 4 files changed, 194 insertions(+), 120 deletions(-) create mode 100644 src/tbf.cpp create mode 100644 src/tbf.h diff --git a/src/Makefile.am b/src/Makefile.am index 38a4b3c..6646f53 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -40,7 +40,8 @@ libgprs_la_SOURCES = \ gsm_timer.cpp \ bitvector.cpp \ pcu_l1_if.cpp \ - pcu_vty.c + pcu_vty.c \ + tbf.cpp if ENABLE_SYSMOBTS libgprs_la_SOURCES += \ @@ -72,7 +73,8 @@ noinst_HEADERS = \ bitvector.h \ pcu_vty.h \ sysmo_l1_if.h \ - femtobts.h + femtobts.h \ + tbf.h osmo_pcu_SOURCES = pcu_main.cpp diff --git a/src/gprs_bssgp_pcu.cpp b/src/gprs_bssgp_pcu.cpp index 4833af8..3432bca 100644 --- a/src/gprs_bssgp_pcu.cpp +++ b/src/gprs_bssgp_pcu.cpp @@ -21,6 +21,7 @@ #include <gprs_rlcmac.h> #include <gprs_bssgp_pcu.h> #include <pcu_l1_if.h> +#include <tbf.h> static struct gprs_bssgp_pcu the_pcu = { 0, }; @@ -103,12 +104,9 @@ static int gprs_bssgp_pcu_rx_dl_ud(struct msgb *msg, struct tlv_parsed *tp) { struct bssgp_ud_hdr *budh; - int8_t tfi; /* must be signed */ - uint32_t tlli; uint8_t *data; uint16_t len; - struct gprs_rlcmac_tbf *tbf; char imsi[16] = "000"; budh = (struct bssgp_ud_hdr *)msgb_bssgph(msg); @@ -123,7 +121,7 @@ static int gprs_bssgp_pcu_rx_dl_ud(struct msgb *msg, struct tlv_parsed *tp) data = (uint8_t *) TLVP_VAL(tp, BSSGP_IE_LLC_PDU); len = TLVP_LEN(tp, BSSGP_IE_LLC_PDU); - if (len > sizeof(tbf->llc_frame)) + if (len > sizeof(gprs_rlcmac_tbf::llc_frame)) { LOGP(DBSSGP, LOGL_NOTICE, "BSSGP TLLI=0x%08x Rx UL-UD IE_LLC_PDU too large\n", tlli); return bssgp_tx_status(BSSGP_CAUSE_COND_IE_ERR, NULL, msg); @@ -154,120 +152,7 @@ static int gprs_bssgp_pcu_rx_dl_ud(struct msgb *msg, struct tlv_parsed *tp) LOGP(DBSSGP, LOGL_INFO, "LLC [SGSN -> PCU] = TLLI: 0x%08x IMSI: %s len: %d\n", tlli, imsi, len); - /* check for existing TBF */ - if ((tbf = tbf_by_tlli(tlli, GPRS_RLCMAC_DL_TBF))) { - LOGP(DRLCMAC, LOGL_INFO, "TBF: APPEND TFI: %u TLLI: 0x%08x\n", tbf->tfi, tbf->tlli); - if (tbf->state == GPRS_RLCMAC_WAIT_RELEASE) { - LOGP(DRLCMAC, LOGL_DEBUG, "TBF in WAIT RELEASE state " - "(T3193), so reuse TBF\n"); - memcpy(tbf->llc_frame, data, len); - tbf->llc_length = len; - memset(&tbf->dir.dl, 0, sizeof(tbf->dir.dl)); /* reset - rlc states */ - tbf->state_flags &= GPRS_RLCMAC_FLAG_TO_MASK; /* keep - to flags */ - tbf->state_flags &= ~(1 << GPRS_RLCMAC_FLAG_CCCH); - if (!tbf->ms_class && ms_class) - tbf->ms_class = ms_class; - tbf_update(tbf); - gprs_rlcmac_trigger_downlink_assignment(tbf, tbf, NULL); - } else { - /* the TBF exists, so we must write it in the queue - * we prepend lifetime in front of PDU */ - struct timeval *tv; - struct msgb *llc_msg = msgb_alloc(len + sizeof(*tv), - "llc_pdu_queue"); - if (!llc_msg) - return -ENOMEM; - tv = (struct timeval *)msgb_put(llc_msg, sizeof(*tv)); - if (the_pcu.bts->force_llc_lifetime) - delay_csec = the_pcu.bts->force_llc_lifetime; - /* keep timestap at 0 for infinite delay */ - if (delay_csec != 0xffff) { - /* calculate timestamp of timeout */ - gettimeofday(tv, NULL); - tv->tv_usec += (delay_csec % 100) * 10000; - tv->tv_sec += delay_csec / 100; - if (tv->tv_usec > 999999) { - tv->tv_usec -= 1000000; - tv->tv_sec++; - } - } - memcpy(msgb_put(llc_msg, len), data, len); - msgb_enqueue(&tbf->llc_queue, llc_msg); - /* set ms class for updating TBF */ - if (!tbf->ms_class && ms_class) - tbf->ms_class = ms_class; - } - } else { - uint8_t trx, ta, ss; - int8_t use_trx; - struct gprs_rlcmac_tbf *old_tbf; - int rc; - - /* check for uplink data, so we copy our informations */ - tbf = tbf_by_tlli(tlli, GPRS_RLCMAC_UL_TBF); - if (tbf && tbf->dir.ul.contention_resolution_done - && !tbf->dir.ul.final_ack_sent) { - use_trx = tbf->trx; - ta = tbf->ta; - ss = 0; - old_tbf = tbf; - } else { - use_trx = -1; - /* we already have an uplink TBF, so we use that TA */ - if (tbf) - ta = tbf->ta; - else { - /* recall TA */ - rc = recall_timing_advance(tlli); - if (rc < 0) { - LOGP(DRLCMAC, LOGL_NOTICE, "TA unknown" - ", assuming 0\n"); - ta = 0; - } else - ta = rc; - } - ss = 1; /* PCH assignment only allows one timeslot */ - old_tbf = NULL; - } - - // Create new TBF (any TRX) - tfi = tfi_find_free(the_pcu.bts, GPRS_RLCMAC_DL_TBF, &trx, use_trx); - if (tfi < 0) { - LOGP(DRLCMAC, LOGL_NOTICE, "No PDCH resource\n"); - /* FIXME: send reject */ - return -EBUSY; - } - /* set number of downlink slots according to multislot class */ - tbf = tbf_alloc(the_pcu.bts, tbf, GPRS_RLCMAC_DL_TBF, tfi, trx, ms_class, - ss); - if (!tbf) { - LOGP(DRLCMAC, LOGL_NOTICE, "No PDCH ressource\n"); - /* FIXME: send reject */ - return -EBUSY; - } - tbf->tlli = tlli; - tbf->tlli_valid = 1; - tbf->ta = ta; - - LOGP(DRLCMAC, LOGL_DEBUG, "TBF: [DOWNLINK] START TFI: %d TLLI: 0x%08x \n", tbf->tfi, tbf->tlli); - - /* new TBF, so put first frame */ - memcpy(tbf->llc_frame, data, len); - tbf->llc_length = len; - - /* trigger downlink assignment and set state to ASSIGN. - * we don't use old_downlink, so the possible uplink is used - * to trigger downlink assignment. if there is no uplink, - * AGCH is used. */ - gprs_rlcmac_trigger_downlink_assignment(tbf, old_tbf, imsi); - } - - /* store IMSI for debugging purpose */ - strncpy(tbf->meas.imsi, imsi, sizeof(tbf->meas.imsi) - 1); - - return 0; + return tbf_handle(the_pcu.bts, tlli, imsi, ms_class, delay_csec, data, len); } int gprs_bssgp_pcu_rx_paging_ps(struct msgb *msg, struct tlv_parsed *tp) diff --git a/src/tbf.cpp b/src/tbf.cpp new file mode 100644 index 0000000..816fdb8 --- /dev/null +++ b/src/tbf.cpp @@ -0,0 +1,160 @@ +/* Copied from gprs_bssgp_pcu.cpp + * + * Copyright (C) 2012 Ivan Klyuchnikov + * Copyright (C) 2013 by Holger Hans Peter Freyther + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. + */ + +#include <tbf.h> +#include <gprs_rlcmac.h> +#include <gprs_debug.h> + +extern "C" { +#include <osmocom/core/msgb.h> +} + +#include <errno.h> +#include <string.h> + +/** + * TODO: split into unit test-able parts... + */ +int tbf_handle(struct gprs_rlcmac_bts *bts, + const uint32_t tlli, const char *imsi, + const uint8_t ms_class, const uint16_t pdu_delay_csec, + const uint8_t *data, const uint16_t len) +{ + struct gprs_rlcmac_tbf *tbf; + int8_t tfi; /* must be signed */ + + /* check for existing TBF */ + if ((tbf = tbf_by_tlli(tlli, GPRS_RLCMAC_DL_TBF))) { + LOGP(DRLCMAC, LOGL_INFO, "TBF: APPEND TFI: %u TLLI: 0x%08x\n", tbf->tfi, tbf->tlli); + if (tbf->state == GPRS_RLCMAC_WAIT_RELEASE) { + LOGP(DRLCMAC, LOGL_DEBUG, "TBF in WAIT RELEASE state " + "(T3193), so reuse TBF\n"); + memcpy(tbf->llc_frame, data, len); + tbf->llc_length = len; + memset(&tbf->dir.dl, 0, sizeof(tbf->dir.dl)); /* reset + rlc states */ + tbf->state_flags &= GPRS_RLCMAC_FLAG_TO_MASK; /* keep + to flags */ + tbf->state_flags &= ~(1 << GPRS_RLCMAC_FLAG_CCCH); + if (!tbf->ms_class && ms_class) + tbf->ms_class = ms_class; + tbf_update(tbf); + gprs_rlcmac_trigger_downlink_assignment(tbf, tbf, NULL); + } else { + /* the TBF exists, so we must write it in the queue + * we prepend lifetime in front of PDU */ + struct timeval *tv; + struct msgb *llc_msg = msgb_alloc(len + sizeof(*tv), + "llc_pdu_queue"); + if (!llc_msg) + return -ENOMEM; + tv = (struct timeval *)msgb_put(llc_msg, sizeof(*tv)); + + uint16_t delay_csec; + if (bts->force_llc_lifetime) + delay_csec = bts->force_llc_lifetime; + else + delay_csec = pdu_delay_csec; + /* keep timestap at 0 for infinite delay */ + if (delay_csec != 0xffff) { + /* calculate timestamp of timeout */ + gettimeofday(tv, NULL); + tv->tv_usec += (delay_csec % 100) * 10000; + tv->tv_sec += delay_csec / 100; + if (tv->tv_usec > 999999) { + tv->tv_usec -= 1000000; + tv->tv_sec++; + } + } + memcpy(msgb_put(llc_msg, len), data, len); + msgb_enqueue(&tbf->llc_queue, llc_msg); + /* set ms class for updating TBF */ + if (!tbf->ms_class && ms_class) + tbf->ms_class = ms_class; + } + } else { + uint8_t trx, ta, ss; + int8_t use_trx; + struct gprs_rlcmac_tbf *old_tbf; + int rc; + + /* check for uplink data, so we copy our informations */ + tbf = tbf_by_tlli(tlli, GPRS_RLCMAC_UL_TBF); + if (tbf && tbf->dir.ul.contention_resolution_done + && !tbf->dir.ul.final_ack_sent) { + use_trx = tbf->trx; + ta = tbf->ta; + ss = 0; + old_tbf = tbf; + } else { + use_trx = -1; + /* we already have an uplink TBF, so we use that TA */ + if (tbf) + ta = tbf->ta; + else { + /* recall TA */ + rc = recall_timing_advance(tlli); + if (rc < 0) { + LOGP(DRLCMAC, LOGL_NOTICE, "TA unknown" + ", assuming 0\n"); + ta = 0; + } else + ta = rc; + } + ss = 1; /* PCH assignment only allows one timeslot */ + old_tbf = NULL; + } + + // Create new TBF (any TRX) + tfi = tfi_find_free(bts, GPRS_RLCMAC_DL_TBF, &trx, use_trx); + if (tfi < 0) { + LOGP(DRLCMAC, LOGL_NOTICE, "No PDCH resource\n"); + /* FIXME: send reject */ + return -EBUSY; + } + /* set number of downlink slots according to multislot class */ + tbf = tbf_alloc(bts, tbf, GPRS_RLCMAC_DL_TBF, tfi, trx, ms_class, + ss); + if (!tbf) { + LOGP(DRLCMAC, LOGL_NOTICE, "No PDCH ressource\n"); + /* FIXME: send reject */ + return -EBUSY; + } + tbf->tlli = tlli; + tbf->tlli_valid = 1; + tbf->ta = ta; + + LOGP(DRLCMAC, LOGL_DEBUG, "TBF: [DOWNLINK] START TFI: %d TLLI: 0x%08x \n", tbf->tfi, tbf->tlli); + + /* new TBF, so put first frame */ + memcpy(tbf->llc_frame, data, len); + tbf->llc_length = len; + + /* trigger downlink assignment and set state to ASSIGN. + * we don't use old_downlink, so the possible uplink is used + * to trigger downlink assignment. if there is no uplink, + * AGCH is used. */ + gprs_rlcmac_trigger_downlink_assignment(tbf, old_tbf, imsi); + } + + /* store IMSI for debugging purpose */ + strncpy(tbf->meas.imsi, imsi, sizeof(tbf->meas.imsi) - 1); + return 0; +} diff --git a/src/tbf.h b/src/tbf.h new file mode 100644 index 0000000..1535e8c --- /dev/null +++ b/src/tbf.h @@ -0,0 +1,27 @@ +/* + * Copyright (C) 2013 by Holger Hans Peter Freyther + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. + */ + +#pragma once + +#include <stdint.h> + +struct gprs_rlcmac_bts; + +int tbf_handle(struct gprs_rlcmac_bts *bts, + const uint32_t tlli, const char *imsi, const uint8_t ms_class, + const uint16_t delay_csec, const uint8_t *data, const uint16_t len); -- 1.8.4.rc3

1 7

[PATCH 1/3] tfi: The tfi_alloc doesn't allocate anything, rename the function
by Holger Hans Peter Freyther 02 Oct '13

02 Oct '13

From: Holger Hans Peter Freyther <holger(a)moiji-mobile.com> Call things by what they do and this function doesn't allocate anything but it is searching for the first unallocated tbf index. --- src/gprs_bssgp_pcu.cpp | 2 +- src/gprs_rlcmac.cpp | 3 ++- src/gprs_rlcmac.h | 3 ++- src/gprs_rlcmac_data.cpp | 4 ++-- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/gprs_bssgp_pcu.cpp b/src/gprs_bssgp_pcu.cpp index 6f67f52..4833af8 100644 --- a/src/gprs_bssgp_pcu.cpp +++ b/src/gprs_bssgp_pcu.cpp @@ -233,7 +233,7 @@ static int gprs_bssgp_pcu_rx_dl_ud(struct msgb *msg, struct tlv_parsed *tp) } // Create new TBF (any TRX) - tfi = tfi_alloc(the_pcu.bts, GPRS_RLCMAC_DL_TBF, &trx, use_trx); + tfi = tfi_find_free(the_pcu.bts, GPRS_RLCMAC_DL_TBF, &trx, use_trx); if (tfi < 0) { LOGP(DRLCMAC, LOGL_NOTICE, "No PDCH resource\n"); /* FIXME: send reject */ diff --git a/src/gprs_rlcmac.cpp b/src/gprs_rlcmac.cpp index 8874983..913ae48 100644 --- a/src/gprs_rlcmac.cpp +++ b/src/gprs_rlcmac.cpp @@ -2,6 +2,7 @@ * * Copyright (C) 2012 Ivan Klyuchnikov * Copyright (C) 2012 Andreas Eversberg <jolly(a)eversberg.eu> + * Copyright (C) 2013 by Holger Hans Peter Freyther * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -176,7 +177,7 @@ void debug_diagram(int diag, const char *format, ...) /* FIXME: spread ressources over multiple TRX. Also add option to use same * TRX in case of existing TBF for TLLI in the other direction. */ /* search for free TFI and return TFI, TRX */ -int tfi_alloc(struct gprs_rlcmac_bts *bts, enum gprs_rlcmac_tbf_direction dir, +int tfi_find_free(struct gprs_rlcmac_bts *bts, enum gprs_rlcmac_tbf_direction dir, uint8_t *_trx, int8_t use_trx) { struct gprs_rlcmac_pdch *pdch; diff --git a/src/gprs_rlcmac.h b/src/gprs_rlcmac.h index ac437f5..4329cb8 100644 --- a/src/gprs_rlcmac.h +++ b/src/gprs_rlcmac.h @@ -1,6 +1,7 @@ /* gprs_rlcmac.h * * Copyright (C) 2012 Ivan Klyuchnikov + * Copyright (C) 2013 by Holger Hans Peter Freyther * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -309,7 +310,7 @@ int sba_alloc(uint8_t *_trx, uint8_t *_ts, uint32_t *_fn, uint8_t ta); struct gprs_rlcmac_sba *sba_find(uint8_t trx, uint8_t ts, uint32_t fn); -int tfi_alloc(struct gprs_rlcmac_bts *bts, enum gprs_rlcmac_tbf_direction dir, +int tfi_find_free(struct gprs_rlcmac_bts *bts, enum gprs_rlcmac_tbf_direction dir, uint8_t *_trx, int8_t use_trx); struct gprs_rlcmac_tbf *tbf_alloc(struct gprs_rlcmac_bts *bts, diff --git a/src/gprs_rlcmac_data.cpp b/src/gprs_rlcmac_data.cpp index 4d5acb9..ff5274f 100644 --- a/src/gprs_rlcmac_data.cpp +++ b/src/gprs_rlcmac_data.cpp @@ -239,7 +239,7 @@ static struct gprs_rlcmac_tbf *alloc_ul_tbf(int8_t use_trx, uint8_t ms_class, uint8_t tfi; /* create new TBF, use sme TRX as DL TBF */ - tfi = tfi_alloc(bts, GPRS_RLCMAC_UL_TBF, &trx, use_trx); + tfi = tfi_find_free(bts, GPRS_RLCMAC_UL_TBF, &trx, use_trx); if (tfi < 0) { LOGP(DRLCMAC, LOGL_NOTICE, "No PDCH ressource\n"); /* FIXME: send reject */ @@ -1174,7 +1174,7 @@ int gprs_rlcmac_rcv_rach(uint8_t ra, uint32_t Fn, int16_t qta) "(AGCH)\n"); } else { // Create new TBF - tfi = tfi_alloc(bts, GPRS_RLCMAC_UL_TBF, &trx, -1); + tfi = tfi_find_free(bts, GPRS_RLCMAC_UL_TBF, &trx, -1); if (tfi < 0) { LOGP(DRLCMAC, LOGL_NOTICE, "No PDCH ressource\n"); /* FIXME: send reject */ -- 1.8.3.2

2 8

alloc_algorithm_a
by Vladimir Rolbin 30 Sep '13

30 Sep '13

Hi all, It looks like usf found (allocated) is never used. I guess something like tbf->dir.ul.usf[ts] = usf is missed. Regards, Vladimir Rolbin.

2 3

[PATCH 01/10] bssgp: Re-indent the switch/case statement
by Holger Freyther 04 Sep '13

04 Sep '13

From: Holger Hans Peter Freyther <holger(a)moiji-mobile.com> --- src/gprs_bssgp_pcu.cpp | 90 +++++++++++++++++++++++++------------------------- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/src/gprs_bssgp_pcu.cpp b/src/gprs_bssgp_pcu.cpp index ae47fb2..4326303 100644 --- a/src/gprs_bssgp_pcu.cpp +++ b/src/gprs_bssgp_pcu.cpp @@ -365,51 +365,51 @@ int gprs_bssgp_pcu_rx_sign(struct msgb *msg, struct tlv_parsed *tp, struct bssgp DEBUGP(DBSSGP, "BSSGP BVCI=%u Rx BVC STATUS\n", bctx->bvci); /* FIXME: send NM_STATUS.ind to NM */ break; - case BSSGP_PDUT_SUSPEND_ACK: - LOGP(DBSSGP, LOGL_DEBUG, "rx BSSGP_PDUT_SUSPEND_ACK\n"); - break; - case BSSGP_PDUT_SUSPEND_NACK: - LOGP(DBSSGP, LOGL_DEBUG, "rx BSSGP_PDUT_SUSPEND_NACK\n"); - break; - case BSSGP_PDUT_BVC_RESET_ACK: - LOGP(DBSSGP, LOGL_DEBUG, "rx BSSGP_PDUT_BVC_RESET_ACK\n"); - if (!the_pcu.bvc_sig_reset) - the_pcu.bvc_sig_reset = 1; - else - the_pcu.bvc_reset = 1; - bvc_timeout(NULL); - break; - case BSSGP_PDUT_PAGING_PS: - LOGP(DBSSGP, LOGL_NOTICE, "RX: [SGSN->PCU] BSSGP_PDUT_PAGING_PS\n"); - gprs_bssgp_pcu_rx_paging_ps(msg, tp); - break; - case BSSGP_PDUT_PAGING_CS: - LOGP(DBSSGP, LOGL_DEBUG, "rx BSSGP_PDUT_PAGING_CS\n"); - break; - case BSSGP_PDUT_RESUME_ACK: - LOGP(DBSSGP, LOGL_DEBUG, "rx BSSGP_PDUT_RESUME_ACK\n"); - break; - case BSSGP_PDUT_RESUME_NACK: - LOGP(DBSSGP, LOGL_DEBUG, "rx BSSGP_PDUT_RESUME_NACK\n"); - break; - case BSSGP_PDUT_FLUSH_LL: - LOGP(DBSSGP, LOGL_DEBUG, "rx BSSGP_PDUT_FLUSH_LL\n"); - break; - case BSSGP_PDUT_BVC_BLOCK_ACK: - LOGP(DBSSGP, LOGL_DEBUG, "rx BSSGP_PDUT_SUSPEND_ACK\n"); - break; - case BSSGP_PDUT_BVC_UNBLOCK_ACK: - LOGP(DBSSGP, LOGL_DEBUG, "rx BSSGP_PDUT_BVC_UNBLOCK_ACK\n"); - the_pcu.bvc_unblocked = 1; - bvc_timeout(NULL); - break; - case BSSGP_PDUT_SGSN_INVOKE_TRACE: - LOGP(DBSSGP, LOGL_DEBUG, "rx BSSGP_PDUT_SGSN_INVOKE_TRACE\n"); - break; - default: - LOGP(DBSSGP, LOGL_NOTICE, "BSSGP BVCI=%u Rx PDU type 0x%02x unknown\n", bctx->bvci, bgph->pdu_type); - rc = bssgp_tx_status(BSSGP_CAUSE_PROTO_ERR_UNSPEC, NULL, msg); - break; + case BSSGP_PDUT_SUSPEND_ACK: + LOGP(DBSSGP, LOGL_DEBUG, "rx BSSGP_PDUT_SUSPEND_ACK\n"); + break; + case BSSGP_PDUT_SUSPEND_NACK: + LOGP(DBSSGP, LOGL_DEBUG, "rx BSSGP_PDUT_SUSPEND_NACK\n"); + break; + case BSSGP_PDUT_BVC_RESET_ACK: + LOGP(DBSSGP, LOGL_DEBUG, "rx BSSGP_PDUT_BVC_RESET_ACK\n"); + if (!the_pcu.bvc_sig_reset) + the_pcu.bvc_sig_reset = 1; + else + the_pcu.bvc_reset = 1; + bvc_timeout(NULL); + break; + case BSSGP_PDUT_PAGING_PS: + LOGP(DBSSGP, LOGL_NOTICE, "RX: [SGSN->PCU] BSSGP_PDUT_PAGING_PS\n"); + gprs_bssgp_pcu_rx_paging_ps(msg, tp); + break; + case BSSGP_PDUT_PAGING_CS: + LOGP(DBSSGP, LOGL_DEBUG, "rx BSSGP_PDUT_PAGING_CS\n"); + break; + case BSSGP_PDUT_RESUME_ACK: + LOGP(DBSSGP, LOGL_DEBUG, "rx BSSGP_PDUT_RESUME_ACK\n"); + break; + case BSSGP_PDUT_RESUME_NACK: + LOGP(DBSSGP, LOGL_DEBUG, "rx BSSGP_PDUT_RESUME_NACK\n"); + break; + case BSSGP_PDUT_FLUSH_LL: + LOGP(DBSSGP, LOGL_DEBUG, "rx BSSGP_PDUT_FLUSH_LL\n"); + break; + case BSSGP_PDUT_BVC_BLOCK_ACK: + LOGP(DBSSGP, LOGL_DEBUG, "rx BSSGP_PDUT_SUSPEND_ACK\n"); + break; + case BSSGP_PDUT_BVC_UNBLOCK_ACK: + LOGP(DBSSGP, LOGL_DEBUG, "rx BSSGP_PDUT_BVC_UNBLOCK_ACK\n"); + the_pcu.bvc_unblocked = 1; + bvc_timeout(NULL); + break; + case BSSGP_PDUT_SGSN_INVOKE_TRACE: + LOGP(DBSSGP, LOGL_DEBUG, "rx BSSGP_PDUT_SGSN_INVOKE_TRACE\n"); + break; + default: + LOGP(DBSSGP, LOGL_NOTICE, "BSSGP BVCI=%u Rx PDU type 0x%02x unknown\n", bctx->bvci, bgph->pdu_type); + rc = bssgp_tx_status(BSSGP_CAUSE_PROTO_ERR_UNSPEC, NULL, msg); + break; } return rc; } -- 1.8.3.2

4 16

TBF and API design.. (boolean/int trap)
by Holger Hans Peter Freyther 24 Aug '13

24 Aug '13

Good Evening, I started a branch to add a basic structure to the TBF handling. The reasons why I am doing this is for another lengthy email. I just came across a piece of code and would like to create some awareness: int write_immediate_assignment(bitvec * dest, uint8_t downlink, uint8_t ra, uint32_t ref_fn, uint8_t ta, uint16_t arfcn, uint8_t ts, uint8_t tsc, uint8_t tfi, uint8_t usf, uint32_t tlli, uint8_t polling, uint32_t fn, uint8_t single_block, uint8_t alpha, uint8_t gamma, int8_t ta_idx); and invocations like: if (sb) plen = write_immediate_assignment(immediate_assignment, 0, ra, Fn, qta >> 2, bts->trx[trx].arfcn, ts, bts->trx[trx].pdch[ts].tsc, 0, 0, 0, 0, sb_fn, 1, bts->alpha, bts->gamma, -1); else plen = write_immediate_assignment(immediate_assignment, 0, ra, Fn, tbf->ta, tbf->arfcn, tbf->first_ts, tbf->tsc, tbf->tfi, tbf->dir.ul.usf[tbf->first_ts], 0, 0, 0, 0, bts->alpha, bts->gamma, -1); With the above method and two invocations it is difficult to get all arguments right and it is a strong argument against ever writing code like this. With C++ one could create overloads that take the bts and tbf as parameters. Or if one really only wants to have a single method use a struct to pass the parameters. This has the benefit of naming the parameters and will certainly require the same amount of stack space as the current option. So please, don't write methods like the above as they make maintaining the code more difficult than it needs to be. 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

2 1

C++ for the PCU
by Holger Hans Peter Freyther 05 Aug '13

05 Aug '13

Dear Ivan, all, I started to do work on the PCU and didn't compile C++ for a long time. I was surprised how long it takes to compile the code. From my point of view we are taking the hit of g++ without really using any of the features that C++ vould provide (e.g. no stack variables with automatic cleanup..).

4 5

[PATCH 1/9] misc: Ignore the two osmo-pcu binaries and files created by vim
by Holger Hans Peter Freyther 02 Aug '13

02 Aug '13

From: Holger Hans Peter Freyther <holger(a)moiji-mobile.com> --- .gitignore | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.gitignore b/.gitignore index 032b9cf..4f418d8 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ *.o *.lo *.a +*.sw? Makefile.in Makefile .deps @@ -22,3 +23,7 @@ core core.* osmoappdesc.pyc + +# binaries +src/osmo-pcu +src/osmo-pcu-remote -- 1.8.3.2

3 13

[PATCH 1/8] misc: Introduce a struct osmo_pcu and move things into it.
by Holger Freyther 30 Jul '13

30 Jul '13

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

3 11

[PATCH 1/2] misc: Remove if (timer_pending) stop_timer idiom from the code
by Holger Hans Peter Freyther 29 Jul '13

29 Jul '13

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

4 4

Weird usage of gprs_ns_vty_init from gprs_bssgp_create
by Holger Hans Peter Freyther 12 Jul '13

12 Jul '13

Dear Andreas, Ivan, when trying to understand the CPU usage of the PCU without having any subscribers I noticed the following backtrace: #0 _int_malloc (av=0x402fe4d4, bytes=52) at malloc.c:3241 #1 0x4023a3b8 in __GI___libc_malloc (bytes=52) at malloc.c:2859 #2 0x4929b6e8 in _talloc_zero () from /usr/lib/libosmocore.so.4 #3 0x49422e44 in vector_init () from /usr/lib/libosmovty.so.0 #4 0x4941f1f8 in install_element () from /usr/lib/libosmovty.so.0 #5 0x492e602c in gprs_ns_vty_init () from /usr/lib/libosmogb.so.2 #6 0x0001b670 in gprs_bssgp_create (local_port=<optimized out>, sgsn_ip=184194049, sgsn_port=<optimized out>, nsei=<optimized out>, nsvci=2, bvci=2, mcc=<optimized out>, mnc=<optimized out>, lac=1, rac=0, cell_id=0) at gprs_bssgp_pcu.cpp:585 #7 0x00016990 in pcu_rx_info_ind (info_ind=0x7) at pcu_l1_if.cpp:431 #8 pcu_rx (msg_type=<optimized out>, pcu_prim=0x3) at pcu_l1_if.cpp:577 #9 0x00017914 in pcu_sock_read (bfd=0x6a3f0) at sysmo_sock.cpp:151 #10 pcu_sock_cb (bfd=0x6a3f0, flags=1) at sysmo_sock.cpp:218 #11 0x49294ff4 in osmo_select_main () from /usr/lib/libosmocore.so.4 #12 0x0000a668 in main (argc=<optimized out>, argv=0xbee40b04) at pcu_main.cpp:219 This means when a info indication is received gprs_bssgp_create will be called which will re-initialize the VTY commands but more important re-set vty_nsi inside the VTY code of libosmogb. But it also means that vty_nsi will be a dangling pointer in case of 1.) gprs_ns_nsip_connect, btsctx_alloc, gprs_ns_nsip_listen fail 2.) Between the sysmobts software exiting and re-starting and bringing OML back up 3.) More cases that I might not understand. The dangling pointer will lead to a crash when the VTY of the PCU is used in such situation/window. does any of you care to fix that race? I think the easiest would be to just close/open/bind/listen/connect on the NS once we get a new config. 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

4 10

← Newer
1
...
44
45
46
47
48
49
50
51
Older →

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

osmocom-net-gprs