http_client: fix for broken connection
http_client is not handling connection termination by server due to
keep alive timeout. At present client is not aware of connection
termination from server. So whenever next redfish is event ready to be
sent, the client will try send/receives data over broken connection.
After failed operation the client will try to restart the connection
by closing the current connection.
Problems:
1) Restart is not attempted on all failure paths.
Eg: stream_truncated error was ignored, which usually happens when try
to read from broken connection, due to which retry is never performed.
2) Ssl shutdown over broken connection often fails to call the shutdown
callback
3) ssl session was reused for new connection attempt. Which is wrong
Solution:
This patch will try to reattempt the connection in all failure cases.
It uses new socket object and new ssl session for the retries
Tested by:
Test normal event flow between redfish-event clients and the BMC
Test failure event flow between redfish-event clients and the BMC
Tested the bad path by keeping the setup idle for 3 hours on the above
two setups. Verified the events flow after this idle time
Change-Id: I3d725b9d77bea22e2e8860e01ee0dfc971789008
Signed-off-by: Abhilash Raju <abhilash.kollam@gmail.com>
Signed-off-by: Ed Tanous <ed@tanous.net>
diff --git a/http/http_client.hpp b/http/http_client.hpp
index 8c27d6d..4bae31d 100644
--- a/http/http_client.hpp
+++ b/http/http_client.hpp
@@ -148,6 +148,8 @@
// Ascync callables
std::function<void(bool, uint32_t, Response&)> callback;
+ boost::asio::io_context& ioc;
+
#ifdef BMCWEB_DBUS_DNS_RESOLVER
using Resolver = async_resolve::Resolver;
#else
@@ -379,6 +381,16 @@
unsigned int respCode = parser->get().result_int();
BMCWEB_LOG_DEBUG("recvMessage() Header Response Code: {}", respCode);
+ // Handle the case of stream_truncated. Some servers close the ssl
+ // connection uncleanly, so check to see if we got a full response
+ // before we handle this as an error.
+ if (!parser->is_done())
+ {
+ state = ConnState::recvFailed;
+ waitAndRetry();
+ return;
+ }
+
// Make sure the received response code is valid as defined by
// the associated retry policy
if (connPolicy->invalidResp(respCode))
@@ -487,7 +499,15 @@
}
// Let's close the connection and restart from resolve.
- doClose(true);
+ shutdownConn(true);
+ }
+
+ void restartConnection()
+ {
+ BMCWEB_LOG_DEBUG("{}, id: {} restartConnection", host,
+ std::to_string(connId));
+ initializeConnection(host.scheme() == "https");
+ doResolve();
}
void shutdownConn(bool retry)
@@ -511,7 +531,7 @@
{
// Now let's try to resend the data
state = ConnState::retry;
- doResolve();
+ restartConnection();
}
else
{
@@ -586,16 +606,10 @@
}
}
- public:
- explicit ConnectionInfo(
- boost::asio::io_context& iocIn, const std::string& idIn,
- const std::shared_ptr<ConnectionPolicy>& connPolicyIn,
- boost::urls::url_view hostIn, unsigned int connIdIn) :
- subId(idIn),
- connPolicy(connPolicyIn), host(hostIn), connId(connIdIn),
- resolver(iocIn), conn(iocIn), timer(iocIn)
+ void initializeConnection(bool ssl)
{
- if (host.scheme() == "https")
+ conn = boost::asio::ip::tcp::socket(ioc);
+ if (ssl)
{
std::optional<boost::asio::ssl::context> sslCtx =
ensuressl::getSSLClientContext();
@@ -618,6 +632,18 @@
setCipherSuiteTLSext();
}
}
+
+ public:
+ explicit ConnectionInfo(
+ boost::asio::io_context& iocIn, const std::string& idIn,
+ const std::shared_ptr<ConnectionPolicy>& connPolicyIn,
+ boost::urls::url_view hostIn, unsigned int connIdIn) :
+ subId(idIn),
+ connPolicy(connPolicyIn), host(hostIn), connId(connIdIn), ioc(iocIn),
+ resolver(iocIn), conn(iocIn), timer(iocIn)
+ {
+ initializeConnection(host.scheme() == "https");
+ }
};
class ConnectionPool : public std::enable_shared_from_this<ConnectionPool>