[pve-devel] [WIP v2 cluster/network/manager/qemu-server/container 00/10] Add support for DHCP servers to SDN

[pve-devel] [WIP v2 cluster/network/manager/qemu-server/container 00/10] Add support for DHCP servers to SDN

[Stefan Hanreich]

This is a WIP patch series, since I will be gone for 3 weeks and wanted to
share my current progress with the DHCP support for SDN.

This patch series adds support for automatically deploying dnsmasq as a DHCP
server to a simple SDN Zone.

While certainly not 100% polished on some ends (looking at restarting systemd
services in particular), the general idea behind the mechanism shows. I wanted
to gather some feedback on how I approached designing the plugins and the
config regeneration process before comitting to this design by creating an API
and UI around it.

You need to install dnsmasq (and disable it afterwards):

  apt install dnsmasq && systemctl disable --now dnsmasq


You can use the following example configuration for deploying a DHCP server in
a SDN subnet:

/etc/pve/sdn/dhcp.cfg:

  dnsmasq: nat


/etc/pve/sdn/zones.cfg:

  simple: DHCPNAT
          ipam pve


/etc/pve/sdn/vnets.cfg:

  vnet: dhcpnat
          zone DHCPNAT


/etc/pve/sdn/subnets.cfg:

  subnet: DHCPNAT-10.1.0.0-16
          vnet dhcpnat
          dhcp-dns-server 10.1.0.1
          dhcp-range server=nat,start-address=10.1.0.100,end-address=10.1.0.200
          gateway 10.1.0.1
          snat 1


Then apply the SDN configuration:

  pvesh set /cluster/sdn

You need to apply the SDN configuration once after adding the dhcp-range lines
to the configuration, since the running configuration is used for managing
DHCP. It will not work otherwise!

For testing it can be helpful to monitor the following files (e.g. with watch)
to find out what is happening
  * /etc/dnsmasq.d/<dhcp_id>/ethers (on each node)
  * /etc/pve/priv/ipam.db

Changes from v1 -> v2:
  * added hooks for handling DHCP when starting / stopping / .. VMs and CTs
  * Get an IP from IPAM and register that IP in the DHCP server
    (pve only for now)
  * remove lease-time, since it is now infinite and managed by the VM lifecycle
  * add hooks for setting & deleting DHCP mappings to DHCP plugins
  * modified interface of the abstract class to reflect new requirements
  * added helpers in existing SDN classes
  * simplified DHCP configuration settings



pve-cluster:

Stefan Hanreich (1):
  cluster files: add dhcp.cfg

 src/PVE/Cluster.pm  | 1 +
 src/pmxcfs/status.c | 1 +
 2 files changed, 2 insertions(+)


pve-network:

Stefan Hanreich (6):
  subnets: vnets: preparations for DHCP plugins
  dhcp: add abstract class for DHCP plugins
  dhcp: subnet: add DHCP options to subnet configuration
  dhcp: add DHCP plugin for dnsmasq
  ipam: Add helper methods for DHCP to PVE IPAM
  dhcp: regenerate config for DHCP servers on reload

 debian/control                         |   1 +
 src/PVE/Network/SDN.pm                 |  11 +-
 src/PVE/Network/SDN/Dhcp.pm            | 192 +++++++++++++++++++++++++
 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm    | 186 ++++++++++++++++++++++++
 src/PVE/Network/SDN/Dhcp/Makefile      |   8 ++
 src/PVE/Network/SDN/Dhcp/Plugin.pm     |  83 +++++++++++
 src/PVE/Network/SDN/Ipams/PVEPlugin.pm |  64 +++++++++
 src/PVE/Network/SDN/Makefile           |   3 +-
 src/PVE/Network/SDN/SubnetPlugin.pm    |  32 +++++
 src/PVE/Network/SDN/Subnets.pm         |  43 ++++--
 src/PVE/Network/SDN/Vnets.pm           |  27 ++--
 11 files changed, 622 insertions(+), 28 deletions(-)
 create mode 100644 src/PVE/Network/SDN/Dhcp.pm
 create mode 100644 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
 create mode 100644 src/PVE/Network/SDN/Dhcp/Makefile
 create mode 100644 src/PVE/Network/SDN/Dhcp/Plugin.pm


pve-manager:

Stefan Hanreich (1):
  sdn: regenerate DHCP config on reload

 PVE/API2/Network.pm | 1 +
 1 file changed, 1 insertion(+)


qemu-server:

Stefan Hanreich (1):
  sdn: dhcp: add DHCP setup to vm-network-scripts

 PVE/QemuServer.pm                 | 14 ++++++++++++++
 vm-network-scripts/pve-bridge     |  3 +++
 vm-network-scripts/pve-bridgedown | 19 +++++++++++++++++++
 3 files changed, 36 insertions(+)


pve-container:

Stefan Hanreich (1):
  sdn: dhcp: setup DHCP mappings in LXC hooks

 src/PVE/LXC.pm            | 10 ++++++++++
 src/lxc-pve-poststop-hook |  1 +
 src/lxc-pve-prestart-hook |  9 +++++++++
 3 files changed, 20 insertions(+)


Summary over all repositories:
  20 files changed, 681 insertions(+), 28 deletions(-)

-- 
murpp v0.4.0

[pve-devel] [WIP v2 pve-cluster 01/10] cluster files: add dhcp.cfg

[Stefan Hanreich]

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/Cluster.pm  | 1 +
 src/pmxcfs/status.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/src/PVE/Cluster.pm b/src/PVE/Cluster.pm
