[Ns-developers] Preliminary release of NS-3 WiMAX module

[Jahanzeb Farooq]

Hi Mathieu,
Thank you very much for your feedback.

> 1) your repository contains a lot of automatically-generated files. 
> For examples, it contains a lot of files under the build directory and
> under the .waf-1.3.2-b67d68bfb915473cfa1a2e2290022f98 directory. All of
> these should be removed.

Yes you are right. I had already noticed this and had removed all
unwanted files but then realized that they were actually only removed
from my working directory and not in the repository (perhaps I need to
learn Mercurial more). Actually this happened when after merging to the
new NS-3 I accidentally committed without specifying the file names
explicitly.

> 3) After installing itpp 3.10.11, I still get the following 
> compilation error:
> ../src/devices/wimax/wimax-ofdm-phy/modulator80216.h:51: error: ‘BPSK_c’
> does not name a type
> I suspect that I need to get the right version of itpp to get that
> symbol defined: which version should I get ?

This is the first time I am seeing this error. Please note that you also
have to install LAPACK, ATLAS, BLASS and FFTW libraries separately. In
my case I have installed the following: lapack (3.1.1.1), lapack-devel
(3.1.1.1), atlas (3.6.0.11), atlas-devel (3.6.0.11), fftw (3.1.1) and
fftw-devel (3.1.1) (BLASS was automatically installed during installing
LAPACK).

The IT++ version I have installed is the development version 3.99.3.1 as
instructed by Fabio. I downloaded it from IT++ web site.

> 5) It is very unclear to me which version of ns-3 you have based your
> work on. I looked at the mercurial history contained in your tree and
> the most recent common ancestor I could find between your tree and
> ns-3-dev is 1393:d0f6f85074685c859f3e5d5c58382c681c7ab915 which is a
> changeset which happened just after release 3.0.10, that is, somewhere
> in september 2007. So, there are 2 options:

It is version 3.0.12. I recently merged my work to it.

> - you have merged your tree with ns-3-dev _by hand_ without using
> mercurial's merging capabilities. If this is the case, I am afraid that
> this is going to make merging your code back to ns-3-dev impossible
> because it will trigger thousands of lines of code of conflicts in our
> core code. It also makes reviewing the current code vastly harder than
> it should be.

Yes you are right. I merged it by hand and did not use mercurial for it.
It was a mistake now I understand.

> I suspect that you are in the second case above and, if I am right, 
> you have a couple of options:
>
>   a) try to run "hg pull http://code.nsnam.org/ns-3-dev" in your tree,
> deal with the conflicts if there are any, and repost a repo for review
> 
>   b) create a clean copy of ns-3-dev, copy your own code in there, and
> hg add && hg ci it in the new repository. Please, make sure you do not
> add unecessary and unwanted files. i.e., add each file separately.
> .......
>
> a) might be painful and create a scary and horrible mercurial history.
> b) will destroy all _your_ development history, c) will preserve both
> your history and the ns-3 history correctly. Based on the issue 1)
> outlined above (lots of unwanted files present in your
> repository/history), I would suggest that you go with b) because we
> really do not want to keep in our history all the files you have added
> there.

Thanks for suggesting the solutions. I think I will go for option (b) as
you have also suggested. However loosing the development history is
painful therefore may be I shall try option (a) first.

> For example, I see that you seem to be a using a PacketClassifier base
> class and you seem to be requesting each user to provide an explicit
> classifier for each device: the classifier is set at construction time
> and cannot change ever, even for the BS. That implementation choice
> seems to imply that it is not possible to create dynamic wimax
> connections with your code because each connection needs to be
> associated with a set of classification rules sent by the BS to the SSs
> once the connection is established to allow each SS to classify its
> traffic to different connections. I have to confess that I would have
> expected to see a single PacketClassifier class (not virtual) which
> would be able to parse and execute the classification rules as defined
> by the spec and which would be fed the classification rules sent by the
> BS to the SS and the classification rules built by the BS for itself.

I agree with your comments. However note that the PacketClassifier has
only recently been added, and it is only a simplified version, with
minimum functionality. And like number of other "immature" components of
the module its usage would also be more clear and refined with the
progress of the work. Furthermore since currently the feature of dynamic
connections (or service flow creation) is not implemented, I used
PacketClassifier in its simplest possible way.

7) It would be really helpful to know what your TODO is over the next
> couple of months: it is hard, as-is, to figure out what is really just
> code waiting to be implemented shortly and what is ignored because you
> don't care about modelizing it. The end of README attempts to answer
> that question but it would be very helpful to have a more detailed plan
> of action about what you want to do. For example, the README seems to
> imply that scheduling will be implemented before the dynamic creation
> and management of service flows which seems really backward: I would
> have expected the latter to be implemented first properly with a very
> stupid scheduler.

May be you did not get it right here or may be I was not clear. It did
not mean the scheduler but the scheduling services (BE, UGS, rtPS,
nrtPS). As much I understand its implementation is not really dependant
on service flow management. I may temporarily assume four fixed number
of connections, one for each scheduling service, in order to continue
the work and then later on add the dynamic service flow management
feature. Furthermore as I had mentioned in the README even before the
scheduling services, I need to implement the bandwidth request and
management functionality as it is required for the scheduling services.
Hence the TODO is as follows:

1-bandwidth request mechanism and management
2-scheduling services
3-service flow management

Please also note that this specifies only the _major_ tasks in the TODO.
Besides this there are numerous small things to add or update here and
there as well as the refinement of things already implemented, which are
not mentioned in the TODO. Since the module/code is evolving rapidly
there are number of small and big things being added and removed from
the TODO on regular basis, therefore I did not include it in the README.

> 8) There is a lot of code to digest here (14 KLOC) so, it is going to
> take me a while to go through it all and make more interesting comments
> about the code itself. Generally, I have to say that such big code drops
> make it very very hard to provide good reviews: it would have been
> vastly easier to comment on this work incrementally and piece-by-piece
> rather than as a whole. 

Thank you very much for your comments and the effort you put. Please
take your time and also it would be great for you to save your time if
you give feedback on more consistent, mature components of the module
and for the time being ignore the immature ones as they are rapidly
evolving with the progress in development. Any feedback especially about
the code design/structure would be very much appreciated, e.g., I see in
your Wifi module that there is a MAC class which is subclassed for
stations and AP, in the contrast in my code the MAC functionality is
included in the net-device class, may be I need to revise it, etc. 


Thanks


-
Jahanzeb Farooq (Tippu)



