[pve-devel] [PATCH dnsmasq 0/1] purge previous ip/mac lease of dhcp reply

[pve-devel] [PATCH dnsmasq 0/1] purge previous ip/mac lease of dhcp reply

[Alexandre Derumier]

This patch is specific and work only with dnsmask static lease.

If we try to allocate an existing leased ip to a new mac,
we need to purge first the lease.

This patch is doing it directly in dhcp reply phase.


I have made a deb with the patch for testing:
https://mutulin1.odiso.net:/dnsmasq-base_2.89-1_amd64.deb


ex:

guest ask for ip 192.168.2.10 with mac 12:45:6f:39:c2:a6

Nov 15 22:45:05 formationkvm3 dnsmasq-dhcp[846333]: DHCPDISCOVER(vnetpve) 192.168.2.10 12:45:6f:39:c2:a6
Nov 15 22:45:05 formationkvm3 dnsmasq-dhcp[846333]: DHCPOFFER(vnetpve) 192.168.2.10 12:45:6f:39:c2:a6
Nov 15 22:45:05 formationkvm3 dnsmasq-dhcp[846333]: DHCPREQUEST(vnetpve) 192.168.2.10 12:45:6f:39:c2:a6
Nov 15 22:45:05 formationkvm3 dnsmasq-dhcp[846333]: DHCPACK(vnetpve) 192.168.2.10 12:45:6f:39:c2:a6 testovn1


remove remove nic from guest

create a new nic in another guest, 192.168.2.10 is allocated to new mac 12:45:a3:ed:c8:36

we write ether file and reload dnsmasq

Nov 15 22:45:53 formationkvm3 systemd[1]: Reloading dnsmasq@simpve.service - dnsmasq (simpve) - A lightweight DHCP and caching DNS server...
Nov 15 22:45:53 formationkvm3 dnsmasq[846333]: cleared cache
Nov 15 22:45:53 formationkvm3 dnsmasq-dhcp[846333]: read /etc/dnsmasq.d/simpve/ethers
Nov 15 22:45:53 formationkvm3 systemd[1]: Reloaded dnsmasq@simpve.service - dnsmasq (simpve) - A lightweight DHCP and caching DNS server.

but the old mac:ip is still in lease memory of dnsmasq process


the guest is doing a dhcp query


here the patch: we purge the old lease
Nov 15 22:45:59 formationkvm3 dnsmasq-dhcp[846333]: workaround - pruning old lease

then the guest is able to retrieve the ip.
Nov 15 22:45:59 formationkvm3 dnsmasq-dhcp[846333]: DHCPDISCOVER(vnetpve) 192.168.2.10 12:45:a3:ed:c8:36 no address available
Nov 15 22:46:02 formationkvm3 dnsmasq-dhcp[846333]: DHCPDISCOVER(vnetpve) 192.168.2.10 12:45:a3:ed:c8:36
Nov 15 22:46:02 formationkvm3 dnsmasq-dhcp[846333]: DHCPOFFER(vnetpve) 192.168.2.10 12:45:a3:ed:c8:36
Nov 15 22:46:02 formationkvm3 dnsmasq-dhcp[846333]: DHCPREQUEST(vnetpve) 192.168.2.10 12:45:a3:ed:c8:36
Nov 15 22:46:02 formationkvm3 dnsmasq-dhcp[846333]: DHCPACK(vnetpve) 192.168.2.10 12:45:a3:ed:c8:36 testovn1

Alexandre Derumier (1):
  purge old ip-mac lease  on dhcpreply

 src/rfc2131.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

-- 
2.39.2

[pve-devel] [PATCH dnsmasq 1/1] purge old ip-mac lease on dhcpreply

[Alexandre Derumier]

---
 src/rfc2131.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/rfc2131.c b/src/rfc2131.c
index 17e97b5..2a4ce76 100644
--- a/src/rfc2131.c
+++ b/src/rfc2131.c
@@ -1095,7 +1095,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
 
 	  if ((opt = option_find(mess, sz, OPTION_REQUESTED_IP, INADDRSZ)))	 
 	    addr = option_addr(opt);
