diff --git a/README.md b/README.md index b99164d..519ab8c 100644 --- a/README.md +++ b/README.md @@ -20,7 +20,7 @@ The BMC must have IPMI over LAN enabled and an account with the User privilege: ## Upgrading -Version 1.2.03 makes the `protected` fields of the protocol classes (`AbstractIpmiRunner`, `MessageHandler`, `IpmiLanMessage`, `ConfidentialityAlgorithm`, `IntegrityAlgorithm`) `private`. Subclasses must use the new `protected` accessors instead; see [Upgrading from 1.2.02](https://metricshub.org/ipmi-java/upgrading.html#upgrading-from-1-2-02) for the list. The `IpmiClient` API is unchanged. The Full, Compact and Event-Only sensor records now share the `AbstractSensorRecord` superclass, and commands can check responses with `IpmiCommandCoder.validateResponse()`; both are described on the same page. The user name and password are now encoded in UTF-8 whatever the platform charset, and the BMC key is used as raw bytes; as a result, `AuthenticationAlgorithm.getKeyExchangeAuthenticationCode()` and `checkKeyExchangeAuthenticationCode()` take the key and password as `byte[]` instead of `String`. `UdpMessenger.getSentPackets()` is removed, and the `CONST1`/`CONST2` constants of `IntegrityAlgorithm` and `ConfidentialityAesCbc128` are now `private`. `IpmiConnector.closeConnection()` releases the connection, whose handle then throws `IllegalStateException`; the keep-alive is actually sent with the default configuration, as a `Connection.KeepAlive` request whose reply and timeout are not reported to the listeners; and `Constants.TIMEOUT`, which nothing reads, is deprecated. +Version 1.2.03 makes the `protected` fields of the protocol classes (`AbstractIpmiRunner`, `MessageHandler`, `IpmiLanMessage`, `ConfidentialityAlgorithm`, `IntegrityAlgorithm`) `private`. Subclasses must use the new `protected` accessors instead; see [Upgrading from 1.2.02](https://metricshub.org/ipmi-java/upgrading.html#upgrading-from-1-2-02) for the list. The `IpmiClient` API is unchanged. The Full, Compact and Event-Only sensor records now share the `AbstractSensorRecord` superclass, and commands can check responses with `IpmiCommandCoder.validateResponse()`; both are described on the same page. The user name and password are now encoded in UTF-8 whatever the platform charset, and the BMC key is used as raw bytes; as a result, `AuthenticationAlgorithm.getKeyExchangeAuthenticationCode()` and `checkKeyExchangeAuthenticationCode()` take the key and password as `byte[]` instead of `String`. `UdpMessenger.getSentPackets()` is removed, and the `CONST1`/`CONST2` constants of `IntegrityAlgorithm` and `ConfidentialityAesCbc128` are now `private`. `IpmiConnector.closeConnection()` releases the connection, whose handle then throws `IllegalStateException`; the keep-alive is actually sent with the default configuration, as a one-way Get Device ID whose reply and timeout are not reported to the listeners; a one-way IPMI message keeps its tag reserved until its reply or its timeout; and `Constants.TIMEOUT`, which nothing reads, is deprecated. ## Build instructions diff --git a/src/main/java/org/metricshub/ipmi/client/runner/GetSensorsRunner.java b/src/main/java/org/metricshub/ipmi/client/runner/GetSensorsRunner.java index b919ba2..aee8889 100644 --- a/src/main/java/org/metricshub/ipmi/client/runner/GetSensorsRunner.java +++ b/src/main/java/org/metricshub/ipmi/client/runner/GetSensorsRunner.java @@ -45,13 +45,15 @@ import org.metricshub.ipmi.core.coding.payload.lan.IPMIException; import org.metricshub.ipmi.core.coding.protocol.AuthenticationType; import org.metricshub.ipmi.core.common.TypeConverter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Get Full And Compact Sensor records */ public class GetSensorsRunner extends AbstractIpmiRunner> { - private static final int OEM_EVENT_READING_TYPE = 127; + private static final Logger LOGGER = LoggerFactory.getLogger(GetSensorsRunner.class); public GetSensorsRunner(IpmiClientConfiguration ipmiConfiguration) { super(ipmiConfiguration); @@ -88,12 +90,9 @@ public List call() throws Exception { // Only Full and Compact sensor records have a reading associated // with them (see IPMI specification for details) if (sensorRecord instanceof FullSensorRecord || sensorRecord instanceof CompactSensorRecord) { - int recordReadingId = TypeConverter - .byteToInt(((AbstractSensorRecord) sensorRecord).getSensorNumber()); - // If our record has got a reading associated, we get request // for it - GetSensorReadingResponseData data = getSensorRecordReading(recordReadingId); + GetSensorReadingResponseData data = getSensorRecordReading((AbstractSensorRecord) sensorRecord); // Build the states e.g. deviceName=OK|deviceName=Device Present String states = buildStates(data, sensorRecord); @@ -146,12 +145,13 @@ static String buildStates(final GetSensorReadingResponseData data, final SensorR final AbstractSensorRecord record = (AbstractSensorRecord) sensorRecord; final String deviceName = record.getName(); - if (record.getEventReadingType() == OEM_EVENT_READING_TYPE) { - return buildOemState(data.getRaw(), deviceName); - } - final List events = data.getStatesAsserted(record.getSensorType(), record.getEventReadingType()); + // Like any discrete sensor, an OEM sensor with no state asserted reports no state + if (ReadingType.isOem(record.getEventReadingType())) { + return events.isEmpty() ? Utils.EMPTY : buildOemState(data.getRaw(), deviceName); + } + return appendReadingTypes(events, deviceName); } catch (Exception e) { @@ -160,7 +160,7 @@ static String buildStates(final GetSensorReadingResponseData data, final SensorR } /** - * Build the state for oem event reading type (0x7f) + * Build the state of an OEM event/reading type (70h-7Fh) * * @param raw a byte array of the raw IPMI command data * @param deviceName the name of the device @@ -210,32 +210,36 @@ private static String createStateEntry(final String deviceName, final ReadingTyp } /** - * Using the given reading id run the GetSensorReading request to get reading data + * Run the GetSensorReading request of a sensor record. A reading the BMC does not provide (DataNotPresent) or + * refuses with another completion code costs that sensor its reading, not the whole walk. * - * @param recordReadingId the reading identifier of the sensor record - * @return {@link GetSensorReadingResponseData} instance - * @throws Exception at sendMessage or if the error completion code is not DataNotPresent + * @param sensorRecord the Full or Compact sensor record + * @return {@link GetSensorReadingResponseData} instance, or null when the BMC returned no reading + * @throws Exception at sendMessage when the BMC does not answer */ - private GetSensorReadingResponseData getSensorRecordReading(final int recordReadingId) throws Exception { + GetSensorReadingResponseData getSensorRecordReading(final AbstractSensorRecord sensorRecord) + throws Exception { + int sensorNumber = TypeConverter.byteToInt(sensorRecord.getSensorNumber()); try { - // If we have a reading id means the reading data (e.g. temperature) is potentially available so let's perform the - // re - if (recordReadingId >= 0) { - return (GetSensorReadingResponseData) getConnector() - .sendMessage( - getHandle(), - new GetSensorReading( - IpmiVersion.V20, - getHandle().getCipherSuite(), - AuthenticationType.RMCPPlus, - recordReadingId)); - - } + return (GetSensorReadingResponseData) getConnector() + .sendMessage( + getHandle(), + new GetSensorReading( + IpmiVersion.V20, + getHandle().getCipherSuite(), + AuthenticationType.RMCPPlus, + sensorNumber)); } catch (IPMIException e) { if (e.getCompletionCode() != CompletionCode.DataNotPresent) { - throw e; + LOGGER + .warn( + "Failed to read sensor {} ({}) on {}: {}", + sensorNumber, + sensorRecord.getName(), + getIpmiConfiguration().getHostname(), + e.getMessage()); } + return null; } - return null; } } diff --git a/src/main/java/org/metricshub/ipmi/core/api/sync/IpmiConnector.java b/src/main/java/org/metricshub/ipmi/core/api/sync/IpmiConnector.java index 135f0e6..946221a 100644 --- a/src/main/java/org/metricshub/ipmi/core/api/sync/IpmiConnector.java +++ b/src/main/java/org/metricshub/ipmi/core/api/sync/IpmiConnector.java @@ -29,6 +29,7 @@ import org.metricshub.ipmi.core.coding.commands.PrivilegeLevel; import org.metricshub.ipmi.core.coding.commands.ResponseData; import org.metricshub.ipmi.core.coding.commands.session.GetChannelAuthenticationCapabilitiesResponseData; +import org.metricshub.ipmi.core.coding.payload.CompletionCode; import org.metricshub.ipmi.core.coding.payload.lan.IPMIException; import org.metricshub.ipmi.core.coding.protocol.PayloadType; import org.metricshub.ipmi.core.coding.security.CipherSuite; @@ -402,20 +403,15 @@ private ResponseData sendThroughAsyncConnector( ResponseData responseData = null; int tries = 0; - int tag = -1; boolean messageSent = false; while (!messageSent) { try { ++tries; - if (tag >= 0) { - tag = asyncConnector.retry(connectionHandle, tag, request.getSupportedPayloadType()); - } - - if (tag < 0) { - tag = asyncConnector.sendMessage(connectionHandle, request, !waitForResponse); - } + // A fresh tag for every try: the request that timed out or was answered left the queue, or is about + // to, and a reply to the same tag would then be dropped as an orphan + int tag = asyncConnector.sendMessage(connectionHandle, request, !waitForResponse); logger.debug("Sending message with tag {}, try {}", tag, tries); @@ -429,27 +425,36 @@ private ResponseData sendThroughAsyncConnector( } catch (IPMIException e) { handleErrorResponse(tries, e); } catch (Exception e) { - handleRetriesWhenException(tries, e); + // No reply in time: the BMC already had the whole message timeout, the request is sent again at once + handleRetriesWhenException(tries, e, false); } } return responseData; } - private void handleRetriesWhenException(int tries, Exception e) throws Exception { + /** + * Throws the exception when the request was already sent again {@link #retries} times (retries + 1 tries), + * otherwise lets the caller send it again, after a random pause of up to {@link #idleTime} ms if asked. + */ + private void handleRetriesWhenException(int tries, Exception e, boolean pause) throws Exception { if (tries > retries) { throw e; - } else { + } + if (pause) { long sleepTime = (random.nextLong() % (idleTime / 2)) + (idleTime / 2); - Thread.sleep(sleepTime); - logger.warn("Receiving message failed, retrying", e); } + logger.warn("Receiving message failed, retrying: {}: {}", e.getClass().getSimpleName(), e.getMessage()); } + /** + * Retries a request the BMC answered with a transient completion code: after a pause when the BMC says it is busy, + * at once after a timeout on its side (C3h), where it already waited for the device it could not reach. + */ private void handleErrorResponse(int tries, IPMIException e) throws Exception { if (e.getCompletionCode().isTransient()) { - handleRetriesWhenException(tries, e); + handleRetriesWhenException(tries, e, e.getCompletionCode() != CompletionCode.Timeout); } else { throw e; } diff --git a/src/main/java/org/metricshub/ipmi/core/coding/commands/CommandCodes.java b/src/main/java/org/metricshub/ipmi/core/coding/commands/CommandCodes.java index 44e4d15..a7c02c6 100644 --- a/src/main/java/org/metricshub/ipmi/core/coding/commands/CommandCodes.java +++ b/src/main/java/org/metricshub/ipmi/core/coding/commands/CommandCodes.java @@ -39,6 +39,11 @@ public final class CommandCodes { */ public static final byte GET_CHASSIS_STATUS = 0x01; + /** + * An IPMI code for Get Device ID command (Application network function) + */ + public static final byte GET_DEVICE_ID = 0x01; + /** * An IPMI code for Chassis Control command */ diff --git a/src/main/java/org/metricshub/ipmi/core/coding/commands/fru/record/ChassisType.java b/src/main/java/org/metricshub/ipmi/core/coding/commands/fru/record/ChassisType.java index 98b5cad..067b790 100644 --- a/src/main/java/org/metricshub/ipmi/core/coding/commands/fru/record/ChassisType.java +++ b/src/main/java/org/metricshub/ipmi/core/coding/commands/fru/record/ChassisType.java @@ -168,7 +168,7 @@ public static ChassisType parseInt(int value) { case LAPTOP: return LapTop; default: - logger.error("Invalid value: " + value); + logger.debug("Invalid value: {}", value); return Other; } } diff --git a/src/main/java/org/metricshub/ipmi/core/coding/commands/fru/record/FruMultiRecordType.java b/src/main/java/org/metricshub/ipmi/core/coding/commands/fru/record/FruMultiRecordType.java index 28d048d..e6a9164 100644 --- a/src/main/java/org/metricshub/ipmi/core/coding/commands/fru/record/FruMultiRecordType.java +++ b/src/main/java/org/metricshub/ipmi/core/coding/commands/fru/record/FruMultiRecordType.java @@ -88,7 +88,7 @@ public static FruMultiRecordType parseInt(int value) { case EXTENDEDCOMPATIBILITYRECORD: return ExtendedCompatibilityRecord; default: - logger.error("Invalid value: " + value); + logger.debug("Invalid value: {}", value); return Unspecified; } } diff --git a/src/main/java/org/metricshub/ipmi/core/coding/commands/fru/record/ManagementAccessRecordType.java b/src/main/java/org/metricshub/ipmi/core/coding/commands/fru/record/ManagementAccessRecordType.java index ae276bb..291823f 100644 --- a/src/main/java/org/metricshub/ipmi/core/coding/commands/fru/record/ManagementAccessRecordType.java +++ b/src/main/java/org/metricshub/ipmi/core/coding/commands/fru/record/ManagementAccessRecordType.java @@ -67,7 +67,7 @@ public static ManagementAccessRecordType parseInt(int value) { case COMPONENTMANAGEMENTURL: return ComponentManagementURL; default: - logger.error("Invalid value: " + value); + logger.debug("Invalid value: {}", value); return Unspecified; } } diff --git a/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/DeviceType.java b/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/DeviceType.java index af6e9e4..6ab1aae 100644 --- a/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/DeviceType.java +++ b/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/DeviceType.java @@ -172,7 +172,7 @@ public static DeviceType parseInt(int value) { case EEPROM24C02: return Eeprom24C02; default: - logger.error("Invalid value: " + value); + logger.debug("Invalid value: {}", value); return Other; } } diff --git a/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/EntityId.java b/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/EntityId.java index d1a19ee..6ea8e69 100644 --- a/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/EntityId.java +++ b/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/EntityId.java @@ -351,7 +351,7 @@ public static EntityId parseInt(int value) { case BASEBOARD: return Baseboard; default: - logger.error("Invalid value: " + value); + logger.debug("Invalid value: {}", value); return Other; } } diff --git a/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/ReadingType.java b/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/ReadingType.java index a65fb2c..ee66fe8 100644 --- a/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/ReadingType.java +++ b/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/ReadingType.java @@ -1119,7 +1119,19 @@ public int getCode() { } /** - * Determines type of discrete sensor reading. + * Tells whether an event/reading type is OEM: the IPMI 2.0 specification (Table 42-1) reserves {@code 70h} to + * {@code 7Fh} for OEM use, and defines none of their states. + * + * @param eventReadingType the event/reading type code of a sensor record + * @return true for an OEM event/reading type + */ + public static boolean isOem(int eventReadingType) { + return eventReadingType >= 0x70 && eventReadingType <= 0x7f; + } + + /** + * Determines type of discrete sensor reading. The states of an OEM event/reading type or of an OEM sensor type are + * {@link #UnknownOEMEvent}. * * @param sensorType * - {@link SensorType} of the sensor @@ -1132,7 +1144,7 @@ public int getCode() { */ public static ReadingType parseInt(SensorType sensorType, int eventReadingType, int offset) { - if (sensorType == SensorType.Oem) { + if (sensorType == SensorType.Oem || isOem(eventReadingType)) { return UnknownOEMEvent; } @@ -1670,10 +1682,8 @@ public static ReadingType parseInt(SensorType sensorType, int eventReadingType, case MONITORASICIC: return MonitorAsicIc; default: - logger - .warn( - "Invalid value: " + value + " (" + Integer.toHexString(value) - + ") for sensor " + sensorType); + // BMCs assert states the reading type does not define (HP iLO sets bits 6 and 7 of generic types) + logger.debug("Invalid value: {} ({}) for sensor {}", value, Integer.toHexString(value), sensorType); return Unknown; } } diff --git a/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/SensorType.java b/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/SensorType.java index 18eb0ce..4a8f21d 100644 --- a/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/SensorType.java +++ b/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/SensorType.java @@ -231,7 +231,7 @@ public static SensorType parseInt(int value) { if (value >= OEM) { return Oem; } - logger.error("Invalid value: " + value); + logger.debug("Invalid value: {}", value); return Oem; } } diff --git a/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/SensorUnit.java b/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/SensorUnit.java index 5ed5c76..6c7ae23 100644 --- a/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/SensorUnit.java +++ b/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/SensorUnit.java @@ -532,7 +532,7 @@ public static SensorUnit parseInt(int value) { case CORRECTABLEERROR: return CorrectableError; default: - logger.error("Invalid value: " + value); + logger.debug("Invalid value: {}", value); return Other; } } diff --git a/src/main/java/org/metricshub/ipmi/core/connection/Connection.java b/src/main/java/org/metricshub/ipmi/core/connection/Connection.java index 2491bf2..fdf5791 100644 --- a/src/main/java/org/metricshub/ipmi/core/connection/Connection.java +++ b/src/main/java/org/metricshub/ipmi/core/connection/Connection.java @@ -23,16 +23,23 @@ */ import org.metricshub.ipmi.core.coding.PayloadCoder; +import org.metricshub.ipmi.core.coding.commands.CommandCodes; +import org.metricshub.ipmi.core.coding.commands.IpmiCommandCoder; import org.metricshub.ipmi.core.coding.commands.IpmiVersion; import org.metricshub.ipmi.core.coding.commands.PrivilegeLevel; import org.metricshub.ipmi.core.coding.commands.ResponseData; -import org.metricshub.ipmi.core.coding.commands.session.GetChannelAuthenticationCapabilities; import org.metricshub.ipmi.core.coding.commands.session.GetChannelAuthenticationCapabilitiesResponseData; import org.metricshub.ipmi.core.coding.commands.session.GetChannelCipherSuitesResponseData; import org.metricshub.ipmi.core.coding.commands.session.OpenSessionResponseData; import org.metricshub.ipmi.core.coding.commands.session.Rakp1ResponseData; import org.metricshub.ipmi.core.coding.commands.session.Rakp3ResponseData; import org.metricshub.ipmi.core.coding.payload.IpmiPayload; +import org.metricshub.ipmi.core.coding.payload.lan.IPMIException; +import org.metricshub.ipmi.core.coding.payload.lan.IpmiLanMessage; +import org.metricshub.ipmi.core.coding.payload.lan.IpmiLanRequest; +import org.metricshub.ipmi.core.coding.payload.lan.NetworkFunction; +import org.metricshub.ipmi.core.coding.protocol.AuthenticationType; +import org.metricshub.ipmi.core.coding.protocol.IpmiMessage; import org.metricshub.ipmi.core.coding.protocol.Ipmiv20Message; import org.metricshub.ipmi.core.coding.protocol.PayloadType; import org.metricshub.ipmi.core.coding.security.AuthenticationRakpHmacSha1; @@ -680,24 +687,43 @@ public void notify(StateMachineAction action) { } /** - * The keep-alive request: queued like any request, so that its tag stays reserved until its reply arrives or it - * times out, but owned by nobody: its reply is discarded and its timeout is not reported to the listeners. + * The keep-alive request: a Get Device ID, as ipmitool sends. */ - public static final class KeepAlive extends GetChannelAuthenticationCapabilities { + private static final class KeepAlive extends IpmiCommandCoder { + + private KeepAlive(CipherSuite cipherSuite) { + super(IpmiVersion.V20, cipherSuite, AuthenticationType.RMCPPlus); + } + + @Override + public byte getCommandCode() { + return CommandCodes.GET_DEVICE_ID; + } + + @Override + public NetworkFunction getNetworkFunction() { + return NetworkFunction.ApplicationRequest; + } + + @Override + protected IpmiLanMessage preparePayload(int sequenceNumber) { + return new IpmiLanRequest(getNetworkFunction(), getCommandCode(), null, TypeConverter.intToByte(sequenceNumber)); + } + /** - * Creates the keep-alive request of a session. - * - * @param cipherSuite the {@link CipherSuite} of the session + * Never called: the keep-alive is sent one-way, its reply is discarded unread. */ - public KeepAlive(CipherSuite cipherSuite) { - super(IpmiVersion.V20, IpmiVersion.V20, cipherSuite, PrivilegeLevel.Callback, TypeConverter.intToByte(0xe)); + @Override + public ResponseData getResponseData(IpmiMessage message) throws IPMIException { + validateResponse(message); + return null; } } /** - * {@link TimerTask} runner - periodically sends a no-op message to keep the session up. The message is a - * {@link KeepAlive}: its reply is discarded, while a reply to the same command sent by the application is - * delivered to the application. When the message queue is full, the keep-alive of this period is skipped. + * {@link TimerTask} runner - periodically sends a no-op message to keep the session up: a Get Device ID sent + * one-way, whose reply and timeout are not reported to the listeners. When the message queue is full, the + * keep-alive of this period is skipped. */ @Override public void run() { @@ -706,7 +732,7 @@ public void run() { return; } try { - sendMessage(new KeepAlive(((SessionValid) current).getCipherSuite()), false); + sendMessage(new KeepAlive(((SessionValid) current).getCipherSuite()), true); } catch (Exception e) { LOGGER.error("Keep-alive failed: " + e.getMessage(), e); } diff --git a/src/main/java/org/metricshub/ipmi/core/connection/IpmiMessageHandler.java b/src/main/java/org/metricshub/ipmi/core/connection/IpmiMessageHandler.java index 0383606..6fdaee8 100644 --- a/src/main/java/org/metricshub/ipmi/core/connection/IpmiMessageHandler.java +++ b/src/main/java/org/metricshub/ipmi/core/connection/IpmiMessageHandler.java @@ -27,6 +27,7 @@ import org.metricshub.ipmi.core.coding.commands.ResponseData; import org.metricshub.ipmi.core.coding.payload.lan.IpmiLanMessage; import org.metricshub.ipmi.core.coding.protocol.Ipmiv20Message; +import org.metricshub.ipmi.core.connection.queue.QueueElement; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -73,8 +74,9 @@ protected void handleIncomingMessageInternal(Ipmiv20Message message) { if (message.getPayload() instanceof IpmiLanMessage) { IpmiLanMessage lanMessagePayload = (IpmiLanMessage) message.getPayload(); - PayloadCoder coder = getMessageQueue().getMessageFromQueue(lanMessagePayload.getSequenceNumber()); int tag = lanMessagePayload.getSequenceNumber(); + QueueElement element = getMessageQueue().getElement(tag); + PayloadCoder coder = element == null ? null : element.getRequest(); LOGGER.debug("Received message with tag " + tag); @@ -86,14 +88,8 @@ protected void handleIncomingMessageInternal(Ipmiv20Message message) { return; } - if (coder instanceof Connection.KeepAlive) { - // Nobody waits for the reply of the keep-alive: it only frees the tag - getMessageQueue().remove(tag); - return; - } - - // A late reply to a one-way message, whose tag is not reserved, may carry the tag of a newer queued - // request: the reply of a request answers the command of that request, and the request stays queued + // A late reply to a request that timed out may carry the tag of a newer queued request: the reply of a + // request answers the command of that request, and the request stays queued if (coder instanceof IpmiCommandCoder && !answers((IpmiCommandCoder) coder, lanMessagePayload)) { LOGGER .debug( @@ -102,6 +98,12 @@ protected void handleIncomingMessageInternal(Ipmiv20Message message) { return; } + if (element.isOneWay()) { + // Nobody waits for the reply of a one-way message (the keep-alive): it only frees the tag + getMessageQueue().remove(tag); + return; + } + try { ResponseData responseData = coder.getResponseData(message); getConnection().notifyResponseListeners(getConnection().getHandle(), tag, responseData, null); diff --git a/src/main/java/org/metricshub/ipmi/core/connection/MessageHandler.java b/src/main/java/org/metricshub/ipmi/core/connection/MessageHandler.java index c4e0517..c1c576d 100644 --- a/src/main/java/org/metricshub/ipmi/core/connection/MessageHandler.java +++ b/src/main/java/org/metricshub/ipmi/core/connection/MessageHandler.java @@ -95,15 +95,17 @@ public MessageHandler(Connection connection, int timeout, int minSequenceNumber, * ID of the current session. * @param isOneWay * flag indicating, if message is one way and we shouldn't await response, - * or it isn't and needs response from remote system - * @return sequence number of the sent message + * or it isn't and needs response from remote system. A one-way IPMI message is queued all the same, so that + * its tag stays reserved until its reply arrives or it times out, but its reply and its timeout are not + * reported (see {@link #takeTag(PayloadCoder, boolean)}). + * @return sequence number of the sent message, or -1 when the queue is full * @throws ConnectionException when could not send message due to some problems with connection */ public int sendMessage(PayloadCoder payloadCoder, StateMachine stateMachine, int sessionId, boolean isOneWay) throws ConnectionException { validateSessionState(stateMachine); - int seq = isOneWay ? messageQueue.getSequenceNumber() : messageQueue.add(payloadCoder); + int seq = takeTag(payloadCoder, isOneWay); if (seq > 0) { stateMachine .doTransition(new Sendv20Message(payloadCoder, sessionId, seq, connection.getNextSessionSequenceNumber())); @@ -112,6 +114,18 @@ public int sendMessage(PayloadCoder payloadCoder, StateMachine stateMachine, int return seq; } + /** + * Takes the tag of a message to send by queuing it: the tag stays reserved until its reply arrives or it times + * out. + * + * @param payloadCoder the message to send + * @param isOneWay true when nobody waits for the reply + * @return the tag of the message, or -1 when the queue is full + */ + protected int takeTag(PayloadCoder payloadCoder, boolean isOneWay) { + return messageQueue.add(payloadCoder, isOneWay); + } + /** * Attempts to retry sending message with given tag, assuming that this message exists in message queue. * @@ -178,6 +192,12 @@ public void tearDown() { messageQueue.tearDown(); } + /** + * Returns a sequence number for a message sent outside the queue (the Close Session request, the SOL ACK-only + * packets). + * + * @return a sequence number that no queued request holds + */ public int getSequenceNumber() { return messageQueue.getSequenceNumber(); } diff --git a/src/main/java/org/metricshub/ipmi/core/connection/SolMessageHandler.java b/src/main/java/org/metricshub/ipmi/core/connection/SolMessageHandler.java index def4655..5d550f9 100644 --- a/src/main/java/org/metricshub/ipmi/core/connection/SolMessageHandler.java +++ b/src/main/java/org/metricshub/ipmi/core/connection/SolMessageHandler.java @@ -45,6 +45,15 @@ public SolMessageHandler(Connection connection, int timeout) throws IOException super(connection, timeout, SolMessage.MIN_SEQUENCE_NUMBER, SolMessage.MAX_SEQUENCE_NUMBER); } + /** + * The one-way SOL messages are the ACK-only packets, which go out with packet sequence number 0 and which the BMC + * never acknowledges (IPMI 2.0 section 15.9): they are not queued, so that they hold no slot of the window. + */ + @Override + protected int takeTag(PayloadCoder payloadCoder, boolean isOneWay) { + return isOneWay ? getSequenceNumber() : super.takeTag(payloadCoder, isOneWay); + } + /** * Assuming that given message is SOL message, reads both data and acknowledge information from it, * notifying registered listeners about incoming data. diff --git a/src/main/java/org/metricshub/ipmi/core/connection/queue/MessageQueue.java b/src/main/java/org/metricshub/ipmi/core/connection/queue/MessageQueue.java index 1d69e3c..c477df3 100644 --- a/src/main/java/org/metricshub/ipmi/core/connection/queue/MessageQueue.java +++ b/src/main/java/org/metricshub/ipmi/core/connection/queue/MessageQueue.java @@ -136,6 +136,20 @@ private synchronized void releaseTag(int tag) { * that value. */ public int add(PayloadCoder request) { + return add(request, false); + } + + /** + * Adds request to the queue and generates the tag. A one-way request holds its tag like any other until its reply + * arrives or it times out, so that a late reply cannot be taken for the reply of a newer request; its reply and + * its timeout are not reported to the listeners. + * + * @param request the request to queue + * @param oneWay true when nobody waits for the reply + * @return Sequence number of the message if it was added to the queue, -1 otherwise. The tag used to identify + * message is equal to that value. + */ + public int add(PayloadCoder request, boolean oneWay) { run(); boolean first = true; synchronized (queue) { @@ -166,7 +180,7 @@ public int add(PayloadCoder request) { lastSequenceNumber = sequenceNumber; - QueueElement element = new QueueElement(sequenceNumber, request); + QueueElement element = new QueueElement(sequenceNumber, request, oneWay); queue.add(element); return sequenceNumber; @@ -240,8 +254,10 @@ public boolean containsId(int sequenceNumber) { } /** - * Returns the sequence number for a message that awaits no reply: it skips the tags of the queued requests, so - * a reply to the one-way message can never be taken for the reply of a queued request. + * Returns a sequence number for a message sent outside the queue (the Close Session request, the SOL ACK-only + * packets): it skips the tags of the queued requests. + * + * @return a sequence number that no queued request holds */ public int getSequenceNumber() { synchronized (lastSequenceNumberLock) { @@ -254,6 +270,23 @@ public int getSequenceNumber() { } } + /** + * Returns the queued request with the given tag, with its one-way flag. + * + * @param tag the tag of the request + * @return the {@link QueueElement} of the request, or null if no request with the given tag awaits a reply + */ + public QueueElement getElement(int tag) { + synchronized (queue) { + for (QueueElement element : queue) { + if (element.getId() == tag && element.getRequest() != null) { + return element; + } + } + } + return null; + } + /** * Returns message with the given sequence number from the queue or null if * no message with the given tag is currently in the queue. @@ -353,7 +386,7 @@ private boolean messageJustTimedOut(QueueElement oldestQueueElement) { /** * Removes the oldest message from the queue; when it timed out (rather than being answered), the response - * listeners are told so, which lets the sender retry it with a fresh tag. Nobody waits for a keep-alive: its + * listeners are told so, which lets the sender retry it with a fresh tag. Nobody waits for a one-way message: its * timeout is not reported. */ private void processObsoleteMessage(QueueElement message, boolean done) { @@ -362,7 +395,7 @@ private void processObsoleteMessage(QueueElement message, boolean done) { queue.remove(0); releaseTag(tag); - if (!done && !(message.getRequest() instanceof Connection.KeepAlive)) { + if (!done && !message.isOneWay()) { logger.debug("Message timed out, tag: {}", tag); try { connection diff --git a/src/main/java/org/metricshub/ipmi/core/connection/queue/QueueElement.java b/src/main/java/org/metricshub/ipmi/core/connection/queue/QueueElement.java index d3f0a9d..3c835b0 100644 --- a/src/main/java/org/metricshub/ipmi/core/connection/queue/QueueElement.java +++ b/src/main/java/org/metricshub/ipmi/core/connection/queue/QueueElement.java @@ -38,14 +38,32 @@ public class QueueElement { private PayloadCoder request; private ResponseData response; private Date timestamp; + private final boolean oneWay; public QueueElement(int id, PayloadCoder request) { + this(id, request, false); + } + + /** + * @param id the tag of the request + * @param request the request awaiting its reply + * @param oneWay true when nobody waits for the reply: the reply and the timeout of the request are not reported + */ + public QueueElement(int id, PayloadCoder request, boolean oneWay) { this.id = id; this.request = request; + this.oneWay = oneWay; timestamp = new Date(); retries = 0; } + /** + * @return true when nobody waits for the reply: the element only reserves the tag until the reply or the timeout + */ + public boolean isOneWay() { + return oneWay; + } + public int getId() { return id; } diff --git a/src/main/resources/vxipmi.properties b/src/main/resources/vxipmi.properties index 7bda221..dac06de 100644 --- a/src/main/resources/vxipmi.properties +++ b/src/main/resources/vxipmi.properties @@ -1,4 +1,4 @@ #Indicates how many times the message will be retried on a failure retries=3 -#Idle time in ms before resending a message that didn't receive answer. +#Upper bound in ms of the random pause before resending a request the BMC answered as busy (node busy, out of resources, initialization in progress). idleTime=4000 diff --git a/src/site/markdown/configuration.md b/src/site/markdown/configuration.md index 54f46b4..2e00f2d 100644 --- a/src/site/markdown/configuration.md +++ b/src/site/markdown/configuration.md @@ -93,9 +93,9 @@ this deadline; see [Timeouts and Errors](timeouts-and-errors.html). ### Keep-alive -While a session is open, the client sends a no-op message (Get Channel Authentication -Capabilities) every `pingPeriod` **milliseconds**, so that the BMC does not close the session for -inactivity during a long collection. +While a session is open, the client sends a no-op message (Get Device ID, as `ipmitool` does) every +`pingPeriod` **milliseconds**, so that the BMC does not close the session for inactivity during a +long collection. | `pingPeriod` | Behavior | | --- | --- | diff --git a/src/site/markdown/installation.md b/src/site/markdown/installation.md index d12c905..036a29e 100644 --- a/src/site/markdown/installation.md +++ b/src/site/markdown/installation.md @@ -57,9 +57,9 @@ What is logged, and at which level: | Level | Messages | | --- | --- | -| `ERROR` | A value the decoders do not know (`Invalid value: ...` for an entity ID, sensor type or unit), a failed session handshake, exceptions in the receiving and keep-alive threads. | -| `WARN` | An SDR record that cannot be decoded and is skipped, a FRU that cannot be read or decoded (the FRU is then truncated or missing), a message that failed and is resent, a packet whose integrity check failed, a response listener that threw while a timeout was reported. | -| `DEBUG` | Each message sent, with its tag and attempt number; each message that timed out; a session that could not be closed cleanly; every lookup of a [`connection.properties`](timeouts-and-errors.html#library-wide-defaults) value. | +| `ERROR` | A failed session handshake, exceptions in the receiving and keep-alive threads. | +| `WARN` | An SDR record that cannot be decoded and is skipped, a FRU that cannot be read or decoded (the FRU is then truncated or missing), a sensor whose reading the BMC refused, a message that failed and is resent, a packet whose integrity check failed, a response listener that threw while a timeout was reported. | +| `DEBUG` | Each message sent, with its tag and attempt number; each message that timed out; a value the decoders do not model (`Invalid value: ...` for an entity ID, sensor type, unit or state); a session that could not be closed cleanly; every lookup of a [`connection.properties`](timeouts-and-errors.html#library-wide-defaults) value. | > [!TIP] > Set the `org.metricshub.ipmi` logger to `WARN` in production, and to `DEBUG` when diagnosing a diff --git a/src/site/markdown/low-level-api.md b/src/site/markdown/low-level-api.md index 277e9b6..8e999a2 100644 --- a/src/site/markdown/low-level-api.md +++ b/src/site/markdown/low-level-api.md @@ -97,11 +97,12 @@ port (or always pass `0`), and call `tearDown()` when you are done with it. | `closeConnection(handle)` | Close and release the connection; the handle is no longer usable. | | `tearDown()` | Close every connection and release the local port. | -The keep-alive is a `Connection.KeepAlive` request, a Get Channel Authentication Capabilities that -the connection queues every `pingPeriod` ms while a session is open. Nobody owns it: its reply is -discarded instead of being delivered to the listeners, and its timeout is not reported. A request -of that class sent by the application gets the same treatment; send a plain -`GetChannelAuthenticationCapabilities` to get the reply. +The keep-alive is a Get Device ID that the connection sends one-way every `pingPeriod` ms while a +session is open. A one-way IPMI message (the keep-alive, or a request sent with +`sendOneWayMessage()`) is queued like any request: its tag stays reserved until its reply arrives +or it times out, so that a late reply cannot be taken for the reply of a later request, and it +takes one of the 8 slots of the message window until then. Its reply is discarded instead of +being delivered to the listeners, and its timeout is not reported. ### Choosing the cipher suite diff --git a/src/site/markdown/sensors.md b/src/site/markdown/sensors.md index 257a4a7..183bb39 100644 --- a/src/site/markdown/sensors.md +++ b/src/site/markdown/sensors.md @@ -44,7 +44,7 @@ Each [`Sensor`](apidocs/org/metricshub/ipmi/client/model/Sensor.html) holds: | `getEntityId()`, `getDeviceId()` | The entity the sensor belongs to: its type (`EntityId.Processor`, `EntityId.PowerSupply`, ...) and instance number | | `isFull()`, `isCompact()` | Whether the record is a Full Sensor record (an analog sensor with a conversion formula and thresholds) or a Compact one (usually a discrete sensor) | | `getRecord()` | The decoded record: a [`FullSensorRecord`](apidocs/org/metricshub/ipmi/core/coding/commands/sdr/record/FullSensorRecord.html) or a [`CompactSensorRecord`](apidocs/org/metricshub/ipmi/core/coding/commands/sdr/record/CompactSensorRecord.html), both [`AbstractSensorRecord`](apidocs/org/metricshub/ipmi/core/coding/commands/sdr/record/AbstractSensorRecord.html) | -| `getData()` | The [`GetSensorReadingResponseData`](apidocs/org/metricshub/ipmi/core/coding/commands/sdr/GetSensorReadingResponseData.html), or `null` when the BMC has no reading for the sensor (completion code `DataNotPresent`) | +| `getData()` | The [`GetSensorReadingResponseData`](apidocs/org/metricshub/ipmi/core/coding/commands/sdr/GetSensorReadingResponseData.html), or `null` when the BMC returns no reading for the sensor: completion code `DataNotPresent`, or another error completion code, logged at `WARN` ([Errors that do not fail the call](timeouts-and-errors.html#errors-that-do-not-fail-the-call)) | | `getStates()` | The asserted states, as `sensorName=state|sensorName=state...`, or an empty string | ### Readings @@ -99,8 +99,10 @@ CPU0_Status=Presence detected The raw states are available as `getData().getStatesAsserted(record.getSensorType(), record.getEventReadingType())`, a list of [`ReadingType`](apidocs/org/metricshub/ipmi/core/coding/commands/sdr/record/ReadingType.html). -For OEM sensors (event/reading type `0x7F`), whose states the specification does not define, -the state is the raw reading: `sensorName=0xHHLL`. +For OEM sensors (event/reading types `0x70` to `0x7F`), whose states the specification does not +define, the state is the raw value of the state bytes: `sensorName=0xHHLL` (`0xLL` when the BMC +returns a single state byte). An OEM sensor with no state asserted reports no state, like any +discrete sensor. ## Text output format diff --git a/src/site/markdown/supported-commands.md b/src/site/markdown/supported-commands.md index 5418f5b..d1305fb 100644 --- a/src/site/markdown/supported-commands.md +++ b/src/site/markdown/supported-commands.md @@ -17,7 +17,7 @@ ones `IpmiClient` uses. A command not listed here can be added by | Command | Class | NetFn / Cmd | Used by `IpmiClient` | | --- | --- | --- | --- | | **Session** | | | | -| Get Channel Authentication Capabilities | `GetChannelAuthenticationCapabilities` | App / `38h` | every call, and the keep-alive | +| Get Channel Authentication Capabilities | `GetChannelAuthenticationCapabilities` | App / `38h` | every call | | Get Channel Cipher Suites | `GetChannelCipherSuites` | App / `54h` | every call (unless `skipAuth`) | | RMCP+ Open Session | `OpenSession` | (payload) | every call | | RAKP Message 1 / 3 | `Rakp1`, `Rakp3` | (payload) | every call | diff --git a/src/site/markdown/timeouts-and-errors.md b/src/site/markdown/timeouts-and-errors.md index d59b143..e230984 100644 --- a/src/site/markdown/timeouts-and-errors.md +++ b/src/site/markdown/timeouts-and-errors.md @@ -32,18 +32,20 @@ never keep the JVM alive. Below the overall timeout, each message has its own timeout and is retried: 1. The request is sent, and the client waits for the reply up to the **per-message timeout**. -2. Without a reply, the request is sent again, after a random pause of up to `idleTime` - (4 000 ms), up to `retries` (3) times. +2. Without a reply, the request is sent again at once (the BMC already had the whole timeout), up + to `retries` (3) times. 3. When every try failed, the call fails with a `ConnectionException`: `Command timed out` during the session handshake, `Message timed out` in the session. -BMC replies with a *transient* completion code — node busy, out of resources, initialization in -progress, timeout — are retried the same way. Any other error completion code fails at once. +BMC replies with a *transient* completion code are retried too. In the session, the request is +sent again after a random pause of up to `idleTime` (4 000 ms) when the BMC says it is busy (node +busy, out of resources, initialization in progress), and at once after a timeout on the BMC side +(`C3h`), where the BMC already waited for the device it could not reach; the steps of the session +handshake are sent again at once. Any other error completion code fails at once. The per-message timeout is **5 s** by default, and `IpmiClient` caps it by the overall timeout. -With the defaults, a lost reply costs the per-message timeout plus the pause, then the request -is sent again; a BMC that never answers fails after 4 tries, about 20 s (plus the pauses) into -the call. +With the defaults, a lost reply costs the per-message timeout, then the request is sent again; a +BMC that never answers fails after 4 tries, about 20 s into the call. `IpmiClientConfiguration` does not expose the per-message timeout ([#101](https://github.com/metricshub/ipmi-java/issues/101)). To change it: @@ -62,7 +64,7 @@ The defaults come from two properties files packaged in the jar, read through th | --- | --- | --- | --- | | `timeout` | `5000` | Per-message timeout, in ms | When each connection is created | | `retries` | `3` | How many times a failed message is sent again | When each `IpmiConnector` is created | -| `idleTime` | `4000` | Upper bound of the random pause before a retry, in ms | When each `IpmiConnector` is created | +| `idleTime` | `4000` | Upper bound of the random pause before resending a request the BMC answered as busy, in ms | When each `IpmiConnector` is created | | `pingPeriod` | `30000` | Keep-alive period, in ms, when the configuration's `pingPeriod` is `-1` | When each `IpmiConnector` is created | Override them at application startup, from a single thread, before the first IPMI call: the @@ -97,7 +99,7 @@ Common causes wrapped in the `ExecutionException`: | `IllegalArgumentException: Authentication check failed` | The RAKP handshake failed: the BMC's proof does not match the password or the [BMC key](configuration.html#bmc-key). The credentials are sent once. | | `IPMIException: Unauthorized name.`, `Invalid role.`, ... | The BMC refused the session: unknown user, user not allowed over the LAN channel or at the User level, cipher suite refused ([Troubleshooting](troubleshooting.html#the-login-fails)). | | `ConnectionException: Command timed out` / `Message timed out` | No reply after all the [tries](#per-message-timeout-and-retries) of a message: `Command timed out` during the session handshake, the usual symptom of a wrong host, a closed UDP port or IPMI over LAN disabled; `Message timed out` in the session. | -| `IPMIException` | The BMC answered with an error completion code. `getCompletionCode()` returns it, for example `InsufficientPrivilege` (`0xD4`). | +| `IPMIException` | The BMC answered with an error completion code. `getCompletionCode()` returns it, for example `InsufficentPrivilege` (sic, `0xD4`). | | `IllegalArgumentException: ... is not yet implemented.` | The chosen cipher suite uses an algorithm the client does not implement (xRC4, MD5-128). See [cipher suites](preparing-the-bmc.html#cipher-suites). | | `Exception: Cannot get the available cipher suites.` | The BMC returned an empty cipher suite list. | | `UnknownHostException` | The host name cannot be resolved. | @@ -113,9 +115,10 @@ Some problems are logged at the `WARN` level and the call goes on with what it c ([Supported Commands](supported-commands.html#oem-and-unknown-records)); * a **FRU** that cannot be read (for example a FRU device that is not present) is reported truncated or not at all ([FRU Inventory](fru-inventory.html#how-the-frus-are-read)); -* a **sensor** whose reading is not available (completion code `DataNotPresent`) is returned - without reading data. +* a **sensor** whose reading is not available (completion code `DataNotPresent`, not logged) or + refused with another completion code is returned without reading data. -Unknown values in a record (an entity ID, a sensor type or a unit the library does not know) are -logged at the `ERROR` level as `Invalid value: ...` and replaced with a default (`Other` for an -entity ID); the record is still decoded. +Values a record may carry but the library does not model (an OEM or chassis-specific entity ID, a +reserved device or sensor type, a state the reading type does not define) are logged at the +`DEBUG` level as `Invalid value: ...` and replaced with a default (`Other` for an entity ID); the +record is still decoded. diff --git a/src/site/markdown/troubleshooting.md b/src/site/markdown/troubleshooting.md index 4045cab..d68a58f 100644 --- a/src/site/markdown/troubleshooting.md +++ b/src/site/markdown/troubleshooting.md @@ -51,7 +51,7 @@ sent once; the `ExecutionException` wraps the reason: Check the account with `ipmitool -I lanplus ... -L USER chassis status`: `ipmitool` reports `RAKP 2 HMAC is invalid` for a wrong password and `unauthorized name` for an unknown user. -## `IPMIException: Insufficient privilege level` (`0xD4`) +## `IPMIException: Cannot execute command due to insufficient privilege level ...` (`0xD4`) The BMC refused a command at the session's privilege level. `IpmiClient` always uses the **User** level, which the specification allows for every command it sends; if a BMC refuses @@ -64,6 +64,12 @@ one, check: With the [low-level API](low-level-api.html#privilege-level), open the session with the level the command needs (Operator for Chassis Control, Administrator for configuration commands). +When commands that worked earlier in the same session start failing with `0xD4`, the session was +revoked, and sending the command again in the same session does not help. On HP iLO 5, this is +what happens when another client deletes the sessions, for example a management tool logged in +as an administrator; the revocation shows at a whole minute of the session's age (60 s, 120 s, +...). Each `IpmiClient` call opens its own session, so the next call works again. + ## `... is not yet implemented.` `IllegalArgumentException: Confidentiality algorithm XRC4-128 is not yet implemented.` (or @@ -76,6 +82,7 @@ to make it use suite 3 or 17. | Symptom | Cause | | --- | --- | | `WARN Skipping SDR record ...` | A record that cannot be decoded is skipped; the other sensors are still returned. [SDR records](supported-commands.html#sdr-records) lists what is decoded. | +| `WARN Failed to read sensor () on : ...` | The BMC refused the Get Sensor Reading of that sensor with an error completion code: the sensor is returned without reading or states, and the other sensors are still returned. With the `0xD4` message, the BMC may have revoked the session (see the `0xD4` section above). | | `WARN Failed to read FRU at offset , the FRU data is truncated there: Requested Sensor, data, or record not present` | The FRU is declared in the SDR repository but not present, for example an empty power supply bay. Usually harmless: the reading stops there and the areas read so far are decoded. | | `WARN Failed to read FRU ` | The BMC did not answer Get FRU Inventory Area Info for that FRU, or its data is not in the IPMI FRU format (for example the SPD data of a memory module, [#107](https://github.com/metricshub/ipmi-java/issues/107)). The other FRUs are still returned. | | `WARN The info area at offset is truncated: skipped` | The read stopped before the end of that area (see above): the complete areas of the FRU are still returned. | diff --git a/src/site/markdown/upgrading.md b/src/site/markdown/upgrading.md index b47d005..3269087 100644 --- a/src/site/markdown/upgrading.md +++ b/src/site/markdown/upgrading.md @@ -24,6 +24,11 @@ The `IpmiClient` API is unchanged, and the client is more tolerant of real-world is 5 s by default (it was 5 minutes) and capped by the overall timeout, a retried message waits for the reply of the resent request, and the handshake steps wait for the elapsed time rather than a number of sleeps ([Timeouts and Errors](timeouts-and-errors.html)); +* a request is sent again at once after a lost reply or a timeout on the BMC side (`C3h`), where + 1.2.02 paused for a random time of up to `idleTime` (4 s) before each resend; with the 5 s + message timeout, these pauses made a collection on a Dell iDRAC 8, which answers `C3h` for its + absent FRUs and loses a few replies, take over two minutes; the pause now only follows the + completion codes that say the BMC is busy; * a BMC that never answers now fails the session handshake with `ExecutionException` wrapping `ConnectionException: Command timed out`, about 20 s into the call, where 1.2.02 threw `TimeoutException` at the overall timeout; @@ -39,9 +44,19 @@ The `IpmiClient` API is unchanged, and the client is more tolerant of real-world reply is sent again; * the [keep-alive](configuration.html#keep-alive) is actually sent: with the default `pingPeriod` (`-1`) 1.2.02 sent no keep-alive at all, so a session could expire during a long collection; the - keep-alive is now one message every 30 s by default, whose reply is discarded, and a - Get Channel Authentication Capabilities command sent by the application in a session gets its - reply (1.2.02 dropped it, as it did the keep-alive replies); + keep-alive is now a Get Device ID every 30 s by default, as `ipmitool` sends, whose reply is + discarded, and a Get Channel Authentication Capabilities command sent by the application in a + session gets its reply (1.2.02 dropped it, as it did the keep-alive replies); +* a one-way IPMI message (`IpmiConnector.sendOneWayMessage()`, `IpmiAsyncConnector.sendMessage()` + with `isOneWay`) is queued like any request: its tag stays reserved, and it takes a slot of the + 8-message window, until its reply arrives or it times out, where 1.2.02 did not reserve the tag, + so it could be handed out again while the reply was still on its way and a late reply taken for + the reply of a later request; its reply and its timeout are + still not reported (the Serial over LAN acknowledgements, which the BMC never answers, are not + queued); +* a sensor whose Get Sensor Reading fails with an error completion code is returned without + reading, and a `WARN` names it, where 1.2.02 failed the whole call (it tolerated + `DataNotPresent` only); * `IpmiConnector.closeConnection()` releases the connection: its handle then throws `IllegalStateException` instead of addressing a disconnected connection, and a session that fails to be established by `SerialOverLan` closes its own connection instead of tearing down the @@ -86,7 +101,18 @@ The decoders follow the IPMI 2.0 and FRU specifications more closely; the visibl Info fails, returns FRU 0 once, and stops reading a FRU at its first unreadable chunk instead of shifting the following chunks into the gap; * reserved values of the rate unit, modifier unit usage and power restore policy decode to - `None` or `Unknown` instead of throwing. + `None` or `Unknown` instead of throwing; +* the sensors of the OEM event/reading types `70h` to `7Eh` (Cisco IMC, Dell iDRAC and Fujitsu iRMC + use them for presence and module sensors) report the raw value of their state bytes + (`sensorName=0xHHLL`) like those of `7Fh`, where 1.2.02 reported `sensorName=Unknown`; an OEM + sensor (`70h` to `7Fh`) with no state asserted reports no state, where 1.2.02 reported the + state bytes of a `7Fh` one; `ReadingType.parseInt()`, and so `getStatesAsserted()` and the event + of a SEL record, returns `UnknownOEMEvent` for the states of `70h` to `7Fh`, where 1.2.02 + returned `Unknown` unless the sensor type was OEM; the new `ReadingType.isOem()` tells these + types apart; +* a value the decoders do not model (an OEM or chassis-specific entity ID, a reserved device or + sensor type, a state the reading type does not define) is logged at `DEBUG` instead of `ERROR` + or `WARN`. Code that **uses the low-level API** to read FRUs: the `offset` and `countToRead` arguments of the `ReadFruData` constructors are now **in bytes** whatever the access unit of the device, and diff --git a/src/test/java/org/metricshub/ipmi/client/runner/GetSensorsRunnerTest.java b/src/test/java/org/metricshub/ipmi/client/runner/GetSensorsRunnerTest.java index c360586..092bd31 100644 --- a/src/test/java/org/metricshub/ipmi/client/runner/GetSensorsRunnerTest.java +++ b/src/test/java/org/metricshub/ipmi/client/runner/GetSensorsRunnerTest.java @@ -1,14 +1,26 @@ package org.metricshub.ipmi.client.runner; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import java.net.InetAddress; +import java.util.concurrent.atomic.AtomicReference; + import org.junit.jupiter.api.Test; +import org.metricshub.ipmi.client.IpmiClientConfiguration; import org.metricshub.ipmi.client.Utils; +import org.metricshub.ipmi.core.api.async.ConnectionHandle; +import org.metricshub.ipmi.core.api.sync.IpmiConnector; +import org.metricshub.ipmi.core.coding.PayloadCoder; +import org.metricshub.ipmi.core.coding.commands.ResponseData; import org.metricshub.ipmi.core.coding.commands.sdr.GetSensorReadingResponseData; import org.metricshub.ipmi.core.coding.commands.sdr.record.CompactSensorRecord; import org.metricshub.ipmi.core.coding.commands.sdr.record.FullSensorRecord; import org.metricshub.ipmi.core.coding.commands.sdr.record.SensorType; +import org.metricshub.ipmi.core.coding.payload.CompletionCode; +import org.metricshub.ipmi.core.coding.payload.lan.IPMIException; +import org.metricshub.ipmi.core.connection.ConnectionException; class GetSensorsRunnerTest { @@ -35,6 +47,7 @@ void testBuildStates() { final byte[] raw = { 1, 2, 3, 127 }; final GetSensorReadingResponseData data = validReading(); data.setRaw(raw); + data.setStatesAsserted(new boolean[] { true, true }); final CompactSensorRecord record = new CompactSensorRecord(); record.setName(DEVICE_NAME); record.setEventReadingType(OEM_EVENT_READING_TYPE); @@ -61,6 +74,7 @@ void testBuildStates() { final byte[] raw = { 1, 2, 3, 127 }; final GetSensorReadingResponseData data = validReading(); data.setRaw(raw); + data.setStatesAsserted(new boolean[] { true, true }); final FullSensorRecord record = new FullSensorRecord(); record.setName(DEVICE_NAME); record.setEventReadingType(OEM_EVENT_READING_TYPE); @@ -83,6 +97,68 @@ void testBuildStates() { } } + @Test + void everyOemEventReadingTypeReportsTheRawStates() { + // Cisco IMC and Dell iDRAC use 70h for their Entity Presence and Module/Board sensors + for (int eventReadingType = 0x70; eventReadingType <= 0x7f; eventReadingType++) { + final GetSensorReadingResponseData data = validReading(); + data.setRaw(new byte[] { 1, 2, 3, 0 }); + data.setStatesAsserted(new boolean[] { true, true }); + final CompactSensorRecord record = new CompactSensorRecord(); + record.setName(DEVICE_NAME); + record.setSensorType(SensorType.EntityPresence); + record.setEventReadingType(eventReadingType); + + assertEquals("name=0x0003", GetSensorsRunner.buildStates(data, record), Integer.toHexString(eventReadingType)); + + // No state asserted (bit 7 of byte 4 is reserved and set): no state, as for any discrete sensor + final GetSensorReadingResponseData idle = validReading(); + idle.setRaw(new byte[] { 1, 2, 0, (byte) 0x80 }); + idle.setStatesAsserted(new boolean[15]); + assertEquals(Utils.EMPTY, GetSensorsRunner.buildStates(idle, record), Integer.toHexString(eventReadingType)); + } + } + + @Test + void aRefusedReadingCostsOnlyThatSensor() throws Exception { + final AtomicReference failure = new AtomicReference<>(); + final IpmiConnector connector = new IpmiConnector(0, 0) { + @Override + public ResponseData sendMessage(ConnectionHandle handle, PayloadCoder request) throws Exception { + throw failure.get(); + } + }; + try { + final GetSensorsRunner runner = new GetSensorsRunner( + new IpmiClientConfiguration("bmc", "user", new char[0], null, false, 1)) { + @Override + protected IpmiConnector getConnector() { + return connector; + } + + @Override + protected ConnectionHandle getHandle() { + return new ConnectionHandle(0, InetAddress.getLoopbackAddress(), 623); + } + }; + final CompactSensorRecord record = new CompactSensorRecord(); + record.setName(DEVICE_NAME); + + // HP iLO answers D4h once the session was revoked + failure.set(new IPMIException(CompletionCode.InsufficentPrivilege)); + assertNull(runner.getSensorRecordReading(record)); + + failure.set(new IPMIException(CompletionCode.DataNotPresent)); + assertNull(runner.getSensorRecordReading(record)); + + // A BMC that does not answer fails the walk + failure.set(new ConnectionException("Message timed out")); + assertThrows(ConnectionException.class, () -> runner.getSensorRecordReading(record)); + } finally { + connector.tearDown(); + } + } + @Test void statesOfAnUnavailableOrUnscannedSensorAreNotReported() { final boolean[] statesAsserted = { true, false }; diff --git a/src/test/java/org/metricshub/ipmi/core/api/sync/IpmiConnectorTest.java b/src/test/java/org/metricshub/ipmi/core/api/sync/IpmiConnectorTest.java index e5cd670..93166fe 100644 --- a/src/test/java/org/metricshub/ipmi/core/api/sync/IpmiConnectorTest.java +++ b/src/test/java/org/metricshub/ipmi/core/api/sync/IpmiConnectorTest.java @@ -2,11 +2,31 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.lang.reflect.Field; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.TimeUnit; +import java.util.function.Function; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; import org.metricshub.ipmi.core.api.async.ConnectionHandle; +import org.metricshub.ipmi.core.api.async.IpmiAsyncConnector; +import org.metricshub.ipmi.core.coding.commands.IpmiVersion; +import org.metricshub.ipmi.core.coding.commands.PrivilegeLevel; +import org.metricshub.ipmi.core.coding.commands.chassis.GetChassisStatus; +import org.metricshub.ipmi.core.coding.protocol.AuthenticationType; +import org.metricshub.ipmi.core.coding.security.CipherSuite; +import org.metricshub.ipmi.core.common.PropertiesManager; +import org.metricshub.ipmi.core.connection.Connection; import org.metricshub.ipmi.core.connection.ConnectionException; +import org.metricshub.ipmi.core.connection.ConnectionManager; +import org.metricshub.ipmi.core.sm.StateMachine; +import org.metricshub.ipmi.core.sm.states.SessionValid; import org.metricshub.ipmi.core.transport.FakeBmc; class IpmiConnectorTest { @@ -35,6 +55,22 @@ class IpmiConnectorTest { private static final int TIMEOUT_MS = 500; + /** The session ID of the console in the sessions the tests fake. */ + private static final int SESSION_ID = 1; + + /** Offset of the IPMI payload in an RMCP+ datagram of cipher suite 0. */ + private static final int PAYLOAD_OFFSET = 16; + + /** Get Chassis Status response data: power on, no last power event, no chassis state flag. */ + private static final byte[] CHASSIS_STATUS = { 0x01, 0x00, 0x00 }; + + /** + * Long enough for any pause before a resend to show: a random pause of up to one hour either shows in the gap or + * trips the timeout of the test, where a resend without pause comes within the message timeout plus one tick of + * the queue timer. + */ + private static final String LONG_IDLE_TIME = "3600000"; + @Test void aBadReplyFailsTheStepAtOnceAndLeavesItRetriable() throws Exception { try (FakeBmc bmc = new FakeBmc(request -> TRUNCATED_REPLY)) { @@ -56,4 +92,117 @@ void aBadReplyFailsTheStepAtOnceAndLeavesItRetriable() throws Exception { } } } + + @Test + @Timeout(30) + void aLostReplyIsSentAgainWithoutPause() throws Exception { + // The BMC drops the first request and answers the second one + long gapMs = gapBetweenTwoTries(tries -> tries == 1 ? null : 0x00); + assertTrue(gapMs < TIMEOUT_MS + 1500, "the request was sent again after " + gapMs + " ms"); + } + + @Test + @Timeout(30) + void aTimeoutOnTheBmcSideIsSentAgainWithoutPause() throws Exception { + // C3h, "Timeout while processing command": the BMC already waited for the device it could not reach + long gapMs = gapBetweenTwoTries(tries -> tries == 1 ? 0xc3 : 0x00); + assertTrue(gapMs < 1000, "the request was sent again after " + gapMs + " ms"); + } + + /** + * Sends a Get Chassis Status in a faked session to a BMC that answers each try with the given completion code (or + * not at all, for null), and returns the time between the first two tries. + */ + private static long gapBetweenTwoTries(Function completionCodeOfTry) throws Exception { + PropertiesManager properties = PropertiesManager.getInstance(); + String idleTime = properties.getProperty("idleTime"); + properties.setProperty("idleTime", LONG_IDLE_TIME); + List arrivals = new CopyOnWriteArrayList<>(); + try (FakeBmc bmc = new FakeBmc(request -> { + arrivals.add(System.nanoTime()); + Integer completionCode = completionCodeOfTry.apply(arrivals.size()); + return completionCode == null ? null : sessionReply(request, completionCode, CHASSIS_STATUS); + })) { + // No keep-alive: the BMC sees the requests of the test only + IpmiConnector connector = new IpmiConnector(0, 0); + try { + ConnectionHandle handle = connector + .createConnection(bmc.getAddress(), bmc.getPort(), CipherSuite.getEmpty(), PrivilegeLevel.User); + connector.setTimeout(handle, TIMEOUT_MS); + openSession(connector, handle); + + assertNotNull( + connector + .sendMessage( + handle, + new GetChassisStatus(IpmiVersion.V20, CipherSuite.getEmpty(), AuthenticationType.RMCPPlus))); + assertEquals(2, arrivals.size(), "the request must be sent twice"); + return TimeUnit.NANOSECONDS.toMillis(arrivals.get(1) - arrivals.get(0)); + } finally { + connector.tearDown(); + } + } finally { + properties.setProperty("idleTime", idleTime); + } + } + + /** Puts the connection in a session of cipher suite 0, as if the handshake had succeeded. */ + private static void openSession(IpmiConnector connector, ConnectionHandle handle) throws Exception { + IpmiAsyncConnector asyncConnector = (IpmiAsyncConnector) field(IpmiConnector.class, "asyncConnector") + .get(connector); + ConnectionManager manager = (ConnectionManager) field(IpmiAsyncConnector.class, "connectionManager") + .get(asyncConnector); + Connection connection = manager.getConnection(handle.getHandle()); + ((StateMachine) field(Connection.class, "stateMachine").get(connection)) + .setCurrent(new SessionValid(CipherSuite.getEmpty(), SESSION_ID)); + } + + private static Field field(Class type, String name) throws NoSuchFieldException { + Field field = type.getDeclaredField(name); + field.setAccessible(true); + return field; + } + + /** + * The reply of a BMC to an IPMI request sent in a session of cipher suite 0 (neither authenticated nor + * encrypted): same tag and command, response network function, given completion code and data. + */ + private static byte[] sessionReply(byte[] request, int completionCode, byte[] data) { + byte[] payload = new byte[8 + data.length]; + payload[0] = (byte) 0x81; + payload[1] = (byte) ((((request[PAYLOAD_OFFSET + 1] & 0xff) >> 2) + 1) << 2); + payload[2] = (byte) -(payload[0] + payload[1]); + payload[3] = 0x20; + payload[4] = request[PAYLOAD_OFFSET + 4]; + payload[5] = request[PAYLOAD_OFFSET + 5]; + payload[6] = (byte) completionCode; + System.arraycopy(data, 0, payload, 7, data.length); + byte checksum = 0; + for (int i = 3; i < payload.length - 1; i++) { + checksum += payload[i]; + } + payload[payload.length - 1] = (byte) -checksum; + + byte[] reply = new byte[PAYLOAD_OFFSET + payload.length]; + byte[] header = { + 0x06, + 0x00, + (byte) 0xff, + 0x07, + 0x06, + 0x00, + SESSION_ID, + 0x00, + 0x00, + 0x00, + 0x01, + 0x00, + 0x00, + 0x00, + (byte) payload.length, + 0x00 }; + System.arraycopy(header, 0, reply, 0, PAYLOAD_OFFSET); + System.arraycopy(payload, 0, reply, PAYLOAD_OFFSET, payload.length); + return reply; + } } diff --git a/src/test/java/org/metricshub/ipmi/core/coding/commands/sdr/record/ReadingTypeTest.java b/src/test/java/org/metricshub/ipmi/core/coding/commands/sdr/record/ReadingTypeTest.java new file mode 100644 index 0000000..65d9347 --- /dev/null +++ b/src/test/java/org/metricshub/ipmi/core/coding/commands/sdr/record/ReadingTypeTest.java @@ -0,0 +1,30 @@ +package org.metricshub.ipmi.core.coding.commands.sdr.record; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +class ReadingTypeTest { + + @Test + void eventReadingTypes70hTo7FhAreOem() { + assertFalse(ReadingType.isOem(0x6f), "sensor-specific"); + assertTrue(ReadingType.isOem(0x70)); + assertTrue(ReadingType.isOem(0x7f)); + assertFalse(ReadingType.isOem(0x80), "reserved"); + } + + @Test + void statesOfAnOemEventReadingTypeAreUnknownOemEvents() { + assertEquals(ReadingType.UnknownOEMEvent, ReadingType.parseInt(SensorType.EntityPresence, 0x70, 3)); + assertEquals(ReadingType.UnknownOEMEvent, ReadingType.parseInt(SensorType.ModuleBoard, 0x75, 0)); + } + + @Test + void anUndefinedStateIsUnknown() { + // HP iLO asserts state 6 of the Device Present reading type (08h), which defines states 0 and 1 + assertEquals(ReadingType.Unknown, ReadingType.parseInt(SensorType.Fan, 0x08, 6)); + } +} diff --git a/src/test/java/org/metricshub/ipmi/core/connection/ConnectionTest.java b/src/test/java/org/metricshub/ipmi/core/connection/ConnectionTest.java index ef05066..a7f2b61 100644 --- a/src/test/java/org/metricshub/ipmi/core/connection/ConnectionTest.java +++ b/src/test/java/org/metricshub/ipmi/core/connection/ConnectionTest.java @@ -2,6 +2,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -33,7 +34,14 @@ import org.metricshub.ipmi.core.sm.states.SessionValid; import org.metricshub.ipmi.core.transport.UdpMessage; import java.util.List; +import java.util.Map; import java.util.concurrent.CountDownLatch; +import org.metricshub.ipmi.core.coding.commands.IpmiCommandCoder; +import org.metricshub.ipmi.core.coding.payload.lan.NetworkFunction; +import org.metricshub.ipmi.core.coding.payload.sol.SolAckState; +import org.metricshub.ipmi.core.coding.sol.SolCoder; +import org.metricshub.ipmi.core.connection.queue.MessageQueue; +import org.metricshub.ipmi.core.connection.queue.QueueElement; import org.metricshub.ipmi.core.coding.commands.session.GetChannelCipherSuitesResponseData; import org.metricshub.ipmi.core.sm.actions.ResponseAction; @@ -205,7 +213,7 @@ public void processRequest(IpmiPayload payload) { } @Test - void theKeepAliveReplyIsDiscardedAndFreesItsTag() throws Exception { + void theKeepAliveIsAOneWayGetDeviceIdWhoseReplyIsDiscarded() throws Exception { Connection connection = connect(TIMEOUT_MS); try { openSession(connection); @@ -223,11 +231,20 @@ public void processRequest(IpmiPayload payload) { }); connection.run(); int keepAliveTag = 1; // the first tag of a new connection - // The keep-alive reply (command 38h) is not delivered, and its tag is free again - connection.notify(new MessageAction(reply(keepAliveTag, (byte) 0x38))); + MessageQueue queue = handler(connection, PayloadType.Ipmi).getMessageQueue(); + QueueElement keepAlive = queue.getElement(keepAliveTag); + assertNotNull(keepAlive, "the keep-alive must hold its tag in the queue"); + assertTrue(keepAlive.isOneWay()); + IpmiCommandCoder command = (IpmiCommandCoder) keepAlive.getRequest(); + assertEquals(NetworkFunction.ApplicationRequest, command.getNetworkFunction()); + assertEquals(0x01, command.getCommandCode(), "Get Device ID"); + + // The keep-alive reply is not delivered, and frees the tag + connection.notify(new MessageAction(reply(keepAliveTag, (byte) 0x01))); assertEquals(-1, notifiedTag.get(), "the keep-alive reply must not reach the listeners"); + assertNull(queue.getElement(keepAliveTag)); - // The same command sent by the application gets its reply + // A request sent by the application gets its reply GetChannelAuthenticationCapabilities request = new GetChannelAuthenticationCapabilities( IpmiVersion.V20, IpmiVersion.V20, @@ -243,6 +260,42 @@ public void processRequest(IpmiPayload payload) { } } + @Test + void oneWayIpmiMessagesAreQueuedButNotTheSolAcknowledgements() throws Exception { + Connection connection = connect(60000); + try { + MessageHandler ipmi = handler(connection, PayloadType.Ipmi); + int tag = ipmi + .takeTag( + new GetChannelAuthenticationCapabilities( + IpmiVersion.V20, + IpmiVersion.V20, + CipherSuite.getEmpty(), + PrivilegeLevel.Callback, + (byte) 0xe), + true); + assertTrue(ipmi.getMessageQueue().getElement(tag).isOneWay()); + + // The BMC never acknowledges an ACK-only SOL packet: a stream of them must not fill the 8-slot window + MessageHandler sol = handler(connection, PayloadType.Sol); + for (int i = 0; i < 32; i++) { + int solTag = sol + .takeTag(new SolCoder((byte) (i % 15 + 1), (byte) 1, SolAckState.ACK, CipherSuite.getEmpty()), true); + assertTrue(solTag > 0, "ACK " + i + " got no tag"); + assertNull(sol.getMessageQueue().getElement(solTag), "ACK " + i + " was queued"); + } + } finally { + connection.disconnect(); + } + } + + @SuppressWarnings("unchecked") + private static MessageHandler handler(Connection connection, PayloadType payloadType) throws Exception { + Field field = Connection.class.getDeclaredField("messageHandlers"); + field.setAccessible(true); + return ((Map) field.get(connection)).get(payloadType); + } + /** A minimal Application (07h) IPMI LAN response with the given tag (rqSeq, bits 7:2 of byte 4) and command. */ private static Ipmiv20Message reply(int tag, byte command) { return reply(tag, (byte) 0x07, command); @@ -284,7 +337,7 @@ public void processRequest(IpmiPayload payload) { (byte) 0xe); int tag = connection.sendMessage(request, false); - // A late reply to a one-way Get Device ID (command 01h) whose tag was reused by the request + // A late reply to a Get Device ID (command 01h) that timed out and whose tag was reused by the request connection.notify(new MessageAction(reply(tag, (byte) 0x01))); assertEquals(-1, notifiedTag.get(), "a reply to another command must not answer the queued request"); diff --git a/src/test/java/org/metricshub/ipmi/core/connection/queue/MessageQueueTest.java b/src/test/java/org/metricshub/ipmi/core/connection/queue/MessageQueueTest.java index 2dd2ee8..ce4d4cd 100644 --- a/src/test/java/org/metricshub/ipmi/core/connection/queue/MessageQueueTest.java +++ b/src/test/java/org/metricshub/ipmi/core/connection/queue/MessageQueueTest.java @@ -2,6 +2,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.Arrays; @@ -137,17 +138,46 @@ void oneWaySequenceNumbersSkipTheQueuedTags() { } @Test - void aTimedOutKeepAliveIsNotReported() throws Exception { + void aTimedOutOneWayMessageIsNotReported() throws Exception { MessageQueue queue = newQueue(); try { connection.registerListener(recorder()); - int keepAlive = queue.add(new Connection.KeepAlive(CipherSuite.getEmpty())); + int oneWay = queue.add(request(), true); int request = queue.add(request()); Thread.sleep(TIMER_TICK_MS); - assertFalse(queue.containsId(keepAlive), "the keep-alive must leave the queue"); + assertFalse(queue.containsId(oneWay), "the one-way message must leave the queue"); assertEquals(Collections.singletonList(request + ":Message timed out"), reported); } finally { queue.tearDown(); } } + + @Test + void aOneWayMessageHoldsItsTagAndItsSlotUntilItsReply() { + // Only the reply may release the one-way message here, not a timeout on a slow machine + MessageQueue queue = new MessageQueue( + connection, + 60000, + IpmiLanMessage.MIN_SEQUENCE_NUMBER, + IpmiLanMessage.MAX_SEQUENCE_NUMBER); + try { + int oneWay = queue.add(request(), true); + assertTrue(queue.getElement(oneWay).isOneWay()); + // The rest of the window, with requests answered at once + for (int i = 1; i < WINDOW_SIZE; i++) { + int tag = queue.add(request()); + assertTrue(tag > 0 && tag != oneWay, "add " + i + " took tag " + tag); + assertFalse(queue.getElement(tag).isOneWay()); + queue.remove(tag); + } + assertEquals(-1, queue.add(request()), "the window slides only once the one-way message is answered"); + + // Its reply frees the tag and the window + queue.remove(oneWay); + assertNull(queue.getElement(oneWay)); + assertTrue(queue.add(request()) > 0); + } finally { + queue.tearDown(); + } + } }