| Age | Commit message (Collapse) | Author | Files | Lines |
|
Three error paths name a property or condition that has nothing to do
with the failure, so a client cannot tell what to correct.
- account_service: a RoleId that maps to no privilege reports
PropertyValueNotInList for Locked with the value true. RoleId is the
property that was rejected, and its submitted value is what the client
needs to see.
- systems: a PATCH of TrustedModuleRequiredToBoot on a system with no
TPM.Policy object reports PropertyValueNotInList with the value
"ComputerSystem", which is neither a submitted value nor an
enumeration member. Nothing is wrong with the value; the property
cannot be written on this system, which is PropertyNotWritable.
- openbmc_managers: Direction is checked against the enumeration
{Ceiling, Floor}, so a rejected value is PropertyValueNotInList, not
PropertyValueTypeError. The arguments were also reversed, since
PropertyValueTypeError takes the value first.
Tested:
Verified on a QEMU BMC, machine cypress, running a downstream OpenBMC
build carrying this change.
PATCH /redfish/v1/AccountService/Accounts/root with {"RoleId":"Bogus"}
returns HTTP 400 and Base.1.19.PropertyValueNotInList with MessageArgs
["\"Bogus\"", "RoleId"], where the same request previously reported
Locked with the value true.
Not exercised on that build: the TrustedModuleRequiredToBoot site,
because that platform has a TPM.Policy object so the empty-subtree
branch never runs; and the OEM fan Direction site, because the machine
exposes no PID or stepwise fan controllers to PATCH.
Change-Id: I408456a7040f095b43a952532f761ec197c24146
Signed-off-by: Bill Chan <bill_chan@jabil.com>
|
|
Long lambdas have been documented as an anti-pattern for some time.[1]
Despite this being generally understood, bmcweb has a long ways to go
cleaning these up, and routinely code is submitted in violation of this
anti-pattern.
Invent an ast-grep rule that can identify when new examples of this
anti-pattern are added, and ignore the existing 200+ examples that are
in the codebase already using ast-grep ignore. These flags will give us
something to search for as we clean this up, and will help to prevent
new instances from being added unintentionally.
[1] https://github.com/openbmc/docs/blob/master/anti-patterns.md#very-long-lambda-callbacks
Tested: Comment only change. ast-grep passes. Manually removing an
ast-grep ignore flag shows as a failure in ast-grep scan
Change-Id: I77d634a393884969f184d2c39c02cc08288d5a29
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
`redfish-core/schema/oem/openbmc/` oem schema defines 'Chassis' property
for fan zones but the implementation forms invalid chassis links.
Affected options: redfish-oem-manager-fan-data=enabled (default)
Using following configuration, plus a few fans and pid controller
(a typical single-host 2U server with 3 fans, Tyan S8030 board)
```
{
"FailSafePercent": 100,
"MinThermalOutput": 10,
"Name": "Zone0",
"Type": "Pid.Zone"
},
```
It is straightforward to get a response like below
```
...
"FanZones": {
"@odata.id": "/redfish/v1/Managers/bmc#/Oem/OpenBmc/Fan/FanZones",
"@odata.type": "#OpenBMCManager.v1_0_0.Manager.FanZones",
"Zone0": {
"@odata.id": "/redfish/v1/Managers/bmc#/Oem/OpenBmc/Fan/FanZones/Zone0",
"@odata.type": "#OpenBMCManager.v1_0_0.Manager.FanZone",
"Chassis": {
"@odata.id": "/redfish/v1/Chassis/Zone0"
},
"FailSafePercent": 100.0,
"MinThermalOutput": 10.0
}
},
...
```
when querying
```
curl --insecure --user root:root https://${bmc}/redfish/v1/Managers/bmc#/Oem/OpenBmc/Fan
```
For reference, the chassis collection
```
{
"@odata.id": "/redfish/v1/Chassis",
"@odata.type": "#ChassisCollection.ChassisCollection",
"Members": [
{
"@odata.id": "/redfish/v1/Chassis/MBX_1_57_Chassis"
},
{
"@odata.id": "/redfish/v1/Chassis/Tyan_S8030_Baseboard"
}
],
"Members@odata.count": 2,
"Name": "Chassis Collection"
}
```
Since that configuration is representative of various boards and the bug
has been seen by others before [1] (in terms of a fan zone and chassis
sharing the same name, suggesting ill-formed link), fix the
implementation to use the result of GetManagedObjects call and find
valid chassis path there.
This is to allow redfish validator to pass with default meson options
and a common system configuration. Since it's a config dependent failure
it would be great for others to test and share their result.
Inspection of the code causing validation failure:
```
auto pids = std::make_shared<GetPIDValues>(asyncResp);
pids->run();
then run(); returns and `~GetPIDValues()` is called
which calls processingComplete
which calls asyncPopulatePid
```
Inside `asyncPopulatePid` it does `dbus::utility::getManagedObjects`
and iterates over the results
```
112 for (const auto& pathPair : managedObj)
113 {
114 for (const auto& intfPair : pathPair.second)
```
then checks for an interface
```
180 if (intfPair.first == pidZoneConfigurationIface)
181 {
182 sdbusplus::message::object_path pidPath(
183 pathPair.first.str);
184 std::string chassis = pidPath.filename();
185 if (chassis.empty())
186 {
187 chassis = "#IllegalValue";
188 }
```
and simply uses the object path from PID Zone config interface to
extract the leaf and insert that as the chassis link.
It can only work in case the Board/Chassis interface is on the same
object path which is unlikely.
Tested: on Tyan S8030.
Result after the change, the optional property now contains the correct
chassis link.
```
...
"FanZones": {
"@odata.id": "/redfish/v1/Managers/bmc#/Oem/OpenBmc/Fan/FanZones",
"@odata.type": "#OpenBMCManager.v1_0_0.Manager.FanZones",
"Zone0": {
"@odata.id": "/redfish/v1/Managers/bmc#/Oem/OpenBmc/Fan/FanZones/Zone0",
"@odata.type": "#OpenBMCManager.v1_0_0.Manager.FanZone",
"Chassis": {
"@odata.id": "/redfish/v1/Chassis/MBX_1_57_Chassis"
},
"FailSafePercent": 100.0,
"MinThermalOutput": 10.0
}
},
...
```
```
/tmp/rsv-venv/bin/rf_service_validator \
--auth Session -i https://${bmc}:443 \
-u ${username} -p ${password} --payload 'Tree' /redfish/v1/Managers/bmc
...
Elapsed time: 0:00:32
Listing any warnings and errors:
Results Summary:
Pass: 766, Fail: 0, Warning: 0
Validation has succeeded.
```
RF validator Tree validation errors are reduced compared to previous.
References:
[1] https://discordapp.com/channels/775381525260664832/1449737223493910559/1450273124804333598
Change-Id: I2a2db456f42c5dafa451f69362b1d9c8a094e86e
Signed-off-by: Alexander Hansen <alexander.hansen@9elements.com>
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
Extend the OpenBMCManager OEM fan configuration to cover the two
additional PID parameters already supported by phosphor-pid-control:
* DCoefficient (derivative term of the PID loop)
* CheckHysteresisWithSetpoint (boolean indicating whether input
hysteresis is applied around the setpoint)
These fields are now exposed through the Oem/OpenBmc/Fan/PidControllers
Redfish interface and correctly mapped to the underlying D-Bus
PidConfiguration objects.
To keep the OEM schema backwards compatible, introduce a new
version OpenBMCManager.v1_1_0 that adds the two properties to the
PidController definition, and update bmcweb to reference the new
schema version.
Test(qemu evb-ast2600):
Load Fantable via entity-manager
/var/configuration/system.json content:
https://github.com/YouPengWu/ToReviewer/blob/main/85785
/Test_case/Bmcweb(my_commit)/system.txt
Verify OEM properties in Manager resource
GET /redfish/v1/Managers/bmc:
https://github.com/YouPengWu/ToReviewer/blob/main/85785
/Test_case/Bmcweb(my_commit)/Managers-bmc-oem.png
Verify OpenBMCManager OEM schema exposure
GET /redfish/v1/JsonSchemas/OpenBMCManager:
https://github.com/YouPengWu/ToReviewer/blob/main/85785
/Test_case/Bmcweb(my_commit)/redfish-oem-schema.png
Run Redfish Service Validator (RSV)
Summary:
https://github.com/YouPengWu/ToReviewer/blob/main/85785
/Test_case/Bmcweb(my_commit)/redfish-validator-summary.png
Details:
https://github.com/YouPengWu/ToReviewer/blob/main/85785
/Test_case/Bmcweb(my_commit)/redfish-validator-details.png
Full log:
https://github.com/YouPengWu/ToReviewer/blob/main/85785
/Test_case/Bmcweb(my_commit)/ConformanceLog_12_14_2025_180449.txt
Baseline (community/original) RSV log
for comparison (no new failures introduced):
https://github.com/YouPengWu/ToReviewer/blob/main/85785
/Test_case/Bmcweb(57d41)/ConformanceLog_12_14_2025_174610.txt
Change-Id: Ide1a118f9fd27eb94e911997d99e5934fe3e1095
Signed-off-by: You Peng Wu <twpeng50606@gmail.com>
|
|
This lambda needs to go away. There's no way it should've been accepted
in the first place, but it was written in a different time.
Tested: Functional in next commit. Unit tests in patch.
Change-Id: I81360460c23329169f441b6bea02d28a8b410eca
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
The sdbusplus headers provide shortened aliases for many types.
Switch to using them to provide better code clarity and shorter
lines. Possible replacements are for:
* bus_t
* exception_t
* manager_t
* match_t
* message_t
* object_t
* slot_t
* object_path
Change-Id: Iace20f9ad26e8d9dc234979e7a4087d599da2641
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
|
|
This tidy check can transform code to use std::ranges. Enable the
check, apply the fixes it proposes.
Tested: Redfish service validator passes in qemu
Change-Id: I3f21b27d3d30277f71b9c8a2c584a22bc16865e9
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
This utility function is being removed for several reasons. First, it
does not verify the full string on URIs and paths, so things like
/foo/bar/baz/valid_id would still pass this check.
Second, it is used for both URIs and dbus paths, both of which we have
better utility functions these days respectively, boost::url for urls
and sdbusplus::message::object_path for dbus paths. Neither of the two
is escaped properly when this function is used.
Therefore, remove it and replace it with the appropriate alternatives.
The existing URI functions were found to not accept fragments (given
they are rarely used in PATCH). Add support for fragments to cover the
getNthStringFromPath use cases.
Tested: Redfish service validator passes.
Change-Id: Ibc6755ad69397123d7fef0e0b764042bbb48888b
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
AccumulateSetPoint is a boolean type, but the current parsing logic
for the xyz.openbmc_project.Configuration.Pid.Zone interface assumes
all properties are of type double. This mismatch causes an internal
error when processing AccumulateSetPoint
Fixed
bmcwebd: [openbmc_managers.hpp:284] Field Illegal AccumulateSetPoint
bmcwebd: [error_messages.cpp:1250] Internal Error /usr/src/debug/
bmcweb/1.0+git/redfish-core/lib/openbmc/openbmc_managers.hpp(285:56)
`redfish::asyncPopulatePid(const std::string&, const std::string&,
const std::string&, const std::vector<std::__cxx11::basic_string<char>,
std::allocator<std::__cxx11::basic_string<char> > >&,
const std::shared_ptr<bmcweb::AsyncResp>&)::<lambda
(const boost::system::error_code&, const dbus::utility::
ManagedObjectType&)>`:
Change-Id: Idd11afa067fd20e047a28f72307231e2b76bc593
Signed-off-by: Peter Yin <peter.yin@quantatw.com>
|
|
Extension of OEM route infra to support registration of handlers for OEM
patch requests. When patch request is made on a redfish resource, first
the main route handler will be called and if request patch payload
contains any OEM fragments then, registered OEM patch handler will be
called.
Tested
1. UT passes with new test cases added for OEM patch handling
2. Patch on FAN OEM property works as expected
```
Step 1: Creating new fan controller...
Create PATCH data:
{
"Oem": {
"OpenBmc": {
"Fan": {
"FanControllers": {
"Fan_TEST_391715": {
"FFGainCoefficient": 2.0,
"Zones": [
{
"@odata.id": "/redfish/v1/Managers/bmc#/Oem/OpenBmc/Fan/FanZones/Zone_1"
}
]
}
}
}
}
}
}
HTTP Response Code (PATCH /redfish/v1/Managers/bmc): 200
HTTP Response Code (GET /redfish/v1/Managers/bmc): 200
✓ Fan controller created successfully
Step 2: Updating the fan controller...
Update PATCH data:
{
"Oem": {
"OpenBmc": {
"Fan": {
"FanControllers": {
"Fan_TEST_391715": {
"FFGainCoefficient": 3.0
}
}
}
}
}
}
HTTP Response Code (PATCH /redfish/v1/Managers/bmc): 200
HTTP Response Code (GET /redfish/v1/Managers/bmc): 200
Final Configuration:
{
"@odata.id": "/redfish/v1/Managers/bmc#/Oem/OpenBmc/Fan/FanControllers/Fan_TEST_391715",
"@odata.type": "#OpenBMCManager.v1_0_0.Manager.FanController",
"FFGainCoefficient": 3.0,
"Zones": [
{
"@odata.id": "/redfish/v1/Managers/bmc#/Oem/OpenBmc/Fan/FanZones/Zone_1"
}
]
}
✓ Fan controller updated successfully
```
Test Summary
```
[+] Tests DateTime update after NTP disable. Payload: {DateTime: <date-string>}. Expects: 204 success, validates date matches update.: PASSED
[-] Tests invalid property in request. Payload: {InvalidProperty: 'value', DateTime: <date-string>}. Expects: 400 PropertyUnknown error, validates DateTime unchanged.: PASSED
[-] Tests fan controller with invalid property. Payload: Oem/OpenBmc/Fan/FanControllers with InvalidProperty. Expects: 400 PropertyUnknown error, fan not created.: PASSED
[-] Tests empty PATCH request. Payload: {}. Expects: 400 MalformedJSON error.: PASSED
[-] Tests malformed fan controller JSON. Payload: Fan property as string instead of object. Expects: 400 PropertyValueTypeError error.: PASSED
[-] Tests DateTime with wrong type. Payload: {DateTime: 12345}. Expects: 400 PropertyValueTypeError error, DateTime unchanged.: PASSED
[-] Tests PATCH to invalid manager path. Payload: Valid DateTime and fan update to /invalid_bmc. Expects: 404 ResourceNotFound error.: PASSED
[+] Tests fan controller creation. Payload: Oem/OpenBmc/Fan/FanControllers with FFGainCoefficient and Zones. Expects: 200 success with success message.: PASSED
[-] Tests fan controller without required Zones. Payload: Oem/OpenBmc/Fan/FanControllers with only FFGainCoefficient. Expects: 500 InternalError, fan not created.: PASSED
[+] Tests combined DateTime and fan update. Payload: DateTime and Oem/OpenBmc/Fan/FanControllers. Expects: 200 success with success message.: PASSED
[-] Tests PATCH with wrong Content-Type header. Payload: Valid DateTime update with text/plain content-type. Expects: 400 UnrecognizedRequestBody error.: PASSED
[+] Tests fan controller creation and update. Payload: Create with FFGainCoefficient=2.0, then update to 3.0. Expects: 200 success for both operations, verifies all properties.: PASSED
```
Change-Id: Ib2498b6a4db0343d5d4a405a5a8e4d78f615bed8
Signed-off-by: Rohit PAI <rohitpai77@gmail.com>
|
|
OEM fragments handlers need not mention the OEM key in the JSON this
will be added by the OEM route handler based on the fragment
registration information
Tests
1. Service validator passed on daytonax image
2. OpenBMC Manager schema payload contain the FAN OEM properties in the
correct order.
```
curl -k -u root:0penBmc https://localhost:2443/redfish/v1/Managers/bmc
{
"@odata.id": "/redfish/v1/Managers/bmc",
"@odata.type": "#Manager.v1_14_0.Manager",
….
"Id": "bmc",
"LastResetTime": "2025-04-14T08:48:34+00:00",
….
"NetworkProtocol": {
"@odata.id": "/redfish/v1/Managers/bmc/NetworkProtocol"
},
"Oem": {
"OpenBmc": {
"@odata.id": "/redfish/v1/Managers/bmc#/Oem/OpenBmc",
"@odata.type": "#OpenBMCManager.v1_0_0.Manager",
"Certificates": {
"@odata.id": "/redfish/v1/Managers/bmc/Truststore/Certificates"
},
"Fan": {
"@odata.id": "/redfish/v1/Managers/bmc#/Oem/OpenBmc/Fan",
"@odata.type": "#OpenBMCManager.v1_0_0.Manager.Fan",
"FanControllers": {
"@odata.id": "/redfish/v1/Managers/bmc#/Oem/OpenBmc/Fan/FanControllers",
"@odata.type": "#OpenBMCManager.v1_0_0.Manager.FanControllers",
"Fan_SYS0": {
"@odata.id": "/redfish/v1/Managers/bmc#/Oem/OpenBmc/Fan/FanControllers/Fan_SYS0",
"@odata.type": "#OpenBMCManager.v1_0_0.Manager.FanController",
"FFGainCoefficient": 1.0,
"FFOffCoefficient": 0.0,
"ICoefficient": 0.0,
"ILimitMax": 0.0,
"ILimitMin": 0.0,
"Inputs": [
"Fan_SYS0_0",
"Fan_SYS0_1"
],
"NegativeHysteresis": 0.0,
"OutLimitMax": 100.0,
"OutLimitMin": 10.0,
"Outputs": [
"Pwm 1"
],
"PCoefficient": 0.0,
"PositiveHysteresis": 0.0,
"SlewNeg": 0.0,
"SlewPos": 0.0,
"Zones": [
{
"@odata.id": "/redfish/v1/Managers/bmc#/Oem/OpenBmc/Fan/FanZones/Zone_1"
}
]
},
……
}
}
}
},
"PowerState": "On",
"ServiceEntryPointUUID": "3c9e6cf1-3ed7-4105-87b5-31beb31e8f4d",
"Status": {
"Health": "OK",
"State": "Enabled"
},
"UUID": "a5d927e1-e8cb-462c-8f73-7d01ab2c2657"
}
```
Change-Id: Ib30dbaed8f2bf2c361f857cb4a0c6b0ab15ee65a
Signed-off-by: Rohit PAI <ropai@nvidia.com>
|
|
Initial copy was done to avoid request object going out of scope before
OEM handler are invoked.
The MR avoids the whole copy of the request object and create a sub
route object which contains elements required for OEM route handling.
Tested
- Service Validator Passes
- OpenBMC OEM properties and rendered well.
Change-Id: I3ef80a130afe6ab764a13704a8b672f5b0635126
Signed-off-by: Rohit PAI <ropai@nvidia.com>
|
|
Adding async_method_call in dbus utility gives us a place where we can
intercept method call requests from dbus to potentially add
logging/caching.
An example of logging is in the later commit:
https://gerrit.openbmc.org/c/openbmc/bmcweb/+/78265/
We already do this for setProperty, this moves the method calls to
follow a similar pattern.
Tested: Redfish service validator passes.
Change-Id: I6d2c96e2b6b6a023ed2138106a55faebca161592
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|
|
Goal of the MR is to provide infrastructure support in bmcweb to manage
the OEM fragment handling separately. OEM schema are vendor defined and
per DMTF resource we could have multiple vendor defined OEM schema to be
enabled.
The feature allows registration of route handler per schema per OEM
namespace.
Example
```
REDFISH_SUB_ROUTE<"/redfish/v1/Managers/<str>/#/Oem/OpenBmc">(service,
HttpVerb::Get)(oemOpenBmcCallback);
REDFISH_SUB_ROUTE<"/redfish/v1/Managers/<str>/#/Oem/Nvidia">(service,
HttpVerb::Get)(oemNidiaCallback);
```
We can have separate vendor defined route handlers per resource. Each of
these route handlers can populate their own vendor specific OEM data.
The OEM code can be better organized and enabled/disabled as per the
platform needs. The current MR has the code changes related to handling
GET requests alone. The feature only supports requests
where the response payload is JSON.
Tests
- All UT cases passes
- New UT added for RF OEM router passes
- Service Validator passes on qemu
- GET Response on Manager/bmc resource contains the OEM fragment
```
curl -c cjar -b cjar -k -X GET https://127.0.0.1:2443/redfish/v1/Managers/bmc
{
"@odata.id": "/redfish/v1/Managers/bmc",
"@odata.type": "#Manager.v1_14_0.Manager",
"Oem": {
"OpenBmc": {
"@odata.id": "/redfish/v1/Managers/bmc#/Oem/OpenBmc",
"@odata.type": "#OpenBMCManager.v1_0_0.Manager",
"Certificates": {
"@odata.id": "/redfish/v1/Managers/bmc/Truststore/Certificates"
}
}
},
"UUID": "40575e98-90d7-4c10-9eb5-8d8a7156c9b9"
}
```
Change-Id: Ic82aa5fe760eda31e2792fbdfb6884ac3ea613dc
Signed-off-by: Rohit PAI <rohitpai77@gmail.com>
|
|
With the support of OEM route infrastructure each OEM implementation
can be separated into an OEM route handler. The MR migrates OEM resource
of manager resource into new files and route handlers
Tested
- All unit tests are passing
- GET request on /redfish/v1/Managers/<bmcid> has OpenBMC OEM properties
Change-Id: I935524dcdad6a6cc38a5532b6e7e7ffa1cb0369f
Signed-off-by: rohitpai <rohitpai77@gmail.com>
Signed-off-by: Ed Tanous <etanous@nvidia.com>
|