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

all lists on lists.proxmox.com
 help / color / mirror / Atom feed

* [pve-devel] [PATCH v2 container 0/4] Bugfixes for DHCP
@ 2023-11-20 19:19 Stefan Hanreich
  2023-11-20 19:19 ` [pve-devel] [PATCH v2 pve-container 1/4] hotplug network: Only change IPAM when MAC or bridge changes Stefan Hanreich
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Stefan Hanreich @ 2023-11-20 19:19 UTC (permalink / raw)
  To: pve-devel

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




^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pve-devel] [PATCH v2 pve-container 1/4] hotplug network: Only change IPAM when MAC or bridge changes
  2023-11-20 19:19 [pve-devel] [PATCH v2 container 0/4] Bugfixes for DHCP Stefan Hanreich
@ 2023-11-20 19:19 ` Stefan Hanreich
  2023-11-20 19:19 ` [pve-devel] [PATCH v2 pve-container 2/4] network: Do not always reserve new IP in IPAM Stefan Hanreich
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Stefan Hanreich @ 2023-11-20 19:19 UTC (permalink / raw)
  To: pve-devel

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




^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pve-devel] [PATCH v2 pve-container 2/4] network: Do not always reserve new IP in IPAM
  2023-11-20 19:19 [pve-devel] [PATCH v2 container 0/4] Bugfixes for DHCP Stefan Hanreich
  2023-11-20 19:19 ` [pve-devel] [PATCH v2 pve-container 1/4] hotplug network: Only change IPAM when MAC or bridge changes Stefan Hanreich
@ 2023-11-20 19:19 ` Stefan Hanreich
  2023-11-20 19:19 ` [pve-devel] [PATCH v2 pve-container 3/4] config: Use LXC Config instead of QemuServer for parsing net Stefan Hanreich
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Stefan Hanreich @ 2023-11-20 19:19 UTC (permalink / raw)
  To: pve-devel

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




^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pve-devel] [PATCH v2 pve-container 3/4] config: Use LXC Config instead of QemuServer for parsing net
  2023-11-20 19:19 [pve-devel] [PATCH v2 container 0/4] Bugfixes for DHCP Stefan Hanreich
  2023-11-20 19:19 ` [pve-devel] [PATCH v2 pve-container 1/4] hotplug network: Only change IPAM when MAC or bridge changes Stefan Hanreich
  2023-11-20 19:19 ` [pve-devel] [PATCH v2 pve-container 2/4] network: Do not always reserve new IP in IPAM Stefan Hanreich
@ 2023-11-20 19:19 ` Stefan Hanreich
  2023-11-20 19:19 ` [pve-devel] [PATCH v2 pve-container 4/4] create: Do not call create_ifaces_ipams_ips Stefan Hanreich
  2023-11-22 13:42 ` [pve-devel] applied: [PATCH v2 container 0/4] Bugfixes for DHCP Thomas Lamprecht
  4 siblings, 0 replies; 6+ messages in thread
From: Stefan Hanreich @ 2023-11-20 19:19 UTC (permalink / raw)
  To: pve-devel

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




^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pve-devel] [PATCH v2 pve-container 4/4] create: Do not call create_ifaces_ipams_ips
  2023-11-20 19:19 [pve-devel] [PATCH v2 container 0/4] Bugfixes for DHCP Stefan Hanreich
                   ` (2 preceding siblings ...)
  2023-11-20 19:19 ` [pve-devel] [PATCH v2 pve-container 3/4] config: Use LXC Config instead of QemuServer for parsing net Stefan Hanreich
@ 2023-11-20 19:19 ` Stefan Hanreich
  2023-11-22 13:42 ` [pve-devel] applied: [PATCH v2 container 0/4] Bugfixes for DHCP Thomas Lamprecht
  4 siblings, 0 replies; 6+ messages in thread
From: Stefan Hanreich @ 2023-11-20 19:19 UTC (permalink / raw)
  To: pve-devel

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




^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pve-devel] applied:  [PATCH v2 container 0/4] Bugfixes for DHCP
  2023-11-20 19:19 [pve-devel] [PATCH v2 container 0/4] Bugfixes for DHCP Stefan Hanreich
                   ` (3 preceding siblings ...)
  2023-11-20 19:19 ` [pve-devel] [PATCH v2 pve-container 4/4] create: Do not call create_ifaces_ipams_ips Stefan Hanreich
@ 2023-11-22 13:42 ` Thomas Lamprecht
  4 siblings, 0 replies; 6+ messages in thread
From: Thomas Lamprecht @ 2023-11-22 13:42 UTC (permalink / raw)
  To: Proxmox VE development discussion, Stefan Hanreich

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




^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-11-22 13:42 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-20 19:19 [pve-devel] [PATCH v2 container 0/4] Bugfixes for DHCP Stefan Hanreich
2023-11-20 19:19 ` [pve-devel] [PATCH v2 pve-container 1/4] hotplug network: Only change IPAM when MAC or bridge changes Stefan Hanreich
2023-11-20 19:19 ` [pve-devel] [PATCH v2 pve-container 2/4] network: Do not always reserve new IP in IPAM Stefan Hanreich
2023-11-20 19:19 ` [pve-devel] [PATCH v2 pve-container 3/4] config: Use LXC Config instead of QemuServer for parsing net Stefan Hanreich
2023-11-20 19:19 ` [pve-devel] [PATCH v2 pve-container 4/4] create: Do not call create_ifaces_ipams_ips Stefan Hanreich
2023-11-22 13:42 ` [pve-devel] applied: [PATCH v2 container 0/4] Bugfixes for DHCP Thomas Lamprecht

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.

Service provided by Proxmox Server Solutions GmbH | Privacy | Legal