[Ns-developers] release update [feedback needed]

[Tom Henderson]

Mathieu Lacage wrote:
> On Tue, 2009-06-09 at 16:03 +0000, Tom Henderson wrote:
> 
>> I like the general idea.  It could be expanded to align with how
> 
> Shall I infer from the above that you approve of the patch as-is and
> that I can push it ? Or that you plan to review it carefully later ?

this patch cleans up a sore spot in the implementation.  More work needs 
to be done for static multicast routes, but regression tests pass, and I 
think it is an improvement.

The main API is in the Ipv4RoutingProtocol base class:
+  /**
+   * \param i the index of the interface we are being notified about
+   * \param isUp is this interface coming up or down ?
+   *
+   * Protocols are expected to implement this method to be notified of 
the state change of
+   * an interface in a node. When a new protocol is added to a node, it 
is notified
+   * of all existing interfaces.
+   */
+  virtual void NotifyInterface (uint32_t i, bool isUp) = 0;

This is similar to what is found in RTM_NEWLINK and RTM_DELLINK for 
rtnetlink in Linux (man rtnetlink), except that the boolean flag allows 
both to be in a single method NotifyInterface.  Since the routing 
protocol consuming this message will presumably be able to acquire a 
pointer to class Ipv4 to get whatever other information is needed, the 
interface index is probably sufficient information to pass up.

The questions/concerns I have:

1) implicit assumptions are made about the order in which these 
operations occur:
- set an IP interface to up
- add an IP address to an interface
- set an IPv4RoutingProtocol to the Ipv4 object
- add an additional routing protocol to the Ipv4ListRouting protocol

There are certain combinations of operations that will cause this to 
lose information; e.g.
- add the Ipv4ListRouting to the Ipv4, and subsequently add protocols 
like OLSR (the protocols added later will not get the notifications)
- add an IP address to an interface (such as a second address) after the 
first address was added and interface was SetUp()

so, it would help to add code to deal with users possibly rearranging 
these operations.  For instance, I think that probably these additions 
are needed:
   - NotifyAddAddress (uint32_t i, Ipv4InterfaceAddress addr)
   - NotifyRemoveAddress (uint32_t i, Ipv4InterfaceAddress addr)

or, for symmetry with NotifyInterface(), it could be:
   - NotifyAddress (uint32_t i, Ipv4InterfaceAddress addr, bool IsAdded);

and probably Ipv4ListRoutingImpl::AddRoutingProtocol () needs to notify 
the new routing protocol added of the interfaces, in case it is added 
after being hooked to Ipv4.

2) implicit assumption about the user not repeating these methods or
calling them out of order-- e.g. can NotifyInterface (i, true); be 
called multiple times (resulting in multiple redundant routes)?  Should, 
for instance, the current state of an interface be saved so that these 
calls can be detected and avoided?

3) I had thought about putting in an rtnetlink-like framework for what 
you posted, but that may be overkill for the moment.  However, suppose 
someone wanted to add an rtnetlink notification framework to ns-3 in the 
future.  What kind of notification framework would you recommend? 
Multicast messaging internally?  Some kind of callback framework (could 
you have multiple users register callbacks somewhere for the same event, 
and where would they register (a Netlink object?)?

Thanks for the patch,
Tom