Subject: Re: [dtn-users] A problem with dtntunnel

From: "[email-masked]" <[email-masked]>
Date: Mon, 17 Dec 2012 15:39:54 +0300

Hi David,

Thanks for your patch, it works! :) Connection is cleanly terminated now.

However, there still seems to be a problem. I run dtntunnel over a link
with high latency and about 30% packet loss. The receiving nc sometimes
gets FIN packet before it receives all the data. If I understand it right,
on a lossy link bundles may come out of order. In this case the EOF bundle
may come too early. I think that the reordering procedure in dtntunnel
should be reviewed. Just for information, pure TCP performs better that DTN
over this link.

2012/12/14 David Zoller <[email-masked]>

> Hi Sergey,
> I can't stay away from a problem that is nagging at me so I made some time
> this morning. Going back to the original TCPTunnel.cc code, when a bundle
> is received that has the EOF indication, an empty bundle with the same EOF
> flag set needs to be returned to close the loop (the second FIN). There may
> still be some loose ends or clean up needed but try the code below at the
> end of the while loop.
>
> I did not do a packet capture but I saw that if the receiving nc is not
> running when you start the sender nc then the receiving dtntunnel logs an
> error on the failed connect and returns an EOF bundle which closes the
> sender nc. The EOF bundle could possibly be enhanced to include an error
> message.
>
> I agree that some applications could benefit from a "pretend to be a
> receiver" option with the default working as a bidirectional end-to-end TCP
> connection.
>
> Thanks for dragging me along kicking and screaming on this lesson in DTN
> and TCP tunneling. I've only been using dtntunnel as a one way UDP
> transmitter. I see a good fit for the TCP tunnel in an upcoming project and
> I'll try to make the pitch to use it. This is a pretty cool little app.
> Best regards,
> DZ
>
>
>
>
>
> 484             delete b_recv;
> 485
> 486             if (recv_hdr->eof_) {
> 487                 log_info("bundle had eof bit set... closing
> connection");
> 488
> 489                 if ( !sock_eof ) {
> 490                     sock_eof = true;
> 491                     // send an empty bundle back since we did not
> initiate the close
> 492                     dtn::APIBundle* b = new dtn::APIBundle();
> 493                     hdr.eof_ = 1;
> 494                     hdr.seqno_ = ntohl(send_seqno++);
> 495                     memcpy(b->payload_.buf(sizeof(hdr)), &hdr,
> sizeof(hdr));
> 496                     b->payload_.set_len(sizeof(hdr));
> 497                     int err;
> 498                     if ((err = tunnel->send_bundle(b, &dest_eid_)) !=
> DTN_SUCCESS) {
> 499                         log_err("error sending final socket closed
> packet bundle: %s",
> 500                                 dtn_strerror(err));
> 501                         tcptun_->kill_connection(this);
> 502                         exit(1);
> 503                     }
> 504                     delete b;
> 505                 }
> 506                 sock_.close();
> 507                 goto done;
> 508             }
> 509         }
> 510     }
> 511
> 512  done:
>
>

-- 
Best regards,
Sergey Sireskin

---

From: "[email-masked]" <[email-masked]>
Date: Mon, 17 Dec 2012 16:09:07 +0300

A series of tests showed that the original dtntunnel doesn't have the
problem with a too early FIN.

-- 
Best regards,
Sergey Sireskin

---

From: "Zoller, David A. (MSFC-EO60)[HOSC SERVICES CONTRACT]" <[email-masked]>
Date: Mon, 17 Dec 2012 07:15:18 -0600

Hi Sergey,
I don't have your setup and this has not been tested but give this a try. In TCPTunnel::Connection::handle_bundle in the while(1) loop add the test to see if the next_seqno_ is a match:

    while (1) {
        iter = reorder_table_.find(next_seqno_);
        if (iter == reorder_table_.end()) {
            break;
        }

        // add this code:
        // if seqno does not match then still a bundle missing
        if (iter->first != next_seqno_) {
            break;
        }

        bundle = iter->second;
        log_debug("delivering %zu byte bundle with seqno %d (from reorder table)",
                  bundle->payload_.len(), next_seqno_);

        reorder_table_.erase(iter);
        next_seqno_++;

        queue_.push_back(bundle);
    }

David Zoller
COLSA Corporation
HOSC / C107
*Office: (256) 544-1820
*EMail: [email-masked]<mailto:[email-masked]>

From: [email-masked] [mailto:[email-masked]] On Behalf Of [email-masked]
Sent: Monday, December 17, 2012 6:40 AM
To: David Zoller
Cc: [email-masked]
Subject: Re: [dtn-users] A problem with dtntunnel

Hi David,

However, there still seems to be a problem. I run dtntunnel over a link with high latency and about 30% packet loss. The receiving nc sometimes gets FIN packet before it receives all the data. If I understand it right, on a lossy link bundles may come out of order. In this case the EOF bundle may come too early. I think that the reordering procedure in dtntunnel should be reviewed. Just for information, pure TCP performs better that DTN over this link.
2012/12/14 David Zoller <[email-masked]<mailto:[email-masked]>>
Hi Sergey,
I can't stay away from a problem that is nagging at me so I made some time this morning. Going back to the original TCPTunnel.cc code, when a bundle is received that has the EOF indication, an empty bundle with the same EOF flag set needs to be returned to close the loop (the second FIN). There may still be some loose ends or clean up needed but try the code below at the end of the while loop.

I did not do a packet capture but I saw that if the receiving nc is not running when you start the sender nc then the receiving dtntunnel logs an error on the failed connect and returns an EOF bundle which closes the sender nc. The EOF bundle could possibly be enhanced to include an error message.

I agree that some applications could benefit from a "pretend to be a receiver" option with the default working as a bidirectional end-to-end TCP connection.

Thanks for dragging me along kicking and screaming on this lesson in DTN and TCP tunneling. I've only been using dtntunnel as a one way UDP transmitter. I see a good fit for the TCP tunnel in an upcoming project and I'll try to make the pitch to use it. This is a pretty cool little app.
Best regards,
DZ

484             delete b_recv;
485
486             if (recv_hdr->eof_) {
487                 log_info("bundle had eof bit set... closing connection");
488
489                 if ( !sock_eof ) {
490                     sock_eof = true;
491                     // send an empty bundle back since we did not initiate the close
492                     dtn::APIBundle* b = new dtn::APIBundle();
493                     hdr.eof_ = 1;
494                     hdr.seqno_ = ntohl(send_seqno++);
495                     memcpy(b->payload_.buf(sizeof(hdr)), &hdr, sizeof(hdr));
496                     b->payload_.set_len(sizeof(hdr));
497                     int err;
498                     if ((err = tunnel->send_bundle(b, &dest_eid_)) != DTN_SUCCESS) {
499                         log_err("error sending final socket closed packet bundle: %s",
500                                 dtn_strerror(err));
501                         tcptun_->kill_connection(this);
502                         exit(1);
503                     }
504                     delete b;
505                 }
506                 sock_.close();
507                 goto done;
508             }
509         }
510     }
511
512  done:

--
Best regards,
Sergey Sireskin

---

From: "[email-masked]" <[email-masked]>
Date: Mon, 17 Dec 2012 16:43:22 +0300

David,

I added this code and a log_debug inside the if-clause. According to the
log, this condition doesn't get true.
A have attached my logs and tcpdumps. Please, see dtntunnel_T.log - it's a
receiving dtntunnel log.
There are 4 bundles after a eof bundle. This doesn't happen with the
original dtntunnel.

-- 
Best regards,
Sergey Sireskin

2012/12/17 Zoller, David A. (MSFC-EO60)[HOSC SERVICES CONTRACT] <
[email-masked]>