On Mon, 2008-06-02 at 08:59 -0700, Mathieu Lacage wrote: 
> hi tippu,
> 
> On Wed, 2008-05-28 at 19:38 +0200, Jahanzeb Farooq wrote: 
> > Hi all,
> > It's been a while being a silent member of the mailing list. I hereby
> > announce the preliminary release (version 0.9.0 ??) of the WiMAX module
> > for NS-3. The module currently implements the fundamental
> > functionalities of the WiMAX standard. Following is a brief outline of
> > the functionalities implemented:
> 
> This is all pretty cool ! 
> 
> I went through the repository you posted:
> 
> > http://yans.inria.fr/code/ns-3-wimax/
> 
> So, here are couple of comments:
>   1) your repository contains a lot of automatically-generated files.
> For examples, it contains a lot of files under the build directory and
> under the .waf-1.3.2-b67d68bfb915473cfa1a2e2290022f98 directory. All of
> these should be removed.
> 
>   2) The code does not build by default unless we install itpp: I think
> that you need to conditionally compile the itpp-dependent PHY
> implementation based on whether the itpp library could be found. For
> those who are interested, on my ubuntu system, I had to run the
> following command: "sudo apt-get install libitpp-dev"
> 
>   3) After installing itpp 3.10.11, I still get the following
> compilation error:
> ../src/devices/wimax/wimax-ofdm-phy/modulator80216.h:51: error: ‘BPSK_c’
> does not name a type
> I suspect that I need to get the right version of itpp to get that
> symbol defined: which version should I get ?
> 
>   4) the files examples/simple-wimax*.cc and src/devices/wimax/*.h|*.cc
> are executables: you should remove their exec bit with chmod and commit
> them. i.e., "chmod a-x file".
> 
>   5) It is very unclear to me which version of ns-3 you have based your
> work on. I looked at the mercurial history contained in your tree and
> the most recent common ancestor I could find between your tree and
> ns-3-dev is 1393:d0f6f85074685c859f3e5d5c58382c681c7ab915 which is a
> changeset which happened just after release 3.0.10, that is, somewhere
> in september 2007. So, there are 2 options:
>   - you really have based your work on that version and, if this is the
> case, you _must_ update your tree to ns-3-dev: there is little point in
> reviewing it as-is.
>   - you have merged your tree with ns-3-dev _by hand_ without using
> mercurial's merging capabilities. If this is the case, I am afraid that
> this is going to make merging your code back to ns-3-dev impossible
> because it will trigger thousands of lines of code of conflicts in our
> core code. It also makes reviewing the current code vastly harder than
> it should be.
> 
> I suspect that you are in the second case above and, if I am right, you
> have a couple of options:
> 
>   a) try to run "hg pull http://code.nsnam.org/ns-3-dev" in your tree,
> deal with the conflicts if there are any, and repost a repo for review
> 
>   b) create a clean copy of ns-3-dev, copy your own code in there, and
> hg add && hg ci it in the new repository. Please, make sure you do not
> add unecessary and unwanted files. i.e., add each file separately.
> 
>   c) attempt to re-do your merge.I suspect very much that the
> problematic changeset is 1439:9bc15c0b4233 and, if this is the case,
> what you need to do is create a clone with the previous changeset: "hg
> clone -r 1438:9ff95f45ab40 ns-3-wimax ns-3-wimax-for-merge", pull
> ns-3-dev in ns-3-wimax-for-merge, and run "hg merge" (or "hg imerge",
> depending on what you like best). Before doing this, I suggest strongly
> that you install the _latest_ version of mercurial and read a bit about
> it and about maintaining branches and merging (see, for example:
> http://hgbook.red-bean.com/hgbookch3.html#x7-550003)
> 
> a) might be painful and create a scary and horrible mercurial history.
> b) will destroy all _your_ development history, c) will preserve both
> your history and the ns-3 history correctly. Based on the issue 1)
> outlined above (lots of unwanted files present in your
> repository/history), I would suggest that you go with b) because we
> really do not want to keep in our history all the files you have added
> there.
> 
> 
> 6) I read through the README which seems to be doing a good job of
> explaining what the code implements, and how. The rationale behind the
> structure of the code is, however, much less clear to me. 
> 
> For example, I see that you seem to be a using a PacketClassifier base
> class and you seem to be requesting each user to provide an explicit
> classifier for each device: the classifier is set at construction time
> and cannot change ever, even for the BS. That implementation choice
> seems to imply that it is not possible to create dynamic wimax
> connections with your code because each connection needs to be
> associated with a set of classification rules sent by the BS to the SSs
> once the connection is established to allow each SS to classify its
> traffic to different connections. I have to confess that I would have
> expected to see a single PacketClassifier class (not virtual) which
> would be able to parse and execute the classification rules as defined
> by the spec and which would be fed the classification rules sent by the
> BS to the SS and the classification rules built by the BS for itself.
> 
> 7) It would be really helpful to know what your TODO is over the next
> couple of months: it is hard, as-is, to figure out what is really just
> code waiting to be implemented shortly and what is ignored because you
> don't care about modelizing it. The end of README attempts to answer
> that question but it would be very helpful to have a more detailed plan
> of action about what you want to do. For example, the README seems to
> imply that scheduling will be implemented before the dynamic creation
> and management of service flows which seems really backward: I would
> have expected the latter to be implemented first properly with a very
> stupid scheduler.
> 
> 8) There is a lot of code to digest here (14 KLOC) so, it is going to
> take me a while to go through it all and make more interesting comments
> about the code itself. Generally, I have to say that such big code drops
> make it very very hard to provide good reviews: it would have been
> vastly easier to comment on this work incrementally and piece-by-piece
> rather than as a whole. 
> 
> 
> regards,
> Mathieu
>