From cc32c99916fbd7ca724527a61fb27d889115a435 Mon Sep 17 00:00:00 2001 From: Jarrod Johnson Date: Mon, 23 Jul 2018 15:02:00 -0400 Subject: [PATCH] Improve pyghmi performance The session.py uses lists and struct liberally. We can be clearer and more efficient by just working with bytearrays instead, which are better fit for this purpose. Change-Id: I280db9322c9a4f89470d93cf6df56b18966edb51 --- pyghmi/ipmi/command.py | 13 ++-- pyghmi/ipmi/console.py | 18 ++---- pyghmi/ipmi/private/session.py | 113 ++++++++++++++------------------- 3 files changed, 64 insertions(+), 80 deletions(-) diff --git a/pyghmi/ipmi/command.py b/pyghmi/ipmi/command.py index 3aa81570..db0b1c59 100644 --- a/pyghmi/ipmi/command.py +++ b/pyghmi/ipmi/command.py @@ -412,7 +412,7 @@ class Command(object): retry=retry, timeout=timeout) if 'error' in rsp: raise exc.IpmiException(rsp['error'], rsp['code']) - rsp['data'] = buffer(bytearray(rsp['data'])) + rsp['data'] = buffer(rsp['data']) return rsp def raw_command(self, netfn, command, bridge_request=(), data=(), @@ -434,10 +434,13 @@ class Command(object): :param timeout: A custom amount of time to wait for initial reply :returns: dict -- The response from IPMI device """ - return self.ipmi_session.raw_command(netfn=netfn, command=command, - bridge_request=bridge_request, - data=data, delay_xmit=delay_xmit, - retry=retry, timeout=timeout) + rsp = self.ipmi_session.raw_command(netfn=netfn, command=command, + bridge_request=bridge_request, + data=data, delay_xmit=delay_xmit, + retry=retry, timeout=timeout) + if 'data' in rsp: + rsp['data'] = list(rsp['data']) + return rsp def get_power(self): """Get current power state of the managed system diff --git a/pyghmi/ipmi/console.py b/pyghmi/ipmi/console.py index fbbe4aa8..06e7194e 100644 --- a/pyghmi/ipmi/console.py +++ b/pyghmi/ipmi/console.py @@ -258,14 +258,12 @@ class Console(object): breakbyte = 0 if sendbreak: breakbyte = 0b10000 - payload = struct.pack("BBBB", self.myseq, 0, 0, breakbyte) - payload += output + payload = bytearray((self.myseq, 0, 0, breakbyte)) + output self.lasttextsize = len(output) needskeepalive = False if self.lasttextsize == 0: needskeepalive = True self.awaitingack = True - payload = struct.unpack("%dB" % len(payload), payload) self.lastpayload = payload self.send_payload(payload, retry=False, needskeepalive=needskeepalive) retries = 5 @@ -344,10 +342,10 @@ class Console(object): remdatalen = len(payload[4:]) # store remote len before dupe # retry logic, we must ack *this* many even if it is # a retry packet with new partial data - remdata = struct.pack("%dB" % remdatalen, *payload[4:]) + remdata = bytes(payload[4:]) if newseq == self.remseq: # it is a retry, but could have new data if remdatalen > self.lastsize: - remdata = remdata[4 + self.lastsize:] + remdata = bytes(remdata[4 + self.lastsize:]) else: # no new data... remdata = "" else: # TODO(jbjohnso) what if remote sequence number is wrong?? @@ -355,7 +353,7 @@ class Console(object): self.lastsize = remdatalen if remdata: # Do not subject callers to empty data self._print_data(remdata) - ackpayload = (0, self.remseq, remdatalen, 0) + ackpayload = bytearray((0, self.remseq, remdatalen, 0)) # Why not put pending data into the ack? because it's rare # and might be hard to decide what to do in the context of # retry situation @@ -376,7 +374,6 @@ class Console(object): else: # retry all or part of packet, but in a new form # also add pending output for efficiency and ease newtext = self.lastpayload[4 + ackcount:] - newtext = struct.pack("B"*len(newtext), *newtext) with self.outputlock: if (self.pendingoutput and not isinstance(self.pendingoutput[0], dict)): @@ -474,16 +471,16 @@ class ServerConsole(Console): remdatalen = len(payload[4:]) # store remote len before dupe # retry logic, we must ack *this* many even if it is # a retry packet with new partial data - remdata = struct.pack("%dB" % remdatalen, *payload[4:]) + remdata = bytes(payload[4:]) if newseq == self.remseq: # it is a retry, but could have new data if remdatalen > self.lastsize: - remdata = remdata[4 + self.lastsize:] + remdata = bytes(remdata[4 + self.lastsize:]) else: # no new data... remdata = "" else: # TODO(jbjohnso) what if remote sequence number is wrong?? self.remseq = newseq self.lastsize = remdatalen - ackpayload = (0, self.remseq, remdatalen, flag) + ackpayload = bytearray((0, self.remseq, remdatalen, flag)) # Why not put pending data into the ack? because it's rare # and might be hard to decide what to do in the context of # retry situation @@ -499,7 +496,6 @@ class ServerConsole(Console): self.awaitingack = False if nacked and not breakdetected: # the BMC was in some way unhappy newtext = self.lastpayload[4 + ackcount:] - newtext = struct.pack("B"*len(newtext), *newtext) with self.outputlock: if (self.pendingoutput and not isinstance(self.pendingoutput[0], dict)): diff --git a/pyghmi/ipmi/private/session.py b/pyghmi/ipmi/private/session.py index 366e9627..88469818 100644 --- a/pyghmi/ipmi/private/session.py +++ b/pyghmi/ipmi/private/session.py @@ -248,18 +248,18 @@ def _aespad(data): """ipmi demands a certain pad scheme, per table 13-20 AES-CBC encrypted payload fields. """ - newdata = list(data) currlen = len(data) + 1 # need to count the pad length field as well neededpad = currlen % 16 if neededpad: # if it happens to be zero, hurray, but otherwise invert the # sense of the padding neededpad = 16 - neededpad padval = 1 + pad = bytearray(neededpad) while padval <= neededpad: - newdata.append(padval) + pad[padval - 1] = padval padval += 1 - newdata.append(neededpad) - return newdata + pad.append(neededpad) + return pad def _checksum(*data): # Two's complement over the data @@ -628,16 +628,16 @@ class Session(object): """This function generate message for bridge request. It is a part of ipmi payload. """ - head = [constants.IPMI_BMC_ADDRESS, - constants.netfn_codes['application'] << 2] + head = bytearray((constants.IPMI_BMC_ADDRESS, + constants.netfn_codes['application'] << 2)) check_sum = _checksum(*head) # NOTE(fengqian): according IPMI Figure 14-11, rqSWID is set to 81h - boday = [0x81, self.seqlun, constants.IPMI_SEND_MESSAGE_CMD, - 0x40 | channel] + boday = bytearray((0x81, self.seqlun, constants.IPMI_SEND_MESSAGE_CMD, + 0x40 | channel)) # NOTE(fengqian): Track request self._add_request_entry((constants.netfn_codes['application'] + 1, self.seqlun, constants.IPMI_SEND_MESSAGE_CMD)) - return head + [check_sum] + boday + return head + bytearray((check_sum,)) + boday def _add_request_entry(self, entry=()): """This function record the request with netfn, sequence number and @@ -691,12 +691,12 @@ class Session(object): # figure 13-4, first two bytes are rsaddr and # netfn, for non-bridge request, rsaddr is always 0x20 since we are # addressing BMC while rsaddr is specified forbridge request - header = [rsaddr, netfn << 2] + header = bytearray((rsaddr, netfn << 2)) - reqbody = [rqaddr, self.seqlun, command] + list(data) - headsum = _checksum(*header) - bodysum = _checksum(*reqbody) - payload = header + [headsum] + reqbody + [bodysum] + reqbody = bytearray((rqaddr, self.seqlun, command)) + data + headsum = bytearray((_checksum(*header),)) + bodysum = bytearray((_checksum(*reqbody),)) + payload = header + headsum + reqbody + bodysum if bridge_request: payload = bridge_msg + payload # NOTE(fengqian): For bridge request, another check sum is needed. @@ -799,11 +799,13 @@ class Session(object): if retry is None: retry = not self.servermode if self.servermode: - data = [code] + list(data) + data = bytearray((code,)) + bytearray(data) if netfn is None: netfn = self.clientnetfn if command is None: command = self.clientcommand + else: + data = bytearray(data) ipmipayload = self._make_ipmi_payload(netfn, command, bridge_request, data) payload_type = constants.payload_types['ipmi'] @@ -835,10 +837,12 @@ class Session(object): payload_type = self.last_payload_type if not payload: payload = self.lastpayload - message = [0x6, 0, 0xff, 0x07] # constant RMCP header for IPMI + message = bytearray(b'\x06\x00\xff\x07') # constant IPMI RMCP header if retry: self.lastpayload = payload self.last_payload_type = payload_type + if not isinstance(payload, bytearray): + payload = bytearray(payload) message.append(self.authtype) baretype = payload_type if self.integrityalgo: @@ -853,10 +857,10 @@ class Session(object): elif baretype not in constants.payload_types.values(): raise NotImplementedError( "Unrecognized payload type %d" % baretype) - message += struct.unpack("!4B", struct.pack("> 8) iv = os.urandom(16) - message += list(struct.unpack("16B", iv)) - payloadtocrypt = _aespad(payload) + message += iv + payloadtocrypt = bytes(payload + _aespad(payload)) crypter = Cipher( algorithm=algorithms.AES(self.aeskey), mode=modes.CBC(iv), backend=self._crypto_backend ) encryptor = crypter.encryptor() - plaintext = struct.pack("%dB" % len(payloadtocrypt), - *payloadtocrypt) - crypted = encryptor.update(plaintext) + encryptor.finalize() - crypted = list(struct.unpack("%dB" % len(crypted), crypted)) - message += crypted + message += encryptor.update(payloadtocrypt + ) + encryptor.finalize() else: # no confidetiality algorithm message.append(psize & 0xff) message.append(psize >> 8) - message += list(payload) + message += payload if self.integrityalgo: # see table 13-8, # RMCP+ packet format # TODO(jbjohnso): SHA256 which is now @@ -904,18 +905,15 @@ class Session(object): neededpad = (len(message) - 2) % 4 if neededpad: neededpad = 4 - neededpad - message += [0xff] * neededpad + message += b'\xff' * neededpad message.append(neededpad) message.append(7) # reserved, 7 is the required value for the # specification followed - integdata = message[4:] - authcode = hmac.new(self.k1, - struct.pack("%dB" % len(integdata), - *integdata), + message += hmac.new(self.k1, + bytes(message[4:]), hashlib.sha1).digest()[:12] # SHA1-96 # per RFC2404 truncates to 96 bits - message += struct.unpack("12B", authcode) - self.netpacket = struct.pack("!%dB" % len(message), *message) + self.netpacket = message # advance idle timer since we don't need keepalive while sending # packets out naturally with util.protect(KEEPALIVE_SESSIONS): @@ -937,19 +935,14 @@ class Session(object): if padneeded < 0: raise exc.IpmiException("Password is too long for ipmi 1.5") password += '\x00' * padneeded - passdata = struct.unpack("16B", password) if checkremotecode: - seqbytes = struct.unpack("!4B", - struct.pack("> 2 + response = {'netfn': payload[1] >> 2} # ^^ remove header of rsaddr/netfn/lun/checksum/rq/seq/lun del payload[0:5] # remove the trailing checksum