Skip to content

Conversation

@wido
Copy link

@wido wido commented Nov 6, 2024

This commit adds the option to pass an IPv6 address instead of an IPv4 address to use as local tunnel IP address. With this change it's possible to use IPv6 as the underlay for a VXLAN based network without the need for IPv4.

This commit adds the option to pass an IPv6 address instead of an IPv4 address to use as
local tunnel IP address. With this change it's possible to use IPv6 as the underlay for
a VXLAN based network without the need for IPv4.
@jbemmel
Copy link

jbemmel commented Feb 2, 2025

"vxlan-remoteip": {
                "help": "vxlan remote ip",
                "validvals": ["<ipv4>"],   <!-- Needs changing
                "example": ["vxlan-remoteip 172.16.22.127"],
                "multiline": True
            },

vxlan-remoteip needs to allow ipv6 too (was this PR tested?)

@wido
Copy link
Author

wido commented Feb 3, 2025

"vxlan-remoteip": {
                "help": "vxlan remote ip",
                "validvals": ["<ipv4>"],   <!-- Needs changing
                "example": ["vxlan-remoteip 172.16.22.127"],
                "multiline": True
            },

vxlan-remoteip needs to allow ipv6 too (was this PR tested?)

Only in case you set a static address. I didn't test that part as I mainly work with EVPN and BGP with VXLAN.

@hvisage
Copy link

hvisage commented Feb 10, 2025

I noticed in #182 #172 #175 that the emphasis was on a separate vxlan-<local|remote>ipv6 attributes -> I do like the way this code differentiates in the same code flow for both the v4 & v6 IPs

@jbemmel
Copy link

jbemmel commented Feb 10, 2025

I noticed in #182 #172 #175 that the emphasis was on a separate vxlan-<local|remote>ipv6 attributes -> I do like the way this code differentiates in the same code flow for both the v4 & v6 IPs

It maps to /etc/network/interfaces config items (see e.g. https://manpages.ubuntu.com/manpages/jammy/man5/interfaces-vxlan.5.html) so those other PRs would add needless complexity by splitting things up that then have to be recombined

So this PR is on the right track, but I think it should add support for static IPv6 VXLAN tunnels as well, not just vxlan-local-tunnelip

@apalrd
Copy link

apalrd commented Mar 31, 2025

I agree remoteip must support IPv6 as well.

I am glad there isn't support for splitting up the attributes, that seemed like a poor choice. But it's been almost 5 years with no progress on supporting IPv6 in vxlan in ifupdown2.

@hvisage
Copy link

hvisage commented Mar 31, 2025

Is it only ifupdown2 that needs to add support, ie. the kernel (via things like iproute2) already supports it?

@apalrd
Copy link

apalrd commented Mar 31, 2025

git blame on iproute2 shows it has supported IPv6 since vxlan was added to iproute2 13 years ago

ifupdown2 somehow still does not support it, and over the years, has ignored patches to support it (which you pointed to), despite issue being brought up in discussions all the way back in 2018 (#50)

It looks like ifupdown-ng doesn't have any issues with IPv6 in this case either, their commit to merge vxlan support in 2020 does not appear to have any v4/v6 sensitivities. They however are just calling iproute2 with the IP (and iproute2 works properly), so they won't catch errors if the user tries to change the address family of an interface which already exists (not that ifupdown2 would either).

This patch has been floating around for 5 months now with no action to merge. Sure, the remoteip issue probably should be fixed, but that won't stop anyone using EVPN, and a patch to fix remoteip was already submitted 5 years ago (#172) that we could pull from, except Julien rejected that one for dumb reasons.

@julienfortin who do we have to ping to get stuff merged into ifupdown2? It's embarrassing how long it's taken to get proper IPv6 support here

@wido
Copy link
Author

wido commented Apr 2, 2025

I will try to get some work done in this PR to add the missing IPv6 features. iproute2 is indeed not the problem, it's ifupdown2.

@julienfortin
Copy link
Contributor

@apalrd nothing prevents you from taking any patches you want on your own local copy and building your own deb :)

There's already enough work for me, I'm slow taking on patches as I don't want to introduces conflicts and regression. I'll try to push my latest changes soon.

@apalrd
Copy link

apalrd commented Apr 2, 2025

I built my own deb from #172 a year ago and I've been happy with it. I'm trying to support other installations and not having v6 support for vxlan in Proxmox is kinda a big deal for me. I could hand off my debs to everyone who asks for it, but I could also work to get the changes merged upstream.

The Proxmox team is basically deciding on if they should keep their own patch set for this or waiting for upstream (this repo) to merge them, and don't want to merge a patch which will be rejected here if the attribute names may change - since you previously demanded a new attribute vxlan-local-tunnelip6 - and naming changes would obviously break their vxlan automation. So IPv6 support is in limbo there until ifupdown2 either merges or rejects any one of these changes.

@hvisage
Copy link

hvisage commented Apr 2, 2025

I'm slow taking on patches as I don't want to introduces conflicts and regression.

The Proxmox team is basically deciding on if they should keep their own patch set for this or waiting for upstream (this repo) to merge them,

Yeah, the reasons I'm interested in this, as it'll mean either a fork of this repo to makes things work in PVE etc. or somebody assisting you in making it "official" and we don't have yet another fork in the wild

@scyto

This comment was marked as outdated.

@apalrd
Copy link

apalrd commented Apr 23, 2025

@julienfortin Do you have any changes on your end which would affect vxlan, or do you mean 'your changes' (and presumably Nvidia's changes) in general?

@julienfortin
Copy link
Contributor

julienfortin commented Apr 24, 2025

@julienfortin Do you have any changes on your end which would affect vxlan, or do you mean 'your changes' (and presumably Nvidia's changes) in general?

My changes are Nvidia changes. I pushed most of it a few months ago - since then we've fixed a few issues and added a couple features/policies (there's not a lot of new changes in the vxlan addon).
I will push those changes in a few weeks. I suggest not sending any PR until you see the latest code to avoid unnecessary conflicts.

In the next few weeks i'll also be working on some refactorings and performance improvements.

I can try to reach out to a debian dev to make a new upload. If you know any please tag them :)

@guerby
Copy link

guerby commented Jul 7, 2025

@julienfortin Do you have any changes on your end which would affect vxlan, or do you mean 'your changes' (and presumably Nvidia's changes) in general?

My changes are Nvidia changes. I pushed most of it a few months ago - since then we've fixed a few issues and added a couple features/policies (there's not a lot of new changes in the vxlan addon). I will push those changes in a few weeks. I suggest not sending any PR until you see the latest code to avoid unnecessary conflicts.

In the next few weeks i'll also be working on some refactorings and performance improvements.

I can try to reach out to a debian dev to make a new upload. If you know any please tag them :)

Hi, is there any reason to not merge this PR? github says it applies cleanly. Thanks!

@MatthieuCoder
Copy link

@julienfortin Do you happen to have any news regarding the merging of this PR into the upstream ifupdown2 ?

@apalrd
Copy link

apalrd commented Sep 12, 2025

@juleienfortin do you need more maintainer help for ifupdown2? This PR has been around for almost a year now, and it solves the same issue as another open and PR from 2020 which you also never merged. As far as we know there is nothing wrong with either of these PRs, they just need to be reviewed by you to be merged.

If nvidia is unable to support ifupdown2 then the community would be happy to review PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants