From: Renato Westphal Date: Fri, 15 May 2020 00:34:12 +0000 (-0300) Subject: *: convert northbound callbacks to new error handling model X-Git-Tag: base_7.5~291^2~4 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=10bdc68f0c13e;p=mirror%2Ffrr.git *: convert northbound callbacks to new error handling model The northbound configuration callbacks should now print error messages to the provided buffer (args->errmsg) instead of logging them directly. This will allow the northbound layer to forward the error messages to the northbound clients in addition to logging them. NOTE: many callbacks are returning errors without providing any error message. This needs to be fixed long term. Signed-off-by: Renato Westphal --- diff --git a/isisd/isis_nb_config.c b/isisd/isis_nb_config.c index a649e896fa..9633e46415 100644 --- a/isisd/isis_nb_config.c +++ b/isisd/isis_nb_config.c @@ -116,8 +116,8 @@ int isis_instance_area_address_create(struct nb_cb_create_args *args) addr.addr_len = dotformat2buff(buff, net_title); memcpy(addr.area_addr, buff, addr.addr_len); if (addr.area_addr[addr.addr_len - 1] != 0) { - flog_warn( - EC_LIB_NB_CB_CONFIG_VALIDATE, + snprintf( + args->errmsg, args->errmsg_len, "nsel byte (last byte) in area address must be 0"); return NB_ERR_VALIDATION; } @@ -125,8 +125,8 @@ int isis_instance_area_address_create(struct nb_cb_create_args *args) /* Check that the SystemID portions match */ if (memcmp(isis->sysid, GETSYSID((&addr)), ISIS_SYS_ID_LEN)) { - flog_warn( - EC_LIB_NB_CB_CONFIG_VALIDATE, + snprintf( + args->errmsg, args->errmsg_len, "System ID must not change when defining additional area addresses"); return NB_ERR_VALIDATION; } @@ -332,8 +332,8 @@ int isis_instance_lsp_mtu_modify(struct nb_cb_modify_args *args) && circuit->state != C_STATE_UP) continue; if (lsp_mtu > isis_circuit_pdu_size(circuit)) { - flog_warn( - EC_LIB_NB_CB_CONFIG_VALIDATE, + snprintf( + args->errmsg, args->errmsg_len, "ISIS area contains circuit %s, which has a maximum PDU size of %zu", circuit->interface->name, isis_circuit_pdu_size(circuit)); @@ -1047,6 +1047,7 @@ int isis_instance_redistribute_ipv6_metric_modify( */ static int isis_multi_topology_common(enum nb_event event, const struct lyd_node *dnode, + char *errmsg, size_t errmsg_len, const char *topology, bool create) { struct isis_area *area; @@ -1056,8 +1057,8 @@ static int isis_multi_topology_common(enum nb_event event, switch (event) { case NB_EV_VALIDATE: if (mtid == (uint16_t)-1) { - flog_warn(EC_LIB_NB_CB_CONFIG_VALIDATE, - "Unknown topology %s", topology); + snprintf(errmsg, errmsg_len, "Unknown topology %s", + topology); return NB_ERR_VALIDATION; } break; @@ -1100,6 +1101,7 @@ int isis_instance_multi_topology_ipv4_multicast_create( struct nb_cb_create_args *args) { return isis_multi_topology_common(args->event, args->dnode, + args->errmsg, args->errmsg_len, "ipv4-multicast", true); } @@ -1107,6 +1109,7 @@ int isis_instance_multi_topology_ipv4_multicast_destroy( struct nb_cb_destroy_args *args) { return isis_multi_topology_common(args->event, args->dnode, + args->errmsg, args->errmsg_len, "ipv4-multicast", false); } @@ -1126,15 +1129,17 @@ int isis_instance_multi_topology_ipv4_multicast_overload_modify( int isis_instance_multi_topology_ipv4_management_create( struct nb_cb_create_args *args) { - return isis_multi_topology_common(args->event, args->dnode, "ipv4-mgmt", - true); + return isis_multi_topology_common(args->event, args->dnode, + args->errmsg, args->errmsg_len, + "ipv4-mgmt", true); } int isis_instance_multi_topology_ipv4_management_destroy( struct nb_cb_destroy_args *args) { - return isis_multi_topology_common(args->event, args->dnode, "ipv4-mgmt", - false); + return isis_multi_topology_common(args->event, args->dnode, + args->errmsg, args->errmsg_len, + "ipv4-mgmt", false); } /* @@ -1154,6 +1159,7 @@ int isis_instance_multi_topology_ipv6_unicast_create( struct nb_cb_create_args *args) { return isis_multi_topology_common(args->event, args->dnode, + args->errmsg, args->errmsg_len, "ipv6-unicast", true); } @@ -1161,6 +1167,7 @@ int isis_instance_multi_topology_ipv6_unicast_destroy( struct nb_cb_destroy_args *args) { return isis_multi_topology_common(args->event, args->dnode, + args->errmsg, args->errmsg_len, "ipv6-unicast", false); } @@ -1181,6 +1188,7 @@ int isis_instance_multi_topology_ipv6_multicast_create( struct nb_cb_create_args *args) { return isis_multi_topology_common(args->event, args->dnode, + args->errmsg, args->errmsg_len, "ipv6-multicast", true); } @@ -1188,6 +1196,7 @@ int isis_instance_multi_topology_ipv6_multicast_destroy( struct nb_cb_destroy_args *args) { return isis_multi_topology_common(args->event, args->dnode, + args->errmsg, args->errmsg_len, "ipv6-multicast", false); } @@ -1207,15 +1216,17 @@ int isis_instance_multi_topology_ipv6_multicast_overload_modify( int isis_instance_multi_topology_ipv6_management_create( struct nb_cb_create_args *args) { - return isis_multi_topology_common(args->event, args->dnode, "ipv6-mgmt", - true); + return isis_multi_topology_common(args->event, args->dnode, + args->errmsg, args->errmsg_len, + "ipv6-mgmt", true); } int isis_instance_multi_topology_ipv6_management_destroy( struct nb_cb_destroy_args *args) { - return isis_multi_topology_common(args->event, args->dnode, "ipv6-mgmt", - false); + return isis_multi_topology_common(args->event, args->dnode, + args->errmsg, args->errmsg_len, + "ipv6-mgmt", false); } /* @@ -1235,6 +1246,7 @@ int isis_instance_multi_topology_ipv6_dstsrc_create( struct nb_cb_create_args *args) { return isis_multi_topology_common(args->event, args->dnode, + args->errmsg, args->errmsg_len, "ipv6-dstsrc", true); } @@ -1242,6 +1254,7 @@ int isis_instance_multi_topology_ipv6_dstsrc_destroy( struct nb_cb_destroy_args *args) { return isis_multi_topology_common(args->event, args->dnode, + args->errmsg, args->errmsg_len, "ipv6-dstsrc", false); } @@ -1721,10 +1734,10 @@ int lib_interface_isis_create(struct nb_cb_create_args *args) min_mtu = DEFAULT_LSP_MTU; #endif /* ifndef FABRICD */ if (actual_mtu < min_mtu) { - flog_warn(EC_LIB_NB_CB_CONFIG_VALIDATE, - "Interface %s has MTU %" PRIu32 - ", minimum MTU for the area is %" PRIu32 "", - ifp->name, actual_mtu, min_mtu); + snprintf(args->errmsg, args->errmsg_len, + "Interface %s has MTU %" PRIu32 + ", minimum MTU for the area is %" PRIu32 "", + ifp->name, actual_mtu, min_mtu); return NB_ERR_VALIDATION; } break; @@ -1801,9 +1814,9 @@ int lib_interface_isis_area_tag_modify(struct nb_cb_modify_args *args) area_tag = yang_dnode_get_string(args->dnode, NULL); if (circuit && circuit->area && circuit->area->area_tag && strcmp(circuit->area->area_tag, area_tag)) { - flog_warn(EC_LIB_NB_CB_CONFIG_VALIDATE, - "ISIS circuit is already defined on %s", - circuit->area->area_tag); + snprintf(args->errmsg, args->errmsg_len, + "ISIS circuit is already defined on %s", + circuit->area->area_tag); return NB_ERR_VALIDATION; } } @@ -1839,9 +1852,9 @@ int lib_interface_isis_circuit_type_modify(struct nb_cb_modify_args *args) if (circuit && circuit->state == C_STATE_UP && circuit->area->is_type != IS_LEVEL_1_AND_2 && circuit->area->is_type != circ_type) { - flog_warn(EC_LIB_NB_CB_CONFIG_VALIDATE, - "Invalid circuit level for area %s", - circuit->area->area_tag); + snprintf(args->errmsg, args->errmsg_len, + "Invalid circuit level for area %s", + circuit->area->area_tag); return NB_ERR_VALIDATION; } break; @@ -2163,16 +2176,16 @@ int lib_interface_isis_network_type_modify(struct nb_cb_modify_args *args) if (!circuit) break; if (circuit->circ_type == CIRCUIT_T_LOOPBACK) { - flog_warn( - EC_LIB_NB_CB_CONFIG_VALIDATE, + snprintf( + args->errmsg, args->errmsg_len, "Cannot change network type on loopback interface"); return NB_ERR_VALIDATION; } if (net_type == CIRCUIT_T_BROADCAST && circuit->state == C_STATE_UP && !if_is_broadcast(circuit->interface)) { - flog_warn( - EC_LIB_NB_CB_CONFIG_VALIDATE, + snprintf( + args->errmsg, args->errmsg_len, "Cannot configure non-broadcast interface for broadcast operation"); return NB_ERR_VALIDATION; } @@ -2208,8 +2221,8 @@ int lib_interface_isis_passive_modify(struct nb_cb_modify_args *args) if (!ifp) return NB_OK; if (if_is_loopback(ifp)) { - flog_warn(EC_LIB_NB_CB_CONFIG_VALIDATE, - "Loopback is always passive"); + snprintf(args->errmsg, args->errmsg_len, + "Loopback is always passive"); return NB_ERR_VALIDATION; } } @@ -2312,7 +2325,8 @@ int lib_interface_isis_disable_three_way_handshake_modify( * /frr-interface:lib/interface/frr-isisd:isis/multi-topology/ipv4-unicast */ static int lib_interface_isis_multi_topology_common( - enum nb_event event, const struct lyd_node *dnode, uint16_t mtid) + enum nb_event event, const struct lyd_node *dnode, char *errmsg, + size_t errmsg_len, uint16_t mtid) { struct isis_circuit *circuit; bool value; @@ -2321,8 +2335,8 @@ static int lib_interface_isis_multi_topology_common( case NB_EV_VALIDATE: circuit = nb_running_get_entry(dnode, NULL, false); if (circuit && circuit->area && circuit->area->oldmetric) { - flog_warn( - EC_LIB_NB_CB_CONFIG_VALIDATE, + snprintf( + errmsg, errmsg_len, "Multi topology IS-IS can only be used with wide metrics"); return NB_ERR_VALIDATION; } @@ -2344,7 +2358,8 @@ int lib_interface_isis_multi_topology_ipv4_unicast_modify( struct nb_cb_modify_args *args) { return lib_interface_isis_multi_topology_common( - args->event, args->dnode, ISIS_MT_IPV4_UNICAST); + args->event, args->dnode, args->errmsg, args->errmsg_len, + ISIS_MT_IPV4_UNICAST); } /* @@ -2355,7 +2370,8 @@ int lib_interface_isis_multi_topology_ipv4_multicast_modify( struct nb_cb_modify_args *args) { return lib_interface_isis_multi_topology_common( - args->event, args->dnode, ISIS_MT_IPV4_MULTICAST); + args->event, args->dnode, args->errmsg, args->errmsg_len, + ISIS_MT_IPV4_MULTICAST); } /* @@ -2366,7 +2382,8 @@ int lib_interface_isis_multi_topology_ipv4_management_modify( struct nb_cb_modify_args *args) { return lib_interface_isis_multi_topology_common( - args->event, args->dnode, ISIS_MT_IPV4_MGMT); + args->event, args->dnode, args->errmsg, args->errmsg_len, + ISIS_MT_IPV4_MGMT); } /* @@ -2377,7 +2394,8 @@ int lib_interface_isis_multi_topology_ipv6_unicast_modify( struct nb_cb_modify_args *args) { return lib_interface_isis_multi_topology_common( - args->event, args->dnode, ISIS_MT_IPV6_UNICAST); + args->event, args->dnode, args->errmsg, args->errmsg_len, + ISIS_MT_IPV6_UNICAST); } /* @@ -2388,7 +2406,8 @@ int lib_interface_isis_multi_topology_ipv6_multicast_modify( struct nb_cb_modify_args *args) { return lib_interface_isis_multi_topology_common( - args->event, args->dnode, ISIS_MT_IPV6_MULTICAST); + args->event, args->dnode, args->errmsg, args->errmsg_len, + ISIS_MT_IPV6_MULTICAST); } /* @@ -2399,7 +2418,8 @@ int lib_interface_isis_multi_topology_ipv6_management_modify( struct nb_cb_modify_args *args) { return lib_interface_isis_multi_topology_common( - args->event, args->dnode, ISIS_MT_IPV6_MGMT); + args->event, args->dnode, args->errmsg, args->errmsg_len, + ISIS_MT_IPV6_MGMT); } /* @@ -2409,5 +2429,6 @@ int lib_interface_isis_multi_topology_ipv6_dstsrc_modify( struct nb_cb_modify_args *args) { return lib_interface_isis_multi_topology_common( - args->event, args->dnode, ISIS_MT_IPV6_DSTSRC); + args->event, args->dnode, args->errmsg, args->errmsg_len, + ISIS_MT_IPV6_DSTSRC); } diff --git a/lib/if.c b/lib/if.c index 9efd298a4f..fbe6f836a3 100644 --- a/lib/if.c +++ b/lib/if.c @@ -1561,8 +1561,8 @@ static int lib_interface_destroy(struct nb_cb_destroy_args *args) case NB_EV_VALIDATE: ifp = nb_running_get_entry(args->dnode, NULL, true); if (CHECK_FLAG(ifp->status, ZEBRA_INTERFACE_ACTIVE)) { - zlog_warn("%s: only inactive interfaces can be deleted", - __func__); + snprintf(args->errmsg, args->errmsg_len, + "only inactive interfaces can be deleted"); return NB_ERR_VALIDATION; } break; diff --git a/lib/vrf.c b/lib/vrf.c index 9df5d19516..fb64589287 100644 --- a/lib/vrf.c +++ b/lib/vrf.c @@ -1082,8 +1082,8 @@ static int lib_vrf_destroy(struct nb_cb_destroy_args *args) case NB_EV_VALIDATE: vrfp = nb_running_get_entry(args->dnode, NULL, true); if (CHECK_FLAG(vrfp->status, VRF_ACTIVE)) { - zlog_debug("%s Only inactive VRFs can be deleted", - __func__); + snprintf(args->errmsg, args->errmsg_len, + "Only inactive VRFs can be deleted"); return NB_ERR_VALIDATION; } break; diff --git a/zebra/zebra_nb_config.c b/zebra/zebra_nb_config.c index 5b87a18a06..948ef51320 100644 --- a/zebra/zebra_nb_config.c +++ b/zebra/zebra_nb_config.c @@ -941,13 +941,15 @@ int lib_interface_zebra_ip_addrs_create(struct nb_cb_create_args *args) case NB_EV_VALIDATE: if (prefix.family == AF_INET && ipv4_martian(&prefix.u.prefix4)) { - zlog_debug("invalid address %s", - prefix2str(&prefix, buf, sizeof(buf))); + snprintf(args->errmsg, args->errmsg_len, + "invalid address %s", + prefix2str(&prefix, buf, sizeof(buf))); return NB_ERR_VALIDATION; } else if (prefix.family == AF_INET6 && ipv6_martian(&prefix.u.prefix6)) { - zlog_debug("invalid address %s", - prefix2str(&prefix, buf, sizeof(buf))); + snprintf(args->errmsg, args->errmsg_len, + "invalid address %s", + prefix2str(&prefix, buf, sizeof(buf))); return NB_ERR_VALIDATION; } break; @@ -982,16 +984,18 @@ int lib_interface_zebra_ip_addrs_destroy(struct nb_cb_destroy_args *args) /* Check current interface address. */ ifc = connected_check_ptp(ifp, &prefix, NULL); if (!ifc) { - zlog_debug("interface %s Can't find address\n", - ifp->name); + snprintf(args->errmsg, args->errmsg_len, + "interface %s Can't find address\n", + ifp->name); return NB_ERR_VALIDATION; } } else if (prefix.family == AF_INET6) { /* Check current interface address. */ ifc = connected_check(ifp, &prefix); if (!ifc) { - zlog_debug("interface can't find address %s", - ifp->name); + snprintf(args->errmsg, args->errmsg_len, + "interface can't find address %s", + ifp->name); return NB_ERR_VALIDATION; } } else @@ -999,7 +1003,8 @@ int lib_interface_zebra_ip_addrs_destroy(struct nb_cb_destroy_args *args) /* This is not configured address. */ if (!CHECK_FLAG(ifc->conf, ZEBRA_IFC_CONFIGURED)) { - zlog_debug("interface %s not configured", ifp->name); + snprintf(args->errmsg, args->errmsg_len, + "interface %s not configured", ifp->name); return NB_ERR_VALIDATION; } @@ -1244,8 +1249,8 @@ int lib_vrf_zebra_ribs_rib_create(struct nb_cb_create_args *args) switch (args->event) { case NB_EV_VALIDATE: if (!zrt) { - zlog_debug("%s: vrf %s table is not found.", __func__, - vrf->name); + snprintf(args->errmsg, args->errmsg_len, + "vrf %s table is not found.", vrf->name); return NB_ERR_VALIDATION; } break; @@ -1376,7 +1381,8 @@ int lib_route_map_entry_match_condition_source_protocol_modify( case NB_EV_VALIDATE: type = yang_dnode_get_string(args->dnode, NULL); if (proto_name2num(type) == -1) { - zlog_warn("%s: invalid protocol: %s", __func__, type); + snprintf(args->errmsg, args->errmsg_len, + "invalid protocol: %s", type); return NB_ERR_VALIDATION; } return NB_OK; @@ -1470,8 +1476,9 @@ int lib_route_map_entry_set_action_source_v4_modify( memset(&p, 0, sizeof(p)); yang_dnode_get_ipv4p(&p, args->dnode, NULL); if (zebra_check_addr(&p) == 0) { - zlog_warn("%s: invalid IPv4 address: %s", __func__, - yang_dnode_get_string(args->dnode, NULL)); + snprintf(args->errmsg, args->errmsg_len, + "invalid IPv4 address: %s", + yang_dnode_get_string(args->dnode, NULL)); return NB_ERR_VALIDATION; } @@ -1482,8 +1489,9 @@ int lib_route_map_entry_set_action_source_v4_modify( break; } if (pif == NULL) { - zlog_warn("%s: is not a local adddress: %s", __func__, - yang_dnode_get_string(args->dnode, NULL)); + snprintf(args->errmsg, args->errmsg_len, + "is not a local adddress: %s", + yang_dnode_get_string(args->dnode, NULL)); return NB_ERR_VALIDATION; } return NB_OK; @@ -1536,8 +1544,9 @@ int lib_route_map_entry_set_action_source_v6_modify( memset(&p, 0, sizeof(p)); yang_dnode_get_ipv6p(&p, args->dnode, NULL); if (zebra_check_addr(&p) == 0) { - zlog_warn("%s: invalid IPv6 address: %s", __func__, - yang_dnode_get_string(args->dnode, NULL)); + snprintf(args->errmsg, args->errmsg_len, + "invalid IPv6 address: %s", + yang_dnode_get_string(args->dnode, NULL)); return NB_ERR_VALIDATION; } @@ -1548,8 +1557,9 @@ int lib_route_map_entry_set_action_source_v6_modify( break; } if (pif == NULL) { - zlog_warn("%s: is not a local adddress: %s", __func__, - yang_dnode_get_string(args->dnode, NULL)); + snprintf(args->errmsg, args->errmsg_len, + "is not a local adddress: %s", + yang_dnode_get_string(args->dnode, NULL)); return NB_ERR_VALIDATION; } return NB_OK;