search

insomniacslk / dhcp

DHCPv6 and DHCPv4 packet library, client and server written in Go
BSD 3-Clause "New" or "Revised" License
719 stars 170 forks source link

client: add functionality to release the lease #311

Closed f1-outsourcing closed 4 years ago

f1-outsourcing commented 5 years ago

I was testing with your examples/client6 and wondered how I can add there an client id of vendor id to send with the dhcp request

pmazzini commented 5 years ago

I was testing with your examples/client6 and wondered how I can add there an client id

You can do something like:

conversation, err := client.Exchange(*iface, dhcpv6.WithClientID(...))

Have a look at the modifiers in https://github.com/insomniacslk/dhcp/blob/master/dhcpv6/modifiers.go#L12

f1-outsourcing commented 5 years ago

Yes thanks! I do not thing there is a function like the Exchange to release the ip address, or is there?

pmazzini commented 5 years ago

No, there is nothing at the moment but it can be added.

It would probably be similar to what https://github.com/rtr7/router7 is doing.

https://github.com/rtr7/router7/blob/master/internal/dhcp6/dhcp6.go#L305

Would you like to submit a PR?

pmazzini commented 5 years ago

Should this issue be closed / renamed?

f1-outsourcing commented 5 years ago

No, there is nothing at the moment but it can be added.

It would probably be similar to what https://github.com/rtr7/router7 is doing.

https://github.com/rtr7/router7/blob/master/internal/dhcp6/dhcp6.go#L305

Hmmm, I am using still ipv4, this NewRequestFromAdvertise is not available there and the v4 section of ther rtr7 is a bit cryptic to me https://github.com/rtr7/router7/blob/3765287e97a4a7c911e03f2f01fd1145d2b46c46/internal/dhcp4/dhcp4.go#L160 I am also not to familiar with the dhcp protocol. I guess you have to initiate this advertisement first to send the release? I assume the procedure is the same for ipv4 as for ipv6?

Would you like to submit a PR?

pmazzini commented 5 years ago

Oh right that is v6 code. Ok lets leave this issue to track the missing release lease functionality from the client.

f1-outsourcing commented 5 years ago

Ok, but am I correct that the Release function would be very similar to the Exchange function? Or does it require some already available data from a previous data exchange stored in a connection variable or so? In a linux dhcp client, there is always this process dhclient running, I guess to refresh leases etc. So I can assume it is storing data and reusing it in the communication and also in the release of the lease.

However I am using your library in a cni ipam plugin dhcpslim[0] to request an ip address from the dhcp server. But where the regular cni ipam dhcp plugin keeps daemons running to refresh ip leases etc. I was thinking of just getting an ip and assigning it, and releasing it when the container shuts down. That means I am not going to keep processes running for maintaining a dhcp connection and I have to send a release request to the dhcp without the knowledge of a previous connection. Maybe just with the dhcp-client-identifier option. Would this be possible?

Aside from maybe an increased risk of incorrect lease administration on the dhcp server, is there something else I need to be beware of?

[0] https://github.com/f1-outsourcing/plugins/tree/hostrouteif/plugins/ipam/dhcpslim

I

pmazzini commented 5 years ago

Ok, but am I correct that the Release function would be very similar to the Exchange function? Or does it require some already available data from a previous data exchange stored in a connection variable or so?

It would be similar but some data such from the previous exchange is needed, such as the IP you are releasing.

In a linux dhcp client, there is always this process dhclient running, I guess to refresh leases etc. So I can assume it is storing data and reusing it in the communication and also in the release of the lease.

Yes, that is right. It is constantly running to renew the lease, etc.

I was thinking of just getting an ip and assigning it, and releasing it when the container shuts down.

The problem that I see with this is that if the client does not honour the lease time, the server may assign the same IP to another client.

It depends on your server configuration.

and releasing it when the container shuts down.

You can avoid releasing it and letting the lease expire but you may run out of IPs.

Again it depends on your server configuration.

pmazzini commented 5 years ago

@f1-outsourcing shall I close this?

f1-outsourcing commented 5 years ago

Should this not stay open until it is implemented? Looks like I need to have this fixed sooner that expected, because it is quite annoying having to clear the leases manually

f1-outsourcing commented 5 years ago

What do you think of this? I am not to familiar with go nor dhcp protocol, but with this it looks like the dhcp server is receiving the release request.

https://github.com/insomniacslk/dhcp/compare/master...f1-outsourcing:master

pmazzini commented 5 years ago

Is the server actually releasing the lease? It doesn't look like the release request is including YourClientIP.

Also, I would look at the new client nclient4.

f1-outsourcing commented 5 years ago

Yes, using this

client.Release(ipamConf.IPAM.DhcpIf, dhcpv4.WithClientIP(ip.IP))

hujun-open commented 4 years ago

I have coded lease & release support for nclient4, tested with a 3rd party DHCP server; there is an example in lease.go; if you think it is ok, I will create a pull request

https://github.com/insomniacslk/dhcp/compare/master...hujun-open:dhcpv4_release

hugelgupf commented 4 years ago

Open a PR and we can comment on it!

hujun-open commented 4 years ago

PR opened #386