FreeIPMI Coding by Albert Chu chu11@llnl.gov These are some short descriptions on coding style, API style, design decisions, and various other thoughts. 1) Code Style ------------- The GNU coding style was selected for FreeIPMI. Please try to follow the coding style used in the rest of FreeIPMI. Here's a short example that covers the generics of our code style. int main(int argc, char **argv) { int a = 0; int b = 1; if (a == 1) printf("yoda\n"); if (a == 5 || b == 1) { printf("foobar\n"); printf("xyzzy\n"); } while (a++ < 5) printf("lala\n"); while (b++ < 7) { printf("blah\n"); printf("garble\n"); } } 2) Parameter Checking --------------------- Please carefully check the input parameters on the inputs your program and/or functions take. Minor parsing issues can lead to catastrophic mistakes in IPMI. For example, suppose you have a --power-control option that takes a number to represent a type of operation (on, off, etc.). Suppose a user inputs "--power-control=foobar". The "foobar" will be read as a '0' by strtoul(). If not properly checked, the '0' can be passed to the IPMI Chassis Control command, which uses the '0' to power off a node. In programs, when appropriate, output error messages to the user indicating that how and why the inputted parameters were incorrect. 3) Code Consistency ------------------- Please keep your code as consistent as possible to other code in FreeIPMI. That includes code indenting style, brace style, API style, and naming convention (which is discussed in more detail below). Although there may be situations that a particular API style or naming convention will make things easier for you and your code (such as shortening the name of a function, decreasing the number of parameters you need to pass to a function via a struct, etc.), we ask that your code be consistent so that it does not confuse other developers. If there is a distinct technical reason that you must use a different API style, please bring it up with the FreeIPMI authors. For example, pretty much all of the "fill" functions in libfreeipmi take the exact parameters they need to fill the fiid object which is passed along as a parameter. All parameters are passed by value, not by a pointer or other method (e.g. object, struct, etc.). Exceptions do exist. For example, fill_cmd_chassis_identify() takes parameters passed by pointer instead of passed by value. The reason is that both fields are optional and need not be filled according to the IPMI specification. The pointer gives the caller the ability to set values (by passing a valid pointer) or not (by passing NULL). 4) Libfreeipmi naming/function parameter conventions ---------------------------------------------------- The naming style in libfreeipmi was developed primarily for the purpose of readability when code is being compared to the IPMI specification. Due to the size of the IPMI spec, there will be a lot of code. In earlier versions of FreeIPMI, there was confusion on where code was located, what parameters were called, how parameters should be input, etc. due to different people using different abbreviations styles, putting functions out of order with the spec, in different files, using/not-using different bitmasks, etc. The code has been auditted and cleaned up since then. So when adding new functions/templates/parameters/files/etc. to libfreeipmi, please name them consistently to the rest of the libfreeipmi library and the IPMI specification. This includes: - naming functions/templates/parameters/files based on the spec - in most cases, not abbreviating any words (or using consistent abbreviations in the rest of the library, check first!) - matching parameter lists to the templates and in the same order - ordering functions/templates/parameters/files/etc. consistently with the spec. For example: ipmi-messaging-support-cmds.c is the file for messaging support commands, chapter 22 of the IPMI 2.0 spec. tmpl_cmd_get_channel_authentication_capabilities_rq tmpl_cmd_get_channel_authentication_capabilities_rs are the templates for the Get Channel Authentication Capapilities command. fiid_template_t tmpl_cmd_get_channel_authentication_capabilities_rq = { { 8, "cmd", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 4, "channel_number", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 3, "reserved1", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 1, "get_ipmi_v2.0_extended_data", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 4, "maximum_privilege_level", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 4, "reserved2", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 0, "", 0} }; fiid_template_t tmpl_cmd_get_channel_authentication_capabilities_rs = { { 8, "cmd", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED | FIID_FIELD_MAKES_PACKET_SUFFICIENT}, { 8, "comp_code", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED | FIID_FIELD_MAKES_PACKET_SUFFICIENT}, { 8, "channel_number", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 1, "authentication_type.none", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 1, "authentication_type.md2", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 1, "authentication_type.md5", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 1, "authentication_type.reserved1", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 1, "authentication_type.straight_password_key", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 1, "authentication_type.oem_prop", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 1, "authentication_type.reserved2", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 1, "authentication_type.ipmi_v2.0_extended_capabilities_available", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 1, "authentication_status.anonymous_login", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 1, "authentication_status.null_username", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 1, "authentication_status.non_null_username", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 1, "authentication_status.user_level_authentication", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 1, "authentication_status.per_message_authentication", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 1, "authentication_status.k_g", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 2, "authentication_status.reserved", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 1, "channel_supports_ipmi_v1.5_connections", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 1, "channel_supports_ipmi_v2.0_connections", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 6, "reserved", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 24, "oem_id", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 8, "oem_auxiliary_data", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, { 0, "", 0} }; The fields listed in the templates above directly match table 22-15, the Get Channel Authentication Capabilities request and response commands. int fill_cmd_get_channel_authentication_capabilities (uint8_t channel_number, uint8_t maximum_privilege_level, fiid_obj_t obj_cmd_rq); The function above matches the naming and takes exactly the parameters needed by the Get Channel Authentication Capabilities request template. The coding conditions specified above may lead to function names or function parameters names that exceed the 80 column mark or having very long parameter lists. We accept this annoyance (or poor coding style, we admit it), as we consider matching the specification as a more important need in libfreeipmi. For example: int fill_cmd_set_lan_configuration_parameters_authentication_type_enables (uint8_t channel_number, uint8_t callback_level_none, uint8_t callback_level_md2, uint8_t callback_level_md5, uint8_t callback_level_straight_password, uint8_t callback_level_oem_proprietary, uint8_t user_level_none, uint8_t user_level_md2, uint8_t user_level_md5, uint8_t user_level_straight_password, uint8_t user_level_oem_proprietary, uint8_t operator_level_none, uint8_t operator_level_md2, uint8_t operator_level_md5, uint8_t operator_level_straight_password, uint8_t operator_level_oem_proprietary, uint8_t admin_level_none, uint8_t admin_level_md2, uint8_t admin_level_md5, uint8_t admin_level_straight_password, uint8_t admin_level_oem_proprietary, uint8_t oem_level_none, uint8_t oem_level_md2, uint8_t oem_level_md5, uint8_t oem_level_straight_password, uint8_t oem_level_oem_proprietary, fiid_obj_t obj_cmd_rq); The function name and parameters look pretty long and terrible. But the names and fields exactly match the get authentication type enables fields listed in Table 23-4. There should be very little difficulty understanding what this funciton does, how it should be called, and what the parameters are if you are reading along with the spec. Because we want the code to match the IPMI spec as closely as possible, we currently accept the code inefficiencies (due to large stacks of parameters) that come with having long parameters lists and the atrocities of having gigantic 25+ parameter function calls in code. 5) Fiid vs. other Marshalling/Unmarshalling Styles -------------------------------------------------- Several programmers have asked us why we have chosen a relatively unpopular/different method to marshall/unmarshall IPMI packets and build network packets. First, lets discuss several classic methods for marshalling/unmarshalling data when using structs to represent a packet. Method A: Marshall/Unmarshall "manually": ----------------------------------------- struct packet { uint8_t field_1; /* 1 bit */ uint8_t field_2; /* 3 bits */ uint8_t field_3; /* 4 bits */ int16_t field_4; /* 16 bits */ }; my_marshall_function(struct packet *pkt, char *buf, unsigned int buflen) { buf[0] |= pkt->field_1 & 0x1; buf[0] |= (pkt->field_2 << 1) & 0x0E; buf[0] |= (pkt->field_3 << 4) & 0xF0; /* assuming network byte order here */ buf[1] |= (pkt->field_4 & 0xFF00) >> 8; buf[2] |= pkt->field_4 & 0x00FF; } my_unmarshall_function(struct packet *pkt, char *buf, unsigned int buflen) { pkt->field_1 = buf[0] & 0x01; pkt->field_2 = buf[0] & 0x0E >> 1; pkt->field_3 = buf[0] & 0xF0 >> 4; #if LITTLE_ENDIAN_HOST pkt->field_4 = buf[2] | buf[1] << 8;; #else pkt->field_4 = buf[1] | buf[2] << 8;; #endif } general_usage_example() { struct packet pkt; char buf[1024]; int len; pkt.field_1 = 1; pkt.field_2 = 2; pkt.field_3 = 3; pkt.field_4 = 5; my_marshall_function(&pkt, buf, 1024); /* then do something with the buffer */ len = my_receive_data_function(buf); my_unmarshall_function(&pkt, buf, len); printf("field_1 is: %d\n", pkt.field_1); } Pros: A) No need to deal with struct packing issues in the compiler. B) The struct definition describes packets closely and is relatively easy to use and understand. C) Relatively efficient. D) General usage code size is relatively small. E) General usage need not determine field type (e.g. is it an unsigned or signed integer). Cons: A) Have to deal with endian problems. B) Lots of marshalling and unmarshalling code are required for each packet type. C) Relatively difficult to deal with optional fields. (You'll need flags in the struct to indicate if a field was set/unset, or validate the fields via protocol definition knowledge.) D) Relatively difficult to deal with variable length fields. (You'll need a length parameter in the struct to indicate the length of a field.) E) Packet dumps/debugging is relatively poor (you only get hex) or you have to create debug functions to handle each packet type. Method B: Cast a buffer to a packed struct: ------------------------------------------- For Example: struct packet { uint8_t field_1 : 1; uint8_t field_2 : 3; uint8_t field_3 : 4; int16_t field_4; }; my_marshall_function(struct packet *pkt, char *buf, unsigned int buflen) { memcpy(buf, pkt, sizeof(struct packet)); #if LITTLE_ENDIAN_HOST swap(&buf[1], &buf[2]); #endif } my_unmarshall_function(struct packet *pkt, char *buf, unsigned int buflen) { *pkt = *((struct packet *)buf); #if LITTLE_ENDIAN_HOST pkt->field_4 = ntohs(pkt->field_4); #endif } general_usage_example() { struct packet pkt; char buf[1024]; int len; pkt.field_1 = 1; pkt.field_2 = 2; pkt.field_3 = 3; pkt.field_4 = 5; my_marshall_function(&pkt, buf, 1024); /* then do something with the buffer */ len = my_receive_data_function(buf); my_unmarshall_function(&pkt, buf, len); printf("field_1 is: %d\n", pkt.field_1); } Pros: A) Not too much marshalling and unmarshalling code is required. B) General usage code size is relatively small. C) The struct definition describes packets exactly and is relatively easy to use and understand. D) Very efficient (little actual marshalling/unmarshalling needs to be done.) E) General usage need not determine field type (e.g. is it an unsigned or signed integer). Cons: A) Have to deal with endian problems. B) Have to deal with portability of struct packing techniques (there are differences in compilers, but nowadays, this may be easier/more portable than I originally believed it to be). C) Difficult to deal with optional fields (no flags can be put in the struct to indicate if a field was set/unset, can only validate the fields via protocol definition knowledge.) D) No mechanism to deal with variable length fields (no length field can be put in the struct to indicate the field length.) E) Packet dumps/debugging is relatively poor (you only get hex) or you have to create debug functions to handle each packet type. Our Method C: string_name -> bitmask mapping -------------------------------------------- The "FreeIPMI Interface Definition" or 'fiid' API in libfreeipmi uses a string_name/bit_count template and an API to get and set values in a packet to handle marshalling/unmarshalling. The following are a few of the API functions used for FIID to give you an idea for the fiid API: fiid_obj_t fiid_obj_create (fiid_template_t tmpl); int32_t fiid_obj_errnum(fiid_obj_t obj); int8_t fiid_obj_clear (fiid_obj_t obj); int8_t fiid_obj_set (fiid_obj_t obj, char *field, uint64_t val); int8_t fiid_obj_get (fiid_obj_t obj, char *field, uint64_t *val); int32_t fiid_obj_get_all (fiid_obj_t obj, uint8_t *data, uint32_t data_len); int32_t fiid_obj_set_all (fiid_obj_t obj, uint8_t *data, uint32_t data_len); The following is the fiid equivalent in the previous examples: fiid_template_t tmpl_example = { {1, "field_1", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, {3, "field_2", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, {4, "field_3", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, {16, "field_4", FIID_FIELD_REQUIRED | FIID_FIELD_LENGTH_FIXED}, {0, "", 0} }; general_usage_example() { fiid_obj_t obj; char buf[1024]; int len; uint64_t val; obj = fiid_obj_create(tmpl_example); fiid_obj_set(obj, "field_1", 1); fiid_obj_set(obj, "field_2", 2); fiid_obj_set(obj, "field_3", 3); fiid_obj_set(obj, "field_4", 5); /* "marshall" the packet */ fiid_obj_get_all(obj, buf, 1024); /* then do something with the buffer */ fiid_obj_clear(obj); len = my_receive_data_function(buf); /* "unmarshall" the packet */ fiid_obj_set_all(obj, buf, len); fiid_obj_get(obj, "field_1", &val); printf("field_1 is: %d\n", (int16_t)val); } The pros and cons of the fiid method are: Pros: A) No need to deal with endian problems (handled internally in the API). B) No need to deal with struct packing issues (bit shifts are handled internally in the API). C) Easier to deal with optional fields (For marshalling, don't set a field. For unmarshalling, the api can identify if a field is set or not). D) Easier to deal with variable length fields (For marshalling, set whatever length you want. For unmarshalling, the api can identify the length of the field read). E) Templates describe the packets exactly. F) Easy to do large packet dumps and debug (fields and values easily output and identified). G) Significantly reduce the amount of marshalling, unmarshalling, and debug code needed (the API "handles" it all already). Cons: A) Need to learn/use a reasonably large API and learn/use all the templates. B) Pretty inefficient (lots of string comparisons). C) Lots of duplicate templates (relative inability to "inherit" structs without #define mangling/hackery). D) General usage code size is increased. E) General usage must determine and cast field to appropriate type (e.g. is it an unsigned or signed integer). (Side Comments: Some other networking APIs have a similar API, but use macros/enums for the field names rather than strings. Many of the above benefits are identical, except the debug dump output capabilities are weaker in exchange for better performance. Some other networking APIs may return a type of a field (e.g. signed vs unsigned, 16bit vs 32bit, etc.). That would remove need to determine casting in general usage in exchange for larger general usage code size.) The big reasons why this was developed and chosen over traditional methods. A) The IPMI specification is very large, so reducing code size weighed in as an important factor for the authors. This allowed there to be fewer marshalling/unmarshalling/debug functions. By one authors counting in the specification, there are 304 different base payloads in the IPMI specification. This does not include permutations of payloads due to different versions, optional fields, headers, trailers, encryption, oem extensions, record formats data is stored in, etc. B) There are a relatively large number of optional fields and variable length fields in the IPMI specification. As stated above, the traditional struct based marshalling/unmarshalling have issues with handling these. C) The lack of IPMI compliance from vendors is a well known problem in the open-source community. The templates have saved developers countless hours of debugging time due to the easy method by which packets can be dumped with their fields and values quickly identified. It is very easy to find vendor IPMI compliance problems very quickly. Here's an example of a dump: pwopr2: : RMCP Header: pwopr2: : ------------ pwopr2: [ 6h] = version[ 8b] pwopr2: [ 0h] = reserved[ 8b] pwopr2: [ FFh] = sequence_number[ 8b] pwopr2: [ 7h] = message_class.class[ 5b] pwopr2: [ 0h] = message_class.reserved[ 2b] pwopr2: [ 0h] = message_class.ack[ 1b] pwopr2: : IPMI Session Header: pwopr2: : -------------------- pwopr2: [ 0h] = authentication_type[ 8b] pwopr2: [ 0h] = session_sequence_number[32b] pwopr2: [ 0h] = session_id[32b] pwopr2: [ 9h] = ipmi_msg_len[ 8b] pwopr2: : IPMI Message Header: pwopr2: : -------------------- pwopr2: [ 20h] = rs_addr[ 8b] pwopr2: [ 0h] = rs_lun[ 2b] pwopr2: [ 6h] = net_fn[ 6b] pwopr2: [ C8h] = checksum1[ 8b] pwopr2: [ 81h] = rq_addr[ 8b] pwopr2: [ 0h] = rq_lun[ 2b] pwopr2: [ 26h] = rq_seq[ 6b] pwopr2: : IPMI Command Data: pwopr2: : ------------------ pwopr2: [ 38h] = cmd[ 8b] pwopr2: [ Eh] = channel_number[ 4b] pwopr2: [ 0h] = reserved1[ 3b] pwopr2: [ 1h] = get_ipmi_v2.0_extended_data[ 1b] pwopr2: [ 2h] = maximum_privilege_level[ 4b] pwopr2: [ 0h] = reserved2[ 4b] pwopr2: : IPMI Trailer: pwopr2: : -------------- pwopr2: [ 1Fh] = checksum2[ 8b] 6) Non-generic error messages ----------------------------- Under some circumstances, it may be preferred to return generic error messages to the user, so that a malicious user cannot infer remote login information from different error messages returned. For example, returning a generic error message of "Permission Denied" would not give a malicious user information on whether the username or password was input incorrectly. Although implemented earlier on, the authors have elected to not implement this now. There are many vendor implementations of IPMI and many configuration options (authentication mechanism, cipher suite id, username, password, K_g, privilege level) needed for proper IPMI session establishment. The number of error messages that could be mapped into a generic "Permission Denied" would make it too difficult for users to determine why they failed to connect properly. The overall worth of implementing a generic "Permission Denied" error message just doesn't seem worth it now. 7) Get Channel Authentication Capabilities Command -------------------------------------------------- The Get Channel Authentication Capabilities Command is typically the first packet sent in the IPMI session. It returns information on the remote machine's support of: A) IPMI 1.5 authentication mechanisms (e.g. md2, md5, etc.) B) IPMI 1.5 and/or IPMI 2.0 C) per msg authentication D) K_g status E) null username/non-null username/anonymous logins Currently, in FreeIPMI, we check each of these values during the session setup to determine if a person can connect to the remote machine later in the protocol: A) If the user input an unsupported authentication mechanism, we return an error. B) If the user requested IPMI 2.0, but the remote machine doesn't support IPMI 2.0, we return an error. C) We determine if per msg authentication should be considered later in the protocol session. D) If the user was required/not-required to input a K_g value, we return an error appropriately. E) If the user input an unsupported username/password combination, we return an error appropriately. There is a question as to what values above, if any, need to be checked and appropriate errors returned to the user. The Get Channel Authentication Capabilities command is often implemented incorrectly by a number of vendors, so that overall benefit of checks has been put in question. The authors have elected to keep all the checks for the following reasons. * 'A' and 'B' should be checked to avoid potential timeouts: - Later in the protocol, the password could be sent/hashed incorrectly, leading to a timeout because packets are not accepted by the remote machine. - If the remote machine does not support IPMI 2.0, later packets could timeout because the remote machine does not recognize the packet format. * 'C''s checks could be skipped as long as per msg authentication was not supported. * 'D''s checks could be skipped, because an improper null vs non-null K_g will be caught later during IPMI 2.0 authentication. * 'E''s checks are the most complicated. An improper null vs non-null username will be caught later during IPMI 1.5 and IPMI 2.0 authentication. An improper null vs non-null password can be caught later during IPMI 2.0 authentication, but may result in a timeout during IPMI 1.5 authentication. An argument could also be made that the speed at which an invalid username/password error is returned to a user could also give a malicious user information on the username/password of the remote BMC. In the end, the authors have felt the overall positive benefits provided by the checking of these values provides more than the negative implications. Changes in the overall industry implementation could change this viewpoint later. 8) Configuration tool callback design ------------------------------------- Bmc-config, Ipmi-chassis-config, Ipmi-pef-config, and Ipmi-sensors-config (henceforce together just called Configtools) are coded with a archicture that reads/writes each configurable field in the BMC separately. As an example, suppose we have the following BMC configuration file we'd like to commit. FieldA Value1 FieldB.1 Value2 FieldB.2 Value3 FieldB.3 Value4 FieldB.4 Value5 Suppose FieldA is read/written using a single IPMI packet and fields FieldB.1-FieldB.4 can be read/written using a single IPMI packet. In the architecture that the Configtools are currently based on, the above would require 5 read requests to read all 5 values. It would require 1 read request for FieldA, 4 read requests for FieldB.1-FieldB.4, and 5 write requests to write the values. Obviously, this sounds like (and is!) very inefficient. The authors acknowledge that the code is very inefficient b/c it will cause an excess number of request/response packets to be generated. With a large number of inputs the Configtools can be slow. Here are some of the major reasons why this was done and is still kept. A) Due to widely varying IPMI versions and implementations, this handles the write configuration case best. Suppose FieldB.2 is only configurable on IPMI 2.0 systems but not IPMI 1.5 systems. Suppose (perhaps b/c it is optional in the IPMI specification) FieldB.3 is supported by some vendors but not other vendors. Suppose FieldB.4 is simply not implemented correctly by the vendor. This architecture allows the majority of the configuration to succeed on a specific platform, and allows the end user to know exactly what fields may or may not be configurable. If all 4 fields of FieldB.1-FieldB.4 were written at the same time, there is currently no method in the IPMI protocol to know what field was configured incorrectly and why (only a generic error of "invalid input" is returned, but you won't know which field it is). In the future, functionality could be added to retry each field separately if there was such a failure, however that would add another piece of complexity into the code we currently don't have time to add. (Not to mention, can we trust that all the IPMI firmware writers will return consistent error codes such that this could be implemented across a large number of motherboards). B) There are several (and possibly more future) vendor compliance problems that can be (or will need to be) worked around. By using this architecture each specific field can be worked around independently depending on the vendor. These workarounds need to be handled on both the read and write conditions. One of the major fallouts from this design is that if an invalid/illegal configuration exists on the motherboard by default, some configuration values may not be configurable. For example, suppose we want to write the following config to the BMC. FieldA.1 Value1 FieldA.2 Value2 FieldA.3 Value3 FieldA.4 Value4 The architecture of the config tools will read FieldA.1-FieldA.4 from the BMC, change only FieldA.1, then try to write all the fields back to the BMC. Then it would be repeated for FieldA.2, etc. However, suppose the default setting on the motherboard for FieldA.4 is illegal. Then each time we attempt to write FieldA.1, FieldA.2, and FieldA.3, an invalid input error will be returned b/c FieldA.4 is illegal. Things cannot change until FieldA.4 is modified. In a worse scenario, suppose the default setting on the motherboard is illegal for both FieldA.3 and FieldA.4. That means we will receive an invalid input error for the config of FieldA.1 through FieldA.4. Currently, this has been seen a very small minority of systems and work arounds have been added for those systems. Another similar fallout from this design is that the vendor must allow "piecemeal" configuration. In other words, the vendor must allow a subset of the fields to perhaps be configured "incorrectly" while the other subset may be configured "correctly". Some vendors require that fields be written "simultaneously", and do not support the ability to alter configuration one by one. Again, this has been seen a very small minority of systems and work arounds have been added for those systems. 9) Dealing with workarounds --------------------------- There is an admitted conflict in determining whether vendor compliance issues should be handled automatically vs. a specified workaround. On one hand, we would like for the tools to operate as simply for the users as possible without the need to specify strange workarounds or options on the command line. For example, we could detect vendor product-IDs early in the protocol, and if necessary for a particular vendor, turn on the workarounds. On the other hand, some workarounds cannot be detected properly all of the time. For example, the workaround may exist on one firmware release vs. another firmware release. It may exist between one product of a vendor vs. another product from the vendor. Another example, is that while we can make a pretty decent guess what the vendor intended, ultimately, there's no real way to know if the guess is correct. A number of these workarounds are due to vendor compliance problems that are sometimes so intrusive (e.g. using a different hashing algorithm for keys) they must require a workaround on the command line b/c there is really no other way to handle it. However, some could be handled seemlessly, but would require altered behavior to handle the "common case" or "lowest common denominator" of all IPMI protocols. The general rule that I've come to is that if the workaround changes some "normal" or "good" behavior, it must require a specific workaround on the command line. Although it may/will be annoying to a number of users, I feel it is better for the long term. It can hopefully also pressure vendors into fixing their implementations down the road. As an example, on some motherboards, we found that System Event Log (SEL) records reported an invalid sensor generator ID. We found that the reported generator ID was shifted off by one. Thus, as a workround, if a SDR entry cannot be found for a respective system event, we will also search for a SDR entry using the generator ID shifted by one, and if the SDR entry is found, we assume the original generator ID was just off by one and we use the located SDR record. This workaround is seemless and doesn't involve an option on the command line. In contrast, we found on some other motherboards that some SEL records report an invalid event record type. Unlike the above situation, there is no additional information from this record that can give us any indication if we parsed it correctly or incorrectly. As far as we can tell, it just might be an OEM specific record. Therefore, we implemented a workaround called "assumesystemevent", which the user can specify to assume a valid system event record no matter what. Admittedly, the area is grey, and at some point, it's a judgement call :-) 10) Dealing with OEM extensions ------------------------------- Similar to the "Dealing with workarounds" question above, there is a similar question of how to deal with OEM extensions. Should code automatically detect the manufacturer and product to determine if OEM extensions can be handled or should be output? We would like the tools to operate as simply for the users without specifying options on the command line. However, can we trust that a vendor will implement their extensions consistently across motherboards, products, or even firmware revisions? The general decision is that there will be an option for the user to specify if they would like OEM interpreted output if available. Many FreeIPMI tools come with a --interpret-oem-data option for this situation. If a motherboard is specifically supported by FreeIPMI, the user is free to use and trust the OEM support. However, if OEM extensions happen to work for a unlisted motherboard, the user must take the output with some grain of salt.