mtcp-stack / mtcp

mTCP: A Highly Scalable User-level TCP Stack for Multicore Systems
Other
1.98k stars 436 forks source link

Maybe a bug in RaisePendingStreamEvents #201

Closed fomy closed 6 years ago

fomy commented 6 years ago
    if (socket->epoll & MTCP_EPOLLOUT) {
        struct tcp_send_vars *sndvar = stream->sndvar;
        if (!sndvar->sndbuf || 
                (sndvar->sndbuf && sndvar->sndbuf->len < sndvar->snd_wnd)) {
            if (!(socket->events & MTCP_EPOLLOUT)) {
                TRACE_EPOLL("Socket %d: Adding write event\n", socket->id);
                AddEpollEvent(ep, USR_SHADOW_EVENT_QUEUE, socket, MTCP_EPOLLOUT);
            }
        }
    }

Hi,

I notice that only when sndvar->sndbuf->len < sndvar->snd_wnd, mtcp will raise an event. Should this condition be sndvar->snd_wnd > 0?

Thanks

eunyoung14 commented 6 years ago

Hi, this block checks whether there is send buffer available.

  1. mTCP's sending buffer is created on demand. If sndbuf is null, it means that there was no prior mtcp_write call and we can create new send buffer in the future.
  2. If sending buffer is already present, it checks whether there is enough room in the buffer. If the length of payloads in the buffer is less than the send window, it means that the app can send more data.

If one of those two conditions is satisfied, mTCP raises an MTCP_EPOLLOUT event so that the application can send. If sndvar->snd_wnd is 0, I think the application shouldn't send anything.

fomy commented 6 years ago

Sure.

But your code is

sndvar->sndbuf->len < sndvar->snd_wnd

rather than sndvar->snd_wnd > 0

fomy commented 6 years ago

Here is the code where snd_wnd is calculated:

sndvar->snd_wnd = sndvar->sndbuf->size - sndvar->sndbuf->len;

sndvar->snd_wnd indicates how many room available in the buf, and sndbuf->len indicates how many bytes are already in the buf.

sndvar->sndbuf->len < sndvar->snd_wnd will cause half of the buf cannot be used.

eunyoung14 commented 6 years ago

Now it makes sense! I was confused a little before you edited the comment. I'll fix the condition to sndvar->snd_wnd > 0. Thanks for reporting.