Hello,

I am using FreeRTOS v8.2.1 and I ran into a strange issue in FreeRTOS-Plus-UDP in FreeRTOS_DHCP.c.

Take a look at the "static void prvSendDHCPDiscover( xMACAddress_t *pxMACAddress )" function:

    pucUDPPayloadBuffer = prvCreatePartDHCPMessage( &xAddress, pxMACAddress, dhcpREQUEST_OPCODE, ucDHCPDiscoverOptions, sizeof( ucDHCPDiscoverOptions ) );

	iptraceSENDING_DHCP_DISCOVER();
	if( FreeRTOS_sendto( xDHCPSocket, pucUDPPayloadBuffer, ( sizeof( xDHCPMessage_t ) + sizeof( ucDHCPDiscoverOptions ) ), FREERTOS_ZERO_COPY, &xAddress, sizeof( xAddress ) ) == 0 )

Notice that the FREERTOSZEROCOPY flag is passed to FreeRTOSsendto() as ulFlags and pucUDPPayloadBuffer as pvBuffer. FreeRTOSsendto() run into the following code:

    if( ( ulFlags & FREERTOS_ZERO_COPY ) == 0 )
	{
        ...
    else
    {
        /* When zero copy is used, pvBuffer is a pointer to the
        payload of a buffer that has already been obtained from the
        stack.  Obtain the network buffer pointer from the buffer. */
        pucBuffer = ( uint8_t * ) pvBuffer;
        pucBuffer -= ( ipBUFFER_PADDING + sizeof( xUDPPacket_t ) );
        pxNetworkBuffer = * ( ( xNetworkBufferDescriptor_t ** ) pucBuffer );
    }

When FREERTOSZEROCOPY is set it sets up the pxNetworkBuffer pointer by reading out a pointer from pvBuffer. But pvBuffer doesn't contain any pointer to any valid data space, that's a DHCP message made by prvCreatePartDHCPMessage().

The next lines in FreeRTOS_sendto() causes crash because it writes to "illegal" memory locations (pxNetworkBuffer is initialized with garbage):

    if( pxNetworkBuffer != NULL )
    {
        pxNetworkBuffer->xDataLength = xTotalDataLength;
        pxNetworkBuffer->usPort = pxDestinationAddress->sin_port;
        pxNetworkBuffer->usBoundPort = ( uint16_t ) socketGET_SOCKET_ADDRESS( pxSocket );
        pxNetworkBuffer->ulIPAddress = pxDestinationAddress->sin_addr;
        ...

In my case this error is fixed by calling FreeRTOS_sendto() with ulFlags = 0 in prvSendDHCPDiscover().

    pucUDPPayloadBuffer = prvCreatePartDHCPMessage( &xAddress, pxMACAddress, dhcpREQUEST_OPCODE, ucDHCPDiscoverOptions, sizeof( ucDHCPDiscoverOptions ) );

	iptraceSENDING_DHCP_DISCOVER();
	if( FreeRTOS_sendto( xDHCPSocket, pucUDPPayloadBuffer, ( sizeof( xDHCPMessage_t ) + sizeof( ucDHCPDiscoverOptions ) ), 0, &xAddress, sizeof( xAddress ) ) == 0 )

[Please do not write "Bug in" in a subject line, unless it has been confirmed as a bug, other wise it is confusing for readers of the archive. "Possible bug in" would be more appropriate.]

I have just stepped through this code to investigate for you, and this is what I find:

• prvSendDHCPDiscover() calls prvCreatePartDHCPMessage(), which calls FreeRTOSGetUDPPayloadBuffer(), which calls pxNetworkBufferGet() (I'm using BufferAllocation2.c).

--- pxNetworkBufferGet() allocates a xNetworkBufferDescriptort, with the Ethernet buffer referenced from the structure's pucEthernetBuffer member. A pointer to the xNetworkBufferDescriptort structure is placed at the beginning of the buffer pointed to by pucEthernetBuffer.

--- FreeRTOSGetUDPPayloadBuffer() returns a pointer to pucEthernetBuffer with an offset large enough to hold the UPD headers. So the returned value is after the space for the UDP header, which is itself after a pointer back to the xNetworkBufferDescriptort structure.

• prvCreatePartDHCPMessage() places the DHCP message at the address returned by FreeRTOSGetUDPPayloadBuffer(), then returns the same address as that returned by FreeRTOSGetUDPPayloadBuffer(). So the address returned to prvSendDHCPDiscover() has the xNetworkBufferDescriptor_t structure and space for the UDP header behind it, and the DHCP message in front of it.

• prvSendDHCPDiscover() calls FreeRTOSsendto() with the FREERTOSZERO_COPY flag set.

• Inside FreeRTOS_sendto():

The following lines move the pointer back to the xNetworkBufferDescriptort structure by first removing the padding which holds the pointer being retrieved, and the space that was left for the UDP header: pucBuffer = ( uint8t * ) pvBuffer; pucBuffer -= ( ipBUFFERPADDING + sizeof( xUDPPackett ) );

That leaves the pointer pointing to the xNetworkBufferDescriptort structure, which is dereferenced using the following line: pxNetworkBuffer = * ( ( xNetworkBufferDescriptort ** ) pucBuffer );

So that would appear to be correct to me. I know my explanation above is a little complex to follow, but I verified this by writing some known values into the structure when it was allocated, then checking that the known values still existed in the structure when it was retrieved again using the code above.

I think your post is saying that the code above does not retrieve the correct buffer, whereas when I step through the code it does appear to retrieve the correct buffer - did I misinterpret your post?

Regards.

Hi,

"I think your post is saying that the code above does not retrieve the correct buffer,"

Exactly. I am using BufferAllocation_1.c.

I have taken a look into BufferAllocation_2.c and I see the black magic in that:

   /* Store a pointer to the network buffer structure in the
    buffer storage area, then move the buffer pointer on past the
    stored pointer so the pointer value is not overwritten by the
    application when the buffer is used. */
    *( ( xNetworkBufferDescriptor_t ** ) ( pxReturn->pucEthernetBuffer ) ) = pxReturn;
    pxReturn->pucEthernetBuffer += ipBUFFER_PADDING;

BufferAllocation1.c doesn't have this stuff. Is BufferAllocation1.c obsolete and not supported?

Ok - that explains the discrepancy.

I don't use BufferAllocation_1 myself with FreeRTOS+UDP but do with FreeRTOS+TCP - I will have a look and see how it is handled there.

Regards.

Hi Endre,

Is BufferAllocation_1.c obsolete and not supported?

BufferAllocation_1.c is up-to-date and fully supported.

BufferAllocation2 is for systems with a smaller amount of RAM. Every network buffer will be malloc'd when needed, and free'd when released. If you have enough RAM, BufferAllocation1 will be faster, no calls are made to pvPortMalloc()/vPortFree().

Could you show your version of vNetworkInterfaceAllocateRAMToBuffers()?

Or, alternatively, you might compare your code with the following example:

~~~~

/* Allocate static space: */ static unsigned char networkpackets[ ipconfigNUMNETWORKBUFFERDESCRIPTORS * UNIT_SIZE ] attribute ((aligned (8)));

void vNetworkInterfaceAllocateRAMToBuffers( xNetworkBufferDescriptort *pxNetworkBuffers ) { unsigned char *rambuffer = networkpackets; int x; for (x = 0; x < ipconfigNUMNETWORKBUFFERDESCRIPTORS; x++) { pxNetworkBuffers[ x ].pucEthernetBuffer = rambuffer + ipBUFFERPADDING; *( ( unsigned * ) rambuffer ) = ( unsigned ) ( &pxNetworkBuffers[ x ] ); rambuffer += UNIT_SIZE; } } ~~~~

There is nothing magical about 'ipBUFFER_PADDING', it is all described on the web pages.

See e.g. http://www.freertos.org/FreeRTOS-Plus/FreeRTOSPlusTCP/EmbeddedEthernetPorting.html#vNetworkInterfaceAllocateRAMToBuffers

Regards.

Hi,

My vNetworkInterfaceAllocateRAMToBuffers() was simpler. I didn't find any documentation for it just some sample codes.

Here is my buffer initialization:

static uint8_t packetBuffer[ipconfigNUM_NETWORK_BUFFERS][ipTOTAL_ETHERNET_FRAME_SIZE];

void vNetworkInterfaceAllocateRAMToBuffers(xNetworkBufferDescriptor_t pxNetworkBuffers[ipconfigNUM_NETWORK_BUFFERS]) {
	int i;
	/* scheduler is not yet running here */
	for(i = 0; i < ipconfigNUM_NETWORK_BUFFERS; i++) {
		pxNetworkBuffers[i].pucEthernetBuffer = packetBuffer[i];
	}
}

So I have to place the pointer into the beginning of the ethernetBuffer. NetworkInterface.h could contain some comments referring to this.

Thanks.

Hi,

You can use the code as I posted here above.

Although a lot of documentation has been written, some more explanation:

A "Network Buffer Descriptor" holds a pointer to a "Network Buffer".

A "Network Buffer" is a character pointer. It must be 4-byte aligned plus 2 bytes.

~~~~~

/* An example of the physical layout of a network buffer. */
0x0E001000  // pointer to "Network Buffer Descriptor"
0x0E001001  // This is a const pointer and should never change.
0x0E001002
0x0E001003

0x0E001004  // Space for flags
0x0E001005
0x0E001006
0x0E001007

0x0E001008  // 2-byte filler (ipconfigPACKET_FILLER_SIZE)
0x0E001009
0x0E00100A  // pucEthernetBuffer points to here, start of "Network Buffer"
0x0E00100B

0x0E00100C

~~~~~

A "Network Buffer" will hold actual network packets.

When a "Network Buffer" is passed with the FREERTOS_ZERO_COPY flag, the driver wants to know the containing "Network Buffer Descriptor". This address can be found 10 bytes before the "Network Buffer".

The function vNetworkInterfaceAllocateRAMToBuffers() makes a double binding, It will set:

pxNetworkBuffers[ x ].pucEthernetBuffer = ram_buffer + ipBUFFER_PADDING;

and it will set the pointer to the owning "Network Buffer Descriptor":

*( ( unsigned * ) ram_buffer ) = ( unsigned ) ( &pxNetworkBuffers[ x ] );

The broader picture: if we all had megabytes of RAM and gigahertz of speed, the above code would not be necessary. The Zero-copy and the special alignment make that FreeRTOS+TCP performs well on smaller and slower MCU's as well.

Regards.

Just to clarify a possible cross communication - Endre is referring to the +UDP code, whereas Hein is referring to the version shipped with the +TCP code.

The WinPCap NetworkInterface.c file has just been updated: https://sourceforge.net/p/freertos/code/HEAD/tree/trunk/FreeRTOS-Plus/Source/FreeRTOS-Plus-UDP/portable/NetworkInterface/WinPCap/NetworkInterface.c

Note: line 120 - the buffer size has been increased by portBUFFER_PADDING bytes. Line 472 onwards - vNetworkInterfaceAllocateRAMToBuffers() has been updated to place a pointer back to the referencing structure at the start of the buffer.

Please let us know if this works for you. Apologies for the inconvenience - the Buffer_1 scheme is not normally used with the UDP stack as that tends to be used on smaller MCUs. Whereas it is used in the TCP stack (where it was correct) as that is sometimes used on much larger systems with more RAM.

Regards.

I have modified my vNetworkInterfaceAllocateRAMToBuffers() based on the code in WinPCap NetworkInterface.c. It seems to be working. Thanks.

Richard wrote:

Cross communication - Endre is referring to the +UDP code, whereas Hein is referring to the version shipped with the +TCP code.

Oops, now I understand better what you were writing.

The functioning hasn't changed much though, in the UDP-code there was no 2-byte filler yet, or:

#define ipBUFFER_PADDING        ( 8 )

has become:

#define ipBUFFER_PADDING        ( 8 + ipconfigPACKET_FILLER_SIZE )

The two-way pointers haven't changed neither, and the back-pointer was / is needed by all FREERTOSZEROCOPY code.

You are right that in the UDP-only code, BufferAllocation_1.c is less documented and it was applied less often. It would work well with on the platforms SAM4E, LPC18xx which had a function:

static void prvGMACDeferredInterruptHandlerTask( void *pvParameters )

which would set the back-pointer after receiving a message:

*( ( xNetworkBufferDescriptor_t ** )
    ( ( pxNetworkBuffer->pucEthernetBuffer - ipBUFFER_PADDING ) )
    ) = pxNetworkBuffer;

I have modified my vNetworkInterfaceAllocateRAMToBuffers() based on the code in WinPCap NetworkInterface.c. It seems to be working.

Glad to hear!

Regards