For now we can push these changes. In future there is a need to consider for option-2 as mentioned below i.e., having "svc-id + adest" as part of process_info_db key.
Thanks, Ramesh. On 9/3/2014 1:12 PM, Hans Feldt wrote: > So can I push this now since Neel had no objections? > Thanks, > Hans > >> -----Original Message----- >> From: ramesh betham [mailto:[email protected]] >> Sent: den 2 september 2014 10:42 >> To: [email protected]; Hans Feldt >> Subject: Re: [devel] [PATCH 1 of 1] mds: make process_info_db independent of >> control block [#1024] >> >> Hans, >> >> My 2 cents: >> From the brief review, this patch works well for the current requirement. >> >> However this patch creates an additional process_info_db structs even >> for those svc_ids who are not participating in authentication (for ex: >> mds-up event of IMMD). This can be avoided if process_info_db struct is >> created only at mds_register_callback and update the 'count' too. >> >> Also it has an issue to deal with i.e., in a process if only a subset of >> subscriber svc-id's are participating in authentication. This can be >> handled by making process_info_db key as "adest + svc_id". As we don't >> have such requirement currently, can take it up as further enhancement. >> >> Thanks, >> Ramesh. >> >> On 9/2/2014 12:18 PM, Hans Feldt wrote: >>> osaf/libs/core/mds/include/mds_core.h | 7 +++-- >>> osaf/libs/core/mds/mds_c_api.c | 27 +++++++++++++++++-------- >>> osaf/libs/core/mds/mds_c_db.c | 37 >>> +++++++++++++++++++++++++++++----- >>> osaf/libs/core/mds/mds_dt_common.c | 5 ++- >>> osaf/libs/core/mds/mds_main.c | 17 +++++++++++++-- >>> 5 files changed, 70 insertions(+), 23 deletions(-) >>> >>> >>> The process_info_db needs to be available early before MDS is initialized. >>> So >>> its initialization is moved to mds_auth_server_create(). The patricia tree >>> cannot be located in the control block which is dynamically created so it is >>> now statically allocated. >>> >>> Create process info upon two events, SVC UP and registration using connected >>> socket. In case SVC UP comes first, credentials are left empty until socket >>> registration fills them in. >>> >>> diff --git a/osaf/libs/core/mds/include/mds_core.h >>> b/osaf/libs/core/mds/include/mds_core.h >>> --- a/osaf/libs/core/mds/include/mds_core.h >>> +++ b/osaf/libs/core/mds/include/mds_core.h >>> @@ -288,7 +288,6 @@ typedef struct mds_mcm_cb { >>> NCS_PATRICIA_TREE subtn_results; >>> NCS_PATRICIA_TREE svc_list; /* Tree of MDS_SVC_INFO information */ >>> NCS_PATRICIA_TREE vdest_list; /* Tree of MDS_VDEST_INFO information */ >>> - NCS_PATRICIA_TREE process_info_db; /* all known local MDS dests */ >>> } MDS_MCM_CB; >>> >>> /* Global MDSCB */ >>> @@ -308,13 +307,15 @@ typedef struct mds_process_info { >>> uid_t uid; >>> gid_t gid; >>> pid_t pid; >>> - int count; >>> + int count; // ref count, entry deleted at zero >>> } MDS_PROCESS_INFO; >>> >>> MDS_PROCESS_INFO *mds_process_info_get(MDS_DEST mds_dest); >>> int mds_process_info_add(MDS_PROCESS_INFO *info); >>> int mds_process_info_del(MDS_PROCESS_INFO *info); >>> -int mds_process_info_cnt(void); >>> +int mds_process_info_db_init(void); >>> +int mds_process_info_enabled(void); >>> + >>> >>> /* ******************************************** */ >>> /* ******************************************** */ >>> diff --git a/osaf/libs/core/mds/mds_c_api.c b/osaf/libs/core/mds/mds_c_api.c >>> --- a/osaf/libs/core/mds/mds_c_api.c >>> +++ b/osaf/libs/core/mds/mds_c_api.c >>> @@ -1601,7 +1601,23 @@ else (entry exists) >>> MDS_PROCESS_INFO *info = mds_process_info_get(adest); >>> if (info != NULL) { >>> info->count++; >>> - TRACE("svc %d up cnt:%d, db cnt:%d", svc_id, info->count, >>> mds_process_info_cnt()); >>> + TRACE("svc UP process_info EXIST, svc:%d cnt:%d, adest:%lx", >>> + svc_id, info->count, adest); >>> + } else if (mds_process_info_enabled()) { >>> + /* If process_info does not exist, create and fill in what we >>> have. >>> + * Especially count is later needed to garbage collect. >>> + */ >>> + MDS_PROCESS_INFO *info = calloc(1, sizeof(MDS_PROCESS_INFO)); >>> + osafassert(info); >>> + info->mds_dest = adest; >>> + info->count = 1; >>> + TRACE("svc UP process_info NOTEXIST, svc:%d, adest:%lx", >>> svc_id, adest); >>> + int rc = mds_process_info_add(info); >>> + osafassert(rc == NCSCC_RC_SUCCESS); >>> + } else { >>> + /* do nothing, this happens in library code or in servers not >>> using >>> + * the authentication service in MDS. */ >>> + ; >>> } >>> >>> status = >>> mds_svc_tbl_query(m_MDS_GET_PWE_HDL_FROM_SVC_HDL(local_svc_hdl), >>> @@ -2656,7 +2672,7 @@ else (entry exists) >>> MDS_PROCESS_INFO *info = mds_process_info_get(adest); >>> if (info != NULL) { >>> info->count--; >>> - TRACE("svc %d down cnt:%d, db cnt:%d", svc_id, info->count, >>> mds_process_info_cnt()); >>> + TRACE("svc %d DOWN cnt:%d, adest:%lx", svc_id, info->count, >>> adest); >>> if (info->count == 0) { >>> mds_process_info_del(info); >>> free(info); >>> @@ -3823,13 +3839,6 @@ uint32_t mds_mcm_init(void) >>> >>> ncs_patricia_tree_add(&gl_mds_mcm_cb->vdest_list, (NCS_PATRICIA_NODE >>> *)vdest_for_adest_node); >>> >>> - memset(&pat_tree_params, 0, sizeof(pat_tree_params)); >>> - pat_tree_params.key_size = sizeof(MDS_DEST); >>> - if (NCSCC_RC_SUCCESS != >>> ncs_patricia_tree_init(&gl_mds_mcm_cb->process_info_db, &pat_tree_params)) { >>> - m_MDS_LOG_ERR("MCM_API : patricia_tree_init:proc_info :failure, >>> L mds_mcm_init"); >>> - return NCSCC_RC_FAILURE; >>> - } >>> - >>> return NCSCC_RC_SUCCESS; >>> } >>> >>> diff --git a/osaf/libs/core/mds/mds_c_db.c b/osaf/libs/core/mds/mds_c_db.c >>> --- a/osaf/libs/core/mds/mds_c_db.c >>> +++ b/osaf/libs/core/mds/mds_c_db.c >>> @@ -2331,17 +2331,27 @@ uint32_t mds_subtn_res_tbl_cleanup(void) >>> return NCSCC_RC_SUCCESS; >>> } >>> >>> +/****************************************************************************/ >>> +/* Process info database, stores mapping between mdsdest and pid >>> + * Only used by some services. Tree itself cannot be located in control >>> block >>> + * which is dynamically allocated. >>> + */ >>> + >>> +static NCS_PATRICIA_TREE process_info_db; /* all known local MDS dests */ >>> + >>> MDS_PROCESS_INFO *mds_process_info_get(MDS_DEST mds_dest) >>> { >>> - return (MDS_PROCESS_INFO *) >>> ncs_patricia_tree_get(&gl_mds_mcm_cb->process_info_db, >>> - (uint8_t *)&mds_dest); >>> + if (process_info_db.n_nodes > 0) >>> + return (MDS_PROCESS_INFO *) >>> ncs_patricia_tree_get(&process_info_db, >>> + (uint8_t *)&mds_dest); >>> + return NULL; >>> } >>> >>> int mds_process_info_add(MDS_PROCESS_INFO *info) >>> { >>> TRACE_ENTER2("dest:%lx, pid:%d", info->mds_dest, info->pid); >>> info->patnode.key_info = (uint8_t *)&info->mds_dest; >>> - int rc = ncs_patricia_tree_add(&gl_mds_mcm_cb->process_info_db, >>> + int rc = ncs_patricia_tree_add(&process_info_db, >>> (NCS_PATRICIA_NODE *)&info->patnode); >>> return rc; >>> } >>> @@ -2349,14 +2359,29 @@ int mds_process_info_add(MDS_PROCESS_INF >>> int mds_process_info_del(MDS_PROCESS_INFO *info) >>> { >>> TRACE_ENTER2("dest:%lx, pid:%d", info->mds_dest, info->pid); >>> - int rc = ncs_patricia_tree_del(&gl_mds_mcm_cb->process_info_db, >>> + int rc = ncs_patricia_tree_del(&process_info_db, >>> (NCS_PATRICIA_NODE *)&info->patnode); >>> return rc; >>> } >>> >>> -int mds_process_info_cnt(void) >>> +int mds_process_info_db_init(void) >>> { >>> - return gl_mds_mcm_cb->process_info_db.n_nodes; >>> + NCS_PATRICIA_PARAMS pat_tree_params = {0}; >>> + >>> + /* locking not needed */ >>> + pat_tree_params.key_size = sizeof(MDS_DEST); >>> + if (NCSCC_RC_SUCCESS != ncs_patricia_tree_init( >>> + &process_info_db, &pat_tree_params)) { >>> + syslog(LOG_ERR, "%s: patricia_tree_init failed", __FUNCTION__); >>> + return NCSCC_RC_FAILURE; >>> + } >>> + >>> + return NCSCC_RC_SUCCESS; >>> +} >>> + >>> +int mds_process_info_enabled(void) >>> +{ >>> + return process_info_db.params.key_size > 0; >>> } >>> >>> /********************************************************* >>> diff --git a/osaf/libs/core/mds/mds_dt_common.c >>> b/osaf/libs/core/mds/mds_dt_common.c >>> --- a/osaf/libs/core/mds/mds_dt_common.c >>> +++ b/osaf/libs/core/mds/mds_dt_common.c >>> @@ -270,8 +270,6 @@ uint32_t mdtm_process_recv_message_commo >>> abort(); >>> } >>> >>> - MDS_PROCESS_INFO *info = mds_process_info_get(adest); >>> - >>> if (MDTM_DIRECT == flag) { >>> uint32_t xch_id = 0; >>> uint8_t prot_ver = 0; >>> @@ -429,6 +427,9 @@ uint32_t mdtm_process_recv_message_commo >>> reassem_queue->recv.pri = (prot_ver & MDTM_PRI_MASK) + 1; >>> reassem_queue->recv.snd_type = msg_snd_type; >>> reassem_queue->recv.src_seq_num = svc_seq_num; >>> + >>> + /* fill in credentials (if any) */ >>> + MDS_PROCESS_INFO *info = mds_process_info_get(adest); >>> if (info != NULL) { >>> reassem_queue->recv.pid = info->pid; >>> reassem_queue->recv.uid = info->uid; >>> diff --git a/osaf/libs/core/mds/mds_main.c b/osaf/libs/core/mds/mds_main.c >>> --- a/osaf/libs/core/mds/mds_main.c >>> +++ b/osaf/libs/core/mds/mds_main.c >>> @@ -153,8 +153,9 @@ static void mds_register_callback(int fd >>> >>> osaf_mutex_lock_ordie(&gl_mds_library_mutex); >>> >>> - if (mds_process_info_get(mds_dest) == NULL) { >>> - MDS_PROCESS_INFO *info = malloc(sizeof(MDS_PROCESS_INFO)); >>> + MDS_PROCESS_INFO *info = mds_process_info_get(mds_dest); >>> + if (info == NULL) { >>> + MDS_PROCESS_INFO *info = calloc(1, sizeof(MDS_PROCESS_INFO)); >>> osafassert(info); >>> info->mds_dest = mds_dest; >>> info->uid = creds->uid; >>> @@ -165,6 +166,10 @@ static void mds_register_callback(int fd >>> } else { >>> /* this happens in clients that uses both OM and OI */ >>> TRACE("dest %lx already exist", mds_dest); >>> + // just update credentials >>> + info->uid = creds->uid; >>> + info->pid = creds->pid; >>> + info->gid = creds->gid; >>> } >>> >>> osaf_mutex_unlock_ordie(&gl_mds_library_mutex); >>> @@ -188,8 +193,13 @@ static void mds_register_callback(int fd >>> */ >>> int mds_auth_server_create(const char *name) >>> { >>> + if (mds_process_info_db_init() != NCSCC_RC_SUCCESS) { >>> + syslog(LOG_ERR, "%s: mds_process_info_db_init failed", >>> __FUNCTION__); >>> + return NCSCC_RC_FAILURE; >>> + } >>> + >>> if (osaf_auth_server_create(name, mds_register_callback) != 0) { >>> - syslog(LOG_ERR, "MDS_LIB_CREATE: osaf_auth_server_create >>> failed"); >>> + syslog(LOG_ERR, "%s: osaf_auth_server_create failed", >>> __FUNCTION__); >>> return NCSCC_RC_FAILURE; >>> } >>> >>> @@ -233,6 +243,7 @@ int mds_auth_server_connect(const char * >>> if (type != MDS_REGISTER_RESP) { >>> TRACE_3("wrong type %d", type); >>> rc = NCSCC_RC_FAILURE; >>> + goto fail; >>> } >>> int status = ncs_decode_32bit(&p); >>> TRACE("received type:%d, status:%d", type, status); >>> >>> ------------------------------------------------------------------------------ >>> Slashdot TV. >>> Video for Nerds. Stuff that matters. >>> http://tv.slashdot.org/ >>> _______________________________________________ >>> Opensaf-devel mailing list >>> [email protected] >>> https://lists.sourceforge.net/lists/listinfo/opensaf-devel ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that matters. http://tv.slashdot.org/ _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
