osmo-pcu[master]: EDGE: fix wrong encoding of LH bits

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Minh-Quang Nguyen gerrit-no-reply at lists.osmocom.org
Mon Oct 16 13:50:01 UTC 2017


Patch Set 3:

> > > > I got an error about missing QEMU when running make check
 > > without
 > > > > our LC15 toolchain built from Yocto framework.
 > > >
 > > > I suspect you're still doing a cross-compile, i.e. the
 > > > './configure' was executed with the environment for your
 > > > cross-toolchain?
 > > >
 > > > > Any suggestion?
 > > >
 > > > If you do a native compile on your PC, rendering an x86/x68_64
 > > > executable for osmo-pcu, make check should work for master. 
 > This
 > > > is what all of us execute on our various development machines
 > > (all
 > > > flavors of Linux from Gentoo to Arch to Debian to Ubuntu), and
 > > > which is also what our jenkins.osmoocom.org is testing
 > > continuously
 > > > e.g. for verifying patch submissions.
 > > >
 > > > In case of doubt, feel free to open a redmine issue attaching
 > > your
 > > > config.log and "make check" output.  Thanks!
 > >
 > > I found out an issue with patch set #2 using two successive
 > > bitvec_set_bit(dest, H ,2) calls making bit index increased by 4
 > > bits which is not correct.
 > 
 > In my original comments I had suggested to use two consecutive
 > "bitvec_set_bit(dest, H)" in order to append two "H" bits, not
 > to duplicate the "bitvec_write_field(dest, &wp, 3, 2);", which will
 > in fact append four bits (two times '2' bits), both of them '1' as
 > 3 = 11b.
 > 
 > > I have also noticed that my MS is able to send packet control ack
 > > as 4 successive PRACH bursts without this patch using a clean
 > Yocto
 > > image built with up-to-date libraries.
 > 
 > This is good to hear, but it could simply be coincidence. Whether
 > or
 > not H=1 or H=0 depends on the bit offset into the MAC block (due to
 > the 0x2B padding).
 > 
 > I still think your patch has found an important bug in the encoding
 > that should be fixed.
 > 
 > Did you validate the rest octet contents of the old code with an
 > independent
 > decoder like a TEMS Pocket phone or the like?  Did you confirm the
 > current osmo-pcu code is performing correct encoding irrespective
 > of
 > the offset of the rest octets (depends on variable-length data
 > ahead
 > of the rest octets)?
 > 
 > > Therefore, I would prefer to drop this patch.
 > 
 > I would prefer to keep it, with the "bitvec_set_bit(dest, H)" as
 > suggested earlier.  It should fix an encoding bug, let's not
 > abandon it unless we know for sure the old code is correct.
 > 
 > Thanks again!

Update:

I did validate LH  bit encoding using test phone. 

I discovered that H bit encoding in below section of write_ia_rest_downlink fucntion in encoding.cpp was wrong. This leads to MS does not send Control ACK as 4 PRACH when network initiates IMM ASS via BCCH.

The MS is only able to decode correct information if I use bitvec_write_field_lh rather than bitvec_write_field for 'H'


if (tbf->is_egprs_enabled()) {
		/* see GMS 44.018, 10.5.2.16 */
		unsigned int ws_enc = (tbf->m_window.ws() - 64) / 32;
		bitvec_write_field(dest, &wp, 1, 1);  // "H"
		bitvec_write_field(dest, &wp, ws_enc, 5); // EGPRS Window Size
		bitvec_write_field(dest, &wp, 0x0, 2);    // LINK_QUALITY_MEASUREMENT_MODE
		bitvec_write_field(dest, &wp, 0, 1);      // BEP_PERIOD2 not present
	}

LH bits encoding in other functions using bitvec_write_field are fine because they are not affected by 0x2B masking of LH encoding.

This could explain why the MS seems to work fine if I do not use control ack as 4 PRACH and performing DL IMM ASS via BCCH

I wrote a small test program to encode IA octets in DL IMM ASS. Below result shows that the the last two bytes are encoded differently between bitvec_write_field and bitvec_write_field_lh.

=== start test_bitvec_ia_octet_encode_pkt_dl_ass ===
Encoded PKT DL ASS IA Rest Octets: dd ea db ee f8 00 22 31 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b
=== end test_bitvec_ia_octet_encode_pkt_dl_ass ===


=== start test_bitvec_lh_ia_octet_encode_pkt_dl_ass ===
Encoded PKT DL ASS IA Rest Octets: dd ea db ee f8 00 20 31 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b
=== end test_bitvec_lh_ia_octet_encode_pkt_dl_ass ===

I prefer to use bitvec_write_field_lh rather than bitvec_write_field to encoding LH bit in order to avoid confusing in current working encoded LH bits and wrong encoded LH bits in write_ia_rest_downlink function.

-- 
To view, visit https://gerrit.osmocom.org/3991
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I75dd5bebc74eea85edf9582607c774d0bba0d2a6
Gerrit-PatchSet: 3
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: Minh-Quang Nguyen <minh-quang.nguyen at nutaq.com>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Minh-Quang Nguyen <minh-quang.nguyen at nutaq.com>
Gerrit-HasComments: No



More information about the gerrit-log mailing list