handler: Fix request passing for legacy commands
Legacy OEM commands didn't handle the IANA numbers themselves as part
of their handler body, and expect the request buffer to not contain
them. We can't pass the raw buffer without removing the already
extracted prefixes.
This should make legacy OEM commands work again.
Also reworks group handling to become consistent with OEM handling. This
happens to fix cases where groupIds were not being returned with error
codes.
Change-Id: I10efe8004f2c2b262f48980852b46317035ca367
Signed-off-by: William A. Kennington III <wak@google.com>
diff --git a/dcmihandler.cpp b/dcmihandler.cpp
index edbca8f..de3d92b 100644
--- a/dcmihandler.cpp
+++ b/dcmihandler.cpp
@@ -37,8 +37,8 @@
constexpr auto DCMI_OPTION_12_MASK = 0x01;
constexpr auto DCMI_ACTIVATE_DHCP_MASK = 0x01;
constexpr auto DCMI_ACTIVATE_DHCP_REPLY = 0x00;
-constexpr auto DCMI_SET_CONF_PARAM_REQ_PACKET_MAX_SIZE = 0x05;
-constexpr auto DCMI_SET_CONF_PARAM_REQ_PACKET_MIN_SIZE = 0x04;
+constexpr auto DCMI_SET_CONF_PARAM_REQ_PACKET_MAX_SIZE = 0x04;
+constexpr auto DCMI_SET_CONF_PARAM_REQ_PACKET_MIN_SIZE = 0x03;
constexpr auto DHCP_TIMING1 = 0x04; // 4 sec
constexpr auto DHCP_TIMING2_UPPER = 0x00; // 2 min
constexpr auto DHCP_TIMING2_LOWER = 0x78;
@@ -310,18 +310,10 @@
return IPMI_CC_INVALID;
}
- auto requestData =
- reinterpret_cast<const dcmi::GetPowerLimitRequest*>(request);
std::vector<uint8_t> outPayload(sizeof(dcmi::GetPowerLimitResponse));
auto responseData =
reinterpret_cast<dcmi::GetPowerLimitResponse*>(outPayload.data());
- if (requestData->groupID != dcmi::groupExtId)
- {
- *data_len = 0;
- return IPMI_CC_INVALID_FIELD_REQUEST;
- }
-
sdbusplus::bus::bus sdbus{ipmid_get_sd_bus_connection()};
uint32_t pcapValue = 0;
bool pcapEnable = false;
@@ -337,8 +329,6 @@
return IPMI_CC_UNSPECIFIED_ERROR;
}
- responseData->groupID = dcmi::groupExtId;
-
/*
* Exception action if power limit is exceeded and cannot be controlled
* with the correction time limit is hardcoded to Hard Power Off system
@@ -380,15 +370,6 @@
auto requestData =
reinterpret_cast<const dcmi::SetPowerLimitRequest*>(request);
- std::vector<uint8_t> outPayload(sizeof(dcmi::SetPowerLimitResponse));
- auto responseData =
- reinterpret_cast<dcmi::SetPowerLimitResponse*>(outPayload.data());
-
- if (requestData->groupID != dcmi::groupExtId)
- {
- *data_len = 0;
- return IPMI_CC_INVALID_FIELD_REQUEST;
- }
sdbusplus::bus::bus sdbus{ipmid_get_sd_bus_connection()};
@@ -406,10 +387,7 @@
log<level::INFO>("Set Power Cap",
entry("POWERCAP=%u", requestData->powerLimit));
- responseData->groupID = dcmi::groupExtId;
- memcpy(response, outPayload.data(), outPayload.size());
- *data_len = outPayload.size();
-
+ *data_len = 0;
return IPMI_CC_OK;
}
@@ -426,15 +404,6 @@
auto requestData =
reinterpret_cast<const dcmi::ApplyPowerLimitRequest*>(request);
- std::vector<uint8_t> outPayload(sizeof(dcmi::ApplyPowerLimitResponse));
- auto responseData =
- reinterpret_cast<dcmi::ApplyPowerLimitResponse*>(outPayload.data());
-
- if (requestData->groupID != dcmi::groupExtId)
- {
- *data_len = 0;
- return IPMI_CC_INVALID_FIELD_REQUEST;
- }
sdbusplus::bus::bus sdbus{ipmid_get_sd_bus_connection()};
@@ -452,10 +421,7 @@
log<level::INFO>("Set Power Cap Enable",
entry("POWERCAPENABLE=%u", requestData->powerLimitAction));
- responseData->groupID = dcmi::groupExtId;
- memcpy(response, outPayload.data(), outPayload.size());
- *data_len = outPayload.size();
-
+ *data_len = 0;
return IPMI_CC_OK;
}
@@ -469,12 +435,6 @@
auto responseData =
reinterpret_cast<dcmi::GetAssetTagResponse*>(outPayload.data());
- if (requestData->groupID != dcmi::groupExtId)
- {
- *data_len = 0;
- return IPMI_CC_INVALID_FIELD_REQUEST;
- }
-
// Verify offset to read and number of bytes to read are not exceeding the
// range.
if ((requestData->offset > dcmi::assetTagMaxOffset) ||
@@ -497,8 +457,6 @@
return IPMI_CC_UNSPECIFIED_ERROR;
}
- responseData->groupID = dcmi::groupExtId;
-
// Return if the asset tag is not populated.
if (!assetTag.size())
{
@@ -544,12 +502,6 @@
auto responseData =
reinterpret_cast<dcmi::SetAssetTagResponse*>(outPayload.data());
- if (requestData->groupID != dcmi::groupExtId)
- {
- *data_len = 0;
- return IPMI_CC_INVALID_FIELD_REQUEST;
- }
-
// Verify offset to read and number of bytes to read are not exceeding the
// range.
if ((requestData->offset > dcmi::assetTagMaxOffset) ||
@@ -580,7 +532,6 @@
dcmi::writeAssetTag(assetTag);
- responseData->groupID = dcmi::groupExtId;
responseData->tagLength = assetTag.size();
memcpy(response, outPayload.data(), outPayload.size());
*data_len = outPayload.size();
@@ -606,8 +557,7 @@
*data_len = 0;
- if (requestData->groupID != dcmi::groupExtId ||
- requestData->bytes > dcmi::maxBytes ||
+ if (requestData->bytes > dcmi::maxBytes ||
requestData->offset + requestData->bytes > dcmi::maxCtrlIdStrLen)
{
return IPMI_CC_INVALID_FIELD_REQUEST;
@@ -629,7 +579,6 @@
auto responseStr = hostName.substr(requestData->offset, requestData->bytes);
auto responseStrLen = std::min(static_cast<std::size_t>(requestData->bytes),
responseStr.length() + 1);
- responseData->groupID = dcmi::groupExtId;
responseData->strLen = hostName.length();
std::copy(begin(responseStr), end(responseStr), responseData->data);
@@ -650,8 +599,7 @@
*data_len = 0;
- if (requestData->groupID != dcmi::groupExtId ||
- requestData->bytes > dcmi::maxBytes ||
+ if (requestData->bytes > dcmi::maxBytes ||
requestData->offset + requestData->bytes > dcmi::maxCtrlIdStrLen + 1 ||
(requestData->offset + requestData->bytes ==
dcmi::maxCtrlIdStrLen + 1 &&
@@ -700,7 +648,6 @@
return IPMI_CC_UNSPECIFIED_ERROR;
}
- responseData->groupID = dcmi::groupExtId;
responseData->offset = requestData->offset + requestData->bytes;
*data_len = sizeof(*responseData);
return IPMI_CC_OK;
@@ -767,12 +714,6 @@
return IPMI_CC_INVALID_FIELD_REQUEST;
}
- if (requestData->groupID != dcmi::groupExtId)
- {
- *data_len = 0;
- return IPMI_CC_INVALID_FIELD_REQUEST;
- }
-
auto responseData = reinterpret_cast<dcmi::GetDCMICapResponse*>(response);
// For each capabilities in a parameter fill the data from
@@ -807,7 +748,6 @@
}
}
- responseData->groupID = dcmi::groupExtId;
responseData->major = DCMI_SPEC_MAJOR_VERSION;
responseData->minor = DCMI_SPEC_MINOR_VERSION;
responseData->paramRevision = DCMI_PARAMETER_REVISION;
@@ -995,13 +935,6 @@
return IPMI_CC_INVALID_FIELD_REQUEST;
}
- if (requestData->groupID != dcmi::groupExtId)
- {
- log<level::ERR>("Invalid Group ID",
- entry("GROUP_ID=%d", requestData->groupID));
- return IPMI_CC_INVALID_FIELD_REQUEST;
- }
-
if (requestData->sensorType != dcmi::temperatureSensorType)
{
log<level::ERR>("Invalid sensor type",
@@ -1034,7 +967,6 @@
return IPMI_CC_UNSPECIFIED_ERROR;
}
- responseData->groupID = dcmi::groupExtId;
size_t payloadSize = temps.size() * sizeof(dcmi::temp_readings::Response);
if (!temps.empty())
{
@@ -1104,19 +1036,15 @@
{
auto requestData =
reinterpret_cast<const dcmi::SetConfParamsRequest*>(request);
- auto responseData =
- reinterpret_cast<dcmi::SetConfParamsResponse*>(response);
- if (requestData->groupID != dcmi::groupExtId ||
- *data_len < DCMI_SET_CONF_PARAM_REQ_PACKET_MIN_SIZE ||
+ if (*data_len < DCMI_SET_CONF_PARAM_REQ_PACKET_MIN_SIZE ||
*data_len > DCMI_SET_CONF_PARAM_REQ_PACKET_MAX_SIZE)
{
- log<level::ERR>("Invalid Group ID or Invalid Requested Packet size",
- entry("GROUP_ID=%d", requestData->groupID),
+ log<level::ERR>("Invalid Requested Packet size",
entry("PACKET SIZE=%d", *data_len));
+ *data_len = 0;
return IPMI_CC_INVALID_FIELD_REQUEST;
}
-
*data_len = 0;
try
@@ -1168,9 +1096,6 @@
log<level::ERR>(e.what());
return IPMI_CC_UNSPECIFIED_ERROR;
}
- responseData->groupID = dcmi::groupExtId;
- *data_len = sizeof(dcmi::SetConfParamsResponse);
-
return IPMI_CC_OK;
}
@@ -1186,11 +1111,9 @@
responseData->data[0] = 0x00;
- if (requestData->groupID != dcmi::groupExtId ||
- *data_len != sizeof(dcmi::GetConfParamsRequest))
+ if (*data_len != sizeof(dcmi::GetConfParamsRequest))
{
- log<level::ERR>("Invalid Group ID or Invalid Requested Packet size",
- entry("GROUP_ID=%d", requestData->groupID),
+ log<level::ERR>("Invalid Requested Packet size",
entry("PACKET SIZE=%d", *data_len));
return IPMI_CC_INVALID_FIELD_REQUEST;
}
@@ -1240,7 +1163,6 @@
return IPMI_CC_UNSPECIFIED_ERROR;
}
- responseData->groupID = dcmi::groupExtId;
responseData->major = DCMI_SPEC_MAJOR_VERSION;
responseData->minor = DCMI_SPEC_MINOR_VERSION;
responseData->paramRevision = DCMI_CONFIG_PARAMETER_REVISION;
@@ -1260,17 +1182,9 @@
}
ipmi_ret_t rc = IPMI_CC_OK;
- auto requestData =
- reinterpret_cast<const dcmi::GetPowerReadingRequest*>(request);
auto responseData =
reinterpret_cast<dcmi::GetPowerReadingResponse*>(response);
- if (requestData->groupID != dcmi::groupExtId)
- {
- *data_len = 0;
- return IPMI_CC_INVALID_FIELD_REQUEST;
- }
-
sdbusplus::bus::bus bus{ipmid_get_sd_bus_connection()};
int64_t power = 0;
try
@@ -1284,7 +1198,6 @@
entry("PROPERTY=%s", SENSOR_VALUE_PROP));
return IPMI_CC_UNSPECIFIED_ERROR;
}
- responseData->groupID = dcmi::groupExtId;
// TODO: openbmc/openbmc#2819
// Minimum, Maximum, Average power, TimeFrame, TimeStamp,
@@ -1425,13 +1338,6 @@
return IPMI_CC_INVALID_FIELD_REQUEST;
}
- if (requestData->groupID != dcmi::groupExtId)
- {
- log<level::ERR>("Invalid Group ID",
- entry("GROUP_ID=%d", requestData->groupID));
- return IPMI_CC_INVALID_FIELD_REQUEST;
- }
-
if (requestData->sensorType != dcmi::temperatureSensorType)
{
log<level::ERR>("Invalid sensor type",
@@ -1473,7 +1379,6 @@
return IPMI_CC_UNSPECIFIED_ERROR;
}
- responseData->groupID = dcmi::groupExtId;
size_t payloadSize = sensors.size() * sizeof(dcmi::sensor_info::Response);
if (!sensors.empty())
{
diff --git a/dcmihandler.hpp b/dcmihandler.hpp
index a188c25..4f35bc6 100644
--- a/dcmihandler.hpp
+++ b/dcmihandler.hpp
@@ -129,9 +129,8 @@
*/
struct GetAssetTagRequest
{
- uint8_t groupID; //!< Group extension identification.
- uint8_t offset; //!< Offset to read.
- uint8_t bytes; //!< Number of bytes to read.
+ uint8_t offset; //!< Offset to read.
+ uint8_t bytes; //!< Number of bytes to read.
} __attribute__((packed));
/** @struct GetAssetTagResponse
@@ -140,7 +139,6 @@
*/
struct GetAssetTagResponse
{
- uint8_t groupID; //!< Group extension identification.
uint8_t tagLength; //!< Total asset tag length.
} __attribute__((packed));
@@ -150,9 +148,8 @@
*/
struct SetAssetTagRequest
{
- uint8_t groupID; //!< Group extension identification.
- uint8_t offset; //!< Offset to write.
- uint8_t bytes; //!< Number of bytes to write.
+ uint8_t offset; //!< Offset to write.
+ uint8_t bytes; //!< Number of bytes to write.
} __attribute__((packed));
/** @struct SetAssetTagResponse
@@ -161,7 +158,6 @@
*/
struct SetAssetTagResponse
{
- uint8_t groupID; //!< Group extension identification.
uint8_t tagLength; //!< Total asset tag length.
} __attribute__((packed));
@@ -211,23 +207,12 @@
*/
bool getPcapEnabled(sdbusplus::bus::bus& bus);
-/** @struct GetPowerLimitRequest
- *
- * DCMI payload for Get Power Limit command request.
- */
-struct GetPowerLimitRequest
-{
- uint8_t groupID; //!< Group extension identification.
- uint16_t reserved; //!< Reserved
-} __attribute__((packed));
-
/** @struct GetPowerLimitResponse
*
* DCMI payload for Get Power Limit command response.
*/
struct GetPowerLimitResponse
{
- uint8_t groupID; //!< Group extension identification.
uint16_t reserved; //!< Reserved.
uint8_t exceptionAction; //!< Exception action.
uint16_t powerLimit; //!< Power limit requested in watts.
@@ -249,7 +234,6 @@
*/
struct SetPowerLimitRequest
{
- uint8_t groupID; //!< Group extension identification.
uint16_t reserved; //!< Reserved
uint8_t reserved1; //!< Reserved
uint8_t exceptionAction; //!< Exception action.
@@ -259,15 +243,6 @@
uint16_t samplingPeriod; //!< Statistics sampling period in seconds.
} __attribute__((packed));
-/** @struct SetPowerLimitResponse
- *
- * DCMI payload for Set Power Limit command response.
- */
-struct SetPowerLimitResponse
-{
- uint8_t groupID; //!< Group extension identification.
-} __attribute__((packed));
-
/** @brief Enable or disable the power capping
*
* @param[in] bus - dbus connection
@@ -281,29 +256,18 @@
*/
struct ApplyPowerLimitRequest
{
- uint8_t groupID; //!< Group extension identification.
uint8_t powerLimitAction; //!< Power limit activation
uint16_t reserved; //!< Reserved
} __attribute__((packed));
-/** @struct ApplyPowerLimitResponse
- *
- * DCMI payload for Acticate/Deactivate Power Limit command response.
- */
-struct ApplyPowerLimitResponse
-{
- uint8_t groupID; //!< Group extension identification.
-} __attribute__((packed));
-
/** @struct GetMgmntCtrlIdStrRequest
*
* DCMI payload for Get Management Controller Identifier String cmd request.
*/
struct GetMgmntCtrlIdStrRequest
{
- uint8_t groupID; //!< Group extension identification.
- uint8_t offset; //!< Offset to read.
- uint8_t bytes; //!< Number of bytes to read.
+ uint8_t offset; //!< Offset to read.
+ uint8_t bytes; //!< Number of bytes to read.
} __attribute__((packed));
/** @struct GetMgmntCtrlIdStrResponse
@@ -312,9 +276,8 @@
*/
struct GetMgmntCtrlIdStrResponse
{
- uint8_t groupID; //!< Group extension identification.
- uint8_t strLen; //!< ID string length.
- char data[]; //!< ID string
+ uint8_t strLen; //!< ID string length.
+ char data[]; //!< ID string
} __attribute__((packed));
/** @struct SetMgmntCtrlIdStrRequest
@@ -323,10 +286,9 @@
*/
struct SetMgmntCtrlIdStrRequest
{
- uint8_t groupID; //!< Group extension identification.
- uint8_t offset; //!< Offset to write.
- uint8_t bytes; //!< Number of bytes to read.
- char data[]; //!< ID string
+ uint8_t offset; //!< Offset to write.
+ uint8_t bytes; //!< Number of bytes to read.
+ char data[]; //!< ID string
} __attribute__((packed));
/** @struct GetMgmntCtrlIdStrResponse
@@ -335,8 +297,7 @@
*/
struct SetMgmntCtrlIdStrResponse
{
- uint8_t groupID; //!< Group extension identification.
- uint8_t offset; //!< Last Offset Written.
+ uint8_t offset; //!< Last Offset Written.
} __attribute__((packed));
/** @enum DCMICapParameters
@@ -357,8 +318,7 @@
*/
struct GetDCMICapRequest
{
- uint8_t groupID; //!< Group extension identification.
- uint8_t param; //!< Capability parameter selector.
+ uint8_t param; //!< Capability parameter selector.
} __attribute__((packed));
/** @struct GetDCMICapRequest
@@ -367,7 +327,6 @@
*/
struct GetDCMICapResponse
{
- uint8_t groupID; //!< Group extension identification.
uint8_t major; //!< DCMI Specification Conformance - major ver
uint8_t minor; //!< DCMI Specification Conformance - minor ver
uint8_t paramRevision; //!< Parameter Revision = 02h
@@ -406,7 +365,6 @@
*/
struct GetTempReadingsRequest
{
- uint8_t groupID; //!< Group extension identification.
uint8_t sensorType; //!< Type of the sensor
uint8_t entityId; //!< Entity ID
uint8_t entityInstance; //!< Entity Instance (0 means all instances)
@@ -419,7 +377,6 @@
*/
struct GetTempReadingsResponseHdr
{
- uint8_t groupID; //!< Group extension identification.
uint8_t numInstances; //!< No. of instances for requested id
uint8_t numDataSets; //!< No. of sets of temperature data
} __attribute__((packed));
@@ -526,7 +483,6 @@
*/
struct GetPowerReadingRequest
{
- uint8_t groupID; //!< Group extension identification.
uint8_t mode; //!< Mode
uint8_t modeAttribute; //!< Mode Attributes
} __attribute__((packed));
@@ -538,7 +494,6 @@
*/
struct GetPowerReadingResponse
{
- uint8_t groupID; //!< Group extension identification.
uint16_t currentPower; //!< Current power in watts
uint16_t minimumPower; //!< Minimum power over sampling duration
//!< in watts
@@ -558,7 +513,6 @@
*/
struct GetSensorInfoRequest
{
- uint8_t groupID; //!< Group extension identification.
uint8_t sensorType; //!< Type of the sensor
uint8_t entityId; //!< Entity ID
uint8_t entityInstance; //!< Entity Instance (0 means all instances)
@@ -571,7 +525,6 @@
*/
struct GetSensorInfoResponseHdr
{
- uint8_t groupID; //!< Group extension identification.
uint8_t numInstances; //!< No. of instances for requested id
uint8_t numRecords; //!< No. of record ids in the response
} __attribute__((packed));
@@ -594,23 +547,12 @@
*/
struct SetConfParamsRequest
{
- uint8_t groupID; //!< Group extension identification.
uint8_t paramSelect; //!< Parameter selector.
uint8_t setSelect; //!< Set Selector (use 00h for parameters that only
//!< have one set).
uint8_t data[]; //!< Configuration parameter data.
} __attribute__((packed));
-/** @struct SetConfParamsResponse
- *
- * DCMI Set DCMI Configuration Parameters Command response.
- * Refer DCMI specification Version 1.1 Section 6.1.2
- */
-struct SetConfParamsResponse
-{
- uint8_t groupID; //!< Group extension identification.
-} __attribute__((packed));
-
/** @struct GetConfParamsRequest
*
* DCMI Get DCMI Configuration Parameters Command.
@@ -618,7 +560,6 @@
*/
struct GetConfParamsRequest
{
- uint8_t groupID; //!< Group extension identification.
uint8_t paramSelect; //!< Parameter selector.
uint8_t setSelect; //!< Set Selector. Selects a given set of parameters
//!< under a given Parameter selector value. 00h if
@@ -632,7 +573,6 @@
*/
struct GetConfParamsResponse
{
- uint8_t groupID; //!< Group extension identification.
uint8_t major; //!< DCMI Spec Conformance - major ver = 01h.
uint8_t minor; //!< DCMI Spec Conformance - minor ver = 05h.
uint8_t paramRevision; //!< Parameter Revision = 01h.
diff --git a/include/ipmid/handler.hpp b/include/ipmid/handler.hpp
index c84bffd..53b0bb8 100644
--- a/include/ipmid/handler.hpp
+++ b/include/ipmid/handler.hpp
@@ -310,17 +310,18 @@
executeCallback(message::Request::ptr request) override
{
message::Response::ptr response = request->makeResponse();
- size_t len = request->payload.size();
// allocate a big response buffer here
response->payload.resize(
getChannelMaxTransferSize(request->ctx->channel));
+ size_t len = request->payload.size() - request->payload.rawIndex;
Cc ccRet{ccSuccess};
try
{
- ccRet = handler_(request->ctx->netFn, request->ctx->cmd,
- request->payload.data(), response->payload.data(),
- &len, handlerCtx);
+ ccRet =
+ handler_(request->ctx->netFn, request->ctx->cmd,
+ request->payload.data() + request->payload.rawIndex,
+ response->payload.data(), &len, handlerCtx);
}
catch (const std::exception& e)
{
@@ -399,16 +400,18 @@
executeCallback(message::Request::ptr request) override
{
message::Response::ptr response = request->makeResponse();
- size_t len = request->payload.size();
// allocate a big response buffer here
response->payload.resize(
getChannelMaxTransferSize(request->ctx->channel));
+ size_t len = request->payload.size() - request->payload.rawIndex;
Cc ccRet{ccSuccess};
try
{
- ccRet = handler_(request->ctx->cmd, request->payload.data(),
- response->payload.data(), &len);
+ ccRet =
+ handler_(request->ctx->cmd,
+ request->payload.data() + request->payload.rawIndex,
+ response->payload.data(), &len);
}
catch (const std::exception& e)
{