| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Since the GetSubTreePaths method has been implemented in dbus_utility
and this commit is to integrate all the places where the
GetSubTreePaths method is called, and use the method in dbus_utility
uniformly.
Requires https://gerrit.openbmc.org/c/openbmc/sdbusplus/+/60020 to
build.
Tested: Redfish Validator Passed
Signed-off-by: George Liu <liuxiwei@inspur.com>
Change-Id: Ie4140d4484a7e4f4b943013f4371ffd2d44a22e9
|
|
cppcheck correctly notes that a lot of our variables can be declared at
more specific scopes, and in every case, it seems to be correct.
Tested: Redfish service validator passes. Unit test coverage on others.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ia4414410d0e8f74a3bd40fdc0e0232450d1a6416
|
|
The new boost URL now interops properly with std::string_view, which is
great, and cleans up a bunch of mediocre code to convert one to another.
It has also been pulled into boost-proper, so we no longer need a
boost-url dependency that's separate.
Unfortunately, boost url makes these improvements by changing
boost::string_view for boost::urls::const_string, which causes us to
have some compile errors on the missing type.
The bulk of these changes fall into a couple categories, and have to be
executed in one commit.
string() is replaced with buffer() on the url and url_view types
boost::string_view is replaced by std::string_view for many times, in
many cases removing a temporary that we had in the code previously.
Tested: Code compiles with boost 1.81.0 beta.
Redfish service validator passes.
Pretty good unit test coverage for URL-specific use cases.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I8d3dc89b53d1cc390887fe53605d4867f75f76fd
|
|
OpenBMC tends to have a significant problem in doing the appropriate
lookups from the schema files, and many bugs have been injected by users
picking a bad enum, or mistyping the casing of an enum value.
At the same time, nlohmann::json has recently added first class support
for enums, https://json.nlohmann.me/features/enum_conversion/
This commit attempts to build a set of redfish includes file with all
the available Redfish enums in an easy to use enum class. This makes it
very clear which enums are supported by the schemas we produce, and adds
very little to no extra boilerplate on the human-written code we
produced previously.
Note, in the generated enum class, because of our use of the clang-tidy
check for macros, the clang-tidy check needs an exception for these
macros that don't technically follow the coding standard. This seems
like a reasonable compromise, and in this case, given that nlohmann
doesn't support a non-macro version of this.
One question that arises is what this does to the binary size.... Under
the current compiler optimizations, and with the current best practices,
it leads to an overall increase in binary size of ~1200 bytes for the
enum machinery, then approximately 200 bytes for every call site we
switch over. We should decide if this nominal increase is reasonable.
Tested: Redfish protocol validator runs with same number of failures as
previously.
Redfish Service Validator passes (one unrelated qemu-specific exception)
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I7c7ee4db0823f7c57ecaa59620b280b53a46e2c1
|
|
Refactor getCollectionMembers to make sure all Url created with dbus
paths are generated via UrlFromPieces helper function. This allow us to
manage all URL generated from dbus in one place and allow us to make
future changes to affect all resources.
We can make changes to all resources easier if they are all managed by
one function.
Tested:
Redfish Validator Passed. All Collections working as expected and match
previous implmentation.
Change-Id: I5d3b2b32f047ce4f20a2287a36a3e099efd6eace
Signed-off-by: Willy Tu <wltu@google.com>
|
|
Like other C++ projects, unit tests normally are in a separate repo and
respect the folder structure of the file under test.
This commit deleted all "ut" folder and move tests to a "test" folder.
The test folder also has similar structure as the main folder.
This commit also made neccessary include changes to make codes compile.
Unused tests are untouched.
Tested: unit test passed.
Reference:
[1] https://github.com/grpc/grpc/tree/master/test
[2] https://github.com/boostorg/core/tree/414dfb466878af427d33b36e6ccf84d21c0e081b/test
[3] Many other OpenBMC repos: https://github.com/openbmc/entity-manager/tree/master/test
[4] https://stackoverflow.com/questions/2360734/whats-a-good-directory-structure-for-larger-c-projects-using-makefile
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I4521c7ef5fa03c47cca5c146d322bbb51365ee96
|
|
The existing code doesn't propogate errors of subqueries correctly.
This commit corrects the behavior, so that the final response gets all
error message of subqueries and the "highest priority" HTTP code.
DMTF doesn't specify how expand queries handle error codes, since using
subqueries is an implementation choice that we made in this project.
What we did here follows existing behavior of this project, and follows
the error message section of the Redfish spec;
[1] https://redfish.dmtf.org/schemas/DSP0266_1.15.1.html#error-responses
As for now, this commit uses the worst HTTP code among all the error
code. See query_param.hpp, function |propogateErrorCode| for detailed
order of the errror codes.
Tested:
1. this is difficult to test, but I hijacked the code so it returns
errors in TaskServices, then I verified that "/redfish/v1?$expand=."
correctly returns 500 and the gets the error message set.
2. unit test so that when there are multiple errors, the final response
gets a generate error message.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I0c1ebdd9015f389801db9150d687027485f1203c
|
|
There's a few last places (outside of tests) where we still use
nlohmann brace initialization. Per the transforms we've been doing,
move these to constructing the objects explicitly, using operator[],
nlohmann::object_t and nlohmann::array_t. Theses were found by manual
inspection grepping for all uses of nlohmann::json.
This is done to reduce binary size and reduce the number of intermediate
objects being constructed. This commit saves a trivial amount of size
(~4KB, Half a percent of total) and in addition but makes our
construction consistent.
Tested:
Redfish service validator passes.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I7478479a9fdc41b254eef325002d413c1fb411a0
|
|
The following error reports have started to be reported by clang-tidy:
* readability-qualified-auto - add 'const' to `auto&` iterators
* bugprone-use-after-move - add break in loop after element is found
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I5314559f62f58aa032d4c74946b8e3e4ce6be808
|
|
used sdbusplus::unpackPropertiesNoThrow in other places, also replaced
all usages of "GetAll" with sdbusplus::asio::getAllProperties
bmcweb size: 2697640 -> 2685336 (-12304)
compressed size: 1129728 -> 1126078 (-3650)
Tested:
- Executed redfish service validator, no new errors detected
Change-Id: I916e462e004fcbde67c209daef295de8f5fb68eb
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
|
|
The most outstanding concerns for $select query have been resolved. We
added a set of restrictions: character set, property length, # of
properties, which makes this feature safe to use.
This commit takes $select out of the insecure flag, so every system can
start to use it. This decision has been made in Discord, available at
[1] https://discord.com/channels/775381525260664832/994314752102760559/1006650821569675355
Tested:
1. unit test passed
2. no new service validator failure on hardware
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I1f669cd35afcc1a65473a3ed665768e172a423bc
|
|
This is a minor reorganization of helper code from ethernet (which is
redfish specific) into utils. This function is generic, and should be
in ip utils.
Tested: Code compiles. Code move only.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I4aac5a98379c9844aea6c21d2294d1ed7ae2ea9b
|
|
It was accidentally missed in previous round of iwyu clean up.
Tested: build.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I6857409848a99594d5c91168cf4a760e456e8c90
|
|
SensorsAsyncResp has existed for a long time, and has slowly morphed
from its intended usage (as an RAII response object) into a
conglomeration of all possible things that a sensor request could want.
This leads to a ton of inefficient queries, and lots of data being held
for much longer than we'd like.
This commit tries to start breaking things apart, and follow the
patterns we use elsewhere, passing AsyncResp where a response object is
needed, and passing specialized data structures only into the scopes
where they're needed. This significantly increases the performance of
the /redfish/v1/Chassis/<>/Sensors/<sensor> URI.
The optimization changes the URI such that in includes both the sensor
type as well as the sensor name in the URI, meaning that from a given
tree, we can directly look up the sensor path, instead of having to look
up all sensor paths, and do a filename() compare on them.
Implementation-wise, there is one main difference in user-facing
behavior, in that instead of using a mechanized version of the sensor
name for the URI (aka /redfish/v1/Chassis/my_chassis/Sensors/my_sensor)
the URI now contains the sensor type (ex
/redfish/v1/Chassis/my_chassis/Sensors/temperature_my_sensor). One
implementation note: because fan_pwm and fan_tach namespaces have an
underscore in them, we normalize these in the URI to fanpwm and fantach
respectively such that we can differentiate between the two without
looping, and special case them on the other side. This seems like a
reasonable compromise.
The above means that when a request comes in to query the sensor, we no
longer have to pull all sensors to identify the one that matches the
name, and we can go directly to the mapper to determine which sensor we
need, with a GetObject query. This significantly reduces the amount of
time to grab the information from a single sensor.
To accomplish this, the per-sensor methods needed broken down into
pieces that allowed loading a single sensor at a time, rather than a
complete GetManagedObjects call. In practice, this just means breaking
out one helper function, such that the new code can directly call
GetAll.
In a few places, const std::string& had to be replaced with
std::string_view, because the new sensor API can directly inline its
const char* parameters for types, which allows it to avoid constructing
a string copy to do it.
Tested:
Redfish service validator passes on a S7106 system, and shows a timing
of ~40-50ms per sensor request, which is in line with what we'd expect
for a keepalive function using Session auth.
'''
curl --insecure -w "@curl-format.txt" -H "X-Auth-Token: nOIarWLRFkFN14qVONs0" https://192.168.10.140/redfish/v1/Chassis/Tyan_S7106_Baseboard/Sensors/temperature_sys_air_inlet
'''
returns timing that is on the order of 125ms. On this setup,
ServiceRoot (which should do no dbus calls) returns in 90ms, so the
sensor implementation itself is on the order of 40% of the timing.
TelemetryService functions as expected
'''
curl -k --user "root:0penBmc" -X POST https://$bmc/redfish/v1/TelemetryService/MetricReportDefinitions/ -d '{"Id": "lxw1", "Metrics": [{"MetricId": "123", "MetricProperties": ["/redfish/v1/Chassis/Tyan_S7106_Baseboard/Power#/Voltages/0"]}], "MetricReportDefinitionType": "OnRequest", "ReportActions": ["LogToMetricReportsCollection"], "Schedule": {"RecurrenceInterval": "100"}}'
'''
Succeeds.
Also succeeds with MetricProperties set to:
/redfish/v1/Chassis/Tyan_S7106_Baseboard/Sensors/voltage_vcc5
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: If42f531b385c3b611b100c1c485a1e4e877c5512
|
|
Putting the maxEntriesPerPage next to the top parameter makes it more
clear about its intention as Ed suggested. Here it is also renamed to
maxTop to illustrate its relationship with top.
Tested:
Build, unit test and Redfish Service Validator passed.
Change-Id: I035eea7e33d78439685a81092a4dd9332cfc501a
Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>
|
|
We have two functions, leftZeroPadding, and padZeros that effectively do
the same thing. leftZeroPadding has only one usage, and padZeros is
debatably more efficient (given it doesn't need to construct an
intermediate string). Move the one usage of leftZeroPadding to
padZeros, and remove leftZeroPadding.
One minor change needs to be made to padZeros, in that it needs to
accept a long int instead of an int, given that some implementations use
long int as their duration object.
Tested: Unit tests pass. Good coverage on time functions.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Icbec45787ff940d098e18b61741b7476f4263558
|
|
We've accumulated several time utility functions in the http classes.
Time isn't a core HTTP primitive, so http is not where those functions
below.
This commit moves all the time functions from the crow::utility
namespace into the redfish::time_utils namespace, as well as moves the
unit tests.
No code changes where made to the individual functions, with the
exception of changing the namespace on the unit tests.
Tested: Unit tests pass.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I8493375f60aea31899c84ae703e0f71a17dbdb73
|
|
This commit fixes one of the TODOs: make $select work on object arrays.
This is according to https://github.com/DMTF/Redfish/issues/5188, where
array index doesn't count as the properity prefix.
To make sure reserved properties are selected on every node, this commit
also refactors some of the logics when inserting new properties.
Tested:
1. unit test
2. tested on hardware,
URL: /redfish/v1/Chassis/chassis/Thermal?$select=Temperatures/Name
{
"@odata.id": "/redfish/v1/Chassis/chassis/Thermal",
"@odata.type": "#Thermal.v1_4_0.Thermal",
"Temperatures": [
{
"@odata.id": "/redfish/v1/Chassis/chassis/Thermal#/Temperatures/0",
"@odata.type": "#Thermal.v1_3_0.Temperature",
"Name": "Abc"
},
{
"@odata.id": "/redfish/v1/Chassis/chassis/Thermal#/Temperatures/1",
"@odata.type": "#Thermal.v1_3_0.Temperature",
"Name": "Xyz"
}
]
}
3. no new service validator failures
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: Ibfa22c0f42018cd0d482b4a19fff6dcd0cd346d1
|
|
used sdbusplus::unpackPropertiesNoThrow in processor.hpp, also replaced
all usages of "GetAll" with sdbusplus::asio::getAllProperties
bmcweb size: 2681176 -> 2677080 (-4096)
compressed size: 1129892 -> 1128633 (-1259)
Tested:
Performed get on:
- redfish/v1/Systems/system/Processors
- redfish/v1/Systems/system/Processors/cpu0
Get result before and after the change was the same
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
Change-Id: Ib28d842e348ebd8f160ec23b629ee4c4b280329b
|
|
The previous implementation has a downside: it stores all intermediate
paths in a hashset. This increases the space complexity. This commit
implements a more efficient (both time and space) algorithm: Trie.
The commit contructs a trie from the values of $select. Upon delegation,
it delegates the whole trie for now. When doing recursive select, now we
only need to keep the current JSON node and corresponding TrieNode.
Given the following assumption:
1. size of the JSON tree (#nodes) is N
2. length of the $select properties is M
3. average length of a single $select property is K
The previous implementation has the following complexity:
1. time: worst case O(N + M * K), when a property is like /a/a/a/a/a
2. space: O(N + M * K^2), when a property is like /a/a/a/a/a
The current implementation improves complexity to:
1. time: O(N + M * K)
2. space: O(N + M * K)
The total image (squashfs) decreases 4096 bytes. The binaray size
also decreases 4096 bytes.
Tested:
1. $select works on real hardware
2. No new service validator failures
3. Added more unit tests
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: If9f09db8c76e4e1abb6fa9b760be3816d9eb9f96
|
|
There is a bug where, when running an expand query, headers from the
response object get dropped. These headers include OData.type, and the
newly minted Link header, as well as possible others.
This was actually noted in a TODO, although the author of the TODO,
didn't fully understand the consequences at the time, and thought there
was no functional impact.
To resolve this, this commit resolves the TODO, and allows the Response
object to be moved out, instead of having to create a new one, which
preserves all the response state. To do this, it creates a move
constructor on the Response object for this use. The move constructor
is relatively benign, with one caveat, that we might be moving while in
a completion handler (as is the most common use). So both the existing
operator= and Response() move constructor are amended to handle this
case, and simply null out the response object in the copied object,
which would be correct behavior, given that each callback handler should
only be called once per Response object.
Tested:
curl --insecure --user root:0penBmc -vvvv
https://192.168.7.2/redfish/v1\?\$expand\=\*\(\$levels\=2\)
returns the same body as previously, now with the included:
OData-Version: 4.0
Allow: Get
headers in the response.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I221364dd4304903b37cacb1386f621b073a0a891
|
|
The array branch is never used since we don't yet support $select in
object arrays. This commit can be merged before any other optimization:
two pointers, Trie, etc.
Surprisingly, this commit doesn't save any binary size. The binary
doesn't change at all.
Tested:
1. unit test
2. tested on real hardware, $select is working as expected.
URL:
/redfish/v1/Systems/system?$select=Status
{
"@odata.id": "/redfish/v1/Systems/system",
"@odata.type": "#ComputerSystem.v1_16_0.ComputerSystem",
"Status": {
"Health": "OK",
"HealthRollup": "OK",
"State": "Enabled"
}
}
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: Ia00f1e2b8d9a5787f8c6d819d2f817808364abd5
|
|
This commits implement the generic handler for the $select query in
the Redfish Spec, section 7.3.3.
$select takes a comma separated list of properties, and only these
properties will be returned in the response.
As a first iteration, this commits doesn't handle $select combined with
$expand. It returns an unimplemented error in that case. I am currently
working with DMTF and getting their clarification. See this issue for
details: https://github.com/DMTF/Redfish/issues/5058.
It also leaves other TODOs in the comment of |processSelect|. Today,
$select is put behind the insecure-query flag.
Tested:
0. No $select is performed when the flag is disabled.
1. The core codes are just JSON manipulation. Tested in unit tests.
2. On hardware,
URL: /redfish/v1/Systems/system/ResetActionInfo?$expand=.&$select=Id
400 Bad Request
URL: /redfish/v1/Systems/system?$select=ProcessorSummary/Status
{
"@odata.id": "/redfish/v1/Systems/system",
"@odata.type": "#ComputerSystem.v1_16_0.ComputerSystem",
"ProcessorSummary": {
"Status": {
"Health": "OK",
"HealthRollup": "OK",
"State": "Disabled"
}
}
}
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I5c570e3a0a37cbab160aafb8107ff8a5cc99a6c1
|
|
With the inclusion of ce8ea743055f1b82c60790db40aa3295e03bdf9c it looks
like we're now returning 400 error codes, with a response error of
QueryNotSupportedOnResource for resources which don't support top and
skip (like RegistryFile). This would imply that the Query object NEEDS
a way to represent "user didn't provide us a skip/top parameter" which
arguably means this needs to go back to a std::optional<size_t>.
The error gets added from:
https://github.com/openbmc/bmcweb/blob/d5c80ad9c07b94465d8ea62d2b6f87c30cac765e/redfish-core/include/utils/query_param.hpp#L304
and appears to be a basic logic error in that now all queries assume
that the user provided top and skip, which fails for non-collections.
This commit moves that direction, changing the Query object back to
std::optional<size_t>. This has the unintended consequence of now
putting the idea of "defaults" back into the per-delegated handlers. This
seems reasonable, as arguably the defaults for each individual collection
are going to be different, and at some point we might want to take advant
age of that.
Tested:
1. Tested on Romulus QEMU. All passed.
2. Tested on s7106, Validator passed.
Signed-off-by: Ed Tanous <edtanous@google.com>
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I9f912957d130694b281c6e391602de158eaddcb3
|
|
These changes are done by running iwyu manually under clang14.
Suppressed some obvious impl or details headers. Kept the recommended
public headers.
IWYU can increase readability, make maintenance easier, and avoid errors
in some cases. See details in
https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/WhyIWYU.md.
This commit also uses its best effort to correct obvious errors through
iwyu pragma.
See reference here:
https://github.com/include-what-you-use/include-what-you-use#how-to-correct-iwyu-mistakes
Tested: unit test passed.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I983b6f75601707cbb0f2f04546c3362ff4ba7fee
|
|
getValidChassisPath shouldn't rely on SensorAsyncResp object. Move
it to utils to wider use.
Tested: Redfish service Validator passed.
Change-Id: I418b7f0f3846fd001392536e2943f062b1bdb5cd
Signed-off-by: Zhenwei Chen <zhenweichen0207@gmail.com>
|
|
Current code initializes $top to std::numeric_limits<size_t>::max(),
when adding with a non-zero $skip value, it overflows. This patch
solves this issue by initializing it to maxEntriesPerPage.
Fixes c937d2b ("Make log services use parameter delegation").
Tested:
Verified providing only $skip in the query parameter in /redfish/v1
/Systems/system/LogServices/EventLog/Entries is properly handled.
Change-Id: Id5668cecda97a78f803941d6eb2e1aa0ba9495aa
Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>
|
|
According to "Table 8 - Query parameters" of Redfish spec, the value of
$top can be 0. And $skip can also be 0 to support getting all members,
though it is not specified in spec. This commit updates the error
message accordingly.
Tested:
Error message change only since 0 is already supported in bmcweb.
Change-Id: I5cc3fd7d283f8ee4dfca9325615545d1446e2133
Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>
|
|
cppcheck correctly notes that a lot of variables in the new code can be
const. Make most of them const.
Tested: WIP
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I8f37b6353fd707923f533e1d61c5b5419282bf23
|
|
The Redfish specification for PATCH of arrays defines a number of
requirements.
- Setting a value to null, should remove it from the list.
- Setting a value to empty object "{}" should leave the value unmodified
- Values at indexes larger than whats included in the PATCH request
shall be removed.
This commit attempts to fix this behavior for NTPServers and make it
correct. It does this by first getting the list of NTP servers, then
walking the list in parallel with the list given in the patch, and
either modifying or changing the list as the spec requires before
setting the setting across the system.
It also turns out that the current behavior of unpacking nlohmann::json
objects requires an object to be an array, object, or null, which
doesn't allow unpacking the strings required in this case, so that check
is removed. A quick inspection shows that we don't unpack nlohmann
objects very often, and this should have no impact.
Tested:
Redfish-protocol-validator tests for NTPServers now pass
'''
curl -vvvv --insecure --user root:0penBmc https://192.168.7.2/redfish/v1/Managers/bmc/NetworkProtocol -X PATCH -d '{"NTP": {"NTPServers": []}}'
'''
Used to patch values succeeds with various "good" values;
["time-a-b.nist.gov", "time-b-b.nist.gov"]
[{}, {}]
["time-a-b.nist.gov", null]
[]
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I23a8febde34817bb0b934e46e2b77ff391b52a57
|
|
This patchset is the conclusion of a multi-year effort to try to fix
shadowed variable names. Variables seem to be shadowed all over, and in
most places they exist, there's a "code smell" of things that aren't
doing what the author intended.
This commit attempts to clean up these in several ways by:
1. Renaming variables where appropriate.
2. Preferring to refer to member variables directly when operating
within a class
3. Rearranging code so that pass through variables are handled in the
calling scope, rather than passing them through.
These patterns are applied throughout the codebase, to the point where
-Wshadow can be enabled in meson.build.
Tested: Code compiles, unit tests pass. Still need to run redfish
service validator.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: If703398c2282f9e096ca2694fd94515de36a098b
|
|
In {Top,Skip}Positive cases, canDelegate{Top,Skip} is set to false,
which is the same as the default value used in Negative cases. This
patch changes these value to true and update test cases.
Tested:
Build and unit test pass.
Change-Id: Iead9e5ed805344b1c399a7bbf586c16164f15554
Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>
|
|
Put all test cases into the anonymous namespace. It's shown that a lot
of codes (namespaces) are not needed anymore
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: Ib9866272d2d05103b2b792b361cb4c20fee67004
|
|
IWYU. Use <> for dependency headers and "" for bmcweb headers.
Reference:
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sf12-prefer-the-quoted-form-of-include-for-files-relative-to-the-including-file-and-the-angle-bracket-form-everywhere-else
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I80133bb38fbea886c9c7207b2ec16c0aeacca6d5
|
|
https://gerrit.openbmc.org/c/openbmc/bmcweb/+/54577/5/redfish-core/include/utils/sw_utils.hpp#377
changed the spelling of Updateable.
This caused the bmcweb bump to fail the Redfish Validator.
This most likely was just an accident.
The Error is:
ERROR - Updatable not defined in schema SoftwareInventory.v1_1_0 (check
version, spelling and casing)
ERROR - Attempting Updateable (from Updatable)?
Updatable, although more common, is not the Redfish property name.
From https://redfish.dmtf.org/schemas/SoftwareInventory.v1_7_0.json:
"Updateable": {
"description": "An indication of whether the Update...
Tested: None.
Change-Id: I8d2ab12f26e5df7ee35c5363acf70c1977fbcfdb
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
Rename to Software so we can reuse it for software inventory. This make
is more consistent to the SoftwareVersion dbus interface that is used.
Change-Id: I97fb10fccf744a6d6d4cba57f970759431bf4744
Signed-off-by: Willy Tu <wltu@google.com>
|
|
Clang is complaining about Wcovered-switch-default. This change removes
the default branch, instead, upon unexpected Enum type, it returns an
error data structure and sets 500 to the HTTP response.
The "Exhaustive switch pattern" was suggested in
https://abseil.io/tips/147.
Tested:
1. compiles on clang.
2. in mock environment, $expand is working fine.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I751dc446c57010a01d24e0a4edae7a1cc58a0e8f
|
|
|processAllParams| now can take reference of parsed |Query| given that
it's read-only now. The only copy is kept by the lambda.
Tested:
1. on my mock environment, query parameter works as expected. Tested
$only, $expand, $top, and $skip.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I1edf53d3b0e030c7370eb7ba937644d5ced074dc
|
|
Have seen where between the mapper call and the inner call here to
phosphor-bmc-code-mgmt, phosphor-bmc-code-mgmt deleted the image.
This was during code update and phosphor-bmc-code-mgmt was deleting the
backup image.
Redfish lists all associated images under the manager resource:
"SoftwareImages": {
"description": "The images that are associated with this manager.",
...
See https://redfish.dmtf.org/schemas/Manager.v1_15_0.json.
bmcweb needs to look at the image purpose hence the call to the backup
image.
EBADR is the resource not found error code.
If EBADR is returned when populating the firmware properties just
leave off the firmware properties. These properties aren't required.
Discussed in discord here:
https://discord.com/channels/775381525260664832/981260009256140852/981263933442785290
We do similar checks for an EBADR return code other places in bmcweb.
Tested: Everything looked the same.
To actually test this code path had to be creative.
Made this call look at a bad path:
*version;
}
},
- obj.second[0].first, obj.first,
+ obj.second[0].first, obj.first + "badid",
"org.freedesktop.DBus.Properties", "GetAll",
"xyz.openbmc_project.Software.Version");
When doing so I saw the following traces but no internal error:
(2022-06-01 20:29:41) [ERROR "fw_utils.hpp":139] error_code = generic:53
(2022-06-01 20:29:41) [ERROR "fw_utils.hpp":140] error msg = Invalid request descriptor
The firmware version and software links were left off.
The GUI handled this missing information well.
The validator passed.
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
Change-Id: I9d8cd8b04acadfdd10f660cf9b7ca5dc6f36b4d0
|
|
The usual updates to make code compile on clang again. Extra semicolons
that have snuck in, missing inline and static definitions.
Tested: Code compiles on clang.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Id7f889de98cafaa89471d75ed3e3bb97ab3855cd
|
|
For any expand query, the current implementation does all queries in a
single MultiAsyncResp, where the code sends a bunch of requests without
Query parameters. This makes it impossible to invoke efficient expand
handlers, since efficent handlers will only be invoked when a query has
$expand in its parameters. (Delegation only happens when the query
contains query parameters)
To solve it, in this commit, we proposed to send a bunch of requests
**WITH** Query parameters in MultiAsyncResp. This makes
"/redfish/v1/Chassis/chassis?expand=.($levels=2)" be able to invoke
efficient expand handlers that we developed for sensors, which existing
implementation can't do. This decreases latency by nearly 100 times (the
improvement that efficient sensor expand handler provides) on real
hardware which contains 5+ chassis and totally 220+ sensors.
This commit aligns with future $select support well, since the recursive
queries can add $select as part of the query parameters.
With this commit, though we create multiple MultiAsyncResp objects
memory doesn't increase significantly; part of the reason is that we are
not copying Query anymore in MultiAsyncResp.
No out-of-memory issues are found when 4 threads are querying
expand=levels=6 at the service root on a real large hardware which
contains 2+ sockets, 5+ chassis, 220+ sensors, 30+ DIMMs, etc.
Tested:
1. On real hardware, /redfish/v1/Chassis?$expand=.(level=3) is giving
the correct result and invokes efficient sensor Expand handler
2. stress test
```
for i in {1..4};
do
echo "thread $i"
wget -qO- 'http://localhost:18080/redfish/v1?$expand=*($levels=6)' > "/tmp/$i.log" &
done
for i in {1..1000};
do
top -b -n 1 | grep bmcweb >> /tmp/bmcweb_ori.log
sleep 1
done
```
Results
```
25878 2856 root R 194m 20% 1 38% /tmp/bmcweb_after
19005 2856 root R 215m 22% 1 36% /tmp/bmcweb_ori
```
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I0e661db0263f56dd0cab66047a0a5d4fff31b69a
|
|
clang-tidy has a setting, LambdaBodyIndentation, which it says:
"For callback-heavy code, it may improve readability to have the
signature indented two levels and to use OuterScope."
bmcweb is very callback heavy code. Try to enable it and see if that
improves things. There are many cases where the length of a lambda call
will change, and reindent the entire lambda function. This is really
bad for code reviews, as it's difficult to see the lines changed. This
commit should resolve it. This does have the downside of reindenting a
lot of functions, which is unfortunate, but probably worth it in the
long run.
All changes except for the .clang-format file were made by the robot.
Tested: Code compiles, whitespace changes only.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib4aa2f1391fada981febd25b67dcdb9143827f43
|
|
These are Google only resources powered by Hoth DBus interface.
The ComponentsProtected links is hardcoded for now.
But it will be queried from DBus and interpreted accordingly in the
future.
TEST:
$curl -u root:0penBmc -X GET http://[::1]:$PORT/google/v1/RootOfTrustCollection
{
"@odata.id": "/google/v1/RootOfTrustCollection",
"@odata.type": "#RootOfTrustCollection.RootOfTrustCollection",
"Members": [
{
"@odata.id": "/google/v1/RootOfTrustCollection/Hoth"
}
],
"Members@odata.count": 1
}
$ curl -u root:0penBmc -X GET http://[::1]:$PORT/google/v1/RootOfTrustCollection/Hoth
{
"@odata.id": "/google/v1/RootOfTrustCollection/Hoth",
"@odata.type": "#RootOfTrust.v1_0_0.RootOfTrust",
"Actions": {
"#RootOfTrust.SendCommand": {
"target": "/google/v1/RootOfTrustCollection/Hoth/Actions/RootOfTrust.SendCommand"
}
},
"Id": "Hoth",
"Location": {
"PartLocation": {
"ServiceLabel": "Hoth",
"Locationtype": "Embedded"
}
},
"Name": "RootOfTrust-Hoth",
"Status": {
"State": "Enabled"
}
$ curl -u root:0penBmc -X POST -d @req.json -H "Content-Type: application/json" http://[::1]:$PORT/google/v1/RootOfTrustCollection/Hoth/Actions/RootOfTrust.SendCommand
{
"CommandResponse": "033B0000"
}
Signed-off-by: Vidya Satyamsetti <satyamsetti@google.com>
Change-Id: If64612468bb89e6d9251d848697608b7daf37339
|
|
Brace initialization of json objects, while quite interesting from an
academic sense, are very difficult for people to grok, and lead to
inconsistencies. This patchset aims to remove a majority of them in
lieu of operator[]. Interestingly, this saves about 1% of the binary
size of bmcweb.
This also has an added benefit that as a design pattern, we're never
constructing a new object, then moving it into place, we're always
adding to the existing object, which in the future _could_ make things
like OEM schemas or properties easier, as there's no case where we're
completely replacing the response object.
Tested:
Ran redfish service validator. No new failures.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Iae409b0a40ddd3ae6112cb2d52c6f6ab388595fe
|
|
$top and $skip are parameters for controlling the responses of
collections, to limit their size, per the Redfish specification section
7.4.
$skip=integer "Applies to resource collections. Returns a subset of the
members in a resource collection, or an empty set of members if the
$skip value is greater than or equal to the member count. This paging
query parameter defines the number of members in the resource collection
to skip."
$top=<integer> "Applies to resource collections. Defines the number of
members to show in the response. Minimum value is 0 , though a value of
0 returns an empty set of members."
This commit implements them within the resource query.
Tested:
curl --insecure --user root:0penBmc https://localhost:18080/redfish/v1/Registries\?\$top\=1
Returns 1 value. Walking through values of 1-5 (there are 4 registries
currently) returns the appropriate sizes of collection (with 5 returning
4 entries).
curl --insecure --user root:0penBmc https://localhost:18080/redfish/v1/Registries\?\$skip\=0
Returns the collection. $skip values of 0-5 return descending number of
results.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ied8a8f8338f119173509fb4b7ba2bd4a6c49cae8
|
|
Old codes handle links incorrectly; when links appear before some
keys, old codes don't expand these keys.
Tested:
1. On real hardware, Expand with links are working correctly.
2. Unit tests.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I028b55579d833f23120987a24cef4442fdd5800d
|
|
The commit prior to this one added support for delegation of $expand and
$only query param types; This commit adds support for delegation of top
and skip (which we already have a few handlers for) and moves them to
the new style.
Note, this makes top and skip query params NOT below the
insecure-enable-redfish-query. top and skip have existed for a while,
and are unlikely to have security issues, as they're relatively simple
transforms.
Tested:
curl --insecure --user root:0penBmc https://192.168.7.2/redfish/v1/Managers/bmc/LogServices/Journal/Entries\?\$top\=3\&\$skip\=0
With varying $top between 1-5 and $skip between 0-5 gave the expected
number of log results.
Unit tests pass.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ia213a5e929c40579825eaf251e4b9159bc84c802
|
|
insecure-enable-redfish-query is really only intended to protect the
user from things that might run the system out of resources, like
expand, or complex filter queries (ie queries that might pop the
stack). This commit message moves the location where the parameters are
enabled/disabled into the parser itself, such that some parameters (like
top and skip in the next commit) can be executed outside of this option
flag.
Because of moving the expand support deeper in the call stack, some unit
tests now need to be aware of whether or not expand is supported in the
configuration.
Tested:
Enabled query option through local.conf with
EXTRA_OEMESON:pn-bmcweb:append =
"-Dinsecure-enable-redfish-query='enabled'"
Then did:
curl --insecure --user root:0penBmc https://192.168.7.2/redfish/v1\?\$expand\=\*
Query expanded as expected;
set insecure-enable-redfish-query='disabled'
and observed that the same curl query returned
QueryParameterValueFormatError, which is expected.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I24fbc2c9f64628d6457dd117b61ff22b276b0682
|
|
From the quoted section of the spec in the patchset, we should be
ignoring odata annotations on PATCH requests. This commit implements a
preliminary loop through the json object, and removes the odata items
before processing begins.
Tested:
curl -vvvv --insecure --user root:0penBmc -X PATCH -d '{"@odata.etag":
"my_etag"}' https://192.168.7.2/redfish/v1/AccountService/Accounts/root
returns: Base.1.11.0.NoOperation
Redfish protocol validator now passes the REQ_PATCH_ODATA_PROPS test.
Included unit tests passing.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I62be75342681d147b8536fd122bbc793eeaa3788
|
|
This change is adding helper template function, which can be used both
to validate and read segments from segments_view returned by boost_url
parser. Number of segments is also validated - in case when argument
count differs from them, false will be returned. In case when we want to
validate only existence of a segment, special argument can be passed in
its place: 'anySegment'.
Reasoning why url_view was chosen instead of strings:
- This way code generation is kept minimal.
- There are multiple parse functions in boost_url with different rules,
but all of them return url_view. This solution should accommodate
every use case.
Testing done:
- Unit tests are added, passing.
- Refactored part of telemetry to use this new approach, no regression
spotted during simple POST/GET tests.
Change-Id: I677a34e1ee570d33f2322a80dc1629f88273e0d5
Signed-off-by: Szymon Dompke <szymon.dompke@intel.com>
|
