Open
支持通用EP端响应mctp命令功能 #20
byosoft_wupengfeiAdd on 3 hours ago
支持通用EP端响应mctp命令功能 #20
Open
byosoft_wupengfeiAdd on 3 hours ago
byosoft_wupengfei
3 hours ago

当前PR是否有AI参与:

[x] 否

[ ] 是
__1. AI Agent 平台:
__2. AI 模型:
__3. Prompt上下文 :

PR功能描述 / 为什么需要这个合入:

添加支持通用EP端响应mctp命令功能

修改方案:

新增BMC支持EP端响应mctp pldm命令

验证方案:

在仿真环境下用工具模拟验证

测试建议:

模拟网卡发送报文验证BMC回包情况

自验证报告:

待后续结果出来之后补充

该PR关联的issue

fixes #10
fixes #9
fixes #8
fixes #7
fixes #6
fixes #5
fixes #4

希望检视人员了解:

likedislike
Merge blocked
byosoft_wupengfeibyosoft_wupengfei
3 hours ago Created pull request, commit cf1429fd
byosoft_wupengfeibyosoft_wupengfei
3 hours ago Associated issue: [需求]: 支持经 IMU 桥接接收 MCTP VDM 报文,[需求]: 支持 BMC 作为 MCTP EP 接收并响应请求,[需求]: 支持 PLDM 协议框架接收对端硬件主动发送的报文,[需求]: 支持 requester/responder 双角色并发隔离,[需求]: 支持 responder 令牌管理响应,[需求]: 支持响应 PLDM Base 命令,[需求]: 支持响应 GetDateTime 命令
openUBMC-robotopenUBMC-robotCollaborators
3 hours ago Set u012924942,xiaoxinglxs,weixin_37569011,sam_h1973,net_swift_wangchen,dongyushan as approver
openUBMC-robotopenUBMC-robotCollaborators
3 hours ago Added label: sig/sig-component-drivers
openUBMC-robot
openUBMC-robotCollaborators
3 hours ago Comments:

欢迎来到 openUBMC 社区

Hey @Byo_wupengfei , 感谢你对社区的贡献.

机器人使用手册

有关指令的使用,可以点击 说明文档 或 论坛 查看详情。开发人员可以在每个PR或Issue下方评论特定指令来触发机器人任务。您可以自助配置此仓库的 PR 合并规则,更多详情,请参阅机器人自助配置使用手册。

联系指引

有疑问可以联系 SIG: sig-component-drivers ,
维护者是: @dongyushan, @net_swift_wangchen, @sam_h1973, @u012924942, @weixin_37569011, @xiaoxinglxs ,
优秀贡献者是: @hzy1010, @leessential .

likedislike
openUBMC-robotopenUBMC-robotCollaborators
3 hours ago Added label: openUBMC-cla/no
openUBMC-robot
openUBMC-robotCollaborators
3 hours ago Comments:

CLA 签署引导

@Byo_wupengfei , 感谢您的代码贡献。

以下 Commit(s) 没有签署 Contributor License Agreement (CLA)。

Commit 原因
91982e2b Add new feature 这条 Commit 的邮箱未签署 CLA !请检查下邮箱是否正确配置。

如果您需要签署 CLA,您可以点击此链接去签署 CLA 。

如果需要检查邮箱是否配置正确, 可以点击 FAQs 去查看使用手册 。

在完成 CLA 签署或者更新完邮箱后, 需再次评论 /check-cla 来检查 CLA 的签署状态。

likedislike
openubmc-ci-botCollaborators
3 hours ago Comments:

/ai review

likedislike
atomgit-bot
atomgit-botCollaborators
3 hours ago Comments:

代码审查

