Merge blocked
欢迎来到 openUBMC 社区
Hey @xiaoxinglxs , 感谢你对社区的贡献.
机器人使用手册
有关指令的使用,可以点击 说明文档 或 论坛 查看详情。开发人员可以在每个PR或Issue下方评论特定指令来触发机器人任务。您可以自助配置此仓库的 PR 合并规则,更多详情,请参阅机器人自助配置使用手册。
联系指引
有疑问可以联系 SIG: sig-component-drivers ,
维护者是: @dongyushan, @net_swift_wangchen, @sam_h1973, @u012924942, @weixin_37569011, @xiaoxinglxs ,
优秀贡献者是: @hzy1010, @leessential .


CI已开始运行,查看流水线详情:流水线链接


/ai review


代码审查
审查结论
按文件确认(4 个变更文件均已逐一审查)
| 文件 | 变更内容 | 结论 |
|---|---|---|
mds/service.json |
版本 1.130.15 → 1.130.21 |
无问题(仅值变更,键名拼写正确,版本单调递增) |
src/lualib/mctp_discovery.lua |
UUID 身份派生、身份后台任务、Control 查询应答、长度校验、Set EID 处理 | 4 项问题(见下) |
src/lualib/mctp_endpoints/ep_mctp_ctrl.lua |
响应沿原请求路由返回、RQ/TO 与 datagram 准入 | 无问题(dest = src 与 ep_mctp:copy_ctxs 在 rx 时刷新 ctx 的语义一致;msg.rq ~= tag_owner 对请求/响应两个方向均成立) |
test/unit/test_discovery.lua |
新增 TestMctpControlAdmission 及既有用例补齐 ep_name 实参 |
1 项问题(缺关键接线用例);其余新用例断言与 mctp_cmds 位域定义(<<allocation:2,_:2,assignment:2,_:2,eid_set,eid_pool_size>>)逐字节核对一致 |
发现统计
- P0:0
- P1:0
- P2:1 —
has_pldm_responder依赖的mctp_engine.pldm_responder.reg_count全仓无任何写入方(只有单测手工赋值),生产恒为 nil,导致Get Message Type Support永远上报"无消息类型"、Get Version Support(type=1)永远回 0x80,PR 声称的动态能力上报不可达。 - P3:4 —
version_response把请求第 2 字节当版本串使用(#number潜在异常);身份任务加载成功后 1Hz 永久空转不 stop;身份加载失败把 pcall 的err丢弃且仅 debug 打印一次;init中身份任务注册(生产 UUID 唯一来源)无任何用例覆盖。
总体风险判断
本次改动方向正确且防御性输入校验(RQ/TO、datagram、固定长度、保留 EID、UUID 格式与冻结校验)做得较完整,Set EID / Get EID 的地址与 Tag/IID 回包逻辑经代码级核对自洽,无内存、注入或越权类高危问题,建议不阻塞合入。
主要风险集中在"能力发现"这条新链路上:pldm_responder 无生产者会让新增的两个查询命令恒回"不支持",属于功能哑火而非崩溃,但会误导对端放弃 PLDM 建链,建议合入前补上登记源或先固定为不支持;身份任务的空转与错误静默则会影响长期可维护性与现场定位效率。
| 类型 | 数量 |
|---|---|
| 🔴 阻塞 | 0 |
| 🟡 建议 | 5 |
💬 仅评论


🔵 Low Priority
新增的 init 分支(mctp_discovery.lua:82-92)用 c_tasks.get_instance():new_task('mctp_identity'..system_id):loop(...) 注册每 1000ms 执行一次的后台任务,但循环体只有 if not self.endpoint_uuid then self:load_endpoint_identity() end,成功后没有任何 :stop()/返回终止逻辑,self.identity_task 也从未在其它地方被引用(全仓仅此一处赋值)。
证据链:同类任务 start_update_route_table/start_smbus_eid_poll_task 均有显式 stop 入口(stop_update_route_table 等),而身份任务没有;且 update_os_power_status(false) → set_discovered(false) → on_discovery_changed 只做 mctp_engine:reset() 与 stop_update_route_table(),不会停止身份任务。
失败模式:身份一旦加载成功,该任务仍按 1Hz 永久唤醒(每个 system 一个),空转消耗 skynet 定时器与调度;系统下电/复位也不会停止,属于长期存在的后台空转任务。功能上不致命,但属于本次新增的可靠性/资源浪费点。
建议:在循环体内于身份加载成功后停止任务,例如 if self.endpoint_uuid then self.identity_task:stop(); return end,并在 update_os_power_status/mctp_engine:reset() 路径中一并停止或重启该任务,避免永久空转。


🔵 Low Priority
新增的 load_endpoint_identity(mctp_discovery.lua:132-168)用 local ok, loaded = pcall(...) 包裹整段 DBus 取 UUID 逻辑,但失败时把 pcall 返回的错误对象完全丢弃:日志固定为 '[System%s]MCTP endpoint identity is not ready' 且级别是 log:debug,并用 identity_warned 只打印一次。
证据链:该函数是 endpoint_uuid 的唯一生产来源(Get Endpoint UUID 直接依赖它,未就绪即回 NOT_READY),而其中包含两套不同调用面——先是 bus:timeout_call(...) 调 bmc.kepler.Mdb.GetSubObjects,随后是 bus.bus:timeout_call(...) 调 GetAllWithContext(同一对象两种访问路径)。任一处签名/字段名(如 bus.bus、GetSubObjects 的 a{ss}sias 入参、values.UUID:value())与实际框架不符,都会被 pcall 静默吞掉。
失败模式:只要该调用面与真实框架有偏差,Get Endpoint UUID 将永久返回 NOT_READY,而现场只有一条 debug 级、无错误详情的日志,无法定位是对象不存在、存在歧义还是 API 不匹配;默认日志级别下等同于完全静默。
建议:把 pcall 的第二个返回值保存下来(local ok, err = pcall(...)),首次失败时用 info/warn 级别记录 err(后续重试仍可保持仅一次),便于现场判断是对象缺失/多实例还是 DBus API 不匹配。


🔵 Low Priority
新增行 local typ, versions = req.req_data:byte(1)(mctp_discovery.lua:177):string.byte 多返回值语义使 versions 取到的是请求第 2 个字节,而其余分支又把 versions 当作"版本列表字符串"参与 #versions // 4 与字符串拼接。
证据链:正常路径下 control_rx_handler 已用 REQUEST_LENGTH[4] = 1 保证 payload 恰为 1 字节,此时 byte(1) 只返回一个值、versions 为 nil,因此暂时不炸;但 version_response 是公开方法,一旦被其它入口(如后续 Control 分发改造、直接单测/复用)以 2 字节以上 payload 调用,typ 落在 0/255/(1 且已注册) 之外时,versions 会是第 2 个字节的数字(Lua 中 0 也是真值),随后执行 #versions 直接抛 attempt to get length of a number value,请求处理协程报错、该请求得不到任何响应。
失败模式:payload 长度 >1 且 message type 不受支持 ⇒ 运行时异常而非按 DSP0236 回 0x80。同时该赋值语义上具误导性(versions 既表示"第 2 字节"又表示"版本串")。
建议:只取第一个字节并显式声明版本串变量,例如 local typ = req.req_data:byte(1) + local versions,消除把数字当字符串长度的隐患。
| 177
| - local typ |
|
177 | + local typ = req.req_data:byte(1) |
|
178 | + local versions |


changed this line on 8cfca330 view diff detail
🟡 Medium Priority
变更行(mctp_discovery.lua:171-173、235、184)新增 has_pldm_responder(),读取 self.mctp_engine.pldm_responder.reg_count,并据此决定 Get Message Type Support 返回 '\1\1'(支持 PLDM)还是 '\0'(不支持),以及 Get MCTP Version Support(type=1) 返回 0x80 还是 DSP0241 绑定版本。
证据链:在 src 全量 grep pldm_responder|reg_count,除本文件的读取处外没有任何赋值/注册代码(src/lualib 下 mctp_engine.lua、ep_pldm.lua、mctp_mdb_mgmt.lua 均无该字段);单测中也是测试手工写入 self.engine.pldm_responder = { reg_count = 1 }(test/unit/test_discovery.lua:163)才使该分支成立。
失败模式:生产环境 pldm_responder 恒为 nil ⇒ Get Message Type Support 永远回 "0 个消息类型",Get Version Support(type=1) 永远回 0x80 不支持。请求方(Host 固件/其它端点)据此判定 BMC 无 PLDM 能力而放弃 PLDM 建链,PR 声称的"按实际已注册的 PLDM responder 动态上报能力"实际不可达;单测覆盖的是生产不可能出现的状态,形成"测试绿、功能哑"的假阳性。另外若将来有人写入一个不含 reg_count 的表,responder.reg_count > 0 会抛 nil 比较异常(该路径也被本次新增的 1Hz 身份/查询路径直接暴露)。
建议:在真正登记 PLDM responder 的位置(如 mctp_endpoints/ep_pldm.lua 或 mctp_engine 的 endpoint 注册/释放回调)维护 mctp_engine.pldm_responder(至少含 reg_count),并在 has_pldm_responder 中对 reg_count 做类型判断(type(responder.reg_count) == 'number' and responder.reg_count > 0);若本轮确实无法接入登记源,则应先固定返回"不支持",并在 PR/注释中明确该能力暂不可用,避免单测伪造不存在字段。


changed this line on 8cfca330 view diff detail
🔵 Low Priority
新增的核心行为"后台有界读取掉电保存的 Assembly.UUID"依赖 init() 中 if self.mctp_engine.bus then ... new_task('mctp_identity'..system_id):loop(...):set_timeout_ms(1000) end 这段接线(mctp_discovery.lua:82-92):它是在生产环境中给 endpoint_uuid 赋值的唯一入口,没有它 Get Endpoint UUID 永远回 NOT_READY。
证据链:新增测试 TestMctpControlAdmission:setup 用 c_mctp_engine.new(nil, 1) 构造(bus == nil),identity 分支被跳过;test_identity_discovery_retries_and_freezes_valid_device_uuid(test/unit/test_discovery.lua:177-234)是在构造之后直接调用 load_endpoint_identity(),并且 make_fake_task 的 loop() 直接返回而不执行循环体(test/unit/test_discovery.lua:26-28)。
失败模式:若 init() 中的任务注册写错(任务名重复、loop 回调签名不匹配、set_timeout_ms 调用顺序变化)或该分支被漏掉,endpoint_uuid 在生产中永远为 nil,而 114 个单测仍全绿,回归完全不可见。PR 自验证报告也承认"后台身份任务的注册/自动重试接线尚待实际部署验证"。
建议:补一个用例:用带 bus 的 engine 构造 discovery(stub c_tasks.get_instance 记录被创建的任务名与 set_timeout_ms 参数,并让 fake task 的 loop 捕获回调),断言 init 后存在名为 mctp_identity<system_id> 的任务、超时 1000ms,且手动执行一次回调后 endpoint_uuid 被写入、再次执行不会再发 DBus 调用。


###注意:
当前PR代码已经发生变化(代码更新或者PR关闭),所以移除以下标签: limit_1k_suc.


CI已开始运行,查看流水线详情:流水线链接


人工检视报告
感谢 @xiaoxinglxs 的贡献!本次改动方向正确,补齐了 Control 查询应答、稳定设备 UUID 身份与 Set EID 语义,并补充了较完整的单测覆盖;防御性输入校验(RQ/TO、datagram、固定请求长度、保留 EID、UUID 格式与冻结校验)做得比较扎实,无内存/注入/越权类高危问题,建议不阻塞合入。
📋 检视概览
| 项目 | 详情 |
|---|---|
| 评级 | 良好 |
| 严重问题 | 0 |
| 警告(未处理) | 1 |
| 建议(未处理) | 1 |
已确认修复的 AI 机器人意见 ✅
逐一复核了此前机器人意见在当前提交(8cfca330)下的状态:
| # | 意见 | 当前状态 |
|---|---|---|
| 1 | 身份任务 1Hz 永久空转不 stop | ✅ 已修复:循环体已加入 task:stop()(mctp_discovery.lua:83-90) |
| 2 | version_response 多返回值 byte(1) 的取长度隐患 | ✅ 已修复:改为 local typ = byte(1) 加独立 local versions(mctp_discovery.lua:182-183) |
| 3 | 身份任务注册/停止接线无覆盖 | ✅ 已补充:test_identity_background_retries_and_stops_after_success 覆盖任务创建、超时参数与回调停止逻辑 |
| 4 | has_pldm_responder 依赖的 pldm_responder.reg_count 全仓无写入方 | ⚠️ 仍待处理(中优先级,见下) |
| 5 | load_endpoint_identity 丢弃 pcall 错误、仅打印一次 | ⚠️ 部分处理:日志级别已提升为 notice,但 err 仍被丢弃(低优先级,见下) |
需关注的问题(沿用已有行内评论,不重复提交)
| # | 文件 | 级别 | 说明 |
|---|---|---|---|
| 1 | src/lualib/mctp_discovery.lua:175-178 | 🟡 警告 | 对应上方意见 4:pldm_responder.reg_count 在 src/ 内无任何写入方,生产环境 has_pldm_responder() 恒为 false。需在真实 PLDM responder 登记源维护该字段(并对其加 type() == number 判断),否则 Get Message Type Support / Get Version Support(type=1) 将永远回不支持,新能力链路实际不可达(且若将来写入不含 reg_count 的表会抛 nil 比较异常)。 |
| 2 | src/lualib/mctp_discovery.lua:156-160 | 💡 建议 | 对应上方意见 5:pcall 的第二个返回值 err 仍被丢弃,现场若 DBus 调用面与真实框架不符,只能看到一条 notice 日志而无法定位具体失败原因,建议首次失败时带上 err 详情。 |
最终结论
本次提交相比上一版已实质推进:3 条低风险意见(任务空转、字节多返回值、接线测试)已修复或补齐覆盖,代码质量良好。核心剩余风险聚焦在 PLDM 能力上报的生产者缺失——建议合入前为 mctp_engine.pldm_responder 接入真实登记源(或本轮先固定返回不支持并明确注释),避免测试绿、功能哑的假阳性。其余 Control 回包地址/Tag/IID、Set EID 语义、UUID 稳定性经代码级与字节级核对一致。


| 任务名称 | 结果 |
|---|---|
| pre-commit检查 | ✅ SUCCESS |
| PR描述检查 | ✅ SUCCESS |
| 防投毒检查 | ✅ SUCCESS |
| 开源合规检查 | ✅ SUCCESS |
| 编译构建 | ✅ SUCCESS |
| UT测试 | ✅ SUCCESS |
| IT测试 | ✅ SUCCESS |
| 代码覆盖率 | ✅ SUCCESS |
| 修改行数检查 | ✅ SUCCESS |
| 头文件检查 | ✅ SUCCESS |
| AI Review | ✅ SUCCESS |


当前PR是否有AI参与:
[ ] 否
[x] 是
__1. AI Agent 平台: OpenAI Codex
__2. AI 模型: gpt-6-astra
__3. Prompt上下文 : 继续BMC侧SP888-MCTP开发,补齐Control基础查询并进行独立只读复核、真实服务验证;mctpd PR仅提交评审,暂不合入。
PR功能描述 / 为什么需要这个合入:
Get Endpoint ID调用了不存在的方法,收到正常查询会抛错;查询响应固定发给Bus Owner,也会错过其他请求端。UUID、版本及消息类型查询缺失,妨碍能力发现。
修改方案:
验证方案:
真实C++协议入口执行Control线包测试,检查地址、Tag/IID、错误完成码、非法输入不改状态、UUID稳定性和能力动态变化;真实来宾IPC探针及后续集成线包验收分开记录。
测试建议:
继续在43000整合!11~!15后验收实际发现、服务重启、身份加载及地址变化,回归NC-SI/PLDM。模板复制必须保持设备身份唯一。
自验证报告:
该PR关联的issue
fixes #16
希望检视人员了解:
仅补齐Control基础应答,完整可信入口/Ready、Control专用预算、Vendor业务登记及V2另行继续。没有注册的Vendor能力不虚报;既有Vendor通知能力的正式登记仍待完成。合入顺序!11→!12→!13→!14→!15→本PR(版本1.130.16~1.130.21),但本轮所有mctpd PR暂不合入;未使用/check-pr。启用合并后关闭任务,不提前关闭任务。