BIND 10 #991: Method to send a IPv4 packet to a client without an address

[BIND 10 Development]

#991: Method to send a IPv4 packet to a client without an address
-------------------------------------+-------------------------------------
            Reporter:  shane         |                        Owner:
                Type:  enhancement   |  marcin
            Priority:  medium        |                       Status:
           Component:  dhcp4         |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-DHCP-20130411
         Sub-Project:  DHCP          |                   Resolution:
Estimated Difficulty:  0.0           |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by tmark):

 * owner:  tmark => marcin


Comment:

 Review Comments:

 1. dhcp4_srv.cc -  Comment wording is wrong:
 line 521 -  "...client requesting new lease. We scan end response to"
 s/scan end/can send/

 2. dchp4_srv.cc -  else clause comment (line 781) says
 "...On the other hand, if we fail to set remote
 address to broadcast here, we can't unit-test the case when
 server doesn't support direct responses to the client
 which doesn't have address yet."

 Does this imply that answer-setRemoteAddr() can fail? If so should we be
 checking for that failure?


 3. dhcp4_srv_unittest.cc - in testDiscoverRequest(), there are 5 more
 tests
 (lines 437-446) of response content following the call to messageCheck().
 Why
 aren't these in messageCheck() it seems like they belong there.

 4. dhcp4_srv_unittest.cc - You "repeat the test" on line 453. Won't this
 will
 call processRequest for both a DISCOVER and a REQUEST type, regardless of
 the
 value of msg_type?

 5. pkt_filter, general -

 Doesn't this concept also apply to IPv6? The implementation for
 pkt_filter_inet.h
 is IPv4 specific.  Currently, an IfaceMgr isn't instantiated only for 4 or
 6,
 rather it supports both based on the send or receive methods invoked.  It
 would
 seem that if packet fitlers will be needed for v6: then either there
 should be
 protocol-specific derivations of pkt_filter and IfaceMgr will need an
 instance of each; or pkt_filter needs protocol specific sends/receives.

 6. PktFilterInet::openSocket() - if IP_PKTINFO is supported we try set
 socket
 option for it and throw if it fails.  What if IP_PKTINFO isn't defined?
 Isn't
 that just as "fatal"?

 7. PktFilterInet::openSocket()  - exception message for SO_BROADCAST
 option failure says "Failed to set SO_BINDTODEVICE".

 8. Code does not compile under OS-X: (I suspect it won't compile under
 Solaris
 either).

 libtool: compile:  g++ -DHAVE_CONFIG_H -I. -I../../.. -I../../../src/lib
 -I../../../src/lib -I/opt/local/include -I/opt/local/include
 -D__APPLE_USE_RFC_3542 -DOS_BSD -I../../../ext/asio
 -I../../../ext/coroutine -DASIO_DISABLE_THREADS=1 -Wall -Wextra -Wwrite-
 strings -Woverloaded-virtual -Wno-sign-compare -Werror -fPIC -Wno-missing-
 field-initializers -g -O2 -MT libb10_dhcp___la-iface_mgr_bsd.lo -MD -MP
 -MF .deps/libb10_dhcp___la-iface_mgr_bsd.Tpo -c iface_mgr_bsd.cc  -fno-
 common -DPIC -o .libs/libb10_dhcp___la-iface_mgr_bsd.o
 iface_mgr_bsd.cc: In member function 'int
 isc::dhcp::IfaceMgr::openSocket4(isc::dhcp::Iface&, const
 isc::asiolink::IOAddress&, uint16_t, bool, bool)':
 iface_mgr_bsd.cc:54: error: 'socket_handler_' was not declared in this
 scope
 iface_mgr_bsd.cc:61: error: 'SO_BINDTODEVICE' was not declared in this
 scope


 Under Debian everything builds, unit tests pass.  I even ran a test with
 perfdchp and every
 thing works fine.

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