-	  
+
 	  if (have_config(config, CONFIG_ADDR))
 	    {
 	      inet_ntop(AF_INET, &config->addr, daemon->addrbuff, ADDRSTRLEN);
@@ -1104,11 +1104,14 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index,
 		  ltmp != lease &&
 		  !config_has_mac(config, ltmp->hwaddr, ltmp->hwaddr_len, ltmp->hwaddr_type))
 		{
-		  int len;
-		  unsigned char *mac = extended_hwaddr(ltmp->hwaddr_type, ltmp->hwaddr_len,
-						       ltmp->hwaddr, ltmp->clid_len, ltmp->clid, &len);
-		  my_syslog(MS_DHCP | LOG_WARNING, _("not using configured address %s because it is leased to %s"),
-			    daemon->addrbuff, print_mac(daemon->namebuff, mac, len));
+		  lease_prune(ltmp, now);
+		  my_syslog(MS_DHCP | LOG_WARNING, _("workaround - pruning old lease"));
+
+		  //int len;
+		  //unsigned char *mac = extended_hwaddr(ltmp->hwaddr_type, ltmp->hwaddr_len,
+		  //				       ltmp->hwaddr, ltmp->clid_len, ltmp->clid, &len);
+		  //my_syslog(MS_DHCP | LOG_WARNING, _("not using configured address %s because it is leased to %s"),
+		  //	    daemon->addrbuff, print_mac(daemon->namebuff, mac, len));
 		}
 	      else
 		{
-- 
2.39.2

Re: [pve-devel] [PATCH dnsmasq 0/1] purge previous ip/mac lease of dhcp reply

[Stefan Hanreich]

Maybe this [1][2] could be a less intrusive solution for this issue?

[1] https://manpages.ubuntu.com/manpages/focal/en/man1/dhcp_release.1.html
[2] https://packages.debian.org/de/sid/dnsmasq-utils

Re: [pve-devel] [PATCH dnsmasq 0/1] purge previous ip/mac lease of dhcp reply

[DERUMIER, Alexandre]

>>Maybe this [1][2] could be a less intrusive solution for this issue?

Yes, dhcp release packet should be the way, but I don't known if can
simply forge packet why any mac ?

I'll test it this afternoon to see if it's work.

Re: [pve-devel] [PATCH dnsmasq 0/1] purge previous ip/mac lease of dhcp reply

[DERUMIER, Alexandre]

-------- Message initial --------
De: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
À: pve-devel@lists.proxmox.com <pve-devel@lists.proxmox.com>,
aderumier@odiso.com <aderumier@odiso.com>, s.hanreich@proxmox.com
<s.hanreich@proxmox.com>
Objet: Re: [pve-devel] [PATCH dnsmasq 0/1] purge previous ip/mac lease
of dhcp reply
Date: 16/11/2023 14:47:20

> > Maybe this [1][2] could be a less intrusive solution for this
> > issue?

>>Yes, dhcp release packet should be the way, but I don't known if can
>simply forge packet why any mac ?
>>>
>>I'll test it this afternoon to see if it's work.


mmm,It's not working, the dhcp release packet never reach the bridge


I have also try to forge the packet in python with scapy, 
same bahviour.


from scapy.all import send, IP, UDP, BOOTP, DHCP, str2mac
import random

releaseMAC = '12:45:a3:ed:c8:36'
releaseIP='192.168.2.10'
serverIP='192.168.2.1'
releaseMACraw = str2mac(releaseMAC)

dhcp_release =
IP(dst=serverIP)/UDP(sport=68,dport=67)/BOOTP(chaddr=releaseMACraw,
ciaddr=releaseIP, xid=random.randint(0,
0xFFFFFFFF))/DHCP(options=[('message-type','release'), 'end'])
send(dhcp_release)

Re: [pve-devel] [PATCH dnsmasq 0/1] purge previous ip/mac lease of dhcp reply

[Wolfgang Bumiller]

On Fri, Nov 17, 2023 at 06:49:27AM +0000, DERUMIER, Alexandre wrote:
> -------- Message initial --------
> De: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
> À: pve-devel@lists.proxmox.com <pve-devel@lists.proxmox.com>,
> aderumier@odiso.com <aderumier@odiso.com>, s.hanreich@proxmox.com
> <s.hanreich@proxmox.com>
> Objet: Re: [pve-devel] [PATCH dnsmasq 0/1] purge previous ip/mac lease
> of dhcp reply
> Date: 16/11/2023 14:47:20
> 
> > > Maybe this [1][2] could be a less intrusive solution for this
> > > issue?
> 
> >>Yes, dhcp release packet should be the way, but I don't known if can
> >simply forge packet why any mac ?
> >>>
> >>I'll test it this afternoon to see if it's work.
> 
> 
> mmm,It's not working, the dhcp release packet never reach the bridge

What command did you use?

If all you need is the `lease_prune()` call from your C patch, dnsmasq
also does this on a SIGALRM so you could try to see if sending that
helps.

(dnsmasq also has a dbus api to add/remove leases for the worst case...)

Otherwise, at first glance the C patch seems to potentially break some
other cases, but I don't know the code at all.

Re: [pve-devel] [PATCH dnsmasq 0/1] purge previous ip/mac lease of dhcp reply

[DERUMIER, Alexandre]

#What command did you use?

dhcp_release <bridge> <maclease> <iplease>

or

dhcp_release <vmtap> <maclease> <iplease>



>>If all you need is the `lease_prune()` call from your C patch,
>>dnsmasq
>>also does this on a SIGALRM so you could try to see if sending that
>>helps.
>>
>>(dnsmasq also has a dbus api to add/remove leases for the worst
>>case...)

ah ok ! didn't known that, I'll try the SIGALRM && dbus to see. Thanks
!



>>Otherwise, at first glance the C patch seems to potentially break
>>some
>>other cases, but I don't know the code at all.

Yes, this will break dynamic leases as it's always flush leases at any
request.

Re: [pve-devel] [PATCH dnsmasq 0/1] purge previous ip/mac lease of dhcp reply

[DERUMIER, Alexandre]

> > If all you need is the `lease_prune()` call from your C patch,
> > dnsmasq
> > also does this on a SIGALRM so you could try to see if sending that
> > helps.
> > 
> > (dnsmasq also has a dbus api to add/remove leases for the worst
> > case...)

>>ah ok ! didn't known that, I'll try the SIGALRM && dbus to see.
>>Thanks
>>!

SIGALRM don't seem to work, 


but looking at dbus doc, that seem really even better
https://github.com/imp/dnsmasq/blob/master/dbus/DBus-interface

we could directly manage leases, without need to manage the ether file
and without reload.

Re: [pve-devel] [PATCH dnsmasq 0/1] purge previous ip/mac lease of dhcp reply

[DERUMIER, Alexandre]

> > If all you need is the `lease_prune()` call from your C patch,
> > dnsmasq
> > also does this on a SIGALRM so you could try to see if sending that
> > helps.
> > 
> > (dnsmasq also has a dbus api to add/remove leases for the worst
> > case...)

> > ah ok ! didn't known that, I'll try the SIGALRM && dbus to see.
> > Thanks
> > !

>>SIGALRM don't seem to work, 
>>

>>but looking at dbus doc, that seem really even better
>>https://github.com/imp/dnsmasq/blob/master/dbus/DBus-interface
>>
>>we could directly manage leases, without need to manage the ether
>>file
>>and without reload.


Ok, it's working with dbus to update the lease (and etherfile still
needed)

ex:


use Net::DBus;

my $bus = Net::DBus->system();
my $dnsmasq = $bus->get_service("uk.org.thekelleys.dnsmasq");
my $manager = $dnsmasq-
>get_object("/uk/org/thekelleys/dnsmasq","uk.org.thekelleys.dnsmasq");

my @hostname = unpack("C*", "vmhostname");
$manager->AddDhcpLease('192.168.2.10','12:45:6D:33:3C:E8', \@hostname,
undef, 0, 0, 0);




The problem is that dbus is only working with 1 instance of dnsmasq. :/

That mean it'll not work if we need mulitple instance, in differents
zones/vrf for example

Re: [pve-devel] [PATCH dnsmasq 0/1] purge previous ip/mac lease of dhcp reply

[Stefan Hanreich]

On 11/17/23 11:46, DERUMIER, Alexandre wrote:
> The problem is that dbus is only working with 1 instance of dnsmasq. :/
> 
> That mean it'll not work if we need mulitple instance, in differents
> zones/vrf for example
You should be able to set the service name via `--enable-dbus` then you
can handle multiple instances if I'm not mistaken.

Re: [pve-devel] [PATCH dnsmasq 0/1] purge previous ip/mac lease of dhcp reply

[DERUMIER, Alexandre]

On 11/17/23 11:46, DERUMIER, Alexandre wrote:
> The problem is that dbus is only working with 1 instance of dnsmasq.
> :/
> 
> That mean it'll not work if we need mulitple instance, in differents
> zones/vrf for example
>>You should be able to set the service name via `--enable-dbus` then
>>you
>>can handle multiple instances if I'm not mistaken.

oh yes , indeed, just found the doc about it.

So, I think it should work. 


thanks !