Merge pull request #438 from skliper/fix19-clarify_pdu_sizing

Fix #19, Clarify PDU size limits

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.
  */

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

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 <BR>
   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". </LI>
       </UL>
   </UL>
-**/
+**/

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
 {

fsw/src/cf_cfdp_types.h

@@ -410,24 +410,16 @@ 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
  *
  * Keeps the state of the current output PDU in the CF engine
  */
 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;
 
 /**