Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion protocol/authz_record_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ TEST_F(LibHothTest, authz_erase_test) {
.WillOnce(DoAll(CopyResp(&dummy, 0), Return(LIBHOTH_ERR_TIMEOUT)));

EXPECT_EQ(libhoth_authz_record_erase(&hoth_dev_),
LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_CMD_EXEC, HOTH_HOST_SPACE_LIBHOTH,
LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_NONE, HOTH_HOST_SPACE_LIBHOTH,
LIBHOTH_ERR_TIMEOUT));
}

Expand Down
21 changes: 11 additions & 10 deletions protocol/host_cmd.c
Original file line number Diff line number Diff line change
Expand Up @@ -224,24 +224,25 @@ libhoth_error libhoth_hostcmd_exec_v2(struct libhoth_device* dev,
return LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_CMD_EXEC, HOTH_HOST_SPACE_POSIX,
-status);
}
status = libhoth_send_request(dev, &req, sizeof(req.hdr) + req_payload_size);
if (status != LIBHOTH_OK) {
fprintf(stderr, "libhoth_send_request() failed: %d\n", status);
return LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_CMD_EXEC, HOTH_HOST_SPACE_LIBHOTH,
status);
libhoth_error err =
libhoth_send_request_v2(dev, &req, sizeof(req.hdr) + req_payload_size);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason for using the libhoth_send_request_v2 here?

I would expect we could just implement the v2 callbacks for each of the transports, then just delete the originals and change the helper interface.

Maybe I'm not understanding the transition for this?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stevenportley This is Gemini response. I think she worded better than I did. lert me know if this makes sense. ```
"That is a very fair point, and in a single-repository project, changing the helper interface directly would indeed be the cleanest approach!

│ The reason we are introducing libhoth_send_request_v2 alongside the original is to prevent breaking compilation for external consumers (like
│ gbmc_havend in the BMC repo and hsm_usb_interface in the Haven HSM repo) which depend on libhoth but live in separate repositories.

│ If we changed the signature of the existing libhoth_send_request directly to return libhoth_error , it would immediately break those external
│ builds the next time they update their libhoth dependency.

│ By introducing the _v2 helpers, we allow those external teams to update libhoth safely, migrate their code to the V2 APIs at their own pace, and
│ once everyone is fully migrated, we will submit a cleanup PR to remove the legacy V1 helpers and rename these back to the clean interface."

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this implies that we :

  • add the v2 helpers
  • change everything using libhoth to use the v2 helpers
  • change the original to the new signature
  • change everything using libhoth back to original
  • delete the v2 helpers.

If this were a heavily used API and the signature change was complicated, I would be tempted to go with this, but the signature change is just int -> uint64_t for the return type. This is only used in one or two places (from what I can tell), and most uses just compare the return type to LIBHOTH_OK.

Maybe try just making the signature change and seeing if it compiles okay with hothd before doing all of this?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awh thanks for clarifying the external user's use-cases. I think this would be an simplified approach that Ill look into.

if (err != HOTH_SUCCESS) {
fprintf(stderr, "libhoth_send_request_v2() failed: 0x%016llx\n",
(unsigned long long)err);
return err;
}
struct {
struct hoth_host_response hdr;
uint8_t
payload_buf[LIBHOTH_MAILBOX_SIZE - sizeof(struct hoth_host_response)];
} resp;
size_t resp_size = 0;
status = libhoth_receive_response(dev, &resp, sizeof(resp), &resp_size,
err = libhoth_receive_response_v2(dev, &resp, sizeof(resp), &resp_size,
HOTH_CMD_TIMEOUT_MS_DEFAULT);
if (status != LIBHOTH_OK) {
fprintf(stderr, "libhoth_receive_response() failed: %d\n", status);
return LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_CMD_EXEC, HOTH_HOST_SPACE_LIBHOTH,
status);
if (err != HOTH_SUCCESS) {
fprintf(stderr, "libhoth_receive_response_v2() failed: 0x%016llx\n",
(unsigned long long)err);
return err;
}
status = validate_ec_response_header(&resp.hdr, resp.payload_buf, resp_size);
if (status != 0) {
Expand Down
1 change: 1 addition & 0 deletions protocol/test/libhoth_device_mock.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ static int reconnect(struct libhoth_device* dev) {
}

LibHothTest::LibHothTest() {
std::memset(&hoth_dev_, 0, sizeof(hoth_dev_));
hoth_dev_.user_ctx = &mock_;
hoth_dev_.send = send;
hoth_dev_.receive = receive;
Expand Down
3 changes: 3 additions & 0 deletions transports/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ cc_library(
name = "libhoth_device",
srcs = ["libhoth_device.c"],
hdrs = ["libhoth_device.h"],
deps = [
"//protocol:libhoth_status",
],
)

cc_library(
Expand Down
219 changes: 193 additions & 26 deletions transports/libhoth_device.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,46 @@

#include "libhoth_device.h"

static libhoth_error libhoth_error_from_legacy(uint16_t context, int status) {
if (status == 0) {
return HOTH_SUCCESS;
}
if (status < 0) {
return LIBHOTH_ERR_CONSTRUCT(context, HOTH_HOST_SPACE_POSIX, -status);
}
return LIBHOTH_ERR_CONSTRUCT(context, HOTH_HOST_SPACE_LIBHOTH,
(uint32_t)status);
}

static int libhoth_error_to_legacy(libhoth_error err) {
if (err == HOTH_SUCCESS) {
return 0;
}
uint32_t space = LIBHOTH_ERR_GET_SPACE(err);
uint32_t code = LIBHOTH_ERR_GET_CODE(err);

if (space == HOTH_HOST_SPACE_LIBHOTH) {
return (int)code;
}
if (space == HOTH_HOST_SPACE_POSIX || space == HOTH_HOST_SPACE_LIBUSB) {
return -(int)code;
}
return LIBHOTH_ERR_FAIL;
}

int libhoth_send_request(struct libhoth_device* dev, const void* request,
size_t request_size) {
if (dev == NULL) {
return LIBHOTH_ERR_INVALID_PARAMETER;
}
return dev->send(dev, request, request_size);
if (dev->send != NULL) {
return dev->send(dev, request, request_size);
}
if (dev->send_v2 != NULL) {
libhoth_error err = dev->send_v2(dev, request, request_size);
return libhoth_error_to_legacy(err);
}
return LIBHOTH_ERR_FAIL;
}

int libhoth_receive_response(struct libhoth_device* dev, void* response,
Expand All @@ -35,36 +69,150 @@ int libhoth_receive_response(struct libhoth_device* dev, void* response,
if (dev == NULL) {
return LIBHOTH_ERR_INVALID_PARAMETER;
}
return dev->receive(dev, response, max_response_size, actual_size,
timeout_ms);
if (dev->receive != NULL) {
return dev->receive(dev, response, max_response_size, actual_size,
timeout_ms);
}
if (dev->receive_v2 != NULL) {
libhoth_error err = dev->receive_v2(dev, response, max_response_size,
actual_size, timeout_ms);
return libhoth_error_to_legacy(err);
}
return LIBHOTH_ERR_FAIL;
}

int libhoth_device_reconnect(struct libhoth_device* dev) {
libhoth_error libhoth_send_request_v2(struct libhoth_device* dev,
const void* request,
size_t request_size) {
if (dev == NULL) {
return LIBHOTH_ERR_INVALID_PARAMETER;
return LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_INIT, HOTH_HOST_SPACE_LIBHOTH,
LIBHOTH_ERR_INVALID_PARAMETER);
}
if (dev->send_v2 != NULL) {
return dev->send_v2(dev, request, request_size);
}
if (dev->send != NULL) {
int status = dev->send(dev, request, request_size);
return libhoth_error_from_legacy(HOTH_CTX_NONE, status);
}
return LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_INIT, HOTH_HOST_SPACE_LIBHOTH,
LIBHOTH_ERR_FAIL);
}

if (dev->reconnect == NULL) {
return LIBHOTH_ERR_UNSUPPORTED_VERSION;
libhoth_error libhoth_receive_response_v2(struct libhoth_device* dev,
void* response,
size_t max_response_size,
size_t* actual_size, int timeout_ms) {
if (dev == NULL) {
return LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_INIT, HOTH_HOST_SPACE_LIBHOTH,
LIBHOTH_ERR_INVALID_PARAMETER);
}
if (dev->receive_v2 != NULL) {
return dev->receive_v2(dev, response, max_response_size, actual_size,
timeout_ms);
}
if (dev->receive != NULL) {
int status =
dev->receive(dev, response, max_response_size, actual_size, timeout_ms);
return libhoth_error_from_legacy(HOTH_CTX_NONE, status);
}
return LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_INIT, HOTH_HOST_SPACE_LIBHOTH,
LIBHOTH_ERR_FAIL);
}

return dev->reconnect(dev);
int libhoth_device_reconnect(struct libhoth_device* dev) {
if (dev == NULL) {
return LIBHOTH_ERR_INVALID_PARAMETER;
}
if (dev->reconnect != NULL) {
return dev->reconnect(dev);
}
if (dev->reconnect_v2 != NULL) {
libhoth_error err = dev->reconnect_v2(dev);
return libhoth_error_to_legacy(err);
}
return LIBHOTH_ERR_UNSUPPORTED_VERSION;
}

int libhoth_device_close(struct libhoth_device* dev) {
if (dev == NULL) {
return LIBHOTH_ERR_INVALID_PARAMETER;
}

int status = dev->close(dev);
int status = 0;
if (dev->close != NULL) {
status = dev->close(dev);
} else if (dev->close_v2 != NULL) {
libhoth_error err = dev->close_v2(dev);
status = libhoth_error_to_legacy(err);
} else {
status = LIBHOTH_ERR_FAIL;
}
free(dev);
return status;
}

libhoth_error libhoth_device_reconnect_v2(struct libhoth_device* dev) {
if (dev == NULL) {
return LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_INIT, HOTH_HOST_SPACE_LIBHOTH,
LIBHOTH_ERR_INVALID_PARAMETER);
}
if (dev->reconnect_v2 != NULL) {
return dev->reconnect_v2(dev);
}
if (dev->reconnect != NULL) {
int status = dev->reconnect(dev);
return libhoth_error_from_legacy(HOTH_CTX_NONE, status);
}
return LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_INIT, HOTH_HOST_SPACE_LIBHOTH,
LIBHOTH_ERR_UNSUPPORTED_VERSION);
}

libhoth_error libhoth_device_close_v2(struct libhoth_device* dev) {
if (dev == NULL) {
return LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_INIT, HOTH_HOST_SPACE_LIBHOTH,
LIBHOTH_ERR_INVALID_PARAMETER);
}
libhoth_error err = HOTH_SUCCESS;
if (dev->close_v2 != NULL) {
err = dev->close_v2(dev);
} else if (dev->close != NULL) {
int status = dev->close(dev);
err = libhoth_error_from_legacy(HOTH_CTX_NONE, status);
}
free(dev);
return err;
}

int libhoth_claim_device(struct libhoth_device* dev, uint32_t timeout_us) {
if (dev == NULL) {
return LIBHOTH_ERR_INVALID_PARAMETER;
}
libhoth_error err = libhoth_claim_device_v2(dev, timeout_us);
return libhoth_error_to_legacy(err);
}

int libhoth_release_device(struct libhoth_device* dev) {
if (dev == NULL) {
return LIBHOTH_ERR_INVALID_PARAMETER;
}
if (dev->release != NULL) {
return dev->release(dev);
}
if (dev->release_v2 != NULL) {
libhoth_error err = dev->release_v2(dev);
return libhoth_error_to_legacy(err);
}
return LIBHOTH_ERR_FAIL;
}

libhoth_error libhoth_claim_device_v2(struct libhoth_device* dev,
uint32_t timeout_us) {
if (dev == NULL) {
return LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_INIT, HOTH_HOST_SPACE_LIBHOTH,
LIBHOTH_ERR_INVALID_PARAMETER);
}

enum {
// The maximum time to sleep per attempt.
// Limited by `usleep()` to <1 second.
MAX_SINGLE_SLEEP_US = 1000 * 1000 - 1,
BACKOFF_FACTOR = 2,
INITIAL_WAIT_US = 10 * 1000,
Expand All @@ -74,43 +222,62 @@ int libhoth_claim_device(struct libhoth_device* dev, uint32_t timeout_us) {
uint32_t total_waiting_us = 0;

while (true) {
int status = dev->claim(dev);
libhoth_error err = HOTH_SUCCESS;
if (dev->claim_v2 != NULL) {
err = dev->claim_v2(dev);
} else if (dev->claim != NULL) {
int status = dev->claim(dev);
err = libhoth_error_from_legacy(HOTH_CTX_NONE, status);
} else {
err = LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_INIT, HOTH_HOST_SPACE_LIBHOTH,
LIBHOTH_ERR_FAIL);
}

if (status != LIBHOTH_ERR_INTERFACE_BUSY) {
// We either claimed the device or encountered an unexpected error. Let
// the caller know.
return status;
uint32_t space = LIBHOTH_ERR_GET_SPACE(err);
uint32_t code = LIBHOTH_ERR_GET_CODE(err);
if (err == HOTH_SUCCESS || space != HOTH_HOST_SPACE_LIBHOTH ||
code != LIBHOTH_ERR_INTERFACE_BUSY) {
return err;
}

if (total_waiting_us >= timeout_us) {
// We've exhausted our waiting budget. We couldn't claim the device
// within the configured timeout.
fprintf(stderr, "libhoth: timed out claiming transport after %dus\n",
timeout_us);
return LIBHOTH_ERR_INTERFACE_BUSY;
return LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_INIT, HOTH_HOST_SPACE_LIBHOTH,
LIBHOTH_ERR_INTERFACE_BUSY);
}

usleep(wait_us);

if (total_waiting_us <= UINT32_MAX - wait_us) {
total_waiting_us += wait_us;
} else {
// Saturate at integer upper bound to prevent overflow.
total_waiting_us = UINT32_MAX;
}

if (wait_us <= MAX_SINGLE_SLEEP_US / BACKOFF_FACTOR) {
wait_us *= BACKOFF_FACTOR;
} else {
// Saturate at the `usleep()` max sleep bound.
wait_us = MAX_SINGLE_SLEEP_US;
}
}

// Unreachable
return LIBHOTH_ERR_FAIL;
return LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_INIT, HOTH_HOST_SPACE_LIBHOTH,
LIBHOTH_ERR_FAIL);
}

int libhoth_release_device(struct libhoth_device* dev) {
return dev->release(dev);
libhoth_error libhoth_release_device_v2(struct libhoth_device* dev) {
if (dev == NULL) {
return LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_INIT, HOTH_HOST_SPACE_LIBHOTH,
LIBHOTH_ERR_INVALID_PARAMETER);
}
if (dev->release_v2 != NULL) {
return dev->release_v2(dev);
}
if (dev->release != NULL) {
int status = dev->release(dev);
return libhoth_error_from_legacy(HOTH_CTX_NONE, status);
}
return LIBHOTH_ERR_CONSTRUCT(HOTH_CTX_INIT, HOTH_HOST_SPACE_LIBHOTH,
LIBHOTH_ERR_FAIL);
}
Loading
Loading