Controlling network driver access between prvEMACHandlerTask and xNetworkInterfaceOutput

heinbali01 wrote on Thursday, August 18, 2016:

if ipconfigZERO_COPY_TX_DRIVER == 1, the IP task can keep running until
it attempts to xmit again and then stall if the transmit operation is
still active.

I don;t think so. Suppose that the IP-task wants to send a new packet while the first one hasn’t been delivered yet, the new packet will stay in the EMAC working queue. This queue serves as a buffer between the two tasks.

Performance-wise, I think it is important that the IP-task keeps on working also when the (relatively slow) SPI bus is busy. In a high-volume TCP conversation, the IP-task has a lot of work to do.

joehinkle wrote on Thursday, August 18, 2016:

I agree with you if stack processing is a major application concern.

I don’t think there is one right answer, it comes down to what are the requirements of the application.

Queueing keeps the IP stack processing as fast as possible – trade off is more memory usage for queues and more code complexity.

I was biased in my previous statement as I did NOT implement a SPI interface - I was interfacing directly with a ENET – so my time delay was very small. SPI delay for xmit could be VERY long in comparison.

glenenglish wrote on Saturday, August 20, 2016:

thanks all for the input.

My perspective is that IPTask should be busy preparing and processing packets.
someone else should do the waiting. - IE in this case another thread or threads. Given that there is only one device attachment facility, if you will , IE the SPI bus, in this application, it makes sense for the EMACS task to wait on either rx interrupt OR wait new descriptor available for sending, one or the other since it can only do one or the other.

The only thing that should stop IPTask would be if there is no free buffers to do anything with.
If EMACS runs out of buffers to put stuff, it still has to read and dump the packet in the ethernet chip buffers, and then it can just dump it. And of course IPTask waiting on the SPI makes the situation WORSE because the way to free up buffers is for IPTask to process receive packets- which eventually freesup the buffer.

If there are no buffers for assembling a new packet, that of course goes back to the caller and fails the send.

Now, what I DO need to look through the code is to figure out what happens in the case that the interruot handler YieldFromISR() executes but the ‘target thread’ in this case, EMACS, is NOT waiting- busy doing something else. I’ll look at the source code and trace that one through. My feeling is that if YieldFrom ISR executes and the target thread is not blocked, then Yield from ISR should do nothing…

YieldFromISR is a very effective and handy feature in FreeRTOS. I was quite impressed when I read the feature.

see, I have 500 pps of 1kB size going in and out , (in addition to the dsp work) and the buffers are fairly small so there is little time to waste, IE my implementation needs to be efficient.

The SPI DMA timing is much tighter. (the push is on for latency also so I am just using the double-buffer feature in the STM, am not chaining say 8 buffers) …I could probably get away with not using YieldFromISR on the ethernet IRQhandler, but on the SPI DMA completion handler, yield forn ISR is VERY handing at meeting the very strict SPI buffer deadline.

cheers

heinbali01 wrote on Saturday, August 20, 2016:

The only thing that should stop IPTask would be if there is no free buffers to
do anything with.

In the example drivers, the number of available Network Buffers is monitored and logged when it drops.

You could add a rule, e.g. stop using Network Buffers when the number drops below e.g. 3.

When the IP-task finds that there are no more Network Buffers, it will give up and wait for the next thing to do.

My feeling is that if YieldFrom ISR executes and the target
thread is not blocked, then Yield from ISR should do nothing…

void MY_ISR()
{
BaseType_t xSwitchRequired = pdFALSE;

    /* Check the interrupt status. clear the necessary flags. */
    xQueueSendFromISR( xQueue, &( xItem ), &( xSwitchRequired ) );
    portEND_SWITCHING_ISR( xSwitchRequired );	/* formerly called portYIELD_FROM_ISR() *
}

Sounds correct to me. If the ISR calls e.g. xQueueSendFromISR (), this might unblock your prvEMACHandlerTask(). But if that task was already in a non-blocked state, it will just carry on. The next call to xQueueReceive() will succeed without blocking.

Your prvEMACHandlerTask() will always do very short transactions and therefore I would give it a high priority.
The latency between calling portEND_SWITCHING_ISR() and the unblocking of the task is very small.

Using two alternating SPI buffers sounds good.

Regards.

glenenglish wrote on Saturday, August 20, 2016:

Hi Hein.
agreed on all your comments.

I implemented a QueueSet

Queue Length of 1 for the BinarySemaphore from the ISR
and Queue length of nbuffer descriptors (maybe need to think about that more) for the NetworkOutput thread.
NetworkOutputThread posts a type containing the Descriptor and the bReleaseAfterSend items and returns immediately to IPTask main.

The EMACS-task waits on the QueueSet,

I could have implemented as a single queue, but then it is possible that the IRQ handler might be way down the queue if NetworkOutput pushed alot of sends into the queue… I want the irq handler semaphore to be handled next iteration, so the queue set is better for that.

It worked first go. (took about an hour) . seems to be solid. I’ll have a look at the YieldFromISR rtos code.
cheers

heinbali01 wrote on Saturday, August 20, 2016:

I implemented a QueueSet

I’m not sure if that is useful / necessary.

I thought of a single working queue with each element having the size of the struct EMACEvent_t:

For example:

typedef enum
{
	eEMAC_SendPacket,
	eEMAC_CheckPHY,
	eEMAC_TXDone,
	eEMAC_RXDone,
    ...
} eEMACEvent_t;

typedef struct
{
	eEMACEvent_t eEventType;
	void *pxData;
} EMACEvent_t;

So for every event, 8 bytes will be queued. The field pxData may point to a Network Buffer or remain NULL if not in use.

The boolean parameter bReleaseAfterSend will always be pdTRUE if you define :

    #define ipconfigZERO_COPY_TX_DRIVER     1

So xNetworkInterfaceOutput() can be implemented as I described here above. Just queue the Network Buffer.

a single queue, but then it is possible that the IRQ handler might be way
down the queue if NetworkOutput pushed alot of sends into the queue

There is an alternative: you can define:

    #define ipconfigUSE_LINKED_RX_MESSAGES      1

It will add a pxNextBuffer pointer to each Network Buffer ( see include/FreeRTOS_IP.h ).

Now suppose that you receive a mix of ISR-events and packets to be sent:

You can put the packets together (link them) by setting the pxNextBuffer field:

switch( xMsg.eEventType )
{
    case eEMAC_SendPacket:
    {
        NetworkBufferDescriptor_t * pxNew = ( NetworkBufferDescriptor_t * ) xMsg.pxData;
	    pxNew->pxNextBuffer = NULL;
        if( pxFirst == NULL )
        {
            pxFirst = pxNew;
        }
        if( pxLast != NULL )
        {
            pxLast->pxNextBuffer = pxNew;
        }
        pxLast = pxNew;
    }
    break;
    case eEMAC_CheckPHY:
    ...

In that way you can empty the working queue each time the task wakes up.

If you want you can also write me directly ( h point tibosch at freertos point org ), then maybe we can go quicker through all this. And I’m not sure if all these implementation details are interesting for everyone :slight_smile:

glenenglish wrote on Saturday, August 20, 2016:

Hi Hein
Quite happy to keep it in forum if it is beneficial for someone doing similar and doing a search.

I’m very happy with the degree of assistnce you are providing.

OK on the ipconfigUSE_LINKED_RX_MESSAGES. I will read up.


Something on the queues, and similar sort of structures…
xQueueCreate()
and
xQueueCreateStatic()

The static version would be useful for fast access say out of TCM / or NON cached memory (instead of potentially causing a cache miss) - a nice function.


cheers