From: Jim M. <jm...@hp...> - 2012-02-15 18:12:46
Attachments:
ipmitool_sessionclose.patch
|
All, We have run across the following problem when using the ipmitool lanplus interface to perform bridged IPMI commands (using the -b -t or -m switches in conjunction with -I lanplus). Use of lanplus and bridging will result in a "Close Session command failure". This problem is due to two issues in the code. 1. ipmi_lan_poll_recv does not properly return the ipmi response packet pointer on a successful bridged requests. 1. The ipmi_close_session_cmd does not set the target address to the BMC before issuing the close. This lanplus problem is specific to bridged commands because these are the only cases where the target address in the interface structure is modified from its compile time initial value of (IPMI_BMC_SLAVE_ADDR). In bridged case, the open will always be done with the target_address set to IPMI_BMC_SLAVE_ADDR. but the subsequent bridged command request modifies the target_address based on the specified command line arguments to -b, -t or -m. When the close is finally done, the target_address in the interface structure used for close no longer correctly addresses the BMC, so the close fails. Note: The lan interface, lan_close_session_cmd, correctly sets the target address to IPMI_BMC_SLAVE_ADDR so the lan interface does not have this issue. I've attached a patch with does resolve the "Close Session command failure" for lanplus. Please let me know if I correctly characterized the problem and applied a correct solution. -- Jim Mankovich | jm...@hp... -- |
From: Zdenek S. <zde...@gm...> - 2012-02-16 13:53:26
|
On Wed, Feb 15, 2012 at 7:12 PM, Jim Mank <jm...@hp...> wrote: > All, [...] > I've attached a patch with does resolve the "Close Session command failure" > for lanplus. > Please let me know if I correctly characterized the problem and applied a > correct solution. > I don't know about this issue(without looking at it further), but indentations in the first part of attached file are wrong :) I presume you've tested attached patch. btw looking at 'lib/ipmi_main.c': --- uint8_t target_addr = 0; [...] case 't': if (str2uchar(optarg, &target_addr) != 0) { lprintf(LOG_ERR, "Invalid parameter given or out of range for '-t'."); rc = -1; goto out_free; } break; [...] ipmi_main_intf->target_addr = target_addr; [...] --- I presume '-t' should accept either FQDN or IP address, thus lines above are wrong, aren't they? Or is it a hex as in case of '-m'? Thanks, Z. > -- Jim Mankovich | jm...@hp... -- > > > ------------------------------------------------------------------------------ > Virtualization & Cloud Management Using Capacity Planning > Cloud computing makes use of virtualization - but cloud computing > also focuses on allowing computing to be delivered as a service. > http://www.accelacomm.com/jaw/sfnl/114/51521223/ > _______________________________________________ > Ipmitool-devel mailing list > Ipm...@li... > https://lists.sourceforge.net/lists/listinfo/ipmitool-devel > |
From: Andy C. <and...@us...> - 2012-02-16 14:21:51
|
Zdenek, RE: 'lib/ipmi_main.c' > I presume '-t' should accept either FQDN or IP address, thus lines > above are wrong, aren't they? Or is it a hex as in case of '-m'? The -t option supplies an IPMI slave address (one byte). Andy |
From: Zdenek S. <zde...@gm...> - 2012-02-16 14:29:09
|
On Thu, Feb 16, 2012 at 3:21 PM, Andy Cress <and...@us...> wrote: > Zdenek, > > RE: 'lib/ipmi_main.c' >> I presume '-t' should accept either FQDN or IP address, thus lines >> above are wrong, aren't they? Or is it a hex as in case of '-m'? > > The -t option supplies an IPMI slave address (one byte). > > Andy > That was close. Thanks Andy! Z. |
From: Jim M. <jm...@hp...> - 2012-02-16 16:43:59
|
Zdenek, To answer your questions, I did test the patch and it does resolve the problem. I just now fixed up the indentation on the return statement. As Andy pointed out, -t is used to specify a one byte IPMI slave address so the parsing of the -t argument is correct. -- Jim Mankovich | jm...@hp... -- On 2/16/2012 6:53 AM, Zdenek Styblik wrote: > On Wed, Feb 15, 2012 at 7:12 PM, Jim Mank<jm...@hp...> wrote: >> All, > [...] >> I've attached a patch with does resolve the "Close Session command failure" >> for lanplus. >> Please let me know if I correctly characterized the problem and applied a >> correct solution. >> > I don't know about this issue(without looking at it further), but > indentations in the first part of attached file are wrong :) > I presume you've tested attached patch. > > btw looking at 'lib/ipmi_main.c': > > --- > uint8_t target_addr = 0; > [...] > case 't': > if (str2uchar(optarg,&target_addr) != 0) { > lprintf(LOG_ERR, "Invalid parameter given or out of range for '-t'."); > rc = -1; > goto out_free; > } > break; > [...] > ipmi_main_intf->target_addr = target_addr; > [...] > --- > > I presume '-t' should accept either FQDN or IP address, thus lines > above are wrong, aren't they? Or is it a hex as in case of '-m'? > > Thanks, > Z. > >> -- Jim Mankovich | jm...@hp... -- >> >> >> ------------------------------------------------------------------------------ >> Virtualization& Cloud Management Using Capacity Planning >> Cloud computing makes use of virtualization - but cloud computing >> also focuses on allowing computing to be delivered as a service. >> http://www.accelacomm.com/jaw/sfnl/114/51521223/ >> _______________________________________________ >> Ipmitool-devel mailing list >> Ipm...@li... >> https://lists.sourceforge.net/lists/listinfo/ipmitool-devel >> |
From: Jim M. <jm...@hp...> - 2012-02-17 14:55:09
Attachments:
ipmitool_sessionclose.patch
|
All, Just to close the loop, here is the patch with the indentation changed :) It looks to me like the indentation in this part of the file is pretty messed up. Is it reasonable for me to fix up more indentation before I commit this change? -- Jim Mankovich | jm...@hp... -- On 2/16/2012 9:43 AM, Jim Mank wrote: > Zdenek, > > To answer your questions, I did test the patch and it does resolve the > problem. > I just now fixed up the indentation on the return statement. > As Andy pointed out, -t is used to specify a one byte IPMI slave address > so the parsing > of the -t argument is correct. > > -- Jim Mankovich | jm...@hp... -- > > > On 2/16/2012 6:53 AM, Zdenek Styblik wrote: >> On Wed, Feb 15, 2012 at 7:12 PM, Jim Mank<jm...@hp...> wrote: >>> All, >> [...] >>> I've attached a patch with does resolve the "Close Session command failure" >>> for lanplus. >>> Please let me know if I correctly characterized the problem and applied a >>> correct solution. >>> >> I don't know about this issue(without looking at it further), but >> indentations in the first part of attached file are wrong :) >> I presume you've tested attached patch. >> >> btw looking at 'lib/ipmi_main.c': >> >> --- >> uint8_t target_addr = 0; >> [...] >> case 't': >> if (str2uchar(optarg,&target_addr) != 0) { >> lprintf(LOG_ERR, "Invalid parameter given or out of range for '-t'."); >> rc = -1; >> goto out_free; >> } >> break; >> [...] >> ipmi_main_intf->target_addr = target_addr; >> [...] >> --- >> >> I presume '-t' should accept either FQDN or IP address, thus lines >> above are wrong, aren't they? Or is it a hex as in case of '-m'? >> >> Thanks, >> Z. >> >>> -- Jim Mankovich | jm...@hp... -- >>> >>> >>> ------------------------------------------------------------------------------ >>> Virtualization& Cloud Management Using Capacity Planning >>> Cloud computing makes use of virtualization - but cloud computing >>> also focuses on allowing computing to be delivered as a service. >>> http://www.accelacomm.com/jaw/sfnl/114/51521223/ >>> _______________________________________________ >>> Ipmitool-devel mailing list >>> Ipm...@li... >>> https://lists.sourceforge.net/lists/listinfo/ipmitool-devel >>> > ------------------------------------------------------------------------------ > Virtualization& Cloud Management Using Capacity Planning > Cloud computing makes use of virtualization - but cloud computing > also focuses on allowing computing to be delivered as a service. > http://www.accelacomm.com/jaw/sfnl/114/51521223/ > _______________________________________________ > Ipmitool-devel mailing list > Ipm...@li... > https://lists.sourceforge.net/lists/listinfo/ipmitool-devel > |
From: Zdenek S. <zde...@gm...> - 2012-02-17 15:13:16
|
On Fri, Feb 17, 2012 at 3:54 PM, Jim Mank <jm...@hp...> wrote: > All, > > Just to close the loop, here is the patch with the indentation changed :) > > It looks to me like the indentation in this part of the file is pretty > messed up. Is it reasonable for me to fix up more indentation before I > commit this change? > Yes, it is always reasonable. But it is up to you whether you want or not. As I've said, there is a lot of work to be done :) btw as for my question about '-t' - I was the one whom put input checking in place, so it was kind of panic attack whether I haven't messed up something. Z. > > -- Jim Mankovich | jm...@hp... -- > > On 2/16/2012 9:43 AM, Jim Mank wrote: >> >> Zdenek, >> >> To answer your questions, I did test the patch and it does resolve the >> problem. >> I just now fixed up the indentation on the return statement. >> As Andy pointed out, -t is used to specify a one byte IPMI slave address >> so the parsing >> of the -t argument is correct. >> >> -- Jim Mankovich | jm...@hp... -- >> >> >> On 2/16/2012 6:53 AM, Zdenek Styblik wrote: >>> >>> On Wed, Feb 15, 2012 at 7:12 PM, Jim Mank<jm...@hp...> wrote: >>>> >>>> All, >>> >>> [...] >>>> >>>> I've attached a patch with does resolve the "Close Session command >>>> failure" >>>> for lanplus. >>>> Please let me know if I correctly characterized the problem and applied >>>> a >>>> correct solution. >>>> >>> I don't know about this issue(without looking at it further), but >>> indentations in the first part of attached file are wrong :) >>> I presume you've tested attached patch. >>> >>> btw looking at 'lib/ipmi_main.c': >>> >>> --- >>> uint8_t target_addr = 0; >>> [...] >>> case 't': >>> if (str2uchar(optarg,&target_addr) != 0) { >>> lprintf(LOG_ERR, "Invalid parameter given >>> or out of range for '-t'."); >>> rc = -1; >>> goto out_free; >>> } >>> break; >>> [...] >>> ipmi_main_intf->target_addr = target_addr; >>> [...] >>> --- >>> >>> I presume '-t' should accept either FQDN or IP address, thus lines >>> above are wrong, aren't they? Or is it a hex as in case of '-m'? >>> >>> Thanks, >>> Z. >>> >>>> -- Jim Mankovich | jm...@hp... -- >>>> >>>> >>>> >>>> ------------------------------------------------------------------------------ >>>> Virtualization& Cloud Management Using Capacity Planning >>>> Cloud computing makes use of virtualization - but cloud computing >>>> also focuses on allowing computing to be delivered as a service. >>>> http://www.accelacomm.com/jaw/sfnl/114/51521223/ >>>> _______________________________________________ >>>> Ipmitool-devel mailing list >>>> Ipm...@li... >>>> https://lists.sourceforge.net/lists/listinfo/ipmitool-devel >>>> >> >> ------------------------------------------------------------------------------ >> Virtualization& Cloud Management Using Capacity Planning >> Cloud computing makes use of virtualization - but cloud computing >> also focuses on allowing computing to be delivered as a service. >> http://www.accelacomm.com/jaw/sfnl/114/51521223/ >> _______________________________________________ >> Ipmitool-devel mailing list >> Ipm...@li... >> https://lists.sourceforge.net/lists/listinfo/ipmitool-devel >> > > ------------------------------------------------------------------------------ > Virtualization & Cloud Management Using Capacity Planning > Cloud computing makes use of virtualization - but cloud computing > also focuses on allowing computing to be delivered as a service. > http://www.accelacomm.com/jaw/sfnl/114/51521223/ > _______________________________________________ > Ipmitool-devel mailing list > Ipm...@li... > https://lists.sourceforge.net/lists/listinfo/ipmitool-devel > |
From: Jim M. <jm...@hp...> - 2012-02-17 15:46:05
|
Zdenek, I'm going clean up the indentation some and then commit the change. Thanks, -- Jim Mankovich | jm...@hp... -- On 2/17/2012 8:13 AM, Zdenek Styblik wrote: > On Fri, Feb 17, 2012 at 3:54 PM, Jim Mank<jm...@hp...> wrote: >> All, >> >> Just to close the loop, here is the patch with the indentation changed :) >> >> It looks to me like the indentation in this part of the file is pretty >> messed up. Is it reasonable for me to fix up more indentation before I >> commit this change? >> > Yes, it is always reasonable. But it is up to you whether you want or > not. As I've said, there is a lot of work to be done :) > > btw as for my question about '-t' - I was the one whom put input > checking in place, so it was kind of panic attack whether I haven't > messed up something. > > Z. > >> -- Jim Mankovich | jm...@hp... -- >> >> On 2/16/2012 9:43 AM, Jim Mank wrote: >>> Zdenek, >>> >>> To answer your questions, I did test the patch and it does resolve the >>> problem. >>> I just now fixed up the indentation on the return statement. >>> As Andy pointed out, -t is used to specify a one byte IPMI slave address >>> so the parsing >>> of the -t argument is correct. >>> >>> -- Jim Mankovich | jm...@hp... -- >>> >>> >>> On 2/16/2012 6:53 AM, Zdenek Styblik wrote: >>>> On Wed, Feb 15, 2012 at 7:12 PM, Jim Mank<jm...@hp...> wrote: >>>>> All, >>>> [...] >>>>> I've attached a patch with does resolve the "Close Session command >>>>> failure" >>>>> for lanplus. >>>>> Please let me know if I correctly characterized the problem and applied >>>>> a >>>>> correct solution. >>>>> >>>> I don't know about this issue(without looking at it further), but >>>> indentations in the first part of attached file are wrong :) >>>> I presume you've tested attached patch. >>>> >>>> btw looking at 'lib/ipmi_main.c': >>>> >>>> --- >>>> uint8_t target_addr = 0; >>>> [...] >>>> case 't': >>>> if (str2uchar(optarg,&target_addr) != 0) { >>>> lprintf(LOG_ERR, "Invalid parameter given >>>> or out of range for '-t'."); >>>> rc = -1; >>>> goto out_free; >>>> } >>>> break; >>>> [...] >>>> ipmi_main_intf->target_addr = target_addr; >>>> [...] >>>> --- >>>> >>>> I presume '-t' should accept either FQDN or IP address, thus lines >>>> above are wrong, aren't they? Or is it a hex as in case of '-m'? >>>> >>>> Thanks, >>>> Z. >>>> >>>>> -- Jim Mankovich | jm...@hp... -- >>>>> >>>>> >>>>> >>>>> ------------------------------------------------------------------------------ >>>>> Virtualization& Cloud Management Using Capacity Planning >>>>> Cloud computing makes use of virtualization - but cloud computing >>>>> also focuses on allowing computing to be delivered as a service. >>>>> http://www.accelacomm.com/jaw/sfnl/114/51521223/ >>>>> _______________________________________________ >>>>> Ipmitool-devel mailing list >>>>> Ipm...@li... >>>>> https://lists.sourceforge.net/lists/listinfo/ipmitool-devel >>>>> >>> ------------------------------------------------------------------------------ >>> Virtualization& Cloud Management Using Capacity Planning >>> Cloud computing makes use of virtualization - but cloud computing >>> also focuses on allowing computing to be delivered as a service. >>> http://www.accelacomm.com/jaw/sfnl/114/51521223/ >>> _______________________________________________ >>> Ipmitool-devel mailing list >>> Ipm...@li... >>> https://lists.sourceforge.net/lists/listinfo/ipmitool-devel >>> >> ------------------------------------------------------------------------------ >> Virtualization& Cloud Management Using Capacity Planning >> Cloud computing makes use of virtualization - but cloud computing >> also focuses on allowing computing to be delivered as a service. >> http://www.accelacomm.com/jaw/sfnl/114/51521223/ >> _______________________________________________ >> Ipmitool-devel mailing list >> Ipm...@li... >> https://lists.sourceforge.net/lists/listinfo/ipmitool-devel >> > |