Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

addons: vxlan: add vxlan-local-tunnelip6 #182

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

moepman
Copy link
Contributor

@moepman moepman commented Nov 16, 2020

Signed-off-by: Markus Hauschild [email protected]

@moepman
Copy link
Contributor Author

moepman commented Nov 16, 2020

Since #172 seems to be going in the wrong direction I gave it a try using a new, distinct attribute. Please review and comment.

@moepman
Copy link
Contributor Author

moepman commented Nov 16, 2020

For a very simple test case it does show that is has successfully set the local tunnel ipv6 addr:

root@test ~ # ifquery vx-tst
auto vx-tst
iface vx-tst
        vxlan-id 1337
        vxlan-local-tunnelip6 fdef:1337:cafe::1

root@test ~ # ifquery -r vx-tst
auto vx-tst
iface vx-tst
        vxlan-id 1337
        vxlan-port 4789
        vxlan-ageing 300
        vxlan-learning on
        vxlan-local-tunnelip6 fdef:1337:cafe::1/128
        address fe80::38d9:ccff:fe42:8adf/64

) or self.syntax_check_localip_anycastip_equal(
ifaceobj.name,
ifaceobj.get_attr_value_first('vxlan-local-tunnelip6') or self._vxlan_local_tunnelip6,
self._clagd_vxlan_anycast_ip)
return True

def syntax_check_localip_anycastip_equal(self, ifname, local_ip, anycast_ip):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The syntax check will still report "tunnelip" even if tunnelip6 is identical to clagd-vxlan-anycast-ip.

I am unsure wether clagd-vxlan-anycast-ip could even be IPv6 or (if that is not the case) how this should be properly refactored.

@roopa-prabhu
Copy link
Contributor

roopa-prabhu commented Nov 16, 2020 via email

@moepman moepman marked this pull request as ready for review November 17, 2020 14:41
@moepman
Copy link
Contributor Author

moepman commented Nov 19, 2020

I know it is quite a bit of duplicated code, but I am open to suggestions on how to improve this, so that it can be merged.

@moepman
Copy link
Contributor Author

moepman commented Jul 21, 2021

@julienfortin it is a bit sad that you didn't implement a change similar to this to solve #175 in your recent vxlan changes can you still do this or should I give it another try?

@MPStudyly
Copy link

Any updates on this? We just wanted to setup our new cluster ipv6 only to simplify networking a lot and just stumbled across this. Do we really have to resort to v4 to get VXLAN working? This is a huge PITA and not expected at all :(

@julienfortin
Copy link
Contributor

@MPStudyly are you a NVIDIA Cumulus Linux user?

If you need this patch now and if you are happy with it, you can always patch your system and use it.
For CL we can create an FR to bring this into the next release.

I'm quite busy with other stuff at the moment. Though I would like this patch to be different, more like a refactor of the existing ip4 code, and accommodate both v4 and v6 settings through the same function instead of code duplicates.

@MPStudyly
Copy link

@MPStudyly are you a NVIDIA Cumulus Linux user?

No, I'm on Proxmox, using it's native SDN features to setup VXLAN. Proxmox/Debian(generally?) relies on ifupdown2 as well.

If you need this patch now and if you are happy with it, you can always patch your system and use it. For CL we can create an FR to bring this into the next release.

While Proxmox is quite open about local modifications, I'd rather avoid this in a production environment. For our use case we were able to resort to IPv4 in a minimal fashion to work around this limitation. We still want to throw it out ASAP.

I'm quite busy with other stuff at the moment. Though I would like this patch to be different, more like a refactor of the existing ip4 code, and accommodate both v4 and v6 settings through the same function instead of code duplicates.

I fully understand this approach 👍 Yesterdays post was mostly made out of the initial frustration, learning of this (kinda stupid) limitation. Especially after seeing this is being open for almost 4 years now ^^" Don't feel pushed by my request though. I'm already glad to hear this wasn't forgotten :)

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.

4 participants