From: hyuna0213.jo Date: Thu, 23 Jun 2016 00:10:34 +0000 (+0900) Subject: [JIRA-1151] IoTivity Blockwise transfer is sort of broken when X-Git-Tag: 1.2.0+RC1~274 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d7c1aa344bcf0f14e2f233789e5e5aa83bbbf610;p=platform%2Fupstream%2Fiotivity.git [JIRA-1151] IoTivity Blockwise transfer is sort of broken when paired with slow response - When a resource is set as slow response resource, and it has a large enough payload to be sent as blockwise transfer data, and a CON retrieve message is sent to the resource, the coap messaging mechanism break down. - Remove the unnecessary code Change-Id: I4bfabc4f457f637a8a88e6c237c25831c7b6c776 Signed-off-by: hyuna0213.jo Reviewed-on: https://gerrit.iotivity.org/gerrit/8895 Tested-by: jenkins-iotivity Reviewed-by: Mushfiqul Islam Reviewed-by: jihwan seo Reviewed-by: Ashok Babu Channa --- diff --git a/resource/csdk/connectivity/src/cablockwisetransfer.c b/resource/csdk/connectivity/src/cablockwisetransfer.c index 8f5f131..a6a9cf5 100644 --- a/resource/csdk/connectivity/src/cablockwisetransfer.c +++ b/resource/csdk/connectivity/src/cablockwisetransfer.c @@ -346,6 +346,16 @@ CAResult_t CAReceiveBlockWiseData(coap_pdu_t *pdu, const CAEndpoint_t *endpoint, return CA_STATUS_FAILED; } + // If we didn't send the last block message and received EMPTY message, + // we have to remain the block data from list. + CABlockData_t *data = CAGetBlockDataFromBlockDataList(blockDataID); + if (data && (data->block1.m || data->block2.m)) + { + OIC_LOG(DEBUG, TAG, "this is normal EMPTY message for blockwise-transfer."); + CADestroyBlockID(blockDataID); + return CA_STATUS_OK; + } + CARemoveBlockDataFromList(blockDataID); CADestroyBlockID(blockDataID); return CA_NOT_SUPPORTED; @@ -483,23 +493,20 @@ CAResult_t CAProcessNextStep(const coap_pdu_t *pdu, const CAData_t *receivedData return CA_STATUS_FAILED; } - if (data->requestInfo) - { - data->requestInfo->info.messageId = pdu->hdr->coap_hdr_udp_t.id; - } - if (data->responseInfo) { + data->responseInfo->info.type = + (pdu->hdr->coap_hdr_udp_t.type == CA_MSG_CONFIRM) ? + CA_MSG_ACKNOWLEDGE : CA_MSG_NONCONFIRM; data->responseInfo->info.messageId = pdu->hdr->coap_hdr_udp_t.id; - } - res = CAAddSendThreadQueue(data, blockID); - if (CA_STATUS_OK != res) - { - OIC_LOG(ERROR, TAG, "add has failed"); - return res; + res = CAAddSendThreadQueue(data, blockID); + if (CA_STATUS_OK != res) + { + OIC_LOG(ERROR, TAG, "add has failed"); + return res; + } } - break; case CA_OPTION1_RESPONSE: @@ -578,6 +585,21 @@ CAResult_t CAProcessNextStep(const coap_pdu_t *pdu, const CAData_t *receivedData return CA_STATUS_OK; } +static CAResult_t CASendDirectEmptyResponse(const CAEndpoint_t *endpoint, uint16_t messageId) +{ + OIC_LOG(DEBUG, TAG, "Entering CASendDirectEmptyResponse"); + CAResponseInfo_t respInfo = { + .result = CA_EMPTY + }; + respInfo.info.type = CA_MSG_ACKNOWLEDGE; + respInfo.info.messageId = messageId; + + CAResult_t caResult = CASendResponse(endpoint, &respInfo); + + OIC_LOG(DEBUG, TAG, "Exit CASendDirectEmptyResponse"); + return caResult; +} + CAResult_t CASendBlockMessage(const coap_pdu_t *pdu, CAMessageType_t msgType, const CABlockDataID_t *blockID) { @@ -619,9 +641,18 @@ CAResult_t CASendBlockMessage(const coap_pdu_t *pdu, CAMessageType_t msgType, } else { - OIC_LOG(DEBUG, TAG, "need new msgID"); + // if the received response message type is CON, send empty message. + // and then, send next block request message with new messagId. + if (msgType == CA_MSG_CONFIRM) + { + CASendDirectEmptyResponse(data->remoteEndpoint, + data->requestInfo->info.messageId); + sentMsgType = CA_MSG_CONFIRM; + } + if (data->requestInfo) { + OIC_LOG(DEBUG, TAG, "need new msgID"); data->requestInfo->info.messageId = 0; data->requestInfo->info.type = sentMsgType; } @@ -782,6 +813,44 @@ CAResult_t CAReceiveLastBlock(const CABlockDataID_t *blockID, const CAData_t *re return CA_STATUS_OK; } +static CABlockData_t* CACheckTheExistOfBlockData(const CABlockDataID_t* blockDataID, + coap_pdu_t *pdu, const CAEndpoint_t *endpoint, + uint8_t blockType) +{ + // Get BlockData data. If does not exist, create a new data + CABlockData_t *data = CAGetBlockDataFromBlockDataList(blockDataID); + if (!data) + { + OIC_LOG(DEBUG, TAG, "block data doesn't exist in list. create new one"); + + CAData_t *cadata = CACreateNewDataSet(pdu, endpoint); + if (!cadata) + { + OIC_LOG(ERROR, TAG, "data is null"); + return NULL; + } + + data = CACreateNewBlockData(cadata); + if (!data) + { + OIC_LOG(ERROR, TAG, "failed to create a new block data"); + CADestroyDataSet(cadata); + return NULL; + } + CADestroyDataSet(cadata); + } + + // update BLOCK OPTION type + CAResult_t res = CAUpdateBlockOptionType(blockDataID, blockType); + if (CA_STATUS_OK != res) + { + OIC_LOG(ERROR, TAG, "update has failed"); + return NULL; + } + + return data; +} + // TODO make pdu const after libcoap is updated to support that. CAResult_t CASetNextBlockOption1(coap_pdu_t *pdu, const CAEndpoint_t *endpoint, const CAData_t *receivedData, coap_block_t block, @@ -806,39 +875,15 @@ CAResult_t CASetNextBlockOption1(coap_pdu_t *pdu, const CAEndpoint_t *endpoint, return CA_STATUS_FAILED; } - // Get BlockData data. If does not exist, create a new data - CABlockData_t *data = CAGetBlockDataFromBlockDataList(blockDataID); + CABlockData_t *data = CACheckTheExistOfBlockData(blockDataID, pdu, endpoint, + COAP_OPTION_BLOCK1); if (!data) { - OIC_LOG(DEBUG, TAG, "block data doesn't exist in list. create new one"); - - CAData_t *cadata = CACreateNewDataSet(pdu, endpoint); - if (!cadata) - { - OIC_LOG(ERROR, TAG, "data is null"); - CADestroyBlockID(blockDataID); - return CA_STATUS_FAILED; - } - - data = CACreateNewBlockData(cadata); - if (!data) - { - OIC_LOG(ERROR, TAG, "failed to create a new block data"); - CADestroyDataSet(cadata); - CADestroyBlockID(blockDataID); - return CA_STATUS_FAILED; - } - CADestroyDataSet(cadata); - } - - // update BLOCK OPTION1 type - CAResult_t res = CAUpdateBlockOptionType(blockDataID, COAP_OPTION_BLOCK1); - if (CA_STATUS_OK != res) - { - OIC_LOG(ERROR, TAG, "update has failed"); + OIC_LOG(ERROR, TAG, "Failed to create or get block data"); goto exit; } + CAResult_t res = CA_STATUS_OK; uint8_t blockWiseStatus = CA_BLOCK_UNKNOWN; uint32_t code = pdu->hdr->coap_hdr_udp_t.code; if (CA_GET == code || CA_POST == code || CA_PUT == code || CA_DELETE == code) @@ -886,15 +931,8 @@ CAResult_t CASetNextBlockOption1(coap_pdu_t *pdu, const CAEndpoint_t *endpoint, { OIC_LOG_V(DEBUG, TAG, "M bit is %d", block.m); - if (0 == block.m) - { - // Last block is received - blockWiseStatus = CA_OPTION1_REQUEST_LAST_BLOCK; - } - else - { - blockWiseStatus = CA_OPTION1_REQUEST_BLOCK; - } + blockWiseStatus = (0 == block.m) ? + CA_OPTION1_REQUEST_LAST_BLOCK : CA_OPTION1_REQUEST_BLOCK; } } else @@ -979,39 +1017,15 @@ CAResult_t CASetNextBlockOption2(coap_pdu_t *pdu, const CAEndpoint_t *endpoint, return CA_STATUS_FAILED; } - // Get BlockData data. If does not exist, create a new data - CABlockData_t *data = CAGetBlockDataFromBlockDataList(blockDataID); + CABlockData_t *data = CACheckTheExistOfBlockData(blockDataID, pdu, endpoint, + COAP_OPTION_BLOCK2); if (!data) { - OIC_LOG(DEBUG, TAG, "block data doesn't exist in list. create new one"); - - CAData_t *cadata = CACreateNewDataSet(pdu, endpoint); - if (!cadata) - { - OIC_LOG(ERROR, TAG, "data is null"); - CADestroyBlockID(blockDataID); - return CA_STATUS_FAILED; - } - - data = CACreateNewBlockData(cadata); - if (!data) - { - OIC_LOG(ERROR, TAG, "failed to create a new block data"); - CADestroyDataSet(cadata); - CADestroyBlockID(blockDataID); - return CA_STATUS_FAILED; - } - CADestroyDataSet(cadata); - } - - // set Block Option Type - CAResult_t res = CAUpdateBlockOptionType(blockDataID, COAP_OPTION_BLOCK2); - if (CA_STATUS_OK != res) - { - OIC_LOG(ERROR, TAG, "update has failed"); + OIC_LOG(ERROR, TAG, "Failed to create or get block data"); goto exit; } + CAResult_t res = CA_STATUS_OK; uint8_t blockWiseStatus = CA_BLOCK_UNKNOWN; if (0 == block.num && CA_GET == pdu->hdr->coap_hdr_udp_t.code && 0 == block.m) { @@ -2042,6 +2056,7 @@ CAData_t* CACreateNewDataSet(const coap_pdu_t *pdu, const CAEndpoint_t *endpoint CAGetResponseInfoFromPDU(pdu, resInfo, endpoint); requestInfo->method = CA_GET; + requestInfo->info.messageId = CAGetMessageIdFromPduBinaryData(pdu->hdr, pdu->length); requestInfo->info.resourceUri = OICStrdup(resInfo->info.resourceUri); // after copying the resource uri, destroy response info. @@ -2205,8 +2220,7 @@ CAResult_t CAHandleBlockErrorResponse(coap_block_t *block, uint16_t blockType, return CA_STATUS_OK; } -CAResult_t CAUpdateBlockOptionType(const CABlockDataID_t *blockID, - uint8_t blockType) +CAResult_t CAUpdateBlockOptionType(const CABlockDataID_t *blockID, uint8_t blockType) { OIC_LOG(DEBUG, TAG, "IN-UpdateBlockOptionType"); VERIFY_NON_NULL(blockID, TAG, "blockID"); @@ -2333,93 +2347,35 @@ CAResult_t CACheckBlockDataValidation(const CAData_t *sendData, CABlockData_t ** VERIFY_NON_NULL(sendData, TAG, "sendData"); VERIFY_NON_NULL(blockData, TAG, "blockData"); - CABlockDataID_t* blockDataID = NULL; - if(sendData->requestInfo) - { - blockDataID = CACreateBlockDatablockId( - (CAToken_t)sendData->requestInfo->info.token, - sendData->requestInfo->info.tokenLength, - sendData->remoteEndpoint->port); - } - else if(sendData->responseInfo) - { - blockDataID = CACreateBlockDatablockId( - (CAToken_t)sendData->responseInfo->info.token, - sendData->responseInfo->info.tokenLength, - sendData->remoteEndpoint->port); - } - else + if (sendData->responseInfo) { - OIC_LOG(ERROR, TAG, "sendData doesn't have requestInfo or responseInfo"); - return CA_STATUS_FAILED; - } - - if (NULL == blockDataID || blockDataID->idLength < 1) - { - OIC_LOG(ERROR, TAG, "blockId is null"); - CADestroyBlockID(blockDataID); - return CA_STATUS_FAILED; - } - - ca_mutex_lock(g_context.blockDataListMutex); - - size_t len = u_arraylist_length(g_context.dataList); - for (size_t i = 0; i < len; i++) - { - CABlockData_t *currData = (CABlockData_t *) u_arraylist_get(g_context.dataList, i); - if (!currData) + CABlockDataID_t* blockDataID = CACreateBlockDatablockId( + (CAToken_t)sendData->responseInfo->info.token, + sendData->responseInfo->info.tokenLength, + sendData->remoteEndpoint->port); + if (NULL == blockDataID || blockDataID->idLength < 1) { - continue; + OIC_LOG(ERROR, TAG, "blockId is null"); + CADestroyBlockID(blockDataID); + return CA_STATUS_FAILED; } - if (sendData->requestInfo) // sendData is requestMessage - { - OIC_LOG(DEBUG, TAG, "Send request"); - if (NULL != currData->blockDataId - && NULL != currData->blockDataId->id - && currData->blockDataId->idLength > 0 - && NULL != sendData->requestInfo->info.token) - { - if (CABlockidMatches(currData, blockDataID)) - { - OIC_LOG(ERROR, TAG, "already sent"); - continue; - } - } - } - else if (sendData->responseInfo) // sendData is responseMessage + CABlockData_t *storedData = CAGetBlockDataFromBlockDataList(blockDataID); + if (storedData) { - OIC_LOG(DEBUG, TAG, "Send response"); - if (NULL != currData->blockDataId - && NULL != currData->blockDataId->id - && currData->blockDataId->idLength > 0 - && NULL != sendData->responseInfo->info.token) + OIC_LOG(DEBUG, TAG, "Send response about the received block request."); + if (storedData->sentData) { - if (CABlockidMatches(currData, blockDataID)) - { - // set sendData - if (NULL != currData->sentData) - { - OIC_LOG(DEBUG, TAG, "init block number"); - CADestroyDataSet(currData->sentData); - } - currData->sentData = CACloneCAData(sendData); - *blockData = currData; - ca_mutex_unlock(g_context.blockDataListMutex); - CADestroyBlockID(blockDataID); - return CA_STATUS_OK; - } + OIC_LOG(DEBUG, TAG, "init block number"); + CADestroyDataSet(storedData->sentData); } + storedData->sentData = CACloneCAData(sendData); + *blockData = storedData; + CADestroyBlockID(blockDataID); + return CA_STATUS_OK; } - else - { - OIC_LOG(ERROR, TAG, "no CAInfo data"); - continue; - } + CADestroyBlockID(blockDataID); } - ca_mutex_unlock(g_context.blockDataListMutex); - - CADestroyBlockID(blockDataID); return CA_STATUS_FAILED; } diff --git a/resource/csdk/stack/src/ocserverrequest.c b/resource/csdk/stack/src/ocserverrequest.c index b2f9d2e..0526e9d 100644 --- a/resource/csdk/stack/src/ocserverrequest.c +++ b/resource/csdk/stack/src/ocserverrequest.c @@ -497,6 +497,7 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse) CopyDevAddrToEndpoint(&serverRequest->devAddr, &responseEndpoint); + responseInfo.info.messageId = serverRequest->coapID; responseInfo.info.resourceUri = serverRequest->resourceUrl; responseInfo.result = ConvertEHResultToCAResult(ehResponse->ehResult, serverRequest->method); @@ -516,6 +517,8 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse) else if(!serverRequest->notificationFlag && serverRequest->slowFlag && serverRequest->qos == OC_HIGH_QOS) { + // To assign new messageId in CA. + responseInfo.info.messageId = 0; responseInfo.info.type = CA_MSG_CONFIRM; } else if(!serverRequest->notificationFlag) @@ -529,7 +532,6 @@ OCStackResult HandleSingleResponse(OCEntityHandlerResponse * ehResponse) } char rspToken[CA_MAX_TOKEN_LEN + 1] = {}; - responseInfo.info.messageId = serverRequest->coapID; responseInfo.info.token = (CAToken_t)rspToken; memcpy(responseInfo.info.token, serverRequest->requestToken, serverRequest->tokenLength);