[x] I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.
Describe the bug
When 2 entities are transferring a file in Reliable mode, if a Cancel command is sent to the sender entity, there appears to be something broken with the resulting handshaking with the receiver entity, which ultimately does not allow the sender entity to terminate the transaction gracefully.
When the sender receives a Cancel command, it issues an EOF (cancel) PDU to the receiver. Upon receiving this EOF-PDU, the receiver immediately terminates, without any response to the sender. This results in the sender being stuck in a wait-for-EOF-Ack state and only terminating when the inactivity timer is reached.
To Reproduce
Steps to reproduce the behavior:
Start a file transfer between 2 CFDP entities in Reliable mode
Send a Cancel command to the sender entity
Observe that receiver entity immediately terminates the transaction (whereas per the CFDP specification, it should send an EOF-Ack, a FIN PDU, and waiting for the FIN-Ack from the sender before terminating)
Observe that the sender entity does not terminate correctly because it continues to wait for the EOF-Ack from the receiver
Expected behavior
File transfer in Reliable mode started between 2 entities
Cancel command sent to sender entity
Sender entity issues a EOF (cancel) PDU to the receiver entity
Receiver entity issues a EOF-Ack PDU to the sender entity
Receiver entity issues a FIN PDU to the sender entity
Sender entity issues a FIN-Ack PDU to the receiver entity and terminates
Receiver entity terminates upon receiving the FIN-Ack
Code snips
I believe issue is in the CF_CFDP_R2_SubstateRecvEof function logic in cf_cfdp_r.c.
The logic defers sending of the EOF-Ack until after CF_CFDP_R2_SubstateRecvEof runs to completion:
/* always ack the EOF, even if we're not done */t->state_data.r.r2.eof_cc = FGV(eof->cc, PDU_FLAGS_CC);t->flags.rx.send_ack = 1; /* defer sending ack to tick handling */
EOF (cancel) PDU then follows a path that eventually results in a call to CF_CFDP_R2_Reset:
if(t->state_data.r.r2.eof_cc==CC_NO_ERROR) {CF_CFDP_R2_Complete(t, 1); /* CF_CFDP_R2_Complete() will change state */}else {CF_CFDP_R2_Reset(t);}
Then within CF_CFDP_R2_Reset, the current transaction is already reset and freed.
if((t->state_data.r.sub_state==RECV_WAIT_FOR_FIN_ACK)||(t->state_data.r.r2.eof_cc!=CC_NO_ERROR)||(t->history->cc!=CC_NO_ERROR)||t->flags.rx.canceled) {CF_CFDP_R1_Reset(t); /* it's done */}
My fix: added an "else if" section to CF_CFDP_R2_SubstateRecvEof:
else if (t->state_data.r.r2.eof_cc == CF_CFDP_ConditionCode_CANCEL_REQUEST_RECEIVED) {t->flags.rx.send_fin = 1;t->history->txn_stat = CF_CFDP_ConditionCode_CANCEL_REQUEST_RECEIVED;}
Checklist (Please check before submitting)
Describe the bug When 2 entities are transferring a file in Reliable mode, if a Cancel command is sent to the sender entity, there appears to be something broken with the resulting handshaking with the receiver entity, which ultimately does not allow the sender entity to terminate the transaction gracefully.
When the sender receives a Cancel command, it issues an EOF (cancel) PDU to the receiver. Upon receiving this EOF-PDU, the receiver immediately terminates, without any response to the sender. This results in the sender being stuck in a wait-for-EOF-Ack state and only terminating when the inactivity timer is reached.
To Reproduce Steps to reproduce the behavior:
Expected behavior
Code snips I believe issue is in the CF_CFDP_R2_SubstateRecvEof function logic in cf_cfdp_r.c.
The logic defers sending of the EOF-Ack until after CF_CFDP_R2_SubstateRecvEof runs to completion:
/* always ack the EOF, even if we're not done */t->state_data.r.r2.eof_cc = FGV(eof->cc, PDU_FLAGS_CC);t->flags.rx.send_ack = 1; /* defer sending ack to tick handling */EOF (cancel) PDU then follows a path that eventually results in a call to CF_CFDP_R2_Reset:
if(t->state_data.r.r2.eof_cc==CC_NO_ERROR) {CF_CFDP_R2_Complete(t, 1); /* CF_CFDP_R2_Complete() will change state */}else {CF_CFDP_R2_Reset(t);}Then within CF_CFDP_R2_Reset, the current transaction is already reset and freed.
if((t->state_data.r.sub_state==RECV_WAIT_FOR_FIN_ACK)||(t->state_data.r.r2.eof_cc!=CC_NO_ERROR)||(t->history->cc!=CC_NO_ERROR)||t->flags.rx.canceled) {CF_CFDP_R1_Reset(t); /* it's done */}My fix: added an "else if" section to CF_CFDP_R2_SubstateRecvEof:
else if (t->state_data.r.r2.eof_cc == CF_CFDP_ConditionCode_CANCEL_REQUEST_RECEIVED) {t->flags.rx.send_fin = 1;t->history->txn_stat = CF_CFDP_ConditionCode_CANCEL_REQUEST_RECEIVED;}System observed on:
Reporter Info Katie Chou