* [pve-devel] [PATCH pve-network] ipam: improve update / delete behavior
@ 2023-11-22 12:29 Stefan Hanreich
2023-11-22 12:29 ` [pve-devel] [PATCH pve-manager] ipam: send ip to delete endpoint Stefan Hanreich
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Stefan Hanreich @ 2023-11-22 12:29 UTC (permalink / raw)
To: pve-devel
Currently when updating or deleting a mapping in the IPAM we would
delete all existing entries in the IPAM with that mac address. Now we
only delete the specific entry we are updating / deleting.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
src/PVE/API2/Network/SDN/Ips.pm | 20 +++++++++++++-------
src/PVE/Network/SDN/Subnets.pm | 3 +++
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/src/PVE/API2/Network/SDN/Ips.pm b/src/PVE/API2/Network/SDN/Ips.pm
index 6989b9b..0003b2a 100644
--- a/src/PVE/API2/Network/SDN/Ips.pm
+++ b/src/PVE/API2/Network/SDN/Ips.pm
@@ -28,6 +28,11 @@ __PACKAGE__->register_method ({
zone => get_standard_option('pve-sdn-zone-id'),
vnet => get_standard_option('pve-sdn-vnet-id'),
mac => get_standard_option('mac-addr'),
+ ip => {
+ type => 'string',
+ format => 'ip',
+ description => 'The IP address to delete',
+ },
},
},
returns => { type => 'null' },
@@ -36,13 +41,12 @@ __PACKAGE__->register_method ({
my $vnet = extract_param($param, 'vnet');
my $mac = extract_param($param, 'mac');
+ my $ip = extract_param($param, 'ip');
eval {
- PVE::Network::SDN::Vnets::del_ips_from_mac($vnet, $mac);
+ PVE::Network::SDN::Vnets::del_ip($vnet, $ip, '', $mac);
};
- my $error = $@;
-
- die "$error\n" if $error;
+ die "$@\n" if $@;
return undef;
},
@@ -117,7 +121,10 @@ __PACKAGE__->register_method ({
my $vmid = extract_param($param, 'vmid');
my $ip = extract_param($param, 'ip');
- my ($old_ip4, $old_ip6) = PVE::Network::SDN::Vnets::del_ips_from_mac($vnet, $mac, '');
+ my ($old_ip4, $old_ip6) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnet, $mac);
+ my $old_ip = (Net::IP::ip_get_version($ip) == 4) ? $old_ip4 : $old_ip6;
+
+ PVE::Network::SDN::Vnets::del_ip($vnet, $old_ip, '', $mac);
eval {
PVE::Network::SDN::Vnets::add_ip($vnet, $ip, '', $mac, $vmid);
@@ -125,8 +132,7 @@ __PACKAGE__->register_method ({
my $error = $@;
if ($error) {
- PVE::Network::SDN::Vnets::add_ip($vnet, $old_ip4, '', $mac, $vmid) if $old_ip4;
- PVE::Network::SDN::Vnets::add_ip($vnet, $old_ip6, '', $mac, $vmid) if $old_ip6;
+ PVE::Network::SDN::Vnets::add_ip($vnet, $old_ip, '', $mac, $vmid);
}
die "$error\n" if $error;
diff --git a/src/PVE/Network/SDN/Subnets.pm b/src/PVE/Network/SDN/Subnets.pm
index 8e2a6aa..8f113b4 100644
--- a/src/PVE/Network/SDN/Subnets.pm
+++ b/src/PVE/Network/SDN/Subnets.pm
@@ -305,6 +305,9 @@ sub add_ip {
$plugin->add_ip($plugin_config, $subnetid, $subnet, $ip, $hostname, $mac, $vmid, $is_gateway);
};
die $@ if $@;
+
+ eval { PVE::Network::SDN::Ipams::add_cache_mac_ip($mac, $ip) if $mac; };
+ warn $@ if $@;
}
eval {
--
2.39.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pve-devel] [PATCH pve-manager] ipam: send ip to delete endpoint
2023-11-22 12:29 [pve-devel] [PATCH pve-network] ipam: improve update / delete behavior Stefan Hanreich
@ 2023-11-22 12:29 ` Stefan Hanreich
2023-11-22 13:36 ` [pve-devel] applied: " Thomas Lamprecht
2023-11-22 13:18 ` [pve-devel] [PATCH pve-network] ipam: improve update / delete behavior Thomas Lamprecht
2023-11-22 13:45 ` [pve-devel] " Stefan Lendl
2 siblings, 1 reply; 6+ messages in thread
From: Stefan Hanreich @ 2023-11-22 12:29 UTC (permalink / raw)
To: pve-devel
The ip parameter has been added to the delete endpoint, so only a
specific mapping gets deleted instead of all mappings for that mac
address. Reflect this change in the UI.
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
www/manager6/tree/DhcpTree.js | 1 +
1 file changed, 1 insertion(+)
diff --git a/www/manager6/tree/DhcpTree.js b/www/manager6/tree/DhcpTree.js
index b7baba606..b5fbafe03 100644
--- a/www/manager6/tree/DhcpTree.js
+++ b/www/manager6/tree/DhcpTree.js
@@ -108,6 +108,7 @@ Ext.define('PVE.sdn.DhcpTree', {
let params = {
zone: data.zone,
mac: data.mac,
+ ip: data.ip,
};
let encodedParams = Ext.Object.toQueryString(params);
--
2.39.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH pve-network] ipam: improve update / delete behavior
2023-11-22 12:29 [pve-devel] [PATCH pve-network] ipam: improve update / delete behavior Stefan Hanreich
2023-11-22 12:29 ` [pve-devel] [PATCH pve-manager] ipam: send ip to delete endpoint Stefan Hanreich
@ 2023-11-22 13:18 ` Thomas Lamprecht
[not found] ` <55cfaf0f-b4c9-4698-89a5-faaf8ca6a000@proxmox.com>
2023-11-22 13:45 ` [pve-devel] " Stefan Lendl
2 siblings, 1 reply; 6+ messages in thread
From: Thomas Lamprecht @ 2023-11-22 13:18 UTC (permalink / raw)
To: Proxmox VE development discussion, Stefan Hanreich
Am 22/11/2023 um 13:29 schrieb Stefan Hanreich:
> Currently when updating or deleting a mapping in the IPAM we would
> delete all existing entries in the IPAM with that mac address. Now we
> only delete the specific entry we are updating / deleting.
>
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
> src/PVE/API2/Network/SDN/Ips.pm | 20 +++++++++++++-------
> src/PVE/Network/SDN/Subnets.pm | 3 +++
> 2 files changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/src/PVE/API2/Network/SDN/Ips.pm b/src/PVE/API2/Network/SDN/Ips.pm
> index 6989b9b..0003b2a 100644
> --- a/src/PVE/API2/Network/SDN/Ips.pm
> +++ b/src/PVE/API2/Network/SDN/Ips.pm
> @@ -28,6 +28,11 @@ __PACKAGE__->register_method ({
> zone => get_standard_option('pve-sdn-zone-id'),
> vnet => get_standard_option('pve-sdn-vnet-id'),
> mac => get_standard_option('mac-addr'),
> + ip => {
> + type => 'string',
> + format => 'ip',
> + description => 'The IP address to delete',
> + },
> },
> },
> returns => { type => 'null' },
> @@ -36,13 +41,12 @@ __PACKAGE__->register_method ({
>
> my $vnet = extract_param($param, 'vnet');
> my $mac = extract_param($param, 'mac');
> + my $ip = extract_param($param, 'ip');
>
> eval {
> - PVE::Network::SDN::Vnets::del_ips_from_mac($vnet, $mac);
> + PVE::Network::SDN::Vnets::del_ip($vnet, $ip, '', $mac);
> };
> - my $error = $@;
> -
> - die "$error\n" if $error;
> + die "$@\n" if $@;
why bother with the eval then? or does something set $@ manually in the
called method?
>
> return undef;
> },
> @@ -117,7 +121,10 @@ __PACKAGE__->register_method ({
> my $vmid = extract_param($param, 'vmid');
> my $ip = extract_param($param, 'ip');
>
> - my ($old_ip4, $old_ip6) = PVE::Network::SDN::Vnets::del_ips_from_mac($vnet, $mac, '');
> + my ($old_ip4, $old_ip6) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnet, $mac);
> + my $old_ip = (Net::IP::ip_get_version($ip) == 4) ? $old_ip4 : $old_ip6;
> +
> + PVE::Network::SDN::Vnets::del_ip($vnet, $old_ip, '', $mac);
>
> eval {
> PVE::Network::SDN::Vnets::add_ip($vnet, $ip, '', $mac, $vmid);
> @@ -125,8 +132,7 @@ __PACKAGE__->register_method ({
> my $error = $@;
>
> if ($error) {
> - PVE::Network::SDN::Vnets::add_ip($vnet, $old_ip4, '', $mac, $vmid) if $old_ip4;
> - PVE::Network::SDN::Vnets::add_ip($vnet, $old_ip6, '', $mac, $vmid) if $old_ip6;
> + PVE::Network::SDN::Vnets::add_ip($vnet, $old_ip, '', $mac, $vmid);
> }
>
> die "$error\n" if $error;
> diff --git a/src/PVE/Network/SDN/Subnets.pm b/src/PVE/Network/SDN/Subnets.pm
> index 8e2a6aa..8f113b4 100644
> --- a/src/PVE/Network/SDN/Subnets.pm
> +++ b/src/PVE/Network/SDN/Subnets.pm
> @@ -305,6 +305,9 @@ sub add_ip {
> $plugin->add_ip($plugin_config, $subnetid, $subnet, $ip, $hostname, $mac, $vmid, $is_gateway);
> };
> die $@ if $@;
> +
> + eval { PVE::Network::SDN::Ipams::add_cache_mac_ip($mac, $ip) if $mac; };
> + warn $@ if $@;
is this really related?
> }
>
> eval {
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pve-devel] applied: [PATCH pve-network] ipam: improve update / delete behavior
[not found] ` <55cfaf0f-b4c9-4698-89a5-faaf8ca6a000@proxmox.com>
@ 2023-11-22 13:26 ` Thomas Lamprecht
0 siblings, 0 replies; 6+ messages in thread
From: Thomas Lamprecht @ 2023-11-22 13:26 UTC (permalink / raw)
To: Stefan Hanreich, PVE development discussion
Am 22/11/2023 um 14:22 schrieb Stefan Hanreich:
>> is this really related?
>
> somewhat. add_ip never added a cache entry in macs.db which didn't cause
> any issues when we added gateways via add_ip. But now that we use it in
> update as well we need to create the entries in this function as well
> since otherwise they wont get picked up by the DHCP server.
OK then, applied thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pve-devel] applied: [PATCH pve-manager] ipam: send ip to delete endpoint
2023-11-22 12:29 ` [pve-devel] [PATCH pve-manager] ipam: send ip to delete endpoint Stefan Hanreich
@ 2023-11-22 13:36 ` Thomas Lamprecht
0 siblings, 0 replies; 6+ messages in thread
From: Thomas Lamprecht @ 2023-11-22 13:36 UTC (permalink / raw)
To: Proxmox VE development discussion, Stefan Hanreich
Am 22/11/2023 um 13:29 schrieb Stefan Hanreich:
> The ip parameter has been added to the delete endpoint, so only a
> specific mapping gets deleted instead of all mappings for that mac
> address. Reflect this change in the UI.
>
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
> www/manager6/tree/DhcpTree.js | 1 +
> 1 file changed, 1 insertion(+)
>
>
applied, thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH pve-network] ipam: improve update / delete behavior
2023-11-22 12:29 [pve-devel] [PATCH pve-network] ipam: improve update / delete behavior Stefan Hanreich
2023-11-22 12:29 ` [pve-devel] [PATCH pve-manager] ipam: send ip to delete endpoint Stefan Hanreich
2023-11-22 13:18 ` [pve-devel] [PATCH pve-network] ipam: improve update / delete behavior Thomas Lamprecht
@ 2023-11-22 13:45 ` Stefan Lendl
2 siblings, 0 replies; 6+ messages in thread
From: Stefan Lendl @ 2023-11-22 13:45 UTC (permalink / raw)
To: Stefan Hanreich, pve-devel
I tested this with multiple Subnets:
It works if MAC is 1x in IPv4 subnet and 1x in IPv6 subnet.
- updating either IPv4 or IPv6
- other one persists
- update IPv4 to be in another subnet
- auto-selects the new subnet
Encountered issues:
- change IPv4 to IPv6 (and vice versa)
> can't find any subnet for ip at /usr/share/perl5/PVE/Network/SDN/Subnets.pm line 114. (500)
- When manually creating a mapping, there is no way to set a VMID (but works)
- consecutive updating the mapping does not work
> vmid: type check ('integer') failed - got ''
- Updates to the IPAM mapping do not propagate to dnsmasq's ethers file
- also not after Reload
- In comparison `qm set 109 --net1 model=virtio,bridge=dhcpnat3` will
update ethers immediately
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-11-22 13:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-22 12:29 [pve-devel] [PATCH pve-network] ipam: improve update / delete behavior Stefan Hanreich
2023-11-22 12:29 ` [pve-devel] [PATCH pve-manager] ipam: send ip to delete endpoint Stefan Hanreich
2023-11-22 13:36 ` [pve-devel] applied: " Thomas Lamprecht
2023-11-22 13:18 ` [pve-devel] [PATCH pve-network] ipam: improve update / delete behavior Thomas Lamprecht
[not found] ` <55cfaf0f-b4c9-4698-89a5-faaf8ca6a000@proxmox.com>
2023-11-22 13:26 ` [pve-devel] applied: " Thomas Lamprecht
2023-11-22 13:45 ` [pve-devel] " Stefan Lendl
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox