*: 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 <renato@opensourcerouting.org>

diff --git a/isisd/isis_nb_config.c b/isisd/isis_nb_config.c
index a649e896facdcc4affff4c1ae874d46402e8c404..9633e46415fe0997196c0610e7197da1efc0fc3d 100644 (file)
--- 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 9efd298a4f2a65588f3fb602d3952e955a5a18a6..fbe6f836a34caf72939713cc171cf483adbf2ea6 100644 (file)
--- 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 9df5d1951610c09120907f161ff6781c676d73a6..fb64589287fbf8b00a0ffaf6469ecae4c2466323 100644 (file)
--- 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 5b87a18a068ca6ca70b57d41155cad81ee74144d..948ef513203a729cbd1893d7907c4a14cf0c04fd 100644 (file)
--- 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;