> Hi Sergey,****
>
> I don’t have your setup and this has not been tested but give this a try.
> In TCPTunnel::Connection::handle_bundle in the while(1) loop add the test
> to see if the next_seqno_ is a match:****
>
> ** **
>
>     while (1) {****
>
>         iter = reorder_table_.find(next_seqno_);****
>
>         if (iter == reorder_table_.end()) {****
>
>             break;****
>
>         }****
>
> ** **
>
>         // add this code:****
>
>         // if seqno does not match then still a bundle missing****
>
>         if (iter->first != next_seqno_) {****
>
>             break;****
>
>         }****
>
> ** **
>
>         bundle = iter->second;****
>
>         log_debug("delivering %zu byte bundle with seqno %d (from reorder
> table)",****
>
>                   bundle->payload_.len(), next_seqno_);****
>
>     ****
>
>         reorder_table_.erase(iter);****
>
>         next_seqno_++;****
>
>     ****
>
>         queue_.push_back(bundle);****
>
>     }   ****
>
> ** **
>
> ** **
>
> ** **
>
> ** **
>
> David Zoller****
>
> COLSA Corporation****
>
> HOSC / C107  ****
>
> (Office: (256) 544-1820
> *EMail: [email-masked]****
>
> ** **
>
> *From:* [email-masked] [mailto:[email-masked]] *On
> Behalf Of [email-masked]
> *Sent:* Monday, December 17, 2012 6:40 AM
> *To:* David Zoller
>
> *Cc:* [email-masked]
> *Subject:* Re: [dtn-users] A problem with dtntunnel****
>
> ** **
>
> Hi David,
>
>
> over this link.****
> 2012/12/14 David Zoller <[email-masked]>****
>
> Hi Sergey,****
>
> end of the while loop.****
>
> ** **
>
> message.****
> ** **
> connection.****
> ** **
> I'll try to make the pitch to use it. This is a pretty cool little app.***
> *
>
> Best regards,****
>
> DZ****
>
> ** **
>
> ** **
>
> ** **
>
> ** **
>
> ** **
>
> 484             delete b_recv;****
>
> 485 ****
>
> 486             if (recv_hdr->eof_) {****
>
> connection");****
> 488 ****
> 489                 if ( !sock_eof ) {****
>
> 490                     sock_eof = true;****
>
> initiate the close****
> 492                     dtn::APIBundle* b = new dtn::APIBundle();****
>
> 493                     hdr.eof_ = 1;****
>
> 494                     hdr.seqno_ = ntohl(send_seqno++);****
>
> sizeof(hdr));****
> 496                     b->payload_.set_len(sizeof(hdr));****
>
> 497                     int err;****
>
> DTN_SUCCESS) {****
> packet bundle: %s",****
> 500                                 dtn_strerror(err));****
>
> 501                         tcptun_->kill_connection(this);****
>
> 502                         exit(1);****
>
> 503                     }****
>
> 504                     delete b;****
>
> 505                 }****
>
> 506                 sock_.close();****
>
> 507                 goto done;****
>
> 508             }****
>
> 509         }****
>
> 510     }****
>
> 511 ****
>
> 512  done:****
>
> ** **
>
>
>
>
> --
> Best regards,
> Sergey Sireskin
>
> ****
>

---

From: "[email-masked]" <[email-masked]>
Date: Mon, 17 Dec 2012 17:04:37 +0300

David,

The problem is probably not with the reordering. Can it be, that "if
(recv_hdr->eof_)" is not the right condition to close a connection? Or
maybe it is not a sufficient condition? Which connection is recv_hdr about?
Can it be a local TCP connection on the receiving side (dtntunnel <-> nc)?
If it is, then IMO this condition becomes true when the receiving
application doesn't send any data to the sending application, please see
line 406 of the original code.

2012/12/17 [email-masked] <[email-masked]>

>> I can't stay away from a problem that is nagging at me so I made some
>> time this morning. Going back to the original TCPTunnel.cc code, when a
>> bundle is received that has the EOF indication, an empty bundle with the
>> same EOF flag set needs to be returned to close the loop (the second FIN).
>> There may still be some loose ends or clean up needed but try the code
>> below at the end of the while loop.****
>>
>> ** **
>>
>> I'll try to make the pitch to use it. This is a pretty cool little app.**
>> **
>>
>> Best regards,****
>>
>> DZ****
>>
>> ** **
>>
>> ** **
>>
>> ** **
>>
>> ** **
>>
>> ** **
>>
>> 484             delete b_recv;****
>>
>> 485 ****
>>

