CCI_EVENT_NONE used in tcp_handle_conn_reply

[robstewart57]

In the event_type typedef, the documentation for CCI_EVENT_NONE states that:

"/*! Never use - for internal CCI use only. */"

However, in the implementation of tcp_handle_conn_reply it is used here. The receiver when they next call cci_get_event(..) will receive this "internal" event.

Has the CCI_EVENT_NONE event been misused in the implementation of tcp_handle_conn_reply ?

[scottatchley]

On May 13, 2013, at 10:59 AM, Rob Stewart notifications@github.com wrote:

In the event_type typedef, the documentation for CCI_EVENT_NONE states that:

"/*! Never use - for internal CCI use only. */"

However, in the implementation of tcp_handle_conn_reply it is used here. The receiver when they next call cci_get_event(..) will receive this "internal" event.

Has the CCI_EVENT_NONE event been misused in the implementation of tcp_handle_conn_reply ?

Hi Rob,

I believe this is fixed in master. Are you using the latest? If so, can you provide a test?

Thanks,

Scott

[robstewart57]

HEAD of master still uses CCI_EVENT_NONE in tcp_handle_conn_reply.
https://github.com/CCI/cci/blob/master/src/plugins/ctp/tcp/ctp_tcp_api.c

Is it fixed in a local branch not yet pushed to github's master branch?

[scottatchley]

On May 13, 2013, at 11:12 AM, Rob Stewart notifications@github.com wrote:

HEAD of master still uses CCI_EVENT_NONE in tcp_handle_conn_reply.
https://github.com/CCI/cci/blob/master/src/plugins/ctp/tcp/ctp_tcp_api.c

Is it fixed in a local branch not yet pushed to github's master branch?

This is generating the TCP_MSG_CONN_ACK. We do not use it to generate a user event. In tcp_progress_conn_sends(), we see that it is a TCP_MSG_CONN_ACK and we recycle the tx.

We set the type to CCI_EVENT_NONE as a sanity check. Thus, if we accidentally return it to the user, it should raise red flags. :-)

This part was added in commit 946ffc6:

TCP: vastly improve the connection state handling

But many other changes were needed after that.

What are you seeing?

Scott

[robstewart57]

The latest I have on this. Question: Should CCI_EVENT_NONE ever be seen in ctp_tcp_get_event ? I have an (unfortunately non-trivial) application, that produces this behaviour. When I run it with CCI_DEBUG=all, I see the following:

cci: entering ctp_tcp_get_event
cci: tcp_poll_events: poll found 1 events
cci: tcp_poll_events: revents 0x1
cci: tcp_handle_recv: conn 0x7fb012405730 recv'd message
cci: tcp_handle_recv: msg type conn_ack a=0 b=1 conn=0x7fb012405730
cci: tcp_handle_conn_ack: recv'd conn_ack from conn 0x7fb012405730
cci: tcp_handle_conn_ack: conn 0x7fb012405730 ready
cci: entering tcp_progress_queued
cci: exiting  tcp_progress_queued
cci: ctp_tcp_get_event: found CCI_EVENT_NONE on conn 0x7fb012405730
cci: exiting  ctp_tcp_get_event

So, am I correct in thinking CCI_EVENT_NONE is returned by cci_get_event, using the tcp implementation?

[scottatchley]

On May 13, 2013, at 5:14 PM, Rob Stewart notifications@github.com wrote:

The latest I have on this. Question: Should CCI_EVENT_NONE ever be seen in ctp_tcp_get_event ? I have an (unfortunately non-trivial) application, that produces this behaviour. When I run it with CCI_DEBUG=all, I see the following:

cci: entering ctp_tcp_get_event
cci: tcp_poll_events: poll found 1 events
cci: tcp_poll_events: revents 0x1
cci: tcp_handle_recv: conn 0x7fb012405730 recv'd message
cci: tcp_handle_recv: msg type conn_ack a=0 b=1 conn=0x7fb012405730
cci: tcp_handle_conn_ack: recv'd conn_ack from conn 0x7fb012405730
cci: tcp_handle_conn_ack: conn 0x7fb012405730 ready
cci: entering tcp_progress_queued
cci: exiting tcp_progress_queued
cci: ctp_tcp_get_event: found CCI_EVENT_NONE on conn 0x7fb012405730
cci: exiting ctp_tcp_get_event

So, am I correct in thinking CCI_EVENT_NONE is returned by cci_get_event, using the tcp implementation?

Ok, this clearly is a bug.

Let me send you a patch today that will give us more info about the event.

Scott

[robstewart57]

OK no problem, happy to apply and re-run. My use case program always gives exactly the same erroneous behaviour, so reproducing it won't be a problem.

[scottatchley]

On May 14, 2013, at 7:25 AM, Rob Stewart notifications@github.com wrote:

OK no problem, happy to apply and re-run. My use case program always gives exactly the same erroneous behaviour, so reproducing it won't be a problem.

Rob,

I have attached a patch.

Can you tell more about the use case? Is a single client connecting multiple times? Is the server handling multiple connections from multiple peers in parallel? Are the peers trying to connect to each other concurrently?

Scott

[robstewart57]

Scott, attachments don't work via GitHub comments. Perhaps send to the email address that you have for me.

[robstewart57]

Scott, I have produced three runs that produce CCI_EVENT_NONE. I have snipped with "...." repetitions of the same tcp_progress_queued and tcp_get_event sequences. Here they are:
https://gist.github.com/robstewart57/5576106

[scottatchley]

Hmm, I replied with another patch inline (i.e. in the text). It does not seem like github accepted it.

[scottatchley]

Rob,

I have pushed a fix. Can you try it out?

Scott

[robstewart57]

Hi Scott,

I can confirm that commit 2c6975d fixed the issue. I cannot reproduce the erroneous behaviour. Thanks!

[scottatchley]

Thanks for the bug report and testing!

I'll close this issue.