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

If ip6 but no gw6 is provided, fall back to ra #1

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

Conversation

JanGerritsen
Copy link

Current behavior and problem:
IPv6 router advertisements are only evaluated, if the PVE configuration is set to SLAAC. It is not possible to set a fixed (static) IPv6 address, but let the system discover the address of the router.

Change:
Since the gateway address is already optional, this patch changes the following:
If a static IPv6 address is specified, but no gateway address is given, enable router discovery via router advertisement.
If both are specified, router advertisement stays disabled.

PS: This is my first patch to this project. I couldn’t find a contributor documentation, so I am not sure what your procedure for patches is. Please let me know if you need additional input.

PPS:
Since the indentation of the original code was off, I have tried to match it as good as possible.

@ThomasLamprecht
Copy link
Contributor

Hello! Thanks for your contribution, but we do not use GitHub for our workflow, the repos here are a read-only mirror to avoid name-squatting (happened in the past).

Please see our wiki for how to contribute:
https://pve.proxmox.com/wiki/Developer_Documentation

Note also the CLA requirement (see at the bottom).

The general intent of your patch seems alright, but I did not look into it all too closely.

W.r.t. to your code-changes some comments upfront:

  • the indentation is wrong, we curretnly still use a 4 level indentation with every two full level been collapsed into a tab:
    https://pve.proxmox.com/wiki/Perl_Style_Guide
  • the $gw variable is unused
    You could simply use a post-if here:
$accept_ra = 'true' if !defined($d->{gw6});

or keep the "full-blown" if, but drop the my $gw = part if it's not used anyway.

@deviantintegral
Copy link

I filed a feature request similar to this at https://bugzilla.proxmox.com/show_bug.cgi?id=5538.

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

Successfully merging this pull request may close these issues.

3 participants