Merge pull request FRRouting#3 from donaldsharp/more_error_stuff

More error stuff

eigrpd/eigrp_network.c

@@ -37,6 +37,7 @@
 #include "privs.h"
 #include "table.h"
 #include "vty.h"
+#include "lib_errors.h"
 
 #include "eigrpd/eigrp_structs.h"
 #include "eigrpd/eigrpd.h"
@@ -58,17 +59,18 @@ int eigrp_sock_init(void)
 	int ret, hincl = 1;
 
 	if (eigrpd_privs.change(ZPRIVS_RAISE))
-		zlog_err("eigrp_sock_init: could not raise privs, %s",
-			 safe_strerror(errno));
+		zlog_ferr(LIB_ERR_PRIVILEGES,
+			  "eigrp_sock_init: could not raise privs, %s",
+			  safe_strerror(errno));
 
 	eigrp_sock = socket(AF_INET, SOCK_RAW, IPPROTO_EIGRPIGP);
 	if (eigrp_sock < 0) {
 		int save_errno = errno;
 		if (eigrpd_privs.change(ZPRIVS_LOWER))
 			zlog_err("eigrp_sock_init: could not lower privs, %s",
 				 safe_strerror(errno));
-		zlog_err("eigrp_read_sock_init: socket: %s",
-			 safe_strerror(save_errno));
+		zlog_ferr(LIB_ERR_SOCKET, "eigrp_read_sock_init: socket: %s",
+			  safe_strerror(save_errno));
 		exit(1);
 	}
 
@@ -79,8 +81,9 @@ int eigrp_sock_init(void)
 	if (ret < 0) {
 		int save_errno = errno;
 		if (eigrpd_privs.change(ZPRIVS_LOWER))
-			zlog_err("eigrp_sock_init: could not lower privs, %s",
-				 safe_strerror(errno));
+			zlog_ferr(LIB_ERR_PRIVILEGES,
+				  "eigrp_sock_init: could not lower privs, %s",
+				  safe_strerror(errno));
 		zlog_warn("Can't set IP_HDRINCL option for fd %d: %s",
 			  eigrp_sock, safe_strerror(save_errno));
 	}
@@ -91,8 +94,9 @@ int eigrp_sock_init(void)
 	if (ret < 0) {
 		int save_errno = errno;
 		if (eigrpd_privs.change(ZPRIVS_LOWER))
-			zlog_err("eigrpd_sock_init: could not lower privs, %s",
-				 safe_strerror(errno));
+			zlog_ferr(LIB_ERR_PRIVILEGES,
+				  "eigrpd_sock_init: could not lower privs, %s",
+				  safe_strerror(errno));
 		zlog_warn("can't set sockopt IP_TOS %d to socket %d: %s", tos,
 			  eigrp_sock, safe_strerror(save_errno));
 		close(eigrp_sock); /* Prevent sd leak. */
@@ -109,8 +113,9 @@ int eigrp_sock_init(void)
 		zlog_warn("Can't set pktinfo option for fd %d", eigrp_sock);
 
 	if (eigrpd_privs.change(ZPRIVS_LOWER)) {
-		zlog_err("eigrp_sock_init: could not lower privs, %s",
-			 safe_strerror(errno));
+		zlog_ferr(LIB_ERR_PRIVILEGES,
+			  "eigrp_sock_init: could not lower privs, %s",
+			  safe_strerror(errno));
 	}
 
 	return eigrp_sock;
@@ -123,8 +128,8 @@ void eigrp_adjust_sndbuflen(struct eigrp *eigrp, unsigned int buflen)
 	if (eigrp->maxsndbuflen >= buflen)
 		return;
 	if (eigrpd_privs.change(ZPRIVS_RAISE))
-		zlog_err("%s: could not raise privs, %s", __func__,
-			 safe_strerror(errno));
+		zlog_ferr(LIB_ERR_PRIVILEGES, "%s: could not raise privs, %s",
+			  __func__, safe_strerror(errno));
 
 	/* Now we try to set SO_SNDBUF to what our caller has requested
 	 * (the MTU of a newly added interface). However, if the OS has
@@ -143,8 +148,8 @@ void eigrp_adjust_sndbuflen(struct eigrp *eigrp, unsigned int buflen)
 	else
 		zlog_warn("%s: failed to get SO_SNDBUF", __func__);
 	if (eigrpd_privs.change(ZPRIVS_LOWER))
-		zlog_err("%s: could not lower privs, %s", __func__,
-			 safe_strerror(errno));
+		zlog_ferr(LIB_ERR_PRIVILEGES, "%s: could not lower privs, %s",
+			  __func__, safe_strerror(errno));
 }
 
 int eigrp_if_ipmulticast(struct eigrp *top, struct prefix *p,

eigrpd/eigrp_packet.c

@@ -42,6 +42,7 @@
 #include "checksum.h"
 #include "md5.h"
 #include "sha256.h"
+#include "lib_errors.h"
 
 #include "eigrpd/eigrp_structs.h"
 #include "eigrpd/eigrpd.h"
@@ -1212,8 +1213,9 @@ uint16_t eigrp_add_internalTLV_to_stream(struct stream *s,
 		stream_putw(s, length);
 		break;
 	default:
-		zlog_err("%s: Unexpected prefix length: %d",
-			 __PRETTY_FUNCTION__, pe->destination->prefixlen);
+		zlog_ferr(LIB_ERR_DEVELOPMENT,
+			  "%s: Unexpected prefix length: %d",
+			  __PRETTY_FUNCTION__, pe->destination->prefixlen);
 		return 0;
 	}
 	stream_putl(s, 0x00000000);

eigrpd/eigrp_topology.c

@@ -37,6 +37,7 @@
 #include "log.h"
 #include "linklist.h"
 #include "vty.h"
+#include "lib_errors.h"
 
 #include "eigrpd/eigrp_structs.h"
 #include "eigrpd/eigrpd.h"
@@ -408,7 +409,8 @@ eigrp_topology_update_distance(struct eigrp_fsm_action_message *msg)
 		}
 		break;
 	default:
-		zlog_err("%s: Please implement handler", __PRETTY_FUNCTION__);
+		zlog_ferr(LIB_ERR_DEVELOPMENT, "%s: Please implement handler",
+			  __PRETTY_FUNCTION__);
 		break;
 	}
 distance_done:

eigrpd/eigrpd.c

@@ -170,12 +170,7 @@ static struct eigrp *eigrp_new(const char *AS)
 	eigrp->fd = eigrp_socket;
 	eigrp->maxsndbuflen = getsockopt_so_sendbuf(eigrp->fd);
 
-	if ((eigrp->ibuf = stream_new(EIGRP_PACKET_MAX_LEN + 1)) == NULL) {
-		zlog_err(
-			"eigrp_new: fatal error: stream_new (%u) failed allocating ibuf",
-			EIGRP_PACKET_MAX_LEN + 1);
-		exit(1);
-	}
+	eigrp->ibuf = stream_new(EIGRP_PACKET_MAX_LEN + 1);
 
 	eigrp->t_read = NULL;
 	thread_add_read(master, eigrp_read, eigrp, eigrp->fd, &eigrp->t_read);

ldpd/ldpd.c

@@ -42,6 +42,7 @@
 #include "filter.h"
 #include "qobj.h"
 #include "libfrr.h"
+#include "lib_errors.h"
 
 static void		 ldpd_shutdown(void);
 static pid_t		 start_child(enum ldpd_process, char *, int, int);
@@ -450,8 +451,9 @@ start_child(enum ldpd_process p, char *argv0, int fd_async, int fd_sync)
 
 	nullfd = open("/dev/null", O_RDONLY | O_NOCTTY);
 	if (nullfd == -1) {
-		zlog_err("%s: failed to open /dev/null: %s", __func__,
-			 safe_strerror(errno));
+		zlog_ferr(LIB_ERR_SYSTEM_CALL,
+			  "%s: failed to open /dev/null: %s", __func__,
+			  safe_strerror(errno));
 	} else {
 		dup2(nullfd, 0);
 		dup2(nullfd, 1);

lib/buffer.c

@@ -458,9 +458,9 @@ in one shot. */
 	while (written > 0) {
 		struct buffer_data *d;
 		if (!(d = b->head)) {
-			zlog_err(
-				"%s: corruption detected: buffer queue empty, "
-				"but written is %lu",
+			zlog_ferr(
+				LIB_ERR_DEVELOPMENT,
+				"%s: corruption detected: buffer queue empty, but written is %lu",
 				__func__, (unsigned long)written);
 			break;
 		}

lib/ferr.c

@@ -175,13 +175,6 @@ static ferr_r ferr_set_va(const char *file, int line, const char *func,
 
 	if (!error) {
 		error = XCALLOC(MTYPE_ERRINFO, sizeof(*error));
-		if (!error) {
-			/* we're screwed */
-			zlog_err("out of memory while allocating error info");
-			raise(SIGSEGV);
-			abort(); /* raise() can return, but raise(SIGSEGV) shall
-				    not */
-		}
 
 		pthread_setspecific(errkey, error);
 	}

lib/ferr.h

@@ -96,15 +96,41 @@ struct ferr {
 };
 
 /* Numeric ranges assigned to daemons for use as error codes. */
-#define LIB_FERR_START 0x01000001
-#define LIB_FERR_END 0x01FFFFFF
-#define BGP_FERR_START 0x02000000
-#define BGP_FERR_END 0x02FFFFFF
-#define OSPF_FERR_START 0x03000001
-#define OSPF_FERR_END 0x03FFFFFF
-#define ZEBRA_FERR_START 0x04000001
-#define ZEBRA_FERR_END 0x04FFFFFF
-#define END_FERR 0xFFFFFFFF
+#define BABEL_FERR_START   0x01000001
+#define BABEL_FRRR_END     0x01FFFFFF
+#define BGP_FERR_START     0x02000001
+#define BGP_FERR_END       0x02FFFFFF
+#define EIGRP_FERR_START   0x03000001
+#define EIGRP_FERR_END     0x03FFFFFF
+#define ISIS_FERR_START    0x04000001
+#define ISIS_FERR_END      0x04FFFFFF
+#define LDP_FERR_START     0x05000001
+#define LDP_FERR_END       0x05FFFFFF
+#define LIB_FERR_START     0x06000001
+#define LIB_FERR_END       0x06FFFFFF
+#define NHRP_FERR_START    0x07000001
+#define NHRP_FERR_END      0x07FFFFFF
+#define OSPF_FERR_START    0x08000001
+#define OSPF_FERR_END      0x08FFFFFF
+#define OSPFV3_FERR_START  0x09000001
+#define OSPFV3_FERR_END    0x09FFFFFF
+#define PBR_FERR_START     0x0A000001
+#define PBR_FERR_END       0x0AFFFFFF
+#define PIM_FERR_START     0x0B000001
+#define PIM_FERR_STOP      0x0BFFFFFF
+#define RIP_FERR_START     0x0C000001
+#define RIP_FERR_STOP      0x0CFFFFFF
+#define RIPNG_FERR_START   0x0D000001
+#define RIPNG_FERR_STOP    0x0DFFFFFF
+#define SHARP_FERR_START   0x0E000001
+#define SHARP_FERR_END     0x0EFFFFFF
+#define VTYSH_FERR_START   0x0F000001
+#define VTYSH_FRR_END      0x0FFFFFFF
+#define WATCHFRR_FRR_START 0x10000001
+#define WATCHFRR_FRR_END   0x10FFFFFF
+#define ZEBRA_FERR_START   0xF1000001
+#define ZEBRA_FERR_END     0xF1FFFFFF
+#define END_FERR           0xFFFFFFFF
 
 struct ferr_ref {
 	/* Unique error code displayed to end user as a reference. -1 means

lib/frr_zmq.c

@@ -24,6 +24,7 @@
 #include "memory.h"
 #include "frr_zmq.h"
 #include "log.h"
+#include "lib_errors.h"
 
 DEFINE_MTYPE_STATIC(LIB, ZEROMQ_CB, "ZeroMQ callback")
 
@@ -140,7 +141,8 @@ static int frrzmq_read_msg(struct thread *t)
 	return 0;
 
 out_err:
-	zlog_err("ZeroMQ read error: %s(%d)", strerror(errno), errno);
+	zlog_ferr(LIB_ERR_ZMQ, "ZeroMQ read error: %s(%d)", strerror(errno),
+		  errno);
 	if (cb->read.cb_error)
 		cb->read.cb_error(cb->read.arg, cb->zmqsock);
 	return 1;
@@ -252,7 +254,8 @@ static int frrzmq_write_msg(struct thread *t)
 	return 0;
 
 out_err:
-	zlog_err("ZeroMQ write error: %s(%d)", strerror(errno), errno);
+	zlog_ferr(LIB_ERR_ZMQ, "ZeroMQ write error: %s(%d)", strerror(errno),
+		  errno);
 	if (cb->write.cb_error)
 		cb->write.cb_error(cb->write.arg, cb->zmqsock);
 	return 1;

lib/if.c

@@ -23,6 +23,7 @@
 
 #include "linklist.h"
 #include "vector.h"
+#include "lib_errors.h"
 #include "vty.h"
 #include "command.h"
 #include "vrf.h"

lib/if.h

@@ -297,28 +297,32 @@ DECLARE_QOBJ_TYPE(interface)
 
 #define IFNAME_RB_INSERT(vrf, ifp)                                             \
 	if (RB_INSERT(if_name_head, &vrf->ifaces_by_name, (ifp)))              \
-		zlog_err(                                                      \
+		zlog_ferr(                                                     \
+			LIB_ERR_INTERFACE,                                     \
 			"%s(%s): corruption detected -- interface with this "  \
 			"name exists already in VRF %u!",                      \
 			__func__, (ifp)->name, (ifp)->vrf_id);
 
 #define IFNAME_RB_REMOVE(vrf, ifp)                                             \
 	if (RB_REMOVE(if_name_head, &vrf->ifaces_by_name, (ifp)) == NULL)      \
-		zlog_err(                                                      \
+		zlog_ferr(                                                     \
+			LIB_ERR_INTERFACE,                                     \
 			"%s(%s): corruption detected -- interface with this "  \
 			"name doesn't exist in VRF %u!",                       \
 			__func__, (ifp)->name, (ifp)->vrf_id);
 
 #define IFINDEX_RB_INSERT(vrf, ifp)                                            \
 	if (RB_INSERT(if_index_head, &vrf->ifaces_by_index, (ifp)))            \
-		zlog_err(                                                      \
+		zlog_ferr(                                                     \
+			LIB_ERR_INTERFACE,                                     \
 			"%s(%u): corruption detected -- interface with this "  \
 			"ifindex exists already in VRF %u!",                   \
 			__func__, (ifp)->ifindex, (ifp)->vrf_id);
 
 #define IFINDEX_RB_REMOVE(vrf, ifp)                                            \
 	if (RB_REMOVE(if_index_head, &vrf->ifaces_by_index, (ifp)) == NULL)    \
-		zlog_err(                                                      \
+		zlog_ferr(                                                     \
+			LIB_ERR_INTERFACE,                                     \
 			"%s(%u): corruption detected -- interface with this "  \
 			"ifindex doesn't exist in VRF %u!",                    \
 			__func__, (ifp)->ifindex, (ifp)->vrf_id);

lib/lib_errors.c

@@ -76,6 +76,30 @@ static struct ferr_ref ferr_lib_err[] = {
 		.description = "FRR has detected a problem with the snmp library it uses\nA callback from this subsystem has indicated some error",
 		.suggestion = "Examine callback message and ensure snmp is properly setup and working"
 	},
+	{
+		.code = LIB_ERR_INTERFACE,
+		.title = "Interface Subsystem Error",
+		.description = "FRR has detected a problem with interface data from the kernel as it deviates\nfrom what we would expect to happen via normal netlink messaging",
+		.suggestion = "Open an Issue with all relevant log files and restart FRR"
+	},
+	{
+		.code = LIB_ERR_NS,
+		.title = "NameSpace Subsystem Error",
+		.description = "FRR has detected a problem with NameSpace data from the kernel as it deviates\nfrom what we would expect to happen via normal kernel messaging",
+		.suggestion = "Open an Issue with all relevant log files and restart FRR"
+	},
+	{
+		.code = LIB_ERR_DEVELOPMENT,
+		.title = "Developmental Escape Error",
+		.description = "FRR has detected an issue where new development has not properly\nupdated all code paths.",
+		.suggestion = "Open an Issue with all relevant log files"
+	},
+	{
+		.code = LIB_ERR_ZMQ,
+		.title = "ZMQ Subsystem Error",
+		.description = "FRR has detected an issue with the Zero MQ subsystem and ZeroMQ\nis not working properly now",
+		.suggestion = "Open an Issue with all relevant log files and restart FRR"
+	},
 	{
 		.code = END_FERR,
 	}

lib/lib_errors.h

@@ -21,7 +21,6 @@
 #define __LIB_ERRORS_H__
 
 #include "ferr.h"
-#include "lib_errors.h"
 
 enum lib_ferr_refs {
 	LIB_ERR_PRIVILEGES = LIB_FERR_START,
@@ -33,6 +32,10 @@ enum lib_ferr_refs {
 	LIB_ERR_SYSTEM_CALL,
 	LIB_ERR_VTY,
 	LIB_ERR_SNMP,
+	LIB_ERR_INTERFACE,
+	LIB_ERR_NS,
+	LIB_ERR_DEVELOPMENT,
+	LIB_ERR_ZMQ,
 };
 
 extern void lib_error_init(void);

lib/libfrr.c

@@ -790,8 +790,9 @@ static void frr_terminal_close(int isexit)
 
 	nullfd = open("/dev/null", O_RDONLY | O_NOCTTY);
 	if (nullfd == -1) {
-		zlog_err("%s: failed to open /dev/null: %s", __func__,
-			 safe_strerror(errno));
+		zlog_ferr(LIB_ERR_SYSTEM_CALL,
+			  "%s: failed to open /dev/null: %s", __func__,
+			  safe_strerror(errno));
 	} else {
 		dup2(nullfd, 0);
 		dup2(nullfd, 1);
@@ -860,8 +861,9 @@ void frr_run(struct thread_master *master)
 	} else if (di->daemon_mode) {
 		int nullfd = open("/dev/null", O_RDONLY | O_NOCTTY);
 		if (nullfd == -1) {
-			zlog_err("%s: failed to open /dev/null: %s", __func__,
-				 safe_strerror(errno));
+			zlog_ferr(LIB_ERR_SYSTEM_CALL,
+				  "%s: failed to open /dev/null: %s", __func__,
+				  safe_strerror(errno));
 		} else {
 			dup2(nullfd, 0);
 			dup2(nullfd, 1);