From a413e390563205476656a9005ca447f5b626872f Mon Sep 17 00:00:00 2001 From: Suryakanth Sekar Date: Wed, 6 Mar 2019 10:35:56 +0530 Subject: [PATCH 2/2] Sync GetSession Info cmd based on Upstream review Signed-off-by: Suryakanth Sekar --- comm_module.cpp | 12 +++---- command/session_cmds.cpp | 71 ++++++++++++++++++---------------------- sessions_manager.cpp | 10 +++--- sessions_manager.hpp | 2 +- socket_channel.hpp | 33 ++++++++++--------- 5 files changed, 59 insertions(+), 69 deletions(-) diff --git a/comm_module.cpp b/comm_module.cpp index 7a1a17d..2546583 100644 --- a/comm_module.cpp +++ b/comm_module.cpp @@ -54,13 +54,11 @@ void sessionSetupCommands() session::Privilege::CALLBACK, false}, // Session Info Command - { - { - (static_cast(message::PayloadType::IPMI) << 16) | - static_cast(command::NetFns::APP) | 0x3D - }, - &getSessionInfo, session::Privilege::USER, false - }, + {{(static_cast(message::PayloadType::IPMI) << 16) | + static_cast(command::NetFns::APP) | 0x3D}, + &getSessionInfo, + session::Privilege::USER, + false}, }; for (auto& iter : commands) diff --git a/command/session_cmds.cpp b/command/session_cmds.cpp index 4beeb6e..0c3a4ed 100644 --- a/command/session_cmds.cpp +++ b/command/session_cmds.cpp @@ -8,7 +8,7 @@ namespace command { -// Defined as per IPMI sepcification +// Defined as per IPMI specification static constexpr uint8_t searchCurrentSession = 0x00; static constexpr uint8_t searchSessionByHandle = 0xFE; static constexpr uint8_t searchSessionByID = 0xFF; @@ -111,20 +111,6 @@ std::vector getSessionInfo(const std::vector& inPayload, reinterpret_cast(outPayload.data()); uint32_t reqSessionID = handler.sessionID; response->completionCode = IPMI_CC_OK; - if (inPayload.size() == 1 && request->sessionIndex != 0) - { - if (request->sessionIndex <= session::MAX_SESSION_COUNT) - { - reqSessionID = std::get(singletonPool) - .getSessionIDbyHandle(request->sessionIndex); - } - else - { - response->completionCode = IPMI_CC_INVALID_FIELD_REQUEST; - outPayload.resize(sizeof(response->completionCode)); - return std::move(outPayload); - } - } // Here we look for session info according to session index parameter switch (request->sessionIndex) @@ -132,29 +118,22 @@ std::vector getSessionInfo(const std::vector& inPayload, // Look for current active session which this cmd is received over case searchCurrentSession: // Request data should only contain session index byte - if (inPayload.size() != 1) + if (inPayload.size() != sizeof(request->sessionIndex)) { response->completionCode = IPMI_CC_REQ_DATA_LEN_INVALID; outPayload.resize(sizeof(response->completionCode)); - return std::move(outPayload); - } - // To look for current active session which the command came over, - // the session ID cannot be 0. - if (0 == reqSessionID) - { - response->completionCode = IPMI_CC_INVALID_FIELD_REQUEST; - outPayload.resize(sizeof(response->completionCode)); - return std::move(outPayload); + return outPayload; } break; case searchSessionByHandle: // Request data should only contain session index byte and Session // handle - if (inPayload.size() != 2) + if (inPayload.size() != (sizeof(request->sessionIndex) + + sizeof(request->sessionHandle))) { response->completionCode = IPMI_CC_REQ_DATA_LEN_INVALID; outPayload.resize(sizeof(response->completionCode)); - return std::move(outPayload); + return outPayload; } // Retrieve session id based on session handle @@ -168,7 +147,7 @@ std::vector getSessionInfo(const std::vector& inPayload, { response->completionCode = IPMI_CC_INVALID_FIELD_REQUEST; outPayload.resize(sizeof(response->completionCode)); - return std::move(outPayload); + return outPayload; } break; case searchSessionByID: @@ -178,23 +157,38 @@ std::vector getSessionInfo(const std::vector& inPayload, { response->completionCode = IPMI_CC_REQ_DATA_LEN_INVALID; outPayload.resize(sizeof(response->completionCode)); - return std::move(outPayload); + return outPayload; } reqSessionID = endian::from_ipmi(request->sessionID); break; default: - if (inPayload.size() != 1) + if (inPayload.size() == sizeof(request->sessionIndex)) + { + if (request->sessionIndex <= session::MAX_SESSION_COUNT) + { + reqSessionID = + std::get(singletonPool) + .getSessionIDbyHandle(request->sessionIndex); + } + else + { + response->completionCode = IPMI_CC_REQ_DATA_LEN_INVALID; + outPayload.resize(sizeof(response->completionCode)); + return outPayload; + } + } + else { response->completionCode = IPMI_CC_REQ_DATA_LEN_INVALID; outPayload.resize(sizeof(response->completionCode)); - return std::move(outPayload); + return outPayload; } } response->totalSessionCount = session::MAX_SESSION_COUNT; response->activeSessioncount = - std::get(singletonPool).getNoOfActiveSession(); + std::get(singletonPool).getActiveSessionCount(); response->sessionHandle = 0; if (reqSessionID != 0) { @@ -207,9 +201,9 @@ std::vector getSessionInfo(const std::vector& inPayload, } catch (std::exception& e) { - response->completionCode = IPMI_CC_UNSPECIFIED_ERROR; + response->completionCode = IPMI_CC_REQ_DATA_LEN_INVALID; outPayload.resize(sizeof(response->completionCode)); - return std::move(outPayload); + return outPayload; } response->sessionHandle = std::get(singletonPool) .getSessionHandle(reqSessionID); @@ -218,25 +212,24 @@ std::vector getSessionInfo(const std::vector& inPayload, { response->completionCode = IPMI_CC_UNSPECIFIED_ERROR; outPayload.resize(sizeof(response->completionCode)); - return std::move(outPayload); + return outPayload; } response->userID = userId; // userId; response->privLevel = static_cast(sessionInfo->curPrivLevel); response->chanNum = sessionInfo->chNum; // byte7 3:0 response->ipmiVer = ipmi20VerSession; // byte7 7:4 - response->remoteIpAddr = - sessionInfo->channelPtr->getRemoteAddressInbytes(); response->remotePort = sessionInfo->channelPtr->getPort(); // remoteSessionPort; + response->remoteIpAddr = + sessionInfo->channelPtr->getRemoteAddressInBytes(); - std::cerr << "\nSessionInfo:" << (int)reqSessionID; // TODO: Filling the Remote MACAddress } else { outPayload.resize(4); } - return std::move(outPayload); + return outPayload; } } // namespace command diff --git a/sessions_manager.cpp b/sessions_manager.cpp index 9f3210b..c6897c6 100644 --- a/sessions_manager.cpp +++ b/sessions_manager.cpp @@ -152,15 +152,13 @@ std::shared_ptr Manager::getSession(SessionID sessionID, void Manager::cleanStaleEntries() { - uint8_t sessionIndex = 0; for (auto iter = sessionsMap.begin(); iter != sessionsMap.end();) { auto session = iter->second; if ((session->getBMCSessionID() != SESSION_ZERO) && !(session->isSessionActive())) { - sessionIndex = getSessionHandle(session->getBMCSessionID()); - sessionHandleMap[sessionIndex] = 0; + sessionHandleMap[getSessionHandle(session->getBMCSessionID())] = 0; iter = sessionsMap.erase(iter); } else @@ -172,8 +170,8 @@ void Manager::cleanStaleEntries() uint8_t Manager::storeSessionHandle(SessionID bmcSessionID) { - // Zero handler is reserved for invalid session. - //index starts with 1, for direct usage. Index 0 reserved + // Handler index 0 is reserved for invalid session. + // index starts with 1, for direct usage. Index 0 reserved for (uint8_t i = 1; i <= MAX_SESSION_COUNT; i++) { if (sessionHandleMap[i] == 0) @@ -206,7 +204,7 @@ uint8_t Manager::getSessionHandle(SessionID bmcSessionID) const } return 0; } -uint8_t Manager::getNoOfActiveSession() const +uint8_t Manager::getActiveSessionCount() const { uint8_t count = 0; for (const auto& it : sessionsMap) diff --git a/sessions_manager.hpp b/sessions_manager.hpp index f6ed1c3..3ff213e 100644 --- a/sessions_manager.hpp +++ b/sessions_manager.hpp @@ -82,7 +82,7 @@ class Manager std::shared_ptr getSession(SessionID sessionID, RetrieveOption option = RetrieveOption::BMC_SESSION_ID); - uint8_t getNoOfActiveSession() const; + uint8_t getActiveSessionCount() const; uint8_t getSessionHandle(SessionID bmcSessionID) const; uint8_t storeSessionHandle(SessionID bmcSessionID); uint32_t getSessionIDbyHandle(uint8_t sessionHandle) const; diff --git a/socket_channel.hpp b/socket_channel.hpp index 349701e..8b64740 100644 --- a/socket_channel.hpp +++ b/socket_channel.hpp @@ -52,33 +52,34 @@ class Channel } /** - * @brief Fetch the port number of the remote peer - * - * Returns the port number of the remote peer + * @brief Fetch the IP address of the remote peer * - * @return Port number + * Returns the IP address of the remote peer which is connected to this + * socket * + * @return IP address of the remote peer */ - auto getPort() const + std::uint32_t getRemoteAddressInBytes() const { - return endpoint.port(); + const boost::asio::ip::address& addr = endpoint.address(); + if (addr.is_v4()) + { + return addr.to_v4().to_uint(); + } + return 0; } /** - * @brief Return the binary representation of the remote IPv4 address + * @brief Fetch the port number of the remote peer * - * getSessionInfo needs to return the remote IPv4 addresses of each session + * Returns the port number of the remote peer + * + * @return Port number * - * @return A uint32_t representation of the remote IPv4 address */ - std::uint32_t getRemoteAddressInbytes() + auto getPort() const { - const boost::asio::ip::address& addr = endpoint.address(); - if (addr.is_v4()) - { - return addr.to_v4().to_uint(); - } - return 0; + return endpoint.port(); } /** -- 2.17.1