FreeRTOS-TCP improvement?

Hi, I’m just in the process of testing out some stuff with FreeRTOS TCP and I have it working fine, but there are some changes that make it more useful when using the “event driven” mode, I’m wondering whether you would be open for me submitting changes for these. Basically, the event driven stuff works fine, but it’s difficult to link it to other parts of the code, so I have tested the water by making a few changes. Mainly I created a define “ipconfigSOCKETHASUSER_DATA” and added a field to the TCP socket (just like the user semaphore) that is a void * for user data to be stored. I added a setsockopt field called FREERTOSSOSET_USERDATA which allows you to call setsockopt to set the value of this. At the moment, I’m “fudging” getting this userdata by including the private ip header and accessing it directly from say the connection or rx callback, I can then cast this to (in my case a class) and use further callbacks to schedule operations to occur. Ideally, there would be a getsockopt which would allow you to read the userdata value, but as getsockopt has not been implemented at all, I kind of went for my naughty but easy solution of accessing the field. From my point of view it makes the callbacks much more useful because the socket carries around extra information which I can use to quickly inform other layers there is something to do, without having to traverse lists to find the data assosicated with a socket. Is this something of interest?

FreeRTOS-TCP improvement?

Hi Adrian, your technique is not an unusual one, passing a void pointer in a call-back. The callee will cast that pointer to a known structure type. Every FreeRTOS task gets a void pointer in the same way. But we’re always kind of conservative when judging new changes. It should not be too deviant from BSD, it should not add too much work or complexity, it should not be prone to errors etc. You are proposing to make the struct a member of a socket. I tend to reverse this relationship: I create my struct that owns a BSD socket. The FreeRTOS+TCP call-back is a C-function, e.g. of this type: void vOnConnected( Sockett xSocket, BaseTypet xConnected ); I store all Socket_t‘s in a sorted tree, and the corresponding CSocket can be looked-up easily: ~~~ void vOnConnected( Sockett xSocket, BaseTypet xConnected ) { CSocket xCSocket = xLookupSocket( xSocket ); if( xCSocket != NULL ) { xCSocket->vOnConnected( xConnected ); } else { / strange to get here. */ } } ~~~ The addition that you propose doesn’t disturb users that do not use it. We can make it a conditional feature. But the implementation would be complex though: we would get two sets of call-backs (application hooks): the old ones with Socket_t, and the new versions with the user void pointer. And therefore it would lead to a lot of new code and documentation. But I’m curious to hear what Richard thinks of it. Regards.

FreeRTOS-TCP improvement?

I wrote:
But the implementation would be complex though: we would get two sets of call-backs (application hooks): the old ones with Socket_t, and the new versions with th user void pointer
Mistake: that is not needed. Once called you can callFREERTOS_SO_GET_USERDATA to find your user struct.

FreeRTOS-TCP improvement?

Thanks for the response. I’m still unsure of the best way of achieving what I want, my first thought was exactly as you have suggested above, obviously that incurs a lookup penalty (depending on the method of storing). I think it may be the way to go, it’s certainly cleaner than using a select on a group of sockets and easier to manage than using user semaphores. The issue I have is that I have a virtual machine running in a task, it’s even driven so for the most part it’s “asleep”, but there could be various wake up conditions, i.e input changed, uart data, socket data, timer elapsed etc, so thinking about this more it’s probably cleaner to use an event group and search each time for any inputs, uarts, sockets that may have caused the condition. No issues about queues and not being able to store the “event” in the queue (so events getting “lost”), TCP windowing should manage the flow control. I realise this is the reverse of what my original message said and the opposite of the changes I quickly cobbled together! As an aside, it would be nice if on the FreeRTOS documentation page these callbacks were actually documented rather than just saying “for advanced users”, I had to obviously go digging through code to find out the exacty implementation details for the callbacks. It’s also so nice to be back using FreeRTOS, it was always a brilliant piece of software but I think the last version I used was 4 or 5 so there have been a lot of improvements since then, especially the addition of the TCP stack! I’m not a fan of LwIP so happy to never see that again!

FreeRTOS-TCP improvement?

Hi Adrian,
I’m still unsure of the best way of achieving what I want, my first thought was exactly as you have suggested above, obviously that incurs a lookup penalty (depending on the method of storing).
setsockopt() is not implemented as an API but as normal function which reads and writes directly in the socket object. That is the fastest possible way. It is certainly faster than looking op a Socket_t pointer in a list of sockets.
I think it may be the way to go, it’s certainly cleaner than using a select on a group of sockets and easier to manage than using user semaphores.
You will find good example of using FreeRTOS_select() here: ~~~ FreeRTOS-Plus-TCPprotocolsFTP FreeRTOS-Plus-TCPprotocolsHTTP ~~~ It is also efficient and it is mostly compatible with other BSD socket libraries.
but there could be various wake up conditions, i.e input changed, uart data, socket data, timer elapsed etc probably cleaner to use an event group
An event group has either 24 or 8 different “events”, so when woken-up you have a bit-mask you can test and only pay attention to events that have really happened. You can bind a semaphore to a +TCP socket. When the semaphore is given to, you will have to check each event (or socket) that may have triggered. When working Real-Time, with timing constraints, this is not desirable. For me personally, it works fine. Checking e.g. 4 sockets in polling mode only takes very little time… compared to sending a TCP or UDP packets for instance.
I realise this is the reverse of what my original message said and the opposite of the changes I quickly cobbled together!
It is worth to think a long time about a solution. You don’t not want to create a separate task for every new socket.
As an aside, it would be nice if on the FreeRTOS documentation page these callbacks were actually documented rather than just saying “for advanced users”
We feel sorry about that. We felt that these features are less mainstream, less essential, and so they received less attention. As anyone, we always want to do more than time allows us to do. Please read the attachment multiple_sockets_per_task.txt, which describes/summarises the three methods. ~~~ 1) Semaphores 2) Socket call-backs 3) Use select ~~~
It’s also so nice to be back using FreeRTOS
Welcome back !

FreeRTOS-TCP improvement?

And just like that….I hit a wall. Is something broken in the way the recv callback is used? As far as I understand it (from the note you attached), returning 0 from it should cause the data to be stored in the socket so it can be retrieved later using a recv, except it doesn’t, it just endlessly loops around inside “lTCPAddRxdata” because no bytes are consumed and it therefore calls the callback again and again and again and again…. ~~~ if( pxSocket->u.xTCP.pxHandleReceive( (Sockett *)pxSocket, ( void* )ucReadPtr, ( sizet ) ulCount ) != pdFALSE ) { uxStreamBufferGet( pxStream, 0ul, NULL, ( size_t ) ulCount, pdFALSE ); } ~~~ to get the behaviour as described in that note I had to alter it to: ~~~ if( pxSocket->u.xTCP.pxHandleReceive( (Sockett *)pxSocket, ( void* )ucReadPtr, ( sizet ) ulCount ) != pdFALSE ) { uxStreamBufferGet( pxStream, 0ul, NULL, ( size_t ) ulCount, pdFALSE ); continue; }
                    break;
                }
            } //else
        #endif /* ipconfigUSE_CALLBACKS */
~~~ The addition of a continue in the case where the callback consumed, and a break if it didn’t and the removal of the else so that the standard packet processing gets called. This allows me to use the callback to basically generate a “data available” which can then be used to wake up a task (without having to do a select or involving multiple semaphores). I haven’t completely tested this (i.e I haven’t issued the recv), but given that it effectively works like the callback isn’t there, it should all be fine. I’m just in the process of adding extra functionality to test this, I just thought I’d ask the question above based on my observation.

FreeRTOS-TCP improvement?

Pretty sure this my modification here won’t work for all folks, it does in my case because I never consume inside the callback. I implemented a bytes available function in the virtual machine and as expected it keeps going up according to the bytes I send to the socket. Still reading that note you attached the implementation as far as I can see does not match what the note says should happen. Now, I’m almost considering going the semaphore route, and keeping track of the sockets and polling them when a socket event happens, slower and I have to maintain a list of sockets that are in use. (in my case, there’s a virtual machine that is using the sockets)

FreeRTOS-TCP improvement?

