- 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

Reply via email to