From 9147cd1332abbbaf3b2fd8cfca8b7f48b1c8150b Mon Sep 17 00:00:00 2001 From: "Mathias Jochum, Servus Intralogistics" Date: Thu, 20 Mar 2025 16:40:13 +0100 Subject: [PATCH 1/3] Use the expected sequence number for challenge ACK messages That avoids avoid endless ACK/RST retransmissions. FreeRTOS-Plus-TCP issue #1235 --- source/FreeRTOS_TCP_IP.c | 2 +- source/FreeRTOS_TCP_Transmission.c | 11 ++++++++++- source/include/FreeRTOS_TCP_Transmission.h | 3 ++- .../FreeRTOS_TCP_Transmission_utest.c | 2 +- 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/source/FreeRTOS_TCP_IP.c b/source/FreeRTOS_TCP_IP.c index 6ba71a33f2..4909f42ea7 100644 --- a/source/FreeRTOS_TCP_IP.c +++ b/source/FreeRTOS_TCP_IP.c @@ -882,7 +882,7 @@ pxSocket->u.xTCP.xTCPWindow.xSize.ulRxWindowLength ) != pdFALSE ) ) { /* Send a challenge ACK. */ - ( void ) prvTCPSendChallengeAck( pxNetworkBuffer ); + ( void ) prvTCPSendChallengeAck( pxNetworkBuffer, pxSocket->u.xTCP.xTCPWindow.rx.ulCurrentSequenceNumber ); } else { diff --git a/source/FreeRTOS_TCP_Transmission.c b/source/FreeRTOS_TCP_Transmission.c index 3b0abe63f7..ad4db00388 100644 --- a/source/FreeRTOS_TCP_Transmission.c +++ b/source/FreeRTOS_TCP_Transmission.c @@ -1352,11 +1352,20 @@ * unexpected but still within the window. * * @param[in] pxNetworkBuffer The network buffer descriptor with the packet. + * @param[in] ulCurrentSequenceNumber The current sequence number of the connection. * * @return Returns the value back from #prvTCPSendSpecialPacketHelper. */ - BaseType_t prvTCPSendChallengeAck( NetworkBufferDescriptor_t * pxNetworkBuffer ) + BaseType_t prvTCPSendChallengeAck( NetworkBufferDescriptor_t * pxNetworkBuffer, + uint32_t ulCurrentSequenceNumber ) { + ProtocolHeaders_t * pxProtocolHeaders = ( ( ProtocolHeaders_t * ) + &( pxNetworkBuffer->pucEthernetBuffer[ ipSIZE_OF_ETH_HEADER + uxIPHeaderSizePacket( pxNetworkBuffer ) ] ) ); + + /* Correct the sequence number to avoid an endless ACK/RST sequence. When sending the + * same number again the next RST will not match as well. */ + pxProtocolHeaders->xTCPHeader.ulSequenceNumber = FreeRTOS_htonl( ulCurrentSequenceNumber ); + return prvTCPSendSpecialPacketHelper( pxNetworkBuffer, tcpTCP_FLAG_ACK ); } /*-----------------------------------------------------------*/ diff --git a/source/include/FreeRTOS_TCP_Transmission.h b/source/include/FreeRTOS_TCP_Transmission.h index 1e48fbc30a..e1259a4f21 100644 --- a/source/include/FreeRTOS_TCP_Transmission.h +++ b/source/include/FreeRTOS_TCP_Transmission.h @@ -149,7 +149,8 @@ BaseType_t prvSendData( FreeRTOS_Socket_t * pxSocket, * case #3. In summary, an RST was received with a sequence number that is * unexpected but still within the window. */ -BaseType_t prvTCPSendChallengeAck( NetworkBufferDescriptor_t * pxNetworkBuffer ); +BaseType_t prvTCPSendChallengeAck( NetworkBufferDescriptor_t * pxNetworkBuffer, + uint32_t ulCurrentSequenceNumber ); /* * Reply to a peer with the RST flag on, in case a packet can not be handled. diff --git a/test/unit-test/FreeRTOS_TCP_Transmission/FreeRTOS_TCP_Transmission_utest.c b/test/unit-test/FreeRTOS_TCP_Transmission/FreeRTOS_TCP_Transmission_utest.c index bdfae76934..e2c5b157fb 100644 --- a/test/unit-test/FreeRTOS_TCP_Transmission/FreeRTOS_TCP_Transmission_utest.c +++ b/test/unit-test/FreeRTOS_TCP_Transmission/FreeRTOS_TCP_Transmission_utest.c @@ -2610,7 +2610,7 @@ void test_prvTCPSendChallengeAck( void ) eARPGetCacheEntry_ExpectAnyArgsAndReturn( eResolutionCacheHit ); eARPGetCacheEntry_ReturnThruPtr_ppxEndPoint( &pxEndPoint ); - Return = prvTCPSendChallengeAck( pxNetworkBuffer ); + Return = prvTCPSendChallengeAck( pxNetworkBuffer, 0x3333 ); TEST_ASSERT_EQUAL( pdFALSE, Return ); TEST_ASSERT_EQUAL( 1, NetworkInterfaceOutputFunction_Stub_Called ); TEST_ASSERT_EQUAL( tcpTCP_FLAG_ACK, pxTCPPacket->xTCPHeader.ucTCPFlags ); From 877fe74d0fd000f6e8ea0c2503f6cce8b3b30ef0 Mon Sep 17 00:00:00 2001 From: "Mathias Jochum, Servus Intralogistics" Date: Mon, 24 Mar 2025 15:29:27 +0100 Subject: [PATCH 2/3] Corrected acknowledge number in challenge ACK packets --- source/FreeRTOS_TCP_IP.c | 3 ++- source/FreeRTOS_TCP_Transmission.c | 14 +++++++++++--- source/include/FreeRTOS_TCP_Transmission.h | 3 ++- .../FreeRTOS_TCP_Transmission_utest.c | 4 +++- 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/source/FreeRTOS_TCP_IP.c b/source/FreeRTOS_TCP_IP.c index 4909f42ea7..12a2c4c0c4 100644 --- a/source/FreeRTOS_TCP_IP.c +++ b/source/FreeRTOS_TCP_IP.c @@ -882,7 +882,8 @@ pxSocket->u.xTCP.xTCPWindow.xSize.ulRxWindowLength ) != pdFALSE ) ) { /* Send a challenge ACK. */ - ( void ) prvTCPSendChallengeAck( pxNetworkBuffer, pxSocket->u.xTCP.xTCPWindow.rx.ulCurrentSequenceNumber ); + ( void ) prvTCPSendChallengeAck( pxNetworkBuffer, pxSocket->u.xTCP.xTCPWindow.rx.ulCurrentSequenceNumber, + pxSocket->u.xTCP.xTCPWindow.ulOurSequenceNumber ); } else { diff --git a/source/FreeRTOS_TCP_Transmission.c b/source/FreeRTOS_TCP_Transmission.c index ad4db00388..f752f31980 100644 --- a/source/FreeRTOS_TCP_Transmission.c +++ b/source/FreeRTOS_TCP_Transmission.c @@ -1353,18 +1353,26 @@ * * @param[in] pxNetworkBuffer The network buffer descriptor with the packet. * @param[in] ulCurrentSequenceNumber The current sequence number of the connection. + * @param[in] ulOurSequenceNumber The SEQ number to send out. * * @return Returns the value back from #prvTCPSendSpecialPacketHelper. */ BaseType_t prvTCPSendChallengeAck( NetworkBufferDescriptor_t * pxNetworkBuffer, - uint32_t ulCurrentSequenceNumber ) + uint32_t ulCurrentSequenceNumber, + uint32_t ulOurSequenceNumber ) { ProtocolHeaders_t * pxProtocolHeaders = ( ( ProtocolHeaders_t * ) &( pxNetworkBuffer->pucEthernetBuffer[ ipSIZE_OF_ETH_HEADER + uxIPHeaderSizePacket( pxNetworkBuffer ) ] ) ); - /* Correct the sequence number to avoid an endless ACK/RST sequence. When sending the - * same number again the next RST will not match as well. */ + /* In https://tools.ietf.org/html/rfc5961#section-3.2 the field values are defined as follows: + * + * + * + * The prvTCPSendSpecialPacketHelper function uses the sequence number of the packet as the + * ACK number and the ACK number as the sequence number, therefore the values are set swapped + * here to match the RFC. */ pxProtocolHeaders->xTCPHeader.ulSequenceNumber = FreeRTOS_htonl( ulCurrentSequenceNumber ); + pxProtocolHeaders->xTCPHeader.ulAckNr = FreeRTOS_htonl( ulOurSequenceNumber ); return prvTCPSendSpecialPacketHelper( pxNetworkBuffer, tcpTCP_FLAG_ACK ); } diff --git a/source/include/FreeRTOS_TCP_Transmission.h b/source/include/FreeRTOS_TCP_Transmission.h index e1259a4f21..8b030f4caa 100644 --- a/source/include/FreeRTOS_TCP_Transmission.h +++ b/source/include/FreeRTOS_TCP_Transmission.h @@ -150,7 +150,8 @@ BaseType_t prvSendData( FreeRTOS_Socket_t * pxSocket, * unexpected but still within the window. */ BaseType_t prvTCPSendChallengeAck( NetworkBufferDescriptor_t * pxNetworkBuffer, - uint32_t ulCurrentSequenceNumber ); + uint32_t ulCurrentSequenceNumber, + uint32_t ulOurSequenceNumber ); /* * Reply to a peer with the RST flag on, in case a packet can not be handled. diff --git a/test/unit-test/FreeRTOS_TCP_Transmission/FreeRTOS_TCP_Transmission_utest.c b/test/unit-test/FreeRTOS_TCP_Transmission/FreeRTOS_TCP_Transmission_utest.c index e2c5b157fb..6dc8bc64de 100644 --- a/test/unit-test/FreeRTOS_TCP_Transmission/FreeRTOS_TCP_Transmission_utest.c +++ b/test/unit-test/FreeRTOS_TCP_Transmission/FreeRTOS_TCP_Transmission_utest.c @@ -2610,11 +2610,13 @@ void test_prvTCPSendChallengeAck( void ) eARPGetCacheEntry_ExpectAnyArgsAndReturn( eResolutionCacheHit ); eARPGetCacheEntry_ReturnThruPtr_ppxEndPoint( &pxEndPoint ); - Return = prvTCPSendChallengeAck( pxNetworkBuffer, 0x3333 ); + Return = prvTCPSendChallengeAck( pxNetworkBuffer, 0x3333, 0x4444 ); TEST_ASSERT_EQUAL( pdFALSE, Return ); TEST_ASSERT_EQUAL( 1, NetworkInterfaceOutputFunction_Stub_Called ); TEST_ASSERT_EQUAL( tcpTCP_FLAG_ACK, pxTCPPacket->xTCPHeader.ucTCPFlags ); TEST_ASSERT_EQUAL( 0x50, pxTCPPacket->xTCPHeader.ucTCPOffset ); + TEST_ASSERT_EQUAL( 0x3333, FreeRTOS_ntohl( pxTCPPacket->xTCPHeader.ulAckNr ) ); + TEST_ASSERT_EQUAL( 0x4444, FreeRTOS_ntohl( pxTCPPacket->xTCPHeader.ulSequenceNumber ) ); } /* test prvTCPSendReset function */ From 9c5a2161cd39cec3356c198f7c58a352d4490201 Mon Sep 17 00:00:00 2001 From: Mathias Jochum <156287669+mj-servus@users.noreply.github.com> Date: Tue, 25 Mar 2025 13:32:50 +0100 Subject: [PATCH 3/3] Update source/FreeRTOS_TCP_Transmission.c Co-authored-by: Ahmed Ismail <64546783+AhmedIsmail02@users.noreply.github.com> --- source/FreeRTOS_TCP_Transmission.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/FreeRTOS_TCP_Transmission.c b/source/FreeRTOS_TCP_Transmission.c index f752f31980..1e25476e5b 100644 --- a/source/FreeRTOS_TCP_Transmission.c +++ b/source/FreeRTOS_TCP_Transmission.c @@ -1352,7 +1352,7 @@ * unexpected but still within the window. * * @param[in] pxNetworkBuffer The network buffer descriptor with the packet. - * @param[in] ulCurrentSequenceNumber The current sequence number of the connection. + * @param[in] ulCurrentSequenceNumber The current expected sequence value by the connection. * @param[in] ulOurSequenceNumber The SEQ number to send out. * * @return Returns the value back from #prvTCPSendSpecialPacketHelper.