Hi Harshad, Thank you very much for your great explanations. Hopefully, we'll get some more comments back over the next couple of weeks on the ones that are still outstanding. :-)
Thanks much, Carol On Fri, 2008-06-13 at 10:58 +0530, [EMAIL PROTECTED] wrote: > Hi Carol, > Please find my comments below. > > **************************************************************************************************** > 16) Harshad Prabhu's 8/10/07 patch to ipmi_hpmfwupg.c for HPM.1 > upgrade. > > http://sourceforge.net/mailarchive/forum.php?forum_name=ipmitool-devel&max_rows=25&style=nested&viewmonth=200708&viewday=10 > > > My Comments: This patch moves the "Send initiate command" stanza down > a > bit within in the HpmfwupgUpgradeStage() routine. Harshad, it looks > like the code currently in ipmi_hpmfwupg.c has this stanza moved down > even farther in the routine. I assume the current code works for you? > **************************************************************************************************** > > Just saw the current code of 1.18 version of ipmi_hpmfwupg.c and its > taken care. > > > **************************************************************************************************** > 18) Harshad Prabhu's 9/12/07 lan.patch. "Patch for lan.c file > related > to bridged message". > > http://sourceforge.net/mailarchive/message.php?msg_name=OF74776770.85DDAD30-ON88257354.005C1443-88257354.006461A9%40radisys.com > > My comments: This patch looks interesting -- I assume its intent is > to > add support for IPMIv2.0/RMCP+ unauthenticated messages? Harshad, > could > you please send a bit more info on the patch such as whether it's > fixing > a problem you found, why the check for the response netfn is > necessary, > etc.? Also, I'm hoping some other folks with more expertise in the > lan > area than me will review this and send in their comments. > **************** > Currently the code handles well when the authentication is there, by > default it assumes that the user is doing some authentication > therefore it checks "if (rsp->data_len == 38)". Now when the user does > not give any authentication then the response data length gets reduced > to 22. So the condition is checked that if the authentication is TRUE > then response data length should be 38 and if there is NO > AUTHENTICATION then the response data length is 22. > And one more thing the current code assumes that its checking the send > message command (0x34) [Application NetFn] "if > (rsp->payload.ipmi_response.cmd == 0x34)" and does not check the > netfunction at all. Problem is that there might be a OEM command > number 0x34 then there will be clash, so added a check that the > response netfunction is 0x07 (Application) so that it takes care that > only APPLICATION netfn - Sendmessage (0x34) is verified here. > > > **************************************************************************************************** > 20) Harshad Prabhu's 9/18 patch to ipmi_hpmfwupg > http://sourceforge.net/mailarchive/forum.php?forum_name=ipmitool-devel&max_rows=25&style=nested&viewmonth=200709&viewday=18 > > My Comments: Looks like there are lots of nice formatting and output > fixes in there among other things. Would want to get some review > comments from some of the other HPM folks, too, since there are so > many > changes including some functional changes. Hopefully, we'll get some > more reviewer eyes on this over the next couple of weeks. :-) > > **************************************************************************************************** > > Most of them are formatting. Only one functionality added was that > whenever LongDuration commands are sent then the GetUpgradeStatus was > sent immediately the moment we get that the command is in PROGRESS. So > added some delay to ensure that we send GetUpgradeStatus command after > some delay. The current code handles delays between two > GetUpgradeStatus but not between the LongDurationCommand and > GetUpgrade Status. Hopefully it should not break anything.. > > ( APPLICATION ) ( BMC ) > > Long Duration Command ----------> > <-------- (0x80) Command is in Progress > GetUpgrade Status (immediately)-----> > <some delay is there > > GetUpgrade Status -----------------> > ...... > > Afer adding the patch > > ( APPLICATION ) ( BMC ) > Long Duration Command ----------> > <-------- (0x80) Command is in Progress > <ADDED SOME DELAY HERE, BECOZ sometimes the BMC is Upgrading and not > in mood to respond> > GetUpgrade Status -----------------> > <some delay is there > > GetUpgrade Status -----------------> > ..... > > Thanks > Harshad > ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://sourceforge.net/services/buy/index.php _______________________________________________ Ipmitool-devel mailing list Ipmitool-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ipmitool-devel
