- Function tet_mds_cb_enc/dec (default for broadcast type) encode/decode received messages use 16 bits for message length but mdstest verify broadcast big message size 66000 (> 16bits) which cause decode incorrect length and memory does not free properly to get wrong message length.
- Also, there is a case that mds library already posting mailbox messages (if mds_q_ownership true), but mds client does not retrieve it then uninstall mds, a proper cleanup callback is needed to cleanup these mailbox messages. --- src/mds/apitest/mdstipc_conf.c | 28 ++++++++++----------- src/mds/mds_c_db.c | 46 ++++++++++++++++++++++++++++------ src/mds/mds_c_sndrcv.c | 4 +-- src/mds/mds_core.h | 1 + src/mds/mds_tipc_fctrl_intf.cc | 9 ++++++- 5 files changed, 63 insertions(+), 25 deletions(-) diff --git a/src/mds/apitest/mdstipc_conf.c b/src/mds/apitest/mdstipc_conf.c index eb0de75c5..cd2f80791 100644 --- a/src/mds/apitest/mdstipc_conf.c +++ b/src/mds/apitest/mdstipc_conf.c @@ -16,6 +16,7 @@ * */ +#include "mds/mds_core.h" #include "mds/mds_papi.h" #include "base/ncs_mda_papi.h" #include "base/ncssysf_tsk.h" @@ -540,16 +541,15 @@ uint32_t mds_service_uninstall(MDS_HDL mds_hdl, MDS_SVC_ID svc_id) return NCSCC_RC_FAILURE; } } + void tet_mds_free_msg(NCSCONTEXT msg_to_be_freed) { - TET_MDS_RECVD_MSG_INFO *p_recvd_info; - - p_recvd_info = (TET_MDS_RECVD_MSG_INFO *)msg_to_be_freed; - if (p_recvd_info != NULL) { - printf("Freeing up all the messages in the MDS Q"); - if (p_recvd_info->msg) - free(p_recvd_info->msg); - free(p_recvd_info); + MDS_MCM_MSG_ELEM *msgelem = (MDS_MCM_MSG_ELEM *)msg_to_be_freed; + + if (msgelem != NULL) { + if (msgelem->type == MDS_DATA_TYPE) + mds_mcm_free_msg_memory(msgelem->info.data.enc_msg); + m_MMGR_FREE_MSGELEM(msgelem); } } @@ -1864,9 +1864,9 @@ uint32_t tet_mds_cb_enc(NCSMDS_CALLBACK_INFO *mds_to_svc_info) mds_to_svc_info->info.enc.o_msg_fmt_ver); /* ENCODE length */ - p8 = ncs_enc_reserve_space(mds_to_svc_info->info.enc.io_uba, 2); - ncs_encode_16bit(&p8, msg->send_len); - ncs_enc_claim_space(mds_to_svc_info->info.enc.io_uba, 2); + p8 = ncs_enc_reserve_space(mds_to_svc_info->info.enc.io_uba, sizeof(uint32_t)); + ncs_encode_32bit(&p8, msg->send_len); + ncs_enc_claim_space(mds_to_svc_info->info.enc.io_uba, sizeof(uint32_t)); /* ENCODE data */ ncs_encode_n_octets_in_uba(mds_to_svc_info->info.enc.io_uba, @@ -1898,9 +1898,9 @@ uint32_t tet_mds_cb_dec(NCSMDS_CALLBACK_INFO *mds_to_svc_info) /* DECODE length */ p8 = ncs_dec_flatten_space(mds_to_svc_info->info.dec.io_uba, - (uint8_t *)&msg->recvd_len, 2); - msg->recvd_len = ncs_decode_16bit(&p8); - ncs_dec_skip_space(mds_to_svc_info->info.dec.io_uba, 2); + (uint8_t *)&msg->recvd_len, sizeof(uint32_t)); + msg->recvd_len = ncs_decode_32bit(&p8); + ncs_dec_skip_space(mds_to_svc_info->info.dec.io_uba, sizeof(uint32_t)); /*Decode data*/ /*msg->recvd_data = (char *) malloc(msg->recvd_len+1);*/ diff --git a/src/mds/mds_c_db.c b/src/mds/mds_c_db.c index e1991517e..3c4586299 100644 --- a/src/mds/mds_c_db.c +++ b/src/mds/mds_c_db.c @@ -29,6 +29,36 @@ #include "base/ncs_main_papi.h" extern bool tipc_mode_enabled; extern uint32_t mds_mcm_check_intranode(MDS_DEST adest); + +/***************************************************** +Function NAME: mdtm_wrapper_free_cb() +Description: adapt free callback follow NCS_IPC_CB prototype +Returns : bool +*****************************************************/ +MDS_Q_MSG_FREE_CB q_msg_free_cb; +bool mdtm_wrapper_free_cb(NCSCONTEXT arg, NCSCONTEXT msg) +{ + if (q_msg_free_cb) + q_msg_free_cb(msg); + return true; +} + +/***************************************************** +Function NAME: mdtm_q_mbx_cleanup() +Returns : void +*****************************************************/ +bool mdtm_q_mbx_cleanup(NCSCONTEXT arg, NCSCONTEXT msg) +{ + MDS_MCM_MSG_ELEM *msgelem = (MDS_MCM_MSG_ELEM *)msg; + + if (msgelem != NULL) { + if (msgelem->type == MDS_DATA_TYPE) + mds_mcm_free_msg_memory(msgelem->info.data.enc_msg); + m_MMGR_FREE_MSGELEM(msgelem); + } + return true; +} + /***************************************************** Function NAME: get_adest_details() Returns : <node[nodeid]:processname[pid]> @@ -805,7 +835,7 @@ uint32_t mds_svc_tbl_add(NCSMDS_INFO *info) svc_info->i_msg_loss_indication = info->info.svc_install.i_msg_loss_indication; - if (svc_info->q_ownership == 1) { + if (svc_info->q_ownership == true) { if (m_NCS_IPC_CREATE(&svc_info->q_mbx) != NCSCC_RC_SUCCESS) { m_MMGR_FREE_SVC_INFO(svc_info); m_MDS_LOG_ERR("MDS:DB: Can't Create SVC Mailbox"); @@ -860,9 +890,11 @@ uint32_t mds_svc_tbl_del(MDS_PWE_HDL pwe_hdl, MDS_SVC_ID svc_id, m_MDS_LEAVE(); return NCSCC_RC_FAILURE; } else { - if (svc_info->q_ownership == 1) { + if (svc_info->q_ownership == true) { + q_msg_free_cb = msg_free_cb; m_NCS_IPC_DETACH(&svc_info->q_mbx, - (NCS_IPC_CB)msg_free_cb, NULL); + (NCS_IPC_CB)mdtm_wrapper_free_cb, + NULL); m_NCS_IPC_RELEASE(&svc_info->q_mbx, NULL); } ncs_patricia_tree_del(&gl_mds_mcm_cb->svc_list, @@ -1151,12 +1183,10 @@ uint32_t mds_svc_tbl_cleanup(void) svc_hdl = svc_info->svc_hdl; /* Delete service table entry */ - if (svc_info->q_ownership == 1) { - /* todo put cleanup function instead of NULL in - * (NCS_IPC_CB) */ - m_NCS_IPC_DETACH(&svc_info->q_mbx, (NCS_IPC_CB)NULL, + if (svc_info->q_ownership == true) { + m_NCS_IPC_DETACH(&svc_info->q_mbx, + (NCS_IPC_CB)mdtm_q_mbx_cleanup, NULL); - /* todo to provide cleanup callback */ m_NCS_IPC_RELEASE(&svc_info->q_mbx, NULL); } diff --git a/src/mds/mds_c_sndrcv.c b/src/mds/mds_c_sndrcv.c index 06bebf51a..557f68a4b 100644 --- a/src/mds/mds_c_sndrcv.c +++ b/src/mds/mds_c_sndrcv.c @@ -131,7 +131,7 @@ static uint32_t mds_mcm_del_all_bcast_list(SEND_MSG *msg); uint32_t mds_send(NCSMDS_INFO *info); -static uint32_t mds_mcm_free_msg_memory(MDS_ENCODED_MSG msg); +uint32_t mds_mcm_free_msg_memory(MDS_ENCODED_MSG msg); static uint32_t mcm_pvt_normal_svc_snd(MDS_HDL env_hdl, MDS_SVC_ID fr_svc_id, NCSCONTEXT msg, MDS_DEST to_dest, @@ -4842,7 +4842,7 @@ uint32_t mds_mcm_ll_data_rcv(MDS_DATA_RECV *recv) * * ****************************************************************************/ -static uint32_t mds_mcm_free_msg_memory(MDS_ENCODED_MSG msg) +uint32_t mds_mcm_free_msg_memory(MDS_ENCODED_MSG msg) { switch (msg.encoding) { case MDS_ENC_TYPE_CPY: diff --git a/src/mds/mds_core.h b/src/mds/mds_core.h index 91d3675a1..dad62cdc5 100644 --- a/src/mds/mds_core.h +++ b/src/mds/mds_core.h @@ -585,6 +585,7 @@ extern uint32_t mds_mcm_validate_scope(NCSMDS_SCOPE_TYPE local_scope, MDS_SVC_ID remote_svc_id, bool my_pcon); extern uint32_t mds_mcm_free_msg_uba_start(MDS_ENCODED_MSG msg); +extern uint32_t mds_mcm_free_msg_memory(MDS_ENCODED_MSG msg); extern void get_adest_details(MDS_DEST adest, char *adest_details); extern void get_subtn_adest_details(MDS_PWE_HDL pwe_hdl, MDS_SVC_ID svc_id, diff --git a/src/mds/mds_tipc_fctrl_intf.cc b/src/mds/mds_tipc_fctrl_intf.cc index 685b4283a..4270cf26a 100644 --- a/src/mds/mds_tipc_fctrl_intf.cc +++ b/src/mds/mds_tipc_fctrl_intf.cc @@ -186,6 +186,13 @@ uint32_t process_flow_event(const Event& evt) { return rc; } +bool mds_fctrl_mbx_cleanup(NCSCONTEXT arg, NCSCONTEXT msg) { + Event *evt = reinterpret_cast<Event*>(msg); + if (evt != nullptr) + delete evt; + return true; +} + uint32_t process_all_events(void) { enum { FD_FCTRL = 0, NUM_FDS }; @@ -305,7 +312,7 @@ uint32_t mds_tipc_fctrl_shutdown(void) { strerror(errno)); } - m_NCS_IPC_DETACH(&mbx_events, nullptr, nullptr); + m_NCS_IPC_DETACH(&mbx_events, mds_fctrl_mbx_cleanup, nullptr); m_NCS_IPC_RELEASE(&mbx_events, nullptr); for (auto i : portid_map) delete i.second; -- 2.17.1 _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel