From 14a0cb39452d11bf74155dda418039d541be568a Mon Sep 17 00:00:00 2001 From: Jacob Hageman Date: Wed, 13 Mar 2024 21:09:10 +0000 Subject: [PATCH] Fix #19, Clarify PDU size limits --- config/default_cf_interface_cfg.h | 23 ++++++++++++++--------- config/default_cf_tblstruct.h | 6 +++--- docs/dox_src/cfs_cf.dox | 29 +++++++++++++---------------- fsw/src/cf_cfdp_pdu.h | 10 ++++++---- fsw/src/cf_cfdp_types.h | 24 ++++++------------------ 5 files changed, 42 insertions(+), 50 deletions(-) diff --git a/config/default_cf_interface_cfg.h b/config/default_cf_interface_cfg.h index 4fab3120..577d72ee 100644 --- a/config/default_cf_interface_cfg.h +++ b/config/default_cf_interface_cfg.h @@ -82,16 +82,21 @@ * @brief Max PDU size. * * @par Description: - * The max PDU size across all channels in the system. Keep in mind that - * the max filedata PDU will be smaller than this. This size includes - * the PDU headers and everything. While this is the max value for all - * channels, the outgoing_file_chunk_size in the configuration table - * is different for each channel so a smaller size can be used. + * Limits the maximum possible Tx PDU size. Note the resulting CCSDS packet + * also includes a CCSDS header and CF_PDU_ENCAPSULATION_EXTRA_TRAILING_BYTES. + * The outgoing file data chunk size is also limited from the table configuration + * or by set parameter command, which is checked against this value + * (+ smallest possible PDU header). + * + * @par Note: + * This does NOT limit Rx PDUs, since the file data is written from + * the transport packet to the file. * * @par Limits: + * Since PDUs are wrapped in CCSDS packets, need to respect any + * CCSDS packet size limits on the system. * */ -/* CF_MAX_PDU_SIZE must be the max possible PDU for any channel. Channels can be configured with a smaller max. */ #define CF_MAX_PDU_SIZE (512) /** @@ -113,8 +118,8 @@ /** * @brief Number of trailing bytes to add to CFDP PDU * - * @par Description - *    Additional padding bytes to be appended to the tail of CFDP PDUs + * @par Description + * Additional padding bytes to be appended to the tail of CFDP PDUs * This reserves extra space to the software bus encapsulation buffer for every * CFDP PDU such that platform-specific trailer information may be added. This * includes, but is not limited to a separate CRC or error control field in addition @@ -126,7 +131,7 @@ * Set to 0 to disable this feature, such that the software bus buffer * encapsulates only the CFDP PDU and no extra bytes are added. * - *  @par Limits: + * @par Limits: * Maximum value is the difference between the maximum size of a CFDP PDU and the * maximum size of an SB message. */ diff --git a/config/default_cf_tblstruct.h b/config/default_cf_tblstruct.h index bf78a073..30df4779 100644 --- a/config/default_cf_tblstruct.h +++ b/config/default_cf_tblstruct.h @@ -48,9 +48,9 @@ typedef struct CF_ConfigTable CF_ChannelConfig_t chan[CF_NUM_CHANNELS]; /**< \brief Channel configuration */ - uint16 outgoing_file_chunk_size; /**< maximum size of outgoing file data PDUs - must be - * smaller than file data character array */ - char tmp_dir[CF_FILENAME_MAX_PATH]; /**< directory to put temp files */ + uint16 outgoing_file_chunk_size; /**< \brief maximum size of outgoing file data chunk in a PDU. + * Limited by CF_MAX_PDU_SIZE minus the PDU header(s) */ + char tmp_dir[CF_FILENAME_MAX_PATH]; /**< \brief directory to put temp files */ } CF_ConfigTable_t; #endif diff --git a/docs/dox_src/cfs_cf.dox b/docs/dox_src/cfs_cf.dox index 0d5d9138..f5022144 100644 --- a/docs/dox_src/cfs_cf.dox +++ b/docs/dox_src/cfs_cf.dox @@ -452,21 +452,18 @@ configuration table. When the queue entry is 'pushed-off' the history queue, the memory for the queue will be available. - For incoming file transactions, CF uses a statically allocated buffer for the - incoming PDU. The size of this buffer is defined by the platform configuration - parameter, CF_MAX_PDU_SIZE. The incoming PDU's are copied from the Software Bus - into this buffer and then passed to the engine. - - For outgoing file transactions, the engine uses a statically allocated buffer for - PDUs. The size of this buffer is defined by platform configuration parameter, - CF_MAX_PDU_SIZE. The engine informs the CF app when it has a PDU - ready to go out. In response to this, the CF app checks with the downlink app - (e.g. TO) to see if it is ready to receive a PDU. This is done by the CF app - trying to 'take' the throttling semaphore defined in the CF configuration - table. If the CF app successfully 'takes' the semaphore, it gives a green - light to the engine and the PDU is then released by the engine and sent to the - software bus via the zero-copy delivery mode. There is a green light counter - and a red light counter for each output channel in telemetry. + For incoming file transactions, CF writes file data from the incoming packet + directly to the file without the use of an internal buffer. Incoming transaction + size is limited by the underlying transport layer. + + For outgoing file transactions CF requests a buffer from SB sized for + CF_MAX_PDU_SIZE, the CCSDS header size, and CF_PDU_ENCAPSULATION_EXTRA_TRAILING_BYTES. + Outgoing packets are then filed up to this maximum value for transmission. Note + the maximum outgoing size can also be limited by the table or set parameter command + for outgoing_file_chunk_size. + + Generation of packets can be flow controlled via a throttling semaphore + defined in the CF configuration table. Prev: \ref cfscfovrdsn
Next: \ref cfscfdg @@ -1495,4 +1492,4 @@ CF_UnionArgs_Payload_t; increase the "inactivity timer" or increase the "max number of bytes per wakeup to calculate CRC". -**/ \ No newline at end of file +**/ diff --git a/fsw/src/cf_cfdp_pdu.h b/fsw/src/cf_cfdp_pdu.h index d60f19d6..e0cc4ff4 100644 --- a/fsw/src/cf_cfdp_pdu.h +++ b/fsw/src/cf_cfdp_pdu.h @@ -369,11 +369,13 @@ typedef struct CF_CFDP_PduFileDataHeader } CF_CFDP_PduFileDataHeader_t; /** - * @brief PDU file data content + * @brief + * PDU file data content typedef for limit checking outgoing_file_chunk_size + * table value and set parameter command. * - * To serve as a sanity check, this should accommodate the largest data block possible. - * In that light, it should be sized based on the minimum encoded header size, rather than - * the maximum, as that case leaves the most space for data. + * This definition allows for the largest data block possible, as CF_MAX_PDU_SIZE - + * the minimum possible header size. In practice the outgoing file chunk size is limited by + * whichever is smaller; the remaining data, remaining space in the packet, and outgoing_file_chunk_size. */ typedef struct CF_CFDP_PduFileDataContent { diff --git a/fsw/src/cf_cfdp_types.h b/fsw/src/cf_cfdp_types.h index f2051511..a7da8a4c 100644 --- a/fsw/src/cf_cfdp_types.h +++ b/fsw/src/cf_cfdp_types.h @@ -410,10 +410,6 @@ typedef struct CF_Channel uint8 tick_type; } CF_Channel_t; -/* NOTE: the use of CF_CFDP_PduHeader_t below is correct, but the CF_PduRecvMsg_t and CF_PduSendMsg_t - * structures are both longer than these definitions. They are always backed by a buffer - * of size CF_MAX_PDU_SIZE */ - /** * @brief CF engine output state * @@ -421,13 +417,9 @@ typedef struct CF_Channel */ typedef struct CF_Output { - CFE_SB_Buffer_t * msg; /**< \brief Binary message to be sent to underlying transport */ - CF_EncoderState_t encode; /**< \brief Encoding state (while building message) */ - - /** - * \brief Temporary R/W buffer for holding output PDUs while working with them - */ - CF_Logical_PduBuffer_t tx_pdudata; + CFE_SB_Buffer_t *msg; /**< \brief Binary message to be sent to underlying transport */ + CF_EncoderState_t encode; /**< \brief Encoding state (while building message) */ + CF_Logical_PduBuffer_t tx_pdudata; /**< \brief Tx PDU logical values */ } CF_Output_t; /** @@ -437,13 +429,9 @@ typedef struct CF_Output */ typedef struct CF_Input { - CFE_SB_Buffer_t * msg; /**< \brief Binary message received from underlying transport */ - CF_DecoderState_t decode; /**< \brief Decoding state (while interpreting message) */ - - /** - * \brief Temporary R/W buffer for holding input PDUs while working with them - */ - CF_Logical_PduBuffer_t rx_pdudata; + CFE_SB_Buffer_t *msg; /**< \brief Binary message received from underlying transport */ + CF_DecoderState_t decode; /**< \brief Decoding state (while interpreting message) */ + CF_Logical_PduBuffer_t rx_pdudata; /**< \brief Rx PDU logical values */ } CF_Input_t; /**