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

Reply via email to