Hi,
The good news, is that hopping and ts[0:4] work just fine and were surprisingly easy to implement. The bugs I thought I had were in fact not related to these and just prior bugs that were never noticed before. (see my sylvain/testing branch).
Only L1 is changed, so you'll need to tweak l23 (either the mobile app or layer23 for quick tests) if you want to try them out somehow.
Current limitations:
- SDCCH8 subchannel 4,5,6,7 won't work.
For those, we need to TX and RX in the same frame ... which just isn't possible with the way the primitives work right now. (each assume to have the full frame for themselve).
- TS >= 4 may not work
When using TS>=4 we should also change the interrupt alignment so that the DSP still has time to do it's job. Switching 'forward' is easy, but when switching 'back', we need to be conscious that we need to skip 1 frame at the very least ...
IMHO to solve both those issue, we need a better scheduler/primitives that possibly have some knowledge of the 'span' (in terms of qbit) of each operation. Because if we do a TX on TS=5 for instance, there is no way we can do a RX on TS=0 on the next frame, the tpu window overlap.
- No ciphering.
That should be trivial to add. Just setup the fn and the kc and set the flag ... the 'l1s.next_time' contains the godd time of the frame TX/RX frame during the primitive execution, so that's the FN to use.
Once that's done, the few bits missing from mobile app could be added (and from a quick look, not much is missing ... just the l1ctl_* calls because they don't exists yet) and a full location update with auth and ciphering could be done.
The need for a SIM interface grows quickly :)
Some 'gotchas' you might encounter that are not fixed yet:
- The L1 completion handler crashes. In target/firmware/layer1/prim_tx_nb.c , l1s.completion[L1_COMPL_TX_NB] is set in l1s_tx_test() but that's never called. So you need to comment out l1s_compl_sched(L1_COMPL_TX_NB) in the 'resp' handler temporarly until a proper fix.
- Wireshark SDCCH8 subchannel decoding in IMMEDIATE ASSIGNEMENT is just plain wrong. (source code uses % 0x38 instead of & 0x38), need to send a patch for this
- layer23 doesn't currently filter much the IMM.ASS it receives so you may end up on a channel that's not yours ... beware.
Sylvain
Hi Sylvain,
On Wed, Jun 23, 2010 at 10:29:36AM +0200, Sylvain Munaut wrote:
The good news, is that hopping and ts[0:4] work just fine and were surprisingly easy to implement.
Thanks a lot for your work and the good news!
The bugs I thought I had were in fact not related to these and just prior bugs that were never noticed before. (see my sylvain/testing branch).
ok, I will look at it, probably at some point tomorrow.
Current limitations:
SDCCH8 subchannel 4,5,6,7 won't work.
For those, we need to TX and RX in the same frame ... which just
isn't possible with the way the primitives work right now. (each assume to have the full frame for themselve).
yes, this is one of the shortcuts Dieter and me were willing to take to get layer1 going in the beginning.
TS >= 4 may not work
When using TS>=4 we should also change the interrupt alignment so
that the DSP still has time to do it's job. Switching 'forward' is easy, but when switching 'back', we need to be conscious that we need to skip 1 frame at the very least ...
IMHO to solve both those issue, we need a better scheduler/primitives that possibly have some knowledge of the 'span' (in terms of qbit) of each operation. Because if we do a TX on TS=5 for instance, there is no way we can do a RX on TS=0 on the next frame, the tpu window overlap.
agreed. but I haven't looked at the problem hard/long enough to come up with details for a solution.
No ciphering.
That should be trivial to add. Just setup the fn and the kc and set
the flag ... the 'l1s.next_time' contains the godd time of the frame TX/RX frame during the primitive execution, so that's the FN to use.
indeed, it probably is very easy to do right now. And you are the best candidate to do it, given you did the OpenBSC crypto support.
The need for a SIM interface grows quickly :)
well, as inidicated, at the moment we're likely to run layer23 (or at least the '3' part) on the PC for a long time to come, a PC/SC reader interface might actually make more sense than remotely driving the sim card reader in the phone.
Some 'gotchas' you might encounter that are not fixed yet:
- The L1 completion handler crashes. In target/firmware/layer1/prim_tx_nb.c ,
l1s.completion[L1_COMPL_TX_NB] is set in l1s_tx_test() but that's never called. So you need to comment out l1s_compl_sched(L1_COMPL_TX_NB) in the 'resp' handler temporarly until a proper fix.
the proper fix is to either move all the l1s.completion[] assignments to l1s_init(), as they only need to be set once - or to have a initialization function for each primitive. The first option requires making the completion functions non-static, which I would actually try to avoid. But having an init callback function for each primitive that only assigns one pointer is probably a stupid idea, too.
- Wireshark SDCCH8 subchannel decoding in IMMEDIATE ASSIGNEMENT is
just plain wrong. (source code uses % 0x38 instead of & 0x38), need to send a patch for this
ok, good (sending patch). probably best to send an example pcap along with it.
Hi Sylvain,
On Wed, Jun 23, 2010 at 07:30:31PM +0200, Harald Welte wrote:
Some 'gotchas' you might encounter that are not fixed yet:
- The L1 completion handler crashes. In target/firmware/layer1/prim_tx_nb.c ,
l1s.completion[L1_COMPL_TX_NB] is set in l1s_tx_test() but that's never called. So you need to comment out l1s_compl_sched(L1_COMPL_TX_NB) in the 'resp' handler temporarly until a proper fix.
the proper fix is to either move all the l1s.completion[] assignments to l1s_init(), as they only need to be set once - or to have a initialization function for each primitive. The first option requires making the completion functions non-static, which I would actually try to avoid. But having an init callback function for each primitive that only assigns one pointer is probably a stupid idea, too.
I think the cleanest solution is to use __attribute__ ((constructor)) like this:
diff --git a/src/target/firmware/layer1/prim_fbsb.c b/src/target/firmware/layer1/prim_fbsb.c index fe87996..2c58033 100644 --- a/src/target/firmware/layer1/prim_fbsb.c +++ b/src/target/firmware/layer1/prim_fbsb.c @@ -561,5 +561,9 @@ void l1s_fbsb_req(uint8_t base_fn, struct l1ctl_fbsb_req *req) else if (fbs.req.flags & L1CTL_FBSB_F_SB) tdma_schedule_set(base_fn, sb_sched_set, 0);
+} + +static __attribute__ ((constructor)) void l1s_prim_fbsb_init(void) +{ l1s.completion[L1_COMPL_FB] = &l1a_fb_compl; } diff --git a/src/target/firmware/layer1/prim_rach.c b/src/target/firmware/layer1/prim_rach.c index b8fcaae..6781384 100644 --- a/src/target/firmware/layer1/prim_rach.c +++ b/src/target/firmware/layer1/prim_rach.c @@ -128,5 +128,9 @@ void l1a_rach_req(uint8_t fn51, uint8_t ra) l1a_unlock_sync();
memset(&last_rach, 0, sizeof(last_rach)); +} + +static __attribute__ ((constructor)) void prim_rach_init(void) +{ l1s.completion[L1_COMPL_RACH] = &l1a_rach_compl; } diff --git a/src/target/firmware/layer1/prim_tx_nb.c b/src/target/firmware/layer1/prim_tx_nb.c index 7a72ca3..b721a86 100644 --- a/src/target/firmware/layer1/prim_tx_nb.c +++ b/src/target/firmware/layer1/prim_tx_nb.c @@ -188,8 +188,6 @@ void l1s_tx_test(uint8_t base_fn, uint8_t type) tdma_schedule(base_fn + 4, &l1s_tx_resp, 2, 2, 0); tdma_schedule(base_fn + 5, &l1s_tx_resp, 2, 3, 0); } - - l1s.completion[L1_COMPL_TX_NB] = &l1a_tx_nb_compl; }
/* sched sets for uplink */ @@ -203,3 +201,7 @@ const struct tdma_sched_item nb_sched_set_ul[] = { SCHED_END_SET() };
+static __attribute__ ((constructor)) void prim_tx_nb_init(void) +{ + l1s.completion[L1_COMPL_TX_NB] = &l1a_tx_nb_compl; +}
however, this means that our linker script and initialization code actually supports the constructor construct. I will take care about this today.
Hi again,
On Thu, Jun 24, 2010 at 09:44:25AM +0200, Harald Welte wrote:
On Wed, Jun 23, 2010 at 07:30:31PM +0200, Harald Welte wrote:
Some 'gotchas' you might encounter that are not fixed yet:
- The L1 completion handler crashes. In target/firmware/layer1/prim_tx_nb.c ,
l1s.completion[L1_COMPL_TX_NB] is set in l1s_tx_test() but that's never called. So you need to comment out l1s_compl_sched(L1_COMPL_TX_NB) in the 'resp' handler temporarly until a proper fix.
the proper fix is to either move all the l1s.completion[] assignments to l1s_init(), as they only need to be set once - or to have a initialization function for each primitive. The first option requires making the completion functions non-static, which I would actually try to avoid. But having an init callback function for each primitive that only assigns one pointer is probably a stupid idea, too.
I think the cleanest solution is to use __attribute__ ((constructor)) like this:
I've now finished the neccessary linker script magic and code to actually call the constructor functions. It has all been merged to master.
baseband-devel@lists.osmocom.org
-
Harald Welte -
Sylvain Munaut