[Ns-developers] NetDevice link change callback

[Mathieu Lacage]

On Thu, 2009-08-06 at 15:50 -0600, Tom Henderson wrote:

> >> 1) use of TracedCallback?
> >> Another option is to use a TracedCallback here, which some other 
> >> developers have recommended in the past to use when there needs to be a 
> >> list of callbacks.
> >> 
> >> I don't have a strong opinion but this question keeps popping up (also 
> >> in Qasim's conntrack code) so I think we should clarify whether 
> >> TracedCallback should also be used in non-tracing scenarios whenever you
> 
> >> want a std::list of callbacks, or whether we want another general 
> >> CallbackList of some sort that is not used by tracing code.
> > 
> > I think that all you would need to make TracedCallback generic is change
> > its name to something which does not include 'Trace'. i.e., its API and
> > implementation has no dependency on the tracing code.
> 
> Maybe what would be best is to add something like the CallbackList
> suggested but allow user to optionally specify order or priority of
> callbacks-- if no priority is specified, they all get priority 0 and it
> behaves like TracedCallback.

It's not clear to me what this priority would be used for. Do you have a
specific usecase in mind ? If not, I would support simply renaming
TracedCallback to NotifierCallback or something similar (better
suggestions for a name would be welcome) and typedef NotifierCallback to
TracedCallback to avoid changing our existing codebase.

> >> 2) should we report link change, or link up?
> >> The method name is not really suggestive of how the callback works, 
> >> which does not call when the link changes in all cases but only when the
> 
> >> link goes to up.  So, I would suggest either "AddLinkUpCallback" or 
> >> "SetLinkChangeCallback" with an extra argument such as an enum for Up or
> 
> >> Down.
> > 
> > The current implementation reports link change events, not link up
> > events. i.e., see src/devices/wifi/wifi-net-devices.cc. The current API
> > documentation in src/node/net-device.h should be improved instead of
> > changing the method name I think.
> 
> Only WifiNetDevice reports all link changes, but you can't tell from the
> callback what type of event it was.  The other devices (PointToPoint, Csma,

IsLinkUp is expected to be used from within the notification callback to
query the state of the link. That was the original intend of the device
API design.

> Emu) just call it upon going up, perhaps because they can't go down (it is
> really only used at node startup time).

Yes, none of them can go down which is why they never report link down
events.

Anyway, to summarize, I see no real need to change this API since it
provides the needed functionality. What is needed is improving the
doxygen which does not align with the intend of the original design.

> >> I think others have raised the question "can I put a NetDevice into down
> 
> >> state?" and presently the answer is no, but in practice, IFF_UP flag of 
> >> a netdevice is settable, and it seems like we might want to add the 
> >> capability to configure an interface to down state, which would trigger 
> >> upcalls to both Ipv4 and Ipv6 stacks (and to the routing protocols).
> > 
> > What would be the expected semantics of that flag exactly ? Would you
> > expect the NetDevice subclasses to honor calling SetIfDown by ignoring
> > packets being passed down with NetDevice::Send* ?
> > 
> 
> Yes.  I would try to match it to what happens in a real system when
> "ifconfig eth0 down" is called.  Note that this typically will trigger
> upcalls or netlink notifications, which cause the upper layer to refrain
> from sending packets down to it when it is in a down state.  

I think that it would be nice if adding such a flag would not require
adding extra send/receive logic to net device implementations but I
could live with it.

Mathieu