asio::connection: Expose dbus message to handler on error

unpack was hiding the dbus message when the response contained an error.
This was hiding error conditions in some cases, for example in
redf

asio::connection: Expose dbus message to handler on error

unpack was hiding the dbus message when the response contained an error.
This was hiding error conditions in some cases, for example in
redfish-core/lib/account_service.hpp processAfterCreateUser. When this
function attempted to access the underlying message, it would throw an
exception that mapped to a generic error handler. Exposing the dbus
message enables this function to show the real issue in its response.

Tested:

This bug can be triggered with robot test
Verify_Error_Upon_Creating_Same_Users_With_Different_Privileges and also
with curl

```
curl
-X POST "${ACCOUNTS_URL}/" \
-H "Content-Type: application/json" \
-d '{"UserName": "test_user", "Password": "TestPwd123", "RoleId": "Administrator", "Enabled": true}'
curl
-X POST "${ACCOUNTS_URL}/" \
-H "Content-Type: application/json" \
-d '{"UserName": "test_user", "Password": "TestPwd123", "RoleId": "ReadOnly", "Enabled": true}'
```

Before this patch, the second command above will log in the BMC

```
root@bmc:~# journalctl -f -u bmcweb -u xyz.openbmc_project.User.Manager.service
Feb 13 07:00:25 bmc phosphor-user-manager[495]: User 'test_user' already exists
Feb 13 07:00:25 bmc phosphor-user-manager[495]: Specified user name already exists.
Feb 13 07:00:25 bmc bmcweb[646]: [error_messages.cpp:1253] Internal Error ../git/redfish-core/lib/account_service.hpp(286:32) `void redfish::userErrorMessageHandler(const sd_bus_error*, const std::shared_ptr<bmcweb::AsyncResp>&, const std::string&, const std::string&)`:
```

And redfish

```
{
"error": {
"@Message.ExtendedInfo": [
{
"@odata.type": "#Message.v1_1_1.Message",
"Message": "The request failed due to an internal service error. The service is still operational.",
"MessageArgs": [],
"MessageId": "Base.1.19.InternalError",
"MessageSeverity": "Critical",
"Resolution": "Resubmit the request. If the problem persists, consider resetting the service."
}
],
"code": "Base.1.19.InternalError",
"message": "The request failed due to an internal service error. The service is still operational."
}
}
```

After

```
Feb 14 03:56:16 bmc systemd[1]: Started Start bmcweb server.
Feb 14 03:56:20 bmc phosphor-user-manager[497]: User 'test_user' already exists
Feb 14 03:56:20 bmc bmcweb[18926]: [account_service.hpp:1810] processAfterCreateUser failed for user 'test_user': ec=5 (Input/output error), ec.category=generic, dbus error name=xyz.openbmc_project.User.Common.Error.UserNameExist
s, dbus error message=Specified user name already exists.
Feb 14 03:56:20 bmc phosphor-user-manager[497]: Specified user name already exists.
```

Redfish

```
{
"UserName@Message.ExtendedInfo": [
{
"@odata.type": "#Message.v1_1_1.Message",
"Message": "The requested resource of type ManagerAccount with the property UserName with the value 'test_user' already exists.",
"MessageArgs": [
"ManagerAccount",
"UserName",
"test_user"
],
"MessageId": "Base.1.19.ResourceAlreadyExists",
"MessageSeverity": "Critical",
"Resolution": "Do not repeat the create operation as the resource was already created."
}
]
}
```

Added new unit test too, all 43 pass.

Change-Id: Iaefd2cfa4c1064a9b300e7322d8c0585640faac6
Signed-off-by: Nico Brailovsky <nicobrailovsky@meta.com>

show more ...

sdbusplus:test_message_call: Make test more resilient to timing errors

This test seems to be flaky and prone to timing issues related to
delivery of dbus messages. This patch makes the waiting for m

sdbusplus:test_message_call: Make test more resilient to timing errors

