Al,

Attached is a cleaned up patch that changes the function to static and renames it. The message format is now included in a format that hopefully makes sense. I tried making it look like the other documentation in ipmi-oem-dell.c. I changed the error message to display if the completion code is non-zero. I'm open to removing the error handler if you still think it's unnecessary.

A very simple patch to the manpage is also included. All it does is add cmc-firmware-version to the list of get-system-info subcommands. The system support listed for get-system-info is still valid.

Let me know if I should make any other changes.

Ryan

On 05/18/2012 12:53 PM, Ryan Cox wrote:
Al,

Thanks. I did indeed have some problems with email that should now be fixed.

I'll make the changes in the next few days. You are correct that the reason for the error handling was to make a nicer error message. The alternative was to leave it as "dell:get-system-info failed: No error message found for command DFh, network function 06h, and completion code 80h. Please report to <[email protected]>".

I'll be sure to add the response format in comments and an entry in the manpage.

Ryan

On 05/18/2012 12:41 PM, Albert Chu wrote:
This was my reply if it didn't get through your filters

Al

-- Albert Chu [email protected] Computer Scientist High Performance Systems Division Lawrence Livermore National Laboratory

Re: [Freeipmi-devel] PATCH: ipmi-oem dell get-system-info.eml

Subject:
Re: [Freeipmi-devel] PATCH: ipmi-oem dell get-system-info cmc-firmware-version
From:
Albert Chu <[email protected]>
Date:
05/18/2012 12:19 PM

To:
Ryan Cox <[email protected]>
CC:
"[email protected]" <[email protected]>


Hey Ryan,

For the most part looks fine.  I assume the final patch will also clean
up the code comments to describe the CMC version packet:-)   Just a few
notes:

+int
+ipmi_oem_dell_get_cmc_firmware_version (ipmi_oem_state_data_t
*state_data)

since this is an internal function it should be static and not have an
entry in the header file.  For naming consistency, it should probably be
_get_dell_cmc_firmware_version() or something similar to the other
functions called by ipmi_oem_get_system_info().

+  if(rs_len != 20) {
+      pstdout_fprintf (state_data->pstate,
+                       stderr,
+                       "dell:get-system-info 'cmc-firmware-version'
option not supported on this system\n");
+      goto cleanup;
+  }

I don't think this check is really necessary, it'll be done via
ipmi_oem_check_response_and_completion_code().  I suppose you added it
for a nicer error message, but with ipmi-oem, I think there is a larger
acceptance of "you gotta know what you're doing" b/c it's so
OEM/motherboard/system/product specific (vs ipmi-sensors and more
standard tools).  The manpage should document the motherboards its
confirmed against or types of motherboards its expected to work with.

Al

On Fri, 2012-05-18 at 10:34 -0700, Ryan Cox wrote:
>  This patch adds "ipmi-oem dell get-system-info cmc-firmware-version".
>     From a blade, it grabs the firmware version of the CMC for the
>  enclosure that the blade is in.  It appears to only show the active CMC
>  firmware and not show anything for the standby CMC (if there is one).  I
>  have no idea what it would show if there is a redundant CMC using
>  different firmware.  This patch has worked for every Dell blade (10G,
>  11G, 12G) we have in an M1000e enclosure, including M1000e's with one
>  and two CMC's.
> > The version can be grabbed with: ipmi-raw 0 6 0x59 0 0xDF 1 0
>  rcvd: 59 00 11 01 00 00 00 00 33 2E 30 33 00 00 00 00 00 00 00 00
> > "3.03" in this case. I have tried this with several 3.x and 4.x
>  versions and the version numbers are all 4 characters long.
> > As far as I can tell, there is no easy way to grab this through existing
>  generic functions in ipmi-oem-dell.c.  Those functions appear to assume
>  set selector and block selector 0, though the set selector is different
>  in this case.  I had to decide to either reinvent the wheel somewhat or
>  modify the other functions to take selectors as parameters instead of
>  assuming 0.  I took the easy route and reinvented the wheel, though I'm
>  open to changing that if needed.  I figured I should at least get some
>  feedback before submitting a patch that changes a whole lot of code.
> > This patch may need some reworking, but it has been tested successfully
>  on: M600, M610, M610x, M910, and a pre-prod 12G system.  On a rackmount,
>  it returns "dell:get-system-info 'cmc-firmware-version' option not
>  supported on this system".
> > Let me know if there's anything that needs to be changed and if you
>  would prefer the slight duplication of code that I did versus adding set
>  and block selector parameters to several functions.
>
-- Albert Chu [email protected] Computer Scientist High Performance Systems Division Lawrence Livermore National Laboratory _______________________________________________ Freeipmi-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/freeipmi-devel

--
Ryan Cox
Systems Administrator
Fulton Supercomputing Lab
Brigham Young University


_______________________________________________
Freeipmi-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/freeipmi-devel

--
Ryan Cox
Systems Administrator
Fulton Supercomputing Lab
Brigham Young University

--- ipmi-oem.8.pre.in.orig	2012-05-21 12:04:05.403167000 -0600
+++ ipmi-oem.8.pre.in	2012-05-18 09:39:12.468223000 -0600
@@ -80,11 +80,11 @@ Valid keys are \fIguid\fR, \fIasset\-tag
 \fIboard\-revision\fR, \fIplatform\-model\-name\fR,
 \fIslot\-number\fR, \fIsystem\-revision\fR, \fIidrac\-info\fR,
 \fIidrac\-ipv4\-url\fR, \fIidrac\-gui\-webserver\-control\fR,
-\fIcmc\-ipv4\-url\fR, \fIcmc\-ipv6\-info\fR, \fIcmc\-ipv6\-url\fR,
-\fImac\-addresses\fR.  Command confirmed to work on Dell Poweredge
-2900, 2950, R610, R710, M600, M610, M610X, M910, and R905.  However,
-individual system information options may not be readable or available
-on every system.
+\fIcmc\-firmware\-version\fR, \fIcmc\-ipv4\-url\fR,
+\fIcmc\-ipv6\-info\fR, \fIcmc\-ipv6\-url\fR, \fImac\-addresses\fR.  
+Command confirmed to work on Dell Poweredge 2900, 2950, R610, R710,
+M600, M610, M610X, M910, and R905.  However, individual system
+information options may not be readable or available on every system.
 .TP
 .B get-nic-selection
 This OEM command will determine the current NIC selection for IPMI as
--- ipmi-oem-dell.c.orig	2012-05-18 09:19:48.962028000 -0600
+++ ipmi-oem-dell.c	2012-05-21 11:58:55.617101000 -0600
@@ -374,6 +374,7 @@
 #define IPMI_OEM_DELL_PORT_MAP_SLOT_MAPPING_1_2_OR_1_4        1
 #define IPMI_OEM_DELL_PORT_MAP_SLOT_MAPPING_1_8               2
 
+#define IPMI_OEM_DELL_CMC_FIRMWARE_VERSION 0xDF
 
 /* Will call ipmi_cmd_get_system_info_parameters only once, b/c field
  * requested is defined by OEM to be < 16 bytes in length
@@ -1797,6 +1798,94 @@ _output_dell_system_info_11g_or_12g_mac_
   return (rv);
 }
 
+static int
+_get_dell_cmc_firmware_version (ipmi_oem_state_data_t *state_data)
+{
+  uint8_t bytes_rq[IPMI_OEM_MAX_BYTES];
+  uint8_t bytes_rs[IPMI_OEM_MAX_BYTES];
+  uint8_t bytes_read[4];
+  int rs_len;
+  int rv = -1;
+
+  assert (state_data);
+  assert (!state_data->prog_data->args->oem_options_count);
+
+  /* Dell Poweredge OEM
+   *
+   * Ryan Cox: found by walking through addresses
+   *
+   * Get CMC Firmware Version
+   *
+   * 0x06 - OEM network function
+   * 0x59 - OEM cmd
+   * 0x00
+   * 0xDF - CMC Info?
+   * 0x01 - set selector (0x00 contains at least the CMC IPv4 addr)
+   * 0x00 - block selector
+   * 
+   * Get CMC Firmware Version Response
+   *
+   * 0x59 - OEM cmd
+   * 0x?? - Completion Code
+   * 0x11 - length
+   * 0x01 - set selector
+   * 0x00 - unknown
+   * 0x00 - unknown
+   * 0x00 - unknown
+   * 0x00 - unknown
+   * 0x?? - Firmware version char 0
+   * 0x?? - Firmware version char 1
+   * 0x?? - Firmware version char 2
+   * 0x?? - Firmware version char 3
+   * trailing 8 bytes: 0x00 - unknown
+   *
+   */
+
+  bytes_rq[0] = IPMI_CMD_GET_SYSTEM_INFO_PARAMETERS;
+  bytes_rq[1] = 0x00;
+  bytes_rq[2] = IPMI_OEM_DELL_CMC_FIRMWARE_VERSION;
+  bytes_rq[3] = 0x01;
+  bytes_rq[4] = 0x00;
+
+  if ((rs_len = ipmi_cmd_raw (state_data->ipmi_ctx,
+                              0, /* lun */
+                              IPMI_NET_FN_APP_RQ, /* network function */
+                              bytes_rq, /* data */
+                              5, /* num bytes */
+                              bytes_rs,
+                              IPMI_OEM_MAX_BYTES)) < 0)
+    {
+      pstdout_fprintf (state_data->pstate,
+                       stderr,
+                       "ipmi_cmd_raw: %s\n",
+                       ipmi_ctx_errormsg (state_data->ipmi_ctx));
+      goto cleanup;
+    }
+
+  if(rs_len != 20) {
+      pstdout_fprintf (state_data->pstate,
+                       stderr,
+                       "dell:get-system-info 'cmc-firmware-version' option not supported on this system\n");
+      goto cleanup;
+  }
+
+  if (ipmi_oem_check_response_and_completion_code (state_data,
+                                                   bytes_rs,
+                                                   rs_len,
+                                                   20,
+                                                   IPMI_OEM_DELL_CMC_FIRMWARE_VERSION,
+                                                   IPMI_NET_FN_APP_RQ,
+                                                   NULL) < 0)
+    goto cleanup;
+
+    memcpy(bytes_read, &bytes_rs[8], 4);
+    pstdout_printf (state_data->pstate, "%s\n", bytes_read);
+  
+  rv = 0;
+ cleanup:
+  return (rv);
+}
+
 int
 ipmi_oem_dell_get_system_info (ipmi_oem_state_data_t *state_data)
 {
@@ -1824,6 +1913,7 @@ ipmi_oem_dell_get_system_info (ipmi_oem_
 		      "Option: idrac-info\n"
 		      "Option: idrac-ipv4-url\n"
 		      "Option: idrac-gui-webserver-control\n"
+		      "Option: cmc-firmware-version\n"
 		      "Option: cmc-ipv4-url\n"
 		      "Option: cmc-ipv6-info\n"
 		      "Option: cmc-ipv6-url\n"
@@ -1857,6 +1947,7 @@ ipmi_oem_dell_get_system_info (ipmi_oem_
       && strcasecmp (state_data->prog_data->args->oem_options[0], "idrac-info")
       && strcasecmp (state_data->prog_data->args->oem_options[0], "idrac-ipv4-url")
       && strcasecmp (state_data->prog_data->args->oem_options[0], "idrac-gui-webserver-control")
+      && strcasecmp (state_data->prog_data->args->oem_options[0], "cmc-firmware-version")
       && strcasecmp (state_data->prog_data->args->oem_options[0], "cmc-ipv4-url")
       && strcasecmp (state_data->prog_data->args->oem_options[0], "cmc-ipv6-info")
       && strcasecmp (state_data->prog_data->args->oem_options[0], "cmc-ipv6-url")
@@ -2197,6 +2288,11 @@ ipmi_oem_dell_get_system_info (ipmi_oem_
 		      "%s\n",
 		      idrac_web_gui_server_control_str);
     }
+   else if (!strcasecmp (state_data->prog_data->args->oem_options[0], "cmc-firmware-version"))
+    {
+       if (_get_dell_cmc_firmware_version (state_data) < 0)
+         goto cleanup;
+    }
   else if (!strcasecmp (state_data->prog_data->args->oem_options[0], "cmc-ipv4-url"))
     {
       if (_get_dell_system_info_long_string (state_data,
_______________________________________________
Freeipmi-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/freeipmi-devel

Reply via email to