Skip to content

fix(tproxy): add ifindex to routing_result to handle interface mismat…#926

Open
olicesx wants to merge 1 commit intodaeuniverse:mainfrom
olicesx:fix/wan-egress-routing-map-conflict
Open

fix(tproxy): add ifindex to routing_result to handle interface mismat…#926
olicesx wants to merge 1 commit intodaeuniverse:mainfrom
olicesx:fix/wan-egress-routing-map-conflict

Conversation

@olicesx
Copy link
Copy Markdown
Contributor

@olicesx olicesx commented Feb 3, 2026

…ches in WAN egress

Background

Checklist

Full Changelogs

  • [Implement ...]

Issue Reference

Closes #[issue number]

Test Result

Copilot AI review requested due to automatic review settings February 3, 2026 02:19
@olicesx olicesx requested a review from a team as a code owner February 3, 2026 02:19
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes an interface mismatch issue in the tproxy WAN egress path by adding interface index tracking to the routing result cache. When packets traverse different network interfaces, cached routing decisions from one interface could incorrectly be applied to another interface, leading to routing errors.

Changes:

  • Added ifindex field to the routing_result struct to track which interface a routing decision was made for
  • Added interface index validation in WAN egress TCP path to reject cached entries from different interfaces
  • Updated all code paths that create routing_result entries to store the current interface index

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Member

@jschwinger233 jschwinger233 left a comment

Choose a reason for hiding this comment

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

代码没啥问题但我不太确定注释是准确的,请再看一下。

// (e.g., LAN entries used by WAN traffic).
if (routing_result->ifindex != skb->ifindex) {
// Interface mismatch - treat as new connection to get fresh routing.
return TC_ACT_OK;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

代码是对的但注释不对

// Interface mismatch - treat as new connection to get fresh routing.

不是 treat as new connection to get fresh routing,直接 return TC_ACT_OK 没有重新路由、这也不是 new connection 而是 stale connection。

(e.g., LAN entries used by WAN traffic).

讲道理,lan 和 wan 的 routing result 不太可能被混用,因为查询 key 是用流量的 tuple,wan 和 lan 的流量 source ip 应该是不同的(否则局域网上两个设备相同 ip 了)。

我的理解是,这主要是解决多 wan interface 绑定下路由混淆的问题,比如

  • wan 流量1:216.119.143.2 -> 1.1.1.1,路由到了 eth0
  • wan 流量2: 216.119.143.2 -> 1.1.1.1,相同的 tuple,但是由于设置了 mark 所以路由到了 eth1,两个 wan interface 上的 dae 路由会被错误地共享。

Copy link
Copy Markdown
Member

@jschwinger233 jschwinger233 Feb 3, 2026

Choose a reason for hiding this comment

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

嗷我懂了,如果路由上没有 masquerading 那 lan_ingress 的流量会原封不动 forward 到 wan_egress,就会冲突了。

其实 #925 (avoid conflicts with LAN ingress) 的问题也解决了。

那就只有 treat as new connection to get fresh routing 这句注释可能不对

__u8 pname[TASK_COMM_LEN];
__u32 pid;
__u8 dscp;
__u32 ifindex;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

有结构体空洞,但无所谓了,就这样吧。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

结构体里有24B的padding

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.

4 participants