From 36af84cdbb660953d73804a12dfda056c3ac9b08 Mon Sep 17 00:00:00 2001 From: Rashmica Gupta Date: Wed, 12 Apr 2023 16:16:28 +1000 Subject: [PATCH] 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 --- include/libpldm/base.h | 2 + include/libpldm/meson.build | 1 + include/libpldm/requester/instance-id.h | 92 ++++++++++++ src/requester/instance-id.c | 177 ++++++++++++++++++++++++ src/requester/meson.build | 1 + 5 files changed, 273 insertions(+) create mode 100644 include/libpldm/requester/instance-id.h create mode 100644 src/requester/instance-id.c diff --git a/include/libpldm/base.h b/include/libpldm/base.h index 26e0825..7580efb 100644 --- a/include/libpldm/base.h +++ b/include/libpldm/base.h @@ -11,6 +11,8 @@ extern "C" { #include "pldm_types.h" +typedef uint8_t pldm_tid_t; + /** @brief PLDM Types */ enum pldm_supported_types { diff --git a/include/libpldm/meson.build b/include/libpldm/meson.build index 0d25754..b3da4b2 100644 --- a/include/libpldm/meson.build +++ b/include/libpldm/meson.build @@ -29,6 +29,7 @@ endif if get_option('requester-api').enabled() libpldm_headers += files( + 'requester/instance-id.h', 'requester/pldm.h' ) endif diff --git a/include/libpldm/requester/instance-id.h b/include/libpldm/requester/instance-id.h new file mode 100644 index 0000000..eebaecb --- /dev/null +++ b/include/libpldm/requester/instance-id.h @@ -0,0 +1,92 @@ +#ifndef INSTANCE_ID_H +#define INSTANCE_ID_H + +#ifdef __cplusplus +extern "C" { +#endif + +#include "libpldm/base.h" +#include + +typedef uint8_t pldm_instance_id_t; +struct pldm_instance_db; + +#ifdef __STDC_HOSTED__ +/** + * @brief Instantiates an instance ID database object for a given database path + * + * @param[out] ctx - *ctx must be NULL, and will point to a PLDM instance ID + * database object on success. + * @param[in] dbpath - the path to the instance ID database file to use + * + * @return int - Returns 0 on success. Returns -EINVAL if ctx is NULL or *ctx + * is not NULL. Returns -ENOMEM if memory couldn't be allocated. + * Returns the errno if the database couldn't be opened. + * */ +int pldm_instance_db_init(struct pldm_instance_db **ctx, const char *dbpath); + +/** + * @brief Instantiates an instance ID database object for the default database + * path + * + * @param[out] ctx - *ctx will point to a PLDM instance ID database object on + * success. + * + * @return int - Returns 0 on success. Returns -EINVAL if ctx is NULL or *ctx + * is not NULL. Returns -ENOMEM if memory couldn't be allocated. + * Returns the errno if the database couldn't be opened. + * */ +int pldm_instance_db_init_default(struct pldm_instance_db **ctx); + +/** + * @brief Destroys an instance ID database object + * + * @param[in] ctx - PLDM instance ID database object + * + * @return int - Returns 0 on success or if *ctx is NULL. No specific errors are + * specified. + * */ +int pldm_instance_db_destroy(struct pldm_instance_db *ctx); + +/** + * @brief Allocates an instance ID for a destination TID from the instance ID + * database + * + * @param[in] ctx - PLDM instance ID database object + * @param[in] tid - PLDM TID + * @param[in] iid - caller owned pointer to a PLDM instance ID object. On + * success, this points to an instance ID to use for a PLDM request + * message. + * + * @return int - Returns 0 on success if we were able to allocate an instance + * ID. Returns -EINVAL if the iid pointer is NULL. Returns -EAGAIN + * if a successive call may succeed. Returns -EPROTO if the + * operation has entered an undefined state. + */ +int pldm_instance_id_alloc(struct pldm_instance_db *ctx, pldm_tid_t tid, + pldm_instance_id_t *iid); + +/** + * @brief Frees an instance ID previously allocated by pldm_instance_id_alloc + * + * @param[in] ctx - PLDM instance ID database object + * @param[in] tid - PLDM TID + * @param[in] iid - If this instance ID was not previously allocated by + * pldm_instance_id_alloc then EINVAL is returned. + * + * @return int - Returns 0 on success. Returns -EINVAL if the iid supplied was + * not previously allocated by pldm_instance_id_alloc or it has + * previously been freed. Returns -EAGAIN if a successive call may + * succeed. Returns -EPROTO if the operation has entered an + * undefined state. + */ +int pldm_instance_id_free(struct pldm_instance_db *ctx, pldm_tid_t tid, + pldm_instance_id_t iid); + +#endif /* __STDC_HOSTED__*/ + +#ifdef __cplusplus +} +#endif + +#endif /* INSTANCE_ID_H */ diff --git a/src/requester/instance-id.c b/src/requester/instance-id.c new file mode 100644 index 0000000..e3cf41f --- /dev/null +++ b/src/requester/instance-id.c @@ -0,0 +1,177 @@ +// NOLINTNEXTLINE(bugprone-reserved-identifier,cert-dcl37-c,cert-dcl51-cpp) +#define _GNU_SOURCE +#include "libpldm/requester/instance-id.h" +#include "libpldm/pldm.h" +#include +#include +#include +#include + +#define PLDM_TID_MAX 256 +#define PLDM_INST_ID_MAX 32 + +struct pldm_instance_db { + pldm_instance_id_t prev[PLDM_TID_MAX]; + int lock_db_fd; +}; + +static inline int iid_next(pldm_instance_id_t cur) +{ + return (cur + 1) % PLDM_INST_ID_MAX; +} + +int pldm_instance_db_init(struct pldm_instance_db **ctx, const char *dbpath) +{ + struct pldm_instance_db *l_ctx; + + /* Make sure the provided pointer was initialised to NULL. In the future + * if we stabilise the ABI and expose the struct definition the caller + * can potentially pass a valid pointer to a struct they've allocated + */ + if (!ctx || *ctx) { + return -EINVAL; + } + + l_ctx = calloc(1, sizeof(struct pldm_instance_db)); + if (!l_ctx) { + return -ENOMEM; + } + + /* Initialise previous ID values so the next one is zero */ + for (int i = 0; i < PLDM_TID_MAX; i++) { + l_ctx->prev[i] = 31; + } + + /* Lock database may be read-only, either by permissions or mountpoint + */ + l_ctx->lock_db_fd = open(dbpath, O_RDONLY | O_CLOEXEC); + if (l_ctx->lock_db_fd < 0) { + free(l_ctx); + return -errno; + } + *ctx = l_ctx; + + return 0; +} + +int pldm_instance_db_init_default(struct pldm_instance_db **ctx) +{ + return pldm_instance_db_init(ctx, + "/usr/share/libpldm/instance-db/default"); +} + +int pldm_instance_db_destroy(struct pldm_instance_db *ctx) +{ + if (!ctx) { + return 0; + } + close(ctx->lock_db_fd); + free(ctx); + return 0; +} + +int pldm_instance_id_alloc(struct pldm_instance_db *ctx, pldm_tid_t tid, + pldm_instance_id_t *iid) +{ + static const struct flock cfls = { + .l_type = F_RDLCK, + .l_whence = SEEK_SET, + .l_len = 1, + }; + static const struct flock cflx = { + .l_type = F_WRLCK, + .l_whence = SEEK_SET, + .l_len = 1, + }; + uint8_t l_iid; + + if (!iid) { + return -EINVAL; + } + + l_iid = ctx->prev[tid]; + if (l_iid >= PLDM_INST_ID_MAX) { + return -EPROTO; + } + + while ((l_iid = iid_next(l_iid)) != ctx->prev[tid]) { + struct flock flop; + off_t loff; + int rc; + + /* Derive the instance ID offset in the lock database */ + loff = tid * PLDM_INST_ID_MAX + l_iid; + + /* Reserving the TID's IID. Done via a shared lock */ + flop = cfls; + flop.l_start = loff; + rc = fcntl(ctx->lock_db_fd, F_OFD_SETLK, &flop); + if (rc < 0) { + if (errno == EAGAIN || errno == EINTR) { + return -EAGAIN; + } + return -EPROTO; + } + + /* + * If we *may* promote the lock to exclusive then this IID is + * only reserved by us. This is now our allocated IID. + * + * If we *may not* promote the lock to exclusive then this IID + * is also reserved on another file descriptor. Move on to the + * next IID index. + * + * Note that we cannot actually *perform* the promotion in + * practice because this is prevented by the lock database being + * opened O_RDONLY. + */ + flop = cflx; + flop.l_start = loff; + rc = fcntl(ctx->lock_db_fd, F_OFD_GETLK, &flop); + if (rc < 0) { + if (errno == EAGAIN || errno == EINTR) { + return -EAGAIN; + } + return -EPROTO; + } + + /* F_UNLCK is the type of the lock if we could successfully + * promote it to F_WRLCK */ + if (flop.l_type == F_UNLCK) { + ctx->prev[tid] = l_iid; + *iid = l_iid; + return 0; + } + if (flop.l_type != F_RDLCK) { + return -EPROTO; + } + } + + /* Failed to allocate an IID after a full loop. Make the caller try + * again */ + return -EAGAIN; +} + +int pldm_instance_id_free(struct pldm_instance_db *ctx, pldm_tid_t tid, + pldm_instance_id_t iid) +{ + static const struct flock cflu = { + .l_type = F_UNLCK, + .l_whence = SEEK_SET, + .l_len = 1, + }; + struct flock flop; + int rc; + + flop = cflu; + flop.l_start = tid * PLDM_INST_ID_MAX + iid; + rc = fcntl(ctx->lock_db_fd, F_OFD_SETLK, &flop); + if (rc < 0) { + if (errno == EAGAIN || errno == EINTR) { + return -EAGAIN; + } + return -EPROTO; + } + + return 0; +} diff --git a/src/requester/meson.build b/src/requester/meson.build index 6798d02..663b607 100644 --- a/src/requester/meson.build +++ b/src/requester/meson.build @@ -1,3 +1,4 @@ libpldm_sources += files( + 'instance-id.c', 'pldm.c' )