pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-cbc/+/28854 )
Change subject: *_smscb_peer_fsm: Immediately NACK if Tx of msg failed ......................................................................
*_smscb_peer_fsm: Immediately NACK if Tx of msg failed
This makes the smscb_message_fsm receive immediate notice that this peer failed to deliver the message, avoiding need to wait for 10 seconds timeout.
Change-Id: I1f3911accb327f3378d65902d53d86fb298fd528 --- M src/cbsp_smscb_peer_fsm.c M src/sbcap_smscb_peer_fsm.c M src/smscb_message_fsm.c 3 files changed, 27 insertions(+), 19 deletions(-)
Approvals: Jenkins Builder: Verified laforge: Looks good to me, approved fixeria: Looks good to me, but someone else must approve
diff --git a/src/cbsp_smscb_peer_fsm.c b/src/cbsp_smscb_peer_fsm.c index 872008c..8a435f9 100644 --- a/src/cbsp_smscb_peer_fsm.c +++ b/src/cbsp_smscb_peer_fsm.c @@ -266,11 +266,9 @@ case SMSCB_PEER_E_CREATE: /* send it to peer */ rc = peer_new_cbc_message(mp->peer, mp->cbcmsg); - if (rc == 0) { - /* wait for peers' response */ - osmo_fsm_inst_state_chg(fi, SMSCB_S_WAIT_WRITE_ACK, 10, - T_WAIT_WRITE_ACK); - } + osmo_fsm_inst_state_chg(fi, SMSCB_S_WAIT_WRITE_ACK, 10, T_WAIT_WRITE_ACK); + if (rc != 0) /* Immediately timeout the message */ + fi->fsm->timer_cb(fi); break; default: OSMO_ASSERT(0); @@ -309,6 +307,7 @@ { struct cbc_message_peer *mp = (struct cbc_message_peer *) fi->priv; struct osmo_cbsp_decoded *cbsp; + int rc;
switch (event) { case SMSCB_PEER_E_REPLACE: /* send WRITE-REPLACE to BSC */ @@ -320,8 +319,10 @@ /* TODO: we assume that the replace will always affect all original cells */ cbsp_append_cell_list(&cbsp->u.write_replace.cell_list, cbsp, mp); // TODO: ALL OTHER DATA - cbc_cbsp_link_tx(mp->peer->link.cbsp, cbsp); + rc = cbc_cbsp_link_tx(mp->peer->link.cbsp, cbsp); osmo_fsm_inst_state_chg(fi, SMSCB_S_WAIT_REPLACE_ACK, 10, T_WAIT_REPLACE_ACK); + if (rc != 0) /* Immediately timeout the message */ + fi->fsm->timer_cb(fi); break; case SMSCB_PEER_E_STATUS: /* send MSG-STATUS-QUERY to BSC */ cbsp = osmo_cbsp_decoded_alloc(mp->peer, CBSP_MSGT_MSG_STATUS_QUERY); @@ -330,8 +331,10 @@ cbsp->u.msg_status_query.old_serial_nr = mp->cbcmsg->msg.serial_nr; cbsp_append_cell_list(&cbsp->u.msg_status_query.cell_list, cbsp, mp); cbsp->u.msg_status_query.channel_ind = CBSP_CHAN_IND_BASIC; - cbc_cbsp_link_tx(mp->peer->link.cbsp, cbsp); + rc = cbc_cbsp_link_tx(mp->peer->link.cbsp, cbsp); osmo_fsm_inst_state_chg(fi, SMSCB_S_WAIT_STATUS_ACK, 10, T_WAIT_STATUS_ACK); + if (rc != 0) /* Immediately timeout the message */ + fi->fsm->timer_cb(fi); break; default: OSMO_ASSERT(0); @@ -423,8 +426,6 @@ } }
- - static int smscb_p_fsm_timer_cb(struct osmo_fsm_inst *fi) { switch (fi->T) { @@ -454,6 +455,7 @@ { struct cbc_message_peer *mp = (struct cbc_message_peer *) fi->priv; struct osmo_cbsp_decoded *cbsp; + int rc;
switch (event) { case SMSCB_PEER_E_DELETE: /* send KILL to BSC */ @@ -478,8 +480,10 @@ OSMO_ASSERT(cbsp->u.kill.channel_ind); *(cbsp->u.kill.channel_ind) = CBSP_CHAN_IND_BASIC; } - cbc_cbsp_link_tx(mp->peer->link.cbsp, cbsp); + rc = cbc_cbsp_link_tx(mp->peer->link.cbsp, cbsp); osmo_fsm_inst_state_chg(fi, SMSCB_S_WAIT_DELETE_ACK, 10, T_WAIT_DELETE_ACK); + if (rc != 0) /* Immediately timeout the message */ + fi->fsm->timer_cb(fi); break; default: OSMO_ASSERT(0); @@ -497,7 +501,8 @@ [SMSCB_S_INIT] = { .name = "INIT", .in_event_mask = S(SMSCB_PEER_E_CREATE), - .out_state_mask = S(SMSCB_S_WAIT_WRITE_ACK), + .out_state_mask = S(SMSCB_S_WAIT_WRITE_ACK) | + S(SMSCB_S_ACTIVE), .action = smscb_p_fsm_init, }, [SMSCB_S_WAIT_WRITE_ACK] = { diff --git a/src/sbcap_smscb_peer_fsm.c b/src/sbcap_smscb_peer_fsm.c index 706a98f..87d5e16 100644 --- a/src/sbcap_smscb_peer_fsm.c +++ b/src/sbcap_smscb_peer_fsm.c @@ -116,11 +116,9 @@ case SMSCB_PEER_E_CREATE: /* send it to peer */ rc = peer_new_cbc_message(mp->peer, mp->cbcmsg); - if (rc == 0) { - /* wait for peers' response */ - osmo_fsm_inst_state_chg(fi, SMSCB_S_WAIT_WRITE_ACK, 10, - T_WAIT_WRITE_ACK); - } + osmo_fsm_inst_state_chg(fi, SMSCB_S_WAIT_WRITE_ACK, 10, T_WAIT_WRITE_ACK); + if (rc != 0) /* Immediately timeout the message */ + fi->fsm->timer_cb(fi); break; default: OSMO_ASSERT(0); @@ -256,6 +254,7 @@ SBcAP_SBC_AP_PDU_t *sbcap; A_SEQUENCE_OF(void) *as_pdu; SBcAP_Write_Replace_Warning_Indication_IEs_t *ie; + int rc;
switch (event) { case SMSCB_PEER_E_DELETE: /* send Stop-Warning to MME */ @@ -269,13 +268,16 @@ break; } if ((sbcap = sbcap_gen_stop_warning_req(mp->peer, mp->cbcmsg))) { - cbc_sbcap_link_tx(mp->peer->link.sbcap, sbcap); + rc = cbc_sbcap_link_tx(mp->peer->link.sbcap, sbcap); } else { LOGP(DSBcAP, LOGL_ERROR, "[%s] Tx SBc-AP Stop-Warning-Request: msg gen failed\n", mp->peer->name); + rc = -1; } osmo_fsm_inst_state_chg(fi, SMSCB_S_WAIT_DELETE_ACK, 10, T_WAIT_DELETE_ACK); + if (rc != 0) /* Immediately timeout the message */ + fi->fsm->timer_cb(fi); break; case SMSCB_PEER_E_SBCAP_WRITE_IND: sbcap = (SBcAP_SBC_AP_PDU_t *)data; @@ -305,7 +307,8 @@ [SMSCB_S_INIT] = { .name = "INIT", .in_event_mask = S(SMSCB_PEER_E_CREATE), - .out_state_mask = S(SMSCB_S_WAIT_WRITE_ACK), + .out_state_mask = S(SMSCB_S_WAIT_WRITE_ACK) | + S(SMSCB_S_ACTIVE), .action = smscb_p_fsm_init, }, [SMSCB_S_WAIT_WRITE_ACK] = { diff --git a/src/smscb_message_fsm.c b/src/smscb_message_fsm.c index 9281736..e7f96c0 100644 --- a/src/smscb_message_fsm.c +++ b/src/smscb_message_fsm.c @@ -84,7 +84,7 @@ /* check if any per-peer children have not yet received the ACK or * timed out */ llist_for_each_entry(peer_fi, &fi->proc.children, proc.child) { - if (peer_fi->state == SMSCB_S_WAIT_WRITE_ACK) + if (peer_fi->state != SMSCB_S_ACTIVE) return; } rest_it_op_set_http_result(cbcmsg->it_op, 201, "Created"); // FIXME: error cases