This test seems to be flaky and prone to timing issues related to
delivery of dbus messages. This patch makes the waiting for message
delivery a bit more resilient by giving giving the test the chance to
check a few times for message delivery before failing the test.

Tested:

Before patch:

```
ninja: Entering directory `/workdir/sdbusplus/build'
ninja: no work to do.
1/100 sdbusplus:test_message_call OK 0.20s
2/100 sdbusplus:test_message_call OK 0.20s
...
30/100 sdbusplus:test_message_call FAIL 0.13s exit status 1
>>> MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 MESON_TEST_ITERATION=62 LD_LIBRARY_PATH=/workdir/sdbusplus/build/ M
ALLOC_PERTURB_=232 UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 /workdir/sdbusplus/build/test/test-message_call

31/100 sdbusplus:test_message_call OK 0.13s
32/100 sdbusplus:test_message_call OK 0.12s
33/100 sdbusplus:test_message_call FAIL 0.12s exit status 1
>>> MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 LD_LIBRARY_PATH=/workdir/sdbusplus/build/ MALLOC_PERTURB_=114 MESON
_TEST_ITERATION=65 UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 /workdir/sdbusplus/build/test/test-message_call
...
95/100 sdbusplus:test_message_call OK 0.35s
96/100 sdbusplus:test_message_call OK 0.36s

Ok: 87
Fail: 9

Full log written to /workdir/sdbusplus/build/meson-logs/testlog.txt
```

With this patch applied, been running a few 10K iteration tests, have
seen no failures so far.

Tags:
Change-Id: I5824b65568bf9544c78645ef9a38f46458ec1cbb
Signed-off-by: Nico Brailovsky <nicobrailovsky@fb.com>

show more ...

(a)server: support object_path overloads

In order to support safer handling of object paths, directly support
overloads for passing `const sdbusplus::message::object_path&` in
addition to the `char*

(a)server: support object_path overloads

In order to support safer handling of object paths, directly support
overloads for passing `const sdbusplus::message::object_path&` in
addition to the `char* path` functions.

Tested:

New unit tests have been added for server and aserver, for both regular
and property-provided constructors.

Change-Id: I63cb16f4cce75f99ebbbd5ced44ef4a219c7a74a
Signed-off-by: Alexander Hansen <alexander.hansen@9elements.com>

show more ...

meson: reformat with meson formatter

Apply the `meson format` results.

Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I3684dcf87ca77bcdfcfb34a4944f40756be90123

clang-tidy: async:context: replace optional access

Add local accessor for the async ctx in the test fixture and switch to
using that, rather than raw access to a std::optional. This resolves
clang-

clang-tidy: async:context: replace optional access

Add local accessor for the async ctx in the test fixture and switch to
using that, rather than raw access to a std::optional. This resolves
clang-tidy warnings for bugprone-unchecked-optional-access.

Change-Id: Iead8fca940d80003dea87199a9df6dfb05305579
Signed-off-by: Jayanth Othayoth <ojayanth@gmail.com>

show more ...

clang-tidy:unpack_properties guard optional access

Replace ASSERT_TRUE checks with explicit `if (!opt.has_value())` guards
and early return to satisfy bugprone-unchecked-optional-access.

Change-Id:

clang-tidy:unpack_properties guard optional access

Replace ASSERT_TRUE checks with explicit `if (!opt.has_value())` guards
and early return to satisfy bugprone-unchecked-optional-access.

Change-Id: I426b869b30d7631fe6ea973a48b4892765785b8f
Signed-off-by: Jayanth Othayoth <ojayanth@gmail.com>

show more ...

clang-tidy: remove unused private fields

Dropped unused members `fd` and `wd` from mutex.cpp to resolve
-Wunused-private-field errors under -Werror. No functional changes.

