Skip to content

Conversation

@Victor-Jhong
Copy link
Contributor

Historically, most platforms used a 5000 ms timeout for PLDM messaging and FW update
operations. PLDM_MSG_TIMEOUT_MS was modified in common code 14 months ago (PR #2034)
to 30000 ms for a specific platform, and several platforms, including Minerva-AG,
overrode the value locally to retain the original 5000 ms behavior.

A similar situation occurred when PLDM_FW_UPDATE_TIMEOUT_MS was later introduced with a
hard-coded 30000 ms value (PR #2519), which again unintentionally changed behavior on
platforms that continue to rely on the 5000 ms timeout.

A similar cross-platform issue also occurred when MCTP_DEFAULT_MSG_MAX_SIZE was updated
from 64 to 244 in PR #2588. That change inadvertently affected platforms that were not
prepared for the larger message size. This highlights the need for common definitions
to avoid enforcing behavior changes across platforms unless explicitly intended.

This patch adds an #ifndef guard to the PLDM_FW_UPDATE_TIMEOUT_MS definition so that
platforms may override the timeout as needed. Platforms using the 30000 ms value will
retain their behavior, while Minerva-AG can restore its 5000 ms timeout locally.

To avoid impacting other platforms' expected defaults, would it be better for future
common changes to follow a similar pattern, keeping the baseline value unchanged while
allowing platforms with different requirements to override as needed via #ifndef?

@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 Dec 26, 2025
@meta-codesync
Copy link

meta-codesync bot commented Dec 26, 2025

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

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

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

Development

Successfully merging this pull request may close these issues.

1 participant