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

[pve-devel] [PATCH container 0/3] 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 (3):
  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

 src/PVE/LXC.pm        | 26 ++++++++++++++++----------
 src/PVE/LXC/Config.pm |  6 ++++--
 2 files changed, 20 insertions(+), 12 deletions(-)


Summary over all repositories:
  2 files changed, 20 insertions(+), 12 deletions(-)

-- 
murpp v0.4.0

[pve-devel] [PATCH pve-container 1/3] 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 pve-container 2/3] 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 pve-container 3/3] 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 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index c239715..071faca 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -2771,7 +2771,7 @@ 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);
     }
@@ -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