[pve-devel] [PATCH v2 container 0/4] Bugfixes for DHCP

[pve-devel] [PATCH v2 container 0/4] Bugfixes for DHCP

[Stefan Hanreich]

When editing a NIC there was a possibility of duplicate IPAM entries being
generated. This should be fixed with this patch series.

Additionally when creating vNICs for a container there were multiple warnings
in the log due to the LXC module trying to use the QemuServer module for
parsing net lines in the container configuration.



pve-container:

Stefan Hanreich (4):
  hotplug network: Only change IPAM when MAC or bridge changes
  network: Do not always reserve new IP in IPAM
  config: Use LXC Config instead of QemuServer for parsing net
  create: Do not call create_ifaces_ipams_ips

 src/PVE/API2/LXC.pm   |  1 -
 src/PVE/LXC.pm        | 28 +++++++++++++++++-----------
 src/PVE/LXC/Config.pm |  6 ++++--
 3 files changed, 21 insertions(+), 14 deletions(-)


Summary over all repositories:
  3 files changed, 21 insertions(+), 14 deletions(-)

-- 
murpp v0.4.0

[pve-devel] [PATCH v2 pve-container 1/4] hotplug network: Only change IPAM when MAC or bridge changes

[Stefan Hanreich]

Currently a new IPAM entry is created everytime a NIC config changes.
When editing properties other than MAC or Bridge this could lead to
duplicated entries in the IPAM. Only reserve a new IP when the bridge
or MAC changes or the NIC is completely new.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/LXC.pm | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 2dad83d..c239715 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -965,9 +965,12 @@ sub update_net {
 
 	    PVE::Network::veth_delete($veth);
 
-	    if ($have_sdn) {
+	    if ($have_sdn && safe_string_ne($oldnet->{hwaddr}, $newnet->{hwaddr})) {
 		eval { PVE::Network::SDN::Vnets::del_ips_from_mac($oldnet->{bridge}, $oldnet->{hwaddr}, $conf->{hostname}) };
 		warn $@ if $@;
+
+		PVE::Network::SDN::Vnets::add_next_free_cidr($newnet->{bridge}, $conf->{hostname}, $newnet->{hwaddr}, $vmid, undef, 1);
+		PVE::Network::SDN::Vnets::add_dhcp_mapping($newnet->{bridge}, $newnet->{hwaddr});
 	    }
 
 	    delete $conf->{$opt};
@@ -976,13 +979,15 @@ sub update_net {
 	    hotplug_net($vmid, $conf, $opt, $newnet, $netid);
 
 	} else {
-	    if (safe_string_ne($oldnet->{bridge}, $newnet->{bridge}) ||
+	    my $bridge_changed = safe_string_ne($oldnet->{bridge}, $newnet->{bridge});
+
+	    if ($bridge_changed ||
 		safe_num_ne($oldnet->{tag}, $newnet->{tag}) ||
 		safe_num_ne($oldnet->{firewall}, $newnet->{firewall}) ||
 		safe_boolean_ne($oldnet->{link_down}, $newnet->{link_down})
 	    ) {
-
 		if ($oldnet->{bridge}) {
+		    my $oldbridge = $oldnet->{bridge};
 
 		    PVE::Network::tap_unplug($veth);
 		    foreach (qw(bridge tag firewall)) {
@@ -991,13 +996,13 @@ sub update_net {
 		    $conf->{$opt} = PVE::LXC::Config->print_lxc_network($oldnet);
 		    PVE::LXC::Config->write_config($vmid, $conf);
 
-		    if ($have_sdn) {
-			eval { PVE::Network::SDN::Vnets::del_ips_from_mac($oldnet->{bridge}, $oldnet->{hwaddr}, $conf->{hostname}) };
+		    if ($have_sdn && $bridge_changed) {
+			eval { PVE::Network::SDN::Vnets::del_ips_from_mac($oldbridge, $oldnet->{hwaddr}, $conf->{hostname}) };
 			warn $@ if $@;
 		    }
 		}
 
-		if ($have_sdn) {
+		if ($have_sdn && $bridge_changed) {
 		    PVE::Network::SDN::Vnets::add_next_free_cidr($newnet->{bridge}, $conf->{hostname}, $newnet->{hwaddr}, $vmid, undef, 1);
 		}
 		PVE::LXC::net_tap_plug($veth, $newnet);
@@ -1016,6 +1021,9 @@ sub update_net {
 	    PVE::LXC::Config->write_config($vmid, $conf);
 	}
     } else {
+	PVE::Network::SDN::Vnets::add_next_free_cidr($newnet->{bridge}, $conf->{hostname}, $newnet->{hwaddr}, $vmid, undef, 1);
+	PVE::Network::SDN::Vnets::add_dhcp_mapping($newnet->{bridge}, $newnet->{hwaddr});
+
 	hotplug_net($vmid, $conf, $opt, $newnet, $netid);
     }
 
@@ -1030,8 +1038,6 @@ sub hotplug_net {
     my $eth = $newnet->{name};
 
     if ($have_sdn) {
-	PVE::Network::SDN::Vnets::add_next_free_cidr($newnet->{bridge}, $conf->{hostname}, $newnet->{hwaddr}, $vmid, undef, 1);
-	PVE::Network::SDN::Vnets::add_dhcp_mapping($newnet->{bridge}, $newnet->{hwaddr});
 	PVE::Network::SDN::Zones::veth_create($veth, $vethpeer, $newnet->{bridge}, $newnet->{hwaddr});
     } else {
 	PVE::Network::veth_create($veth, $vethpeer, $newnet->{bridge}, $newnet->{hwaddr});
-- 
2.39.2

[pve-devel] [PATCH v2 pve-container 2/4] network: Do not always reserve new IP in IPAM

[Stefan Hanreich]

Currently when updating the network configuration of a container, SDN
would always create a new entry in the IPAM. Only create a new entry
when the bridge or MAC changes or the NIC is completely new.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/LXC/Config.pm | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index 823a2b9..53662b7 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -1500,13 +1500,15 @@ sub vmconfig_apply_pending {
 		my $net = $class->parse_lxc_network($conf->{pending}->{$opt});
 		$conf->{pending}->{$opt} = $class->print_lxc_network($net);
 		if ($have_sdn) {
-		    if($conf->{$opt}) {
+		    if ($conf->{$opt}) {
 			my $old_net = $class->parse_lxc_network($conf->{$opt});
 			if ($old_net->{bridge} ne $net->{bridge} || $old_net->{hwaddr} ne $net->{hwaddr}) {
 			    PVE::Network::SDN::Vnets::del_ips_from_mac($old_net->{bridge}, $old_net->{hwaddr}, $conf->{name});
+			    PVE::Network::SDN::Vnets::add_next_free_cidr($net->{bridge}, $conf->{hostname}, $net->{hwaddr}, $vmid, undef, 1);
 			}
+		    } else {
+			PVE::Network::SDN::Vnets::add_next_free_cidr($net->{bridge}, $conf->{hostname}, $net->{hwaddr}, $vmid, undef, 1);
 		    }
-		    PVE::Network::SDN::Vnets::add_next_free_cidr($net->{bridge}, $conf->{hostname}, $net->{hwaddr}, $vmid, undef, 1);
 		}
 	    }
 	};
-- 
2.39.2

[pve-devel] [PATCH v2 pve-container 3/4] config: Use LXC Config instead of QemuServer for parsing net

[Stefan Hanreich]

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/LXC.pm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index c239715..847b8c8 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -2771,9 +2771,9 @@ sub create_ifaces_ipams_ips {
 
     for my $opt (keys %$conf) {
 	next if $opt !~ m/^net(\d+)$/;
-	my $net = PVE::QemuServer::parse_net($conf->{$opt});
+	my $net = PVE::LXC::Config->parse_lxc_network($conf->{$opt});
 	next if $net->{type} ne 'veth';
-        PVE::Network::SDN::Vnets::add_next_free_cidr($net->{bridge}, $conf->{hostname}, $net->{hwaddr}, $vmid, undef, 1);
+	PVE::Network::SDN::Vnets::add_next_free_cidr($net->{bridge}, $conf->{hostname}, $net->{hwaddr}, $vmid, undef, 1);
     }
 }
 
@@ -2784,7 +2784,7 @@ sub delete_ifaces_ipams_ips {
 
     for my $opt (keys %$conf) {
 	next if $opt !~ m/^net(\d+)$/;
-	my $net = PVE::QemuServer::parse_net($conf->{$opt});
+	my $net = PVE::LXC::Config->parse_lxc_network($conf->{$opt});
 	eval { PVE::Network::SDN::Vnets::del_ips_from_mac($net->{bridge}, $net->{hwaddr}, $conf->{hostname}) };
 	warn $@ if $@;
     }
-- 
2.39.2

[pve-devel] [PATCH v2 pve-container 4/4] create: Do not call create_ifaces_ipams_ips

[Stefan Hanreich]

Since create_vm already calls update_pct_config, which in turn calls
vmconfig_apply_pending we do not need to explicitly create the IPAM
entries when creating a container from scratch.

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

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index ee4fdca..dabcc14 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -479,7 +479,6 @@ __PACKAGE__->register_method({
 			my $lxc_setup = PVE::LXC::Setup->new($conf, $rootdir);
 			$lxc_setup->template_fixup($conf);
 		    } else {
-			PVE::LXC::create_ifaces_ipams_ips($conf, $vmid);
 			my $lxc_setup = PVE::LXC::Setup->new($conf, $rootdir); # detect OS
 			PVE::LXC::Config->write_config($vmid, $conf); # safe config (after OS detection)
 			$lxc_setup->post_create_hook($password, $ssh_keys);
-- 
2.39.2

[pve-devel] applied: [PATCH v2 container 0/4] Bugfixes for DHCP

[Thomas Lamprecht]

Am 20/11/2023 um 20:19 schrieb Stefan Hanreich:
> When editing a NIC there was a possibility of duplicate IPAM entries being
> generated. This should be fixed with this patch series.
> 
> Additionally when creating vNICs for a container there were multiple warnings
> in the log due to the LXC module trying to use the QemuServer module for
> parsing net lines in the container configuration.
> 
> 
> 
> pve-container:
> 
> Stefan Hanreich (4):
>   hotplug network: Only change IPAM when MAC or bridge changes
>   network: Do not always reserve new IP in IPAM
>   config: Use LXC Config instead of QemuServer for parsing net
>   create: Do not call create_ifaces_ipams_ips
> 
>  src/PVE/API2/LXC.pm   |  1 -
>  src/PVE/LXC.pm        | 28 +++++++++++++++++-----------
>  src/PVE/LXC/Config.pm |  6 ++++--
>  3 files changed, 21 insertions(+), 14 deletions(-)
> 
> 
> Summary over all repositories:
>   3 files changed, 21 insertions(+), 14 deletions(-)
> 

for the record: these patches got already applied by Wolfgang:

https://git.proxmox.com/?p=pve-container.git;a=commit;h=32de2c46c892f08c81e3cf801835a2c136ea1103
https://git.proxmox.com/?p=pve-container.git;a=commit;h=89d74337349de9e7cd48b428360a9f7ad72edbba
https://git.proxmox.com/?p=pve-container.git;a=commit;h=0b06b9fbfbc85e4c921af24cf83cf3e5e54faac3
https://git.proxmox.com/?p=pve-container.git;a=commit;h=68a3a4257904be3f7e48490f659fd903385542a8