Message ID | 20181112050855.11714-1-sam@mendozajonas.com |
---|---|
State | Accepted |
Headers | show |
Series | discover: Nicely format IPMI response buffers | expand |
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 */ >
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 */
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(-)