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

linux: add missing netfilter definitions #2876

Merged
merged 1 commit into from Aug 24, 2022
Merged

Conversation

phi-gamma
Copy link
Contributor

@phi-gamma phi-gamma commented Aug 16, 2022

Bring libc up to parity with Linux v5.18 headers wrt. to Netfilter.

libc-test fails due to unrelated issues on my Fedora box but the
changes should be inocuous enough IMO.

Note there’s a warning in the code indicating potential trouble due to this
issue
. I’ll sort this out if
the CI run fails.

@rust-highfive
Copy link

r? @Amanieu

(rust-highfive has picked a reviewer for you, use r? to override)

@phi-gamma
Copy link
Contributor Author

The “Docker Linux Tier1” CI run fails like so:

cargo:warning=/checkout/target/i686-unknown-linux-gnu/debug/build/libc-test-4eae72712e4eb4b5/out/main.c:30472:66: error: 'NFNL_SUBSYS_HOOK' undeclared here (not in a function); did you mean 'NFNL_SUBSYS_ULOG'?

So the caveat about musl relying on pre-historic kernel headers
still seems to apply. Could this change be made a compile-time
conditional? To it sounds rather suboptimal to let one specific
target hold back the crate like this.

Let’s see about those other changes.

@Amanieu
Copy link
Member

Amanieu commented Aug 16, 2022

You can add exceptions to the tests in libc-test/build.rs. You will see many other cases where exceptions are added due to old kernel headers in CI. Make sure to mark the kernel version the constants were introduced in so we can remove the exceptions when upgrading the CI images.

@phi-gamma
Copy link
Contributor Author

phi-gamma commented Aug 16, 2022 via email

@phi-gamma phi-gamma force-pushed the netfilter branch 5 times, most recently from 1784a2f to 78a76f8 Compare August 17, 2022 07:42
pub const NFNL_SUBSYS_HOOK: ::c_int = 12;
pub const NFNL_SUBSYS_COUNT: ::c_int = 13;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

The exception in build.rs should apply to both glibc and musl, so you shouldn't need to add this cfg_if.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True. I could swear I submitted that in an earlier revision and it resulted in a CI failure,
prompting me to add that cfg_if. Anyways, CI seems to be happy now without it.

@Amanieu
Copy link
Member

Amanieu commented Aug 20, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Aug 20, 2022

📌 Commit 8c194e6 has been approved by Amanieu

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Aug 20, 2022

⌛ Testing commit 8c194e6 with merge a7e6fac...

bors added a commit that referenced this pull request Aug 20, 2022
linux: add missing netfilter definitions

Bring ``libc`` up to parity with Linux v5.18 headers wrt. to Netfilter.

``libc-test`` fails due to unrelated issues on my Fedora box but the
changes should be inocuous enough IMO.

Note there’s a warning in the code indicating potential trouble due to [this
issue](#1628). I’ll sort this out if
the CI run fails.
@bors
Copy link
Contributor

bors commented Aug 20, 2022

💔 Test failed - checks-actions

Fill in missing constants available as of Linux v5.18. The
relevant UAPI headers are

- nfnetlink.h
- nfnetlink_log.h
- nfnetlink_queue.h
@phi-gamma
Copy link
Contributor Author

phi-gamma commented Aug 21, 2022 via email

@phi-gamma
Copy link
Contributor Author

S-waiting-on-author

@JohnTitor could you clarify what you’re waiting for? I updated the pr to address
the issues that became apparent during the bors run.

@JohnTitor
Copy link
Member

Oh, missed you pushed a commit after the failure, @bors r+

@bors
Copy link
Contributor

bors commented Aug 24, 2022

📌 Commit 12f3b38 has been approved by JohnTitor

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Aug 24, 2022

⌛ Testing commit 12f3b38 with merge af5de50...

@bors
Copy link
Contributor

bors commented Aug 24, 2022

☀️ Test successful - checks-actions, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13, checks-cirrus-freebsd-14
Approved by: JohnTitor
Pushing af5de50 to master...

@bors bors merged commit af5de50 into rust-lang:master Aug 24, 2022
@phi-gamma
Copy link
Contributor Author

phi-gamma commented Aug 24, 2022 via email

@phi-gamma
Copy link
Contributor Author

phi-gamma commented Oct 11, 2022 via email

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

Successfully merging this pull request may close these issues.

None yet

5 participants