FreeRTOS+TCP minor bug-finds

rwt33 wrote on Monday, December 01, 2014:

Another documentation question: Is it ok to call vReleaseNetworkBufferAndDescriptor from an ISR? The documentation page for it only mentions “pxGetNetworkBufferWithDescriptor() must not be called from an interrupt service routine (ISR)”, bur doesn’t say vReleaseNetworkBufferAndDescriptor can’t be. Copy and paste error?

rtel wrote on Monday, December 01, 2014:

If you are using BufferAllocation_2.c then this is not same to use from
an ISR - although you could modify the memory allocation functions to
make it safe I would not recommend it. It is best instead to defer the
processing to a task. The next FreeRTOS version introduces a new
feature that makes that faster and easier (does not require a semaphore).

Regards.

heinbali01 wrote on Monday, December 01, 2014:

Same thoughts: try to defer all buffer processing to a normal task.

In one of the ports (for Xilinx/Zynq) you will find:


    /* Three ISR's for TX, RX and error handling: */

    void emac_send_handler(void *arg)
    {
    xemacpsif_s   *xemacpsif;
    long xHigherPriorityTaskWoken = pdFALSE;

        xemacpsif = (xemacpsif_s *)(arg);

        /* In this port for FreeRTOS+TCP, the EMAC interrupts
        will only set a bit in "isr_events". The task in
        NetworkInterface will wake-up and do the necessary work. */

        xemacpsif->isr_events |= EMACPS_TX_EVENT;
        xemacpsif->txBusy = pdFALSE;

        if( xEMACEventSemaphore != NULL )
        {
            xSemaphoreGiveFromISR( xEMACEventSemaphore,
                &xHigherPriorityTaskWoken );
        }

        portYIELD_FROM_ISR( xHigherPriorityTaskWoken );
    }

    void emac_recv_handler(void *arg)
    {
        /* Same as above, setting the RX flag */
        xemacpsif->isr_events |= EMACPS_RX_EVENT;
        ...
    }

    void emac_error_handler(void *arg)
    {
        /* Same as above, setting the ERR flag */
        xemacpsif->isr_events |= EMACPS_ERR_EVENT;
        ...
    }

The task handling the NIC will do the actual work:


for( ; ; )
{
    if( ( xemacpsif->isr_events & EMACPS_ALL_EVENT ) == 0 )
    {
        xSemaphoreTake( xEMACEventSemaphore, 100 );
    }

    if( ( xemacpsif->isr_events & EMACPS_RX_EVENT ) != 0 )
    {
        xemacpsif->isr_events &= ~EMACPS_RX_EVENT;
        xResult = emacps_check_rx( xemacpsif );
    }

    if( ( xemacpsif->isr_events & EMACPS_TX_EVENT ) != 0 )
    {
        xemacpsif->isr_events &= ~EMACPS_TX_EVENT;
        emacps_check_tx( xemacpsif );
    }

    if( ( xemacpsif->isr_events & EMACPS_ERR_EVENT ) != 0 )
    {
        xemacpsif->isr_events &= ~EMACPS_ERR_EVENT;
        emacps_check_errors( xemacpsif );
    }
    check_phy_link_status( );
}

Note that sending messages always happens from the IP-task.

FreeRTOS+TCP is 100% thread-safe, not interrupt-safe. As you most probably know, there is a very small delay between calling xSemaphoreGiveFromISR() and the waking up from xSemaphoreTake().

The above approach has the advantage that the configured priorities govern the processing, and not the hardware :slight_smile:

Personally, I tend to give the highest priority to the NIC task. We don’t want to miss packets due to an RX overflow. The above routine emacps_check_rx() will only put received messages in a queue, and free the DMA buffers.

The second highest priority is for the IP-task, and all other tasks get a lower priority.

Good luck,
Hein

rwt33 wrote on Thursday, December 04, 2014:

Hi guys,
When dealing with link up/down events, I call FreeRTOS_NetworkDown[FromISR] and then indicate network back up from xNetworkInterfaceInitialise. Is it recommended to block for long periods in xNetworkInterfaceInitialise while waiting for the link to come up?

rtel wrote on Thursday, December 04, 2014:

I don’t think that should be necessary as the IP task will itself delay in between calls to xNetworkInterfaceInitialise(). Search FreeRTOS_IP.c for ipINITIALISATION_RETRY_DELAY to see where this is done. Let us know if that is not what you meant, or think it should be done in some other way.

Regards.

rwt33 wrote on Monday, December 08, 2014:

Sweet, I’ve got a nicely running stack. DMA & Interrupt driven, it achieves ~3.8Mb which I figure isn’t that for a 10Mb NIC on a 9MHz SPI and no IP checksum offloading.
Are you guys ready to NIC drivers for FreeRTOS+TCP?

rtel wrote on Monday, December 08, 2014:

Excellent! You can have the honour of being the first person to upload
a driver (hopefully a build project showing how to use the driver ;o) to
the +TCP forum in the Interactive site:
http://interactive.freertos.org/forums/21211265-FreeRTOS-TCP

Regards.

rwt33 wrote on Tuesday, December 16, 2014:

Good call. I’m midst refactoring the code to try decouple hardware peripheral dependencies. I’ll then post to the forum

rwt33 wrote on Wednesday, December 17, 2014:

I’m having a few issues now with the link going up/down. Before I start digging into my driver, have you guys done much testing with links going up & down? Specifically I’m having problems when the link has been up, goes down and then comes back up.
If the link is there at startup it’s fine, or if the link is missing at startup xNetworkInterfaceInitialise polls until the link comes up. It’s only if the link has been up, goes down and then comes back up that I’m having problems.
Of course there’s a very high chance it’s my driver also. Just wondering if you’ve done much exploration in that area.
Cheers guys

rtel wrote on Wednesday, December 17, 2014:

Hi Robert,

There is some reliance on the driver here to inform the stack that the
link has gone down. When the driver tells the stack the link has gone
down the stack should then loop, periodically attempting to
re-initialise. Re-initialising should then start the DHCP process
again. We can have a look, but in the mean time if you see anything
that needs changing let us know.

Regards.

heinbali01 wrote on Wednesday, December 17, 2014:

Hi Robert,

If your device runs on a battery and if it is being plugged-in and out, yes I would have it run DHCP again. The new network might be totally different.

At the other hand, if the device has a fixed place and a fixed power supply, I would just poll the Link Status of the PHY. As long as the LS is high, you rarely poll it, may after not receiving any packet for e.g. 5 seconds.

While the LS is low, just discard outgoing messages. I would poll the LS status frequently (at least every second) just to see when it comes up again.

When I get a chance I will try the first option: a reinitialisation including HDCP after the Link Status comes up again.

Regards.

hmf55 wrote on Thursday, February 26, 2015:

Hi,
I’m reading this thread having the same issue. I’m using the download 141019 not knowing
if this bug is already fixed.
I’m porting to the RX63N from renesas (Trying to get the zero Copy DMA running).

The problem I’figurured out is in “FreeRTOS_IP.c” in Function
‘pxUDPPayloadBuffer_to_NetworkBuffer’ (line 916)

test for buffer alignment:

if( ( ( ( uint32_t ) pvBuffer ) & portBYTE_ALIGNMENT_MASK ) == 0 )

this could not work since “pvBuffer” is misaligned since “sizeof( xUDPPacket_t )”
is not 32 bit aligned.

I’ve changed that to

if( ( ( ( uint32_t ) (pucBuffer + ipBUFFER_PADDING) ) & portBYTE_ALIGNMENT_MASK ) == 0 )

wich works for me.

Thanks.

heinbali01 wrote on Thursday, February 26, 2015:

Hi,

Thanks for letting us know. There was indeed a typo in the mentioned function:

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

I’m porting to the RX63N from Renesas

Wow great! +TPC hasn’t been ported to that part yet.
If you like, please share your work (i.e. NetworkInterface.c) with the community.
In return you may get an “early adopter status”.
Please drop an email to FreeRTOS if you want to know more about this.

Regards,
Hein

hmf55 wrote on Tuesday, March 03, 2015:

Hi,
I think I found the next bug:
in “FreeRTOS_Sockets.c” in Line 788 the port is set with out considering the endianess:

Instead of

pxNetworkBuffer->usPort = pxDestinationAddress->sin_port;

I use this

pxNetworkBuffer->usPort = FreeRTOS_htons(pxDestinationAddress->sin_port);

which worked for me.

How can I share my work? Drop the files here?

rtel wrote on Tuesday, March 03, 2015:

That’s interesting - we are testing with both endians so I’m curious how
we have missed that. I would be grateful if you could either attach the
effected file to the post, or maybe better still (so there are not too
many versions floating around the internet) send the file to me. I
think you have my email address already - otherwise use the “business
contact” link on http://www.FreeRTOS.org/contact and just mark it for my
attention.

Regards (Richard).

heinbali01 wrote on Tuesday, March 03, 2015:

Hi,

Thanks a lot for reporting, but it doesn’t look like a bug.

This is the function you wrote about:

int32_t FreeRTOS_sendto( xSocket_t xSocket, const void *pvBuffer,
    size_t xTotalDataLength, uint32_t ulFlags,
    const struct freertos_sockaddr *pxDestinationAddress,
    socklen_t xDestinationAddressLength )
{
    ....
    pxNetworkBuffer->usPort = pxDestinationAddress->sin_port;
    ....
    pxNetworkBuffer->ulIPAddress = pxDestinationAddress->sin_addr;
    ....
}

The fields of pxDestinationAddress must be set before calling sendto(), for instance as follows:

    /* FreeRTOS_inet_addr() returns an IP address in network-byte-order: */
    pxDestinationAddress->sin_addr = FreeRTOS_inet_addr( "192.168.1.110" );

    /* This would have the same effect: */
    pxDestinationAddress->sin_addr = FreeRTOS_htonl( 0xC0A8016E );

    pxDestinationAddress->sin_port = FreeRTOS_htons( 10000 );

When BSD sockets were first invented, everyone agreed that all values must be stored internally in the network-byte-order.

