Opened 19 months ago

Last modified 17 months ago

#5616 new defect

Make DHO_DHCP_MESSAGE_TYPE be first

Reported by: Vladkansk Owned by:
Priority: medium Milestone: Outstanding Tasks
Component: dhcp Version: git
Keywords: packOptions4, DHO_DHCP_MESSAGE_TYPE Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

Hello!
There are some switches that user IP-MAC-PORT Binding by DHCP, for example D-Link DES series uses dhcp address_binding, a lot of that switches checks the DHCPACK message and accepts traffic on the port only from IP ip_ciaddr on this port. This switches look only first option in ACK message (I think that's because of memory limits on switches) and if it is DHCPACK they write ciaddr to table of accepts.
I used warkaound in LibDHCP::packOptions4
inserting loop with pushing DHO_DHCP_MESSAGE_TYPE to first place

void
LibDHCP::packOptions4(isc::util::OutputBuffer& buf,
                     const OptionCollection& options) {
    OptionPtr agent;
    OptionPtr end;
    for (OptionCollection::const_iterator it = options.begin();
        it != options.end(); ++it) {

        // DHCP Message type should be first
        switch (it->first) {
            case DHO_DHCP_MESSAGE_TYPE:
                it->second->pack(buf);
                break;
        }
    }
    for (OptionCollection::const_iterator it = options.begin();
         it != options.end(); ++it) {

        // RAI and END options must be last.
        switch (it->first) {
            case DHO_DHCP_AGENT_OPTIONS:
                agent = it->second;
                break;
            case DHO_END:
                end = it->second;
                break;
            default:
                it->second->pack(buf);
                break;
        }
    }

    // Add the RAI option if it exists.
    if (agent) {
       agent->pack(buf);
    }

    // And at the end the END option.
    if (end)  {
       end->pack(buf);
    }
}

I think you can do it better :)

Subtickets

Change History (1)

comment:1 Changed 17 months ago by tomek

  • Milestone changed from Kea-proposed to Outstanding Tasks

We discussed this problem during internal Kea call. This is essentially a client bug.
As far as we can tell, there is no RFC that says message type option should go out first.

Modifying Kea the way you proposed would do three things. First, it would hide the problem of broken clients. Second, it would encourage other vendors to do similar things. Third, it would degrade performance slightly for everyone.

Our recommendation here is to turn this into a hook that's installed on pkt4_send or perhaps buffer4_send.

Moving this ticket to outstanding. If we get another bug report of a similar nature, we'll write a hook library and will likely call it broken-clients or something similar.

Note: See TracTickets for help on using tickets.