Fixed below errors.
'''
.

clang-tidy: remove unused private fields

Dropped unused members `fd` and `wd` from mutex.cpp to resolve
-Wunused-private-field errors under -Werror. No functional changes.

Fixed below errors.
'''
../test/async/mutex.cpp:94:9: error: private field 'fd' is not used [-Werror,-Wunused-private-field]
94 | int fd = -1;
| ^
../test/async/mutex.cpp:95:9: error: private field 'wd' is not used [-Werror,-Wunused-private-field]
95 | int wd = -1;
'''

Change-Id: If8079277c6e1be2760de8896997d7da4df6e1a98
Signed-off-by: Jayanth Othayoth <ojayanth@gmail.com>

show more ...

clang-tidy: fix lax exception spec in fixture

Explicitly declare MutexTest destructor as `noexcept override` to match
::testing::Test. This resolves Clang error about exception specification
being m

clang-tidy: fix lax exception spec in fixture

Explicitly declare MutexTest destructor as `noexcept override` to match
::testing::Test. This resolves Clang error about exception specification
being more lax than the base under -Werror. No functional changes.

Fixed below errors
'''
../test/async/mutex.cpp:12:7: error: exception specification of overriding function
is more lax than base version
12 | class MutexTest : public ::testing::Test
'''

Change-Id: I0b3d0775ecdda4408e794ff4220dd564595b54f6
Signed-off-by: Jayanth Othayoth <ojayanth@gmail.com>

show more ...

message: ensure support for flat-map

Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: Id50c1e5a4dbf86a16668f1a51c00a82fd2319c73

use unpack syntax

Rather than defining a variable and then reading it from a message,
we also supports directly unpack-ing from the message. Use this
syntax instead as it is more efficient and succ

use unpack syntax

Rather than defining a variable and then reading it from a message,
we also supports directly unpack-ing from the message. Use this
syntax instead as it is more efficient and succinct.

Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I2ce2aa3e6a43d8866ba9b3920dd3165b7b6b9f6e

show more ...

test: unpack_properties: remove unused boost include

Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I30e8752b4d3a8951dd1352126b8664fa5b8f53aa

test: split up long-running tests

gtests runs testcases sequentially as part of a test suite which is
causing long test runs. Split up the tests into separate files to
optimize test run duration.

T

test: split up long-running tests

gtests runs testcases sequentially as part of a test suite which is
causing long test runs. Split up the tests into separate files to
optimize test run duration.

Test suite definition is done via `suite.hpp`/`suite.cpp`.

There is a tradeoff here since the test folder structure now does not
directly mirror the source folder structure anymore. The folder name
is chosen to clarify which implementation file is being tested.

Timeouts inside testcases are not touched, it is assumed these were
chosen correctly.

In case of fdio_timed there is dependency on a folder, which is made
unique via `getpid()` call to avoid races.

Command: `time ninja -C build test`

Before: (as of current master branch, commit
`c6fee5a94bbb4b4fbb6212942f0f2cfa3049c255`)
```
real 0m18.581s
user 0m9.977s
sys 0m5.494s
```

After:
```
real 0m6.430s
user 0m10.021s
sys 0m5.501s
```

Change-Id: I2ba096cb8f9d8ffcc146448d22b7e75a1c25d103
Signed-off-by: Alexander Hansen <alexander.hansen@9elements.com>

show more ...

expose DBus interface signal names as symbols

Enable a way to access the signal names of a given DBus interface as a
constexpr symbol via the header.

4 use-cases:

- printing error / debug logs wit

expose DBus interface signal names as symbols

Enable a way to access the signal names of a given DBus interface as a
constexpr symbol via the header.

4 use-cases:

- printing error / debug logs with the signal name.
e.g. 'received signal ${SIGNAL_NAME}'

- matching on DBus signals in applications which do not yet use the PDI
generated bindings.

- preventing typos in DBus signal names

- estimating the impact of removing a given DBus signal from an
interface. When using these symbols, it would cause a build failure in
applications relying on the existence of that signal.

This change is similar to [1] and goes into the same direction.

Tested: Newly written unit test passes.

References:
[1] d2571922bfdb4f6b41ba4fbc45b8a4272793fd40

Change-Id: I9c20d744955c883302f349c5351dbdba1753466b
Signed-off-by: Alexander Hansen <alexander.hansen@9elements.com>

show more ...

expose DBus interface method names as symbols

Enable a way to access the method names of a given DBus interface as a
constexpr symbol via the header.

4 use-cases:

- printing error / debug logs wit

expose DBus interface method names as symbols

Enable a way to access the method names of a given DBus interface as a
constexpr symbol via the header.

4 use-cases:

- printing error / debug logs with the method name.
e.g. 'error calling ${METHOD_NAME}'

- accessing DBus methods in applications which do not yet use the PDI
generated bindings.

- preventing typos in DBus method names

- estimating the impact of removing a given DBus method from an
interface. When using these symbols, it would cause a build failure in
applications relying on the existence of that method.

This change is similar to [1] and goes into the same direction.

Tested: Newly written unit test passes.

References:
[1] d2571922bfdb4f6b41ba4fbc45b8a4272793fd40

Change-Id: Id423c3a668dd1a8346040f4380476d5e02468ddb
Signed-off-by: Alexander Hansen <alexander.hansen@9elements.com>

show more ...

expose DBus interface property names as symbols

Enable a way to access the property names of a given DBus interface as a
constexpr symbol via the header [1]

4 use-cases:

- printing error / debug l

expose DBus interface property names as symbols

Enable a way to access the property names of a given DBus interface as a
constexpr symbol via the header [1]

4 use-cases:

- printing error / debug logs with the property name.
e.g. '${property} missing from configuration'

- accessing DBus properties in applications which do not yet use the PDI
generated bindings.

- preventing typos in DBus property names

- estimating the impact of removing a given DBus property from an
interface. When using these symbols, it would cause a build failure in
applications relying on the existence of that property.

Tested: a newly added unit test is using this feature.

References:
[1] https://discordapp.com/channels/775381525260664832/867820390406422538/1423317550732673206

Change-Id: I7b2906b6b1b445c3a49868ff1d50ced6ccaa5336
Signed-off-by: Alexander Hansen <alexander.hansen@9elements.com>

show more ...

add support for async mutex

Add support for async mutex for synchronizing coroutines/tasks using
sender receiver framework. Also, add lock_guard support which provides a
RAII wrapper for mutex to ow

add support for async mutex

Add support for async mutex for synchronizing coroutines/tasks using
sender receiver framework. Also, add lock_guard support which provides a
RAII wrapper for mutex to own it for the duration of a scoped block.

Tested: Add Unit Test using gtest
```
1/1 test_async_mutex OK 2.01s

Ok: 1
Expected Fail: 0
Fail: 0
Unexpected Pass: 0
Skipped: 0
Timeout: 0
```

Change-Id: I691528885a94b9cf55d4b7f2fb0c1e0e6d0ab84f
Signed-off-by: Jagpal Singh Gill <paligill@gmail.com>
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>

show more ...

add support for timed fdio

For certain operations such as reading and writing to a fd for request
response flows, the fd needs to have a timeout to avoid everlasting
hangs in case of a unresponsive

add support for timed fdio

For certain operations such as reading and writing to a fd for request
response flows, the fd needs to have a timeout to avoid everlasting
hangs in case of a unresponsive responder. To support this, the fdio has
been extended to add timeout support by throwing an exception in case of
timeout.

Tested: Unit Test
```
1/1 test_async_fdio_timed OK 10.03s

Ok: 1
Expected Fail: 0
Fail: 0
Unexpected Pass: 0
Skipped: 0
Timeout: 0
```

Change-Id: Ib219e4a4c55125785e6c1571488e445ba3379244
Signed-off-by: Jagpal Singh Gill <paligill@gmail.com>

show more ...

asio: unpack in place

The existing code uses several different tuples to do the unpack, then
concatenates them with tuple_cat. Simplify the code by generating a
tuple of references to the dbus args

asio: unpack in place

The existing code uses several different tuples to do the unpack, then
concatenates them with tuple_cat. Simplify the code by generating a
tuple of references to the dbus args, and unpack directly to the main
struct, thus simplifying the flow.

To accomplish this the make_dbus_args_tuple helper method is added,
which strips the non-dbus arguments from a callback so they can be
unpacked separately. This is relatively complex because this code tries
to support

callback(boost::error_code, message_t, dbus_arg);
as well as
callback(boost::error_code, dbus_arg);

Dynamically dependent on which overload is provided. There's likely
more cleanup that can happen there to separate these two, but this
should at least help.

Tested: OpenBMC launches and runs. Redfish service validator (which
should hit the majority of these overloads) passes.

Change-Id: I6bb7ee960459a505eb8633b1cdb30cd2b3037466
Signed-off-by: Ed Tanous <etanous@nvidia.com>

show more ...

Allow appending std::vector<bool>

std::vector<bool> when done with a for loop returns the type of
std::_bit_reference, which doesn't have a direct conversion to the
underlying boolean type.

Rather

Allow appending std::vector<bool>

std::vector<bool> when done with a for loop returns the type of
std::_bit_reference, which doesn't have a direct conversion to the
underlying boolean type.

Rather than relying on the type returned by begin(), rely on
T::value_type to specifically get the type in the container.

There isn't a particular use for this, but
https://gerrit.openbmc.org/c/openbmc/entity-manager/+/81474

Is trying to consolidate code, and having all types use the same
pack/unpack code is advantageous to avoid special cases. I don't
know of a place we're actually packing/unpacking arrays of bool.

Tested: Booted gb200nvl-bmc. Services launched, no new crashes seen.

Change-Id: I07fc150a190ae44f7bdc90531ad2a4ce7f47e0a1
Signed-off-by: Ed Tanous <etanous@nvidia.com>

show more ...

Remove grouping for append

These APIs are fairly complex. Similar to what was done for the
read_single class of values, simplify these using c++17 template
expressions.

Tested: Booted gb200nvl-bmc

Remove grouping for append

These APIs are fairly complex. Similar to what was done for the
read_single class of values, simplify these using c++17 template
expressions.

Tested: Booted gb200nvl-bmc. Services launched, no new crashes seen.

Change-Id: If2604113efa578062890f11d18ecca7a9f249e9c
Signed-off-by: Ed Tanous <etanous@nvidia.com>

show more ...

watchdog: add support for systemd watchdog

Add interfaces and handling for systemd watchdog petting.

Systemd service files can specify a watchdog timeout and systemd will
expect the application to

watchdog: add support for systemd watchdog

Add interfaces and handling for systemd watchdog petting.

Systemd service files can specify a watchdog timeout and systemd will
expect the application to periodically poke a software watchdog, or
else the service will be restarted. This is enabled with the
'WatchdogSec=' service file directive. Add primitives for
interacting with the watchdog APIs.

Enable automatic support in the `async::context` for this watchdog
handling, such that if the watchdog is required (by checking
sd_watchdog_enabled) the daemon will automatically pet at the
appropriate rate, assuming that the `async::context` is functioning
correctly.

Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I68caf7b2c7166ca402b07ecee3db65f75365aa72

show more ...

Use sd-bus array methods for read and append

Currently sdbusplus uses sd_bus_message_read_basic and
message_append_basic for decoding and encoding of arrays. For large
arrays, this imposes a signif

Use sd-bus array methods for read and append

Currently sdbusplus uses sd_bus_message_read_basic and
message_append_basic for decoding and encoding of arrays. For large
arrays, this imposes a significant overhead in calling the library each
time. This leads to some extreme performance degradation in some cases,
for example, passing a 4K bytes data array over sdbusplus interface now
takes 4096 + 2 calls of sd_bus_message_read_basic and
message_append_basic, this will consume about 10ms CPU time on a Aspeed
2600 platform for each package on both send and receive side.

While in this case, a DBus interface design should likely opt for using
an FD rather than an array of bytes, this isn't a reason to not optimize
this case.

sd-bus, in version 240 added methods to deal with this performance
degradation, namely sd_bus_message_read_array and
sd_bus_message_append_array, which each only require a single call to
load values into the array using pointers and lengths.

This patchset is based on the one submitted here:
https://gerrit.openbmc.org/c/openbmc/sdbusplus/+/68614

But opts for a different approach to a number of technical details on
how it accomplishes the result, including changing the underlying sd-bus
calls used to utilize ones that don't require sdbus internal malloc,
avoiding unit tests ASAN issues in the process.

This commit adds support for utilizing the above methods when doing
calls to the library if the following requirements are met:
1. The container is contiguous (using the std::contiguous_iterator
concept)
2. The container represents a list of values of type int/uint 8,16,32,
or 64 bits wide. Note, per the sd-bus documentation, arrays of bool are
explicitly not supported (presumably because the internal representation
of a bool array for both libc++ and sd-bus is implementation defined).

To accomplish this, the arrays handling for contiguous arrays is moved
to use concepts, simplifying the code significantly, and allowing the
use of std::contiguous_iterator.

Change-Id: I3bd9f97ed160835e8c30c302b80553a1d450bbf9
Signed-off-by: Ed Tanous <ed@tanous.net>
Signed-off-by: Yongbing Chen <yongbingchen@google.com>

show more ...

Prepare for one shot array reading

With the patch changing the behavior for arrays of numbers, we want
these tests to continue testing the iteration array read paths. In the
next commit, std::vecto

Prepare for one shot array reading

With the patch changing the behavior for arrays of numbers, we want
these tests to continue testing the iteration array read paths. In the
next commit, std::vector<uint8_t> will no longer be using iteration
paths, so change the unit test to use strings instead. Next commit will
add back equivalent coverage.

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

show more ...

test: vtable: clean up FIXME

We are now using a much newer version of systemd than the commented
out code implied. Add in the commented-out check.

Signed-off-by: Patrick Williams <patrick@stwcx.xy

test: vtable: clean up FIXME

We are now using a much newer version of systemd than the commented
out code implied. Add in the commented-out check.

Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: Iff144af23f39ce38556123bc27ec20b508c828bb

show more ...

aserver: constructor to initialize properties

Functions like 'emit_added()' do not work correctly if properties are
not initialized. Often, the initial values for the properties of an
interface are

aserver: constructor to initialize properties

Functions like 'emit_added()' do not work correctly if properties are
not initialized. Often, the initial values for the properties of an
interface are known, such as

- Software Version Interface
- Software ActivationProgress Interface
- Software ApplyTime Interface
- ...

How was this previously handled?

- Write a wrapper class for the specific aserver class.
This allows to initialize the properties, as they are protected
members. Example [2]

- Write a wrapper class and provide methods to get/set the variables.
This is done in [1]

- Set each property with a method call, which emits
'PropertyChanged' signal by default, not ideal.
This also does not work if the interface is ever refactored to have
additional properties.

To facilitate using these interfaces without having to write a wrapper
class to inherit from them, and to avoid having to manually set each
property, which can easily be forgotten, provide a constructor to
allow for passing a struct with values for all the properties.

Tested: Unit tests pass, but still needs to be build-tested against
openbmc/openbmc. The concern there is mostly due to an added template
parameter.

References:
[1] https://github.com/openbmc/sdbusplus/blob/da8574d5888b2c1622f5482a47adc7a12ffa0d0e/example/calculator-aserver.cpp#L44
[2] https://github.com/openbmc/phosphor-bmc-code-mgmt/blob/4983b138ea8fc70bd66fe4d30500e6252629fa5d/common/src/software.cpp#L29

Change-Id: I873cbca97ae16b19bfbf622303f4d52e2563a62c
Signed-off-by: Alexander Hansen <alexander.hansen@9elements.com>
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>

show more ...