eth: Support looking up alternate interace channels
This makes it possible to use the GetEthDevice command to look up
interfaces other than the default NCSI interface.
Change-Id: I3a5563743a28b39adc753d8957f68f0bc330cf3a
Signed-off-by: William A. Kennington III <wak@google.com>
diff --git a/README.md b/README.md
index 853eebe..eb79fd5 100644
--- a/README.md
+++ b/README.md
@@ -51,11 +51,12 @@
**Per the above, if the version number doesn't fit in a byte it'll be cast to
size.**
-#### GetNcsiEthDevice - SubCommand 0x02
+#### GetEthDevice - SubCommand 0x02
The BMC itself must have hard-coded into the image, which ethernet device is
connected to the host NIC. This is true also in the mapping of ethernet device
-to channel number. The channel number is used to configure the ethernet device
+to channel number. Alternatively, you can pass a specific interface
+name for channel lookup. The channel number is used to configure the ethernet device
over IPMI, instead of the interface name. This is because we leverage the
current IPMI command set to read and write the networking configuration. Sys
can be programmed already to have this information in the board protobuf,
@@ -66,6 +67,7 @@
|Byte(s) |Value |Data
|--------|-------|----
|0x00|0x02|Subcommand
+|0x01... |if_name| (optional) The interface name, not null-terminated
Response
diff --git a/eth.cpp b/eth.cpp
index 127309a..0c85d28 100644
--- a/eth.cpp
+++ b/eth.cpp
@@ -40,8 +40,8 @@
#define MAX_IPMI_BUFFER 64
#endif
-ipmi_ret_t getEthDevice(const uint8_t*, uint8_t* replyBuf, size_t* dataLen,
- const HandlerInterface* handler)
+ipmi_ret_t getEthDevice(const uint8_t* reqBuf, uint8_t* replyBuf,
+ size_t* dataLen, const HandlerInterface* handler)
{
if ((*dataLen) < sizeof(struct EthDeviceRequest))
{
@@ -49,8 +49,11 @@
static_cast<uint32_t>(*dataLen));
return IPMI_CC_REQ_DATA_LEN_INVALID;
}
+ reqBuf += sizeof(struct EthDeviceRequest);
+ *dataLen -= sizeof(struct EthDeviceRequest);
- std::tuple<std::uint8_t, std::string> details = handler->getEthDetails();
+ std::tuple<std::uint8_t, std::string> details = handler->getEthDetails(
+ std::string(reinterpret_cast<const char*>(reqBuf), *dataLen));
std::string device = std::get<1>(details);
if (device.length() == 0)
diff --git a/handler.cpp b/handler.cpp
index a9f428e..d03632e 100644
--- a/handler.cpp
+++ b/handler.cpp
@@ -66,10 +66,14 @@
using InternalFailure =
sdbusplus::xyz::openbmc_project::Common::Error::InternalFailure;
-std::tuple<std::uint8_t, std::string> Handler::getEthDetails() const
+std::tuple<std::uint8_t, std::string>
+ Handler::getEthDetails(std::string intf) const
{
- return std::make_tuple<std::uint8_t, std::string>(
- ::ipmi::getChannelByName(NCSI_IF_NAME_STR), NCSI_IF_NAME_STR);
+ if (intf.empty())
+ {
+ intf = NCSI_IF_NAME_STR;
+ }
+ return std::make_tuple(::ipmi::getChannelByName(intf), std::move(intf));
}
std::int64_t Handler::getRxPackets(const std::string& name) const
diff --git a/handler.hpp b/handler.hpp
index f4b1bbd..fdb2585 100644
--- a/handler.hpp
+++ b/handler.hpp
@@ -24,7 +24,8 @@
*
* @return tuple of ethernet details (channel, if name).
*/
- virtual std::tuple<std::uint8_t, std::string> getEthDetails() const = 0;
+ virtual std::tuple<std::uint8_t, std::string>
+ getEthDetails(std::string intf) const = 0;
/**
* Return the value of rx_packets, given a if_name.
diff --git a/handler_impl.hpp b/handler_impl.hpp
index 807114e..4bd2e03 100644
--- a/handler_impl.hpp
+++ b/handler_impl.hpp
@@ -24,7 +24,8 @@
_configFile(entityConfigPath){};
~Handler() = default;
- std::tuple<std::uint8_t, std::string> getEthDetails() const override;
+ std::tuple<std::uint8_t, std::string>
+ getEthDetails(std::string intf) const override;
std::int64_t getRxPackets(const std::string& name) const override;
VersionTuple getCpldVersion(unsigned int id) const override;
void psuResetDelay(std::uint32_t delay) const override;
diff --git a/test/common.cpp b/test/common.cpp
index 08c3eaa..f8f635f 100644
--- a/test/common.cpp
+++ b/test/common.cpp
@@ -3,8 +3,8 @@
namespace ipmi
{
-std::uint8_t getChannelByName(const std::string&)
+std::uint8_t getChannelByName(const std::string& chName)
{
- return 1;
+ return chName.size() + 10;
}
} // namespace ipmi
diff --git a/test/eth_unittest.cpp b/test/eth_unittest.cpp
index 1053d4b..9807c91 100644
--- a/test/eth_unittest.cpp
+++ b/test/eth_unittest.cpp
@@ -27,10 +27,10 @@
size_t dataLen = request.size();
std::uint8_t reply[MAX_IPMI_BUFFER];
const std::uint8_t expectedAnswer[4] = {'e', 't', 'h', '0'};
- const std::uint8_t expectedChannel = 1;
+ const std::uint8_t expectedChannel = 14;
HandlerMock hMock;
- EXPECT_CALL(hMock, getEthDetails())
+ EXPECT_CALL(hMock, getEthDetails(""))
.WillOnce(Return(std::make_tuple(
expectedChannel,
std::string(expectedAnswer,
@@ -47,5 +47,30 @@
sizeof(expectedAnswer)));
}
+TEST(EthCommandTest, ValidPopulatedReturnsSuccess)
+{
+ std::vector<std::uint8_t> request = {SysOEMCommands::SysGetEthDevice, 'e'};
+ size_t dataLen = request.size();
+ std::uint8_t reply[MAX_IPMI_BUFFER];
+ const std::uint8_t expectedAnswer[1] = {'e'};
+ const std::uint8_t expectedChannel = 11;
+
+ HandlerMock hMock;
+ EXPECT_CALL(hMock, getEthDetails("e"))
+ .WillOnce(Return(std::make_tuple(
+ expectedChannel,
+ std::string(expectedAnswer,
+ expectedAnswer + sizeof(expectedAnswer)))));
+
+ EXPECT_EQ(IPMI_CC_OK,
+ getEthDevice(request.data(), &reply[0], &dataLen, &hMock));
+ struct EthDeviceReply check;
+ std::memcpy(&check, &reply[0], sizeof(check));
+ EXPECT_EQ(check.subcommand, SysOEMCommands::SysGetEthDevice);
+ EXPECT_EQ(check.channel, expectedChannel);
+ EXPECT_EQ(check.ifNameLength, sizeof(expectedAnswer));
+ EXPECT_EQ(0, std::memcmp(expectedAnswer, &reply[sizeof(check)],
+ sizeof(expectedAnswer)));
+}
} // namespace ipmi
} // namespace google
diff --git a/test/handler_mock.hpp b/test/handler_mock.hpp
index edd2aec..841d986 100644
--- a/test/handler_mock.hpp
+++ b/test/handler_mock.hpp
@@ -20,7 +20,8 @@
public:
~HandlerMock() = default;
- MOCK_CONST_METHOD0(getEthDetails, std::tuple<std::uint8_t, std::string>());
+ MOCK_CONST_METHOD1(getEthDetails,
+ std::tuple<std::uint8_t, std::string>(std::string));
MOCK_CONST_METHOD1(getRxPackets, std::int64_t(const std::string&));
MOCK_CONST_METHOD1(getCpldVersion,
std::tuple<std::uint8_t, std::uint8_t, std::uint8_t,
diff --git a/test/handler_unittest.cpp b/test/handler_unittest.cpp
index 99782fa..b93e877 100644
--- a/test/handler_unittest.cpp
+++ b/test/handler_unittest.cpp
@@ -16,12 +16,10 @@
TEST(HandlerTest, EthCheckValidHappy)
{
- // The code returns compiled-in information, and therefore cannot really
- // fail.
Handler h;
- std::tuple<std::uint8_t, std::string> result = h.getEthDetails();
- EXPECT_EQ(1, std::get<0>(result));
- EXPECT_STREQ("eth0", std::get<1>(result).c_str());
+ std::tuple<std::uint8_t, std::string> result = h.getEthDetails("et");
+ EXPECT_EQ(12, std::get<0>(result));
+ EXPECT_STREQ("et", std::get<1>(result).c_str());
}
TEST(HandlerTest, CableCheckIllegalPath)