Re: [Freeipmi-devel] fiid reimplementation notes

[Albert Chu]

> With the exception of one SDR bug that's making ipmi-sensors say "cat
> ate teh fish" on a corner case

A light-bulb went off in my head.  This is now this is fixed.

Al

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


----- Original Message -----
From: Albert Chu <address@hidden>
Date: Friday, February 3, 2006 6:22 pm
Subject: [Freeipmi-devel] fiid reimplementation notes

> With the exception of one SDR bug that's making ipmi-sensors say "cat
> ate teh fish" on a corner case, the core work is complete.  Its in the
> branch 'al_fiid_rearchitect_branch'.  
> 
> Notes:
> 
> 1) Good stuff I remember when re-working code to use the new API
> 
> assemble/unassemble:
> 
> Using the new fiid_obj_set_block(), fiid_obj_get_block(),
> fiid_obj_set_all(), and fiid_obj_get_all() functions, there is no more
> need to calculate various offsets.  The code (to me) looks a lot 
> cleanerthan it did before.
> 
> No more passing around templates:
> 
> Reduces data to manage.
> 
> Additional parameter checking:
> 
> Template information is now stored in the fiid_obj_t, so we now
> (finally, after me harping about this for probably 2 years :-)) 
> have the
> ability to ensure that an object passed to a function has the correct
> length and is the object of the correct type.
> 
> LAN Session Header:
> 
> The multiple session header templates (tmpl_hdr_session,
> tmpl_hdr_session_auth, tmpl_hdr_session_auth_calc) are now just one
> (tmpl_hdr_session) b/c of optional field support for the auth_code.
> 
> SDR record lengths:
> 
> Workarounds are no longer needed to get around the unexpected 
> length of
> record types using the Get SDR record command.
> 
> SDR record types:
> 
> Workarounds are no longer needed to get around the variable length
> string sensor/device names at the ends of SDR record types.
> 
> I think there were a few more workarounds that were eliminated b/c of
> suport for optional/variable length data, but I can't remember them.
> 
> 2) Not so good stuff I noticed when re-working the code
> 
> No more alloca:
> 
> If we want to abstract away the underlying data structures, the 
> castingof fiid_obj_t to uint8_t * has to go away.
> 
> Longer code:
> 
> Some code ended up a bit longer.  Some longer code was needed b/c of
> cleanup logic that was needed.  Parameter checking made things longer,
> but it really should have been there from the start.
> 
> Reserved fields have to be set:
> 
> With optional fields being supported, reserved fields must be set in
> fill_* functions.  Otherwise the new fiid interface assumes the data
> isn't set, is optional, and thus shouldn't be stuffed in a packet.
> 
> 3) TODO stuff
> 
> I certainly don't know every subsection of the IPMI spec by heart.  I
> may  have guessed some fields were required & fixed length, when in 
> factthey were optional or variable length.  Hopefully anything will 
> come up
> during additional testing.
> 
> Some code cleanup is needed to remove duplicate code.  Now that
> templates  do not need to be passed around to fiid functions, a lot 
> moreduplicate code exists, which can be collapsed into functions.
> 
> Some code ended up a lot longer b/c earlier code worked off the
> assumption that fiid_obj_t's were char *buffers.  The code logic used
> from before didn't fit in with the new fiid API too well, so a lot of
> additional code was added to get around it.  Once the code logic is
> fixed, the code should be clean and happy again.
> 
> I don't have a machine with PEF, so all of my reimplementation 
> needs to
> be tested.
> 
> 4) Maybe todo
> 
> I'm still debating the naming of a few of the fiid functions.
> 
> Also, I need to rethink the packet templates formats.  The common case
> is for fields to be a fixed length and required for transmission.  
> So it
> would be wiser for me to make that the default, and optional or 
> variablelength fields the exception. 
> 
> Al
> 
> --
> Albert Chu
> address@hidden
> 925-422-5311
> Computer Scientist
> High Performance Systems Division
> Lawrence Livermore National Laboratory
> 
> 
> 
> 
> _______________________________________________
> Freeipmi-devel mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/freeipmi-devel
>