index cfa2583..aac4574 100644
--- a/src/PVE/Cluster.pm
+++ b/src/PVE/Cluster.pm
@@ -78,6 +78,7 @@ my $observed = {
     'sdn/subnets.cfg' => 1,
     'sdn/ipams.cfg' => 1,
     'sdn/dns.cfg' => 1,
+    'sdn/dhcp.cfg' => 1,
     'sdn/.running-config' => 1,
     'virtual-guest/cpu-models.conf' => 1,
     'mapping/pci.cfg' => 1,
diff --git a/src/pmxcfs/status.c b/src/pmxcfs/status.c
index c8094ac..4993fc1 100644
--- a/src/pmxcfs/status.c
+++ b/src/pmxcfs/status.c
@@ -107,6 +107,7 @@ static memdb_change_t memdb_change_array[] = {
 	{ .path = "sdn/subnets.cfg" },
 	{ .path = "sdn/ipams.cfg" },
 	{ .path = "sdn/dns.cfg" },
+	{ .path = "sdn/dhcp.cfg" },
 	{ .path = "sdn/.running-config" },
 	{ .path = "virtual-guest/cpu-models.conf" },
 	{ .path = "firewall/cluster.fw" },
-- 
2.39.2

[pve-devel] [WIP v2 pve-network 02/10] subnets: vnets: preparations for DHCP plugins

[Stefan Hanreich]

Add the option to retrieve the running configuration instead of only
the pending configuration via the config methods. Refactor methods
using the running config to utilize the new parameter.

Additionally include the id for subnets, since it is needed later by
some of the DHCP plugins.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/Network/SDN/Subnets.pm | 25 +++++++++++++------------
 src/PVE/Network/SDN/Vnets.pm   | 27 +++++++++++++--------------
 2 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/src/PVE/Network/SDN/Subnets.pm b/src/PVE/Network/SDN/Subnets.pm
index 6bb42e5..f654d3a 100644
--- a/src/PVE/Network/SDN/Subnets.pm
+++ b/src/PVE/Network/SDN/Subnets.pm
@@ -23,7 +23,9 @@ sub sdn_subnets_config {
     my $scfg = $cfg->{ids}->{$id};
     die "sdn subnet '$id' does not exist\n" if (!$noerr && !$scfg);
 
-    if($scfg) {
+    if ($scfg) {
+	$scfg->{id} = $id;
+
 	my ($zone, $network, $mask) = split(/-/, $id);
 	$scfg->{cidr} = "$network/$mask";
 	$scfg->{zone} = $zone;
@@ -35,7 +37,14 @@ sub sdn_subnets_config {
 }
 
 sub config {
-    my $config = cfs_read_file("sdn/subnets.cfg");
+    my ($running) = @_;
+
+    if ($running) {
+	my $cfg = PVE::Network::SDN::running_config();
+	return $cfg->{subnets};
+    }
+
+    return cfs_read_file("sdn/subnets.cfg");
 }
 
 sub write_config {
@@ -61,16 +70,8 @@ sub complete_sdn_subnet {
 sub get_subnet {
     my ($subnetid, $running) = @_;
 
-    my $cfg = {};
-    if($running) {
-	my $cfg = PVE::Network::SDN::running_config();
-	$cfg = $cfg->{subnets};
-    } else {
-	$cfg = PVE::Network::SDN::Subnets::config();
-    }
-
-    my $subnet = PVE::Network::SDN::Subnets::sdn_subnets_config($cfg, $subnetid, 1);
-    return $subnet;
+    my $cfg = PVE::Network::SDN::Subnets::config($running);
+    return PVE::Network::SDN::Subnets::sdn_subnets_config($cfg, $subnetid, 1);
 }
 
 sub find_ip_subnet {
diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm
index 1106c9f..39bdda0 100644
--- a/src/PVE/Network/SDN/Vnets.pm
+++ b/src/PVE/Network/SDN/Vnets.pm
@@ -26,6 +26,13 @@ sub sdn_vnets_config {
 }
 
 sub config {
+    my ($running) = @_;
+
+    if ($running) {
+	my $cfg = PVE::Network::SDN::running_config();
+	return $cfg->{vnets};
+    }
+
     return cfs_read_file("sdn/vnets.cfg");
 }
 
@@ -54,31 +61,23 @@ sub get_vnet {
 
     return if !$vnetid;
 
-    my $scfg = {};
-    if($running) {
-	my $cfg = PVE::Network::SDN::running_config();
-	$scfg = $cfg->{vnets};
-    } else {
-	$scfg = PVE::Network::SDN::Vnets::config();
-    }
-
-    my $vnet = PVE::Network::SDN::Vnets::sdn_vnets_config($scfg, $vnetid, 1);
-
-    return $vnet;
+    my $cfg = PVE::Network::SDN::Vnets::config($running);
+    return PVE::Network::SDN::Vnets::sdn_vnets_config($cfg, $vnetid, 1);
 }
 
 sub get_subnets {
-    my ($vnetid) = @_;
+    my ($vnetid, $running) = @_;
 
     my $subnets = undef;
-    my $subnets_cfg = PVE::Network::SDN::Subnets::config();
+    my $subnets_cfg = PVE::Network::SDN::Subnets::config($running);
+
     foreach my $subnetid (sort keys %{$subnets_cfg->{ids}}) {
 	my $subnet = PVE::Network::SDN::Subnets::sdn_subnets_config($subnets_cfg, $subnetid);
 	next if !$subnet->{vnet} || ($vnetid && $subnet->{vnet} ne $vnetid);
 	$subnets->{$subnetid} = $subnet;
     }
-    return $subnets;
 
+    return $subnets;
 }
 
 sub get_subnet_from_vnet_cidr {
-- 
2.39.2

[pve-devel] [WIP v2 pve-network 03/10] dhcp: add abstract class for DHCP plugins

[Stefan Hanreich]

This abstract class provides several hooks that should be called
during the config generation process, they expose the functionality
for the different configuration tasks required from the DHCP plugins.

add_ip_mapping
Adds a mapping from MAC address to an IP for a given DHCP server. The
DHCP server will then always assign the given IP address to the MAC.

del_ip_mapping
Deletes all mappings for a given MAC address for a given DHCP server.

before_regenerate
Should be called before the plugin does any configuration tasks. The
main usage for this hook is tearing down old instances.

after_regenerate
Should be called after the plugin has finished generating any
configuration. The main usage for this hook is to perform cleanup and
restart / reload services.

before_configure
Should be called before creating the configuration for a specific DHCP
instance, as defined in the dhcp.cfg. This can be used for performing
instance-specific setup.

after_configure
Should be called after the configuration for a specific DHCP instance,
as defined in the dhcp.cfg. This will mainly be used for enabling and
restarting / reloading a specific instance of a DHCP server.

configure_subnet
This function configures the settings for a specific subnet (that can
contain multiple DHCP ranges). This sets global settings for a
specific subnet such as DNS server or gateway.

configure_range
This configures a DHCP range that is available for a given Subnet.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/Network/SDN/Dhcp/Makefile  |  8 +++
 src/PVE/Network/SDN/Dhcp/Plugin.pm | 83 ++++++++++++++++++++++++++++++
 src/PVE/Network/SDN/Makefile       |  1 +
 3 files changed, 92 insertions(+)
 create mode 100644 src/PVE/Network/SDN/Dhcp/Makefile
 create mode 100644 src/PVE/Network/SDN/Dhcp/Plugin.pm

diff --git a/src/PVE/Network/SDN/Dhcp/Makefile b/src/PVE/Network/SDN/Dhcp/Makefile
new file mode 100644
index 0000000..1e9b6d3
--- /dev/null
+++ b/src/PVE/Network/SDN/Dhcp/Makefile
@@ -0,0 +1,8 @@
+SOURCES=Plugin.pm
+
+
+PERL5DIR=${DESTDIR}/usr/share/perl5
+
+.PHONY: install
+install:
+	for i in ${SOURCES}; do install -D -m 0644 $$i ${PERL5DIR}/PVE/Network/SDN/Dhcp/$$i; done
diff --git a/src/PVE/Network/SDN/Dhcp/Plugin.pm b/src/PVE/Network/SDN/Dhcp/Plugin.pm
new file mode 100644
index 0000000..75979e8
--- /dev/null
+++ b/src/PVE/Network/SDN/Dhcp/Plugin.pm
@@ -0,0 +1,83 @@
+package PVE::Network::SDN::Dhcp::Plugin;
+
+use strict;
+use warnings;
+
+use PVE::Cluster;
+use PVE::JSONSchema qw(get_standard_option);
+
+use base qw(PVE::SectionConfig);
+
+PVE::Cluster::cfs_register_file('sdn/dhcp.cfg',
+    sub { __PACKAGE__->parse_config(@_); },
+    sub { __PACKAGE__->write_config(@_); },
+);
+
+my $defaultData = {
+    propertyList => {
+	type => {
+	    description => "Plugin type.",
+	    format => 'pve-configid',
+	    type => 'string',
+	},
+	node => {
+	    type => 'array',
+	    description => 'A list of nodes where this DHCP server should be deployed',
+	    items => get_standard_option('pve-node'),
+	},
+    },
+};
+
+sub private {
+    return $defaultData;
+}
+
+sub options {
+    return {
+	node => {
+	    optional => 1,
+	},
+    };
+}
+
+sub add_ip_mapping {
+    my ($class, $dhcp_config, $mac, $ip) = @_;
+    die 'implement in sub class';
+}
+
+sub del_ip_mapping {
+    my ($class, $dhcp_config, $mac) = @_;
+    die 'implement in sub class';
+}
+
+sub configure_range {
+    my ($class, $dhcp_config, $subnet_config, $range_config) = @_;
+    die 'implement in sub class';
+}
+
+sub configure_subnet {
+    my ($class, $dhcp_config, $subnet_config) = @_;
+    die 'implement in sub class';
+}
+
+sub before_configure {
+    my ($class, $dhcp_config) = @_;
+    die 'implement in sub class';
+}
+
+sub after_configure {
+    my ($class, $dhcp_config) = @_;
+    die 'implement in sub class';
+}
+
+sub before_regenerate {
+    my ($class) = @_;
+    die 'implement in sub class';
+}
+
+sub after_regenerate {
+    my ($class, $dhcp_config) = @_;
+    die 'implement in sub class';
+}
+
+1;
diff --git a/src/PVE/Network/SDN/Makefile b/src/PVE/Network/SDN/Makefile
index 92cfcd0..848f7d4 100644
--- a/src/PVE/Network/SDN/Makefile
+++ b/src/PVE/Network/SDN/Makefile
@@ -10,4 +10,5 @@ install:
 	make -C Zones install
 	make -C Ipams install
 	make -C Dns install
+	make -C Dhcp install
 
-- 
2.39.2

[pve-devel] [WIP v2 pve-network 04/10] dhcp: subnet: add DHCP options to subnet configuration

[Stefan Hanreich]

Parse the dhcp-ranges when getting the configuration via the Subnet
class.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/Network/SDN/SubnetPlugin.pm | 32 +++++++++++++++++++++++++++++
 src/PVE/Network/SDN/Subnets.pm      | 18 ++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/src/PVE/Network/SDN/SubnetPlugin.pm b/src/PVE/Network/SDN/SubnetPlugin.pm
index 15b370f..47b8406 100644
--- a/src/PVE/Network/SDN/SubnetPlugin.pm
+++ b/src/PVE/Network/SDN/SubnetPlugin.pm
@@ -61,6 +61,23 @@ sub private {
     return $defaultData;
 }
 
+my $dhcp_range_fmt = {
+    server => {
+	type => 'pve-configid',
+	description => 'ID of the DHCP server responsible for managing this range',
+    },
+    'start-address' => {
+	type => 'ip',
+	description => 'Start address for the DHCP IP range',
+    },
+    'end-address' => {
+	type => 'ip',
+	description => 'End address for the DHCP IP range',
+    },
+};
+
+PVE::JSONSchema::register_format('pve-sdn-dhcp-range', $dhcp_range_fmt);
+
 sub properties {
     return {
         vnet => {
@@ -84,6 +101,19 @@ sub properties {
             type => 'string', format => 'dns-name',
             description => "dns domain zone prefix  ex: 'adm' -> <hostname>.adm.mydomain.com",
         },
+	'dhcp-range' => {
+	    type => 'array',
+	    description => 'A list of DHCP ranges for this subnet',
+	    items => {
+		type => 'string',
+		format => 'pve-sdn-dhcp-range',
+	    }
+	},
+	'dhcp-dns-server' => {
+	    type => 'ip',
+	    description => 'IP address for the DNS server',
+	    optional => 1,
+	},
     };
 }
 
@@ -94,6 +124,8 @@ sub options {
 #	routes => { optional => 1 },
 	snat => { optional => 1 },
 	dnszoneprefix => { optional => 1 },
+	'dhcp-range' => { optional => 1 },
+	'dhcp-dns-server' => { optional => 1 },
     };
 }
 
diff --git a/src/PVE/Network/SDN/Subnets.pm b/src/PVE/Network/SDN/Subnets.pm
index f654d3a..dd9e697 100644
--- a/src/PVE/Network/SDN/Subnets.pm
+++ b/src/PVE/Network/SDN/Subnets.pm
@@ -8,6 +8,7 @@ use Net::IP;
 use NetAddr::IP qw(:lower);
 
 use PVE::Cluster qw(cfs_read_file cfs_write_file cfs_lock_file);
+use PVE::JSONSchema qw(parse_property_string);
 use PVE::Network::SDN::Dns;
 use PVE::Network::SDN::Ipams;
 
@@ -31,6 +32,23 @@ sub sdn_subnets_config {
 	$scfg->{zone} = $zone;
 	$scfg->{network} = $network;
 	$scfg->{mask} = $mask;
+
+	if ($scfg->{'dhcp-range'}) {
+	    my @dhcp_ranges;
+
+	    foreach my $element (@{$scfg->{'dhcp-range'}}) {
+		my $dhcp_range = eval { parse_property_string('pve-sdn-dhcp-range', $element) };
+
+		if ($@ || !$dhcp_range) {
+		    warn "Unable to parse dhcp-range string: $element\n";
+		    warn "$@\n" if $@;
+		    next;
+		}
+
+		push @dhcp_ranges, $dhcp_range;
+	    }
+	    $scfg->{'dhcp-range'} = \@dhcp_ranges;
+	}
     }
 
     return $scfg;
-- 
2.39.2

[pve-devel] [WIP v2 pve-network 05/10] dhcp: add DHCP plugin for dnsmasq

[Stefan Hanreich]

The plugin generates several dnsmasq configuration files from the SDN
configuration.

/etc/default/dnsmasq.<dhcp_id>
This file specifies the configuration directory for the dnsmasq
instance (/etc/dnsmasq.d/<dhcp_id>). It also sets the configuration
file to /dev/null so the default configuration from the package has
no influence on the dnsmasq configuration.

/etc/dnsmasq.d/<dhcp_id>/00-default.conf
The default configration does several things:
* disable DNS functionality.
* make dnsmasq listen only on the interfaces where it should provide
  DHCP (contrary to the default configuration, which listens on all
  interfaces). This is particularly important when running multiple
  instances of dnsmasq.
* Set the lease file to /var/lib/misc/dnsmasq.<dhcp_id>.leases
* add some security-related settings
* add some platform-specific settings
* set dnsmasq to only hand out IP addresses to known hosts

/etc/dnsmasq.d/<dhcp_id>/10-<subnet_id>.conf
This file contains the subnet specific settings, which usually
includes:
* Listen address
* Default gateway (for IPv4)
* DNS server

/etc/dnsmasq.d/<dhcp_id>/10-<subnet_id>-ranges.conf
This file contains the DHCP ranges configured for the subnet
<subnet_id>.

/etc/dnsmasq.d/<dhcp_id>/ethers
This file contains the MAC address to IP mappings for all VMs and CTs
that should be used by dnsmasq DHCP. There is no need to split them on
a subnet level, since the MAC <-> IP mappings should always be unique.

Currently regenerating and reloading dnsmasq is very sledgehammery. It
deletes all existing subnet configuration files and disables + stops
all currently running dnsmasq instances. Then it enables + starts all
dnsmasq instances that have been recreated. I intend to improve this
behaviour in the future either by getting access to the old
configuration or using systemd targets.

Adding new MAC address mappings is done with a simple reload that does
not disrupt the dnsmasq daemon in any way.

This plugin currently only works for simple Zones with subnets that
have a gateway configured, since I use the gateway as listening
address for dnsmasq.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 debian/control                      |   1 +
 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 186 ++++++++++++++++++++++++++++
 src/PVE/Network/SDN/Dhcp/Makefile   |   2 +-
 3 files changed, 188 insertions(+), 1 deletion(-)
 create mode 100644 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm

diff --git a/debian/control b/debian/control
index 8b720c3..4424096 100644
--- a/debian/control
+++ b/debian/control
@@ -24,6 +24,7 @@ Depends: libpve-common-perl (>= 5.0-45),
          ${misc:Depends},
          ${perl:Depends},
 Recommends: frr-pythontools (>= 8.5.1~), ifupdown2
+Suggests: dnsmasq
 Description: Proxmox VE's SDN (Software Defined Network) stack
  This package contains the Software Defined Network (tech preview) for
  Proxmox VE.
diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
new file mode 100644
index 0000000..af109b8
--- /dev/null
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -0,0 +1,186 @@
+package PVE::Network::SDN::Dhcp::Dnsmasq;
+
+use strict;
+use warnings;
+
+use base qw(PVE::Network::SDN::Dhcp::Plugin);
+
+use Net::IP qw(:PROC);
+use PVE::Tools qw(file_set_contents run_command lock_file);
+
+use File::Copy;
+
+my $DNSMASQ_CONFIG_ROOT = '/etc/dnsmasq.d';
+my $DNSMASQ_DEFAULT_ROOT = '/etc/default';
+my $DNSMASQ_LEASE_ROOT = '/var/lib/misc';
+
+sub type {
+    return 'dnsmasq';
+}
+
+sub del_ip_mapping {
+    my ($class, $dhcp_config, $mac) = @_;
+
+    my $ethers_file = "$DNSMASQ_CONFIG_ROOT/$dhcp_config->{id}/ethers";
+    my $ethers_tmp_file = "$ethers_file.tmp";
+
+    my $removeFn = sub {
+	open(my $in, '<', $ethers_file) or die "Could not open file '$ethers_file' $!\n";
+	open(my $out, '>', $ethers_tmp_file) or die "Could not open file '$ethers_tmp_file' $!\n";
+
+        while (my $line = <$in>) {
+	    next if $line =~ m/^$mac/;
+	    print $out $line;
+	}
+
+	close $in;
+	close $out;
+
+	move $ethers_tmp_file, $ethers_file;
+
+	chmod 0644, $ethers_file;
+    };
+
+    PVE::Tools::lock_file($ethers_file, 10, $removeFn);
+
+    if ($@) {
+	warn "Unable to remove $mac from the dnsmasq configuration: $@\n";
+	return;
+    }
+
+    my $service_name = "dnsmasq\@$dhcp_config->{id}";
+    PVE::Tools::run_command(['systemctl', 'reload', $service_name]);
+}
+
+sub add_ip_mapping {
+    my ($class, $dhcp_config, $mac, $ip) = @_;
+    my $ethers_file = "$DNSMASQ_CONFIG_ROOT/$dhcp_config->{id}/ethers";
+
+    my $appendFn = sub {
+	open(my $fh, '>>', $ethers_file) or die "Could not open file '$ethers_file' $!\n";
+	print $fh "$mac,$ip\n";
+	close $fh;
+    };
+
+    PVE::Tools::lock_file($ethers_file, 10, $appendFn);
+
+    if ($@) {
+	warn "Unable to add $mac/$ip to the dnsmasq configuration: $@\n";
+	return;
+    }
+
+    my $service_name = "dnsmasq\@$dhcp_config->{id}";
+    PVE::Tools::run_command(['systemctl', 'reload', $service_name]);
+}
+
+sub configure_subnet {
+    my ($class, $dhcp_config, $subnet_config) = @_;
+
+    die "No gateway defined for subnet $subnet_config->{id}"
+	if !$subnet_config->{gateway};
+
+    my $tag = $subnet_config->{id};
+
+    my @dnsmasq_config = (
+	"listen-address=$subnet_config->{gateway}",
+    );
+
+    my $option_string;
+    if (ip_is_ipv6($subnet_config->{network})) {
+	$option_string = 'option6';
+	push @dnsmasq_config, "enable-ra";
+    } else {
+	$option_string = 'option';
+	push @dnsmasq_config, "dhcp-option=tag:$tag,$option_string:router,$subnet_config->{gateway}";
+    }
+
+    push @dnsmasq_config, "dhcp-option=tag:$tag,$option_string:dns-server,$subnet_config->{'dhcp-dns-server'}"
+	if $subnet_config->{'dhcp-dns-server'};
+
+    PVE::Tools::file_set_contents(
+	"$DNSMASQ_CONFIG_ROOT/$dhcp_config->{id}/10-$subnet_config->{id}.conf",
+	join("\n", @dnsmasq_config) . "\n"
+    );
+}
+
+sub configure_range {
+    my ($class, $dhcp_config, $subnet_config, $range_config) = @_;
+
+    my $range_file = "$DNSMASQ_CONFIG_ROOT/$dhcp_config->{id}/10-$subnet_config->{id}.ranges.conf",
+    my $tag = $subnet_config->{id};
+
+    open(my $fh, '>>', $range_file) or die "Could not open file '$range_file' $!\n";
+    print $fh "dhcp-range=set:$tag,$range_config->{'start-address'},$range_config->{'end-address'}\n";
+    close $fh;
+}
+
+sub before_configure {
+    my ($class, $dhcp_config) = @_;
+
+    my $config_directory = "$DNSMASQ_CONFIG_ROOT/$dhcp_config->{id}";
+
+    mkdir($config_directory, 755) if !-d $config_directory;
+
+    my $default_config = <<CFG;
+CONFIG_DIR='$config_directory,\*.conf'
+DNSMASQ_OPTS="--conf-file=/dev/null"
+CFG
+
+    PVE::Tools::file_set_contents(
+	"$DNSMASQ_DEFAULT_ROOT/dnsmasq.$dhcp_config->{id}",
+	$default_config
+    );
+
+    my $default_dnsmasq_config = <<CFG;
+except-interface=lo
+bind-dynamic
+no-resolv
+no-hosts
+dhcp-leasefile=$DNSMASQ_LEASE_ROOT/dnsmasq.$dhcp_config->{id}.leases
+dhcp-hostsfile=$config_directory/ethers
+dhcp-ignore=tag:!known
+
+# Send an empty WPAD option. This may be REQUIRED to get windows 7 to behave.
+dhcp-option=252,"\\n"
+
+# Send microsoft-specific option to tell windows to release the DHCP lease
+# when it shuts down. Note the "i" flag, to tell dnsmasq to send the
+# value as a four-byte integer - that's what microsoft wants.
+dhcp-option=vendor:MSFT,2,1i
+
+# If a DHCP client claims that its name is "wpad", ignore that.
+# This fixes a security hole. see CERT Vulnerability VU#598349
+dhcp-name-match=set:wpad-ignore,wpad
+dhcp-ignore-names=tag:wpad-ignore
+CFG
+
+    PVE::Tools::file_set_contents(
+	"$config_directory/00-default.conf",
+	$default_dnsmasq_config
+    );
+
+    unlink glob "$config_directory/10-*.conf";
+}
+
+sub after_configure {
+    my ($class, $dhcp_config) = @_;
+
+    my $service_name = "dnsmasq\@$dhcp_config->{id}";
+
+    PVE::Tools::run_command(['systemctl', 'enable', $service_name]);
+    PVE::Tools::run_command(['systemctl', 'restart', $service_name]);
+}
+
+sub before_regenerate {
+    my ($class) = @_;
+
+    PVE::Tools::run_command(['systemctl', 'stop', "dnsmasq@*"]);
+    PVE::Tools::run_command(['systemctl', 'disable', 'dnsmasq@']);
+}
+
+sub after_regenerate {
+    my ($class) = @_;
+    # noop
+}
+
+1;
diff --git a/src/PVE/Network/SDN/Dhcp/Makefile b/src/PVE/Network/SDN/Dhcp/Makefile
index 1e9b6d3..6546513 100644
--- a/src/PVE/Network/SDN/Dhcp/Makefile
+++ b/src/PVE/Network/SDN/Dhcp/Makefile
@@ -1,4 +1,4 @@
-SOURCES=Plugin.pm
+SOURCES=Plugin.pm Dnsmasq.pm
 
 
 PERL5DIR=${DESTDIR}/usr/share/perl5
-- 
2.39.2

[pve-devel] [WIP v2 pve-network 06/10] ipam: Add helper methods for DHCP to PVE IPAM

[Stefan Hanreich]

Those methods are used by the DHCP plugins to attain the next free
IP address for a given DHCP range, as well as delete all entries with
a certain MAC address.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/Network/SDN/Ipams/PVEPlugin.pm | 64 ++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
index 3e8ffc5..fcc8282 100644
--- a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
@@ -156,6 +156,70 @@ sub add_next_freeip {
     return "$freeip/$mask";
 }
 
+sub add_dhcp_ip {
+    my ($class, $subnet, $dhcp_range, $data) = @_;
+
+    my $cidr = $subnet->{cidr};
+    my $zone = $subnet->{zone};
+
+    cfs_lock_file($ipamdb_file, undef, sub {
+	my $db = read_db();
+
+	my $dbzone = $db->{zones}->{$zone};
+	die "zone '$zone' doesn't exist in IPAM DB\n" if !$dbzone;
+
+	my $dbsubnet = $dbzone->{subnets}->{$cidr};
+	die "subnet '$cidr' doesn't exist in IPAM DB\n" if !$dbsubnet;
+
+	my $ip = new Net::IP ("$dhcp_range->{'start-address'} - $dhcp_range->{'end-address'}")
+	    or die "Invalid IP address(es) in DHCP Range!\n";
+
+	do {
+	    my $ip_address = $ip->ip();
+	    if (!$dbsubnet->{ips}->{$ip_address}) {
+		$dbsubnet->{ips}->{$ip_address} = $data;
+		write_db($db);
+
+		return $ip_address;
+	    }
+	} while (++$ip);
+
+	die "No free IP left in DHCP Range $dhcp_range->{'start-address'}:$dhcp_range->{'end-address'}}\n";
+    });
+}
+
+sub del_dhcp_ip {
+    my ($class, $subnet, $mac) = @_;
+
+    my $cidr = $subnet->{cidr};
+    my $zone = $subnet->{zone};
+
+    my $returned_ip = undef;
+
+    cfs_lock_file($ipamdb_file, undef, sub {
+	my $db = read_db();
+
+	die "zone $zone don't exist in ipam db" if !$db->{zones}->{$zone};
+	my $dbzone = $db->{zones}->{$zone};
+
+	die "subnet $cidr don't exist in ipam db" if !$dbzone->{subnets}->{$cidr};
+	my $dbsubnet = $dbzone->{subnets}->{$cidr};
+
+	foreach my $ip_address (keys %{$dbsubnet->{ips}}) {
+	    my $data = $dbsubnet->{ips}->{$ip_address};
+	    next if !$data->{mac} || $data->{mac} ne $mac;
+
+	    delete $dbsubnet->{ips}->{$ip_address};
+	    write_db($db);
+
+	    $returned_ip = $ip_address;
+	}
+    });
+    die "$@" if $@;
+
+    return $returned_ip;
+}
+
 sub del_ip {
     my ($class, $plugin_config, $subnetid, $subnet, $ip) = @_;
 
-- 
2.39.2

[pve-devel] [WIP v2 pve-network 07/10] dhcp: regenerate config for DHCP servers on reload

[Stefan Hanreich]

Regenerate the configuration files for the different DHCP server
plugins when applying SDN settings by calling the respective hooks of
the plugin responsible for configuring a DHCP instance.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/Network/SDN.pm       |  11 +-
 src/PVE/Network/SDN/Dhcp.pm  | 192 +++++++++++++++++++++++++++++++++++
 src/PVE/Network/SDN/Makefile |   2 +-
 3 files changed, 203 insertions(+), 2 deletions(-)
 create mode 100644 src/PVE/Network/SDN/Dhcp.pm

diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm
index 057034f..5c059bc 100644
--- a/src/PVE/Network/SDN.pm
+++ b/src/PVE/Network/SDN.pm
@@ -12,6 +12,7 @@ use PVE::Network::SDN::Vnets;
 use PVE::Network::SDN::Zones;
 use PVE::Network::SDN::Controllers;
 use PVE::Network::SDN::Subnets;
+use PVE::Network::SDN::Dhcp;
 
 use PVE::Tools qw(extract_param dir_glob_regex run_command);
 use PVE::Cluster qw(cfs_read_file cfs_write_file cfs_lock_file);
@@ -149,13 +150,15 @@ sub commit_config {
     my $zones_cfg = PVE::Network::SDN::Zones::config();
     my $controllers_cfg = PVE::Network::SDN::Controllers::config();
     my $subnets_cfg = PVE::Network::SDN::Subnets::config();
+    my $dhcp_cfg = PVE::Network::SDN::Dhcp::config();
 
     my $vnets = { ids => $vnets_cfg->{ids} };
     my $zones = { ids => $zones_cfg->{ids} };
     my $controllers = { ids => $controllers_cfg->{ids} };
     my $subnets = { ids => $subnets_cfg->{ids} };
+    my $dhcp = { ids => $dhcp_cfg->{ids} };
 
-     $cfg = { version => $version, vnets => $vnets, zones => $zones, controllers => $controllers, subnets => $subnets };
+    $cfg = { version => $version, vnets => $vnets, zones => $zones, controllers => $controllers, subnets => $subnets, dhcps => $dhcp };
 
     cfs_write_file($running_cfg, $cfg);
 }
@@ -231,6 +234,12 @@ sub generate_controller_config {
     PVE::Network::SDN::Controllers::reload_controller() if $reload;
 }
 
+sub generate_dhcp_config {
+    my ($reload) = @_;
+
+    PVE::Network::SDN::Dhcp::regenerate_config($reload);
+}
+
 sub encode_value {
     my ($type, $key, $value) = @_;
 
diff --git a/src/PVE/Network/SDN/Dhcp.pm b/src/PVE/Network/SDN/Dhcp.pm
new file mode 100644
index 0000000..b92c73a
--- /dev/null
+++ b/src/PVE/Network/SDN/Dhcp.pm
@@ -0,0 +1,192 @@
+package PVE::Network::SDN::Dhcp;
+
+use strict;
+use warnings;
+
+use PVE::Cluster qw(cfs_read_file);
+
+use PVE::Network::SDN;
+use PVE::Network::SDN::Ipams::Plugin;
+use PVE::Network::SDN::SubnetPlugin;
+use PVE::Network::SDN::Dhcp qw(config);
+use PVE::Network::SDN::Subnets qw(sdn_subnets_config config);
+use PVE::Network::SDN::Dhcp::Plugin;
+use PVE::Network::SDN::Dhcp::Dnsmasq;
+
+use PVE::INotify qw(nodename);
+
+PVE::Network::SDN::Dhcp::Plugin->init();
+
+PVE::Network::SDN::Dhcp::Dnsmasq->register();
+PVE::Network::SDN::Dhcp::Dnsmasq->init();
+
+sub config {
+    my ($running) = @_;
+
+    if ($running) {
+	my $cfg = PVE::Network::SDN::running_config();
+	return $cfg->{dhcps};
+    }
+
+    return cfs_read_file('sdn/dhcp.cfg');
+}
+
+sub sdn_dhcps_config {
+    my ($cfg, $id, $noerr) = @_;
+
+    die "No DHCP ID specified!\n" if !$id;
+
+    my $dhcp_config = $cfg->{ids}->{$id};
+    die "SDN DHCP '$id' does not exist!\n" if (!$noerr && !$dhcp_config);
+
+    if ($dhcp_config) {
+	$dhcp_config->{id} = $id;
+    }
+
+    return $dhcp_config;
+}
+
+sub get_dhcp {
+    my ($dhcp_id, $running) = @_;
+
+    return if !$dhcp_id;
+
+    my $cfg = PVE::Network::SDN::Dhcp::config($running);
+    return PVE::Network::SDN::Dhcp::sdn_dhcps_config($cfg, $dhcp_id, 1);
+}
+
+sub add_mapping {
+    my ($vmid, $vnet, $mac) = @_;
+
+    my $vnet_config = PVE::Network::SDN::Vnets::get_vnet($vnet, 1);
+    return if !$vnet_config;
+
+    my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnet, 1);
+
+    for my $subnet_id (keys %{$subnets}) {
+	my $subnet_config = $subnets->{$subnet_id};
+
+	next if !$subnet_config->{'dhcp-range'};
+
+	foreach my $dhcp_range (@{$subnet_config->{'dhcp-range'}}) {
+	    my $dhcp_config = PVE::Network::SDN::Dhcp::get_dhcp($dhcp_range->{server});
+
+	    if (!$dhcp_config) {
+		warn "Cannot find configuration for DHCP server $dhcp_range->{server}";
+		next;
+	    }
+
+	    my $ipam_plugin = PVE::Network::SDN::Ipams::Plugin->lookup('pve');
+
+	    my $data = {
+		vmid => $vmid,
+		mac => $mac,
+	    };
+
+	    my $ip = $ipam_plugin->add_dhcp_ip($subnet_config, $dhcp_range, $data);
+
+	    next if !$ip;
+
+	    my $dhcp_plugin = PVE::Network::SDN::Dhcp::Plugin->lookup($dhcp_config->{type});
+	    $dhcp_plugin->add_ip_mapping($dhcp_config, $mac, $ip);
+
+	    return $ip;
+	}
+    }
+}
+
+sub remove_mapping {
+    my ($vnet, $mac) = @_;
+
+    my $vnet_config = PVE::Network::SDN::Vnets::get_vnet($vnet, 1);
+    return if !$vnet_config;
+
+    my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnet, 1);
+
+    for my $subnet_id (keys %{$subnets}) {
+	my $subnet_config = $subnets->{$subnet_id};
+	next if !$subnet_config->{'dhcp-range'};
+
+	my $ipam_plugin = PVE::Network::SDN::Ipams::Plugin->lookup('pve');
+	$ipam_plugin->del_dhcp_ip($subnet_config, $mac);
+
+	foreach my $dhcp_range (@{$subnet_config->{'dhcp-range'}}) {
+	    my $dhcp_config = PVE::Network::SDN::Dhcp::get_dhcp($dhcp_range->{server});
+
+	    if (!$dhcp_config) {
+		warn "Cannot find configuration for DHCP server $dhcp_range->{server}";
+		next;
+	    }
+
+	    my $dhcp_plugin = PVE::Network::SDN::Dhcp::Plugin->lookup($dhcp_config->{type});
+	    $dhcp_plugin->del_ip_mapping($dhcp_config, $mac);
+	}
+    }
+}
+
+sub regenerate_config {
+    my ($reload) = @_;
+
+    my $dhcps = PVE::Network::SDN::Dhcp::config();
+    my $subnets = PVE::Network::SDN::Subnets::config();
+
+    my $plugins = PVE::Network::SDN::Dhcp::Plugin->lookup_types();
+
+    my $nodename = PVE::INotify::nodename();
+
+    foreach my $plugin_name (@$plugins) {
+	my $plugin = PVE::Network::SDN::Dhcp::Plugin->lookup($plugin_name);
+
+	eval { $plugin->before_regenerate() };
+	die "Could not run before_regenerate for DHCP plugin $plugin_name $@\n" if $@;
+    }
+
+    foreach my $dhcp_id (keys %{$dhcps->{ids}}) {
+	my $dhcp_config = PVE::Network::SDN::Dhcp::sdn_dhcps_config($dhcps, $dhcp_id);
+	my $plugin = PVE::Network::SDN::Dhcp::Plugin->lookup($dhcp_config->{type});
+
+	eval { $plugin->before_configure($dhcp_config) };
+	die "Could not run before_configure for DHCP server $dhcp_id $@\n" if $@;
+    }
+
+    foreach my $subnet_id (keys %{$subnets->{ids}}) {
+	my $subnet_config = PVE::Network::SDN::Subnets::sdn_subnets_config($subnets, $subnet_id);
+	next if !$subnet_config->{'dhcp-range'};
+
+	my @configured_servers = ();
+
+	foreach my $dhcp_range (@{$subnet_config->{'dhcp-range'}}) {
+	    my $dhcp_config = PVE::Network::SDN::Dhcp::sdn_dhcps_config($dhcps, $dhcp_range->{server});
+	    my $plugin = PVE::Network::SDN::Dhcp::Plugin->lookup($dhcp_config->{type});
+
+	    next if $dhcp_config->{node} && !grep(/^$nodename$/, @{$dhcp_config->{node}});
+
+	    if (!grep(/^$subnet_id$/, @configured_servers)) {
+		eval { $plugin->configure_subnet($dhcp_config, $subnet_config) };
+		warn "Could not configure Subnet $subnet_id: $@\n" if $@;
+
+		push @configured_servers, $subnet_id;
+	    }
+
+	    eval { $plugin->configure_range($dhcp_config, $subnet_config, $dhcp_range) };
+	    warn "Could not configure DHCP range for $subnet_id: $@\n" if $@;
+	}
+    }
+
+    foreach my $dhcp_id (keys %{$dhcps->{ids}}) {
+	my $dhcp_config = PVE::Network::SDN::Dhcp::sdn_dhcps_config($dhcps, $dhcp_id);
+	my $plugin = PVE::Network::SDN::Dhcp::Plugin->lookup($dhcp_config->{type});
+
+	eval { $plugin->after_configure($dhcp_config) };
+	warn "Could not run after_configure for DHCP server $dhcp_id $@\n" if $@;
+    }
+
+    foreach my $plugin_name (@$plugins) {
+	my $plugin = PVE::Network::SDN::Dhcp::Plugin->lookup($plugin_name);
+
+	eval { $plugin->after_regenerate() };
+	warn "Could not run after_regenerate for DHCP plugin $plugin_name $@\n" if $@;
+    }
+}
+
+1;
diff --git a/src/PVE/Network/SDN/Makefile b/src/PVE/Network/SDN/Makefile
index 848f7d4..3e6e5fb 100644
--- a/src/PVE/Network/SDN/Makefile
+++ b/src/PVE/Network/SDN/Makefile
@@ -1,4 +1,4 @@
-SOURCES=Vnets.pm VnetPlugin.pm Zones.pm Controllers.pm Subnets.pm SubnetPlugin.pm Ipams.pm Dns.pm
+SOURCES=Vnets.pm VnetPlugin.pm Zones.pm Controllers.pm Subnets.pm SubnetPlugin.pm Ipams.pm Dns.pm Dhcp.pm
 
 
 PERL5DIR=${DESTDIR}/usr/share/perl5
-- 
2.39.2

[pve-devel] [WIP v2 pve-manager 08/10] sdn: regenerate DHCP config on reload

[Stefan Hanreich]

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 PVE/API2/Network.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/PVE/API2/Network.pm b/PVE/API2/Network.pm
index 00d964a79..f39f04f52 100644
--- a/PVE/API2/Network.pm
+++ b/PVE/API2/Network.pm
@@ -660,6 +660,7 @@ __PACKAGE__->register_method({
 
 	    if ($have_sdn) {
 		PVE::Network::SDN::generate_zone_config();
+		PVE::Network::SDN::generate_dhcp_config();
 	    }
 
 	    my $err = sub {
-- 
2.39.2

[pve-devel] [WIP v2 qemu-server 09/10] sdn: dhcp: add DHCP setup to vm-network-scripts

[Stefan Hanreich]

When setting up the bridge for the VMs, also set up the DHCP mappings
in the respective DHCP plugins if the VM has interfaces on SDN
networks that utilize DHCP.

Also remove the mapping in the VM cleanup function, so the
mappings also get removed when stopping the VM forcefully.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 PVE/QemuServer.pm                 | 14 ++++++++++++++
 vm-network-scripts/pve-bridge     |  3 +++
 vm-network-scripts/pve-bridgedown | 19 +++++++++++++++++++
 3 files changed, 36 insertions(+)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 2cd8948..6c1e463 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -6098,6 +6098,18 @@ sub cleanup_pci_devices {
     PVE::QemuServer::PCI::remove_pci_reservation($vmid);
 }
 
+sub cleanup_sdn_dhcp {
+    my ($vmid, $conf) = @_;
+
+    for my $k (keys %$conf) {
+	next if $k !~ /^net(\d+)/;
+	my $netconf = $conf->{$k};
+	my $net = PVE::QemuServer::parse_net($netconf);
+
+	PVE::Network::SDN::Dhcp::remove_mapping($net->{bridge}, $net->{macaddr});
+    }
+}
+
 sub vm_stop_cleanup {
     my ($storecfg, $vmid, $conf, $keepActive, $apply_pending_changes) = @_;
 
@@ -6131,6 +6143,8 @@ sub vm_stop_cleanup {
 
 	cleanup_pci_devices($vmid, $conf);
 
+	cleanup_sdn_dhcp($vmid, $conf);
+
 	vmconfig_apply_pending($vmid, $conf, $storecfg) if $apply_pending_changes;
     };
     warn $@ if $@; # avoid errors - just warn
diff --git a/vm-network-scripts/pve-bridge b/vm-network-scripts/pve-bridge
index d37ce33..5c8acdf 100755
--- a/vm-network-scripts/pve-bridge
+++ b/vm-network-scripts/pve-bridge
@@ -10,6 +10,7 @@ use PVE::Network;
 my $have_sdn;
 eval {
     require PVE::Network::SDN::Zones;
+    require PVE::Network::SDN::Dhcp;
     $have_sdn = 1;
 };
 
@@ -44,6 +45,8 @@ my $net = PVE::QemuServer::parse_net($netconf);
 die "unable to parse network config '$netid'\n" if !$net;
 
 if ($have_sdn) {
+    PVE::Network::SDN::Dhcp::add_mapping($vmid, $net->{bridge}, $net->{macaddr});
+
     PVE::Network::SDN::Zones::tap_create($iface, $net->{bridge});
     PVE::Network::SDN::Zones::tap_plug($iface, $net->{bridge}, $net->{tag}, $net->{firewall}, $net->{trunks}, $net->{rate});
 } else {
diff --git a/vm-network-scripts/pve-bridgedown b/vm-network-scripts/pve-bridgedown
index d18d88f..a220660 100755
--- a/vm-network-scripts/pve-bridgedown
+++ b/vm-network-scripts/pve-bridgedown
@@ -4,6 +4,13 @@ use strict;
 use warnings;
 use PVE::Network;
 
+my $have_sdn;
+eval {
+    require PVE::Network::SDN::Zones;
+    require PVE::Network::SDN::Dhcp;
+    $have_sdn = 1;
+};
+
 my $iface = shift;
 
 die "no interface specified\n" if !$iface;
@@ -11,6 +18,18 @@ die "no interface specified\n" if !$iface;
 die "got strange interface name '$iface'\n" 
     if $iface !~ m/^tap(\d+)i(\d+)$/;
 
+my $vmid = $1;
+my $netid = "net$2";
+
+my $conf = PVE::QemuConfig->load_config($vmid);
+
+my $netconf = $conf->{$netid};
+my $net = PVE::QemuServer::parse_net($netconf);
+
+if ($have_sdn) {
+    PVE::Network::SDN::Dhcp::remove_mapping($net->{bridge}, $net->{macaddr});
+}
+
 PVE::Network::tap_unplug($iface);
 
 exit 0;
-- 
2.39.2

[pve-devel] [WIP v2 pve-container 10/10] sdn: dhcp: setup DHCP mappings in LXC hooks

[Stefan Hanreich]

Setup DHCP mappings if a container has interfaces on a SDN network
managed via DHCP.

Additionally remove the mapping in the stop_cleanup function so the
mapping gets removed when forcefully stopping the container.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/LXC.pm            | 10 ++++++++++
 src/lxc-pve-poststop-hook |  1 +
 src/lxc-pve-prestart-hook |  9 +++++++++
 3 files changed, 20 insertions(+)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index c9b5ba7..bd8eb63 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -916,6 +916,14 @@ sub vm_stop_cleanup {
     eval {
 	my $vollist = PVE::LXC::Config->get_vm_volumes($conf);
 	PVE::Storage::deactivate_volumes($storage_cfg, $vollist);
+
+	for my $k (keys %$conf) {
+	    next if $k !~ /^net(\d+)/;
+	    my $net = PVE::LXC::Config->parse_lxc_network($conf->{$k});
+	    next if $net->{type} ne 'veth';
+
+	    PVE::Network::SDN::Dhcp::remove_mapping($net->{bridge}, $net->{hwaddr});
+	}
     };
     warn $@ if $@; # avoid errors - just warn
 }
@@ -1012,6 +1020,8 @@ sub hotplug_net {
     my $eth = $newnet->{name};
 
     if ($have_sdn) {
+	PVE::Network::SDN::Dhcp::add_mapping($vmid, $newnet->{bridge}, $newnet->{macaddr});
+
 	PVE::Network::SDN::Zones::veth_create($veth, $vethpeer, $newnet->{bridge}, $newnet->{hwaddr});
     } else {
 	PVE::Network::veth_create($veth, $vethpeer, $newnet->{bridge}, $newnet->{hwaddr});
diff --git a/src/lxc-pve-poststop-hook b/src/lxc-pve-poststop-hook
index 2fe97ec..e7d46c7 100755
--- a/src/lxc-pve-poststop-hook
+++ b/src/lxc-pve-poststop-hook
@@ -12,6 +12,7 @@ use PVE::LXC::Config;
 use PVE::LXC::Tools;
 use PVE::LXC;
 use PVE::Network;
+use PVE::Network::SDN::Dhcp;
 use PVE::RESTEnvironment;
 use PVE::Storage;
 use PVE::Tools;
diff --git a/src/lxc-pve-prestart-hook b/src/lxc-pve-prestart-hook
index 936d0bf..5b2484e 100755
--- a/src/lxc-pve-prestart-hook
+++ b/src/lxc-pve-prestart-hook
@@ -15,6 +15,7 @@ use PVE::LXC::Config;
 use PVE::LXC::Setup;
 use PVE::LXC::Tools;
 use PVE::LXC;
+use PVE::Network::SDN::Dhcp;
 use PVE::RESTEnvironment;
 use PVE::SafeSyslog;
 use PVE::Storage;
@@ -140,6 +141,14 @@ PVE::LXC::Tools::lxc_hook('pre-start', 'lxc', sub {
 	}
 	PVE::Tools::file_set_contents($devlist_file, $devlist);
     }
+
+    for my $k (keys %$conf) {
+	next if $k !~ /^net(\d+)/;
+	my $net = PVE::LXC::Config->parse_lxc_network($conf->{$k});
+	next if $net->{type} ne 'veth';
+
+	PVE::Network::SDN::Dhcp::add_mapping($vmid, $net->{bridge}, $net->{hwaddr});
+    }
 });
 
 # Leftover cgroups prevent lxc from starting without any useful information
-- 
2.39.2

Re: [pve-devel] [WIP v2 cluster/network/manager/qemu-server/container 00/10] Add support for DHCP servers to SDN

[DERUMIER, Alexandre]

Hi Stefan,

Thanks for sharing !

I'll try to deeply test it this week or next week.

Maybe try to see if we can use pve ipam as cache in front of external
ipam.



-------- Message initial --------
De: Stefan Hanreich <s.hanreich@proxmox.com>
Répondre à: Proxmox VE development discussion <pve-
devel@lists.proxmox.com>
À: pve-devel@lists.proxmox.com
Objet: [pve-devel] [WIP v2 cluster/network/manager/qemu-
server/container 00/10] Add support for DHCP servers to SDN
Date: 17/10/2023 15:54:57

This is a WIP patch series, since I will be gone for 3 weeks and wanted
to
share my current progress with the DHCP support for SDN.

This patch series adds support for automatically deploying dnsmasq as a
DHCP
server to a simple SDN Zone.

While certainly not 100% polished on some ends (looking at restarting
systemd
services in particular), the general idea behind the mechanism shows. I
wanted
to gather some feedback on how I approached designing the plugins and
the
config regeneration process before comitting to this design by creating
an API
and UI around it.

You need to install dnsmasq (and disable it afterwards):

  apt install dnsmasq && systemctl disable --now dnsmasq


You can use the following example configuration for deploying a DHCP
server in
a SDN subnet:

/etc/pve/sdn/dhcp.cfg:

  dnsmasq: nat


/etc/pve/sdn/zones.cfg:

  simple: DHCPNAT
          ipam pve


/etc/pve/sdn/vnets.cfg:

  vnet: dhcpnat
          zone DHCPNAT


/etc/pve/sdn/subnets.cfg:

  subnet: DHCPNAT-10.1.0.0-16
          vnet dhcpnat
          dhcp-dns-server 10.1.0.1
          dhcp-range server=nat,start-address=10.1.0.100,end-
address=10.1.0.200
          gateway 10.1.0.1
          snat 1


Then apply the SDN configuration:

  pvesh set /cluster/sdn

You need to apply the SDN configuration once after adding the dhcp-
range lines
to the configuration, since the running configuration is used for
managing
DHCP. It will not work otherwise!

For testing it can be helpful to monitor the following files (e.g. with
watch)
to find out what is happening
  * /etc/dnsmasq.d/<dhcp_id>/ethers (on each node)
  * /etc/pve/priv/ipam.db

Changes from v1 -> v2:
  * added hooks for handling DHCP when starting / stopping / .. VMs and
CTs
  * Get an IP from IPAM and register that IP in the DHCP server
    (pve only for now)
  * remove lease-time, since it is now infinite and managed by the VM
lifecycle
  * add hooks for setting & deleting DHCP mappings to DHCP plugins
  * modified interface of the abstract class to reflect new
requirements
  * added helpers in existing SDN classes
  * simplified DHCP configuration settings



pve-cluster:

Stefan Hanreich (1):
  cluster files: add dhcp.cfg

 src/PVE/Cluster.pm  | 1 +
 src/pmxcfs/status.c | 1 +
 2 files changed, 2 insertions(+)


pve-network:

Stefan Hanreich (6):
  subnets: vnets: preparations for DHCP plugins
  dhcp: add abstract class for DHCP plugins
  dhcp: subnet: add DHCP options to subnet configuration
  dhcp: add DHCP plugin for dnsmasq
  ipam: Add helper methods for DHCP to PVE IPAM
  dhcp: regenerate config for DHCP servers on reload

 debian/control                         |   1 +
 src/PVE/Network/SDN.pm                 |  11 +-
 src/PVE/Network/SDN/Dhcp.pm            | 192 +++++++++++++++++++++++++
 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm    | 186 ++++++++++++++++++++++++
 src/PVE/Network/SDN/Dhcp/Makefile      |   8 ++
 src/PVE/Network/SDN/Dhcp/Plugin.pm     |  83 +++++++++++
 src/PVE/Network/SDN/Ipams/PVEPlugin.pm |  64 +++++++++
 src/PVE/Network/SDN/Makefile           |   3 +-
 src/PVE/Network/SDN/SubnetPlugin.pm    |  32 +++++
 src/PVE/Network/SDN/Subnets.pm         |  43 ++++--
 src/PVE/Network/SDN/Vnets.pm           |  27 ++--
 11 files changed, 622 insertions(+), 28 deletions(-)
 create mode 100644 src/PVE/Network/SDN/Dhcp.pm
 create mode 100644 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
 create mode 100644 src/PVE/Network/SDN/Dhcp/Makefile
 create mode 100644 src/PVE/Network/SDN/Dhcp/Plugin.pm


pve-manager:

Stefan Hanreich (1):
  sdn: regenerate DHCP config on reload

 PVE/API2/Network.pm | 1 +
 1 file changed, 1 insertion(+)


qemu-server:

Stefan Hanreich (1):
  sdn: dhcp: add DHCP setup to vm-network-scripts

 PVE/QemuServer.pm                 | 14 ++++++++++++++
 vm-network-scripts/pve-bridge     |  3 +++
 vm-network-scripts/pve-bridgedown | 19 +++++++++++++++++++
 3 files changed, 36 insertions(+)


pve-container:

Stefan Hanreich (1):
  sdn: dhcp: setup DHCP mappings in LXC hooks

 src/PVE/LXC.pm            | 10 ++++++++++
 src/lxc-pve-poststop-hook |  1 +
 src/lxc-pve-prestart-hook |  9 +++++++++
 3 files changed, 20 insertions(+)


Summary over all repositories:
  20 files changed, 681 insertions(+), 28 deletions(-)

Re: [pve-devel] [WIP v2 cluster/network/manager/qemu-server/container 00/10] Add support for DHCP servers to SDN

[Stefan Hanreich]

Some additional things we've discussed off-list:

Currently for VMs Migration & Hibernation are not working - everything
else in the lifecycle of VMs/CTs should be covered.



For Migration:
It currently creates an additional mapping in the IPAM and doesn't
delete the existing mapping from the DHCP server config.
I'd say we change this to upsert instead of add (for both IPAM and DHCP
Plugins). This means when adding a mapping for a VM, check if there is
already one, and then return that mapping without doing anything.

Then we just need to delete the existing mapping from the source during
the migration, which could simply be done somewhere in QemuMigrate.

Having upsert, instead of add, would also make implementations with
distributed DHCP servers like kea easier.


For Hibernation:
Having Upsert would solve the issue with Hibernation as well. Here we
need to make sure to not delete the entry from the IPAM, since we use a
memory snapshot rather than using the guest's hibernation feature. That
means the DHCP lease 'persists' in the VM.



We will also need to expose the functionality via the Web UI, for that I
had the following things in mind:

* Add Create/Edit/Delete DHCP to either `Options` or in a new IPAM/DHCP
panel (see below)
* Add a tree view of the current PVE IPAM state, similar to the resource
mapping, as a new panel
* add the `dhcp-range` fields to the Subnet Edit Dialog (possibly in a
new tab in the edit dialogue)



Another thing: What happens when a user changes the MAC address via the
UI? I'd either disallow it completely or we need to update the DHCP
configuration files and IPAM


On 10/17/23 15:54, Stefan Hanreich wrote:
> This is a WIP patch series, since I will be gone for 3 weeks and wanted to
> share my current progress with the DHCP support for SDN.
> 
> This patch series adds support for automatically deploying dnsmasq as a DHCP
> server to a simple SDN Zone.
> 
> While certainly not 100% polished on some ends (looking at restarting systemd
> services in particular), the general idea behind the mechanism shows. I wanted
> to gather some feedback on how I approached designing the plugins and the
> config regeneration process before comitting to this design by creating an API
> and UI around it.
> 
> You need to install dnsmasq (and disable it afterwards):
> 
>   apt install dnsmasq && systemctl disable --now dnsmasq
> 
> 
> You can use the following example configuration for deploying a DHCP server in
> a SDN subnet:
> 
> /etc/pve/sdn/dhcp.cfg:
> 
>   dnsmasq: nat
> 
> 
> /etc/pve/sdn/zones.cfg:
> 
>   simple: DHCPNAT
>           ipam pve
> 
> 
> /etc/pve/sdn/vnets.cfg:
> 
>   vnet: dhcpnat
>           zone DHCPNAT
> 
> 
> /etc/pve/sdn/subnets.cfg:
> 
>   subnet: DHCPNAT-10.1.0.0-16
>           vnet dhcpnat
>           dhcp-dns-server 10.1.0.1
>           dhcp-range server=nat,start-address=10.1.0.100,end-address=10.1.0.200
>           gateway 10.1.0.1
>           snat 1
> 
> 
> Then apply the SDN configuration:
> 
>   pvesh set /cluster/sdn
> 
> You need to apply the SDN configuration once after adding the dhcp-range lines
> to the configuration, since the running configuration is used for managing
> DHCP. It will not work otherwise!
> 
> For testing it can be helpful to monitor the following files (e.g. with watch)
> to find out what is happening
>   * /etc/dnsmasq.d/<dhcp_id>/ethers (on each node)
>   * /etc/pve/priv/ipam.db
> 
> Changes from v1 -> v2:
>   * added hooks for handling DHCP when starting / stopping / .. VMs and CTs
>   * Get an IP from IPAM and register that IP in the DHCP server
>     (pve only for now)
>   * remove lease-time, since it is now infinite and managed by the VM lifecycle
>   * add hooks for setting & deleting DHCP mappings to DHCP plugins
>   * modified interface of the abstract class to reflect new requirements
>   * added helpers in existing SDN classes
>   * simplified DHCP configuration settings
> 
> 
> 
> pve-cluster:
> 
> Stefan Hanreich (1):
>   cluster files: add dhcp.cfg
> 
>  src/PVE/Cluster.pm  | 1 +
>  src/pmxcfs/status.c | 1 +
>  2 files changed, 2 insertions(+)
> 
> 
> pve-network:
> 
> Stefan Hanreich (6):
>   subnets: vnets: preparations for DHCP plugins
>   dhcp: add abstract class for DHCP plugins
>   dhcp: subnet: add DHCP options to subnet configuration
>   dhcp: add DHCP plugin for dnsmasq
>   ipam: Add helper methods for DHCP to PVE IPAM
>   dhcp: regenerate config for DHCP servers on reload
> 
>  debian/control                         |   1 +
>  src/PVE/Network/SDN.pm                 |  11 +-
>  src/PVE/Network/SDN/Dhcp.pm            | 192 +++++++++++++++++++++++++
>  src/PVE/Network/SDN/Dhcp/Dnsmasq.pm    | 186 ++++++++++++++++++++++++
>  src/PVE/Network/SDN/Dhcp/Makefile      |   8 ++
>  src/PVE/Network/SDN/Dhcp/Plugin.pm     |  83 +++++++++++
>  src/PVE/Network/SDN/Ipams/PVEPlugin.pm |  64 +++++++++
>  src/PVE/Network/SDN/Makefile           |   3 +-
>  src/PVE/Network/SDN/SubnetPlugin.pm    |  32 +++++
>  src/PVE/Network/SDN/Subnets.pm         |  43 ++++--
>  src/PVE/Network/SDN/Vnets.pm           |  27 ++--
>  11 files changed, 622 insertions(+), 28 deletions(-)
>  create mode 100644 src/PVE/Network/SDN/Dhcp.pm
>  create mode 100644 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
>  create mode 100644 src/PVE/Network/SDN/Dhcp/Makefile
>  create mode 100644 src/PVE/Network/SDN/Dhcp/Plugin.pm
> 
> 
> pve-manager:
> 
> Stefan Hanreich (1):
>   sdn: regenerate DHCP config on reload
> 
>  PVE/API2/Network.pm | 1 +
>  1 file changed, 1 insertion(+)
> 
> 
> qemu-server:
> 
> Stefan Hanreich (1):
>   sdn: dhcp: add DHCP setup to vm-network-scripts
> 
>  PVE/QemuServer.pm                 | 14 ++++++++++++++
>  vm-network-scripts/pve-bridge     |  3 +++
>  vm-network-scripts/pve-bridgedown | 19 +++++++++++++++++++
>  3 files changed, 36 insertions(+)
> 
> 
> pve-container:
> 
> Stefan Hanreich (1):
>   sdn: dhcp: setup DHCP mappings in LXC hooks
> 
>  src/PVE/LXC.pm            | 10 ++++++++++
>  src/lxc-pve-poststop-hook |  1 +
>  src/lxc-pve-prestart-hook |  9 +++++++++
>  3 files changed, 20 insertions(+)
> 
> 
> Summary over all repositories:
>   20 files changed, 681 insertions(+), 28 deletions(-)
> 

Re: [pve-devel] [WIP v2 cluster/network/manager/qemu-server/container 00/10] Add support for DHCP servers to SDN

[Stefan Hanreich]

> Maybe try to see if we can use pve ipam as cache in front of external
> ipam.

Yes, it would also be cool if you could look at implementing the two
newly added methods from the PVEPlugin for Netbox / Phpipam, since you
have more experience with those.

I also looked into merging those two methods, but haven't really found
an elegant solution which is why I left them as separate methods for now.

Kind Regards

Re: [pve-devel] [WIP v2 cluster/network/manager/qemu-server/container 00/10] Add support for DHCP servers to SDN

[DERUMIER, Alexandre]

-------- Message initial --------
De: Stefan Hanreich <s.hanreich@proxmox.com>
À: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
"DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
Objet: Re: [pve-devel] [WIP v2 cluster/network/manager/qemu-
server/container 00/10] Add support for DHCP servers to SDN
Date: 17/10/2023 18:05:55

> Maybe try to see if we can use pve ipam as cache in front of external
> ipam.

>>Yes, it would also be cool if you could look at implementing the two
>>newly added methods from the PVEPlugin for Netbox / Phpipam, since
>>you
>>have more experience with those.

>>I also looked into merging those two methods, but haven't really
>>found
>>an elegant solution which is why I left them as separate methods for
>>now.
>>
>>Kind Regards


Yes, sure , no problem ! (I don't have read your code yet)

Re: [pve-devel] [WIP v2 cluster/network/manager/qemu-server/container 00/10] Add support for DHCP servers to SDN

[DERUMIER, Alexandre]

>>Another thing: What happens when a user changes the MAC address via
>>the
>>UI? I'd either disallow it completely or we need to update the DHCP
>>configuration files and IPAM


when mac address is changed online, the nic is doing unplug then
replug.

So technically, it's just

- unplug nic:  delete ipam / clean dhcp

- hotplug nic: add ipam/ add dhcp.


The guest os should automaticaly delete old ip on unplug, and reask a
new ip with dhcp on hotplug.

Re: [pve-devel] [WIP v2 pve-network 05/10] dhcp: add DHCP plugin for dnsmasq

[DERUMIER, Alexandre]

>>Adding new MAC address mappings is done with a simple reload that
>>does
>>not disrupt the dnsmasq daemon in any way.


Don't have read the code yet, but note that

dhcp-hostsfile=      for mac reservation  (different than dynamic
leases),

can be modify without any reload of dnsmasq.

Re: [pve-devel] [WIP v2 cluster/network/manager/qemu-server/container 00/10] Add support for DHCP servers to SDN

[Stefan Lendl]

I am currently working on the SDN feature.  This is an initial review of
the patch series and I am trying to make a strong case against ephemeral
DHCP IP reservation.

The current state of the patch series invokes the IPAM on every VM/CT
start/stop to add or remove the IP from the IPAM.
This triggers the dnsmasq config generation on the specific host with
only the MAC/IP mapping of that particular host.

From reading the discussion of the v1 patch series I understand this
approach tries to implement the ephemeral IP reservation strategy. From
off-list conversations with Stefan Hanreich, I agree that having
ephemeral IP reservation coordinated by the IPAM requires us to
re-implement DHCP functionality in the IPAM and heavily rely on syncing
between the different services.

To maintain reliable sync we need to hook into many different places
where the IPAM need to be queried.  Any issues with the implementation
may lead to IPAM and DHCP local config state running out of sync causing
network issues duplicate multiple IPs.

Furthermore, every interaction with the IPAM requires a cluster-wide
lock on the IPAM. Having a central cluster-wide lock on every VM
start/stop/migrate will significantly limit parallel operations.  Event
starting two VMs in parallel will be limited by this central lock. At
boot trying to start many VMs (ideally as much in parallel as possible)
is limited by the central IPAM lock even further.

I argue that we shall not support ephemeral IPs altogether.
The alternative is to make all IPAM reservations persistent.

Using persistent IPs only reduces the interactions of VM/CTs with the
IPAM to a minimum of NIC joining a subnet and NIC leaving a subnet. I am
deliberately not referring to VMs because a VM may be part of multiple
VNets or even multiple times in the same VNet (regardless if that is
sensible).

Cases the IPAM needs to be involved:

- NIC with DHCP enabled VNet is added to VM config
- NIC with DHCP enabled VNet is removed from VM config
- NIC is assigned to another Bridge
  can be treated as individual leave + join events

Cases that are explicitly not covered but may be added if desired:

- Manually assign an IP address on a NIC
  will not be automatically visible in the IPAM
- Manually change the MAC on a NIC
  don't do that > you are on your own.
  Not handled > change in IPAM manually

Once an IP is reserved via IPAM, the dnsmasq config can be generated
stateless and idempotent from the pve IPAM and is identical on all nodes
regardless if a VM/CT actually resides on that node or is running or
stopped.  This is especially useful for VM migration because the IP
stays consistent without spacial considering.

Snapshot/revert, backup/restore, suspend/hibernate/resume cases are
automatically covered because the IP will already be reserved for that
MAC.

If the admin wants to change, the IP of a VM this can be done via the
IPAM API/UI which will have to be implemented separately.

A limitation of this approach vs dynamic IP reservation is that the IP
range on the subnet needs to be large enough to hold all IPs of all,
even stopped, VMs in that subnet. This is in contrast to default DHCP
functionality where only the number of actively running VMs is limited.
It should be enough to mention this in the docs.

I will further review the code an try to implement the aforementioned
approach.

Best regards,
Stefan Lendl

Stefan Hanreich <s.hanreich@proxmox.com> writes:

> This is a WIP patch series, since I will be gone for 3 weeks and wanted to
> share my current progress with the DHCP support for SDN.
>
> This patch series adds support for automatically deploying dnsmasq as a DHCP
> server to a simple SDN Zone.
>
> While certainly not 100% polished on some ends (looking at restarting systemd
> services in particular), the general idea behind the mechanism shows. I wanted
> to gather some feedback on how I approached designing the plugins and the
> config regeneration process before comitting to this design by creating an API
> and UI around it.
>
> You need to install dnsmasq (and disable it afterwards):
>
>   apt install dnsmasq && systemctl disable --now dnsmasq
>
>
> You can use the following example configuration for deploying a DHCP server in
> a SDN subnet:
>
> /etc/pve/sdn/dhcp.cfg:
>
>   dnsmasq: nat
>
>
> /etc/pve/sdn/zones.cfg:
>
>   simple: DHCPNAT
>           ipam pve
>
>
> /etc/pve/sdn/vnets.cfg:
>
>   vnet: dhcpnat
>           zone DHCPNAT
>
>
> /etc/pve/sdn/subnets.cfg:
>
>   subnet: DHCPNAT-10.1.0.0-16
>           vnet dhcpnat
>           dhcp-dns-server 10.1.0.1
>           dhcp-range server=nat,start-address=10.1.0.100,end-address=10.1.0.200
>           gateway 10.1.0.1
>           snat 1
>
>
> Then apply the SDN configuration:
>
>   pvesh set /cluster/sdn
>
> You need to apply the SDN configuration once after adding the dhcp-range lines
> to the configuration, since the running configuration is used for managing
> DHCP. It will not work otherwise!
>
> For testing it can be helpful to monitor the following files (e.g. with watch)
> to find out what is happening
>   * /etc/dnsmasq.d/<dhcp_id>/ethers (on each node)
>   * /etc/pve/priv/ipam.db
>
> Changes from v1 -> v2:
>   * added hooks for handling DHCP when starting / stopping / .. VMs and CTs
>   * Get an IP from IPAM and register that IP in the DHCP server
>     (pve only for now)
>   * remove lease-time, since it is now infinite and managed by the VM lifecycle
>   * add hooks for setting & deleting DHCP mappings to DHCP plugins
>   * modified interface of the abstract class to reflect new requirements
>   * added helpers in existing SDN classes
>   * simplified DHCP configuration settings
>
>
>
> pve-cluster:
>
> Stefan Hanreich (1):
>   cluster files: add dhcp.cfg
>
>  src/PVE/Cluster.pm  | 1 +
>  src/pmxcfs/status.c | 1 +
>  2 files changed, 2 insertions(+)
>
>
> pve-network:
>
> Stefan Hanreich (6):
>   subnets: vnets: preparations for DHCP plugins
>   dhcp: add abstract class for DHCP plugins
>   dhcp: subnet: add DHCP options to subnet configuration
>   dhcp: add DHCP plugin for dnsmasq
>   ipam: Add helper methods for DHCP to PVE IPAM
>   dhcp: regenerate config for DHCP servers on reload
>
>  debian/control                         |   1 +
>  src/PVE/Network/SDN.pm                 |  11 +-
>  src/PVE/Network/SDN/Dhcp.pm            | 192 +++++++++++++++++++++++++
>  src/PVE/Network/SDN/Dhcp/Dnsmasq.pm    | 186 ++++++++++++++++++++++++
>  src/PVE/Network/SDN/Dhcp/Makefile      |   8 ++
>  src/PVE/Network/SDN/Dhcp/Plugin.pm     |  83 +++++++++++
>  src/PVE/Network/SDN/Ipams/PVEPlugin.pm |  64 +++++++++
>  src/PVE/Network/SDN/Makefile           |   3 +-
>  src/PVE/Network/SDN/SubnetPlugin.pm    |  32 +++++
>  src/PVE/Network/SDN/Subnets.pm         |  43 ++++--
>  src/PVE/Network/SDN/Vnets.pm           |  27 ++--
>  11 files changed, 622 insertions(+), 28 deletions(-)
>  create mode 100644 src/PVE/Network/SDN/Dhcp.pm
>  create mode 100644 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
>  create mode 100644 src/PVE/Network/SDN/Dhcp/Makefile
>  create mode 100644 src/PVE/Network/SDN/Dhcp/Plugin.pm
>
>
> pve-manager:
>
> Stefan Hanreich (1):
>   sdn: regenerate DHCP config on reload
>
>  PVE/API2/Network.pm | 1 +
>  1 file changed, 1 insertion(+)
>
>
> qemu-server:
>
> Stefan Hanreich (1):
>   sdn: dhcp: add DHCP setup to vm-network-scripts
>
>  PVE/QemuServer.pm                 | 14 ++++++++++++++
>  vm-network-scripts/pve-bridge     |  3 +++
>  vm-network-scripts/pve-bridgedown | 19 +++++++++++++++++++
>  3 files changed, 36 insertions(+)
>
>
> pve-container:
>
> Stefan Hanreich (1):
>   sdn: dhcp: setup DHCP mappings in LXC hooks
>
>  src/PVE/LXC.pm            | 10 ++++++++++
>  src/lxc-pve-poststop-hook |  1 +
>  src/lxc-pve-prestart-hook |  9 +++++++++
>  3 files changed, 20 insertions(+)
>
>
> Summary over all repositories:
>   20 files changed, 681 insertions(+), 28 deletions(-)
>
> --
> murpp v0.4.0
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [WIP v2 cluster/network/manager/qemu-server/container 00/10] Add support for DHCP servers to SDN

[Thomas Lamprecht]

Am 23/10/2023 um 14:40 schrieb Stefan Lendl:
> I am currently working on the SDN feature.  This is an initial review of
> the patch series and I am trying to make a strong case against ephemeral
> DHCP IP reservation.

Stefan Hanreich's reply to the cover letter already mentions upserts, those
will avoid basically all problems while allowing for some dynamic changes.

> The current state of the patch series invokes the IPAM on every VM/CT
> start/stop to add or remove the IP from the IPAM.
> This triggers the dnsmasq config generation on the specific host with
> only the MAC/IP mapping of that particular host.
> 
> From reading the discussion of the v1 patch series I understand this
> approach tries to implement the ephemeral IP reservation strategy. From
> off-list conversations with Stefan Hanreich, I agree that having
> ephemeral IP reservation coordinated by the IPAM requires us to
> re-implement DHCP functionality in the IPAM and heavily rely on syncing
> between the different services.
> 
> To maintain reliable sync we need to hook into many different places
> where the IPAM need to be queried.  Any issues with the implementation
> may lead to IPAM and DHCP local config state running out of sync causing
> network issues duplicate multiple IPs.

The same is true for permanent reservations, wherever that reservation is
saved needs to be in sync with IPAM, e.g., also on backup restore (into a
new env), if subnets change their configured CIDRs, ...

> 
> Furthermore, every interaction with the IPAM requires a cluster-wide
> lock on the IPAM. Having a central cluster-wide lock on every VM
> start/stop/migrate will significantly limit parallel operations.  Event
> starting two VMs in parallel will be limited by this central lock. At
> boot trying to start many VMs (ideally as much in parallel as possible)
> is limited by the central IPAM lock even further.

Cluster wide locks are relatively cheap, especially if one avoids having
a long critical section, i.e., query IPAM while still unlocked, then 
read and update the state locked, if the newly received IP is already
in there then simply give up lock again and repeat.

We also have a clusters wide lock for starting HA guests, to set the
wanted ha-resource state, that is no issue at all, you can start/stop
many orders of magnitudes more VMs than any HW/Storage could cope with.

> 
> I argue that we shall not support ephemeral IPs altogether.
> The alternative is to make all IPAM reservations persistent.


> 
> Using persistent IPs only reduces the interactions of VM/CTs with the
> IPAM to a minimum of NIC joining a subnet and NIC leaving a subnet. I am
> deliberately not referring to VMs because a VM may be part of multiple
> VNets or even multiple times in the same VNet (regardless if that is
> sensible).

Yeah, talking about vNICs / veth's is the better term here, guests are
only indirectly relevant.

> 
> Cases the IPAM needs to be involved:
> 
> - NIC with DHCP enabled VNet is added to VM config
> - NIC with DHCP enabled VNet is removed from VM config
> - NIC is assigned to another Bridge
>   can be treated as individual leave + join events

and:

- subnet config is changed
- vNIC changes from SDN-DHCP managed to manual, or vice versa
  Albeit that can almost be treated like vNet leave/join though

 
> Cases that are explicitly not covered but may be added if desired:
> 
> - Manually assign an IP address on a NIC
>   will not be automatically visible in the IPAM

This sounds like you want to save the state in the VM config, which I'm
rather skeptical about, and would try hard to avoid. We also would need
to differ between bridges that are part of DHCP-managed SDN and others,
as else a user could set some IP but nothing would happen.

> - Manually change the MAC on a NIC
>   don't do that > you are on your own.

FWIW, a clone is such a change, and we have to support that, otherwise
the MAC field needs to get some warning hints or even become read-only
in the UI.

>   Not handled > change in IPAM manually
> 
> Once an IP is reserved via IPAM, the dnsmasq config can be generated
> stateless and idempotent from the pve IPAM and is identical on all nodes
> regardless if a VM/CT actually resides on that node or is running or
> stopped.  This is especially useful for VM migration because the IP
> stays consistent without spacial considering.

That should be orthogonal to the feature set, if we have all the info
saved somewhere else

But this also speaks against having it in the VM config, as that would
mean that every node needs to parse every guests' config periodically,
which is way worse than some cluster lock and breaks with our base
axiom that guests are owned by their current node, and only by that,
and a node should not really alter behavior dependent on some "foreign"
guest.

> 
> Snapshot/revert, backup/restore, suspend/hibernate/resume cases are
> automatically covered because the IP will already be reserved for that
> MAC.

Not really, restore to another setup is broken, one could resume the
VM after having changed CIDRs of a subnet, making that broken too, ...

> 
> If the admin wants to change, the IP of a VM this can be done via the
> IPAM API/UI which will have to be implemented separately.

Providing Overrides can be fine, but IMO that all should be still in
the SDN state, not per-VM one, and ideally use a common API.


> A limitation of this approach vs dynamic IP reservation is that the IP
> range on the subnet needs to be large enough to hold all IPs of all,
> even stopped, VMs in that subnet. This is in contrast to default DHCP
> functionality where only the number of actively running VMs is limited.
> It should be enough to mention this in the docs.

In production setups it should not matter _that_ much, but it might
be a bit of a PITA if one has a few "archived" VMs or the like, but
that alone would

> 
> I will further review the code an try to implement the aforementioned
> approach.

You can naturally experiment, but I'd also try the upsert proposal from
Stefan H., as IMO that sounds like a good balance.

[pve-devel] [RFC SDN DHCP] Add and Remove DHCP mappings on vNIC add/remove

[Stefan Lendl]

Date: Fri, 27 Oct 2023 13:00:49 +0200
From 96fee0866727a1188b1debd805c625c598816b98 Mon Sep 17 00:00:00 2001
This patch series defines IPs, once allocated by the IPAM as persistant
until the vNIC is removed from the VM or the VM is destroyed.

The dnsmasq ethers file is a pure product of the IPAM database and does
not have it's own state, therefore removing the potential issue of wrong
sync state between IPAM and dnsmasq.
Updating the ethers file is only done on the effected node.

Technically, starting the VM still adds the DHCP mapping. This is needed
at the moment because during migration, this triggers generating the
ethers file on the target node.  If the IPAM already contains the
mapping, it returns the existing IP (upsert)

Alternative approach would be to trigger an update of the ethers file on
all nodes on IPAM update.

Stopping a guest does not effect IPAM/DHCP.

Destroying a guest removes the associated IPs from the IPAM.

-- 

[pve-devel] [RFC SDN DHCP] Add and Remove DHCP mappings on vNIC add/remove

[Stefan Lendl]

Date: Fri, 27 Oct 2023 13:00:49 +0200
From 96fee0866727a1188b1debd805c625c598816b98 Mon Sep 17 00:00:00 2001
This patch series defines IPs, once allocated by the IPAM as persistant
until the vNIC is removed from the VM or the VM is destroyed.

The dnsmasq ethers file is a pure product of the IPAM database and does
not have it's own state, therefore removing the potential issue of wrong
sync state between IPAM and dnsmasq.
Updating the ethers file is only done on the effected node.

Technically, starting the VM still adds the DHCP mapping. This is needed
at the moment because during migration, this triggers generating the
ethers file on the target node.  If the IPAM already contains the
mapping, it returns the existing IP (upsert)

Alternative approach would be to trigger an update of the ethers file on
all nodes on IPAM update.

Stopping a guest does not effect IPAM/DHCP.

Destroying a guest removes the associated IPs from the IPAM.

-- 

[pve-devel] [RFC pve-network 1/3] dhcp add ip returns IP if already present for MAC

[Stefan Lendl]

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
---
 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm    | 23 +++++++++++++++++++++++
 src/PVE/Network/SDN/Ipams/PVEPlugin.pm | 16 ++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index af109b8..6f8b1c4 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -18,6 +18,29 @@ sub type {
     return 'dnsmasq';
 }
 
+sub generate_config {
+    my ($class, $dhcp_config, $ip_mappings) = @_;
+
+    my $ethers_file = "$DNSMASQ_CONFIG_ROOT/$dhcp_config->{id}/ethers";
+    my $ethers_tmp_file = "$ethers_file.tmp";
+
+    open(my $out, '>', $ethers_file) or die "Could not open file '$ethers_file' $!\n";
+
+    foreach my $ip (keys %$ip_mappings) {
+	if (exists $ip_mappings->{$ip}{mac}) {
+	    my $mac = $ip_mappings->{$ip}{mac};
+	    print $out "$mac,$ip\n";
+	}
+    }
+
+    close $out;
+
+    chmod 0644, $ethers_file;
+
+    my $service_name = "dnsmasq\@$dhcp_config->{id}";
+    PVE::Tools::run_command(['systemctl', 'reload', $service_name]);
+}
+
 sub del_ip_mapping {
     my ($class, $dhcp_config, $mac) = @_;
 
diff --git a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
index fcc8282..40b4a8f 100644
--- a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
@@ -161,6 +161,8 @@ sub add_dhcp_ip {
 
     my $cidr = $subnet->{cidr};
     my $zone = $subnet->{zone};
+    my $vmid = $data->{vmid};
+    my $mac = $data->{mac};
 
     cfs_lock_file($ipamdb_file, undef, sub {
 	my $db = read_db();
@@ -174,11 +176,25 @@ sub add_dhcp_ip {
 	my $ip = new Net::IP ("$dhcp_range->{'start-address'} - $dhcp_range->{'end-address'}")
 	    or die "Invalid IP address(es) in DHCP Range!\n";
 
+	do {
+	    my $ip_address = $ip->ip();
+	    if (exists $dbsubnet->{ips}->{$ip_address} &&
+		exists $dbsubnet->{ips}->{$ip_address}->{mac} &&
+		$dbsubnet->{ips}->{$ip_address}->{mac} eq $mac) {
+		print "IP '$ip_address' already exist for $mac in $vmid\n";
+
+		return $ip_address;
+	    }
+	} while (++$ip);
+
+	$ip = new Net::IP ("$dhcp_range->{'start-address'} - $dhcp_range->{'end-address'}");
+
 	do {
 	    my $ip_address = $ip->ip();
 	    if (!$dbsubnet->{ips}->{$ip_address}) {
 		$dbsubnet->{ips}->{$ip_address} = $data;
 		write_db($db);
+		print "New IP '$ip_address' added for $mac at $vmid\n";
 
 		return $ip_address;
 	    }
-- 
2.41.0

[pve-devel] [RFC pve-network 2/3] always generate dnsmasq ethers file

[Stefan Lendl]

Makes dnsmasq stateless and can be generated from the IPAM.
On dhcp_add_ip always generate the entire ethers file.

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
---
 src/PVE/Network/SDN/Dhcp.pm | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/PVE/Network/SDN/Dhcp.pm b/src/PVE/Network/SDN/Dhcp.pm
index b92c73a..b854cce 100644
--- a/src/PVE/Network/SDN/Dhcp.pm
+++ b/src/PVE/Network/SDN/Dhcp.pm
@@ -87,8 +87,14 @@ sub add_mapping {
 
 	    next if !$ip;
 
+	    # generates ethers file every time
+	    my $ipam_db = $ipam_plugin->read_db();
+	    my $dbzone = $ipam_db->{zones}->{$subnet_config->{zone}};
+	    my $dbsubnet = $dbzone->{subnets}->{$subnet_config->{cidr}};
+	    my $dbsubnet_ips = $dbsubnet->{ips};
+
 	    my $dhcp_plugin = PVE::Network::SDN::Dhcp::Plugin->lookup($dhcp_config->{type});
-	    $dhcp_plugin->add_ip_mapping($dhcp_config, $mac, $ip);
+	    $dhcp_plugin->generate_config($dhcp_config, $dbsubnet_ips);
 
 	    return $ip;
 	}
-- 
2.41.0

[pve-devel] [RFC pve-network 3/3] touch the ethers file when creating the dnsmasq config

[Stefan Lendl]

removes an error from journal when newly DHCP server is configured

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
---
 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index 6f8b1c4..39e4cce 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -182,6 +182,11 @@ CFG
 	$default_dnsmasq_config
     );
 
+    # touch the ethers file to avoid errors before creation
+    my $ethers_file = "$DNSMASQ_CONFIG_ROOT/$dhcp_config->{id}/ethers";
+    open(my $e, '>>', $ethers_file) or die "Could not open file '$ethers_file' $!\n";
+    close $e;
+
     unlink glob "$config_directory/10-*.conf";
 }
 
-- 
2.41.0

[pve-devel] [RFC pve-network] do not remove DHCP mapping on stop

[Stefan Lendl]

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
---
 src/PVE/LXC.pm            | 8 --------
 src/lxc-pve-poststop-hook | 1 -
 2 files changed, 9 deletions(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index bd8eb63..a7de9b8 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -916,14 +916,6 @@ sub vm_stop_cleanup {
     eval {
 	my $vollist = PVE::LXC::Config->get_vm_volumes($conf);
 	PVE::Storage::deactivate_volumes($storage_cfg, $vollist);
-
-	for my $k (keys %$conf) {
-	    next if $k !~ /^net(\d+)/;
-	    my $net = PVE::LXC::Config->parse_lxc_network($conf->{$k});
-	    next if $net->{type} ne 'veth';
-
-	    PVE::Network::SDN::Dhcp::remove_mapping($net->{bridge}, $net->{hwaddr});
-	}
     };
     warn $@ if $@; # avoid errors - just warn
 }
diff --git a/src/lxc-pve-poststop-hook b/src/lxc-pve-poststop-hook
index e7d46c7..2fe97ec 100755
--- a/src/lxc-pve-poststop-hook
+++ b/src/lxc-pve-poststop-hook
@@ -12,7 +12,6 @@ use PVE::LXC::Config;
 use PVE::LXC::Tools;
 use PVE::LXC;
 use PVE::Network;
-use PVE::Network::SDN::Dhcp;
 use PVE::RESTEnvironment;
 use PVE::Storage;
 use PVE::Tools;
-- 
2.41.0

[pve-devel] [RFC pve-network 4/5] do not remove DHCP mapping on VM stop

[Stefan Lendl]

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
---
 PVE/QemuServer.pm                 | 2 --
 vm-network-scripts/pve-bridgedown | 4 ----
 2 files changed, 6 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 6c1e463..710259b 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -6143,8 +6143,6 @@ sub vm_stop_cleanup {
 
 	cleanup_pci_devices($vmid, $conf);
 
-	cleanup_sdn_dhcp($vmid, $conf);
-
 	vmconfig_apply_pending($vmid, $conf, $storecfg) if $apply_pending_changes;
     };
     warn $@ if $@; # avoid errors - just warn
diff --git a/vm-network-scripts/pve-bridgedown b/vm-network-scripts/pve-bridgedown
index a220660..8d58947 100755
--- a/vm-network-scripts/pve-bridgedown
+++ b/vm-network-scripts/pve-bridgedown
@@ -26,10 +26,6 @@ my $conf = PVE::QemuConfig->load_config($vmid);
 my $netconf = $conf->{$netid};
 my $net = PVE::QemuServer::parse_net($netconf);
 
-if ($have_sdn) {
-    PVE::Network::SDN::Dhcp::remove_mapping($net->{bridge}, $net->{macaddr});
-}
-
 PVE::Network::tap_unplug($iface);
 
 exit 0;
-- 
2.41.0

[pve-devel] [RFC pve-network 5/5] DHCP mappings on vNIC add/remove

[Stefan Lendl]

add DHCP mapping on vNIC add/update and VM clone (new mac)
remove DHCP mapping on vNIC delete and VM destroy

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
---
 PVE/API2/Qemu.pm  | 25 +++++++++++++++++++++++++
 PVE/QemuServer.pm |  2 ++
 2 files changed, 27 insertions(+)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 38bdaab..1b16fa5 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -49,6 +49,8 @@ use PVE::SSHInfo;
 use PVE::Replication;
 use PVE::StorageTunnel;
 
+use PVE::Network::SDN;
+
 BEGIN {
     if (!$ENV{PVE_GENERATING_DOCS}) {
 	require PVE::HA::Env::PVE2;
@@ -1804,6 +1806,9 @@ my $update_vm_api  = sub {
 		    }
 		    PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
 		    PVE::QemuConfig->write_config($vmid, $conf);
+
+		    my $net = PVE::QemuServer::parse_net($conf->{$opt});
+		    PVE::Network::SDN::Dhcp::remove_mapping($net->{bridge}, $net->{macaddr});
 		} else {
 		    PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
 		    PVE::QemuConfig->write_config($vmid, $conf);
@@ -1881,6 +1886,18 @@ my $update_vm_api  = sub {
 			);
 		    }
 		    $conf->{pending}->{$opt} = $param->{$opt};
+
+		    my $new_net = PVE::QemuServer::parse_net($param->{$opt});
+		    if (exists $conf->{$opt}) {
+			my $old_net = PVE::QemuServer::parse_net($conf->{$opt});
+			if ($old_net->{bridge} ne $new_net->{bridge} or
+			    $old_net->{macaddr} ne $new_net->{macaddr}) {
+			    print "Bridge or MAC changed: $conf->{$opt} -> $param->{$opt}\n";
+			    PVE::Network::SDN::Dhcp::remove_mapping($old_net->{bridge}, $old_net->{macaddr});
+			}
+		    }
+		    PVE::Network::SDN::Dhcp::add_mapping($vmid, $new_net->{bridge}, $new_net->{macaddr});
+
 		} else {
 		    $conf->{pending}->{$opt} = $param->{$opt};
 
@@ -3763,6 +3780,14 @@ __PACKAGE__->register_method({
 
 		PVE::QemuConfig->write_config($newid, $newconf);
 
+		foreach my $opt (keys %$newconf) {
+		    if ($opt =~ m/^net(\d+)$/) {
+			my $value = $newconf->{$opt};
+			my $net = PVE::QemuServer::parse_net($value);
+			PVE::Network::SDN::Dhcp::add_mapping($newid, $net->{bridge}, $net->{macaddr});
+		    }
+		}
+
 		if ($target) {
 		    # always deactivate volumes - avoid lvm LVs to be active on several nodes
 		    PVE::Storage::deactivate_volumes($storecfg, $vollist, $snapname) if !$running;
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 710259b..8a63ec3 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2337,6 +2337,8 @@ sub destroy_vm {
 	});
     }
 
+    cleanup_sdn_dhcp($vmid, $conf);
+
     if (defined $replacement_conf) {
 	PVE::QemuConfig->write_config($vmid, $replacement_conf);
     } else {
-- 
2.41.0

[pve-devel] [RFC SDN DHCP] Add and Remove DHCP mappings on vNIC add/remove

[Stefan Lendl]

Sorry, Sending this again because I noticed that I messed up the subject
prefixes for the patches.



This patch series defines IPs, once allocated by the IPAM as persistant
until the vNIC is removed from the VM or the VM is destroyed.

The dnsmasq ethers file is a pure product of the IPAM database and does
not have it's own state, therefore removing the potential issue of wrong
sync state between IPAM and dnsmasq.
Updating the ethers file is only done on the effected node.

Technically, starting the VM still adds the DHCP mapping. This is needed
at the moment because during migration, this triggers generating the
ethers file on the target node.  If the IPAM already contains the
mapping, it returns the existing IP (upsert)

Alternative approach would be to trigger an update of the ethers file on
all nodes on IPAM update.

Stopping a guest does not effect IPAM/DHCP.

Destroying a guest removes the associated IPs from the IPAM.

-- 

[pve-devel] [RFC pve-network 1/6] dhcp add ip returns IP if already present for MAC

[Stefan Lendl]

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
---
 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm    | 23 +++++++++++++++++++++++
 src/PVE/Network/SDN/Ipams/PVEPlugin.pm | 16 ++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index af109b8..6f8b1c4 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -18,6 +18,29 @@ sub type {
     return 'dnsmasq';
 }
 
+sub generate_config {
+    my ($class, $dhcp_config, $ip_mappings) = @_;
+
+    my $ethers_file = "$DNSMASQ_CONFIG_ROOT/$dhcp_config->{id}/ethers";
+    my $ethers_tmp_file = "$ethers_file.tmp";
+
+    open(my $out, '>', $ethers_file) or die "Could not open file '$ethers_file' $!\n";
+
+    foreach my $ip (keys %$ip_mappings) {
+	if (exists $ip_mappings->{$ip}{mac}) {
+	    my $mac = $ip_mappings->{$ip}{mac};
+	    print $out "$mac,$ip\n";
+	}
+    }
+
+    close $out;
+
+    chmod 0644, $ethers_file;
+
+    my $service_name = "dnsmasq\@$dhcp_config->{id}";
+    PVE::Tools::run_command(['systemctl', 'reload', $service_name]);
+}
+
 sub del_ip_mapping {
     my ($class, $dhcp_config, $mac) = @_;
 
diff --git a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
index fcc8282..40b4a8f 100644
--- a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
@@ -161,6 +161,8 @@ sub add_dhcp_ip {
 
     my $cidr = $subnet->{cidr};
     my $zone = $subnet->{zone};
+    my $vmid = $data->{vmid};
+    my $mac = $data->{mac};
 
     cfs_lock_file($ipamdb_file, undef, sub {
 	my $db = read_db();
@@ -174,11 +176,25 @@ sub add_dhcp_ip {
 	my $ip = new Net::IP ("$dhcp_range->{'start-address'} - $dhcp_range->{'end-address'}")
 	    or die "Invalid IP address(es) in DHCP Range!\n";
 
+	do {
+	    my $ip_address = $ip->ip();
+	    if (exists $dbsubnet->{ips}->{$ip_address} &&
+		exists $dbsubnet->{ips}->{$ip_address}->{mac} &&
+		$dbsubnet->{ips}->{$ip_address}->{mac} eq $mac) {
+		print "IP '$ip_address' already exist for $mac in $vmid\n";
+
+		return $ip_address;
+	    }
+	} while (++$ip);
+
+	$ip = new Net::IP ("$dhcp_range->{'start-address'} - $dhcp_range->{'end-address'}");
+
 	do {
 	    my $ip_address = $ip->ip();
 	    if (!$dbsubnet->{ips}->{$ip_address}) {
 		$dbsubnet->{ips}->{$ip_address} = $data;
 		write_db($db);
+		print "New IP '$ip_address' added for $mac at $vmid\n";
 
 		return $ip_address;
 	    }
-- 
2.41.0

[pve-devel] [RFC pve-network 2/6] always generate dnsmasq ethers file

[Stefan Lendl]

Makes dnsmasq stateless and can be generated from the IPAM.
On dhcp_add_ip always generate the entire ethers file.

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
---
 src/PVE/Network/SDN/Dhcp.pm | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/PVE/Network/SDN/Dhcp.pm b/src/PVE/Network/SDN/Dhcp.pm
index b92c73a..b854cce 100644
--- a/src/PVE/Network/SDN/Dhcp.pm
+++ b/src/PVE/Network/SDN/Dhcp.pm
@@ -87,8 +87,14 @@ sub add_mapping {
 
 	    next if !$ip;
 
+	    # generates ethers file every time
+	    my $ipam_db = $ipam_plugin->read_db();
+	    my $dbzone = $ipam_db->{zones}->{$subnet_config->{zone}};
+	    my $dbsubnet = $dbzone->{subnets}->{$subnet_config->{cidr}};
+	    my $dbsubnet_ips = $dbsubnet->{ips};
+
 	    my $dhcp_plugin = PVE::Network::SDN::Dhcp::Plugin->lookup($dhcp_config->{type});
-	    $dhcp_plugin->add_ip_mapping($dhcp_config, $mac, $ip);
+	    $dhcp_plugin->generate_config($dhcp_config, $dbsubnet_ips);
 
 	    return $ip;
 	}
-- 
2.41.0

[pve-devel] [RFC pve-network 3/6] touch the ethers file when creating the dnsmasq config

[Stefan Lendl]

removes an error from journal when newly DHCP server is configured

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
---
 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index 6f8b1c4..39e4cce 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -182,6 +182,11 @@ CFG
 	$default_dnsmasq_config
     );
 
+    # touch the ethers file to avoid errors before creation
+    my $ethers_file = "$DNSMASQ_CONFIG_ROOT/$dhcp_config->{id}/ethers";
+    open(my $e, '>>', $ethers_file) or die "Could not open file '$ethers_file' $!\n";
+    close $e;
+
     unlink glob "$config_directory/10-*.conf";
 }
 
-- 
2.41.0

[pve-devel] [RFC pve-container 4/6] do not remove DHCP mapping on stop

[Stefan Lendl]

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
---
 src/PVE/LXC.pm            | 8 --------
 src/lxc-pve-poststop-hook | 1 -
 2 files changed, 9 deletions(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index bd8eb63..a7de9b8 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -916,14 +916,6 @@ sub vm_stop_cleanup {
     eval {
 	my $vollist = PVE::LXC::Config->get_vm_volumes($conf);
 	PVE::Storage::deactivate_volumes($storage_cfg, $vollist);
-
-	for my $k (keys %$conf) {
-	    next if $k !~ /^net(\d+)/;
-	    my $net = PVE::LXC::Config->parse_lxc_network($conf->{$k});
-	    next if $net->{type} ne 'veth';
-
-	    PVE::Network::SDN::Dhcp::remove_mapping($net->{bridge}, $net->{hwaddr});
-	}
     };
     warn $@ if $@; # avoid errors - just warn
 }
diff --git a/src/lxc-pve-poststop-hook b/src/lxc-pve-poststop-hook
index e7d46c7..2fe97ec 100755
--- a/src/lxc-pve-poststop-hook
+++ b/src/lxc-pve-poststop-hook
@@ -12,7 +12,6 @@ use PVE::LXC::Config;
 use PVE::LXC::Tools;
 use PVE::LXC;
 use PVE::Network;
-use PVE::Network::SDN::Dhcp;
 use PVE::RESTEnvironment;
 use PVE::Storage;
 use PVE::Tools;
-- 
2.41.0

[pve-devel] [RFC qemu-server 5/6] do not remove DHCP mapping on VM stop

[Stefan Lendl]

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
---
 PVE/QemuServer.pm                 | 2 --
 vm-network-scripts/pve-bridgedown | 4 ----
 2 files changed, 6 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 6c1e463..710259b 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -6143,8 +6143,6 @@ sub vm_stop_cleanup {
 
 	cleanup_pci_devices($vmid, $conf);
 
-	cleanup_sdn_dhcp($vmid, $conf);
-
 	vmconfig_apply_pending($vmid, $conf, $storecfg) if $apply_pending_changes;
     };
     warn $@ if $@; # avoid errors - just warn
diff --git a/vm-network-scripts/pve-bridgedown b/vm-network-scripts/pve-bridgedown
index a220660..8d58947 100755
--- a/vm-network-scripts/pve-bridgedown
+++ b/vm-network-scripts/pve-bridgedown
@@ -26,10 +26,6 @@ my $conf = PVE::QemuConfig->load_config($vmid);
 my $netconf = $conf->{$netid};
 my $net = PVE::QemuServer::parse_net($netconf);
 
-if ($have_sdn) {
-    PVE::Network::SDN::Dhcp::remove_mapping($net->{bridge}, $net->{macaddr});
-}
-
 PVE::Network::tap_unplug($iface);
 
 exit 0;
-- 
2.41.0

[pve-devel] [RFC qemu-server 6/6] DHCP mappings on vNIC add/remove

[Stefan Lendl]

add DHCP mapping on vNIC add/update and VM clone (new mac)
remove DHCP mapping on vNIC delete and VM destroy

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
---
 PVE/API2/Qemu.pm  | 25 +++++++++++++++++++++++++
 PVE/QemuServer.pm |  2 ++
 2 files changed, 27 insertions(+)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 38bdaab..1b16fa5 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -49,6 +49,8 @@ use PVE::SSHInfo;
 use PVE::Replication;
 use PVE::StorageTunnel;
 
+use PVE::Network::SDN;
+
 BEGIN {
     if (!$ENV{PVE_GENERATING_DOCS}) {
 	require PVE::HA::Env::PVE2;
@@ -1804,6 +1806,9 @@ my $update_vm_api  = sub {
 		    }
 		    PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
 		    PVE::QemuConfig->write_config($vmid, $conf);
+
+		    my $net = PVE::QemuServer::parse_net($conf->{$opt});
+		    PVE::Network::SDN::Dhcp::remove_mapping($net->{bridge}, $net->{macaddr});
 		} else {
 		    PVE::QemuConfig->add_to_pending_delete($conf, $opt, $force);
 		    PVE::QemuConfig->write_config($vmid, $conf);
@@ -1881,6 +1886,18 @@ my $update_vm_api  = sub {
 			);
 		    }
 		    $conf->{pending}->{$opt} = $param->{$opt};
+
+		    my $new_net = PVE::QemuServer::parse_net($param->{$opt});
+		    if (exists $conf->{$opt}) {
+			my $old_net = PVE::QemuServer::parse_net($conf->{$opt});
+			if ($old_net->{bridge} ne $new_net->{bridge} or
+			    $old_net->{macaddr} ne $new_net->{macaddr}) {
+			    print "Bridge or MAC changed: $conf->{$opt} -> $param->{$opt}\n";
+			    PVE::Network::SDN::Dhcp::remove_mapping($old_net->{bridge}, $old_net->{macaddr});
+			}
+		    }
+		    PVE::Network::SDN::Dhcp::add_mapping($vmid, $new_net->{bridge}, $new_net->{macaddr});
+
 		} else {
 		    $conf->{pending}->{$opt} = $param->{$opt};
 
@@ -3763,6 +3780,14 @@ __PACKAGE__->register_method({
 
 		PVE::QemuConfig->write_config($newid, $newconf);
 
+		foreach my $opt (keys %$newconf) {
+		    if ($opt =~ m/^net(\d+)$/) {
+			my $value = $newconf->{$opt};
+			my $net = PVE::QemuServer::parse_net($value);
+			PVE::Network::SDN::Dhcp::add_mapping($newid, $net->{bridge}, $net->{macaddr});
+		    }
+		}
+
 		if ($target) {
 		    # always deactivate volumes - avoid lvm LVs to be active on several nodes
 		    PVE::Storage::deactivate_volumes($storecfg, $vollist, $snapname) if !$running;
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 710259b..8a63ec3 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2337,6 +2337,8 @@ sub destroy_vm {
 	});
     }
 
+    cleanup_sdn_dhcp($vmid, $conf);
+
     if (defined $replacement_conf) {
 	PVE::QemuConfig->write_config($vmid, $replacement_conf);
     } else {
-- 
2.41.0

Re: [pve-devel] [WIP v2 pve-network 06/10] ipam: Add helper methods for DHCP to PVE IPAM

[Stefan Lendl]

Stefan Hanreich <s.hanreich@proxmox.com> writes:

> Those methods are used by the DHCP plugins to attain the next free
> IP address for a given DHCP range, as well as delete all entries with
> a certain MAC address.
>
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>  src/PVE/Network/SDN/Ipams/PVEPlugin.pm | 64 ++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
>
> diff --git a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
> index 3e8ffc5..fcc8282 100644
> --- a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
> +++ b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
> @@ -156,6 +156,70 @@ sub add_next_freeip {
>      return "$freeip/$mask";
>  }
>
> +sub add_dhcp_ip {
> +    my ($class, $subnet, $dhcp_range, $data) = @_;
> +
> +    my $cidr = $subnet->{cidr};
> +    my $zone = $subnet->{zone};
> +
> +    cfs_lock_file($ipamdb_file, undef, sub {
> +	my $db = read_db();
> +
> +	my $dbzone = $db->{zones}->{$zone};
> +	die "zone '$zone' doesn't exist in IPAM DB\n" if !$dbzone;
> +
> +	my $dbsubnet = $dbzone->{subnets}->{$cidr};
> +	die "subnet '$cidr' doesn't exist in IPAM DB\n" if !$dbsubnet;
> +
> +	my $ip = new Net::IP ("$dhcp_range->{'start-address'} - $dhcp_range->{'end-address'}")
> +	    or die "Invalid IP address(es) in DHCP Range!\n";
> +
> +	do {
> +	    my $ip_address = $ip->ip();
> +	    if (!$dbsubnet->{ips}->{$ip_address}) {
> +		$dbsubnet->{ips}->{$ip_address} = $data;
> +		write_db($db);
> +
> +		return $ip_address;
> +	    }
> +	} while (++$ip);
> +
> +	die "No free IP left in DHCP Range $dhcp_range->{'start-address'}:$dhcp_range->{'end-address'}}\n";
> +    });
> +}

This duplicates existing functionality to find a new IP.
Regarding the reply from Alexandre, we may not need to implement
dhcp_range feature and solely rely on IPAM generating a new IP.

> +
> +sub del_dhcp_ip {
> +    my ($class, $subnet, $mac) = @_;
> +
> +    my $cidr = $subnet->{cidr};
> +    my $zone = $subnet->{zone};
> +
> +    my $returned_ip = undef;
> +
> +    cfs_lock_file($ipamdb_file, undef, sub {
> +	my $db = read_db();
> +
> +	die "zone $zone don't exist in ipam db" if !$db->{zones}->{$zone};
> +	my $dbzone = $db->{zones}->{$zone};
> +
> +	die "subnet $cidr don't exist in ipam db" if !$dbzone->{subnets}->{$cidr};
> +	my $dbsubnet = $dbzone->{subnets}->{$cidr};
> +
> +	foreach my $ip_address (keys %{$dbsubnet->{ips}}) {
> +	    my $data = $dbsubnet->{ips}->{$ip_address};
> +	    next if !$data->{mac} || $data->{mac} ne $mac;
> +
> +	    delete $dbsubnet->{ips}->{$ip_address};
> +	    write_db($db);
> +
> +	    $returned_ip = $ip_address;
> +	}
> +    });
> +    die "$@" if $@;
> +
> +    return $returned_ip;
> +}
> +
>  sub del_ip {
>      my ($class, $plugin_config, $subnetid, $subnet, $ip) = @_;

Re: [pve-devel] [RFC SDN DHCP] Add and Remove DHCP mappings on vNIC add/remove

[Thomas Lamprecht]

Am 27/10/2023 um 13:29 schrieb Stefan Lendl:
> Sorry, Sending this again because I noticed that I messed up the subject
> prefixes for the patches.

I'd also prefer having such series as separate thread, not in reply to another
(already big) series..

Also, if I didn't overlook something, you nowhere state if this patch series
is on top of Stefan Hanreich's, or stand alone..

Re: [pve-devel] [RFC SDN DHCP] Add and Remove DHCP mappings on vNIC add/remove

[Stefan Lendl]

It is on top of Stefan Hanreich's patch series therefore I replied here
as I consider it a continuation of his efforts.

Thomas Lamprecht <t.lamprecht@proxmox.com> writes:

> Am 27/10/2023 um 13:29 schrieb Stefan Lendl:
>> Sorry, Sending this again because I noticed that I messed up the subject
>> prefixes for the patches.
>
> I'd also prefer having such series as separate thread, not in reply to another
> (already big) series..
>
> Also, if I didn't overlook something, you nowhere state if this patch series
> is on top of Stefan Hanreich's, or stand alone..

Re: [pve-devel] [RFC SDN DHCP] Add and Remove DHCP mappings on vNIC add/remove

[Thomas Lamprecht]

Am 27/10/2023 um 13:52 schrieb Thomas Lamprecht:
> Am 27/10/2023 um 13:29 schrieb Stefan Lendl:
>> Sorry, Sending this again because I noticed that I messed up the subject
>> prefixes for the patches.
> 
> I'd also prefer having such series as separate thread, not in reply to another
> (already big) series..
> 
> Also, if I didn't overlook something, you nowhere state if this patch series
> is on top of Stefan Hanreich's, or stand alone..
> 

And if you use the `--cover-letter` option from git send-email, or git
format-patch to base off the cover letter, you would automatically get
a nice summary over commits and total diffstat here.

Re: [pve-devel] [RFC SDN DHCP] Add and Remove DHCP mappings on vNIC add/remove

[Thomas Lamprecht]

Am 27/10/2023 um 13:54 schrieb Stefan Lendl:
> It is on top of Stefan Hanreich's patch series therefore I replied here
> as I consider it a continuation of his efforts.

Please state that the next time.

Also, please avoid top posting on the mailing list and trim your replies!

https://git-send-email.io/top-posting.html

Re: [pve-devel] [WIP v2 cluster/network/manager/qemu-server/container 00/10] Add support for DHCP servers to SDN

[Stefan Lendl]

Thomas Lamprecht <t.lamprecht@proxmox.com> writes:

> Am 23/10/2023 um 14:40 schrieb Stefan Lendl:
>> I am currently working on the SDN feature.  This is an initial review of
>> the patch series and I am trying to make a strong case against ephemeral
>> DHCP IP reservation.
>
> Stefan Hanreich's reply to the cover letter already mentions upserts, those
> will avoid basically all problems while allowing for some dynamic changes.
>

I totally agree with upserts and my patches add this functionality.

>> The current state of the patch series invokes the IPAM on every VM/CT
>> start/stop to add or remove the IP from the IPAM.
>> This triggers the dnsmasq config generation on the specific host with
>> only the MAC/IP mapping of that particular host.
>>
>> From reading the discussion of the v1 patch series I understand this
>> approach tries to implement the ephemeral IP reservation strategy. From
>> off-list conversations with Stefan Hanreich, I agree that having
>> ephemeral IP reservation coordinated by the IPAM requires us to
>> re-implement DHCP functionality in the IPAM and heavily rely on syncing
>> between the different services.
>>
>> To maintain reliable sync we need to hook into many different places
>> where the IPAM need to be queried.  Any issues with the implementation
>> may lead to IPAM and DHCP local config state running out of sync causing
>> network issues duplicate multiple IPs.
>
> The same is true for permanent reservations, wherever that reservation is
> saved needs to be in sync with IPAM, e.g., also on backup restore (into a
> new env), if subnets change their configured CIDRs, ...
>

Yes, agreed but it's arguably less states and situation that need to be
synced.

The current implementation had a different state per node and depended
on the online/offline state of the guest.

It is currently not allowed to change the CIDR of a subnet.

>>
>> Furthermore, every interaction with the IPAM requires a cluster-wide
>> lock on the IPAM. Having a central cluster-wide lock on every VM
>> start/stop/migrate will significantly limit parallel operations.  Event
>> starting two VMs in parallel will be limited by this central lock. At
>> boot trying to start many VMs (ideally as much in parallel as possible)
>> is limited by the central IPAM lock even further.
>
> Cluster wide locks are relatively cheap, especially if one avoids having
> a long critical section, i.e., query IPAM while still unlocked, then
> read and update the state locked, if the newly received IP is already
> in there then simply give up lock again and repeat.
>
> We also have a clusters wide lock for starting HA guests, to set the
> wanted ha-resource state, that is no issue at all, you can start/stop
> many orders of magnitudes more VMs than any HW/Storage could cope with.
>
>>
>> I argue that we shall not support ephemeral IPs altogether.
>> The alternative is to make all IPAM reservations persistent.
>
>
>>
>> Using persistent IPs only reduces the interactions of VM/CTs with the
>> IPAM to a minimum of NIC joining a subnet and NIC leaving a subnet. I am
>> deliberately not referring to VMs because a VM may be part of multiple
>> VNets or even multiple times in the same VNet (regardless if that is
>> sensible).
>
> Yeah, talking about vNICs / veth's is the better term here, guests are
> only indirectly relevant.
>
>>
>> Cases the IPAM needs to be involved:
>>
>> - NIC with DHCP enabled VNet is added to VM config
>> - NIC with DHCP enabled VNet is removed from VM config
>> - NIC is assigned to another Bridge
>>   can be treated as individual leave + join events
>
> and:
>
> - subnet config is changed
> - vNIC changes from SDN-DHCP managed to manual, or vice versa
>   Albeit that can almost be treated like vNet leave/join though
>
>
>> Cases that are explicitly not covered but may be added if desired:
>>
>> - Manually assign an IP address on a NIC
>>   will not be automatically visible in the IPAM
>
> This sounds like you want to save the state in the VM config, which I'm
> rather skeptical about, and would try hard to avoid. We also would need
> to differ between bridges that are part of DHCP-managed SDN and others,
> as else a user could set some IP but nothing would happen.
>

I am sorry, my explanation was not clear here. I do not want to store IP
inside the VM config.  I agree that this would not be ideal.  If a user
configures an IP from inside the VM, we have no way of tracking that IP.

For now, every added vNIC gets an IP from the IPAM, and if the guest is
configured to use DHCP, it will get this IP from the DHCP server.

If the user decides to manually configure the IP, he will have to
reserve it in the IPAM, and mark the IP as "manual".
This will prevent the IPAM from allocating the IP again and keep the
IP/MAC mapping even if the VM is destroyed.

This is not implemented yet, but sketched out with Mira off-list.

>> - Manually change the MAC on a NIC
>>   don't do that > you are on your own.
>
> FWIW, a clone is such a change, and we have to support that, otherwise
> the MAC field needs to get some warning hints or even become read-only
> in the UI.
>
>>   Not handled > change in IPAM manually
>>
>> Once an IP is reserved via IPAM, the dnsmasq config can be generated
>> stateless and idempotent from the pve IPAM and is identical on all nodes
>> regardless if a VM/CT actually resides on that node or is running or
>> stopped.  This is especially useful for VM migration because the IP
>> stays consistent without spacial considering.
>
> That should be orthogonal to the feature set, if we have all the info
> saved somewhere else
>
> But this also speaks against having it in the VM config, as that would
> mean that every node needs to parse every guests' config periodically,
> which is way worse than some cluster lock and breaks with our base
> axiom that guests are owned by their current node, and only by that,
> and a node should not really alter behavior dependent on some "foreign"
> guest.
>
>>
>> Snapshot/revert, backup/restore, suspend/hibernate/resume cases are
>> automatically covered because the IP will already be reserved for that
>> MAC.
>
> Not really, restore to another setup is broken, one could resume the
> VM after having changed CIDRs of a subnet, making that broken too, ...
>
>>
>> If the admin wants to change, the IP of a VM this can be done via the
>> IPAM API/UI which will have to be implemented separately.
>
> Providing Overrides can be fine, but IMO that all should be still in
> the SDN state, not per-VM one, and ideally use a common API.
>
>
>> A limitation of this approach vs dynamic IP reservation is that the IP
>> range on the subnet needs to be large enough to hold all IPs of all,
>> even stopped, VMs in that subnet. This is in contrast to default DHCP
>> functionality where only the number of actively running VMs is limited.
>> It should be enough to mention this in the docs.
>
> In production setups it should not matter _that_ much, but it might
> be a bit of a PITA if one has a few "archived" VMs or the like, but
> that alone would
>
>>
>> I will further review the code an try to implement the aforementioned
>> approach.
>
> You can naturally experiment, but I'd also try the upsert proposal from
> Stefan H., as IMO that sounds like a good balance.

Re: [pve-devel] [WIP v2 cluster/network/manager/qemu-server/container 00/10] Add support for DHCP servers to SDN

[DERUMIER, Alexandre]

> 
> Furthermore, every interaction with the IPAM requires a cluster-wide
> lock on the IPAM. Having a central cluster-wide lock on every VM
> start/stop/migrate will significantly limit parallel operations. 
> Event
> starting two VMs in parallel will be limited by this central lock. At
> boot trying to start many VMs (ideally as much in parallel as
> possible)
> is limited by the central IPAM lock even further.

>>Cluster wide locks are relatively cheap, especially if one avoids
>>having
>>a long critical section, i.e., query IPAM while still unlocked, then 
>>read and update the state locked, if the newly received IP is already
>>in there then simply give up lock again and repeat.

>>We also have a clusters wide lock for starting HA guests, to set the
>>wanted ha-resource state, that is no issue at all, you can start/stop
>>many orders of magnitudes more VMs than any HW/Storage could cope
>>with.


You also need to think about external ipam, where maybe it'll take some
seconds to find an available ip and allocate it. (it's depend of size
of the subnet, could have also dns update, ...)

so, it'll really limit the parallelism of vm start.



(Personnaly, If we have choice between reserved at vm/nic create &&
ephemeral  at vm start, it's ok me).






> Once an IP is reserved via IPAM, the dnsmasq config can be generated
> stateless and idempotent from the pve IPAM and is identical on all
nodes
> regardless if a VM/CT actually resides on that node or is running or
> stopped.  This is especially useful for VM migration because the IP
> stays consistent without spacial considering.
>>
>>That should be orthogonal to the feature set, if we have all the info
>>saved somewhere else

>>But this also speaks against having it in the VM config, as that
>>would
>>mean that every node needs to parse every guests' config
>>periodically,
>>which is way worse than some cluster lock and breaks with our base
>>axiom that guests are owned by their current node, and only by that,
>>and a node should not really alter behavior dependent on some
>>"foreign"
>>guest.

I think that is really more simple to add ip in local dnsmasq at vm
start

dnsmasq --dhcp-hostsfile=/var/lib/reservation.txt

echo "mac ip" >> /var/lib/reservation.txt
SIGUP dnsmasq



for persistant ip, we just need search previously allocated ip-mac in
ipam, then write reservation to dnsmasq and start vm


for epheral ip, we need to find && allocated a free ip in ipam, then
write the ip/mac in dnsmasq and start vm




and for external ipam, I had proposed to use local ipam as read cache.

When allocating a new ip (persistent or ephemeral):
   search mac/ip exist in external ipam
           true:  write it to local pve ipam cache
           false: allocate a new free ip in external ipam  && write it
to local pve ipam cache

Like this, for persistant ips, we don't care if external ipam is down
at vm start.
and we can also reuse local ipam ips list for firewall or other stuff,
without need to call the external ipam api.




 

Re: [pve-devel] [WIP v2 cluster/network/manager/qemu-server/container 00/10] Add support for DHCP servers to SDN

[Stefan Lendl]

Hi Alexandre, I am proposing a slightly different view.

I think it's better to keep all IPs, managed by the IPAM in the IPAM and
the VM only configures as DHCP.

I would implement the 4 mentioned events (vNIC create, destroy, start,
stop) in the SDN module and limit interactions between VM configs and
the SDN module to these events.

On NIC create: the it calls the SDN::nic_join_vnet($bridge, $mac)
function that handles IPAM registration if necessary triggers generating
DHCP config and so on. Same approach for the other SDN related events.

All the logic is implemented in the SDN module. This reduces coupling
between VM logic and SDN logic.

"DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com> writes:

> Hi Stefan (Lendl),
>
> I'm totally agreed with you, we should have persistent reservation,
> at vm create/nic plug, nic delete, vm delete.
>
> At least , for my usage with multiple cluster on different datacenters,
> I really can wait to call ipam to api at each start (for scalability or
> for security if ipam is down)
>
>
> This also allow to simply do reservations in dnsmasq file without any
> need to restart it. (AFAIK, openstack is using dnsmasq like this too)
>
>
> I'm not sure if true dynamic ephemral ip , changing at each vm
> stop/start is interesting for a server vm usage. (maybe for desktop
> vmwhere you share a small pool of ip, but I personnaly don't known any
> proxmox users using proxmox ve for this)
>
>
> see my proposal here (with handle ephemeral && reserved, but it's even
> easier with only reserved):
>
> https://lists.proxmox.com/pipermail/pve-devel/2023-September/059169.html
>
>
>
>
> "
> I think we could implement ipam call like:
>
>
> create vm or add a new nic  -->
> -----------------------------
> qm create ... -net0
> bridge=vnet,....,ip=(auto|192.168.0.1|dynamic),ip6=(..)
>
>
> auto : search a free ip in ipam.  write the ip address in net0: ...,ip=
> ip field
>
> 192.168.0.1:  check if ip is free in ipam && register ip in ipam. write
> the ip in ip field.
>
>
> dynamic: write "ephemeral" in net0: ....,ip=ephemeral (This is a
> dynamic ip registered at vm start, and release at vm stop)
>
>
>
> vm start
> ---------
> - if ip=ephemeral, find && register a free ip in ipam, write it in vm
> net0: ...,ip=192.168.0.10[E] .   (maybe with a special flag [E] to
> indicate it's ephemeral)
> - read ip from vm config && inject in dhcp
>
>
> vm_stop
> -------
> if ip is ephemeral (netX: ip=192.168.0.10[E]),  delete ip from ipam,
> set ip=ephemeral in vm config
>
>
> vm_destroy or nic remove/unplug
> -------------------------
> if netX: ...,ip=192.168.0.10   ,  remove ip from ipam
>
>
>
> nic update when vm is running:
> ------------------------------
> if ip is defined : netX: ip=192.168.0.10,  we don't allow bridge change
> or ip change, as vm is not notified about theses changes, and still use
> old ip.
>
> We can allow nic hot-unplug && hotplug. (guest os will remove the ip on
> nic removal, and will call dhcp again on nic hotplug)
>
>
>
>
> nic hotplug with ip=auto:
> -------------------------
>
> --> add nic in pending state ----> find ip in ipam && write it in
> pending ---> do the hotplug in qemu.
>
> We need to handle the config revert to remove ip from ipam if the nic
> hotplug is blocked in pending state(I never see this case until os
> don't have pci_hotplug module loaded, but it's better to be carefull )
>
> "
>
>
>>>I am currently working on the SDN feature.  This is an initial review
>>>of
>>>the patch series and I am trying to make a strong case against
>>>ephemeral
>>>DHCP IP reservation.
>>>
>>>The current state of the patch series invokes the IPAM on every VM/CT
>>>start/stop to add or remove the IP from the IPAM.
>>>This triggers the dnsmasq config generation on the specific host with
>>>only the MAC/IP mapping of that particular host.
>
>
>
>
>
> From reading the discussion of the v1 patch series I understand this
> approach tries to implement the ephemeral IP reservation strategy. From
> off-list conversations with Stefan Hanreich, I agree that having
> ephemeral IP reservation coordinated by the IPAM requires us to
> re-implement DHCP functionality in the IPAM and heavily rely on syncing
> between the different services.
>
> To maintain reliable sync we need to hook into many different places
> where the IPAM need to be queried.  Any issues with the implementation
> may lead to IPAM and DHCP local config state running out of sync
> causing
> network issues duplicate multiple IPs.
>
> Furthermore, every interaction with the IPAM requires a cluster-wide
> lock on the IPAM. Having a central cluster-wide lock on every VM
> start/stop/migrate will significantly limit parallel operations.  Event
> starting two VMs in parallel will be limited by this central lock. At
> boot trying to start many VMs (ideally as much in parallel as possible)
> is limited by the central IPAM lock even further.
>
> I argue that we shall not support ephemeral IPs altogether.
> The alternative is to make all IPAM reservations persistent.
>
> Using persistent IPs only reduces the interactions of VM/CTs with the
> IPAM to a minimum of NIC joining a subnet and NIC leaving a subnet. I
> am
> deliberately not referring to VMs because a VM may be part of multiple
> VNets or even multiple times in the same VNet (regardless if that is
> sensible).
>
> Cases the IPAM needs to be involved:
>
> - NIC with DHCP enabled VNet is added to VM config
> - NIC with DHCP enabled VNet is removed from VM config
> - NIC is assigned to another Bridge
>   can be treated as individual leave + join events
>
> Cases that are explicitly not covered but may be added if desired:
>
> - Manually assign an IP address on a NIC
>   will not be automatically visible in the IPAM
> - Manually change the MAC on a NIC
>   don't do that > you are on your own.
>   Not handled > change in IPAM manually
>
> Once an IP is reserved via IPAM, the dnsmasq config can be generated
> stateless and idempotent from the pve IPAM and is identical on all
> nodes
> regardless if a VM/CT actually resides on that node or is running or
> stopped.  This is especially useful for VM migration because the IP
> stays consistent without spacial considering.
>
> Snapshot/revert, backup/restore, suspend/hibernate/resume cases are
> automatically covered because the IP will already be reserved for that
> MAC.
>
> If the admin wants to change, the IP of a VM this can be done via the
> IPAM API/UI which will have to be implemented separately.
>
> A limitation of this approach vs dynamic IP reservation is that the IP
> range on the subnet needs to be large enough to hold all IPs of all,
> even stopped, VMs in that subnet. This is in contrast to default DHCP
> functionality where only the number of actively running VMs is limited.
> It should be enough to mention this in the docs.
>
> I will further review the code an try to implement the aforementioned
> approach.
>
> Best regards,
> Stefan Lendl
>
> Stefan Hanreich <s.hanreich@proxmox.com> writes:
>
>> This is a WIP patch series, since I will be gone for 3 weeks and
>> wanted to
>> share my current progress with the DHCP support for SDN.
>>
>> This patch series adds support for automatically deploying dnsmasq as
>> a DHCP
>> server to a simple SDN Zone.
>>
>> While certainly not 100% polished on some ends (looking at restarting
>> systemd
>> services in particular), the general idea behind the mechanism shows.
>> I wanted
>> to gather some feedback on how I approached designing the plugins and
>> the
>> config regeneration process before comitting to this design by
>> creating an API
>> and UI around it.
>>
>> You need to install dnsmasq (and disable it afterwards):
>>
>>   apt install dnsmasq && systemctl disable --now dnsmasq
>>
>>
>> You can use the following example configuration for deploying a DHCP
>> server in
>> a SDN subnet:
>>
>> /etc/pve/sdn/dhcp.cfg:
>>
>>   dnsmasq: nat
>>
>>
>> /etc/pve/sdn/zones.cfg:
>>
>>   simple: DHCPNAT
>>           ipam pve
>>
>>
>> /etc/pve/sdn/vnets.cfg:
>>
>>   vnet: dhcpnat
>>           zone DHCPNAT
>>
>>
>> /etc/pve/sdn/subnets.cfg:
>>
>>   subnet: DHCPNAT-10.1.0.0-16
>>           vnet dhcpnat
>>           dhcp-dns-server 10.1.0.1
>>           dhcp-range server=nat,start-address=10.1.0.100,end-
>> address=10.1.0.200
>>           gateway 10.1.0.1
>>           snat 1
>>
>>
>> Then apply the SDN configuration:
>>
>>   pvesh set /cluster/sdn
>>
>> You need to apply the SDN configuration once after adding the dhcp-
>> range lines
>> to the configuration, since the running configuration is used for
>> managing
>> DHCP. It will not work otherwise!
>>
>> For testing it can be helpful to monitor the following files (e.g.
>> with watch)
>> to find out what is happening
>>   * /etc/dnsmasq.d/<dhcp_id>/ethers (on each node)
>>   * /etc/pve/priv/ipam.db
>>
>> Changes from v1 -> v2:
>>   * added hooks for handling DHCP when starting / stopping / .. VMs
>> and CTs
>>   * Get an IP from IPAM and register that IP in the DHCP server
>>     (pve only for now)
>>   * remove lease-time, since it is now infinite and managed by the VM
>> lifecycle
>>   * add hooks for setting & deleting DHCP mappings to DHCP plugins
>>   * modified interface of the abstract class to reflect new
>> requirements
>>   * added helpers in existing SDN classes
>>   * simplified DHCP configuration settings
>>
>>
>>
>> pve-cluster:
>>
>> Stefan Hanreich (1):
>>   cluster files: add dhcp.cfg
>>
>>  src/PVE/Cluster.pm  | 1 +
>>  src/pmxcfs/status.c | 1 +
>>  2 files changed, 2 insertions(+)
>>
>>
>> pve-network:
>>
>> Stefan Hanreich (6):
>>   subnets: vnets: preparations for DHCP plugins
>>   dhcp: add abstract class for DHCP plugins
>>   dhcp: subnet: add DHCP options to subnet configuration
>>   dhcp: add DHCP plugin for dnsmasq
>>   ipam: Add helper methods for DHCP to PVE IPAM
>>   dhcp: regenerate config for DHCP servers on reload
>>
>>  debian/control                         |   1 +
>>  src/PVE/Network/SDN.pm                 |  11 +-
>>  src/PVE/Network/SDN/Dhcp.pm            | 192
>> +++++++++++++++++++++++++
>>  src/PVE/Network/SDN/Dhcp/Dnsmasq.pm    | 186
>> ++++++++++++++++++++++++
>>  src/PVE/Network/SDN/Dhcp/Makefile      |   8 ++
>>  src/PVE/Network/SDN/Dhcp/Plugin.pm     |  83 +++++++++++
>>  src/PVE/Network/SDN/Ipams/PVEPlugin.pm |  64 +++++++++
>>  src/PVE/Network/SDN/Makefile           |   3 +-
>>  src/PVE/Network/SDN/SubnetPlugin.pm    |  32 +++++
>>  src/PVE/Network/SDN/Subnets.pm         |  43 ++++--
>>  src/PVE/Network/SDN/Vnets.pm           |  27 ++--
>>  11 files changed, 622 insertions(+), 28 deletions(-)
>>  create mode 100644 src/PVE/Network/SDN/Dhcp.pm
>>  create mode 100644 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
>>  create mode 100644 src/PVE/Network/SDN/Dhcp/Makefile
>>  create mode 100644 src/PVE/Network/SDN/Dhcp/Plugin.pm
>>
>>
>> pve-manager:
>>
>> Stefan Hanreich (1):
>>   sdn: regenerate DHCP config on reload
>>
>>  PVE/API2/Network.pm | 1 +
>>  1 file changed, 1 insertion(+)
>>
>>
>> qemu-server:
>>
>> Stefan Hanreich (1):
>>   sdn: dhcp: add DHCP setup to vm-network-scripts
>>
>>  PVE/QemuServer.pm                 | 14 ++++++++++++++
>>  vm-network-scripts/pve-bridge     |  3 +++
>>  vm-network-scripts/pve-bridgedown | 19 +++++++++++++++++++
>>  3 files changed, 36 insertions(+)
>>
>>
>> pve-container:
>>
>> Stefan Hanreich (1):
>>   sdn: dhcp: setup DHCP mappings in LXC hooks
>>
>>  src/PVE/LXC.pm            | 10 ++++++++++
>>  src/lxc-pve-poststop-hook |  1 +
>>  src/lxc-pve-prestart-hook |  9 +++++++++
>>  3 files changed, 20 insertions(+)
>>
>>
>> Summary over all repositories:
>>   20 files changed, 681 insertions(+), 28 deletions(-)
>>
>> --
>> murpp v0.4.0
>>
>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel@lists.proxmox.com
>> https://antiphishing.cetsi.fr/proxy/v3?i=d1l4NXNNaWE4SWZqU0dLWcuTfdxE
>> d98NfWIp9dma5kY&r=MXJUa0FrUVJqc1UwYWxNZ-
>> tuXduEO8AMVnCvYVMprCZ3oPilgy3nXcuJTOGH5iK84rVRg8cukFAROdxYRgFTTg&f=c2
>> xMdVN4Smh2R2tOZDdIRKCk7WEocHpTPMerT1Q-
>> Aq5qwr8l2xvAWuOGvFsV3frp2oSAgxNUQCpJDHp2iUmTWg&u=https%3A//lists.prox
>> mox.com/cgi-bin/mailman/listinfo/pve-devel&k=fjzS
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://antiphishing.cetsi.fr/proxy/v3?i=d1l4NXNNaWE4SWZqU0dLWcuTfdxEd9
> 8NfWIp9dma5kY&r=MXJUa0FrUVJqc1UwYWxNZ-
> tuXduEO8AMVnCvYVMprCZ3oPilgy3nXcuJTOGH5iK84rVRg8cukFAROdxYRgFTTg&f=c2xM
> dVN4Smh2R2tOZDdIRKCk7WEocHpTPMerT1Q-
> Aq5qwr8l2xvAWuOGvFsV3frp2oSAgxNUQCpJDHp2iUmTWg&u=https%3A//lists.proxmo
> x.com/cgi-bin/mailman/listinfo/pve-devel&k=fjzS

Re: [pve-devel] [WIP v2 cluster/network/manager/qemu-server/container 00/10] Add support for DHCP servers to SDN

[DERUMIER, Alexandre]

-------- Message initial --------
De: Stefan Lendl <s.lendl@proxmox.com>
À: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
Cc: pve-devel@lists.proxmox.com <pve-devel@lists.proxmox.com>
Objet: Re: [pve-devel] [WIP v2 cluster/network/manager/qemu-
server/container 00/10] Add support for DHCP servers to SDN
Date: 27/10/2023 14:53:25


Hi Alexandre, I am proposing a slightly different view.

>>I think it's better to keep all IPs, managed by the IPAM in the IPAM
>>and the VM only configures as DHCP.


Yes, I'm thinking exactly the same !   


I had tried 2year ago to implement ipam with static ip in vm
configuration (+ipam), and they are a lot of corner case.




>>I would implement the 4 mentioned events (vNIC create, destroy,
>>start,
>>stop) in the SDN module and limit interactions between VM configs and
>>the SDN module to these events.

>>
>>On NIC create: the it calls the SDN::nic_join_vnet($bridge, $mac)
>>function that handles IPAM registration if necessary triggers
>>generating
>>DHCP config and so on. Same approach for the other SDN related
>>events.
>>
>>All the logic is implemented in the SDN module. This reduces coupling
>>between VM logic and SDN logic.


sound great :)

"DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com> writes:

> Hi Stefan (Lendl),
> 
> I'm totally agreed with you, we should have persistent reservation,
> at vm create/nic plug, nic delete, vm delete.
> 
> At least , for my usage with multiple cluster on different
> datacenters,
> I really can wait to call ipam to api at each start (for scalability
> or
> for security if ipam is down)
> 
> 
> This also allow to simply do reservations in dnsmasq file without any
> need to restart it. (AFAIK, openstack is using dnsmasq like this too)
> 
> 
> I'm not sure if true dynamic ephemral ip , changing at each vm
> stop/start is interesting for a server vm usage. (maybe for desktop
> vmwhere you share a small pool of ip, but I personnaly don't known
> any
> proxmox users using proxmox ve for this)
> 
> 
> see my proposal here (with handle ephemeral && reserved, but it's
> even
> easier with only reserved):
> 
> https://antiphishing.cetsi.fr/proxy/v3?i=YXJwbnI5ZGY3YXM2MThBYc__j3mP
> QdDC0mZ08oRIJLw&r=d2RpVFJVaTVtcFJRWFNMYgYCddP93Y9SOEaGwAD-
> 9JdLrx2JwwKfs9Sn_uiRQCCUgqnCg4WLD-
> gLY0eKXrXX4A&f=SVN0TjFBb1k5Qk8zQ2E1YT-
> NJ2Y2fJYrRVcVAuRs9UYfyMFrtkoDLcaTV9MhYQZD&u=https%3A//lists.proxmox.c
> om/pipermail/pve-devel/2023-September/059169.html&k=ogd1
> 
> 
> 
> 
> "
> I think we could implement ipam call like:
> 
> 
> create vm or add a new nic  -->
> -----------------------------
> qm create ... -net0
> bridge=vnet,....,ip=(auto|192.168.0.1|dynamic),ip6=(..)
> 
> 
> auto : search a free ip in ipam.  write the ip address in net0:
> ...,ip=
> ip field
> 
> 192.168.0.1:  check if ip is free in ipam && register ip in ipam.
> write
> the ip in ip field.
> 
> 
> dynamic: write "ephemeral" in net0: ....,ip=ephemeral (This is a
> dynamic ip registered at vm start, and release at vm stop)
> 
> 
> 
> vm start
> ---------
> - if ip=ephemeral, find && register a free ip in ipam, write it in vm
> net0: ...,ip=192.168.0.10[E] .   (maybe with a special flag [E] to
> indicate it's ephemeral)
> - read ip from vm config && inject in dhcp
> 
> 
> vm_stop
> -------
> if ip is ephemeral (netX: ip=192.168.0.10[E]),  delete ip from ipam,
> set ip=ephemeral in vm config
> 
> 
> vm_destroy or nic remove/unplug
> -------------------------
> if netX: ...,ip=192.168.0.10   ,  remove ip from ipam
> 
> 
> 
> nic update when vm is running:
> ------------------------------
> if ip is defined : netX: ip=192.168.0.10,  we don't allow bridge
> change
> or ip change, as vm is not notified about theses changes, and still
> use
> old ip.
> 
> We can allow nic hot-unplug && hotplug. (guest os will remove the ip
> on
> nic removal, and will call dhcp again on nic hotplug)
> 
> 
> 
> 
> nic hotplug with ip=auto:
> -------------------------
> 
> --> add nic in pending state ----> find ip in ipam && write it in
> pending ---> do the hotplug in qemu.
> 
> We need to handle the config revert to remove ip from ipam if the nic
> hotplug is blocked in pending state(I never see this case until os
> don't have pci_hotplug module loaded, but it's better to be carefull
> )
> 
> "
> 
> 
> > > I am currently working on the SDN feature.  This is an initial
> > > review
> > > of
> > > the patch series and I am trying to make a strong case against
> > > ephemeral
> > > DHCP IP reservation.
> > > 
> > > The current state of the patch series invokes the IPAM on every
> > > VM/CT
> > > start/stop to add or remove the IP from the IPAM.
> > > This triggers the dnsmasq config generation on the specific host
> > > with
> > > only the MAC/IP mapping of that particular host.
> 
> 
> 
> 
> 
> From reading the discussion of the v1 patch series I understand this
> approach tries to implement the ephemeral IP reservation strategy.
> From
> off-list conversations with Stefan Hanreich, I agree that having
> ephemeral IP reservation coordinated by the IPAM requires us to
> re-implement DHCP functionality in the IPAM and heavily rely on
> syncing
> between the different services.
> 
> To maintain reliable sync we need to hook into many different places
> where the IPAM need to be queried.  Any issues with the
> implementation
> may lead to IPAM and DHCP local config state running out of sync
> causing
> network issues duplicate multiple IPs.
> 
> Furthermore, every interaction with the IPAM requires a cluster-wide
> lock on the IPAM. Having a central cluster-wide lock on every VM
> start/stop/migrate will significantly limit parallel operations. 
> Event
> starting two VMs in parallel will be limited by this central lock. At
> boot trying to start many VMs (ideally as much in parallel as
> possible)
> is limited by the central IPAM lock even further.
> 
> I argue that we shall not support ephemeral IPs altogether.
> The alternative is to make all IPAM reservations persistent.
> 
> Using persistent IPs only reduces the interactions of VM/CTs with the
> IPAM to a minimum of NIC joining a subnet and NIC leaving a subnet. I
> am
> deliberately not referring to VMs because a VM may be part of
> multiple
> VNets or even multiple times in the same VNet (regardless if that is
> sensible).
> 
> Cases the IPAM needs to be involved:
> 
> - NIC with DHCP enabled VNet is added to VM config
> - NIC with DHCP enabled VNet is removed from VM config
> - NIC is assigned to another Bridge
>   can be treated as individual leave + join events
> 
> Cases that are explicitly not covered but may be added if desired:
> 
> - Manually assign an IP address on a NIC
>   will not be automatically visible in the IPAM
> - Manually change the MAC on a NIC
>   don't do that > you are on your own.
>   Not handled > change in IPAM manually
> 
> Once an IP is reserved via IPAM, the dnsmasq config can be generated
> stateless and idempotent from the pve IPAM and is identical on all
> nodes
> regardless if a VM/CT actually resides on that node or is running or
> stopped.  This is especially useful for VM migration because the IP
> stays consistent without spacial considering.
> 
> Snapshot/revert, backup/restore, suspend/hibernate/resume cases are
> automatically covered because the IP will already be reserved for
> that
> MAC.
> 
> If the admin wants to change, the IP of a VM this can be done via the
> IPAM API/UI which will have to be implemented separately.
> 
> A limitation of this approach vs dynamic IP reservation is that the
> IP
> range on the subnet needs to be large enough to hold all IPs of all,
> even stopped, VMs in that subnet. This is in contrast to default DHCP
> functionality where only the number of actively running VMs is
> limited.
> It should be enough to mention this in the docs.
> 
> I will further review the code an try to implement the aforementioned
> approach.
> 
> Best regards,
> Stefan Lendl
> 
> Stefan Hanreich <s.hanreich@proxmox.com> writes:
> 
> > This is a WIP patch series, since I will be gone for 3 weeks and
> > wanted to
> > share my current progress with the DHCP support for SDN.
> > 
> > This patch series adds support for automatically deploying dnsmasq
> > as
> > a DHCP
> > server to a simple SDN Zone.
> > 
> > While certainly not 100% polished on some ends (looking at
> > restarting
> > systemd
> > services in particular), the general idea behind the mechanism
> > shows.
> > I wanted
> > to gather some feedback on how I approached designing the plugins
> > and
> > the
> > config regeneration process before comitting to this design by
> > creating an API
> > and UI around it.
> > 
> > You need to install dnsmasq (and disable it afterwards):
> > 
> >   apt install dnsmasq && systemctl disable --now dnsmasq
> > 
> > 
> > You can use the following example configuration for deploying a
> > DHCP
> > server in
> > a SDN subnet:
> > 
> > /etc/pve/sdn/dhcp.cfg:
> > 
> >   dnsmasq: nat
> > 
> > 
> > /etc/pve/sdn/zones.cfg:
> > 
> >   simple: DHCPNAT
> >           ipam pve
> > 
> > 
> > /etc/pve/sdn/vnets.cfg:
> > 
> >   vnet: dhcpnat
> >           zone DHCPNAT
> > 
> > 
> > /etc/pve/sdn/subnets.cfg:
> > 
> >   subnet: DHCPNAT-10.1.0.0-16
> >           vnet dhcpnat
> >           dhcp-dns-server 10.1.0.1
> >           dhcp-range server=nat,start-address=10.1.0.100,end-
> > address=10.1.0.200
> >           gateway 10.1.0.1
> >           snat 1
> > 
> > 
> > Then apply the SDN configuration:
> > 
> >   pvesh set /cluster/sdn
> > 
> > You need to apply the SDN configuration once after adding the dhcp-
> > range lines
> > to the configuration, since the running configuration is used for
> > managing
> > DHCP. It will not work otherwise!
> > 
> > For testing it can be helpful to monitor the following files (e.g.
> > with watch)
> > to find out what is happening
> >   * /etc/dnsmasq.d/<dhcp_id>/ethers (on each node)
> >   * /etc/pve/priv/ipam.db
> > 
> > Changes from v1 -> v2:
> >   * added hooks for handling DHCP when starting / stopping / .. VMs
> > and CTs
> >   * Get an IP from IPAM and register that IP in the DHCP server
> >     (pve only for now)
> >   * remove lease-time, since it is now infinite and managed by the
> > VM
> > lifecycle
> >   * add hooks for setting & deleting DHCP mappings to DHCP plugins
> >   * modified interface of the abstract class to reflect new
> > requirements
> >   * added helpers in existing SDN classes
> >   * simplified DHCP configuration settings
> > 
> > 
> > 
> > pve-cluster:
> > 
> > Stefan Hanreich (1):
> >   cluster files: add dhcp.cfg
> > 
> >  src/PVE/Cluster.pm  | 1 +
> >  src/pmxcfs/status.c | 1 +
> >  2 files changed, 2 insertions(+)
> > 
> > 
> > pve-network:
> > 
> > Stefan Hanreich (6):
> >   subnets: vnets: preparations for DHCP plugins
> >   dhcp: add abstract class for DHCP plugins
> >   dhcp: subnet: add DHCP options to subnet configuration
> >   dhcp: add DHCP plugin for dnsmasq
> >   ipam: Add helper methods for DHCP to PVE IPAM
> >   dhcp: regenerate config for DHCP servers on reload
> > 
> >  debian/control                         |   1 +
> >  src/PVE/Network/SDN.pm                 |  11 +-
> >  src/PVE/Network/SDN/Dhcp.pm            | 192
> > +++++++++++++++++++++++++
> >  src/PVE/Network/SDN/Dhcp/Dnsmasq.pm    | 186
> > ++++++++++++++++++++++++
> >  src/PVE/Network/SDN/Dhcp/Makefile      |   8 ++
> >  src/PVE/Network/SDN/Dhcp/Plugin.pm     |  83 +++++++++++
> >  src/PVE/Network/SDN/Ipams/PVEPlugin.pm |  64 +++++++++
> >  src/PVE/Network/SDN/Makefile           |   3 +-
> >  src/PVE/Network/SDN/SubnetPlugin.pm    |  32 +++++
> >  src/PVE/Network/SDN/Subnets.pm         |  43 ++++--
> >  src/PVE/Network/SDN/Vnets.pm           |  27 ++--
> >  11 files changed, 622 insertions(+), 28 deletions(-)
> >  create mode 100644 src/PVE/Network/SDN/Dhcp.pm
> >  create mode 100644 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
> >  create mode 100644 src/PVE/Network/SDN/Dhcp/Makefile
> >  create mode 100644 src/PVE/Network/SDN/Dhcp/Plugin.pm
> > 
> > 
> > pve-manager:
> > 
> > Stefan Hanreich (1):
> >   sdn: regenerate DHCP config on reload
> > 
> >  PVE/API2/Network.pm | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > 
> > qemu-server:
> > 
> > Stefan Hanreich (1):
> >   sdn: dhcp: add DHCP setup to vm-network-scripts
> > 
> >  PVE/QemuServer.pm                 | 14 ++++++++++++++
> >  vm-network-scripts/pve-bridge     |  3 +++
> >  vm-network-scripts/pve-bridgedown | 19 +++++++++++++++++++
> >  3 files changed, 36 insertions(+)
> > 
> > 
> > pve-container:
> > 
> > Stefan Hanreich (1):
> >   sdn: dhcp: setup DHCP mappings in LXC hooks
> > 
> >  src/PVE/LXC.pm            | 10 ++++++++++
> >  src/lxc-pve-poststop-hook |  1 +
> >  src/lxc-pve-prestart-hook |  9 +++++++++
> >  3 files changed, 20 insertions(+)
> > 
> > 
> > Summary over all repositories:
> >   20 files changed, 681 insertions(+), 28 deletions(-)
> > 
> > --
> > murpp v0.4.0
> > 
> > 
> > _______________________________________________
> > pve-devel mailing list
> > pve-devel@lists.proxmox.com
> > https://antiphishing.cetsi.fr/proxy/v3?i=YXJwbnI5ZGY3YXM2MThBYc__j3
> > mPQdDC0mZ08oRIJLw&r=d2RpVFJVaTVtcFJRWFNMYgYCddP93Y9SOEaGwAD-
> > 9JdLrx2JwwKfs9Sn_uiRQCCUgqnCg4WLD-
> > gLY0eKXrXX4A&f=SVN0TjFBb1k5Qk8zQ2E1YT-
> > NJ2Y2fJYrRVcVAuRs9UYfyMFrtkoDLcaTV9MhYQZD&u=https%3A//antiphishing.
> > cetsi.fr/proxy/v3%3Fi%3Dd1l4NXNNaWE4SWZqU0dLWcuTfdxE&k=ogd1
> > d98NfWIp9dma5kY&r=MXJUa0FrUVJqc1UwYWxNZ-
> > tuXduEO8AMVnCvYVMprCZ3oPilgy3nXcuJTOGH5iK84rVRg8cukFAROdxYRgFTTg&f=
> > c2
> > xMdVN4Smh2R2tOZDdIRKCk7WEocHpTPMerT1Q-
> > Aq5qwr8l2xvAWuOGvFsV3frp2oSAgxNUQCpJDHp2iUmTWg&u=https%3A//lists.pr
> > ox
> > mox.com/cgi-bin/mailman/listinfo/pve-devel&k=fjzS
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://antiphishing.cetsi.fr/proxy/v3?i=YXJwbnI5ZGY3YXM2MThBYc__j3mP
> QdDC0mZ08oRIJLw&r=d2RpVFJVaTVtcFJRWFNMYgYCddP93Y9SOEaGwAD-
> 9JdLrx2JwwKfs9Sn_uiRQCCUgqnCg4WLD-
> gLY0eKXrXX4A&f=SVN0TjFBb1k5Qk8zQ2E1YT-
> NJ2Y2fJYrRVcVAuRs9UYfyMFrtkoDLcaTV9MhYQZD&u=https%3A//antiphishing.ce
> tsi.fr/proxy/v3%3Fi%3Dd1l4NXNNaWE4SWZqU0dLWcuTfdxEd9&k=ogd1
> 8NfWIp9dma5kY&r=MXJUa0FrUVJqc1UwYWxNZ-
> tuXduEO8AMVnCvYVMprCZ3oPilgy3nXcuJTOGH5iK84rVRg8cukFAROdxYRgFTTg&f=c2
> xM
> dVN4Smh2R2tOZDdIRKCk7WEocHpTPMerT1Q-
> Aq5qwr8l2xvAWuOGvFsV3frp2oSAgxNUQCpJDHp2iUmTWg&u=https%3A//lists.prox
> mo
> x.com/cgi-bin/mailman/listinfo/pve-devel&k=fjzS

Re: [pve-devel] [RFC pve-network] do not remove DHCP mapping on stop

[DERUMIER, Alexandre]

hi,
I'm back from Holiday, and I'll finally time to work on dhcp.


I wonder if we couldn't add a property on subnet or dhcp,
where user could choose between ephemeral ip (create a vm start /
delete at vm stop),

or reserved ip

(reserved a vm|nic create,  deleted a vm|nic delete)


This should match behaviour of different cloud provider (gcp,aws,...)
or other hypervisors.



What do you think about it ?



-------- Message initial --------
De: Stefan Lendl <s.lendl@proxmox.com>
Répondre à: Proxmox VE development discussion <pve-
devel@lists.proxmox.com>
À: pve-devel@lists.proxmox.com
Objet: [pve-devel] [RFC pve-network] do not remove DHCP mapping on stop
Date: 27/10/2023 13:20:20

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
---
 src/PVE/LXC.pm            | 8 --------
 src/lxc-pve-poststop-hook | 1 -
 2 files changed, 9 deletions(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index bd8eb63..a7de9b8 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -916,14 +916,6 @@ sub vm_stop_cleanup {
     eval {
 	my $vollist = PVE::LXC::Config->get_vm_volumes($conf);
 	PVE::Storage::deactivate_volumes($storage_cfg, $vollist);
-
-	for my $k (keys %$conf) {
-	    next if $k !~ /^net(\d+)/;
-	    my $net = PVE::LXC::Config->parse_lxc_network($conf-
>{$k});
-	    next if $net->{type} ne 'veth';
-
-	    PVE::Network::SDN::Dhcp::remove_mapping($net->{bridge},
$net->{hwaddr});
-	}
     };
     warn $@ if $@; # avoid errors - just warn
 }
diff --git a/src/lxc-pve-poststop-hook b/src/lxc-pve-poststop-hook
index e7d46c7..2fe97ec 100755
--- a/src/lxc-pve-poststop-hook
+++ b/src/lxc-pve-poststop-hook
@@ -12,7 +12,6 @@ use PVE::LXC::Config;
 use PVE::LXC::Tools;
 use PVE::LXC;
 use PVE::Network;
-use PVE::Network::SDN::Dhcp;
 use PVE::RESTEnvironment;
 use PVE::Storage;
 use PVE::Tools;

Re: [pve-devel] [RFC pve-network] do not remove DHCP mapping on stop

[Stefan Hanreich]

On 11/8/23 15:32, DERUMIER, Alexandre wrote:
> hi,
> I'm back from Holiday, and I'll finally time to work on dhcp.
>

Welcome back! It's also my first day after holiday today.

> I wonder if we couldn't add a property on subnet or dhcp,
> where user could choose between ephemeral ip (create a vm start /
> delete at vm stop),
> 
> or reserved ip
> 
> (reserved a vm|nic create,  deleted a vm|nic delete)
> 

That sounds like something we could implement. I've talked to Stefan
Lendl today and we thought that ephemeral is something we can leave out
for now and implement later (although we definitely wanted to add
support for it!)

Since we want to have at least basic DHCP support in the next release
(which is not far away) we decided to focus on a minimal featureset
first and then build upon it.

Re: [pve-devel] [RFC pve-network] do not remove DHCP mapping on stop

[DERUMIER, Alexandre]

Hi Stefan !

> I wonder if we couldn't add a property on subnet or dhcp,
> where user could choose between ephemeral ip (create a vm start /
> delete at vm stop),
> 
> or reserved ip
> 
> (reserved a vm|nic create,  deleted a vm|nic delete)
> 

>>That sounds like something we could implement. I've talked to Stefan
>>Lendl today and we thought that ephemeral is something we can leave
>>out
>>for now and implement later (although we definitely wanted to add
>>support for it!)

ok, no problem. 

>>Since we want to have at least basic DHCP support in the next release
>>(which is not far away) we decided to focus on a minimal featureset
>>first and then build upon it.

Could it be possible to send merged series with both your current work
and Stefan Lendl work ?

I'm a bit lost in the mailing.


I'll try to update external ipam code with your current patches.
I'm currently doing tests with netbox, it seem than ip ranges is
supported now, and finding free ip in ranges too.
I'll look at phpipam.

Re: [pve-devel] [RFC pve-network 2/6] always generate dnsmasq ethers file

[DERUMIER, Alexandre]

Personnaly, I really think that we shouldn't generate the whole
dhcp config (reading the full ipam db), each time we allocate a single
ip.


With external ipam, that mean 1api call for each subnet, it can be
really slow.
(for example, I have 400 subnets in productions)

and this is only done on local node, so we'll to regenerated in other
node if we migrate vm, restore backup, HA moving vm,....



I think that mac-ip should be added at vm start,
it's really more simple and light.





-------- Message initial --------
De: Stefan Lendl <s.lendl@proxmox.com>
Répondre à: Proxmox VE development discussion <pve-
devel@lists.proxmox.com>
À: pve-devel@lists.proxmox.com
Objet: [pve-devel] [RFC pve-network 2/6] always generate dnsmasq ethers
file
Date: 27/10/2023 13:29:56

Makes dnsmasq stateless and can be generated from the IPAM.
On dhcp_add_ip always generate the entire ethers file.

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
---
 src/PVE/Network/SDN/Dhcp.pm | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/PVE/Network/SDN/Dhcp.pm b/src/PVE/Network/SDN/Dhcp.pm
index b92c73a..b854cce 100644
--- a/src/PVE/Network/SDN/Dhcp.pm
+++ b/src/PVE/Network/SDN/Dhcp.pm
@@ -87,8 +87,14 @@ sub add_mapping {
 
 	    next if !$ip;
 
+	    # generates ethers file every time
+	    my $ipam_db = $ipam_plugin->read_db();
+	    my $dbzone = $ipam_db->{zones}->{$subnet_config->{zone}};
+	    my $dbsubnet = $dbzone->{subnets}->{$subnet_config-
>{cidr}};
+	    my $dbsubnet_ips = $dbsubnet->{ips};
+
 	    my $dhcp_plugin = PVE::Network::SDN::Dhcp::Plugin-
>lookup($dhcp_config->{type});
-	    $dhcp_plugin->add_ip_mapping($dhcp_config, $mac, $ip);
+	    $dhcp_plugin->generate_config($dhcp_config,
$dbsubnet_ips);
 
 	    return $ip;
 	}

Re: [pve-devel] [RFC qemu-server 6/6] DHCP mappings on vNIC add/remove

[DERUMIER, Alexandre]

From my previous mail,

Here I'll allocate only in ipam,   but not generate dhcp config.


-------- Message initial --------
De: Stefan Lendl <s.lendl@proxmox.com>
Répondre à: Proxmox VE development discussion <pve-
devel@lists.proxmox.com>
À: pve-devel@lists.proxmox.com
Objet: [pve-devel] [RFC qemu-server 6/6] DHCP mappings on vNIC
add/remove
Date: 27/10/2023 13:30:00

add DHCP mapping on vNIC add/update and VM clone (new mac)
remove DHCP mapping on vNIC delete and VM destroy

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
---
 PVE/API2/Qemu.pm  | 25 +++++++++++++++++++++++++
 PVE/QemuServer.pm |  2 ++
 2 files changed, 27 insertions(+)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 38bdaab..1b16fa5 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -49,6 +49,8 @@ use PVE::SSHInfo;
 use PVE::Replication;
 use PVE::StorageTunnel;
 
+use PVE::Network::SDN;
+
 BEGIN {
     if (!$ENV{PVE_GENERATING_DOCS}) {
 	require PVE::HA::Env::PVE2;
@@ -1804,6 +1806,9 @@ my $update_vm_api  = sub {
 		    }
 		    PVE::QemuConfig->add_to_pending_delete($conf,
$opt, $force);
 		    PVE::QemuConfig->write_config($vmid, $conf);
+
+		    my $net = PVE::QemuServer::parse_net($conf-
>{$opt});
+		    PVE::Network::SDN::Dhcp::remove_mapping($net-
>{bridge}, $net->{macaddr});
 		} else {
 		    PVE::QemuConfig->add_to_pending_delete($conf,
$opt, $force);
 		    PVE::QemuConfig->write_config($vmid, $conf);
@@ -1881,6 +1886,18 @@ my $update_vm_api  = sub {
 			);
 		    }
 		    $conf->{pending}->{$opt} = $param->{$opt};
+
+		    my $new_net = PVE::QemuServer::parse_net($param-
>{$opt});
+		    if (exists $conf->{$opt}) {
+			my $old_net =
PVE::QemuServer::parse_net($conf->{$opt});
+			if ($old_net->{bridge} ne $new_net->{bridge}
or
+			    $old_net->{macaddr} ne $new_net-
>{macaddr}) {
+			    print "Bridge or MAC changed: $conf-
>{$opt} -> $param->{$opt}\n";
+			   
PVE::Network::SDN::Dhcp::remove_mapping($old_net->{bridge}, $old_net-
>{macaddr});
+			}
+		    }
+		    PVE::Network::SDN::Dhcp::add_mapping($vmid,
$new_net->{bridge}, $new_net->{macaddr});
+
 		} else {
 		    $conf->{pending}->{$opt} = $param->{$opt};
 
@@ -3763,6 +3780,14 @@ __PACKAGE__->register_method({
 
 		PVE::QemuConfig->write_config($newid, $newconf);
 
+		foreach my $opt (keys %$newconf) {
+		    if ($opt =~ m/^net(\d+)$/) {
+			my $value = $newconf->{$opt};
+			my $net = PVE::QemuServer::parse_net($value);
+			PVE::Network::SDN::Dhcp::add_mapping($newid,
$net->{bridge}, $net->{macaddr});
+		    }
+		}
+
 		if ($target) {
 		    # always deactivate volumes - avoid lvm LVs to be
active on several nodes
 		    PVE::Storage::deactivate_volumes($storecfg,
$vollist, $snapname) if !$running;
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 710259b..8a63ec3 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2337,6 +2337,8 @@ sub destroy_vm {
 	});
     }
 
+    cleanup_sdn_dhcp($vmid, $conf);
+
     if (defined $replacement_conf) {
 	PVE::QemuConfig->write_config($vmid, $replacement_conf);
     } else {

Re: [pve-devel] [WIP v2 pve-network 05/10] dhcp: add DHCP plugin for dnsmasq

[DERUMIER, Alexandre]

>>/etc/default/dnsmasq.<dhcp_id>
>>This file specifies the configuration directory for the dnsmasq
>>instance (/etc/dnsmasq.d/<dhcp_id>). It also sets the configuration
>>file to /dev/null so the default configuration from the package has
>>no influence on the dnsmasq configuration.
>>
>>/etc/dnsmasq.d/<dhcp_id>/00-default.conf



About <dhcp_id>,  

I think we should have 1 dhcp_id by zone. (or it could even be the zone
name).

As we can have multiple zone, with same subnet, but in differents vrf.

(this also need a different dnsmasq process in a different vrf)



Also, currently, I'm not sure why we need to define the dhcp in
/etc/pve/sdn/dhcp.cfg ?

couldn't we simply add something like : "dhcp:1"  or "dhcp:dnsmasq"  on
the zone ?

Re: [pve-devel] [WIP v2 pve-network 05/10] dhcp: add DHCP plugin for dnsmasq

[Stefan Hanreich]

On 11/8/23 18:18, DERUMIER, Alexandre wrote:
> Also, currently, I'm not sure why we need to define the dhcp in
> /etc/pve/sdn/dhcp.cfg ?
> 
> couldn't we simply add something like : "dhcp:1"  or "dhcp:dnsmasq"  on
> the zone ?

That sounds like a good idea and would simplify stuff a lot. I'll try
and merge the 3 patch series into something workable for Thomas, since
currently it's a bit of a mess to review / apply.

It might take until next week, since I am currently also working on the
API and the UI for this feature and I want to produce a version that
includes the improvements from Stefan and you as well as the API / UI.

Re: [pve-devel] [WIP v2 cluster/network/manager/qemu-server/container 00/10] Add support for DHCP servers to SDN

[DERUMIER, Alexandre]

also,

about dhcp-ranges in this patch series, I think it could be great to
make it optionnal, as some external couldn't support it.

(netbox seem to support it, but I don't have looked at next_free api
yet  ,  phpipam don't seem to support it).

A lot of external ipam tool only search about free ip in the full
subnet.

So maybe something like : no dhcp-range = any ip from the subnet.

-------- Message initial --------
De: Stefan Lendl <s.lendl@proxmox.com>
Répondre à: Proxmox VE development discussion <pve-
devel@lists.proxmox.com>
À: pve-devel@lists.proxmox.com
Objet: Re: [pve-devel] [WIP v2 cluster/network/manager/qemu-
server/container 00/10] Add support for DHCP servers to SDN
Date: 23/10/2023 12:27:06


I am currently working on the SDN feature.  This is an initial review
of
the patch series and I am trying to make a strong case against
ephemeral
DHCP IP reservation.

The current state of the patch series invokes the IPAM on every VM/CT
start/stop to add or remove the IP from the IPAM.
This triggers the dnsmasq config generation on the specific host with
only the MAC/IP mapping of that particular host.

From reading the discussion of the v1 patch series I understand this
approach tries to implement the ephemeral IP reservation strategy. From
off-list conversations with Stefan Hanreich, I agree that having
ephemeral IP reservation coordinated by the IPAM requires us to
re-implement DHCP functionality in the IPAM and heavily rely on syncing
between the different services.

To maintain reliable sync we need to hook into many different places
where the IPAM need to be queried.  Any issues with the implementation
may lead to IPAM and DHCP local config state running out of sync
causing
network issues duplicate multiple IPs.

Furthermore, every interaction with the IPAM requires a cluster-wide
lock on the IPAM. Having a central cluster-wide lock on every VM
start/stop/migrate will significantly limit parallel operations.  Event
starting two VMs in parallel will be limited by this central lock. At
boot trying to start many VMs (ideally as much in parallel as possible)
is limited by the central IPAM lock even further.

I argue that we shall not support ephemeral IPs altogether.
The alternative is to make all IPAM reservations persistent.

Using persistent IPs only reduces the interactions of VM/CTs with the
IPAM to a minimum of NIC joining a subnet and NIC leaving a subnet. I
am
deliberately not referring to VMs because a VM may be part of multiple
VNets or even multiple times in the same VNet (regardless if that is
sensible).

Cases the IPAM needs to be involved:

- NIC with DHCP enabled VNet is added to VM config
- NIC with DHCP enabled VNet is removed from VM config
- NIC is assigned to another Bridge
  can be treated as individual leave + join events

Cases that are explicitly not covered but may be added if desired:

- Manually assign an IP address on a NIC
  will not be automatically visible in the IPAM
- Manually change the MAC on a NIC
  don't do that > you are on your own.
  Not handled > change in IPAM manually

Once an IP is reserved via IPAM, the dnsmasq config can be generated
stateless and idempotent from the pve IPAM and is identical on all
nodes
regardless if a VM/CT actually resides on that node or is running or
stopped.  This is especially useful for VM migration because the IP
stays consistent without spacial considering.

Snapshot/revert, backup/restore, suspend/hibernate/resume cases are
automatically covered because the IP will already be reserved for that
MAC.

If the admin wants to change, the IP of a VM this can be done via the
IPAM API/UI which will have to be implemented separately.

A limitation of this approach vs dynamic IP reservation is that the IP
range on the subnet needs to be large enough to hold all IPs of all,
even stopped, VMs in that subnet. This is in contrast to default DHCP
functionality where only the number of actively running VMs is limited.
It should be enough to mention this in the docs.

I will further review the code an try to implement the aforementioned
approach.

Best regards,
Stefan Lendl

Stefan Hanreich <s.hanreich@proxmox.com> writes:

> This is a WIP patch series, since I will be gone for 3 weeks and
> wanted to
> share my current progress with the DHCP support for SDN.
> 
> This patch series adds support for automatically deploying dnsmasq as
> a DHCP
> server to a simple SDN Zone.
> 
> While certainly not 100% polished on some ends (looking at restarting
> systemd
> services in particular), the general idea behind the mechanism shows.
> I wanted
> to gather some feedback on how I approached designing the plugins and
> the
> config regeneration process before comitting to this design by
> creating an API
> and UI around it.
> 
> You need to install dnsmasq (and disable it afterwards):
> 
>   apt install dnsmasq && systemctl disable --now dnsmasq
> 
> 
> You can use the following example configuration for deploying a DHCP
> server in
> a SDN subnet:
> 
> /etc/pve/sdn/dhcp.cfg:
> 
>   dnsmasq: nat
> 
> 
> /etc/pve/sdn/zones.cfg:
> 
>   simple: DHCPNAT
>           ipam pve
> 
> 
> /etc/pve/sdn/vnets.cfg:
> 
>   vnet: dhcpnat
>           zone DHCPNAT
> 
> 
> /etc/pve/sdn/subnets.cfg:
> 
>   subnet: DHCPNAT-10.1.0.0-16
>           vnet dhcpnat
>           dhcp-dns-server 10.1.0.1
>           dhcp-range server=nat,start-address=10.1.0.100,end-
> address=10.1.0.200
>           gateway 10.1.0.1
>           snat 1
> 
> 
> Then apply the SDN configuration:
> 
>   pvesh set /cluster/sdn
> 
> You need to apply the SDN configuration once after adding the dhcp-
> range lines
> to the configuration, since the running configuration is used for
> managing
> DHCP. It will not work otherwise!
> 
> For testing it can be helpful to monitor the following files (e.g.
> with watch)
> to find out what is happening
>   * /etc/dnsmasq.d/<dhcp_id>/ethers (on each node)
>   * /etc/pve/priv/ipam.db
> 
> Changes from v1 -> v2:
>   * added hooks for handling DHCP when starting / stopping / .. VMs
> and CTs
>   * Get an IP from IPAM and register that IP in the DHCP server
>     (pve only for now)
>   * remove lease-time, since it is now infinite and managed by the VM
> lifecycle
>   * add hooks for setting & deleting DHCP mappings to DHCP plugins
>   * modified interface of the abstract class to reflect new
> requirements
>   * added helpers in existing SDN classes
>   * simplified DHCP configuration settings
> 
> 
> 
> pve-cluster:
> 
> Stefan Hanreich (1):
>   cluster files: add dhcp.cfg
> 
>  src/PVE/Cluster.pm  | 1 +
>  src/pmxcfs/status.c | 1 +
>  2 files changed, 2 insertions(+)
> 
> 
> pve-network:
> 
> Stefan Hanreich (6):
>   subnets: vnets: preparations for DHCP plugins
>   dhcp: add abstract class for DHCP plugins
>   dhcp: subnet: add DHCP options to subnet configuration
>   dhcp: add DHCP plugin for dnsmasq
>   ipam: Add helper methods for DHCP to PVE IPAM
>   dhcp: regenerate config for DHCP servers on reload
> 
>  debian/control                         |   1 +
>  src/PVE/Network/SDN.pm                 |  11 +-
>  src/PVE/Network/SDN/Dhcp.pm            | 192
> +++++++++++++++++++++++++
>  src/PVE/Network/SDN/Dhcp/Dnsmasq.pm    | 186
> ++++++++++++++++++++++++
>  src/PVE/Network/SDN/Dhcp/Makefile      |   8 ++
>  src/PVE/Network/SDN/Dhcp/Plugin.pm     |  83 +++++++++++
>  src/PVE/Network/SDN/Ipams/PVEPlugin.pm |  64 +++++++++
>  src/PVE/Network/SDN/Makefile           |   3 +-
>  src/PVE/Network/SDN/SubnetPlugin.pm    |  32 +++++
>  src/PVE/Network/SDN/Subnets.pm         |  43 ++++--
>  src/PVE/Network/SDN/Vnets.pm           |  27 ++--
>  11 files changed, 622 insertions(+), 28 deletions(-)
>  create mode 100644 src/PVE/Network/SDN/Dhcp.pm
>  create mode 100644 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
>  create mode 100644 src/PVE/Network/SDN/Dhcp/Makefile
>  create mode 100644 src/PVE/Network/SDN/Dhcp/Plugin.pm
> 
> 
> pve-manager:
> 
> Stefan Hanreich (1):
>   sdn: regenerate DHCP config on reload
> 
>  PVE/API2/Network.pm | 1 +
>  1 file changed, 1 insertion(+)
> 
> 
> qemu-server:
> 
> Stefan Hanreich (1):
>   sdn: dhcp: add DHCP setup to vm-network-scripts
> 
>  PVE/QemuServer.pm                 | 14 ++++++++++++++
>  vm-network-scripts/pve-bridge     |  3 +++
>  vm-network-scripts/pve-bridgedown | 19 +++++++++++++++++++
>  3 files changed, 36 insertions(+)
> 
> 
> pve-container:
> 
> Stefan Hanreich (1):
>   sdn: dhcp: setup DHCP mappings in LXC hooks
> 
>  src/PVE/LXC.pm            | 10 ++++++++++
>  src/lxc-pve-poststop-hook |  1 +
>  src/lxc-pve-prestart-hook |  9 +++++++++
>  3 files changed, 20 insertions(+)
> 
> 
> Summary over all repositories:
>   20 files changed, 681 insertions(+), 28 deletions(-)
> 
> --
> murpp v0.4.0
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://antiphishing.cetsi.fr/proxy/v3?i=d1l4NXNNaWE4SWZqU0dLWcuTfdxE
> d98NfWIp9dma5kY&r=MXJUa0FrUVJqc1UwYWxNZ-
> tuXduEO8AMVnCvYVMprCZ3oPilgy3nXcuJTOGH5iK84rVRg8cukFAROdxYRgFTTg&f=c2
> xMdVN4Smh2R2tOZDdIRKCk7WEocHpTPMerT1Q-
> Aq5qwr8l2xvAWuOGvFsV3frp2oSAgxNUQCpJDHp2iUmTWg&u=https%3A//lists.prox
> mox.com/cgi-bin/mailman/listinfo/pve-devel&k=fjzS


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://antiphishing.cetsi.fr/proxy/v3?i=d1l4NXNNaWE4SWZqU0dLWcuTfdxEd9
8NfWIp9dma5kY&r=MXJUa0FrUVJqc1UwYWxNZ-
tuXduEO8AMVnCvYVMprCZ3oPilgy3nXcuJTOGH5iK84rVRg8cukFAROdxYRgFTTg&f=c2xM
dVN4Smh2R2tOZDdIRKCk7WEocHpTPMerT1Q-
Aq5qwr8l2xvAWuOGvFsV3frp2oSAgxNUQCpJDHp2iUmTWg&u=https%3A//lists.proxmo
x.com/cgi-bin/mailman/listinfo/pve-devel&k=fjzS

Re: [pve-devel] [WIP v2 cluster/network/manager/qemu-server/container 00/10] Add support for DHCP servers to SDN

[DERUMIER, Alexandre]

Hi Stefan (Lendl),

I'm totally agreed with you, we should have persistent reservation,
at vm create/nic plug, nic delete, vm delete.

At least , for my usage with multiple cluster on different datacenters,
I really can wait to call ipam to api at each start (for scalability or
for security if ipam is down)


This also allow to simply do reservations in dnsmasq file without any
need to restart it. (AFAIK, openstack is using dnsmasq like this too)


I'm not sure if true dynamic ephemral ip , changing at each vm
stop/start is interesting for a server vm usage. (maybe for desktop
vmwhere you share a small pool of ip, but I personnaly don't known any
proxmox users using proxmox ve for this)


see my proposal here (with handle ephemeral && reserved, but it's even
easier with only reserved):

https://lists.proxmox.com/pipermail/pve-devel/2023-September/059169.html




"
I think we could implement ipam call like:


create vm or add a new nic  --> 
-----------------------------
qm create ... -net0
bridge=vnet,....,ip=(auto|192.168.0.1|dynamic),ip6=(..)


auto : search a free ip in ipam.  write the ip address in net0: ...,ip=
ip field 

192.168.0.1:  check if ip is free in ipam && register ip in ipam. write
the ip in ip field.


dynamic: write "ephemeral" in net0: ....,ip=ephemeral (This is a
dynamic ip registered at vm start, and release at vm stop)



vm start
---------
- if ip=ephemeral, find && register a free ip in ipam, write it in vm
net0: ...,ip=192.168.0.10[E] .   (maybe with a special flag [E] to
indicate it's ephemeral)
- read ip from vm config && inject in dhcp


vm_stop
-------
if ip is ephemeral (netX: ip=192.168.0.10[E]),  delete ip from ipam,
set ip=ephemeral in vm config


vm_destroy or nic remove/unplug
-------------------------
if netX: ...,ip=192.168.0.10   ,  remove ip from ipam



nic update when vm is running:
------------------------------
if ip is defined : netX: ip=192.168.0.10,  we don't allow bridge change
or ip change, as vm is not notified about theses changes, and still use
old ip.

We can allow nic hot-unplug && hotplug. (guest os will remove the ip on
nic removal, and will call dhcp again on nic hotplug)




nic hotplug with ip=auto:
-------------------------

--> add nic in pending state ----> find ip in ipam && write it in
pending ---> do the hotplug in qemu.

We need to handle the config revert to remove ip from ipam if the nic
hotplug is blocked in pending state(I never see this case until os
don't have pci_hotplug module loaded, but it's better to be carefull )

"


>>I am currently working on the SDN feature.  This is an initial review
>>of
>>the patch series and I am trying to make a strong case against
>>ephemeral
>>DHCP IP reservation.
>>
>>The current state of the patch series invokes the IPAM on every VM/CT
>>start/stop to add or remove the IP from the IPAM.
>>This triggers the dnsmasq config generation on the specific host with
>>only the MAC/IP mapping of that particular host.





From reading the discussion of the v1 patch series I understand this
approach tries to implement the ephemeral IP reservation strategy. From
off-list conversations with Stefan Hanreich, I agree that having
ephemeral IP reservation coordinated by the IPAM requires us to
re-implement DHCP functionality in the IPAM and heavily rely on syncing
between the different services.

To maintain reliable sync we need to hook into many different places
where the IPAM need to be queried.  Any issues with the implementation
may lead to IPAM and DHCP local config state running out of sync
causing
network issues duplicate multiple IPs.

Furthermore, every interaction with the IPAM requires a cluster-wide
lock on the IPAM. Having a central cluster-wide lock on every VM
start/stop/migrate will significantly limit parallel operations.  Event
starting two VMs in parallel will be limited by this central lock. At
boot trying to start many VMs (ideally as much in parallel as possible)
is limited by the central IPAM lock even further.

I argue that we shall not support ephemeral IPs altogether.
The alternative is to make all IPAM reservations persistent.

Using persistent IPs only reduces the interactions of VM/CTs with the
IPAM to a minimum of NIC joining a subnet and NIC leaving a subnet. I
am
deliberately not referring to VMs because a VM may be part of multiple
VNets or even multiple times in the same VNet (regardless if that is
sensible).

Cases the IPAM needs to be involved:

- NIC with DHCP enabled VNet is added to VM config
- NIC with DHCP enabled VNet is removed from VM config
- NIC is assigned to another Bridge
  can be treated as individual leave + join events

Cases that are explicitly not covered but may be added if desired:

- Manually assign an IP address on a NIC
  will not be automatically visible in the IPAM
- Manually change the MAC on a NIC
  don't do that > you are on your own.
  Not handled > change in IPAM manually

Once an IP is reserved via IPAM, the dnsmasq config can be generated
stateless and idempotent from the pve IPAM and is identical on all
nodes
regardless if a VM/CT actually resides on that node or is running or
stopped.  This is especially useful for VM migration because the IP
stays consistent without spacial considering.

Snapshot/revert, backup/restore, suspend/hibernate/resume cases are
automatically covered because the IP will already be reserved for that
MAC.

If the admin wants to change, the IP of a VM this can be done via the
IPAM API/UI which will have to be implemented separately.

A limitation of this approach vs dynamic IP reservation is that the IP
range on the subnet needs to be large enough to hold all IPs of all,
even stopped, VMs in that subnet. This is in contrast to default DHCP
functionality where only the number of actively running VMs is limited.
It should be enough to mention this in the docs.

I will further review the code an try to implement the aforementioned
approach.

Best regards,
Stefan Lendl

Stefan Hanreich <s.hanreich@proxmox.com> writes:

> This is a WIP patch series, since I will be gone for 3 weeks and
> wanted to
> share my current progress with the DHCP support for SDN.
> 
> This patch series adds support for automatically deploying dnsmasq as
> a DHCP
> server to a simple SDN Zone.
> 
> While certainly not 100% polished on some ends (looking at restarting
> systemd
> services in particular), the general idea behind the mechanism shows.
> I wanted
> to gather some feedback on how I approached designing the plugins and
> the
> config regeneration process before comitting to this design by
> creating an API
> and UI around it.
> 
> You need to install dnsmasq (and disable it afterwards):
> 
>   apt install dnsmasq && systemctl disable --now dnsmasq
> 
> 
> You can use the following example configuration for deploying a DHCP
> server in
> a SDN subnet:
> 
> /etc/pve/sdn/dhcp.cfg:
> 
>   dnsmasq: nat
> 
> 
> /etc/pve/sdn/zones.cfg:
> 
>   simple: DHCPNAT
>           ipam pve
> 
> 
> /etc/pve/sdn/vnets.cfg:
> 
>   vnet: dhcpnat
>           zone DHCPNAT
> 
> 
> /etc/pve/sdn/subnets.cfg:
> 
>   subnet: DHCPNAT-10.1.0.0-16
>           vnet dhcpnat
>           dhcp-dns-server 10.1.0.1
>           dhcp-range server=nat,start-address=10.1.0.100,end-
> address=10.1.0.200
>           gateway 10.1.0.1
>           snat 1
> 
> 
> Then apply the SDN configuration:
> 
>   pvesh set /cluster/sdn
> 
> You need to apply the SDN configuration once after adding the dhcp-
> range lines
> to the configuration, since the running configuration is used for
> managing
> DHCP. It will not work otherwise!
> 
> For testing it can be helpful to monitor the following files (e.g.
> with watch)
> to find out what is happening
>   * /etc/dnsmasq.d/<dhcp_id>/ethers (on each node)
>   * /etc/pve/priv/ipam.db
> 
> Changes from v1 -> v2:
>   * added hooks for handling DHCP when starting / stopping / .. VMs
> and CTs
>   * Get an IP from IPAM and register that IP in the DHCP server
>     (pve only for now)
>   * remove lease-time, since it is now infinite and managed by the VM
> lifecycle
>   * add hooks for setting & deleting DHCP mappings to DHCP plugins
>   * modified interface of the abstract class to reflect new
> requirements
>   * added helpers in existing SDN classes
>   * simplified DHCP configuration settings
> 
> 
> 
> pve-cluster:
> 
> Stefan Hanreich (1):
>   cluster files: add dhcp.cfg
> 
>  src/PVE/Cluster.pm  | 1 +
>  src/pmxcfs/status.c | 1 +
>  2 files changed, 2 insertions(+)
> 
> 
> pve-network:
> 
> Stefan Hanreich (6):
>   subnets: vnets: preparations for DHCP plugins
>   dhcp: add abstract class for DHCP plugins
>   dhcp: subnet: add DHCP options to subnet configuration
>   dhcp: add DHCP plugin for dnsmasq
>   ipam: Add helper methods for DHCP to PVE IPAM
>   dhcp: regenerate config for DHCP servers on reload
> 
>  debian/control                         |   1 +
>  src/PVE/Network/SDN.pm                 |  11 +-
>  src/PVE/Network/SDN/Dhcp.pm            | 192
> +++++++++++++++++++++++++
>  src/PVE/Network/SDN/Dhcp/Dnsmasq.pm    | 186
> ++++++++++++++++++++++++
>  src/PVE/Network/SDN/Dhcp/Makefile      |   8 ++
>  src/PVE/Network/SDN/Dhcp/Plugin.pm     |  83 +++++++++++
>  src/PVE/Network/SDN/Ipams/PVEPlugin.pm |  64 +++++++++
>  src/PVE/Network/SDN/Makefile           |   3 +-
>  src/PVE/Network/SDN/SubnetPlugin.pm    |  32 +++++
>  src/PVE/Network/SDN/Subnets.pm         |  43 ++++--
>  src/PVE/Network/SDN/Vnets.pm           |  27 ++--
>  11 files changed, 622 insertions(+), 28 deletions(-)
>  create mode 100644 src/PVE/Network/SDN/Dhcp.pm
>  create mode 100644 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
>  create mode 100644 src/PVE/Network/SDN/Dhcp/Makefile
>  create mode 100644 src/PVE/Network/SDN/Dhcp/Plugin.pm
> 
> 
> pve-manager:
> 
> Stefan Hanreich (1):
>   sdn: regenerate DHCP config on reload
> 
>  PVE/API2/Network.pm | 1 +
>  1 file changed, 1 insertion(+)
> 
> 
> qemu-server:
> 
> Stefan Hanreich (1):
>   sdn: dhcp: add DHCP setup to vm-network-scripts
> 
>  PVE/QemuServer.pm                 | 14 ++++++++++++++
>  vm-network-scripts/pve-bridge     |  3 +++
>  vm-network-scripts/pve-bridgedown | 19 +++++++++++++++++++
>  3 files changed, 36 insertions(+)
> 
> 
> pve-container:
> 
> Stefan Hanreich (1):
>   sdn: dhcp: setup DHCP mappings in LXC hooks
> 
>  src/PVE/LXC.pm            | 10 ++++++++++
>  src/lxc-pve-poststop-hook |  1 +
>  src/lxc-pve-prestart-hook |  9 +++++++++
>  3 files changed, 20 insertions(+)
> 
> 
> Summary over all repositories:
>   20 files changed, 681 insertions(+), 28 deletions(-)
> 
> --
> murpp v0.4.0
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://antiphishing.cetsi.fr/proxy/v3?i=d1l4NXNNaWE4SWZqU0dLWcuTfdxE
> d98NfWIp9dma5kY&r=MXJUa0FrUVJqc1UwYWxNZ-
> tuXduEO8AMVnCvYVMprCZ3oPilgy3nXcuJTOGH5iK84rVRg8cukFAROdxYRgFTTg&f=c2
> xMdVN4Smh2R2tOZDdIRKCk7WEocHpTPMerT1Q-
> Aq5qwr8l2xvAWuOGvFsV3frp2oSAgxNUQCpJDHp2iUmTWg&u=https%3A//lists.prox
> mox.com/cgi-bin/mailman/listinfo/pve-devel&k=fjzS


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://antiphishing.cetsi.fr/proxy/v3?i=d1l4NXNNaWE4SWZqU0dLWcuTfdxEd9
8NfWIp9dma5kY&r=MXJUa0FrUVJqc1UwYWxNZ-
tuXduEO8AMVnCvYVMprCZ3oPilgy3nXcuJTOGH5iK84rVRg8cukFAROdxYRgFTTg&f=c2xM
dVN4Smh2R2tOZDdIRKCk7WEocHpTPMerT1Q-
Aq5qwr8l2xvAWuOGvFsV3frp2oSAgxNUQCpJDHp2iUmTWg&u=https%3A//lists.proxmo
x.com/cgi-bin/mailman/listinfo/pve-devel&k=fjzS

Re: [pve-devel] [WIP v2 cluster/network/manager/qemu-server/container 00/10] Add support for DHCP servers to SDN

[Stefan Lendl]

PS: Sorry for double posting. this mail was sent with invalid
In-Reply-To and References Headers.

I sent it again with the correct Headers after I managed to correctly
setup my mail client.

Stefan Lendl <s.lendl@proxmox.com> writes:

> I am currently working on the SDN feature.  This is an initial review of
> the patch series and I am trying to make a strong case against ephemeral
> DHCP IP reservation.
>
> The current state of the patch series invokes the IPAM on every VM/CT
> start/stop to add or remove the IP from the IPAM.
> This triggers the dnsmasq config generation on the specific host with
> only the MAC/IP mapping of that particular host.
>
> From reading the discussion of the v1 patch series I understand this
> approach tries to implement the ephemeral IP reservation strategy. From
> off-list conversations with Stefan Hanreich, I agree that having
> ephemeral IP reservation coordinated by the IPAM requires us to
> re-implement DHCP functionality in the IPAM and heavily rely on syncing
> between the different services.
>
> To maintain reliable sync we need to hook into many different places
> where the IPAM need to be queried.  Any issues with the implementation
> may lead to IPAM and DHCP local config state running out of sync causing
> network issues duplicate multiple IPs.
>
> Furthermore, every interaction with the IPAM requires a cluster-wide
> lock on the IPAM. Having a central cluster-wide lock on every VM
> start/stop/migrate will significantly limit parallel operations.  Event
> starting two VMs in parallel will be limited by this central lock. At
> boot trying to start many VMs (ideally as much in parallel as possible)
> is limited by the central IPAM lock even further.
>
> I argue that we shall not support ephemeral IPs altogether.
> The alternative is to make all IPAM reservations persistent.
>
> Using persistent IPs only reduces the interactions of VM/CTs with the
> IPAM to a minimum of NIC joining a subnet and NIC leaving a subnet. I am
> deliberately not referring to VMs because a VM may be part of multiple
> VNets or even multiple times in the same VNet (regardless if that is
> sensible).
>
> Cases the IPAM needs to be involved:
>
> - NIC with DHCP enabled VNet is added to VM config
> - NIC with DHCP enabled VNet is removed from VM config
> - NIC is assigned to another Bridge
>   can be treated as individual leave + join events
>
> Cases that are explicitly not covered but may be added if desired:
>
> - Manually assign an IP address on a NIC
>   will not be automatically visible in the IPAM
> - Manually change the MAC on a NIC
>   don't do that > you are on your own.
>   Not handled > change in IPAM manually
>
> Once an IP is reserved via IPAM, the dnsmasq config can be generated
> stateless and idempotent from the pve IPAM and is identical on all nodes
> regardless if a VM/CT actually resides on that node or is running or
> stopped.  This is especially useful for VM migration because the IP
> stays consistent without spacial considering.
>
> Snapshot/revert, backup/restore, suspend/hibernate/resume cases are
> automatically covered because the IP will already be reserved for that
> MAC.
>
> If the admin wants to change, the IP of a VM this can be done via the
> IPAM API/UI which will have to be implemented separately.
>
> A limitation of this approach vs dynamic IP reservation is that the IP
> range on the subnet needs to be large enough to hold all IPs of all,
> even stopped, VMs in that subnet. This is in contrast to default DHCP
> functionality where only the number of actively running VMs is limited.
> It should be enough to mention this in the docs.
>
> I will further review the code an try to implement the aforementioned
> approach.
>
> Best regards,
> Stefan Lendl
>
> Stefan Hanreich <s.hanreich@proxmox.com> writes:
>
>> This is a WIP patch series, since I will be gone for 3 weeks and wanted to
>> share my current progress with the DHCP support for SDN.
>>
>> This patch series adds support for automatically deploying dnsmasq as a DHCP
>> server to a simple SDN Zone.
>>
>> While certainly not 100% polished on some ends (looking at restarting systemd
>> services in particular), the general idea behind the mechanism shows. I wanted
>> to gather some feedback on how I approached designing the plugins and the
>> config regeneration process before comitting to this design by creating an API
>> and UI around it.
>>
>> You need to install dnsmasq (and disable it afterwards):
>>
>>   apt install dnsmasq && systemctl disable --now dnsmasq
>>
>>
>> You can use the following example configuration for deploying a DHCP server in
>> a SDN subnet:
>>
>> /etc/pve/sdn/dhcp.cfg:
>>
>>   dnsmasq: nat
>>
>>
>> /etc/pve/sdn/zones.cfg:
>>
>>   simple: DHCPNAT
>>           ipam pve
>>
>>
>> /etc/pve/sdn/vnets.cfg:
>>
>>   vnet: dhcpnat
>>           zone DHCPNAT
>>
>>
>> /etc/pve/sdn/subnets.cfg:
>>
>>   subnet: DHCPNAT-10.1.0.0-16
>>           vnet dhcpnat
>>           dhcp-dns-server 10.1.0.1
>>           dhcp-range server=nat,start-address=10.1.0.100,end-address=10.1.0.200
>>           gateway 10.1.0.1
>>           snat 1
>>
>>
>> Then apply the SDN configuration:
>>
>>   pvesh set /cluster/sdn
>>
>> You need to apply the SDN configuration once after adding the dhcp-range lines
>> to the configuration, since the running configuration is used for managing
>> DHCP. It will not work otherwise!
>>
>> For testing it can be helpful to monitor the following files (e.g. with watch)
>> to find out what is happening
>>   * /etc/dnsmasq.d/<dhcp_id>/ethers (on each node)
>>   * /etc/pve/priv/ipam.db
>>
>> Changes from v1 -> v2:
>>   * added hooks for handling DHCP when starting / stopping / .. VMs and CTs
>>   * Get an IP from IPAM and register that IP in the DHCP server
>>     (pve only for now)
>>   * remove lease-time, since it is now infinite and managed by the VM lifecycle
>>   * add hooks for setting & deleting DHCP mappings to DHCP plugins
>>   * modified interface of the abstract class to reflect new requirements
>>   * added helpers in existing SDN classes
>>   * simplified DHCP configuration settings
>>
>>
>>
>> pve-cluster:
>>
>> Stefan Hanreich (1):
>>   cluster files: add dhcp.cfg
>>
>>  src/PVE/Cluster.pm  | 1 +
>>  src/pmxcfs/status.c | 1 +
>>  2 files changed, 2 insertions(+)
>>
>>
>> pve-network:
>>
>> Stefan Hanreich (6):
>>   subnets: vnets: preparations for DHCP plugins
>>   dhcp: add abstract class for DHCP plugins
>>   dhcp: subnet: add DHCP options to subnet configuration
>>   dhcp: add DHCP plugin for dnsmasq
>>   ipam: Add helper methods for DHCP to PVE IPAM
>>   dhcp: regenerate config for DHCP servers on reload
>>
>>  debian/control                         |   1 +
>>  src/PVE/Network/SDN.pm                 |  11 +-
>>  src/PVE/Network/SDN/Dhcp.pm            | 192 +++++++++++++++++++++++++
>>  src/PVE/Network/SDN/Dhcp/Dnsmasq.pm    | 186 ++++++++++++++++++++++++
>>  src/PVE/Network/SDN/Dhcp/Makefile      |   8 ++
>>  src/PVE/Network/SDN/Dhcp/Plugin.pm     |  83 +++++++++++
>>  src/PVE/Network/SDN/Ipams/PVEPlugin.pm |  64 +++++++++
>>  src/PVE/Network/SDN/Makefile           |   3 +-
>>  src/PVE/Network/SDN/SubnetPlugin.pm    |  32 +++++
>>  src/PVE/Network/SDN/Subnets.pm         |  43 ++++--
>>  src/PVE/Network/SDN/Vnets.pm           |  27 ++--
>>  11 files changed, 622 insertions(+), 28 deletions(-)
>>  create mode 100644 src/PVE/Network/SDN/Dhcp.pm
>>  create mode 100644 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
>>  create mode 100644 src/PVE/Network/SDN/Dhcp/Makefile
>>  create mode 100644 src/PVE/Network/SDN/Dhcp/Plugin.pm
>>
>>
>> pve-manager:
>>
>> Stefan Hanreich (1):
>>   sdn: regenerate DHCP config on reload
>>
>>  PVE/API2/Network.pm | 1 +
>>  1 file changed, 1 insertion(+)
>>
>>
>> qemu-server:
>>
>> Stefan Hanreich (1):
>>   sdn: dhcp: add DHCP setup to vm-network-scripts
>>
>>  PVE/QemuServer.pm                 | 14 ++++++++++++++
>>  vm-network-scripts/pve-bridge     |  3 +++
>>  vm-network-scripts/pve-bridgedown | 19 +++++++++++++++++++
>>  3 files changed, 36 insertions(+)
>>
>>
>> pve-container:
>>
>> Stefan Hanreich (1):
>>   sdn: dhcp: setup DHCP mappings in LXC hooks
>>
>>  src/PVE/LXC.pm            | 10 ++++++++++
>>  src/lxc-pve-poststop-hook |  1 +
>>  src/lxc-pve-prestart-hook |  9 +++++++++
>>  3 files changed, 20 insertions(+)
>>
>>
>> Summary over all repositories:
>>   20 files changed, 681 insertions(+), 28 deletions(-)
>>
>> --
>> murpp v0.4.0
>>
>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [WIP v2 cluster/network/manager/qemu-server/container 00/10] Add support for DHCP servers to SDN

[Stefan Lendl]

I am currently working on the SDN feature.  This is an initial review of
the patch series and I am trying to make a strong case against ephemeral
DHCP IP reservation.

The current state of the patch series invokes the IPAM on every VM/CT
start/stop to add or remove the IP from the IPAM.
This triggers the dnsmasq config generation on the specific host with
only the MAC/IP mapping of that particular host.

From reading the discussion of the v1 patch series I understand this
approach tries to implement the ephemeral IP reservation strategy. From
off-list conversations with Stefan Hanreich, I agree that having
ephemeral IP reservation coordinated by the IPAM requires us to
re-implement DHCP functionality in the IPAM and heavily rely on syncing
between the different services.

To maintain reliable sync we need to hook into many different places
where the IPAM need to be queried.  Any issues with the implementation
may lead to IPAM and DHCP local config state running out of sync causing
network issues duplicate multiple IPs.

Furthermore, every interaction with the IPAM requires a cluster-wide
lock on the IPAM. Having a central cluster-wide lock on every VM
start/stop/migrate will significantly limit parallel operations.  Event
starting two VMs in parallel will be limited by this central lock. At
boot trying to start many VMs (ideally as much in parallel as possible)
is limited by the central IPAM lock even further.

I argue that we shall not support ephemeral IPs altogether.
The alternative is to make all IPAM reservations persistent.

Using persistent IPs only reduces the interactions of VM/CTs with the
IPAM to a minimum of NIC joining a subnet and NIC leaving a subnet. I am
deliberately not referring to VMs because a VM may be part of multiple
VNets or even multiple times in the same VNet (regardless if that is
sensible).

Cases the IPAM needs to be involved:

- NIC with DHCP enabled VNet is added to VM config
- NIC with DHCP enabled VNet is removed from VM config
- NIC is assigned to another Bridge
  can be treated as individual leave + join events

Cases that are explicitly not covered but may be added if desired:

- Manually assign an IP address on a NIC
  will not be automatically visible in the IPAM
- Manually change the MAC on a NIC
  don't do that > you are on your own.
  Not handled > change in IPAM manually

Once an IP is reserved via IPAM, the dnsmasq config can be generated
stateless and idempotent from the pve IPAM and is identical on all nodes
regardless if a VM/CT actually resides on that node or is running or
stopped.  This is especially useful for VM migration because the IP
stays consistent without spacial considering.

Snapshot/revert, backup/restore, suspend/hibernate/resume cases are
automatically covered because the IP will already be reserved for that
MAC.

If the admin wants to change, the IP of a VM this can be done via the
IPAM API/UI which will have to be implemented separately.

A limitation of this approach vs dynamic IP reservation is that the IP
range on the subnet needs to be large enough to hold all IPs of all,
even stopped, VMs in that subnet. This is in contrast to default DHCP
functionality where only the number of actively running VMs is limited.
It should be enough to mention this in the docs.

I will further review the code an try to implement the aforementioned
approach.

Best regards,
Stefan Lendl

Stefan Hanreich <s.hanreich@proxmox.com> writes:

> This is a WIP patch series, since I will be gone for 3 weeks and wanted to
> share my current progress with the DHCP support for SDN.
>
> This patch series adds support for automatically deploying dnsmasq as a DHCP
> server to a simple SDN Zone.
>
> While certainly not 100% polished on some ends (looking at restarting systemd
> services in particular), the general idea behind the mechanism shows. I wanted
> to gather some feedback on how I approached designing the plugins and the
> config regeneration process before comitting to this design by creating an API
> and UI around it.
>
> You need to install dnsmasq (and disable it afterwards):
>
>   apt install dnsmasq && systemctl disable --now dnsmasq
>
>
> You can use the following example configuration for deploying a DHCP server in
> a SDN subnet:
>
> /etc/pve/sdn/dhcp.cfg:
>
>   dnsmasq: nat
>
>
> /etc/pve/sdn/zones.cfg:
>
>   simple: DHCPNAT
>           ipam pve
>
>
> /etc/pve/sdn/vnets.cfg:
>
>   vnet: dhcpnat
>           zone DHCPNAT
>
>
> /etc/pve/sdn/subnets.cfg:
>
>   subnet: DHCPNAT-10.1.0.0-16
>           vnet dhcpnat
>           dhcp-dns-server 10.1.0.1
>           dhcp-range server=nat,start-address=10.1.0.100,end-address=10.1.0.200
>           gateway 10.1.0.1
>           snat 1
>
>
> Then apply the SDN configuration:
>
>   pvesh set /cluster/sdn
>
> You need to apply the SDN configuration once after adding the dhcp-range lines
> to the configuration, since the running configuration is used for managing
> DHCP. It will not work otherwise!
>
> For testing it can be helpful to monitor the following files (e.g. with watch)
> to find out what is happening
>   * /etc/dnsmasq.d/<dhcp_id>/ethers (on each node)
>   * /etc/pve/priv/ipam.db
>
> Changes from v1 -> v2:
>   * added hooks for handling DHCP when starting / stopping / .. VMs and CTs
>   * Get an IP from IPAM and register that IP in the DHCP server
>     (pve only for now)
>   * remove lease-time, since it is now infinite and managed by the VM lifecycle
>   * add hooks for setting & deleting DHCP mappings to DHCP plugins
>   * modified interface of the abstract class to reflect new requirements
>   * added helpers in existing SDN classes
>   * simplified DHCP configuration settings
>
>
>
> pve-cluster:
>
> Stefan Hanreich (1):
>   cluster files: add dhcp.cfg
>
>  src/PVE/Cluster.pm  | 1 +
>  src/pmxcfs/status.c | 1 +
>  2 files changed, 2 insertions(+)
>
>
> pve-network:
>
> Stefan Hanreich (6):
>   subnets: vnets: preparations for DHCP plugins
>   dhcp: add abstract class for DHCP plugins
>   dhcp: subnet: add DHCP options to subnet configuration
>   dhcp: add DHCP plugin for dnsmasq
>   ipam: Add helper methods for DHCP to PVE IPAM
>   dhcp: regenerate config for DHCP servers on reload
>
>  debian/control                         |   1 +
>  src/PVE/Network/SDN.pm                 |  11 +-
>  src/PVE/Network/SDN/Dhcp.pm            | 192 +++++++++++++++++++++++++
>  src/PVE/Network/SDN/Dhcp/Dnsmasq.pm    | 186 ++++++++++++++++++++++++
>  src/PVE/Network/SDN/Dhcp/Makefile      |   8 ++
>  src/PVE/Network/SDN/Dhcp/Plugin.pm     |  83 +++++++++++
>  src/PVE/Network/SDN/Ipams/PVEPlugin.pm |  64 +++++++++
>  src/PVE/Network/SDN/Makefile           |   3 +-
>  src/PVE/Network/SDN/SubnetPlugin.pm    |  32 +++++
>  src/PVE/Network/SDN/Subnets.pm         |  43 ++++--
>  src/PVE/Network/SDN/Vnets.pm           |  27 ++--
>  11 files changed, 622 insertions(+), 28 deletions(-)
>  create mode 100644 src/PVE/Network/SDN/Dhcp.pm
>  create mode 100644 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
>  create mode 100644 src/PVE/Network/SDN/Dhcp/Makefile
>  create mode 100644 src/PVE/Network/SDN/Dhcp/Plugin.pm
>
>
> pve-manager:
>
> Stefan Hanreich (1):
>   sdn: regenerate DHCP config on reload
>
>  PVE/API2/Network.pm | 1 +
>  1 file changed, 1 insertion(+)
>
>
> qemu-server:
>
> Stefan Hanreich (1):
>   sdn: dhcp: add DHCP setup to vm-network-scripts
>
>  PVE/QemuServer.pm                 | 14 ++++++++++++++
>  vm-network-scripts/pve-bridge     |  3 +++
>  vm-network-scripts/pve-bridgedown | 19 +++++++++++++++++++
>  3 files changed, 36 insertions(+)
>
>
> pve-container:
>
> Stefan Hanreich (1):
>   sdn: dhcp: setup DHCP mappings in LXC hooks
>
>  src/PVE/LXC.pm            | 10 ++++++++++
>  src/lxc-pve-poststop-hook |  1 +
>  src/lxc-pve-prestart-hook |  9 +++++++++
>  3 files changed, 20 insertions(+)
>
>
> Summary over all repositories:
>   20 files changed, 681 insertions(+), 28 deletions(-)
>
> --
> murpp v0.4.0
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel