discover: Nicely format IPMI response buffers
diff mbox series

Message ID 20181112050855.11714-1-sam@mendozajonas.com
State Accepted
Headers show
Series
  • discover: Nicely format IPMI response buffers
Related show

Commit Message

Samuel Mendoza-Jonas Nov. 12, 2018, 5:08 a.m. UTC
A few places where we print out the response buffer from an IPMI command
weren't updated when log timestamps were added, resulting in very hard
to read output. Add a little helper to format buffers and use it to
print these with only one timestamp.

Example:

[04:59:01] ipmi_get_bmc_versions: BMC version resp [0][16]:
0x00 0x20 0x01 0x02 0x13 0x02 0xbf 0x00
0x00 0x00 0xbb 0xaa 0x58 0x98 0x01 0x00

Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
---
 discover/ipmi.c             | 33 +++++++++++++++------------------
 discover/platform-powerpc.c | 23 ++++++++---------------
 lib/util/util.c             | 17 +++++++++++++++++
 lib/util/util.h             |  1 +
 4 files changed, 41 insertions(+), 33 deletions(-)

Comments

Samuel Mendoza-Jonas Nov. 16, 2018, 3:21 a.m. UTC | #1
On Mon, 2018-11-12 at 16:08 +1100, Samuel Mendoza-Jonas wrote:
> A few places where we print out the response buffer from an IPMI command
> weren't updated when log timestamps were added, resulting in very hard
> to read output. Add a little helper to format buffers and use it to
> print these with only one timestamp.
> 
> Example:
> 
> [04:59:01] ipmi_get_bmc_versions: BMC version resp [0][16]:
> 0x00 0x20 0x01 0x02 0x13 0x02 0xbf 0x00
> 0x00 0x00 0xbb 0xaa 0x58 0x98 0x01 0x00
> 
> Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>

Merged as cf30fdd3

