Re: [Freeipmi-devel] fiid release

[Albert Chu]

> 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). 

Ahh, ok.  I think when I saw all of the "data.foo.val" definitions in
the fiid array, I assumed you were going to copy over to the strucutres
or something.

Al


--
Albert Chu
address@hidden
Lawrence Livermore National Laboratory

----- Original Message -----
From: Anand Babu <address@hidden>
Date: Saturday, December 20, 2003 3:40 am
Subject: Re: [Freeipmi-devel] fiid release

> ,----[ 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>
>