[pve-devel] [PATCH ifupdown2 1/1] Correctly handle IPv6 addresses in vxlan

[pve-devel] [PATCH ifupdown2 1/1] Correctly handle IPv6 addresses in vxlan

[apalrd via pve-devel]

[-- Attachment #1: Type: message/rfc822, Size: 7238 bytes --]

From: apalrd <andrew@apalrd.net>
To: pve-devel@lists.proxmox.com
Cc: apalrd <andrew@apalrd.net>
Subject: [PATCH ifupdown2 1/1] Correctly handle IPv6 addresses in vxlan
Date: Tue,  8 Oct 2024 00:01:09 -0400
Message-ID: <20241008040109.322473-2-andrew@apalrd.net>

---
 ifupdown2/addons/vxlan.py | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/ifupdown2/addons/vxlan.py b/ifupdown2/addons/vxlan.py
index 084aec9..4aa8e50 100644
--- a/ifupdown2/addons/vxlan.py
+++ b/ifupdown2/addons/vxlan.py
@@ -51,7 +51,7 @@ class vxlan(Vxlan, moduleBase):
             },
             "vxlan-local-tunnelip": {
                 "help": "vxlan local tunnel ip",
-                "validvals": ["<ipv4>"],
+                "validvals": ["<ipv4>,<ipv6>"],
                 "example": ["vxlan-local-tunnelip 172.16.20.103"]
             },
             "vxlan-svcnodeip": {
@@ -66,7 +66,7 @@ class vxlan(Vxlan, moduleBase):
             },
             "vxlan-remoteip": {
                 "help": "vxlan remote ip",
-                "validvals": ["<ipv4>"],
+                "validvals": ["<ipv4>,<ipv6>"],
                 "example": ["vxlan-remoteip 172.16.22.127"],
                 "multiline": True
             },
@@ -521,7 +521,7 @@ class vxlan(Vxlan, moduleBase):
             local = self._vxlan_local_tunnelip
 
         if link_exists:
-            cached_ifla_vxlan_local = cached_vxlan_ifla_info_data.get(Link.IFLA_VXLAN_LOCAL)
+            cached_ifla_vxlan_local = cached_vxlan_ifla_info_data.get(Link.IFLA_VXLAN_LOCAL) or cached_vxlan_ifla_info_data.get(Link.IFLA_VXLAN_LOCAL6)
 
             # on ifreload do not overwrite anycast_ip to individual ip
             # if clagd has modified
@@ -547,7 +547,7 @@ class vxlan(Vxlan, moduleBase):
 
         if local:
             try:
-                local = ipnetwork.IPv4Address(local)
+                local = ipnetwork.IPAddress(local)
 
                 if local.initialized_with_prefixlen:
                     self.logger.warning("%s: vxlan-local-tunnelip %s: netmask ignored" % (ifname, local))
@@ -559,13 +559,19 @@ class vxlan(Vxlan, moduleBase):
         if local:
             if local != cached_ifla_vxlan_local:
                 self.logger.info("%s: set vxlan-local-tunnelip %s" % (ifname, local))
-                user_request_vxlan_info_data[Link.IFLA_VXLAN_LOCAL] = local
+                if local.version == 6:
+                    user_request_vxlan_info_data[Link.IFLA_VXLAN_LOCAL6] = local
+                else:
+                    user_request_vxlan_info_data[Link.IFLA_VXLAN_LOCAL] = local
 
                 # if both local-ip and anycast-ip are identical the function prints a warning
                 self.syntax_check_localip_anycastip_equal(ifname, local, self._clagd_vxlan_anycast_ip)
         elif cached_ifla_vxlan_local:
             self.logger.info("%s: removing vxlan-local-tunnelip (cache %s)" % (ifname, cached_ifla_vxlan_local))
-            user_request_vxlan_info_data[Link.IFLA_VXLAN_LOCAL] = None
+            if cached_ifla_vxlan_local.version == 6:
+                user_request_vxlan_info_data[Link.IFLA_VXLAN_LOCAL6] = None
+            else:
+                user_request_vxlan_info_data[Link.IFLA_VXLAN_LOCAL] = None
 
         return local
 
@@ -1236,7 +1242,7 @@ class vxlan(Vxlan, moduleBase):
         if remoteips:
             try:
                 for remoteip in remoteips:
-                    ipnetwork.IPv4Address(remoteip)
+                    ipnetwork.IPAddress(remoteip)
             except Exception as e:
                 self.log_error('%s: vxlan-remoteip: %s' % (ifaceobj.name, str(e)))
 
@@ -1244,7 +1250,7 @@ class vxlan(Vxlan, moduleBase):
         # purge any removed remote ip
         old_remoteips = self.get_old_remote_ips(ifaceobj.name)
 
