Skip to content

fix l7 ignore retrans only - #11838

Open
yuanchaoa wants to merge 2 commits into
v6.6from
66-fix-l7-ignore-retrans-only
Open

fix l7 ignore retrans only#11838
yuanchaoa wants to merge 2 commits into
v6.6from
66-fix-l7-ignore-retrans-only

Conversation

@yuanchaoa

Copy link
Copy Markdown
Contributor

This PR is for:

  • Agent

fix l7 ignore retrans only

Steps to reproduce the bug

Changes to fix the bug

Affected branches

  • 6.6

Checklist

  • Added unit test to verify the fix.
  • Verified eBPF program runs successfully on linux 4.14.x.
  • Verified eBPF program runs successfully on linux 4.19.x.
  • Verified eBPF program runs successfully on linux 5.2.x.

@yuanchaoa

Copy link
Copy Markdown
Contributor Author

@claude review一下当前pr中的代码修改,结论使用中文

@claude

claude Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Claude finished @yuanchaoa's task in 4m 34s —— View job


任务进度

  • 读取仓库 CLAUDE.md 与 PR 上下文
  • 审查本次 PR 的代码修改
  • 用中文整理并发布 review 结论

Review 结论

我这轮 review 发现 1 个建议修正的问题

  1. agent/src/flow_generator/perf/mod.rs:597
    这里把 l4.parse()所有非 RetransPacket 错误都吞掉了,只在 RetransPacket 时提前返回。结果是 agent/src/flow_generator/perf/tcp.rs:1055 返回 InvalidPacketTimestamp 时,也会继续进入 self.l7_parse()

    这会改变现有语义:对于乱序/旧时间戳的 TCP 包,之前会直接停止本次解析;现在则可能继续做 L7 解析,从而产出重复或时序错误的 L7 日志/指标。按这个 PR 的目标来看,应该是“只对 retrans 场景特殊处理”,而不是把 InvalidPacketTimestamp 这类非良性错误也放过去。

    建议至少继续向上传递 InvalidPacketTimestamp(以及其他不应忽略的错误),只对确认为可放行的场景放宽。

    Fix this →

补充说明

  • agent/src/policy/policy.rs 这部分改动的意图比较清晰,主要是让 OTel 查询绕过 endpoint fastpath;我这轮没有在这里看到同等级别的 correctness 问题。
  • 我尝试执行最小化验证(cargo test --manifest-path agent/Cargo.toml test_handshake_retrans -- --exact),但当前运行环境没有拿到对应执行权限,所以以上结论基于代码阅读和调用链分析。
  • 此外,这个 PR 目前也还没有补上针对上述行为变化的单测,建议在修正时一并加上回归用例。

@yuanchaoa
yuanchaoa force-pushed the 66-fix-l7-ignore-retrans-only branch from 6ae9d04 to 406461f Compare July 31, 2026 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant