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

route: treat routes with via nexthops as universe scoped as well #389

Closed
wants to merge 1 commit into from

Conversation

KanjiMonster
Copy link
Contributor

RTA_VIA is a RTA_GATEWAY with added address family, so we should treat them them equivalent for guessing the scope.

RTA_VIA is a RTA_GATEWAY with added address family, so we should treat
them them equivalent for guessing the scope.

Signed-off-by: Jonas Gorski <[email protected]>
@KanjiMonster
Copy link
Contributor Author

Error: file "include/base/nl-base-utils.h" has style issues.

I didn't even touch that file ...

@thom311
Copy link
Owner

thom311 commented May 29, 2024

Error: file "include/base/nl-base-utils.h" has style issues.

I didn't even touch that file ...

this seems to be a change in behavior of clang-format in Fedora 40. That's a bit annoying.

The version shipped by current Fedora 40 is the correct version for formatting. The file needs reformatting, I will do that.

thom311 pushed a commit that referenced this pull request May 29, 2024
RTA_VIA is a RTA_GATEWAY with added address family, so we should treat
them them equivalent for guessing the scope.

Signed-off-by: Jonas Gorski <[email protected]>

#389
@thom311
Copy link
Owner

thom311 commented May 29, 2024

merged as 3268820.

Thank you for the patch!!

@thom311 thom311 closed this May 29, 2024
@thom311
Copy link
Owner

thom311 commented May 29, 2024

Error: file "include/base/nl-base-utils.h" has style issues.

I didn't even touch that file ...

this seems to be a change in behavior of clang-format in Fedora 40. That's a bit annoying.

The version shipped by current Fedora 40 is the correct version for formatting. The file needs reformatting, I will do that.

if you use ./tools/clang-format-container.sh for formatting (which may be a good idea, if you don't run Fedora 40), then you would need to regenerate the container. Just issue a podman rmi localhost/libnl-code-format-f40.

@KanjiMonster
Copy link
Contributor Author

Error: file "include/base/nl-base-utils.h" has style issues.

I didn't even touch that file ...

this seems to be a change in behavior of clang-format in Fedora 40. That's a bit annoying.

The version shipped by current Fedora 40 is the correct version for formatting. The file needs reformatting, I will do that.

The weird/annyoing thing is that the "original" run ran fine with clang-format 18.1.1, but clang-format 18.1.6 now triggers this.

A version bump that minor shouldn't change behaviour ... .

if you use ./tools/clang-format-container.sh for formatting (which may be a good idea, if you don't run Fedora 40), then you would need to regenerate the container.

Well, since the file I touched was part of the ignored files in ./tools/clang-format.sh, I didn't think to even have to run clang-format ;-).

I did run a manual clang-format call on the file itself though, to make sure the code I touched is at least not introducing more issues.

@thom311
Copy link
Owner

thom311 commented May 29, 2024

@KanjiMonster I think I would release libnl-3.10 soon. Do you have anything that still should go in and to wait for?

There will also be a libnl-3.11 etc., so missing it may not be a problem.

@KanjiMonster
Copy link
Contributor Author

I have nothing urgent, so feel free to release whenever. All open things I have open are either too small to be a blocker, or too large to get fixed/implemented soon.

We are using a heavily patched libnl anyway, so I'm no stranger to backporting things if I need them.

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.

2 participants