Enable init checker

clang-tidy added cppcoreguidelines-init-variables as a check, which is
something we already enforce to some extent, but getting CI to enforce
it will help reviews move faster.

T

Enable init checker

clang-tidy added cppcoreguidelines-init-variables as a check, which is
something we already enforce to some extent, but getting CI to enforce
it will help reviews move faster.

Tested: Code compiles. Noop changes.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I7e10950de617b1d3262265572b1703f2e60b69d0

show more ...

Enable clang-tidy forward reference checks

Clang-13 adds new checks we can turn on, which find quite a few errors.

Tested: Code compiles

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I

Enable clang-tidy forward reference checks

Clang-13 adds new checks we can turn on, which find quite a few errors.

Tested: Code compiles

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I74b780760014c898cc440b37aea640b33e91c439

show more ...

Enforce const correctness

For all async calls, we should be consistently capturing non trivial
objects by const reference. This corrects bmcweb to be consistent and
capture errors by const value, a

Enforce const correctness

For all async calls, we should be consistently capturing non trivial
objects by const reference. This corrects bmcweb to be consistent and
capture errors by const value, and objects by const reference.

Tested: Code compiles. Trivial changes.

This saves about 300 bytes on our compressed binary size.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib3e0b6edef9803a1c480701556949488406305d4

show more ...

Consistently use ManagedObjectType

Some subsystems seem to have invented their own typedefs for this stuff,
move to using the one typedef in dbus::utility so we're consistent, and
we reduce our temp

Consistently use ManagedObjectType

Some subsystems seem to have invented their own typedefs for this stuff,
move to using the one typedef in dbus::utility so we're consistent, and
we reduce our templates.

Tested: code compiles

This saves a negligible amount (104 bytes compressed) on our binary
size.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I952ea1f960aa703808d0ac80f35dc24cdd8d5027

show more ...

Fix variable naming for NIC IP entry

The variable niciPentry is really hard to understand at first glance,
rename it to nicIpEntry.

Tested:
Build pass.

Change-Id: Ie11efbb25e45c40435216fa10c094a0f

Fix variable naming for NIC IP entry

The variable niciPentry is really hard to understand at first glance,
rename it to nicIpEntry.

Tested:
Build pass.

Change-Id: Ie11efbb25e45c40435216fa10c094a0f190d0fe8
Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>

show more ...

Move to common variant

This saves approximately 34kB in the compressed binary size of bmcweb
due to reduced template instantiations. This amounts to a 2.5%
reduction in the overall size.

Note, the

Move to common variant

This saves approximately 34kB in the compressed binary size of bmcweb
due to reduced template instantiations. This amounts to a 2.5%
reduction in the overall size.

Note, there were a few places where we broke const-correctness in the
form of pulling a non-const reference out of a const variant. This
new variant now requires const correctness, so some consts are
added where required.

Tested: Code compiles.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I6a60c8881c1268627eedb4ffddf16689dc5f6ed2

show more ...

Remove non-standard IPv6StaticAddress properties

AddressOrigin and AddressState are not defined in redfish schema of
IPv6StaticAddress, which causes redfish validator error, so remove
them.

Tested:

Remove non-standard IPv6StaticAddress properties

AddressOrigin and AddressState are not defined in redfish schema of
IPv6StaticAddress, which causes redfish validator error, so remove
them.

Tested:
Redfish validator passed.

Change-Id: I165f22662cecf25cefb2c0f6ce4777eb681e9b23
Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>

show more ...

Enhance logs for networkd, hypervisor networkd app

Everytime a user does a GET on the ethernet interface redfish endpoint
the journal logs gets filled with errors like the following:

curl command:

Enhance logs for networkd, hypervisor networkd app

Everytime a user does a GET on the ethernet interface redfish endpoint
the journal logs gets filled with errors like the following:

curl command:
GET https://${bmc}/redfish/v1/Managers/bmc/EthernetInterfaces/eth1
GET https://${bmc}/redfish/v1/Systems/hypervisor/EthernetInterfaces/eth1

[ERROR "hyperv \
isor_system.hpp":211] Got extra property: Type on the /xyz/openbmc_pr \
oject/network/hypervisor/eth0/ipv4/addr0 object

[ERROR "ethernet.hpp":499] Got extra property: Type on the \
/xyz/openbmc_project/network/hypervisor/eth0/ipv4/addr0 object

