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

refactor/optimize: remove dead code #411

Merged
merged 2 commits into from
Jan 23, 2024

Conversation

jschwinger233
Copy link
Member

Background

As #383 got merged, some logic seems obsolete and safe to remove.

This PR removes lan_egress bpf, followed by removal of lanWan flag at control plane.

Checklist

Full Changelogs

  • [Implement ...]

Issue Reference

Closes #[issue number]

Test Result

@dae-prow dae-prow bot added the chore label Jan 4, 2024
@jschwinger233 jschwinger233 marked this pull request as ready for review January 4, 2024 14:15
@jschwinger233 jschwinger233 requested a review from a team as a code owner January 4, 2024 14:15
@jschwinger233 jschwinger233 requested a review from mzz2017 January 4, 2024 14:15
@umlka
Copy link

umlka commented Jan 4, 2024

该PR导致了一个额外的问题:无法解析任何域名

日志如下

time="Jan 05 00:48:07" level=warning msg="handlePkt: listen udp 192.168.1.1:53: bind: address already in use"
time="Jan 05 00:48:08" level=warning msg="handlePkt: failed to write cached DNS resp: listen udp 192.168.1.1:53: bind: address already in use"

@jschwinger233
Copy link
Member Author

@umlka 是不是你在dae运行的主机上同时还有一个进程绑定*:53

@umlka
Copy link

umlka commented Jan 4, 2024

@umlka 是不是你在dae运行的主机上同时还有一个进程绑定*:53

安装在openwrt上面,另一个绑定的进程是dnsmasq,0.5.0版本是正常的,我路由规则添加了pname(dnsmasq) -> must_direct

@umlka
Copy link

umlka commented Jan 4, 2024

@umlka 是不是你在dae运行的主机上同时还有一个进程绑定*:53

安装在openwrt上面,另一个绑定的进程是dnsmasq,0.5.0版本是正常的,我路由规则添加了pname(dnsmasq) -> must_direct

更改dnsmasq的监听端口为非53端口后可以正常使用了,但是这样会出现一些异常情况,比如路由器(通过PPPoE拨号上网, dae无法代理)本机上的DNS解析全部失效以及三星手机连不上路由器Wi-Fi(提示Wi-Fi无网络)
将dae换回0.5.0版本后,通过tcpdump抓包发现手机IP向路由器IP:53端口查询域名connectivity.samsung.com.cn,在dae DNS路由规则内reject该域名后复现上述连不上路由器Wi-Fi的情况(该情况可通过修改DHCP option 6将公告DNS改为任意可直接访问的公共DNS解决)
@jschwinger233

@sumire88
Copy link
Contributor

@jschwinger233 Any updates on this one yet?

@sumire88 sumire88 closed this Jan 11, 2024
@sumire88 sumire88 reopened this Jan 11, 2024
@jschwinger233
Copy link
Member Author

@sumire88 Have to wait #414

@sumire88
Copy link
Contributor

@sumire88 Have to wait #414

Sure, thanks for sharing the progress.

@sumire88
Copy link
Contributor

@sumire88 Have to wait #414

Now that #414 has been merged, shall we move on?

@sumire88 sumire88 changed the title chore: Remove dead code refactor/optimize: remove dead code Jan 12, 2024
With daeuniverse#383, dae no longer encap
UDP, leaving this decap UDP logic obsolete.

Removal of lan_egress also ends up with several helper functions unused,
this commit deletes them as well.

As for scenario of LAN==WAN, after this commit only wan_egress will be
attached on LAN. Wan_egress therefore will pass all traffic sent from
dae because pid_is_control_plane() gives true, so it looks like no
concerns.
Also no longer necessary.
Copy link
Contributor

@sumire88 sumire88 left a comment

Choose a reason for hiding this comment

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

LGTM. Let's move on!

@sumire88
Copy link
Contributor

@jschwinger233 @mzz2017 Shall we move on?

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.

@jschwinger233 jschwinger233 merged commit 883437b into daeuniverse:main Jan 23, 2024
27 checks passed
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