From 20cc6e7414bc8a7ea5c4b1166ea8376a4dd0bb11 Mon Sep 17 00:00:00 2001 From: Vignesh Babu Date: Mon, 27 Sep 2021 10:42:16 -0700 Subject: [PATCH] Revert "pass subchannel address separately instead of as a channel arg (#27443)" (#27489) This reverts commit 57982f0ec6e77e7bd077ccad94b97122a2bd6ace. --- .../filters/client_channel/client_channel.cc | 5 +- .../client_channel/client_channel_factory.h | 36 ++--- .../ext/filters/client_channel/connector.h | 36 ++--- .../client_channel/global_subchannel_pool.h | 1 + .../ext/filters/client_channel/subchannel.cc | 134 ++++++++++++------ .../ext/filters/client_channel/subchannel.h | 65 ++++++--- .../subchannel_pool_interface.cc | 19 +-- .../subchannel_pool_interface.h | 10 +- .../chttp2/client/chttp2_connector.cc | 7 +- .../chttp2/client/insecure/channel_create.cc | 37 ++--- .../client/secure/secure_channel_create.cc | 37 ++--- src/core/lib/address_utils/parse_address.cc | 2 - test/core/end2end/goaway_server_test.cc | 1 - test/cpp/microbenchmarks/bm_call_create.cc | 1 - 14 files changed, 227 insertions(+), 164 deletions(-) diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index 4ffd76a5d00..c12a1d98128 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -954,6 +954,7 @@ class ClientChannel::ClientChannelControlHelper }; // Add channel args needed for the subchannel. absl::InlinedVector args_to_add = { + Subchannel::CreateSubchannelAddressArg(&address.address()), SubchannelPoolInterface::CreateChannelArg( chand_->subchannel_pool_.get()), }; @@ -965,10 +966,10 @@ class ClientChannel::ClientChannelControlHelper grpc_channel_args* new_args = grpc_channel_args_copy_and_add_and_remove( &args, args_to_remove, GPR_ARRAY_SIZE(args_to_remove), args_to_add.data(), args_to_add.size()); + gpr_free(args_to_add[0].value.string); // Create subchannel. RefCountedPtr subchannel = - chand_->client_channel_factory_->CreateSubchannel(address.address(), - new_args); + chand_->client_channel_factory_->CreateSubchannel(new_args); grpc_channel_args_destroy(new_args); if (subchannel == nullptr) return nullptr; // Make sure the subchannel has updated keepalive time. diff --git a/src/core/ext/filters/client_channel/client_channel_factory.h b/src/core/ext/filters/client_channel/client_channel_factory.h index 0e00edc5afe..75d74d676b2 100644 --- a/src/core/ext/filters/client_channel/client_channel_factory.h +++ b/src/core/ext/filters/client_channel/client_channel_factory.h @@ -1,18 +1,20 @@ -// -// Copyright 2015 gRPC authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// +/* + * + * Copyright 2015 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ #ifndef GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_CLIENT_CHANNEL_FACTORY_H #define GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_CLIENT_CHANNEL_FACTORY_H @@ -31,7 +33,7 @@ class ClientChannelFactory { // Creates a subchannel with the specified args. virtual RefCountedPtr CreateSubchannel( - const grpc_resolved_address& address, const grpc_channel_args* args) = 0; + const grpc_channel_args* args) = 0; // Returns a channel arg containing the specified factory. static grpc_arg CreateChannelArg(ClientChannelFactory* factory); @@ -43,4 +45,4 @@ class ClientChannelFactory { } // namespace grpc_core -#endif // GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_CLIENT_CHANNEL_FACTORY_H +#endif /* GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_CLIENT_CHANNEL_FACTORY_H */ diff --git a/src/core/ext/filters/client_channel/connector.h b/src/core/ext/filters/client_channel/connector.h index 3e193be4cd2..20e25b5bc19 100644 --- a/src/core/ext/filters/client_channel/connector.h +++ b/src/core/ext/filters/client_channel/connector.h @@ -1,18 +1,20 @@ -// -// Copyright 2015 gRPC authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// +/* + * + * Copyright 2015 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ #ifndef GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_CONNECTOR_H #define GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_CONNECTOR_H @@ -33,8 +35,6 @@ namespace grpc_core { class SubchannelConnector : public InternallyRefCounted { public: struct Args { - // Address to connect to. - grpc_resolved_address* address; // Set of pollsets interested in this connection. grpc_pollset_set* interested_parties; // Deadline for connection. @@ -76,4 +76,4 @@ class SubchannelConnector : public InternallyRefCounted { } // namespace grpc_core -#endif // GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_CONNECTOR_H +#endif /* GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_CONNECTOR_H */ diff --git a/src/core/ext/filters/client_channel/global_subchannel_pool.h b/src/core/ext/filters/client_channel/global_subchannel_pool.h index 5b29a89a39d..7ff9b070023 100644 --- a/src/core/ext/filters/client_channel/global_subchannel_pool.h +++ b/src/core/ext/filters/client_channel/global_subchannel_pool.h @@ -32,6 +32,7 @@ namespace grpc_core { // should be only one instance of this class. Init() should be called once at // the filter initialization time; Shutdown() should be called once at the // filter shutdown time. +// TODO(juanlishen): Enable subchannel retention. class GlobalSubchannelPool final : public SubchannelPoolInterface { public: // The ctor and dtor are not intended to use directly. diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc index 1406b751835..855d1a32eda 100644 --- a/src/core/ext/filters/client_channel/subchannel.cc +++ b/src/core/ext/filters/client_channel/subchannel.cc @@ -1,18 +1,20 @@ -// -// Copyright 2015 gRPC authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// +/* + * + * Copyright 2015 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ #include @@ -653,39 +655,42 @@ Subchannel::Subchannel(SubchannelKey key, GRPC_TRACE_FLAG_ENABLED(grpc_trace_subchannel_refcount) ? "Subchannel" : nullptr), key_(std::move(key)), - pollset_set_(grpc_pollset_set_create()), connector_(std::move(connector)), backoff_(ParseArgsForBackoffValues(args, &min_connect_timeout_ms_)) { GRPC_STATS_INC_CLIENT_SUBCHANNELS_CREATED(); - GRPC_CLOSURE_INIT(&on_connecting_finished_, OnConnectingFinished, this, - grpc_schedule_on_exec_ctx); - // Check proxy mapper to determine address to connect to and channel - // args to use. - address_for_connect_ = key_.address(); + pollset_set_ = grpc_pollset_set_create(); + grpc_resolved_address* addr = + static_cast(gpr_malloc(sizeof(*addr))); + GetAddressFromSubchannelAddressArg(args, addr); grpc_resolved_address* new_address = nullptr; grpc_channel_args* new_args = nullptr; - if (ProxyMapperRegistry::MapAddress(address_for_connect_, args, &new_address, - &new_args)) { + if (ProxyMapperRegistry::MapAddress(*addr, args, &new_address, &new_args)) { GPR_ASSERT(new_address != nullptr); - address_for_connect_ = *new_address; - gpr_free(new_address); + gpr_free(addr); + addr = new_address; } - if (new_args != nullptr) { - args_ = new_args; - } else { - args_ = grpc_channel_args_copy(args); - } - // Initialize channelz. - const bool channelz_enabled = grpc_channel_args_find_bool( - args_, GRPC_ARG_ENABLE_CHANNELZ, GRPC_ENABLE_CHANNELZ_DEFAULT); + static const char* keys_to_remove[] = {GRPC_ARG_SUBCHANNEL_ADDRESS}; + grpc_arg new_arg = CreateSubchannelAddressArg(addr); + gpr_free(addr); + args_ = grpc_channel_args_copy_and_add_and_remove( + new_args != nullptr ? new_args : args, keys_to_remove, + GPR_ARRAY_SIZE(keys_to_remove), &new_arg, 1); + gpr_free(new_arg.value.string); + if (new_args != nullptr) grpc_channel_args_destroy(new_args); + GRPC_CLOSURE_INIT(&on_connecting_finished_, OnConnectingFinished, this, + grpc_schedule_on_exec_ctx); + const grpc_arg* arg = grpc_channel_args_find(args_, GRPC_ARG_ENABLE_CHANNELZ); + const bool channelz_enabled = + grpc_channel_arg_get_bool(arg, GRPC_ENABLE_CHANNELZ_DEFAULT); + arg = grpc_channel_args_find( + args_, GRPC_ARG_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE); + const grpc_integer_options options = { + GRPC_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE_DEFAULT, 0, INT_MAX}; + size_t channel_tracer_max_memory = + static_cast(grpc_channel_arg_get_integer(arg, options)); if (channelz_enabled) { - const size_t channel_tracer_max_memory = - static_cast(grpc_channel_args_find_integer( - args_, GRPC_ARG_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE, - {GRPC_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE_DEFAULT, 0, - INT_MAX})); channelz_node_ = MakeRefCounted( - grpc_sockaddr_to_uri(&key_.address()), channel_tracer_max_memory); + GetTargetAddress(), channel_tracer_max_memory); channelz_node_->AddTraceEvent( channelz::ChannelTrace::Severity::Info, grpc_slice_from_static_string("subchannel created")); @@ -706,8 +711,8 @@ Subchannel::~Subchannel() { RefCountedPtr Subchannel::Create( OrphanablePtr connector, - const grpc_resolved_address& address, const grpc_channel_args* args) { - SubchannelKey key(address, args); + const grpc_channel_args* args) { + SubchannelKey key(args); SubchannelPoolInterface* subchannel_pool = SubchannelPoolInterface::GetSubchannelPoolFromChannelArgs(args); GPR_ASSERT(subchannel_pool != nullptr); @@ -745,6 +750,14 @@ void Subchannel::ThrottleKeepaliveTime(int new_keepalive_time) { } } +const char* Subchannel::GetTargetAddress() { + const grpc_arg* addr_arg = + grpc_channel_args_find(args_, GRPC_ARG_SUBCHANNEL_ADDRESS); + const char* addr_str = grpc_channel_arg_get_string(addr_arg); + GPR_ASSERT(addr_str != nullptr); // Should have been set by LB policy. + return addr_str; +} + channelz::SubchannelNode* Subchannel::channelz_node() { return channelz_node_.get(); } @@ -835,6 +848,44 @@ void Subchannel::Orphan() { health_watcher_map_.ShutdownLocked(); } +grpc_arg Subchannel::CreateSubchannelAddressArg( + const grpc_resolved_address* addr) { + return grpc_channel_arg_string_create( + const_cast(GRPC_ARG_SUBCHANNEL_ADDRESS), + gpr_strdup(addr->len > 0 ? grpc_sockaddr_to_uri(addr).c_str() : "")); +} + +const char* Subchannel::GetUriFromSubchannelAddressArg( + const grpc_channel_args* args) { + const grpc_arg* addr_arg = + grpc_channel_args_find(args, GRPC_ARG_SUBCHANNEL_ADDRESS); + const char* addr_str = grpc_channel_arg_get_string(addr_arg); + GPR_ASSERT(addr_str != nullptr); // Should have been set by LB policy. + return addr_str; +} + +namespace { + +void UriToSockaddr(const char* uri_str, grpc_resolved_address* addr) { + absl::StatusOr uri = URI::Parse(uri_str); + if (!uri.ok()) { + gpr_log(GPR_ERROR, "%s", uri.status().ToString().c_str()); + GPR_ASSERT(uri.ok()); + } + if (!grpc_parse_uri(*uri, addr)) memset(addr, 0, sizeof(*addr)); +} + +} // namespace + +void Subchannel::GetAddressFromSubchannelAddressArg( + const grpc_channel_args* args, grpc_resolved_address* addr) { + const char* addr_uri_str = GetUriFromSubchannelAddressArg(args); + memset(addr, 0, sizeof(*addr)); + if (*addr_uri_str != '\0') { + UriToSockaddr(addr_uri_str, addr); + } +} + namespace { // Returns a string indicating the subchannel's connectivity state change to @@ -937,7 +988,6 @@ void Subchannel::OnRetryAlarm(void* arg, grpc_error_handle error) { void Subchannel::ContinueConnectingLocked() { SubchannelConnector::Args args; - args.address = &address_for_connect_; args.interested_parties = pollset_set_; const grpc_millis min_deadline = min_connect_timeout_ms_ + ExecCtx::Get()->Now(); diff --git a/src/core/ext/filters/client_channel/subchannel.h b/src/core/ext/filters/client_channel/subchannel.h index 60b8261e27e..9667fe7fd72 100644 --- a/src/core/ext/filters/client_channel/subchannel.h +++ b/src/core/ext/filters/client_channel/subchannel.h @@ -1,18 +1,20 @@ -// -// Copyright 2015 gRPC authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// +/* + * + * Copyright 2015 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ #ifndef GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_SUBCHANNEL_H #define GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_SUBCHANNEL_H @@ -37,6 +39,9 @@ #include "src/core/lib/transport/connectivity_state.h" #include "src/core/lib/transport/metadata.h" +// Channel arg containing a URI indicating the address to connect to. +#define GRPC_ARG_SUBCHANNEL_ADDRESS "grpc.subchannel_address" + namespace grpc_core { class SubchannelCall; @@ -185,10 +190,10 @@ class Subchannel : public DualRefCounted { ABSL_GUARDED_BY(&mu_); }; - // Creates a subchannel. + // Creates a subchannel given \a connector and \a args. static RefCountedPtr Create( OrphanablePtr connector, - const grpc_resolved_address& address, const grpc_channel_args* args); + const grpc_channel_args* args); // The ctor and dtor are not intended to use directly. Subchannel(SubchannelKey key, OrphanablePtr connector, @@ -200,6 +205,10 @@ class Subchannel : public DualRefCounted { // will have an affect when the subchannel creates a new ConnectedSubchannel. void ThrottleKeepaliveTime(int new_keepalive_time) ABSL_LOCKS_EXCLUDED(mu_); + // Gets the string representing the subchannel address. + // Caller doesn't take ownership. + const char* GetTargetAddress(); + const grpc_channel_args* channel_args() const { return args_; } channelz::SubchannelNode* channelz_node(); @@ -238,11 +247,27 @@ class Subchannel : public DualRefCounted { void AttemptToConnect() ABSL_LOCKS_EXCLUDED(mu_); // Resets the connection backoff of the subchannel. + // TODO(roth): Move connection backoff out of subchannels and up into LB + // policy code (probably by adding a SubchannelGroup between + // SubchannelList and SubchannelData), at which point this method can + // go away. void ResetBackoff() ABSL_LOCKS_EXCLUDED(mu_); // Tears down any existing connection, and arranges for destruction void Orphan() override ABSL_LOCKS_EXCLUDED(mu_); + // Returns a new channel arg encoding the subchannel address as a URI + // string. Caller is responsible for freeing the string. + static grpc_arg CreateSubchannelAddressArg(const grpc_resolved_address* addr); + + // Returns the URI string from the subchannel address arg in \a args. + static const char* GetUriFromSubchannelAddressArg( + const grpc_channel_args* args); + + // Sets \a addr from the subchannel address arg in \a args. + static void GetAddressFromSubchannelAddressArg(const grpc_channel_args* args, + grpc_resolved_address* addr); + private: // A linked list of ConnectivityStateWatcherInterfaces that are monitoring // the subchannel's state. @@ -325,11 +350,9 @@ class Subchannel : public DualRefCounted { // The subchannel pool this subchannel is in. RefCountedPtr subchannel_pool_; + // TODO(juanlishen): Consider using args_ as key_ directly. // Subchannel key that identifies this subchannel in the subchannel pool. const SubchannelKey key_; - // Actual address to connect to. May be different than the address in - // key_ if overridden by proxy mapper. - grpc_resolved_address address_for_connect_; // Channel args. grpc_channel_args* args_; // pollset_set tracking who's interested in a connection being setup. @@ -376,4 +399,4 @@ class Subchannel : public DualRefCounted { } // namespace grpc_core -#endif // GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_SUBCHANNEL_H +#endif /* GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_SUBCHANNEL_H */ diff --git a/src/core/ext/filters/client_channel/subchannel_pool_interface.cc b/src/core/ext/filters/client_channel/subchannel_pool_interface.cc index 61b977dc236..283b70d00a9 100644 --- a/src/core/ext/filters/client_channel/subchannel_pool_interface.cc +++ b/src/core/ext/filters/client_channel/subchannel_pool_interface.cc @@ -31,9 +31,8 @@ namespace grpc_core { TraceFlag grpc_subchannel_pool_trace(false, "subchannel_pool"); -SubchannelKey::SubchannelKey(const grpc_resolved_address& address, - const grpc_channel_args* args) { - Init(address, args, grpc_channel_args_normalize); +SubchannelKey::SubchannelKey(const grpc_channel_args* args) { + Init(args, grpc_channel_args_normalize); } SubchannelKey::~SubchannelKey() { @@ -41,7 +40,7 @@ SubchannelKey::~SubchannelKey() { } SubchannelKey::SubchannelKey(const SubchannelKey& other) { - Init(other.address_, other.args_, grpc_channel_args_copy); + Init(other.args_, grpc_channel_args_copy); } SubchannelKey& SubchannelKey::operator=(const SubchannelKey& other) { @@ -49,36 +48,28 @@ SubchannelKey& SubchannelKey::operator=(const SubchannelKey& other) { return *this; } grpc_channel_args_destroy(const_cast(args_)); - Init(other.address_, other.args_, grpc_channel_args_copy); + Init(other.args_, grpc_channel_args_copy); return *this; } SubchannelKey::SubchannelKey(SubchannelKey&& other) noexcept { - address_ = other.address_; args_ = other.args_; other.args_ = nullptr; } SubchannelKey& SubchannelKey::operator=(SubchannelKey&& other) noexcept { - address_ = other.address_; args_ = other.args_; other.args_ = nullptr; return *this; } bool SubchannelKey::operator<(const SubchannelKey& other) const { - if (address_.len < other.address_.len) return true; - if (address_.len > other.address_.len) return false; - int r = memcmp(address_.addr, other.address_.addr, address_.len); - if (r < 0) return true; - if (r > 0) return false; return grpc_channel_args_compare(args_, other.args_) < 0; } void SubchannelKey::Init( - const grpc_resolved_address& address, const grpc_channel_args* args, + const grpc_channel_args* args, grpc_channel_args* (*copy_channel_args)(const grpc_channel_args* args)) { - address_ = address; args_ = copy_channel_args(args); } diff --git a/src/core/ext/filters/client_channel/subchannel_pool_interface.h b/src/core/ext/filters/client_channel/subchannel_pool_interface.h index 7042f15574d..8a8a0cb6934 100644 --- a/src/core/ext/filters/client_channel/subchannel_pool_interface.h +++ b/src/core/ext/filters/client_channel/subchannel_pool_interface.h @@ -25,7 +25,6 @@ #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/debug/trace.h" #include "src/core/lib/gprpp/ref_counted.h" -#include "src/core/lib/iomgr/resolve_address.h" namespace grpc_core { @@ -36,8 +35,7 @@ extern TraceFlag grpc_subchannel_pool_trace; // A key that can uniquely identify a subchannel. class SubchannelKey { public: - SubchannelKey(const grpc_resolved_address& address, - const grpc_channel_args* args); + explicit SubchannelKey(const grpc_channel_args* args); ~SubchannelKey(); // Copyable. @@ -49,17 +47,13 @@ class SubchannelKey { bool operator<(const SubchannelKey& other) const; - const grpc_resolved_address& address() const { return address_; } - const grpc_channel_args* args() const { return args_; } - private: // Initializes the subchannel key with the given \a args and the function to // copy channel args. void Init( - const grpc_resolved_address& address, const grpc_channel_args* args, + const grpc_channel_args* args, grpc_channel_args* (*copy_channel_args)(const grpc_channel_args* args)); - grpc_resolved_address address_; const grpc_channel_args* args_; }; diff --git a/src/core/ext/transport/chttp2/client/chttp2_connector.cc b/src/core/ext/transport/chttp2/client/chttp2_connector.cc index 03e95a13c5c..96dd4eb7bf2 100644 --- a/src/core/ext/transport/chttp2/client/chttp2_connector.cc +++ b/src/core/ext/transport/chttp2/client/chttp2_connector.cc @@ -28,6 +28,7 @@ #include #include "src/core/ext/filters/client_channel/connector.h" +#include "src/core/ext/filters/client_channel/subchannel.h" #include "src/core/ext/transport/chttp2/transport/chttp2_transport.h" #include "src/core/lib/address_utils/sockaddr_utils.h" #include "src/core/lib/channel/channel_args.h" @@ -53,6 +54,8 @@ Chttp2Connector::~Chttp2Connector() { void Chttp2Connector::Connect(const Args& args, Result* result, grpc_closure* notify) { + grpc_resolved_address addr; + Subchannel::GetAddressFromSubchannelAddressArg(args.channel_args, &addr); grpc_endpoint** ep; { MutexLock lock(&mu_); @@ -80,9 +83,9 @@ void Chttp2Connector::Connect(const Args& args, Result* result, grpc_tcp_client_connect( &connected_, ep, grpc_slice_allocator_create(resource_quota_, - grpc_sockaddr_to_string(args.address, false), + grpc_sockaddr_to_string(&addr, false), args.channel_args), - args.interested_parties, args.channel_args, args.address, args.deadline); + args.interested_parties, args.channel_args, &addr, args.deadline); } void Chttp2Connector::Shutdown(grpc_error_handle error) { diff --git a/src/core/ext/transport/chttp2/client/insecure/channel_create.cc b/src/core/ext/transport/chttp2/client/insecure/channel_create.cc index 9680a74e7d3..40b3f6de220 100644 --- a/src/core/ext/transport/chttp2/client/insecure/channel_create.cc +++ b/src/core/ext/transport/chttp2/client/insecure/channel_create.cc @@ -1,18 +1,20 @@ -// -// Copyright 2015 gRPC authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// +/* + * + * Copyright 2015 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ #include @@ -35,12 +37,11 @@ namespace grpc_core { class Chttp2InsecureClientChannelFactory : public ClientChannelFactory { public: RefCountedPtr CreateSubchannel( - const grpc_resolved_address& address, const grpc_channel_args* args) override { grpc_channel_args* new_args = grpc_default_authority_add_if_not_present(args); - RefCountedPtr s = Subchannel::Create( - MakeOrphanable(), address, new_args); + RefCountedPtr s = + Subchannel::Create(MakeOrphanable(), new_args); grpc_channel_args_destroy(new_args); return s; } diff --git a/src/core/ext/transport/chttp2/client/secure/secure_channel_create.cc b/src/core/ext/transport/chttp2/client/secure/secure_channel_create.cc index c76b6e96d37..e6fccf66964 100644 --- a/src/core/ext/transport/chttp2/client/secure/secure_channel_create.cc +++ b/src/core/ext/transport/chttp2/client/secure/secure_channel_create.cc @@ -1,18 +1,20 @@ -// -// Copyright 2015 gRPC authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// +/* + * + * Copyright 2015 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ #include @@ -41,7 +43,6 @@ namespace grpc_core { class Chttp2SecureClientChannelFactory : public ClientChannelFactory { public: RefCountedPtr CreateSubchannel( - const grpc_resolved_address& address, const grpc_channel_args* args) override { grpc_channel_args* new_args = GetSecureNamingChannelArgs(args); if (new_args == nullptr) { @@ -49,8 +50,8 @@ class Chttp2SecureClientChannelFactory : public ClientChannelFactory { "Failed to create channel args during subchannel creation."); return nullptr; } - RefCountedPtr s = Subchannel::Create( - MakeOrphanable(), address, new_args); + RefCountedPtr s = + Subchannel::Create(MakeOrphanable(), new_args); grpc_channel_args_destroy(new_args); return s; } diff --git a/src/core/lib/address_utils/parse_address.cc b/src/core/lib/address_utils/parse_address.cc index c254d2eb194..a621514414e 100644 --- a/src/core/lib/address_utils/parse_address.cc +++ b/src/core/lib/address_utils/parse_address.cc @@ -83,7 +83,6 @@ namespace grpc_core { grpc_error_handle UnixSockaddrPopulate(absl::string_view path, grpc_resolved_address* resolved_addr) { - memset(resolved_addr, 0, sizeof(*resolved_addr)); struct sockaddr_un* un = reinterpret_cast(resolved_addr->addr); const size_t maxlen = sizeof(un->sun_path) - 1; @@ -100,7 +99,6 @@ grpc_error_handle UnixSockaddrPopulate(absl::string_view path, grpc_error_handle UnixAbstractSockaddrPopulate( absl::string_view path, grpc_resolved_address* resolved_addr) { - memset(resolved_addr, 0, sizeof(*resolved_addr)); struct sockaddr_un* un = reinterpret_cast(resolved_addr->addr); const size_t maxlen = sizeof(un->sun_path) - 1; diff --git a/test/core/end2end/goaway_server_test.cc b/test/core/end2end/goaway_server_test.cc index 4d4509bc53a..e0aaef622f1 100644 --- a/test/core/end2end/goaway_server_test.cc +++ b/test/core/end2end/goaway_server_test.cc @@ -123,7 +123,6 @@ static grpc_ares_request* my_dns_lookup_ares_locked( } else { *addresses = absl::make_unique(); grpc_sockaddr_in sa; - memset(&sa, 0, sizeof(sa)); sa.sin_family = GRPC_AF_INET; sa.sin_addr.s_addr = 0x100007f; sa.sin_port = grpc_htons(static_cast(g_resolve_port)); diff --git a/test/cpp/microbenchmarks/bm_call_create.cc b/test/cpp/microbenchmarks/bm_call_create.cc index ae27fd34d9b..73d31678eae 100644 --- a/test/cpp/microbenchmarks/bm_call_create.cc +++ b/test/cpp/microbenchmarks/bm_call_create.cc @@ -324,7 +324,6 @@ static void DoNothing(void* /*arg*/, grpc_error_handle /*error*/) {} class FakeClientChannelFactory : public grpc_core::ClientChannelFactory { public: grpc_core::RefCountedPtr CreateSubchannel( - const grpc_resolved_address& /*address*/, const grpc_channel_args* /*args*/) override { return nullptr; }