d2g / dhcp4client

DHCP Client
Mozilla Public License 2.0
38 stars 30 forks source link

client: add support for easier custom option handling #24

Closed dcbw closed 4 years ago

dcbw commented 5 years ago

client: add support for easier custom option handling

Allow callers to pass arbitrary options to the various packet creation functions. The full Request() will pass the same set of options to the discover and request packets.

    options := dhcp4.Options{
            dhcp4.OptionClientIdentifier: clientID,
            dhcp4.dhcp4.OptionParameterRequestList: []byte{byte(dhcp4.OptionRouter)},
    }
    discovery := client.DiscoverPacket(options)

or

    options := dhcp4.Options{
            dhcp4.OptionClientIdentifier: clientID,
            dhcp4.dhcp4.OptionParameterRequestList: []byte{byte(dhcp4.OptionRouter)},
    }
    ok, ack, err := c.Request(options)

Fixes: https://github.com/d2g/dhcp4client/issues/22 Fixes: https://github.com/d2g/dhcp4client/pull/23

@d2g @mccv1r0

dcbw commented 5 years ago

@d2g I'm not really familiar with the codebeat issues, but I'm not sure what I should do to reduce the number of branches/statements/etc. I suppose I could create a separate new function to add the ClientID that then each of these would call?

d2g commented 5 years ago

Not worried about the code beat issue it was something I was trying out I don't see any issue with the code.

This isn't the way I was going with options. My intention is to allow you to pass the options you want to set and receive back based on message type and the option.

I've not thought about the feasibility etc but that was my initial thought. What do you think of that approach?

This would resolve #22, #23 also.

dcbw commented 5 years ago

@d2g do you mean something like:

func (c *Client) Request(options Options) (bool, dhcp4.Packet, error) {
        discoveryPacket, err := c.SendDiscoverPacket(options)
        if err != nil {
                return false, discoveryPacket, err
        }
        ...
}

and plumb that through SendDiscoverPacket() and maybe SendRequest() to add each option to the packet?

d2g commented 5 years ago

Yes, and plumbing the reverse in also to allow you to get the response options.

What do you think of the idea? I'll look to implement it into V2 (which is slowly getting where i want it)

dcbw commented 5 years ago

@d2g something like the repush? Should be mostly applicable to V2 as well. I'm not quite sure what you mean on for plubming the reverse, do you mean passing back packet.ParseOptions() in addition to the reply packets?

dcbw commented 5 years ago

@d2g any thoughts on this one? Thanks!

jellonek commented 5 years ago

@d2g ping

d2g commented 5 years ago

For reference when looking at the v2 implementation of this feature: RFC 4361

dcbw commented 5 years ago

@d2g has v2 obsoleted this PR? Or is it still relevant for v1? Thanks!