Support #3023
closedsrc/common/pcu_sock.c/pcu_sock_close(): The array 'ts->lchan' is being utilized as a pointer to single object, ts->lchan[0] would be clearer
Added by fixeria about 6 years ago. Updated over 5 years ago.
100%
Description
The gsm_bts_trx_ts struct has array of lchans:
struct gsm_lchan lchan[TS_MAX_LCHAN];
However, in the pcu_sock_close() it's utilized as a pointer to single object:
for (j = 0; j < 8; j++) {
ts = &trx->ts[j];
if (ts->mo.nm_state.operational == NM_OPSTATE_ENABLED
&& ts->pchan == GSM_PCHAN_PDCH) {
ts->lchan->rel_act_kind = LCHAN_REL_ACT_PCU;
l1sap_chan_rel(trx, gsm_lchan2chan_nr(ts->lchan));
}
}
Is it intended?
Updated by neels over 5 years ago
- Tracker changed from Bug to Support
- Subject changed from src/common/pcu_sock.c/pcu_sock_close(): The array 'ts->lchan' is being utilized as a pointer to single object to src/common/pcu_sock.c/pcu_sock_close(): The array 'ts->lchan' is being utilized as a pointer to single object, ts->lchan[0] would be clearer
- Status changed from New to Feedback
- Assignee changed from neels to fixeria
- Priority changed from Normal to Low
Updated by fixeria over 5 years ago
since any timeslot used for PDCH has only one lchan anyway
Hmm, but (according to scheduler.h and trx_chan_desc) there are two plchan types:
TRXC_PTCCH and TRXC_PDTCH. neels, are you sure we don't need to distinguish between them?
Updated by laforge over 5 years ago
Technically, the PDCH consists of various sub-channels such as PDTCH, PACCH, PTCCH.
I just don't think we are representing that difference to OsmoBTS in the gsm_lchan
structure, as it doesn't really make any difference (the messages are all encoded
the same). gsm_chan_t also doesn't have anything for PTCCH or PACCH specifically
Hmm, but (according to scheduler.h and trx_chan_desc) there are two plchan types:
TRXC_PTCCH and TRXC_PDTCH. neels, are you sure we don't need to distinguish between them?
This is "just" for osmo-bts-trx, as the PTCCH is scheduled at specific points
in time during downlink and uplink. The separation is made only by the following
macro:
#define L1SAP_IS_PTCCH(fn) (((fn % 52) 12) || ((fn % 52) 38))
Regards,
Harald
Updated by fixeria over 5 years ago
Hi,
I just checked GSM TS 05.02, and I am getting confused again...
Technically, the PDCH consists of various sub-channels such as PDTCH, PACCH, PTCCH.
[...]
I just don't think we are representing that difference to OsmoBTS in the gsm_lchan
structure, as it doesn't really make any difference (the messages are all encoded
the same). gsm_chan_t also doesn't have anything for PTCCH or PACCH specifically
Actually, we do distinguish between both PDTCH and PTCCH.
Please see the definition of PDCH multi-frame in 'scheduler_mframe.c':
https://git.osmocom.org/osmo-bts/tree/src/common/scheduler_mframe.c#n926
and definition of both logical channels in 'scheduler.c':
https://git.osmocom.org/osmo-bts/tree/src/common/scheduler.c#n156
They are actually using different convolutional encoding, thus different
logical channel handlers: 'tx_pdtch_fn' vs 'tx_data_fn'. So I still think
they should be activated and deactivated separately.
According to GSM 05.02, PTCCH stands for Packet Timing advance Control Channel,
so we should transmit Timing advance related messages there, not traffic.
What looks strange for me is that they both are using 0xc0 as channel number
so both logical channel handlers do dequeue and encode same messages from queue.
This looks odd to me, and I guess this is a mistake.
Updated by laforge over 5 years ago
fixeria wrote:
Hi,
I just checked GSM TS 05.02, and I am getting confused again...
Technically, the PDCH consists of various sub-channels such as PDTCH, PACCH, PTCCH.
[...]
I just don't think we are representing that difference to OsmoBTS in the gsm_lchan
structure, as it doesn't really make any difference (the messages are all encoded
the same). gsm_chan_t also doesn't have anything for PTCCH or PACCH specificallyActually, we do distinguish between both PDTCH and PTCCH.
Please see the definition of PDCH multi-frame in 'scheduler_mframe.c':
https://git.osmocom.org/osmo-bts/tree/src/common/scheduler_mframe.c#n926
well, as I wrote, only osmo-bts-trx is doing that distinction :P
scheduler_common is only used by osmo-bts-{trx,virtual} and -virtual has no GPRS support
They are actually using different convolutional encoding, thus different
logical channel handlers: 'tx_pdtch_fn' vs 'tx_data_fn'. So I still think
they should be activated and deactivated separately.
the other PHYs like osmo-bts-sysmo,octphy, ... solve this by having multiple different "L1 SAPI" for those channels, and activating a given lchan will then activate all "L1 SAPI" for that lchan. The same applies for normal CS channels, e.g. a SDCCH lchan will activate L1 SAPIs for sdcch-downlink, sdcch-uplink, sacch-downlink, sacch-uplink, ...
According to GSM 05.02, PTCCH stands for Packet Timing advance Control Channel,
so we should transmit Timing advance related messages there, not traffic.
yes, that's correct.
Updated by fixeria over 5 years ago
Ok, it seems I was confused too much. Sorry.
Just checked how does the channels (de)activation work when one (dis)connects OsmoPCU:
<0009> pcu_sock.c:715 PCU socket has LOST connection <0001> oml.c:76 Sending PCU version report to BSC: <0006> l1sap.c:1517 deactivating channel chan_nr=0xc5 trx=0 <0006> lchan.c:31 (bts=0,trx=0,ts=5,ss=0) state ACTIVE -> NONE <0006> scheduler.c:624 Deactivating PDTCH on trx=0 ts=5 <0006> scheduler.c:624 Deactivating PTCCH on trx=0 ts=5 <0006> l1sap.c:647 deactivate confirm chan_nr=0xc5 trx=0 <0000> rsl.c:698 (bts=0,trx=0,ts=5,ss=0) PCU rel ack for unexpected lchan kind <0000> rsl.c:714 (bts=0,trx=0,ts=5,ss=0) not sending REL ACK <0006> l1sap.c:1517 deactivating channel chan_nr=0xc6 trx=0 <0006> lchan.c:31 (bts=0,trx=0,ts=6,ss=0) state ACTIVE -> NONE <0006> scheduler.c:624 Deactivating PDTCH on trx=0 ts=6 <0006> scheduler.c:624 Deactivating PTCCH on trx=0 ts=6 <0006> l1sap.c:647 deactivate confirm chan_nr=0xc6 trx=0 <0000> rsl.c:698 (bts=0,trx=0,ts=6,ss=0) PCU rel ack for unexpected lchan kind <0000> rsl.c:714 (bts=0,trx=0,ts=6,ss=0) not sending REL ACK <0006> l1sap.c:1517 deactivating channel chan_nr=0xc7 trx=0 <0006> lchan.c:31 (bts=0,trx=0,ts=7,ss=0) state ACTIVE -> NONE <0006> scheduler.c:624 Deactivating PDTCH on trx=0 ts=7 <0006> scheduler.c:624 Deactivating PTCCH on trx=0 ts=7 <0006> l1sap.c:647 deactivate confirm chan_nr=0xc7 trx=0 <0000> rsl.c:698 (bts=0,trx=0,ts=7,ss=0) PCU rel ack for unexpected lchan kind <0000> rsl.c:714 (bts=0,trx=0,ts=7,ss=0) not sending REL ACK
Both PDTCH and PTCCH are being deactivated on PCU disconnect, so never mind.
Updated by fixeria over 5 years ago
- Status changed from Feedback to Resolved
- % Done changed from 0 to 100
The patch has been submitted: https://gerrit.osmocom.org/11258/. Closing now.
