Just a bit of an update so people can make comments on the current direction:
typedef struct fiid_field { uint32_t max_field_len; char key[FIID_FIELD_MAX]; } fiid_field_t; typedef fiid_field_t fiid_template_t[]; struct fiid_field_data { uint32_t max_field_len; char key[FIID_FIELD_MAX]; unsigned int field_len; }; struct fiid_obj { uint32_t magic; uint8_t *data; unsigned int data_len; struct fiid_field_data *field_data; unsigned int field_data_len; }; int8_t fiid_template_field_lookup (fiid_template_t tmpl, uint8_t *field); int8_t fiid_obj_field_lookup (fiid_obj_t obj, uint8_t *field); fiid_obj_t fiid_obj_create (fiid_template_t tmpl); int8_t fiid_obj_destroy (fiid_obj_t obj); fiid_obj_t fiid_obj_dup (fiid_obj_t src_obj); int8_t fiid_obj_clear (fiid_obj_t obj); int8_t fiid_obj_clear_field (fiid_obj_t obj, uint8_t *field); int8_t fiid_obj_set (fiid_obj_t obj, uint8_t *field, uint64_t val); int8_t fiid_obj_get (fiid_obj_t obj, uint8_t *field, uint64_t *val); int8_t fiid_obj_set_data (fiid_obj_t obj, uint8_t *field, uint8_t *data, uint32_t data_len); int8_t fiid_obj_get_data (fiid_obj_t obj, uint8_t *field, uint8_t *data, uint32_t data_len); int8_t fiid_obj_set_block (fiid_obj_t obj, uint8_t *field_start, uint8_t *field_end, uint8_t *data, uint32_t data_len); int8_t fiid_obj_get_block (fiid_obj_t obj, uint8_t *field_start, uint8_t *field_end, uint8_t *data, uint32_t data_len); A few comments: 1) Now that we're moving to abstract away the internal implementation of fiid obj's, many of the original functions like fiid_obj_field_start() are static functions and are hidden from the user. 2) I believe the choice to call the early interface functions "fiid_obj_malloc", "fiid_obj_free", and "fiid_obj_memset", were based upon the fact that it was known that a fiid_obj_t was a void * pointer. Now that we have abstracted that away, the function have been renamed "fiid_obj_create", "fiid_obj_destroy", and "fiid_obj_clear". This is a semantic issue and I would be willing to change this if people disagree. 3) After a fiid object is created, the templates are never passed around anymore. 4) I'm going to develop an iterator interface for this API soon too. Al -- Albert Chu [EMAIL PROTECTED] 925-422-5311 Computer Scientist High Performance Systems Division Lawrence Livermore National Laboratory ----- Original Message ----- From: Albert Chu <[EMAIL PROTECTED]> Date: Thursday, January 12, 2006 1:45 pm Subject: Re: [Freeipmi-devel] FIID Re-Implementation > > > 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; > > Hmmm. Thinking about this again. I'm not sure the flags are even > necessary. I believe the 'len' field of a 'struct fiid_setting' > will be > able to take care of both of these issues. If the 'len' written to > thefiid_obj_t is zero, then clearly it is optional or dependent and > shouldn't be sent in the packet. Whatever 'len' it is set to will > clearly determine if it is variable length or not. > > Al > > -- > Albert Chu > [EMAIL PROTECTED] > 925-422-5311 > Computer Scientist > High Performance Systems Division > Lawrence Livermore National Laboratory > > > ----- Original Message ----- > From: Albert Chu <[EMAIL PROTECTED]> > Date: Thursday, January 12, 2006 8:45 am > Subject: [Freeipmi-devel] FIID Re-Implementation > > > 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 > > ipmi1.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 > > citedas a OEM type. > > > > The following is my proposal for the fiid re-architecture. It > > attemptsto 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 > > fieldin 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 > > theauth-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 > > thefield 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 > > shouldhopefully 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 > > thisre-work. When it gets time to the details, we will work it out. > > > > Al > > > > Al > > > > -- > > Albert Chu > > [EMAIL PROTECTED] > > 925-422-5311 > > Computer Scientist > > High Performance Systems Division > > Lawrence Livermore National Laboratory > > > > > > > > > > _______________________________________________ > > Freeipmi-devel mailing list > > Freeipmi-devel@gnu.org > > http://lists.gnu.org/mailman/listinfo/freeipmi-devel > > > > _______________________________________________ Freeipmi-devel mailing list Freeipmi-devel@gnu.org http://lists.gnu.org/mailman/listinfo/freeipmi-devel