[Ns-bugs] [Bug 407] OLSR is missing HNA support

[code@nsnam.ece.gatech.edu]

http://www.nsnam.org/bugzilla/show_bug.cgi?id=407





--- Comment #36 from suresh.lalith at gmail.com  2010-03-12 11:07:17 EDT ---
(In reply to comment #35)
> ///
> /// \brief Adds an Ipv4StaticRouting protocol Association
> ///        can generate an HNA message for
> ///
> void
> RoutingProtocol::AddRoutingTableAssociation (Ptr<Ipv4StaticRouting>
> routingTable)
> {
>   m_routingTableAssociation = routingTable;
> }
> 
> The naming does not agree with the function.  If there is only one static table
> that can be used as source for HNA messages, then the method should be
> SetRoutingTableAssociation.  If you wish to keep AddRoutingTableAssociation
> then you need to provide an implementation that can really accept multiple
> tables, not just one.
> 
> Also, I don't see the point of this method and AddHostNetworkAssociation to be
> declared as private.  Isn't this supposed to be used by the simulation scripts?

Actually, I kept testing by using these methods from within the agent code
itself. My bad! As you've suggested, I'll prepare an example script and attach
that as well next.

> 
> In olsr-state.h:
> 
> +  AssociationSet m_associationSet;    ///< Association Set (RFC 3626, section
> 12.2)
> +  Associations m_associations;    ///< The Host Network Associations of the
> node
> 
> I think you should revise the comments to make it clearer the difference
> between the two associations.  I suggest for instance:
> 
> +  AssociationSet m_associationSet;    ///< Association Set (RFC 3626, section
> 12.2).  These associations are the result of HNA messages received from other
> OLSR nodes.
> +  Associations m_associations;    ///< The local Host Network Associations of
> the node that it will advertise to the network using OLSR HNA messages.
> 

Ok.

> 
> [in several places] Please don't use this construct to initialize a structure
> tmp value:
> +           
> associations.push_back((olsr::MessageHeader::Hna::Association){route.GetDestNetwork
> (), route.GetDestNetworkMask ()});
> 
> I think this is a GCC extension. Instead use the typical notation with an
> intermediate variable (hopefully the optimized code will be the same):
> 
> + olsr::MessageHeader::Hna::Association assoc = {route.GetDestNetwork (),
> route.GetDestNetworkMask ()};
> + associations.push_back (assoc);

Ok.

> 
> Finally, we could really use an example program that uses this, to test the
> implementation and exemplify the API.  We can't be sure this works without an
> example.  In fact, with those methods private, I suspect it doesn't do anything
> for now... :-)

Will do that as well! :)

-- 
Configure bugmail: http://www.nsnam.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.