e984a461 | 07-Sep-2024 |
Andrew Jeffery <andrew@codeconstruct.com.au> |
meson: Format build files with `meson format`
Additionally, add the formatting command to `scripts/pre-submit` so it's run by OpenBMC's CI (via `scripts/run-ci`).
Change-Id: Ifb8fc06106b8cfa9155e98
meson: Format build files with `meson format`
Additionally, add the formatting command to `scripts/pre-submit` so it's run by OpenBMC's CI (via `scripts/run-ci`).
Change-Id: Ifb8fc06106b8cfa9155e986528b769a5ca450b4c Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
860a43d9 | 22-Aug-2024 |
Andrew Jeffery <andrew@codeconstruct.com.au> |
compiler: Provide LIBPLDM_CC_UNUSED
Ensure that we have __has_attribute available, and then further ensure that the unused attribute is provided. Once satisfied, define LIBPDLM_CC_UNUSED and replace
compiler: Provide LIBPLDM_CC_UNUSED
Ensure that we have __has_attribute available, and then further ensure that the unused attribute is provided. Once satisfied, define LIBPDLM_CC_UNUSED and replace raw use of __attribute__((unused)).
Change-Id: I2433039297d5fdedb8b8d99b30e73e4542d9069f Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
2b440d4c | 24-Jul-2024 |
Andrew Jeffery <andrew@codeconstruct.com.au> |
msgbuf: Ensure memmem() is correctly typed and visible where required
To ensure memmem() is visible _GNU_SOURCE needs to be defined early, at least before any system headers are included. Define it
msgbuf: Ensure memmem() is correctly typed and visible where required
To ensure memmem() is visible _GNU_SOURCE needs to be defined early, at least before any system headers are included. Define it in the build flags as clang-tidy will re-order includes based on vibes rather than dependencies. Finally, clean up the remaining compiler warnings by dropping the unnecessary casts.
Note that _GNU_SOURCE implies _DEFAULT_SOURCE, so we drop the latter:
> Since glibc 2.19, defining _GNU_SOURCE also has the effect of > implicitly defining _DEFAULT_SOURCE. Before glibc 2.20, defining > _GNU_SOURCE also had the effect of implicitly defining _BSD_SOURCE > and _SVID_SOURCE.
https://www.man7.org/linux/man-pages/man7/feature_test_macros.7.html
Fixes: #12 Fixes: 1523778d2739 ("msgbuf: Add pldm_msgbuf_span_string_utf16()") Change-Id: I9206f7616740790a89366762cce11d3045471b97 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
e4240679 | 28-Jun-2024 |
Andrew Jeffery <andrew@codeconstruct.com.au> |
requester: instance-id: Release read lock on conflict
36af84cdbb66 ("requester: Add new APIs for instance ID allocation and freeing") introduced the new instance ID allocation APIs, and some unbalan
requester: instance-id: Release read lock on conflict
36af84cdbb66 ("requester: Add new APIs for instance ID allocation and freeing") introduced the new instance ID allocation APIs, and some unbalanced locking along with it. When a conflict arose on an instance ID, the read lock was not released by the non-owning caller.
Release the lock on conflict and on error, and add a test case to prevent regression.
gitlint-ignore: UC1, B1 Fixes: 36af84cdbb66 ("requester: Add new APIs for instance ID allocation and freeing") Reported-by: Jerry Chen <jerry_c_chen@wiwynn.com> Change-Id: Iecd1583c6b8863b458cc4fbf1ac42b20ca2a3433 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
efb40069 | 09-Nov-2023 |
Andrew Jeffery <andrew@codeconstruct.com.au> |
libpldm: More fixes for header use
b0c1d20a5bde ("libpldm: Fix header use") only did half the job, addressing compilation concerns for the library itself. As it turns out there were still plenty of
libpldm: More fixes for header use
b0c1d20a5bde ("libpldm: Fix header use") only did half the job, addressing compilation concerns for the library itself. As it turns out there were still plenty of places that used local includes for paths to the public headers.
Apply the rest of the header cleanups to hopefully get us into a consistent state.
Fixes: b0c1d20a5bde ("libpldm: Fix header use") Change-Id: I8d4b59c9241770583abb30389452af0a32b18b99 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
d12dd36e | 09-Nov-2023 |
Andrew Jeffery <andrew@codeconstruct.com.au> |
libpldm: Rationalise the local and installed path of pldm.h
Since chopping libpldm out of openbmc/pldm.git there's been a symlink from `include/libpldm/pldm.h` pointing to `include/libpldm/requester
libpldm: Rationalise the local and installed path of pldm.h
Since chopping libpldm out of openbmc/pldm.git there's been a symlink from `include/libpldm/pldm.h` pointing to `include/libpldm/requester/pldm.h`. The file list contained in the `libpldm_headers` variable defined by `include/libpldm/meson.build` contained an entry for `requester/pldm.h`, though not the symlink found at `include/libpldm/pldm.h`.
Prior to a7989cd65d51 ("meson: Fix for OEM header collision issue") `install_headers(libpldm_headers, ...)` directive didn't specify `preserve_path: true`, therefore the `requester/pldm.h` header file was installed directly under `${INCLUDEDIR}/libpldm` as `${INCLUDEDIR}/libpldm/pldm.h`, and no file was installed to `${INCLUDEDIR}/libpldm/requester/pldm.h`.
All dependent applications using the declarations in `pldm.h` therefore included the header using the directive `#include <libpldm/pldm.h>`. However, internal to the libpldm implementation we were using the non-symlinked path in the local tree: `#include "libpldm/requester/pldm.h"`
With a7989cd65d51 ("meson: Fix for OEM header collision issue") we rationalised the meson variables used to install the headers as part of properly separating OEM headers from one-another. This lead to specifying `preserve_path: true` for the `install_headers(libpldm_headers, ...)` meson directive, which further lead to the installed location of `pldm.h` changing to `${INCLUDEDIR}/libpldm/requester/pldm.h`. This broke all consuming applications which were necessarily using `#include <libpldm/pldm.h>` as outlined above.
Get rid of the symlink at `include/libpldm/pldm.h`, move `include/libpldm/requester/pldm.h` there instead, and fix up all internal references to the header. This unbreaks applications consuming libpldm, and prevents internal inconsistencies going forward.
Fixes: a7989cd65d51 ("meson: Fix for OEM header collision issue") Change-Id: I24f1e25303890c41fd3da6323c67d838022cc55d Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
b0c1d20a | 07-Nov-2023 |
Andrew Jeffery <andrew@codeconstruct.com.au> |
libpldm: Fix header use
The headers need to work whether we're building libpldm in the repo or we're building another project depending on the headers in the system include directory.
Tidy up the p
libpldm: Fix header use
The headers need to work whether we're building libpldm in the repo or we're building another project depending on the headers in the system include directory.
Tidy up the paths involved and switch to defining the public headers as system headers for the purpose of the build.
Change-Id: I49413988c94d393ea5761bc4684edcd2c2482a98 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
show more ...
|
691668fe | 01-Nov-2023 |
Patrick Williams <patrick@stwcx.xyz> |
license: add spdx identifier to all files
Signed-off-by: Patrick Williams <patrick@stwcx.xyz> Change-Id: Ifddd07cc0eb5edeb2dcb410747073d68c6631cb1 |
0a6d6821 | 22-Aug-2023 |
Andrew Jeffery <andrew@aj.id.au> |
transport: Stabilise core transport and implementation APIs
The following two patches demonstrate their use in pldmd:
1. https://gerrit.openbmc.org/c/openbmc/pldm/+/63652 2. https://gerrit.openbmc.
transport: Stabilise core transport and implementation APIs
The following two patches demonstrate their use in pldmd:
1. https://gerrit.openbmc.org/c/openbmc/pldm/+/63652 2. https://gerrit.openbmc.org/c/openbmc/pldm/+/65676
In the process, deprecate the old requester APIs. These are unsuitable for OpenBMC going forward, which will rely on AF_MCTP sockets for the MCTP transport implementation.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Change-Id: I94a9d9ff5fc67d5d6cf9a2393ebef12fa7b277d7
show more ...
|
24576290 | 30-Jul-2023 |
Rashmica Gupta <rashmica@linux.ibm.com> |
transport: Generalise the pldm_transport_recv_msg() API
Currently pldm_transport_recv_msg() only works for requesters as the TID param is an input. Responders need the source TID of the message rece
transport: Generalise the pldm_transport_recv_msg() API
Currently pldm_transport_recv_msg() only works for requesters as the TID param is an input. Responders need the source TID of the message received so they know where to send the response.
The TID was being used to look up the EID mapped to the TID and failing the function call if it didn't match. This check doesn't need to happen at this level, and can be added in at the requester API level if required.
Make the TID param an output, and use the EID of the message to lookup the TID.
Change-Id: I671dbfe2d94a9ad8d77ea0ef150f1c744f928c53 Signed-off-by: Rashmica Gupta <rashmica@linux.ibm.com>
show more ...
|
d10c6b0c | 23-Jul-2023 |
Rashmica Gupta <rashmica@linux.ibm.com> |
requester: Use the EID as the TID when we don't have the TID
We are doing this in other places, so for consistencies sake do it here too.
Signed-off-by: Rashmica Gupta <rashmica@linux.ibm.com> Chan
requester: Use the EID as the TID when we don't have the TID
We are doing this in other places, so for consistencies sake do it here too.
Signed-off-by: Rashmica Gupta <rashmica@linux.ibm.com> Change-Id: I94935561dd3871ba3b0b179b4311fafcb3bcb1b4
show more ...
|
43a7985d | 17-Jul-2023 |
Thu Nguyen <thu@os.amperecomputing.com> |
requester: Fix response buffer cast in pldm_send_recv()
With the latest libpldm code version 0.4, sometimes the calling `pldmtool platform GetPDR` command while polling the sensors will be ended wit
requester: Fix response buffer cast in pldm_send_recv()
With the latest libpldm code version 0.4, sometimes the calling `pldmtool platform GetPDR` command while polling the sensors will be ended with the errors `Failed to receive RC = 3`. This error code is printed when the `pldm_send_recv()` responses PLDM_REQUESTER_NOT_RESP_MSG. In the `pldm_send_recv()`, `hdr` is `struct pldm_msg_hdr` pointer and `pldm_resp_msg` is a double pointer, type casting the double pointer to the pointer is incorrect.
Tested: 1. Call `pldmtool platform GetPDR` command while running the sensor reading. 2. No `Failed to receive RC = 3`
Fixes: 0411b712b746 ("transport: Make APIs work for all types of messages") Signed-off-by: Thu Nguyen <thu@os.amperecomputing.com> Change-Id: I1efadc80bbd8803d0b97cb634eb8bd7df9d279b9
show more ...
|
4e1ba8a7 | 28-Jun-2023 |
Andrew Jeffery <andrew@aj.id.au> |
requester: Return PLDM_REQUESTER_OPEN_FAIL from pldm_open() on error
As it stood the reimplementation of pldm_open() passed back the return value of the pldm_transport_mctp_demux_*() APIs, which don
requester: Return PLDM_REQUESTER_OPEN_FAIL from pldm_open() on error
As it stood the reimplementation of pldm_open() passed back the return value of the pldm_transport_mctp_demux_*() APIs, which don't align with the specified behaviour of pldm_open()'s return values.
Rework the return values such that PLDM_REQUESTER_OPEN_FAIL is always returned on error. This fixes error handling in at least openpower-occ-control, which only tested for that value and considered all other values as success.
Further, handle any external close(2) of the returned file descriptor. This again caters to openpower-occ-control which issues close() in its response handler.
Fixes: 39f883259956 ("requester: Make pldm_open() return existing fd") Fixes: c1b66f420912 ("requester: Add new APIs to support multiple transports") Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Change-Id: I7144f6ecf0fdfbbc3a2a418a651207c012e0db54
show more ...
|
986df2a1 | 28-Jun-2023 |
Andrew Jeffery <andrew@aj.id.au> |
requester: Mark pldm_close() as LIBPLDM_ABI_TESTING
pldm_close() exists in the implementation but isn't exposed in the corresponding header. As there should be no current users of it, take the oppor
requester: Mark pldm_close() as LIBPLDM_ABI_TESTING
pldm_close() exists in the implementation but isn't exposed in the corresponding header. As there should be no current users of it, take the opportunity to remove it from the stable ABI.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Change-Id: I11f50a8bd39d7113bf97bc32a34d0556233afa14
show more ...
|
f89befe3 | 15-Jun-2023 |
Andrew Jeffery <andrew@aj.id.au> |
meson: Force inclusion of config.h via `-include`
Make sure there's no ambiguity about which config.h is used for includes in the event that libpldm acquires subprojects, or is used as a subproject
meson: Force inclusion of config.h via `-include`
Make sure there's no ambiguity about which config.h is used for includes in the event that libpldm acquires subprojects, or is used as a subproject itself.
Tested: `meson compile -C build` succeeds
Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Change-Id: I08c3bdc807cc268d2401e06f0bfaee07f89ba534
show more ...
|
0411b712 | 30-May-2023 |
Rashmica Gupta <rashmica@linux.ibm.com> |
transport: Make APIs work for all types of messages
These transport APIs were written with the idea of being used by a requester. However they don't need to be, so remove these checks so we can send
transport: Make APIs work for all types of messages
These transport APIs were written with the idea of being used by a requester. However they don't need to be, so remove these checks so we can send and receive both requests and responses.
Put the checks into the old API calls so we don't break anyone.
Change-Id: I194e22341025a00388fa7e031d2c6db05976874f Signed-off-by: Rashmica Gupta <rashmica@linux.ibm.com>
show more ...
|
a3035938 | 16-Jun-2023 |
Rashmica Gupta <rashmica@linux.ibm.com> |
requester: Add check before accessing hdr in pldm_recv()
Fixes "pldmSoftPowerOff.service: Main process exited, code=dumped, status=11/SEGV".
Signed-off-by: Rashmica Gupta <rashmica@linux.ibm.com> C
requester: Add check before accessing hdr in pldm_recv()
Fixes "pldmSoftPowerOff.service: Main process exited, code=dumped, status=11/SEGV".
Signed-off-by: Rashmica Gupta <rashmica@linux.ibm.com> Change-Id: I198b1bac8c4c849cb1b8756f070fd5aa62c24f24
show more ...
|
9d2a1c6a | 04-Jun-2023 |
Andrew Jeffery <andrew@aj.id.au> |
libpldm: Explicit deprecated, stable and testing ABI classes
Experimenting with new APIs is important, but ABI stability of the library is also important. We wish to have the freedom to add APIs wit
libpldm: Explicit deprecated, stable and testing ABI classes
Experimenting with new APIs is important, but ABI stability of the library is also important. We wish to have the freedom to add APIs without being burdened by them being immediately set in stone.
We implement this wish by introducing three classes of ABI:
1. deprecated 2. stable 3. testing
These are enforced by corresponding function attributes:
1. LIBPLDM_ABI_DEPRECATED 2. LIBPLDM_ABI_STABLE 3. LIBPLDM_ABI_TESTING
Symbol visibility in the library is flipped to 'hidden' by default, so one of these annotations must be used for the symbol to be exposed.
With these classes in place there are now clear points in time at which we update the ABI dumps captured under the abi/ directory: When an API is migrated from the 'testing' class to the 'stable' class, or when removed from the 'deprecated' class.
Which classes of functions are exposed by the build is controlled by the new 'abi' meson option. The option is of array type which contains the list of ABI classes the build should consider. It defaults to enabling all classes to provide test coverage in CI. The classes used should be constrained to deprecated and stable (and not test) in any dependent projects.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Change-Id: I25402e20c7be9c9f264f9ccd7ac36b384823734c
show more ...
|
39f88325 | 12-May-2023 |
Rashmica Gupta <rashmica@linux.ibm.com> |
requester: Make pldm_open() return existing fd
Considering how this is used, it makes more sense to return the fd of the open transport rather than error out.
Fixes: c1b66f420912 ("requester: Add n
requester: Make pldm_open() return existing fd
Considering how this is used, it makes more sense to return the fd of the open transport rather than error out.
Fixes: c1b66f420912 ("requester: Add new APIs to support multiple transports") Change-Id: Ibff526a013d0f4000217d8b0b1004ff51814d820 Signed-off-by: Rashmica Gupta <rashmica@linux.ibm.com>
show more ...
|
37dd6a3d | 12-May-2023 |
Andrew Jeffery <andrew@aj.id.au> |
clang-format: copy latest and re-format
clang-format-16 has some backwards incompatible changes that require additional settings for best compatibility and re-running the formatter. Copy the latest
clang-format: copy latest and re-format
clang-format-16 has some backwards incompatible changes that require additional settings for best compatibility and re-running the formatter. Copy the latest .clang-format from the docs repository[1] and reformat the repository.
[1] https://gerrit.openbmc.org/c/openbmc/docs/+/63441
Further, shift the fixup for C's `_Static_assert` into src/msgbuf.h to prevent a clang-tidy-16 error:
``` /data0/jenkins/workspace/ci-repository/openbmc/libpldm/src/msgbuf.h:315:2: error: '_Static_assert' is a C11 extension [clang-diagnostic-c11-extensions,-warnings-as-errors] _Static_assert(sizeof(*dst) == sizeof(ldst), ^ ```
And fix up the function prototype in the definition of `pldm_open()`:
``` ../src/requester/pldm.c:128:16: error: a function declaration without a prototype is deprecated in all versions of C [clang-diagnostic-strict-prototypes,-warnings-as-errors] void pldm_close() ^ void ```
Change-Id: I57b53f51914e39237e733d024e62ab41b3d306c1 Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
show more ...
|
a6f0cf3e | 23-Apr-2023 |
Andrew Jeffery <andrew@aj.id.au> |
instance-id: Ensure database is appropriately sized
Fail initialisation if we know that we may fail to satisfy valid allocations.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Change-Id: Iff38b27
instance-id: Ensure database is appropriately sized
Fail initialisation if we know that we may fail to satisfy valid allocations.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Change-Id: Iff38b27fd324da57a1d81b6ad3c5951e369ce3b3
show more ...
|
0aea707b | 23-Apr-2023 |
Andrew Jeffery <andrew@aj.id.au> |
instance-id: Track existing allocations for each db instance
OFD locking allows merging and splitting of locks, which means that it won't fail an attempt to "recursively" lock the range. We were rel
instance-id: Track existing allocations for each db instance
OFD locking allows merging and splitting of locks, which means that it won't fail an attempt to "recursively" lock the range. We were relying on that to prevent double allocation when the available instance IDs for a TID were exhausted.
Given that we can't rely on it, explicitly track the allocations in the local state.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Change-Id: Ic0f6903935ac712a86a7967d1cceb5a0c463878b
show more ...
|
a918a628 | 21-Apr-2023 |
Andrew Jeffery <andrew@aj.id.au> |
include: Move instance-id.h under libpldm/
The installed location for the header is `${includedir}/libpldm/instance-id.h`. Make the in-tree location equivalent so we don't have a slightly jarring in
include: Move instance-id.h under libpldm/
The installed location for the header is `${includedir}/libpldm/instance-id.h`. Make the in-tree location equivalent so we don't have a slightly jarring inconsistency.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Change-Id: I9c1f324e18ba504261845187e88e35db0d8ef873
show more ...
|
c1b66f42 | 08-Dec-2022 |
Rashmica Gupta <rashmica@linux.ibm.com> |
requester: Add new APIs to support multiple transports
This patch has two goals: (1) enable consumers to send PLDM messages over different transports and (2) do this in a way that allows us to move
requester: Add new APIs to support multiple transports
This patch has two goals: (1) enable consumers to send PLDM messages over different transports and (2) do this in a way that allows us to move towards a cleaner and more complete set of requester APIs.
The sole transport option of MCTP via the userspace mctp-demux-daemon is being deprecated. New transports are being added: MCTP via the kernel (AF_MCTP) and eventually NC-SI. As such, the requester APIs need updating to support multiple transports, as well as not having MCTP specific details in the APIs. To avoid a flag day, the current APIs (pldm_send, etc) have been rewritten terms of the new APIs.
The current APIs operate at the transport level - they don't implement all of the behaviour necessary for a requester. As such, the new APIs to send/recv a message have the prefix `pldm_transport`, rather than `pldm_requester`. Given the level that these APIs are operating at, these only send and receive a PLDM message. Any additional logic, such as looking for a response with a particular instance ID, belongs at the requester abstraction level.
Some of the missing behaviours to fully be a PLDM requester are: assigning instance IDs, request retransmission, implementing timeouts, and enforcing only one PLDM request to a specific TID at a time. These things are currently implemented in pldmd, meaning any consumer other than pldmd using the libpldm "requester" APIs doesn't get the full functionality of a requester and has to implement these things themselves.
We would like to eventually move these behaviours into libpldm so the libpldm requester APIs actually implement what is required to be a PLDM requester.
The next steps to add in a full set of requester APIs, while enabling the use of multiple transports looks something like this:
1) add instance id APIs into libpldm. 2) convert pldmd to use libpldm instance id APIs - so all users of PLDM are still using the same instance id allocation method. 3) convert all consumers of libpldm over to using the new libpldm APIs, including the instance id functions. 4) add in the AF_MCTP transport and move consumers over to it. 5) refactor the encode/decode functions to only encode/decode and not frame the message (ie, remove the instance id from these functions) 6) add additional requester functionality into libpldm, and have these use the `pldm_transport` APIs directly. 7) move consumers over to the new `pldm_requester` APIs. 8) remove unused code from pldmd.
Signed-off-by: Rashmica Gupta <rashmica@linux.ibm.com> Change-Id: I06e602831f360bbd0efda53d410bfb5080b3100d
show more ...
|
36af84cd | 12-Apr-2023 |
Rashmica Gupta <rashmica@linux.ibm.com> |
requester: Add new APIs for instance ID allocation and freeing
This patch starts the move of instance id handling into libpldm. Currently pldmd allocates instance ids, and exposes a DBus call for ex
requester: Add new APIs for instance ID allocation and freeing
This patch starts the move of instance id handling into libpldm. Currently pldmd allocates instance ids, and exposes a DBus call for external apps to get instance ids. It relies on exploiting a behaviour of mctp-demux to reclaim used ids by monitoring all incoming PLDM responses. When moving to AF_MCTP, PLDM messages are not broadcast to all PLDM sockets, and so this method of reclaiming ids won't work automatically.
As instance id handling is moving into libpldm eventually, we may as well do that now rather than adding an additional step to address this issue. So an allocate and free function for instance ids is added in this patch. As this implementation uses file locking, we have the feature of any ids belonging to a process that dies being automatically reclaimed into the id pool.
Some context behind the file based range locking design for instance IDs can be found here: https://amboar.github.io/notes/2023/03/28/motivating-a-new-scheme-for-pldm-instance-id-management-in-openbmc.html and https://amboar.github.io/notes/2023/03/29/a-global-pldm-instance-id-allocator-for-libpldm.html
Change-Id: Ia19870b1bcae9e614bda6e475b290faa0b327a73 Signed-off-by: Rashmica Gupta <rashmica@linux.ibm.com>
show more ...
|