Add NetDeviceQueueInterface methods to implement flow control

Add NetDeviceQueueInterface methods to implement flow control

Please find a description in this mailing list post:

http://mailman.isi.edu/pipermail/ns-developers/2017-January/013761.html

[Tom Henderson, 2017-01-15 23:35:30]

Looks OK to me; this issue needs cc ns-3-reviews@googlegroups.com.

Also consider to add some paragraph to the Sphinx documentation on guidance to
designers of new NetDevice classes (what they need to do for bound callbacks at
DoInitialize() time).

[Stefano Avallone, 2017-01-17 10:11:49]

Thanks a lot for the review. Just a note for other reviewers: I am soon going to
publish a slightly modified version of this patch.

[Stefano Avallone, 2017-01-19 22:24:10]

Changes with respect to the first version:

- updated to match the second version of the patch to make Queue a template
class

- the static methods that handle enqueue/dequeue/drop events have been moved
from NetDeviceQueueInterface to NetDeviceQueue (to account for future features)

- Handling out of order dequeues has been moved from
PointToPointNetDevice::Dequeue to Queue::Dequeue, so that all devices can
benefit. The issue is explained in the doxygen of Queue::Dequeue:

    * Before returning a dequeued packet, the dequeue traced callback is
invoked.
    * In some cases (e.g., the queue is used by a netdevice supporting flow
control)
    * such traced callback might trigger a new dequeue operation. Thus, a new
    * packet would be returned to the caller before the packet that had been
    * previously dequeued. If the flag that controls the check of out of order
    * packet dequeues is set to true, a new call to Dequeue quits immediately if
    * a previously dequeued packet has not been returned yet to the caller.

So, IMO this issue should be really solved within the Queue object.

[Stefano Avallone, 2017-01-19 22:28:54]

The third version adds support for flow control and BQL to CsmaNetDevice.

[Stefano Avallone, 2017-01-24 17:16:06]

Changes with respect to the third version:

- removed the "workaround" of keeping track of pending packets in
Queue::Dequeue. The real fix is to postpone calling the wake callback until the
current operations are done (I used the ScheduleNow method)

- added support for flow control and BQL to SimpleNetDevices

- added a test to check the flow control mechanism. Basically, we aim to verify
that all the packets in the queue disc are drained by the device

[Luciano Chaves, 2017-02-01 12:45:39]

Hi,

I've just reviewed this code online and it seems ok to me. I've just pointed a
suggestion on documentation. The NsLog thing is very interesting solution to the
implementation inside .h file (it creates a lot of additional code, but solves
the problem).

I would like to apply the patch on my ns-3-dev working copy and test it with
some simulation scripts that I have, but I've tried it on top of ns-3.26 tag and
tip tag and got some errors. On top of which revision should I apply this patch?
(I'm sorry if this is a dumb question, but I'm not used to this code review
system).

Cheers,
Luciano.

https://codereview.appspot.com/313270043/diff/60001/src/network/model/net-dev...
File src/network/model/net-device.h (right):

https://codereview.appspot.com/313270043/diff/60001/src/network/model/net-dev...
src/network/model/net-device.h:384: *   - when the device queues have been
created
From what I got, these three trace connections are performed by the
ConnectQueueTraces method, right? So here, on this documentation, I would
simplify this and say that when the device queue have been create device must
call ConnectQueueTraces (I've got somewhat confused by this text).

[Stefano Avallone, 2017-02-01 13:51:40]

On 2017/02/01 12:45:39, Luciano Chaves wrote:
> I would like to apply the patch on my ns-3-dev working copy and test it with
> some simulation scripts that I have, but I've tried it on top of ns-3.26 tag
and
> tip tag and got some errors. On top of which revision should I apply this
patch?

After I posted these code reviews, some commits were pushed to ns-3-dev that
conflicted with my work. I rebased my work on the current ns-3-dev to fix these
conflicts but did not updated the code reviews. You can get my work on top of
the latest ns-3-dev by pulling my github branch:

git pull https://github.com/stavallo/ns-3-dev-git.git
git checkout -b tc-next --track origin/tc-next

Bests,
Stefano

[Stefano Avallone, 2017-02-01 13:51:52]

https://codereview.appspot.com/313270043/diff/60001/src/network/model/net-dev...
File src/network/model/net-device.h (right):

https://codereview.appspot.com/313270043/diff/60001/src/network/model/net-dev...
src/network/model/net-device.h:384: *   - when the device queues have been
created
You got it right. Actually, I added ConnectQueueTraces after writing this
documentation. I'll definitely update it as you suggested, thanks.

[Luciano Chaves, 2017-02-04 20:55:55]

Hi,

I’ve tried to test the code from your github branch, but there’s something wrong
with the code on tc-next that is raising segfault.
When running the first.cc example, this is the gdb output:

Starting program: /tmp/luciano/ns-3-dev-git/build/scratch/first 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".

Program received signal SIGSEGV, Segmentation fault.
0x00007fffeebd4243 in std::string::size() const () from
/usr/lib/x86_64-linux-gnu/libstdc++.so.6
(gdb) bt
#0  0x00007fffeebd4243 in std::string::size() const () from
/usr/lib/x86_64-linux-gnu/libstdc++.so.6
#1  0x00007ffff6ed45e0 in std::operator+<char, std::char_traits<char>,
std::allocator<char> > (__lhs=0x7fffeffbd610 "ns3::", 
    __rhs=<error reading variable: Cannot access memory at address
0xffffffffffffffe8>) at /usr/include/c++/4.9/bits/basic_string.tcc:700
#2  0x00007fffeff7cbc3 in ns3::Queue<ns3::Packet>::GetTypeId () at
../src/network/utils/queue.h:454
#3  0x00007fffeff066db in ns3::TypeId::SetParent<ns3::Queue<ns3::Packet> >
(this=0x7fffffffe400) at ./ns3/type-id.h:647
#4  0x00007fffeff0606b in ns3::DropTailQueue<ns3::Packet>::GetTypeId () at
../src/network/utils/drop-tail-queue.h:80
#5  0x00007fffeff065a5 in
ns3::ObjectPacketDropTailQueueRegistrationClass::ObjectPacketDropTailQueueRegistrationClass
(
    this=0x7ffff021eb21 <ns3::ObjectPacketDropTailQueueRegistrationVariable>) at
../src/network/utils/drop-tail-queue.cc:23
#6  0x00007fffeff05f28 in __static_initialization_and_destruction_0
(__initialize_p=1, __priority=65535) at
../src/network/utils/drop-tail-queue.cc:23
#7  0x00007fffeff05f5e in _GLOBAL__sub_I_drop_tail_queue.cc(void) () at
../src/network/utils/drop-tail-queue.cc:25
#8  0x00007ffff7dea9ba in call_init (l=<optimized out>, argc=argc@entry=1,
argv=argv@entry=0x7fffffffe578, env=env@entry=0x7fffffffe588) at dl-init.c:78
#9  0x00007ffff7deaaa3 in call_init (env=0x7fffffffe588, argv=0x7fffffffe578,
argc=1, l=<optimized out>) at dl-init.c:36
#10 _dl_init (main_map=0x7ffff7ffe1a8, argc=1, argv=0x7fffffffe578,
env=0x7fffffffe588) at dl-init.c:126
#11 0x00007ffff7ddd1ca in _dl_start_user () from /lib64/ld-linux-x86-64.so.2
#12 0x0000000000000001 in ?? ()
#13 0x00007fffffffe7e4 in ?? ()
#14 0x0000000000000000 in ?? ()
(gdb) frame 2
#2  0x00007fffeff7cbc3 in ns3::Queue<ns3::Packet>::GetTypeId () at
../src/network/utils/queue.h:454
454	                     "ns3::" + TypeParamName + "::TracedCallback")
(gdb) up
#3  0x00007fffeff066db in ns3::TypeId::SetParent<ns3::Queue<ns3::Packet> >
(this=0x7fffffffe400) at ./ns3/type-id.h:647
647	  return SetParent (T::GetTypeId ());
(gdb) up
#4  0x00007fffeff0606b in ns3::DropTailQueue<ns3::Packet>::GetTypeId () at
../src/network/utils/drop-tail-queue.h:80
80	    .template AddConstructor<DropTailQueue<Item> > ()
(gdb) frame 2
#2  0x00007fffeff7cbc3 in ns3::Queue<ns3::Packet>::GetTypeId () at
../src/network/utils/queue.h:454
454	                     "ns3::" + TypeParamName + "::TracedCallback")
(gdb) p TypeParamName
$1 = <error reading variable: Cannot access memory at address
0xffffffffffffffe8>