Regards.

hmf55 wrote on Wednesday, March 04, 2015:

oh sorry, one should - RTFM - first.
Many Thanks.

Am 03.03.2015 um 19:13 schrieb Hein Tibosch:

Hi,

Thanks a lot for reporting, but it doesn’t look like a bug.

This is the function you wrote about:

int32_t FreeRTOS_sendto( xSocket_t xSocket, const void *pvBuffer,
size_t xTotalDataLength, uint32_t ulFlags,
const struct freertos_sockaddr *pxDestinationAddress,
socklen_t xDestinationAddressLength )
{

pxNetworkBuffer->usPort = pxDestinationAddress->sin_port;

pxNetworkBuffer->ulIPAddress = pxDestinationAddress->sin_addr;

}

The fields of |pxDestinationAddress| must be set before calling |sendto()|, for instance as follows:

 /* FreeRTOS_inet_addr() returns an IP address in network-byte-order: */
 pxDestinationAddress->sin_addr  =  FreeRTOS_inet_addr(  "192.168.1.110"  );

 /* This would have the same effect: */
 pxDestinationAddress->sin_addr  =  FreeRTOS_htonl(  0xC0A8016E  );

 pxDestinationAddress->sin_port  =  FreeRTOS_htons(  10000  );

When BSD sockets were first invented, everyone agreed that all values must be stored internally in the network-byte-order.

Regards.


FreeRTOS+TCP minor bug-finds https://sourceforge.net/p/freertos/discussion/382005/thread/5591a1a9/?limit=25&page=1#3048


Sent from sourceforge.net because you indicated interest in SourceForge.net: Log In to SourceForge.net https://sourceforge.net/p/freertos/discussion/382005

To unsubscribe from further messages, please visit SourceForge.net: Log In to SourceForge.net https://sourceforge.net/auth/subscriptions

heinbali01 wrote on Wednesday, March 04, 2015:

oh sorry, one should - RTFM - first.

Haha, I don’t blame you, the forced network byte order is very confusing :slight_smile:

I have never understood why, on a little-endian machine, port-numbers and IP-addresses must be stored ‘the other way around’. The swapping could also have remained hidden, somewhere in the driver.

E.g. HTTP is at port 80 (0x50), and when you inspect sin_addr you will see port 20480 (0x5000).

Within the +TCP library you’ll see several fields and variables that are in native endianness, such as these socket fields:

    uint32_t xIPTcpSocket::ulRemoteIP;      /* IP address of remote machine */
    uint16_t xIPTcpSocket::usRemotePort;    /* Port on remote machine */
    uint16_t xFreeRTOS_Socket_t::usLocPort; /* Local port on this machine */

The reason was that it made debugging a lot easier.

Good luck!

andymcc0 wrote on Wednesday, March 04, 2015:

Perhaps not a ‘bug’ as such, but I struggled with DHCP until I cleared the ‘BROADCAST’ flag from the flags field of the DHCP message.

i.e. in FreeRTOS_DHCP.C:prvCreatePartDHCPMessage() replace
pxDHCPMessage->usFlags = dhcpBROADCAST;
with
pxDHCPMessage->usFlags = 0;

More detail in section 3.1.1 of RFC1542
http://tools.ietf.org/html/rfc1542
which implies that it should only be set exceptionally.

As an aside, I hope to make available an example project using FreeRTOS+TCP+CLI, built using LPCXpresso/LPCOpen and running on the Embedded Artists LPC4088 OEM + Base board.

Andy McC

heinbali01 wrote on Thursday, March 05, 2015:

Hi Andy,

Thanks for reporting this.

I just tried out both settings, with and without the DHCP BROADCAST flag.

pxDHCPMessage->usFlags = dhcpBROADCAST;
+TCP : 0.0.0.0      255.255.255.255  ..:ff:ff:ff  DHCP Discover
DHCP : 192.168.2.3  255.255.255.255  ..:ff:ff:ff  DHCP Offer
+TCP : 0.0.0.0      255.255.255.255  ..:ff:ff:ff  DHCP Request
DHCP : 192.168.2.3  255.255.255.255  ..:ff:ff:ff  DHCP ACK


pxDHCPMessage->usFlags = 0;
+TCP : 0.0.0.0      255.255.255.255  ..:ff:ff:ff  DHCP Discover
DHCP : 192.168.2.3  192.168.2.125    ..:a3:94:22  DHCP Offer
+TCP : 0.0.0.0      255.255.255.255  ..:ff:ff:ff  DHCP Request
DHCP : 192.168.2.3  192.168.2.125    ..:a3:94:22  DHCP ACK

The difference is that the DHCP server will ‘unicast’ the Offer and ACK responses.

Is your device able to receive broadcast messages?

Register: RXFILTERCTRL at address 0x2008 4200
Bit:      1 ABE AcceptBroadcastEn. When set to 1,
          all broadcast frames are accepted. 0

It might be an idea to use ‘unicast’ in the first instance, and only if that fails start ‘Discover’ with the BROADCAST flag.

Regards.