Bug 2610

Hi,


This flow control and BQL support to CsmaNetDevice is already reported at https://www.nsnam.org/bugzilla/﷒0﷓.

I'm intensively using the patch proposed on https://codereview.appspot.com/314910043 on my simulations and it is working smoothly.
I would like to see this traffic control support for CSMA devices on ns-3.27, specially because without it some packets can get stuck inside traffic control queues in some specific situations, leading to wrong results on simulations.

I can upload a revised patch here, if necessary.

This looks good to me; just a few indentations to fix; e.g.:

+      if (txq)
+      {
+        NS_LOG_DEBUG

to 

+      if (txq)
+        {
+          NS_LOG_DEBUG


We can then close this and 2373, correct?

(In reply to Tom Henderson from comment #2)
> This looks good to me; just a few indentations to fix; e.g.:
> 
> +      if (txq)
> +      {
> +        NS_LOG_DEBUG
> 
> to 
> 
> +      if (txq)
> +        {
> +          NS_LOG_DEBUG
> 
> 
> We can then close this and 2373, correct?

Yes, in my opinion we can close this issue and apply the patch from 2373.
The csma netdevice, channel and helper classes have some simple inconsistencies regarding coding style and documentation. I was think on opening a new issue, revise the code and upload a patch to fix all inconsistencies (I can do this on the next days). But it would be good to do this after applying this patch and also the full-duplex extension (https://www.nsnam.org/bugzilla/﷒0﷓).
Do you think this is a good strategy?

I believe I came up with a much better approach to adding support for flow control and BQL to netdevices, which exploits the enqueue/dequeue traced callbacks of the Queue class. Such approach is included in the effort to convert the Queue class into a template class:

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

I have applied such approach to csma as well. You can find it on github:

https://github.com/stavallo/ns-3-dev-git/commits/tc-next

note 1: Please disregard the "Hack to add interrupt coalescence" commit (as the title says, it is just an experiment...)

note 2: I often rewrite the history and force push, so if you plan to update make sure to remove the out-of-tree commits before pulling

If you have a chance to test it, it would be great to hear from you :-)

Thanks

(In reply to Stefano Avallone from comment #4)
> I believe I came up with a much better approach to adding support for flow
> control and BQL to netdevices, which exploits the enqueue/dequeue traced
> callbacks of the Queue class. Such approach is included in the effort to
> convert the Queue class into a template class:
> 
> http://mailman.isi.edu/pipermail/ns-developers/2017-January/013761.html
> 

Hi Stefano,

This is, indeed, a much better (and easier) approach to solve this problem.
Do you think that this new Queue approach would be available for ns-3.26? If yes, I vote for this approach instead of proposed patch on bug 2373. If no, we could apply the patch for this release and update the code for the next one. 

> I have applied such approach to csma as well. You can find it on github:
> 
> https://github.com/stavallo/ns-3-dev-git/commits/tc-next
> 
> note 1: Please disregard the "Hack to add interrupt coalescence" commit (as
> the title says, it is just an experiment...)
> 
> note 2: I often rewrite the history and force push, so if you plan to update
> make sure to remove the out-of-tree commits before pulling
> 
> If you have a chance to test it, it would be great to hear from you :-)

I'll definitely try it on my experiments.
I'm current implementing some new features on the OpenFlow 1.3 module that I've been developing for the last 2 years (http://www.lrc.ic.unicamp.br/ofswitch13/index.html) and I also need to use multiple queues on switch ports. So, I believe that converting Queue to a template will help me on that too. 

> 
> Thanks

Thanks you.

(In reply to Luciano from comment #5)
> This is, indeed, a much better (and easier) approach to solve this problem.
> Do you think that this new Queue approach would be available for ns-3.26? If
> yes, I vote for this approach instead of proposed patch on bug 2373. If no,
> we could apply the patch for this release and update the code for the next
> one. 

Actually, this new approach is not strictly dependent on the work to make Queue a template class. Making Queue a template class is essential to apply such approach to wifi, but not to apply it to point-to-point and csma. Thus, with a bit of effort, I could apply this approach to the current ns-3-dev independently of the work to make Queue a template class.

However, I would like ns-3.27 to have flow control for wifi as well. And for that the Queue rework is necessary.

Tom was kind enough to have a look at the Queue rework and gave a positive feedback (which doesn't mean however he is comfortable with pushing it in time for ns-3.27). Let's also see what other developers thinks about it.

*** This bug has been marked as a duplicate of bug 2373 ***