[lldp-devel] [PATCH] lldpad: Fix logging to no longer use message IDs

Mark Rustad mark.d.rustad at intel.com
Thu Aug 11 21:08:56 UTC 2011


lldpad's logging was overloading the first parameter to the log_message
function as both a logging level and a message ID. There are really not
many message IDs in use, so resolve the conflict by using only log level.
This also adds a proper printf-attribute to the logging function so
that arguments can be checked and resolve those as well. A few messages
that were changed now have more useful content.

Signed-off-by: Mark Rustad <mark.d.rustad at intel.com>
Tested-by: Ross Brattain <ross.b.brattain at intel.com>
---
 ecp/ecp.c          |    4 +
 ecp/ecp_rx.c       |   12 ++--
 ecp/ecp_tx.c       |   13 ++---
 include/messages.h |   30 +---------
 lldp/rx.c          |    4 +
 lldp_evb.c         |    8 +--
 lldp_mand.c        |    4 +
 lldp_vdp.c         |   24 +++-----
 lldpad.c           |    6 +-
 log.c              |  150 +---------------------------------------------------
 10 files changed, 38 insertions(+), 217 deletions(-)

diff --git a/ecp/ecp.c b/ecp/ecp.c
index 7d9be89..638a8cb 100644
--- a/ecp/ecp.c
+++ b/ecp/ecp.c
@@ -118,9 +118,9 @@ void ecp_ack_timeout_handler(void *eloop_data, void *user_ctx)
 			   __func__, __LINE__, vd->ifname, vd->ecp.ackTimer);
 		ecp_tx_run_sm(vd);
 	} else {
-		LLDPAD_DBG("%s(%i)-%s: BUG ! handler called but"
+		LLDPAD_DBG("%s-%s: BUG ! handler called but"
 			   "vdp->ecp.ackTimer not expired (%i) !\n",
-			   __func__, __LINE__, vd->ecp.ackTimer);
+			   __func__, vd->ifname, vd->ecp.ackTimer);
 	}
 }
 
diff --git a/ecp/ecp_rx.c b/ecp/ecp_rx.c
index 74e9f32..a56cd3b 100644
--- a/ecp/ecp_rx.c
+++ b/ecp/ecp_rx.c
@@ -513,8 +513,8 @@ void ecp_rx_change_state(struct vdp_data *vd, u8 newstate)
 		assert(vd->ecp.rx.state == ECP_RX_RECEIVE_ECPDU);
 		break;
 	default:
-		LLDPAD_ERR("ERROR: The ECP_RX State Machine is broken!\n");
-		log_message(MSG_ERR_RX_SM_INVALID, "%s", vd->ifname);
+		LLDPAD_ERR("%s: LLDP rx state machine setting invalid state %d\n",
+			   vd->ifname, newstate);
 	}
 
 	LLDPAD_DBG("%s(%i)-%s: state change %s -> %s\n", __func__, __LINE__,
@@ -585,8 +585,8 @@ bool ecp_set_rx_state(struct vdp_data *vd)
 		ecp_rx_change_state(vd, ECP_RX_RECEIVE_WAIT);
 		return false;
 	default:
-		LLDPAD_ERR("ERROR: The ECP_RX State Machine is broken!\n");
-		log_message(MSG_ERR_RX_SM_INVALID, "%s", vd->ifname);
+		LLDPAD_ERR("%s: LLDP RX state machine in invalid state %d\n",
+			   vd->ifname, vd->ecp.rx.state);
 		return false;
 	}
 }
@@ -628,8 +628,8 @@ void ecp_rx_run_sm(struct vdp_data *vd)
 			}
 			break;
 		default:
-			LLDPAD_ERR("ERROR: The ECP_RX State Machine is broken!\n");
-			log_message(MSG_ERR_TX_SM_INVALID, "%s", vd->ifname);
+			LLDPAD_ERR("%s: LLDP RX state machine in invalid state %d\n",
+				   vd->ifname, vd->ecp.rx.state);
 		}
 	} while (ecp_set_rx_state(vd) == true);
 
diff --git a/ecp/ecp_tx.c b/ecp/ecp_tx.c
index f9ee3d7..c81e101 100644
--- a/ecp/ecp_tx.c
+++ b/ecp/ecp_tx.c
@@ -377,8 +377,8 @@ static void ecp_tx_change_state(struct vdp_data *vd, u8 newstate)
 		assert(vd->ecp.tx.state == ECP_TX_WAIT_FOR_ACK);
 		break;
 	default:
