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

fix(bpf): sk_lookup_udp for listener only #401

Merged
merged 1 commit into from
Jan 2, 2024

Conversation

jschwinger233
Copy link
Member

@jschwinger233 jschwinger233 commented Jan 2, 2024

Background

Consider two consecutive dig @1.1.1.1 one.one.one.one: dae will create a UDP socket binding 1.1.1.1:53 for the 1st DNS query, then by the time the 2nd one arrives at wan_egress@eth0, sk_lookup_udp will match that newly created UDP socket by mistake.

As lookup for established UDP is not necessary at all, this PR skips that to avoid this tragedy bug.

Fixes: #383

Checklist

Full Changelogs

  • [Implement ...]

Issue Reference

Closes #[issue number]

Test Result

@jschwinger233 jschwinger233 requested a review from a team as a code owner January 2, 2024 11:01
@jschwinger233 jschwinger233 requested a review from mzz2017 January 2, 2024 11:01
@mzz2017
Copy link
Contributor

mzz2017 commented Jan 2, 2024

Great, it's so smooth now.

@sumire88
Copy link
Contributor

sumire88 commented Jan 2, 2024

Has this patch been fully tested yet?

@sumire88 sumire88 changed the title bpf: sk_lookup_udp for listener only patch(bpf): sk_lookup_udp for listener only Jan 2, 2024
@mzz2017 mzz2017 changed the title patch(bpf): sk_lookup_udp for listener only fix(bpf): sk_lookup_udp for listener only Jan 2, 2024
Copy link
Contributor

@mzz2017 mzz2017 left a comment

Choose a reason for hiding this comment

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

Nice catch!

@mzz2017
Copy link
Contributor

mzz2017 commented Jan 2, 2024

Has this patch been fully tested yet?

Yes. Let's move on.

@mzz2017 mzz2017 added the tested label Jan 2, 2024
Copy link
Contributor

@dae-prow dae-prow bot left a comment

Choose a reason for hiding this comment

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

🧪 Since the PR has been fully tested, please consider merging it.

@mzz2017 mzz2017 merged commit 97cb490 into daeuniverse:main Jan 2, 2024
24 checks passed
@sumire88 sumire88 added the fix label Jan 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants