FreeRTOS_TCP - zero copy - ipBUFFER_PADDING

joehinkle wrote on Sunday, June 19, 2016:

My question is not about ipconfigPACKET_FILLER_SIZE, it’s about the other 8 bytes.

FreeRTOS_IP.h defines those additional 8 bytes as:

uint32_t pointer;	// word-aligned
uchar_8 filler[6];

Specifically the filler[6]; Why 6? Why not 2?

If filler was filler[2] - the buffer would still be 32bit aligned.

I’m using 50 buffers, so that’s an extra 200 bytes wasted is there is not a good reason for them.

******* ALSO ******

The use of the define …ipconfigPACKET_FILLER_SIZE

Besides being used in association with network buffers, it is also used in FreeRTOS_IP_Private.h

uint8_t fillPacket[ ipconfigPACKET_FILLER_SIZE ];

It’s part of the socket definition.

Question … Why have a socket element size be defined by a network buffer define?

I searched for fillPacket and it’s never used … I suggest removing it or changing the define that specifies its size.

Thanks.

rtel wrote on Sunday, June 19, 2016:

I think this is to ensure members of the packet structure that appear later on become aligned in a way that is more efficient on less capable architectures. It is also required by some DMA peripherals on some hardware (being somewhat non-specific) which require more than 4-byte alignment.

There is a comment that may help in FreeRTOS_IP.h, but I think the comment is actually out of date, but does show that at some point the definition was only 2. I have marked the comment as invalid, so it will get updated too. As it appears now:

/* Space left at the beginning of a network buffer storage area to store a
pointer back to the network buffer.  Should be a multiple of 8 to ensure 8 byte
alignment is maintained on architectures that require it.

In order to get a 32-bit alignment of network packets, an offset of 2 bytes
would be desirable, as defined by ipconfigPACKET_FILLER_SIZE.  So the malloc'd
buffer will have the following contents:
	uint32_t pointer;	// word-aligned
	uchar_8 filler[6];
	<< ETH-header >>	// half-word-aligned
	uchar_8 dest[6];    // start of pucEthernetBuffer
	uchar_8 dest[6];
	uchar16_t type;
	<< IP-header >>		// word-aligned
	uint8_t ucVersionHeaderLength;
	etc
 */

I have also queried the your other question about fillPacket (which doesn’t comply with the naming standard) so will get back to you on that, but as it is not used anywhere, I think it is in place to ensure the alignment of the buffer that follows it in the structure.

joehinkle wrote on Sunday, June 19, 2016:

How I read the comment — the 2 is actually referring to the two bytes associated with ipconfigPACKET_FILLER_SIZE … Those 2 bytes will position the buffer such that the 32 bit entries in the IP header are 32bit aligned.

The filler is only there because THOSE 2 bytes are ipconfigPACKET_FILLER_SIZE + “pointer” + 4 extra bytes – which are the ones I am questioning.

As for my second question and your reply about alignment within a structure … then the premise is faulty since the alignment has nothing to do with network bufferers.

When ipconfigPACKET_FILLER_SIZE is set to 0 … all the suggested alignment disappears.

I don’t think we’ve yet answered my two questions — sorry.

joehinkle wrote on Sunday, June 19, 2016:

To be more specific … I writing a zero copy driver for an NXP K64 (I wrote a K60 for a different stack several years ago).

The EMAC provides a config setting to ignore the first two bytes of the buffer — there by aligning the IP address fields. So payload is actually 1518 but allocated buffer is 1520.

You might want to clarify on the web site when using the zero copy methed – that the actual address that is placed inside the DMA descriptor (at least in the Kinetis K series) is:

Excerpt from web site on zero copy Receive

 Copy the pointer to the newly allocated Ethernet frame to a temporary  variable. 

    pucTemp = pxDescriptor->pucEthernetBuffer;

    /* This example assumes that the DMADescriptor_t type has a member
    called pucEthernetBuffer that points to the Ethernet buffer containing
    the received data, and a member called xDataLength that holds the length
    of the received data.  Update the newly allocated network buffer descriptor
    to point to the Ethernet buffer that contains the received data. */
    pxDescriptor->pucEthernetBuffer = pxDMARxDescriptor->pucEthernetBuffer;
    pxDescriptor->xDataLength = pxDMARxDescriptor->xDataLength;

    /* Update the Ethernet Rx DMA descriptor to point to the newly allocated
    Ethernet buffer. */
    
    pxDMARxDescriptor->puxEthernetBuffer = pucTemp;
    
Last line SHOULD be ....

pxDMARxDescriptor->puxEthernetBuffer = pucTemp - 2;

The minus 2 is to set the pointer where the EMAC will ignore the first 2 bytes.

So ... with the 2 extra bytes (ipconfigPACKET_FILLER_SIZE) and the 4 byte pointer that needs to be placed at the front ... that requires NO Extra filler to be involved... so in reality ... the filler in the header is just the size required for ipconfigPACKET_FILLER_SIZE.

