hi,
i like to announce that multislot supports has been finished and is
working well.
it supports all classes, but i could not test other classes than
semi-duplex class 12. the algorithm allocates as many downlink slots as
allowed by class and as avaiable. it will only allocate a single uplink
slot. there is no dynamic dl/up allocation depending on traffic.
the code is located at osmo-pcu: jolly branch: git log 4b470ffe
regards,
andreas
Hi,
this is not meant as an intensive review on memory allocations
in the PCU code. While browsing the code of the ready-to-send
method to select the next payload I noticed the code below:
RlcMacDownlink_t * mac_control_block = (RlcMacDownlink_t *)malloc(sizeof(RlcMacDownlink_t));
LOGP(DRLCMAC, LOGL_DEBUG, "+++++++++++++++++++++++++ TX : Packet Paging Request +++++++++++++++++++++++++\n");
decode_gsm_rlcmac_downlink(pag_vec, mac_control_block);
LOGPC(DCSN1, LOGL_NOTICE, "\n");
LOGP(DRLCMAC, LOGL_DEBUG, "------------------------- TX : Packet Paging Request -------------------------\n");
bitvec_free(pag_vec);
This and other all other places really should use talloc to make finding
memory leaks more easy (e.g. by printing the leak report), the second part
is that we appear to leak this on every paging message sent by the PCU and
the third is actually a question. Should we use a GCC extension that
helps dealing with local variables? GCC can call a cleanup function
on exit of the scope, an example can be seen here[1] nad this[2] is
the description of the feature.
holger
[1] https://mail.gnome.org/archives/gtk-devel-list/2011-November/msg00050.html
[2] http://gcc.gnu.org/onlinedocs/gcc/Variable-Attributes.html#Variable-Attribu…
--
- 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
Hi,
one more email for tonight. Using clang/smatch from time to time
can highlight certain issues. The easiest way to invoke it is this:
$ make CC="clang --analyze" CXX="clang++ --analyze"
1.)
gprs_bssgp_pcu.cpp:241:6: warning: Access to field 'state' results in a dereference of a null pointer (loaded from variable 'bctx')
if (bctx->state & BVC_S_BLOCKED && pdu_type != BSSGP_PDUT_STATUS)
^~~~
the handling of bctx is a bit weird, in theory it can be NULL but
I am not sure if we are likely to receive the messages that would
make the PCU crash though. gprs_bssgp_pcu_rcvmsg can call the above
function/line with a NULL bctx.
2.)
gprs_rlcmac.cpp:728:25: warning: Assigned value is garbage or undefined
tbf->dir.ul.usf[ts] = usf[ts];
^ ~~~~~~~
Probably true, alloc_algorithm_b is really too big to be readable to
verify that this is not a false positive.
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
Holger Hans Peter Freyther wrote:
> in the above it is possible that !poll_tbf and usf == 0x7, is this the
> right behavior in case no TBF could be found?
hi holger,
if poll_tbf is set, we must use an unassigned usf, because only the
polled mobile may transmit. (usf 7 is never assigned.) if poll_tbf is
not set and if no uplink ressource is required, i use usf 7 (any other
usf would also be ok, since we don't need any mobile to transmit on
uplink, but i use 7, because it is never assigend to any mobile).
regards,
andreas
From: Holger Hans Peter Freyther <holger(a)freyther.de>
libosmocore might not be in the standard include path,
add the CFLAGS to the preprocessor flags. This is fixing
the build on the Osmocom Jenkins.
---
src/Makefile.am | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/Makefile.am b/src/Makefile.am
index 041831f..98574fa 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -18,7 +18,7 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#
-AM_CPPFLAGS = $(STD_DEFINES_AND_INCLUDES)
+AM_CPPFLAGS = $(STD_DEFINES_AND_INCLUDES) $(LIBOSMOCORE_CFLAGS) $(LIBOSMOGB_CFLAGS) $(LIBOSMOGSM_CFLAGS)
AM_CXXFLAGS = -Wall -ldl -pthread
noinst_LTLIBRARIES = libgprs.la
--
1.7.10.4
Hi all!
while reviewing the current PCU code in the git repository, it occurred
to me that somehow the jolly_new branch doesn't seem to be based on
master, and the only common ancestor is
9b06ff0c4c49f1927b9029d38e16670a7b7301fb from June 15.
In fact, Ivan seems to have made a number of changes concurrently with
Andreas, but not basing on each other's code. It's really a big mess,
from what I can tell.
I'm referring to the followign commit's by Ivan:
a9e6dc5084627e7c279ba08de7a7809e97ebc539
d78ee736239414021fde8010179f42b86464a238
Which are completely unrelated to the work that Andreas has been doing
at the same time (all his commit's from 2012-06-27 on, i.e.
39621c41f303e24b7324dc4c91447a449d2a654b and later.
I strongly recomend that you coordinate more and re-view each others'
code better.
And regarding the messy situatin with master vs. jolly_new: I think the
only practical solution is to drop one of the two parallel and
incompatible changes regarding the RLC/MAC and TBF establishment
changes.
Do you have any input on how to resolve this specific issue? I think
none of us can afford to waste resources on duplication of work and
creating virtually un-mergeable branches :/
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)
hi laforge,
just curios about two lines at oml.c:
rlcc->parameter[T_DL_TBF_EXT] = *(uint16_t *)cur * 10;
rlcc->parameter[T_UL_TBF_EXT] = *(uint16_t *)cur * 10;
1. isnt it * 2 instead of * 10? i just saw a comment at openbsc for 0,
250 which says: 500.
2. did you forget ntoh(*(uint16_t *)cur)?
regards,
andreas
hi,
finally the TBF acknowledge mode works in up- and downlink. i have
tested it with attachment/detachment and routing area update. changes
are commited to jolly branch.
don't be shocked, but there great changes to previous gprs_rlcmac.cpp
code. it is split into three sources:
- gprs_rlcmac.cpp: general functions to handle TBF instances,
hand-hacked codings
- gprs_rlcmac_data.cpp: handling of uplink and downlink TBF
- gprs_rlcmac_sched.cpp: scheduler to handle ready-to-send requests.
during packet idle mode, assignment of up- or downlink TBF is performed
using IMMEDIATE ASSIGMENT on AGCH. during TBF uplink, downlink
assignment is performed using PACKET DOWNLINK ASSIGNMENT on PACCH. after
TBF downlink, further downlink assignment is performed using PACKET
DOWNLINK ASSIGNMENT. during TBF downlink, uplink assignment is performed
using PACKET UPLINK assignment. this way an attachment / RA update /
detach will complete while staying in packet transfer mode, so no
further assignment must be done on AGCH.
the acknowledged mode keeps states about transmitted and received
blocks, so unreceived or unacknowledged block are retransmitted as
defined in TS 04.60. downlink RLC/MAC data and control blocks are not
queued, but generated at ready-to-send request from layer 1.
a scheduler is used to schedule next RLC/MAC block. it uses round-robin
scheduling, if more than two flows are active in one direction. in
downlink direction, control blocks are prioized. in uplink direction,
polling of mobile is priorized. (USF is set to apply uplink ressource to
MS.) only one timeslot (first PDCH) is used to share the ressources,
currently.
there is a short description (tbf.txt) about states and the process of
current code.
in order to detect missing responses on polling MS, a
gprs_rlcmac_poll_timeout() function is called. layer 1 interface must
implement detection of missed polled uplink control blocks. there are
two ways to perform this:
- The received frame is bad (BFI).
- The GSM indicates that the block should have been already received.
currently pcu_l1_if.c is broken, because none of the detection above is
performed.
best regards,
andreas
Hi Andreas,
I've looked through your commits and looks like you're doing a very
quick progress. Your changes are overlapping with Ivan's work and I
would recommend to merge them the master as often as possible to avoid
duplicated efforts and code divergence. From this quick review I've
got impression that code needs clean up before the merge, so it would
be good if you share your plans on this.
As a general comment, I think we could do merging of your code into
master, but it would be much, much easier if you commit in smaller
"atomic" chunks. E.g. these two last commits contain more changes then
is written in the commit logs and in my opinion should be splitted
into several independent commits.
Note, that I just want to share my opinion and it's up to you and Ivan
to decide how to work together efficiently.
On Thu, Jul 5, 2012 at 6:42 AM, git repository hosting
<gitosis(a)osmocom.org> wrote:
> This is an automated email from the git hooks/post-receive script. It was
> generated because a ref change was pushed to the repository containing
> the project "UNNAMED PROJECT".
>
> The branch, jolly has been updated
> via f3d060c6310bfa979225b871ba463284d3cda887 (commit)
> via 06b195e43e36c0b5100ab03e80fcc87a10db9fc5 (commit)
> from e6228b34a75efcb6b0700ac29672d62539860fbf (commit)
>
> Those revisions listed above that are new to this repository have
> not appeared on any other notification email; so we list those
> revisions in full, below.
>
> - Log -----------------------------------------------------------------
> http://cgit.osmocom.org/cgit/osmo-pcu/commit/?id=f3d060c6310bfa979225b871ba…
>
> commit f3d060c6310bfa979225b871ba463284d3cda887
> Author: Andreas Eversberg <jolly(a)eversberg.eu>
> Date: Thu Jul 5 07:38:49 2012 +0200
>
> Fixed pseudo length of IMMEDIATE ASSIGNMENT message.
>
> The pseudo length may not include the rest-octets, so it stays compatible
> to non-GPRS phones.
>
> At pcu_l1_if.c (OpenBTS) no pseudo length is given, so the frame is
> only 22 bytes long. I could not test if it works.
>
> http://cgit.osmocom.org/cgit/osmo-pcu/commit/?id=06b195e43e36c0b5100ab03e80…
>
> commit 06b195e43e36c0b5100ab03e80fcc87a10db9fc5
> Author: Andreas Eversberg <jolly(a)eversberg.eu>
> Date: Thu Jul 5 07:34:29 2012 +0200
>
> Use cell informations received from PCU socket interface
>
> Cell info is received from socket interface. The given parameters are
> used to replace the hardcoded values (at least for l1 socket interface).
>
> -----------------------------------------------------------------------
>
> Summary of changes:
> configure.ac | 8 ++
> src/Makefile.am | 19 ++----
> src/gprs_bssgp_pcu.cpp | 178 ++++++++++++++++++++++++++++++++++++++++++++--
> src/gprs_bssgp_pcu.h | 24 ++----
> src/gprs_debug.cpp | 2 +-
> src/gprs_rlcmac.cpp | 41 ++++++-----
> src/gprs_rlcmac.h | 25 ++++---
> src/gprs_rlcmac_data.cpp | 79 +++++++++++++-------
> src/pcu_l1_if.cpp | 37 +++++++++-
> src/pcu_main.cpp | 123 ++++++++++++++++++--------------
> src/sysmo_l1_if.cpp | 94 ++++++++++++++++++++++---
> 11 files changed, 471 insertions(+), 159 deletions(-)
>
>
> hooks/post-receive
> --
> UNNAMED PROJECT
>
--
Regards,
Alexander Chemeris.
CEO, Fairwaves LLC / ООО УмРадио
http://fairwaves.ru