[Freeipmi-devel] FIID Re-Implementation

[Albert Chu]

Howdy all,

I would like to discuss this topic from the FreeIPMI meeting more deeply.

I would like to propose we leave my ipmi 2.0 branch (al_ipmi_2_0_branch)
sitting.  I think it's best not to merge it into the head until our
agreed upon objectives for the next generation library are met for ipmi
1.5 first.  I am open to discussion about this too though.

Fiid Re-Architecture:
---------------------

As was discussed at the meeting, there are two major problems now
showing up due to IPMI 2.0, many more "optional" or "dependent" fields
exist and many are variable length. Some examples:

- The length of authcode/integrity fields are dependent on the digest
algorithm used.

- OEM fields are sent dependent on if the payload is specifically cited
as a OEM type.

The following is my proposal for the fiid re-architecture.  It attempts
to abstract information far more to deal with the above issues and
issues with later IPMI versions.

#defines and structure re-definitions
-------------------------------------

#define FIID_FIELD_REQUIRED         0x0
#define FIID_FIELD_OPTIONAL         0x1
#define FIID_FIELD_DEPENDENT        0x2

#define FIID_FIELD_FIXED_LENGTH     0x0
#define FIID_FIELD_VARIABLE_LENGTH  0x1

typedef struct fiid_field
{
  uint32_t max_field_len;
  char key[FIID_FIELD_MAX];
  uint8_t requirement_flag;
  uint8_t length_flag;
} fiid_field_t;

typedef fiid_field_t const fiid_template_t[];

struct fiid_setting
{
  fiid_field_t field;
  uint32_t len;
}

struct fiid_obj
{
  void *data;
  unsigned int data_len;
  struct fiid_setting *settings;
  unsigned int settings_len;
};

struct struct fiid_obj *fiid_obj_t;

Notes:
------

fiid_field_t now includes a flags.  One flag will determine if the field
in the template is required, dependent, or optional.  Dependent means
some factor determines if the field should be sent. An example of this
is the authcode field of an IPMI 1.5 packet.  It is only sent if if the
auth-type is not NONE.  An optional field is something that simply
optional.  Off the top of my head, the last byte in a "Get Chassis
Status" command is optional.  The other flag is to determine if the the
field is fixed or variable length.

fiid_obj_t holds the data, the data length for bounds checking, object
settings, and the length of the settings array.

The fiid_obj_settings_t contains the data in the fields of a
fiid_template_t. Note, that I have currently elected to not have
pointers back to the template, because users have the ability to
alter/create/destroy templates through fiid_template_make.  Instead, the
data from the template will be copied over.

It also includes a len field to indicate how much data has been copied
into the object for this particular field.  So if the len is 0, don't
copy the data into a packet (i.e. perhaps the data was optional). This
len variable fixes the variable length issue of some fields in IPMI 2.0.
 By abstracting away the "settings" in the above format, this should
hopefully make things more extensible in future versions of IPMI.

fiid interface:
---------------

I currently don't imagine a need to alter the fiid API (although some
changes may be done for cleanup if we decide to).  Here's a list of
changes that would have to be done underneath in the code:

fiid_obj_calloc/alloc/malloc - Must build fiid_obj_settings_t array and
 build structure.  

fiid_obj_free - Must free new structures appropriately.

fiid_obj_memset/fiid_obj_memset_field - Must clear 'len' field in
fiid_obj_settings_t array.

fiid_obj_set/fiid_obj_set_data - Must set 'len' field appropriately.

fiid_obj_get/fiid_obj_get_data - Must account for variable 'len'
appropriately.

fiid_obj_set/fiid_obj_get/fiid_obj_set_data/fiid_obj_get_data: The
template is no longer required.  Would we elect to remove it from the API??

libfreeipmi changes:
--------------------

Various code logic must be altered to appropriately use the fiid
interface and abstract away the new structures:

i.e.

memcpy(pkt,obj + field_offset,field_len)

would have to change to:

fiid_obj_get_data(obj,tmpl,field_name,pkt,pkt_len)

Memsetting of objects must be changed to using the fiid_obj_memset()
function, because memset() itself is bad.

I think you get the idea.

API changes:
------------

If we desire to (we need not) we can eliminate passing of templates to
many API functions.

Needless to say there are various issues that we need to discuss.

I believe we came to a reasonable agreement on this change at the
FreeIPMI meeting, so I will (probably soon) begin a branch to begin this
re-work.  When it gets time to the details, we will work it out.

Al

Al

--
Albert Chu
address@hidden
925-422-5311
Computer Scientist
High Performance Systems Division
Lawrence Livermore National Laboratory