Re: fairwaves HO patches

Hi Ivan, as I'm working towards load-based handover and incorporating your patches, I found that most of it is concerned with libmsc. In contrast, I am currently working on osmo-bsc, in libbsc, and would like to ask your advice. Some parts of the patch aren't easy to understand for me, and I'd like to make sure that I am not dismissing parts of it as non-applicable to libbsc even though they might be important. Since your patches were written, the code has changed. Now that we have the separate osmo-bsc, we will need two layers of handover: intra-BSC and inter-BSC. Intra-BSC is a handover between two cells that are serviced by the same BSC, and the higher layers (MSC) should not even notice that anything has happened -- MSC has asked the BSC to service a call by BSSAP Assignment, and the BSC is free to choose and change around the lchans it assigns to that. That is the layer I'm currently dug into. Inter-BSC is a handover between cells that are serviced by two different BSCs. That seems to be the land your patch is improving. The MSC is involved and so is MNCC. (Both MSC and BSC levels will need their own DTAP cache, and they are by definition completely independent -- MSC caches the DTAP coming from MNCC during Inter-BSC handover, BSC caches DTAP from MSC during Intra-BSC handover.) Since your patch is applied onto openbsc.git, where all of BSC and MSC are still one osmo-nitb, I want to make sure that I sort your patch right. Do some of its semantics apply to osmo-bsc master, even if the code changed? The smaller patches are either already applied to osmo-bsc master, or I've submitted them on gerrit just now: handover_decision: Add more log messages to get more information about HO causes in logs handover_decision: Fix condition for power budget handover attempt handover_logic: set correct link to bts for subscriber_connection in case of moving this connection to another bts Remaining are a small and *the* complex one: transaction: Add new function trans_find_by_lchan handover: Implement proper handover procedure handling at any stage of the call Here is the last one with inline questions, I hope they are not too stupid; or too long, it ended up being a lot to read. Thanks in advance for taking a look: of the call implemented in order to support handover at any stage of the call. control messages was introduced for gsm_subscriber_connection. RR messages needed for handover procedure) should be buffered in ho_queue. should be ignored. (in_handover=0). MS on new lchan. performed on new lchan. (in_handover=0). MS on old lchan. performed on old lchan. HO-Fail expired: (in_handover=0). without sending them to MS. performed. b/openbsc/include/openbsc/bsc_api.h gsm_subscriber_connection *conn, int cipher, b/openbsc/include/openbsc/gsm_data.h *bsc_subscr_con_allocate(struct gsm_lchan *lcha gsm_subscriber_connection *conn) *conn) int link_id) b/openbsc/src/libbsc/handover_logic.c (Your ho_queue seems to live in libbsc, while most of the patch seems to be concerned with libmsc. But nevermind, from jolly's patches I already have a similar queue in osmo-bsc, and we'll probably use yours for libmsc.) *new_lchan) In current osmo-bsc master, we already set conn->ho_lchan before sending out the chan activation request. I'd actually assume setting the flag only now, after the activ ack, is a bit too late? *new_lchan) GSM48_CMODE_SIGN) Please explain ... what case / behavior is this fixing? Do we ever see CMODE_SIGN handovers? Would we also need to check for GSM48_CMODE_DATA_*? *new_lchan) connect_port; connect_ip; So if an RTP connect to MNCC is pending, we copy the old lchan's RTP port and IP? Which is this, the MNCC / call router side IP and port? Why not set this at the initiation of the HO already? *new_lchan) I'm puzzled by this removal. No actions during ho_fail? Is this really intended, or just some rebase artifact? 08.c struct gsm_subscriber_connection sending to MS, " (here DTAP handled in libmsc ends up in the ho_queue that otherwise seems to live in libbsc ... as I said above this queue will probably move to libmsc altogether to become part of osmo-msc master.) *old_lchan, 0); message.*/ I notice that in the current head, the entire switch_for_handover() has been dropped; it was doing libbsc lchan stuff from within libmsc. Hence we must have added similar logic in osmo-bsc.git and completely dropped this. I think the commit re-implementing handover in osmo-bsc is http://git.osmocom.org/osmo-bsc/commit/?id=39c609b7c924524172ad311bdf89f9 2b7ccf175a It appears that lchan->abis_ip.connect_ip and connect_port aren't used at all in osmo-bsc master, but are still present in the struct. I'll ask others about that. In any case, the code base has changed substantially, and this patch hunk no longer applies at all. Am I interpreting this hunk correctly: it moves the ipacc_mdcx to tell the new lchan about its RTP peer to a later stage? In current osmo-bsc master, it seems that this ipacc_mdcx happens as soon as the ipacc_crcx is complete, seen in osmo-bsc/src/osmo-bsc/osmo_bsc_audio.c in handle_abisip_signal(), always using the IP:port the MSC sent us. subsys, unsigned int signal, What are the semantics here? It seems odd to move from a check of a conn-specific state (conn->mncc_rtp_bridge) to a check of a global value (net->mncc_state). In any case, this is MNCC related and should not impact Intra-BSC handover, right? subsys, unsigned int signal, trans->tch_recv) { request\n"); uint32_t callref, int enable) disabled\n"); If we have a conn and using external MNCC, don't continue at all? I'm not following, would be nice if the comment explained why we need to drop out here. (added some more code context manually) BTS will and enable); *net, uint32_t callref, int cmd callref) This is to tell the call router that the payload type has changed, right? I've asked in the redmine about whether/how we'd convey an RTP payload change to the MNCC in case of an Intra-BSC handover... https://osmocom.org/issues/1606#note-45 > +{ > + int msg_type; > + struct gsm_network *net = lchan->ts->trx->bts->network; > + > + LOGP(DMNCC, LOGL_NOTICE, "%s sending pending RTP modify ind.\n", > + gsm_lchan_name(lchan)); > + > + switch (lchan->abis_ip.rtp_payload) { > + case RTP_PT_GSM_FULL: > + msg_type = GSM_TCHF_FRAME; > + break; > + case RTP_PT_GSM_EFR: > + msg_type = GSM_TCHF_FRAME_EFR; > + break; > + case RTP_PT_GSM_HALF: > + msg_type = GSM_TCHH_FRAME; > + break; > + case RTP_PT_AMR: > + msg_type = GSM_TCH_FRAME_AMR; > + break; > + default: > + LOGP(DMNCC, LOGL_ERROR, "%s unknown payload type %d\n", > + gsm_lchan_name(lchan), lchan->abis_ip.rtp_payload); > + msg_type = 0; > + break; > + mncc_recv_rtp(net, callref, MNCC_RTP_MODIFY, > + lchan->abis_ip.bound_ip, > + lchan->abis_ip.bound_port, > + lchan->abis_ip.rtp_payload, > + msg_type); > +} > + > static int tch_rtp_create(struct gsm_network *net, uint32_t callref) > { > struct gsm_bts *bts; > @@ -3374,6 +3423,9 @@ static int tch_rtp_create(struct gsm_network *net, uint32_t callref) chan_type=%s\n", Am I reading right: if we're doing handover, the MSC shouldn't sent another BSSAP Assignment to the BSC; instead, the BSC level figures out another lchan and done ... ? *net, void *arg) We're not telling the BTS about the call router's IP:port anymore? Please explain... lchan) transaction.\n", int signal) !lchan->conn->in_handover) { if we're in handover, we don't need to tell MNCC that RTP got connected, because that already happened when the call got established initially? So mncc_rtp_connect_pending has a second meaning during handover? connect ind.\n", *lchan, int signal) *trans) > +{ > + struct msgb *msg; > + enum gsm48_chan_mode m; > + > + while (!llist_empty(&lchan->conn->ho_queue)) { > + msg = msgb_dequeue(&lchan->conn->ho_queue); > + gsm48_conn_sendmsg(msg, lchan->conn, trans); > + if (trans->conn->mncc_rtp_create_pending && > + lchan->tch_mode == GSM48_CMODE_SIGN) { GSM_LCHAN_TCH_H); Wait, now we *do* send a BSSAP Assignment after all? (excuse if I'm being noob, I'm still finding my way through handover in general) shouldn't we rather dequeue the cached DTAP after this instead of before? lchan->abis_ip.connect_port, 0); for new_lchan.\n", > + gsm_lchan_name(new_lchan)); > + ho_queue_clean(new_lchan->conn); > + return 0; > + ho_resumption(new_lchan, trans); > + return 0; > +} > + > +static int ho_fail(struct gsm_lchan *old_lchan) > +{ > + struct gsm_trans *trans; > + > + old_lchan->conn->in_handover = 0; > + trans = trans_find_by_lchan(old_lchan); > + if (trans) > + ho_resumption(old_lchan, trans); > + else { > + LOGP(DHO, LOGL_ERROR, "%s HO fail, but no transaction for old_lchan.\n", > + gsm_lchan_name(old_lchan)); > + ho_queue_clean(old_lchan->conn); > + gsm0808_ho_clear(old_lchan->conn); > + return 0; > +} > + > +static int ho_detect(struct gsm_lchan *new_lchan) > +{ > + struct gsm_trans *trans; > + struct gsm_lchan *old_lchan; > + > + trans = trans_find_by_lchan(new_lchan); > + > + if (!trans) { > + LOGP(DHO, LOGL_ERROR, "%s HO detected, but no transaction for new_lchan" > + " with enabled tch_recv.\n", > + gsm_lchan_name(new_lchan)); > + return 0; > + if (!new_lchan->conn->mncc_rtp_bridge) { > + LOGP(DHO, LOGL_ERROR, "%s HO detected, but connection not in mncc_rtp_bridge mode.\n", > + gsm_lchan_name(new_lchan)); > + return 0; > + old_lchan = bsc_handover_pending(new_lchan); > + if (!old_lchan) { > + LOGP(DHO, LOGL_ERROR, "%s HO detected, but no old_lchan for handover.\n", > + gsm_lchan_name(new_lchan)); > + return 0; > + LOGP(DHO, LOGL_DEBUG, "HO detected, forwarding RTP\n"); > + rsl_ipacc_mdcx(new_lchan, > + old_lchan->abis_ip.connect_ip, > + old_lchan->abis_ip.connect_port, 0); > + > + mncc_recv_rtp_modify(new_lchan, trans->callref); > + > + return 0; > +} > + > +/* some other part of the code sends us a signal */ > +static int handle_lchan_signal(unsigned int subsys, unsigned int signal, > + void *handler_data, void *signal_data) > +{ > + struct lchan_signal_data *lchan_data; > + struct gsm_lchan *lchan; > + > + lchan_data = signal_data; > + switch (subsys) { > + case SS_LCHAN: > + lchan = lchan_data->lchan; > + if (!lchan->conn) > + return 0; > + switch (signal) { > + case S_LCHAN_HANDOVER_DETECT: > + return ho_detect(lchan); > + case S_LCHAN_HANDOVER_COMPL: > + return ho_complete(lchan); > + case S_LCHAN_HANDOVER_FAIL: > + return ho_fail(lchan); > + } > + break; > + return 0; > +} > > static struct downstate { > uint32_t states; > @@ -3853,6 +4031,11 @@ static int gsm0408_rcv_cc(struct gsm_subscriber_connection *conn, struct msgb *m > gsm48_cc_msg_name(msg_type), trans?(trans->cc.state):0, > gsm48_cc_state_name(trans?(trans->cc.state):0)); > > + if (conn->in_handover) { > + DEBUGP(DCC, "Message unhandled, handover procedure.\n"); > + return 0; If in handover, drop all CC on the floor? How about a call release, i.e. I hang up while I'm coincidentally being handovered? Thanks again! ~N