-- 
Best regards,
Sergey Sireskin

---

From: "Zoller, David A. (MSFC-EO60)[HOSC SERVICES CONTRACT]" <[email-masked]>
Date: Mon, 17 Dec 2012 13:40:04 -0600

Sergey,
That was my bad. In the patch below I moved the "delete b_recv;" line above the "if (recv_hdr->eof_) {" block as a short cut to delete it before the possible exit. Move it back down below the if block and you should be okay.
Sorry, that was one of those possible loose ends I mentioned :),
DZ

David Zoller
COLSA Corporation
HOSC / C107
*Office: (256) 544-1820

From: [email-masked] [mailto:[email-masked]]
Sent: Monday, December 17, 2012 8:05 AM
To: Zoller, David A. (MSFC-EO60)[HOSC SERVICES CONTRACT]
Cc: David Zoller; [email-masked]
Subject: Re: [dtn-users] A problem with dtntunnel

David,

The problem is probably not with the reordering. Can it be, that "if (recv_hdr->eof_)" is not the right condition to close a connection? Or maybe it is not a sufficient condition? Which connection is recv_hdr about? Can it be a local TCP connection on the receiving side (dtntunnel <-> nc)? If it is, then IMO this condition becomes true when the receiving application doesn't send any data to the sending application, please see line 406 of the original code.
2012/12/17 [email-masked]<mailto:[email-masked]> <[email-masked]<mailto:[email-masked]>>
David,

I added this code and a log_debug inside the if-clause. According to the log, this condition doesn't get true.
A have attached my logs and tcpdumps. Please, see dtntunnel_T.log - it's a receiving dtntunnel log.
There are 4 bundles after a eof bundle. This doesn't happen with the original dtntunnel.

--
Best regards,
Sergey Sireskin

2012/12/17 Zoller, David A. (MSFC-EO60)[HOSC SERVICES CONTRACT] <[email-masked]<mailto:[email-masked]>>

