search

nasa / CF

The Core Flight System (cFS) CFDP application.
Apache License 2.0
79 stars 45 forks source link

Support for segment metadata on File Data PDU #129

Closed jphickey closed 2 years ago

jphickey commented 2 years ago

According to CCSDS 727.0-B-5, table 5-14, a file data PDU may have segment metadata included. This is indicated by a flag being set in the main PDU header, which means the data PDU has two extra fields as well as the specified number of segment metadata information blocks.

Currently CF does not even check for this bit or the presence of these fields.

While it may not be required to support it, on the receive side CF should be required to check for and actively discard/reject packets for which these bits are set (and therefore it does not understand). Instead, as it stands right now, it will read the extra fields as part of the offset, and generally corrupt the entire file, if this flag is set by a sender. Hopefully the CRC check would detect the corruption, but it shouldn't do that to begin with.

Current code that receives the file data header only reads a single offset field, there is no provision to check for and handle the extra fields here, just the offset:

https://github.com/nasa/CF/blob/a894069439316ef89ad7751f3a03036930158a07/fsw/src/cf_cfdp.c#L1003

jphickey commented 2 years ago

calling this a bug because if a sender ever includes these extra fields, CF will not interpret the packets correctly, will misinterpret the data as offset, and corrupt the file.

jphickey commented 2 years ago

Recommendation for now is to at least check if this bit is set on recv, and reject it if so. This will at least solve misinterpreting the bits problem, which is the big issue. Support for this feature can be added in a future version, if desired.

skliper commented 2 years ago

Concur.

jphickey commented 2 years ago

Clarification - actually PR #137 solves the bit-misinterpretation problem, but still leaves the problem if that this information is inside a PDU, then it is ignored by CF, which may or may not be a real problem. Still think rejecting and reporting is the best course of action, so at least raises awareness that there is an incompatibility between the two CF implementations, rather than silently ignoring it.