transporthandler: Generic set exception handling
This makes it easier to handle errors across multiple command types as
they all have common return codes for specific types of failures.
Change-Id: I033a171b5fca78a62b52424db970ecfdf7a15e17
Signed-off-by: William A. Kennington III <wak@google.com>
diff --git a/meson.build b/meson.build
index 67c5caf..b300247 100644
--- a/meson.build
+++ b/meson.build
@@ -117,6 +117,7 @@
mapper = dependency('libmapper')
boost_coroutine = cpp.find_library('boost_coroutine', required: true)
sdbusplus_dep = dependency('sdbusplus')
+stdplus_dep = dependency('stdplus')
if cpp.has_header_symbol(
'nlohmann/json.hpp',
@@ -200,6 +201,7 @@
# ipmid
ipmid_pre = [
sdbusplus_dep,
+ stdplus_dep,
phosphor_logging_dep,
phosphor_dbus_interfaces_dep,
boost_coroutine,
diff --git a/transporthandler.cpp b/transporthandler.cpp
index 67084cb..1ffe911 100644
--- a/transporthandler.cpp
+++ b/transporthandler.cpp
@@ -1,11 +1,16 @@
#include "transporthandler.hpp"
+#include <stdplus/raw.hpp>
+
+#include <array>
+
using phosphor::logging::commit;
using phosphor::logging::elog;
using phosphor::logging::entry;
using phosphor::logging::level;
using phosphor::logging::log;
using sdbusplus::error::xyz::openbmc_project::common::InternalFailure;
+using sdbusplus::error::xyz::openbmc_project::common::InvalidArgument;
using sdbusplus::server::xyz::openbmc_project::network::EthernetInterface;
using sdbusplus::server::xyz::openbmc_project::network::IP;
using sdbusplus::server::xyz::openbmc_project::network::Neighbor;
@@ -656,6 +661,27 @@
return setStatus[channel] = SetStatus::Complete;
}
+/** @brief Unpacks the trivially copyable type from the message */
+template <typename T>
+static T unpackT(message::Payload& req)
+{
+ std::array<uint8_t, sizeof(T)> bytes;
+ if (req.unpack(bytes) != 0)
+ {
+ throw ccReqDataLenInvalid;
+ }
+ return stdplus::raw::copyFrom<T>(bytes);
+}
+
+/** @brief Ensure the message is fully unpacked */
+static void unpackFinal(message::Payload& req)
+{
+ if (!req.fullyUnpacked())
+ {
+ throw ccReqDataTruncated;
+ }
+}
+
/**
* Define placeholder command handlers for the OEM Extension bytes for the Set
* LAN Configuration Parameters and Get LAN Configuration Parameters
@@ -719,7 +745,7 @@
bool isValidMACAddress(const ether_addr& mac)
{
// check if mac address is empty
- if (equal(mac, ether_addr{}))
+ if (stdplus::raw::equal(mac, ether_addr{}))
{
return false;
}
@@ -755,8 +781,8 @@
static_cast<uint8_t>(EChannelMediumType::lan8032);
}
-RspType<> setLan(Context::ptr ctx, uint4_t channelBits, uint4_t reserved1,
- uint8_t parameter, message::Payload& req)
+RspType<> setLanInt(Context::ptr ctx, uint4_t channelBits, uint4_t reserved1,
+ uint8_t parameter, message::Payload& req)
{
const uint8_t channel = convertCurrentChannelNum(
static_cast<uint8_t>(channelBits), ctx->channel);
@@ -779,10 +805,11 @@
{
uint2_t flag;
uint6_t rsvd;
- if (req.unpack(flag, rsvd) != 0 || !req.fullyUnpacked())
+ if (req.unpack(flag, rsvd) != 0)
{
return responseReqDataLenInvalid();
}
+ unpackFinal(req);
if (rsvd)
{
return responseInvalidFieldRequest();
@@ -830,13 +857,8 @@
{
return responseCommandNotAvailable();
}
- in_addr ip;
- std::array<uint8_t, sizeof(ip)> bytes;
- if (req.unpack(bytes) != 0 || !req.fullyUnpacked())
- {
- return responseReqDataLenInvalid();
- }
- copyInto(ip, bytes);
+ auto ip = unpackT<in_addr>(req);
+ unpackFinal(req);
channelCall<reconfigureIfAddr4>(channel, ip, std::nullopt);
return responseSuccess();
}
@@ -844,10 +866,11 @@
{
uint4_t flag;
uint4_t rsvd;
- if (req.unpack(flag, rsvd) != 0 || !req.fullyUnpacked())
+ if (req.unpack(flag, rsvd) != 0)
{
return responseReqDataLenInvalid();
}
+ unpackFinal(req);
if (rsvd)
{
return responseInvalidFieldRequest();
@@ -873,14 +896,8 @@
}
case LanParam::MAC:
{
- ether_addr mac;
- std::array<uint8_t, sizeof(mac)> bytes;
- if (req.unpack(bytes) != 0 || !req.fullyUnpacked())
- {
- return responseReqDataLenInvalid();
- }
- copyInto(mac, bytes);
-
+ auto mac = unpackT<ether_addr>(req);
+ unpackFinal(req);
if (!isValidMACAddress(mac))
{
return responseInvalidFieldRequest();
@@ -894,13 +911,8 @@
{
return responseCommandNotAvailable();
}
- in_addr netmask;
- std::array<uint8_t, sizeof(netmask)> bytes;
- if (req.unpack(bytes) != 0 || !req.fullyUnpacked())
- {
- return responseReqDataLenInvalid();
- }
- copyInto(netmask, bytes);
+ auto netmask = unpackT<in_addr>(req);
+ unpackFinal(req);
uint8_t prefix = netmaskToPrefix(netmask);
if (prefix < MIN_IPV4_PREFIX_LENGTH)
{
@@ -915,41 +927,31 @@
{
return responseCommandNotAvailable();
}
- in_addr gateway;
- std::array<uint8_t, sizeof(gateway)> bytes;
- if (req.unpack(bytes) != 0 || !req.fullyUnpacked())
- {
- return responseReqDataLenInvalid();
- }
- copyInto(gateway, bytes);
+ auto gateway = unpackT<in_addr>(req);
+ unpackFinal(req);
channelCall<setGatewayProperty<AF_INET>>(channel, gateway);
return responseSuccess();
}
case LanParam::Gateway1MAC:
{
- ether_addr gatewayMAC;
- std::array<uint8_t, sizeof(gatewayMAC)> bytes;
- if (req.unpack(bytes) != 0 || !req.fullyUnpacked())
- {
- return responseReqDataLenInvalid();
- }
- copyInto(gatewayMAC, bytes);
+ auto gatewayMAC = unpackT<ether_addr>(req);
+ unpackFinal(req);
channelCall<reconfigureGatewayMAC<AF_INET>>(channel, gatewayMAC);
return responseSuccess();
}
case LanParam::VLANId:
{
- uint12_t vlanData = 0;
- uint3_t reserved = 0;
- bool vlanEnable = 0;
+ uint12_t vlanData;
+ uint3_t rsvd;
+ bool vlanEnable;
- if (req.unpack(vlanData) || req.unpack(reserved) ||
- req.unpack(vlanEnable) || !req.fullyUnpacked())
+ if (req.unpack(vlanData, rsvd, vlanEnable) != 0)
{
return responseReqDataLenInvalid();
}
+ unpackFinal(req);
- if (reserved)
+ if (rsvd)
{
return responseInvalidFieldRequest();
}
@@ -979,10 +981,11 @@
case LanParam::IPFamilyEnables:
{
uint8_t enables;
- if (req.unpack(enables) != 0 || !req.fullyUnpacked())
+ if (req.unpack(enables) != 0)
{
return responseReqDataLenInvalid();
}
+ unpackFinal(req);
switch (static_cast<IPFamilyEnables>(enables))
{
case IPFamilyEnables::DualStack:
@@ -1003,20 +1006,22 @@
uint8_t set;
uint7_t rsvd;
bool enabled;
- in6_addr ip;
- std::array<uint8_t, sizeof(ip)> ipbytes;
uint8_t prefix;
uint8_t status;
- if (req.unpack(set, rsvd, enabled, ipbytes, prefix, status) != 0 ||
- !req.fullyUnpacked())
+ if (req.unpack(set, rsvd, enabled) != 0)
{
return responseReqDataLenInvalid();
}
+ auto ip = unpackT<in6_addr>(req);
+ if (req.unpack(prefix, status) != 0)
+ {
+ return responseReqDataLenInvalid();
+ }
+ unpackFinal(req);
if (rsvd)
{
return responseInvalidFieldRequest();
}
- copyInto(ip, ipbytes);
if (enabled)
{
if (prefix < MIN_IPV6_PREFIX_LENGTH ||
@@ -1024,23 +1029,7 @@
{
return responseParmOutOfRange();
}
- try
- {
- channelCall<reconfigureIfAddr6>(channel, set, ip, prefix);
- }
- catch (const sdbusplus::exception_t& e)
- {
- if (std::string_view err{
- "xyz.openbmc_project.Common.Error.InvalidArgument"};
- err == e.name())
- {
- return responseInvalidFieldRequest();
- }
- else
- {
- throw;
- }
- }
+ channelCall<reconfigureIfAddr6>(channel, set, ip, prefix);
}
else
{
@@ -1057,10 +1046,11 @@
{
std::bitset<8> control;
constexpr uint8_t reservedRACCBits = 0xfc;
- if (req.unpack(control) != 0 || !req.fullyUnpacked())
+ if (req.unpack(control) != 0)
{
return responseReqDataLenInvalid();
}
+ unpackFinal(req);
if (std::bitset<8> expected(control &
std::bitset<8>(reservedRACCBits));
expected.any())
@@ -1075,35 +1065,26 @@
}
case LanParam::IPv6StaticRouter1IP:
{
- in6_addr gateway;
- std::array<uint8_t, sizeof(gateway)> bytes;
- if (req.unpack(bytes) != 0 || !req.fullyUnpacked())
- {
- return responseReqDataLenInvalid();
- }
- copyInto(gateway, bytes);
+ auto gateway = unpackT<in6_addr>(req);
+ unpackFinal(req);
channelCall<setGatewayProperty<AF_INET6>>(channel, gateway);
return responseSuccess();
}
case LanParam::IPv6StaticRouter1MAC:
{
- ether_addr mac;
- std::array<uint8_t, sizeof(mac)> bytes;
- if (req.unpack(bytes) != 0 || !req.fullyUnpacked())
- {
- return responseReqDataLenInvalid();
- }
- copyInto(mac, bytes);
+ auto mac = unpackT<ether_addr>(req);
+ unpackFinal(req);
channelCall<reconfigureGatewayMAC<AF_INET6>>(channel, mac);
return responseSuccess();
}
case LanParam::IPv6StaticRouter1PrefixLength:
{
uint8_t prefix;
- if (req.unpack(prefix) != 0 || !req.fullyUnpacked())
+ if (req.unpack(prefix) != 0)
{
return responseReqDataLenInvalid();
}
+ unpackFinal(req);
if (prefix != 0)
{
return responseInvalidFieldRequest();
@@ -1112,25 +1093,23 @@
}
case LanParam::IPv6StaticRouter1PrefixValue:
{
- std::array<uint8_t, sizeof(in6_addr)> bytes;
- if (req.unpack(bytes) != 0 || !req.fullyUnpacked())
- {
- return responseReqDataLenInvalid();
- }
+ unpackT<in6_addr>(req);
+ unpackFinal(req);
// Accept any prefix value since our prefix length has to be 0
return responseSuccess();
}
case LanParam::cipherSuitePrivilegeLevels:
{
- uint8_t reserved;
+ uint8_t rsvd;
std::array<uint4_t, ipmi::maxCSRecords> cipherSuitePrivs;
- if (req.unpack(reserved, cipherSuitePrivs) || !req.fullyUnpacked())
+ if (req.unpack(rsvd, cipherSuitePrivs))
{
return responseReqDataLenInvalid();
}
+ unpackFinal(req);
- if (reserved)
+ if (rsvd)
{
return responseInvalidFieldRequest();
}
@@ -1159,6 +1138,27 @@
return response(ccParamNotSupported);
}
+RspType<> setLan(Context::ptr ctx, uint4_t channelBits, uint4_t reserved1,
+ uint8_t parameter, message::Payload& req)
+{
+ try
+ {
+ return setLanInt(ctx, channelBits, reserved1, parameter, req);
+ }
+ catch (ipmi::Cc cc)
+ {
+ return response(cc);
+ }
+ catch (const sdbusplus::exception_t& e)
+ {
+ if (std::string_view{InvalidArgument::errName} == e.name())
+ {
+ return responseInvalidFieldRequest();
+ }
+ throw;
+ }
+}
+
RspType<message::Payload> getLan(Context::ptr ctx, uint4_t channelBits,
uint3_t reserved, bool revOnly,
uint8_t parameter, uint8_t set, uint8_t block)
@@ -1239,7 +1239,7 @@
{
addr = ifaddr->address;
}
- ret.pack(dataRef(addr));
+ ret.pack(stdplus::raw::asView<char>(addr));
return responseSuccess(std::move(ret));
}
case LanParam::IPSrc:
@@ -1253,7 +1253,7 @@
case LanParam::MAC:
{
ether_addr mac = channelCall<getMACProperty>(channel);
- ret.pack(dataRef(mac));
+ ret.pack(stdplus::raw::asView<char>(mac));
return responseSuccess(std::move(ret));
}
case LanParam::SubnetMask:
@@ -1265,7 +1265,7 @@
prefix = ifaddr->prefix;
}
in_addr netmask = prefixToNetmask(prefix);
- ret.pack(dataRef(netmask));
+ ret.pack(stdplus::raw::asView<char>(netmask));
return responseSuccess(std::move(ret));
}
case LanParam::Gateway1:
@@ -1273,7 +1273,7 @@
auto gateway =
channelCall<getGatewayProperty<AF_INET>>(channel).value_or(
in_addr{});
- ret.pack(dataRef(gateway));
+ ret.pack(stdplus::raw::asView<char>(gateway));
return responseSuccess(std::move(ret));
}
case LanParam::Gateway1MAC:
@@ -1284,7 +1284,7 @@
{
mac = neighbor->mac;
}
- ret.pack(dataRef(mac));
+ ret.pack(stdplus::raw::asView<char>(mac));
return responseSuccess(std::move(ret));
}
case LanParam::VLANId:
@@ -1389,7 +1389,7 @@
channelCall<getGatewayProperty<AF_INET6>>(channel).value_or(
in6_addr{});
}
- ret.pack(dataRef(gateway));
+ ret.pack(stdplus::raw::asView<char>(gateway));
return responseSuccess(std::move(ret));
}
case LanParam::IPv6StaticRouter1MAC:
@@ -1400,7 +1400,7 @@
{
mac = neighbor->mac;
}
- ret.pack(dataRef(mac));
+ ret.pack(stdplus::raw::asView<char>(mac));
return responseSuccess(std::move(ret));
}
case LanParam::IPv6StaticRouter1PrefixLength:
@@ -1411,7 +1411,7 @@
case LanParam::IPv6StaticRouter1PrefixValue:
{
in6_addr prefix{};
- ret.pack(dataRef(prefix));
+ ret.pack(stdplus::raw::asView<char>(prefix));
return responseSuccess(std::move(ret));
}
case LanParam::cipherSuitePrivilegeLevels:
diff --git a/transporthandler.hpp b/transporthandler.hpp
index 0cfaf01..db5df56 100644
--- a/transporthandler.hpp
+++ b/transporthandler.hpp
@@ -17,23 +17,21 @@
#include <phosphor-logging/log.hpp>
#include <sdbusplus/bus.hpp>
#include <sdbusplus/exception.hpp>
+#include <stdplus/raw.hpp>
#include <user_channel/channel_layer.hpp>
#include <xyz/openbmc_project/Common/error.hpp>
#include <xyz/openbmc_project/Network/EthernetInterface/server.hpp>
#include <xyz/openbmc_project/Network/IP/server.hpp>
#include <xyz/openbmc_project/Network/Neighbor/server.hpp>
-#include <array>
#include <bitset>
#include <cinttypes>
#include <cstdint>
-#include <cstring>
#include <fstream>
#include <functional>
#include <optional>
#include <string>
#include <string_view>
-#include <type_traits>
#include <unordered_map>
#include <unordered_set>
#include <utility>
@@ -183,44 +181,6 @@
std::string logicalPath;
};
-/** @brief A trivial helper used to determine if two PODs are equal
- *
- * @params[in] a - The first object to compare
- * @params[in] b - The second object to compare
- * @return True if the objects are the same bytewise
- */
-template <typename T>
-bool equal(const T& a, const T& b)
-{
- static_assert(std::is_trivially_copyable_v<T>);
- return std::memcmp(&a, &b, sizeof(T)) == 0;
-}
-
-/** @brief Copies bytes from an array into a trivially copyable container
- *
- * @params[out] t - The container receiving the data
- * @params[in] bytes - The data to copy
- */
-template <size_t N, typename T>
-void copyInto(T& t, const std::array<uint8_t, N>& bytes)
-{
- static_assert(std::is_trivially_copyable_v<T>);
- static_assert(N == sizeof(T));
- std::memcpy(&t, bytes.data(), bytes.size());
-}
-
-/** @brief Gets a generic view of the bytes in the input container
- *
- * @params[in] t - The data to reference
- * @return A string_view referencing the bytes in the container
- */
-template <typename T>
-std::string_view dataRef(const T& t)
-{
- static_assert(std::is_trivially_copyable_v<T>);
- return {reinterpret_cast<const char*>(&t), sizeof(T)};
-}
-
/** @brief Determines the ethernet interface name corresponding to a channel
* Tries to map a VLAN object first so that the address information
* is accurate. Otherwise it gets the standard ethernet interface.
@@ -586,7 +546,7 @@
{
continue;
}
- if (!equal(*neighIP, ip))
+ if (!stdplus::raw::equal(*neighIP, ip))
{
continue;
}