Hi Neel, About "2. mpurged may not be required", I explained that in the commit message. If we set 'mOriginatingconn' to 0, it will fail to get the CcbId list for the connection. We will have orphaned CCBs.
I will send new patch for review. BR, Hung Nguyen - DEK Technologies -------------------------------------------------------------------------------- From: Neelakanta Reddy [email protected] Sent: Tuesday, May 17, 2016 5:32PM To: Hung Nguyen, Zoran Milinkovic [email protected], [email protected] Cc: Opensaf-devel [email protected] Subject: Re: [PATCH 1 of 1] imm: Discard client connection when receiving A2ND_CL_TIMEOUT [#1817] 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
