ACK, Mathi.
On Fri, Sep 7, 2018 at 3:44 PM Alex Jones <ajo...@rbbn.com> wrote: > saPlmReadinessTrackResponse sometimes returns SA_AIS_OK, when invalid > parameters are passed. > > SaPlmReadinessTrackResponseT parameter is not checked for range. Also, > the msg is sent asynchronously from the agent to plmd, so that errors > from plmd cannot be passed back to the agent. > > Check the SaPlmReadinessTrackResponseT parameter when passed in, and > change the message from asynch to sync, so that errors can be passed > back. > --- > src/plm/agent/plma_api.c | 29 ++++++++++++++++++--- > src/plm/common/plms_common_utils.c | 1 + > src/plm/common/plms_edu.c | 1 + > src/plm/common/plms_evt.h | 3 ++- > src/plm/plmd/plms_adm_fsm.c | 52 > ++++++++++++++++++++++++-------------- > 5 files changed, 62 insertions(+), 24 deletions(-) > > diff --git a/src/plm/agent/plma_api.c b/src/plm/agent/plma_api.c > index 596175e51..3ca8a8c71 100644 > --- a/src/plm/agent/plma_api.c > +++ b/src/plm/agent/plma_api.c > @@ -2974,6 +2974,7 @@ SaAisErrorT > saPlmReadinessTrackResponse(SaPlmEntityGroupHandleT entityGrpHdl, > { > PLMA_CB *plma_cb = plma_ctrlblk; > PLMS_EVT plm_in_evt; > + PLMS_EVT *plm_out_res = NULL; > SaAisErrorT rc = SA_AIS_OK; > uint32_t proc_rc = NCSCC_RC_SUCCESS; > PLMA_ENTITY_GROUP_INFO *group_info; > @@ -2994,6 +2995,12 @@ SaAisErrorT > saPlmReadinessTrackResponse(SaPlmEntityGroupHandleT entityGrpHdl, > rc = SA_AIS_ERR_INVALID_PARAM; > goto end; > } > + if (response < SA_PLM_CALLBACK_RESPONSE_OK || > + response > SA_PLM_CALLBACK_RESPONSE_ERROR) { > + TRACE("response parameter is invalid"); > + rc = SA_AIS_ERR_INVALID_PARAM; > + goto end; > + } > if (!plma_cb->plms_svc_up) { > LOG_ER("PLMA : PLM SERVICE DOWN"); > rc = SA_AIS_ERR_TRY_AGAIN; > @@ -3027,10 +3034,10 @@ SaAisErrorT > saPlmReadinessTrackResponse(SaPlmEntityGroupHandleT entityGrpHdl, > plm_in_evt.req_evt.agent_track.track_cbk_res.invocation_id = > invocation; > plm_in_evt.req_evt.agent_track.track_cbk_res.response = response; > > - /* Send a mds async msg to PLMS to obtain group handle for this */ > - proc_rc = plms_mds_normal_send(plma_cb->mds_hdl, > NCSMDS_SVC_ID_PLMA, > - &plm_in_evt, plma_cb->plms_mdest_id, > - NCSMDS_SVC_ID_PLMS); > + proc_rc = plm_mds_msg_sync_send(plma_cb->mds_hdl, > NCSMDS_SVC_ID_PLMA, > + NCSMDS_SVC_ID_PLMS, > + plma_cb->plms_mdest_id, > &plm_in_evt, > + &plm_out_res, PLMS_MDS_SYNC_TIME); > > if (NCSCC_RC_SUCCESS != proc_rc) { > LOG_ER( > @@ -3038,7 +3045,21 @@ SaAisErrorT > saPlmReadinessTrackResponse(SaPlmEntityGroupHandleT entityGrpHdl, > rc = SA_AIS_ERR_TRY_AGAIN; > goto end; > } > + > + /* Verify if the response if ok */ > + if (!plm_out_res) { > + rc = SA_AIS_ERR_TRY_AGAIN; > + goto end; > + } > + if (plm_out_res->res_evt.error != SA_AIS_OK) { > + rc = plm_out_res->res_evt.error; > + goto end; > + } > + > end: > + if (plm_out_res) > + plms_free_evt(plm_out_res); > + > TRACE_LEAVE(); > return rc; > } > diff --git a/src/plm/common/plms_common_utils.c > b/src/plm/common/plms_common_utils.c > index c56093747..9837b8480 100644 > --- a/src/plm/common/plms_common_utils.c > +++ b/src/plm/common/plms_common_utils.c > @@ -148,6 +148,7 @@ SaUint32T plms_free_evt(PLMS_EVT *evt) > case PLMS_AGENT_GRP_DEL_RES: > case PLMS_AGENT_TRACK_START_RES: > case PLMS_AGENT_TRACK_STOP_RES: > + case PLMS_AGENT_TRACK_RESP_RES: > free(evt); > break; > case PLMS_AGENT_TRACK_READINESS_IMPACT_RES: > diff --git a/src/plm/common/plms_edu.c b/src/plm/common/plms_edu.c > index 1b0a2a8ec..d5f445cb4 100644 > --- a/src/plm/common/plms_edu.c > +++ b/src/plm/common/plms_edu.c > @@ -717,6 +717,7 @@ uint32_t plms_evt_test_res_type(NCSCONTEXT arg) > case PLMS_AGENT_GRP_ADD_RES: > case PLMS_AGENT_GRP_DEL_RES: > case PLMS_AGENT_TRACK_STOP_RES: > + case PLMS_AGENT_TRACK_RESP_RES: > case PLMS_AGENT_TRACK_READINESS_IMPACT_RES: > return PLMS_EDU_PLMS_COMMON_RESP; > ; > diff --git a/src/plm/common/plms_evt.h b/src/plm/common/plms_evt.h > index e87c6325e..557579968 100644 > --- a/src/plm/common/plms_evt.h > +++ b/src/plm/common/plms_evt.h > @@ -245,7 +245,8 @@ typedef enum { > PLMS_AGENT_GRP_DEL_RES, > PLMS_AGENT_TRACK_START_RES, > PLMS_AGENT_TRACK_STOP_RES, > - PLMS_AGENT_TRACK_READINESS_IMPACT_RES > + PLMS_AGENT_TRACK_READINESS_IMPACT_RES, > + PLMS_AGENT_TRACK_RESP_RES > > } PLMS_EVT_RES_TYPE; > > diff --git a/src/plm/plmd/plms_adm_fsm.c b/src/plm/plmd/plms_adm_fsm.c > index a29dc28e0..84b42efde 100644 > --- a/src/plm/plmd/plms_adm_fsm.c > +++ b/src/plm/plmd/plms_adm_fsm.c > @@ -4771,22 +4771,27 @@ SaUint32T plms_cbk_response_process(PLMS_EVT *evt) > PLMS_TRACK_INFO *trk_info; > PLMS_ENTITY_GROUP_INFO_LIST *head; > PLMS_AGENT_TRACK_OP *res = &evt->req_evt.agent_track; > + PLMS_EVT evt_resp = { 0, PLMS_RES }; > > TRACE_ENTER2("Grp_hdl: %llu,resp: %d, inv_id: %llu", > res->grp_handle, > res->track_cbk_res.response, > res->track_cbk_res.invocation_id); > > + evt_resp.res_evt.res_type = PLMS_AGENT_TRACK_RESP_RES; > + evt_resp.res_evt.ntf_id = SA_NTF_IDENTIFIER_UNUSED; > + evt_resp.res_evt.error = SA_AIS_OK; > + > /* Get to the group */ > grp = (PLMS_ENTITY_GROUP_INFO *)ncs_patricia_tree_get( > &(cb->entity_group_info), (SaUint8T *)&(res->grp_handle)); > if (NULL == grp) { > - LOG_ER("Response can not be processed as the group\ > - corresponding to grp_handle %llu not found in plms\ > - datebase.", > + LOG_ER("Response can not be processed as the group" > + "corresponding to grp_handle %llu not found in plms" > + "datebase.", > res->grp_handle); > > - TRACE_LEAVE2("ret_err: %d", NCSCC_RC_FAILURE); > - return NCSCC_RC_FAILURE; > + evt_resp.res_evt.error = SA_AIS_ERR_BAD_HANDLE; > + goto end; > } > /* If inv is not part of grp->invocation_list, then > -- PLMS receives RESPONSE_REJECT from one of the > @@ -4798,12 +4803,12 @@ SaUint32T plms_cbk_response_process(PLMS_EVT *evt) > inv_trk = plms_inv_to_cbk_in_grp_find(grp->invocation_list, > > res->track_cbk_res.invocation_id); > if (NULL == inv_trk) { > - LOG_ER("Invocation id mentioned in the resp, is not\ > - found in the grp->inocation_list. inv_id: %llu", > + LOG_ER("Invocation id mentioned in the resp, is not" > + "found in the grp->inocation_list. inv_id: %llu", > res->track_cbk_res.invocation_id); > > - TRACE_LEAVE2("ret_err: %d", NCSCC_RC_FAILURE); > - return NCSCC_RC_FAILURE; > + evt_resp.res_evt.error = SA_AIS_ERR_INVALID_PARAM; > + goto end; > } > switch (res->track_cbk_res.response) { > > @@ -4825,8 +4830,7 @@ SaUint32T plms_cbk_response_process(PLMS_EVT *evt) > response. trk_cnt: %d", > trk_info->track_count); > > - TRACE_LEAVE2("ret_err: %d", NCSCC_RC_SUCCESS); > - return NCSCC_RC_SUCCESS; > + goto end; > } > /* Oh.. Got the all the responses. Send next cbk. */ > if (SA_PLM_CHANGE_VALIDATE == trk_info->change_step) { > @@ -4836,11 +4840,12 @@ SaUint32T plms_cbk_response_process(PLMS_EVT *evt) > ret_err = > plms_cbk_start_resp_ok_err_proc(trk_info); > > } else { > - LOG_ER("Change step can not be anything otherthan\ > - START/VALIDATE. change_step: %d", > + LOG_ER("Change step can not be anything other than" > + "START/VALIDATE. change_step: %d", > trk_info->change_step); > - TRACE_LEAVE2("ret_err: %d", NCSCC_RC_FAILURE); > - return NCSCC_RC_FAILURE; > + > + evt_resp.res_evt.error = SA_AIS_ERR_INVALID_PARAM; > + goto end; > } > break; > > @@ -4851,11 +4856,11 @@ SaUint32T plms_cbk_response_process(PLMS_EVT *evt) > /* Hold on. Application can not reject if the cbk is not > a validate cbk. */ > if (SA_PLM_CHANGE_VALIDATE != trk_info->change_step) { > - LOG_ER("Response can not be rejected for callback\ > - other than VALIDATE."); > + LOG_ER("Response can not be rejected for callback" > + "other than VALIDATE."); > > - TRACE_LEAVE2("ret_err: %d", NCSCC_RC_FAILURE); > - return NCSCC_RC_FAILURE; > + evt_resp.res_evt.error = SA_AIS_ERR_INVALID_PARAM; > + goto end; > } > /* Clean up the inv_to_cbk nodes belongs to this > trk_info.*/ > head = trk_info->group_info_list; > @@ -4936,6 +4941,15 @@ SaUint32T plms_cbk_response_process(PLMS_EVT *evt) > break; > } > > +end: > + ret_err = plm_send_mds_rsp(cb->mds_hdl, NCSMDS_SVC_ID_PLMS, > &evt->sinfo, > + &evt_resp); > + > + if (NCSCC_RC_SUCCESS != ret_err) { > + LOG_ER("Sync resp to PLMA for track response FAILED. > ret_val " > + "= %d", ret_err); > + } > + > TRACE_LEAVE2("ret_err: %d", ret_err); > return ret_err; > } > -- > 2.14.4 > > _______________________________________________ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel