Skip checksum validation on ATS/RTS table entries

[jphickey]

Checklist (Please check before submitting)

• I reviewed the Contributing Guide.
• I reviewed the README file to see if the feature is in the major future work.
• I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
When reading ATS/RTS table entries, SC validates the checksum on the entry before transmitting the message.

In order for this to validate, this implies that the ATS/RTS table embeds a correct message checksum within the message data. However, this is not easy to do, and there is no tool to aid in this process (either via elf2cfetbl or some other table-generation tool).

Furthermore, if the outgoing command gets a sequence number or timestamp update, this necessitates [re]computing the checksum.

Describe the solution you'd like
Add an option to SC to skip the call to CFE_MSG_ValidateChecksum()

Describe alternatives you've considered
Could be removed entirely. Not sure what the original requirement was that justified having this in there.

Additional context
Note that the ATS/RTS info comes from a table, and the table data should be protected by its own checksum and validation. So requiring/expecting that the data inside that also have a checksum is redundant, in addition to being impractical.

The two calls are here:

SC/fsw/src/sc_cmds.c

Line 104 in afe3cf4

CFE_MSG_ValidateChecksum(&EntryPtr->Msg, &ChecksumValid);

And here:

SC/fsw/src/sc_cmds.c

Line 332 in afe3cf4

CFE_MSG_ValidateChecksum(&EntryPtr->Msg, &ChecksumValid);

Requester Info
Joseph Hickey, Vantage Systems, Inc.

[skliper]

Note it's clunky and problematic for complex commands, but I just use macros in the rts tables as a workaround. Ex:

#define SC_DSFILTERFILE_CKSUM(msgid, fileidx) ((0x96 ^ (msgid >> 8) ^ msgid ^ fileidx) & 0xFF)
#define SC_DSFILTERFILE_ENTRY(time, msgid, fileidx)                                             \
    {                                                                                           \
        {.TimeTag = time},                                                                      \
        {                                                                                       \
            CFE_MSG_CMD_HDR_INIT(DS_CMD_MID, sizeof(DS_FilterFileCmd_t), DS_SET_FILTER_FILE_CC, \
                                 SC_DSFILTERFILE_CKSUM(msgid, fileidx)),                        \
            {                                                                                   \
                .MessageID = CFE_SB_MSGID_WRAP_VALUE(msgid), .FileTableIndex = fileidx          \
            }                                                                                   \
        }                                                                                       \
    }

[skliper]

Above breaks for changes in DS_CMD_MID or CC values, but that could be handled (I just don't). If someone did use sequence or time stamp then definitely breaks. Although that's beyond what SC can handle anyways at this point.

[jphickey]

Yes - sorta/kinda possible for the trivial checksum in the default cmd header.

Stakeholders are using a lot more complex checksum that makes this kinda thing much more difficult. Furthermore, some header fields are updated in real time (timestamping, sequence numbering) that makes any sort of initial checksum irrelevant, it has to be calculated anyway.

The main question is - would it cause any heartburn to simply remove this call to ValidateChecksum()? Doesn't seem to be a strong reason to have it in there. Other apps that send commands from a table (e.g. HS) don't do this. I don't think removing this will break any use cases, but it will permit use cases that are needed by the stakeholders.

[skliper]

I'm not opposed to removing based on how I use the app and construct the tables.

[jphickey]

Feedback from other stakeholders indicates this should be made into an "opt-out" platform config option. Thus this will preserve historical behavior (to validate the checksum prior to send) as a default, but give users who have a complex checksum and/or need to recalculate it anyway the option to do so.

[skliper]

@jphickey any stakeholder able to provide historical justification for this checksum?

[jphickey]

@jphickey any stakeholder able to provide historical justification for this checksum?

In short -- no, not really. Only that it gave "reassurance" that the message wasn't bad. The fact that we have other mechanisms to do error checking (TBL and CS) that already confirm the table is good didn't seem to sway opinion.

In the end, decided to name this as a "header update" switch. It directly controls the 2nd parameter to CFE_SB_TransmitMsg. But if set true, this also disables/bypasses the ValidateChecksum call prior to TransmitMsg, as its going to be recalculated.