Skip to content

fby35: enhance kcs error handling #2650

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.

Sign up for GitHub

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

Jump to bottom
Open
Jerry-wiwynn wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fby35: enhance kcs error handling #2650

Jerry-wiwynn wants to merge 1 commit into from
+91 −58

Conversation

@Jerry-wiwynn
Copy link
Contributor

@Jerry-wiwynn Jerry-wiwynn commented Jan 12, 2026

Description

  1. Reduce PLDM_OEM_IPMI_BRIDGE timeout duration
  2. Enhance kcs error handling

Motivation

  1. The default timeout for ipmitool is 15 seconds 1, so need to shorten PLDM_OEM_IPMI_BRIDGE timeout_ms.

  2. When a KCS error occurs, a completion code should be returned.

Test Plan:

Build and test OK on EMR

CMD_STORAGE_ADD_SEL error handing test ok:
Host (rsp 0xc8 == CC_LENGTH_EXCEEDED):
[root@250424-611-fbk6 ~]# time ipmitool raw 0x0a 0x44 0x01 0x01 0x01 0x01 0x01 0x01 0x01 0x01 0x01 0x01 0x01 0x01 0x01 0x01 0x01 0x01 0x04 Unable to send RAW command (channel=0x0 netfn=0xa lun=0x0 cmd=0x44 rsp=0xc8): Request data field length limit exceeded

BIC:
uart:~$ [00:18:46.401,000] kcs: ADD SEL event data length error, rc = 19

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 12, 2026
@meta-codesync
Copy link

meta-codesync bot commented Jan 12, 2026

@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this in D90479021. (Because this pull request was imported automatically, there will not be any future comments.)

@pat-lin-wiwynn pat-lin-wiwynn force-pushed the fby35-kcs-enhance-error-handling branch from bdd2105 to e12ecf2 Compare January 12, 2026 10:20
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jan 12, 2026

@Jerry-wiwynn has updated the pull request. You must reimport the pull request before landing.

@pat-lin-wiwynn
Copy link
Contributor

pat-lin-wiwynn commented Jan 12, 2026

Cppcheck's memory leak warning for kcs_work is a false positive.
The memory is intentionally kept alive for the asynchronous handler and
is properly freed via SAFE_FREE within the callback itself.

jamesatha
jamesatha requested changes Jan 15, 2026
View reviewed changes
Copy link
Contributor

@jamesatha jamesatha left a comment

Choose a reason for hiding this comment

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

Since you were working on the same method, please address the lint issue:
common/service/host/kcs.c:418:2:Memory leak: kcs_work

@pat-lin-wiwynn pat-lin-wiwynn force-pushed the fby35-kcs-enhance-error-handling branch from e12ecf2 to a3e1d16 Compare January 16, 2026 00:53
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jan 16, 2026

@Jerry-wiwynn has updated the pull request. You must reimport the pull request before landing.

@pat-lin-wiwynn
fby35: enhance kcs error handling
c6c34f7 
Summary:
Description
1. Reduce PLDM_OEM_IPMI_BRIDGE timeout duration
2. Enhance kcs error handling

Motivation
1. The default timeout for ipmitool is 15 seconds [1], so need to
   shorten PLDM_OEM_IPMI_BRIDGE timeout_ms.

2. When a KCS error occurs, a completion code should be returned.

[1]: https://github.com/ipmitool/ipmitool/blob/master/src/plugins/open/open.c#L87

Test Plan:
Build and test OK on EMR

CMD_STORAGE_ADD_SEL error handing test ok:
Host (rsp 0xc8 == CC_LENGTH_EXCEEDED):
[root@250424-611-fbk6 ~]# time ipmitool raw 0x0a 0x44 0x01 0x01 0x01 0x01 0x01 0x01 0x01 0x01 0x01 0x01 0x01 0x01 0x01 0x01 0x01 0x01 0x04
Unable to send RAW command (channel=0x0 netfn=0xa lun=0x0 cmd=0x44 rsp=0xc8): Request data field length limit exceeded

BIC:
uart:~$ [00:18:46.401,000] <err> kcs: ADD SEL event data length error, rc = 19
@pat-lin-wiwynn pat-lin-wiwynn force-pushed the fby35-kcs-enhance-error-handling branch from a3e1d16 to c6c34f7 Compare January 16, 2026 02:46
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jan 16, 2026

@Jerry-wiwynn has updated the pull request. You must reimport the pull request before landing.

@pat-lin-wiwynn
Copy link
Contributor

pat-lin-wiwynn commented Jan 16, 2026

Since you were working on the same method, please address the lint issue: common/service/host/kcs.c:418:2:Memory leak: kcs_work

done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jamesatha jamesatha jamesatha requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Jerry-wiwynn @facebook-github-bot @pat-lin-wiwynn @jamesatha