-        if vxlan_purge_remotes or remoteips or (remoteips != old_remoteips):
+        if vxlan_purge_remotes or (isinstance(remoteips,list) and remoteips != old_remoteips):
             # figure out the diff for remotes and do the bridge fdb updates
             # only if provisioned by user and not by an vxlan external
             # controller.
@@ -1281,8 +1287,8 @@ class vxlan(Vxlan, moduleBase):
                         "00:00:00:00:00:00",
                         None, True, addr
                     )
-                except Exception:
-                    pass
+                except Exception as e:
+                    self.log_error('%s: vxlan-remoteip<add>: %s' % (ifaceobj.name, str(e)))
 
         self.vxlan_remote_ip_map(ifaceobj, vxlan_mcast_grp_map)
 
-- 
2.39.5



[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [pve-devel] [PATCH ifupdown2 1/1] Correctly handle IPv6 addresses in vxlan

[Stefan Hanreich]

Hi, it seems like ifupdown2 will be merging a PR for VXLAN IPv6 support
in the near future [1]. Hence why I think we should be holding off on
this, until we know which solution (there are several PRs for this
actually) they merge. Otherwise we run into the problem of having
incompatible / custom settings for this.

[1]
https://github.com/CumulusNetworks/ifupdown2/pull/318#issuecomment-2541947074


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

Re: [pve-devel] [PATCH ifupdown2 1/1] Correctly handle IPv6 addresses in vxlan

[Andrew via pve-devel]

[-- Attachment #1: Type: message/rfc822, Size: 4619 bytes --]

From: Andrew <andrew@apalrd.net>
To: Stefan Hanreich <s.hanreich@proxmox.com>
Cc: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH ifupdown2 1/1] Correctly handle IPv6 addresses in vxlan
Date: Mon, 31 Mar 2025 04:45:16 +0000
Message-ID: <DE78AE8B-31EE-447A-962D-719F3C924700@apalrd.net>

Just noticed I didn’t reply to this before - the patch set in ifupdown2 does not actually address IPv6 local tunnel IPs at all.

> On Jan 23, 2025, at 04:26, Stefan Hanreich <s.hanreich@proxmox.com> wrote:
> 
> Hi, it seems like ifupdown2 will be merging a PR for VXLAN IPv6 support
> in the near future [1]. Hence why I think we should be holding off on
> this, until we know which solution (there are several PRs for this
> actually) they merge. Otherwise we run into the problem of having
> incompatible / custom settings for this.
> 
> [1]
> https://github.com/CumulusNetworks/ifupdown2/pull/318#issuecomment-2541947074
> 




[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [pve-devel] [PATCH ifupdown2 1/1] Correctly handle IPv6 addresses in vxlan

[Stefan Hanreich]

On 3/31/25 06:45, Andrew wrote:
> Just noticed I didn’t reply to this before - the patch set in ifupdown2 does not actually address IPv6 local tunnel IPs at all.

3.9 itself doesn't, but 3.10 should contain the respective patches


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

Re: [pve-devel] [PATCH ifupdown2 1/1] Correctly handle IPv6 addresses in vxlan

[Andrew via pve-devel]

[-- Attachment #1: Type: message/rfc822, Size: 4928 bytes --]

From: Andrew <andrew@apalrd.net>
To: Stefan Hanreich <s.hanreich@proxmox.com>
Cc: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH ifupdown2 1/1] Correctly handle IPv6 addresses in vxlan
Date: Mon, 31 Mar 2025 14:38:08 +0000
Message-ID: <511405BE-CEF9-4C26-8E75-A4235FC58420@apalrd.net>

Given the delays in ifupdown2 development (the patches for vxlan IPv6 have been sitting un-reviewed for 5 months now, and this is the 3rd patch set in * 5 years * that has tried to fix this issue without being merged), can we continue waiting years for them to fix?

The most recent patch in ifupdown2 does not modify the names of the attributes, so going forward with this patch in PVE and updating pve-network accordingly will not cause rework in pve-network if ifupdown ever releases software. 

Andrew

> On Mar 31, 2025, at 03:22, Stefan Hanreich <s.hanreich@proxmox.com> wrote:
> 
> On 3/31/25 06:45, Andrew wrote:
>> Just noticed I didn’t reply to this before - the patch set in ifupdown2 does not actually address IPv6 local tunnel IPs at all.
> 
> 3.9 itself doesn't, but 3.10 should contain the respective patches
> 




[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [pve-devel] [PATCH ifupdown2 1/1] Correctly handle IPv6 addresses in vxlan

[DERUMIER, Alexandre via pve-devel]

[-- Attachment #1: Type: message/rfc822, Size: 19952 bytes --]

From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "andrew@apalrd.net" <andrew@apalrd.net>
Cc: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>
Subject: Re: [PATCH ifupdown2 1/1] Correctly handle IPv6 addresses in vxlan
Date: Wed, 9 Oct 2024 15:37:55 +0000
Message-ID: <9045eca45de7aa50fd817fb9221cfa04c524ff19.camel@groupe-cyllene.com>

Try to look at ifupdown2 github, their are 2 old pull request about
this (never merged/ never completed)



https://github.com/CumulusNetworks/ifupdown2/pull/172

"
For this we would need a new attribute vxlan-local-tunnelip6, we don't
want to reuse the same attribute for ipv6.
We are using netlink to configure vxlans, so it's important to use a
different attribute to set the proper netlink attribute (I don't want
to have things like if IPAddress(value).version == 6:  set
Link.IFLA_VXLAN_LOCAL
"

https://github.com/CumulusNetworks/ifupdown2/pull/182


so, at minimum, this need to use a different "vxlan-local-tunnelip6"
attribute for ipv6


-------- Message initial --------
De: apalrd <andrew@apalrd.net>
À: pve-devel@lists.proxmox.com
Cc: apalrd <andrew@apalrd.net>
Objet: [PATCH ifupdown2 1/1] Correctly handle IPv6 addresses in vxlan
Date: 08/10/2024 06:01:09

---
 ifupdown2/addons/vxlan.py | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/ifupdown2/addons/vxlan.py b/ifupdown2/addons/vxlan.py
index 084aec9..4aa8e50 100644
--- a/ifupdown2/addons/vxlan.py
+++ b/ifupdown2/addons/vxlan.py
@@ -51,7 +51,7 @@ class vxlan(Vxlan, moduleBase):
             },
             "vxlan-local-tunnelip": {
                 "help": "vxlan local tunnel ip",
-                "validvals": ["<ipv4>"],
+                "validvals": ["<ipv4>,<ipv6>"],
                 "example": ["vxlan-local-tunnelip 172.16.20.103"]
             },
             "vxlan-svcnodeip": {
@@ -66,7 +66,7 @@ class vxlan(Vxlan, moduleBase):
             },
             "vxlan-remoteip": {
                 "help": "vxlan remote ip",
-                "validvals": ["<ipv4>"],
+                "validvals": ["<ipv4>,<ipv6>"],
                 "example": ["vxlan-remoteip 172.16.22.127"],
                 "multiline": True
             },
@@ -521,7 +521,7 @@ class vxlan(Vxlan, moduleBase):
             local = self._vxlan_local_tunnelip
 
         if link_exists:
-            cached_ifla_vxlan_local =
cached_vxlan_ifla_info_data.get(Link.IFLA_VXLAN_LOCAL)
+            cached_ifla_vxlan_local =
cached_vxlan_ifla_info_data.get(Link.IFLA_VXLAN_LOCAL) or
cached_vxlan_ifla_info_data.get(Link.IFLA_VXLAN_LOCAL6)
 
             # on ifreload do not overwrite anycast_ip to individual ip
             # if clagd has modified
@@ -547,7 +547,7 @@ class vxlan(Vxlan, moduleBase):
 
         if local:
             try:
-                local = ipnetwork.IPv4Address(local)
+                local = ipnetwork.IPAddress(local)
 
                 if local.initialized_with_prefixlen:
                     self.logger.warning("%s: vxlan-local-tunnelip %s:
netmask ignored" % (ifname, local))
@@ -559,13 +559,19 @@ class vxlan(Vxlan, moduleBase):
         if local:
             if local != cached_ifla_vxlan_local:
                 self.logger.info("%s: set vxlan-local-tunnelip %s" %
(ifname, local))
-                user_request_vxlan_info_data[Link.IFLA_VXLAN_LOCAL] =
local
+                if local.version == 6:
+                   
user_request_vxlan_info_data[Link.IFLA_VXLAN_LOCAL6] = local
+                else:
+                   
user_request_vxlan_info_data[Link.IFLA_VXLAN_LOCAL] = local
 
                 # if both local-ip and anycast-ip are identical the
function prints a warning
                 self.syntax_check_localip_anycastip_equal(ifname,
local, self._clagd_vxlan_anycast_ip)
         elif cached_ifla_vxlan_local:
             self.logger.info("%s: removing vxlan-local-tunnelip (cache
%s)" % (ifname, cached_ifla_vxlan_local))
-            user_request_vxlan_info_data[Link.IFLA_VXLAN_LOCAL] = None
+            if cached_ifla_vxlan_local.version == 6:
+                user_request_vxlan_info_data[Link.IFLA_VXLAN_LOCAL6] =
None
+            else:
+                user_request_vxlan_info_data[Link.IFLA_VXLAN_LOCAL] =
None
 
         return local
 
@@ -1236,7 +1242,7 @@ class vxlan(Vxlan, moduleBase):
         if remoteips:
             try:
                 for remoteip in remoteips:
-                    ipnetwork.IPv4Address(remoteip)
+                    ipnetwork.IPAddress(remoteip)
             except Exception as e:
                 self.log_error('%s: vxlan-remoteip: %s' %
(ifaceobj.name, str(e)))
 
@@ -1244,7 +1250,7 @@ class vxlan(Vxlan, moduleBase):
         # purge any removed remote ip
         old_remoteips = self.get_old_remote_ips(ifaceobj.name)
 
-        if vxlan_purge_remotes or remoteips or (remoteips !=
old_remoteips):
+        if vxlan_purge_remotes or (isinstance(remoteips,list) and
remoteips != old_remoteips):
             # figure out the diff for remotes and do the bridge fdb
updates
             # only if provisioned by user and not by an vxlan external
             # controller.
@@ -1281,8 +1287,8 @@ class vxlan(Vxlan, moduleBase):
                         "00:00:00:00:00:00",
                         None, True, addr
                     )
-                except Exception:
-                    pass
+                except Exception as e:
+                    self.log_error('%s: vxlan-remoteip<add>: %s' %
(ifaceobj.name, str(e)))
 
         self.vxlan_remote_ip_map(ifaceobj, vxlan_mcast_grp_map)
 


[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [pve-devel] [PATCH ifupdown2 1/1] Correctly handle IPv6 addresses in vxlan

[Andrew via pve-devel]

[-- Attachment #1: Type: message/rfc822, Size: 11178 bytes --]

From: Andrew <andrew@apalrd.net>
To: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
Cc: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>
Subject: Re: [PATCH ifupdown2 1/1] Correctly handle IPv6 addresses in vxlan
Date: Wed, 09 Oct 2024 16:55:50 +0000
Message-ID: <17214981-4406-4100-AFF6-9F70E12E421B@apalrd.net>

Yes, I read all of the PRs and discussion on ifupdown2 GitHub before implementing this.

Ultimately I disagreed with the solution to use a separate parameter for IPv6, for the following reasons:
- We can only have one local tunnel IP, so having two parameters means we need to check if the other one has been set (since setting both would be invalid)
- There are already other cases in ifupdown2 which do ip.version == 6 including common parameters like address and gateway, and most parameters do take both IPv4 and IPv6 addresses (such as the remoteip field in vxlan), so having one parameter for both families would be consistent with other parameters in the interfaces file which take both families (regardless of the kernel implementation having two fields instead of one in this case)
- ifupdown-ng already solved this problem using a single parameter instead of two, so doing something else would diverge ifupdown2 vs ifupdown-ng syntax which should be the same

I could add additional error checking to ensure that remoteip[] and localip are the same address family if you’d like. Currently that results in a Netlink exception which gets passed back as an error message. The kernel only allows one address family for a vxlan interface.

Thanks,

Andrew

> On Oct 9, 2024, at 11:37, DERUMIER, Alexandre <alexandre.derumier@groupe-cyllene.com> wrote:
> 
> Try to look at ifupdown2 github, their are 2 old pull request about
> this (never merged/ never completed)
> 
> 
> 
> https://github.com/CumulusNetworks/ifupdown2/pull/172
> 
> "
> For this we would need a new attribute vxlan-local-tunnelip6, we don't
> want to reuse the same attribute for ipv6.
> We are using netlink to configure vxlans, so it's important to use a
> different attribute to set the proper netlink attribute (I don't want
> to have things like if IPAddress(value).version == 6:  set
> Link.IFLA_VXLAN_LOCAL
> "
> 
> https://github.com/CumulusNetworks/ifupdown2/pull/182
> 
> 
> so, at minimum, this need to use a different "vxlan-local-tunnelip6"
> attribute for ipv6
> 
> 
> -------- Message initial --------
> De: apalrd <andrew@apalrd.net>
> À: pve-devel@lists.proxmox.com
> Cc: apalrd <andrew@apalrd.net>
> Objet: [PATCH ifupdown2 1/1] Correctly handle IPv6 addresses in vxlan
> Date: 08/10/2024 06:01:09
> 
> ---
>  ifupdown2/addons/vxlan.py | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/ifupdown2/addons/vxlan.py b/ifupdown2/addons/vxlan.py
> index 084aec9..4aa8e50 100644
> --- a/ifupdown2/addons/vxlan.py
> +++ b/ifupdown2/addons/vxlan.py
> @@ -51,7 +51,7 @@ class vxlan(Vxlan, moduleBase):
>              },
>              "vxlan-local-tunnelip": {
>                  "help": "vxlan local tunnel ip",
> -                "validvals": ["<ipv4>"],
> +                "validvals": ["<ipv4>,<ipv6>"],
>                  "example": ["vxlan-local-tunnelip 172.16.20.103"]
>              },
>              "vxlan-svcnodeip": {
> @@ -66,7 +66,7 @@ class vxlan(Vxlan, moduleBase):
>              },
>              "vxlan-remoteip": {
>                  "help": "vxlan remote ip",
> -                "validvals": ["<ipv4>"],
> +                "validvals": ["<ipv4>,<ipv6>"],
>                  "example": ["vxlan-remoteip 172.16.22.127"],
>                  "multiline": True
>              },
> @@ -521,7 +521,7 @@ class vxlan(Vxlan, moduleBase):
>              local = self._vxlan_local_tunnelip
>  
>          if link_exists:
> -            cached_ifla_vxlan_local =
> cached_vxlan_ifla_info_data.get(Link.IFLA_VXLAN_LOCAL)
> +            cached_ifla_vxlan_local =
> cached_vxlan_ifla_info_data.get(Link.IFLA_VXLAN_LOCAL) or
> cached_vxlan_ifla_info_data.get(Link.IFLA_VXLAN_LOCAL6)
>  
>              # on ifreload do not overwrite anycast_ip to individual ip
>              # if clagd has modified
> @@ -547,7 +547,7 @@ class vxlan(Vxlan, moduleBase):
>  
>          if local:
>              try:
> -                local = ipnetwork.IPv4Address(local)
> +                local = ipnetwork.IPAddress(local)
>  
>                  if local.initialized_with_prefixlen:
>                      self.logger.warning("%s: vxlan-local-tunnelip %s:
> netmask ignored" % (ifname, local))
> @@ -559,13 +559,19 @@ class vxlan(Vxlan, moduleBase):
>          if local:
>              if local != cached_ifla_vxlan_local:
>                  self.logger.info("%s: set vxlan-local-tunnelip %s" %
> (ifname, local))
> -                user_request_vxlan_info_data[Link.IFLA_VXLAN_LOCAL] =
> local
> +                if local.version == 6:
> +                   
> user_request_vxlan_info_data[Link.IFLA_VXLAN_LOCAL6] = local
> +                else:
> +                   
> user_request_vxlan_info_data[Link.IFLA_VXLAN_LOCAL] = local
>  
>                  # if both local-ip and anycast-ip are identical the
> function prints a warning
>                  self.syntax_check_localip_anycastip_equal(ifname,
> local, self._clagd_vxlan_anycast_ip)
>          elif cached_ifla_vxlan_local:
>              self.logger.info("%s: removing vxlan-local-tunnelip (cache
> %s)" % (ifname, cached_ifla_vxlan_local))
> -            user_request_vxlan_info_data[Link.IFLA_VXLAN_LOCAL] = None
> +            if cached_ifla_vxlan_local.version == 6:
> +                user_request_vxlan_info_data[Link.IFLA_VXLAN_LOCAL6] =
> None
> +            else:
> +                user_request_vxlan_info_data[Link.IFLA_VXLAN_LOCAL] =
> None
>  
>          return local
>  
> @@ -1236,7 +1242,7 @@ class vxlan(Vxlan, moduleBase):
>          if remoteips:
>              try:
>                  for remoteip in remoteips:
> -                    ipnetwork.IPv4Address(remoteip)
> +                    ipnetwork.IPAddress(remoteip)
>              except Exception as e:
>                  self.log_error('%s: vxlan-remoteip: %s' %
> (ifaceobj.name, str(e)))
>  
> @@ -1244,7 +1250,7 @@ class vxlan(Vxlan, moduleBase):
>          # purge any removed remote ip
>          old_remoteips = self.get_old_remote_ips(ifaceobj.name)
>  
> -        if vxlan_purge_remotes or remoteips or (remoteips !=
> old_remoteips):
> +        if vxlan_purge_remotes or (isinstance(remoteips,list) and
> remoteips != old_remoteips):
>              # figure out the diff for remotes and do the bridge fdb
> updates
>              # only if provisioned by user and not by an vxlan external
>              # controller.
> @@ -1281,8 +1287,8 @@ class vxlan(Vxlan, moduleBase):
>                          "00:00:00:00:00:00",
>                          None, True, addr
>                      )
> -                except Exception:
> -                    pass
> +                except Exception as e:
> +                    self.log_error('%s: vxlan-remoteip<add>: %s' %
> (ifaceobj.name, str(e)))
>  
>          self.vxlan_remote_ip_map(ifaceobj, vxlan_mcast_grp_map)
>  
> 




[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [pve-devel] [PATCH ifupdown2 1/1] Correctly handle IPv6 addresses in vxlan

[DERUMIER, Alexandre via pve-devel]

[-- Attachment #1: Type: message/rfc822, Size: 26084 bytes --]

From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "andrew@apalrd.net" <andrew@apalrd.net>
Cc: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>
Subject: Re: [PATCH ifupdown2 1/1] Correctly handle IPv6 addresses in vxlan
Date: Wed, 9 Oct 2024 21:05:39 +0000
Message-ID: <99349e23b9a957cdf81d5bf2fa8c6af9f6b24ad0.camel@groupe-cyllene.com>

Personnally, I'm ok with your patch

>>Ultimately I disagreed with the solution to use a separate parameter
>>for IPv6, for the following reasons:
>>- We can only have one local tunnel IP, so having two parameters
>>means we need to check if the other one has been set (since setting
>>both would be invalid)
>>- There are already other cases in ifupdown2 which do ip.version == 6
>>including common parameters like address and gateway, and most
>>parameters do take both IPv4 and IPv6 addresses (such as the remoteip
>>field in vxlan), so having one parameter for both families would be
>>consistent with other parameters in the interfaces file which take
>>both families (regardless of the kernel implementation having two
>>fields instead of one in this case)
>>- ifupdown-ng already solved this problem using a single parameter
>>instead of two, so doing something else would diverge ifupdown2 vs
>>ifupdown-ng syntax which should be the same


but we need to be able to not diverge too much from upstream,
if one day (ok that's waiting since 4 years...) an official
support is added by cumulus/nvidia.
(to be honest, the upstream is a lot less active since nvidia have buy
cumulus/mellanox)

That's mean maintain both syntax, or plan a config update,
between 2 majors pve releases. Not a big deal.


but yes, we already use "address ..." for example, for both ipv6/ipv4.
and here, as you said, we can have only ipv4 or ipv6 for local tunnel.


>>I could add additional error checking to ensure that remoteip[] and
>>localip are the same address family if you’d like. Currently that
>>results in a Netlink exception which gets passed back as an error
>>message. The kernel only allows one address family for a vxlan
>>interface.

yes, it could be great to test it.



BTW, I don't have followed the ifupdown-ng project since a long time.
(just follow the early days). Is the project really active and have
almost same features than ifupdown2 ?  (netlink support, reload support
with diff of running config,...)

-------- Message initial --------
De: Andrew <andrew@apalrd.net>
À: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
Cc: pve-devel@lists.proxmox.com <pve-devel@lists.proxmox.com>
Objet: Re: [PATCH ifupdown2 1/1] Correctly handle IPv6 addresses in
vxlan
Date: 09/10/2024 18:55:50

Yes, I read all of the PRs and discussion on ifupdown2 GitHub before
implementing this.

Ultimately I disagreed with the solution to use a separate parameter
for IPv6, for the following reasons:
- We can only have one local tunnel IP, so having two parameters means
we need to check if the other one has been set (since setting both
would be invalid)
- There are already other cases in ifupdown2 which do ip.version == 6
including common parameters like address and gateway, and most
parameters do take both IPv4 and IPv6 addresses (such as the remoteip
field in vxlan), so having one parameter for both families would be
consistent with other parameters in the interfaces file which take both
families (regardless of the kernel implementation having two fields
instead of one in this case)
- ifupdown-ng already solved this problem using a single parameter
instead of two, so doing something else would diverge ifupdown2 vs
ifupdown-ng syntax which should be the same

I could add additional error checking to ensure that remoteip[] and
localip are the same address family if you’d like. Currently that
results in a Netlink exception which gets passed back as an error
message. The kernel only allows one address family for a vxlan
interface.

Thanks,

Andrew

> On Oct 9, 2024, at 11:37, DERUMIER, Alexandre
> <alexandre.derumier@groupe-cyllene.com> wrote:
> 
> Try to look at ifupdown2 github, their are 2 old pull request about
> this (never merged/ never completed)
> 
> 
> 
> https://github.com/CumulusNetworks/ifupdown2/pull/172
> 
> "
> For this we would need a new attribute vxlan-local-tunnelip6, we
> don't
> want to reuse the same attribute for ipv6.
> We are using netlink to configure vxlans, so it's important to use a
> different attribute to set the proper netlink attribute (I don't want
> to have things like if IPAddress(value).version == 6:  set
> Link.IFLA_VXLAN_LOCAL
> "
> 
> https://github.com/CumulusNetworks/ifupdown2/pull/182
> 
> 
> so, at minimum, this need to use a different "vxlan-local-tunnelip6"
> attribute for ipv6
> 
> 
> -------- Message initial --------
> De: apalrd <andrew@apalrd.net>
> À: pve-devel@lists.proxmox.com
> Cc: apalrd <andrew@apalrd.net>
> Objet: [PATCH ifupdown2 1/1] Correctly handle IPv6 addresses in vxlan
> Date: 08/10/2024 06:01:09
> 
> ---
>  ifupdown2/addons/vxlan.py | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/ifupdown2/addons/vxlan.py b/ifupdown2/addons/vxlan.py
> index 084aec9..4aa8e50 100644
> --- a/ifupdown2/addons/vxlan.py
> +++ b/ifupdown2/addons/vxlan.py
> @@ -51,7 +51,7 @@ class vxlan(Vxlan, moduleBase):
>              },
>              "vxlan-local-tunnelip": {
>                  "help": "vxlan local tunnel ip",
> -                "validvals": ["<ipv4>"],
> +                "validvals": ["<ipv4>,<ipv6>"],
>                  "example": ["vxlan-local-tunnelip 172.16.20.103"]
>              },
>              "vxlan-svcnodeip": {
> @@ -66,7 +66,7 @@ class vxlan(Vxlan, moduleBase):
>              },
>              "vxlan-remoteip": {
>                  "help": "vxlan remote ip",
> -                "validvals": ["<ipv4>"],
> +                "validvals": ["<ipv4>,<ipv6>"],
>                  "example": ["vxlan-remoteip 172.16.22.127"],
>                  "multiline": True
>              },
> @@ -521,7 +521,7 @@ class vxlan(Vxlan, moduleBase):
>              local = self._vxlan_local_tunnelip
>  
>          if link_exists:
> -            cached_ifla_vxlan_local =
> cached_vxlan_ifla_info_data.get(Link.IFLA_VXLAN_LOCAL)
> +            cached_ifla_vxlan_local =
> cached_vxlan_ifla_info_data.get(Link.IFLA_VXLAN_LOCAL) or
> cached_vxlan_ifla_info_data.get(Link.IFLA_VXLAN_LOCAL6)
>  
>              # on ifreload do not overwrite anycast_ip to individual
> ip
>              # if clagd has modified
> @@ -547,7 +547,7 @@ class vxlan(Vxlan, moduleBase):
>  
>          if local:
>              try:
> -                local = ipnetwork.IPv4Address(local)
> +                local = ipnetwork.IPAddress(local)
>  
>                  if local.initialized_with_prefixlen:
>                      self.logger.warning("%s: vxlan-local-tunnelip
> %s:
> netmask ignored" % (ifname, local))
> @@ -559,13 +559,19 @@ class vxlan(Vxlan, moduleBase):
>          if local:
>              if local != cached_ifla_vxlan_local:
>                  self.logger.info("%s: set vxlan-local-tunnelip %s" %
> (ifname, local))
> -                user_request_vxlan_info_data[Link.IFLA_VXLAN_LOCAL]
> =
> local
> +                if local.version == 6:
> +                   
> user_request_vxlan_info_data[Link.IFLA_VXLAN_LOCAL6] = local
> +                else:
> +                   
> user_request_vxlan_info_data[Link.IFLA_VXLAN_LOCAL] = local
>  
>                  # if both local-ip and anycast-ip are identical the
> function prints a warning
>                  self.syntax_check_localip_anycastip_equal(ifname,
> local, self._clagd_vxlan_anycast_ip)
>          elif cached_ifla_vxlan_local:
>              self.logger.info("%s: removing vxlan-local-tunnelip
> (cache
> %s)" % (ifname, cached_ifla_vxlan_local))
> -            user_request_vxlan_info_data[Link.IFLA_VXLAN_LOCAL] =
> None
> +            if cached_ifla_vxlan_local.version == 6:
> +                user_request_vxlan_info_data[Link.IFLA_VXLAN_LOCAL6]
> =
> None
> +            else:
> +                user_request_vxlan_info_data[Link.IFLA_VXLAN_LOCAL]
> =
> None
>  
>          return local
>  
> @@ -1236,7 +1242,7 @@ class vxlan(Vxlan, moduleBase):
>          if remoteips:
>              try:
>                  for remoteip in remoteips:
> -                    ipnetwork.IPv4Address(remoteip)
> +                    ipnetwork.IPAddress(remoteip)
>              except Exception as e:
>                  self.log_error('%s: vxlan-remoteip: %s' %
> (ifaceobj.name, str(e)))
>  
> @@ -1244,7 +1250,7 @@ class vxlan(Vxlan, moduleBase):
>          # purge any removed remote ip
>          old_remoteips = self.get_old_remote_ips(ifaceobj.name)
>  
> -        if vxlan_purge_remotes or remoteips or (remoteips !=
> old_remoteips):
> +        if vxlan_purge_remotes or (isinstance(remoteips,list) and
> remoteips != old_remoteips):
>              # figure out the diff for remotes and do the bridge fdb
> updates
>              # only if provisioned by user and not by an vxlan
> external
>              # controller.
> @@ -1281,8 +1287,8 @@ class vxlan(Vxlan, moduleBase):
>                          "00:00:00:00:00:00",
>                          None, True, addr
>                      )
> -                except Exception:
> -                    pass
> +                except Exception as e:
> +                    self.log_error('%s: vxlan-remoteip<add>: %s' %
> (ifaceobj.name, str(e)))
>  
>          self.vxlan_remote_ip_map(ifaceobj, vxlan_mcast_grp_map)
>  
> 




[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [pve-devel] [PATCH ifupdown2 1/1] Correctly handle IPv6 addresses in vxlan

[Stefan Hanreich]

On 10/9/24 23:05, DERUMIER, Alexandre wrote:
> but we need to be able to not diverge too much from upstream,
> if one day (ok that's waiting since 4 years...) an official
> support is added by cumulus/nvidia.
> (to be honest, the upstream is a lot less active since nvidia have buy
> cumulus/mellanox)
> 
> That's mean maintain both syntax, or plan a config update,
> between 2 majors pve releases. Not a big deal.

yes, it seems a bit tricky, since we'd have to predict what ifupdown2 is
doing - but with the current speed of ifupdown2 development, I'm not
sure if VXLAN IPv6 support will land anytime soon. There's a similar PR
open though in the ifupdown2 github [1].

Since there is currently discussion about moving away from ifupdown2 on
debian-devel [2] [3] and the point of contention is not really about IF
they will be moving away from ifupdown2 but rather WHAT the replacement
will be, we could support our implementation (with possible
compatibility patches) while we move to a new network stack.

> BTW, I don't have followed the ifupdown-ng project since a long time.
> (just follow the early days). Is the project really active and have
> almost same features than ifupdown2 ?  (netlink support, reload support
> with diff of running config,...)

Judging from the current discussion on debian-devel, ifupdown-ng doesn't
seem to be a candidate for the Debian network stack moving forward.
There's some heated discussion between netplan and systemd-networkd,
with some (from my perception) preference for systemd-networkd so we'll
be monitoring that closely for our trixie release.


I'll test this series a bit this week and report back. Would you be
interested in sending a v2 afterwards? (particularly wrt to the perl
style issues). Otherwise I can also take this patch series over. in any
case, have you signed a CLA with us already? Otherwise we cannot include
your patches in our repositories.

[1] https://github.com/CumulusNetworks/ifupdown2/pull/315
[2] https://lists.debian.org/debian-devel/2024/07/msg00098.html
[3] https://lists.debian.org/debian-devel/2024/09/msg00240.html


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

Re: [pve-devel] [PATCH ifupdown2 1/1] Correctly handle IPv6 addresses in vxlan

[Andrew via pve-devel]

[-- Attachment #1: Type: message/rfc822, Size: 6089 bytes --]

From: Andrew <andrew@apalrd.net>
To: Stefan Hanreich <s.hanreich@proxmox.com>
Cc: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>, "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>
Subject: Re: [PATCH ifupdown2 1/1] Correctly handle IPv6 addresses in vxlan
Date: Mon, 25 Nov 2024 15:09:48 +0000
Message-ID: <2DB93EBD-8A9D-4CB9-8992-9A9C85742505@apalrd.net>

Hello,

Yes, I’ve signed the CLA. 

You’re welcome to take it over to fix the style issues. 

At least for Trixie it seems like PVE staying on ifupdown2 would make sense, given how tightly integrated it’s syntax is with SDN.

Andrew

> On Nov 25, 2024, at 05:25, Stefan Hanreich <s.hanreich@proxmox.com> wrote:
> 
> 
> On 10/9/24 23:05, DERUMIER, Alexandre wrote:
>> but we need to be able to not diverge too much from upstream,
>> if one day (ok that's waiting since 4 years...) an official
>> support is added by cumulus/nvidia.
>> (to be honest, the upstream is a lot less active since nvidia have buy
>> cumulus/mellanox)
>> 
>> That's mean maintain both syntax, or plan a config update,
>> between 2 majors pve releases. Not a big deal.
> 
> yes, it seems a bit tricky, since we'd have to predict what ifupdown2 is
> doing - but with the current speed of ifupdown2 development, I'm not
> sure if VXLAN IPv6 support will land anytime soon. There's a similar PR
> open though in the ifupdown2 github [1].
> 
> Since there is currently discussion about moving away from ifupdown2 on
> debian-devel [2] [3] and the point of contention is not really about IF
> they will be moving away from ifupdown2 but rather WHAT the replacement
> will be, we could support our implementation (with possible
> compatibility patches) while we move to a new network stack.
> 
>> BTW, I don't have followed the ifupdown-ng project since a long time.
>> (just follow the early days). Is the project really active and have
>> almost same features than ifupdown2 ?  (netlink support, reload support
>> with diff of running config,...)
> 
> Judging from the current discussion on debian-devel, ifupdown-ng doesn't
> seem to be a candidate for the Debian network stack moving forward.
> There's some heated discussion between netplan and systemd-networkd,
> with some (from my perception) preference for systemd-networkd so we'll
> be monitoring that closely for our trixie release.
> 
> 
> I'll test this series a bit this week and report back. Would you be
> interested in sending a v2 afterwards? (particularly wrt to the perl
> style issues). Otherwise I can also take this patch series over. in any
> case, have you signed a CLA with us already? Otherwise we cannot include
> your patches in our repositories.
> 
> [1] https://github.com/CumulusNetworks/ifupdown2/pull/315
> [2] https://lists.debian.org/debian-devel/2024/07/msg00098.html
> [3] https://lists.debian.org/debian-devel/2024/09/msg00240.html
> 




[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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