From a6645f3f4c0b3e12a3f26203a898908a8277ddd7 Mon Sep 17 00:00:00 2001 From: James Simmons Date: Tue, 9 Apr 2024 13:23:44 -0400 Subject: [PATCH] LU-17638 utils: break up Netlink error handling In the current code when function yaml_netlink_msg_complete() calls yaml_netlink_msg_error() the arg becomes NULL. So break up yaml_netlink_msg_error() into two functions. One called by the netlink err callback and the other called directly by yaml_netlink_msg_complete(). Also change the libyaml read_handler_data to yaml_parser itself since its life cycle is outside the library itself so no chance of it disappear on us while executing library code. Fixes: d3ef8f6 ("LU-9680 lnet: add NLM_F_DUMP_FILTERED support") Test-Parameters: trivial testlist=sanity-lnet Change-Id: Iacb1e9c8929cd8a78a14580d909f94f2569fa5a3 Signed-off-by: James Simmons Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/54412 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Frank Sehr Reviewed-by: Arshad Hussain Reviewed-by: Oleg Drokin --- lnet/utils/lnetconfig/liblnetconfig_netlink.c | 42 ++++++---- lnet/utils/lnetctl.c | 108 +++++++++++++++++--------- 2 files changed, 99 insertions(+), 51 deletions(-) diff --git a/lnet/utils/lnetconfig/liblnetconfig_netlink.c b/lnet/utils/lnetconfig/liblnetconfig_netlink.c index 74a4b78..066679770 100644 --- a/lnet/utils/lnetconfig/liblnetconfig_netlink.c +++ b/lnet/utils/lnetconfig/liblnetconfig_netlink.c @@ -955,7 +955,8 @@ static bool cleanup_children(struct yaml_nl_node *parent) */ static int yaml_netlink_msg_parse(struct nl_msg *msg, void *arg) { - struct yaml_netlink_input *data = arg; + yaml_parser_t *parser = arg; + struct yaml_netlink_input *data = parser->read_handler_data; struct nlmsghdr *nlh = nlmsg_hdr(msg); if (nlh->nlmsg_flags & NLM_F_CREATE) { @@ -1036,11 +1037,10 @@ static int yaml_netlink_msg_parse(struct nl_msg *msg, void *arg) /* This is the libnl callback for when an error has happened * kernel side. An error message is sent back to the user. */ -static int yaml_netlink_msg_error(struct sockaddr_nl *who, - struct nlmsgerr *errmsg, void *arg) +static int yaml_netlink_parse_msg_error(struct nlmsgerr *errmsg, + yaml_parser_t *parser) { struct nlmsghdr *nlh = (void *)errmsg - NLMSG_HDRLEN; - struct yaml_netlink_input *data = arg; if ((nlh->nlmsg_type == NLMSG_ERROR || nlh->nlmsg_flags & NLM_F_ACK_TLVS) && errmsg->error) { @@ -1048,6 +1048,7 @@ static int yaml_netlink_msg_error(struct sockaddr_nl *who, * cache the source of the error. */ const char *errstr = nl_geterror(nl_syserr2nlerr(errmsg->error)); + struct yaml_netlink_input *data = parser->read_handler_data; #ifdef HAVE_USRSPC_NLMSGERR /* Newer kernels support NLM_F_ACK_TLVS in nlmsg_flags @@ -1065,11 +1066,24 @@ static int yaml_netlink_msg_error(struct sockaddr_nl *who, } } #endif /* HAVE_USRSPC_NLMSGERR */ + parser->error = YAML_READER_ERROR; + data = parser->read_handler_data; data->errmsg = errstr; data->error = errmsg->error; - data->parser->error = YAML_READER_ERROR; data->complete = true; } + + return parser->error; +} + +/* This is the libnl callback for when an error has happened + * kernel side. An error message is sent back to the user. + */ +static int yaml_netlink_msg_error(struct sockaddr_nl *who, + struct nlmsgerr *errmsg, void *arg) +{ + yaml_netlink_parse_msg_error(errmsg, (yaml_parser_t *)arg); + return NL_STOP; } @@ -1079,18 +1093,20 @@ static int yaml_netlink_msg_error(struct sockaddr_nl *who, */ static int yaml_netlink_msg_complete(struct nl_msg *msg, void *arg) { - struct yaml_netlink_input *data = arg; struct nlmsghdr *nlh = nlmsg_hdr(msg); + struct yaml_netlink_input *data; + yaml_parser_t *parser = arg; /* For the case of NLM_F_DUMP the kernel will send error msgs * yet not be labled NLMSG_ERROR which results in this code * path being executed. */ - yaml_netlink_msg_error(NULL, nlmsg_data(nlh), arg); - if (data->parser->error == YAML_READER_ERROR) + if (yaml_netlink_parse_msg_error(nlmsg_data(nlh), parser) == + YAML_READER_ERROR) return NL_STOP; /* Free internal data. */ + data = parser->read_handler_data; if (data->root) { cleanup_children(data->root); free(data->root); @@ -1218,10 +1234,10 @@ yaml_parser_set_input_netlink(yaml_parser_t *reply, struct nl_sock *nl, buf->nl = nl; buf->async = stream; buf->parser = reply; - yaml_parser_set_input(buf->parser, yaml_netlink_read_handler, buf); + yaml_parser_set_input(reply, yaml_netlink_read_handler, buf); rc = nl_socket_modify_cb(buf->nl, NL_CB_VALID, NL_CB_CUSTOM, - yaml_netlink_msg_parse, buf); + yaml_netlink_msg_parse, reply); if (rc < 0) { yaml_parser_set_reader_error(reply, "netlink msg recv setup failed", @@ -1230,7 +1246,7 @@ yaml_parser_set_input_netlink(yaml_parser_t *reply, struct nl_sock *nl, } rc = nl_socket_modify_cb(buf->nl, NL_CB_FINISH, NL_CB_CUSTOM, - yaml_netlink_msg_complete, buf); + yaml_netlink_msg_complete, reply); if (rc < 0) { yaml_parser_set_reader_error(reply, "netlink msg cleanup setup failed", @@ -1238,8 +1254,8 @@ yaml_parser_set_input_netlink(yaml_parser_t *reply, struct nl_sock *nl, goto failed; } - rc = nl_socket_modify_err_cb(nl, NL_CB_CUSTOM, yaml_netlink_msg_error, - buf); + rc = nl_socket_modify_err_cb(buf->nl, NL_CB_CUSTOM, yaml_netlink_msg_error, + reply); if (rc < 0) { yaml_parser_set_reader_error(reply, "failed to register error handling", diff --git a/lnet/utils/lnetctl.c b/lnet/utils/lnetctl.c index afab4f3..d47f11a 100644 --- a/lnet/utils/lnetctl.c +++ b/lnet/utils/lnetctl.c @@ -1437,7 +1437,7 @@ emitter_error: } static int yaml_lnet_route(char *nw, char *gw, int hops, int prio, int sen, - int version, int flags) + int version, int flags, FILE *fp) { struct nid_node head, *entry; struct nl_sock *sk = NULL; @@ -1586,8 +1586,7 @@ emitter_error: if (rc == 1) { yaml_emitter_set_indent(&debug, LNET_DEFAULT_INDENT); - yaml_emitter_set_output_file(&debug, - stdout); + yaml_emitter_set_output_file(&debug, fp); rc = yaml_emitter_dump(&debug, &errmsg); } yaml_emitter_delete(&debug); @@ -1675,7 +1674,7 @@ static int jt_add_route(int argc, char **argv) } rc = yaml_lnet_route(network, gateway, hop, prio, sen, - LNET_GENL_VERSION, NLM_F_CREATE); + LNET_GENL_VERSION, NLM_F_CREATE, stdout); if (rc <= 0) { if (rc == -EOPNOTSUPP) goto old_api; @@ -1927,7 +1926,7 @@ static int yaml_lnet_config_ni(char *net_id, char *ip2net, struct lnet_dlc_network_descr *nw_descr, struct lnet_ioctl_config_lnd_tunables *tunables, int healthv, struct cfs_expr_list *global_cpts, - int version, int flags) + int version, int flags, FILE *fp) { struct lnet_dlc_intf_descr *intf; struct nl_sock *sk = NULL; @@ -2289,7 +2288,7 @@ emitter_error: if (rc == 1) { yaml_emitter_set_indent(&debug, LNET_DEFAULT_INDENT); - yaml_emitter_set_output_file(&debug, stdout); + yaml_emitter_set_output_file(&debug, fp); rc = yaml_emitter_dump(&debug, &errmsg); } yaml_emitter_delete(&debug); @@ -2498,7 +2497,7 @@ static int jt_add_ni(int argc, char **argv) rc = yaml_lnet_config_ni(net_id, ip2net, &nw_descr, found ? &tunables : NULL, -1, (cpt_rc == 0) ? global_cpts : NULL, - LNET_GENL_VERSION, NLM_F_CREATE); + LNET_GENL_VERSION, NLM_F_CREATE, stdout); if (rc <= 0) { if (rc == -EOPNOTSUPP) goto old_api; @@ -2569,7 +2568,7 @@ static int jt_del_route(int argc, char **argv) } rc = yaml_lnet_route(network, gateway, -1, -1, -1, LNET_GENL_VERSION, - 0); + 0, stdout); if (rc <= 0) { if (rc == -EOPNOTSUPP) goto old_api; @@ -2648,7 +2647,7 @@ static int jt_del_ni(int argc, char **argv) } rc = yaml_lnet_config_ni(net_id, NULL, &nw_descr, NULL, -1, NULL, - LNET_GENL_VERSION, 0); + LNET_GENL_VERSION, 0, stdout); if (rc <= 0) { if (rc != -EOPNOTSUPP) return rc; @@ -2721,7 +2720,7 @@ static int jt_show_route(int argc, char **argv) } rc = yaml_lnet_route(network, gateway, hop, prio, -1, - detail, NLM_F_DUMP); + detail, NLM_F_DUMP, stdout); if (rc <= 0) { if (rc == -EOPNOTSUPP) goto old_api; @@ -2842,7 +2841,7 @@ int yaml_lnet_config_ni_healthv(int healthv, bool all, char *nidstr, int cpp, rc = yaml_lnet_config_ni(net_id, NULL, &nw_descr, cpp != -1 ? &tunables : NULL, healthv, NULL, - LNET_GENL_VERSION, NLM_F_REPLACE); + LNET_GENL_VERSION, NLM_F_REPLACE, stdout); if (rc <= 0) { if (rc == -EOPNOTSUPP) goto old_api; @@ -2976,7 +2975,7 @@ report_reply_error: static int yaml_lnet_peer(char *prim_nid, char *nidstr, bool disable_mr, int health_value, int state, bool list_only, - int version, int flags) + int version, int flags, FILE *fp) { struct nl_sock *sk = NULL; const char *msg = NULL; @@ -3371,7 +3370,7 @@ int yaml_lnet_config_peer_ni_healthv(int healthv, bool all, char *lpni_nid, rc = yaml_lnet_peer(lpni_nid ? lpni_nid : "", all ? "" : NULL, false, healthv, state, false, LNET_GENL_VERSION, - NLM_F_REPLACE); + NLM_F_REPLACE, stdout); if (rc <= 0) { if (rc == -EOPNOTSUPP) goto old_api; @@ -3512,7 +3511,7 @@ static int jt_show_net(int argc, char **argv) } rc = yaml_lnet_config_ni(network, NULL, NULL, NULL, -1, NULL, - detail, NLM_F_DUMP); + detail, NLM_F_DUMP, stdout); if (rc <= 0) { if (rc != -EOPNOTSUPP) return rc; @@ -4354,123 +4353,155 @@ static int jt_export(int argc, char **argv) return -1; } - rc = lustre_lnet_show_net(NULL, 2, -1, &show_rc, &err_rc, backup); + rc = yaml_lnet_config_ni(NULL, NULL, NULL, NULL, -1, NULL, + flags & NLM_F_DUMP_FILTERED ? 1 : 2, + flags, f); + if (rc < 0) { + if (rc == -EOPNOTSUPP) + goto old_api; + } + + rc = yaml_lnet_route(NULL, NULL, -1, -1, -1, LNET_GENL_VERSION, + flags, f); + if (rc < 0) { + if (rc == -EOPNOTSUPP) + goto old_route; + } + + rc = lustre_lnet_show_routing(-1, &show_rc, &err_rc, backup); if (rc != LUSTRE_CFG_RC_NO_ERR) { - cYAML_print_tree2file(stderr, err_rc); + cYAML_print_tree2file(f, err_rc); cYAML_free_tree(err_rc); err_rc = NULL; } + rc = yaml_lnet_peer(NULL, NULL, false, -1, false, false, + flags & NLM_F_DUMP_FILTERED ? 0 : 3, + flags, f); + if (rc < 0) { + if (rc == -EOPNOTSUPP) + goto old_peer; + } + goto show_others; + +old_api: + rc = lustre_lnet_show_net(NULL, 2, -1, &show_rc, &err_rc, backup); + if (rc != LUSTRE_CFG_RC_NO_ERR) { + cYAML_print_tree2file(f, err_rc); + cYAML_free_tree(err_rc); + err_rc = NULL; + } +old_route: rc = lustre_lnet_show_route(NULL, NULL, -1, -1, 1, -1, &show_rc, &err_rc, backup); if (rc != LUSTRE_CFG_RC_NO_ERR) { - cYAML_print_tree2file(stderr, err_rc); + cYAML_print_tree2file(f, err_rc); cYAML_free_tree(err_rc); err_rc = NULL; } rc = lustre_lnet_show_routing(-1, &show_rc, &err_rc, backup); if (rc != LUSTRE_CFG_RC_NO_ERR) { - cYAML_print_tree2file(stderr, err_rc); + cYAML_print_tree2file(f, err_rc); cYAML_free_tree(err_rc); err_rc = NULL; } - +old_peer: rc = lustre_lnet_show_peer(NULL, 2, -1, &show_rc, &err_rc, backup); if (rc != LUSTRE_CFG_RC_NO_ERR) { - cYAML_print_tree2file(stderr, err_rc); + cYAML_print_tree2file(f, err_rc); cYAML_free_tree(err_rc); err_rc = NULL; } - +show_others: rc = lustre_lnet_show_numa_range(-1, &show_rc, &err_rc); if (rc != LUSTRE_CFG_RC_NO_ERR) { - cYAML_print_tree2file(stderr, err_rc); + cYAML_print_tree2file(f, err_rc); cYAML_free_tree(err_rc); err_rc = NULL; } rc = lustre_lnet_show_max_intf(-1, &show_rc, &err_rc); if (rc != LUSTRE_CFG_RC_NO_ERR) { - cYAML_print_tree2file(stderr, err_rc); + cYAML_print_tree2file(f, err_rc); cYAML_free_tree(err_rc); err_rc = NULL; } rc = lustre_lnet_show_discovery(-1, &show_rc, &err_rc); if (rc != LUSTRE_CFG_RC_NO_ERR) { - cYAML_print_tree2file(stderr, err_rc); + cYAML_print_tree2file(f, err_rc); cYAML_free_tree(err_rc); err_rc = NULL; } rc = lustre_lnet_show_drop_asym_route(-1, &show_rc, &err_rc); if (rc != LUSTRE_CFG_RC_NO_ERR) { - cYAML_print_tree2file(stderr, err_rc); + cYAML_print_tree2file(f, err_rc); cYAML_free_tree(err_rc); err_rc = NULL; } rc = lustre_lnet_show_retry_count(-1, &show_rc, &err_rc); if (rc != LUSTRE_CFG_RC_NO_ERR) { - cYAML_print_tree2file(stderr, err_rc); + cYAML_print_tree2file(f, err_rc); err_rc = NULL; } rc = lustre_lnet_show_transaction_to(-1, &show_rc, &err_rc); if (rc != LUSTRE_CFG_RC_NO_ERR) { - cYAML_print_tree2file(stderr, err_rc); + cYAML_print_tree2file(f, err_rc); err_rc = NULL; } rc = lustre_lnet_show_hsensitivity(-1, &show_rc, &err_rc); if (rc != LUSTRE_CFG_RC_NO_ERR) { - cYAML_print_tree2file(stderr, err_rc); + cYAML_print_tree2file(f, err_rc); err_rc = NULL; } rc = lustre_lnet_show_recov_intrv(-1, &show_rc, &err_rc); if (rc != LUSTRE_CFG_RC_NO_ERR) { - cYAML_print_tree2file(stderr, err_rc); + cYAML_print_tree2file(f, err_rc); err_rc = NULL; } rc = lustre_lnet_show_rtr_sensitivity(-1, &show_rc, &err_rc); if (rc != LUSTRE_CFG_RC_NO_ERR) { - cYAML_print_tree2file(stderr, err_rc); + cYAML_print_tree2file(f, err_rc); err_rc = NULL; } rc = lustre_lnet_show_lnd_timeout(-1, &show_rc, &err_rc); if (rc != LUSTRE_CFG_RC_NO_ERR) { - cYAML_print_tree2file(stderr, err_rc); + cYAML_print_tree2file(f, err_rc); err_rc = NULL; } rc = lustre_lnet_show_response_tracking(-1, &show_rc, &err_rc); if (rc != LUSTRE_CFG_RC_NO_ERR) { - cYAML_print_tree2file(stderr, err_rc); + cYAML_print_tree2file(f, err_rc); cYAML_free_tree(err_rc); err_rc = NULL; } rc = lustre_lnet_show_recovery_limit(-1, &show_rc, &err_rc); if (rc != LUSTRE_CFG_RC_NO_ERR) { - cYAML_print_tree2file(stderr, err_rc); + cYAML_print_tree2file(f, err_rc); cYAML_free_tree(err_rc); err_rc = NULL; } rc = lustre_lnet_show_max_recovery_ping_interval(-1, &show_rc, &err_rc); if (rc != LUSTRE_CFG_RC_NO_ERR) { - cYAML_print_tree2file(stderr, err_rc); + cYAML_print_tree2file(f, err_rc); cYAML_free_tree(err_rc); err_rc = NULL; } rc = lustre_lnet_show_udsp(-1, -1, &show_rc, &err_rc); if (rc != LUSTRE_CFG_RC_NO_ERR) { - cYAML_print_tree2file(stderr, err_rc); + cYAML_print_tree2file(f, err_rc); cYAML_free_tree(err_rc); err_rc = NULL; } @@ -4554,7 +4585,7 @@ static int jt_peer_nid_common(int argc, char **argv, int cmd) } rc = yaml_lnet_peer(prim_nid, nidstr, !is_mr, -1, -1, false, - LNET_GENL_VERSION, flags); + LNET_GENL_VERSION, flags, stdout); if (rc <= 0) { if (rc == -EOPNOTSUPP) goto old_api; @@ -4631,7 +4662,7 @@ static int jt_show_peer(int argc, char **argv) } rc = yaml_lnet_peer(nid, NULL, false, -1, -1, false, detail, - NLM_F_DUMP); + NLM_F_DUMP, stdout); if (rc <= 0) { if (rc == -EOPNOTSUPP) goto old_api; @@ -4661,7 +4692,8 @@ static int jt_list_peer(int argc, char **argv) if (rc) return rc; - rc = yaml_lnet_peer(NULL, NULL, false, -1, -1, true, 0, NLM_F_DUMP); + rc = yaml_lnet_peer(NULL, NULL, false, -1, -1, true, 0, NLM_F_DUMP, + stdout); if (rc <= 0) { if (rc == -EOPNOTSUPP) goto old_api; -- 1.8.3.1