> ---
>  discover/ipmi.c             | 33 +++++++++++++++------------------
>  discover/platform-powerpc.c | 23 ++++++++---------------
>  lib/util/util.c             | 17 +++++++++++++++++
>  lib/util/util.h             |  1 +
>  4 files changed, 41 insertions(+), 33 deletions(-)
> 
> diff --git a/discover/ipmi.c b/discover/ipmi.c
> index 840fdee5..ae02bb0a 100644
> --- a/discover/ipmi.c
> +++ b/discover/ipmi.c
> @@ -326,6 +326,7 @@ void ipmi_get_bmc_mac(struct ipmi *ipmi, uint8_t *buf)
>  	uint16_t resp_len = 8;
>  	uint8_t resp[8];
>  	uint8_t req[] = { 0x1, 0x5, 0x0, 0x0 };
> +	char *debug_buf;
>  	int i, rc;
>  
>  	rc = ipmi_transaction(ipmi, IPMI_NETFN_TRANSPORT,
> @@ -334,14 +335,15 @@ void ipmi_get_bmc_mac(struct ipmi *ipmi, uint8_t *buf)
>  			resp, &resp_len,
>  			ipmi_timeout);
>  
> -	pb_debug_fn("BMC MAC resp [%d][%d]:\n", rc, resp_len);
> +	debug_buf = format_buffer(ipmi, resp, resp_len);
> +	pb_debug_fn("BMC MAC resp [%d][%d]:\n%s\n",
> +			rc, resp_len, debug_buf);
> +	talloc_free(debug_buf);
>  
>  	if (rc == 0 && resp_len > 0) {
>  		for (i = 2; i < resp_len; i++) {
> -		        pb_debug(" %x", resp[i]);
>  			buf[i - 2] = resp[i];
>  		}
> -		pb_debug("\n");
>  	}
>  
>  }
> @@ -354,7 +356,8 @@ void ipmi_get_bmc_versions(struct ipmi *ipmi, struct system_info *info)
>  {
>  	uint16_t resp_len = 16;
>  	uint8_t resp[16], bcd;
> -	int i, rc;
> +	char *debug_buf;
> +	int rc;
>  
>  	/* Retrieve info from current side */
>  	rc = ipmi_transaction(ipmi, IPMI_NETFN_APP,
> @@ -363,13 +366,10 @@ void ipmi_get_bmc_versions(struct ipmi *ipmi, struct system_info *info)
>  			resp, &resp_len,
>  			ipmi_timeout);
>  
> -	pb_debug_fn("BMC version resp [%d][%d]:\n", rc, resp_len);
> -	if (resp_len > 0) {
> -		for (i = 0; i < resp_len; i++) {
> -		        pb_debug(" %x", resp[i]);
> -		}
> -		pb_debug("\n");
> -	}
> +	debug_buf = format_buffer(ipmi, resp, resp_len);
> +	pb_debug_fn("BMC version resp [%d][%d]:\n%s\n",
> +			rc, resp_len, debug_buf);
> +	talloc_free(debug_buf);
>  
>  	if (rc == 0 && (resp_len == 12 || resp_len == 16)) {
>  		info->bmc_current = talloc_array(info, char *, 4);
> @@ -407,13 +407,10 @@ void ipmi_get_bmc_versions(struct ipmi *ipmi, struct system_info *info)
>  			resp, &resp_len,
>  			ipmi_timeout);
>  
> -	pb_debug_fn("BMC golden resp [%d][%d]:\n", rc, resp_len);
> -	if (resp_len > 0) {
> -		for (i = 0; i < resp_len; i++) {
> -		        pb_debug(" %x", resp[i]);
> -		}
> -		pb_debug("\n");
> -	}
> +	debug_buf = format_buffer(ipmi, resp, resp_len);
> +	pb_debug_fn("BMC golden resp [%d][%d]:\n%s\n",
> +			rc, resp_len, debug_buf);
> +	talloc_free(debug_buf);
>  
>  	if (rc == 0 && (resp_len == 12 || resp_len == 16)) {
>  		info->bmc_golden = talloc_array(info, char *, 4);
> diff --git a/discover/platform-powerpc.c b/discover/platform-powerpc.c
> index 2929077e..f8f33054 100644
> --- a/discover/platform-powerpc.c
> +++ b/discover/platform-powerpc.c
> @@ -368,6 +368,7 @@ static int get_ipmi_bootdev_ipmi(struct platform_powerpc *platform,
>  {
>  	uint16_t resp_len;
>  	uint8_t resp[8];
> +	char *debug_buf;
>  	int rc;
>  	uint8_t req[] = {
>  		0x05, /* parameter selector: boot flags */
> @@ -392,10 +393,9 @@ static int get_ipmi_bootdev_ipmi(struct platform_powerpc *platform,
>  		return -1;
>  	}
>  
> -	pb_debug("IPMI get_bootdev response:\n");
> -	for (int i = 0; i < resp_len; i++)
> -		pb_debug("%x ", resp[i]);
> -	pb_debug("\n");
> +	debug_buf = format_buffer(platform, resp, resp_len);
> +	pb_debug_fn("IPMI get_bootdev response:\n%s\n", debug_buf);
> +	talloc_free(debug_buf);
>  
>  	if (resp[0] != 0) {
>  		pb_log("platform: non-zero completion code %d from IPMI req\n",
> @@ -472,6 +472,7 @@ static void get_ipmi_network_override(struct platform_powerpc *platform,
>  	uint16_t min_len = 12, resp_len = 53, version;
>  	const uint32_t magic_value = 0x21706221;
>  	uint8_t resp[resp_len];
> +	char *debug_buf;
>  	uint32_t cookie;
>  	bool persistent;
>  	int i, rc;
> @@ -487,17 +488,9 @@ static void get_ipmi_network_override(struct platform_powerpc *platform,
>  			resp, &resp_len,
>  			ipmi_timeout);
>  
> -	pb_debug("IPMI net override resp [%d][%d]:\n", rc, resp_len);
> -	if (resp_len > 0) {
> -		for (i = 0; i < resp_len; i++) {
> -		        pb_debug(" %02x", resp[i]);
> -			if (i && (i + 1) % 16 == 0 && i != resp_len - 1)
> -				pb_debug("\n");
> -			else if (i && (i + 1) % 8 == 0)
> -				pb_debug(" ");
> -		}
> -		pb_debug("\n");
> -	}
> +	debug_buf = format_buffer(platform, resp, resp_len);
> +	pb_debug_fn("IPMI net override response:\n%s\n", debug_buf);
> +	talloc_free(debug_buf);
>  
>  	if (rc) {
>  		pb_debug("IPMI network config option unavailable\n");
> diff --git a/lib/util/util.c b/lib/util/util.c
> index a18926c1..36d45b84 100644
> --- a/lib/util/util.c
> +++ b/lib/util/util.c
> @@ -19,6 +19,7 @@
>  #include <assert.h>
>  
>  #include <util/util.h>
> +#include <talloc/talloc.h>
>  
>  static const char hex[] = { '0', '1', '2', '3', '4', '5', '6', '7',
>  			    '8', '9', 'a', 'b', 'c', 'd', 'e', 'f', };
> @@ -47,3 +48,19 @@ void mac_str(uint8_t *mac, unsigned int maclen, char *buf, unsigned int buflen)
>  
>  	return;
>  }
> +
> +char *format_buffer(void *ctx, const uint8_t *buf, unsigned int len)
> +{
> +	char *str;
> +	unsigned int i;
> +
> +	if (len == 0)
> +		return "";
> +
> +	str = talloc_asprintf(ctx, "0x%02x%s", buf[0], len > 1 ? " " : "");
> +	for (i = 1; i < len; i++)
> +		str = talloc_asprintf_append(str, "0x%02x%s", buf[i],
> +			((i + 1) % 8 == 0 && i != len - 1) ? "\n" : " ");
> +
> +	return str;
> +}
> diff --git a/lib/util/util.h b/lib/util/util.h
> index 39966d0b..a579221a 100644
> --- a/lib/util/util.h
> +++ b/lib/util/util.h
> @@ -50,6 +50,7 @@
>  	do { (void)sizeof(char[(x)?1:-1]); } while (0)
>  
>  void mac_str(uint8_t *mac, unsigned int maclen, char *buf, unsigned int buflen);
> +char *format_buffer(void *ctx, const uint8_t *buf, unsigned int len);
>  
>  #endif /* UTIL_H */
>

Patch
diff mbox series

diff --git a/discover/ipmi.c b/discover/ipmi.c
index 840fdee5..ae02bb0a 100644
--- a/discover/ipmi.c
+++ b/discover/ipmi.c
@@ -326,6 +326,7 @@  void ipmi_get_bmc_mac(struct ipmi *ipmi, uint8_t *buf)
 	uint16_t resp_len = 8;
 	uint8_t resp[8];
 	uint8_t req[] = { 0x1, 0x5, 0x0, 0x0 };
+	char *debug_buf;
 	int i, rc;
 
 	rc = ipmi_transaction(ipmi, IPMI_NETFN_TRANSPORT,
@@ -334,14 +335,15 @@  void ipmi_get_bmc_mac(struct ipmi *ipmi, uint8_t *buf)
 			resp, &resp_len,
 			ipmi_timeout);
 
-	pb_debug_fn("BMC MAC resp [%d][%d]:\n", rc, resp_len);
+	debug_buf = format_buffer(ipmi, resp, resp_len);
+	pb_debug_fn("BMC MAC resp [%d][%d]:\n%s\n",
+			rc, resp_len, debug_buf);
+	talloc_free(debug_buf);
 
 	if (rc == 0 && resp_len > 0) {
 		for (i = 2; i < resp_len; i++) {
-		        pb_debug(" %x", resp[i]);
 			buf[i - 2] = resp[i];
 		}
-		pb_debug("\n");
 	}
 
 }
@@ -354,7 +356,8 @@  void ipmi_get_bmc_versions(struct ipmi *ipmi, struct system_info *info)
 {
 	uint16_t resp_len = 16;
 	uint8_t resp[16], bcd;
-	int i, rc;
+	char *debug_buf;
+	int rc;
 
 	/* Retrieve info from current side */
 	rc = ipmi_transaction(ipmi, IPMI_NETFN_APP,
@@ -363,13 +366,10 @@  void ipmi_get_bmc_versions(struct ipmi *ipmi, struct system_info *info)
 			resp, &resp_len,
 			ipmi_timeout);
 
-	pb_debug_fn("BMC version resp [%d][%d]:\n", rc, resp_len);
-	if (resp_len > 0) {
-		for (i = 0; i < resp_len; i++) {
-		        pb_debug(" %x", resp[i]);
-		}
-		pb_debug("\n");
-	}
+	debug_buf = format_buffer(ipmi, resp, resp_len);
+	pb_debug_fn("BMC version resp [%d][%d]:\n%s\n",
+			rc, resp_len, debug_buf);
+	talloc_free(debug_buf);
 
 	if (rc == 0 && (resp_len == 12 || resp_len == 16)) {
 		info->bmc_current = talloc_array(info, char *, 4);
@@ -407,13 +407,10 @@  void ipmi_get_bmc_versions(struct ipmi *ipmi, struct system_info *info)
 			resp, &resp_len,
 			ipmi_timeout);
 
-	pb_debug_fn("BMC golden resp [%d][%d]:\n", rc, resp_len);
-	if (resp_len > 0) {
-		for (i = 0; i < resp_len; i++) {
-		        pb_debug(" %x", resp[i]);
-		}
-		pb_debug("\n");
-	}
+	debug_buf = format_buffer(ipmi, resp, resp_len);
+	pb_debug_fn("BMC golden resp [%d][%d]:\n%s\n",
+			rc, resp_len, debug_buf);
+	talloc_free(debug_buf);
 
 	if (rc == 0 && (resp_len == 12 || resp_len == 16)) {
 		info->bmc_golden = talloc_array(info, char *, 4);
diff --git a/discover/platform-powerpc.c b/discover/platform-powerpc.c
index 2929077e..f8f33054 100644
--- a/discover/platform-powerpc.c
+++ b/discover/platform-powerpc.c
@@ -368,6 +368,7 @@  static int get_ipmi_bootdev_ipmi(struct platform_powerpc *platform,
 {
 	uint16_t resp_len;
 	uint8_t resp[8];
+	char *debug_buf;
 	int rc;
 	uint8_t req[] = {
 		0x05, /* parameter selector: boot flags */
@@ -392,10 +393,9 @@  static int get_ipmi_bootdev_ipmi(struct platform_powerpc *platform,
 		return -1;
 	}
 
-	pb_debug("IPMI get_bootdev response:\n");
-	for (int i = 0; i < resp_len; i++)
-		pb_debug("%x ", resp[i]);
-	pb_debug("\n");
+	debug_buf = format_buffer(platform, resp, resp_len);
+	pb_debug_fn("IPMI get_bootdev response:\n%s\n", debug_buf);
+	talloc_free(debug_buf);
 
 	if (resp[0] != 0) {
 		pb_log("platform: non-zero completion code %d from IPMI req\n",
@@ -472,6 +472,7 @@  static void get_ipmi_network_override(struct platform_powerpc *platform,
 	uint16_t min_len = 12, resp_len = 53, version;
 	const uint32_t magic_value = 0x21706221;
 	uint8_t resp[resp_len];
+	char *debug_buf;
 	uint32_t cookie;
 	bool persistent;
 	int i, rc;
@@ -487,17 +488,9 @@  static void get_ipmi_network_override(struct platform_powerpc *platform,
 			resp, &resp_len,
 			ipmi_timeout);
 
-	pb_debug("IPMI net override resp [%d][%d]:\n", rc, resp_len);
-	if (resp_len > 0) {
-		for (i = 0; i < resp_len; i++) {
-		        pb_debug(" %02x", resp[i]);
-			if (i && (i + 1) % 16 == 0 && i != resp_len - 1)
-				pb_debug("\n");
-			else if (i && (i + 1) % 8 == 0)
-				pb_debug(" ");
-		}
-		pb_debug("\n");
-	}
+	debug_buf = format_buffer(platform, resp, resp_len);
+	pb_debug_fn("IPMI net override response:\n%s\n", debug_buf);
+	talloc_free(debug_buf);
 
 	if (rc) {
 		pb_debug("IPMI network config option unavailable\n");
diff --git a/lib/util/util.c b/lib/util/util.c
index a18926c1..36d45b84 100644
--- a/lib/util/util.c
+++ b/lib/util/util.c
@@ -19,6 +19,7 @@ 
 #include <assert.h>
 
 #include <util/util.h>
+#include <talloc/talloc.h>
 
 static const char hex[] = { '0', '1', '2', '3', '4', '5', '6', '7',
 			    '8', '9', 'a', 'b', 'c', 'd', 'e', 'f', };
@@ -47,3 +48,19 @@  void mac_str(uint8_t *mac, unsigned int maclen, char *buf, unsigned int buflen)
 
 	return;
 }
+
+char *format_buffer(void *ctx, const uint8_t *buf, unsigned int len)
+{
+	char *str;
+	unsigned int i;
+
+	if (len == 0)
+		return "";
+
+	str = talloc_asprintf(ctx, "0x%02x%s", buf[0], len > 1 ? " " : "");
+	for (i = 1; i < len; i++)
+		str = talloc_asprintf_append(str, "0x%02x%s", buf[i],
+			((i + 1) % 8 == 0 && i != len - 1) ? "\n" : " ");
+
+	return str;
+}
diff --git a/lib/util/util.h b/lib/util/util.h
index 39966d0b..a579221a 100644
--- a/lib/util/util.h
+++ b/lib/util/util.h
@@ -50,6 +50,7 @@ 
 	do { (void)sizeof(char[(x)?1:-1]); } while (0)
 
 void mac_str(uint8_t *mac, unsigned int maclen, char *buf, unsigned int buflen);
+char *format_buffer(void *ctx, const uint8_t *buf, unsigned int len);
 
 #endif /* UTIL_H */