Hi Hung,

Reviewed and tested the patch.
Ack with the following comments.

1.  Remove the memset as the client node is getting deleted

memset(&(cl_node->tmpSinfo), 0, sizeof(IMMSV_SEND_INFO));

2.  mpurged  may not be required:
if mOriginatingconn is set to "0" then the immnd will drop the reply 
saying that client went down.

where ever the mOriginatingconn is getting added must also be checked 
for mpurged, which may not be required as the client is not there, IMMND 
will drop the message.

Eg: in ccbAbort
  clVector.push_back(ccb->mOriginatingConn);

/Neel.

On 2016/05/17 08:42 AM, Hung Nguyen wrote:
>   osaf/services/saf/immsv/immnd/ImmModel.cc |  43 
> +++++++++++++++++++++---------
>   osaf/services/saf/immsv/immnd/immnd_evt.c |   4 ++
>   2 files changed, 34 insertions(+), 13 deletions(-)
>
>
> Add new member 'mPurged' to struct 'CcbInfo'.
> It will be set to true when the ccb continuation is purged.
> In the "ccb continuation" functions, mPurged is checked to
> decide whether to response to client or not.
>
> The value of 'mOriginatingConn' is needed to
> get the CcbId when discarding the client (getCcbIdsForOrigCon).
> So it will not be cleared when the ccb continuation is purged.
>
> diff --git a/osaf/services/saf/immsv/immnd/ImmModel.cc 
> b/osaf/services/saf/immsv/immnd/ImmModel.cc
> --- a/osaf/services/saf/immsv/immnd/ImmModel.cc
> +++ b/osaf/services/saf/immsv/immnd/ImmModel.cc
> @@ -350,7 +350,7 @@ struct CcbInfo
>       CcbInfo(): mId(0), mAdminOwnerId(0), mCcbFlags(0), mOriginatingConn(0),
>                  mOriginatingNode(0), mState(IMM_CCB_ILLEGAL), 
> mVeto(SA_AIS_OK),
>                  mWaitStartTime((time_t) 0), mOpCount(0), mPbeRestartId(0),
> -               mErrorStrings(NULL), mAugCcbParent(NULL) {}
> +               mErrorStrings(NULL), mAugCcbParent(NULL), mPurged(false) {}
>       bool isOk() {return mVeto == SA_AIS_OK;}
>       bool isActive() {return (mState < IMM_CCB_COMMITTED);}
>       void addObjReadLock(ObjectInfo* obj, std::string& objName);
> @@ -376,6 +376,8 @@ struct CcbInfo
>       ImmsvAttrNameList* mErrorStrings;/*Error strings generated by current 
> op */
>       AugCcbParent*     mAugCcbParent;
>       ObjectSet         mSafeReadSet;
> +    bool              mPurged; /* True if the ccb continuation is purged.
> +                                  Note that on remote nodes, mPurged is 
> always 'false' */
>   };
>   typedef std::vector<CcbInfo*> CcbVector;
>   
> @@ -10538,7 +10540,10 @@ ImmModel::ccbObjDelContinuation(immsv_oi
>           osafassert(/*(omuti->second->mContinuationId == 0) ||*/
>                  (omuti->second->mContinuationId == (SaUint32T) rsp->inv));
>   
> -        *reqConn = ccb->mOriginatingConn;
> +        /* Only send response when ccb continuation is not purged */
> +        if (!ccb->mPurged) {
> +            *reqConn = ccb->mOriginatingConn;
> +        }
>   
>           *augDelete = omuti->second->mIsAugDelete;
>   
> @@ -10627,7 +10632,10 @@ ImmModel::ccbCompletedContinuation(immsv
>       if(ccb->mState == IMM_CCB_VALIDATED) {
>           LOG_IN("GOING FROM IMM_CCB_VALIDATED to IMM_CCB_PREPARE Ccb:%u", 
> ccbId);
>           ccb->mState = IMM_CCB_PREPARE;
> -        *reqConn = ccb->mOriginatingConn;
> +        /* Only send response when ccb continuation is not purged */
> +        if (!ccb->mPurged) {
> +            *reqConn = ccb->mOriginatingConn;
> +        }
>           goto done;
>       }
>       
> @@ -10673,7 +10681,10 @@ ImmModel::ccbCompletedContinuation(immsv
>                   ix->second->mContinuationId, rsp->inv);
>           }
>   
> -        *reqConn = ccb->mOriginatingConn;
> +        /* Only send response when ccb continuation is not purged */
> +        if (!ccb->mPurged) {
> +            *reqConn = ccb->mOriginatingConn;
> +        }
>   
>           if(rsp->result != SA_AIS_OK) {
>               if(ccb->mVeto == SA_AIS_OK) {
> @@ -10810,7 +10821,11 @@ ImmModel::ccbObjCreateContinuation(SaUin
>           ccb->mAugCcbParent = NULL;
>       }
>   
> -    *reqConn = ccb->mOriginatingConn;
> +    /* Only send response when ccb continuation is not purged */
> +    if (!ccb->mPurged) {
> +        *reqConn = ccb->mOriginatingConn;
> +    }
> +
>       if((ccb->mVeto == SA_AIS_OK) && (error != SA_AIS_OK)) {
>           LOG_NO("ImmModel::ccbObjCreateContinuation: "
>               "implementer returned error, Ccb aborted with error: %u",
> @@ -10897,7 +10912,11 @@ ImmModel::ccbObjModifyContinuation(SaUin
>           ccb->mAugCcbParent = NULL;
>       }
>   
> -    *reqConn = ccb->mOriginatingConn;
> +    /* Only send response when ccb continuation is not purged */
> +    if (!ccb->mPurged) {
> +        *reqConn = ccb->mOriginatingConn;
> +    }
> +
>       if((ccb->mVeto == SA_AIS_OK) && (error != SA_AIS_OK)) {
>           LOG_IN("ImmModel::ccbObjModifyContinuation: "
>               "implementer returned error, Ccb aborted with error: %u", 
> error);
> @@ -13693,13 +13712,11 @@ ImmModel::purgeSyncRequest(SaUint32T cli
>       }
>   
>       if(ccbFound != sCcbVector.end()) {
> -        /* To drop the ccb continuation, we simply set mOriginatingConn to 
> zero,
> -           which is the value it has on all nodes except at the node where 
> the
> -           originating connection resides. By setting it to zero, we ensure 
> that
> -           no attempt can be made to reply to that connection for any 
> continuation
> -           on this ccb (e.g. a late arriving OI reply.
> -        */
> -        (*ccbFound)->mOriginatingConn = 0;
> +        /* To drop the ccb continuation, we set mPurged to true. By that,
> +           we ensure that no attempt can be made to reply to that connection
> +           for any continuation on this ccb (e.g. a late arriving OI reply).
> +         */
> +        (*ccbFound)->mPurged = true;
>           purged = true;
>           TRACE_5("Purged Ccb continuation for ccb:%u in state %u",
>               (*ccbFound)->mId, (*ccbFound)->mState);
> diff --git a/osaf/services/saf/immsv/immnd/immnd_evt.c 
> b/osaf/services/saf/immsv/immnd/immnd_evt.c
> --- a/osaf/services/saf/immsv/immnd/immnd_evt.c
> +++ b/osaf/services/saf/immsv/immnd/immnd_evt.c
> @@ -2127,6 +2127,10 @@ static uint32_t        immnd_evt_proc_cl_imma_t
>               */
>               memset(&(cl_node->tmpSinfo), 0, sizeof(IMMSV_SEND_INFO));
>               
> osafassert(immnd_mds_client_not_busy(&(cl_node->tmpSinfo))==SA_AIS_OK);
> +
> +             immnd_proc_imma_discard_connection(cb, cl_node, false);
> +             osafassert(immnd_client_node_del(cb, cl_node)  == 
> NCSCC_RC_SUCCESS);
> +             free(cl_node);
>       } else {
>               /* The request could not be purged (depends on request type),
>                  or the reply has already been arrived but came too late


------------------------------------------------------------------------------
Mobile security can be enabling, not merely restricting. Employees who
bring their own devices (BYOD) to work are irked by the imposition of MDM
restrictions. Mobile Device Manager Plus allows you to control only the
apps on BYO-devices by containerizing them, leaving personal data untouched!
https://ad.doubleclick.net/ddm/clk/304595813;131938128;j
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to