-		LLDPAD_ERR("ERROR: The ECP_TX State Machine is broken!\n");
-		log_message(MSG_ERR_TX_SM_INVALID, "%s", vd->ifname);
+		LLDPAD_ERR("%s: LLDP TX state machine setting invalid state %d\n",
+			   vd->ifname, newstate);
 	}
 
 	LLDPAD_DBG("%s(%i)-%s: state change %s -> %s\n", __func__, __LINE__,
@@ -462,8 +462,8 @@ static bool ecp_set_tx_state(struct vdp_data *vd)
 		}
 		return false;
 	default:
-		LLDPAD_ERR("ERROR: The TX State Machine is broken!\n");
-		log_message(MSG_ERR_TX_SM_INVALID, "%s", vd->ifname);
+		LLDPAD_ERR("%s: LLDP TX state machine in invalid state %d\n",
+			   vd->ifname, vd->ecp.tx.state);
 		return false;
 	}
 }
@@ -506,9 +506,8 @@ void ecp_tx_run_sm(struct vdp_data *vd)
 			       vd->ifname, vd->ecp.lastSequence);
 			break;
 		default:
-			LLDPAD_ERR("%s(%i): ERROR The TX State Machine is broken!\n", __func__,
-			       __LINE__);
-			log_message(MSG_ERR_TX_SM_INVALID, "%s", vd->ifname);
+			LLDPAD_ERR("%s: LLDP TX state machine in invalid state %d\n",
+				   vd->ifname, vd->ecp.tx.state);
 		}
 	} while (ecp_set_tx_state(vd) == true);
 
diff --git a/include/messages.h b/include/messages.h
index 795a0df..2991d33 100644
--- a/include/messages.h
+++ b/include/messages.h
@@ -29,37 +29,11 @@
 #include <syslog.h>
 #include <stdbool.h>
 
-#define MSG_INFO_DEBUG_STRING 1
-
-#define MSG_ERR_SERVICE_START_FAILURE 14
-#define MSG_ERR_RESOURCE_MEMORY 15
-#define MSG_ERR_ADD_CARD_FAILURE 16
-#define MSG_ERR_DCB_INVALID_TX_TOTAL_BWG 17
-#define MSG_ERR_DCB_INVALID_RX_TOTAL_BWG 18
-#define MSG_ERR_DCB_INVALID_TX_BWG_IDX 19
-#define MSG_ERR_DCB_INVALID_RX_BWG_IDX 20
-#define MSG_ERR_DCB_INVALID_TX_LSP_NZERO_BW_TC 21
-#define MSG_ERR_DCB_INVALID_RX_LSP_NZERO_BW_TC 22
-#define MSG_ERR_DCB_TOO_MANY_LSP_PGIDS 23
-#define MSG_ERR_DCB_INVALID_TX_ZERO_BW_TC 24
-#define MSG_ERR_DCB_INVALID_RX_ZERO_BW_TC 25
-#define MSG_ERR_DCB_INVALID_TX_LSP_NZERO_BWG 26
-#define MSG_ERR_DCB_INVALID_RX_LSP_NZERO_BWG 27
-#define MSG_ERR_DCB_INVALID_TX_BWG 28
-#define MSG_ERR_DCB_INVALID_RX_BWG 29
-#define MSG_ERR_TX_SM_INVALID 30
-#define MSG_ERR_RX_SM_INVALID 31
-#define MSG_ERR_DCB_INVALID_CONFIG_FILE 32
-
-#define MSG_INFO_LLINK_DISABLED 37
-#define MSG_INFO_LLINK_ENABLED 38
-#define MSG_INFO_LLINK_OPER 39
-#define MSG_ERR_LLINK_NONOPER 40
-
 extern bool daemonize;
 extern int loglvl;
 
-void log_message(__u32 dwMsgId, const char *pFormat, ...);
+void log_message(int loglvl, const char *pFormat, ...)
+	__attribute__((__format__(__printf__, 2, 3)));
 
 #define LLDPAD_ERR(...) log_message(LOG_ERR,  __VA_ARGS__)
 #define LLDPAD_WARN(...) log_message(LOG_WARNING, __VA_ARGS__)
diff --git a/lldp/rx.c b/lldp/rx.c
index bbb2e2f..3954808 100644
--- a/lldp/rx.c
+++ b/lldp/rx.c
@@ -375,8 +375,8 @@ void rxProcessFrame(struct port * port)
 		}
 
 		if (!tlv_stored) {
-			LLDPAD_INFO("\nrxProcessFrame: allocated TLV (%lu) "
-				   " was not stored! (%p)\n", tlv->type, tlv);
+			LLDPAD_INFO("%s: allocated TLV %u was not stored! %p\n",
+				   __func__, tlv->type, tlv);
 			tlv = free_unpkd_tlv(tlv);
 			port->stats.statsTLVsUnrecognizedTotal++;
 		}
diff --git a/lldp_evb.c b/lldp_evb.c
index fe43af5..3895d71 100644
--- a/lldp_evb.c
+++ b/lldp_evb.c
@@ -648,17 +648,13 @@ struct lldp_module *evb_register(void)
 
 	mod = malloc(sizeof(*mod));
 	if (!mod) {
-		LLDPAD_ERR("failed to malloc module data\n");
-		log_message(MSG_ERR_SERVICE_START_FAILURE,
-			"%s", "failed to malloc module data");
+		LLDPAD_ERR("lldpad failed to start - failed to malloc module data\n");
 		goto out_err;
 	}
 	ud = malloc(sizeof(struct evb_user_data));
 	if (!ud) {
 		free(mod);
-		LLDPAD_ERR("failed to malloc module user data\n");
-		log_message(MSG_ERR_SERVICE_START_FAILURE,
-			"%s", "failed to malloc module user data");
+		LLDPAD_ERR("lldpad failed to start - failed to malloc module user data\n");
 		goto out_err;
 	}
 	LIST_INIT(&ud->head);
diff --git a/lldp_mand.c b/lldp_mand.c
index b5e8092..1bc4358 100644
--- a/lldp_mand.c
+++ b/lldp_mand.c
@@ -211,7 +211,7 @@ bld_config:
 
 	/* if invalid subtype, fall back to build */
 	if (!CHASSIS_ID_INVALID(chassis.sub)) {
-		LLDPAD_DBG("%s:%s:from config %d bytes:str=%s\n",
+		LLDPAD_DBG("%s:%s:from config %zd bytes:str=%s\n",
 			__func__, md->ifname, length, chastr);
 		/* TODO: validate the loaded tlv */
 		goto bld_tlv;
@@ -358,7 +358,7 @@ bld_config:
 
 	/* if invalid subtype, fall back to build */
 	if (!PORT_ID_INVALID(portid.sub)) {
-		LLDPAD_DBG("%s:%s:from config %d bytes:str=%s\n",
+		LLDPAD_DBG("%s:%s:from config %zd bytes:str=%s\n",
 			__func__, md->ifname, length, porstr);
 		/* TODO: validate the loaded tlv */
 		goto bld_tlv;
diff --git a/lldp_vdp.c b/lldp_vdp.c
index 2875bf5..5b8ce46 100644
--- a/lldp_vdp.c
+++ b/lldp_vdp.c
@@ -596,8 +596,8 @@ static bool vdp_vsi_set_station_state(struct vsi_profile *profile)
 	case VSI_EXIT:
 		return false;
 	default:
-		LLDPAD_ERR("ERROR: The VSI RX State Machine is broken!\n");
-		log_message(MSG_ERR_RX_SM_INVALID, "");
+		LLDPAD_ERR("%s: VSI state machine in invalid state %d\n",
+			   profile->port->ifname, profile->state);
 		return false;
 	}
 }
@@ -668,8 +668,8 @@ void vdp_vsi_sm_station(struct vsi_profile *profile)
 			vdp_remove_profile(profile);
 			break;
 		default:
-			LLDPAD_ERR("ERROR: The VSI RX station State Machine is broken!\n");
-			log_message(MSG_ERR_TX_SM_INVALID, "");
+			LLDPAD_ERR("%s: VSI state machine in invalid state %d\n",
+				   vd->ifname, profile->state);
 		}
 	} while (vdp_vsi_set_station_state(profile) == true);
 
@@ -792,8 +792,8 @@ static bool vdp_vsi_set_bridge_state(struct vsi_profile *profile)
 	case VSI_EXIT:
 		return false;
 	default:
-		LLDPAD_ERR("ERROR: The VSI RX State Machine (bridge) is broken!\n");
-		log_message(MSG_ERR_RX_SM_INVALID, "");
+		LLDPAD_ERR("%s: VSI state machine (bridge) in invalid state %d\n",
+			   profile->port->ifname, profile->state);
 		return false;
 	}
 }
@@ -850,8 +850,8 @@ static void vdp_vsi_sm_bridge(struct vsi_profile *profile)
 			vdp_remove_profile(profile);
 			break;
 		default:
-			LLDPAD_ERR("ERROR: The VSI RX bridge State Machine is broken!\n");
-			log_message(MSG_ERR_TX_SM_INVALID, "");
+			LLDPAD_ERR("%s: VSI state machine in invalid state %d\n",
+				   vd->ifname, profile->state);
 		}
 	} while (vdp_vsi_set_bridge_state(profile) == true);
 
@@ -1454,17 +1454,13 @@ struct lldp_module *vdp_register(void)
 
 	mod = malloc(sizeof(*mod));
 	if (!mod) {
-		LLDPAD_ERR("failed to malloc module data\n");
-		log_message(MSG_ERR_SERVICE_START_FAILURE,
-			"%s", "failed to malloc module data");
+		LLDPAD_ERR("lldpad failed to start - failed to malloc module data\n");
 		goto out_err;
 	}
 	ud = malloc(sizeof(struct vdp_user_data));
 	if (!ud) {
 		free(mod);
-		LLDPAD_ERR("failed to malloc module user data\n");
-		log_message(MSG_ERR_SERVICE_START_FAILURE,
-			"%s", "failed to malloc module user data");
+		LLDPAD_ERR("lldpad failed to start - failed to malloc module user data\n");
 		goto out_err;
 	}
 	LIST_INIT(&ud->head);
diff --git a/lldpad.c b/lldpad.c
index 6105985..a1a7ac1 100644
--- a/lldpad.c
+++ b/lldpad.c
@@ -370,8 +370,7 @@ int main(int argc, char *argv[])
 	 * pid as netlink address.
 	 */
 	if (event_iface_init_user_space() < 0) {
-		log_message(MSG_ERR_SERVICE_START_FAILURE,
-			"%s", "failed to register user space event interface");
+		LLDPAD_ERR("lldpad failed to start - failed to register user space event interface\n");
 		exit(1);
 	}
 
@@ -398,8 +397,7 @@ int main(int argc, char *argv[])
 	if (ctrl_iface_register(clifd) < 0) {
 		if (!daemonize)
 			fprintf(stderr, "failed to register control interface\n");
-		log_message(MSG_ERR_SERVICE_START_FAILURE,
-			    "%s", "failed to register control interface");
+		LLDPAD_ERR("lldpad failed to start - failed to register control interface\n");
 		exit(1);
 	}
 
diff --git a/log.c b/log.c
index e0aa3e5..57ac256 100644
--- a/log.c
+++ b/log.c
@@ -27,160 +27,18 @@
 #include <stdio.h>
 #include <syslog.h>
 #include <stdarg.h>
-#include "dcb_protocol.h"
 #include "messages.h"
-#include "lldpad.h"
 
-
-void log_message(u32 msgid,  const char *format,  ...)
+void log_message(int level, const char *format, ...)
 {
-	int a, b;
-	char fmt[256];
-
 	va_list va, vb;
 	va_start(va, format);
 	va_copy(vb, va);
 
-	if (!daemonize && loglvl >= msgid) {
+	if (daemonize)
+		vsyslog(level, format, vb);
+	else if (loglvl >= level)
 		vprintf(format, vb);
-		va_end(va);
-		return;
-	} else if (!daemonize) {
-		va_end(va);
-		return;
-	}
-
-	switch(msgid) {
-	case MSG_INFO_DEBUG_STRING:
-		vsyslog(LOG_DEBUG, format, vb);
-		break;
-	case MSG_ERR_SERVICE_START_FAILURE:
-		snprintf(fmt, sizeof(fmt), "lldpad failed to start - %s", format);
-		syslog(LOG_ERR, fmt, va_arg(va, char *));
-		break;
-	case MSG_ERR_RESOURCE_MEMORY:
-		break;
-	case MSG_ERR_ADD_CARD_FAILURE:
-		syslog(LOG_ERR,
-			"failed to add interface %s",
-			va_arg(va, char *));
-		break;
-	case MSG_ERR_DCB_INVALID_TX_TOTAL_BWG:
-		syslog(LOG_ERR,
-			"invalid total priority group bandwidth for tx [%d%%]",
-			va_arg(va, int));
-		break;
-	case MSG_ERR_DCB_INVALID_RX_TOTAL_BWG:
-		syslog(LOG_ERR,
-			"invalid total priority group bandwidth for rx [%d%%]",
-			va_arg(va, int));
-		break;
-	case MSG_ERR_DCB_INVALID_TX_BWG_IDX:
-		syslog(LOG_ERR,
-			"invalid transmit priority group index [%d]",
-			va_arg(va, int));
-		break;
-	case MSG_ERR_DCB_INVALID_RX_BWG_IDX:
-		syslog(LOG_ERR,
-			"invalid receive priority group index [%d]",
-			va_arg(va, int));
-		break;
-	case MSG_ERR_DCB_INVALID_TX_LSP_NZERO_BW_TC:
-		a = va_arg(va, int);
-		b = va_arg(va, int);
-		syslog(LOG_ERR,
-			"transmit link strict user priority[%d] has non-zero "
-			"bandwidth [%d%%]", a, b);
-		break;
-	case MSG_ERR_DCB_INVALID_RX_LSP_NZERO_BW_TC:
-		a = va_arg(va, int);
-		b = va_arg(va, int);
-		syslog(LOG_ERR,
-			"receive link strict user priority[%d] has non-zero "
-			"bandwidth [%d%%]", a, b);
-		break;
-	case MSG_ERR_DCB_TOO_MANY_LSP_PGIDS:
-		syslog(LOG_ERR,
-			"only one link strict priority group is allowed [%d%%]",
-			va_arg(va, int));
-		break;
-	case MSG_ERR_DCB_INVALID_TX_ZERO_BW_TC:
-		syslog(LOG_ERR,
-			"transmit user priority[%d] has zero bandwidth",
-			va_arg(va, int));
-		break;
-	case MSG_ERR_DCB_INVALID_RX_ZERO_BW_TC:
-		syslog(LOG_ERR,
-			"receive user priority[%d] has zero bandwidth",
-			va_arg(va, int));
-		break;
-	case MSG_ERR_DCB_INVALID_TX_LSP_NZERO_BWG:
-		a = va_arg(va, int);
-		b = va_arg(va, int);
-		syslog(LOG_ERR,
-			"transmit link strict priority group [%d] has a "
-			"non-zero bandwidth [%d%%]", a, b);
-		break;
-	case MSG_ERR_DCB_INVALID_RX_LSP_NZERO_BWG:
-		a = va_arg(va, int);
-		b = va_arg(va, int);
-		syslog(LOG_ERR,
-			"receive link strict priority group [%d] has a "
-			"non-zero bandwidth [%d%%]", a, b);
-		break;
-	case MSG_ERR_DCB_INVALID_TX_BWG:
-		a = va_arg(va, int);
-		b = va_arg(va, int);
-		syslog(LOG_ERR,
-			"transmit priority group [%d] has invalid total "
-			"bandwidth [%d%%], should be 0 or 100", a, b);
-		break;
-	case MSG_ERR_DCB_INVALID_RX_BWG:
-		a = va_arg(va, int);
-		b = va_arg(va, int);
-		syslog(LOG_ERR,
-			"receive priority group [%d] has invalid total "
-			"bandwidth [%d%%], should be 0 or 100", a, b);
-		break;
-	case MSG_ERR_TX_SM_INVALID:
-		syslog(LOG_ERR,
-			"LLDP transmit state machine encountered an invalid "
-			"state.");
-		break;
-	case MSG_ERR_RX_SM_INVALID:
-		syslog(LOG_ERR,
-			"LLDP receive state machine encountered an invalid "
-			"state.");
-		break;
-	case MSG_ERR_DCB_INVALID_CONFIG_FILE:
-		syslog(LOG_ERR,
-			"lldpad failed to read config file - %s",
-			va_arg(va, char *));
-		break;
-	case MSG_INFO_LLINK_DISABLED:
-		syslog(LOG_INFO,
-			"FCoE logical link on %s is disabled",
-			va_arg(va, char *));
-		break;
-	case MSG_INFO_LLINK_ENABLED:
-		syslog(LOG_INFO,
-			"FCoE logical link on %s is enabled",
-			va_arg(va, char *));
-		break;
-	case MSG_INFO_LLINK_OPER:
-		syslog(LOG_INFO,
-			"FCoE logical link on %s is operational",
-			va_arg(va, char *));
-		break;
-	case MSG_ERR_LLINK_NONOPER:
-		syslog(LOG_ERR,
-			"FCoE logical link on %s is not operational",
-			va_arg(va, char *));
-		break;
-	default:
-		vsyslog(msgid, format, vb);
-		break;
-	}
 
 	va_end(va);
 }




More information about the lldp-devel mailing list