Hi Sergey,

    while (1) {

        // add this code:

        bundle = iter->second;

David Zoller
COLSA Corporation
HOSC / C107
*Office: (256) 544-1820

From: [email-masked]<mailto:[email-masked]> [mailto:[email-masked]<mailto:[email-masked]>] On Behalf Of [email-masked]<mailto:[email-masked]>
Sent: Monday, December 17, 2012 6:40 AM
To: David Zoller

Cc: [email-masked]<mailto:[email-masked]>
Subject: Re: [dtn-users] A problem with dtntunnel

Hi David,

484             delete b_recv;
485

--
Best regards,
Sergey Sireskin

--
Best regards,
Sergey Sireskin

---

From: "[email-masked]" <[email-masked]>
Date: Tue, 18 Dec 2012 10:40:20 +0300

Hi David,

Now it works like a charm, thank you very much!

> Sergey,****
>
> That was my bad. In the patch below I moved the *“delete b_recv;”* line
> above the *“if (recv_hdr->eof_) {”* block as a short cut to delete it
> before the possible exit. Move it back down below the if block and you
> should be okay.****
>
> Sorry, that was one of those possible loose ends I mentioned J,****
>
> DZ****
>
> ** **
>
> David Zoller****
>
> COLSA Corporation****
>
> HOSC / C107  ****
>
> (Office: (256) 544-1820
> *From:* [email-masked] [mailto:[email-masked]]
> *Sent:* Monday, December 17, 2012 8:05 AM
> *To:* Zoller, David A. (MSFC-EO60)[HOSC SERVICES CONTRACT]
> *Cc:* David Zoller; [email-masked]
>
> line 406 of the original code.****
>
> 2012/12/17 [email-masked] <[email-masked]>****
>
> David,
>
> original dtntunnel. ****
> [email-masked]>****
>
> ** **
>
> Hi Sergey,****
>
> *To:* David Zoller****
> Hi David,****
> Sergey Sireskin****

-- 
Best regards,
Sergey Sireskin

---

From: "Zoller, David A. (MSFC-EO60)[HOSC SERVICES CONTRACT]" <[email-masked]>
Date: Tue, 18 Dec 2012 05:58:07 -0600

Hi Sergey,
Looking at the rest of the code, there are probably a couple of memory leaks. The biggie is after the call to "send_bundle(b_xmit" on success b_xmit should be deleted before setting it to NULL (near line 432). Similarly after the send_bundle for the "error connecting" the APIBundle b should also be deleted before going to done (near line 342).
You're welcome,
DZ

David Zoller
COLSA Corporation
Marshall Space Flight Center

Sent: Tuesday, December 18, 2012 1:40 AM

Hi David,

David Zoller
COLSA Corporation
HOSC / C107
*Office: (256) 544-1820

From: [email-masked]<mailto:[email-masked]> [mailto:[email-masked]<mailto:[email-masked]>]
Sent: Monday, December 17, 2012 8:05 AM
To: Zoller, David A. (MSFC-EO60)[HOSC SERVICES CONTRACT]
Cc: David Zoller; [email-masked]<mailto:[email-masked]>

David,

Hi Sergey,

    while (1) {

        // add this code:

        bundle = iter->second;

David Zoller
COLSA Corporation
HOSC / C107
*Office: (256) 544-1820

Hi David,

484             delete b_recv;
485

--
Best regards,
Sergey Sireskin

--
Best regards,
Sergey Sireskin

--
Best regards,
Sergey Sireskin

---

From: "[email-masked]" <[email-masked]>
Date: Tue, 18 Dec 2012 15:07:47 +0300

Hi David,

Thanks for the hint. I have also noticed that memory usage of dtntunnel
noticeably grows while running iperf with multiple flows.
Are you going to submit all the fixes to mercurial?

-- 
Best regards,
Sergey Sireskin

2012/12/18 Zoller, David A. (MSFC-EO60)[HOSC SERVICES CONTRACT] <
[email-masked]>

> Hi Sergey,****
>
> Looking at the rest of the code, there are probably a couple of memory
> leaks. The biggie is after the call to “*send_bundle(b_xmit*” on success
> b_xmit should be deleted before setting it to NULL (near line 432).
> Similarly after the send_bundle for the “error connecting” the APIBundle b
> should also be deleted before going to done (near line 342).****
>
> You’re welcome,****
>
> DZ****
>
> ** **
>
> David Zoller****
>
> COLSA Corporation****
>
> Marshall Space Flight Center****
>
> ** **
>
> *Sent:* Tuesday, December 18, 2012 1:40 AM
>
> Now it works like a charm, thank you very much!****
>
> *Cc:* David Zoller; [email-masked]****
>
>

---

From: "Zoller, David A. (MSFC-EO60)[HOSC SERVICES CONTRACT]" <[email-masked]>
Date: Tue, 18 Dec 2012 06:39:02 -0600

Sergey,
You're persistence got us here so you are welcome to make a patch if you would like. I have a set of patches working their way through export approval now. Once I get those thrown over the fence I don't mind making the additional patch. I added options to override the socket buffer sizes that I only had time to implement on the UDP side that I could look into adding to the TCP side now that I've got some experience with it.
Best regards,
DZ

David Zoller
COLSA Corporation
HOSC / C107
*Office: (256) 544-1820

From: [email-masked] [mailto:[email-masked]]
Sent: Tuesday, December 18, 2012 6:08 AM
To: Zoller, David A. (MSFC-EO60)[HOSC SERVICES CONTRACT]
Cc: [email-masked]
Subject: Re: [dtn-users] A problem with dtntunnel

Hi David,

Thanks for the hint. I have also noticed that memory usage of dtntunnel noticeably grows while running iperf with multiple flows.
Are you going to submit all the fixes to mercurial?

--
Best regards,
Sergey Sireskin

2012/12/18 Zoller, David A. (MSFC-EO60)[HOSC SERVICES CONTRACT] <[email-masked]<mailto:[email-masked]>>
Hi Sergey,
Looking at the rest of the code, there are probably a couple of memory leaks. The biggie is after the call to "send_bundle(b_xmit" on success b_xmit should be deleted before setting it to NULL (near line 432). Similarly after the send_bundle for the "error connecting" the APIBundle b should also be deleted before going to done (near line 342).
You're welcome,
DZ

David Zoller
COLSA Corporation

Hi David,

David Zoller
COLSA Corporation
HOSC / C107
*Office: (256) 544-1820

David,

Hi Sergey,

    while (1) {

        // add this code:

        bundle = iter->second;

David Zoller
COLSA Corporation
HOSC / C107
*Office: (256) 544-1820

Hi David,

484             delete b_recv;
485

---

From: "[email-masked]" <[email-masked]>
Date: Thu, 20 Dec 2012 14:23:40 +0300

Hi David,

I am not a C/C++ programmer, could you please help me once again? Should
delete(b) be added at line 336 of TCPTunnel.cc before goto done?

-- 
Best regards,
Sergey Sireskin

> Sergey,****
>
> You’re persistence got us here so you are welcome to make a patch if you
> would like. I have a set of patches working their way through export
> approval now. Once I get those thrown over the fence I don’t mind making
> the additional patch. I added options to override the socket buffer sizes
> that I only had time to implement on the UDP side that I could look into
> adding to the TCP side now that I’ve got some experience with it. ****
>
> Best regards,****
>
> DZ****
>
> ** **
>
> David Zoller****
>
> COLSA Corporation****
>
> HOSC / C107  ****
>
> (Office: (256) 544-1820
> *Sent:* Tuesday, December 18, 2012 6:08 AM
>
> *Sent:* Tuesday, December 18, 2012 1:40 AM****
>
>

---

From: "Zoller, David A. (MSFC-EO60)[HOSC SERVICES CONTRACT]" <[email-masked]>
Date: Thu, 20 Dec 2012 06:50:22 -0600

Yes, that is correct. Looks like you've done very well in all of this for not being a C/C++ programmer!

David Zoller
COLSA Corporation

From: [email-masked] [mailto:[email-masked]]
Sent: Thursday, December 20, 2012 5:24 AM
To: Zoller, David A. (MSFC-EO60)[HOSC SERVICES CONTRACT]
Cc: [email-masked]
Subject: Re: [dtn-users] A problem with dtntunnel

Hi David,

I am not a C/C++ programmer, could you please help me once again? Should delete(b) be added at line 336 of TCPTunnel.cc before goto done?

--
Best regards,
Sergey Sireskin

David Zoller
COLSA Corporation
HOSC / C107
*Office: (256) 544-1820

Hi David,

David Zoller
COLSA Corporation

Hi David,

David Zoller
COLSA Corporation
HOSC / C107
*Office: (256) 544-1820

David,

Hi Sergey,

    while (1) {

        // add this code:

        bundle = iter->second;

David Zoller
COLSA Corporation
HOSC / C107
*Office: (256) 544-1820

Hi David,

484             delete b_recv;
485

---

From: "[email-masked]" <[email-masked]>
Date: Thu, 20 Dec 2012 15:53:57 +0300

Thanks a lot, David!

I am more experienced in scripting languages, where there is no need to
care about memory.

2012/12/20 Zoller, David A. (MSFC-EO60)[HOSC SERVICES CONTRACT] <
[email-masked]>

> Yes, that is correct. Looks like you’ve done very well in all of this for
> not being a C/C++ programmer!****
>
> ** **
>
> David Zoller****
>
> COLSA Corporation****
>
> *Sent:* Thursday, December 20, 2012 5:24 AM
>
> *Sent:* Tuesday, December 18, 2012 6:08 AM****
>
>
> *To:* Zoller, David A. (MSFC-EO60)[HOSC SERVICES CONTRACT]****
>
> *Cc:* [email-masked]

-- 
Best regards,
Sergey Sireskin

---

From: David Zoller <[email-masked]>
Date: Fri, 14 Dec 2012 11:32:01 -0600

Hi Sergey,

484             delete b_recv;
485