Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Commit

Permalink
Merge pull request #1513 from htibosch/NetworkBuffer_xDataLength
Browse files Browse the repository at this point in the history
FreeRTOS+TCP: Let xDataLength always mean "total number of bytes"
  • Loading branch information
yuhui-zheng committed Nov 25, 2019
2 parents 8dc4ea7 + 67e86d2 commit 93c0aba
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -605,11 +605,11 @@ void vARPGenerateRequestPacket( NetworkBufferDescriptor_t * const pxNetworkBuffe
{
ARPPacket_t *pxARPPacket;

/* Buffer allocation ensures that buffers always have space
for an ARP packet. See buffer allocation implementations 1
and 2 under portable/BufferManagement. */
configASSERT( pxNetworkBuffer );
configASSERT( pxNetworkBuffer->xDataLength >= sizeof(ARPPacket_t) );
/* Buffer allocation ensures that buffers always have space
for an ARP packet. See buffer allocation implementations 1
and 2 under portable/BufferManagement. */
configASSERT( pxNetworkBuffer );
configASSERT( pxNetworkBuffer->xDataLength >= sizeof(ARPPacket_t) );

pxARPPacket = ( ARPPacket_t * ) pxNetworkBuffer->pucEthernetBuffer;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -916,15 +916,16 @@ for testing purposes, by the module iot_test_freertos_tcp.c
uint32_t ulDNSHandlePacket( NetworkBufferDescriptor_t *pxNetworkBuffer )
{
DNSMessage_t *pxDNSMessageHeader;
size_t uxPayloadSize = pxNetworkBuffer->xDataLength - sizeof( UDPPacket_t );

if( pxNetworkBuffer->xDataLength >= sizeof( DNSMessage_t ) )
if( uxPayloadSize >= sizeof( DNSMessage_t ) )
{
pxDNSMessageHeader =
( DNSMessage_t * ) ( pxNetworkBuffer->pucEthernetBuffer + sizeof( UDPPacket_t ) );

/* The parameter pdFALSE indicates that the reply was not expected. */
prvParseDNSReply( ( uint8_t * ) pxDNSMessageHeader,
pxNetworkBuffer->xDataLength,
uxPayloadSize,
pdFALSE );
}

Expand All @@ -939,11 +940,12 @@ DNSMessage_t *pxDNSMessageHeader;
{
UDPPacket_t *pxUDPPacket = ( UDPPacket_t * ) pxNetworkBuffer->pucEthernetBuffer;
uint8_t *pucUDPPayloadBuffer = pxNetworkBuffer->pucEthernetBuffer + sizeof( UDPPacket_t );
size_t uxPayloadSize = pxNetworkBuffer->xDataLength - sizeof( UDPPacket_t );

/* The network buffer data length has already been set to the
length of the UDP payload. */
prvTreatNBNS( pucUDPPayloadBuffer,
pxNetworkBuffer->xDataLength,
uxPayloadSize,
pxUDPPacket->xIPHeader.ulSourceIPAddress );

/* The packet was not consumed. */
Expand Down Expand Up @@ -1172,11 +1174,9 @@ BaseType_t xDoStore = xExpected;
{
BaseType_t xDataLength = uxBufferLength + sizeof( UDPHeader_t ) + sizeof( EthernetHeader_t ) + sizeof( IPHeader_t );

/* The field xDataLength was set to the length of the UDP payload.
The answer (reply) will be longer than the request, so the packet
must be duplicaed into a bigger buffer */
/* Set the size of the outgoing packet. */
pxNetworkBuffer->xDataLength = xDataLength;
pxNewBuffer = pxDuplicateNetworkBufferWithDescriptor( pxNetworkBuffer, xDataLength + 16 );
pxNewBuffer = pxDuplicateNetworkBufferWithDescriptor( pxNetworkBuffer, xDataLength + sizeof( LLMNRAnswer_t ) );

if( pxNewBuffer != NULL )
{
Expand Down Expand Up @@ -1330,14 +1330,10 @@ BaseType_t xDoStore = xExpected;
if( ( xBufferAllocFixedSize == pdFALSE ) && ( pxNetworkBuffer != NULL ) )
{
NetworkBufferDescriptor_t *pxNewBuffer;
BaseType_t xDataLength = pxNetworkBuffer->xDataLength + sizeof( UDPHeader_t ) +
sizeof( EthernetHeader_t ) + sizeof( IPHeader_t );

/* The field xDataLength was set to the length of the UDP payload.
The answer (reply) will be longer than the request, so the packet
must be duplicated into a bigger buffer */
pxNetworkBuffer->xDataLength = xDataLength;
pxNewBuffer = pxDuplicateNetworkBufferWithDescriptor( pxNetworkBuffer, xDataLength + 16 );
/* The field xDataLength was set to the total length of the UDP packet,
i.e. the payload size plus sizeof( UDPPacket_t ). */
pxNewBuffer = pxDuplicateNetworkBufferWithDescriptor( pxNetworkBuffer, pxNetworkBuffer->xDataLength + sizeof( NBNSAnswer_t ) );

if( pxNewBuffer != NULL )
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1122,7 +1122,8 @@ void FreeRTOS_SetAddressConfiguration( const uint32_t *pulIPAddress, const uint3
pxNetworkBuffer->pucEthernetBuffer[ ipSOCKET_OPTIONS_OFFSET ] = FREERTOS_SO_UDPCKSUM_OUT;
pxNetworkBuffer->ulIPAddress = ulIPAddress;
pxNetworkBuffer->usPort = ipPACKET_CONTAINS_ICMP_DATA;
pxNetworkBuffer->xDataLength = xNumberOfBytesToSend + sizeof( ICMPHeader_t );
/* xDataLength is the size of the total packet, including the Ethernet header. */
pxNetworkBuffer->xDataLength = xNumberOfBytesToSend + sizeof( ICMPPacket_t );

/* Send to the stack. */
xStackTxEvent.pvData = pxNetworkBuffer;
Expand Down Expand Up @@ -1611,23 +1612,24 @@ uint8_t ucProtocol;

/* Only proceed if the payload length indicated in the header
appears to be valid. */
if ( pxNetworkBuffer->xDataLength >= sizeof( UDPPacket_t ) )
if ( ( pxNetworkBuffer->xDataLength >= sizeof( UDPPacket_t ) ) && ( FreeRTOS_ntohs( pxUDPPacket->xUDPHeader.usLength ) >= sizeof( UDPHeader_t ) ) )
{
/* Ensure that downstream UDP packet handling has the lesser
* of: the actual network buffer Ethernet frame length, or
* the sender's UDP packet header payload length, minus the
* size of the UDP header.
*
* The size of the UDP packet structure in this implementation
* includes the size of the Ethernet header, the size of
* the IP header, and the size of the UDP header.
size_t uxPayloadSize_1, uxPayloadSize_2;
/* The UDP payload size can be calculated by subtracting the
* header size from `xDataLength`.
* However, the `xDataLength` may be longer that expected,
* e.g. when a small packet is padded with zero's.
* The UDP header contains a field `usLength` reflecting
* the payload size plus the UDP header ( 8 bytes ).
* Set `xDataLength` to the size of the headers,
* plus the lower of the two calculated payload sizes.
*/

pxNetworkBuffer->xDataLength -= sizeof( UDPPacket_t );
if( ( FreeRTOS_ntohs( pxUDPPacket->xUDPHeader.usLength ) - sizeof( UDPHeader_t ) ) <
pxNetworkBuffer->xDataLength )
uxPayloadSize_1 = pxNetworkBuffer->xDataLength - sizeof( UDPPacket_t );
uxPayloadSize_2 = FreeRTOS_ntohs( pxUDPPacket->xUDPHeader.usLength ) - sizeof( UDPHeader_t );
if( uxPayloadSize_1 > uxPayloadSize_2 )
{
pxNetworkBuffer->xDataLength = FreeRTOS_ntohs( pxUDPPacket->xUDPHeader.usLength ) - sizeof( UDPHeader_t );
pxNetworkBuffer->xDataLength = uxPayloadSize_2 + sizeof( UDPPacket_t );
}

/* Fields in pxNetworkBuffer (usPort, ulIPAddress) are network order. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -702,9 +702,11 @@ EventBits_t xEventBits = ( EventBits_t ) 0;
}
taskEXIT_CRITICAL();

/* The returned value is the data length, which may have been capped to
the receive buffer size. */
lReturn = ( int32_t ) pxNetworkBuffer->xDataLength;
/* The returned value is the length of the payload data, which is
calculated at the total packet size minus the headers.
The validity of `xDataLength` prvProcessIPPacket has been confirmed
in 'prvProcessIPPacket()'. */
lReturn = ( int32_t ) ( pxNetworkBuffer->xDataLength - sizeof( UDPPacket_t ) );

if( pxSourceAddress != NULL )
{
Expand Down Expand Up @@ -833,7 +835,8 @@ FreeRTOS_Socket_t *pxSocket;

if( pxNetworkBuffer != NULL )
{
pxNetworkBuffer->xDataLength = xTotalDataLength;
/* xDataLength is the size of the total packet, including the Ethernet header. */
pxNetworkBuffer->xDataLength = xTotalDataLength + sizeof( UDPPacket_t );
pxNetworkBuffer->usPort = pxDestinationAddress->sin_port;
pxNetworkBuffer->usBoundPort = ( uint16_t ) socketGET_SOCKET_PORT( pxSocket );
pxNetworkBuffer->ulIPAddress = pxDestinationAddress->sin_addr;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,22 @@ UDPPacket_t *pxUDPPacket;
IPHeader_t *pxIPHeader;
eARPLookupResult_t eReturned;
uint32_t ulIPAddress = pxNetworkBuffer->ulIPAddress;
size_t uxPayloadSize;

/* Map the UDP packet onto the start of the frame. */
pxUDPPacket = ( UDPPacket_t * ) pxNetworkBuffer->pucEthernetBuffer;

#if ipconfigSUPPORT_OUTGOING_PINGS == 1
if( pxNetworkBuffer->usPort == ipPACKET_CONTAINS_ICMP_DATA )
{
uxPayloadSize = pxNetworkBuffer->xDataLength - sizeof( ICMPPacket_t );
}
else
#endif
{
uxPayloadSize = pxNetworkBuffer->xDataLength - sizeof( UDPPacket_t );
}

/* Determine the ARP cache status for the requested IP address. */
eReturned = eARPGetCacheEntry( &( ulIPAddress ), &( pxUDPPacket->xEthernetHeader.xDestinationAddress ) );

Expand Down Expand Up @@ -109,7 +121,7 @@ uint32_t ulIPAddress = pxNetworkBuffer->ulIPAddress;

pxUDPHeader->usDestinationPort = pxNetworkBuffer->usPort;
pxUDPHeader->usSourcePort = pxNetworkBuffer->usBoundPort;
pxUDPHeader->usLength = ( uint16_t ) ( pxNetworkBuffer->xDataLength + sizeof( UDPHeader_t ) );
pxUDPHeader->usLength = ( uint16_t ) ( uxPayloadSize + sizeof( UDPHeader_t ) );
pxUDPHeader->usLength = FreeRTOS_htons( pxUDPHeader->usLength );
pxUDPHeader->usChecksum = 0u;
}
Expand Down Expand Up @@ -143,16 +155,14 @@ uint32_t ulIPAddress = pxNetworkBuffer->ulIPAddress;
if( pxNetworkBuffer->usPort == ipPACKET_CONTAINS_ICMP_DATA )
{
pxIPHeader->ucProtocol = ipPROTOCOL_ICMP;
pxIPHeader->usLength = ( uint16_t ) ( pxNetworkBuffer->xDataLength + sizeof( IPHeader_t ) );
pxIPHeader->usLength = ( uint16_t ) ( uxPayloadSize + sizeof( IPHeader_t ) + sizeof( ICMPHeader_t ) );
}
else
#endif /* ipconfigSUPPORT_OUTGOING_PINGS */
{
pxIPHeader->usLength = ( uint16_t ) ( pxNetworkBuffer->xDataLength + sizeof( IPHeader_t ) + sizeof( UDPHeader_t ) );
pxIPHeader->usLength = ( uint16_t ) ( uxPayloadSize + sizeof( IPHeader_t ) + sizeof( UDPHeader_t ) );
}

/* The total transmit size adds on the Ethernet header. */
pxNetworkBuffer->xDataLength = pxIPHeader->usLength + sizeof( EthernetHeader_t );
pxIPHeader->usLength = FreeRTOS_htons( pxIPHeader->usLength );
/* HT:endian: changed back to network endian */
pxIPHeader->ulDestinationIPAddress = pxNetworkBuffer->ulIPAddress;
Expand Down

0 comments on commit 93c0aba

Please sign in to comment.