Skip to content

Commit

Permalink
Fix nasa#2378, refactor SB to support additional use cases
Browse files Browse the repository at this point in the history 
Cleans up the internal SB implementation so it can better support future
enhancements such as message integrity, additional header fields and
timestamping.
  • Loading branch information
@jphickey
jphickey committed Nov 16, 2023
1 parent b72dd4e commit c4141f9
Show file tree
Hide file tree
Showing 23 changed files with 2,601 additions and 1,474 deletions.
2 changes: 1 addition & 1 deletion modules/cfe_testcase/src/sb_sendrecv_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ void TestBasicTransmitRecv(void)
UtAssert_INT32_EQ(CFE_SB_ReceiveBuffer(&MsgBuf, PipeId1, -100), CFE_SB_BAD_ARGUMENT);

/*
* Note, the CFE_SB_TransmitMsg now adheres to the "UpdateHeader" flag.
* Note, the CFE_SB_TransmitMsg now adheres to the "IsOrigination" flag.
* Thus, the sequence numbers should come back with the value from the Route (1-2)
* rather than the value the message was filled with initially.
*
Expand Down
73 changes: 45 additions & 28 deletions modules/core_api/fsw/inc/cfe_msg.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,30 +60,6 @@
*/
CFE_Status_t CFE_MSG_Init(CFE_MSG_Message_t *MsgPtr, CFE_SB_MsgId_t MsgId, CFE_MSG_Size_t Size);

/*****************************************************************************/
/**
* \brief Set/compute all dynamically-updated headers on a message
*
* \par Description
* This routine updates all dynamic header fields on a message, and is typically
* invoked via SB just prior to broadcasting the message. Dynamic headers include
* are values that should be computed/updated per message, including:
* - the sequence number
* - the timestamp, if present
* - any error control or checksum fields, if present
*
* The MSG module implementation determines which header fields meet this criteria
* and how they should be computed.
*
* \param[inout] MsgPtr A pointer to the buffer that contains the message @nonnull.
* \param[in] SeqCnt The current sequence number from the message route
*
* \return Execution status, see \ref CFEReturnCodes
* \retval #CFE_SUCCESS \copybrief CFE_SUCCESS
* \retval #CFE_MSG_BAD_ARGUMENT \copybrief CFE_MSG_BAD_ARGUMENT
*/
CFE_Status_t CFE_MSG_UpdateHeader(CFE_MSG_Message_t *MsgPtr, CFE_MSG_SequenceCount_t SeqCnt);

/**\}*/

/** \defgroup CFEAPIMSGHeaderPri cFE Message Primary Header APIs
Expand Down Expand Up @@ -725,12 +701,44 @@ CFE_Status_t CFE_MSG_GetTypeFromMsgId(CFE_SB_MsgId_t MsgId, CFE_MSG_Type_t *Type

/**\}*/

/** \defgroup CFEAPIMSGMsgVerify cFE Message Checking APIs
/** \defgroup CFEAPIMSGMsgIntegrity cFE Message Integrity APIs
* \{
*/

/*****************************************************************************/
/**
* \brief Checks message headers against expected values
* \brief Perform any necessary actions on a newly-created message, prior to sending
*
* \par Description
* This routine updates and/or appends any necessary fields on a message, is
* invoked via SB just prior to broadcasting the message. The actions include
* updating any values that should be computed/updated per message, including:
* - setting the sequence number
* - updating the timestamp, if present
* - computing any error control or checksum fields, if present
*
* The MSG module implementation determines which header fields meet this criteria
* and how they should be computed.
*
* The BufferSize parameter indicates the allocation size message of the buffer that
* holds the message (i.e. the message envelope size). In some implementations, the
* allocated buffer may include extra space in order to append a CRC or digital signature.
*
* \sa CFE_MSG_VerificationAction
*
* \param[inout] MsgPtr A pointer to the buffer that contains the message @nonnull.
* \param[in] BufferSize The size of the buffer encapsulating the message
* \param[out] IsAcceptable Output variable to be set, indicates message acceptability @nonnull
*
* \return Execution status, see \ref CFEReturnCodes
* \retval #CFE_SUCCESS \copybrief CFE_SUCCESS
* \retval #CFE_MSG_BAD_ARGUMENT \copybrief CFE_MSG_BAD_ARGUMENT
*/
CFE_Status_t CFE_MSG_OriginationAction(CFE_MSG_Message_t *MsgPtr, size_t BufferSize, bool *IsAcceptable);

/*****************************************************************************/
/**
* \brief Checks message integrity/acceptability
*
* \par Description
* This routine validates that any error-control field(s) in the message header
Expand All @@ -740,14 +748,23 @@ CFE_Status_t CFE_MSG_GetTypeFromMsgId(CFE_SB_MsgId_t MsgId, CFE_MSG_Type_t *Type
* and may be a no-op if no error checking is implemented. In that case, it
* will always output "true".
*
* \note Due to the fact that software bus uses a multicast architecture, this function
* must not modify the message, as the buffer may be shared among multiple receivers.
* This should generally be the inverse of CFE_MSG_OriginationAction(), but on the
* origination side it may update header fields and/or modify the message, on
* the verification/receive side it must only check those fields, not modify them.
*
* \sa CFE_MSG_OriginationAction
*
* \param[in] MsgPtr Message Pointer @nonnull
* \param[out] VerifyStatus Output variable to be set to verification result @nonnull
* \param[in] BufferSize The size of the buffer encapsulating the message
* \param[out] IsAcceptable Output variable to be set, indicates message acceptability @nonnull
*
* \return Execution status, see \ref CFEReturnCodes
* \retval #CFE_SUCCESS \copybrief CFE_SUCCESS
* \retval #CFE_MSG_BAD_ARGUMENT \copybrief CFE_MSG_BAD_ARGUMENT
*/
CFE_Status_t CFE_MSG_Verify(const CFE_MSG_Message_t *MsgPtr, bool *VerifyStatus);
CFE_Status_t CFE_MSG_VerificationAction(const CFE_MSG_Message_t *MsgPtr, size_t BufferSize, bool *IsAcceptable);

/**\}*/

Expand Down
35 changes: 24 additions & 11 deletions modules/core_api/fsw/inc/cfe_sb.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -402,9 +402,15 @@ CFE_Status_t CFE_SB_UnsubscribeLocal(CFE_SB_MsgId_t MsgId, CFE_SB_PipeId_t PipeI
** software bus will read the message ID from the message header to
** determine which pipes should receive the message.
**
** In general, the "UpdateHeader" parameter should be passed as "true"
** if the message was newly constructed by the sender and is being sent
** for the first time. When forwarding a message that originated from
** The IsOrigination parameter should be passed as "true" if the message was
** newly constructed by the sender and is being sent for the first time. This
** enables the message origination actions as determined by the CFE MSG module,
** which may include (but not limited to):
** - Updating sequence number
** - Updating timestamp
** - Calcualating a CRC, checksum, or other message error control field
**
** Conversely, when forwarding a message that originated from
** an external entity (e.g. messages passing through CI or SBN), the
** parameter should be passed as "false" to not overwrite existing data.
**
Expand All @@ -421,15 +427,15 @@ CFE_Status_t CFE_SB_UnsubscribeLocal(CFE_SB_MsgId_t MsgId, CFE_SB_PipeId_t PipeI
**
** \param[in] MsgPtr A pointer to the message to be sent @nonnull. This must point
** to the first byte of the message header.
** \param[in] UpdateHeader Update the headers of the message
** \param[in] IsOrigination Update the headers of the message
**
** \return Execution status, see \ref CFEReturnCodes
** \retval #CFE_SUCCESS \copybrief CFE_SUCCESS
** \retval #CFE_SB_BAD_ARGUMENT \copybrief CFE_SB_BAD_ARGUMENT
** \retval #CFE_SB_MSG_TOO_BIG \copybrief CFE_SB_MSG_TOO_BIG
** \retval #CFE_SB_BUF_ALOC_ERR \covtest \copybrief CFE_SB_BUF_ALOC_ERR
**/
CFE_Status_t CFE_SB_TransmitMsg(const CFE_MSG_Message_t *MsgPtr, bool UpdateHeader);
CFE_Status_t CFE_SB_TransmitMsg(const CFE_MSG_Message_t *MsgPtr, bool IsOrigination);

/*****************************************************************************/
/**
Expand Down Expand Up @@ -471,6 +477,7 @@ CFE_Status_t CFE_SB_TransmitMsg(const CFE_MSG_Message_t *MsgPtr, bool UpdateHead
** \retval #CFE_SB_NO_MESSAGE \copybrief CFE_SB_NO_MESSAGE
**/
CFE_Status_t CFE_SB_ReceiveBuffer(CFE_SB_Buffer_t **BufPtr, CFE_SB_PipeId_t PipeId, int32 TimeOut);

/** @} */

/** @defgroup CFEAPISBZeroCopy cFE Zero Copy APIs
Expand Down Expand Up @@ -544,9 +551,15 @@ CFE_Status_t CFE_SB_ReleaseMessageBuffer(CFE_SB_Buffer_t *BufPtr);
** internal buffer. The "zero copy" interface can be used to improve
** performance in high-rate, high-volume software bus traffic.
**
** In general, the "UpdateHeader" parameter should be passed as "true"
** if the message was newly constructed by the sender and is being sent
** for the first time. When forwarding a message that originated from
** The IsOrigination parameter should be passed as "true" if the message was
** newly constructed by the sender and is being sent for the first time. This
** enables the message origination actions as determined by the CFE MSG module,
** which may include (but not limited to):
** - Updating sequence number
** - Updating timestamp
** - Calcualating a CRC, checksum, or other message error control field
**
** Conversely, when forwarding a message that originated from
** an external entity (e.g. messages passing through CI or SBN), the
** parameter should be passed as "false" to not overwrite existing data.
**
Expand All @@ -567,15 +580,15 @@ CFE_Status_t CFE_SB_ReleaseMessageBuffer(CFE_SB_Buffer_t *BufPtr);
** -# This function will increment and apply the internally tracked
** sequence counter if set to do so.
**
** \param[in] BufPtr A pointer to the buffer to be sent @nonnull.
** \param[in] UpdateHeader Update the headers of the message
** \param[in] BufPtr A pointer to the buffer to be sent @nonnull.
** \param[in] IsOrigination Update applicable header field(s) of a newly constructed message
**
** \return Execution status, see \ref CFEReturnCodes
** \retval #CFE_SUCCESS \copybrief CFE_SUCCESS
** \retval #CFE_SB_BAD_ARGUMENT \copybrief CFE_SB_BAD_ARGUMENT
** \retval #CFE_SB_MSG_TOO_BIG \copybrief CFE_SB_MSG_TOO_BIG
**/
CFE_Status_t CFE_SB_TransmitBuffer(CFE_SB_Buffer_t *BufPtr, bool UpdateHeader);
CFE_Status_t CFE_SB_TransmitBuffer(CFE_SB_Buffer_t *BufPtr, bool IsOrigination);

/** @} */

Expand Down
32 changes: 32 additions & 0 deletions modules/core_api/ut-stubs/src/cfe_msg_handlers.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,3 +376,35 @@ void UT_DefaultHandler_CFE_MSG_GetNextSequenceCount(void *UserObj, UT_EntryKey_t

UT_Stub_SetReturnValue(FuncKey, return_value);
}

/*------------------------------------------------------------
*
* Default handler for CFE_MSG_OriginationAction coverage stub function
*
*------------------------------------------------------------*/
void UT_DefaultHandler_CFE_MSG_OriginationAction(void *UserObj, UT_EntryKey_t FuncKey, const UT_StubContext_t *Context)
{
bool *IsAcceptable = UT_Hook_GetArgValueByName(Context, "IsAcceptable", bool *);

/* by default just always return true -- a UT case that needs something else can override this handler */
if (IsAcceptable != NULL)
{
*IsAcceptable = true;
}
}

/*------------------------------------------------------------
*
* Default handler for CFE_MSG_VerificationAction coverage stub function
*
*------------------------------------------------------------*/
void UT_DefaultHandler_CFE_MSG_VerificationAction(void *UserObj, UT_EntryKey_t FuncKey, const UT_StubContext_t *Context)
{
bool *IsAcceptable = UT_Hook_GetArgValueByName(Context, "IsAcceptable", bool *);

/* by default just always return true -- a UT case that needs something else can override this handler */
if (IsAcceptable != NULL)
{
*IsAcceptable = true;
}
}
49 changes: 35 additions & 14 deletions modules/core_api/ut-stubs/src/cfe_msg_stubs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ void UT_DefaultHandler_CFE_MSG_GetSubsystem(void *, UT_EntryKey_t, const UT_Stub
void UT_DefaultHandler_CFE_MSG_GetSystem(void *, UT_EntryKey_t, const UT_StubContext_t *);
void UT_DefaultHandler_CFE_MSG_GetType(void *, UT_EntryKey_t, const UT_StubContext_t *);
void UT_DefaultHandler_CFE_MSG_GetTypeFromMsgId(void *, UT_EntryKey_t, const UT_StubContext_t *);
void UT_DefaultHandler_CFE_MSG_OriginationAction(void *, UT_EntryKey_t, const UT_StubContext_t *);
void UT_DefaultHandler_CFE_MSG_ValidateChecksum(void *, UT_EntryKey_t, const UT_StubContext_t *);
void UT_DefaultHandler_CFE_MSG_VerificationAction(void *, UT_EntryKey_t, const UT_StubContext_t *);

/*
* ----------------------------------------------------
Expand Down Expand Up @@ -366,6 +368,24 @@ CFE_Status_t CFE_MSG_Init(CFE_MSG_Message_t *MsgPtr, CFE_SB_MsgId_t MsgId, CFE_M
return UT_GenStub_GetReturnValue(CFE_MSG_Init, CFE_Status_t);
}

/*
* ----------------------------------------------------
* Generated stub function for CFE_MSG_OriginationAction()
* ----------------------------------------------------
*/
CFE_Status_t CFE_MSG_OriginationAction(CFE_MSG_Message_t *MsgPtr, size_t BufferSize, bool *IsAcceptable)
{
UT_GenStub_SetupReturnBuffer(CFE_MSG_OriginationAction, CFE_Status_t);

UT_GenStub_AddParam(CFE_MSG_OriginationAction, CFE_MSG_Message_t *, MsgPtr);
UT_GenStub_AddParam(CFE_MSG_OriginationAction, size_t, BufferSize);
UT_GenStub_AddParam(CFE_MSG_OriginationAction, bool *, IsAcceptable);

UT_GenStub_Execute(CFE_MSG_OriginationAction, Basic, UT_DefaultHandler_CFE_MSG_OriginationAction);

return UT_GenStub_GetReturnValue(CFE_MSG_OriginationAction, CFE_Status_t);
}

/*
* ----------------------------------------------------
* Generated stub function for CFE_MSG_SetApId()
Expand Down Expand Up @@ -623,34 +643,35 @@ CFE_Status_t CFE_MSG_SetType(CFE_MSG_Message_t *MsgPtr, CFE_MSG_Type_t Type)

/*
* ----------------------------------------------------
* Generated stub function for CFE_MSG_UpdateHeader()
* Generated stub function for CFE_MSG_ValidateChecksum()
* ----------------------------------------------------
*/
CFE_Status_t CFE_MSG_UpdateHeader(CFE_MSG_Message_t *MsgPtr, CFE_MSG_SequenceCount_t SeqCnt)
CFE_Status_t CFE_MSG_ValidateChecksum(const CFE_MSG_Message_t *MsgPtr, bool *IsValid)
{
UT_GenStub_SetupReturnBuffer(CFE_MSG_UpdateHeader, CFE_Status_t);
UT_GenStub_SetupReturnBuffer(CFE_MSG_ValidateChecksum, CFE_Status_t);

UT_GenStub_AddParam(CFE_MSG_UpdateHeader, CFE_MSG_Message_t *, MsgPtr);
UT_GenStub_AddParam(CFE_MSG_UpdateHeader, CFE_MSG_SequenceCount_t, SeqCnt);
UT_GenStub_AddParam(CFE_MSG_ValidateChecksum, const CFE_MSG_Message_t *, MsgPtr);
UT_GenStub_AddParam(CFE_MSG_ValidateChecksum, bool *, IsValid);

UT_GenStub_Execute(CFE_MSG_UpdateHeader, Basic, NULL);
UT_GenStub_Execute(CFE_MSG_ValidateChecksum, Basic, UT_DefaultHandler_CFE_MSG_ValidateChecksum);

return UT_GenStub_GetReturnValue(CFE_MSG_UpdateHeader, CFE_Status_t);
return UT_GenStub_GetReturnValue(CFE_MSG_ValidateChecksum, CFE_Status_t);
}

/*
* ----------------------------------------------------
* Generated stub function for CFE_MSG_ValidateChecksum()
* Generated stub function for CFE_MSG_VerificationAction()
* ----------------------------------------------------
*/
CFE_Status_t CFE_MSG_ValidateChecksum(const CFE_MSG_Message_t *MsgPtr, bool *IsValid)
CFE_Status_t CFE_MSG_VerificationAction(const CFE_MSG_Message_t *MsgPtr, size_t BufferSize, bool *IsAcceptable)
{
UT_GenStub_SetupReturnBuffer(CFE_MSG_ValidateChecksum, CFE_Status_t);
UT_GenStub_SetupReturnBuffer(CFE_MSG_VerificationAction, CFE_Status_t);

UT_GenStub_AddParam(CFE_MSG_ValidateChecksum, const CFE_MSG_Message_t *, MsgPtr);
UT_GenStub_AddParam(CFE_MSG_ValidateChecksum, bool *, IsValid);
UT_GenStub_AddParam(CFE_MSG_VerificationAction, const CFE_MSG_Message_t *, MsgPtr);
UT_GenStub_AddParam(CFE_MSG_VerificationAction, size_t, BufferSize);
UT_GenStub_AddParam(CFE_MSG_VerificationAction, bool *, IsAcceptable);

UT_GenStub_Execute(CFE_MSG_ValidateChecksum, Basic, UT_DefaultHandler_CFE_MSG_ValidateChecksum);
UT_GenStub_Execute(CFE_MSG_VerificationAction, Basic, UT_DefaultHandler_CFE_MSG_VerificationAction);

return UT_GenStub_GetReturnValue(CFE_MSG_ValidateChecksum, CFE_Status_t);
return UT_GenStub_GetReturnValue(CFE_MSG_VerificationAction, CFE_Status_t);
}
8 changes: 4 additions & 4 deletions modules/core_api/ut-stubs/src/cfe_sb_stubs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -384,12 +384,12 @@ void CFE_SB_TimeStampMsg(CFE_MSG_Message_t *MsgPtr)
* Generated stub function for CFE_SB_TransmitBuffer()
* ----------------------------------------------------
*/
CFE_Status_t CFE_SB_TransmitBuffer(CFE_SB_Buffer_t *BufPtr, bool UpdateHeader)
CFE_Status_t CFE_SB_TransmitBuffer(CFE_SB_Buffer_t *BufPtr, bool IsOrigination)
{
UT_GenStub_SetupReturnBuffer(CFE_SB_TransmitBuffer, CFE_Status_t);

UT_GenStub_AddParam(CFE_SB_TransmitBuffer, CFE_SB_Buffer_t *, BufPtr);
UT_GenStub_AddParam(CFE_SB_TransmitBuffer, bool, UpdateHeader);
UT_GenStub_AddParam(CFE_SB_TransmitBuffer, bool, IsOrigination);

UT_GenStub_Execute(CFE_SB_TransmitBuffer, Basic, UT_DefaultHandler_CFE_SB_TransmitBuffer);

Expand All @@ -401,12 +401,12 @@ CFE_Status_t CFE_SB_TransmitBuffer(CFE_SB_Buffer_t *BufPtr, bool UpdateHeader)
* Generated stub function for CFE_SB_TransmitMsg()
* ----------------------------------------------------
*/
CFE_Status_t CFE_SB_TransmitMsg(const CFE_MSG_Message_t *MsgPtr, bool UpdateHeader)
CFE_Status_t CFE_SB_TransmitMsg(const CFE_MSG_Message_t *MsgPtr, bool IsOrigination)
{
UT_GenStub_SetupReturnBuffer(CFE_SB_TransmitMsg, CFE_Status_t);

UT_GenStub_AddParam(CFE_SB_TransmitMsg, const CFE_MSG_Message_t *, MsgPtr);
UT_GenStub_AddParam(CFE_SB_TransmitMsg, bool, UpdateHeader);
UT_GenStub_AddParam(CFE_SB_TransmitMsg, bool, IsOrigination);

UT_GenStub_Execute(CFE_SB_TransmitMsg, Basic, UT_DefaultHandler_CFE_SB_TransmitMsg);

Expand Down
2 changes: 1 addition & 1 deletion modules/msg/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
set(${DEP}_SRC
fsw/src/cfe_msg_ccsdspri.c
fsw/src/cfe_msg_init.c
fsw/src/cfe_msg_verify.c
fsw/src/cfe_msg_integrity.c
fsw/src/cfe_msg_msgid_shared.c
fsw/src/cfe_msg_sechdr_checksum.c
fsw/src/cfe_msg_sechdr_fc.c
Expand Down
31 changes: 0 additions & 31 deletions modules/msg/fsw/src/cfe_msg_init.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,34 +53,3 @@ CFE_Status_t CFE_MSG_Init(CFE_MSG_Message_t *MsgPtr, CFE_SB_MsgId_t MsgId, CFE_M

return status;
}

/*----------------------------------------------------------------
*
* Implemented per public API
* See description in header file for argument/return detail
*
*-----------------------------------------------------------------*/
CFE_Status_t CFE_MSG_UpdateHeader(CFE_MSG_Message_t *MsgPtr, CFE_MSG_SequenceCount_t SeqCnt)
{
if (MsgPtr == NULL)
{
return CFE_MSG_BAD_ARGUMENT;
}

/* Sequence count is in the basic CCSDS Primary Hdr, so all msgs have it */
CFE_MSG_SetSequenceCount(MsgPtr, SeqCnt);

/*
* TLM packets have a timestamp in the secondary header.
* This may fail if this is not a TLM packet (that is OK)
*/
CFE_MSG_SetMsgTime(MsgPtr, CFE_TIME_GetTime());

/*
* CMD packets have a checksum in the secondary header.
* This may fail if this is not a CMD packet (that is OK)
*/
CFE_MSG_GenerateChecksum(MsgPtr);

return CFE_SUCCESS;
}
Loading

0 comments on commit c4141f9

Please sign in to comment.