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

Reply via email to