BIND 10 #878: Code to bind to address/port

[BIND 10 Development]

#878: Code to bind to address/port
-------------------------------------+-------------------------------------
                   Reporter:  shane  |                 Owner:  shane
                       Type:         |                Status:  reviewing
  enhancement                        |             Milestone:  Sprint-
                   Priority:  major  |  DHCP-20110712
                  Component:  dhcp   |            Resolution:
                   Keywords:         |             Sensitive:  0
            Defect Severity:  N/A    |           Sub-Project:  DHCP
Feature Depending on Ticket:  dhcp-  |  Estimated Difficulty:  3.0
  echo                               |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by shane):

 Moving on to the iface_mgr class:

   * We probably don't need to expose the instanceCreate() method, since
 this is invoked by the instance() method. Also, Boost provides a utility
 function to only call a function once, so we can use that rather than
 checking for duplicate invocations and throwing an exception in
 instanceCreate().

     http://www.boost.org/doc/libs/1_32_0/doc/html/call_once.html

   * The openSockets() method can be private, as can openSocket() and
 joinMcast() actually.

   * You may as well pass a stream to printIfaces, so printIfaces(ostream&
 out = cout) or something like that.

   * Should the getIface() methods return constant values? I'm thinking:

 {{{
         Iface const* getIface(int ifindex) const;
         Iface const* getIface(const std::string& ifname) const;
 }}}

   * I generally prefer returning NULL instead of (0) (as in getIface()),
 but I don't know if that is actually a BIND 10 style or just my personal
 preference. ;)

   * Do we actually need a constructor for Iface() objects that don't have
 a name and index defined? Isn't such an object actually undefined in a
 way? (I can see we might want a way to define an instance with a name
 only, so that if such an interface becomes available we can use it, but we
 may want to use a special interface ID or set a flag for that case... or
 perhaps make that a separate class.)

   * As you noted, we have to convert this to the logging framework.
 Stephen can probably help with that when he's back from vacation.

   * The !IfaceMgr() constructor is not exception-safe. The control_buf_
 allocation can be moved to the end of the function, since it is not used
 in the constructor itself. The detectIface() helper function should also
 be careful to de-allocate all of its memory when it encounters an error
 (possibly not so important right now since it is a throw-away technique,
 but a comment should be added so it is not forgotten). Likewise, the
 openSockets() function should close any open sockets if it fails. (For
 example the 1^st^ openSocket() call succeeding and the 2^nd^ failing.)

   * openSocket() should close the file descriptor when returning an error,
 such as with setsockopt() or bind().

   * It needs to be clearly documented that the to/from information (addr,
 port, interface) is all included in the packets used in send() and
 receive().

   * In receive(), is 1500 a safe value? I know that jumbo packets exist,
 and in any case a UDP packet can be more than 1500 bytes right?

   * I tend to think receive() on an unknown interface should be considered
 an error. We can't reply to it, correct?

   * Is it possible we will bind multiple ports on a single address? Maybe
 receive() should track that too...

-- 
Ticket URL: <http://bind10.isc.org/ticket/878#comment:14>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development