-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add missing definitions for QNX8 for several BPF-related structures and constants #4769
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| use crate::bpf_insn; | ||
|
|
||
| pub const BPF_LD: u16 = 0x00; | ||
| pub const BPF_LDX: u16 = 0x01; | ||
| pub const BPF_ST: u16 = 0x02; | ||
| pub const BPF_STX: u16 = 0x03; | ||
| pub const BPF_ALU: u16 = 0x04; | ||
| pub const BPF_JMP: u16 = 0x05; | ||
| pub const BPF_RET: u16 = 0x06; | ||
| pub const BPF_MISC: u16 = 0x07; | ||
| pub const BPF_W: u16 = 0x00; | ||
| pub const BPF_H: u16 = 0x08; | ||
| pub const BPF_B: u16 = 0x10; | ||
| pub const BPF_IMM: u16 = 0x00; | ||
| pub const BPF_ABS: u16 = 0x20; | ||
| pub const BPF_IND: u16 = 0x40; | ||
| pub const BPF_MEM: u16 = 0x60; | ||
| pub const BPF_LEN: u16 = 0x80; | ||
| pub const BPF_MSH: u16 = 0xa0; | ||
| pub const BPF_ADD: u16 = 0x00; | ||
| pub const BPF_SUB: u16 = 0x10; | ||
| pub const BPF_MUL: u16 = 0x20; | ||
| pub const BPF_DIV: u16 = 0x30; | ||
| pub const BPF_OR: u16 = 0x40; | ||
| pub const BPF_AND: u16 = 0x50; | ||
| pub const BPF_LSH: u16 = 0x60; | ||
| pub const BPF_RSH: u16 = 0x70; | ||
| pub const BPF_NEG: u16 = 0x80; | ||
| pub const BPF_MOD: u16 = 0x90; | ||
| pub const BPF_XOR: u16 = 0xa0; | ||
| pub const BPF_JA: u16 = 0x00; | ||
| pub const BPF_JEQ: u16 = 0x10; | ||
| pub const BPF_JGT: u16 = 0x20; | ||
| pub const BPF_JGE: u16 = 0x30; | ||
| pub const BPF_JSET: u16 = 0x40; | ||
| pub const BPF_K: u16 = 0x00; | ||
| pub const BPF_X: u16 = 0x08; | ||
| pub const BPF_A: u16 = 0x10; | ||
| pub const BPF_TAX: u16 = 0x00; | ||
| pub const BPF_TXA: u16 = 0x80; | ||
|
|
||
| f! { | ||
| pub fn BPF_CLASS(code: u32) -> u32 { | ||
| code & 0x07 | ||
| } | ||
|
|
||
| pub fn BPF_SIZE(code: u32) -> u32 { | ||
| code & 0x18 | ||
| } | ||
|
|
||
| pub fn BPF_MODE(code: u32) -> u32 { | ||
| code & 0xe0 | ||
| } | ||
|
|
||
| pub fn BPF_OP(code: u32) -> u32 { | ||
| code & 0xf0 | ||
| } | ||
|
|
||
| pub fn BPF_SRC(code: u32) -> u32 { | ||
| code & 0x08 | ||
| } | ||
|
|
||
| pub fn BPF_RVAL(code: u32) -> u32 { | ||
| code & 0x18 | ||
| } | ||
|
|
||
| pub fn BPF_MISCOP(code: u32) -> u32 { | ||
| code & 0xf8 | ||
| } | ||
|
|
||
| pub fn BPF_STMT(code: u16, k: u32) -> bpf_insn { | ||
| bpf_insn { | ||
| code, | ||
| jt: 0, | ||
| jf: 0, | ||
| k, | ||
| } | ||
| } | ||
|
|
||
| pub fn BPF_JUMP(code: u16, k: u32, jt: u8, jf: u8) -> bpf_insn { | ||
| bpf_insn { code, jt, jf, k } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| use crate::prelude::*; | ||
|
|
||
| s! { | ||
| pub struct ifreq { | ||
| /// if name, e.g. "en0" | ||
| pub ifr_name: [c_char; crate::IFNAMSIZ], | ||
| pub ifr_ifru: __c_anonymous_ifr_ifru, | ||
| } | ||
| pub struct ifreq_buffer { | ||
| pub length: size_t, | ||
| pub buffer: *mut c_void, | ||
| } | ||
| } | ||
|
|
||
| s_no_extra_traits! { | ||
| pub union __c_anonymous_ifr_ifru { | ||
| pub ifru_addr: crate::sockaddr, | ||
| pub ifru_dstaddr: crate::sockaddr, | ||
| pub ifru_broadaddr: crate::sockaddr, | ||
| pub ifru_buffer: ifreq_buffer, | ||
| pub ifru_flags: [c_short; 2], | ||
| pub ifru_index: c_short, | ||
| pub ifru_jid: c_int, | ||
| pub ifru_metric: c_int, | ||
| pub ifru_mtu: c_int, | ||
| pub ifru_phys: c_int, | ||
| pub ifru_media: c_int, | ||
| pub ifru_data: *mut c_char, | ||
| pub ifru_cap: [c_int; 2], | ||
| pub ifru_fib: c_uint, | ||
| pub ifru_vlan_pcp: c_uchar, | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -877,7 +877,7 @@ pub const MS_SYNC: c_int = 2; | |||||
| pub const SCM_RIGHTS: c_int = 0x01; | ||||||
| pub const SCM_TIMESTAMP: c_int = 0x02; | ||||||
| cfg_if! { | ||||||
| if #[cfg(not(target_env = "nto71_iosock"))] { | ||||||
| if #[cfg(not(any(target_env = "nto71_iosock", target_env = "nto80")))] { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are these not available on 8.0 either? If this change is related to making tests pass on 8.0 rather than as part of the BPF, would you be able to put it in a separate commit?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, these are not available on 8.0, so this is part of the BPF topic.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still don't follow the connection - why are sysctl things like
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry for the confusion. I was just referring specifically to the There are many other constants defined there that are unrelated to BPF. My assumption is that
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That makes more sense to me, but I'd like the maintainers to double check since it's a bit surprising. @flba-eb @gh-tr @jonathanpallant @japaric could you review and/or test the changes to these constants on QNX 8?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks okay, but I suggest to inverse the logic:
Suggested change
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are these constants all only for the old network stack then? A comment would be nice then if you could add one @rafaeling. |
||||||
| pub const SCM_CREDS: c_int = 0x04; | ||||||
| pub const IFF_NOTRAILERS: c_int = 0x00000020; | ||||||
| pub const AF_INET6: c_int = 24; | ||||||
|
|
||||||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably has to go in
s_no_extra_traitssince the union doesn't have them (you can check that things work with--feature extra_traits)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved the structs into
s_no_extra_traits(as linux does) but building with--features extra_traitsstill fails because the union would needEq,Hash, andPartialEq.On the BSD targets (Apple/FreeBSD/OpenBSD) these unions do implement
Eq,Hash, andPartialEqandstruct ifreqis intos!macro, should I mirror that behavior here as well? (I tried it and also fails with same error, not sure how to proceed)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What exactly is failing? If
__c_anonymous_ifr_ifruandifreqare both ins_no_extra_traits, that macro shouldn't try to derive them.Don't handwrite trait impls, we do have them in some places but we're trying to move away from that because of the easy accidental unsoundness with unions.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dont know why, but retried again and works now, maybe did something wrong before.
Check last commit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I'm going to need to wait for somebody to look at #4769 (comment) though.
This will need a squash before merge.