Attention is currently required from: neels, fixeria, dexter.

View Change

3 comments:

• File src/osmo-bsc/pcu_sock.c:

  • Patch Set #2, Line 801: printf("l1sap_chan_rel(trx,gsm_lchan2chan_nr(ts->lchan));\n");

    either keep this printf, or if it should be dropped, drop it in a separate patch?

    Ack

  • Patch Set #2, Line 801: && ts->pchan_on_init == GSM_PCHAN_OSMO_DYN

    Why does this apply only to dynamic timeslots? I think I understand why, but I'd like to see a code […]

    Ericsson only has fully-dynamic timeslots, AFAIR. But yes, mentioning that here might make sense for the most common reader of this code who has no clue about ericsson specifics.

  • Patch Set #2, Line 941: bts = llist_entry(state->net->bts_list.next, struct gsm_bts, list);

    rather use llist_first_entry_or_null() […]

    I actually think it's a serious problem that only one BTS appears to be suppored here. Not only that, but there's an implicit assumption that the first BTS is the Ericsson BTS served by the PCU?

    In general, a BSC-colocated PCU can of course serve any number of BTSs. Until we address that general problem/shortcoming in our current code, we shouldn't make a blind assumption, but we should make sure that the user can operate this only in a safe configuration. In the worst case, we could check if thre are more than one (ericsson) BTS configured in the BSC, and refuse to operate the PCU socket with an associated error message.

To view, visit change 31497. To unsubscribe, or for help writing mail filters, visit settings.