Skip to content

network: handle io_uring factory safely #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
soulxu merged 10 commits into from
Sep 20, 2022
Merged

network: handle io_uring factory safely #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2f1df28
Add dimitry's implement and move to the core
soulxu Jul 6, 2022
46477a8
pass the io uring factory
soulxu Jul 6, 2022
28b9d79
not set non-blocking flag for iouring
soulxu Jul 14, 2022
5e8ce5c
test socket interface
soulxu Aug 31, 2022
779bcf6
Merge branch 'main' into new_iouring_version_2
soulxu Aug 31, 2022
80c0b7e
chore: move constants to headers
zhxie Sep 13, 2022
aa1ce45
network: handle io_uring factory safely
zhxie Sep 13, 2022
6f63b84
server: fix duplicate socket interface
zhxie Sep 13, 2022
fee498b
chore: remove unused class definitions
zhxie Sep 13, 2022
bac0d87
Merge remote-tracking branch 'soulxu/new_iouring_version_2' into io-u…
zhxie Sep 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions source/common/network/io_uring_socket_handle_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,10 @@
#include "envoy/network/io_handle.h"

#include "source/common/common/logger.h"
#include "source/common/io/io_uring.h"

namespace Envoy {

namespace Io {
class IoUringFactory;
} // namespace Io

namespace Network {

class IoUringSocketHandleImpl;
Expand Down
24 changes: 10 additions & 14 deletions source/common/network/socket_interface_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,6 @@
namespace Envoy {
namespace Network {

namespace {

// TODO (soulxu): making those configurable if needed.
constexpr uint32_t DefaultIoUringSize = 300;
constexpr uint32_t DefaultReadBufferSize = 8192;
constexpr bool UseSubmissionQueuePolling = false;

} // namespace

void DefaultSocketInterfaceExtension::onServerInitialized() {
if (io_uring_factory_ != nullptr) {
io_uring_factory_->onServerInitialized();
Expand Down Expand Up @@ -60,7 +51,7 @@ IoHandlePtr SocketInterfaceImpl::socket(Socket::Type socket_type, Address::Type
#else
int flags = SOCK_NONBLOCK;

if (io_uring_factory_ != nullptr) {
if (io_uring_factory_.lock() != nullptr) {
flags = 0;
}

Expand Down Expand Up @@ -99,7 +90,7 @@ IoHandlePtr SocketInterfaceImpl::socket(Socket::Type socket_type, Address::Type
RELEASE_ASSERT(SOCKET_VALID(result.return_value_),
fmt::format("socket(2) failed, got error: {}", errorDetails(result.errno_)));
IoHandlePtr io_handle =
makeSocket(result.return_value_, socket_v6only, domain, io_uring_factory_.get());
makeSocket(result.return_value_, socket_v6only, domain, io_uring_factory_.lock().get());

#if defined(__APPLE__) || defined(WIN32)
// Cannot set SOCK_NONBLOCK as a ::socket flag.
Expand Down Expand Up @@ -146,10 +137,15 @@ Server::BootstrapExtensionPtr SocketInterfaceImpl::createBootstrapExtension(
const Protobuf::Message&, Server::Configuration::ServerFactoryContext& context) {
// TODO (soulxu): Add runtime flag here.
if (Io::isIoUringSupported()) {
io_uring_factory_ = std::make_unique<Io::IoUringFactoryImpl>(
DefaultIoUringSize, UseSubmissionQueuePolling, context.threadLocal());
std::shared_ptr<Io::IoUringFactoryImpl> io_uring_factory =
std::make_unique<Io::IoUringFactoryImpl>(DefaultIoUringSize, UseSubmissionQueuePolling,
context.threadLocal());
io_uring_factory_ = io_uring_factory;

return std::make_unique<DefaultSocketInterfaceExtension>(*this, io_uring_factory);
} else {
return std::make_unique<DefaultSocketInterfaceExtension>(*this, nullptr);
}
return std::make_unique<DefaultSocketInterfaceExtension>(*this, io_uring_factory_);
}

ProtobufTypes::MessagePtr SocketInterfaceImpl::createEmptyConfigProto() {
Expand Down
11 changes: 8 additions & 3 deletions source/common/network/socket_interface_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ namespace Network {
class DefaultSocketInterfaceExtension : public Network::SocketInterfaceExtension {
public:
DefaultSocketInterfaceExtension(Network::SocketInterface& sock_interface,
std::unique_ptr<Io::IoUringFactory>& io_uring_factory)
std::shared_ptr<Io::IoUringFactory> io_uring_factory)
: Network::SocketInterfaceExtension(sock_interface), io_uring_factory_(io_uring_factory) {}

// Server::BootstrapExtension
void onServerInitialized() override;

protected:
std::unique_ptr<Io::IoUringFactory>& io_uring_factory_;
std::shared_ptr<Io::IoUringFactory> io_uring_factory_;
};

class SocketInterfaceImpl : public SocketInterfaceBase {
Expand All @@ -44,12 +44,17 @@ class SocketInterfaceImpl : public SocketInterfaceBase {
absl::optional<int> domain,
Io::IoUringFactory* io_uring_factory = nullptr);

// TODO (soulxu): making those configurable if needed.
static constexpr uint32_t DefaultIoUringSize = 300;
static constexpr uint32_t DefaultReadBufferSize = 8192;
static constexpr bool UseSubmissionQueuePolling = false;

protected:
virtual IoHandlePtr makeSocket(int socket_fd, bool socket_v6only, absl::optional<int> domain,
Io::IoUringFactory* io_uring_factory = nullptr) const;

private:
std::unique_ptr<Io::IoUringFactory> io_uring_factory_;
std::weak_ptr<Io::IoUringFactory> io_uring_factory_;
Copy link
Copy Markdown
Owner

@soulxu soulxu Sep 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it ok to use shared ptr here?

Copy link
Copy Markdown
Author

@zhxie zhxie Sep 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, which will cause io_uring_factory_ lives as long as SocketInterfaceImpl (static).

};

DECLARE_FACTORY(SocketInterfaceImpl);
Expand Down