From 3d65fe298a954e4388e15a67e5128b662fe74cc1 Mon Sep 17 00:00:00 2001 From: Alexander A Oganezov Date: Tue, 4 Jun 2024 03:25:58 +0000 Subject: [PATCH 01/10] DAOS-15961 cart: Reorganize how envs are handled Change to how cart deals with envariables: - env list is now controlled by CRT_ENV_LIST macro in crt_types_internal.h - all envs stored in structure generated from CRT_ENV_LST (g_env) now, ENV is read out at crt_env_init() time and env strings deallocated at crt_env_fini(). - accsor functions/macros crt_env_init/fini/get/dump are provided. - string-type envs no longer need to be freed after retrieval. With this change, any env to be used will need to appear on the list first, ensuring it gets dumped as well as ensuring proper name usage later. Env name typos when using crt_env_get() will now result in compile time errors Required-githooks: true Signed-off-by: Alexander A Oganezov --- src/cart/crt_init.c | 128 +++++++----------------------- src/cart/crt_internal_types.h | 143 ++++++++++++++++++++++++++++++++++ src/cart/utils/crt_utils.c | 77 ++++++++---------- 3 files changed, 204 insertions(+), 144 deletions(-) diff --git a/src/cart/crt_init.c b/src/cart/crt_init.c index 7e419eb2edd..a49e9fc492d 100644 --- a/src/cart/crt_init.c +++ b/src/cart/crt_init.c @@ -13,54 +13,18 @@ #include "crt_internal.h" struct crt_gdata crt_gdata; +struct crt_envs_t g_envs; static volatile int gdata_init_flag; struct crt_plugin_gdata crt_plugin_gdata; static bool g_prov_settings_applied[CRT_PROV_COUNT]; -/* List of the environment variables used in CaRT */ -static const char *crt_env_names[] = { - "D_PROVIDER", - "D_INTERFACE", - "D_DOMAIN", - "D_PORT", - "D_LOG_STDERR_IN_LOG", - "D_LOG_SIZE", - "D_LOG_FILE", - "D_LOG_FILE_APPEND_PID", - "D_LOG_MASK", - "DD_MASK", - "DD_STDERR", - "DD_SUBSYS", - "CRT_TIMEOUT", - "CRT_ATTACH_INFO_PATH", - "CRT_CREDIT_EP_CTX", - "CRT_CTX_NUM", - "D_FI_CONFIG", - "FI_UNIVERSE_SIZE", - "CRT_ENABLE_MEM_PIN", - "FI_OFI_RXM_USE_SRX", - "D_LOG_FLUSH", - "CRT_MRC_ENABLE", - "CRT_SECONDARY_PROVIDER", - "D_PROVIDER_AUTH_KEY", - "D_PORT_AUTO_ADJUST", - "D_POLL_TIMEOUT", - "D_LOG_FILE_APPEND_RANK", - "D_QUOTA_RPCS", - "D_POST_INIT", - "D_POST_INCR", - "DAOS_SIGNAL_REGISTER", - "D_CLIENT_METRICS_ENABLE", - "D_CLIENT_METRICS_RETAIN", - "D_CLIENT_METRICS_DUMP_DIR", -}; - static void crt_lib_init(void) __attribute__((__constructor__)); static void crt_lib_fini(void) __attribute__((__destructor__)); + /* Library initialization/constructor */ static void crt_lib_init(void) @@ -99,26 +63,6 @@ crt_lib_fini(void) D_RWLOCK_DESTROY(&crt_gdata.cg_rwlock); } -static void -dump_envariables(void) -{ - int i; - - D_INFO("-- ENVARS: --\n"); - for (i = 0; i < ARRAY_SIZE(crt_env_names); i++) { - char *val = NULL; - - d_agetenv_str(&val, crt_env_names[i]); - if (val == NULL) - continue; - if (strcmp(crt_env_names[i], "D_PROVIDER_AUTH_KEY") == 0) - D_INFO("%s = %s\n", crt_env_names[i], "********"); - else - D_INFO("%s = %s\n", crt_env_names[i], val); - d_freeenv_str(&val); - } -} - static void dump_opt(crt_init_options_t *opt) { @@ -138,7 +82,7 @@ dump_opt(crt_init_options_t *opt) if (opt->cio_use_unexpected_size) D_INFO("max_unexpect_size = %d\n", opt->cio_max_unexpected_size); - /* Handle similar to D_PROVIDER_AUTH_KEY in dump_envariables() */ + /* Handle similar to D_PROVIDER_AUTH_KEY */ if (opt->cio_auth_key) D_INFO("auth_key is set\n"); } @@ -213,7 +157,7 @@ prov_data_init(struct crt_prov_gdata *prov_data, crt_provider_t provider, max_num_ctx = CRT_SRV_CONTEXT_NUM; } else { /* Only limit the number of contexts for clients */ - d_getenv_uint("CRT_CTX_NUM", &ctx_num); + crt_env_get(CRT_CTX_NUM, &ctx_num); /* Default setting to the number of cores */ if (opt) @@ -270,27 +214,24 @@ static int data_init(int server, crt_init_options_t *opt) uint32_t fi_univ_size = 0; uint32_t mem_pin_enable = 0; uint32_t is_secondary; - char ucx_ib_fork_init = 0; uint32_t post_init = CRT_HG_POST_INIT, post_incr = CRT_HG_POST_INCR; int rc = 0; D_DEBUG(DB_ALL, "initializing crt_gdata...\n"); - - dump_envariables(); - + crt_env_dump(); D_DEBUG(DB_ALL, "Starting RPCID %#lx. Num cores: %ld\n", crt_gdata.cg_rpcid, crt_gdata.cg_num_cores); /* Set context post init / post incr to tune number of pre-posted recvs */ - d_getenv_uint32_t("D_POST_INIT", &post_init); + crt_env_get(D_POST_INIT, &post_init); crt_gdata.cg_post_init = post_init; - d_getenv_uint32_t("D_POST_INCR", &post_incr); + crt_env_get(D_POST_INCR, &post_incr); crt_gdata.cg_post_incr = post_incr; is_secondary = 0; /* Apply CART-890 workaround for server side only */ if (server) { - d_getenv_uint("CRT_ENABLE_MEM_PIN", &mem_pin_enable); + crt_env_get(CRT_ENABLE_MEM_PIN, &mem_pin_enable); if (mem_pin_enable == 1) mem_pin_workaround(); } else { @@ -298,14 +239,14 @@ static int data_init(int server, crt_init_options_t *opt) * Client-side envariable to indicate that the cluster * is running using a secondary provider */ - d_getenv_uint("CRT_SECONDARY_PROVIDER", &is_secondary); + crt_env_get(CRT_SECONDARY_PROVIDER, &is_secondary); } crt_gdata.cg_provider_is_primary = (is_secondary) ? 0 : 1; if (opt && opt->cio_crt_timeout != 0) timeout = opt->cio_crt_timeout; else - d_getenv_uint("CRT_TIMEOUT", &timeout); + crt_env_get(CRT_TIMEOUT, &timeout); if (timeout == 0 || timeout > 3600) crt_gdata.cg_timeout = CRT_DEFAULT_TIMEOUT_S; @@ -324,36 +265,26 @@ static int data_init(int server, crt_init_options_t *opt) credits = opt->cio_ep_credits; } else { credits = CRT_DEFAULT_CREDITS_PER_EP_CTX; - d_getenv_uint("CRT_CREDIT_EP_CTX", &credits); + crt_env_get(CRT_CREDIT_EP_CTX, &credits); } /* Enable quotas by default only on clients */ crt_gdata.cg_rpc_quota = server ? 0 : CRT_QUOTA_RPCS_DEFAULT; - - d_getenv_uint("D_QUOTA_RPCS", &crt_gdata.cg_rpc_quota); + crt_env_get(D_QUOTA_RPCS, &crt_gdata.cg_rpc_quota); /* Must be set on the server when using UCX, will not affect OFI */ - d_getenv_char("UCX_IB_FORK_INIT", &ucx_ib_fork_init); - if (ucx_ib_fork_init) { - if (server) { - D_INFO("UCX_IB_FORK_INIT was set to %c, setting to n\n", ucx_ib_fork_init); - } else { - D_INFO("UCX_IB_FORK_INIT was set to %c on client\n", ucx_ib_fork_init); - } - } if (server) d_setenv("UCX_IB_FORK_INIT", "n", 1); /* This is a workaround for CART-871 if universe size is not set */ - d_getenv_uint("FI_UNIVERSE_SIZE", &fi_univ_size); + crt_env_get(FI_UNIVERSE_SIZE, &fi_univ_size); if (fi_univ_size == 0) { D_INFO("FI_UNIVERSE_SIZE was not set; setting to 2048\n"); d_setenv("FI_UNIVERSE_SIZE", "2048", 1); } if (credits == 0) { - D_DEBUG(DB_ALL, "CRT_CREDIT_EP_CTX set as 0, flow control " - "disabled.\n"); + D_DEBUG(DB_ALL, "CRT_CREDIT_EP_CTX set as 0, flow control disabled.\n"); } else if (credits > CRT_MAX_CREDITS_PER_EP_CTX) { D_DEBUG(DB_ALL, "ENV CRT_CREDIT_EP_CTX's value %d exceed max " "allowed value, use %d for flow control.\n", @@ -378,15 +309,13 @@ static int data_init(int server, crt_init_options_t *opt) "total number of URI requests for self", "", "net/uri/lookup_self"); if (ret) - D_WARN("Failed to create uri self sensor: "DF_RC"\n", - DP_RC(ret)); + D_WARN("Failed to create uri self sensor: "DF_RC"\n", DP_RC(ret)); ret = d_tm_add_metric(&crt_gdata.cg_uri_other, D_TM_COUNTER, "total number of URI requests for other " "ranks", "", "net/uri/lookup_other"); if (ret) - D_WARN("Failed to create uri other sensor: "DF_RC"\n", - DP_RC(ret)); + D_WARN("Failed to create uri other sensor: "DF_RC"\n", DP_RC(ret)); } gdata_init_flag = 1; @@ -561,7 +490,7 @@ prov_settings_apply(bool primary, crt_provider_t prov, crt_init_options_t *opt) if (prov == CRT_PROV_OFI_CXI) mrc_enable = 1; - d_getenv_uint("CRT_MRC_ENABLE", &mrc_enable); + crt_env_get(CRT_MRC_ENABLE, &mrc_enable); if (mrc_enable == 0) { D_INFO("Disabling MR CACHE (FI_MR_CACHE_MAX_COUNT=0)\n"); d_setenv("FI_MR_CACHE_MAX_COUNT", "0", 1); @@ -633,6 +562,7 @@ crt_init_opt(crt_group_id_t grpid, uint32_t flags, crt_init_options_t *opt) crt_setup_log_fac(); D_INFO("libcart version %s initializing\n", CART_VERSION); + crt_env_init(); if (opt) dump_opt(opt); @@ -664,7 +594,7 @@ crt_init_opt(crt_group_id_t grpid, uint32_t flags, crt_init_options_t *opt) crt_gdata.cg_auto_swim_disable = (flags & CRT_FLAG_BIT_AUTO_SWIM_DISABLE) ? 1 : 0; - d_agetenv_str(&path, "CRT_ATTACH_INFO_PATH"); + crt_env_get(CRT_ATTACH_INFO_PATH, &path); if (path != NULL && strlen(path) > 0) { rc = crt_group_config_path_set(path); if (rc != 0) @@ -679,40 +609,39 @@ crt_init_opt(crt_group_id_t grpid, uint32_t flags, crt_init_options_t *opt) if (opt && opt->cio_auth_key) auth_key = opt->cio_auth_key; else { - d_agetenv_str(&auth_key_env, "D_PROVIDER_AUTH_KEY"); + crt_env_get(D_PROVIDER_AUTH_KEY, &auth_key_env); auth_key = auth_key_env; } if (opt && opt->cio_provider) provider = opt->cio_provider; else { - d_agetenv_str(&provider_env, "D_PROVIDER"); + crt_env_get(D_PROVIDER, &provider_env); provider = provider_env; } if (opt && opt->cio_interface) interface = opt->cio_interface; else { - d_agetenv_str(&interface_env, "D_INTERFACE"); + crt_env_get(D_INTERFACE, &interface_env); interface = interface_env; } if (opt && opt->cio_domain) domain = opt->cio_domain; else { - d_agetenv_str(&domain_env, "D_DOMAIN"); + crt_env_get(D_DOMAIN, &domain_env); domain = domain_env; } if (opt && opt->cio_port) port = opt->cio_port; else { - d_agetenv_str(&port_env, "D_PORT"); + crt_env_get(D_PORT, &port_env); port = port_env; } - d_getenv_bool("D_PORT_AUTO_ADJUST", &port_auto_adjust); - + crt_env_get(D_PORT_AUTO_ADJUST, &port_auto_adjust); rc = __split_arg(provider, ",", &provider_str0, &provider_str1); if (rc != 0) D_GOTO(unlock, rc); @@ -928,11 +857,6 @@ crt_init_opt(crt_group_id_t grpid, uint32_t flags, crt_init_options_t *opt) D_FREE(domain0); D_FREE(provider_str0); D_FREE(auth_key0); - d_freeenv_str(&port_env); - d_freeenv_str(&domain_env); - d_freeenv_str(&interface_env); - d_freeenv_str(&provider_env); - d_freeenv_str(&auth_key_env); if (rc != 0) { D_ERROR("failed, "DF_RC"\n", DP_RC(rc)); @@ -1031,6 +955,8 @@ crt_finalize(void) else D_ERROR("failed, rc: "DF_RC"\n", DP_RC(rc)); + crt_env_fini(); + return rc; } diff --git a/src/cart/crt_internal_types.h b/src/cart/crt_internal_types.h index 60fcc8f5c8e..defe9c13433 100644 --- a/src/cart/crt_internal_types.h +++ b/src/cart/crt_internal_types.h @@ -166,6 +166,149 @@ struct crt_event_cb_priv { #define CRT_CALLBACKS_NUM (4) /* start number of CBs */ #endif +/* + * List of environment variables to read at CaRT library load time. + * for integer envs use ENV() + * for string ones ENV_STR() or ENV_STR_NO_PRINT() + **/ +#define CRT_ENV_LIST \ + ENV_STR(CRT_ATTACH_INFO_PATH) \ + ENV(CRT_CREDIT_EP_CTX) \ + ENV(CRT_CTX_NUM) \ + ENV(CRT_ENABLE_MEM_PIN) \ + ENV_STR(CRT_L_GRP_CFG) \ + ENV(CRT_L_RANK) \ + ENV(CRT_MRC_ENABLE) \ + ENV(CRT_SECONDARY_PROVIDER) \ + ENV(CRT_TIMEOUT) \ + ENV(DAOS_RPC_SIZE_LIMIT) \ + ENV(DAOS_SIGNAL_REGISTER) \ + ENV_STR(DAOS_TEST_SHARED_DIR) \ + ENV_STR(DD_MASK) \ + ENV_STR(DD_STDERR) \ + ENV_STR(DD_SUBSYS) \ + ENV_STR(D_CLIENT_METRICS_DUMP_DIR) \ + ENV(D_CLIENT_METRICS_ENABLE) \ + ENV(D_CLIENT_METRICS_RETAIN) \ + ENV_STR(D_DOMAIN) \ + ENV_STR(D_FI_CONFIG) \ + ENV_STR(D_INTERFACE) \ + ENV_STR(D_LOG_FILE) \ + ENV_STR(D_LOG_FILE_APPEND_PID) \ + ENV_STR(D_LOG_FILE_APPEND_RANK) \ + ENV_STR(D_LOG_FLUSH) \ + ENV_STR(D_LOG_MASK) \ + ENV_STR(D_LOG_SIZE) \ + ENV(D_LOG_STDERR_IN_LOG) \ + ENV(D_POLL_TIMEOUT) \ + ENV_STR(D_PORT) \ + ENV(D_PORT_AUTO_ADJUST) \ + ENV(D_POST_INCR) \ + ENV(D_POST_INIT) \ + ENV_STR(D_PROVIDER) \ + ENV_STR_NO_PRINT(D_PROVIDER_AUTH_KEY) \ + ENV(D_QUOTA_RPCS) \ + ENV(FI_OFI_RXM_USE_SRX) \ + ENV(FI_UNIVERSE_SIZE) \ + ENV(SWIM_PING_TIMEOUT) \ + ENV(SWIM_PROTOCOL_PERIOD_LEN) \ + ENV(SWIM_SUSPECT_TIMEOUT) \ + ENV_STR(UCX_IB_FORK_INIT) + +/* uint env */ +#define ENV(x) \ + unsigned int _##x; \ + int _rc_##x; \ + int _no_print_##x; + +/* char* env */ +#define ENV_STR(x) \ + char* _##x; \ + int _rc_##x; \ + int _no_print_##x; + +#define ENV_STR_NO_PRINT(x) ENV_STR(x) + +struct crt_envs_t { + CRT_ENV_LIST; +}; + +#undef ENV +#undef ENV_STR +#undef ENV_STR_NO_PRINT + +extern struct crt_envs_t g_envs; + +static inline void +crt_env_init(void) { +#define ENV(x) \ +do { \ + g_envs._rc_##x = d_getenv_uint(#x, &g_envs._##x); \ + g_envs._no_print_##x = 0; \ +} while (0); + +#define ENV_STR(x) \ +do { \ + g_envs._rc_##x = d_agetenv_str(&g_envs._##x, #x); \ + g_envs._no_print_##x = 0; \ +} while (0); + +#define ENV_STR_NO_PRINT(x) \ +do { \ + g_envs._rc_##x = d_agetenv_str(&g_envs._##x, #x); \ + g_envs._no_print_##x = 1; \ +} while (0); + + CRT_ENV_LIST; +#undef ENV +#undef ENV_STR +#undef ENV_STR_NO_PRINT +} + +static inline void +crt_env_fini(void) { + +#define ENV(x) (void) +#define ENV_STR(x) d_freeenv_str(&g_envs._##x); +#define ENV_STR_NO_PRINT ENV_STR + + CRT_ENV_LIST + +#undef ENV +#undef ENV_STR +#undef ENV_STR_NO_PRINT + +} + +/* Returns value if env was present at load time */ +#define crt_env_get(name, val) \ +if (g_envs._rc_##name == 0) \ + *val = g_envs._##name; + + +static inline void +crt_env_dump(void) +{ + D_INFO("--- ENV ---\n"); + + /* Only dump envariables that were set */ +#define ENV(x) \ + if (!g_envs._rc_##x && g_envs._no_print_##x == 0) \ + D_INFO("%s = %d\n", #x, g_envs._##x); + +#define ENV_STR(x) \ + if (!g_envs._rc_##x) \ + D_INFO("%s = %s\n", #x, g_envs._no_print_##x ? "****" : g_envs._##x); + +#define ENV_STR_NO_PRINT ENV_STR + + CRT_ENV_LIST; + +#undef ENV +#undef ENV_STR +#undef ENV_STR_NO_PRINT +} + /* structure of global fault tolerance data */ struct crt_plugin_gdata { /* list of progress callbacks */ diff --git a/src/cart/utils/crt_utils.c b/src/cart/utils/crt_utils.c index 31a5daa2a98..07ae00c5c8a 100644 --- a/src/cart/utils/crt_utils.c +++ b/src/cart/utils/crt_utils.c @@ -98,15 +98,14 @@ void write_completion_file(void) { FILE *fptr; - char *dir; + char *dir = NULL; char *completion_file = NULL; - d_agetenv_str(&dir, "DAOS_TEST_SHARED_DIR"); + crt_env_get(DAOS_TEST_SHARED_DIR, &dir); + D_ASSERTF(dir != NULL, - "DAOS_TEST_SHARED_DIR must be set for --write_completion_file " - "option.\n"); + "DAOS_TEST_SHARED_DIR must be set for --write_completion_file option.\n"); D_ASPRINTF(completion_file, "%s/test-servers-completed.txt.%d", dir, getpid()); - d_freeenv_str(&dir); D_ASSERTF(completion_file != NULL, "Error allocating completion_file string\n"); unlink(completion_file); @@ -415,8 +414,8 @@ crtu_dc_mgmt_net_cfg_setenv(const char *name) { int rc; char *provider; - char *cli_srx_set = NULL; - char *crt_timeout = NULL; + int cli_srx_set = 0; + int crt_timeout = 0; char *d_interface; char *d_interface_env = NULL; char *d_domain; @@ -444,42 +443,41 @@ crtu_dc_mgmt_net_cfg_setenv(const char *name) /* If the server has set this, the client must use the same value. */ if (crt_net_cfg_info.srv_srx_set != -1) { - rc = asprintf(&cli_srx_set, "%d", crt_net_cfg_info.srv_srx_set); - if (rc < 0) { - cli_srx_set = NULL; - D_GOTO(cleanup, rc = -DER_NOMEM); - } - D_INFO("setenv FI_OFI_RXM_USE_SRX=%s\n", cli_srx_set); - rc = d_setenv("FI_OFI_RXM_USE_SRX", cli_srx_set, 1); + cli_srx_set = crt_net_cfg_info.srv_srx_set; + D_INFO("setenv FI_OFI_RXM_USE_SRX=%d\n", cli_srx_set); + rc = d_setenv("FI_OFI_RXM_USE_SRX", cli_srx_set ? "1" : "0", 1); if (rc != 0) D_GOTO(cleanup, rc = d_errno2der(errno)); } else { /* Client may not set it if the server hasn't. */ - d_agetenv_str(&cli_srx_set, "FI_OFI_RXM_USE_SRX"); + crt_env_get(FI_OFI_RXM_USE_SRX, &cli_srx_set); if (cli_srx_set) { - D_ERROR("Client set FI_OFI_RXM_USE_SRX to %s, " + D_ERROR("Client set FI_OFI_RXM_USE_SRX to %d, " "but server is unset!\n", cli_srx_set); D_GOTO(cleanup, rc = -DER_INVAL); } } /* Allow client env overrides for these three */ - d_agetenv_str(&crt_timeout, "CRT_TIMEOUT"); + crt_env_get(CRT_TIMEOUT, &crt_timeout); if (!crt_timeout) { - rc = asprintf(&crt_timeout, "%d", crt_net_cfg_info.crt_timeout); - if (rc < 0) { - crt_timeout = NULL; + char *tmp; + crt_timeout = crt_net_cfg_info.crt_timeout; + D_INFO("setenv CRT_TIMEOUT=%d\n", crt_timeout); + + rc = asprintf(&tmp, "%d", crt_net_cfg_info.crt_timeout); + if (rc < 0) D_GOTO(cleanup, rc = -DER_NOMEM); - } - D_INFO("setenv CRT_TIMEOUT=%s\n", crt_timeout); - rc = d_setenv("CRT_TIMEOUT", crt_timeout, 1); + + rc = d_setenv("CRT_TIMEOUT", tmp, 1); + free(tmp); if (rc != 0) D_GOTO(cleanup, rc = d_errno2der(errno)); } else { - D_DEBUG(DB_MGMT, "Using client provided CRT_TIMEOUT: %s\n", crt_timeout); + D_DEBUG(DB_MGMT, "Using client provided CRT_TIMEOUT: %d\n", crt_timeout); } - d_agetenv_str(&d_interface_env, "D_INTERFACE"); + crt_env_get(D_INTERFACE, &d_interface_env); if (!d_interface_env) { d_interface = crt_net_cfg_info.interface; D_INFO("Setting D_INTERFACE=%s\n", d_interface); @@ -491,7 +489,7 @@ crtu_dc_mgmt_net_cfg_setenv(const char *name) D_DEBUG(DB_MGMT, "Using client provided D_INTERFACE: %s\n", d_interface); } - d_agetenv_str(&d_domain_env, "D_DOMAIN"); + crt_env_get(D_DOMAIN, &d_domain_env); if (!d_domain_env) { d_domain = crt_net_cfg_info.domain; D_INFO("Setting D_DOMAIN=%s\n", d_domain); @@ -504,14 +502,10 @@ crtu_dc_mgmt_net_cfg_setenv(const char *name) } D_INFO("CaRT env setup with:\n" - "\tD_INTERFACE=%s, D_DOMAIN: %s, D_PROVIDER: %s, CRT_TIMEOUT: %s\n", + "\tD_INTERFACE=%s, D_DOMAIN: %s, D_PROVIDER: %s, CRT_TIMEOUT: %d\n", d_interface, d_domain, provider, crt_timeout); cleanup: - d_freeenv_str(&d_domain_env); - d_freeenv_str(&d_interface_env); - d_freeenv_str(&crt_timeout); - d_freeenv_str(&cli_srx_set); dc_put_attach_info(&crt_net_cfg_info, crt_net_cfg_resp); return rc; @@ -524,8 +518,8 @@ crtu_cli_start_basic(char *local_group_name, char *srv_group_name, unsigned int total_srv_ctx, bool use_cfg, crt_init_options_t *init_opt, bool use_daos_agent_env) { - char *grp_cfg_file; - uint32_t grp_size; + char *grp_cfg_file = NULL; + uint32_t grp_size = 0; int rc = 0; if (opts.assert_on_error) @@ -582,15 +576,13 @@ crtu_cli_start_basic(char *local_group_name, char *srv_group_name, if (*grp == NULL) D_GOTO(out, rc = -DER_INVAL); - d_agetenv_str(&grp_cfg_file, "CRT_L_GRP_CFG"); + crt_env_get(CRT_L_GRP_CFG, &grp_cfg_file); /* load group info from a config file and * delete file upon return */ rc = crtu_load_group_from_file(grp_cfg_file, - *crt_ctx, *grp, - -1, true); - d_freeenv_str(&grp_cfg_file); + *crt_ctx, *grp, -1, true); if (rc != 0) D_GOTO(out, rc); } @@ -652,15 +644,15 @@ crtu_srv_start_basic(char *srv_group_name, crt_context_t *crt_ctx, pthread_t *progress_thread, crt_group_t **grp, uint32_t *grp_size, crt_init_options_t *init_opt) { - char *grp_cfg_file; - char *my_uri; - d_rank_t my_rank; + char *grp_cfg_file = NULL; + char *my_uri = NULL; + d_rank_t my_rank = CRT_NO_RANK; int rc = 0; if (opts.assert_on_error) D_ASSERTF(opts.is_initialized == true, "crtu_test_init not called.\n"); - rc = d_getenv_uint32_t("CRT_L_RANK", &my_rank); + crt_env_get(CRT_L_RANK, &my_rank); D_ASSERTF(rc == DER_SUCCESS, "Rank can not be retrieve: " DF_RC "\n", DP_RC(rc)); rc = d_log_init(); @@ -707,11 +699,10 @@ crtu_srv_start_basic(char *srv_group_name, crt_context_t *crt_ctx, D_GOTO(out, rc); D_FREE(my_uri); - rc = d_agetenv_str(&grp_cfg_file, "CRT_L_GRP_CFG"); + crt_env_get(CRT_L_GRP_CFG, &grp_cfg_file); /* load group info from a config file and delete file upon return */ rc = crtu_load_group_from_file(grp_cfg_file, crt_ctx[0], *grp, my_rank, true); - d_freeenv_str(&grp_cfg_file); if (rc != 0) D_GOTO(out, rc); From 096319acb25378f211ea56bc3d750452d79bebbc Mon Sep 17 00:00:00 2001 From: Alexander A Oganezov Date: Tue, 4 Jun 2024 03:29:08 +0000 Subject: [PATCH 02/10] style Required-githooks: true Signed-off-by: Alexander A Oganezov --- src/cart/crt_init.c | 9 +- src/cart/crt_internal_types.h | 167 +++++++++++++++++----------------- src/cart/utils/crt_utils.c | 24 ++--- 3 files changed, 99 insertions(+), 101 deletions(-) diff --git a/src/cart/crt_init.c b/src/cart/crt_init.c index a49e9fc492d..22db4d60c27 100644 --- a/src/cart/crt_init.c +++ b/src/cart/crt_init.c @@ -13,7 +13,7 @@ #include "crt_internal.h" struct crt_gdata crt_gdata; -struct crt_envs_t g_envs; +struct crt_envs_t g_envs; static volatile int gdata_init_flag; struct crt_plugin_gdata crt_plugin_gdata; static bool g_prov_settings_applied[CRT_PROV_COUNT]; @@ -24,7 +24,6 @@ crt_lib_init(void) __attribute__((__constructor__)); static void crt_lib_fini(void) __attribute__((__destructor__)); - /* Library initialization/constructor */ static void crt_lib_init(void) @@ -213,7 +212,7 @@ static int data_init(int server, crt_init_options_t *opt) uint32_t credits; uint32_t fi_univ_size = 0; uint32_t mem_pin_enable = 0; - uint32_t is_secondary; + uint32_t is_secondary; uint32_t post_init = CRT_HG_POST_INIT, post_incr = CRT_HG_POST_INCR; int rc = 0; @@ -309,13 +308,13 @@ static int data_init(int server, crt_init_options_t *opt) "total number of URI requests for self", "", "net/uri/lookup_self"); if (ret) - D_WARN("Failed to create uri self sensor: "DF_RC"\n", DP_RC(ret)); + D_WARN("Failed to create uri self sensor: " DF_RC "\n", DP_RC(ret)); ret = d_tm_add_metric(&crt_gdata.cg_uri_other, D_TM_COUNTER, "total number of URI requests for other " "ranks", "", "net/uri/lookup_other"); if (ret) - D_WARN("Failed to create uri other sensor: "DF_RC"\n", DP_RC(ret)); + D_WARN("Failed to create uri other sensor: " DF_RC "\n", DP_RC(ret)); } gdata_init_flag = 1; diff --git a/src/cart/crt_internal_types.h b/src/cart/crt_internal_types.h index defe9c13433..bb72591efd0 100644 --- a/src/cart/crt_internal_types.h +++ b/src/cart/crt_internal_types.h @@ -171,61 +171,61 @@ struct crt_event_cb_priv { * for integer envs use ENV() * for string ones ENV_STR() or ENV_STR_NO_PRINT() **/ -#define CRT_ENV_LIST \ - ENV_STR(CRT_ATTACH_INFO_PATH) \ - ENV(CRT_CREDIT_EP_CTX) \ - ENV(CRT_CTX_NUM) \ - ENV(CRT_ENABLE_MEM_PIN) \ - ENV_STR(CRT_L_GRP_CFG) \ - ENV(CRT_L_RANK) \ - ENV(CRT_MRC_ENABLE) \ - ENV(CRT_SECONDARY_PROVIDER) \ - ENV(CRT_TIMEOUT) \ - ENV(DAOS_RPC_SIZE_LIMIT) \ - ENV(DAOS_SIGNAL_REGISTER) \ - ENV_STR(DAOS_TEST_SHARED_DIR) \ - ENV_STR(DD_MASK) \ - ENV_STR(DD_STDERR) \ - ENV_STR(DD_SUBSYS) \ - ENV_STR(D_CLIENT_METRICS_DUMP_DIR) \ - ENV(D_CLIENT_METRICS_ENABLE) \ - ENV(D_CLIENT_METRICS_RETAIN) \ - ENV_STR(D_DOMAIN) \ - ENV_STR(D_FI_CONFIG) \ - ENV_STR(D_INTERFACE) \ - ENV_STR(D_LOG_FILE) \ - ENV_STR(D_LOG_FILE_APPEND_PID) \ - ENV_STR(D_LOG_FILE_APPEND_RANK) \ - ENV_STR(D_LOG_FLUSH) \ - ENV_STR(D_LOG_MASK) \ - ENV_STR(D_LOG_SIZE) \ - ENV(D_LOG_STDERR_IN_LOG) \ - ENV(D_POLL_TIMEOUT) \ - ENV_STR(D_PORT) \ - ENV(D_PORT_AUTO_ADJUST) \ - ENV(D_POST_INCR) \ - ENV(D_POST_INIT) \ - ENV_STR(D_PROVIDER) \ - ENV_STR_NO_PRINT(D_PROVIDER_AUTH_KEY) \ - ENV(D_QUOTA_RPCS) \ - ENV(FI_OFI_RXM_USE_SRX) \ - ENV(FI_UNIVERSE_SIZE) \ - ENV(SWIM_PING_TIMEOUT) \ - ENV(SWIM_PROTOCOL_PERIOD_LEN) \ - ENV(SWIM_SUSPECT_TIMEOUT) \ +#define CRT_ENV_LIST \ + ENV_STR(CRT_ATTACH_INFO_PATH) \ + ENV(CRT_CREDIT_EP_CTX) \ + ENV(CRT_CTX_NUM) \ + ENV(CRT_ENABLE_MEM_PIN) \ + ENV_STR(CRT_L_GRP_CFG) \ + ENV(CRT_L_RANK) \ + ENV(CRT_MRC_ENABLE) \ + ENV(CRT_SECONDARY_PROVIDER) \ + ENV(CRT_TIMEOUT) \ + ENV(DAOS_RPC_SIZE_LIMIT) \ + ENV(DAOS_SIGNAL_REGISTER) \ + ENV_STR(DAOS_TEST_SHARED_DIR) \ + ENV_STR(DD_MASK) \ + ENV_STR(DD_STDERR) \ + ENV_STR(DD_SUBSYS) \ + ENV_STR(D_CLIENT_METRICS_DUMP_DIR) \ + ENV(D_CLIENT_METRICS_ENABLE) \ + ENV(D_CLIENT_METRICS_RETAIN) \ + ENV_STR(D_DOMAIN) \ + ENV_STR(D_FI_CONFIG) \ + ENV_STR(D_INTERFACE) \ + ENV_STR(D_LOG_FILE) \ + ENV_STR(D_LOG_FILE_APPEND_PID) \ + ENV_STR(D_LOG_FILE_APPEND_RANK) \ + ENV_STR(D_LOG_FLUSH) \ + ENV_STR(D_LOG_MASK) \ + ENV_STR(D_LOG_SIZE) \ + ENV(D_LOG_STDERR_IN_LOG) \ + ENV(D_POLL_TIMEOUT) \ + ENV_STR(D_PORT) \ + ENV(D_PORT_AUTO_ADJUST) \ + ENV(D_POST_INCR) \ + ENV(D_POST_INIT) \ + ENV_STR(D_PROVIDER) \ + ENV_STR_NO_PRINT(D_PROVIDER_AUTH_KEY) \ + ENV(D_QUOTA_RPCS) \ + ENV(FI_OFI_RXM_USE_SRX) \ + ENV(FI_UNIVERSE_SIZE) \ + ENV(SWIM_PING_TIMEOUT) \ + ENV(SWIM_PROTOCOL_PERIOD_LEN) \ + ENV(SWIM_SUSPECT_TIMEOUT) \ ENV_STR(UCX_IB_FORK_INIT) /* uint env */ -#define ENV(x) \ - unsigned int _##x; \ - int _rc_##x; \ - int _no_print_##x; +#define ENV(x) \ + unsigned int _##x; \ + int _rc_##x; \ + int _no_print_##x; /* char* env */ -#define ENV_STR(x) \ - char* _##x; \ - int _rc_##x; \ - int _no_print_##x; +#define ENV_STR(x) \ + char *_##x; \ + int _rc_##x; \ + int _no_print_##x; #define ENV_STR_NO_PRINT(x) ENV_STR(x) @@ -240,51 +240,50 @@ struct crt_envs_t { extern struct crt_envs_t g_envs; static inline void -crt_env_init(void) { -#define ENV(x) \ -do { \ - g_envs._rc_##x = d_getenv_uint(#x, &g_envs._##x); \ - g_envs._no_print_##x = 0; \ -} while (0); - -#define ENV_STR(x) \ -do { \ - g_envs._rc_##x = d_agetenv_str(&g_envs._##x, #x); \ - g_envs._no_print_##x = 0; \ -} while (0); - -#define ENV_STR_NO_PRINT(x) \ -do { \ - g_envs._rc_##x = d_agetenv_str(&g_envs._##x, #x); \ - g_envs._no_print_##x = 1; \ -} while (0); - - CRT_ENV_LIST; +crt_env_init(void) +{ +#define ENV(x) \ + do { \ + g_envs._rc_##x = d_getenv_uint(#x, &g_envs._##x); \ + g_envs._no_print_##x = 0; \ + } while (0); + +#define ENV_STR(x) \ + do { \ + g_envs._rc_##x = d_agetenv_str(&g_envs._##x, #x); \ + g_envs._no_print_##x = 0; \ + } while (0); + +#define ENV_STR_NO_PRINT(x) \ + do { \ + g_envs._rc_##x = d_agetenv_str(&g_envs._##x, #x); \ + g_envs._no_print_##x = 1; \ + } while (0); + + CRT_ENV_LIST; #undef ENV #undef ENV_STR #undef ENV_STR_NO_PRINT } static inline void -crt_env_fini(void) { - -#define ENV(x) (void) -#define ENV_STR(x) d_freeenv_str(&g_envs._##x); -#define ENV_STR_NO_PRINT ENV_STR +crt_env_fini(void) +{ +#define ENV(x) (void) +#define ENV_STR(x) d_freeenv_str(&g_envs._##x); +#define ENV_STR_NO_PRINT ENV_STR CRT_ENV_LIST #undef ENV #undef ENV_STR #undef ENV_STR_NO_PRINT - } /* Returns value if env was present at load time */ -#define crt_env_get(name, val) \ -if (g_envs._rc_##name == 0) \ - *val = g_envs._##name; - +#define crt_env_get(name, val) \ + if (g_envs._rc_##name == 0) \ + *val = g_envs._##name; static inline void crt_env_dump(void) @@ -292,12 +291,12 @@ crt_env_dump(void) D_INFO("--- ENV ---\n"); /* Only dump envariables that were set */ -#define ENV(x) \ - if (!g_envs._rc_##x && g_envs._no_print_##x == 0) \ +#define ENV(x) \ + if (!g_envs._rc_##x && g_envs._no_print_##x == 0) \ D_INFO("%s = %d\n", #x, g_envs._##x); -#define ENV_STR(x) \ - if (!g_envs._rc_##x) \ +#define ENV_STR(x) \ + if (!g_envs._rc_##x) \ D_INFO("%s = %s\n", #x, g_envs._no_print_##x ? "****" : g_envs._##x); #define ENV_STR_NO_PRINT ENV_STR diff --git a/src/cart/utils/crt_utils.c b/src/cart/utils/crt_utils.c index 07ae00c5c8a..a48d80f11bf 100644 --- a/src/cart/utils/crt_utils.c +++ b/src/cart/utils/crt_utils.c @@ -98,13 +98,13 @@ void write_completion_file(void) { FILE *fptr; - char *dir = NULL; + char *dir = NULL; char *completion_file = NULL; crt_env_get(DAOS_TEST_SHARED_DIR, &dir); D_ASSERTF(dir != NULL, - "DAOS_TEST_SHARED_DIR must be set for --write_completion_file option.\n"); + "DAOS_TEST_SHARED_DIR must be set for --write_completion_file option.\n"); D_ASPRINTF(completion_file, "%s/test-servers-completed.txt.%d", dir, getpid()); D_ASSERTF(completion_file != NULL, "Error allocating completion_file string\n"); @@ -414,8 +414,8 @@ crtu_dc_mgmt_net_cfg_setenv(const char *name) { int rc; char *provider; - int cli_srx_set = 0; - int crt_timeout = 0; + int cli_srx_set = 0; + int crt_timeout = 0; char *d_interface; char *d_interface_env = NULL; char *d_domain; @@ -453,7 +453,8 @@ crtu_dc_mgmt_net_cfg_setenv(const char *name) crt_env_get(FI_OFI_RXM_USE_SRX, &cli_srx_set); if (cli_srx_set) { D_ERROR("Client set FI_OFI_RXM_USE_SRX to %d, " - "but server is unset!\n", cli_srx_set); + "but server is unset!\n", + cli_srx_set); D_GOTO(cleanup, rc = -DER_INVAL); } } @@ -518,8 +519,8 @@ crtu_cli_start_basic(char *local_group_name, char *srv_group_name, unsigned int total_srv_ctx, bool use_cfg, crt_init_options_t *init_opt, bool use_daos_agent_env) { - char *grp_cfg_file = NULL; - uint32_t grp_size = 0; + char *grp_cfg_file = NULL; + uint32_t grp_size = 0; int rc = 0; if (opts.assert_on_error) @@ -581,8 +582,7 @@ crtu_cli_start_basic(char *local_group_name, char *srv_group_name, /* load group info from a config file and * delete file upon return */ - rc = crtu_load_group_from_file(grp_cfg_file, - *crt_ctx, *grp, -1, true); + rc = crtu_load_group_from_file(grp_cfg_file, *crt_ctx, *grp, -1, true); if (rc != 0) D_GOTO(out, rc); } @@ -644,9 +644,9 @@ crtu_srv_start_basic(char *srv_group_name, crt_context_t *crt_ctx, pthread_t *progress_thread, crt_group_t **grp, uint32_t *grp_size, crt_init_options_t *init_opt) { - char *grp_cfg_file = NULL; - char *my_uri = NULL; - d_rank_t my_rank = CRT_NO_RANK; + char *grp_cfg_file = NULL; + char *my_uri = NULL; + d_rank_t my_rank = CRT_NO_RANK; int rc = 0; if (opts.assert_on_error) From 304b8bd02e2df29fc9fae67079693e9f61c8017b Mon Sep 17 00:00:00 2001 From: Alexander A Oganezov Date: Tue, 4 Jun 2024 05:08:27 +0000 Subject: [PATCH 03/10] - fix build errors, memset g_envs to 0 Required-githooks: true Signed-off-by: Alexander A Oganezov --- src/cart/crt_init.c | 24 ++++++++++++------------ src/cart/crt_internal_types.h | 1 + 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/cart/crt_init.c b/src/cart/crt_init.c index 22db4d60c27..352163cd58b 100644 --- a/src/cart/crt_init.c +++ b/src/cart/crt_init.c @@ -519,23 +519,23 @@ crt_protocol_info_free(struct crt_protocol_info *protocol_info) int crt_init_opt(crt_group_id_t grpid, uint32_t flags, crt_init_options_t *opt) { - char *provider; - char *provider_env = NULL; - char *interface; - char *interface_env = NULL; - char *domain; - char *domain_env = NULL; - char *auth_key; - char *auth_key_env = NULL; - char *path; bool server = flags & CRT_FLAG_BIT_SERVER; int rc = 0; - char *provider_str0 = NULL; - char *provider_str1 = NULL; crt_provider_t primary_provider; crt_provider_t secondary_provider; crt_provider_t tmp_prov; - char *port; + char *provider = NULL; + char *provider_env = NULL; + char *interface = NULL; + char *interface_env = NULL; + char *domain = NULL; + char *domain_env = NULL; + char *auth_key = NULL; + char *auth_key_env = NULL; + char *path = NULL; + char *provider_str0 = NULL; + char *provider_str1 = NULL; + char *port = NULL; char *port_env = NULL; char *port0 = NULL; char *port1 = NULL; diff --git a/src/cart/crt_internal_types.h b/src/cart/crt_internal_types.h index bb72591efd0..6776ab046e9 100644 --- a/src/cart/crt_internal_types.h +++ b/src/cart/crt_internal_types.h @@ -242,6 +242,7 @@ extern struct crt_envs_t g_envs; static inline void crt_env_init(void) { + memset(&g_envs, 0x0, sizeof(struct crt_envs_t)); #define ENV(x) \ do { \ g_envs._rc_##x = d_getenv_uint(#x, &g_envs._##x); \ From 5499d54da9f64e1536f46a92774d9aad8328ea8d Mon Sep 17 00:00:00 2001 From: Alexander A Oganezov Date: Tue, 4 Jun 2024 18:53:38 +0000 Subject: [PATCH 04/10] fix bug causing cart test servers to be started with wrong ranks - add flag to indicate whether g_envs was inited - assert on crt_env_get() if called when env is not inited - move crt_env_get() in utils after crt_init() Required-githooks: true Signed-off-by: Alexander A Oganezov --- src/cart/crt_init.c | 5 ++++- src/cart/crt_internal_types.h | 12 +++++++++++- src/cart/utils/crt_utils.c | 9 ++++++--- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/cart/crt_init.c b/src/cart/crt_init.c index 352163cd58b..d4e9b212978 100644 --- a/src/cart/crt_init.c +++ b/src/cart/crt_init.c @@ -53,6 +53,9 @@ crt_lib_init(void) crt_gdata.cg_rpcid = start_rpcid; crt_gdata.cg_num_cores = sysconf(_SC_NPROCESSORS_ONLN); crt_gdata.cg_iv_inline_limit = 19456; /* 19KB */ + + /* envs not inited until crt_init() time */ + memset(&g_envs, 0x0, sizeof(struct crt_envs_t)); } /* Library deinit */ @@ -560,7 +563,7 @@ crt_init_opt(crt_group_id_t grpid, uint32_t flags, crt_init_options_t *opt) crt_setup_log_fac(); - D_INFO("libcart version %s initializing\n", CART_VERSION); + D_INFO("libcart (%s) v%s initializing\n", server ? "server" : "client", CART_VERSION); crt_env_init(); if (opt) diff --git a/src/cart/crt_internal_types.h b/src/cart/crt_internal_types.h index 6776ab046e9..57c279c070d 100644 --- a/src/cart/crt_internal_types.h +++ b/src/cart/crt_internal_types.h @@ -231,6 +231,7 @@ struct crt_event_cb_priv { struct crt_envs_t { CRT_ENV_LIST; + bool inited; }; #undef ENV @@ -238,11 +239,15 @@ struct crt_envs_t { #undef ENV_STR_NO_PRINT extern struct crt_envs_t g_envs; +static inline void crt_env_fini(void); static inline void crt_env_init(void) { - memset(&g_envs, 0x0, sizeof(struct crt_envs_t)); + /* release strings if already inited previously */ + if (g_envs.inited) + crt_env_fini(); + #define ENV(x) \ do { \ g_envs._rc_##x = d_getenv_uint(#x, &g_envs._##x); \ @@ -265,6 +270,8 @@ crt_env_init(void) #undef ENV #undef ENV_STR #undef ENV_STR_NO_PRINT + + g_envs.inited = true; } static inline void @@ -279,10 +286,13 @@ crt_env_fini(void) #undef ENV #undef ENV_STR #undef ENV_STR_NO_PRINT + + g_envs.inited = false; } /* Returns value if env was present at load time */ #define crt_env_get(name, val) \ + D_ASSERT(g_envs.inited); \ if (g_envs._rc_##name == 0) \ *val = g_envs._##name; diff --git a/src/cart/utils/crt_utils.c b/src/cart/utils/crt_utils.c index a48d80f11bf..7e3ed2b80d8 100644 --- a/src/cart/utils/crt_utils.c +++ b/src/cart/utils/crt_utils.c @@ -652,9 +652,6 @@ crtu_srv_start_basic(char *srv_group_name, crt_context_t *crt_ctx, if (opts.assert_on_error) D_ASSERTF(opts.is_initialized == true, "crtu_test_init not called.\n"); - crt_env_get(CRT_L_RANK, &my_rank); - D_ASSERTF(rc == DER_SUCCESS, "Rank can not be retrieve: " DF_RC "\n", DP_RC(rc)); - rc = d_log_init(); if (rc != 0) D_GOTO(out, rc); @@ -670,6 +667,12 @@ crtu_srv_start_basic(char *srv_group_name, crt_context_t *crt_ctx, if (rc != 0) D_GOTO(out, rc); + crt_env_get(CRT_L_RANK, &my_rank); + if (my_rank == CRT_NO_RANK) { + D_ERROR("CRT_L_RANK environment variable should have been set by crt_launch\n"); + D_GOTO(out, rc = -DER_INVAL); + } + *grp = crt_group_lookup(NULL); if (!(*grp)) { D_ERROR("Group lookup failed\n"); From 1f8a70649b3a33c1124ce9a764e6898f3cf42405 Mon Sep 17 00:00:00 2001 From: Alexander A Oganezov Date: Tue, 4 Jun 2024 20:51:42 +0000 Subject: [PATCH 05/10] formatting Required-githooks: true Signed-off-by: Alexander A Oganezov --- src/cart/crt_internal_types.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/cart/crt_internal_types.h b/src/cart/crt_internal_types.h index 57c279c070d..75ca1a330da 100644 --- a/src/cart/crt_internal_types.h +++ b/src/cart/crt_internal_types.h @@ -239,7 +239,8 @@ struct crt_envs_t { #undef ENV_STR_NO_PRINT extern struct crt_envs_t g_envs; -static inline void crt_env_fini(void); +static inline void +crt_env_fini(void); static inline void crt_env_init(void) @@ -292,7 +293,7 @@ crt_env_fini(void) /* Returns value if env was present at load time */ #define crt_env_get(name, val) \ - D_ASSERT(g_envs.inited); \ + D_ASSERT(g_envs.inited); \ if (g_envs._rc_##name == 0) \ *val = g_envs._##name; From 6d3563d0eab912ab07266fa8f5c59cccecc6a53e Mon Sep 17 00:00:00 2001 From: Alexander A Oganezov Date: Wed, 5 Jun 2024 08:47:39 +0000 Subject: [PATCH 06/10] - remove wrong d_freenv, causing segfaults in ci testing Required-githooks: true Signed-off-by: Alexander A Oganezov --- src/cart/crt_init.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/cart/crt_init.c b/src/cart/crt_init.c index d4e9b212978..6cef1c0c814 100644 --- a/src/cart/crt_init.c +++ b/src/cart/crt_init.c @@ -606,7 +606,6 @@ crt_init_opt(crt_group_id_t grpid, uint32_t flags, crt_init_options_t *opt) else D_DEBUG(DB_ALL, "set group_config_path as %s.\n", path); } - d_freeenv_str(&path); if (opt && opt->cio_auth_key) auth_key = opt->cio_auth_key; From 96e6794b3df3d4d3b7ab64a69ad8607147d33b12 Mon Sep 17 00:00:00 2001 From: Alexander A Oganezov Date: Wed, 5 Jun 2024 23:46:05 +0000 Subject: [PATCH 07/10] - Change cart utility function to populate crt_init_options_t instead of setting and querying env. Required-githooks: true Signed-off-by: Alexander A Oganezov --- src/cart/utils/crt_utils.c | 119 +++++++++++--------------------- src/cart/utils/crt_utils.h | 4 +- src/utils/self_test/self_test.c | 15 +++- 3 files changed, 57 insertions(+), 81 deletions(-) diff --git a/src/cart/utils/crt_utils.c b/src/cart/utils/crt_utils.c index 7e3ed2b80d8..b631611cef6 100644 --- a/src/cart/utils/crt_utils.c +++ b/src/cart/utils/crt_utils.c @@ -410,22 +410,21 @@ crtu_dc_mgmt_net_cfg_rank_add(const char *name, crt_group_t *group, } int -crtu_dc_mgmt_net_cfg_setenv(const char *name) +crtu_dc_mgmt_net_cfg_setenv(const char *name, crt_init_options_t *opt) { int rc; - char *provider; int cli_srx_set = 0; - int crt_timeout = 0; - char *d_interface; - char *d_interface_env = NULL; - char *d_domain; - char *d_domain_env = NULL; struct dc_mgmt_sys_info crt_net_cfg_info = {0}; + Mgmt__GetAttachInfoResp *crt_net_cfg_resp = NULL; + if (opt == NULL) { + D_ERROR("Wrong NULL opt\n"); + return -DER_INVAL; + } + /* Query the agent for the CaRT network configuration parameters */ - rc = dc_get_attach_info(name, true /* all_ranks */, - &crt_net_cfg_info, &crt_net_cfg_resp); + rc = dc_get_attach_info(name, true, &crt_net_cfg_info, &crt_net_cfg_resp); if (opts.assert_on_error) D_ASSERTF(rc == 0, "dc_get_attach_info() failed, rc=%d\n", rc); @@ -434,80 +433,40 @@ crtu_dc_mgmt_net_cfg_setenv(const char *name) D_GOTO(cleanup, rc); } - /* These two are always set */ - provider = crt_net_cfg_info.provider; - D_INFO("setenv D_PROVIDER=%s\n", provider); - rc = d_setenv("D_PROVIDER", provider, 1); - if (rc != 0) - D_GOTO(cleanup, rc = d_errno2der(errno)); + D_STRNDUP(opt->cio_provider, crt_net_cfg_info.provider, DAOS_SYS_INFO_STRING_MAX); + D_STRNDUP(opt->cio_interface, crt_net_cfg_info.interface, DAOS_SYS_INFO_STRING_MAX); - /* If the server has set this, the client must use the same value. */ - if (crt_net_cfg_info.srv_srx_set != -1) { - cli_srx_set = crt_net_cfg_info.srv_srx_set; - D_INFO("setenv FI_OFI_RXM_USE_SRX=%d\n", cli_srx_set); - rc = d_setenv("FI_OFI_RXM_USE_SRX", cli_srx_set ? "1" : "0", 1); - if (rc != 0) - D_GOTO(cleanup, rc = d_errno2der(errno)); - } else { - /* Client may not set it if the server hasn't. */ - crt_env_get(FI_OFI_RXM_USE_SRX, &cli_srx_set); - if (cli_srx_set) { - D_ERROR("Client set FI_OFI_RXM_USE_SRX to %d, " - "but server is unset!\n", - cli_srx_set); - D_GOTO(cleanup, rc = -DER_INVAL); - } - } - - /* Allow client env overrides for these three */ - crt_env_get(CRT_TIMEOUT, &crt_timeout); - if (!crt_timeout) { - char *tmp; - crt_timeout = crt_net_cfg_info.crt_timeout; - D_INFO("setenv CRT_TIMEOUT=%d\n", crt_timeout); - - rc = asprintf(&tmp, "%d", crt_net_cfg_info.crt_timeout); - if (rc < 0) + /* Domain is optional */ + if (crt_net_cfg_info.domain) { + D_STRNDUP(opt->cio_domain, crt_net_cfg_info.domain, DAOS_SYS_INFO_STRING_MAX); + if (opt->cio_domain == NULL) D_GOTO(cleanup, rc = -DER_NOMEM); - - rc = d_setenv("CRT_TIMEOUT", tmp, 1); - free(tmp); - if (rc != 0) - D_GOTO(cleanup, rc = d_errno2der(errno)); - } else { - D_DEBUG(DB_MGMT, "Using client provided CRT_TIMEOUT: %d\n", crt_timeout); } - crt_env_get(D_INTERFACE, &d_interface_env); - if (!d_interface_env) { - d_interface = crt_net_cfg_info.interface; - D_INFO("Setting D_INTERFACE=%s\n", d_interface); - rc = d_setenv("D_INTERFACE", d_interface, 1); - if (rc != 0) - D_GOTO(cleanup, rc = d_errno2der(errno)); - } else { - d_interface = d_interface_env; - D_DEBUG(DB_MGMT, "Using client provided D_INTERFACE: %s\n", d_interface); + /* Interface and provider must be set */ + if (opt->cio_provider == NULL || opt->cio_interface == NULL) { + D_ERROR("Invalid provider/interface : %s/%s\n", + opt->cio_provider, opt->cio_interface); + D_GOTO(cleanup, rc = -DER_NOMEM); } - crt_env_get(D_DOMAIN, &d_domain_env); - if (!d_domain_env) { - d_domain = crt_net_cfg_info.domain; - D_INFO("Setting D_DOMAIN=%s\n", d_domain); - rc = d_setenv("D_DOMAIN", d_domain, 1); - if (rc != 0) - D_GOTO(cleanup, rc = d_errno2der(errno)); - } else { - d_domain = d_domain_env; - D_DEBUG(DB_MGMT, "Using client provided D_DOMAIN: %s\n", d_domain); - } + /* If the server has set this, the client must use the same value. */ + if (crt_net_cfg_info.srv_srx_set != -1) + cli_srx_set = crt_net_cfg_info.srv_srx_set; + else + cli_srx_set = 0; + + rc = d_setenv("FI_OFI_RXM_USE_SRX", cli_srx_set ? "1" : "0", 1); + if (rc != 0) + D_GOTO(cleanup, rc = d_errno2der(errno)); - D_INFO("CaRT env setup with:\n" - "\tD_INTERFACE=%s, D_DOMAIN: %s, D_PROVIDER: %s, CRT_TIMEOUT: %d\n", - d_interface, d_domain, provider, crt_timeout); + opt->cio_crt_timeout = crt_net_cfg_info.crt_timeout; cleanup: dc_put_attach_info(&crt_net_cfg_info, crt_net_cfg_resp); + D_FREE(opt->cio_provider); + D_FREE(opt->cio_interface); + D_FREE(opt->cio_domain); return rc; } @@ -519,9 +478,11 @@ crtu_cli_start_basic(char *local_group_name, char *srv_group_name, unsigned int total_srv_ctx, bool use_cfg, crt_init_options_t *init_opt, bool use_daos_agent_env) { - char *grp_cfg_file = NULL; - uint32_t grp_size = 0; - int rc = 0; + char *grp_cfg_file = NULL; + uint32_t grp_size = 0; + int rc = 0; + crt_init_options_t local_opt = {0}; + if (opts.assert_on_error) D_ASSERTF(opts.is_initialized == true, "crtu_test_init not called.\n"); @@ -531,9 +492,10 @@ crtu_cli_start_basic(char *local_group_name, char *srv_group_name, D_GOTO(out, rc); if (use_daos_agent_env) { - rc = crtu_dc_mgmt_net_cfg_setenv(srv_group_name); + rc = crtu_dc_mgmt_net_cfg_setenv(srv_group_name, &local_opt); if (rc != 0) D_GOTO(out, rc); + init_opt = &local_opt; } if (init_opt) @@ -636,6 +598,9 @@ crtu_cli_start_basic(char *local_group_name, char *srv_group_name, assert(0); } + D_FREE(local_opt.cio_provider); + D_FREE(local_opt.cio_interface); + D_FREE(local_opt.cio_domain); return rc; } diff --git a/src/cart/utils/crt_utils.h b/src/cart/utils/crt_utils.h index 6f32c531224..fb65947fac9 100644 --- a/src/cart/utils/crt_utils.h +++ b/src/cart/utils/crt_utils.h @@ -1,5 +1,5 @@ /* - * (C) Copyright 2019-2022 Intel Corporation. + * (C) Copyright 2019-2024 Intel Corporation. * * SPDX-License-Identifier: BSD-2-Clause-Patent */ @@ -75,7 +75,7 @@ int crtu_dc_mgmt_net_cfg_rank_add(const char *name, crt_group_t *group, crt_context_t *context); int -crtu_dc_mgmt_net_cfg_setenv(const char *name); +crtu_dc_mgmt_net_cfg_setenv(const char *name, crt_init_options_t *opt); int crtu_cli_start_basic(char *local_group_name, char *srv_group_name, diff --git a/src/utils/self_test/self_test.c b/src/utils/self_test/self_test.c index 6e06af59bd0..633b8048957 100644 --- a/src/utils/self_test/self_test.c +++ b/src/utils/self_test/self_test.c @@ -95,6 +95,9 @@ self_test_init(char *dest_name, crt_context_t *crt_ctx, crt_group_t **srv_grp, p int i; d_rank_t max_rank = 0; int ret; + crt_init_options_t opt = {0}; + crt_init_options_t *init_opt; + /* rank, num_attach_retries, is_server, assert_on_error */ crtu_test_init(0, attach_retries, false, false); @@ -105,19 +108,27 @@ self_test_init(char *dest_name, crt_context_t *crt_ctx, crt_group_t **srv_grp, p fprintf(stderr, "dc_agent_init() failed. ret: %d\n", ret); return ret; } - ret = crtu_dc_mgmt_net_cfg_setenv(dest_name); + ret = crtu_dc_mgmt_net_cfg_setenv(dest_name, &opt); if (ret != 0) { D_ERROR("crtu_dc_mgmt_net_cfg_setenv() failed; ret = %d\n", ret); return ret; } + + init_opt = &opt; + } else { + init_opt = NULL; } if (listen) init_flags |= (CRT_FLAG_BIT_SERVER | CRT_FLAG_BIT_AUTO_SWIM_DISABLE); - ret = crt_init(CRT_SELF_TEST_GROUP_NAME, init_flags); + + ret = crt_init_opt(CRT_SELF_TEST_GROUP_NAME, init_flags, init_opt); if (ret != 0) return ret; + D_FREE(opt.cio_provider); + D_FREE(opt.cio_interface); + D_FREE(opt.cio_domain); g_cart_inited = true; if (attach_info_path) { From de5074fd064c7a8c9e14b12af4061a88f94f251a Mon Sep 17 00:00:00 2001 From: Alexander A Oganezov Date: Wed, 5 Jun 2024 23:46:48 +0000 Subject: [PATCH 08/10] style Required-githooks: true Signed-off-by: Alexander A Oganezov --- src/cart/utils/crt_utils.c | 15 +++++++-------- src/utils/self_test/self_test.c | 3 +-- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/cart/utils/crt_utils.c b/src/cart/utils/crt_utils.c index b631611cef6..5b310d143cb 100644 --- a/src/cart/utils/crt_utils.c +++ b/src/cart/utils/crt_utils.c @@ -413,7 +413,7 @@ int crtu_dc_mgmt_net_cfg_setenv(const char *name, crt_init_options_t *opt) { int rc; - int cli_srx_set = 0; + int cli_srx_set = 0; struct dc_mgmt_sys_info crt_net_cfg_info = {0}; Mgmt__GetAttachInfoResp *crt_net_cfg_resp = NULL; @@ -445,8 +445,8 @@ crtu_dc_mgmt_net_cfg_setenv(const char *name, crt_init_options_t *opt) /* Interface and provider must be set */ if (opt->cio_provider == NULL || opt->cio_interface == NULL) { - D_ERROR("Invalid provider/interface : %s/%s\n", - opt->cio_provider, opt->cio_interface); + D_ERROR("Invalid provider/interface : %s/%s\n", opt->cio_provider, + opt->cio_interface); D_GOTO(cleanup, rc = -DER_NOMEM); } @@ -478,11 +478,10 @@ crtu_cli_start_basic(char *local_group_name, char *srv_group_name, unsigned int total_srv_ctx, bool use_cfg, crt_init_options_t *init_opt, bool use_daos_agent_env) { - char *grp_cfg_file = NULL; - uint32_t grp_size = 0; - int rc = 0; - crt_init_options_t local_opt = {0}; - + char *grp_cfg_file = NULL; + uint32_t grp_size = 0; + int rc = 0; + crt_init_options_t local_opt = {0}; if (opts.assert_on_error) D_ASSERTF(opts.is_initialized == true, "crtu_test_init not called.\n"); diff --git a/src/utils/self_test/self_test.c b/src/utils/self_test/self_test.c index 633b8048957..8cb354d2778 100644 --- a/src/utils/self_test/self_test.c +++ b/src/utils/self_test/self_test.c @@ -95,10 +95,9 @@ self_test_init(char *dest_name, crt_context_t *crt_ctx, crt_group_t **srv_grp, p int i; d_rank_t max_rank = 0; int ret; - crt_init_options_t opt = {0}; + crt_init_options_t opt = {0}; crt_init_options_t *init_opt; - /* rank, num_attach_retries, is_server, assert_on_error */ crtu_test_init(0, attach_retries, false, false); From e32ffbbfa22fd0d9dcb0852df502f9cf3e69d242 Mon Sep 17 00:00:00 2001 From: Alexander A Oganezov Date: Thu, 6 Jun 2024 00:12:03 +0000 Subject: [PATCH 09/10] target-c warning fix Required-githooks: true Signed-off-by: Alexander A Oganezov --- src/cart/utils/crt_utils.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/cart/utils/crt_utils.c b/src/cart/utils/crt_utils.c index 5b310d143cb..90d3861085d 100644 --- a/src/cart/utils/crt_utils.c +++ b/src/cart/utils/crt_utils.c @@ -435,20 +435,10 @@ crtu_dc_mgmt_net_cfg_setenv(const char *name, crt_init_options_t *opt) D_STRNDUP(opt->cio_provider, crt_net_cfg_info.provider, DAOS_SYS_INFO_STRING_MAX); D_STRNDUP(opt->cio_interface, crt_net_cfg_info.interface, DAOS_SYS_INFO_STRING_MAX); + D_STRNDUP(opt->cio_domain, crt_net_cfg_info.domain, DAOS_SYS_INFO_STRING_MAX); - /* Domain is optional */ - if (crt_net_cfg_info.domain) { - D_STRNDUP(opt->cio_domain, crt_net_cfg_info.domain, DAOS_SYS_INFO_STRING_MAX); - if (opt->cio_domain == NULL) - D_GOTO(cleanup, rc = -DER_NOMEM); - } - - /* Interface and provider must be set */ - if (opt->cio_provider == NULL || opt->cio_interface == NULL) { - D_ERROR("Invalid provider/interface : %s/%s\n", opt->cio_provider, - opt->cio_interface); + if (!opt->cio_provider || !opt->cio_interface || !opt->cio_domain) D_GOTO(cleanup, rc = -DER_NOMEM); - } /* If the server has set this, the client must use the same value. */ if (crt_net_cfg_info.srv_srx_set != -1) From 9e1107914d1af7d05ffa81fd74652949f2c5823d Mon Sep 17 00:00:00 2001 From: Alexander A Oganezov Date: Sat, 8 Jun 2024 00:58:08 +0000 Subject: [PATCH 10/10] - address review feedback Required-githooks: true Signed-off-by: Alexander A Oganezov --- src/cart/crt_init.c | 4 ++-- src/cart/crt_internal_types.h | 39 ++++++++++++++++++----------------- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/cart/crt_init.c b/src/cart/crt_init.c index 6cef1c0c814..dea3731873e 100644 --- a/src/cart/crt_init.c +++ b/src/cart/crt_init.c @@ -13,7 +13,7 @@ #include "crt_internal.h" struct crt_gdata crt_gdata; -struct crt_envs_t g_envs; +struct crt_envs crt_genvs; static volatile int gdata_init_flag; struct crt_plugin_gdata crt_plugin_gdata; static bool g_prov_settings_applied[CRT_PROV_COUNT]; @@ -55,7 +55,7 @@ crt_lib_init(void) crt_gdata.cg_iv_inline_limit = 19456; /* 19KB */ /* envs not inited until crt_init() time */ - memset(&g_envs, 0x0, sizeof(struct crt_envs_t)); + memset(&crt_genvs, 0x0, sizeof(crt_genvs)); } /* Library deinit */ diff --git a/src/cart/crt_internal_types.h b/src/cart/crt_internal_types.h index 75ca1a330da..bfa2e5f5b39 100644 --- a/src/cart/crt_internal_types.h +++ b/src/cart/crt_internal_types.h @@ -229,7 +229,7 @@ struct crt_event_cb_priv { #define ENV_STR_NO_PRINT(x) ENV_STR(x) -struct crt_envs_t { +struct crt_envs { CRT_ENV_LIST; bool inited; }; @@ -238,7 +238,8 @@ struct crt_envs_t { #undef ENV_STR #undef ENV_STR_NO_PRINT -extern struct crt_envs_t g_envs; +extern struct crt_envs crt_genvs; + static inline void crt_env_fini(void); @@ -246,25 +247,25 @@ static inline void crt_env_init(void) { /* release strings if already inited previously */ - if (g_envs.inited) + if (crt_genvs.inited) crt_env_fini(); #define ENV(x) \ do { \ - g_envs._rc_##x = d_getenv_uint(#x, &g_envs._##x); \ - g_envs._no_print_##x = 0; \ + crt_genvs._rc_##x = d_getenv_uint(#x, &crt_genvs._##x); \ + crt_genvs._no_print_##x = 0; \ } while (0); #define ENV_STR(x) \ do { \ - g_envs._rc_##x = d_agetenv_str(&g_envs._##x, #x); \ - g_envs._no_print_##x = 0; \ + crt_genvs._rc_##x = d_agetenv_str(&crt_genvs._##x, #x); \ + crt_genvs._no_print_##x = 0; \ } while (0); #define ENV_STR_NO_PRINT(x) \ do { \ - g_envs._rc_##x = d_agetenv_str(&g_envs._##x, #x); \ - g_envs._no_print_##x = 1; \ + crt_genvs._rc_##x = d_agetenv_str(&crt_genvs._##x, #x); \ + crt_genvs._no_print_##x = 1; \ } while (0); CRT_ENV_LIST; @@ -272,14 +273,14 @@ crt_env_init(void) #undef ENV_STR #undef ENV_STR_NO_PRINT - g_envs.inited = true; + crt_genvs.inited = true; } static inline void crt_env_fini(void) { #define ENV(x) (void) -#define ENV_STR(x) d_freeenv_str(&g_envs._##x); +#define ENV_STR(x) d_freeenv_str(&crt_genvs._##x); #define ENV_STR_NO_PRINT ENV_STR CRT_ENV_LIST @@ -288,14 +289,14 @@ crt_env_fini(void) #undef ENV_STR #undef ENV_STR_NO_PRINT - g_envs.inited = false; + crt_genvs.inited = false; } /* Returns value if env was present at load time */ #define crt_env_get(name, val) \ - D_ASSERT(g_envs.inited); \ - if (g_envs._rc_##name == 0) \ - *val = g_envs._##name; + D_ASSERT(crt_genvs.inited); \ + if (crt_genvs._rc_##name == 0) \ + *val = crt_genvs._##name; static inline void crt_env_dump(void) @@ -304,12 +305,12 @@ crt_env_dump(void) /* Only dump envariables that were set */ #define ENV(x) \ - if (!g_envs._rc_##x && g_envs._no_print_##x == 0) \ - D_INFO("%s = %d\n", #x, g_envs._##x); + if (!crt_genvs._rc_##x && crt_genvs._no_print_##x == 0) \ + D_INFO("%s = %d\n", #x, crt_genvs._##x); #define ENV_STR(x) \ - if (!g_envs._rc_##x) \ - D_INFO("%s = %s\n", #x, g_envs._no_print_##x ? "****" : g_envs._##x); + if (!crt_genvs._rc_##x) \ + D_INFO("%s = %s\n", #x, crt_genvs._no_print_##x ? "****" : crt_genvs._##x); #define ENV_STR_NO_PRINT ENV_STR