Skip to content

Implement THIS_MODULE. #81

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.

Sign up for GitHub

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

Jump to bottom
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 •
edited
Loading

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 alex force-pushed the this-module-redux branch from f561250 to fdab4d5 Compare January 22, 2021 23:59
@alex
Copy link
Member Author

alex commented Jan 22, 2021

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

ojeda
ojeda reviewed Jan 23, 2021
View reviewed changes
@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 alex force-pushed the this-module-redux branch from fdab4d5 to 9e44590 Compare January 23, 2021 00:13
@alex alex force-pushed the this-module-redux branch from 9e44590 to faccb0c Compare January 23, 2021 00:14
@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 •
edited
Loading

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...

ojeda
ojeda approved these changes Jan 23, 2021
View reviewed changes
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
@seehearfeel @chenhuacai
LoongArch: BPF: Fix sign-extension mov instructions
772cbe9 
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 <hengqi.chen@gmail.com>
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ojeda ojeda ojeda approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement THIS_MODULE
2 participants
@alex @ojeda