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

thom311 / libnl

Netlink Library Suite

GNU Lesser General Public License v2.1

419 stars 311 forks source link

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

Closed KanjiMonster closed 3 months ago

KanjiMonster commented 3 months ago

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

KanjiMonster commented 3 months ago

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

I didn't even touch that file ...

thom311 commented 3 months ago

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 commented 3 months ago

merged as 326882017876bc22f9d16508a44a8ef98a53d220.

Thank you for the patch!!

thom311 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

@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 commented 3 months ago

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.