Any idea why is this happening?

[]’s
--
Luciano Jerez Chaves
Assistant Professor at DCC/UFJF
PhD candidate at LRC/UNICAMP
http://www.lrc.ic.unicamp.br/~luciano <http://www.lrc.ic.unicamp.br/~luciano>
> On 1 Feb 2017, at 11:51, stavallo@gmail.com wrote:
> 
> On 2017/02/01 12:45:39, Luciano Chaves wrote:
>> I would like to apply the patch on my ns-3-dev working copy and test
> it with
>> some simulation scripts that I have, but I've tried it on top of
> ns-3.26 tag and
>> tip tag and got some errors. On top of which revision should I apply
> this patch?
> 
> After I posted these code reviews, some commits were pushed to ns-3-dev
> that conflicted with my work. I rebased my work on the current ns-3-dev
> to fix these conflicts but did not updated the code reviews. You can get
> my work on top of the latest ns-3-dev by pulling my github branch:
> 
> git pull https://github.com/stavallo/ns-3-dev-git.git
> git checkout -b tc-next --track origin/tc-next
> 
> Bests,
> Stefano
> 
> 
> https://codereview.appspot.com/313270043/

[Stefano Avallone, 2017-02-06 10:57:36]

Hi Luciano,

I cannot reproduce this error here. Could you please tell me which compiler
version you are using and the exact steps you are doing? Also, I have updated
the tc-next branch, maybe you could give it a try.

Thanks,
Stefano

[Luciano Chaves, 2017-02-06 12:53:55]

Hi Stefano,

I’ve just repeated all steps from scratch and the error still persists.
This is what I did: I’ve cloned your repo, checked out the tc-branch, configured
ns-s with examples, compiled ns-3, and then run the first example.

git clone https://github.com/stavallo/ns-3-dev-git.git
cd ns-3-dev-git/
git checkout -b tc-next --track origin/tc-next
./waf configure --enable-examples
./waf --run first --command-template="gdb --args %s"

The complete output terminal is available at http://pastebin.com/h0nRm3ry
<http://pastebin.com/h0nRm3ry>

Here’s my system configurations:

luciano@eco:/tmp/ns-3-dev-git$ uname -a
Linux eco 3.16.0-4-amd64 #1 SMP Debian 3.16.39-1 (2016-12-30) x86_64 GNU/Linux

luciano@eco:/tmp/ns-3-dev-git$ gcc --version
gcc (Debian 4.9.2-10) 4.9.2
Copyright (C) 2014 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

luciano@eco:/tmp/ns-3-dev-git$ cat /etc/issue
Debian GNU/Linux 8 \n \l

Hope this info can help to figure out what’s happening.

Thanks,
--
Luciano Jerez Chaves
Assistant Professor at DCC/UFJF
PhD candidate at LRC/UNICAMP
http://www.lrc.ic.unicamp.br/~luciano <http://www.lrc.ic.unicamp.br/~luciano>
> On 6 Feb 2017, at 08:57, stavallo@gmail.com wrote:
> 
> Hi Luciano,
> 
> I cannot reproduce this error here. Could you please tell me which
> compiler version you are using and the exact steps you are doing? Also,
> I have updated the tc-next branch, maybe you could give it a try.
> 
> Thanks,
> Stefano
> 
> https://codereview.appspot.com/313270043/

[Stefano Avallone, 2017-02-07 08:55:53]

On 2017/02/06 12:53:55, Luciano Chaves wrote:
> Hi Stefano,
> 
> I’ve just repeated all steps from scratch and the error still persists.
> This is what I did: I’ve cloned your repo, checked out the tc-branch,
configured
> ns-s with examples, compiled ns-3, and then run the first example.
> 
> git clone https://github.com/stavallo/ns-3-dev-git.git
> cd ns-3-dev-git/
> git checkout -b tc-next --track origin/tc-next
> ./waf configure --enable-examples
> ./waf --run first --command-template="gdb --args %s"
> 
> The complete output terminal is available at http://pastebin.com/h0nRm3ry
> <http://pastebin.com/h0nRm3ry>
> 
> Here’s my system configurations:
> 
> luciano@eco:/tmp/ns-3-dev-git$ uname -a
> Linux eco 3.16.0-4-amd64 #1 SMP Debian 3.16.39-1 (2016-12-30) x86_64 GNU/Linux
> 
> luciano@eco:/tmp/ns-3-dev-git$ gcc --version
> gcc (Debian 4.9.2-10) 4.9.2
> Copyright (C) 2014 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> 
> luciano@eco:/tmp/ns-3-dev-git$ cat /etc/issue
> Debian GNU/Linux 8 \n \l
> 
> Hope this info can help to figure out what’s happening.
> 
> Thanks,
> --
> Luciano Jerez Chaves
> Assistant Professor at DCC/UFJF
> PhD candidate at LRC/UNICAMP
> http://www.lrc.ic.unicamp.br/~luciano <http://www.lrc.ic.unicamp.br/~luciano>
> > On 6 Feb 2017, at 08:57, mailto:stavallo@gmail.com wrote:
> > 
> > Hi Luciano,
> > 
> > I cannot reproduce this error here. Could you please tell me which
> > compiler version you are using and the exact steps you are doing? Also,
> > I have updated the tc-next branch, maybe you could give it a try.
> > 
> > Thanks,
> > Stefano
> > 
> > https://codereview.appspot.com/313270043/
> 

Hi Luciano,

is it just the first.cc example that gives this error?

Thanks

[Luciano Chaves, 2017-02-07 17:03:34]

Hi Stefano,

The error is happening for all the scripts that I’ve tried on this system (as
long as they import the network module). As shown by the gdb, the error seems to
be during Queue object registration. The TypeParamName inside Queue::GetTypeId()
is invalid and gives the segmentation fault. It seems to be something related to
the Queue template class implementation or/and (possible) some issues with GCC
4.9.2 or any other library on Debian 8. I’ve just tried to compile and test your
tc-next branch on another machine running Ubuntu and GCC 5.4.0 and it worked
very well.

So, to help you find out what’s happening, I’ve just configure a Debian 8 VM
(fresh jessie stable install) with your tc-next branch already compiled into it.
You can download the VM from
http://lrc.ic.unicamp.br/~luciano/files/debian8-ns3.ova
<http://lrc.ic.unicamp.br/~luciano/files/debian8-ns3.ova> and see the error. The
VM username and password are set to “user". For root access, use the “root"
password.

Thanks,
--
Luciano Jerez Chaves
Assistant Professor at DCC/UFJF
PhD candidate at LRC/UNICAMP
http://www.lrc.ic.unicamp.br/~luciano <http://www.lrc.ic.unicamp.br/~luciano>
> On 7 Feb 2017, at 06:55, stavallo@gmail.com wrote:
> 
> On 2017/02/06 12:53:55, Luciano Chaves wrote:
>> Hi Stefano,
> 
>> I’ve just repeated all steps from scratch and the error still
> persists.
>> This is what I did: I’ve cloned your repo, checked out the tc-branch,
> configured
>> ns-s with examples, compiled ns-3, and then run the first example.
> 
>> git clone https://github.com/stavallo/ns-3-dev-git.git
>> cd ns-3-dev-git/
>> git checkout -b tc-next --track origin/tc-next
>> ./waf configure --enable-examples
>> ./waf --run first --command-template="gdb --args %s"
> 
>> The complete output terminal is available at
> http://pastebin.com/h0nRm3ry
>> <http://pastebin.com/h0nRm3ry <http://pastebin.com/h0nRm3ry>>
> 
>> Here’s my system configurations:
> 
>> luciano@eco:/tmp/ns-3-dev-git$ uname -a
>> Linux eco 3.16.0-4-amd64 #1 SMP Debian 3.16.39-1 (2016-12-30) x86_64
> GNU/Linux
> 
>> luciano@eco:/tmp/ns-3-dev-git$ gcc --version
>> gcc (Debian 4.9.2-10) 4.9.2
>> Copyright (C) 2014 Free Software Foundation, Inc.
>> This is free software; see the source for copying conditions.  There
> is NO
>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
> PURPOSE.
> 
>> luciano@eco:/tmp/ns-3-dev-git$ cat /etc/issue
>> Debian GNU/Linux 8 \n \l
> 
>> Hope this info can help to figure out what’s happening.
> 
>> Thanks,
>> --
>> Luciano Jerez Chaves
>> Assistant Professor at DCC/UFJF
>> PhD candidate at LRC/UNICAMP
>> http://www.lrc.ic.unicamp.br/~luciano
> <http://www.lrc.ic.unicamp.br/~luciano
<http://www.lrc.ic.unicamp.br/~luciano>>
>> > On 6 Feb 2017, at 08:57, mailto:stavallo@gmail.com
<mailto:stavallo@gmail.com> wrote:
>> >
>> > Hi Luciano,
>> >
>> > I cannot reproduce this error here. Could you please tell me which
>> > compiler version you are using and the exact steps you are doing?
> Also,
>> > I have updated the tc-next branch, maybe you could give it a try.
>> >
>> > Thanks,
>> > Stefano
>> >
>> > https://codereview.appspot.com/313270043/
<https://codereview.appspot.com/313270043/>
> 
> 
> Hi Luciano,
> 
> is it just the first.cc <http://first.cc/> example that gives this error?
> 
> Thanks
> 
> 
> 
> 
> https://codereview.appspot.com/313270043/
<https://codereview.appspot.com/313270043/>

[Luciano Chaves, 2017-02-07 17:05:27]

Sorry, the link is missing the www...
http://www.lrc.ic.unicamp.br/~luciano/files/debian8-ns3.ova
<http://www.lrc.ic.unicamp.br/~luciano/files/debian8-ns3.ova> 

