Discussion:
[Ipmitool-devel] Edit FRU information update problem?
Björn Spruck
2015-12-02 15:47:52 UTC
Permalink
Hi,

There seems to be some problem with the fru edit function. to be more
exactly its the update of the header which seems to be wrong. if I edit
a valid fru

ipmitool -H xxxx -U yyyy -P yyy -T 0x20 -B 0 -t 0x78 -b 7 fru edit 0
field b 0 "Uni Mainz"

the header is not correctly updated.

before edit:

0000 0000: 01 00 00 01 *04* *08* 00 F2 01 03 19 7D 94 9D C2 41
........ ...}...A
0000 0010: 41 C2 42 42 C2 43 43 C2 44 44 C2 45 45 C1 00 0C A.BB.CC.
DD.EE...
0000 0020: 01 04 19 C2 65 65 C2 66 66 C2 67 67 C2 68 68 C2 ....ee.f
f.gg.hh.
0000 0030: 69 69 C2 6A 6A C2 6B 6B C1 00 00 00 00 00 00 23 ii.jj.kk
.......#
0000 0040: C0 82 06 47 71 5A 31 00 16 00 18 47 71 00 31 5A ...GqZ1.
...Gq.1Z
0000 0050: 16 00 18 FF FF FF FF FF FF FF FF FF FF FF FF FF ........
........

after edit:

0000 0000: 01 00 00 01 *05* *08* 00 F1 01 04 19 7D 94 9D C9 55
........ ...}...U
0000 0010: 6E 69 20 4D 61 69 6E 7A C2 42 42 C2 43 43 C2 44 ni Mainz
.BB.CC.D
0000 0020: 44 C2 45 45 C1 00 00 3B 01 04 19 C2 65 65 C2 66 D.EE...;
....ee.f
0000 0030: 66 C2 67 67 C2 68 68 C2 69 69 C2 6A 6A C2 6B 6B f.gg.hh.
ii.jj.kk
0000 0040: C1 00 00 00 00 00 00 23 C0 82 06 47 71 5A 31 00 .......#
...GqZ1.
0000 0050: 16 00 18 47 71 00 31 5A 16 00 18 FF FF FF FF FF ...Gq.1Z
........

You can notice that all data is moved 8 bytes and the offset for the
first record after the edited one is correctly modified. but the second
offset for the multirecord area) is not modified.

I guess around line 5050-5080 in ipmi_fru.c a line
header.offset.multi += change_size_by_8;
is missing (within some if statements...)

I would like to notice that the parser of the FRU is not detecting the
mismatch between header offsets and actual size of the records (overlap
between product info and multirecord area).
It would be nice to have the FRU information cross-checked before
writing, e.g. overlaps/offset and checksums verified. maybe a command
like fru check <file> for printing and check a dump would be nice to
have here.

Ciao,

Bjoern
Zdenek Styblik
2015-12-02 18:46:51 UTC
Permalink
Post by Björn Spruck
Hi,
There seems to be some problem with the fru edit function. to be more
exactly its the update of the header which seems to be wrong. if I edit
a valid fru
ipmitool -H xxxx -U yyyy -P yyy -T 0x20 -B 0 -t 0x78 -b 7 fru edit 0
field b 0 "Uni Mainz"
the header is not correctly updated.
Good evening Bjorn,

have you tested it with the latest version of ipmitool? I remember
there were some fixes in this area.

Thank you and best regards,
Z.
Post by Björn Spruck
0000 0000: 01 00 00 01 *04* *08* 00 F2 01 03 19 7D 94 9D C2 41
........ ...}...A
0000 0010: 41 C2 42 42 C2 43 43 C2 44 44 C2 45 45 C1 00 0C A.BB.CC.
DD.EE...
0000 0020: 01 04 19 C2 65 65 C2 66 66 C2 67 67 C2 68 68 C2 ....ee.f
f.gg.hh.
0000 0030: 69 69 C2 6A 6A C2 6B 6B C1 00 00 00 00 00 00 23 ii.jj.kk
.......#
0000 0040: C0 82 06 47 71 5A 31 00 16 00 18 47 71 00 31 5A ...GqZ1.
...Gq.1Z
0000 0050: 16 00 18 FF FF FF FF FF FF FF FF FF FF FF FF FF ........
........
0000 0000: 01 00 00 01 *05* *08* 00 F1 01 04 19 7D 94 9D C9 55
........ ...}...U
0000 0010: 6E 69 20 4D 61 69 6E 7A C2 42 42 C2 43 43 C2 44 ni Mainz
.BB.CC.D
0000 0020: 44 C2 45 45 C1 00 00 3B 01 04 19 C2 65 65 C2 66 D.EE...;
....ee.f
0000 0030: 66 C2 67 67 C2 68 68 C2 69 69 C2 6A 6A C2 6B 6B f.gg.hh.
ii.jj.kk
0000 0040: C1 00 00 00 00 00 00 23 C0 82 06 47 71 5A 31 00 .......#
...GqZ1.
0000 0050: 16 00 18 47 71 00 31 5A 16 00 18 FF FF FF FF FF ...Gq.1Z
........
You can notice that all data is moved 8 bytes and the offset for the
first record after the edited one is correctly modified. but the second
offset for the multirecord area) is not modified.
I guess around line 5050-5080 in ipmi_fru.c a line
header.offset.multi += change_size_by_8;
is missing (within some if statements...)
I would like to notice that the parser of the FRU is not detecting the
mismatch between header offsets and actual size of the records (overlap
between product info and multirecord area).
It would be nice to have the FRU information cross-checked before
writing, e.g. overlaps/offset and checksums verified. maybe a command
like fru check <file> for printing and check a dump would be nice to
have here.
Ciao,
Bjoern
------------------------------------------------------------------------------
Go from Idea to Many App Stores Faster with Intel(R) XDK
Give your users amazing mobile app experiences with Intel(R) XDK.
Use one codebase in this all-in-one HTML5 development environment.
Design, debug & build mobile apps & 2D/3D high-impact games for multiple OSs.
http://pubads.g.doubleclick.net/gampad/clk?id=254741911&iu=/4140
_______________________________________________
Ipmitool-devel mailing list
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel
Björn Spruck
2015-12-03 08:20:35 UTC
Permalink
Hello Zdenek,
Post by Zdenek Styblik
have you tested it with the latest version of ipmitool? I remember
there were some fixes in this area.
ubuntu is using:

ipmitool version 1.8.15 and the source code zip I dled from sourceforge
states version 1.8.15 as well

now I checked the git repository (after finally I was able to check it
out, its not directly browseable/visible with http).

The changes made in between these two version (in the relevant file)
seems not to have anything to do with my problem, they are just few lines.

Nevertheless, I checked it with the repo version => same problem.

(just as reference, the output for the command)

String size are not equal, resizing fru to fit new string
Read All FRU area
Fru Size : 512 bytes
Copy to new FRU
Section Length: 24
Padding Length: 1
NumByte Change: 7
Start SecChnge: c2
End SecChnge : 44
Start Section : 1
End Sec wo Pad: c1
End Section : c
New Padding Length: -6
change_size_by_8: 1
New Padding Length: 2
change_size_by_8: 1
header.offset.board: 1
Moving Section Product, from 32 to 40
Moving Remaining Bytes (Multi-Rec , etc..), from 64 to 72
Updating Field : 'AA' with 'Uni Mainz' ... (Length from '194' to '201')
Copying remaining of sections: 15
Calculate New Checksum: ffffff3b
Writing new FRU.
Done.

Ciao,
Bjoern
Björn Spruck
2015-12-03 09:05:21 UTC
Permalink
making the soiry short:

Adding to line 5045 in ipmi_fru.c

if ((f_type == 'c' ) || (f_type == 'b' ) || (f_type == 'p' ))
{
printf("Change multi offset from %d to
%d\n",header.offset.multi,header.offset.multi + change_size_by_8);
header.offset.multi += change_size_by_8;
}

solves the problem for me. but please check for side-effects.
Zdenek Styblik
2015-12-04 15:57:44 UTC
Permalink
Post by Björn Spruck
Hello Zdenek,
Post by Zdenek Styblik
have you tested it with the latest version of ipmitool? I remember
there were some fixes in this area.
ipmitool version 1.8.15 and the source code zip I dled from sourceforge
states version 1.8.15 as well
Post by Björn Spruck
now I checked the git repository (after finally I was able to check it
out, its not directly browseable/visible with http).
Post by Björn Spruck
The changes made in between these two version (in the relevant file)
seems not to have anything to do with my problem, they are just few lines.
Indeed. I've checked it by using % git blame; that evening, but didn't have
energy(and time ever since) to reply.

As for the fix you've posted. Sadly, I have nothing to test it at. However,
I want to ask
Post by Björn Spruck
Nevertheless, I checked it with the repo version => same problem.
(just as reference, the output for the command)
String size are not equal, resizing fru to fit new string
Read All FRU area
Fru Size : 512 bytes
Copy to new FRU
Section Length: 24
Padding Length: 1
NumByte Change: 7
Start SecChnge: c2
End SecChnge : 44
Start Section : 1
End Sec wo Pad: c1
End Section : c
New Padding Length: -6
change_size_by_8: 1
New Padding Length: 2
change_size_by_8: 1
header.offset.board: 1
Moving Section Product, from 32 to 40
Moving Remaining Bytes (Multi-Rec , etc..), from 64 to 72
Updating Field : 'AA' with 'Uni Mainz' ... (Length from '194' to '201')
Copying remaining of sections: 15
Calculate New Checksum: ffffff3b
Writing new FRU.
Done.
Ciao,
Bjoern
Zdenek Styblik
2015-12-04 15:57:46 UTC
Permalink
Post by Björn Spruck
Hello Zdenek,
Post by Zdenek Styblik
have you tested it with the latest version of ipmitool? I remember
there were some fixes in this area.
ipmitool version 1.8.15 and the source code zip I dled from sourceforge
states version 1.8.15 as well
Post by Björn Spruck
now I checked the git repository (after finally I was able to check it
out, its not directly browseable/visible with http).
Post by Björn Spruck
The changes made in between these two version (in the relevant file)
seems not to have anything to do with my problem, they are just few lines.
Indeed. I've checked it by using % git blame; that evening, but didn't have
energy(and time ever since) to reply.

As for the fix you've posted. Sadly, I have nothing to test it at. However,
I want to ask
Post by Björn Spruck
Nevertheless, I checked it with the repo version => same problem.
(just as reference, the output for the command)
String size are not equal, resizing fru to fit new string
Read All FRU area
Fru Size : 512 bytes
Copy to new FRU
Section Length: 24
Padding Length: 1
NumByte Change: 7
Start SecChnge: c2
End SecChnge : 44
Start Section : 1
End Sec wo Pad: c1
End Section : c
New Padding Length: -6
change_size_by_8: 1
New Padding Length: 2
change_size_by_8: 1
header.offset.board: 1
Moving Section Product, from 32 to 40
Moving Remaining Bytes (Multi-Rec , etc..), from 64 to 72
Updating Field : 'AA' with 'Uni Mainz' ... (Length from '194' to '201')
Copying remaining of sections: 15
Calculate New Checksum: ffffff3b
Writing new FRU.
Done.
Ciao,
Bjoern
Zdenek Styblik
2015-12-04 16:02:04 UTC
Permalink
Post by Zdenek Styblik
Post by Björn Spruck
Hello Zdenek,
Post by Zdenek Styblik
have you tested it with the latest version of ipmitool? I remember
there were some fixes in this area.
ipmitool version 1.8.15 and the source code zip I dled from sourceforge
states version 1.8.15 as well
Post by Zdenek Styblik
Post by Björn Spruck
now I checked the git repository (after finally I was able to check it
out, its not directly browseable/visible with http).
Post by Zdenek Styblik
Post by Björn Spruck
The changes made in between these two version (in the relevant file)
seems not to have anything to do with my problem, they are just few lines.
Post by Zdenek Styblik
Indeed. I've checked it by using % git blame; that evening, but didn't
have energy(and time ever since) to reply.
Post by Zdenek Styblik
As for the fix you've posted. Sadly, I have nothing to test it at.
However, I want to ask

... you to either create merge request or open ticket at sourceforge.net
and attach git generated patch. I have no problem with merging the fix.

Btw Good evening Bjorn
Btw2 I hate my phone

Over and out.

Best regards,
Z.
Post by Zdenek Styblik
Post by Björn Spruck
Nevertheless, I checked it with the repo version => same problem.
(just as reference, the output for the command)
String size are not equal, resizing fru to fit new string
Read All FRU area
Fru Size : 512 bytes
Copy to new FRU
Section Length: 24
Padding Length: 1
NumByte Change: 7
Start SecChnge: c2
End SecChnge : 44
Start Section : 1
End Sec wo Pad: c1
End Section : c
New Padding Length: -6
change_size_by_8: 1
New Padding Length: 2
change_size_by_8: 1
header.offset.board: 1
Moving Section Product, from 32 to 40
Moving Remaining Bytes (Multi-Rec , etc..), from 64 to 72
Updating Field : 'AA' with 'Uni Mainz' ... (Length from '194' to '201')
Copying remaining of sections: 15
Calculate New Checksum: ffffff3b
Writing new FRU.
Done.
Ciao,
Bjoern
Björn Spruck
2015-12-11 17:05:02 UTC
Permalink
Post by Zdenek Styblik
... you to either create merge request or open ticket at
sourceforge.net <http://sourceforge.net> and attach git generated
patch. I have no problem with merging the fix.
Done -> http://sourceforge.net/p/ipmitool/patches/108/
Zdenek Styblik
2015-12-13 08:00:04 UTC
Permalink
... you to either create merge request or open ticket at sourceforge.net and
attach git generated patch. I have no problem with merging the fix.
Done -> http://sourceforge.net/p/ipmitool/patches/108/
Please, can you address comments there?

~~~
git clone http://git.code.sf.net/p/ipmitool/source ipmitool-source
cd ipmitool-source
git branch superfeature
git checkout superfeature
# make changes and commits
# and once you're done
git format-patch origin
~~~

Thank you in advance.

Z.

--
Zdenek Styblik
email: ***@gmail.com
jabber: ***@gmail.com

------------------------------------------------------------------------------
Loading...