[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Freeipmi-devel] fiid reimplementation notes
From: |
Albert Chu |
Subject: |
Re: [Freeipmi-devel] fiid reimplementation notes |
Date: |
Fri, 03 Feb 2006 18:44:57 -0800 |
> 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
>