diff --git a/src/fsfw/cfdp/handler/DestHandler.cpp b/src/fsfw/cfdp/handler/DestHandler.cpp index 974bc335..47e6e9c2 100644 --- a/src/fsfw/cfdp/handler/DestHandler.cpp +++ b/src/fsfw/cfdp/handler/DestHandler.cpp @@ -2,14 +2,18 @@ #include +#include #include #include "fsfw/FSFW.h" +#include "fsfw/cfdp/pdu/AckPduCreator.h" +#include "fsfw/cfdp/pdu/AckPduReader.h" #include "fsfw/cfdp/pdu/EofPduReader.h" #include "fsfw/cfdp/pdu/FileDataReader.h" #include "fsfw/cfdp/pdu/FinishedPduCreator.h" #include "fsfw/cfdp/pdu/HeaderReader.h" #include "fsfw/cfdp/pdu/KeepAlivePduCreator.h" +#include "fsfw/cfdp/pdu/NakPduCreator.h" #include "fsfw/objectmanager.h" #include "fsfw/returnvalues/returnvalue.h" #include "fsfw/tmtcservices/TmTcMessage.h" @@ -25,7 +29,13 @@ cfdp::DestHandler::DestHandler(PduSenderIF& pduSender, size_t pduBufSize, DestHa msgToUserVec(params.maxTlvsInOnePdu), transactionParams(params.maxFilenameLen), destParams(std::move(params)), - fsfwParams(fsfwParams) { + fsfwParams(fsfwParams), + // At least one request has to fit, otherwise sendNakSequence() indexes an empty buffer and + // the deferred lost segment procedure could not make progress anyway. + nakSegmentBuf(std::max(destParams.maxSegmentRequestsPerNakPdu, 1)), + positiveAckTimer(0, false), + nakTimer(0, false), + checkTimer(0, false) { transactionParams.pduConf.direction = cfdp::Direction::TOWARDS_SENDER; } @@ -42,8 +52,31 @@ const cfdp::DestHandler::FsmResult& cfdp::DestHandler::stateMachine( return fsmRes; } PduPacketIF& pduPacket = *optPduPacket; - if (pduPacket.getPduType() == FILE_DATA or - (pduPacket.getPduType() == FILE_DIRECTIVE and *pduPacket.getFileDirective() != METADATA)) { + if (pduPacket.getPduType() == FILE_DIRECTIVE and + *pduPacket.getFileDirective() == EOF_DIRECTIVE) { + // D7 of the class 2 plan: an EOF PDU retransmitted after we already finished the + // transaction still has to be acknowledged. Without this the sender keeps retransmitting + // until its positive ACK limit and then declares a fault at the end of a transfer that + // actually succeeded. + result = ackInactiveEofPdu(pduPacket); + if (result != OK) { + fsmRes.result = result; + return fsmRes; + } + return updateFsmRes(errorIdx); + } + if (pduPacket.getPduType() == FILE_DATA) { + // In acknowledged mode a lost metadata PDU is recoverable: start the transaction from the + // PDU header and ask for the metadata with a NAK of scope 0..0. Only acknowledged mode + // transactions can do this, everything else stays an error. + result = startMetadatalessTransaction(pduPacket); + if (result == OK) { + return updateFsmRes(errorIdx); + } + fsmRes.result = DEST_NON_METADATA_PDU_AS_FIRST_PDU; + return fsmRes; + } + if (pduPacket.getPduType() == FILE_DIRECTIVE and *pduPacket.getFileDirective() != METADATA) { fsmRes.result = DEST_NON_METADATA_PDU_AS_FIRST_PDU; return fsmRes; } @@ -52,6 +85,11 @@ const cfdp::DestHandler::FsmResult& cfdp::DestHandler::stateMachine( return updateFsmRes(errorIdx); } + if (fsmRes.state == CfdpState::BUSY_CLASS_2_ACKED) { + fsmAcked(optPduPacket, errorIdx); + return updateFsmRes(errorIdx); + } + if (fsmRes.state == CfdpState::BUSY_CLASS_1_NACKED) { if (fsmRes.step == TransactionStep::RECEIVING_FILE_DATA_PDUS) { if (!optPduPacket.has_value()) { @@ -75,20 +113,88 @@ const cfdp::DestHandler::FsmResult& cfdp::DestHandler::stateMachine( checkAndHandleError(result, errorIdx); } if (fsmRes.step == TransactionStep::SENDING_FINISHED_PDU) { - result = sendFinishedPdu(); - checkAndHandleError(result, errorIdx); - finish(); + // Class 1 has no ACK for the Finished PDU, so the transaction is done the moment it goes + // out. It must actually go out first though: finishing on a failed send means the peer never + // hears that a transfer which did succeed completed, and it has no way to ask again. + if (trySendingFinishedPdu(errorIdx)) { + finish(); + } return updateFsmRes(errorIdx); } - if (fsmRes.state == CfdpState::BUSY_CLASS_2_ACKED) { - // TODO: Will be implemented at a later stage -#if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "CFDP state machine for acknowledged mode not implemented yet" << std::endl; -#endif - } return updateFsmRes(errorIdx); } +void cfdp::DestHandler::fsmAcked( + const std::optional> optPduPacket, uint8_t& errorIdx) { + ReturnValue_t result; + if (optPduPacket.has_value()) { + PduPacketIF& pduPacket = *optPduPacket; + if (pduPacket.getPduType() == FILE_DATA) { + // Retransmitted segments are only expected while the file is still being received. + // handleFileDataPdu writes at the PDU offset, so out of order writes are fine. + if (fsmRes.step == TransactionStep::RECEIVING_FILE_DATA_PDUS or + fsmRes.step == TransactionStep::WAITING_FOR_MISSING_DATA) { + result = handleFileDataPdu(pduPacket); + checkAndHandleError(result, errorIdx); + } + } else if (pduPacket.getPduType() == FILE_DIRECTIVE) { + switch (*pduPacket.getFileDirective()) { + case (METADATA): { + // Either a duplicate, which startTransaction discards, or the retransmission we asked + // for after a lost metadata PDU. + result = handleMetadataPdu(pduPacket); + checkAndHandleError(result, errorIdx); + break; + } + case (EOF_DIRECTIVE): { + result = handleEofPdu(pduPacket); + checkAndHandleError(result, errorIdx); + break; + } + case (ACK): { + result = handleAckPdu(pduPacket); + checkAndHandleError(result, errorIdx); + break; + } + default: + break; + } + } + } + // D2: the ACK for the EOF PDU is emitted here, before TRANSFER_COMPLETION runs the checksum + // pass over the whole received file. On an iOBC that pass takes seconds for a large file, + // which is long enough for the sender's positive ACK timer to fire. + if (fsmRes.step == TransactionStep::SENDING_ACK_PDU) { + result = handleSendingAckPdu(); + checkAndHandleError(result, errorIdx); + } + if (fsmRes.step == TransactionStep::WAITING_FOR_MISSING_DATA) { + result = handleWaitingForMissingData(); + checkAndHandleError(result, errorIdx); + } + if (fsmRes.step == TransactionStep::TRANSFER_COMPLETION) { + result = handleTransferCompletion(); + checkAndHandleError(result, errorIdx); + } + if (fsmRes.step == TransactionStep::SENDING_FINISHED_PDU) { + if (not trySendingFinishedPdu(errorIdx)) { + // Nothing went out, so there is no ACK to wait for: either the send is retried on the next + // call, or the attempt budget ran out and the transaction has already been released. + return; + } + // In acknowledged mode the Finished PDU is itself acknowledged, so the transaction is + // retained until the ACK arrives instead of being finished right here. + transactionParams.positiveAckCounter = 0; + positiveAckTimer.setTimeout(transactionParams.remoteCfg->positiveAckTimerIntervalMs); + fsmRes.step = TransactionStep::WAITING_FOR_FINISHED_ACK; + return; + } + if (fsmRes.step == TransactionStep::WAITING_FOR_FINISHED_ACK) { + result = handleWaitingForFinishedAck(); + checkAndHandleError(result, errorIdx); + } +} + ReturnValue_t cfdp::DestHandler::handleMetadataPdu(const PduPacketIF& pduPacket) { // Process metadata PDU cfdp::StringLv sourceFileName; @@ -118,6 +224,11 @@ ReturnValue_t cfdp::DestHandler::handleFileDataPdu(const PduPacketIF& info) { if (result != OK) { return result; } + if (fsmRes.state == CfdpState::BUSY_CLASS_2_ACKED and not transactionParams.metadataReceived) { + // The metadata PDU was lost, so there is no destination file name to write to yet. Discard + // the payload; it is re-requested by the NAK sequence once the metadata has arrived. + return OK; + } size_t fileSegmentLen = 0; const uint8_t* fileData = fdInfo.getFileData(&fileSegmentLen); if (destParams.cfg.indicCfg.fileSegmentRecvIndicRequired) { @@ -150,8 +261,13 @@ ReturnValue_t cfdp::DestHandler::handleFileDataPdu(const PduPacketIF& info) { } transactionParams.deliveryStatus = FileDeliveryStatus::RETAINED_IN_FILESTORE; transactionParams.vfsErrorCount = 0; - if (fdInfo.getOffset().value() + fileSegmentLen > transactionParams.progress) { - transactionParams.progress = fdInfo.getOffset().value() + fileSegmentLen; + const uint64_t offset = fdInfo.getOffset().value(); + const uint64_t endOfSegment = offset + fileSegmentLen; + if (fsmRes.state == CfdpState::BUSY_CLASS_2_ACKED) { + trackReceivedSegment(offset, endOfSegment); + } + if (endOfSegment > transactionParams.progress) { + transactionParams.progress = endOfSegment; } return result; } @@ -160,7 +276,7 @@ ReturnValue_t cfdp::DestHandler::handleEofPdu(const cfdp::PduPacketIF& info) { size_t pduSize = 0; const auto rawPdu = info.getRawPduData(pduSize); // Process EOF PDU - EofInfo eofInfo(nullptr); + EofInfo eofInfo(&eofFaultLocation); EofPduReader reader(rawPdu, pduSize, eofInfo); ReturnValue_t result = reader.parseData(); if (result != OK) { @@ -179,12 +295,39 @@ ReturnValue_t cfdp::DestHandler::handleEofPdu(const cfdp::PduPacketIF& info) { if (destParams.cfg.indicCfg.eofRecvIndicRequired) { destParams.user.eofRecvIndication(getTransactionId()); } - if (fsmRes.step == TransactionStep::RECEIVING_FILE_DATA_PDUS) { - if (fsmRes.state == CfdpState::BUSY_CLASS_1_NACKED) { + if (fsmRes.state == CfdpState::BUSY_CLASS_1_NACKED) { + if (fsmRes.step == TransactionStep::RECEIVING_FILE_DATA_PDUS) { fsmRes.step = TransactionStep::TRANSFER_COMPLETION; - } else if (fsmRes.state == CfdpState::BUSY_CLASS_2_ACKED) { - fsmRes.step = TransactionStep::SENDING_ACK_PDU; } + return returnvalue::OK; + } + if (fsmRes.state != CfdpState::BUSY_CLASS_2_ACKED) { + return returnvalue::OK; + } + transactionParams.eofReceived = true; + transactionParams.eofConditionCode = eofInfo.getConditionCode(); + if (eofInfo.getConditionCode() != ConditionCode::NO_ERROR) { + // A Cancel EOF ends the transaction. Adopt its condition code so that transfer completion + // reports the cancellation, instead of running a checksum pass over a file the sender has + // already abandoned and then reporting a checksum failure for it. + transactionParams.conditionCode = eofInfo.getConditionCode(); + } + if (eofInfo.getConditionCode() == ConditionCode::NO_ERROR and + transactionParams.fileSize.value() > transactionParams.progress) { + // Everything between the highest offset seen so far and the file size announced by the EOF + // PDU is missing. This is also the only gap that exists for a transfer which lost its tail. + insertLostSegment(transactionParams.progress, transactionParams.fileSize.value()); + } + if (fsmRes.step == TransactionStep::RECEIVING_FILE_DATA_PDUS or + fsmRes.step == TransactionStep::WAITING_FOR_MISSING_DATA) { + // A duplicate EOF arriving during the lost segment procedure means our ACK did not make it + // back, so re-enter the ACK step, which also re-issues the NAK sequence. + fsmRes.step = TransactionStep::SENDING_ACK_PDU; + } else { + // Duplicate EOF for a transaction we are already completing. Re-acknowledge it without + // disturbing the step we are in. + return sendAckPdu(transactionParams.pduConf, FileDirective::EOF_DIRECTIVE, + transactionParams.eofConditionCode, AckTransactionStatus::ACTIVE); } return returnvalue::OK; } @@ -228,8 +371,12 @@ ReturnValue_t cfdp::DestHandler::handleMetadataParseError(ReturnValue_t result, } ReturnValue_t cfdp::DestHandler::startTransaction(const MetadataPduReader& reader) { - if (fsmRes.state != CfdpState::IDLE) { - // According to standard, discard metadata PDU if we are busy + // A metadata PDU received while busy is normally a duplicate and is discarded per the standard. + // The one exception is an acknowledged transaction which was started from a file data or EOF + // PDU because the metadata was lost: this is the retransmission our NAK of scope 0..0 asked for. + const bool lateMetadata = + fsmRes.state == CfdpState::BUSY_CLASS_2_ACKED and not transactionParams.metadataReceived; + if (fsmRes.state != CfdpState::IDLE and not lateMetadata) { return OK; } ReturnValue_t result = OK; @@ -298,18 +445,21 @@ ReturnValue_t cfdp::DestHandler::startTransaction(const MetadataPduReader& reade #endif return FAILED; } - if (reader.getTransmissionMode() == TransmissionMode::UNACKNOWLEDGED) { - fsmRes.state = CfdpState::BUSY_CLASS_1_NACKED; - } else if (reader.getTransmissionMode() == TransmissionMode::ACKNOWLEDGED) { - fsmRes.state = CfdpState::BUSY_CLASS_2_ACKED; - } - if (transactionParams.metadataOnly) { - fsmRes.step = TransactionStep::TRANSFER_COMPLETION; - } else { - // Kind of ugly, make FSM working on packet per packet basis.. - fsmRes.step = TransactionStep::TRANSACTION_START; - fsmRes.step = TransactionStep::RECEIVING_FILE_DATA_PDUS; + if (not lateMetadata) { + if (reader.getTransmissionMode() == TransmissionMode::UNACKNOWLEDGED) { + fsmRes.state = CfdpState::BUSY_CLASS_1_NACKED; + } else if (reader.getTransmissionMode() == TransmissionMode::ACKNOWLEDGED) { + fsmRes.state = CfdpState::BUSY_CLASS_2_ACKED; + } + if (transactionParams.metadataOnly) { + fsmRes.step = TransactionStep::TRANSFER_COMPLETION; + } else { + // Kind of ugly, make FSM working on packet per packet basis.. + fsmRes.step = TransactionStep::TRANSACTION_START; + fsmRes.step = TransactionStep::RECEIVING_FILE_DATA_PDUS; + } } + transactionParams.metadataReceived = true; auto& info = reader.getGenericInfo(); transactionParams.checksumType = info.getChecksumType(); transactionParams.closureRequested = info.isClosureRequested(); @@ -340,13 +490,27 @@ cfdp::CfdpState cfdp::DestHandler::getCfdpState() const { return fsmRes.state; } ReturnValue_t cfdp::DestHandler::handleTransferCompletion() { ReturnValue_t result; - if (transactionParams.checksumType != ChecksumType::NULL_CHECKSUM) { + if (transactionParams.conditionCode != ConditionCode::NO_ERROR) { + // The transaction was cancelled, for example because the NAK or check limit was reached. + // Report that condition code in the Finished PDU rather than running a checksum pass over a + // file which is known to be incomplete. + transactionParams.deliveryCode = FileDeliveryCode::DATA_INCOMPLETE; + } else if (transactionParams.checksumType != ChecksumType::NULL_CHECKSUM) { result = checksumVerification(); if (result != OK) { // TODO: Warning / error handling? } } else { transactionParams.conditionCode = ConditionCode::NO_ERROR; + if (transactionParams.metadataOnly) { + // Nothing was expected, so nothing is missing: a metadata only transaction - a proxy put + // request, say - carries no file data and completes the moment its metadata arrives. Both + // fields are otherwise left at the reset defaults, which report a successful transaction as + // "Data Incomplete" and "Discard deliberately". That contradicts the NO_ERROR condition code + // beside it, and it goes out in the Finished PDU, not just into the log. + transactionParams.deliveryCode = FileDeliveryCode::DATA_COMPLETE; + transactionParams.deliveryStatus = FileDeliveryStatus::FILE_STATUS_UNREPORTED; + } } result = noticeOfCompletion(); if (result != OK) { @@ -396,6 +560,7 @@ void cfdp::DestHandler::fileErrorHandler(Event event, ReturnValue_t result, } void cfdp::DestHandler::finish() { + destParams.lostSegmentsContainer.clear(); transactionParams.reset(); fsmRes.state = CfdpState::IDLE; fsmRes.step = TransactionStep::IDLE; @@ -482,7 +647,7 @@ ReturnValue_t cfdp::DestHandler::sendFinishedPdu() { fsfwParams.eventReporter->forwardEvent(events::SERIALIZATION_ERROR, result, 0); return result; } - pduSender.sendPdu(PduType::FILE_DIRECTIVE, FileDirective::FINISH, pduBuf.data(), serLen); + result = pduSender.sendPdu(PduType::FILE_DIRECTIVE, FileDirective::FINISH, pduBuf.data(), serLen); if (result != OK) { #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "cfdp::DestHandler::sendFinishedPdu: Sending PDU failed" << std::endl; @@ -494,6 +659,33 @@ ReturnValue_t cfdp::DestHandler::sendFinishedPdu() { return OK; } +bool cfdp::DestHandler::trySendingFinishedPdu(uint8_t& errorIdx) { + transactionParams.finishedSendAttempts++; + const ReturnValue_t result = sendFinishedPdu(); + checkAndHandleError(result, errorIdx); + if (result == OK) { + return true; + } + if (transactionParams.finishedSendAttempts < destParams.maxFinishedPduSendAttempts) { + // Leave the step where it is so the next state machine call retries the same PDU. A send + // failure here is usually a full TM store, which drains on its own. + return false; + } +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "cfdp::DestHandler: giving up on the Finished PDU after " + << transactionParams.finishedSendAttempts << " attempts" << std::endl; +#else + sif::printWarning("cfdp::DestHandler: giving up on the Finished PDU after %u attempts\n", + static_cast(transactionParams.finishedSendAttempts)); +#endif + // Only the peer's notification is lost: the file is complete on disk and the local user + // already got its indication from noticeOfCompletion(). So this is not a delivery fault, and + // releasing the handler matters more than the notification - while a transaction is held, every + // incoming metadata PDU is discarded and no new uplink can start. + finish(); + return false; +} + cfdp::DestHandler::TransactionStep cfdp::DestHandler::getTransactionStep() const { return fsmRes.step; } @@ -527,3 +719,355 @@ void cfdp::DestHandler::setEventReporter(EventReportingProxyIF& reporter) { const cfdp::DestHandlerParams& cfdp::DestHandler::getDestHandlerParams() const { return destParams; } + +bool cfdp::DestHandler::pduBelongsToTransaction(const PduPacketIF& pduPacket) const { + size_t pduSize = 0; + const auto rawPdu = pduPacket.getRawPduData(pduSize); + PduHeaderReader reader(rawPdu, pduSize); + if (reader.parseData() != OK) { + return false; + } + EntityId sourceId; + reader.getSourceId(sourceId); + TransactionSeqNum seqNum; + reader.getTransactionSeqNum(seqNum); + // Compared by value rather than with operator==, which also compares the encoded width: the + // peer is free to use a different width than we do for the same number. + return sourceId.getValue() == transactionParams.transactionId.entityId.getValue() and + seqNum.getValue() == transactionParams.transactionId.seqNum.getValue(); +} + +ReturnValue_t cfdp::DestHandler::handleAckPdu(const cfdp::PduPacketIF& info) { + size_t pduSize = 0; + const auto rawPdu = info.getRawPduData(pduSize); + AckInfo ackInfo; + AckPduReader reader(rawPdu, pduSize, ackInfo); + ReturnValue_t result = reader.parseData(); + if (result != OK) { + return result; + } + // ACKs for EOF PDUs belong to the source handler and are routed there, so the only ACK which + // can legitimately reach the destination handler is the one for our Finished PDU. + if (ackInfo.getAckedDirective() != FileDirective::FINISH) { + return OK; + } + if (not pduBelongsToTransaction(info)) { + // The CFDP handler routes ACK PDUs on the acknowledged directive alone, so a late ACK from + // an earlier transaction would otherwise finish whichever one is running now. + return OK; + } + if (fsmRes.step == TransactionStep::WAITING_FOR_FINISHED_ACK) { + finish(); + } + return OK; +} + +ReturnValue_t cfdp::DestHandler::handleSendingAckPdu() { + ReturnValue_t result = + sendAckPdu(transactionParams.pduConf, FileDirective::EOF_DIRECTIVE, + transactionParams.eofConditionCode, AckTransactionStatus::ACTIVE); + if (result != OK) { + return result; + } + if (transactionParams.eofConditionCode != ConditionCode::NO_ERROR) { + // The sender cancelled the transaction. Nothing left to request, report what we have. + transactionParams.conditionCode = transactionParams.eofConditionCode; + fsmRes.step = TransactionStep::TRANSFER_COMPLETION; + return OK; + } + if (isFileComplete()) { + fsmRes.step = TransactionStep::TRANSFER_COMPLETION; + return OK; + } + // D3: deferred lost segment procedure. The NAK sequence for everything known to be missing is + // issued once, here, and then re-issued on NAK timer expiry. + fsmRes.step = TransactionStep::WAITING_FOR_MISSING_DATA; + transactionParams.nakCounter = 0; + transactionParams.checkCounter = 0; + result = sendNakSequence(); + nakTimer.setTimeout(transactionParams.remoteCfg->nakTimerIntervalMs); + checkTimer.setTimeout(transactionParams.remoteCfg->checkTimerIntervalMs); + return result; +} + +ReturnValue_t cfdp::DestHandler::handleWaitingForMissingData() { + if (isFileComplete()) { + fsmRes.step = TransactionStep::TRANSFER_COMPLETION; + return OK; + } + if (checkTimer.hasTimedOut()) { + transactionParams.checkCounter++; + checkTimer.resetTimer(); + if (transactionParams.checkCounter > transactionParams.remoteCfg->checkLimit) { + // A7: without this an incomplete transfer pins the handler forever and no later uplink + // can start. + declareFault(ConditionCode::CHECK_LIMIT_REACHED); + fsmRes.step = TransactionStep::TRANSFER_COMPLETION; + return OK; + } + } + if (nakTimer.hasTimedOut()) { + transactionParams.nakCounter++; + nakTimer.resetTimer(); + if (transactionParams.nakCounter > transactionParams.remoteCfg->nakTimerExpirationLimit) { + declareFault(ConditionCode::NAK_LIMIT_REACHED); + fsmRes.step = TransactionStep::TRANSFER_COMPLETION; + return OK; + } + return sendNakSequence(); + } + return OK; +} + +ReturnValue_t cfdp::DestHandler::handleWaitingForFinishedAck() { + if (not positiveAckTimer.hasTimedOut()) { + return OK; + } + transactionParams.positiveAckCounter++; + positiveAckTimer.resetTimer(); + if (transactionParams.positiveAckCounter > + transactionParams.remoteCfg->positiveAckTimerExpirationLimit) { + // The sender is not acknowledging our Finished PDU. The file itself is already written, so + // report the fault and release the handler instead of holding the transaction forever. + declareFault(ConditionCode::POSITIVE_ACK_LIMIT_REACHED); + finish(); + return OK; + } + return sendFinishedPdu(); +} + +ReturnValue_t cfdp::DestHandler::startMetadatalessTransaction(const PduPacketIF& pduPacket) { + size_t pduSize = 0; + const auto rawPdu = pduPacket.getRawPduData(pduSize); + PduHeaderReader headerReader(rawPdu, pduSize); + ReturnValue_t result = headerReader.parseData(); + if (result != OK) { + return result; + } + if (headerReader.getTransmissionMode() != TransmissionMode::ACKNOWLEDGED) { + // Unacknowledged mode has no way of recovering the metadata PDU, so this stays an error. + return FAILED; + } + EntityId sourceId; + headerReader.getSourceId(sourceId); + if (not destParams.remoteCfgTable.getRemoteCfg(sourceId, &transactionParams.remoteCfg)) { + return FAILED; + } + headerReader.fillConfig(transactionParams.pduConf); + transactionParams.pduConf.crcFlag = transactionParams.remoteCfg->crcOnTransmission; + transactionParams.pduConf.direction = Direction::TOWARDS_SENDER; + transactionParams.transactionId.entityId = transactionParams.pduConf.sourceId; + transactionParams.transactionId.seqNum = transactionParams.pduConf.seqNum; + transactionParams.metadataReceived = false; + transactionParams.metadataOnly = false; + fsmRes.state = CfdpState::BUSY_CLASS_2_ACKED; + fsmRes.step = TransactionStep::RECEIVING_FILE_DATA_PDUS; +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "cfdp::DestHandler: file data PDU without metadata, requesting metadata" + << std::endl; +#else + sif::printWarning("cfdp::DestHandler: file data PDU without metadata, requesting metadata\n"); +#endif + // Nothing at all can be done before the metadata arrives, so this one NAK is not deferred. + result = sendNakSequence(); + transactionParams.nakCounter = 0; + transactionParams.checkCounter = 0; + nakTimer.setTimeout(transactionParams.remoteCfg->nakTimerIntervalMs); + checkTimer.setTimeout(transactionParams.remoteCfg->checkTimerIntervalMs); + return result; +} + +ReturnValue_t cfdp::DestHandler::ackInactiveEofPdu(const PduPacketIF& pduPacket) { + size_t pduSize = 0; + const auto rawPdu = pduPacket.getRawPduData(pduSize); + PduHeaderReader headerReader(rawPdu, pduSize); + ReturnValue_t result = headerReader.parseData(); + if (result != OK) { + return result; + } + if (headerReader.getTransmissionMode() != TransmissionMode::ACKNOWLEDGED) { + return DEST_NON_METADATA_PDU_AS_FIRST_PDU; + } + EofInfo eofInfo(&eofFaultLocation); + EofPduReader eofReader(rawPdu, pduSize, eofInfo); + result = eofReader.parseData(); + if (result != OK) { + return result; + } + PduConfig conf; + headerReader.fillConfig(conf); + conf.direction = Direction::TOWARDS_SENDER; + RemoteEntityCfg* remoteCfg = nullptr; + EntityId sourceId; + headerReader.getSourceId(sourceId); + if (destParams.remoteCfgTable.getRemoteCfg(sourceId, &remoteCfg) and remoteCfg != nullptr) { + conf.crcFlag = remoteCfg->crcOnTransmission; + } else { + conf.crcFlag = false; + } + return sendAckPdu(conf, FileDirective::EOF_DIRECTIVE, eofInfo.getConditionCode(), + AckTransactionStatus::UNRECOGNIZED); +} + +ReturnValue_t cfdp::DestHandler::sendAckPdu(PduConfig& conf, FileDirective ackedDirective, + ConditionCode conditionCode, + AckTransactionStatus status) { + // CFDP 5.2.4: the directive subtype code is 0b0001 for an acknowledged Finished PDU and + // 0b0000 for every other acknowledged directive. + AckInfo ackInfo(ackedDirective, conditionCode, status, + ackedDirective == FileDirective::FINISH ? 1 : 0); + AckPduCreator ackPdu(ackInfo, conf); + size_t serLen = 0; + ReturnValue_t result = ackPdu.serialize(pduBuf.data(), serLen, ackPdu.getSerializedSize()); + if (result != OK) { + fsfwParams.eventReporter->forwardEvent(events::SERIALIZATION_ERROR, result, 0); + return result; + } + result = pduSender.sendPdu(PduType::FILE_DIRECTIVE, FileDirective::ACK, pduBuf.data(), serLen); + if (result != OK) { + fsfwParams.eventReporter->forwardEvent(events::PDU_SEND_ERROR, result, 0); + return result; + } + fsmRes.packetsSent++; + return OK; +} + +ReturnValue_t cfdp::DestHandler::sendNakSequence() { + const bool largeFile = transactionParams.pduConf.largeFile; + const uint64_t endOfScope = transactionParams.eofReceived ? transactionParams.fileSize.value() + : transactionParams.progress; + ReturnValue_t worstResult = OK; + size_t idx = 0; + // D5: the segment requests of one NAK PDU are bounded. What does not fit is carried into the + // next PDU of the sequence instead of being dropped or declared a fault. + auto flushNak = [&]() { + if (idx == 0) { + return; + } + NakInfo nakInfo(Fss(0, largeFile), Fss(endOfScope, largeFile)); + size_t segLen = idx; + size_t maxSegLen = nakSegmentBuf.size(); + nakInfo.setSegmentRequests(nakSegmentBuf.data(), &segLen, &maxSegLen); + NakPduCreator nakPdu(transactionParams.pduConf, nakInfo); + size_t serLen = 0; + ReturnValue_t result = nakPdu.serialize(pduBuf.data(), serLen, nakPdu.getSerializedSize()); + if (result != OK) { + fsfwParams.eventReporter->forwardEvent(events::SERIALIZATION_ERROR, result, 0); + worstResult = result; + idx = 0; + return; + } + result = pduSender.sendPdu(PduType::FILE_DIRECTIVE, FileDirective::NAK, pduBuf.data(), serLen); + if (result != OK) { + fsfwParams.eventReporter->forwardEvent(events::PDU_SEND_ERROR, result, 0); + worstResult = result; + } else { + fsmRes.packetsSent++; + } + idx = 0; + }; + + if (not transactionParams.metadataReceived) { + // CFDP 5.2.6: a segment request of 0 to 0 requests the metadata PDU. + nakSegmentBuf[idx++] = {Fss(0, largeFile), Fss(0, largeFile)}; + if (idx == nakSegmentBuf.size()) { + flushNak(); + } + } + for (const auto& lostSegment : destParams.lostSegmentsContainer) { + nakSegmentBuf[idx++] = {Fss(lostSegment.first, largeFile), Fss(lostSegment.second, largeFile)}; + if (idx == nakSegmentBuf.size()) { + flushNak(); + } + } + flushNak(); + return worstResult; +} + +bool cfdp::DestHandler::isFileComplete() const { + // Class 2 completion is "no gaps left and EOF received", not the class 1 "progress reached the + // file size": a transfer can pass the announced file size with a retransmission while a gap in + // the middle is still outstanding. + return transactionParams.metadataReceived and transactionParams.eofReceived and + destParams.lostSegmentsContainer.empty() and + transactionParams.progress >= transactionParams.fileSize.value(); +} + +void cfdp::DestHandler::trackReceivedSegment(uint64_t offset, uint64_t endOfSegment) { + if (offset > transactionParams.progress) { + // Everything between the high water mark and this segment was skipped over. + insertLostSegment(transactionParams.progress, offset); + } + removeReceivedRange(offset, endOfSegment); +} + +void cfdp::DestHandler::insertLostSegment(uint64_t start, uint64_t end) { + if (end <= start) { + return; + } + auto& container = destParams.lostSegmentsContainer; + // Merge with every entry this range touches so the list cannot accumulate duplicate or + // overlapping gaps when an EOF PDU is retransmitted. + for (auto it = container.begin(); it != container.end();) { + if (it->second < start or it->first > end) { + ++it; + continue; + } + start = std::min(start, it->first); + end = std::max(end, it->second); + it = container.erase(it); + } + if (container.full()) { +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "cfdp::DestHandler: lost segment list full, gap dropped" << std::endl; +#else + sif::printWarning("cfdp::DestHandler: lost segment list full, gap dropped\n"); +#endif + return; + } + container.insert({start, end}); +} + +void cfdp::DestHandler::removeReceivedRange(uint64_t start, uint64_t end) { + if (end <= start) { + return; + } + auto& container = destParams.lostSegmentsContainer; + // A received range can span several gaps, but only the first and the last of them can be left + // with a remainder, so two pending re-insertions are always enough. + std::array, 2> pending{}; + size_t pendingLen = 0; + for (auto it = container.begin(); it != container.end();) { + if (it->second <= start or it->first >= end) { + ++it; + continue; + } + const uint64_t gapStart = it->first; + const uint64_t gapEnd = it->second; + it = container.erase(it); + if (gapStart < start and pendingLen < pending.size()) { + pending[pendingLen++] = {gapStart, start}; + } + if (gapEnd > end and pendingLen < pending.size()) { + pending[pendingLen++] = {end, gapEnd}; + } + } + for (size_t pendingIdx = 0; pendingIdx < pendingLen; pendingIdx++) { + if (container.full()) { + break; + } + container.insert(pending[pendingIdx]); + } +} + +void cfdp::DestHandler::declareFault(ConditionCode code) { + transactionParams.conditionCode = code; + transactionParams.deliveryCode = FileDeliveryCode::DATA_INCOMPLETE; + destParams.cfg.fhBase.reportFault(transactionParams.transactionId, code); +} + +size_t cfdp::DestHandler::getNumLostSegments() const { + return destParams.lostSegmentsContainer.size(); +} + +uint32_t cfdp::DestHandler::getNakCounter() const { return transactionParams.nakCounter; } diff --git a/src/fsfw/cfdp/handler/DestHandler.h b/src/fsfw/cfdp/handler/DestHandler.h index 84a989c4..57800b25 100644 --- a/src/fsfw/cfdp/handler/DestHandler.h +++ b/src/fsfw/cfdp/handler/DestHandler.h @@ -14,10 +14,14 @@ #include "fsfw/cfdp/handler/PduPacketIF.h" #include "fsfw/cfdp/handler/PduSenderIF.h" #include "fsfw/cfdp/handler/mib.h" +#include "fsfw/cfdp/pdu/HeaderReader.h" #include "fsfw/cfdp/pdu/MetadataPduReader.h" +#include "fsfw/cfdp/pdu/NakInfo.h" #include "fsfw/cfdp/pdu/PduConfig.h" +#include "fsfw/cfdp/tlv/EntityIdTlv.h" #include "fsfw/cfdp/tlv/MessageToUserTlv.h" #include "fsfw/storagemanager/StorageManagerIF.h" +#include "fsfw/timemanager/Countdown.h" #include "fsfw/tmtcservices/AcceptsTelemetryIF.h" namespace cfdp { @@ -47,6 +51,17 @@ struct DestHandlerParams { LostSegmentsListBase& lostSegmentsContainer; uint8_t maxTlvsInOnePdu = 20; size_t maxFilenameLen = 255; + //! Upper bound on the number of segment requests packed into a single NAK PDU. The remaining + //! lost segments are carried over into the next NAK of the sequence, so this bounds the PDU + //! size without bounding what can be requested. Each segment request is 8 bytes for a small + //! file and 16 bytes for a large one. + size_t maxSegmentRequestsPerNakPdu = 20; + //! Attempts at sending the Finished PDU before the transaction is released without it. The + //! send can fail on a transient downstream condition (a full TM store), and treating that as + //! sent loses the peer's only notification that the transfer completed. The retry has to be + //! bounded though: a busy destination handler discards incoming metadata PDUs, so holding a + //! transaction forever means no later uplink can start. + uint32_t maxFinishedPduSendAttempts = 5; }; class DestHandler { @@ -57,7 +72,13 @@ class DestHandler { RECEIVING_FILE_DATA_PDUS = 2, SENDING_ACK_PDU = 3, TRANSFER_COMPLETION = 4, - SENDING_FINISHED_PDU = 5 + SENDING_FINISHED_PDU = 5, + //! Class 2 only: the deferred lost segment procedure is running. NAK PDUs have been issued + //! for the known gaps and the handler is waiting for the retransmissions. + WAITING_FOR_MISSING_DATA = 6, + //! Class 2 only: the Finished PDU was sent and its ACK is outstanding. The transaction is + //! retained until the ACK arrives or the positive ACK limit is reached. + WAITING_FOR_FINISHED_ACK = 7 }; struct FsmResult { @@ -105,6 +126,10 @@ class DestHandler { ReturnValue_t sendKeepAlivePdu(); [[nodiscard]] const TransactionId& getTransactionId() const; [[nodiscard]] const DestHandlerParams& getDestHandlerParams() const; + //! Number of gaps currently tracked by the deferred lost segment procedure. Always 0 in class 1. + [[nodiscard]] size_t getNumLostSegments() const; + //! Number of NAK sequences issued for the current transaction. + [[nodiscard]] uint32_t getNakCounter() const; private: struct TransactionParams { @@ -128,8 +153,29 @@ class DestHandler { closureRequested = false; vfsErrorCount = 0; checksumType = ChecksumType::NULL_CHECKSUM; + metadataReceived = false; + eofReceived = false; + eofConditionCode = ConditionCode::NO_ERROR; + positiveAckCounter = 0; + nakCounter = 0; + checkCounter = 0; + finishedSendAttempts = 0; } + //! Attempts made at sending the Finished PDU, see + //! DestHandlerParams::maxFinishedPduSendAttempts. Used by both transmission modes. + uint32_t finishedSendAttempts = 0; + + //! Class 2 only: false while the transaction was started from a file data or EOF PDU because + //! the metadata PDU was lost. The metadata is then requested with a NAK of scope 0..0. + bool metadataReceived = false; + bool eofReceived = false; + //! Condition code of the received EOF PDU, needed to build the matching ACK PDU. + ConditionCode eofConditionCode = ConditionCode::NO_ERROR; + uint32_t positiveAckCounter = 0; + uint32_t nakCounter = 0; + uint32_t checkCounter = 0; + bool metadataOnly = false; ChecksumType checksumType = ChecksumType::NULL_CHECKSUM; bool closureRequested = false; @@ -155,6 +201,20 @@ class DestHandler { DestHandlerParams destParams; cfdp::FsfwParams fsfwParams; FsmResult fsmRes; + //! Scratch space for the segment requests of one NAK PDU. Sized from + //! DestHandlerParams::maxSegmentRequestsPerNakPdu. + std::vector nakSegmentBuf; + //! Guards the Finished PDU, see RemoteEntityCfg::positiveAckTimerIntervalMs. + Countdown positiveAckTimer; + //! Drives the deferred lost segment procedure, see RemoteEntityCfg::nakTimerIntervalMs. + Countdown nakTimer; + //! Guards an incomplete transaction after EOF reception, see RemoteEntityCfg::checkLimit. + Countdown checkTimer; + //! Receives the fault location of an incoming EOF PDU. EofPduReader refuses to parse any EOF + //! whose condition code is not NO_ERROR unless it has somewhere to put that TLV, so without + //! these every Cancel EOF the sender emits would be dropped as unparseable. + cfdp::EntityId eofFaultLocationId; + EntityIdTlv eofFaultLocation{eofFaultLocationId}; ReturnValue_t startTransaction(const MetadataPduReader& reader); ReturnValue_t handleMetadataPdu(const PduPacketIF& pduPacket); @@ -162,9 +222,37 @@ class DestHandler { ReturnValue_t handleEofPdu(const PduPacketIF& info); ReturnValue_t handleMetadataParseError(ReturnValue_t result, const uint8_t* rawData, size_t maxSize); + ReturnValue_t handleAckPdu(const PduPacketIF& info); + //! True if the PDU's source entity ID and sequence number match the running transaction. The + //! CFDP handler routes PDUs by direction only, so this is the only transaction level filter. + [[nodiscard]] bool pduBelongsToTransaction(const PduPacketIF& pduPacket) const; ReturnValue_t handleTransferCompletion(); + //! Class 2 substates, driven from stateMachine(). + void fsmAcked(std::optional> optPduPacket, uint8_t& errorIdx); + ReturnValue_t handleSendingAckPdu(); + ReturnValue_t handleWaitingForMissingData(); + ReturnValue_t handleWaitingForFinishedAck(); + ReturnValue_t startMetadatalessTransaction(const PduPacketIF& pduPacket); + ReturnValue_t ackInactiveEofPdu(const PduPacketIF& pduPacket); + ReturnValue_t sendAckPdu(PduConfig& conf, FileDirective ackedDirective, + ConditionCode conditionCode, AckTransactionStatus status); + ReturnValue_t sendNakSequence(); + [[nodiscard]] bool isFileComplete() const; + void trackReceivedSegment(uint64_t offset, uint64_t endOfSegment); + void insertLostSegment(uint64_t start, uint64_t end); + void removeReceivedRange(uint64_t start, uint64_t end); + void declareFault(ConditionCode code); ReturnValue_t tryBuildingAbsoluteDestName(size_t destNameSize); ReturnValue_t sendFinishedPdu(); + /** + * Sends the Finished PDU, retrying a bounded number of times on a failed send. + * + * @return True if the PDU went out and the caller should advance. False means the caller must + * leave the step alone and do nothing else this iteration: either the send is being + * retried on the next call, or the attempt budget ran out and the transaction was + * already released with finish(). + */ + bool trySendingFinishedPdu(uint8_t& errorIdx); ReturnValue_t noticeOfCompletion(); ReturnValue_t checksumVerification(); void fileErrorHandler(Event event, ReturnValue_t result, const char* info) const; diff --git a/src/fsfw/cfdp/handler/PutRequest.cpp b/src/fsfw/cfdp/handler/PutRequest.cpp index a1abcc14..c197afb7 100644 --- a/src/fsfw/cfdp/handler/PutRequest.cpp +++ b/src/fsfw/cfdp/handler/PutRequest.cpp @@ -14,7 +14,10 @@ cfdp::PutRequest::PutRequest(cfdp::EntityId destId, const uint8_t *msgsToUser, cfdp::PutRequest::PutRequest(cfdp::EntityId destId, cfdp::StringLv &sourceName, cfdp::StringLv &destName) - : destId(std::move(destId)), sourceName(std::move(sourceName)), destName(std::move(destName)) {} + : destId(std::move(destId)), + metadataOnly(false), + sourceName(std::move(sourceName)), + destName(std::move(destName)) {} [[nodiscard]] bool cfdp::PutRequest::isMetadataOnly() const { return metadataOnly; } diff --git a/src/fsfw/cfdp/handler/SourceHandler.cpp b/src/fsfw/cfdp/handler/SourceHandler.cpp index 1198110c..cf914e05 100644 --- a/src/fsfw/cfdp/handler/SourceHandler.cpp +++ b/src/fsfw/cfdp/handler/SourceHandler.cpp @@ -4,9 +4,15 @@ #include +#include "fsfw/cfdp/pdu/AckPduCreator.h" +#include "fsfw/cfdp/pdu/AckPduReader.h" #include "fsfw/cfdp/pdu/EofPduCreator.h" #include "fsfw/cfdp/pdu/FileDataCreator.h" +#include "fsfw/cfdp/pdu/FileDirectiveReader.h" +#include "fsfw/cfdp/pdu/FinishedPduReader.h" +#include "fsfw/cfdp/pdu/HeaderReader.h" #include "fsfw/cfdp/pdu/MetadataPduCreator.h" +#include "fsfw/cfdp/pdu/NakPduReader.h" #include "fsfw/filesystem/HasFileSystemIF.h" #include "fsfw/globalfunctions/arrayprinter.h" #include "fsfw/objectmanager.h" @@ -16,12 +22,28 @@ using namespace returnvalue; +namespace { + +//! True if the PDU is long enough to hold the Finished PDU fields which are mandatory, i.e. the +//! directive byte plus the byte carrying the condition code, delivery code and file status. +//! Anything shorter cannot be trusted even partially. +bool mandatoryFinishedFieldsPresent(const uint8_t* rawPdu, size_t pduSize) { + FileDirectiveReader directiveReader(rawPdu, pduSize); + if (directiveReader.parseData() != returnvalue::OK) { + return false; + } + return directiveReader.getWholePduSize() > directiveReader.getHeaderSize(); +} + +} // namespace + cfdp::SourceHandler::SourceHandler(PduSenderIF& pduSender, size_t pduBufferSize, SourceHandlerParams params, FsfwParams fsfwParams) : pduSender(pduSender), pduBuf(pduBufferSize), sourceParams(std::move(params)), - fsfwParams(fsfwParams) { + fsfwParams(fsfwParams), + positiveAckTimer(0, false) { // The entity ID portion of the transaction ID will always remain fixed. transactionParams.id.entityId = sourceParams.cfg.localId; transactionParams.pduConf.sourceId = sourceParams.cfg.localId; @@ -47,8 +69,19 @@ cfdp::SourceHandler::SourceHandler(PduSenderIF& pduSender, size_t pduBufferSize, transactionParams.pduConf.seqNum.setValue(0); } -cfdp::SourceHandler::FsmResult& cfdp::SourceHandler::fsmNacked() { +cfdp::SourceHandler::FsmResult& cfdp::SourceHandler::fsmNacked( + const std::optional> optPduPacket) { ReturnValue_t result; + if (optPduPacket.has_value() and step == TransactionStep::WAIT_FOR_FINISH) { + PduPacketIF& pduPacket = *optPduPacket; + if (pduPacket.getPduType() == FILE_DIRECTIVE and *pduPacket.getFileDirective() == FINISH and + pduBelongsToTransaction(pduPacket)) { + result = handleFinishedPdu(pduPacket); + if (result != OK) { + addError(result); + } + } + } if (step == TransactionStep::IDLE) { step = TransactionStep::TRANSACTION_START; } @@ -86,6 +119,11 @@ cfdp::SourceHandler::FsmResult& cfdp::SourceHandler::fsmNacked() { } if (transactionParams.closureRequested) { step = TransactionStep::WAIT_FOR_FINISH; + // The Finished PDU is now actually waited for. Bound the wait with the positive ACK timer + // configuration so a lost Finished PDU cannot pin the handler: without closure the + // transaction completed immediately, so a hang here would be a regression. + transactionParams.positiveAckCounter = 0; + positiveAckTimer.setTimeout(transactionParams.remoteCfg.positiveAckTimerIntervalMs); // fsmResult.callStatus = CallStatus::CALL_AFTER_DELAY; } else { step = TransactionStep::NOTICE_OF_COMPLETION; @@ -94,7 +132,20 @@ cfdp::SourceHandler::FsmResult& cfdp::SourceHandler::fsmNacked() { return fsmResult; } if (step == TransactionStep::WAIT_FOR_FINISH) { - // TODO: In case this is a request with closure, wait for finish. + if (not transactionParams.finishedReceived) { + if (not positiveAckTimer.hasTimedOut()) { + return fsmResult; + } + transactionParams.positiveAckCounter++; + positiveAckTimer.resetTimer(); + if (transactionParams.positiveAckCounter <= + transactionParams.remoteCfg.positiveAckTimerExpirationLimit) { + return fsmResult; + } + // Unacknowledged mode has no retransmission, so the only thing left is to report that the + // peer never confirmed the transfer. + declareFault(ConditionCode::INACTIVITY_DETECTED); + } // Done, issue notice of completion step = TransactionStep::NOTICE_OF_COMPLETION; } @@ -117,7 +168,10 @@ const cfdp::SourceHandler::FsmResult& cfdp::SourceHandler::stateMachine( return fsmResult; } if (state == cfdp::CfdpState::BUSY_CLASS_1_NACKED) { - return fsmNacked(); + return fsmNacked(optPduPacket); + } + if (state == cfdp::CfdpState::BUSY_CLASS_2_ACKED) { + return fsmAcked(optPduPacket); } return fsmResult; } @@ -210,7 +264,10 @@ ReturnValue_t cfdp::SourceHandler::transactionStart(PutRequest& putRequest, Remo state = cfdp::CfdpState::BUSY_CLASS_2_ACKED; } else if (transactionParams.pduConf.mode == TransmissionMode::UNACKNOWLEDGED) { state = cfdp::CfdpState::BUSY_CLASS_1_NACKED; + } else { + return TRANSMISSION_MODE_NOT_SUPPORTED; } + retransmitState.reset(); step = TransactionStep::IDLE; uint64_t fileSize = 0; sourceParams.user.vfs.getFileSize(transactionParams.sourceName.data(), fileSize); @@ -230,15 +287,7 @@ ReturnValue_t cfdp::SourceHandler::transactionStart(PutRequest& putRequest, Remo } ReturnValue_t cfdp::SourceHandler::prepareAndSendMetadataPdu() { - cfdp::StringLv sourceName(transactionParams.sourceName.data(), transactionParams.sourceNameSize); - cfdp::StringLv destName(transactionParams.destName.data(), transactionParams.destNameSize); - auto metadataInfo = - MetadataGenericInfo(transactionParams.closureRequested, transactionParams.checksumType, - transactionParams.fileSize); - auto metadataPdu = - MetadataPduCreator(transactionParams.pduConf, metadataInfo, sourceName, destName, nullptr, 0); - ReturnValue_t result = - sendGenericPdu(PduType::FILE_DIRECTIVE, FileDirective::METADATA, metadataPdu); + ReturnValue_t result = sendMetadataPdu(); if (result != OK) { return result; } @@ -247,8 +296,18 @@ ReturnValue_t cfdp::SourceHandler::prepareAndSendMetadataPdu() { return OK; } +ReturnValue_t cfdp::SourceHandler::sendMetadataPdu() { + cfdp::StringLv sourceName(transactionParams.sourceName.data(), transactionParams.sourceNameSize); + cfdp::StringLv destName(transactionParams.destName.data(), transactionParams.destNameSize); + auto metadataInfo = + MetadataGenericInfo(transactionParams.closureRequested, transactionParams.checksumType, + transactionParams.fileSize); + auto metadataPdu = + MetadataPduCreator(transactionParams.pduConf, metadataInfo, sourceName, destName, nullptr, 0); + return sendGenericPdu(PduType::FILE_DIRECTIVE, FileDirective::METADATA, metadataPdu); +} + ReturnValue_t cfdp::SourceHandler::prepareAndSendNextFileDataPdu(bool& noFileDataPdu) { - cfdp::Fss offset(transactionParams.progress); uint64_t lenToRead; uint64_t fileSize = transactionParams.fileSize.value(); noFileDataPdu = false; @@ -267,19 +326,7 @@ ReturnValue_t cfdp::SourceHandler::prepareAndSendNextFileDataPdu(bool& noFileDat lenToRead = transactionParams.remoteCfg.maxFileSegmentLen; } } - FileOpParams fileParams(transactionParams.sourceName.data(), lenToRead); - fileParams.offset = transactionParams.progress; - size_t readLen = 0; - ReturnValue_t result = sourceParams.user.vfs.readFromFile( - transactionParams.sourceName.data(), transactionParams.progress, lenToRead, fileBuf.data(), - readLen, fileBuf.size()); - if (result != returnvalue::OK) { - addError(result); - return result; - } - auto fileDataInfo = FileDataInfo(offset, fileBuf.data(), lenToRead); - auto fileDataPdu = FileDataCreator(transactionParams.pduConf, fileDataInfo); - result = sendGenericPdu(PduType::FILE_DATA, std::nullopt, fileDataPdu); + ReturnValue_t result = sendFileDataPdu(transactionParams.progress, lenToRead); if (result != OK) { return result; } @@ -319,20 +366,23 @@ ReturnValue_t cfdp::SourceHandler::sendGenericPdu(PduType pduType, addError(result); return result; } - pduSender.sendPdu(pduType, fileDirective, pduBuf.data(), serializedLen); + result = pduSender.sendPdu(pduType, fileDirective, pduBuf.data(), serializedLen); + if (result != OK) { + addError(result); + return result; + } fsmResult.packetsSent += 1; - return result; + return OK; } ReturnValue_t cfdp::SourceHandler::noticeOfCompletion() { if (sourceParams.cfg.indicCfg.transactionFinishedIndicRequired) { - // TODO: This could still be improved by caching the Finished PDU parameters. - FileDeliveryStatus deliveryStatus = FileDeliveryStatus::FILE_STATUS_UNREPORTED; - if (transactionParams.closureRequested) { - deliveryStatus = FileDeliveryStatus::RETAINED_IN_FILESTORE; - } - cfdp::TransactionFinishedParams params(transactionParams.id, ConditionCode::NO_ERROR, - FileDeliveryCode::DATA_COMPLETE, deliveryStatus); + // The values reported by the peer's Finished PDU, if one was received. Reporting a hardcoded + // NO_ERROR / DATA_COMPLETE here meant a transfer the receiver rejected still looked + // successful on this side. + cfdp::TransactionFinishedParams params( + transactionParams.id, transactionParams.finishedConditionCode, + transactionParams.finishedDeliveryCode, transactionParams.finishedDeliveryStatus); sourceParams.user.transactionFinishedIndication(params); } return OK; @@ -341,6 +391,7 @@ ReturnValue_t cfdp::SourceHandler::noticeOfCompletion() { ReturnValue_t cfdp::SourceHandler::reset() { step = TransactionStep::IDLE; state = cfdp::CfdpState::IDLE; + retransmitState.reset(); // fsmResult.callStatus = CallStatus::DONE; transactionParams.reset(); return OK; @@ -356,3 +407,328 @@ void cfdp::SourceHandler::addError(ReturnValue_t error) { fsmResult.result = error; } } + +cfdp::SourceHandler::FsmResult& cfdp::SourceHandler::fsmAcked( + const std::optional> optPduPacket) { + ReturnValue_t result; + if (optPduPacket.has_value()) { + handleAckedPdu(*optPduPacket); + } + if (step == TransactionStep::IDLE) { + step = TransactionStep::TRANSACTION_START; + } + if (step == TransactionStep::TRANSACTION_START) { + sourceParams.user.transactionIndication(transactionParams.id); + result = checksumGeneration(); + if (result != OK) { + addError(result); + } + step = TransactionStep::SENDING_METADATA; + } + if (step == TransactionStep::SENDING_METADATA) { + result = prepareAndSendMetadataPdu(); + if (result != OK) { + addError(result); + } + return fsmResult; + } + if (step == TransactionStep::SENDING_FILE_DATA) { + bool noFdPdu = false; + result = prepareAndSendNextFileDataPdu(noFdPdu); + if (result == OK and !noFdPdu) { + return fsmResult; + } + } + if (step == TransactionStep::SENDING_EOF) { + result = prepareAndSendEofPdu(); + if (result != OK) { + addError(result); + } + if (sourceParams.cfg.indicCfg.eofSentIndicRequired) { + sourceParams.user.eofSentIndication(transactionParams.id); + } + // The EOF PDU is acknowledged in class 2. Closure is meaningless here (D8 of the class 2 + // plan): the Finished PDU is mandatory, so the transaction always runs to WAIT_FOR_FINISH. + transactionParams.positiveAckCounter = 0; + positiveAckTimer.setTimeout(transactionParams.remoteCfg.positiveAckTimerIntervalMs); + step = TransactionStep::WAIT_FOR_ACK; + return fsmResult; + } + // Retransmissions requested by the peer take priority over any timer work: a source which + // cannot answer a NAK deadlocks the transfer. One PDU per call keeps the burst bounded in + // exactly the same way the regular file data phase is. + if (servicePendingRetransmissions(result)) { + if (result != OK) { + addError(result); + } + return fsmResult; + } + if (step == TransactionStep::WAIT_FOR_ACK) { + if (not positiveAckTimer.hasTimedOut()) { + return fsmResult; + } + transactionParams.positiveAckCounter++; + positiveAckTimer.resetTimer(); + if (transactionParams.positiveAckCounter > + transactionParams.remoteCfg.positiveAckTimerExpirationLimit) { + declareFault(ConditionCode::POSITIVE_ACK_LIMIT_REACHED); + noticeOfCompletion(); + reset(); + return fsmResult; + } + result = prepareAndSendEofPdu(); + if (result != OK) { + addError(result); + } + return fsmResult; + } + if (step == TransactionStep::WAIT_FOR_FINISH) { + if (not transactionParams.finishedReceived) { + if (not positiveAckTimer.hasTimedOut()) { + return fsmResult; + } + transactionParams.positiveAckCounter++; + positiveAckTimer.resetTimer(); + if (transactionParams.positiveAckCounter <= + transactionParams.remoteCfg.positiveAckTimerExpirationLimit) { + return fsmResult; + } + // The receiver retransmits its Finished PDU on its own positive ACK timer, so reaching + // this point means the downlink is gone rather than a single PDU being lost. + declareFault(ConditionCode::INACTIVITY_DETECTED); + } + step = TransactionStep::NOTICE_OF_COMPLETION; + } + if (step == TransactionStep::NOTICE_OF_COMPLETION) { + noticeOfCompletion(); + reset(); + } + return fsmResult; +} + +bool cfdp::SourceHandler::pduBelongsToTransaction(const PduPacketIF& pduPacket) const { + size_t pduSize = 0; + const auto rawPdu = pduPacket.getRawPduData(pduSize); + PduHeaderReader reader(rawPdu, pduSize); + if (reader.parseData() != OK) { + return false; + } + EntityId sourceId; + reader.getSourceId(sourceId); + TransactionSeqNum seqNum; + reader.getTransactionSeqNum(seqNum); + // Compared by value rather than with operator==, which also compares the encoded width: the + // peer is free to use a different width than we do for the same number. + return sourceId.getValue() == transactionParams.id.entityId.getValue() and + seqNum.getValue() == transactionParams.id.seqNum.getValue(); +} + +void cfdp::SourceHandler::handleAckedPdu(PduPacketIF& pduPacket) { + if (pduPacket.getPduType() != FILE_DIRECTIVE) { + return; + } + if (not pduBelongsToTransaction(pduPacket)) { + // Nothing upstream filters by transaction: the CFDP handler routes on the PDU direction + // alone. A late ACK or Finished PDU from an earlier transaction would otherwise drive + // whichever transaction is running now. + return; + } + ReturnValue_t result = OK; + switch (*pduPacket.getFileDirective()) { + case (FileDirective::ACK): { + result = handleAckPdu(pduPacket); + break; + } + case (FileDirective::NAK): { + result = handleNakPdu(pduPacket); + break; + } + case (FileDirective::FINISH): { + result = handleFinishedPdu(pduPacket); + break; + } + default: + // Keep Alive PDUs carry progress information only, there is nothing to drive from them. + break; + } + if (result != OK) { + addError(result); + } +} + +ReturnValue_t cfdp::SourceHandler::handleAckPdu(const PduPacketIF& pduPacket) { + size_t pduSize = 0; + const auto rawPdu = pduPacket.getRawPduData(pduSize); + AckInfo ackInfo; + AckPduReader reader(rawPdu, pduSize, ackInfo); + ReturnValue_t result = reader.parseData(); + if (result != OK) { + return result; + } + // ACKs for Finished PDUs are routed to the destination handler, so only the EOF ACK can + // legitimately arrive here. + if (ackInfo.getAckedDirective() != FileDirective::EOF_DIRECTIVE) { + return OK; + } + if (step == TransactionStep::WAIT_FOR_ACK) { + step = TransactionStep::WAIT_FOR_FINISH; + // Re-arm the same timer as an inactivity guard for the Finished PDU. + transactionParams.positiveAckCounter = 0; + positiveAckTimer.setTimeout(transactionParams.remoteCfg.positiveAckTimerIntervalMs); + } + return OK; +} + +ReturnValue_t cfdp::SourceHandler::handleFinishedPdu(const PduPacketIF& pduPacket) { + size_t pduSize = 0; + const auto rawPdu = pduPacket.getRawPduData(pduSize); + FinishedInfo finishedInfo; + FinishPduReader reader(rawPdu, pduSize, finishedInfo); + // The condition code, delivery code and file status are parsed before any TLV, so they are + // usable even if this handler cannot hold the optional filestore responses. + ReturnValue_t result = reader.parseData(); + if (result != OK and not mandatoryFinishedFieldsPresent(rawPdu, pduSize)) { + // The PDU is truncated before those fields, so it says nothing at all about how the transfer + // went. Completing the transaction on it would report the default constructed DATA_COMPLETE, + // i.e. a fabricated success, and would discard the state the peer's retransmission needs. + return result; + } + transactionParams.finishedReceived = true; + transactionParams.finishedConditionCode = finishedInfo.getConditionCode(); + transactionParams.finishedDeliveryCode = finishedInfo.getDeliveryCode(); + transactionParams.finishedDeliveryStatus = finishedInfo.getFileStatus(); + if (state == CfdpState::BUSY_CLASS_2_ACKED) { + // The Finished PDU is acknowledged in class 2. This has to happen even for a duplicate, + // because a duplicate means our previous ACK was lost. + ReturnValue_t ackResult = + sendAckPdu(FileDirective::FINISH, transactionParams.finishedConditionCode); + if (ackResult != OK) { + return ackResult; + } + } + if (result != OK) { + // Only the optional TLVs failed to parse, the delivery result above is still valid. + return OK; + } + return OK; +} + +ReturnValue_t cfdp::SourceHandler::handleNakPdu(const PduPacketIF& pduPacket) { + size_t pduSize = 0; + const auto rawPdu = pduPacket.getRawPduData(pduSize); + NakInfo nakInfo(Fss(0), Fss(0)); + size_t maxSegments = retransmitState.segments.size(); + size_t segmentLen = 0; + nakInfo.setSegmentRequests(retransmitState.segments.data(), &segmentLen, &maxSegments); + // The reader writes straight into the segment array, so the previous NAK's bookkeeping is + // invalid the moment parsing starts, whether or not it succeeds. Drop it up front rather than + // leaving indices pointing into a half overwritten array on an error return. + retransmitState.reset(); + NakPduReader reader(rawPdu, pduSize, nakInfo); + ReturnValue_t result = reader.parseData(); + // The segment requests may have been truncated, but whatever was parsed completely into the + // array is still worth retransmitting: the receiver re-NAKs what it does not get. + retransmitState.numSegments = nakInfo.getSegmentRequestsLen(); + retransmitState.currentIdx = 0; + retransmitState.cursor = 0; + retransmitState.metadataPending = false; + // CFDP 5.2.6: a segment request of 0 to 0 asks for the metadata PDU rather than file data. + // Compact it out of the list so the file data path does not have to special case it. + size_t writeIdx = 0; + for (size_t readIdx = 0; readIdx < retransmitState.numSegments; readIdx++) { + const auto& segment = retransmitState.segments[readIdx]; + if (segment.first.value() == 0 and segment.second.value() == 0) { + retransmitState.metadataPending = true; + continue; + } + retransmitState.segments[writeIdx++] = segment; + } + retransmitState.numSegments = writeIdx; + // Reported so a truncated NAK is visible as an error even though the requests which did parse + // are serviced normally. + return result; +} + +bool cfdp::SourceHandler::servicePendingRetransmissions(ReturnValue_t& result) { + result = OK; + if (retransmitState.metadataPending) { + result = sendMetadataPdu(); + if (result == OK) { + // Only clear the request once it actually went out. The peer asked for the metadata + // because it cannot write the file at all without it, and it will not ask again until its + // NAK timer expires. + retransmitState.metadataPending = false; + } + return true; + } + const uint64_t fileSize = transactionParams.fileSize.value(); + while (retransmitState.currentIdx < retransmitState.numSegments) { + const auto& segment = retransmitState.segments[retransmitState.currentIdx]; + uint64_t start = segment.first.value(); + uint64_t end = segment.second.value(); + if (retransmitState.cursor > start) { + start = retransmitState.cursor; + } + if (end > fileSize) { + end = fileSize; + } + if (start >= end) { + retransmitState.currentIdx++; + retransmitState.cursor = 0; + continue; + } + uint64_t lenToSend = end - start; + if (lenToSend > transactionParams.remoteCfg.maxFileSegmentLen) { + lenToSend = transactionParams.remoteCfg.maxFileSegmentLen; + } + result = sendFileDataPdu(start, lenToSend); + if (result != OK) { + // The cursor must not move past data which was never enqueued for downlink, exactly as in + // the forward-only path: the next call retries this same offset. Advancing here would drop + // the segment until the peer's NAK timer re-requests it, and that costs a NAK limit credit. + return true; + } + retransmitState.cursor = start + lenToSend; + if (retransmitState.cursor >= end) { + retransmitState.currentIdx++; + retransmitState.cursor = 0; + } + return true; + } + return false; +} + +ReturnValue_t cfdp::SourceHandler::sendFileDataPdu(uint64_t offset, size_t lenToRead) { + if (lenToRead > fileBuf.size()) { + addError(FILE_SEGMENT_LEN_INVALID); + return FILE_SEGMENT_LEN_INVALID; + } + size_t readLen = 0; + ReturnValue_t result = + sourceParams.user.vfs.readFromFile(transactionParams.sourceName.data(), offset, lenToRead, + fileBuf.data(), readLen, fileBuf.size()); + if (result != returnvalue::OK) { + addError(result); + return result; + } + cfdp::Fss offsetFss(offset, transactionParams.pduConf.largeFile); + auto fileDataInfo = FileDataInfo(offsetFss, fileBuf.data(), lenToRead); + auto fileDataPdu = FileDataCreator(transactionParams.pduConf, fileDataInfo); + return sendGenericPdu(PduType::FILE_DATA, std::nullopt, fileDataPdu); +} + +ReturnValue_t cfdp::SourceHandler::sendAckPdu(FileDirective ackedDirective, + ConditionCode conditionCode) { + // CFDP 5.2.4: the directive subtype code is 0b0001 for an acknowledged Finished PDU and + // 0b0000 for every other acknowledged directive. + AckInfo ackInfo(ackedDirective, conditionCode, AckTransactionStatus::ACTIVE, + ackedDirective == FileDirective::FINISH ? 1 : 0); + AckPduCreator ackPdu(ackInfo, transactionParams.pduConf); + return sendGenericPdu(PduType::FILE_DIRECTIVE, FileDirective::ACK, ackPdu); +} + +void cfdp::SourceHandler::declareFault(ConditionCode code) { + transactionParams.finishedConditionCode = code; + transactionParams.finishedDeliveryCode = FileDeliveryCode::DATA_INCOMPLETE; + sourceParams.cfg.fhBase.reportFault(transactionParams.id, code); +} diff --git a/src/fsfw/cfdp/handler/SourceHandler.h b/src/fsfw/cfdp/handler/SourceHandler.h index 1f05bc1d..eb5265c0 100644 --- a/src/fsfw/cfdp/handler/SourceHandler.h +++ b/src/fsfw/cfdp/handler/SourceHandler.h @@ -1,6 +1,7 @@ #ifndef FSFW_CFDP_CFDPSOURCEHANDLER_H #define FSFW_CFDP_CFDPSOURCEHANDLER_H +#include #include #include @@ -11,8 +12,10 @@ #include "fsfw/cfdp/Fss.h" #include "fsfw/cfdp/handler/PutRequest.h" #include "fsfw/cfdp/handler/mib.h" +#include "fsfw/cfdp/pdu/NakInfo.h" #include "fsfw/events/EventReportingProxyIF.h" #include "fsfw/storagemanager/StorageManagerIF.h" +#include "fsfw/timemanager/Countdown.h" #include "fsfw/tmtcservices/AcceptsTelemetryIF.h" #include "fsfw/util/ProvidesSeqCountIF.h" @@ -81,15 +84,52 @@ class SourceHandler { PduConfig pduConf; cfdp::TransactionId id{}; + //! Number of positive ACK timer expirations for the EOF PDU, or of inactivity timer + //! expirations while waiting for the Finished PDU. + uint32_t positiveAckCounter = 0; + bool finishedReceived = false; + //! Delivery result reported by the peer in its Finished PDU. The defaults are what a + //! transfer without closure reports, which is what the handler did unconditionally before + //! the Finished PDU was actually parsed. + ConditionCode finishedConditionCode = ConditionCode::NO_ERROR; + FileDeliveryCode finishedDeliveryCode = FileDeliveryCode::DATA_COMPLETE; + FileDeliveryStatus finishedDeliveryStatus = FileDeliveryStatus::FILE_STATUS_UNREPORTED; + void reset() { sourceNameSize = 0; destNameSize = 0; fileSize.setFileSize(0, false); progress = 0; closureRequested = false; + positiveAckCounter = 0; + finishedReceived = false; + finishedConditionCode = ConditionCode::NO_ERROR; + finishedDeliveryCode = FileDeliveryCode::DATA_COMPLETE; + finishedDeliveryStatus = FileDeliveryStatus::FILE_STATUS_UNREPORTED; } } transactionParams; + //! Pending retransmissions requested by the last received NAK PDU. Only the most recent NAK is + //! kept: it is the peer's authoritative statement about what is still missing, and bounding the + //! state this way keeps a NAK storm from growing the handler's memory footprint. + struct RetransmitState { + static constexpr size_t MAX_SEGMENTS = 32; + std::array segments{}; + size_t numSegments = 0; + size_t currentIdx = 0; + //! Absolute file offset reached inside the segment at currentIdx, 0 if it was not started. + uint64_t cursor = 0; + bool metadataPending = false; + + void reset() { + numSegments = 0; + currentIdx = 0; + cursor = 0; + metadataPending = false; + } + [[nodiscard]] bool empty() const { return not metadataPending and currentIdx >= numSegments; } + } retransmitState; + PduSenderIF& pduSender; std::vector pduBuf; cfdp::CfdpState state = cfdp::CfdpState::IDLE; @@ -98,13 +138,29 @@ class SourceHandler { SourceHandlerParams sourceParams; cfdp::FsfwParams fsfwParams; FsmResult fsmResult; + //! Guards the EOF PDU in acknowledged mode and the wait for the Finished PDU in both modes. + Countdown positiveAckTimer; - FsmResult& fsmNacked(); + FsmResult& fsmNacked(std::optional> optPduPacket); + FsmResult& fsmAcked(std::optional> optPduPacket); + //! True if the PDU's source entity ID and sequence number match the running transaction. The + //! CFDP handler routes PDUs by direction only, so this is the only transaction level filter. + [[nodiscard]] bool pduBelongsToTransaction(const PduPacketIF& pduPacket) const; + void handleAckedPdu(PduPacketIF& pduPacket); + ReturnValue_t handleFinishedPdu(const PduPacketIF& pduPacket); + ReturnValue_t handleAckPdu(const PduPacketIF& pduPacket); + ReturnValue_t handleNakPdu(const PduPacketIF& pduPacket); + //! Sends one PDU of the outstanding retransmissions, if there are any. + bool servicePendingRetransmissions(ReturnValue_t& result); + ReturnValue_t sendAckPdu(FileDirective ackedDirective, ConditionCode conditionCode); ReturnValue_t checksumGeneration(); + ReturnValue_t sendMetadataPdu(); ReturnValue_t prepareAndSendMetadataPdu(); + ReturnValue_t sendFileDataPdu(uint64_t offset, size_t lenToRead); ReturnValue_t prepareAndSendNextFileDataPdu(bool& noFileDataPdu); ReturnValue_t prepareAndSendEofPdu(); ReturnValue_t noticeOfCompletion(); + void declareFault(ConditionCode code); ReturnValue_t reset(); [[nodiscard]] ReturnValue_t sendGenericPdu(PduType pduType, diff --git a/src/fsfw/cfdp/handler/defs.h b/src/fsfw/cfdp/handler/defs.h index e1d88247..c09d732b 100644 --- a/src/fsfw/cfdp/handler/defs.h +++ b/src/fsfw/cfdp/handler/defs.h @@ -38,4 +38,6 @@ static constexpr ReturnValue_t TARGET_MSG_QUEUE_FULL = returnvalue::makeCode(CID static constexpr ReturnValue_t TM_STORE_FULL = returnvalue::makeCode(CID, 7); static constexpr ReturnValue_t DEST_NON_METADATA_PDU_AS_FIRST_PDU = returnvalue::makeCode(CID, 8); static constexpr ReturnValue_t PDU_BUFFER_TOO_SMALL = returnvalue::makeCode(CID, 9); +//! The resolved transmission mode of a request is not supported by this handler (yet). +static constexpr ReturnValue_t TRANSMISSION_MODE_NOT_SUPPORTED = returnvalue::makeCode(CID, 10); } // namespace cfdp \ No newline at end of file diff --git a/src/fsfw/cfdp/handler/mib.h b/src/fsfw/cfdp/handler/mib.h index 7e4454ef..1263c183 100644 --- a/src/fsfw/cfdp/handler/mib.h +++ b/src/fsfw/cfdp/handler/mib.h @@ -36,6 +36,31 @@ struct RemoteEntityCfg { TransmissionMode defaultTransmissionMode = TransmissionMode::UNACKNOWLEDGED; ChecksumType defaultChecksum = ChecksumType::NULL_CHECKSUM; uint8_t version = CFDP_VERSION_2; + + // Acknowledged mode (class 2) parameters. The names mirror cfdppy.mib.RemoteEntityConfig field + // for field so both ends of a link can be configured from the same set of numbers. The defaults + // are inert for class 1, which uses none of them. + + //! Interval of the positive acknowledgment timer, which guards EOF (source side) and Finished + //! (destination side) PDUs. Must be larger than the peer's worst case time to answer with the + //! matching ACK PDU, or both sides retransmit over each other. + uint32_t positiveAckTimerIntervalMs = 10000; + //! Number of positive ACK timer expirations after which POSITIVE_ACK_LIMIT_REACHED is declared. + uint32_t positiveAckTimerExpirationLimit = 2; + //! Interval of the NAK timer used by the deferred lost segment procedure. + uint32_t nakTimerIntervalMs = 10000; + //! Number of NAK timer expirations after which NAK_LIMIT_REACHED is declared. + uint32_t nakTimerExpirationLimit = 2; + //! If true, a NAK is issued as soon as a gap is detected. If false, the deferred procedure is + //! used and the NAK sequence is only issued once the EOF PDU has arrived. Deferred is the + //! default: on a lossy link the immediate procedure produces a burst of NAK PDUs in the + //! opposite direction exactly when the link is already struggling. + bool immediateNakMode = false; + //! Number of check timer expirations after EOF reception after which CHECK_LIMIT_REACHED is + //! declared and an incomplete transaction is cancelled instead of pinning the handler. + uint32_t checkLimit = 2; + //! Interval of the check timer, see checkLimit. + uint32_t checkTimerIntervalMs = 10000; }; } // namespace cfdp diff --git a/src/fsfw/cfdp/pdu/AckPduCreator.h b/src/fsfw/cfdp/pdu/AckPduCreator.h index c0a89cc3..9cc64f6b 100644 --- a/src/fsfw/cfdp/pdu/AckPduCreator.h +++ b/src/fsfw/cfdp/pdu/AckPduCreator.h @@ -23,6 +23,9 @@ class AckPduCreator : public FileDirectiveCreator { ReturnValue_t serialize(uint8_t** buffer, size_t* size, size_t maxSize, Endianness streamEndianness) const override; + //! Un-hide the convenience overloads of the base class, same as FinishedPduCreator does. + using FileDirectiveCreator::serialize; + private: AckInfo& ackInfo; }; diff --git a/src/fsfw/cfdp/pdu/FinishedPduReader.cpp b/src/fsfw/cfdp/pdu/FinishedPduReader.cpp index 08dd3a82..1785e57d 100644 --- a/src/fsfw/cfdp/pdu/FinishedPduReader.cpp +++ b/src/fsfw/cfdp/pdu/FinishedPduReader.cpp @@ -11,6 +11,15 @@ ReturnValue_t FinishPduReader::parseData() { size_t currentIdx = FileDirectiveReader::getHeaderSize(); const uint8_t* buf = pointers.rawPtr + currentIdx; size_t remSize = FileDirectiveReader::getWholePduSize() - currentIdx; + // Drop the PDU CRC from the parsed range before parseTlvs below, which runs until the range is + // exhausted. Without this a Finished PDU carrying a CRC is rejected as an invalid TLV type - + // including the common NO_ERROR case, where the CRC is the only thing left after the first byte. + if (getCrcFlag()) { + if (remSize < 2) { + return SerializeIF::STREAM_TOO_SHORT; + } + remSize -= 2; + } if (remSize < 1) { return SerializeIF::STREAM_TOO_SHORT; } diff --git a/src/fsfw/cfdp/pdu/MetadataPduReader.cpp b/src/fsfw/cfdp/pdu/MetadataPduReader.cpp index aaf1e08b..484eb5e6 100644 --- a/src/fsfw/cfdp/pdu/MetadataPduReader.cpp +++ b/src/fsfw/cfdp/pdu/MetadataPduReader.cpp @@ -17,6 +17,15 @@ ReturnValue_t MetadataPduReader::parseData() { size_t currentIdx = FileDirectiveReader::getHeaderSize(); const uint8_t* buf = pointers.rawPtr + currentIdx; size_t remSize = FileDirectiveReader::getWholePduSize() - currentIdx; + // The PDU CRC occupies the last two bytes of the PDU. Take it out of the parsed range up front, + // like FileDataReader does: the option loop below consumes bytes until the range is exhausted, + // so a CRC left in place would be deserialized as another TLV and rejected as an invalid type. + if (getCrcFlag()) { + if (remSize < 2) { + return SerializeIF::STREAM_TOO_SHORT; + } + remSize -= 2; + } if (remSize < 1) { return SerializeIF::STREAM_TOO_SHORT; } @@ -38,10 +47,6 @@ ReturnValue_t MetadataPduReader::parseData() { return result; } - if (getCrcFlag() && remSize == 2) { - return returnvalue::OK; - } - if (remSize > 0) { if (optionArrayMaxSize == 0 or optionArray == nullptr) { return cfdp::METADATA_CANT_PARSE_OPTIONS; diff --git a/src/fsfw/cfdp/pdu/NakPduCreator.h b/src/fsfw/cfdp/pdu/NakPduCreator.h index 601078fc..b31edb8d 100644 --- a/src/fsfw/cfdp/pdu/NakPduCreator.h +++ b/src/fsfw/cfdp/pdu/NakPduCreator.h @@ -25,6 +25,9 @@ class NakPduCreator : public FileDirectiveCreator { ReturnValue_t serialize(uint8_t** buffer, size_t* size, size_t maxSize, Endianness streamEndianness) const override; + //! Un-hide the convenience overloads of the base class, same as FinishedPduCreator does. + using FileDirectiveCreator::serialize; + /** * If you change the info struct, you might need to update the directive field length * manually diff --git a/src/fsfw/cfdp/pdu/NakPduReader.cpp b/src/fsfw/cfdp/pdu/NakPduReader.cpp index c67774c0..0746588b 100644 --- a/src/fsfw/cfdp/pdu/NakPduReader.cpp +++ b/src/fsfw/cfdp/pdu/NakPduReader.cpp @@ -11,6 +11,14 @@ ReturnValue_t NakPduReader::parseData() { size_t currentIdx = FileDirectiveReader::getHeaderSize(); const uint8_t* buffer = pointers.rawPtr + currentIdx; size_t remSize = FileDirectiveReader::getWholePduSize() - currentIdx; + // Drop the PDU CRC from the parsed range before the segment request loop below, which runs + // until the range is exhausted and would otherwise read the CRC as a truncated segment request. + if (getCrcFlag()) { + if (remSize < 2) { + return SerializeIF::STREAM_TOO_SHORT; + } + remSize -= 2; + } if (remSize < 1) { return SerializeIF::STREAM_TOO_SHORT; } @@ -34,17 +42,24 @@ ReturnValue_t NakPduReader::parseData() { if (segReqs != nullptr) { size_t idx = 0; while (remSize > 0) { + // Every early return below reports the number of *complete* segment requests written so + // far. Leaving the length at 0 would make a partially parsed NAK indistinguishable from + // an empty one, and a caller which tolerates the error code would then act on nothing. if (idx == maxSegReqs) { + nakInfo.setSegmentRequestLen(idx); return cfdp::NAK_CANT_PARSE_OPTIONS; } result = segReqs[idx].first.deSerialize(&buffer, &remSize, SerializeIF::Endianness::NETWORK); if (result != returnvalue::OK) { + nakInfo.setSegmentRequestLen(idx); return result; } result = segReqs[idx].second.deSerialize(&buffer, &remSize, SerializeIF::Endianness::NETWORK); if (result != returnvalue::OK) { + // The entry at idx is half written, so it is not counted. + nakInfo.setSegmentRequestLen(idx); return result; } idx++; diff --git a/src/fsfw/globalfunctions/arrayprinter.cpp b/src/fsfw/globalfunctions/arrayprinter.cpp index 40dc09f5..e984046f 100644 --- a/src/fsfw/globalfunctions/arrayprinter.cpp +++ b/src/fsfw/globalfunctions/arrayprinter.cpp @@ -56,26 +56,33 @@ void arrayprinter::printHex(const uint8_t *data, size_t size, size_t maxCharPerL std::cout << std::dec << std::setfill(' '); std::cout << "]" << std::endl; #else - // General format: 0x01, 0x02, 0x03 so it is number of chars times 6 - // plus line break plus small safety margin. - char printBuffer[(size + 1) * 7 + 1] = {}; +#if FSFW_DISABLE_PRINTOUT == 0 + // Emitted in fixed size chunks. This used to size one buffer from the input - a variable length + // array of (size + 1) * 7 + 1 bytes on the stack - which is unusable for the sizes this is + // actually called with: dumping a 12 KB receive buffer asks for 84 KB of stack, and overflowed + // an 8 KB task on the iOBC as soon as a frame parse error made it dump one. The output is + // unchanged, it is just flushed as it is built. + constexpr size_t CHUNK_LEN = 128; + // An entry appends at most two hex digits, a separator and a line break. + constexpr size_t MAX_ENTRY_LEN = 4; + char printBuffer[CHUNK_LEN] = {}; size_t currentPos = 0; + printf("hex ["); for (size_t i = 0; i < size; i++) { - // To avoid buffer overflows. - if (sizeof(printBuffer) - currentPos <= 7) { - break; + if (currentPos + MAX_ENTRY_LEN >= CHUNK_LEN) { + printf("%s", printBuffer); + printBuffer[0] = '\0'; + currentPos = 0; } - - currentPos += snprintf(printBuffer + currentPos, 6, "%02x", data[i]); + currentPos += snprintf(printBuffer + currentPos, CHUNK_LEN - currentPos, "%02x", data[i]); if (i < size - 1) { - currentPos += sprintf(printBuffer + currentPos, ","); + currentPos += snprintf(printBuffer + currentPos, CHUNK_LEN - currentPos, ","); if ((i + 1) % maxCharPerLine == 0) { - currentPos += sprintf(printBuffer + currentPos, "\n"); + currentPos += snprintf(printBuffer + currentPos, CHUNK_LEN - currentPos, "\n"); } } } -#if FSFW_DISABLE_PRINTOUT == 0 - printf("hex [%s]\n", printBuffer); + printf("%s]\n", printBuffer); #endif /* FSFW_DISABLE_PRINTOUT == 0 */ #endif } @@ -98,27 +105,30 @@ void arrayprinter::printDec(const uint8_t *data, size_t size, size_t maxCharPerL } std::cout << "]" << std::endl; #else - // General format: 32,243,-12 so it is number of chars times 4 - // plus line break plus small safety margin. - uint16_t expectedLines = ceil((double)size / maxCharPerLine); - char printBuffer[size * 4 + 1 + expectedLines] = {}; +#if FSFW_DISABLE_PRINTOUT == 0 + // Chunked for the same reason as printHex above: the buffer used to be a variable length array + // sized from the input. + constexpr size_t CHUNK_LEN = 128; + // An entry appends at most three digits, a separator and a line break. + constexpr size_t MAX_ENTRY_LEN = 5; + char printBuffer[CHUNK_LEN] = {}; size_t currentPos = 0; + printf("dec ["); for (size_t i = 0; i < size; i++) { - // To avoid buffer overflows. - if (sizeof(printBuffer) - currentPos <= 4) { - break; + if (currentPos + MAX_ENTRY_LEN >= CHUNK_LEN) { + printf("%s", printBuffer); + printBuffer[0] = '\0'; + currentPos = 0; } - - currentPos += snprintf(printBuffer + currentPos, 4, "%d", data[i]); + currentPos += snprintf(printBuffer + currentPos, CHUNK_LEN - currentPos, "%d", data[i]); if (i < size - 1) { - currentPos += sprintf(printBuffer + currentPos, ","); + currentPos += snprintf(printBuffer + currentPos, CHUNK_LEN - currentPos, ","); if ((i + 1) % maxCharPerLine == 0) { - currentPos += sprintf(printBuffer + currentPos, "\n"); + currentPos += snprintf(printBuffer + currentPos, CHUNK_LEN - currentPos, "\n"); } } } -#if FSFW_DISABLE_PRINTOUT == 0 - printf("dec [%s]\n", printBuffer); + printf("%s]\n", printBuffer); #endif /* FSFW_DISABLE_PRINTOUT == 0 */ #endif } diff --git a/unittests/cfdp/PduSenderMock.h b/unittests/cfdp/PduSenderMock.h index e2d1b5a9..c1017e25 100644 --- a/unittests/cfdp/PduSenderMock.h +++ b/unittests/cfdp/PduSenderMock.h @@ -15,6 +15,12 @@ class PduSenderMock : public cfdp::PduSenderIF { ReturnValue_t sendPdu(cfdp::PduType pduType, std::optional fileDirective, const uint8_t* pdu, size_t pduSize) override { + const size_t callIdx = totalSendCalls++; + if (failNextSend or (failSendAtIdx.has_value() and *failSendAtIdx == callIdx) or + (failSendsFromIdx.has_value() and callIdx >= *failSendsFromIdx)) { + failNextSend = false; + return FAILED_SEND_RESULT; + } SentPdu sentPdu; sentPdu.pduType = pduType; sentPdu.fileDirective = fileDirective; @@ -32,5 +38,18 @@ class PduSenderMock : public cfdp::PduSenderIF { return nextPdu; } + // Simulates a downstream send failure (e.g. a full TM store), consumed by the next sendPdu() + // call. Used to verify a caller does not treat a failed send as if the PDU went out. + static constexpr ReturnValue_t FAILED_SEND_RESULT = returnvalue::FAILED; + bool failNextSend = false; + + // Fails the sendPdu() call at this 0-based index instead of the next one. Needed when the + // state machine call under test emits several PDUs and only a later one must fail. + std::optional failSendAtIdx = std::nullopt; + // Fails every sendPdu() call from this 0-based index on, i.e. a downstream which stays broken + // (a TM store which never drains because the downlink itself is stalled). + std::optional failSendsFromIdx = std::nullopt; + size_t totalSendCalls = 0; + std::deque sentPdus; }; \ No newline at end of file diff --git a/unittests/cfdp/handler/testDestHandler.cpp b/unittests/cfdp/handler/testDestHandler.cpp index ccdf6fed..4678fffd 100644 --- a/unittests/cfdp/handler/testDestHandler.cpp +++ b/unittests/cfdp/handler/testDestHandler.cpp @@ -1,15 +1,20 @@ #include #include +#include #include +#include #include #include "OwnedPduPacket.h" #include "cfdp/PduSenderMock.h" #include "fsfw/cfdp.h" +#include "fsfw/cfdp/pdu/AckPduCreator.h" +#include "fsfw/cfdp/pdu/AckPduReader.h" #include "fsfw/cfdp/pdu/EofPduCreator.h" #include "fsfw/cfdp/pdu/FileDataCreator.h" #include "fsfw/cfdp/pdu/MetadataPduCreator.h" +#include "fsfw/cfdp/pdu/NakPduReader.h" #include "mock/AcceptsTmMock.h" #include "mock/EventReportingProxyMock.h" #include "mock/FilesystemMock.h" @@ -46,12 +51,13 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") { PduConfig conf; auto destHandler = DestHandler(senderMock, 4096, dp, fp); - auto metadataPreparation = [&](Fss cfdpFileSize, ChecksumType checksumType) { + auto metadataPreparation = [&](Fss cfdpFileSize, ChecksumType checksumType, + bool closureRequested = false) { const std::string srcNameString = "hello.txt"; const std::string destNameString = "hello-cpy.txt"; StringLv srcName(srcNameString); StringLv destName(destNameString); - MetadataGenericInfo info(false, checksumType, std::move(cfdpFileSize)); + MetadataGenericInfo info(closureRequested, checksumType, std::move(cfdpFileSize)); const TransactionSeqNum seqNum(UnsignedByteField(1)); conf.sourceId = remoteId; conf.destId = localId; @@ -126,6 +132,36 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") { CHECK(destHandler.getTransactionStep() == DestHandler::TransactionStep::IDLE); } + SECTION("Metadata only transfer reports a complete delivery") { + // A proxy put request is metadata only: empty file names, the request itself in a message to + // user, and no file data to wait for - so the transaction completes the moment the metadata + // lands. The delivery fields used to be left at their reset defaults on this path, reporting + // a successful transaction as "Data Incomplete" and "Discard deliberately" next to a NO_ERROR + // condition code, in the Finished PDU as well as in the log. + StringLv emptySrcName; + StringLv emptyDestName; + MetadataGenericInfo info(false, ChecksumType::NULL_CHECKSUM, Fss(0)); + const TransactionSeqNum seqNum(UnsignedByteField(1)); + conf.sourceId = remoteId; + conf.destId = localId; + conf.mode = TransmissionMode::UNACKNOWLEDGED; + conf.seqNum = seqNum; + const MetadataPduCreator creator(conf, info, emptySrcName, emptyDestName, nullptr, 0); + REQUIRE(creator.serialize(pduBuf.data(), serLen, creator.getSerializedSize()) == OK); + auto packet = + OwnedPduPacket(creator.getPduType(), creator.getDirectiveCode(), pduBuf.data(), serLen); + + destHandler.stateMachine(packet); + destHandler.stateMachineNoPacket(); + + REQUIRE(userMock.finishedRecvd.size() == 1); + const auto& finished = userMock.finishedRecvd.back().second; + CHECK(finished.condCode == ConditionCode::NO_ERROR); + CHECK(finished.deliveryCode == FileDeliveryCode::DATA_COMPLETE); + CHECK(finished.status == FileDeliveryStatus::FILE_STATUS_UNREPORTED); + CHECK(destHandler.getCfdpState() == CfdpState::IDLE); + } + SECTION("Empty File Transfer") { const DestHandler::FsmResult& res = destHandler.stateMachineNoPacket(); CHECK(res.result == OK); @@ -219,4 +255,412 @@ TEST_CASE("CFDP Dest Handler", "[cfdp]") { destHandler.stateMachine(eofPacket); eofCheck(res, transactionId); } -} \ No newline at end of file + + SECTION("A failed Finished PDU send is retried, then the transaction is released") { + // Class 1 has no ACK for the Finished PDU, so a failed send used to be indistinguishable + // from a successful one: the transaction was finished either way and the sender never heard + // that a transfer which actually succeeded had completed. + std::string fileData = "hello test data"; + etl::crc32 crcCalc; + crcCalc.add(fileData.begin(), fileData.end()); + Fss cfdpFileSize(fileData.size()); + auto metadataPacket = metadataPreparation(cfdpFileSize, ChecksumType::CRC_32, true); + const DestHandler::FsmResult& res = destHandler.stateMachine(metadataPacket); + Fss offset(0); + FileDataInfo fdPduInfo(offset, reinterpret_cast(fileData.data()), + fileData.size()); + FileDataCreator fdPduCreator(conf, fdPduInfo); + REQUIRE(fdPduCreator.serialize(pduBuf.data(), serLen, fdPduCreator.getSerializedSize()) == OK); + OwnedPduPacket fdPdu(fdPduCreator.getPduType(), std::nullopt, pduBuf.data(), serLen); + destHandler.stateMachine(fdPdu); + + // The Finished PDU send fails on the first attempt only. + senderMock.failNextSend = true; + auto eofPacket = eofPreparation(cfdpFileSize, crcCalc.value()); + destHandler.stateMachine(eofPacket); + CHECK(not senderMock.getNextSentPacket().has_value()); + REQUIRE(res.step == DestHandler::TransactionStep::SENDING_FINISHED_PDU); + + // The retry succeeds and only then is the transaction released. + destHandler.stateMachineNoPacket(); + auto optPacket = senderMock.getNextSentPacket(); + REQUIRE(optPacket.has_value()); + REQUIRE(optPacket->fileDirective.has_value()); + CHECK(*optPacket->fileDirective == FileDirective::FINISH); + CHECK(destHandler.getCfdpState() == CfdpState::IDLE); + } + + SECTION("A Finished PDU which can never be sent still releases the handler") { + // The retry has to be bounded. While a transaction is held the handler refuses every new + // metadata PDU, so retrying forever on a downstream which stays broken would mean no uplink + // can ever start again. + std::string fileData = "hello test data"; + etl::crc32 crcCalc; + crcCalc.add(fileData.begin(), fileData.end()); + Fss cfdpFileSize(fileData.size()); + auto metadataPacket = metadataPreparation(cfdpFileSize, ChecksumType::CRC_32, true); + destHandler.stateMachine(metadataPacket); + Fss offset(0); + FileDataInfo fdPduInfo(offset, reinterpret_cast(fileData.data()), + fileData.size()); + FileDataCreator fdPduCreator(conf, fdPduInfo); + REQUIRE(fdPduCreator.serialize(pduBuf.data(), serLen, fdPduCreator.getSerializedSize()) == OK); + OwnedPduPacket fdPdu(fdPduCreator.getPduType(), std::nullopt, pduBuf.data(), serLen); + destHandler.stateMachine(fdPdu); + + senderMock.failSendsFromIdx = 0; + auto eofPacket = eofPreparation(cfdpFileSize, crcCalc.value()); + destHandler.stateMachine(eofPacket); + for (int idx = 0; idx < 100; idx++) { + destHandler.stateMachineNoPacket(); + } + CHECK(destHandler.getCfdpState() == CfdpState::IDLE); + CHECK(destHandler.getTransactionStep() == DestHandler::TransactionStep::IDLE); + } +} +TEST_CASE("CFDP Dest Handler Acknowledged", "[cfdp]") { + using namespace cfdp; + using namespace returnvalue; + auto localId = EntityId(UnsignedByteField(2)); + auto remoteId = EntityId(UnsignedByteField(3)); + FaultHandlerMock fhMock; + LocalEntityCfg localEntityCfg(localId, IndicationCfg(), fhMock); + FilesystemMock fsMock; + UserMock userMock(fsMock); + RemoteConfigTableMock remoteCfgTableMock; + LostSegmentsList<128> lostSegmentsList; + DestHandlerParams dp(localEntityCfg, userMock, remoteCfgTableMock, lostSegmentsList); + EventReportingProxyMock eventReporterMock; + PduSenderMock senderMock; + FsfwParams fp(&eventReporterMock); + RemoteEntityCfg cfg(remoteId); + // Keep the timers short so the timeout paths are testable without stalling the suite. + cfg.positiveAckTimerIntervalMs = 1; + cfg.positiveAckTimerExpirationLimit = 2; + cfg.nakTimerIntervalMs = 1; + cfg.nakTimerExpirationLimit = 2; + cfg.checkTimerIntervalMs = 100000; + cfg.checkLimit = 2; + remoteCfgTableMock.addRemoteConfig(cfg); + std::array pduBuf{}; + size_t serLen = 0; + PduConfig conf; + auto destHandler = DestHandler(senderMock, 4096, dp, fp); + + const std::string srcNameString = "hello.txt"; + const std::string destNameString = "hello-cpy.txt"; + const TransactionSeqNum seqNum(UnsignedByteField(1)); + conf.sourceId = remoteId; + conf.destId = localId; + conf.mode = TransmissionMode::ACKNOWLEDGED; + conf.seqNum = seqNum; + + std::array fileData{}; + for (size_t idx = 0; idx < fileData.size(); idx++) { + fileData[idx] = static_cast(idx); + } + etl::crc32 crcCalc; + crcCalc.add(fileData.begin(), fileData.end()); + const uint32_t crc32 = crcCalc.value(); + + auto makeMetadataPdu = [&]() { + StringLv srcName(srcNameString); + StringLv destName(destNameString); + MetadataGenericInfo info(false, ChecksumType::CRC_32, Fss(fileData.size())); + const MetadataPduCreator creator(conf, info, srcName, destName, nullptr, 0); + REQUIRE(creator.serialize(pduBuf.data(), serLen, creator.getSerializedSize()) == OK); + return OwnedPduPacket(creator.getPduType(), creator.getDirectiveCode(), pduBuf.data(), serLen); + }; + auto makeFileDataPdu = [&](uint64_t offset, size_t len) { + Fss offsetFss(offset); + FileDataInfo info(offsetFss, fileData.data() + offset, len); + FileDataCreator creator(conf, info); + REQUIRE(creator.serialize(pduBuf.data(), serLen, creator.getSerializedSize()) == OK); + return OwnedPduPacket(creator.getPduType(), std::nullopt, pduBuf.data(), serLen); + }; + auto makeEofPdu = [&]() { + EofInfo info(ConditionCode::NO_ERROR, crc32, Fss(fileData.size())); + EofPduCreator creator(conf, info); + REQUIRE(creator.serialize(pduBuf.data(), serLen, creator.getSerializedSize()) == OK); + return OwnedPduPacket(creator.getPduType(), creator.getDirectiveCode(), pduBuf.data(), serLen); + }; + auto makeFinishedAckPdu = [&]() { + AckInfo info(FileDirective::FINISH, ConditionCode::NO_ERROR, AckTransactionStatus::ACTIVE, 1); + PduConfig ackConf = conf; + AckPduCreator creator(info, ackConf); + REQUIRE(creator.serialize(pduBuf.data(), serLen, creator.getSerializedSize()) == OK); + return OwnedPduPacket(creator.getPduType(), creator.getDirectiveCode(), pduBuf.data(), serLen); + }; + // Pops the next sent PDU and checks it is the expected directive. + auto expectDirective = [&](FileDirective expected) { + auto optPacket = senderMock.getNextSentPacket(); + REQUIRE(optPacket.has_value()); + REQUIRE(optPacket->pduType == PduType::FILE_DIRECTIVE); + REQUIRE(optPacket->fileDirective.has_value()); + REQUIRE(*optPacket->fileDirective == expected); + return *optPacket; + }; + auto parseNak = [&](const SentPdu& pdu, std::vector>& segments) { + NakInfo info(Fss(0), Fss(0)); + std::array segBuf{}; + size_t segLen = 0; + size_t maxSegLen = segBuf.size(); + info.setSegmentRequests(segBuf.data(), &segLen, &maxSegLen); + NakPduReader reader(pdu.rawPdu.data(), pdu.rawPdu.size(), info); + REQUIRE(reader.parseData() == OK); + segments.clear(); + for (size_t idx = 0; idx < info.getSegmentRequestsLen(); idx++) { + segments.emplace_back(segBuf[idx].first.value(), segBuf[idx].second.value()); + } + }; + + SECTION("Nominal acknowledged transfer") { + auto metadataPdu = makeMetadataPdu(); + const DestHandler::FsmResult& res = destHandler.stateMachine(metadataPdu); + REQUIRE(res.state == CfdpState::BUSY_CLASS_2_ACKED); + REQUIRE(res.step == DestHandler::TransactionStep::RECEIVING_FILE_DATA_PDUS); + auto fdPdu = makeFileDataPdu(0, fileData.size()); + destHandler.stateMachine(fdPdu); + REQUIRE(destHandler.getNumLostSegments() == 0); + auto eofPdu = makeEofPdu(); + destHandler.stateMachine(eofPdu); + // D2: the ACK for the EOF PDU is emitted before the Finished PDU, and before the checksum + // verification which produced it. + expectDirective(FileDirective::ACK); + expectDirective(FileDirective::FINISH); + // The transaction is retained until the Finished PDU is acknowledged. + REQUIRE(res.state == CfdpState::BUSY_CLASS_2_ACKED); + REQUIRE(res.step == DestHandler::TransactionStep::WAITING_FOR_FINISHED_ACK); + REQUIRE(userMock.finishedRecvd.size() == 1); + CHECK(userMock.finishedRecvd.back().second.condCode == ConditionCode::NO_ERROR); + auto ackPdu = makeFinishedAckPdu(); + destHandler.stateMachine(ackPdu); + CHECK(destHandler.getCfdpState() == CfdpState::IDLE); + CHECK(destHandler.getTransactionStep() == DestHandler::TransactionStep::IDLE); + } + + SECTION("Cancel EOF is parsed, acknowledged and ends the transaction") { + // A Cancel EOF carries a condition code other than NO_ERROR and a fault location TLV. The + // reader refuses to parse one unless it is given somewhere to put that TLV, so this used to + // fail with "Ca not deserialize fault location" and the whole PDU was dropped: the sender + // never got its ACK and retransmitted to its positive ACK limit, while this handler held the + // transaction open until its own check limit expired. + auto metadataPdu = makeMetadataPdu(); + const DestHandler::FsmResult& res = destHandler.stateMachine(metadataPdu); + REQUIRE(res.state == CfdpState::BUSY_CLASS_2_ACKED); + // Only part of the file arrives before the sender gives up on it. + auto fdPdu = makeFileDataPdu(0, 10); + destHandler.stateMachine(fdPdu); + + EntityId faultLocId(UnsignedByteField(2)); + EntityIdTlv faultLoc(faultLocId); + EofInfo cancelInfo(ConditionCode::CANCEL_REQUEST_RECEIVED, 0, Fss(fileData.size()), &faultLoc); + EofPduCreator cancelCreator(conf, cancelInfo); + REQUIRE(cancelCreator.serialize(pduBuf.data(), serLen, cancelCreator.getSerializedSize()) == + OK); + auto cancelEof = OwnedPduPacket(cancelCreator.getPduType(), cancelCreator.getDirectiveCode(), + pduBuf.data(), serLen); + destHandler.stateMachine(cancelEof); + + // The cancellation is acknowledged rather than ignored, and it is reported with the condition + // code the sender gave instead of a checksum failure over the partial file. + auto ackPdu = expectDirective(FileDirective::ACK); + AckInfo ackInfo; + AckPduReader ackReader(ackPdu.rawPdu.data(), ackPdu.rawPdu.size(), ackInfo); + REQUIRE(ackReader.parseData() == OK); + CHECK(ackInfo.getAckedDirective() == FileDirective::EOF_DIRECTIVE); + CHECK(ackInfo.getAckedConditionCode() == ConditionCode::CANCEL_REQUEST_RECEIVED); + expectDirective(FileDirective::FINISH); + REQUIRE(userMock.finishedRecvd.size() == 1); + CHECK(userMock.finishedRecvd.back().second.condCode == ConditionCode::CANCEL_REQUEST_RECEIVED); + CHECK(userMock.finishedRecvd.back().second.deliveryCode == FileDeliveryCode::DATA_INCOMPLETE); + } + + SECTION("Dropped file data PDU is recovered via NAK") { + auto metadataPdu = makeMetadataPdu(); + const DestHandler::FsmResult& res = destHandler.stateMachine(metadataPdu); + // The segment from 256 to 512 is dropped on the way. + auto firstPdu = makeFileDataPdu(0, 256); + destHandler.stateMachine(firstPdu); + auto thirdPdu = makeFileDataPdu(512, 512); + destHandler.stateMachine(thirdPdu); + REQUIRE(destHandler.getNumLostSegments() == 1); + auto eofPdu = makeEofPdu(); + destHandler.stateMachine(eofPdu); + expectDirective(FileDirective::ACK); + auto nakPdu = expectDirective(FileDirective::NAK); + std::vector> segments; + parseNak(nakPdu, segments); + REQUIRE(segments.size() == 1); + CHECK(segments[0].first == 256); + CHECK(segments[0].second == 512); + REQUIRE(res.step == DestHandler::TransactionStep::WAITING_FOR_MISSING_DATA); + // The retransmission closes the gap and the transfer completes. + auto retransmit = makeFileDataPdu(256, 256); + destHandler.stateMachine(retransmit); + CHECK(destHandler.getNumLostSegments() == 0); + expectDirective(FileDirective::FINISH); + REQUIRE(res.step == DestHandler::TransactionStep::WAITING_FOR_FINISHED_ACK); + REQUIRE(userMock.finishedRecvd.size() == 1); + CHECK(userMock.finishedRecvd.back().second.condCode == ConditionCode::NO_ERROR); + CHECK(userMock.finishedRecvd.back().second.deliveryCode == FileDeliveryCode::DATA_COMPLETE); + } + + SECTION("Dropped metadata PDU is requested with a scope 0 to 0 NAK") { + // The first PDU the destination sees is a file data PDU, which in class 2 starts the + // transaction instead of being rejected. + auto fdPdu = makeFileDataPdu(0, 256); + const DestHandler::FsmResult& res = destHandler.stateMachine(fdPdu); + REQUIRE(res.result == OK); + REQUIRE(res.state == CfdpState::BUSY_CLASS_2_ACKED); + auto nakPdu = expectDirective(FileDirective::NAK); + std::vector> segments; + parseNak(nakPdu, segments); + REQUIRE(segments.size() == 1); + CHECK(segments[0].first == 0); + CHECK(segments[0].second == 0); + // Nothing was written, there is no destination file name yet. + CHECK(fsMock.fileMap.find(destNameString) == fsMock.fileMap.end()); + // The metadata retransmission completes the transaction setup. + auto metadataPdu = makeMetadataPdu(); + destHandler.stateMachine(metadataPdu); + CHECK(fsMock.fileMap.find(destNameString) != fsMock.fileMap.end()); + auto allData = makeFileDataPdu(0, fileData.size()); + destHandler.stateMachine(allData); + auto eofPdu = makeEofPdu(); + destHandler.stateMachine(eofPdu); + expectDirective(FileDirective::ACK); + expectDirective(FileDirective::FINISH); + CHECK(userMock.finishedRecvd.back().second.condCode == ConditionCode::NO_ERROR); + } + + SECTION("Finished PDU is retransmitted on positive ACK timeout") { + auto metadataPdu = makeMetadataPdu(); + destHandler.stateMachine(metadataPdu); + auto fdPdu = makeFileDataPdu(0, fileData.size()); + destHandler.stateMachine(fdPdu); + auto eofPdu = makeEofPdu(); + const DestHandler::FsmResult& res = destHandler.stateMachine(eofPdu); + expectDirective(FileDirective::ACK); + expectDirective(FileDirective::FINISH); + REQUIRE(res.step == DestHandler::TransactionStep::WAITING_FOR_FINISHED_ACK); + // Two expirations are tolerated, each retransmits the Finished PDU. + for (uint32_t idx = 0; idx < cfg.positiveAckTimerExpirationLimit; idx++) { + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + destHandler.stateMachineNoPacket(); + expectDirective(FileDirective::FINISH); + CHECK(destHandler.getCfdpState() == CfdpState::BUSY_CLASS_2_ACKED); + } + // The next one reaches the limit and releases the handler. + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + destHandler.stateMachineNoPacket(); + CHECK(destHandler.getCfdpState() == CfdpState::IDLE); + auto& fhInfo = fhMock.getFhInfo(FaultHandlerCode::IGNORE_ERROR); + REQUIRE(fhInfo.callCount == 1); + CHECK(fhInfo.condCodes.front() == ConditionCode::POSITIVE_ACK_LIMIT_REACHED); + } + + SECTION("NAK limit reached cancels the transaction") { + auto metadataPdu = makeMetadataPdu(); + destHandler.stateMachine(metadataPdu); + auto firstPdu = makeFileDataPdu(0, 256); + destHandler.stateMachine(firstPdu); + auto eofPdu = makeEofPdu(); + const DestHandler::FsmResult& res = destHandler.stateMachine(eofPdu); + expectDirective(FileDirective::ACK); + expectDirective(FileDirective::NAK); + REQUIRE(res.step == DestHandler::TransactionStep::WAITING_FOR_MISSING_DATA); + for (uint32_t idx = 0; idx < cfg.nakTimerExpirationLimit; idx++) { + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + destHandler.stateMachineNoPacket(); + expectDirective(FileDirective::NAK); + } + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + destHandler.stateMachineNoPacket(); + // The transaction is cancelled and the failure is reported in the Finished PDU rather than + // pinning the handler. + expectDirective(FileDirective::FINISH); + REQUIRE(userMock.finishedRecvd.size() == 1); + CHECK(userMock.finishedRecvd.back().second.condCode == ConditionCode::NAK_LIMIT_REACHED); + CHECK(userMock.finishedRecvd.back().second.deliveryCode == FileDeliveryCode::DATA_INCOMPLETE); + auto& fhInfo = fhMock.getFhInfo(FaultHandlerCode::IGNORE_ERROR); + REQUIRE(fhInfo.callCount == 1); + CHECK(fhInfo.condCodes.front() == ConditionCode::NAK_LIMIT_REACHED); + } + + SECTION("EOF PDU for an inactive transaction is acknowledged") { + // D7: the sender retransmitted an EOF after we already finished. Without an ACK it would + // retransmit to its own limit and declare a fault at the end of a successful transfer. + auto eofPdu = makeEofPdu(); + const DestHandler::FsmResult& res = destHandler.stateMachine(eofPdu); + CHECK(res.result == OK); + auto ackPdu = expectDirective(FileDirective::ACK); + AckInfo ackInfo; + AckPduReader reader(ackPdu.rawPdu.data(), ackPdu.rawPdu.size(), ackInfo); + REQUIRE(reader.parseData() == OK); + CHECK(ackInfo.getAckedDirective() == FileDirective::EOF_DIRECTIVE); + CHECK(ackInfo.getTransactionStatus() == AckTransactionStatus::UNRECOGNIZED); + CHECK(destHandler.getCfdpState() == CfdpState::IDLE); + } + + SECTION("A Finished PDU which failed to send is not treated as awaiting its ACK") { + // The EOF PDU produces two sends in one state machine call: the ACK first, then the Finished + // PDU. Fail only the second one, so the peer never sees a Finished PDU at all. + auto metadataPdu = makeMetadataPdu(); + destHandler.stateMachine(metadataPdu); + auto fdPdu = makeFileDataPdu(0, fileData.size()); + destHandler.stateMachine(fdPdu); + senderMock.failSendAtIdx = 1; + auto eofPdu = makeEofPdu(); + destHandler.stateMachine(eofPdu); + expectDirective(FileDirective::ACK); + CHECK(not senderMock.getNextSentPacket().has_value()); + + // Waiting for the ACK of a PDU that was never sent costs a full positive ACK interval before + // the first retransmission, so the step has to stay on the send instead. + CHECK(destHandler.getTransactionStep() == DestHandler::TransactionStep::SENDING_FINISHED_PDU); + } + + SECTION("An unsendable Finished PDU does not pin the handler in acknowledged mode") { + // Keeping the step on SENDING_FINISHED_PDU retries the send, but nothing arms a timer or + // counts the attempts there, so a downstream which stays broken has to be bounded the same + // way class 1 bounds it. Otherwise the transaction is never released and, because a busy + // handler discards incoming metadata PDUs, no later uplink can start. + auto metadataPdu = makeMetadataPdu(); + destHandler.stateMachine(metadataPdu); + auto fdPdu = makeFileDataPdu(0, fileData.size()); + destHandler.stateMachine(fdPdu); + // The EOF ACK goes out, every Finished PDU send after it fails. + senderMock.failSendsFromIdx = 1; + auto eofPdu = makeEofPdu(); + destHandler.stateMachine(eofPdu); + expectDirective(FileDirective::ACK); + for (int idx = 0; idx < 100; idx++) { + destHandler.stateMachineNoPacket(); + } + CHECK(destHandler.getCfdpState() == CfdpState::IDLE); + CHECK(destHandler.getTransactionStep() == DestHandler::TransactionStep::IDLE); + } + + SECTION("A Finished ACK for a different transaction is ignored") { + // CfdpHandler routes ACK PDUs on the acked directive alone, so a late ACK from an earlier + // transaction reaches whichever transaction is running now. + auto metadataPdu = makeMetadataPdu(); + destHandler.stateMachine(metadataPdu); + auto fdPdu = makeFileDataPdu(0, fileData.size()); + destHandler.stateMachine(fdPdu); + auto eofPdu = makeEofPdu(); + destHandler.stateMachine(eofPdu); + expectDirective(FileDirective::ACK); + expectDirective(FileDirective::FINISH); + REQUIRE(destHandler.getTransactionStep() == + DestHandler::TransactionStep::WAITING_FOR_FINISHED_ACK); + + conf.seqNum = TransactionSeqNum(UnsignedByteField(42)); + auto staleAck = makeFinishedAckPdu(); + destHandler.stateMachine(staleAck); + CHECK(destHandler.getCfdpState() == CfdpState::BUSY_CLASS_2_ACKED); + CHECK(destHandler.getTransactionStep() == + DestHandler::TransactionStep::WAITING_FOR_FINISHED_ACK); + } +} diff --git a/unittests/cfdp/handler/testSourceHandler.cpp b/unittests/cfdp/handler/testSourceHandler.cpp index 17dc3849..87e1b1f3 100644 --- a/unittests/cfdp/handler/testSourceHandler.cpp +++ b/unittests/cfdp/handler/testSourceHandler.cpp @@ -1,18 +1,25 @@ #include #include +#include #include #include +#include +#include "OwnedPduPacket.h" #include "cfdp/PduSenderMock.h" #include "fsfw/cfdp.h" #include "fsfw/cfdp/handler/PutRequest.h" #include "fsfw/cfdp/handler/SourceHandler.h" +#include "fsfw/cfdp/pdu/AckPduCreator.h" +#include "fsfw/cfdp/pdu/AckPduReader.h" #include "fsfw/cfdp/pdu/EofPduCreator.h" #include "fsfw/cfdp/pdu/EofPduReader.h" #include "fsfw/cfdp/pdu/FileDataReader.h" +#include "fsfw/cfdp/pdu/FinishedPduCreator.h" #include "fsfw/cfdp/pdu/MetadataPduCreator.h" #include "fsfw/cfdp/pdu/MetadataPduReader.h" +#include "fsfw/cfdp/pdu/NakPduCreator.h" #include "fsfw/tmtcservices/TmTcMessage.h" #include "fsfw/util/SeqCountProvider.h" #include "mock/AcceptsTmMock.h" @@ -196,6 +203,39 @@ TEST_CASE("CFDP Source Handler", "[cfdp]") { genericNoticeOfCompletionCheck(fsmResult, expectedSeqNum); } + SECTION("File data PDU send failure is retried, not dropped") { + // A downstream send failure (e.g. a full TM store) must not be treated as if the PDU went + // out: the FSM has to retry the same segment, not silently advance past lost data. + uint16_t expectedSeqNum = 0; + fsMock.createFile(srcFileName.c_str()); + std::string fileContent = "hello world\n"; + size_t expectedFileSize = fileContent.size(); + fsMock.writeToFile(srcFileName.c_str(), 0, reinterpret_cast(fileContent.data()), + expectedFileSize); + CHECK(sourceHandler.transactionStart(putRequest, cfg) == OK); + const SourceHandler::FsmResult& fsmResult = sourceHandler.stateMachineNoPacket(); + genericMetadataCheck(fsmResult, expectedFileSize, expectedSeqNum); + + // The file data PDU send fails. No packet must be recorded and no progress made. + pduSender.failNextSend = true; + sourceHandler.stateMachineNoPacket(); + CHECK(fsmResult.packetsSent == 0); + CHECK(fsmResult.errors == 1); + CHECK(not pduSender.getNextSentPacket().has_value()); + CHECK(sourceHandler.getStep() == SourceHandler::TransactionStep::SENDING_FILE_DATA); + + // Retrying must send the same segment from offset 0, not skip ahead. + sourceHandler.stateMachineNoPacket(); + onePduSentCheck(fsmResult); + auto optNextPacket = pduSender.getNextSentPacket(); + CHECK(optNextPacket.has_value()); + const auto& [pduType, fileDirective, rawPdu] = *optNextPacket; + FileDataInfo fdInfo; + FileDataReader fdReader(rawPdu.data(), rawPdu.size(), fdInfo); + CHECK(fdReader.parseData() == OK); + CHECK(fdInfo.getOffset().value() == 0); + } + SECTION("Transfer two segment file") { uint16_t expectedSeqNum = 0; // Create 400 bytes of random data. This should result in two file segments, with one @@ -271,4 +311,289 @@ TEST_CASE("CFDP Source Handler", "[cfdp]") { sourceHandler.stateMachineNoPacket(); genericNoticeOfCompletionCheck(fsmResult, expectedSeqNum); } -} \ No newline at end of file +} +TEST_CASE("CFDP Source Handler Acknowledged", "[cfdp]") { + using namespace cfdp; + using namespace returnvalue; + constexpr size_t MAX_FILE_SEGMENT_SIZE = 256; + + auto localId = EntityId(UnsignedByteField(2)); + auto remoteId = EntityId(UnsignedByteField(5)); + FaultHandlerMock fhMock; + LocalEntityCfg localEntityCfg(localId, IndicationCfg(), fhMock); + FilesystemMock fsMock; + UserMock userMock(fsMock); + SeqCountProviderU16 seqCountProvider; + SourceHandlerParams dp(localEntityCfg, userMock, seqCountProvider); + PduSenderMock pduSender; + EventReportingProxyMock eventReporterMock; + FsfwParams fp(&eventReporterMock); + auto sourceHandler = SourceHandler(pduSender, 4096, dp, fp); + + RemoteEntityCfg cfg; + cfg.maxFileSegmentLen = MAX_FILE_SEGMENT_SIZE; + cfg.remoteId = remoteId; + cfg.defaultTransmissionMode = TransmissionMode::ACKNOWLEDGED; + // Keep the timers short so the timeout paths are testable without stalling the suite. + cfg.positiveAckTimerIntervalMs = 1; + cfg.positiveAckTimerExpirationLimit = 2; + + std::string srcFileName = "/tmp/cfdp-acked-test.txt"; + std::string destFileName = "/tmp/cfdp-acked-test2.txt"; + std::array fileData{}; + for (size_t idx = 0; idx < fileData.size(); idx++) { + fileData[idx] = static_cast(idx); + } + fsMock.createFile(srcFileName.c_str()); + fsMock.writeToFile(srcFileName.c_str(), 0, fileData.data(), fileData.size()); + cfdp::StringLv srcNameLv(srcFileName.c_str(), srcFileName.length()); + cfdp::StringLv destNameLv(destFileName.c_str(), destFileName.length()); + PutRequest putRequest(remoteId, srcNameLv, destNameLv); + CHECK(sourceHandler.initialize() == OK); + + // The PDU configuration the peer would use to address this handler. + PduConfig peerConf; + peerConf.sourceId = localId; + peerConf.destId = remoteId; + peerConf.mode = TransmissionMode::ACKNOWLEDGED; + peerConf.seqNum = TransactionSeqNum(UnsignedByteField(0)); + peerConf.direction = Direction::TOWARDS_SENDER; + std::array pduBuf{}; + size_t serLen = 0; + + auto makeEofAckPdu = [&]() { + AckInfo info(FileDirective::EOF_DIRECTIVE, ConditionCode::NO_ERROR, + AckTransactionStatus::ACTIVE, 0); + AckPduCreator creator(info, peerConf); + REQUIRE(creator.serialize(pduBuf.data(), serLen, creator.getSerializedSize()) == OK); + return OwnedPduPacket(creator.getPduType(), creator.getDirectiveCode(), pduBuf.data(), serLen); + }; + auto makeFinishedPdu = [&](ConditionCode condCode, FileDeliveryCode deliveryCode, + FileDeliveryStatus status) { + FinishedInfo info(condCode, deliveryCode, status); + FinishPduCreator creator(peerConf, info); + REQUIRE(creator.serialize(pduBuf.data(), serLen, creator.getSerializedSize()) == OK); + return OwnedPduPacket(creator.getPduType(), creator.getDirectiveCode(), pduBuf.data(), serLen); + }; + auto makeNakPdu = [&](const std::vector>& segments) { + std::vector segBuf; + segBuf.reserve(segments.size()); + for (const auto& segment : segments) { + segBuf.emplace_back(Fss(segment.first), Fss(segment.second)); + } + NakInfo info(Fss(0), Fss(fileData.size())); + size_t segLen = segBuf.size(); + size_t maxSegLen = segBuf.size(); + info.setSegmentRequests(segBuf.data(), &segLen, &maxSegLen); + NakPduCreator creator(peerConf, info); + REQUIRE(creator.serialize(pduBuf.data(), serLen, creator.getSerializedSize()) == OK); + return OwnedPduPacket(creator.getPduType(), creator.getDirectiveCode(), pduBuf.data(), serLen); + }; + auto expectDirective = [&](FileDirective expected) { + auto optPacket = pduSender.getNextSentPacket(); + REQUIRE(optPacket.has_value()); + REQUIRE(optPacket->pduType == PduType::FILE_DIRECTIVE); + REQUIRE(optPacket->fileDirective.has_value()); + CHECK(*optPacket->fileDirective == expected); + return *optPacket; + }; + auto expectFileData = [&](uint64_t expectedOffset, size_t expectedLen) { + auto optPacket = pduSender.getNextSentPacket(); + REQUIRE(optPacket.has_value()); + REQUIRE(optPacket->pduType == PduType::FILE_DATA); + FileDataInfo fdInfo; + FileDataReader reader(optPacket->rawPdu.data(), optPacket->rawPdu.size(), fdInfo); + REQUIRE(reader.parseData() == OK); + CHECK(fdInfo.getOffset().value() == expectedOffset); + size_t len = 0; + const uint8_t* data = fdInfo.getFileData(&len); + CHECK(len == expectedLen); + for (size_t idx = 0; idx < len; idx++) { + CHECK(data[idx] == fileData[expectedOffset + idx]); + } + }; + // Drives the handler until the EOF PDU has been sent, draining every PDU on the way. + auto runUntilEofSent = [&]() { + REQUIRE(sourceHandler.transactionStart(putRequest, cfg) == OK); + REQUIRE(sourceHandler.getState() == CfdpState::BUSY_CLASS_2_ACKED); + sourceHandler.stateMachineNoPacket(); + expectDirective(FileDirective::METADATA); + for (size_t offset = 0; offset < fileData.size(); offset += MAX_FILE_SEGMENT_SIZE) { + sourceHandler.stateMachineNoPacket(); + expectFileData(offset, MAX_FILE_SEGMENT_SIZE); + } + sourceHandler.stateMachineNoPacket(); + expectDirective(FileDirective::EOF_DIRECTIVE); + CHECK(sourceHandler.getStep() == SourceHandler::TransactionStep::WAIT_FOR_ACK); + }; + + SECTION("Nominal acknowledged transfer") { + runUntilEofSent(); + auto ackPdu = makeEofAckPdu(); + sourceHandler.stateMachine(ackPdu); + CHECK(sourceHandler.getStep() == SourceHandler::TransactionStep::WAIT_FOR_FINISH); + auto finishedPdu = makeFinishedPdu(ConditionCode::NO_ERROR, FileDeliveryCode::DATA_COMPLETE, + FileDeliveryStatus::RETAINED_IN_FILESTORE); + sourceHandler.stateMachine(finishedPdu); + // The Finished PDU is acknowledged and the transaction completes. + expectDirective(FileDirective::ACK); + CHECK(sourceHandler.getState() == CfdpState::IDLE); + REQUIRE(userMock.finishedRecvd.size() == 1); + const auto& params = userMock.finishedRecvd.back().second; + CHECK(params.condCode == ConditionCode::NO_ERROR); + CHECK(params.deliveryCode == FileDeliveryCode::DATA_COMPLETE); + CHECK(params.status == FileDeliveryStatus::RETAINED_IN_FILESTORE); + } + + SECTION("Finished PDU condition code is reported instead of a hardcoded success") { + runUntilEofSent(); + auto ackPdu = makeEofAckPdu(); + sourceHandler.stateMachine(ackPdu); + auto finishedPdu = + makeFinishedPdu(ConditionCode::FILE_CHECKSUM_FAILURE, FileDeliveryCode::DATA_INCOMPLETE, + FileDeliveryStatus::DISCARDED_DELIBERATELY); + sourceHandler.stateMachine(finishedPdu); + expectDirective(FileDirective::ACK); + REQUIRE(userMock.finishedRecvd.size() == 1); + const auto& params = userMock.finishedRecvd.back().second; + CHECK(params.condCode == ConditionCode::FILE_CHECKSUM_FAILURE); + CHECK(params.deliveryCode == FileDeliveryCode::DATA_INCOMPLETE); + } + + SECTION("EOF PDU is retransmitted on positive ACK timeout") { + runUntilEofSent(); + for (uint32_t idx = 0; idx < cfg.positiveAckTimerExpirationLimit; idx++) { + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + sourceHandler.stateMachineNoPacket(); + expectDirective(FileDirective::EOF_DIRECTIVE); + CHECK(sourceHandler.getState() == CfdpState::BUSY_CLASS_2_ACKED); + } + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + sourceHandler.stateMachineNoPacket(); + CHECK(sourceHandler.getState() == CfdpState::IDLE); + auto& fhInfo = fhMock.getFhInfo(FaultHandlerCode::IGNORE_ERROR); + REQUIRE(fhInfo.callCount == 1); + CHECK(fhInfo.condCodes.front() == ConditionCode::POSITIVE_ACK_LIMIT_REACHED); + REQUIRE(userMock.finishedRecvd.size() == 1); + CHECK(userMock.finishedRecvd.back().second.condCode == + ConditionCode::POSITIVE_ACK_LIMIT_REACHED); + } + + SECTION("NAK PDU drives segment retransmission") { + runUntilEofSent(); + // Two gaps, the first of which spans more than one file segment. + auto nakPdu = makeNakPdu({{256, 768}, {896, 1024}}); + sourceHandler.stateMachine(nakPdu); + // One PDU per state machine call, exactly like the regular file data phase. + expectFileData(256, MAX_FILE_SEGMENT_SIZE); + sourceHandler.stateMachineNoPacket(); + expectFileData(512, MAX_FILE_SEGMENT_SIZE); + sourceHandler.stateMachineNoPacket(); + expectFileData(896, 128); + // Nothing is outstanding any more, so the handler is back to guarding the EOF PDU. + CHECK(sourceHandler.getStep() == SourceHandler::TransactionStep::WAIT_FOR_ACK); + auto ackPdu = makeEofAckPdu(); + sourceHandler.stateMachine(ackPdu); + auto finishedPdu = makeFinishedPdu(ConditionCode::NO_ERROR, FileDeliveryCode::DATA_COMPLETE, + FileDeliveryStatus::RETAINED_IN_FILESTORE); + sourceHandler.stateMachine(finishedPdu); + expectDirective(FileDirective::ACK); + CHECK(sourceHandler.getState() == CfdpState::IDLE); + } + + SECTION("NAK PDU of scope 0 to 0 retransmits the metadata PDU") { + runUntilEofSent(); + auto nakPdu = makeNakPdu({{0, 0}}); + sourceHandler.stateMachine(nakPdu); + expectDirective(FileDirective::METADATA); + CHECK(sourceHandler.getStep() == SourceHandler::TransactionStep::WAIT_FOR_ACK); + } + + SECTION("A failed retransmission send is retried, not skipped") { + // Same contract as "File data PDU send failure is retried, not dropped" for the forward + // path: a downstream send failure must leave the requested segment outstanding. The peer + // only re-requests it on its NAK timer, so dropping it here burns a NAK limit credit and, + // once the limit is reached, loses the transfer. + runUntilEofSent(); + auto nakPdu = makeNakPdu({{256, 512}}); + pduSender.failNextSend = true; + sourceHandler.stateMachine(nakPdu); + CHECK(not pduSender.getNextSentPacket().has_value()); + + // The retransmission is still outstanding, so the next call has to send that same segment. + sourceHandler.stateMachineNoPacket(); + expectFileData(256, MAX_FILE_SEGMENT_SIZE); + } + + SECTION("A failed metadata retransmission is retried, not dropped") { + // The scope 0 to 0 request is the receiver saying it never got the metadata PDU and cannot + // write the file at all. Losing the answer to a full TM store strands the whole transfer. + runUntilEofSent(); + auto nakPdu = makeNakPdu({{0, 0}}); + pduSender.failNextSend = true; + sourceHandler.stateMachine(nakPdu); + CHECK(not pduSender.getNextSentPacket().has_value()); + + sourceHandler.stateMachineNoPacket(); + expectDirective(FileDirective::METADATA); + } + + SECTION("A NAK with more segment requests than fit still answers the ones which do") { + // NakPduReader fills the caller's array incrementally but reports a segment request length + // of 0 when it runs out of room, so the handler must not rely on that length alone. A peer + // which packs more requests into one NAK than RetransmitState can hold otherwise gets no + // retransmission at all. + runUntilEofSent(); + std::vector> segments; + for (uint64_t idx = 0; idx < 33; idx++) { + segments.emplace_back(idx * 2, idx * 2 + 1); + } + auto nakPdu = makeNakPdu(segments); + sourceHandler.stateMachine(nakPdu); + expectFileData(0, 1); + } + + SECTION("An unparseable Finished PDU does not complete the transaction") { + // A Finished PDU truncated before its condition code byte cannot say anything about how the + // transfer went. Completing on it reports the default-constructed DATA_COMPLETE, i.e. a + // fabricated success, and discards the state the peer's retransmission would have landed in. + runUntilEofSent(); + auto eofAck = makeEofAckPdu(); + sourceHandler.stateMachine(eofAck); + REQUIRE(sourceHandler.getStep() == SourceHandler::TransactionStep::WAIT_FOR_FINISH); + + auto finishedPdu = makeFinishedPdu(ConditionCode::NO_ERROR, FileDeliveryCode::DATA_COMPLETE, + FileDeliveryStatus::RETAINED_IN_FILESTORE); + // Cut the PDU short so the header declares more bytes than are actually present, which is + // what a truncated uplink looks like. + finishedPdu.rawPdu.resize(finishedPdu.rawPdu.size() - 1); + sourceHandler.stateMachine(finishedPdu); + + CHECK(sourceHandler.getState() == CfdpState::BUSY_CLASS_2_ACKED); + CHECK(userMock.finishedRecvd.empty()); + } + + SECTION("An ACK PDU for a different transaction is ignored") { + // Nothing upstream filters by transaction: CfdpHandler routes on direction alone. A late ACK + // from a previous transaction therefore lands in whichever one is running now. + runUntilEofSent(); + peerConf.seqNum = TransactionSeqNum(UnsignedByteField(42)); + auto staleAck = makeEofAckPdu(); + sourceHandler.stateMachine(staleAck); + CHECK(sourceHandler.getStep() == SourceHandler::TransactionStep::WAIT_FOR_ACK); + } + + SECTION("A Finished PDU for a different transaction is ignored") { + runUntilEofSent(); + auto eofAck = makeEofAckPdu(); + sourceHandler.stateMachine(eofAck); + REQUIRE(sourceHandler.getStep() == SourceHandler::TransactionStep::WAIT_FOR_FINISH); + + peerConf.seqNum = TransactionSeqNum(UnsignedByteField(42)); + auto staleFinished = makeFinishedPdu(ConditionCode::NO_ERROR, FileDeliveryCode::DATA_COMPLETE, + FileDeliveryStatus::RETAINED_IN_FILESTORE); + sourceHandler.stateMachine(staleFinished); + CHECK(sourceHandler.getState() == CfdpState::BUSY_CLASS_2_ACKED); + CHECK(userMock.finishedRecvd.empty()); + } +} diff --git a/unittests/cfdp/pdu/PduCrcHelper.h b/unittests/cfdp/pdu/PduCrcHelper.h new file mode 100644 index 00000000..e02b148c --- /dev/null +++ b/unittests/cfdp/pdu/PduCrcHelper.h @@ -0,0 +1,38 @@ +#ifndef FSFW_UNITTESTS_CFDP_PDU_PDUCRCHELPER_H_ +#define FSFW_UNITTESTS_CFDP_PDU_PDUCRCHELPER_H_ + +#include +#include + +#include "fsfw/globalfunctions/CRC.h" + +namespace cfdp::test { + +/** + * Turn an already serialized PDU without a CRC into one carrying a PDU CRC, in place. + * + * The PDU creators cannot produce this themselves: they append the two CRC bytes without counting + * them in the directive data field length, so the PDU they emit is inconsistent with its own + * header. Ground implementations do set the flag - cfdppy does by default - so the readers have to + * cope with it, and building the reference bytes here keeps these tests independent of that + * separate creator defect. + * + * Sets the CRC flag in the first header byte, grows the PDU data field length by two and appends + * the CRC-16/CCITT over the whole PDU, so that a CRC run across the result yields zero - which is + * what PduHeaderReader::performCrcCheckIfApplicable checks. + */ +inline void addPduCrc(uint8_t* buf, size_t& size) { + buf[0] |= 0x02; + auto dataFieldLen = static_cast((buf[1] << 8) | buf[2]); + dataFieldLen += 2; + buf[1] = (dataFieldLen >> 8) & 0xff; + buf[2] = dataFieldLen & 0xff; + uint16_t crc = CRC::crc16ccitt(buf, size); + buf[size] = (crc >> 8) & 0xff; + buf[size + 1] = crc & 0xff; + size += 2; +} + +} // namespace cfdp::test + +#endif /* FSFW_UNITTESTS_CFDP_PDU_PDUCRCHELPER_H_ */ diff --git a/unittests/cfdp/pdu/testFinishedPdu.cpp b/unittests/cfdp/pdu/testFinishedPdu.cpp index b8b395cb..cc28491a 100644 --- a/unittests/cfdp/pdu/testFinishedPdu.cpp +++ b/unittests/cfdp/pdu/testFinishedPdu.cpp @@ -1,6 +1,7 @@ #include #include +#include "PduCrcHelper.h" #include "fsfw/cfdp/pdu/FinishedPduCreator.h" #include "fsfw/cfdp/pdu/FinishedPduReader.h" #include "fsfw/globalfunctions/arrayprinter.h" @@ -187,3 +188,53 @@ TEST_CASE("Finished PDU", "[cfdp][pdu]") { } } } + +TEST_CASE("Finished PDU with CRC", "[cfdp][pdu]") { + using namespace cfdp; + std::array fnBuffer = {}; + uint8_t* buffer = fnBuffer.data(); + size_t sz = 0; + EntityId destId(WidthInBytes::TWO_BYTES, 2); + TransactionSeqNum seqNum(WidthInBytes::TWO_BYTES, 15); + EntityId sourceId(WidthInBytes::TWO_BYTES, 1); + PduConfig pduConf(sourceId, destId, TransmissionMode::ACKNOWLEDGED, seqNum); + + SECTION("Nominal completion") { + // The case every acknowledged transfer ends on: no filestore responses and no fault location, + // so the CRC is the only thing following the condition code byte. The TLV loop used to run + // straight into it and reject the PDU as an invalid TLV type. + FinishedInfo info(cfdp::ConditionCode::NO_ERROR, cfdp::FileDeliveryCode::DATA_COMPLETE, + cfdp::FileDeliveryStatus::RETAINED_IN_FILESTORE); + FinishPduCreator creator(pduConf, info); + REQUIRE(creator.serialize(&buffer, &sz, fnBuffer.size(), SerializeIF::Endianness::NETWORK) == + returnvalue::OK); + cfdp::test::addPduCrc(fnBuffer.data(), sz); + + FinishedInfo infoDeser; + FinishPduReader reader(fnBuffer.data(), sz, infoDeser); + REQUIRE(reader.parseData() == returnvalue::OK); + REQUIRE(reader.getCrcFlag()); + REQUIRE(infoDeser.getConditionCode() == cfdp::ConditionCode::NO_ERROR); + REQUIRE(infoDeser.getDeliveryCode() == cfdp::FileDeliveryCode::DATA_COMPLETE); + REQUIRE(infoDeser.getFileStatus() == cfdp::FileDeliveryStatus::RETAINED_IN_FILESTORE); + } + + SECTION("With fault location") { + EntityIdTlv faultLoc(destId); + FinishedInfo info(cfdp::ConditionCode::FILESTORE_REJECTION, + cfdp::FileDeliveryCode::DATA_INCOMPLETE, + cfdp::FileDeliveryStatus::DISCARDED_DELIBERATELY); + info.setFaultLocation(&faultLoc); + FinishPduCreator creator(pduConf, info); + REQUIRE(creator.serialize(&buffer, &sz, fnBuffer.size(), SerializeIF::Endianness::NETWORK) == + returnvalue::OK); + cfdp::test::addPduCrc(fnBuffer.data(), sz); + + EntityIdTlv faultLocDeser(destId); + FinishedInfo infoDeser; + infoDeser.setFaultLocation(&faultLocDeser); + FinishPduReader reader(fnBuffer.data(), sz, infoDeser); + REQUIRE(reader.parseData() == returnvalue::OK); + REQUIRE(infoDeser.getConditionCode() == cfdp::ConditionCode::FILESTORE_REJECTION); + } +} diff --git a/unittests/cfdp/pdu/testMetadataPdu.cpp b/unittests/cfdp/pdu/testMetadataPdu.cpp index e13b4717..bc9457ce 100644 --- a/unittests/cfdp/pdu/testMetadataPdu.cpp +++ b/unittests/cfdp/pdu/testMetadataPdu.cpp @@ -2,6 +2,7 @@ #include #include +#include "PduCrcHelper.h" #include "fsfw/cfdp/pdu/MetadataPduCreator.h" #include "fsfw/cfdp/pdu/MetadataPduReader.h" #include "fsfw/cfdp/tlv/FilestoreResponseTlv.h" @@ -222,3 +223,58 @@ TEST_CASE("Metadata PDU", "[cfdp][pdu]") { } } } + +TEST_CASE("Metadata PDU with CRC", "[cfdp][pdu]") { + using namespace cfdp; + std::array mdBuffer = {}; + uint8_t* buffer = mdBuffer.data(); + size_t sz = 0; + EntityId destId(WidthInBytes::TWO_BYTES, 2); + TransactionSeqNum seqNum(WidthInBytes::TWO_BYTES, 15); + EntityId sourceId(WidthInBytes::TWO_BYTES, 1); + PduConfig pduConf(sourceId, destId, TransmissionMode::ACKNOWLEDGED, seqNum); + Fss fileSize(0); + MetadataGenericInfo info(false, ChecksumType::NULL_CHECKSUM, fileSize); + std::array tlvDeser{}; + + SECTION("Metadata only with message to user") { + // This is the shape of a proxy put request: no file names, the request itself travels in a + // message to user TLV. Parsing it used to fail with INVALID_TLV_TYPE, because the option loop + // ran into the CRC and deserialized it as a further TLV. + cfdp::StringLv emptySourceName; + cfdp::StringLv emptyDestName; + std::array msg = {0x41, 0x42, 0x43}; + MessageToUserTlv msgToUser(msg.data(), msg.size()); + std::array options{&msgToUser}; + MetadataPduCreator creator(pduConf, info, emptySourceName, emptyDestName, options.data(), + options.size()); + creator.updateDirectiveFieldLen(); + REQUIRE(creator.serialize(&buffer, &sz, mdBuffer.size(), SerializeIF::Endianness::NETWORK) == + returnvalue::OK); + cfdp::test::addPduCrc(mdBuffer.data(), sz); + + MetadataPduReader reader(mdBuffer.data(), sz, info, tlvDeser.data(), tlvDeser.max_size()); + REQUIRE(reader.parseData() == returnvalue::OK); + REQUIRE(reader.getCrcFlag()); + REQUIRE(reader.getNumberOfParsedOptions() == 1); + REQUIRE(tlvDeser[0].getType() == cfdp::TlvType::MSG_TO_USER); + REQUIRE(tlvDeser[0].getLengthField() == msg.size()); + } + + SECTION("No options") { + // The CRC is all that follows the file names here, which the reader handled correctly even + // before the option loop was taught about it. + std::string name = "hello.txt"; + cfdp::StringLv sourceFileName(name); + cfdp::StringLv destFileName(name); + MetadataPduCreator creator(pduConf, info, sourceFileName, destFileName, nullptr, 0); + creator.updateDirectiveFieldLen(); + REQUIRE(creator.serialize(&buffer, &sz, mdBuffer.size(), SerializeIF::Endianness::NETWORK) == + returnvalue::OK); + cfdp::test::addPduCrc(mdBuffer.data(), sz); + + MetadataPduReader reader(mdBuffer.data(), sz, info, tlvDeser.data(), tlvDeser.max_size()); + REQUIRE(reader.parseData() == returnvalue::OK); + REQUIRE(reader.getNumberOfParsedOptions() == 0); + } +} diff --git a/unittests/cfdp/pdu/testNakPdu.cpp b/unittests/cfdp/pdu/testNakPdu.cpp index c3650c63..5d0ea44b 100644 --- a/unittests/cfdp/pdu/testNakPdu.cpp +++ b/unittests/cfdp/pdu/testNakPdu.cpp @@ -1,6 +1,7 @@ #include #include +#include "PduCrcHelper.h" #include "fsfw/cfdp/pdu/NakPduCreator.h" #include "fsfw/cfdp/pdu/NakPduReader.h" #include "fsfw/cfdp/pdu/PduConfig.h" @@ -150,3 +151,54 @@ TEST_CASE("NAK PDU", "[cfdp][pdu]") { REQUIRE(info.getSegmentRequestsMaxLen() == 5); } } + +TEST_CASE("NAK PDU with CRC", "[cfdp][pdu]") { + using namespace cfdp; + std::array nakBuffer = {}; + uint8_t* buffer = nakBuffer.data(); + size_t sz = 0; + EntityId destId(WidthInBytes::TWO_BYTES, 2); + TransactionSeqNum seqNum(WidthInBytes::TWO_BYTES, 15); + EntityId sourceId(WidthInBytes::TWO_BYTES, 1); + PduConfig pduConf(sourceId, destId, TransmissionMode::ACKNOWLEDGED, seqNum); + Fss startOfScope(50); + Fss endOfScope(1050); + NakInfo info(startOfScope, endOfScope); + + SECTION("With segment requests") { + std::array segReqs{ + NakInfo::SegmentRequest(cfdp::Fss(2020), cfdp::Fss(2520)), + NakInfo::SegmentRequest(cfdp::Fss(2932), cfdp::Fss(3021))}; + size_t segReqLen = segReqs.size(); + info.setSegmentRequests(segReqs.data(), &segReqLen, nullptr); + NakPduCreator creator(pduConf, info); + REQUIRE(creator.serialize(&buffer, &sz, nakBuffer.size(), SerializeIF::Endianness::NETWORK) == + returnvalue::OK); + cfdp::test::addPduCrc(nakBuffer.data(), sz); + + std::array segReqsDeser{}; + NakInfo infoDeser(Fss(0), Fss(0)); + size_t maxSegReqs = segReqsDeser.size(); + infoDeser.setSegmentRequests(segReqsDeser.data(), nullptr, &maxSegReqs); + NakPduReader reader(nakBuffer.data(), sz, infoDeser); + REQUIRE(reader.parseData() == returnvalue::OK); + REQUIRE(reader.getCrcFlag()); + REQUIRE(infoDeser.getSegmentRequestsLen() == 2); + REQUIRE(segReqsDeser[0].first.value() == 2020); + REQUIRE(segReqsDeser[1].second.value() == 3021); + } + + SECTION("Without segment requests") { + NakPduCreator creator(pduConf, info); + REQUIRE(creator.serialize(&buffer, &sz, nakBuffer.size(), SerializeIF::Endianness::NETWORK) == + returnvalue::OK); + cfdp::test::addPduCrc(nakBuffer.data(), sz); + + NakInfo infoDeser(Fss(0), Fss(0)); + NakPduReader reader(nakBuffer.data(), sz, infoDeser); + REQUIRE(reader.parseData() == returnvalue::OK); + REQUIRE(infoDeser.getStartOfScope().value() == 50); + REQUIRE(infoDeser.getEndOfScope().value() == 1050); + REQUIRE(infoDeser.getSegmentRequestsLen() == 0); + } +}