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

Implement THIS_MODULE. #81

Merged
merged 1 commit into from
Jan 23, 2021
Merged

Implement THIS_MODULE. #81

merged 1 commit into from
Jan 23, 2021

Conversation

alex
Copy link
Member

@alex alex commented Jan 22, 2021

Closes #15
Fixes #10

@alex alex force-pushed the this-module-redux branch 3 times, most recently from 6e23284 to f561250 Compare January 22, 2021 16:04
@alex
Copy link
Member Author

alex commented Jan 22, 2021

error[E0425]: cannot find value `__this_module` in module `kernel::bindings`

Hmm, anyone have a clue what happened that would lead to this?

@ojeda
Copy link
Member

ojeda commented Jan 22, 2021

The -DMODULE flag is not there in c_flags, so __this_module is never seen by bindgen. If it was previously there at one point, it got probably lost in one of the refactors I did (or perhaps I took it out until it was actually needed and/or we discussed whether it was worth to have 2 sets of bindings, one for MODULE and one for !MODULE).

In rust/Makefile, please add it on line:

--size_t-is-usize -o $@ -- $(bindgen_c_flags) -DMODULE

@alex
Copy link
Member Author

alex commented Jan 22, 2021

Nice catch, would have taken me a while to figure out.

rust/kernel/lib.rs Outdated Show resolved Hide resolved
@ojeda
Copy link
Member

ojeda commented Jan 23, 2021

It failed now but not due to the __this_module, so that should be fine.

@alex
Copy link
Member Author

alex commented Jan 23, 2021

Making progress! (My local dev env is messed up ATM, so I'm kind of relying on CI :-))

@ojeda
Copy link
Member

ojeda commented Jan 23, 2021

I can see that :D

I'm sure GitHub doesn't mind being used as an IDE with all the "edit on the web" features they add here and there...

Copy link
Member

@ojeda ojeda left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of this one!

@alex alex merged commit 745bd9d into rust Jan 23, 2021
@alex alex deleted the this-module-redux branch January 23, 2021 00:28
ojeda pushed a commit that referenced this pull request Dec 13, 2023
We can see that "Short form of movsx, dst_reg = (s8,s16,s32)src_reg" in
include/linux/filter.h, additionally, for BPF_ALU64 the value of the
destination register is unchanged whereas for BPF_ALU the upper 32 bits
of the destination register are zeroed, so it should clear the upper 32
bits for BPF_ALU.

[root@linux fedora]# echo 1 > /proc/sys/net/core/bpf_jit_enable
[root@linux fedora]# modprobe test_bpf

Before:
test_bpf: #81 ALU_MOVSX | BPF_B jited:1 ret 2 != 1 (0x2 != 0x1)FAIL (1 times)
test_bpf: #82 ALU_MOVSX | BPF_H jited:1 ret 2 != 1 (0x2 != 0x1)FAIL (1 times)

After:
test_bpf: #81 ALU_MOVSX | BPF_B jited:1 6 PASS
test_bpf: #82 ALU_MOVSX | BPF_H jited:1 6 PASS

By the way, the bpf selftest case "./test_progs -t verifier_movsx" can
also be fixed with this patch.

Fixes: f48012f ("LoongArch: BPF: Support sign-extension mov instructions")
Acked-by: Hengqi Chen <[email protected]>
Signed-off-by: Tiezhu Yang <[email protected]>
Signed-off-by: Huacai Chen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Implement THIS_MODULE
2 participants