IRATI / stack

RINA implementation for OS/Linux
http://irati.github.io/stack
72 stars 40 forks source link

rina-echo-time broken #1340

Closed fdgonthier closed 3 years ago

fdgonthier commented 3 years ago

On Ubuntu 18.04, commit 4c2329ad9dd4bc7cb0361a6add047ff7d5c86d68 has broken rina-echo-time.

With the commit, this is what I get:

Flow allocation time = 103.63 ms
13265(1617298083)#rina-echo-time (ERR): Failed to clear O_NONBLOCK flag (errno: 9)
13265(1617298083)#rina-echo-time (ERR): write() error: Bad file descriptor

Thread 1 "rina-echo-time" received signal SIGFPE, Arithmetic exception.
0x0000555555561ef7 in Client::pingFlow (this=this@entry=0x7fffffffe120) at et-client.cc:393
393             << "; " << ((sdus_sent - sdus_received)*100/sdus_sent) << "% SDU loss"

With the commit reverted, rina-echo-time works as expected. The floating point error initially mislead me because the problem is actually that librina tries to do some operations on an invalid file descriptor.

Ie: at the end of createFlow in et-client.cc, the following code grabs the file descriptor from the flow:

        if (!quiet) {
                cout << "Flow allocation time = "
                     << time_difference_in_ms(begintp, endtp) << " ms" << endl;
        }

        **fd = flow.fd;**
        port_id = flow.portId;

flow.fd there never can be initialized there because initIodev never gets called. Now maybe the bug is actually in rina-echo-time either and there is a proper way to do this but I'm a little too green right now to figure out a proper resolution.

edugrasa commented 3 years ago

Hi,

You are absolutely right, thanks for spotting this one.

The underlying problem here is that rina-echo-time still uses and old way of interacting with the RINA IRATI implementation (via librina APIs). All applications should use the RINA API defined in https://github.com/IRATI/stack/blob/master/librina/wrap/rina-api/include/rina/api.h, which is also the same API used by the rlite implementation (documentation is here: https://github.com/rlite/rlite#9-rina-api-documentation.

One option to fix this is to update rina-echo-time to use the proper RINA API. But since the functionality of this application is already included in rinaperf, maybe it would be better to just remove rina-echo-time from the repo.

fdgonthier commented 3 years ago

Closing issue as the rina-echo-time is now removed from master.