While extracting the ip object, the properties Type & Gateway in the
implementation are logged as errors, but they are not. The reason is,
these two properties are not used but only the origin, prefix length,
and address are fetched from the dbus to fill the ipv<4/6>config object.

This commit avoids logging the above properties as errors. If there are
properties other than these, then error will logged.

Tested By:

Before the changes:

Nov 26 06:10:46 bmc bmcweb[264]: (2021-11-26 06:10:46) [ERROR "hyperv \
isor_system.hpp":211] Got extra property: Type on the /xyz/openbmc_pr \
oject/network/hypervisor/eth0/ipv4/addr0 object
Nov 26 06:10:46 bmc bmcweb[264]: (2021-11-26 06:10:46) [ERROR "hyperv \
isor_system.hpp":211] Got extra property: Origin on the /xyz/openbmc_ \
project/network/hypervisor/eth1/ipv4/addr0 object
Nov 26 06:10:46 bmc bmcweb[264]: (2021-11-26 06:10:46) [ERROR "hyperv \
isor_system.hpp":211] Got extra property: Type on the /xyz/openbmc_pr \
oject/network/hypervisor/eth1/ipv4/addr0 object

After changes:

These traces wont be logged as errors.

Signed-off-by: Asmitha Karunanithi <asmitk01@in.ibm.com>
Change-Id: I7160d3f71121a9758124a7c29517176e396c333e

show more ...

Update clang-format

refer: https://github.com/openbmc/docs/blob/master/style/cpp/.clang-format
`Don't break long string literals`

Tested: built bmcweb successfully and RedfishValidator Passed.

Sig

Update clang-format

refer: https://github.com/openbmc/docs/blob/master/style/cpp/.clang-format
`Don't break long string literals`

Tested: built bmcweb successfully and RedfishValidator Passed.

Signed-off-by: George Liu <liuxiwei@inspur.com>
Change-Id: Ib58f7c942fd3838592e043c57e0b6ffcdc3d963b

show more ...

Make code compile with clang-13

Clang-13 rightfully warns that the hasWebuiRoute variable isn't declared
as static. This commit resolves that, and adds the static keyword so it
can be used in multi

Make code compile with clang-13

Clang-13 rightfully warns that the hasWebuiRoute variable isn't declared
as static. This commit resolves that, and adds the static keyword so it
can be used in multiple compile units. It also adds the static keyword
to the privilege registry, and the inline keyword to many methods that
now need it.

clang-format is also updated to version 12 in parse_registies.py, as
that's what CI uses, and what most people have installed.

Tested:
Followed clang-tidy instructions in README.md
"bitbake bmcweb" step now succeeds.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Id43b13606754cb37a404799fce155599ac3a3240

show more ...

Automate PrivilegeRegistry to code

This commit attempts to automate the creation of our privileges
structures from the redfish privilege registry. It accomplishes this by
updating parse_registries.

Automate PrivilegeRegistry to code

This commit attempts to automate the creation of our privileges
structures from the redfish privilege registry. It accomplishes this by
updating parse_registries.py to also pull down the privilege registry
from DMTF.
The script then generates privilege_registry.hpp, which include const
defines for all the privilege registry entries in the same format that
the Privileges struct accepts. This allows new clients to simply
reference the variable to these privilege structures, instead of having
to manually (ie error pronely) put the privileges in themselves.

This commit updates all the routes.

For the moment, override and OEM schemas are not considered. Today we
don't have any OEM-specific Redfish routes, so the existing ones inherit
their parents schema. Overrides have other issues, and are already
incorrect as Redfish defines them.

Binary size remains unchanged after this patchset.

Tested:
Ran redfish service validator

Ran test case from f9a6708c4c6490257e2eb6a8c04458f500902476 to ensure
that the new privileges constructor didn't cause us to regress the brace
construction initializer.

Checked binary size with:
gzip -c
$BBPATH/tmp/work/s7106-openbmc-linux-gnueabi/obmc-phosphor-image/1.0-r0/rootfs/usr/bin/bmcweb
| wc -c
1244048

(tested on previous patchset)

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ideede3d5b39d50bffe7fe78a0848bdbc22ac387f

show more ...

Remove ambiguous privileges constructor

There are a number of endpoints that assume that a given routes
privileges are governed by a single set of privileges, instead of
multiple sets ORed together.

Remove ambiguous privileges constructor

There are a number of endpoints that assume that a given routes
privileges are governed by a single set of privileges, instead of
multiple sets ORed together. To handle this, there were two overloads
of the privileges() method, one that took a vector of Privileges, and
one that took an initializer_list of const char*. Unfortunately, this
leads some code in AccountService to pick the wrong overload when it's
called like this
.privileges( {{"ConfigureUsers"}, {"ConfigureManager"},
{"ConfigureSelf"}})

This is supposed to be "User must have ConfigureUsers, or
ConfigureManager, or ConfigureSelf". Currently, because it selects the
wrong overload, it computes to "User must have ConfigureUsers AND
ConfigureManager AND ConfigureSelf.

The double braces are supposed to cause this to form a vector of
Privileges, but it appears that the initializer list gets consumed, and
the single invocation of initializer list is called. Interestingly,
trying to put in a privileges overload of
intializer_list<initializer_list<const char*>> causes the compilation to
fail with an ambiguous call error, which is what I would've expected to
see previously in this case, but alas, I'm only a novice when it comes
to how the C++ standard works in these edge cases. This is likely due
in part to the fact that they were templates of an unused template param
(seemingly copied from the previous method) and SFINAE rules around
templates.

This commit functionally removes one of the privileges overloads, and
adds a second set of braces to every privileges call that previously had
a single set of braces. Previous code will not compile now, which is
IMO a good thing.

This likely popped up in the Node class removal, because the Node class
explicitly constructs a vector of Privilege objects, ensuing it can hit
the right overload

Tested:
Ran Redfish service validator

Tested the specific use case outlined on discord with:
Creating a new user with operator privilege:
```
redfishtool -S Always -u root -p 0penBmc -vvvvvvvvv -r 192.168.7.2
AccountService adduser foo mysuperPass1 Operator
```

Then attempting to list accounts:
```
curl -vvvv --insecure --user foo:mysuperPass1
https://192.168.7.2/redfish/v1/AccountService/Accounts/foo
```

Which succeeded and returned the account in question.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I83e62b70e97f56dc57d43b9081f333a02fe85495

show more ...

Remove the Node class

Fixes #181

Lots of specific details around why the node class have been removed are
in the previous patchsets. This commit actually does the deed and makes
it go away entirel

Remove the Node class

Fixes #181

Lots of specific details around why the node class have been removed are
in the previous patchsets. This commit actually does the deed and makes
it go away entirely.

Now that this is finally done, we can compare binary size. Surprisingly
enough, this series saves a full 72KB of compressed binary size, which
amounts to about 6.4% of the total code size.

Before: 1197632 bytes
After: 1124688 bytes

This IMO makes it worth it, considering we've significantly reduced the
amount of code at the same time.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I3c8688715f933b381cad0be75a079ccfd72c3130

show more ...

Move ethernet interface away from Node class

Pursuant to the other patchsets further up in this chain, continue to
move things to the simpler BMCWEB_ROUTE mechanism. This moves the
Ethernet modules

Move ethernet interface away from Node class

Pursuant to the other patchsets further up in this chain, continue to
move things to the simpler BMCWEB_ROUTE mechanism. This moves the
Ethernet modules up the chain.

Tested:
Ed Tested on redfish service validator, no new errors (unrelated UUID
error present)
Gunnar Tested Series on validator and GUI.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Id1937e0d3f525d58744e6ff6360ef23f66350c66

show more ...

Remove Redfish Node class

Reduces the total number of lines and will allow for easier testing of
the redfish responses.

A main purpose of the node class was to set app.routeDynamic(). However
now a

Remove Redfish Node class

Reduces the total number of lines and will allow for easier testing of
the redfish responses.

A main purpose of the node class was to set app.routeDynamic(). However
now app.routeDynamic can handle the complexity that was once in critical
to node. The macro app.routeDynamic() provides a shorter cleaner
interface to the unerlying app.routeDyanic call. The old pattern set
permissions for 6 interfaces (get, head, patch, put, delete_, and post)
even if only one interface is created. That pattern creates unneeded
code that can be safely removed with no effect.
Unit test for the responses would have to mock the node the class in
order to fully test responses.

see https://github.com/openbmc/bmcweb/issues/181

The following files still need node to be extracted.

virtual_media.hpp
account_service.hpp
redfish_sessions.hpp
ethernet.hpp

The files above use a pattern that is not trivial to address. Often their
responses call an async lambda capturing the inherited class. ie
(https://github.com/openbmc/bmcweb/blob/ffed87b5ad1797ca966d030e7f979770
28d258fa/redfish-core/lib/account_service.hpp#L1393)
At a later point I plan to remove node from the files above.

Tested:
I ran the docker unit test with the following command.
WORKSPACE=$(pwd) UNIT_TEST_PKG=bmcweb
./openbmc-build-scripts/run-unit-test-docker.sh

I ran the validator and this change did not create any issues.
python3 RedfishServiceValidator.py -c config.ini

Signed-off-by: John Edward Broadbent <jebr@google.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I147a0289c52cb4198345b1ad9bfe6fdddf57f3df

show more ...

Fix missed bmcweb::AsyncResp

8d1b46d7 moves bmcweb to using AsyncResp everywhere, and not have
each individual handler creating its own object.

9010ec2e should not have been merged yet. This fixes

Fix missed bmcweb::AsyncResp

8d1b46d7 moves bmcweb to using AsyncResp everywhere, and not have
each individual handler creating its own object.

9010ec2e should not have been merged yet. This fixes a miss in the
review.

Tested: Built and validator passes.
From webui-vue, changed the IP Address.

Change-Id: I04b070ed61b788fa87e9d6fddede502701e9742d
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>

show more ...

Redfish(Network): Modified to support Default gateway on EthernetInterface

Earlier we have the gateway at system level, Now with the change
https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-netw

Redfish(Network): Modified to support Default gateway on EthernetInterface

Earlier we have the gateway at system level, Now with the change
https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-networkd/+/34852/
Gateway has been associated with interface level.
This commit fixes this behaviour.

Tested by:

GET https://${BMC_IP}/redfish/v1/Managers/bmc/EthernetInterfaces/eth0/
PATCH -D '{"IPv4StaticAddresses": [{},{"Address": "10.7.8.7","SubnetMask": "255.255.0.0","Gateway":"10.7.8.1"}]}' https://${BMC_IP}/redfish/v1/Managers/bmc/EthernetInterfaces/eth0

Ran Redfish validator

Signed-off-by: Ravi Teja <raviteja28031990@gmail.com>
Change-Id: I39e76b9552dacfe249c459590b1986d6eba8bb89

show more ...

Using AsyncResp everywhere

Get the core using AsyncResp everywhere, and not have each individual handler
creating its own object.We can call app.handle() without fear of the response
getting ended a

Using AsyncResp everywhere

Get the core using AsyncResp everywhere, and not have each individual handler
creating its own object.We can call app.handle() without fear of the response
getting ended after the first tree is done populating.
Don't use res.end() anymore.

Tested:
1. Validator passed.

Signed-off-by: zhanghaicheng <zhanghch05@inspur.com>
Change-Id: I867367ce4a0caf8c4b3f4e07e06c11feed0782e8

show more ...

ethernet: Fix for MAC address patch response

While Patching MAC address from Redfish, response is showing
as empty but HTTPS status code is "200 OK" on successful case.
So provided fix for Proper re

ethernet: Fix for MAC address patch response

While Patching MAC address from Redfish, response is showing
as empty but HTTPS status code is "200 OK" on successful case.
So provided fix for Proper response on Success.

Tested:
1. Verified RedFish validator passed
2. Verified the response on Redfish by updating MAC address.
PATCH: https://<bmc_ip>/redfish/v1/Managers/bmc/EthernetInterfaces/eth0
Body:
{
"MACAddress": "xx:xx:xx:xx:xx:xx"
}
Response:
{
"@Message.ExtendedInfo": [
{
"@odata.type": "#Message.v1_1_1.Message",
"Message": "Successfully Completed Request",
"MessageArgs": [],
"MessageId": "Base.1.8.1.Success",
"MessageSeverity": "OK",
"Resolution": "None"
}
]
}

Signed-off-by: Sunita Kumari <sunitax.kumari@intel.com>
Signed-off-by: Jayaprakash Mutyala <mutyalax.jayaprakash@intel.com>
Change-Id: I91a34f414b854593b4873e4ed923734030d6562d

show more ...

Fix IPv6DefaultGateway validator warnings

This commit fixes below warnings
WARNING - IPv6DefaultGateway: Empty string found

Tested By:
Ran validator

Signed-off-by: Ravi Teja <raviteja28031990@gmai

Fix IPv6DefaultGateway validator warnings

This commit fixes below warnings
WARNING - IPv6DefaultGateway: Empty string found

Tested By:
Ran validator

Signed-off-by: Ravi Teja <raviteja28031990@gmail.com>
Change-Id: Iac41feeb6cf245348f69915e0f58dd24ae46330e

show more ...

Fix nlohmann::json::dump calls

The nlohmann::json::dump call needs to be called with specific arguments
to avoid throwing in failure cases. http connection already does this
properly, but a bunch o

Fix nlohmann::json::dump calls

The nlohmann::json::dump call needs to be called with specific arguments
to avoid throwing in failure cases. http connection already does this
properly, but a bunch of code has snuck in (mostly in redfish) that
ignores this, and calls it incorrectly. This can potentially lead to a
crash if the wrong thing throws on invalid UTF8 characters.

This audits the whole codebase, and replaces every dump() call with the
correct dump(2, ' ', true, nlohmann::json::error_handler_t::replace)
call. For correct output, the callers should expect no change, and in
practice, this would require injecting non-utf8 characters into the
BMC.

Tested:
Ran several of the endpoints/error conditions in question, including
some of the error cases. Observed correct responses. I don't know of a
security issue that would allow injecting invalid utf8 into the BMC, but
in theory if it were possible, this would prevent a crash.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I4a15b8e260e3db129bc20484ade4ed5449f75ad0

show more ...

Start using sdbusplus::message::filename()

Lots of code gets checked in that does this path checking incorrectly.
So much so, that we have it documented in COMMON_ERRORS.md, yet, we
persist. This p

Start using sdbusplus::message::filename()

Lots of code gets checked in that does this path checking incorrectly.
So much so, that we have it documented in COMMON_ERRORS.md, yet, we
persist. This patchset starts using the new object_path::filename()
method that was added recently to sdbusplus. Overall, it deletes code,
and makes for a much better developer experience.

Tested:
Pulled down several endpoints and verified that filename() method works
properly, and the collections are returned as expected.

curl -vvvv --insecure --user root:0penBmc https://192.168.7.2/redfish/v1/AccountService/Accounts

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ief1e0584394fb139678d3453265f7011bc931f3c

show more ...

Fix .clang-tidy

camelLower is not a type, camelBack is.

Changes were made automatically with clang-tidy --fix-errors

To be able to apply changes automatically, the only way I've found that
works w

Fix .clang-tidy

camelLower is not a type, camelBack is.

Changes were made automatically with clang-tidy --fix-errors

To be able to apply changes automatically, the only way I've found that
works was to build the version of clang/clang-tidy that yocto has, and
run the fix script within bitbake -c devshell bmcweb. Unfortunately,
yocto has clang-tidy 11, which can apparently find a couple extra errors
in tests we already had enabled. As such, a couple of those are also
included.

Tested:
Ran clang-tidy-11 and got a clean result.

Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I9d1080b67f0342229c2f267160849445c065ca51

show more ...

Improve loops & fix cpp check warning

- This commit improves certain while loops to range based for loops.

- This commit also fixes the cppcheck warning that mentions about
performance issues whe

Improve loops & fix cpp check warning

- This commit improves certain while loops to range based for loops.

- This commit also fixes the cppcheck warning that mentions about
performance issues when using postfix operators on non-primitive
types.

Tested By:
- A function is unittested.
- GET on both EthernetInterfaces & certificate service
looks good without any issues.

Signed-off-by: Manojkiran Eda <manojkiran.eda@gmail.com>
Change-Id: I85420f7bf9af45a97e1a93b916f292c2516f5802

show more ...

Turn on ALL perf checks

1st, alphabetize the tidy-list for good housekeeping.

Next, enable all the clang-tidy performance checks, and resolve all the
issues. most of the issues boil down to:
1. Us

Turn on ALL perf checks

1st, alphabetize the tidy-list for good housekeeping.

Next, enable all the clang-tidy performance checks, and resolve all the
issues. most of the issues boil down to:
1. Using std::move on const variables. This does nothing.
2. Passing big variables (like std::string) by value.
3. Using double quotes on a find call, which constructs an intermediate
string, rather than using the character overload.

Tested
Loaded on system, logged in successfully and pulled down webui-vue. No
new errors.

Walked the Redfish tree a bit, and observed no new problems.

Ran redfish service validator. Got no new failures (although there are
a lot of log service deprecation warnings that we should look at).

Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I2238958c4b22c1e554e09a0a1787c744bdbca43e

show more ...