-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
bpf: xdp replace inline asm in csum helpers #32543
base: main
Are you sure you want to change the base?
bpf: xdp replace inline asm in csum helpers #32543
Conversation
463bf29
to
45f6b05
Compare
@julianwiedmann please review |
bpf/include/bpf/ctx/xdp.h
Outdated
@@ -156,28 +156,15 @@ l3_csum_replace(const struct xdp_md *ctx, __u64 off, const __u32 from, | |||
__u32 flags) | |||
{ | |||
__u32 size = flags & BPF_F_HDR_FIELD_MASK; | |||
__sum16 *sum; | |||
int ret; | |||
__sum16 sum[1]; |
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.
why not just __sum16 sum;
and then pass &sum
to the different functions?
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.
Inside xdp_load_bytes, there is a part that calls memcpy.
This is fine if it's an inline asm, but
xdp_load_bytes, it requires memory.
What do you think?
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.
yep, xdp_load_bytes
takes a pointer, I was just suggesting to use:
__sum16 sum;
// ..
ret = xdp_load_bytes(ctx, off, &sum, 2);
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.
Oh, that was my mistake, I apologize. I'll accept the offer right away
/test |
a440100
to
4cb5680
Compare
4fc7847
to
1235c76
Compare
/test |
I retriggered the failed tests, not sure if the failures are related to these changes |
I think there's something wrong with my code, I'll check it out a bit more and fix it. |
This commit is a refactoring due to native support for the bpf_xdp_load_bytes. Signed-off-by: Jack-R-lantern <tjdfkr2421@gmail.com>
1235c76
to
c025bce
Compare
Please ensure your pull request adheres to the following guidelines:
description and a
Fixes: #XXX
line if the commit addresses a particularGitHub issue.
Fixes: <commit-id>
tag, thenplease add the commit author[s] as reviewer[s] to this issue.
This commit is a refactoring due to native support for the bpf_xdp_load_bytes.
replace inline asm to native support xdp_load_bytes()
Fixes: #29487