8000 bpf: nodeport: fix-up error check in rev_nodeport_lb*() for XDP by julianwiedmann · Pull Request #23119 · cilium/cilium · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

bpf: nodeport: fix-up error check in rev_nodeport_lb*() for XDP #23119

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

Merged
merged 1 commit into from
Jan 17, 2023
Merged

bpf: nodeport: fix-up error check in rev_nodeport_lb*() for XDP #23119

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions bpf/lib/nodeport.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1001,7 +1001,7 @@ static __always_inline int rev_nodeport_lb6(struct __ctx_buff *ctx, __u32 *ifind
#endif

ret = lb6_extract_tuple(ctx, ip6, &l4_off, &tuple);
if (ret) {
if (ret < 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (ret < 0) {
if (IS_ERR(ret)) {

Copy link
Member

Choose a reason for hiding this comment

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

I agree we should be using this style in general, but there are several other places in this file that are not yet using this. @julianwiedmann , would you mind following up on this separately to update these and other remaining if (ret < 0) cases in this file?

Copy link
Member Author

Choose a reason for hiding this comment

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

@joestringer yeah I plan to spend a few cycles on the wider error handling scheme.

Think with a bit of effort we should be able to banish CTX_ACT_DROP from all/most our own code (just return a proper negative DROP_* everywhere), and translate all helper calls (eg. for ctx_redirect(), don't we want some DROP_MISSED_BPF_REDIRECT visibility anyway?). At which point IS_ERR() might just turn into a simple ret < 0 check.

if (ret == DROP_NO_SERVICE || ret == DROP_UNKNOWN_L4)
goto out;
return ret;
Expand Down Expand Up @@ -2006,7 +2006,7 @@ static __always_inline int rev_nodeport_lb4(struct __ctx_buff *ctx, __u32 *ifind
#endif /* ENABLE_EGRESS_GATEWAY */

ret = lb4_extract_tuple(ctx, ip4, &l4_off, &tuple);
if (ret) {
if (ret < 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (ret < 0) {
if (IS_ERR(ret)) {

/* If it's not a SVC protocol, we don't need to check for RevDNAT: */
if (ret == DROP_NO_SERVICE || ret == DROP_UNKNOWN_L4)
goto out;
Expand Down
0