edwards3 wrote on Sunday, June 19, 2016:

What about the multiple of 8 requirement? Maybe that should be configurable too?

joehinkle wrote on Sunday, June 19, 2016:

How do you get from CODE back to normal posting.

In the last of my links above … code style got set and I can’t get out!

joehinkle wrote on Monday, June 20, 2016:

I have in my old header file that the xmit and rcv buffers should be 8 aligned but I can no longer find where it states that in the manual.

Can you point to that requirement?

Thanks

rtel wrote on Monday, June 20, 2016:

It’s only a requirement for some DMA hardware, not all.

joehinkle wrote on Monday, June 20, 2016:

Referring to my question on the extra 4 bytes of filler in the network buffer …

I found this post suggesting that the buffer MUST be 8 bytes aligned.

http://www.freertos.org/FreeRTOS_Support_Forum_Archive/November_2014/freertos_FreeRTOS_TCP_minor_bug-finds_5591a1a9j.html

Looking at the code for pxUDPPayloadBuffer_to_NetworkBuffer, it is passed a pointer to a working buffer and then adjusts it for payload and ipBUFFER_PADDING to get a pointer to the start of the buffer created by the NIC driver using BufferManagement_1.c

It then performs a test on the pointer – cast to dword — to test if the last 2 bits are zero. If so, then the buffer is considered valid.

Here’s the line of code:

if( ( ( ( uint32_t ) pucBuffer ) & ( sizeof( pucBuffer ) - 1 ) ) == 0 )

If at the time the original bug was found (see link above) – IF the test was done on sizeof( pucBuffer ) and not on ( sizeof( pucBuffer ) - 1 ) ) as it is today … then 8 byte alignment would have been required back then.

Can anyone check to see if the test has changed because to me – the current code tests for 4 byte alignment — which means the extra 4 byte of filler is not required.

Sorry if it sounds like I’m beating a dead horse, but I think the extra 4 byte filler is left over from previous code and is no longer required.

If no one replies, I will test when I get my driver and stack up and running.

joehinkle wrote on Monday, June 20, 2016:

Never mind – I found my question/concern.

I’ve been searching the MAC spec for the word align. Never found anything related the align 8.

I’m working on the Driver, what to do with the MAC DMA descriptors, and it says the address must be divisable by 8. You get what you search for!!

So, I’m putting the 4 byte filler back in.

I saw in the FreeRtos_IP.h — that it metioned that those bytes might be used as flags in the future.

Are they used as flags today or can the NIC Driver use them if needed?

heinbali01 wrote on Tuesday, June 21, 2016:

Hi Joe,

I’m sorry that I kept silent for so long, but it’s summer holiday time here :slight_smile:

The reason to have the pointer located before the network buffer is the zero-copy option for UDP. A UDP payload buffer can be translated to a network buffer and vice versa with these complementary functions:

NetworkBufferDescriptor_t *pxUDPPayloadBuffer_to_NetworkBuffer( void *pvBuffer )
uint8_t *pcNetworkBuffer_to_UDPPayloadBuffer( NetworkBufferDescriptor_t *pxNetworkBuffer )

You are right that for some platforms, the extra 4 bytes in front of the network buffer are not essential. And so this declaration:

    #define ipBUFFER_PADDING ( 8u + ipconfigPACKET_FILLER_SIZE )

may sometimes be replaced with:

    #define ipBUFFER_PADDING ( 4u + ipconfigPACKET_FILLER_SIZE )

I don’t mind allowing ipBUFFER_PADDING to get overridden by a new item called ipconfigBUFFER_PADDING from FreeRTOSIPConfig.h:

#if( ipconfigBUFFER_PADDING != 0 )
    #define ipBUFFER_PADDING    ipconfigBUFFER_PADDING
#else
    #define ipBUFFER_PADDING    ( 8u + ipconfigPACKET_FILLER_SIZE )
#endif

The following code was introduced just to avoid an exception:

    /* Here a pointer was placed to the network descriptor,
      As a pointer is dereferenced, make sure it is well aligned */
    if( ( ( ( uint32_t ) pucBuffer ) & ( sizeof( pucBuffer ) - 1 ) ) == 0 )

We were in doubt about either using the live test or use a configASSERT().
But as pxUDPPayloadBuffer_to_NetworkBuffer may also be called from user space, we decided to have the above test.

As Richard wrote, the “more than 4-byte” alignment is required by some DMA’s.
The extra 2 bytes ipconfigPACKET_FILLER_SIZE were indeed introduced to get a proper 32-byte alignment for all 32-bit fields in the IP- and protocol headers. Many EMAC’s have an option that says: “skip the first 2 bytes” (as you mentioned).

FreeRTOS+TCP is still being developed further: it will soon have the possibility to use IPv6 headers, and also allow for multiple interfaces (EMAC’s). We’re still looking for more users who want to test the new (still experimental) version.

If you want to use IPv6, the extra 4 bytes will also get a purpose: store the IP-type in case of a IPv4 packet.

Regards.