审查结论

  • [建议] src/lualib-src/libmctp/upper/pldm_protocol.cpp:81 fix: PLDM 响应发送路径仍固定使用请求头,缺少完成码
  • [建议] include/mctp_lib/init.lua:430 fix: register_mctp_cmd 用 reg_ret == false 判失败会漏掉 nil,err 返回
  • [建议] src/lualib-src/libmctp/transport/mctp_protocol.cpp:302 fix: 上层无协议受理的合法报文被计入 incomplete_msg 计数
  • [建议] gen/mctpd/client.lua:50 fix: 新增 MctpCmd 枚举接口的末位实参与同类调用不一致
  • [建议] src/lualib-src/libmctp/transport/mctp_protocol.h:111 fix: make_rx_key 将 uint32_t medium_type 截断为 uint8_t,可能把不同接口的重组上下文合并
  • [建议] gen/mctpd/json_types/MctpCmd.lua:109 fix: 生成代码中 Context 为 nil 时 new(nil) 会直接抛 setmetatable 错误
  • [建议] src/lualib/mctp_cmd_dispatcher.lua:243 fix: 在途名额回收未做异常保护,调用抛错会导致永久忙码
  • [建议] src/lualib/mctp_mdb_mgmt.lua:519 fix: 确认 log:warn 在 mc.logging 中存在,否则重复登记路径会抛错
  • [建议] src/lualib/mctp_engine.lua:489 fix: rx_drop_no_endpoint_count 只写不读,丢弃计数无任何消费方
  • [建议] src/lualib/mctp_endpoints/ep_pldm.lua:21 fix: is_smbus_medium 未防 medium_name 查表为 nil(新增 copy_ctxs 调用点使其进入收包主路径)
  • [建议] src/lualib/mctp_mdb_mgmt.lua:552 fix: 命令对象登记失败后无重试,属性缺失/顺序问题会永久漏注册
  • [建议] test/unit/test_mctp_mdb_mgmt.lua:215 fix: 不在共享 engine 上临时替换 cmd_registry 而不恢复
  • [建议] test/unit/test_libmctp/test_mctp_tx.lua:347 fix: 重置 setup 中的 smbus_eid,避免跨用例状态泄漏
  • [建议] test/unit/test_mctp_engine.lua:22 fix: 新引擎测试用例构造对象前补 c_tasks stub
  • [建议] test/unit/test_mctp_lib.lua:73 test: 注册参数校验用例无法区分各分支,弱断言
  • [建议] test/unit/test_ep_response_e2e.lua:291 test: 入站响应用例无法区分“未分发”与“更早层丢弃”
  • [建议] test/unit/test_ep_response_e2e.lua:30 test: 报文打印默认开启,污染单测结构化输出
  • [建议] test/unit/test_ep_response_e2e.lua:203 test: inject 对入站响应帧硬性要求 pop 返回 0,易误判失败
类型 数量
🔴 阻塞 0
🟡 建议 18

💬 仅评论

likedislike
Incorrect?
Oopenubmc-ci-botCollaborators
3 hours ago Added label: ci-running
openubmc-ci-botCollaborators
3 hours ago Comments:

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

likedislike
Oopenubmc-ci-botCollaborators
3 hours ago Added label: limit_1k_failed
Oopenubmc-ci-botCollaborators
3 hours ago Removed label: ci-running
openubmc-ci-botCollaborators
3 hours ago Comments:
任务名称结果
pre-commit检查✅ SUCCESS
PR描述检查✅ SUCCESS
防投毒检查✅ SUCCESS
开源合规检查✅ SUCCESS
编译构建❌ FAILED
UT测试❌ FAILED
IT测试✅ SUCCESS
代码覆盖率❌ FAILED
修改行数检查❌ FAILED
头文件检查❌ FAILED
AI Review✅ SUCCESS
ARM UT测试❌ FAILED
likedislike
Oopenubmc-ci-botCollaborators
3 hours ago Added label: ci-failed
atomgit-bot
atomgit-botCollaborators3 hours agoStart code review1
src/lualib-src/libmctp/transport/mctp_protocol.h
@@ -84,0 +111,4 @@
111+inline rx_key make_rx_key(const mctp_hdr &hdr, const uint32_t medium_type)
112+{
113+ return rx_key{static_cast<uint8_t>(medium_type), hdr.u.src, hdr.u.dest, hdr.u.msg_tag, static_cast<bool>(hdr.u.to)};
114+}
atomgit-bot
atomgit-bot3 hours agoComments:

🔵 Low Priority

建议:把 rx_key::medium_type 声明为 uint32_t,make_rx_key 直接赋值 medium_type(不截断),使重组键的接口标识与协议栈其他处的 medium_type 语义一致。

Suggestions
114
+ inline rx_key make_rx_key(const mctp_hdr &hdr, const uint32_t medium_type)
115
+ {
116
+ return rx_key{medium_type, hdr.u.src, hdr.u.dest, hdr.u.msg_tag, static_cast<bool>(hdr.u.to)};
114
117
  }
Apply suggestion
likedislike
Incorrect?
atomgit-bot
atomgit-botCollaborators3 hours agoStart code review1
gen/mctpd/client.lua
@@ -48,0 +53,4 @@
53+ 
54+function mctpd_client:ForeachMctpCmdObjects(cb)
55+ return foreach_non_virtual_interface_objects(self:get_bus(), 'bmc.kepler.Systems.Mctp.MctpCmd', cb, false)
56+end
atomgit-bot
atomgit-bot3 hours agoComments:

🔵 Low Priority

gen/mctpd/client.lua:51、55 新增的 GetMctpCmdObjects / ForeachMctpCmdObjects 传 false 作为 get/foreach_non_virtual_interface_objects 的第三个实参,而同一文件内所有其它非虚拟接口调用(第 35、39、43、47、59、63、67、71、116、121 行的 Pmu/PCIeTransport/BlockIO/Env/SmBios)统一传 true。

影响:该布尔实参控制接口对象枚举的路径匹配方式(是否按带参数路径/子路径展开)。若这里本应为 true,GetMctpCmdObjects() 将无法枚举到通过 mctp_lib.register_mctp_cmd 动态注册在 /bmc/kepler/Systems/<id>/Mctp/Cmds/<MT>/<ST>/<CC> 下的对象,业务侧「查询已注册命令」的能力静默失效(注册成功但查不到)。由于该文件为 mako 生成,也可能是生成器依据接口定义输出的正确结果,故仅作核对提示。

触发条件:业务组件调用 mctpd_client:GetMctpCmdObjects() 或 ForeachMctpCmdObjects() 枚举命令对象。

建议:核对 mako 模板/接口模型中对 bmc.kepler.Systems.Mctp.MctpCmd 的路径参数定义,确认末位实参应为 true 还是 false,与 mds/service.json 中 "path": "*" 及 create_mctp_cmd_path 生成的多级路径保持一致。

likedislike
Incorrect?
atomgit-bot
atomgit-botCollaborators3 hours agoStart code review1
src/lualib-src/libmctp/upper/pldm_protocol.cpp
@@ -93,0 +95,4 @@
95+ const pldm_rsp_payload_hdr &rsp_hdr = *reinterpret_cast<const pldm_rsp_payload_hdr *>(data.data());
96+ local_ctx.completion_code = rsp_hdr.pkg_hdr.completion_code;
97+ }
98+ return protocol::pop(local_ctx, slice(data, hdr_len), medium_type);
atomgit-bot
atomgit-bot3 hours agoComments:

🟡 Medium Priority

改动点(pldm_protocol.cpp:81-98)把 pop 的报文头长度按 rq 区分:请求 4 字节、响应 5 字节(响应多 1 字节 completion_code),并在响应分支解析 completion_code。但同文件的发送侧 pldm_protocol::push(第 101-122 行,未随本次改动调整)仍然固定 m_tx_buf.resize(sizeof(pldm_req_payload_hdr))(4 字节)并只填 rq/iid/hdr_ver/pldm_type/command_code,从不写 local_ctx.completion_code。

后果:本 PR 新增的 responder 能力需要向对端回送 PLDM 响应,经 push 发出的帧是「rq=0 但只有 4 字节头且无完成码」的畸形响应。对端(以及本端新逻辑第 83-85 行)在 is_request=false 时要求 data.size() >= 5,空负载响应会被直接丢弃;若有负载,对端会把首个负载字节误读成 completion_code,命令数据整体错位。触发条件:任意 PLDM 命令(如 Issue #9/#10 的 GetTID/GetDateTime)的响应经统一 TX 返回时必然发生。

即本次单方面修改了「头长度由 rq 决定」这一契约,兄弟代码 push 仍保持旧形式,两侧不一致。

建议:同步修改 pldm_protocol::push:按 local_ctx.rq 选择 pldm_req_payload_hdr/pldm_rsp_payload_hdr,响应分支把 local_ctx.completion_code(缺省 0)写入包头,保证收发两侧头长度契约一致。

likedislike
Incorrect?
atomgit-bot
atomgit-botCollaborators3 hours agoStart code review1
include/mctp_lib/init.lua
@@ -334,0 +443,4 @@
443+ cls[MCTP_CMD_INTERFACE].GetResponse = nil
444+ pcall(properties_obj.unregister, properties_obj)
445+ return nil, string.format('register mctp cmd object failed: %s', tostring(reg_err or reg_ret))
446+ end
atomgit-bot
atomgit-bot3 hours agoComments:

🟡 Medium Priority

改动点 include/mctp_lib/init.lua:430-434 与 440-446 用 local reg_ok, reg_ret, reg_err = pcall(properties_obj.register, properties_obj, bus) 后以 if not reg_ok or reg_ret == false 判定失败。

问题:pcall 只在被调函数抛错时返回 ok=false;若 register 以「返回 nil, err」的方式失败(同文件 create_transport_obj 第 148/155 行就完全没有检查返回值,说明该 API 的失败不一定抛错),此时 reg_ok=true, reg_ret=nil, reg_err="...",reg_ret == false 不成立,判定为成功。后果:对象实际未挂到总线,但函数继续执行并返回 handle,调用方(业务组件)误以为注册成功,之后对端请求不会到达该对象的 GetResponse,形成静默失效;且 reg_err 在该分支永远为 nil,tostring(reg_err or reg_ret) 也拿不到真实原因,与注释声称的「保持 nil,err 契约」不符。

触发条件:mdb 对象注册因路径冲突/服务未就绪等原因返回 nil,err 而非抛错。

建议:把成功判定改为显式校验返回值:if not reg_ok or reg_ret == nil or reg_ret == false then(两处同样修改),必要时同时确认 mdb register 的返回契约;失败回滚逻辑保持不变。

likedislike
Incorrect?
atomgit-bot
atomgit-botCollaborators3 hours agoStart code review1
src/lualib-src/libmctp/transport/mctp_protocol.cpp
@@ -268,0 +303,4 @@
303+ // 消息体与消息类型不匹配或长度不足:上层无法解析,丢弃并计数
304+ m_incomplete_msg_cnt++;
305+ return false;
306+ }
atomgit-bot
atomgit-bot3 hours agoComments:

