tools/net: Handle files which don't support sendfile
This adds a fallback read / write model.
Tested: ran against non-sendfile compatible file and it sent to the BMC successfully.
Change-Id: I6fd781ad19cd37376ca90743f799988e50ed460e
Signed-off-by: William A. Kennington III <wak@google.com>
Signed-off-by: Willy Tu <wltu@google.com>
diff --git a/internal/sys.cpp b/internal/sys.cpp
index b4a1ffc..9024666 100644
--- a/internal/sys.cpp
+++ b/internal/sys.cpp
@@ -104,6 +104,11 @@
return ::connect(sockfd, addr, addrlen);
}
+ssize_t SysImpl::send(int sockfd, const void* buf, size_t len, int flags) const
+{
+ return ::send(sockfd, buf, len, flags);
+}
+
ssize_t SysImpl::sendfile(int out_fd, int in_fd, off_t* offset,
size_t count) const
{
diff --git a/internal/sys.hpp b/internal/sys.hpp
index 7c56818..023d1d0 100644
--- a/internal/sys.hpp
+++ b/internal/sys.hpp
@@ -46,6 +46,8 @@
virtual int socket(int domain, int type, int protocol) const = 0;
virtual int connect(int sockfd, const struct sockaddr* addr,
socklen_t addrlen) const = 0;
+ virtual ssize_t send(int sockfd, const void* buf, size_t len,
+ int flags) const = 0;
virtual ssize_t sendfile(int out_fd, int in_fd, off_t* offset,
size_t count) const = 0;
virtual int getaddrinfo(const char* node, const char* service,
@@ -79,6 +81,8 @@
int socket(int domain, int type, int protocol) const override;
int connect(int sockfd, const struct sockaddr* addr,
socklen_t addrlen) const override;
+ ssize_t send(int sockfd, const void* buf, size_t len,
+ int flags) const override;
ssize_t sendfile(int out_fd, int in_fd, off_t* offset,
size_t count) const override;
int getaddrinfo(const char* node, const char* service,
diff --git a/tools/net.cpp b/tools/net.cpp
index 899824d..8b0663e 100644
--- a/tools/net.cpp
+++ b/tools/net.cpp
@@ -28,12 +28,11 @@
#include <ipmiblob/blob_errors.hpp>
#include <stdplus/handle/managed.hpp>
+#include <stdplus/util/cexec.hpp>
#include <cstdint>
#include <cstring>
-#include <memory>
#include <optional>
-#include <string>
#include <vector>
namespace
@@ -110,33 +109,58 @@
off_t offset = 0;
progress->start(fileSize);
+ auto confirmSend = [&]() {
+ if (bytesSent == 0)
+ {
+ return;
+ }
+ struct ipmi_flash::ExtChunkHdr chunk;
+ chunk.length = bytesSent;
+ std::vector<uint8_t> chunkBytes(sizeof(chunk));
+ std::memcpy(chunkBytes.data(), &chunk, sizeof(chunk));
+ /* This doesn't return anything on success. */
+ blob->writeBytes(session, offset - bytesSent, chunkBytes);
+ progress->updateProgress(bytesSent);
+ };
do
{
bytesSent = sys->sendfile(*connFd, *inputFd, &offset, blockSize);
if (bytesSent < 0)
{
- std::fprintf(stderr, "Failed to send data to BMC: %s\n",
- strerror(errno));
- progress->abort();
- return false;
+ // Not all input files support sendfile, fall back to a simple
+ // buffered mechanism if unsupported.
+ if (errno != EINVAL)
+ {
+ CHECK_ERRNO(-1, "Sending data to BMC");
+ }
+ std::array<uint8_t, 4096> buf;
+ size_t left = 0;
+ do
+ {
+ left += CHECK_ERRNO(sys->read(*inputFd, buf.data() + left,
+ buf.size() - left),
+ "Reading data for BMC");
+ bytesSent =
+ CHECK_ERRNO(sys->send(*connFd, buf.data(), left, 0),
+ "Sending data to BMC");
+ std::memmove(buf.data(), buf.data() + bytesSent,
+ left - bytesSent);
+ left -= bytesSent;
+ offset += bytesSent;
+ confirmSend();
+ } while (bytesSent > 0);
+ break;
}
- else if (bytesSent > 0)
- {
- /* Ok, so the data is staged, now send the blob write with
- * the details.
- */
- struct ipmi_flash::ExtChunkHdr chunk;
- chunk.length = bytesSent;
- std::vector<std::uint8_t> chunkBytes(sizeof(chunk));
- std::memcpy(chunkBytes.data(), &chunk, sizeof(chunk));
-
- /* This doesn't return anything on success. */
- blob->writeBytes(session, offset - bytesSent, chunkBytes);
- progress->updateProgress(bytesSent);
- }
+ confirmSend();
} while (bytesSent > 0);
}
+ catch (const std::system_error& e)
+ {
+ std::fprintf(stderr, "%s\n", e.what());
+ progress->abort();
+ return false;
+ }
catch (const ipmiblob::BlobException& b)
{
progress->abort();
diff --git a/tools/test/internal_sys_mock.hpp b/tools/test/internal_sys_mock.hpp
index fcd68bb..c45412d 100644
--- a/tools/test/internal_sys_mock.hpp
+++ b/tools/test/internal_sys_mock.hpp
@@ -31,6 +31,8 @@
MOCK_METHOD(int, socket, (int, int, int), (const override));
MOCK_METHOD(int, connect, (int, const struct sockaddr*, socklen_t),
(const override));
+ MOCK_METHOD(ssize_t, send, (int, const void*, size_t, int),
+ (const override));
MOCK_METHOD(ssize_t, sendfile, (int, int, off_t*, size_t),
(const override));
MOCK_METHOD(int, getaddrinfo,
diff --git a/tools/test/tools_net_unittest.cpp b/tools/test/tools_net_unittest.cpp
index f741edb..b291e1c 100644
--- a/tools/test/tools_net_unittest.cpp
+++ b/tools/test/tools_net_unittest.cpp
@@ -21,6 +21,7 @@
using ::testing::ContainerEq;
using ::testing::DoAll;
using ::testing::Field;
+using ::testing::Ge;
using ::testing::Gt;
using ::testing::InSequence;
using ::testing::NotNull;
@@ -216,5 +217,55 @@
EXPECT_TRUE(handler.sendContents(filePath, session));
}
+TEST_F(NetHandleTest, successFallback)
+{
+ expectOpenFile();
+ expectAddrInfo();
+ expectConnection();
+
+ struct ipmi_flash::ExtChunkHdr chunk;
+ chunk.length = chunkSize;
+ std::vector<std::uint8_t> chunkBytes(sizeof(chunk));
+ std::memcpy(chunkBytes.data(), &chunk, sizeof(chunk));
+
+ {
+ InSequence seq;
+ EXPECT_CALL(sysMock, sendfile(connFd, inFd, _, _))
+ .WillOnce([](int, int, off_t*, size_t) {
+ errno = EINVAL;
+ return -1;
+ });
+
+ std::vector<uint8_t> chunk(chunkSize);
+ for (std::uint32_t offset = 0; offset < fakeFileSize;
+ offset += chunkSize)
+ {
+ for (size_t i = 0; i < chunkSize; ++i)
+ {
+ chunk[i] = i + offset;
+ }
+ EXPECT_CALL(sysMock, read(inFd, _, Ge(chunkSize)))
+ .WillOnce([chunk](int, void* buf, size_t) {
+ memcpy(buf, chunk.data(), chunkSize);
+ return chunkSize;
+ });
+ EXPECT_CALL(sysMock, send(connFd, _, chunkSize, 0))
+ .WillOnce([chunk](int, const void* data, size_t len, int) {
+ std::vector<uint8_t> dcopy(len);
+ memcpy(dcopy.data(), data, len);
+ EXPECT_THAT(dcopy, ContainerEq(chunk));
+ return chunkSize;
+ });
+ EXPECT_CALL(blobMock,
+ writeBytes(session, offset, ContainerEq(chunkBytes)));
+ EXPECT_CALL(progMock, updateProgress(chunkSize));
+ }
+ EXPECT_CALL(sysMock, read(inFd, _, Ge(chunkSize))).WillOnce(Return(0));
+ EXPECT_CALL(sysMock, send(connFd, _, 0, 0)).WillOnce(Return(0));
+ }
+
+ EXPECT_TRUE(handler.sendContents(filePath, session));
+}
+
} // namespace
} // namespace host_tool