I'm one of the authors of HPM.2.
specific to ipmiTool.
performance gains using HPM.2.
you observe a decrease in processing time as our models show.
For example, the Get FRU Data Model shows a 94% decrease in time.
http://www.jblade.com:8080/products/hemi/hpm2/largePacket/models/Hpm2ReadFruInfoModel.jsf
Post by Jim MankovichDmitry,
1.pps-bugfixes.diff
Looks good
2.pps-oem.diff
ipmi_oem.c patch failed to apply to TOB lib/ipmi_oem.c
The other changes look ok to me
3. pps-sol-instance.diff
I prefer accepting the patch that Corey Minyard proposed on 9/12/2012 instead
of this patch as Corey's patch implements the specification of the instance number
within the sol activate and deactivate commands instead of a global
ipmitool switch. Since instance is specific to the sol commands, it seems more
appropriate that the instance specification be done within these commands.
I reviewed Corey's patch on 9/21, but never heard back from Corey on
my comments. The only real question I was waiting on from Corey before
accepting the patch was the question as to why there is a need for the
ability to specify the instance number other than the specification of one
that is done in the existing code.
4.pps-long-packets.diff
The lanplus.c patch failed to apply to TOB src/plugins/lanplus/lanplus.c
In ipmi_fru.c write_fru_area (~line 506), the fru_data_rqst_size is set to the
return value from ipmi_intf_get_max_request_data_size(intf) - 3 whereas
before it was the channel_buf_size - 5. I don't know exactly what the
overhead is nor why there is now less overhead needed than there was
before, can you explain a little as to the change? Also, if the
fru_data_rqst_size that gets calculated is greater than 255, you set it to
255; why doesn't the overhead need to be subtracted out of this?
You modified a compiled out version of write_fru_area on line 621 in ipmi_fru.c,
I think it would be better just to remove this function.
In ipmi_fru.c read_fru_area (line 728), why is the overhead here now one instead
of 9? Why doesn't the overhead need to be subtracted out of the 255?
Pretty
much the same questions I had about write_fru_area above.
ipmi_hpmfwupg.c I don't know about the correctness of the functional changes
you are making, but I did see that you are doing a malloc on line 2294 and you do
not check success/failure of the allocation.
ipmi_main.c
There needs to be man page documentation for all the switches you have added as
well as an explanation of the new serial interfaces and arguments required to connect
using the serial interfaces.
It looks like you have to specific -D ttydev:baud to use the serial
interfaces and that the -X
switch is specific to these interfaces as well. How does one know if
they need to use the
-X switch? Might it be possible to package the serial driver separately from the other
changes?
What platform did you test these serial interfaces on? I would like to see the -r switch
functionality implemented in the fru command since it is specific to only fru commands.
Why did you make the change to do the interface open even when the target_address is
not specified? Could this cause any problems?
ipmi_oem.c changes are ok
ipmi_sdr.c Please use GET_SDR_ENTIRE_RECORD instead of the constant 0xFF on line 2794 and
could you explain why you are setting sdr_max_read_len to 0xFE instead of
GET_SDR_ENTIRE_RECORD
on line 2975. sdr_max_read_len is initialized statically to
GET_SDR_ENTIRE_RECORD so isn't that
the correct maximum?
Why does serial_basic.c copyright mention Sun Microsystems in the text,
but not as a Copyright and
why does serial_terminal.c have a 2003 Sun Copyright? Where these files
derived from Sun source
code in some way? Neither hpm2.c nor hpm2.h references Sun Copyrights.
Thanks for the contributions,
Jim
Post by Dmitry BazhenovHello, Jim,
Here are the updated patches which have your comments addressed. We
decided not to push the changes into ipmi_sdr.c which regard to bridging
commands to sensors since there is a major conflict between PICMG and
generic IPMI systems in this matter.
Post by Dmitry BazhenovThe patches are not dependent on each other (we merged patches 4 and 5
together since serial drivers have strong dependence on large packet
support implementation).
Post by Dmitry Bazhenovo lib/ipmi_sel.c
- zero data before making request to avoid reading of uninitialized
data
o lib/ipmi_mc.c
- do not print AUX info, if IPMC return 12-byte Get Device ID
response
o lib/ipmi_picmg.c
- add support for hexadecimal and octal formats for the command-line
parameters
o configure.in
- fixed build for the newer autotool releases
- fixed cross-compilation
2. pps-oem.diff
o Add support for PPS H8 evaluation board BMC which contains Intel
82751 GbE MAC. RMCP+ support for this MAC controller has several
deviations.
3. pps-sol-instance.diff
o Add support for SOL instances other than 1.
4. pps-long-packets.diff
o Add support for HPM.2 compliant long packets. The implementation is
made against the HPM.2 0.9.2 draft specification, and requires
update for the HPM.2 1.0 release version. Pigeon Point Systems is
going to provide the update later.
o HPM.1 upgrade and FRU Inventory commands were modified to reuse the
long packet support.
o Access to FRU inventory devices with the length restrictions is
made as a command-line option.
o Kontron OEM large packet support is now optional and enabled by the
corresponding OEM command-line parameter.
o Add support for Terminal Mode and Basic Mode direct serial drivers.
We are looking for your further comments.
Regards,
Dmitry
Post by Dmitry BazhenovHello, Jim,
Since both the "Get Channel Info" command and MC Device Locator record
are optional for generic IPMC, I think it is not worth trying to use
them to identify the cases when sensor command bridging should be
inhibited.
Instead, I've started thinking of an explicit command-line parameter
which would signal that the command bridging (which is enabled by
default for backward compatibility) shall be inhibited.
At this moment there are two options for this approach. The first is
using a global command-line option (assign a unused letter). The second
is using a command-specific flag for sensor-related commands ("sdr",
"sensor", maybe there are others).
I would like to know which approach seems more attractive from the
maintainers point of view.
Thanks in advance,
Dmitry
Post by Dmitry BazhenovHello, Jim,
MC Device Locator is a mandatory record for PICMG systems. And this is
where the slave address of the controller is got for comparison.
For systems that do not provide MC Locators, the default IPMITool
behavior (as it is currently implemented) could be retained.
Regards,
Dmitry
Post by Jim MankovichHi Dmitry,
Where does the slave address of the target IPMC on a PICMG system come from that
you will be using to compare against the sensor owner to determine
bridging is not necessary?
FYI: Not all platforms contain an MC Device Locator and some platforms
contain more than one.
Thanks in Advance,
Jim
Post by Dmitry BazhenovHello, Jim,
Thanks for you comments.
The thing is that when doing "sdr" and "sensor" command against PICMG
systems, IPMITool tries to bridge sensor commands to another channel
(primary IPMB) instead of just sending them directly to IPMC.
This is because PICMG IPMCs have slave addresses different from 20h
on
Post by Dmitry BazhenovPost by Dmitry BazhenovPost by Dmitry BazhenovPost by Jim MankovichPost by Dmitry BazhenovIPMB-0, but still have 20h for LAN access.
Nevertheless, I agree that the proposed workaround might break the
sensor reading for some useful cases.
After some thinking I have come to a conclusion, that IPMITool shall
read the Management Controller Device Locator record to get the
IPMB-0
Post by Dmitry BazhenovPost by Dmitry BazhenovPost by Dmitry BazhenovPost by Jim MankovichPost by Dmitry Bazhenovslave address of the controller before sending sensor commands.
If the slave address of the target IPMC is the same as the sensor
owner, than bridging shall not be used, and vice versa.
There should be also a restriction for the case when the target IPMC
is already accessed via double bridging. In this case, if sensors
reside on another IPMC and the access channel is not the same as to
access target IPMC, they can be inaccessible since triple bridging is
impossible. IPMITool shall also send Get Channel Info command to the
target IPMC to check this.
I'll come up with the updates soon.
Regards,
Dmitry
Post by Jim MankovichDimitry,
I took at look at your bugfix patch and found that it has a
dependency
patch
Post by Dmitry BazhenovPost by Dmitry BazhenovPost by Dmitry BazhenovPost by Jim MankovichPost by Dmitry BazhenovPost by Jim Mankovichdepends
on a prior patch already having been applied.
I would like to understand why you want to inhibit bridging sensor requests
more than one level. You mention in your comment that it is a
workaround for
PICMG systems, but the change is not specific to PICMG systems.
Won't
Post by Dmitry BazhenovPost by Dmitry BazhenovPost by Dmitry BazhenovPost by Jim MankovichPost by Dmitry BazhenovPost by Jim Mankovichthis
cause problems on systems that support more than a single level of bridging?
I no expert on how IPMI bridging works, but the code as written (before your
change) would support multiple levels of bridging.
Also, I don't believe the save/restore logic you are using will work
correctly if you
are actually bridging to a different target address. Your logic
is as
if (intf->target_addr == intf->my_addr) {
save_addr = intf->target_addr;
intf->target_addr = target;
save_channel = intf->target_channel;
intf->target_channel = channel;
}
..... (sendrecv) ...
if (intf->target_addr == intf->my_addr) {
intf->target_addr = save_addr;
intf->target_channel = save_channel;
}
If the intf->target_addr is changed to a value different than
intf->my_addr in the
first if block, then the restore will never happen in the second if
block. This is because
when you get to the second if, intf->target_addr won't equal
intf->my_addr anymore so
the intf restore of target_addr/target_channel won't happen.
Post by Dmitry BazhenovHello, IPMITool maintainers,
On behalf of Pigeon Point Systems company,
I would like to contribute the following patches which contains
IPMITool bug-fixes and some new functionality.
o lib/ipmi_sel.c
- zero data before making request to avoid reading of uninitialized
data
o lib/ipmi_mc.c
- do not print AUX info, if IPMC return 12-byte Get Device ID
response
o lib/ipmi_sdr.c
- do not bridge sensor requests when interface bridging is already
used. Is workaround for PICMG systems (ATCA/MicroTCA).
o lib/ipmi_picmg.c
- add support for hexadecimal and octal formats for the command-line
parameters
o configure.in
- fixed build for the newer autotool releases
- fixed cross-compilation
2. pps-oem.diff
o Add support for PPS H8 evaluation board BMC which contains Intel
82751 GbE MAC. RMCP+ support for this MAC controller has several
deviations.
3. pps-sol-instance.diff
o Add support for SOL instances other than 1.
4. pps-long-packets.diff
o Add support for HPM.2 compliant long packets. The implementation is
made against the HPM.2 0.9.2 draft specification, and
requires
Post by Dmitry BazhenovPost by Dmitry BazhenovPost by Dmitry BazhenovPost by Jim MankovichPost by Dmitry BazhenovPost by Jim MankovichPost by Dmitry Bazhenovupdate for the HPM.2 1.0 release version. Pigeon Point Systems is
going to provide the update later.
o HPM.1 upgrade and FRU Inventory commands were modified to
reuse
is
by
Post by Dmitry BazhenovPost by Dmitry BazhenovPost by Dmitry BazhenovPost by Jim MankovichPost by Dmitry BazhenovPost by Jim MankovichPost by Dmitry Bazhenovthe
corresponding OEM command-line parameter.
5. pps-serial-drivers.diff
o Add support for Terminal Mode and Basic Mode direct serial drivers.
The patches with greater numbers depend on patches with lesser numbers
and must be applied in the ascending order.
------------------------------------------------------------------------------
Post by Dmitry BazhenovPost by Dmitry BazhenovPost by Dmitry BazhenovPost by Jim MankovichPost by Dmitry BazhenovPost by Jim MankovichPost by Dmitry BazhenovLive Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats.http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Ipmitool-devel mailing list
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel
------------------------------------------------------------------------------
Post by Dmitry BazhenovPost by Dmitry BazhenovPost by Dmitry BazhenovLive Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond.
Discussions
malware
Post by Dmitry BazhenovPost by Dmitry BazhenovPost by Dmitry Bazhenovthreats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Ipmitool-devel mailing list
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel
------------------------------------------------------------------------------
Don't let slow site performance ruin your business. Deploy New Relic APM
Deploy New Relic app performance management and know exactly
what is happening inside your Ruby, Python, PHP, Java, and .NET app
Try New Relic at no cost today and get our sweet Data Nerd shirt too!
http://p.sf.net/sfu/newrelic-dev2dev
_______________________________________________
Ipmitool-devel mailing list
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel