Skip to content
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

socket options: add support for directly creating ipv4/ipv6 pairs #18769

Merged
merged 7 commits into from
Oct 28, 2021
+32 −19
Merged

socket options: add support for directly creating ipv4/ipv6 pairs #18769

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a0624a3
socket option: add support for directly creating ipv4/ipv6 pairs
goaway Oct 26, 2021
b296c21
fix format
goaway Oct 26, 2021
879582e
update comment
goaway Oct 27, 2021
72213b1
fix format
goaway Oct 27, 2021
68b4ab8
Merge branch 'main' into ms/socket-option-cleanup
goaway Oct 27, 2021
4606ede
try CI again
goaway Oct 27, 2021
e3ef40f
fix mock
goaway Oct 27, 2021
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
8 changes: 8 additions & 0 deletions envoy/network/socket.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,8 +299,16 @@ class Socket {
virtual absl::optional<Details>
getOptionDetails(const Socket& socket,
envoy::config::core::v3::SocketOption::SocketState state) const PURE;

/**
* Implementations should typically return true. Unsupported or placeholder implementations
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe phrase this as Whether the socket option is supported or Whether the socket option can be applied?

I think I'd normally see the comment begin with describing what the function is supposed to do, then fill in with implementation guidelines if appropriate

* may indicate such by returning false.
* @return Whether this is an actual socket option.
*/
virtual bool isSupported() const PURE;
Copy link
Contributor

Choose a reason for hiding this comment

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

What calls this at this layer? Is this something we'll be calling via EM?

Copy link
Contributor Author

@goaway goaway Oct 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

It's called by AddrFamilyAwareSocketOptionImpl. EM itself won't be calling it, but I needed to move it to the parent class (it was actually already replicated across a couple subclasses).

};

using OptionConstPtr = std::unique_ptr<const Option>;
using OptionConstSharedPtr = std::shared_ptr<const Option>;
using Options = std::vector<OptionConstSharedPtr>;
using OptionsSharedPtr = std::shared_ptr<Options>;
Expand Down
20 changes: 10 additions & 10 deletions source/common/network/addr_family_aware_socket_option_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#include "source/common/network/addr_family_aware_socket_option_impl.h"

#include "envoy/common/exception.h"
#include "envoy/common/platform.h"
#include "envoy/common/optref.h"
#include "envoy/config/core/v3/base.pb.h"

#include "source/common/api/os_sys_calls_impl.h"
Expand All @@ -14,11 +14,12 @@ namespace Network {

namespace {

SocketOptionImplOptRef getOptionForSocket(const Socket& socket, SocketOptionImpl& ipv4_option,
SocketOptionImpl& ipv6_option) {
OptRef<const Socket::Option> getOptionForSocket(const Socket& socket,
const Socket::Option& ipv4_option,
const Socket::Option& ipv6_option) {
auto version = socket.ipVersion();
if (!version.has_value()) {
return absl::nullopt;
return {};
}

// If the FD is v4, we can only try the IPv4 variant.
Expand All @@ -38,7 +39,7 @@ SocketOptionImplOptRef getOptionForSocket(const Socket& socket, SocketOptionImpl

bool AddrFamilyAwareSocketOptionImpl::setOption(
Socket& socket, envoy::config::core::v3::SocketOption::SocketState state) const {
return setIpSocketOption(socket, state, ipv4_option_, ipv6_option_);
return setIpSocketOption(socket, state, *ipv4_option_, *ipv6_option_);
}

absl::optional<Socket::Option::Details> AddrFamilyAwareSocketOptionImpl::getOptionDetails(
Expand All @@ -49,21 +50,20 @@ absl::optional<Socket::Option::Details> AddrFamilyAwareSocketOptionImpl::getOpti
return absl::nullopt;
}

return option->get().getOptionDetails(socket, state);
return option.value().get().getOptionDetails(socket, state);
}

bool AddrFamilyAwareSocketOptionImpl::setIpSocketOption(
Socket& socket, envoy::config::core::v3::SocketOption::SocketState state,
const std::unique_ptr<SocketOptionImpl>& ipv4_option,
const std::unique_ptr<SocketOptionImpl>& ipv6_option) {
auto option = getOptionForSocket(socket, *ipv4_option, *ipv6_option);
const Socket::Option& ipv4_option, const Socket::Option& ipv6_option) {
auto option = getOptionForSocket(socket, ipv4_option, ipv6_option);

if (!option.has_value()) {
ENVOY_LOG(warn, "Failed to set IP socket option on non-IP socket");
return false;
}

return option->get().setOption(socket, state);
return option.value().get().setOption(socket, state);
}

} // namespace Network
Expand Down
12 changes: 8 additions & 4 deletions source/common/network/addr_family_aware_socket_option_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ class AddrFamilyAwareSocketOptionImpl : public Socket::Option,
SocketOptionName ipv6_optname, absl::string_view ipv6_value)
: ipv4_option_(std::make_unique<SocketOptionImpl>(in_state, ipv4_optname, ipv4_value)),
ipv6_option_(std::make_unique<SocketOptionImpl>(in_state, ipv6_optname, ipv6_value)) {}
AddrFamilyAwareSocketOptionImpl(Socket::OptionConstPtr&& ipv4_option,
Socket::OptionConstPtr&& ipv6_option)
: ipv4_option_(std::move(ipv4_option)), ipv6_option_(std::move(ipv6_option)) {}

// Socket::Option
bool setOption(Socket& socket,
Expand All @@ -41,6 +44,7 @@ class AddrFamilyAwareSocketOptionImpl : public Socket::Option,
absl::optional<Details>
getOptionDetails(const Socket& socket,
envoy::config::core::v3::SocketOption::SocketState state) const override;
bool isSupported() const override { return true; }

/**
* Set a socket option that applies at both IPv4 and IPv6 socket levels. When the underlying FD
Expand All @@ -59,12 +63,12 @@ class AddrFamilyAwareSocketOptionImpl : public Socket::Option,
*/
static bool setIpSocketOption(Socket& socket,
envoy::config::core::v3::SocketOption::SocketState state,
const std::unique_ptr<SocketOptionImpl>& ipv4_option,
const std::unique_ptr<SocketOptionImpl>& ipv6_option);
const Socket::Option& ipv4_option,
const Socket::Option& ipv6_option);

private:
const std::unique_ptr<SocketOptionImpl> ipv4_option_;
const std::unique_ptr<SocketOptionImpl> ipv6_option_;
const Socket::OptionConstPtr ipv4_option_;
const Socket::OptionConstPtr ipv6_option_;
};

} // namespace Network
Expand Down
5 changes: 1 addition & 4 deletions source/common/network/socket_option_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,7 @@ class SocketOptionImpl : public Socket::Option, Logger::Loggable<Logger::Id::con
absl::optional<Details>
getOptionDetails(const Socket& socket,
envoy::config::core::v3::SocketOption::SocketState state) const override;

bool isSupported() const;
bool isSupported() const override;

/**
* Set the option on the given socket.
Expand All @@ -162,7 +161,5 @@ class SocketOptionImpl : public Socket::Option, Logger::Loggable<Logger::Id::con
const std::vector<uint8_t> value_;
};

using SocketOptionImplOptRef = absl::optional<std::reference_wrapper<SocketOptionImpl>>;

} // namespace Network
} // namespace Envoy
2 changes: 1 addition & 1 deletion source/common/network/win32_redirect_records_option_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ class Win32RedirectRecordsOptionImpl : public Socket::Option,
absl::optional<Details>
getOptionDetails(const Socket& socket,
envoy::config::core::v3::SocketOption::SocketState) const override;
bool isSupported() const override;

bool isSupported() const;
static const Network::SocketOptionName& optionName();

private:
Expand Down
1 change: 1 addition & 0 deletions source/extensions/filters/common/original_src/original_src_socket_option.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class OriginalSrcSocketOption : public Network::Socket::Option {
absl::optional<Details>
getOptionDetails(const Network::Socket& socket,
envoy::config::core::v3::SocketOption::SocketState state) const override;
bool isSupported() const override { return true; }

private:
Network::Address::InstanceConstSharedPtr src_address_;
Expand Down