[Ns-developers] [ns3] netlink sockets

[Mathieu Lacage]

hi liu,

I see that you checked in a lot of new code to handle the netlink
messages received on a netlink socket. A couple of comments on your last
code drop:

1) when you return -1 from a socket function, you _must_ set m_errno
with a meaningful error code: I did spot a couple of places where you
were returning -1 without setting it and I also saw that you use an errp
argument to pass around a pointer to an errno variable. You don't need
that extra errp argument: you can just set m_errno directly

2) the method names are unecessarily long and they should not contain
the parse keyword because you have done the parsing already in
Socket::Send so, ParseNetlinkRouteMessageAtKernel -> HandleRouteMessage
for example. 

3) relying on the relative _values_ of NETLINK_RTM_MAX in every piece of
code makes it needlessly fragile and hard to read. I know that the linux
kernel code does that but, that is no excuse :)
For example:
  if (type < NETLINK_RTM_BASE)
    {
      NS_LOG_INFO("netlink control message type not parsed in kernel");
      return 0;
    }
could be rewritten to:
if (IsMessageTypeControl (type))
  {
    // XXX
  }
So, IsMessageTypeControl is the only function which knows the details of
the ordering of the NETLINK_RTM_ constants

The same holds for the scary test:
((type - NETLINK_RTM_BASE)&3) == 2
which should be encapsulated in an IsMessageTypeXXX method.

4) NetlinkSocket::NetlinkRouteDumpMessageAtKernel could be split with
one function for each message type. The same holds for
NetlinkSocket::NetlinkRouteDoMessageAtKernel

5) style issue:
for (...)
{
  // stuff
}
should be instead:
for (...)
  {
    // stuff
  }

6) need more automatic test program.

I see that examples/simple-netlink-socket.cc does print out the answers
it gets from each message sent to the kernel. It would be nicer if it
could check that the answers are correct and print a single final
output: PASS or FAIL. i.e., you could send your messages on the socket,
not read the answers from MyReceivePacket and, instead, read the answers
in one go from the socket after sending the message and check that they
are correct.

regards,
Mathieu