BIND 10 #3231: server-id for Kea4 does not work properly

[BIND 10 Development]

#3231: server-id for Kea4 does not work properly
-------------------------------------+-------------------------------------
            Reporter:  tomek         |                        Owner:
                Type:  enhancement   |  marcin
            Priority:  medium        |                       Status:
           Component:  dhcp4         |  reviewing
            Keywords:                |                    Milestone:  DHCP-
           Sensitive:  0             |  Kea1.0-alpha
         Sub-Project:  DHCP          |                   Resolution:
Estimated Difficulty:  0             |                 CVSS Scoring:
         Total Hours:  4             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  4
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by tmark):

 * hours:  0 => 4
 * owner:  tmark => marcin
 * totalhours:  0 => 4


Comment:

 I think this is terrific solution. Good job.

 General:

 * Since server id file is now obsolete, this should perhaps be mentioned
 in the !ChangeLog?  Otherwise anyone running b10-dhcp4 now will have this
 file and it will be orphaned.  Either that or we have to come up with a
 way for it to get wiped out?

 -------------------------------------------------------------------------------

 src/bin/dhcp4/dhcp4_src.h

 Why are the methods adjustIfaceData, adjustRemoteAddr, and appendServerId
 declared static?  I saw comments on other existing static methods that say
 it is because they are not dependent upon server state.  I don't agree
 with this approach. It makes it impossible to override them in deriving
 test fixtures.

 As a general rule I try to limit statics to tasks relegated to singleton
 start up, those uninteresting in terms of inheritance, or unaffected if
 there were multiple instances of an object.  Mind you this is my own
 philosophy, I know there are others.

 -------------------------------------------------------------------------------

 src/bin/dhcp4/dhcp4_src.cc

 Dhcpv4Srv::adjustIfaceData()

 *At line 1157, response->getHops() is used to determine which port value
 is used in the response.  In trac3203 (not yet merged),  tomek changed
 this decision to be based upon query->getRemotePort().  One of you appears
 to be wrong.  You may wish to discuss this with him.  Additionally,
 adjustRemoteAddr uses non-zero value of giaddr to detect relay messages,
 so even with this we are using two different ways to check for "relayed-
 ness".

 ------------------------------------------------------------------------------

 Code passes cppcheck.sh, unit tests pass valgrind.

-- 
Ticket URL: <https://bind10.isc.org/ticket/3231#comment:6>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development