🔵 Low Priority

改动点 mctp_protocol.cpp:302-306 在 protocol::pop 返回 false 时累加 m_incomplete_msg_cnt,注释解释为「消息体与消息类型不匹配或长度不足:上层无法解析」。

但 protocol::pop 的 false 语义更宽:protocol.h:183-186 的实现是遍历子协议 for (auto &child : m_childrens) { if (child->pop_data(...)) return true; },全部不受理即返回 false。因此发给本机、格式完全合法但上层未注册协议的消息(例如 NCSI 0x02、SPDM、VD-PCI 等未挂载子协议的消息类型),以及请求在 ENDPOINT 下但对应上层未注册的情况,都会被算成「不完整报文」。

后果:header 中对该计数的定义是「对端消息不完整或校验失败…丢弃且不产生响应」,属于可用于告警/诊断的指标(Issue #7/#5 要求可诊断)。上述误计入会让正常的多协议流量持续抬高 incomplete 计数,运维据此误判链路异常,也会掩盖真正的分片校验失败。

触发条件:对端发送本端未注册上层协议类型的合法 MCTP 报文。

建议:区分「消息类型无上层协议受理」与「报文不完整/长度不足」:可在计数前先判断是否为本端支持的消息类型(或让上层 pop 返回三态),仅对长度/格式失败累加 m_incomplete_msg_cnt,另设 unsupported 计数。

likedislike
Incorrect?
atomgit-bot
atomgit-botCollaborators3 hours agoStart code review1
gen/mctpd/json_types/MctpCmd.lua
@@ -0,0 +116,4 @@
116+ TGetResponseRsp:remove_error_props(errs, self)
117+ validate.CheckUnknowProperty(self, TGetResponseRsp.proto_property, errs, need_convert)
118+ return self
119+end
atomgit-bot
atomgit-bot3 hours agoComments:

🔵 Low Priority

变更行:MctpCmd.lua:112 与 MctpCmd.lua:165 在 validate 中无条件执行 MctpCmd.Context.new(self.Context):validate(...),而 TContext.new/ TContext_from_obj(第 28-34 行)内部是 setmetatable(obj, TContext)。失效模式:当业务侧构造 MctpCmd.GetResponseRsp.new(data)(未传第二个参数 Context)或反序列化得到不含 Context 的报文时,self.Context 为 nil,setmetatable(nil, ...) 抛 “bad argument #1 to 'setmetatable' (table expected, got nil)”,请求/响应处理直接中断而不是返回字段校验错误(对应 Data 字段用 validate.Optional/ OptionalArray 做了可空处理,Context 却没有)。注意本文件是 message.lua.mako 生成的“DO NOT EDIT”文件,修复应落在接口模型/模板侧(把 Context 声明为可选并生成 nil 保护),再重新生成。此为可选改进,非必须修复。

建议:在模型/模板层修正后重新生成:validate 中对 Context 做 nil 判断(if self.Context ~= nil then MctpCmd.Context.new(self.Context):validate(prefix, errs, need_convert) end),或把 Context 声明为可选字段;请求侧(第 165 行)同样处理。不要手工修改此生成文件。

likedislike
Incorrect?
atomgit-bot
atomgit-botCollaborators3 hours agoStart code review1
src/lualib/mctp_engine.lua
@@ -473,2 +486,4 @@
473486 end
474487 
488+ -- 来源未准入:控制端点不存在,丢弃并计数,不产生响应
489+ self.rx_drop_no_endpoint_count = self.rx_drop_no_endpoint_count + 1
atomgit-bot
atomgit-bot3 hours agoComments:

🔵 Low Priority

建议:将 rx_drop_no_endpoint_count 纳入已有诊断/属性输出,或直接移除该字段与两处自增。

likedislike
Incorrect?
atomgit-bot
atomgit-botCollaborators3 hours agoStart code review1
src/lualib/mctp_mdb_mgmt.lua
@@ -380,0 +516,4 @@
516+ end
517+ if existed then
518+ -- 订阅与全量对账可能重复看到同一个对象,按路径判重后直接跳过
519+ log:warn(
atomgit-bot
atomgit-bot3 hours agoComments:

🟡 Medium Priority

建议:核实 mc.logging 是否提供 warn 级别;若不存在则改用 log:notice/log:warning,避免在最高频的重复登记分支抛出 nil 方法调用。

likedislike
Incorrect?
atomgit-bot
atomgit-botCollaborators3 hours agoStart code review1
src/lualib/mctp_endpoints/ep_pldm.lua
@@ -19,1 +20,4 @@
1920 
21+local function is_smbus_medium(medium_type)
22+ return mctp_def.s_match(mctp_def.medium_name[medium_type], 'SMBus')
23+end
atomgit-bot
atomgit-bot3 hours agoComments:

🔵 Low Priority

建议:先取 medium_name 再判空,避免未知介质时 string.match 抛错,与仓库其它位置 medium_name[x] or 'UNKNOWN' 的写法对齐。

Suggestions
23
+ local function is_smbus_medium(medium_type)
24
+ local name = mctp_def.medium_name[medium_type]
25
+ return name ~= nil and mctp_def.s_match(name, 'SMBus') ~= nil
23
26
  end
Apply suggestion
likedislike
Incorrect?
atomgit-bot
atomgit-botCollaborators3 hours agoStart code review1
src/lualib/mctp_cmd_dispatcher.lua
@@ -0,0 +244,4 @@
244+ run_async(function()
245+ local ok, rsp_data, rsp_ctx, call_timed_out = self:call_business(entry, data, { IID = iid })
246+ -- 协程返回即释放名额;迟到分支也要释放,避免名额泄漏
247+ self.inflight = self.inflight - 1
atomgit-bot
atomgit-bot3 hours agoComments:

🟡 Medium Priority

变更行(src/lualib/mctp_cmd_dispatcher.lua:243-247)把 self.inflight 加一放在 run_async 之前、减一放在协程体内 call_business 返回之后,且减一没有放在 pcall/受保护块中。

调用链:dispatch_business → run_async(skynet.fork 协程) → self:call_business(...)。call_business 内部只用 pcall 包住了 mdb.get_object(...) 与 obj:GetResponse(...),而 make_call_context(context.new())与 context.with_context(dctx, ...) 本身在 pcall 之外。一旦 context.with_context/context.new 抛错(上下文栈损坏、dctx 非法、libmc 抛错),或 skynet.fork 协程因其它未捕获错误终止,第 247 行的 self.inflight = self.inflight - 1 不会执行,名额永久泄漏。

失败模式:MCTP_CMD_MAX_INFLIGHT = 10(mctp_def.lua:87),max_inflight 检查在 dispatch_business 入口(第 233 行)。泄漏累计到 10 次后,此后所有入站 PLDM 请求都会命中 self.inflight >= max_inflight 分支,永久返回 NODE_BUSY_CODE(0xC0),端点再也不会被业务应答,且没有计数器/定时器可以自愈(超时回调只置 done,不释放名额)。同样的泄漏也会在“业务协程永不返回且调用硬超时未生效”时发生——名额回收完全依赖调用返回。

建议:把第 245 行的 call_business 调用改为 pcall 包裹(pcall(self.call_business, self, entry, data, { IID = iid })),异常时按调用失败处理,确保第 247 行的名额释放一定执行。

likedislike
Incorrect?
atomgit-bot
atomgit-botCollaborators3 hours agoStart code review1
src/lualib/mctp_mdb_mgmt.lua
@@ -380,0 +562,4 @@
562+ MessageSubType = props.MessageSubType and props.MessageSubType:value(),
563+ CommandCode = props.CommandCode and props.CommandCode:value(),
564+ })
565+end
atomgit-bot
atomgit-bot3 hours agoComments:

🔵 Low Priority

建议:订阅 mdb 属性变更(PropertiesChanged)或在登记失败时做一次延迟重试,避免属性后置/registry 未就绪导致命令永久漏注册。

likedislike
Incorrect?
atomgit-bot
atomgit-botCollaborators3 hours agoStart code review1
test/unit/test_mctp_mdb_mgmt.lua
@@ -188,0 +212,4 @@
212+function TestMctpMdbMgmt:test_register_cmd_object()
213+ c_mctp_mdb_mgmt.system_id = 1
214+ c_mctp_mdb_mgmt.mctp_engine = self.mctp_engine
215+ self.mctp_engine.cmd_registry = c_mctp_cmd_registry.new(1)
atomgit-bot
atomgit-bot3 hours agoComments:

🔵 Low Priority

变更行:test/unit/test_mctp_mdb_mgmt.lua:215 与 :279(self.mctp_engine.cmd_registry = c_mctp_cmd_registry.new(1))。

受影响契约:self.mctp_engine 在 setupClass(第 44 行)创建一次,是本测试类内所有用例共享的实例;c_mctp_mdb_mgmt.mctp_engine 也被赋成同一实例。失败模式:两个新增用例把共享引擎内置的 cmd_registry 整体替换为空的新注册表,用例结束既不恢复也不在 setup/teardown 重建;luaunit 同类内用例执行顺序不保证,若 test_register_cmd_object 先于 test_rpc_method/test_add_objects/test_find_endpoint 执行,后续用例看到的是测试临时构造(甚至已被 unregister 清空)的命令反查表,一旦被测路径读取 cmd_registry 就会出现顺序相关的偶发失败或误通过。修复:在 setup/teardown 中替换与恢复,或为每个用例重建 engine。

建议:在 setup() 中保存并替换 cmd_registry、在 teardown() 中恢复,或每个用例重建 engine,避免污染共享实例的既有序状态。

likedislike
Incorrect?
atomgit-bot
atomgit-botCollaborators3 hours agoStart code review1
test/unit/test_mctp_engine.lua
@@ -0,0 +21,4 @@
21+ 
22+function TestMctpEngineRx:setupClass()
23+ self.engine = c_mctp_engine.new(nil, 1)
24+end
atomgit-bot
atomgit-bot3 hours agoComments:

🔵 Low Priority

变更行:test/unit/test_mctp_engine.lua:22-24(新增文件 setupClass 中直接 c_mctp_engine.new(nil, 1))。

受影响契约:单测运行在 skynet mock 环境,skynet.fork 同步执行、skynet.sleep 为空实现(见本次改动 test/unit/test_mctp_mdb_mgmt.lua:19-31 新增的 stub 与注释,以及 test/unit/test_libmctp/test_mctp_engine.lua 中"new discovery 之前先 hook c_tasks"的既有约定)。失败模式:新文件在构造引擎前没有任何 c_tasks 钩子,若引擎构造或其 rx/mctp_control_rx 触发发现流程启动 mc.orm.tasks 的 loop,在同步 fork + 空 sleep 下会变成同步死循环,使整个单测进程挂死而非给出失败信息。修复:按兄弟用例做法在 setupClass 先替换 c_tasks.get_instance 为假 task,teardownClass 恢复。

建议:setupClass 中先保存并替换 c_tasks.get_instance(loop/set_timeout_ms/stop 均为空实现的假 task)再创建引擎,teardownClass 恢复原函数。

likedislike
Incorrect?
atomgit-bot
atomgit-botCollaborators3 hours agoStart code review1
test/unit/test_mctp_lib.lua
@@ -26,0 +75,4 @@
75+ local handle, err = mctp_lib.register_mctp_cmd(nil, opts)
76+ lu.assertIsNil(handle)
77+ lu.assertEquals(type(err), 'string')
78+ end
atomgit-bot
atomgit-bot3 hours agoComments:

🔵 Low Priority

变更点:test_register_mctp_cmd_invalid_opts 遍历 8 个 mutator,每个只断言 `handle == nil` 与 `type(err) == 'string'`(第 73-78 行)。

行为/契约:这 8 个用例分别覆盖“非表 opts”“缺 message_type”“sub_type 越界”“command_code 为负/非整数/超 16 位”“on_request 非函数”“禁止注册 MCTP Control”等互相独立的校验分支。

失效模式:断言只要求“返回字符串错误”,因此只要实现里存在任意一个粗粒度校验(例如只校验 opts 是 table,或统一返回 "invalid opts"),8 个用例会全部通过,而 sub_type 越界、command_code 非整数/超界、on_request 类型、Control 类型禁止注册这些真正的边界校验即使缺失或写错(如少判 `0x100`、把 `<=` 写成 `<`)也无法被发现。这正是该 PR 新增注册接口的输入边界,属于高风险分支。

建议:把 mutators 改为 { mutate = fn, expect_err = '片段' } 形式,对每个分支断言可区分的错误信息,避免所有分支共用一个粗略校验也能全部通过。

likedislike
Incorrect?
atomgit-bot
atomgit-botCollaborators3 hours agoStart code review1
test/unit/test_libmctp/test_mctp_tx.lua
@@ -289,0 +345,4 @@
345+ local pad = detect_smbus_pad()
346+ lu.assertNotNil(pad, 'detect smbus mctp header length failed')
347+ self.mctp_proto.smbus_eid = 10
348+ local smbus_pad = string.rep('\x00', pad)
atomgit-bot
atomgit-bot3 hours agoComments:

🔵 Low Priority

变更行:test/unit/test_libmctp/test_mctp_tx.lua:347(新增用例 test_different_medium_independent 中 self.mctp_proto.smbus_eid = 10)。

受影响契约:self.mctp_proto 来自 setupClass(第 43 行),是整个测试类共享的单例;setup()(第 54-60 行,本 diff 也改动了这里)只重置 pcie_eid、m_wrong_seq_cnt、m_incomplete_msg_cnt、m_unassoc_msg_cnt,并不重置 smbus_eid。

失败模式:该用例把共享协议的 smbus_eid 从默认值改成 10 后不恢复。luaunit 遍历测试方法的顺序不保证(pairs 顺序),若该用例先于其它用例执行,后续"目的 EID 不是本端"类用例(如 test_wrong_dest_eid_silently_dropped、test_drop_unassoc_counted 中的目的过滤断言)在 SMBus 相关判定上会看到被动过的绑定参数,出现用例顺序相关的偶发失败/误通过。修复:在 setup() 中把 smbus_eid 一并复位(或在该用例末尾恢复原值)。

建议:在 TestMctpSeqValidation:setup() 中补一行 self.mctp_proto.smbus_eid = nil(或保存/恢复原值),使该用例对共享协议的修改不外溢;也可在该用例末行恢复原 smbus_eid。

likedislike
Incorrect?
atomgit-bot
atomgit-botCollaborators3 hours agoStart code review1
test/unit/test_ep_response_e2e.lua
@@ -0,0 +296,4 @@
296+ 
297+ lu.assertEquals(#self.business_calls, 0)
298+ lu.assertEquals(#self.tx, 0)
299+end
atomgit-bot
atomgit-bot3 hours agoComments:

🔵 Low Priority

变更点:新增用例 test_inbound_response_not_dispatched 只断言 `#business_calls == 0` 与 `#tx == 0`。

行为/契约:该用例要验证的是“入站响应(Rq=0)被识别为响应、因此不进入命令分发且不产生应答”。

失效模式:这两个断言只检查“最终没有副作用”。只要报文在 mctp/pldm 层、端点查找阶段或任何更早环节被丢弃(例如 endpoint 未创建、pldm 层因无在途事务拒绝解析),用例同样会通过,从而对“响应误入分发并回包”这一真正回归不敏感。这也是本 PR 的核心需求(responder 只处理 Rq=1)所依赖的关键判定,缺少能证明报文确实到达 PLDM/端点层的断言。

建议:在该用例中补充能证明报文到达 PLDM/端点层的断言(例如用 new_decoder 解析注入帧并断言 ctx.rq == 0),而不仅仅断言无业务回调、无回包。

likedislike
Incorrect?
atomgit-bot
atomgit-botCollaborators3 hours agoStart code review1
test/unit/test_ep_response_e2e.lua
@@ -0,0 +29,4 @@
29+ 
30+-- 自验报告取证明:默认把注入的入站报文和 BMC 回出的响应报文打到标准输出
31+-- (原始线上字节 + 解析后的字段)。设 MCTP_E2E_DUMP=0 可关闭。
32+local DUMP = os.getenv('MCTP_E2E_DUMP') ~= '0'
atomgit-bot
atomgit-bot3 hours agoComments:

🔵 Low Priority

变更点:`local DUMP = os.getenv('MCTP_E2E_DUMP') ~= '0'`(第 32 行)使报文打印默认开启,dump_frame 在 inject/teardown 中对每个用例打印线上字节与解析字段。

行为/契约:单元测试本应由 `test/unit/test.lua` 中的 `lu.LuaUnit.run()` 输出结构化结果。

失效模式:默认开启后,每次单测运行都会向 stdout 混入若干 `[MCTP-E2E] ...` 行。若 CI 以机器可读格式(如 luaunit 的 junit/TAP 输出)收集结果,这些额外行会破坏输出解析,导致结果解析失败或被误判;即使使用文本模式,也会显著放大日志量、淹没真正的失败信息。留证需求可以通过显式开启环境变量满足,不需要默认打开。

建议:把默认行为改为关闭:DUMP 仅在显式设置 MCTP_E2E_DUMP=1 时开启,避免单测输出被默认污染。

Suggestions
32
- local DUMP = os.getenv('MCTP_E2E_DUMP') ~= '0'
32
+ local DUMP = os.getenv('MCTP_E2E_DUMP') == '1'
Apply suggestion
likedislike
Incorrect?
atomgit-bot
atomgit-botCollaborators3 hours agoStart code review1
test/unit/test_ep_response_e2e.lua
@@ -0,0 +203,4 @@
203+function TestMctpEpResponseE2E:inject(frame)
204+ dump_frame(string.format('[%s] RX(peer->BMC)', self.case_name or '?'), frame, BMC_EID)
205+ lu.assertEquals(self.engine.hw_pcie_proto:pop(frame, nil, mctp_def.PHY_MEDIUM_PCI), 0)
206+end
atomgit-bot
atomgit-bot3 hours agoComments:

🔵 Low Priority

变更点:inject() 对每一次注入都调用 `lu.assertEquals(self.engine.hw_pcie_proto:pop(frame, nil, mctp_def.PHY_MEDIUM_PCI), 0)`,把“报文被接收链接受”等同于“返回 0”。

行为/契约:当被注入的是对端主动发来的响应帧(rq=false,见第 295 行 `build_frame({ rq = false })`)时,协议栈没有任何在途事务可匹配,实现对这类无主响应返回非 0(例如“无匹配事务”错误码)是完全合理的语义。

失效模式:一旦实现以非 0 返回表示“该帧被丢弃/无匹配事务”,test_inbound_response_not_dispatched 会在 inject 的这行断言上失败,失败原因与被测行为无关(是测试前置过强),造成误报失败或迫使实现为了通过测试而返回 0,弱化“丢弃无主响应”的表达。

建议:让 inject 接受一个可选参数表示期望的 pop 返回值(请求方向断言 0;响应方向允许 0 或“无匹配事务”错误码),避免测试前置与被测语义耦合。

likedislike
Incorrect?
liuxingsenCollaborators1 hour agoStart code review1
src/lualib/mctp_cmd_dispatcher.lua
@@ -0,0 +258,4 @@
258+ 
259+ -- 回调返回后复查映射:命令可能已注销,或已被新对象以相同三元组重新注册
260+ local current = self.registry:lookup(req.message_type, req.sub_type, req.command_code)
261+ if not current or current.path ~= entry.path then
liuxingsen1 hour agoComments:

[P2] 回调完成时应验证注册条目的身份/代际,而不只是path。current.path ~= entry.path无法识别同路径注销后重新注册;命令path由system/MT/ST/CC确定,替换同一命令仍用同一路径。触发顺序:旧entry的GetResponse在途→unregister(path)→同三元组同path注册新entry→旧GetResponse返回。检查通过并把旧业务状态成功发给对端,cmd_inflight_unregistered仍为0。已直接加载本HEAD原dispatcher与registry,用协程暂停旧请求、注销/重注册后恢复,得到cc=0、body=OLD_OBJECT_RESULT、计数0。建议比较current ~= entry或固化每次注册generation,并增加同path替换测试;现有测试只覆盖注销后不重建。该根因在前一PR !19的199107926意见中也已指出,当前新PR仍保留。

likedislike