From 237fa96285c445d4695f04622553c259922932e5 Mon Sep 17 00:00:00 2001 From: Khang Nguyen Date: Thu, 14 Sep 2023 14:46:00 +0700 Subject: [PATCH 1/2] mctpd: Add timeout overriding using program arguments Currently, the timeout is hardcoded with the value of 250ms. However, when developing, we may want to make the timeout longer to make sure we do not miss any messages due to other reasons. This commit adds timeout overriding using program arguments. This makes tweaking the timeout at runtime possible. Tested: 1. Send a request to a non-existent endpoint. It should timeout in 250ms by default. root@mtmitchell:~# time busctl call xyz.openbmc_project.MCTP /xyz/openbmc_project/mctp \ > au.com.CodeConstruct.MCTP SetupEndpoint say mctppcie0 2 0xCA 0xFE Call failed: MCTP Endpoint did not respond real 0m0.344s user 0m0.016s sys 0m0.000s 2. Change mctpd.service to use longer timeout, 10 seconds for example. ExecStart=/usr/sbin/mctpd -t 10000000 3. Resend the request. It should timeout in 10 seconds. root@mtmitchell:~# time busctl call xyz.openbmc_project.MCTP /xyz/openbmc_project/mctp \ > au.com.CodeConstruct.MCTP SetupEndpoint say mctppcie0 2 0xCA 0xFE Call failed: MCTP Endpoint did not respond real 0m10.083s user 0m0.015s sys 0m0.001s Signed-off-by: Khang Nguyen --- src/mctpd.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/mctpd.c b/src/mctpd.c index b89b01ba..64d4a314 100644 --- a/src/mctpd.c +++ b/src/mctpd.c @@ -3074,9 +3074,10 @@ static int setup_testing(ctx *ctx) { static void print_usage(ctx *ctx) { - fprintf(stderr, "mctpd [-v] [-N]\n"); + fprintf(stderr, "mctpd [-v] [-N] [-t usecs]\n"); fprintf(stderr, " -v verbose\n"); fprintf(stderr, " -N testing mode. Not safe for production\n"); + fprintf(stderr, " -t timeout in usecs\n"); } static int parse_args(ctx *ctx, int argc, char **argv) @@ -3085,12 +3086,16 @@ static int parse_args(ctx *ctx, int argc, char **argv) { .name = "help", .has_arg = no_argument, .val = 'h' }, { .name = "verbose", .has_arg = no_argument, .val = 'v' }, { .name = "testing", .has_arg = no_argument, .val = 'N' }, + { .name = "timeout", .has_arg = required_argument, .val = 't' }, { 0 }, }; int c; + // default values + ctx->mctp_timeout = 250000; // 250ms + for (;;) { - c = getopt_long(argc, argv, "+hvN", options, NULL); + c = getopt_long(argc, argv, "+hvNt:", options, NULL); if (c == -1) break; @@ -3101,6 +3106,13 @@ static int parse_args(ctx *ctx, int argc, char **argv) case 'v': ctx->verbose = true; break; + case 't': + ctx->mctp_timeout = strtoull(optarg, NULL, 10); + if (ctx->mctp_timeout == 0) { + warnx("Timeout must be a non-zero u64"); + return errno; + } + break; case 'h': default: print_usage(ctx); @@ -3138,7 +3150,6 @@ static int setup_config(ctx *ctx) { int rc; // TODO: this will go in a config file or arguments. - ctx->mctp_timeout = 250000; // 250ms ctx->bus_owner = true; rc = fill_uuid(ctx); if (rc < 0) From 2370286061dccd4e8e0438c14b4ab27c507dcefb Mon Sep 17 00:00:00 2001 From: Khang Nguyen Date: Mon, 2 Oct 2023 10:34:25 +0700 Subject: [PATCH 2/2] mctpd: Add bus owner and endpoint switch using arguments Currently, mctpd defaults to run as a bus owner, so ctx->bus_owner is always true. This commit adds a role argument so that we can change the role without editing the source code. Tested: Send a Get EID command to mctpd. See the response: - Running as Bus Owner (no flag): 00 00 02 00 ee 12 00 - Running as Endpoint (-r endpoint): 00 00 02 00 ee 02 00 The second to last byte signifies the endpoint type. (0xee is the EID assigned to mctpd) Signed-off-by: Khang Nguyen --- src/mctpd.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/mctpd.c b/src/mctpd.c index 64d4a314..df7c40f6 100644 --- a/src/mctpd.c +++ b/src/mctpd.c @@ -3074,10 +3074,11 @@ static int setup_testing(ctx *ctx) { static void print_usage(ctx *ctx) { - fprintf(stderr, "mctpd [-v] [-N] [-t usecs]\n"); + fprintf(stderr, "mctpd [-v] [-N] [-t usecs] [-r {bus-owner,endpoint}]\n"); fprintf(stderr, " -v verbose\n"); fprintf(stderr, " -N testing mode. Not safe for production\n"); fprintf(stderr, " -t timeout in usecs\n"); + fprintf(stderr, " -r role of mctpd in the network\n"); } static int parse_args(ctx *ctx, int argc, char **argv) @@ -3087,15 +3088,17 @@ static int parse_args(ctx *ctx, int argc, char **argv) { .name = "verbose", .has_arg = no_argument, .val = 'v' }, { .name = "testing", .has_arg = no_argument, .val = 'N' }, { .name = "timeout", .has_arg = required_argument, .val = 't' }, + { .name = "role", .has_arg = required_argument, .val = 'r' }, { 0 }, }; int c; // default values ctx->mctp_timeout = 250000; // 250ms + ctx->bus_owner = true; for (;;) { - c = getopt_long(argc, argv, "+hvNt:", options, NULL); + c = getopt_long(argc, argv, "+hvNt:r:", options, NULL); if (c == -1) break; @@ -3113,6 +3116,16 @@ static int parse_args(ctx *ctx, int argc, char **argv) return errno; } break; + case 'r': + if (!strcmp(optarg, "bus-owner")) + ctx->bus_owner = true; + else if (!strcmp(optarg, "endpoint")) + ctx->bus_owner = false; + else { + warnx("Role can only be one of: bus-owner, endpoint"); + return -EINVAL; + } + break; case 'h': default: print_usage(ctx); @@ -3149,8 +3162,6 @@ static int fill_uuid(ctx *ctx) static int setup_config(ctx *ctx) { int rc; - // TODO: this will go in a config file or arguments. - ctx->bus_owner = true; rc = fill_uuid(ctx); if (rc < 0) return rc;