Skip to content

Conversation

@jphickey
Copy link
Contributor

Checklist (Please check before submitting)

Describe the contribution
Rework state machines such that all state changes are performed in the tick processor, rather than scattered around in various parts of PDU processing.

This removes many dependencies and assumptions about the order of arrival of PDUs and makes the protocol handling more robust and accurate per the blue book spec.

Notably, this fixes a number of important concerns:

  • Out of order MD arrival is no longer an exception, the state machine is identical regardless if MD comes first or not.
  • Out of order arrival is handled in class 1 (PDUs can arrive in any order)
  • Logic for file retention is consistently applied in both success and error cases - always goes through this part of the state machine.
  • Allows transferring a zero byte file (previously would error out because of the way the "read" call was reached).
  • Files are no longer updated in place. This means an existing file will not be clobbered until a complete new file is received.
  • Bit errors in an FD PDU header simply drop the PDU (it will be NAKed) rather than aborting the entire transaction.
  • ACK/NAK counting is accurately implemented in class 2.
  • The CC response in EOF/FIN packets will be accurate when the ACK limit is reached.
  • Tx-side tick processing simplified, there is now just one tick processor, all the special "cont" and "early_exit" flags removed.

The bottom line: all transactions will proceed through the state machine in the exact same way, whether class 1 or class 2, with transitions all handled in the same place and in a consistent manner, going through all the same checks and validations on every transition in every transaction, regardless of PDU arrival order. This greatly improves adherence to the CFDP protocol spec and ability to deal with imperfect PDU delivery.

Bonus item: The "closure request" feature of class 1 transfers now is supported and works in the engine. (Only missing part is a way to request it when initiating a Tx transaction)

Fixes #481

Testing performed
Execute various file transfers, class 1 and class 2.
Use python script to force protocol anomalies such as out-of-order PDUs and PDUs with errors, confirm correct handling.

Expected behavior changes
Listed above

System(s) tested on
Linux

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@jphickey jphickey force-pushed the fix-481-state-cleanup branch from 8ab71f8 to 04c1d45 Compare November 13, 2025 15:10
@jphickey jphickey force-pushed the fix-481-state-cleanup branch 2 times, most recently from c3a4ea1 to 9191928 Compare November 13, 2025 16:03
Rework state machines such that all state changes are performed in the
tick processor, rather than scattered around in various parts of PDU
processing.

This removes many dependencies and assumptions about the order of
arrival of PDUs and makes the protocol handling more robust and accurate
per the blue book spec.

Notably, this fixes a number of important concerns:
 - Out of order MD arrival is no longer an exception, the state machine
   is identical regardless if MD comes first or not.
 - Out of order arrival is handled in class 1 (PDUs can arrive in any
   order)
 - Logic for file retention is consistently applied in both success and
   error cases - always goes through this part of the state machine.
 - Allows transferring a zero byte file (previously would error out
   because of the way the "read" call was reached).
 - Files are no longer updated in place.  This means an existing file
   will not be clobbered until a complete new file is received.
 - Bit errors in an FD PDU header simply drop the PDU (it will be NAKed)
   rather than aborting the entire transaction.
 - ACK/NAK counting is accurately implemented in class 2.
 - The CC response in EOF/FIN packets will be accurate when the ACK
   limit is reached.
 - Tx-side tick processing simplified, there is now just one tick
   processor, all the special "cont" and "early_exit" flags removed.

The bottom line: all transactions will proceed through the state machine
in the exact same way, whether class 1 or class 2, with transitions all
handled in the same place and in a consistent manner, going through all
the same checks and validations on every transition in every
transaction, regardless of PDU arrival order.  This greatly improves
adherence to the CFDP protocol spec and ability to deal with imperfect
PDU delivery.

Bonus item:  The "closure request" feature of class 1 transfers now is
supported and works in the engine.  (Only missing part is a way to
request it when initiating a Tx transaction)
@jphickey jphickey force-pushed the fix-481-state-cleanup branch from 9191928 to a0914c4 Compare November 14, 2025 15:41
@jphickey jphickey added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Nov 17, 2025
@dzbaker dzbaker added CCB:Approved and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels Dec 3, 2025
@dzbaker dzbaker merged commit c6fe5f4 into nasa:main Dec 17, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Always create a temp file for received data

3 participants