Hi Adrian, thanks for your comments.
As far as I understand it (from the note you attached), returning 0 from it should cause the data to be stored in the socket so it can be retrieved later using a recv, except it doesn’t, it just endlessly loops around inside “lTCPAddRxdata” because no bytes are consumed and it therefore calls the callback again and again…
You’re right, at this moment it only works if your function returns ‘true’. I wasn’t aware of that. I should look-up if the code got broken at some point.
to get the behaviour as described in that note I had to alter it to:
~~~
            if( pxSocket->u.xTCP.pxHandleReceive( (Socket_t *)pxSocket, ( void* )ucReadPtr, ( size_t ) ulCount ) != pdFALSE )
            {
                uxStreamBufferGet( pxStream, 0ul, NULL, ( size_t ) ulCount, pdFALSE );
                continue;
            }

            break;
        }
    } //else
#endif /* ipconfigUSE_CALLBACKS */
~~~
Yes, good idea to solve it this way. But it needs some rethinking.
Pretty sure that my modification here won’t work for all folks, it does in my case because I never consume inside the callback.
As the call-back feature is not documented, there won’t be many people using it. And it would work if the callback function returns a non-zero value.
I implemented a bytes available function in the virtual machine and as expected it keeps going up according to the bytes I send to the socket.
There are some (non-BSD) functions check the buffers of a socket: ~~~ /* Return the number of bytes in the RX buffer. */ BaseType_t FreeRTOS_rx_size( Socket_t xSocket );
/* Return the sizeof of available space in the TX buffer. */
BaseType_t FreeRTOS_tx_space( Socket_t xSocket );

/* Return the number of bytes in the TX buffer. */
BaseType_t FreeRTOS_tx_size( Socket_t xSocket );
~~~
Still reading that note you attached the implementation as far as I can see does not match what the note says should happen.
You are right that when the “receive callback” returns zero, the behaviour is not as expected. But I don’t know of other abnormalities.
Now, I’m almost considering going the semaphore route, and keeping track of the sockets and polling them when a socket event happens, slower and I have to maintain a list of sockets that are in use. (in my case, there’s a virtual machine that is using the sockets)
That is indeed another way. Which is very hybrid, in a sense that any task in your system can wake-up the task ( by giving to it’s semaphore ).

FreeRTOS-TCP improvement?

Thanks Hein…I’m not going mad then!

FreeRTOS-TCP improvement?

I think I’m going to archive this branch in the morning, revert back to plain old FreeRTOS and use a sempahore, queue and queue set to achieve what I want, for the most part the changes will be confined to one section of code. It does mean a “poll” of the sockets to see which ones need servicing and why they need servicing, but I don’t envisage many sockets being created anyway. The same mechanism can be used for other things like serial ports etc as well.

FreeRTOS-TCP improvement?

Instead of a runtime lookup, you can just use a container_of macro: struct mysocket { int foo; float bar; Socket_t socket; char *baz; } void vOnConnected( Sockett xSocket, BaseTypet xConnected ) { struct mysocket *s = container_of(xSocket, struct mysocket, socket); … } (see https://code.woboq.org/linux/linux/include/linux/kernel.h.html#928 for an implementation)

FreeRTOS-TCP improvement?

Hi Thomas, that is (more or less) how I looked up a socket reference. But Adrian’s proposal would be cheaper: looking up the user pointer in a socket.

FreeRTOS-TCP improvement?

i dont think that would work in this case seeing as freertos creates the socket, the extra data won’t be there to be accessed.

FreeRTOS-TCP improvement?

Thanks for all your help and thoughts on this, hopefully if I’ve achieved anything it’s that I found a bug in the callback system. I just reverted the tcp stack back to original (i.e removed all my changes) and modified my socket code so that for every socket that is created, an assosicated semaphore and task is created. I don’t imagine many sockets being created, and I need to see what level of stack I can get away with to minimise the pain of having a task associated with every socket. What this means is that the socket task is effectively very simple, upon starting it reads the rx length and current connection state and then waits forever on the semaphore (for the moment), when an event happens it checks if the rx length or connected state has changed, if so it fires off a callback which starts a chain of evens via a message queue which results in the VM processing the event. I sat and thought about using a singleton semaphore and task, but that complicates matters because then I have to poll each socket to find out which one(s) have caused an event, in the end I went for the easier and less error prone option. What would be a really nice addition would be (a non standard funtion!) something like select, but for a single socket: pseudo code: ~~~ xEvent = FreeRTOSwaitforsocketevent(socket, portMAXDELAY); switch(xEvent) { case EVTCONNECTIONSTATUS_CHANGED: break;
     case EVT_RX:
        break;

      case EVT_TX:
        break;
} ~~~ I’ve effectively created this function using a task. Anyway…..