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

Fix for multicast entries ipv6 routes on Linux #59829

Closed
wants to merge 2 commits into from

Conversation

mkosmach
Copy link

What does this PR do?

Add processing for multicast entries in ip -6 route show table all output

What issues does this PR fix or reference?

Patrial fix (only for multicast entries for now) for #58377

Previous Behavior

backtrace whan network.routes used on LInux without netstat

New Behavior

correct ipv6 route entry in network.routes

Commits signed with GPG?

No

…s/network.py

Patrial fix (only for multicast entries for now) for saltstack#58377
@mkosmach mkosmach requested a review from a team as a code owner March 17, 2021 18:10
@mkosmach mkosmach requested review from Ch3LL and removed request for a team March 17, 2021 18:10
@welcome
Copy link

welcome bot commented Mar 17, 2021

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey.
Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar.
If you have additional questions, email us at core@saltstack.com. We’re glad you’ve joined our community and look forward to doing awesome things with you!

Copy link
Contributor

@Ch3LL Ch3LL left a comment

Choose a reason for hiding this comment

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

This will require test coverage and a changelog file

@Ch3LL Ch3LL added the Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases label Mar 29, 2021
Copy link
Contributor

@votdev votdev left a comment

Choose a reason for hiding this comment

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

LGFM

Here is the output of my test:

root@omv6box:/home/vagrant# ip -6 route show table all
::1 dev lo proto kernel metric 256 pref medium
fe80::/64 dev ens6 proto kernel metric 256 pref medium
fe80::/64 dev ens8 proto kernel metric 256 pref medium
local ::1 dev lo table local proto kernel metric 0 pref medium
local fe80::5054:ff:fe27:25a6 dev ens8 table local proto kernel metric 0 pref medium
local fe80::5054:ff:fe58:56f dev ens6 table local proto kernel metric 0 pref medium
multicast ff00::/8 dev ens6 table local proto kernel metric 256 pref medium
multicast ff00::/8 dev ens8 table local proto kernel metric 256 pref medium
root@omv6box:/home/vagrant#
root@omv6box:/home/vagrant#
root@omv6box:/home/vagrant# salt-call --local --retcode-passthrough --no-color --out=json network.routes inet6
{
    "local": [
        {
            "addr_family": "inet6",
            "destination": "::1",
            "gateway": "::",
            "netmask": "",
            "flags": "U",
            "interface": "lo"
        },
        {
            "addr_family": "inet6",
            "destination": "fe80::/64",
            "gateway": "::",
            "netmask": "",
            "flags": "U",
            "interface": "ens6"
        },
        {
            "addr_family": "inet6",
            "destination": "fe80::/64",
            "gateway": "::",
            "netmask": "",
            "flags": "U",
            "interface": "ens8"
        },
        {
            "addr_family": "inet6",
            "destination": "::1/128",
            "gateway": "::",
            "netmask": "",
            "flags": "U",
            "interface": "lo"
        },
        {
            "addr_family": "inet6",
            "destination": "fe80::5054:ff:fe27:25a6/128",
            "gateway": "::",
            "netmask": "",
            "flags": "U",
            "interface": "ens8"
        },
        {
            "addr_family": "inet6",
            "destination": "fe80::5054:ff:fe58:56f/128",
            "gateway": "::",
            "netmask": "",
            "flags": "U",
            "interface": "ens6"
        },
        {
            "addr_family": "inet6",
            "destination": "ff00::/8",
            "gateway": "table",
            "netmask": "",
            "flags": "U",
            "interface": "ens6"
        },
        {
            "addr_family": "inet6",
            "destination": "ff00::/8",
            "gateway": "table",
            "netmask": "",
            "flags": "U",
            "interface": "ens8"
        }
    ]
}

@Ch3LL
Copy link
Contributor

Ch3LL commented Oct 12, 2021

We will need a test added to our test suite, to ensure this does not regress in the future. Here is some documentation on our test suite and how to get started: https://docs.saltproject.io/en/latest/topics/tutorials/writing_tests.html

And here is documentation on how to add a changelog file: https://docs.saltproject.io/en/latest/topics/development/changelog.html

@votdev
Copy link
Contributor

votdev commented Oct 26, 2021

The issue has been resolved for 3004, see https://github.com/saltstack/salt/blob/v3004/salt/modules/network.py#L574.

@Ch3LL
Copy link
Contributor

Ch3LL commented Dec 13, 2021

@mkosmach you okay closing this based on @votdev 's feedback?

@dwoz
Copy link
Contributor

dwoz commented Dec 10, 2023

Closing this due to inactivity. Anyone should feel free to re-open it if they want to see it through to the end in one release cycle.

@dwoz dwoz closed this Dec 10, 2023
@dwoz dwoz added the Abandoned label Dec 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Abandoned Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants