enhance bmcsetup logic for openbmc #2990

zet809 · 2017-05-05T08:42:50Z

Include 3 main modification:

add isopenbmc attribute for the getbmcconfig command send by getipmi script in xcat-genesis-scripts
modify bmcsetup script to disable unsupported ipmitool in-band command for openbmc
in bmcconfig.pm, read bmc information from openbmc table if the request come from an openbmc.
read default password with key openbmc from passwd table if the request come from an openbmc.

Before modifying:

[p9euh02 /]# ipmitool lan print 1
Set in Progress         : Set Complete
...
IP Address              : 10.6.9.200
Subnet Mask             : 255.0.0.0
MAC Address             : 70:e2:84:14:24:74
Default Gateway IP      : 10.0.0.101
...

Configuring BMC:

[p9euh02 /]# /bin/bmcsetup
ipmitool version 1.8.18
Invalid channel: 16
/bin/bmcsetup: line 151:  3709 Terminated              allowcred.awk
<167>May  8 05:24:45 xcat.genesis.bmcsetup: BMC Information obtained from xCAT
<167>May  8 05:24:45 xcat.genesis.bmcsetup: NUMBMCS=1 ==> BMC IP=10.6.9.254/255.0.0.0, GW=10.0.0.101, VLAN=off
<166>May  8 05:24:45 xcat.genesis.bmcsetup: IPMIVER=2.0, IPMIMFG=42817, XPROD=16975
<166>May  8 05:24:45 xcat.genesis.bmcsetup: Auto detecting LAN channel...
<166>May  8 05:24:45 xcat.genesis.bmcsetup: Detected LAN channel 1
Setting LAN IP Address to 10.6.9.254
Setting LAN Subnet Mask to 255.0.0.0
Setting LAN Default Gateway IP to 10.0.0.101
<167>May  8 05:24:47 xcat.genesis.bmcsetup: snooze for 1 second...

After modifying:

[p9euh02 /]# ipmitool lan print 1
Set in Progress         : Set Complete
...
IP Address              : 10.6.9.254
Subnet Mask             : 255.0.0.0
MAC Address             : 70:e2:84:14:24:74
Default Gateway IP      : 10.0.0.101
...

For the task #2810

gurevichmark · 2017-05-05T12:02:39Z

xCAT-genesis-scripts/bin/bmcsetup

@@ -161,6 +168,7 @@ logger -s -t $log_label -p local4.info "IPMIVER=$IPMIVER, IPMIMFG=$IPMIMFG, XPRO
 #
 # IPMIMFG=2 = IBM
 # IPMIMFG=0 = OpenPower 
+# IPMIMFG=42817 and XPROD=16975 = OpenBMC
 #
 if [ "$IPMIMFG" == 2 ]; then #IBM


Not sure if this is very important, but in some cases in this file we use strings for comparisons and in some cases integers. Should we make this consistent:
if [ "$xxxx" == "yyy" ]
if [ "$xxxx" == yyy ]

Hi, @gurevichmark , thx very much for the comments. You are right we shall keep the consistent. I do that for the OpenBMC part.
But for others, UT is needed if there is any modifications. But I don't thnk we need to put too much effort to verify it on multiple kind of servers, right? What is your idea?

Yes, I agree. Maybe just change the code that gets executed for this PR.

gurevichmark

In this file, we use both = and == in if [] statements. Functionally they are equivalent.
Should we make them the same for readability ?
Also people online suggest that = should be used because == is not POSIX compliant. (dash, in particular, does not recognize it) http://stackoverflow.com/questions/2600281/what-is-the-difference-between-operator-and-in-bash/2601583#2601583

whowutwut · 2017-05-08T17:49:42Z

xCAT-genesis-scripts/bin/bmcsetup

@@ -258,13 +266,20 @@ elif [ "$IPMIMFG" == "674" ]; then # DELL
        ipmitool delloem lan set shared with lom$BMCPORT &>/dev/null
        ipmitool delloem lan set shared with failover all loms &>dev/null
    fi
+elif [ "$IPMIMFG" = "42817" -a "$XPROD" = "16975" ]; then


Can you add a comment here, # IBM OpenPower systems running OpenBMC

whowutwut · 2017-05-08T18:03:07Z

xCAT-genesis-scripts/bin/bmcsetup

 fi

+if [ -z "$ISOPENBMC" ]; then


Suggest that we test for OpenBMC and override the default, and be consistent with this. So default to non-openbmc and then override for OpenBMC always...

Example:

LAN_MED_TYPE="802.3" if [ ! -z "${ISOPENBMC+x}" ]; then # OpenBMC overrides LAN_MED_TYPE="Other LAN" # other here... fi

However, it looks like nothing else is defined at this time to put into this statement, but perhaps it will set the stage for future additions.

whowutwut · 2017-05-08T18:04:11Z

xCAT-genesis-scripts/bin/bmcsetup

+    TRIES=0
+    bmc_config_rc=0
+    # Set Channel Access to apply network setting 
+    #while ! ipmitool -d $idev lan set $LANCHAN access on; do


This line should be deleted.

whowutwut · 2017-05-08T18:07:05Z

xCAT-genesis-scripts/bin/bmcsetup

+# update the node status to 'bmcready' for openbmc, no more configuration is needed.
+if [ ! -z "$ISOPENBMC" ]; then
+    # To enable network configuration for openbmc
+    let idev=0


Why do we need to set this idev=0 if it seems to never increase or change...?

whowutwut · 2017-05-08T18:08:13Z

xCAT-genesis-scripts/bin/bmcsetup

+    if [ ! -z "$XCATMASTER" ]; then
+        if [ "$bmc_config_rc" = "0" ]; then
+            # Wait for some time for the new network setting is ready 
+            snooze


Did we check with firmware team about this snooze, it will default to 1 second, is it even needed?

whowutwut · 2017-05-08T18:16:04Z

xCAT-genesis-scripts/bin/getipmi

@@ -29,10 +29,17 @@ for LANCHAN in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16; do
 done
 BMCMAC=`ipmitool lan print $LANCHAN|grep ^MAC|awk '{print $4}'` #bmcconfig may opt to use DHCP, if so we need to feed up the mac address
 #TODO: need a way to get the DUID the service processor may use, perhaps reserve that for 'ibmsetup' since spec doesn't touch ipv6?
-
+IPMIMFG=`ipmitool mc info |grep "^Manufacturer ID"|awk  '{print $4}'`


Should we consider having a common file to move some of the common functions into another place?

[vhu@victorvm7 test]$ cat ipmi_common #!/bin/sh IPMIMFG="42817" XPROD="16975" if [ "$IPMIMFG" == "42817" -a "$XPROD" == "16975" ]; then ISOPENBMC=1 else ISOPENBMC=0 fi

If we source it in another file, we will get the variables set....

[vhu@victorvm7 test]$ cat getipmi #!/bin/sh echo "BEFORE: ISOPENBMC=$ISOPENBMC" . ./ipmi_common echo "AFTER:ISOPENBMC=$ISOPENBMC"

Result:

[vhu@victorvm7 test]$ ./getipmi BEFORE: ISOPENBMC= AFTER:ISOPENBMC=1

One downside is that we can't just use relative path . ./ipmi_common and we have to make sure we don't get "File not found..."

whowutwut · 2017-05-08T18:17:23Z

xCAT-server/lib/xcat/plugins/bmcconfig.pm

@@ -96,20 +96,24 @@ sub process_request {
    my $request  = shift;
    my $callback = shift;
    my $node     = $request->{'_xcat_clienthost'}->[0];
+    my $open_table = "ipmi";


This variable name doesn't really make sense.. "open table"... Suggest bmc_mgmt_type ?

zet809 · 2017-05-09T08:20:17Z

Hi, @vhu, I had modified based on some of your comments.
Not sure what happened, I can not reply your comments directly, so reply here.

For the question about snooze, I hadn't asked Firmware, but my verification shows that the network can not work immediately after applied. Wait for 1 seconds works for our p9euh01 and p9euh02 server.
Use a common file, I agree with you that it is a good idea. But this code section doesn't work for both getipmi and bmcsetup. So I didn't modify it.
For the line 'set access on', the command will be supported by Firmware later, I can comment it out when it is supported. So, I didn't delete that line.
Thx!

whowutwut

Thanks @zet809 , I am ok with the changes. Can merge in and give it more tries...

whowutwut · 2017-05-10T12:31:34Z

xCAT-genesis-scripts/bin/bmcsetup

+    bmc_config_rc=0
+    # Set Channel Access to apply network setting 
+    #while ! ipmitool -d 0 lan set $LANCHAN access on; do
+    while ! ipmitool -d 0 raw 0x06 0x40 $LANCHAN 0x42 0x44 > /dev/null; do


@zet809 I just followed the github issue you opened, I think we should add some comments and point to the openbmc issue so that we don't forgot why we are doing this raw 0x06 0x40 workaround...

enhance bmcsetup logic for openbmc

fe8e4b8

zet809 added this to the 2.13.4 milestone May 5, 2017

zet809 requested review from whowutwut and gurevichmark May 5, 2017 08:42

gurevichmark reviewed May 5, 2017

View reviewed changes

add set channel access on to apply network setting for OpenBMC

2aa3136

zet809 changed the title ~~[DO NOT MERGE]enhance bmcsetup logic for openbmc~~ enhance bmcsetup logic for openbmc May 8, 2017

whowutwut reviewed May 8, 2017

View reviewed changes

Update bmcsetup related code for OpenBMC

69f38cb

whowutwut approved these changes May 9, 2017

View reviewed changes

gurevichmark merged commit 7271df0 into xcat2:master May 9, 2017

whowutwut reviewed May 10, 2017

View reviewed changes

whowutwut mentioned this pull request Nov 6, 2017

[OpenBMC] Change the bmcsetup code back to use raw ipmi commands for "access on" #4242

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enhance bmcsetup logic for openbmc #2990

enhance bmcsetup logic for openbmc #2990

zet809 commented May 5, 2017 •

edited

gurevichmark May 5, 2017

zet809 May 8, 2017 •

edited

gurevichmark May 8, 2017

gurevichmark left a comment

whowutwut May 8, 2017

whowutwut May 8, 2017

whowutwut May 8, 2017

whowutwut May 8, 2017

whowutwut May 8, 2017

whowutwut May 8, 2017

whowutwut May 8, 2017

zet809 commented May 9, 2017

whowutwut left a comment

whowutwut May 10, 2017

enhance bmcsetup logic for openbmc #2990

enhance bmcsetup logic for openbmc #2990

Conversation

zet809 commented May 5, 2017 • edited

Choose a reason for hiding this comment

zet809 May 8, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gurevichmark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zet809 commented May 9, 2017

whowutwut left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zet809 commented May 5, 2017 •

edited

zet809 May 8, 2017 •

edited