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