[]’s
--
Luciano Jerez Chaves
Assistant Professor at DCC/UFJF
PhD candidate at LRC/UNICAMP
http://www.lrc.ic.unicamp.br/~luciano <http://www.lrc.ic.unicamp.br/~luciano>
> On 7 Feb 2017, at 15:03, Luciano Jerez Chaves <ljerezchaves@gmail.com> wrote:
> 
> Hi Stefano,
> 
> The error is happening for all the scripts that I’ve tried on this system (as
long as they import the network module). As shown by the gdb, the error seems to
be during Queue object registration. The TypeParamName inside Queue::GetTypeId()
is invalid and gives the segmentation fault. It seems to be something related to
the Queue template class implementation or/and (possible) some issues with GCC
4.9.2 or any other library on Debian 8. I’ve just tried to compile and test your
tc-next branch on another machine running Ubuntu and GCC 5.4.0 and it worked
very well.
> 
> So, to help you find out what’s happening, I’ve just configure a Debian 8 VM
(fresh jessie stable install) with your tc-next branch already compiled into it.
You can download the VM from
http://lrc.ic.unicamp.br/~luciano/files/debian8-ns3.ova
<http://lrc.ic.unicamp.br/~luciano/files/debian8-ns3.ova> and see the error. The
VM username and password are set to “user". For root access, use the “root"
password.
> 
> Thanks,
> --
> Luciano Jerez Chaves
> Assistant Professor at DCC/UFJF
> PhD candidate at LRC/UNICAMP
> http://www.lrc.ic.unicamp.br/~luciano <http://www.lrc.ic.unicamp.br/~luciano>
>> On 7 Feb 2017, at 06:55, stavallo@gmail.com <mailto:stavallo@gmail.com>
wrote:
>> 
>> On 2017/02/06 12:53:55, Luciano Chaves wrote:
>>> Hi Stefano,
>> 
>>> I’ve just repeated all steps from scratch and the error still
>> persists.
>>> This is what I did: I’ve cloned your repo, checked out the tc-branch,
>> configured
>>> ns-s with examples, compiled ns-3, and then run the first example.
>> 
>>> git clone https://github.com/stavallo/ns-3-dev-git.git
<https://github.com/stavallo/ns-3-dev-git.git>
>>> cd ns-3-dev-git/
>>> git checkout -b tc-next --track origin/tc-next
>>> ./waf configure --enable-examples
>>> ./waf --run first --command-template="gdb --args %s"
>> 
>>> The complete output terminal is available at
>> http://pastebin.com/h0nRm3ry <http://pastebin.com/h0nRm3ry>
>>> <http://pastebin.com/h0nRm3ry <http://pastebin.com/h0nRm3ry>>
>> 
>>> Here’s my system configurations:
>> 
>>> luciano@eco:/tmp/ns-3-dev-git$ uname -a
>>> Linux eco 3.16.0-4-amd64 #1 SMP Debian 3.16.39-1 (2016-12-30) x86_64
>> GNU/Linux
>> 
>>> luciano@eco:/tmp/ns-3-dev-git$ gcc --version
>>> gcc (Debian 4.9.2-10) 4.9.2
>>> Copyright (C) 2014 Free Software Foundation, Inc.
>>> This is free software; see the source for copying conditions.  There
>> is NO
>>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
>> PURPOSE.
>> 
>>> luciano@eco:/tmp/ns-3-dev-git$ cat /etc/issue
>>> Debian GNU/Linux 8 \n \l
>> 
>>> Hope this info can help to figure out what’s happening.
>> 
>>> Thanks,
>>> --
>>> Luciano Jerez Chaves
>>> Assistant Professor at DCC/UFJF
>>> PhD candidate at LRC/UNICAMP
>>> http://www.lrc.ic.unicamp.br/~luciano
<http://www.lrc.ic.unicamp.br/~luciano>
>> <http://www.lrc.ic.unicamp.br/~luciano
<http://www.lrc.ic.unicamp.br/~luciano>>
>>> > On 6 Feb 2017, at 08:57, mailto:stavallo@gmail.com
<mailto:stavallo@gmail.com> wrote:
>>> >
>>> > Hi Luciano,
>>> >
>>> > I cannot reproduce this error here. Could you please tell me which
>>> > compiler version you are using and the exact steps you are doing?
>> Also,
>>> > I have updated the tc-next branch, maybe you could give it a try.
>>> >
>>> > Thanks,
>>> > Stefano
>>> >
>>> > https://codereview.appspot.com/313270043/
<https://codereview.appspot.com/313270043/>
>> 
>> 
>> Hi Luciano,
>> 
>> is it just the first.cc <http://first.cc/> example that gives this error?
>> 
>> Thanks
>> 
>> 
>> 
>> 
>> https://codereview.appspot.com/313270043/
<https://codereview.appspot.com/313270043/>

[Stefano Avallone, 2017-02-07 22:08:45]

Hi Luciano,

I am pretty sure the culprit is g++ 4.9.2. I have read of some template-related
bugs in g++ that were still not fixed in 4.8...
Anyway, I am thinking of alternative approaches to make examples work with older
versions of g++.
Thanks a lot for setting up a VM, it will help me test alternative solutions.

Bests,
Stefano

[Stefano Avallone, 2017-02-10 18:05:05]

I updated and reviewed the whole stack of patches. Examples and tests now run
fine with g++-4.9.2.

Thanks,
Stefano

[Luciano Chaves, 2017-02-14 15:10:50]

Hi,

I finally got it working here! I’ve tested your changes with some simulation
scripts that I have and everything seems to be ok (of course, with some small
changes on the commands used to change the mode and maxsize attributes that now
belongs to the QueueBase class.

Thank you for your hard work on this :)

--
Luciano Jerez Chaves
Assistant Professor at DCC/UFJF
PhD candidate at LRC/UNICAMP
http://www.lrc.ic.unicamp.br/~luciano <http://www.lrc.ic.unicamp.br/~luciano>
> On 10 Feb 2017, at 16:05, stavallo@gmail.com wrote:
> 
> I updated and reviewed the whole stack of patches. Examples and tests
> now run fine with g++-4.9.2.
> 
> Thanks,
> Stefano
> 
> 
> https://codereview.appspot.com/313270043/