Re: [Freeipmi-devel] fiid release

[Anand Babu]

,----[ Albert Chu <address@hidden> ]
| 1) Given the new framework, I think passing in u_int8_t ** pointers
|    and having the function return a buffer may be better than having
|    the user create the u_int8_t * buffer.  I prefer the later, but
|    given your framework style, I think the former fits better.  I
|    dunno, maybe we should wait until later to decide, no need to
|    decide now.
`----
My choice is more of a policy. If we let the user allocate, he will be
more careful about freeing them.

,----[ Albert Chu <address@hidden> ]
| 2) Where are the kcs_hdr_t and get_dev_id_t structures?  Or have you
|    just not added the code for converting between fiid and
|    structures?? I noticed a lack of freeming memory too ... but I
|    guess you're just ignoring that stuff right now.
`----
kcs_hdr_t, get_dev_id_t, ... and all such structures are gone. With
the new framework, only data type for storing the data is u_int8_t[].

Structures are translated into byte-array + fiid (interface
definition). 

Maintaining structures are a pain. You will have to write a huge
amount of code, filling and extracting the value. There is no need
now.

KNOWN BUGS (2):
  1) fiid_alloc should round the len returned by fiid_obj_len to bytes
  and then allocate memory - Ian reported the bug
  2) ipmi_kcs_get_dev_id should call free the allocated memory.

Its trivial, I will fix them ;)
 

BMC LAN - DHCP option is OEM specific. I'm waiting for Intel to clear
the legal stuff. There are lot of other OEM specific extensions which
are useful to us. I'm asking them to release those h/w specifications
to the public, if they want better free software support.


,----[ Albert Chu <address@hidden> ]
| Overall I think the code is good and I like the framework.  There are
| some API things I think we can cleanup, but they are minor.  Like:
| 
|     cmd_len = fiid_obj_len (fiid_cmd_get_dev_id_rq); hdr_len =
|     fiid_obj_len (fiid_kcs_hdr); pkt_len = hdr_len + cmd_len; hdr =
|     fiid_obj_alloc (fiid_kcs_hdr); cmd = fiid_obj_alloc
|     (fiid_cmd_get_dev_id_rq); pkt = alloca (BITS_ROUND_BYTES
|     (pkt_len));
| 
| But I think we hide this from the user easily later on.
`----
If you notice this time, I have moved the functions which I usually do
outside libfreeipmi into the framework itself. (remember
ipmi-wrapper.c). We even thought about libfish - a higher level
interface to libfreeipmi. We don't need them any more.

With the new interface, things are much simpler. 
For adding a new command,
 - Describe the fiid object for rq and rs.
 - Write a rq function to fill up user data. (optional)
 - Write a high level call which will accept variable user-data, assemble
   the byte-array, write to interface, read from interface and return the
   unassembled byte array.

User can always view the byte-array as a struct with the help of fiid.

,----[ Albert Chu <address@hidden> ]
| I think the overal goal is to still have something like:
| 
| kcs_hdr_t hdr; ipmi_get_dev_id_t dev; u_int8_t **pkt;
| 
| ipmi_kcs_rq_hdr (IPMI_BMC_IPMB_LUN_BMC, IPMI_NET_FN_APP_RQ, &hdr);
| ipmi_get_dev_id_rq (&cmd); pkt_len = assemble_kcs_packet (&hdr, &dev,
| IPMI_GET_DEV_ID_KEY, pkt); // or user creates buffer instead of
| passing in u_int8_t ** ipmi_kcs_write(pkt, pkt_len);
| 
| and all of the fiid stuff is hidden within those functions.
`----

No, it is even easier.

For the user, 

  hdr_rs = fiid_obj_alloc (fiid_kcs_hdr);
  cmd_rs = fiid_obj_alloc (fiid_cmd_get_dev_id_rs);
 
  if (ipmi_kcs_get_dev_id (IPMI_KCS_SMS_IO_BASE_SR870BN4, hdr_rs,
  cmd_rs) != 0)
  {
     err (EXIT_FAILURE, "ipmi_kcs_get_dev_id (%Xh, %p, %p)",
            IPMI_KCS_SMS_IO_BASE_SR870BN4, hdr_rs, cmd_rs);
  }           

  /* Display the result */
  FIID_OBJ_GET (cmd_rs, cmd_rs_len, fiid_cmd_get_dev_id_rs,
    "data.dev_id", &val);
  fprintf (stdout, "Device ID:         %X\n", (unsigned int) val);

  FIID_OBJ_GET (cmd_rs, cmd_rs_len, fiid_cmd_get_dev_id_rs,
    "data.dev_rev.rev", &val);
  fprintf (stdout, "Device Revision:   %X\n", (unsigned int) val);

  free (hdr_rs); free (cmd_rs);


To summarize, for every command, user will have a common fiid
definition and an interface specific call.

ipmi_kcs_get_dev_id (...)
ipmi_lan_get_dev_id (...)

-ab

----- Original Message -----
From: Anand Babu <address@hidden>
Date: Friday, December 19, 2003 10:29 am
Subject: [Freeipmi-devel] fiid release

> Hi Al,
> Please download 
> ftp://ftp.cdclinux.com/pub/ipmi/libfreeipmi-0.0.0-fiid.tar.gz
> ftp://ftp.cdclinux.com/pub/ipmi/freeipmi-utils-fiid.tgz
> 
> Surprisingly fiid model is easier than struct model but is built on
> top of byte-array. No more messing with bits stuff. Its all easy now.
> 
> Go through
> libfreeipmi:
> ipmi-kcs-interface.{c,h}, fiid.{c,h}, bit-ops.{c,h},
> ipmi-dev-global-cmds.{c,h}
> 
> freeipmi-utils:
> bmc-info.c
> 
> Later today, I will release complete freeipmi-utils tools including
> DHCP option. SR870BN4 supports DHCP as an OEM option.
> 
> Ian, you need some info which is not in the IPMI spec to implement
> bmc-config commands. I know those values. I will take care.
> 
> Happy Hacking
> -- 
> _.|_ 
> (_||_)
> Free as in Freedom <www.gnu.org>
> 
> 
> _______________________________________________
> Freeipmi-devel mailing list
> address@hidden
> http://mail.nongnu.org/mailman/listinfo/freeipmi-devel
> 



-- 
 _.|_ 
(_||_)
Free as in Freedom <www.gnu.org>