[En-Nut-Discussion] Bug in UDP socket handling

[Mike Cornelius]

Hi Oliver,

I just noticed this too...

Harald, Why does udpin look like this:-

/*
 * If we stored a previous buffer, remove
 * it and store the new one.
 */
if (sock->so_rx_nb)
	NutNetBufFree(sock->so_rx_nb);
sock->so_rx_nb = nb;

Why not buffer?

Mike



-----Original Message-----
From: en-nut-discussion-admin at egnite.de
[mailto:en-nut-discussion-admin at egnite.de] On Behalf Of Oliver Schulz
Sent: Wednesday, October 22, 2003 11:38 PM
To: en-nut-discussion at egnite.de
Subject: [En-Nut-Discussion] Bug in UDP socket handling


Hello All, hello Harald.

It seems, that I found a bug or a general problem in UDP socket
handling.

I figured out to situations where the bug appears.

1. 
As soon as you create a UDP socket with 'NutUdpCreateSocket' the socket
is ready to receive UDP packets. If a packet is received before some
thread calls 'NutUdpReceiveFrom' the netbuf is stored in the socket and
the receiving event is posted. Because nobody waits for the event at
this time, the event queue is set to SIGNALED. In the first call of
'NutUdpReceiveFrom' the already received UDP packet is noticed and will
be returned to the caller. The netbuf pointer in the socket is set to 0,
but the event queue is still SIGNALED. The next call to
'NutUdpReceiveFrom' will immediately fail, because the event is
SIGNALED, but no netbuf is stored in the socket. The return value is
then 0, which indicates a timeout event...

2. 
Consider that one thread has called 'NutUdpReceiveFrom' on an empty
socket (that means no packets was received yet) and is sleeping now.
Then two or more packets are received one immediately after the other.
That happens for example as an answer from several hosts to a UDP
broadcast (I used NutSNTPGetTime). The first received packet is normally
routed through the IP stack, UDPIn finds the correct socket, stores the
netbuf in the socket and posts the event to the queue. Because only one
thread was in the queue, the queue points now to 0x0000, indicating an
empty queue. The formerly sleeping thread is inserted in the 'runQueue'
and marked as ready to run.

But then the nic receive thread 'rxi5' (which is still running) sees the
next received UDP packets and routes it again to the stack. UDPIn
notices that there's already a stored UDP packet and discards it. The
new packet will be stored in the socket and the event is posted again.
Because the event queue was empty before, it now has the SIGNALED value.
If then the sleeping thread (caller of 'NutUdpReceiveFrom') becomes
active, it get the last received UDP packet and removes the netbuf from
the socket. Because the event queue is still SIGNALED, the next call to
'NutUdpReceiveFrom' will fail like in situation 1.


To fix this issue I suggest to introduce also for UDP sockets a kind of
received packets queue. The NETBUF struct already has a pointer to
another NETBUF for generating lists, so its not very complicated to add
this feature.

If it's done you will not lose UDP packets anymore. Before UDPIn posts
the received event, it has to check the queue. If empty, it must not
post the event. Otherwise we will get the SIGNALED state and
'NutUdpReceiveFrom' can fail again.

Any comments?

Cheers,
Oliver.


_______________________________________________
En-Nut-Discussion mailing list
En-Nut-Discussion at egnite.de
http://www.egnite.de/mailman/listinfo/en-nut-discussion